diff --git a/apps/darkroom-android/Cargo.toml b/apps/darkroom-android/Cargo.toml index 5699149..c51e112 100644 --- a/apps/darkroom-android/Cargo.toml +++ b/apps/darkroom-android/Cargo.toml @@ -19,6 +19,10 @@ dr-ui.workspace = true # For `account::set_data_dir`: only the platform entry point knows where Android # lets this app keep files, and it must be set before any store is opened. dr-sync.workspace = true +# For the panic hook, and for `crash::set_state_dir` — Android has no XDG +# directories, so the entry point is the only place that knows where a crash +# record may be written. +dr-plat.workspace = true # Directly, not just through dr-ui: `android_main` takes an `AndroidApp` and # calls `slint::android::init`, both of which come from this crate. The backend # feature comes from dr-ui's target-specific dependency. diff --git a/apps/darkroom-android/src/lib.rs b/apps/darkroom-android/src/lib.rs index f8c417f..5757a1f 100644 --- a/apps/darkroom-android/src/lib.rs +++ b/apps/darkroom-android/src/lib.rs @@ -39,9 +39,18 @@ fn android_main(app: slint::android::AndroidApp) { // worker thread that panics is invisible: the process survives, the // channel it was writing to closes, and the UI reports only that // something "failed unexpectedly" with no way to find out what. - std::panic::set_hook(Box::new(|info| { - log::error!("panic: {info}"); - })); + // + // This used to be one `log::error!` of the raw panic, which had two + // problems: logcat is a ring buffer that is gone by the time a user + // reports anything, and the raw message can carry a document URI naming + // their library or a credential a library interpolated into an error + // (NFR-SEC-2). `dr_plat::crash` writes a redacted record to disk and logs + // the redacted form. Nothing uploads it. + // + // Before `set_state_dir` on purpose: the hook resolves the directory when + // it fires, so installing it first covers the startup below rather than + // leaving it uncovered. + dr_plat::crash::install(env!("CARGO_PKG_VERSION")); log::info!("DarkRoom v{}", env!("CARGO_PKG_VERSION")); @@ -54,6 +63,10 @@ fn android_main(app: slint::android::AndroidApp) { match app.internal_data_path() { Some(dir) => { log::info!("data dir: {}", dir.display()); + // Crash records go beside the account data rather than under it: + // both are app-private and neither is a cache, which is the whole + // distinction that matters here (see `dr_plat::crash::state_dir`). + dr_plat::crash::set_state_dir(dir.join("state")); dr_sync::account::set_data_dir(dir); } None => log::error!("no internal data path; settings will not persist"), diff --git a/apps/darkroom-desktop/Cargo.toml b/apps/darkroom-desktop/Cargo.toml index 238b7f6..630f2f2 100644 --- a/apps/darkroom-desktop/Cargo.toml +++ b/apps/darkroom-desktop/Cargo.toml @@ -7,6 +7,10 @@ license.workspace = true [dependencies] dr-ui.workspace = true +# For the panic hook alone. Directly rather than through dr-ui, because it has +# to be installed before `dr_ui::run` — a panic during startup is exactly the +# one this exists to catch. +dr-plat.workspace = true anyhow.workspace = true env_logger.workspace = true log.workspace = true diff --git a/apps/darkroom-desktop/src/main.rs b/apps/darkroom-desktop/src/main.rs index 05713c8..149a5ff 100644 --- a/apps/darkroom-desktop/src/main.rs +++ b/apps/darkroom-desktop/src/main.rs @@ -10,6 +10,14 @@ fn main() -> anyhow::Result<()> { )) .init(); + // Immediately after the logger and before anything that could fail. Until + // now a panic on desktop went to stderr and died with the terminal, which + // means every panic a user has ever hit was unreportable: the process + // survives (the panicking worker does not), a control goes dead, and there + // is nothing on disk to say why. The record is local and stays local — + // there is no upload path, by design; see `dr_plat::crash`. + dr_plat::crash::install(env!("CARGO_PKG_VERSION")); + log::info!("DarkRoom v{}", env!("CARGO_PKG_VERSION")); let paths: Vec = std::env::args().skip(1).map(PathBuf::from).collect(); diff --git a/platform/dr-plat/src/crash.rs b/platform/dr-plat/src/crash.rs new file mode 100644 index 0000000..654ab3c --- /dev/null +++ b/platform/dr-plat/src/crash.rs @@ -0,0 +1,620 @@ +//! TRACES: NFR-OPS-2 | NFR-SEC-2 +//! Local crash capture. +//! +//! # What this is, and the half it deliberately is not +//! +//! NFR-OPS-2 is two sentences: *local crash capture always; upload only on +//! explicit opt-in.* Only the first is built here, and the second is not +//! half-built either — there is no upload path, no endpoint, no queue and no +//! "send this later" flag. Opt-in upload needs a server to receive it and a +//! consent flow that states what leaves the device (NFR-SEC-4, and the same +//! preview-and-consent step NFR-OPS-1 requires of the diagnostics bundle); +//! neither exists, and a transport built ahead of the consent is exactly the +//! shape of thing that later gets switched on by default. +//! +//! So a crash record is a file on the user's own disk. Nothing reads it but a +//! person. +//! +//! # Why a panic is worth writing down at all +//! +//! NFR-ARCH-4 says no worker error may panic the process, and the application +//! is built that way — errors are typed and attached to the image or job they +//! belong to. A panic is therefore, by construction, a *bug*: an invariant +//! this codebase believed and got wrong. Before this, one of those was +//! invisible on desktop (stderr, discarded with the terminal) and one line of +//! `log::error!` on Android. What the user saw was a job that stopped, or a +//! channel that closed and a control that went dead, with nothing to report. +//! +//! # What goes in a record, and what may never +//! +//! The rule the content is chosen under is NFR-SEC-2 — credentials never reach +//! logs or plain files — extended to the thing this application is actually +//! about: **a user's library is private, and its shape is private too.** A +//! path is not a neutral technical detail here. `/home/anna/Photos/2019 +//! Divorce/` names something about a person, and a crash record is a file that +//! gets attached to a bug report by someone trying to be helpful. +//! +//! Hence [`redact`], which is applied to the panic message *and* to the +//! backtrace before either is written, and which is deliberately blunt: it +//! removes anything that looks like a path or a URL, keeping only the basename +//! of `.rs` files so a backtrace is still readable. Over-redaction costs +//! legibility; under-redaction costs a user something they cannot take back. +//! +//! NFR-SEC-5 — face data never enters a diagnostics bundle or crash report +//! "under any configuration" — is met structurally rather than by filtering: +//! this module reads no catalog, opens no image, and touches no account. A +//! record is assembled from the panic hook's own arguments and from +//! [`std::env::consts`], and there is no code path from here to an embedding, +//! a crop, or a cluster. The message length cap is the backstop for the +//! remaining case — a panic payload that some *other* module formatted a large +//! value into. +//! +//! # What this leaves cheaper for NFR-OPS-1 +//! +//! The rotating on-disk log is unbuilt, and it wants three things that are +//! here: [`state_dir`] (the XDG state directory, resolved once, overridable +//! for Android where XDG does not exist), [`redact`] (NFR-OPS-1's "automatic +//! redaction of credentials and tokens" is the same function), and `prune` +//! (size-capped rotation is this counting files instead of bytes). A log +//! belongs at `state_dir().join("log")` beside `crash/`, and the diagnostics +//! bundle then has one directory to collect. + +use std::path::{Path, PathBuf}; +use std::sync::OnceLock; +use std::time::{SystemTime, UNIX_EPOCH}; + +/// How many crash records are kept, newest first. +/// +/// Ten because the useful pattern in a crash record is usually a *repeat* — +/// the same panic three launches running is a far stronger report than one — +/// and because these are a few kilobytes each, so the cap exists to stop a +/// crash loop filling a disk rather than to save space. +pub const KEEP_RECORDS: usize = 10; + +/// The longest panic message written to a record. +/// +/// A backstop, not a redaction: [`redact`] handles what must not be written at +/// all. This bounds what a panic that formatted something enormous into its +/// message — a decoded buffer, a `Vec` of embeddings — can put on disk. +const MAX_MESSAGE: usize = 2000; + +/// Android's per-app directory, once the entry point has said what it is. +static STATE_DIR: OnceLock = OnceLock::new(); + +/// Declare the directory this application may keep state in. +/// +/// Only Android needs to call this, and it must call it before a crash rather +/// than before the hook is installed — [`install`] resolves the directory at +/// crash time precisely so that the hook can go in first, covering the startup +/// it would otherwise miss. Neither `XDG_STATE_HOME` nor `HOME` is set there, +/// and the fallback would resolve to a path the app cannot write. +/// +/// Later calls are ignored rather than racing, matching +/// `dr_sync::account::set_data_dir`, which the same entry point calls for the +/// same reason. +pub fn set_state_dir(dir: PathBuf) { + let _ = STATE_DIR.set(dir); +} + +/// Where this application keeps state that is neither configuration nor cache. +/// +/// `$XDG_STATE_HOME/darkroom`, falling back to `~/.local/state/darkroom`. +/// State rather than cache because a crash record must survive the sweep that +/// a cache directory exists to permit, and rather than config because it is +/// not something the user edits. +pub fn state_dir() -> PathBuf { + if let Some(d) = STATE_DIR.get() { + return d.clone(); + } + std::env::var_os("XDG_STATE_HOME") + .map(PathBuf::from) + .unwrap_or_else(|| { + PathBuf::from(std::env::var("HOME").unwrap_or_default()).join(".local/state") + }) + .join("darkroom") +} + +/// Where crash records are written. +pub fn crash_dir() -> PathBuf { + state_dir().join("crash") +} + +/// Install the panic hook. +/// +/// Call once, as early as the entry point can — before the window, before any +/// store, before anything that could itself panic. The directory is resolved +/// lazily inside the hook, so installing this before +/// [`set_state_dir`] is correct rather than merely tolerated. +/// +/// The previously installed hook still runs afterwards. On desktop that is the +/// standard library's, which prints the panic to stderr, and a developer +/// watching a terminal should not lose that because the application started +/// writing files. stderr is the one surface that sees the message unredacted, +/// which is a considered exception: it is ephemeral, local, and never attached +/// to a bug report. +pub fn install(app_version: &str) { + let version = app_version.to_string(); + let previous = std::panic::take_hook(); + + std::panic::set_hook(Box::new(move |info| { + // A panic inside a panic hook aborts the process, which would replace + // a diagnosable crash with an undiagnosable one. Everything below is + // written to be infallible, and this is the admission that "written to + // be" is not the same as "is". + let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let record = compose(&version, info); + // Redacted, because this is a log file and NFR-SEC-2 governs it. + // The unredacted form goes to stderr below, through the hook this + // one chained onto. + log::error!("panic: {}", record.summary); + match write_record(&crash_dir(), &record) { + // The name, not the path: the log this line lands in is a + // sibling of the record, and printing the directory would put + // the user's home in a file they may hand to someone. + Ok(path) => log::error!( + "crash record written: {}", + path.file_name().unwrap_or_default().to_string_lossy() + ), + Err(e) => log::error!("could not write a crash record: {e}"), + } + })); + + previous(info); + })); +} + +/// The crash records on disk, newest first. +/// +/// For a future diagnostics bundle, and for a person looking for the file to +/// attach to a report. +pub fn records() -> Vec { + let Ok(entries) = std::fs::read_dir(crash_dir()) else { + return Vec::new(); + }; + let mut out: Vec<(i64, PathBuf)> = entries + .flatten() + .filter_map(|e| { + let path = e.path(); + Some((timestamp_of(&path)?, path)) + }) + .collect(); + out.sort_by_key(|(when, _)| std::cmp::Reverse(*when)); + out.into_iter().map(|(_, p)| p).collect() +} + +/// One crash, formatted. +struct Record { + /// The whole file. + body: String, + /// One redacted line, for the log. + summary: String, + when: i64, +} + +/// Build a record from what the panic hook was handed. +/// +/// Split from the writing so the *content* rules — what is included, what is +/// redacted, what is capped — are testable without a filesystem, and so a +/// future diagnostics bundle can reuse the same composition. +fn compose(version: &str, info: &std::panic::PanicHookInfo<'_>) -> Record { + let payload = info.payload(); + let message = payload + .downcast_ref::<&str>() + .copied() + .or_else(|| payload.downcast_ref::().map(|s| s.as_str())) + // A panic can carry any `Any`, and `panic_any` is used by some + // libraries. There is nothing to print, and saying so is better than + // an empty field that reads like a bug in this code. + .unwrap_or("(panic payload was not a string)"); + + let message = redact(&truncate(message, MAX_MESSAGE)); + let at = info + .location() + .map(|l| format!("{}:{}:{}", l.file(), l.line(), l.column())) + // `location()` is a compile-time source path, not one of the user's, + // but it goes through the same redaction: a dependency built from a + // registry checkout carries the *builder's* home directory in it. + .map(|s| redact(&s)) + .unwrap_or_else(|| "unknown".to_string()); + let thread = std::thread::current() + .name() + .unwrap_or("unnamed") + .to_string(); + + let when = now(); + let summary = format!("{message} (at {at}, thread {thread})"); + + let body = format!( + "darkroom-crash 1\n\ + version: {version}\n\ + when: {when}\n\ + os: {}\n\ + arch: {}\n\ + thread: {thread}\n\ + at: {at}\n\ + message: {message}\n\ + backtrace:\n{}\n", + std::env::consts::OS, + std::env::consts::ARCH, + redact(&std::backtrace::Backtrace::force_capture().to_string()), + ); + + Record { + body, + summary, + when, + } +} + +/// Write one record into `dir`, then prune. +/// +/// Takes the directory rather than calling [`crash_dir`] so a test can drive +/// the real writing path without an environment variable and without a +/// `OnceLock` it cannot reset. +fn write_record(dir: &Path, record: &Record) -> std::io::Result { + std::fs::create_dir_all(dir)?; + // The pid distinguishes two processes crashing in the same second, which + // is not hypothetical: a background export and the app can be separate + // processes on desktop, and a crash loop retries fast. + let path = dir.join(format!("crash-{}-{}.txt", record.when, std::process::id())); + std::fs::write(&path, &record.body)?; + prune(dir, KEEP_RECORDS); + Ok(path) +} + +/// Delete all but the `keep` newest records in `dir`. +/// +/// Best-effort: failing to delete an old record is no reason to lose the new +/// one, which is already on disk. +fn prune(dir: &Path, keep: usize) { + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + let mut found: Vec<(i64, PathBuf)> = entries + .flatten() + .filter_map(|e| { + let path = e.path(); + Some((timestamp_of(&path)?, path)) + }) + .collect(); + found.sort_by_key(|(when, _)| std::cmp::Reverse(*when)); + for (_, path) in found.into_iter().skip(keep) { + let _ = std::fs::remove_file(path); + } +} + +/// Pull the timestamp back out of a record's filename. +/// +/// Doubles as the filter that keeps anything else in the directory — an +/// editor's backup file, a log — from being counted as a crash record or +/// pruned as one. +fn timestamp_of(path: &Path) -> Option { + let name = path.file_name()?.to_str()?; + let rest = name.strip_prefix("crash-")?.strip_suffix(".txt")?; + rest.split('-').next()?.parse().ok() +} + +/// Remove from `text` everything that would say something about the user. +/// +/// # The rule, and why it is this blunt one +/// +/// A token containing `/` is treated as a path or a URL and removed. Rust +/// source files are the exception and keep their basename, because a backtrace +/// with no filenames is close to useless and `library.rs:1270` says nothing +/// about anybody. A token that reads like a secret — a long opaque run, or the +/// value beside a word like `password` — is removed regardless of shape. +/// +/// Blunter than a list of known-sensitive shapes on purpose. The cost of +/// over-redaction is a diagnostic that is harder to read; the cost of +/// under-redaction is a directory listing of somebody's photographs in a file +/// they may attach to a public bug report. Those are not comparable, and this +/// is not the place to be clever about the boundary. +/// +/// Known limits, stated rather than hidden: a path with no `/` in it (a bare +/// filename) survives, and a person's name that some *other* module formatted +/// into a panic message survives. Both are bounded by the fact that nothing +/// here reads the catalog — see the module documentation — and the second is +/// why `MAX_MESSAGE` exists. +pub fn redact(text: &str) -> String { + let mut out: Vec = Vec::new(); + let mut redact_next = false; + + for token in text.split_inclusive(char::is_whitespace) { + // Whitespace is preserved exactly — a backtrace is read as a shape as + // much as as text — so the classification runs on the token without + // its trailing space and the space is put back. + let trailing: String = token + .chars() + .skip_while(|c| !c.is_whitespace()) + .collect::(); + let word = &token[..token.len() - trailing.len()]; + + if word.is_empty() { + out.push(token.to_string()); + continue; + } + + let replacement = if redact_next { + Some("".to_string()) + } else { + classify(word) + }; + redact_next = names_a_secret(word); + + out.push(match replacement { + Some(r) => format!("{r}{trailing}"), + None => token.to_string(), + }); + } + + out.join("") +} + +/// What one token should be replaced with, or `None` to keep it. +fn classify(word: &str) -> Option { + // `key=value` and `key: value` written as one token. Split at the first + // separator so `Authorization:Bearer` loses the half that matters. + if let Some((head, tail)) = word.split_once(['=', ':']) { + if names_a_secret(head) && !tail.is_empty() { + return Some(format!("{head}=")); + } + } + + // Strip the punctuation a sentence puts around a path — "opening + // /home/x/y.CR3:" — so the classification sees the path itself, then put + // nothing back: the punctuation is not worth the complexity of restoring + // it around a placeholder. + let bare = word.trim_matches(|c: char| matches!(c, '"' | '\'' | '(' | ')' | ',' | ';' | ':')); + + if bare.contains('/') { + // A Rust source path keeps its basename. Line and column are part of + // the same token in a backtrace ("src/library.rs:1270:9"), and they + // are kept: they are facts about this codebase. + if let Some(rs) = rust_source_tail(bare) { + return Some(format!("…/{rs}")); + } + return Some("".to_string()); + } + if bare.starts_with('~') { + return Some("".to_string()); + } + if looks_opaque(bare) { + return Some("".to_string()); + } + None +} + +/// The `library.rs:1270:9` tail of a path naming a Rust source file. +fn rust_source_tail(word: &str) -> Option<&str> { + let tail = word.rsplit('/').next()?; + // The extension is followed by `:line:col` in a backtrace and by nothing + // in a `Location`, so match on the extension rather than on the end. + if tail.contains(".rs") { + Some(tail) + } else { + None + } +} + +/// Whether this word introduces a value that must not be written down. +fn names_a_secret(word: &str) -> bool { + let w = word + .trim_matches(|c: char| !c.is_alphanumeric() && c != '_' && c != '-') + .to_ascii_lowercase(); + matches!( + w.as_str(), + "password" + | "passwd" + | "app-password" + | "app_password" + | "token" + | "secret" + | "bearer" + | "authorization" + | "apikey" + | "api-key" + | "api_key" + | "credential" + | "credentials" + | "cookie" + ) +} + +/// Whether a word looks like a key rather than like prose. +/// +/// A long run of the characters secrets are encoded in, containing both a +/// letter and a digit — which is what an app password, a bearer token or a +/// base64 blob looks like, and what an English word or a Rust identifier does +/// not. +fn looks_opaque(word: &str) -> bool { + word.len() >= 20 + && word + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '+' | '_' | '-' | '=')) + && word.chars().any(|c| c.is_ascii_digit()) + && word.chars().any(|c| c.is_ascii_alphabetic()) +} + +/// Cut `s` to `max` bytes on a character boundary, saying that it was cut. +fn truncate(s: &str, max: usize) -> String { + if s.len() <= max { + return s.to_string(); + } + let mut end = max; + while end > 0 && !s.is_char_boundary(end) { + end -= 1; + } + format!("{}… ({} bytes truncated)", &s[..end], s.len() - end) +} + +/// Seconds since the epoch, or 0 if the clock is before it. +fn now() -> i64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_users_photograph_never_reaches_a_record() { + let r = redact("failed to open /home/anna/Photos/2019 Divorce/IMG_0042.CR3"); + assert!(!r.contains("anna"), "{r}"); + assert!(!r.contains("IMG_0042"), "{r}"); + assert!(!r.contains("Divorce"), "{r}"); + assert!(r.contains("failed to open"), "the diagnosis was lost: {r}"); + } + + #[test] + fn an_android_document_uri_is_a_path_too() { + // SAF hands out `content://` URIs and `primary:DCIM/...` refs rather + // than paths, and both name the user's library just as precisely. + let r = redact("no such image: content://com.android.providers/tree/primary%3ADCIM"); + assert!(!r.contains("DCIM"), "{r}"); + let r = redact("source_ref primary:DCIM/Camera/IMG_1.CR3 missing"); + assert!(!r.contains("IMG_1"), "{r}"); + } + + #[test] + fn a_server_url_is_removed_because_it_names_the_user() { + // A Nextcloud URL is the user's own server, often with their login in + // the path. NFR-SEC-3 is about the wire; this is about the disk. + let r = redact("PROPFIND https://cloud.example.org/remote.php/dav/files/anna/ failed"); + assert!(!r.contains("cloud.example.org"), "{r}"); + assert!(!r.contains("anna"), "{r}"); + assert!(r.contains("PROPFIND"), "{r}"); + } + + #[test] + fn a_credential_never_reaches_a_record() { + // NFR-SEC-2, which forbids credentials in logs and plain files. Both + // spellings: the value beside a naming word, and the value alone. + let r = redact("auth failed: password hunter2correcthorse"); + assert!(!r.contains("hunter2correcthorse"), "{r}"); + + let r = redact("Authorization: Bearer aGVsbG90aGVyZTEyMzQ1Njc4OTA="); + assert!(!r.contains("aGVsbG90aGVyZTEyMzQ1Njc4OTA"), "{r}"); + + let r = redact("rejected app-password=abcde-fghij-12345-klmno-pqrst"); + assert!(!r.contains("abcde-fghij"), "{r}"); + } + + #[test] + fn a_bare_secret_is_caught_by_its_shape() { + // Nextcloud app passwords arrive with no label at all when they are + // interpolated into a message by a library this codebase does not own. + let r = redact("login failed for aBcDe1FgHiJ2kLmNo3PqRsT4uV"); + assert!(!r.contains("aBcDe1FgHiJ"), "{r}"); + } + + #[test] + fn a_backtrace_keeps_the_frames_that_make_it_readable() { + // The whole reason the `.rs` exception exists: redacting these to + // `` leaves a backtrace of nothing but symbol names, and the + // line number is a fact about this codebase rather than about anyone. + let r = redact(" 3: dr_ui::library::run_scan\n at ./ui/dr-ui/src/library.rs:1270:9\n"); + assert!(r.contains("library.rs:1270:9"), "{r}"); + assert!(r.contains("dr_ui::library::run_scan"), "{r}"); + assert!(!r.contains("ui/dr-ui/src"), "{r}"); + // And the shape survives, because a backtrace is read as a shape. + assert!(r.contains('\n'), "{r}"); + assert!(r.starts_with(" 3:"), "{r}"); + } + + #[test] + fn ordinary_words_are_left_alone() { + // Over-redaction has a cost too: a record that says nothing is not + // safer, it is just useless. + let msg = "assertion failed: tier_desired was 3, expected 2"; + assert_eq!(redact(msg), msg); + } + + #[test] + fn an_enormous_payload_is_capped() { + // A panic that formatted a decoded buffer — or, the case NFR-SEC-5 + // cares about, an embedding — into its message. + let huge = "9".repeat(MAX_MESSAGE * 3); + let cut = truncate(&huge, MAX_MESSAGE); + assert!(cut.len() < huge.len()); + assert!(cut.contains("truncated"), "{cut}"); + } + + #[test] + fn truncation_does_not_split_a_character() { + let s = "é".repeat(100); + // 3 is mid-character for a 2-byte encoding. + let cut = truncate(&s, 3); + assert!(cut.starts_with('é')); + } + + #[test] + fn records_are_written_and_rotated() { + let dir = std::env::temp_dir().join(format!("dr-crash-test-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + + for i in 0..(KEEP_RECORDS as i64 + 5) { + write_record( + &dir, + &Record { + body: format!("darkroom-crash 1\nmessage: {i}\n"), + summary: String::new(), + // Distinct seconds, or the pid-suffixed names would + // collide and the rotation would have nothing to count. + when: 1_000 + i, + }, + ) + .unwrap(); + } + + let kept: Vec = std::fs::read_dir(&dir) + .unwrap() + .flatten() + .map(|e| e.path()) + .collect(); + assert_eq!(kept.len(), KEEP_RECORDS); + // The newest survive, not the oldest. + let newest = kept.iter().filter_map(|p| timestamp_of(p)).max(); + let oldest = kept.iter().filter_map(|p| timestamp_of(p)).min(); + assert_eq!(newest, Some(1_000 + KEEP_RECORDS as i64 + 4)); + assert_eq!(oldest, Some(1_000 + 5)); + + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn a_stray_file_is_neither_listed_nor_pruned() { + let dir = std::env::temp_dir().join(format!("dr-crash-stray-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("darkroom.log"), b"not a crash").unwrap(); + + for i in 0..(KEEP_RECORDS as i64 + 5) { + write_record( + &dir, + &Record { + body: String::new(), + summary: String::new(), + when: 2_000 + i, + }, + ) + .unwrap(); + } + + assert!(dir.join("darkroom.log").is_file(), "the log was pruned"); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn the_state_directory_is_neither_config_nor_cache() { + // NFR-OPS-1 puts the log here too, and NFR-OPS-3 keeps preferences + // separate from both. The distinction is what stops a crash record + // being swept away by the thing that is allowed to sweep caches. + let dir = state_dir(); + let s = dir.to_string_lossy(); + assert!(s.ends_with("darkroom"), "{s}"); + assert!(!s.contains("/cache"), "{s}"); + } +} diff --git a/platform/dr-plat/src/lib.rs b/platform/dr-plat/src/lib.rs index 808ed80..77f0b68 100644 --- a/platform/dr-plat/src/lib.rs +++ b/platform/dr-plat/src/lib.rs @@ -4,6 +4,13 @@ //! construction, so `core/` contains no `#[cfg(target_os)]` (NFR-PORT-1, //! ARCH §10). +// Not a trait, and the one module here that is not. It belongs in this crate +// for the same reason the traits do: "where does this platform let an +// application keep state" is a platform question, and both entry points need +// the answer before either has a window. Resolving it in `apps/` would mean +// writing it twice, once per platform, which is the arrangement this crate +// exists to prevent. +pub mod crash; pub mod display; pub mod secrets; pub mod storage;