From c09124f2e9abfd01e57416233d7d6895625e7b3b Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Tue, 1 Sep 2026 20:52:48 -0700 Subject: [PATCH 1/4] chore(release): anchor v0.241.0 for #491 Salvage anchor for the wallet-seed-path lane. Co-Authored-By: Claude --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fea707be..d99e78b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3031,7 +3031,7 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.236.0" +version = "0.241.0" dependencies = [ "async-trait", "axum", diff --git a/Cargo.toml b/Cargo.toml index effdd8e9..92db188e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,7 @@ edition = "2021" # the ROOT manifest (`[workspace.package].version`), so it MUST be set here for a # release to fire (§3.6). The library crates (dig-node-core/dig-runtime/dig-wallet) # keep their own independent versions — only the released binary tracks the workspace version. -version = "0.236.0" +version = "0.241.0" # Release hardening, matching digstore: keep integer-overflow checks ON in release. # The node parses untrusted serialized input and does offset/length arithmetic over From 48e138fba0c190b2eea25c3791051e4f018d2fad Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Tue, 1 Sep 2026 21:31:52 -0700 Subject: [PATCH 2/4] fix(wallet): anchor the wallet base for a service run without orphaning an existing wallet On a stock Linux .deb install the seed was CREATED at /DigWallet/seed.bin - the filesystem root - and the write SUCCEEDED. The shipped unit sets no User=, so systemd sets no $HOME, and no WorkingDirectory=, so the working directory is /; the wallet's LOCALAPPDATA -> HOME -> "." chain therefore collapsed to a relative base, and ProtectSystem=full leaves / writable for the root-run service. The device key resolved from the same broken base, so the pair stayed consistent and the wallet opened normally, which is why nothing surfaced it. Introduce DIG_WALLET_BASE, a single override of the BASE that both wallet roots hang off, and route seed_path() and autoseed::user_base() through one resolver. Overriding the base rather than either directory keeps DigWallet/ and DigNode/device/ siblings by construction - that separation is the partial-exfiltration boundary autoseed's module docs describe, and a per-directory override would let configuration alone collapse it. A service run anchors that base at the machine state dir, but ONLY when no wallet is present at the base it would otherwise have resolved. The operator wallet holds real $DIG for mirror-coin collateral, so re-rooting an existing host would strand the funded seed and mint an empty replacement; Windows LocalSystem services are the live case, since %LOCALAPPDATA% IS set for them. Presence is read through autoseed::presence and an undeterminable answer counts as present. No key material is moved or copied. The resolver is pure and takes its inputs explicitly so the service environment (no LOCALAPPDATA, no HOME) and the Windows ordering are both exercised on the Linux CI runner, without the process-global env serialization this crate already works around. Refs #491 Co-Authored-By: Claude --- SPEC.md | 22 ++++ crates/dig-node-service/src/state.rs | 156 ++++++++++++++++++++++-- crates/dig-wallet/src/autoseed.rs | 16 +-- crates/dig-wallet/src/lib.rs | 170 ++++++++++++++++++++++++++- 4 files changed, 340 insertions(+), 24 deletions(-) diff --git a/SPEC.md b/SPEC.md index 0d1719f5..4629681f 100644 --- a/SPEC.md +++ b/SPEC.md @@ -1568,6 +1568,24 @@ is a property of a SERVICE RUN and not of one packaging target. An `DIG_IDENTITY_DIR` / `DIG_NODE_CACHE` value the operator set explicitly MUST be preserved; a CLI run MUST be left untouched, keeping the user's identity shared with their other DIG tools. +**The wallet base — same anchoring, with one condition.** The node's operator wallet (§16.4) resolves +its base from `DIG_WALLET_BASE`, then `%LOCALAPPDATA%`, then `$HOME`. Inside the packaged systemd +unit none of those is set: the unit declares no `User=`, so systemd sets no `$HOME`, and no +`WorkingDirectory=`, so the working directory is `/`. A relative fallback therefore resolves the seed +to `/DigWallet/seed.bin`, and that write SUCCEEDS — the unit runs as root and `ProtectSystem=full` +leaves `/` writable — so the node operates normally with its wallet at the filesystem root. + +A SERVICE run MUST therefore anchor `DIG_WALLET_BASE` at the resolved state dir, giving +`/DigWallet/seed.bin` and `/DigNode/device/device.key`. The override names the +BASE and never either directory: both roots MUST derive from one value, or the sibling relationship +§16.4 requires could be broken by configuration alone. + +It MUST be adopted ONLY when no wallet is present at the base the service would otherwise have +resolved. The operator wallet holds real $DIG for mirror-coin collateral, so re-rooting a host that +already has one would leave the funded seed unreferenced and mint an empty replacement; a Windows +LocalSystem service, whose `%LOCALAPPDATA%` IS set, is exactly that case. Presence that cannot be +DETERMINED MUST count as present. Key material MUST NOT be moved or copied automatically. + **Creation + ACL — the HARDENING CONTRACT.** The state dir holds the control token that grants FULL local control, so its ACL MUST NOT be world/all-users-readable. On Windows this is the HARD case: `%PROGRAMDATA%` grants `BUILTIN\Users` "create subfolder", so ANY low-priv user can pre-create @@ -4462,6 +4480,10 @@ protected `D:P(A;;FA;;;)` DACL on Windows, never the ACL inherited from `% | `/device.key` | 32 raw CSPRNG bytes, no header | | `/wallet.meta.json` | `origin`, `created_at` (RFC 3339), `ever_funded` | +`` is resolved from `DIG_WALLET_BASE`, then `%LOCALAPPDATA%`, then `$HOME`, by ONE resolver +that both roots below use. A service run anchors that base at the machine state dir under the +condition stated in §7.3a. + `` is `/DigNode/device/` — a **SIBLING** of `` (`/DigWallet/`), never a child. **That separation IS the partial-exfiltration boundary and MUST NOT be collapsed.** Placing the key inside the wallet directory degrades the seal to a diff --git a/crates/dig-node-service/src/state.rs b/crates/dig-node-service/src/state.rs index 0c026d96..ee0a9c18 100644 --- a/crates/dig-node-service/src/state.rs +++ b/crates/dig-node-service/src/state.rs @@ -177,6 +177,10 @@ pub const IDENTITY_DIR_ENV: &str = "DIG_IDENTITY_DIR"; /// The env var that overrides the node's content CACHE dir (read by `dig_node_core`). pub const CACHE_DIR_ENV: &str = "DIG_NODE_CACHE"; +/// The env var that overrides the base BOTH wallet roots hang off. Re-exported from the crate +/// that owns the layout rather than re-spelled here, so the two can never drift apart. +pub use dig_wallet::WALLET_BASE_ENV; + /// PURE decision core (no I/O): the identity + cache directory overrides a SERVICE run must /// adopt, given whether it is a service, the resolved machine `state_dir`, and whether the /// operator has already set each override explicitly. @@ -195,12 +199,35 @@ pub const CACHE_DIR_ENV: &str = "DIG_NODE_CACHE"; /// the process environment, and so the ONE place that mutates the environment is a single /// documented call early in startup. /// +/// # The wallet base, and the one condition on it (dig-node#491) +/// +/// The wallet's own resolver (`DIG_WALLET_BASE`, then `%LOCALAPPDATA%`, then `$HOME`) misses every +/// rung inside the shipped systemd unit: it sets no `User=`, so systemd sets no `$HOME`, and no +/// `WorkingDirectory=`, so the working directory is `/`. The seed was therefore CREATED at +/// `/DigWallet/seed.bin`, and the write SUCCEEDED — the unit runs as root and `ProtectSystem=full` +/// leaves `/` writable — so the node came up with a working wallet at the filesystem root and +/// nothing anywhere said so. +/// +/// `legacy_wallet_present` is what keeps that fix from being worse than the bug. The node's +/// operator wallet holds real $DIG for mirror-coin collateral, so anchoring a host that ALREADY +/// has a wallet at the old location would leave the funded file unreferenced and mint a fresh +/// empty one beside it. This is a live risk on Windows in particular, where `%LOCALAPPDATA%` IS +/// set for a LocalSystem service, so existing installs already hold a seed under the systemprofile +/// path. Existing hosts therefore keep their old base; only a host with no wallet yet adopts the +/// anchored one, and moving key material is left to a deliberate operator-run migration. +/// +/// This is NOT the "derive the wallet base from the node's cache dir" change `crate::wallet_env` +/// argues against, and the difference is the whole design: this is an independent explicit base, +/// adopted only where there is nothing to orphan. +/// /// An override the operator set explicitly is never replaced — their choice outranks ours. pub fn service_data_dir_overrides( is_service: bool, state_dir: &Path, identity_dir_set: bool, cache_dir_set: bool, + wallet_base_set: bool, + legacy_wallet_present: bool, ) -> Vec<(&'static str, PathBuf)> { if !is_service { return Vec::new(); @@ -212,6 +239,9 @@ pub fn service_data_dir_overrides( if !cache_dir_set { out.push((CACHE_DIR_ENV, state_dir.join("cache"))); } + if !wallet_base_set && !legacy_wallet_present { + out.push((WALLET_BASE_ENV, state_dir.to_path_buf())); + } out } @@ -226,11 +256,27 @@ pub fn anchor_service_data_dirs() { return; } let dir = state_dir(); + // Asked BEFORE the override is written, and answered by the crate that owns the layout, so the + // question is genuinely "what would this build have opened a moment ago" rather than a guess + // at the path. `legacy_wallet_present` treats an undeterminable answer as PRESENT, which is + // the direction that cannot strand a funded wallet. + let legacy_wallet = dig_wallet::legacy_wallet_present(); + if legacy_wallet { + // The PATH only. Never the contents, and never a hint at them. + tracing::warn!( + seed = %dig_wallet::legacy_seed_path().display(), + state_dir = %dir.display(), + "a wallet already exists at the pre-#491 location, so this service keeps opening it \ + rather than the machine state dir; move it deliberately if you want it anchored" + ); + } let overrides = service_data_dir_overrides( true, &dir, env_is_set(IDENTITY_DIR_ENV), env_is_set(CACHE_DIR_ENV), + env_is_set(WALLET_BASE_ENV), + legacy_wallet, ); for (key, value) in overrides { // Create it up front so the first write does not race, and so a failure surfaces here @@ -981,8 +1027,14 @@ mod tests { fn a_service_run_anchors_both_identity_and_cache_under_the_machine_state_dir() { // The exact failure this prevents: the packaged unit sets ProtectHome=true, so a seed // written under $HOME fails EROFS and the peer network never starts. - let overrides = - service_data_dir_overrides(true, Path::new("/var/lib/dig-node"), false, false); + let overrides = service_data_dir_overrides( + true, + Path::new("/var/lib/dig-node"), + false, + false, + true, + true, + ); assert_eq!( overrides, vec![ @@ -995,29 +1047,107 @@ mod tests { ); } + // -- The wallet base (#491) --------------------------------------------------------------- + // + // On the shipped systemd unit the wallet's own chain resolves nothing (no `User=` so no + // `$HOME`, and no `LOCALAPPDATA` on Linux) and collapses to the working directory, which for a + // system unit is `/`. The seed was created at `/DigWallet/seed.bin` and the write SUCCEEDED, + // so the node has been running with a wallet at the filesystem root. + + #[test] + fn a_service_with_no_wallet_yet_anchors_the_wallet_base_at_the_state_dir() { + let overrides = service_data_dir_overrides( + true, + Path::new("/var/lib/dig-node"), + true, + true, + false, + false, + ); + assert_eq!( + overrides, + vec![(WALLET_BASE_ENV, PathBuf::from("/var/lib/dig-node"))], + "the BASE, not the wallet directory: the seed and the device key both hang off it" + ); + } + + #[test] + fn a_service_that_already_has_a_wallet_keeps_opening_the_old_one() { + // The safety-critical direction. The node's operator wallet holds real $DIG for + // mirror-coin collateral, so re-rooting an existing install would leave the funded seed + // unreferenced and mint a fresh empty wallet in its place. Existing Windows service + // installs are the live case: `%LOCALAPPDATA%` IS set for LocalSystem, so they already + // hold a seed under the systemprofile path. + assert!( + service_data_dir_overrides( + true, + Path::new("/var/lib/dig-node"), + true, + true, + false, + true + ) + .is_empty(), + "no wallet override may be emitted while a wallet exists at the legacy path" + ); + } + + #[test] + fn an_operator_set_wallet_base_outranks_the_anchor() { + // Even with nothing to orphan: the operator's explicit choice wins, exactly as it does for + // the identity and cache dirs. + assert!(service_data_dir_overrides( + true, + Path::new("/var/lib/dig-node"), + true, + true, + true, + false + ) + .is_empty()); + } + #[test] fn a_cli_run_is_left_entirely_alone() { // The CLI legitimately lives under the user's home and shares that identity with the // user's other DIG tools; anchoring it machine-wide would change where an existing - // user's key is looked up. - assert!( - service_data_dir_overrides(false, Path::new("/var/lib/dig-node"), false, false) - .is_empty() - ); + // user's key is looked up — and for the wallet base that would mean a CLI reading a + // different seed from the one it read yesterday. + assert!(service_data_dir_overrides( + false, + Path::new("/var/lib/dig-node"), + false, + false, + false, + false + ) + .is_empty()); } #[test] fn an_operator_set_override_is_never_replaced() { - let overrides = - service_data_dir_overrides(true, Path::new("/var/lib/dig-node"), true, false); + let overrides = service_data_dir_overrides( + true, + Path::new("/var/lib/dig-node"), + true, + false, + true, + true, + ); assert_eq!( overrides, vec![(CACHE_DIR_ENV, PathBuf::from("/var/lib/dig-node/cache"))], "only the unset one is filled in" ); - assert!( - service_data_dir_overrides(true, Path::new("/var/lib/dig-node"), true, true).is_empty() - ); + assert!(service_data_dir_overrides( + true, + Path::new("/var/lib/dig-node"), + true, + true, + true, + true + ) + .is_empty()); } #[test] @@ -1025,7 +1155,7 @@ mod tests { // A user-level service that could not create /var/lib falls back to a per-user dir; the // anchor must follow it rather than hardcoding the machine path. let legacy = Path::new("/home/u/.local/share/DigNode"); - for (_, path) in service_data_dir_overrides(true, legacy, false, false) { + for (_, path) in service_data_dir_overrides(true, legacy, false, false, false, false) { assert!( path.starts_with(legacy), "{} must live under the resolved state dir", diff --git a/crates/dig-wallet/src/autoseed.rs b/crates/dig-wallet/src/autoseed.rs index b6288c96..61122258 100644 --- a/crates/dig-wallet/src/autoseed.rs +++ b/crates/dig-wallet/src/autoseed.rs @@ -82,8 +82,8 @@ impl WalletPaths { /// Resolve the production layout: the seed and its metadata under `DigWallet/`, the device key /// under the sibling `DigNode/device/`. /// - /// Both roots derive from the same per-user base the wallet already used (`%LOCALAPPDATA%`, - /// falling back to `$HOME`), so this adds no new location contract — only the split. + /// Both roots derive from the same base the wallet already used (`DIG_WALLET_BASE`, then + /// `%LOCALAPPDATA%`, then `$HOME`), so this adds no new location contract — only the split. pub fn resolve(seed: PathBuf) -> Self { let meta = sibling(&seed, "wallet.meta.json"); let device_key = user_base() @@ -106,12 +106,14 @@ pub fn default_paths() -> WalletPaths { WalletPaths::resolve(crate::seed_path()) } -/// The per-user, non-roaming base directory both roots hang off (NC-3's location contract). +/// The base directory both roots hang off (NC-3's location contract). +/// +/// Delegates to [`crate::wallet_base`] rather than re-deriving the chain, so the seed and the +/// device key can never resolve from different bases. A second copy of this resolution is exactly +/// how the sibling relationship documented above would come apart: the two files would still be +/// named correctly and would simply stop being siblings, which nothing downstream checks. fn user_base() -> PathBuf { - let base = std::env::var("LOCALAPPDATA") - .or_else(|_| std::env::var("HOME")) - .unwrap_or_else(|_| ".".to_string()); - PathBuf::from(base) + crate::wallet_base() } /// A path beside `path`, keeping its directory. Falls back to a bare relative name only when diff --git a/crates/dig-wallet/src/lib.rs b/crates/dig-wallet/src/lib.rs index 030d336c..9142e436 100644 --- a/crates/dig-wallet/src/lib.rs +++ b/crates/dig-wallet/src/lib.rs @@ -162,12 +162,87 @@ fn is_self_origin(origin: &str) -> bool { origin == format!("http://127.0.0.1:{port}") || origin == format!("http://localhost:{port}") } +/// The env var that overrides the base directory BOTH wallet roots hang off. +/// +/// It names the BASE, never a wallet directory, and that is the whole point: `DigWallet/` and the +/// device key's `DigNode/device/` are resolved from one value, so they cannot be pointed at +/// unrelated places. See [`crate::autoseed`] — the sibling relationship between those two +/// directories IS the partial-exfiltration boundary, and a per-directory override would let a +/// well-meaning operator collapse it. +pub const WALLET_BASE_ENV: &str = "DIG_WALLET_BASE"; + +/// Pick the base directory the wallet roots hang off, from explicitly-supplied inputs. +/// +/// Pure, and takes its three inputs rather than reading them, for a reason the crate has already +/// paid for once: the environment is process-global, so an env-reading resolver can only be tested +/// under a mutex that serialises every other test touching the same variables (see `ENV_LOCK` in +/// this module's tests), and the Windows arm is then unreachable on a Linux CI runner. Explicit +/// inputs make every arm runnable everywhere. +/// +/// `None` means NOTHING resolved a base. That is a real answer and is deliberately not collapsed +/// into a relative fallback here: a service unit sets no `HOME` and no `WorkingDirectory`, so a +/// relative base resolves against the process working directory, which for a systemd system unit +/// is `/`. The caller decides what to do with the absence. +/// +/// An empty value is treated as unset, matching how the node's other overrides are parsed. +pub fn resolve_wallet_base( + base_override: Option<&str>, + localappdata: Option<&str>, + home: Option<&str>, +) -> Option { + [base_override, localappdata, home] + .into_iter() + .flatten() + .find(|v| !v.trim().is_empty()) + .map(PathBuf::from) +} + +/// The base both wallet roots hang off, resolved from this process's environment. +fn wallet_base() -> PathBuf { + resolve_wallet_base( + std::env::var(WALLET_BASE_ENV).ok().as_deref(), + std::env::var("LOCALAPPDATA").ok().as_deref(), + std::env::var("HOME").ok().as_deref(), + ) + .unwrap_or_else(|| PathBuf::from(".")) +} + +/// The base a build WITHOUT [`WALLET_BASE_ENV`] would have resolved. +/// +/// Used only to answer "is there already a wallet where the previous build put one?" — see +/// [`legacy_wallet_present`]. It keeps the relative fallback precisely because that fallback is +/// where the stray wallet this guard exists to protect actually is. +fn legacy_wallet_base() -> PathBuf { + resolve_wallet_base( + None, + std::env::var("LOCALAPPDATA").ok().as_deref(), + std::env::var("HOME").ok().as_deref(), + ) + .unwrap_or_else(|| PathBuf::from(".")) +} + +/// The seed file a build WITHOUT [`WALLET_BASE_ENV`] would have opened. +pub fn legacy_seed_path() -> PathBuf { + legacy_wallet_base().join("DigWallet").join("seed.bin") +} + +/// Whether a wallet already exists where a pre-override build would have put one. +/// +/// The node's own operator wallet holds real funds, so relocating the base out from under an +/// existing one would strand it: the node would mint a fresh empty wallet and the funded file +/// would sit unreferenced. Answers `true` when presence cannot be determined at all, the same +/// fail-closed direction as [`wallet_exists`] and for the same reason — the unknown case must not +/// be the one that abandons a wallet. +pub fn legacy_wallet_present() -> bool { + !matches!( + autoseed::presence(&legacy_seed_path()), + Ok(autoseed::Presence::Absent) + ) +} + /// Path to the encrypted seed file (per-user, off the profile dir). fn seed_path() -> PathBuf { - let base = std::env::var("LOCALAPPDATA") - .or_else(|_| std::env::var("HOME")) - .unwrap_or_else(|_| ".".to_string()); - PathBuf::from(base).join("DigWallet").join("seed.bin") + wallet_base().join("DigWallet").join("seed.bin") } /// Path to the persisted dapp allow-list (next to the seed file). @@ -1104,6 +1179,93 @@ mod tests { /// the `.await`s in these async tests. Held for the whole body of each such test. static ENV_LOCK: Mutex<()> = Mutex::const_new(()); + // -- Where the wallet base comes from (#491) --------------------------------------------- + // + // These assert on the RESOLVER, not on a file that happens to appear: the bug is that a base + // resolves to something relative, and a fixture that hands the resolver a directory cannot + // exhibit it. The inputs are the ones a systemd system unit really presents - no + // `LOCALAPPDATA`, no `HOME` - because that unit sets no `User=`, so systemd sets no `$HOME`, + // and no `WorkingDirectory=`, so the working directory is `/`. + + #[test] + fn a_service_environment_with_no_home_resolves_no_base_at_all() { + // The defect, stated as a property: with nothing to resolve from, the answer is the + // ABSENCE of a base. The old chain answered `"."` here, which joined to + // `/DigWallet/seed.bin` on a stock `.deb` service - a wallet at the filesystem root, + // written successfully, because that unit runs as root under `ProtectSystem=full`, which + // leaves `/` writable. + assert_eq!(resolve_wallet_base(None, None, None), None); + } + + #[test] + fn an_empty_value_is_treated_as_unset_at_every_rung() { + // An exported-but-empty variable is how a unit file most often "sets" something by + // accident. Falling through to the next rung matches how the node parses its other + // overrides; treating it as a base would root the wallet at the working directory again. + assert_eq!( + resolve_wallet_base(Some(" "), Some(""), Some("/home/op")), + Some(PathBuf::from("/home/op")) + ); + assert_eq!(resolve_wallet_base(Some(""), Some(""), Some("")), None); + } + + #[test] + fn the_override_outranks_both_platform_variables() { + assert_eq!( + resolve_wallet_base( + Some("/var/lib/dig-node"), + Some("C:/Users/op/AppData/Local"), + Some("/home/op") + ), + Some(PathBuf::from("/var/lib/dig-node")) + ); + } + + #[test] + fn localappdata_still_outranks_home_when_no_override_is_set() { + // The Windows ordering is unchanged. Stated explicitly because a LocalSystem service DOES + // get `LOCALAPPDATA` (the systemprofile path), so this rung is what existing Windows + // installs already resolved through - and what must keep resolving for them. + assert_eq!( + resolve_wallet_base( + None, + Some("C:/Windows/system32/config/systemprofile/AppData/Local"), + Some("/home/op") + ), + Some(PathBuf::from( + "C:/Windows/system32/config/systemprofile/AppData/Local" + )) + ); + } + + /// **Proves:** the ONE override moves BOTH roots together. + /// + /// This is why the override names a BASE and not a wallet directory. `autoseed`'s + /// `the_device_key_never_shares_the_wallet_directory` proves the two roots are siblings for a + /// given base; this proves there is no way to give them DIFFERENT bases, which is the failure + /// a per-directory override would have introduced. A test that moved only the seed and checked + /// only the seed would pass under exactly that broken shape. + #[tokio::test] + async fn one_base_override_moves_the_seed_and_the_device_key_together() { + let _g = ENV_LOCK.lock().await; + let td = tempfile::tempdir().unwrap(); + std::env::set_var(WALLET_BASE_ENV, td.path()); + + let paths = autoseed::default_paths(); + assert!(paths.seed.starts_with(td.path()), "seed under the base"); + assert!( + paths.device_key.starts_with(td.path()), + "device key under the SAME base" + ); + assert_eq!(paths.seed.parent().unwrap(), td.path().join("DigWallet")); + assert_eq!( + paths.device_key.parent().unwrap(), + td.path().join("DigNode").join("device") + ); + + std::env::remove_var(WALLET_BASE_ENV); + } + #[test] fn cache_cap_is_floored_so_caching_cant_be_disabled() { // A 0 / tiny request must not disable the cache (which would defeat From b32c096a6c48dfda131238c786057576018458ca Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Wed, 2 Sep 2026 03:47:24 -0700 Subject: [PATCH 3/4] test(wallet): assert the fail-closed direction of legacy_wallet_present N1 from the #499 review: the safety-critical half of the change had no test. An unreadable legacy seed path must count as PRESENT, never as absent, or a service start re-anchors the wallet base away from a funded wallet and mints an empty one beside it. Split the mapping out as presence_counts_as_present so the property is assertable without a platform-specific unreadable-path fixture - the portable NUL-in-the-path fixture cannot be delivered through the environment on Windows, because set_var rejects an interior NUL. The split also makes a refactor to Path::exists() a compile error rather than a silent regression. Co-Authored-By: Claude --- crates/dig-wallet/src/lib.rs | 46 ++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/crates/dig-wallet/src/lib.rs b/crates/dig-wallet/src/lib.rs index 9c85b51c..959c5f18 100644 --- a/crates/dig-wallet/src/lib.rs +++ b/crates/dig-wallet/src/lib.rs @@ -238,10 +238,18 @@ pub fn legacy_seed_path() -> PathBuf { /// fail-closed direction as [`wallet_exists`] and for the same reason — the unknown case must not /// be the one that abandons a wallet. pub fn legacy_wallet_present() -> bool { - !matches!( - autoseed::presence(&legacy_seed_path()), - Ok(autoseed::Presence::Absent) - ) + presence_counts_as_present(autoseed::presence(&legacy_seed_path())) +} + +/// The fail-closed reading of a presence probe: only a definite `Absent` counts as "no wallet". +/// +/// Split out from [`legacy_wallet_present`] so the safety-critical direction is assertable without +/// a platform-specific unreadable-path fixture. It also makes the property structural: a refactor +/// to `Path::exists()` — which collapses every metadata error to `false` — cannot keep this +/// function's `io::Result` argument, so it fails to compile rather than silently stranding a +/// funded wallet. +fn presence_counts_as_present(probe: std::io::Result) -> bool { + !matches!(probe, Ok(autoseed::Presence::Absent)) } /// Path to the encrypted seed file (per-user, off the profile dir). @@ -1230,6 +1238,36 @@ mod tests { ); } + /// **Proves:** an UNREADABLE legacy seed path answers PRESENT, never absent. + /// + /// This is the safety-critical direction of the #491 guard: `legacy_wallet_present` decides + /// whether a service run may re-anchor the wallet base, so an absent answer for a path the + /// process merely could not read would relocate the base out from under a FUNDED wallet and + /// mint a fresh empty one beside it. A later refactor to `Path::exists()` - which collapses + /// every metadata error to `false` - would be green without this assertion. + /// + /// Asserted on the mapping rather than through a real unreadable path, because the portable + /// unreadable fixture (a base containing an interior NUL) cannot be delivered through the + /// environment on Windows: `set_var` rejects it outright. + #[test] + fn an_unreadable_legacy_seed_path_counts_as_present() { + // The control: only a definite Absent may answer "no wallet here". + assert!(!presence_counts_as_present(Ok(autoseed::Presence::Absent))); + assert!(presence_counts_as_present(Ok(autoseed::Presence::Present))); + + // The property under test: an unanswerable existence question must count as PRESENT. + for kind in [ + std::io::ErrorKind::PermissionDenied, + std::io::ErrorKind::InvalidInput, + std::io::ErrorKind::Other, + ] { + assert!( + presence_counts_as_present(Err(std::io::Error::from(kind))), + "an unreadable seed path ({kind:?}) must never be read as an absent wallet" + ); + } + } + #[test] fn the_override_outranks_both_platform_variables() { assert_eq!( From 5ada6d235b693572c1f9b82707b53adf1ef589ee Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Wed, 2 Sep 2026 03:47:28 -0700 Subject: [PATCH 4/4] docs(wallet): make the wallet_env cross-reference a checked link, not a code span MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `crate::wallet_env` reference in `service_data_dir_overrides`' doc was a plain code span naming a module that did not exist on this branch at the time it was written. The module arrived with #489 and is now merged in, so the reference resolves — but a code span is not checked by anything, which is how it came to name a non-existent path in the first place. Upgraded to an intra-doc link, so rustdoc fails if the module is renamed or moved rather than the reference going quietly stale. The one sentence of the argument being distinguished is inlined alongside it, so a reader learns what the paragraph is contrasting without leaving the page. Doc-only; no behaviour change. Co-Authored-By: Claude --- crates/dig-node-service/src/state.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/dig-node-service/src/state.rs b/crates/dig-node-service/src/state.rs index 0c6969d6..dd2e32df 100644 --- a/crates/dig-node-service/src/state.rs +++ b/crates/dig-node-service/src/state.rs @@ -216,9 +216,12 @@ pub use dig_wallet::WALLET_BASE_ENV; /// path. Existing hosts therefore keep their old base; only a host with no wallet yet adopts the /// anchored one, and moving key material is left to a deliberate operator-run migration. /// -/// This is NOT the "derive the wallet base from the node's cache dir" change `crate::wallet_env` -/// argues against, and the difference is the whole design: this is an independent explicit base, -/// adopted only where there is nothing to orphan. +/// This is NOT the "derive the wallet base from the node's cache dir" change +/// [`crate::wallet_env`] argues against. That one would re-root the seed onto a cache directory +/// this function has ALREADY moved, so it would find nothing there and mint a fresh wallet on +/// every existing install. The difference is the whole design: this is an independent explicit +/// base, adopted only where there is nothing to orphan. An intra-doc link rather than a code +/// span, so rustdoc fails if the module the argument lives in ever moves. /// /// An override the operator set explicitly is never replaced — their choice outranks ours. pub fn service_data_dir_overrides(