diff --git a/README.md b/README.md index 63e1152..a1f5eb0 100644 --- a/README.md +++ b/README.md @@ -100,7 +100,7 @@ diffing the two copies and finding nothing but a name between them. - **`netif`** — `wg_address()`, which fails closed when the tunnel is down, and `local_addresses()` for the certificate's SANs. The product name is a parameter so the failure reads as advice rather than as a library complaining. Both split the *lookup* from the *decision*, so the failure path can be tested on a machine that has a tunnel — every machine this runs on does. - **`enroll`** — token generation, hex-SHA-256 storage, constant-time comparison, the `://enroll?…` URI, and the terminal QR. The URI scheme is the parameter, because it is what routes a scan back to the right app. - **`certs`** — the CA generated once and never replaced, the leaf reissued every start. `product` names the organisation and common name and is the whole of what is per-project. Shared because being written twice is worst here: a trust anchor built two ways can be built differently two ways, and the difference surfaces as an opaque handshake failure on a phone. -- **`format`** — the two RON house rules. This was byte-identical in both projects, which made it the clearest thing in the evidence table and the easiest deletion. +- **`format`** — the two RON house rules, plus `write`, which renders a value and replaces a file with it atomically and owner-only. This was byte-identical in both projects, which made it the clearest thing in the evidence table and the easiest deletion. - **`private`** — owner-only files and directories, taken from ai-app's version, with an `append_file` alongside `create_file` because a transcript must never be truncated by being opened. - **`xdg`** — where each project's state lives: `config_home(product)` and `data_home(product)`, resolving the XDG variable, ignoring it unless absolute, and falling back under `$HOME`. Shared because the *reason* is shared and is not obvious from the code — the repo is a mount that resolves at different absolute paths on each side, so config inside it records paths that work on only one, and a CA private key inside it would let the untrusted side mint a leaf the pinned app trusts. @@ -157,7 +157,7 @@ nowhere near the generator. Worth revisiting if it ever needs a second fix. **Should follow, in this order.** Each is already near-identical: 1. ~~The Kotlin `EnrollmentScanActivity`, `PinnedCert`, and the enrollment/Keystore half of `ServerConfig`.~~ Done — see `app/` above. It paid as predicted: ai-app's `ServerConfig` had gained `localNetworkAllowed` and the KTX `edit` block while dev-updater's had not, so the two had drifted in both directions exactly as the evidence suggested. -2. Atomic owner-only config save. Both do temp-file-then-rename with the mode set before the rename; only the schema differs. +2. ~~Atomic owner-only config save.~~ Done — `format::write`, since it is the RON house rules and the owner-only rules used together. The subtlety it now states once is that the temp file is *not always new*: a save killed partway leaves one behind, and reopening it keeps whatever mode it had, which is then renamed over the file holding the enrolled token hashes. There is a test for exactly that, because it cannot happen on a machine where nothing has ever crashed mid-save. **Should not.** Naming these is the point of the exercise: diff --git a/server/src/format.rs b/server/src/format.rs index 6d8d1ec..fd13aa9 100644 --- a/server/src/format.rs +++ b/server/src/format.rs @@ -35,8 +35,13 @@ /// `#![enable(implicit_some)]` header every project file would have to /// remember, and matched on the writing side by `skip_serializing_if` so /// nothing writes back a `Some(...)` a person didn't type. +use std::path::Path; + +use anyhow::{Context, Result}; use serde::{Serialize, de::DeserializeOwned}; +use crate::private; + fn options() -> ron::Options { ron::Options::default().with_default_extension(ron::extensions::Extensions::IMPLICIT_SOME) } @@ -51,6 +56,40 @@ pub fn render(value: &T) -> Result { Ok(unwrap_outer(&text)) } +/// Renders `value` and replaces `path` with it, atomically and owner-only. +/// +/// Whole-file-and-rename rather than an in-place edit, because both +/// projects' config files are small, are read at startup, and hold the +/// enrolled token hashes -- a half-written one would take the server down +/// on its next start with no way to fix it from a phone. The rename is +/// what makes a reader see either the old file or the new one and never +/// part of both. +/// +/// The temp file goes through [`private::write_file`] rather than +/// `std::fs::write`, and that is the subtle half: **the temp file is not +/// always new.** A save killed partway leaves one behind, and opening that +/// again keeps whatever mode it already had -- which is then renamed over +/// the file holding the token hashes. Setting the mode as it is opened +/// covers both the fresh and the leftover case. +pub fn write(path: &Path, value: &T) -> Result<()> { + if let Some(parent) = path.parent() { + private::create_dir(parent)?; + } + let text = render(value).context("serialize config")?; + // Appended rather than substituted, so `config.ron` yields + // `config.ron.tmp` and not `config.tmp` -- a name that cannot collide + // with a real file and that says what it is a temporary copy of. + let tmp = path.with_file_name(format!( + "{}.tmp", + path.file_name() + .unwrap_or_else(|| std::ffi::OsStr::new("config")) + .to_string_lossy() + )); + private::write_file(&tmp, text.as_bytes())?; + std::fs::rename(&tmp, path) + .with_context(|| format!("replace {} with {}", path.display(), tmp.display())) +} + /// Strips the outer `(`/`)` the writer always emits and removes the /// indent level they cost. Deliberately narrow: it accepts only the /// exact shape `PrettyConfig` produces, and leaves anything else alone @@ -146,6 +185,57 @@ mod tests { ); } + #[test] + fn what_is_written_reads_back_and_is_owner_only() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("nested").join("config.ron"); + let value = Demo { + name: "thing".to_string(), + note: None, + }; + + write(&path, &value).expect("write"); + + assert_eq!( + parse::(&std::fs::read_to_string(&path).expect("read")).expect("re-read"), + value, + ); + let mode = std::fs::metadata(&path).expect("stat").permissions().mode(); + assert_eq!(mode & 0o777, 0o600, "config holds token hashes: {mode:o}"); + assert!( + !path.with_extension("ron.tmp").exists(), + "the temp file is renamed away, not left behind", + ); + } + + /// The case the whole thing turns on, and the one that cannot happen on + /// a machine where nothing has ever crashed mid-save: a leftover temp + /// file from an interrupted write is reopened, and if its mode came + /// along it would be renamed straight over the file holding the enrolled + /// token hashes. + #[test] + fn a_leftover_temp_file_cannot_widen_the_config() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("config.ron"); + let tmp = dir.path().join("config.ron.tmp"); + + std::fs::write(&tmp, b"leftover from a save that died").expect("stale temp"); + std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(0o644)).expect("widen"); + + write(&path, &Demo::default()).expect("write"); + + let mode = std::fs::metadata(&path).expect("stat").permissions().mode(); + assert_eq!( + mode & 0o777, + 0o600, + "a world-readable leftover must not become the config: {mode:o}", + ); + } + /// A parse error's line number has to point at the real line, which is /// why the opening paren is not followed by a newline. #[test]