Compare commits

..
9 Commits
Author SHA1 Message Date
iris 4de8bff5f2 Ignore the app half's build outputs, which the last commit added
.gitignore covered the Rust half's target/ only, so `git add -A` swept in
about 190 files of Gradle intermediates -- caches, lint models, .class and
.dex files. Removed from tracking and ignored, in a new commit rather than
by rewriting what was already pushed.
2026-08-28 17:57:18 -04:00
iris 4e423bbfb0 Take the app half too, which was duplicated the same way
Four Kotlin files appeared in both apps. Diffed with the product names
normalised, EnrollmentScanActivity was 31 lines each differing in six --
all of them comments -- and PinnedCert was 58 against 59 with identical
TrustManager logic. That is the same evidence that moved the Rust half.

Two parameters, both per-app, and they fail differently. A wrong URI
scheme means a scanned QR is ignored, which is visible at once. A wrong
Keystore alias means the app cannot unseal the token it already stored,
so an enrolled phone reads as not enrolled and nothing says why -- so
both existing values are recorded in the README rather than left to be
rediscovered.

The certificate alias went the other way and became a constant: it names
an entry in a KeyStore that exists only in memory for the length of one
lazy block, so nothing ever reads it back and having two of them said
nothing.

The drift was in both directions, as the evidence predicted: ai-app had
gained localNetworkAllowed -- which matters because Android 17's
ACCESS_LOCAL_NETWORK denial is invisible at the socket, and without it a
blocked app and an unreachable server produce the same timeout -- and had
moved to the KTX `edit` block. dev-updater had neither, and gets both.

No Compose, deliberately. Nothing here draws anything; the screens are
each app's own because that is where the two products actually differ.

The PinnedCaCertificate generator did not move, and the README says why:
all three things it varies are per-app, so sharing it means a composite
build neither project has. Its one historical bug is already fixed
identically in both copies.
2026-08-28 17:57:07 -04:00
iris 73a32cb897 Add the sixth thing both projects wrote: where state lives
Both servers keep config, the CA's private key and their own state
outside the repo, and both resolve the location by the same rules --
XDG variable, ignored unless absolute, falling back under $HOME, joined
with the product name. ai-app had factored it into one helper taking the
variable and the fallback; dev-updater had the same logic inline in two
functions. That is the test this crate applies: identical but for a
product name.

The reasoning is worth keeping together with the code, so the module doc
carries both halves of why it is outside the repo -- the shared mount
resolves at different absolute paths on each side, which is how every
project once read "not built" on one of them, and a CA key on a mount
the untrusted side can write would let it mint a leaf the pinned app
trusts.

`xdg_dir` takes the environment as an argument so the rules can be
tested without setting process-wide variables, which parallel tests
cannot do without racing. Three tests come across from ai-app, including
the one that matters least often and costs most: a relative setting is
ignored rather than resolved, so a server started from a different
working directory does not quietly look elsewhere for its token hashes
and enroll itself afresh.
2026-08-28 17:33:39 -04:00
iris 841a3a8372 Make the crate do what its documentation already claimed
Review from ai-app's session, acted on.

**A test that asserted nothing.** `a_missing_tunnel_explains_itself`
returned early whenever `wg0` was up -- which it is on this VM and on the
host, so it passed everywhere and never once checked the message. The same
shape of test that let the file-permissions bug survive three codebases.
`netif` now splits the lookup from the decision: `wg_address_among` and
`addresses_among` take the interfaces, so the failure is reachable by
handing in an empty list rather than by hoping the machine has no tunnel.
Four tests where there were two, including that an IPv6-only `wg0` is not
an answer.

**Two modules the docs promised and the crate did not have.** `certs`,
which is the piece where being written twice is worst -- a trust anchor
built two ways can be built differently two ways, and the difference
reaches a phone as an opaque handshake failure. And `format`, the RON
house rules, which were byte-identical in both projects and so the
clearest thing in the evidence table. `product` names the certificate and
is the whole of what is per-project; the test decodes the DER and looks
for it there rather than trusting what was passed in.

