Compare commits
1
Commits
4de8bff5f2
...
db4552f4e4
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
db4552f4e4 |
No files matched your search
@@ -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 `<scheme>://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:
|
||||
|
||||
|
||||
@@ -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<T: Serialize>(value: &T) -> Result<String, ron::Error> {
|
||||
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<T: Serialize>(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::<Demo>(&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]
|
||||
|
||||
Reference in new issue
Block a user