**The README title still said wg-server-app**, three commits after
everything else was renamed.

Two judgement calls promoted from silent to written down, both of which
would otherwise be inherited rather than chosen: `WG_INTERFACE` is a
constant because *these* projects have one tunnel, which is the first
thing a third user should expect to change; and `local_addresses` puts
the emulator's host alias in every certificate, a SAN for an address the
machine does not own.

And a review heuristic the day kept proving: when you find a rule stated,
grep for its siblings. Three bugs today were the correct rule already
written down and applied to one member of a set.
2026-08-28 14:34:48 -04:00
iris 592114bfc9 Name it wg-app-link
`wg-link` was not merely vague. `wg` and `ip link` are both real network
tools, so it reads as something that manages a WireGuard interface --
misleading rather than ambiguous. `app` rules that reading out, and says
which two ends the link is between.

The repository is still called wg-server-app until it is renamed on
gitea; the crate leads.
2026-08-28 13:34:30 -04:00
iris d6d802c3c7 Record the bug the extraction found, and why nobody had caught it
Three copies, one bug, and the correct reasoning already written down in
a comment three functions above the code that needed it. That is an
argument about review rather than about duplication, and it is more
persuasive than any of the similarity percentages above it.
2026-08-28 13:30:41 -04:00
iris 861e6a329b Narrow a file that already existed, not just one being created
`OpenOptions::mode` applies only when the file is created, so opening one
that already exists keeps whatever mode it had. For `append_file` that is
the normal case rather than the odd one -- every write to a transcript
after the first -- and for `create_file` it happens on every rewrite. A
file made wrong by an older version or by hand would have stayed
world-readable for as long as it was only ever appended to.

This is the same reasoning as `create_dir`'s second call, which was
already here and already commented. Missing it on the file paths was the
kind of gap that only shows when the two are read side by side, which is
an argument for this crate existing rather than against it.

Set through the open handle rather than the path, so nothing can swap
what is at that path between the open and the chmod.

Caught by ai-app's session reviewing the module I had taken from their
code, which is the review this repo is supposed to make possible.
2026-08-28 13:30:41 -04:00
iris 7651d491ac The link both projects wrote twice
dev-updater serves APKs to a phone and ai-app runs model sessions for
one. Above the waterline they share nothing. Underneath they are the
same program: bound to wg0 so they are not on the LAN, presenting a
certificate from a CA the app pins, answering only requests carrying a
token enrolled by scanning a QR off the terminal, keeping state in
owner-only files outside the repo.

Three modules, each extracted only after diffing the two copies and
finding nothing between them but a product name and a type parameter.
netif fails closed when the tunnel is down. enroll generates, stores and
compares the token, and prints the QR, with the URI scheme as the one
per-project part. private owns the file modes, taken from ai-app's
version because it had already factored out what dev-updater still has
inline in two places.

Nothing is removed from either project. This is a proposal with a
working core, and the README carries the measured evidence -- the
enrollment scanner activity differs by its package line and nothing
else, the RON format module is byte-identical, and the two copies have
each drifted into holding an improvement the other lacks, which is the
cost being paid today.
2026-08-28 13:30:41 -04:00
iris 995b29f10d Initial commit 2026-08-28 13:25:51 -04:00
6 changed files with 16 additions and 346 deletions

No files matched your search

+2 -2
View File
@@ -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. - **`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. - **`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. - **`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, 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. - **`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.
- **`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. - **`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. - **`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: **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. 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.~~ 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. 2. Atomic owner-only config save. Both do temp-file-then-rename with the mode set before the rename; only the schema differs.
**Should not.** Naming these is the point of the exercise: **Should not.** Naming these is the point of the exercise:
@@ -147,18 +147,6 @@ class ServerStore(private val scheme: String, private val keyAlias: String) {
* the OS drops the traffic, so a blocked app and an unreachable server produce the same connect * the OS drops the traffic, so a blocked app and an unreachable server produce the same connect
* timeout. Without asking explicitly there is no way to tell those apart, and the failure shown * timeout. Without asking explicitly there is no way to tell those apart, and the failure shown
* would blame the server or the tunnel for something neither is doing. * would blame the server or the tunnel for something neither is doing.
*
* **The permission is still required when the server is reached through WireGuard, and the
* platform's own documentation says otherwise.** Android's Local Network Definition describes a
* local network as one that "utilizes a broadcast-capable network interface, such as Wi-Fi or
* Ethernet, but excludes cellular (WWAN) or VPN connections" — read straight, a tunnelled 10.66.0.1
* is excluded and needs nothing. Measured on a real device on 2026-08-28: it is not excluded, and
* without the permission the traffic is dropped. Do not remove the permission on the strength of
* that paragraph.
*
* **Neither project can catch this in an emulator.** The API 36 images both are tested against do
* not enforce the permission at all, so removing it passes every local test and fails only on a
* phone. That asymmetry is the reason this note is here rather than in a commit message.
*/ */
fun localNetworkAllowed(context: Context): Boolean = fun localNetworkAllowed(context: Context): Boolean =
android.os.Build.VERSION.SDK_INT < 37 || android.os.Build.VERSION.SDK_INT < 37 ||
+4 -1
View File
@@ -155,7 +155,10 @@ mod tests {
} }
fn addresses() -> Vec<IpAddr> { fn addresses() -> Vec<IpAddr> {
vec!["192.168.1.5".parse().unwrap(), "127.0.0.1".parse().unwrap()] vec![
"192.168.1.168".parse().unwrap(),
"127.0.0.1".parse().unwrap(),
]
} }
#[test] #[test]
+9 -240
View File
@@ -24,10 +24,7 @@
//! middleware, which stays in each project because it is generic over //! middleware, which stays in each project because it is generic over
//! that project's state. //! that project's state.
use std::fs;
use std::net::IpAddr; use std::net::IpAddr;
use std::path::Path;
use std::time::Duration;
use anyhow::{Context, Result}; use anyhow::{Context, Result};
use base64::Engine; use base64::Engine;
@@ -71,62 +68,8 @@ pub fn token_matches(presented: &str, stored_hashes: &[String]) -> bool {
/// The scheme is the caller's because it is what routes the scan back to /// The scheme is the caller's because it is what routes the scan back to
/// the right app -- `devupdater`, `aiapp` -- and it is the only part of /// the right app -- `devupdater`, `aiapp` -- and it is the only part of
/// enrollment that is per-project. /// enrollment that is per-project.
/// pub fn enrollment_uri(scheme: &str, host: IpAddr, port: u16, token: &str) -> String {
/// `ca_pem` is the trust anchor, and is optional because the two projects format!("{scheme}://enroll?host={host}&port={port}&token={token}")
/// answer "where does the app get the CA?" differently. An app built on
/// the machine its server runs on pins the CA at build time and needs
/// nothing here (pass `None`); one built somewhere else -- ai-app's iris
/// client is cross-compiled in a VM and run against the host's server --
/// cannot, so the CA travels with the link instead. See [`ca_param`] for
/// the encoding and what it costs a QR code.
pub fn enrollment_uri(
scheme: &str,
host: IpAddr,
port: u16,
token: &str,
ca_pem: Option<&str>,
) -> Result<String> {
let mut uri = format!("{scheme}://enroll?host={host}&port={port}&token={token}");
if let Some(ca_pem) = ca_pem {
uri.push_str("&ca=");
uri.push_str(&ca_param(ca_pem)?);
}
Ok(uri)
}
/// The `ca` parameter's value for one PEM certificate: its DER, base64url
/// without padding, so it needs no percent-encoding and survives every
/// splitter a link passes through.
///
/// **The parameter is optional and unrecognised keys are ignored**, so a
/// link carrying it still enrolls an app that predates it -- which is what
/// makes adding it a compatible change to a format three languages parse
/// (this crate mints it; `client_core::config` and `ServerStore.kt` read
/// it).
///
/// Costs a QR code real estate: measured on ai-app's own P-256 CA, a link
/// goes from 89 bytes to 652 and its terminal QR from 45 to 93 columns.
/// That is why the parameter is the minter's choice per call rather than
/// always present.
pub fn ca_param(ca_pem: &str) -> Result<String> {
let der = pem_der(ca_pem)?;
Ok(base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(der))
}
/// The DER inside a PEM certificate: everything between the BEGIN and END
/// lines, whitespace removed, base64-decoded.
fn pem_der(pem: &str) -> Result<Vec<u8>> {
const BEGIN: &str = "-----BEGIN CERTIFICATE-----";
const END: &str = "-----END CERTIFICATE-----";
let body = pem
.split_once(BEGIN)
.and_then(|(_, rest)| rest.split_once(END))
.map(|(body, _)| body)
.context("not a PEM certificate (no BEGIN/END CERTIFICATE lines)")?;
let body: String = body.chars().filter(|c| !c.is_whitespace()).collect();
base64::engine::general_purpose::STANDARD
.decode(body.as_bytes())
.context("a PEM certificate's body is not base64")
} }
/// Prints the one-time enrollment QR, and the URI under it for a person /// Prints the one-time enrollment QR, and the URI under it for a person
@@ -136,17 +79,11 @@ fn pem_der(pem: &str) -> Result<Vec<u8>> {
/// at the terminal, once, and a log line is the wrong shape for something /// at the terminal, once, and a log line is the wrong shape for something
/// that has to be photographed. /// that has to be photographed.
/// ///
/// The QR carries no *secret* beyond the token, which is rotatable: /// The QR carries no trust material. The CA is embedded in the app at
/// `ca_pem`, where the caller passes one, is a public certificate, so /// build time, so photographing the terminal leaks only the token, which
/// photographing the terminal still leaks only the token. /// is rotatable.
pub fn print_enrollment( pub fn print_enrollment(scheme: &str, host: IpAddr, port: u16, token: &str) -> Result<()> {
scheme: &str, let uri = enrollment_uri(scheme, host, port, token);
host: IpAddr,
port: u16,
token: &str,
ca_pem: Option<&str>,
) -> Result<()> {
let uri = enrollment_uri(scheme, host, port, token, ca_pem)?;
let code = qrcode::QrCode::new(uri.as_bytes()).context("render enrollment QR")?; let code = qrcode::QrCode::new(uri.as_bytes()).context("render enrollment QR")?;
let rendered = code let rendered = code
.render::<qrcode::render::unicode::Dense1x2>() .render::<qrcode::render::unicode::Dense1x2>()
@@ -160,74 +97,6 @@ pub fn print_enrollment(
Ok(()) Ok(())
} }
/// How long a spooled enrolment stays valid unused. The link is meant to be
/// opened straight away, by the tool that asked for it; one that was never
/// opened should not stay a valid credential on disk.
pub const PENDING_TTL: Duration = Duration::from_secs(60 * 60);
/// Records a token minted by another process for the running server to
/// adopt on first use -- see [`take_pending`].
///
/// Why a spool rather than writing the config: the server holds its config
/// in memory and writes it back whole, so a second process appending a
/// token to the file loses the race with the next save, silently. Here the
/// other process writes only into `dir` (created private to the user), one
/// file per token, named by the hash and holding the device name; the
/// server owns the config as before and moves the entry across itself.
/// Only the hash touches disk, as with every stored token.
pub fn spool_pending(dir: &Path, name: &str, token: &str) -> Result<()> {
fs::create_dir_all(dir).with_context(|| format!("create {}", dir.display()))?;
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
fs::set_permissions(dir, fs::Permissions::from_mode(0o700))
.with_context(|| format!("restrict {}", dir.display()))?;
}
let path = dir.join(token_hash_hex(token));
fs::write(&path, name).with_context(|| format!("write {}", path.display()))?;
Ok(())
}
/// Adopts a spooled token if `presented` is one: returns the device name
/// it was spooled under and removes the entry, so a spooled token is
/// consumed exactly once and belongs to the config from then on. Anything
/// older than [`PENDING_TTL`] is removed rather than honoured.
///
/// A missing directory is the common case -- nothing has ever been
/// spooled -- and answers `None` like an empty one.
pub fn take_pending(dir: &Path, presented: &str) -> Result<Option<String>> {
let entries = match fs::read_dir(dir) {
Ok(entries) => entries,
Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None),
Err(error) => return Err(error).with_context(|| format!("read {}", dir.display())),
};
let wanted = token_hash_hex(presented);
let mut found = None;
for entry in entries {
let entry = entry.with_context(|| format!("read {}", dir.display()))?;
let path = entry.path();
let fresh = entry
.metadata()
.and_then(|meta| meta.modified())
.ok()
.and_then(|modified| modified.elapsed().ok())
.is_some_and(|age| age < PENDING_TTL);
if !fresh {
let _ = fs::remove_file(&path);
continue;
}
let name = entry.file_name();
let name = name.to_string_lossy();
if bool::from(name.as_bytes().ct_eq(wanted.as_bytes())) {
found = Some(path);
}
}
let Some(path) = found else { return Ok(None) };
let device = fs::read_to_string(&path).with_context(|| format!("read {}", path.display()))?;
fs::remove_file(&path).with_context(|| format!("remove {}", path.display()))?;
Ok(Some(device))
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
@@ -266,112 +135,12 @@ mod tests {
/// back -- so the shape is a contract, not a formatting choice. /// back -- so the shape is a contract, not a formatting choice.
#[test] #[test]
fn the_enrollment_uri_carries_scheme_host_port_and_token() { fn the_enrollment_uri_carries_scheme_host_port_and_token() {
let uri = enrollment_uri( let uri = enrollment_uri("devupdater", "10.66.0.1".parse().unwrap(), 8090, "tok");
"devupdater",
"10.66.0.1".parse().unwrap(),
8090,
"tok",
None,
)
.unwrap();
assert_eq!( assert_eq!(
uri, uri,
"devupdater://enroll?host=10.66.0.1&port=8090&token=tok" "devupdater://enroll?host=10.66.0.1&port=8090&token=tok"
); );
let other = let other = enrollment_uri("aiapp", "10.66.0.1".parse().unwrap(), 8443, "tok");
enrollment_uri("aiapp", "10.66.0.1".parse().unwrap(), 8443, "tok", None).unwrap();
assert!(other.starts_with("aiapp://enroll?")); assert!(other.starts_with("aiapp://enroll?"));
} }
/// The CA rides as base64url of the DER, appended to the same link --
/// so a reader that ignores unknown keys sees exactly the link above.
#[test]
fn a_ca_rides_in_the_link_as_url_safe_base64_der() {
let der = [0x30u8, 0x82, 0x01, 0xfb, 0x3e, 0x7f];
let pem = format!(
"-----BEGIN CERTIFICATE-----\n{}\n-----END CERTIFICATE-----\n",
base64::engine::general_purpose::STANDARD.encode(der)
);
let uri = enrollment_uri(
"aiapp",
"10.66.0.1".parse().unwrap(),
8443,
"tok",
Some(&pem),
)
.unwrap();
let (base, ca) = uri.split_once("&ca=").expect("the ca parameter");
assert_eq!(base, "aiapp://enroll?host=10.66.0.1&port=8443&token=tok");
assert_eq!(
base64::engine::general_purpose::URL_SAFE_NO_PAD
.decode(ca)
.unwrap(),
der,
"the parameter is the certificate's DER, url-safe and unpadded"
);
assert!(
!ca.contains(['+', '/', '=']),
"nothing in it needs percent-encoding: {ca}"
);
}
/// Something that is not a certificate is refused where it is read,
/// rather than minting a link an app can only fail on.
#[test]
fn a_ca_that_is_not_a_certificate_is_named_in_the_error() {
let err = ca_param("hello").unwrap_err().to_string();
assert!(err.contains("PEM certificate"), "{err}");
}
fn scratch_dir() -> std::path::PathBuf {
let dir = std::env::temp_dir().join(format!("wg-app-link-enroll-{}", generate_token()));
fs::create_dir_all(&dir).unwrap();
dir
}
/// A spooled token is adopted once, under the name it was spooled with,
/// and by nothing but that token.
#[test]
fn a_spooled_token_is_taken_exactly_once() {
let dir = scratch_dir();
let token = generate_token();
spool_pending(&dir, "tablet", &token).unwrap();
assert_eq!(take_pending(&dir, "wrong").unwrap(), None);
assert_eq!(
take_pending(&dir, &token).unwrap().as_deref(),
Some("tablet")
);
assert_eq!(take_pending(&dir, &token).unwrap(), None, "consumed");
assert!(
fs::read_dir(&dir).unwrap().next().is_none(),
"nothing left behind"
);
fs::remove_dir_all(dir).unwrap();
}
/// Nothing spooled -- not even the directory -- is an ordinary miss.
#[test]
fn no_spool_is_a_miss() {
let dir = scratch_dir().join("never-made");
assert_eq!(take_pending(&dir, "anything").unwrap(), None);
}
/// An entry past its age is swept rather than honoured.
#[test]
fn a_stale_entry_is_swept_not_honoured() {
let dir = scratch_dir();
let token = generate_token();
spool_pending(&dir, "old", &token).unwrap();
let path = dir.join(token_hash_hex(&token));
let past = std::time::SystemTime::now() - PENDING_TTL - Duration::from_secs(1);
fs::File::options()
.write(true)
.open(&path)
.unwrap()
.set_modified(past)
.unwrap();
assert_eq!(take_pending(&dir, &token).unwrap(), None);
assert!(!path.exists(), "swept");
fs::remove_dir_all(dir).unwrap();
}
} }
-90
View File
@@ -35,13 +35,8 @@
/// `#![enable(implicit_some)]` header every project file would have to /// `#![enable(implicit_some)]` header every project file would have to
/// remember, and matched on the writing side by `skip_serializing_if` so /// remember, and matched on the writing side by `skip_serializing_if` so
/// nothing writes back a `Some(...)` a person didn't type. /// nothing writes back a `Some(...)` a person didn't type.
use std::path::Path;
use anyhow::{Context, Result};
use serde::{Serialize, de::DeserializeOwned}; use serde::{Serialize, de::DeserializeOwned};
use crate::private;
fn options() -> ron::Options { fn options() -> ron::Options {
ron::Options::default().with_default_extension(ron::extensions::Extensions::IMPLICIT_SOME) ron::Options::default().with_default_extension(ron::extensions::Extensions::IMPLICIT_SOME)
} }
@@ -56,40 +51,6 @@ pub fn render<T: Serialize>(value: &T) -> Result<String, ron::Error> {
Ok(unwrap_outer(&text)) 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 /// Strips the outer `(`/`)` the writer always emits and removes the
/// indent level they cost. Deliberately narrow: it accepts only the /// indent level they cost. Deliberately narrow: it accepts only the
/// exact shape `PrettyConfig` produces, and leaves anything else alone /// exact shape `PrettyConfig` produces, and leaves anything else alone
@@ -185,57 +146,6 @@ 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 /// 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. /// why the opening paren is not followed by a newline.
#[test] #[test]
+1 -1
View File
@@ -114,7 +114,7 @@ mod tests {
/// and nothing may appear twice, since these become certificate SANs. /// and nothing may appear twice, since these become certificate SANs.
#[test] #[test]
fn the_certificate_always_covers_loopback_and_the_emulator_alias() { fn the_certificate_always_covers_loopback_and_the_emulator_alias() {
let found = [ip("192.168.1.5"), ip("10.66.0.1"), ip("192.168.1.5")]; let found = [ip("192.168.1.168"), ip("10.66.0.1"), ip("192.168.1.168")];
let addresses = addresses_among(found); let addresses = addresses_among(found);
assert!(addresses.contains(&ip("127.0.0.1"))); assert!(addresses.contains(&ip("127.0.0.1")));