fix(wallet): anchor the service wallet seed instead of resolving it relative to cwd (#491) - #499
Conversation
Salvage anchor for the wallet-seed-path lane. Co-Authored-By: Claude <noreply@anthropic.com>
Sequencing constraint against #489 — read before resuming this lanePR #489 ( 1. Textual overlap. #489 edits 2. #489 argues against re-rooting the wallet, and it is right about the hazard but not about this fix. Its new That hazard is real and is exactly why the do-not-orphan guard on this PR is load-bearing. But the thing it argues against is deriving the wallet base from the node's cache dir. This PR does not do that: it adds an independent explicit base override, anchored to the machine state dir, adopted only when no wallet already exists at the legacy path. Do not let the module docs relitigate the shape. 3. A Windows fact that constrains the guard. On Windows Also from #489: the service anchor is conditional on 4. A new interaction this PR must not create silently. Once the wallet base is anchored, a stock Linux service has |
…ng 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 <noreply@anthropic.com>
loop-security — IN PROGRESS, not the verdictAuditing head 48e138f (resolved from Diff scope confirmed: 6 files, +342/-26 — Method note, stated up front: gitnexus's dig-node index is stale and returns a false-safe Interim findings so far (none gating yet):
Still to do: presence/TOCTOU on |
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
CHANGES-REQUIRED — independent correctness gate
Head audited: 48e138fba0c190b2eea25c3791051e4f018d2fad (matches gh pr view --json headRefOid). Blast radius by grep + direct read, NOT gitnexus: this repo's registered index is stale and impact returns a false-safe impactedCount: 0.
Test counts I observed myself
| crate | result |
|---|---|
cargo test -p dig-wallet --lib |
768 passed; 0 failed; 1 ignored; 0 filtered out |
cargo test -p dig-node-service --lib |
754 passed; 0 failed; 0 ignored; 0 filtered out |
Both match the implementer's report, and 0 filtered out on both, so neither is a filter-matched-nothing green.
What is right, verified
- The systemd doc claims are TRUE.
packaging/linux/systemd/net.dignetwork.dig-node.servicedeclares noUser=and noWorkingDirectory=, setsProtectSystem=full, and runs as root.ProtectSystem=fullremounts/usr,/boot,/efi,/etcread-only and leaves/writable, so/DigWallet/seed.binis a successful write. None of the new prose inlib.rs,state.rsorSPEC.mdis born false on this point. - The do-not-orphan guard survives adversarial reading.
legacy_wallet_present()is evaluated atstate.rs:263, before theset_varloop; and independently of that ordering,legacy_wallet_base()passesNonefor the override, so the legacy answer is override-INDEPENDENT even if the ordering were reversed by a later refactor. Undeterminable presence answers PRESENT. - Nothing reads a wallet path before the anchor.
anchor_service_data_dirs()is the only call site (entrypoint.rs:857), inrun()beforeConfig::from_env()and before dispatch. The Windows SCM path reacheswin_service::runonly viaCommand::RunService, i.e. through the samerun(), so the SCM entry is covered. - The Windows arm is a pure case runnable on the Linux runners. CI is
ubuntu-lateston all jobs, so this matters. - The sibling boundary holds.
autoseed::user_base()now delegates tocrate::wallet_base(), so seed and device key cannot resolve from different bases; the new test asserts both parents (DigWallet,DigNode/device), not merely that the seed moved. - No key material anywhere. The new
warn!carries paths only. NoDebug/Display/Serializeadded toDeviceKey. - Section 908 is not blurred. Every new sentence says "the node's OPERATOR wallet" and ties it to mirror-coin collateral.
- SPEC.md is accurate, including the do-not-orphan condition and "presence that cannot be DETERMINED MUST count as present".
Deferrals — both judged SOUND
- The
"."fallback. Radius measured independently:seed_path()is crate-private; reached viaautoseed::default_paths()atcontrol.rs:2364,server.rs:2723,wallet_bootstrap.rs:27,dig-wallet/examples/operator_address.rs:8, and viaseed_export::default_seed_pathatseed_export_cli.rs:51. Failing closed changes every CLI run, not the service run this ticket is about. Deferring is correct. The residue is real and belongs on the issue: a NON-service run with neither variable still creates./DigWallet/seed.bin. - PR #489 sequencing — see GATING-2. The premise has moved: #489 is MERGED (
f1170d0), so "sequence after #489" now means "in this PR, on the rebase".
GATING findings
GATING-1 — the branch is behind main, and the rebase lands on the exact function #489 made public
origin/main is f1170d0 (#489); git merge-base --is-ancestor origin/main 48e138f fails. #489 changed dig_wallet::autoseed::user_base from private to pub, and dig_node_service::wallet_env::wallet_root_split() now calls it. This PR rewrites that same function's body and doc while it is still private on this branch, so the rebase conflicts there and the merged result MUST keep pub or dig-node-service will not compile.
The fix must NOT resolve that conflict by taking the branch side wholesale — that silently drops pub and the #392 rationale.
GATING-2 — this change makes #489's already-merged ambient-split announcement false on every stock Linux start
After the rebase, on a stock .deb service with no existing wallet: this PR sets DIG_WALLET_BASE=/var/lib/dig-node, so autoseed::user_base() resolves there, while dig_node_core::platform_user_base() still resolves /root via getpwuid. wallet_env::split_of therefore returns Some(WalletRootSplit { cause: Ambient }) on every start, and wallet_bootstrap logs AMBIENT_SPLIT_ROOTS.
That constant now asserts two things that are false for the case this PR creates:
- "the wallet's env-first resolver fell back to the working directory" — it did not; it resolved the anchored state dir.
- "set HOME in the service environment if you want the two halves to share one root" —
DIG_WALLET_BASEoutranksHOMEin the new chain, so following that advice changes nothing.
wallet_env.rs's module doc also still describes the resolver as "ENV-FIRST (LOCALAPPDATA, then HOME, then .)", which is no longer the chain.
This is a contract split, not a doc nit: it is prose an operator is told to act on, and it ships false in the commit that makes it false.
I confirmed it is NOT a functional regression, and the fix must not over-correct on that basis. The cause is Ambient, so mint_decision returns Proceed. A fresh Windows service install also classifies Ambient — the anchor moves wallet_base to %PROGRAMDATA%\DigNode while LOCALAPPDATA still equals node_base, so overridden is false — hence no RefuseSplitRoot regression and no wallet-less Windows install.
The fix must NOT delete or suppress the ambient announcement, and must NOT make the anchor conditional on the split — either trades an honest-log defect for the silence #489 exists to remove. The minimal correct change: name DIG_WALLET_BASE in wallet_env's resolver chain, and either add a distinct announcement for "the service anchored the wallet base" or amend AMBIENT_SPLIT_ROOTS so neither false clause survives. the_announcements_have_no_lost_string_continuation must stay green.
Non-gating (self-resolved; these must not bar merge)
- N1 —
legacy_wallet_present()has no test at all. It is the safety-critical half of this PR and its fail-closed direction is asserted nowhere.autoseed'san_unreadable_seed_path_is_an_error_not_an_absenceis the pattern to copy. - N2 — one cited regression test cannot discriminate, and that is acceptable here.
a_service_environment_with_no_home_resolves_no_base_at_allasserts a property of a function that did not exist before this PR, so it cannot fail on pre-fix code. The genuinely discriminating tests are instate.rs: delete!legacy_wallet_presentanda_service_that_already_has_a_wallet_keeps_opening_the_old_onegoes red; delete the push anda_service_with_no_wallet_yet_anchors_the_wallet_base_at_the_state_dirgoes red. The PR body should say that, rather than implying the resolver test is the regression proof. - N3 —
crate::wallet_envatstate.rs:219names a module absent from this branch. Resolves on the rebase; noted so nobody "fixes" it by deleting the sentence. - N4 — name collision.
dig_wallet::legacy_seed_path(this PR) anddig_wallet::sage::custody::legacy_seed_path(custody.rs:316) mean different things — the pre-#491 operator seed vs the Sagewallet-seed.bin.
Readable-code: the diff reads well — pure decision core with explicitly-supplied inputs, thin env-reading wrappers, WHY-comments naming the failure each guard prevents, doc-comments on every new public item, no cruft. Only readability note is N4.
Resume point: #489 MERGED mid-lane, branch diverged. Exact merge plan.The sequencing note above is superseded. PR #489 merged at 2026-09-02T04:07Z as
Use Conflict 1 —
|
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
loop-security — PASS (with two non-gating advisories)
Audited head 48e138f, resolved from gh pr view 499 --json headRefOid. Diff read as git diff origin/main...48e138f; no writes to the lane's worktree, no mutating git command anywhere.
Method limit, stated up front: gitnexus's dig-node index is stale and returns a false-safe impactedCount: 0 for symbols with real callers, so I used grep plus direct reads, not impact. I did not compile or run the suite — that is the correctness gate's leg. Every claim below is from reading the code at this SHA.
Area-by-area
1. DIG_WALLET_BASE as a key-material redirection primitive — CLEAR, identical to shipped precedent
DIG_WALLET_BASE is read from process environment only (crates/dig-wallet/src/lib.rs:201-208). A repo-wide grep for DIG_WALLET_BASE / WALLET_BASE_ENV returns fifteen hits, all of them the definition, the resolver, doc prose, SPEC prose, or tests — it is not settable via the control RPC, a config file, peer input, or any operator-facing API surface. The only shipped ways to set it are the unit's Environment= line and EnvironmentFile=-/etc/dig-node/dig-node.env.
Permissions on that file, as shipped: the env dir is installed mode 0755 and dig-node.env mode 0644 (packaging/linux/build-deb.sh:30,45), both root-owned via dpkg. Only root can set it.
A root who can write that file can already set DIG_IDENTITY_DIR — which relocates the node's identity seed, key material — and DIG_NODE_CACHE, and can set arbitrary environment for a root process anyway. DIG_WALLET_BASE is emitted by the same function, under the same operator-wins rule, on the same env surface. It grants an attacker nothing they did not already have, and the risk is identical to shipped precedent. Saying so plainly rather than inventing a severity.
Pre-created attacker-owned directory at a chosen base: reaching it requires setting the variable, which requires root on a service that already runs as root. Not a privilege escalation.
2. Directory and file permissions along the NEW path — net improvement, no regression found
| old (stock Linux service) | new | |
|---|---|---|
| seed | /DigWallet/seed.bin |
/var/lib/dig-node/DigWallet/seed.bin |
| device key | /DigNode/device/device.key |
/var/lib/dig-node/DigNode/device/device.key |
| parent mode | / is 0755 — listable and traversable by every local user |
0700 root:root |
/var/lib/dig-nodeis created mode 0700 by the postinst (packaging/linux/build-deb.sh:114);ensure_dir_restrictedsets 0700 on fresh create (crates/dig-node-service/src/state.rs:457-474);harden_unix_direnforces 0700, reads it back, and fails closed by removing the dir if it cannot (state.rs:827-843).- Both secret FILES are 0600 at
opentime either way — the mode is set on theOpenOptionsbeforeopen, not by a later chmod (crates/dig-node-core/src/shared/at_rest.rs:100-105). So contents were never world-readable on the old path. What WAS exposed there is the existence, size and mtime of the node's operator wallet, to every local user. Under a 0700 parent it no longer is. DigWallet/andDigNode/device/are themselves created byfs::create_dir_allwith no explicit mode (at_rest.rs:95-97), so 0755 under the systemd default UMask=0022. Unchanged by this diff, and it was only an exposure on the old path because the grandparent was/. Under a 0700 parent it is inert.- Windows: for a fresh install the base moves from LocalSystem's
%LOCALAPPDATA%to the machine state dir under%PROGRAMDATA%, a directory BUILTIN\Users can create subfolders under by default — SPEC 7.3a names it as the hard case. Three things make that not a regression I can demonstrate: it is the same directory that already holds the control token (full local control), it is hardened and readback-verified by the 501 chokepoint (state.rs:374-388, hardened at install time byservice.rs:1593), and the seed and device-key files get their own explicit protected owner-only DACL independent of the directory ACL (crates/dig-wallet/src/autoseed.rs:525-529). Existing Windows installs are not moved at all, by the guard.
3. The do-not-orphan guard — I tried to defeat it and could not, on any shipped path
- Pinning the service to an attacker-chosen location, or steering it at an attacker-known seed, requires planting a file at the legacy path. On the shipped Linux service that is
/DigWallet/seed.bin— cwd/, mode 0755 root — so root-only. On Windows it is LocalSystem's%LOCALAPPDATA%under the systemprofile tree, SYSTEM/Administrators only. In both cases the attacker already holds the service's own privilege or better. Not an escalation. - TOCTOU between the check (
state.rs:263) and the first wallet open (crates/dig-wallet/src/lib.rs:245, resolved from the env the anchor wrote): a file appearing in that window arrives after the anchor has already been adopted, so the planted file is simply ignored. The window fails in the safe direction. The dangerous order — plant, then start — is the root-only case above. - Unreadable-but-present is handled in the safe direction, and this is load-bearing.
presence()usesPath::try_exists, which returnsErrand notOk(false)when existence cannot be determined (crates/dig-node-core/src/shared/at_rest.rs:45-51), andlegacy_wallet_present()is!matches!(..., Ok(Presence::Absent))(crates/dig-wallet/src/lib.rs:230-236), so an error counts as present. The unknown case cannot strand a funded wallet. - Can the check answer differently from the path the wallet later opens? Yes, in exactly one configuration — see finding A. The decision is still correct there (
!wallet_base_set && !legacy_wallet_present,state.rs:242); only the log line is wrong. - Ordering verified.
anchor_service_data_dirs()runs atcrates/dig-node-service/src/entrypoint.rs:857— after arg parsing, before the tokio runtime is built, and beforewallet_bootstrap::ensure_wallet_seed()atentrypoint.rs:1425. The Windows SCM path is covered becausedig-node installwrites the service run-context into the service environment (service.rs:871), sorunning_as_service()is already true at line 857 and the anchor is live beforewin_service.rs:126.std::env::set_varis still single-threaded at that point.
4. Key material never logged, written or formatted — CLEAR
- The one new log is
state.rs:266-271. Two fields, the legacy seed path and the state dir. Both arePath::display(); neither file is read. DeviceKeyis untouched by the diff and still carries noDebug,DisplayorSerialize(crates/dig-wallet/src/autoseed.rs:140-144).Zeroizingusage is untouched —hex(),as_password()andpassword_strare all unchanged.- No new error type carries bytes:
legacy_wallet_present()returnsbool,legacy_seed_path()returns aPathBuf, and neither reads file content. Diff-wide there is no new derive and no debug-format of any secret-bearing type.
5. The partial-exfiltration boundary — structurally preserved, and this is the strongest part of the change
autoseed::user_base() no longer re-derives the chain; it delegates to crate::wallet_base() (crates/dig-wallet/src/autoseed.rs:113-116). The duplicate is deleted, not kept in sync — the correct fix for exactly the hazard the module docs describe.
Counted on this SHA:
- production joins of the wallet dir name: two —
legacy_seed_path()(lib.rs:226) andseed_path()(lib.rs:245) - production join of the device dir: one —
autoseed.rs:88-92 - callers of
user_base(): one —autoseed.rs:89
All hang off the same wallet_base(). There is no per-directory override anywhere; WALLET_BASE_ENV is the only new variable and it names the base. No configuration reachable by an operator can place the device key inside the wallet directory — doing so would need two independent values, and only one exists.
6. Boundary 908 — CLEAR
The diff adds no signing, no key import and no user-key path. It relocates the node's OWN operator wallet only. Both the SPEC addition and the code comments say "the node's operator wallet" explicitly, and nothing blurs machine custody with user custody.
7. Back-compat 5.1 — CLEAR
Pure path resolution. seed_store and the sealed-container format are untouched; no serialization, section id, field meaning or encoding changed. No older artifact becomes unreadable — and the do-not-orphan guard means an existing artifact is not even moved.
Findings
A. LOW (non-gating) — the do-not-orphan warning states the wrong seed location when the operator has set DIG_WALLET_BASE
crates/dig-node-service/src/state.rs:264-272
The warn fires on legacy_wallet alone, but the decision it narrates also depends on wallet_base_set (state.rs:242).
State, action, impact:
- Operator sets
DIG_WALLET_BASE=/srv/dig-walletin/etc/dig-node/dig-node.env. - The host also carries a stale
/DigWallet/seed.binfrom a pre-fix build. - On start the node logs
seed=/DigWallet/seed.bintogether with "this service keeps opening it rather than the machine state dir" — whilewallet_base()resolves/srv/dig-walletand the node actually opens/srv/dig-wallet/DigWallet/seed.bin.
No attacker input and no privilege gained, so this is not a vulnerability and I am not gating on it. But it is a false statement about where live key material is, on a path that holds real $DIG, and the action it invites is an operator migrating or deleting the wrong file. One condition fixes it: gate the warn on !env_is_set(WALLET_BASE_ENV), or reword it to claim only that a legacy wallet exists.
B. INFO (non-gating) — two documentation/parsing nits
crates/dig-node-service/src/state.rs:219referencescrate::wallet_env, which does not exist in this tree — there is nocrates/dig-node-service/src/wallet_env.rsat this SHA. It arrives with PR #489. It sits in a code span rather than an intra-doc link, so rustdoc will not fail — but a reader on this branch cannot follow it, and it becomes permanently false if #489 does not land.crates/dig-wallet/src/lib.rs:196-197:resolve_wallet_basetrims for the is-it-set test but returns the untrimmed value, so a padded value such as a leading space yields a relative path. Operator typo only, and on a service run it resolves under a root-only cwd, so it fails safe. The existing test covers whitespace-only, not whitespace-padded.
Verdict
PASS. No live vulnerability in this diff.
The change strictly improves the at-rest posture on the shipped Linux service path — a 0755-parented location directly under / becomes 0700 root:root. The one new env var is the same redirection shape as two already shipped beside it in the same function, and is root-only to set. The do-not-orphan guard fails closed in every direction I could push it, and planting a file to defeat it requires privilege the attacker would have to already hold. The sibling boundary between the wallet dir and the device-key dir is now enforced structurally rather than by convention, because the second copy of the resolver is deleted. No key material reaches a log, a format, or a serializer.
Finding A is worth fixing in this PR because it is one condition and it concerns a money-path claim; it is not a merge gate. Finding B is a follow-up ticket at most.
…e wallet-env prose Merge (never rebase) of origin/main f1170d0 into loop/491-wallet-seed-path. Conflicts resolved: - Cargo.toml / Cargo.lock: keep 0.241.0, which is ahead of main's 0.239.0. - dig-wallet autoseed::user_base: keep main's `pub` visibility, which dig_node_service::wallet_env needs to compile, together with this branch's delegation to crate::wallet_base. Both doc rationales are load-bearing and are merged; "The body is unchanged" is deleted because the merge makes it false. Prose corrections in dig-node-service::wallet_env, which arrives with the merge. Once the wallet base is anchored, a stock .deb service resolves wallet_base under the state dir and node_base under the account home, so split_of returns an Ambient split on every start. Ambient means Proceed, so this is not a functional regression - only the text describing it had become false: - the module doc's resolver chain now names DIG_WALLET_BASE first; - AMBIENT_SPLIT_ROOTS no longer claims the resolver "fell back to the working directory", and no longer prescribes setting HOME, which DIG_WALLET_BASE outranks; - the ambient test fixture and its doc use an anchored wallet base. The concat! form is preserved deliberately, and the contains guards are strengthened to name DIG_WALLET_BASE and to reject the inert "set HOME" remedy, so a rewording cannot keep them green while saying something else. Security findings from the gate round: - state.rs: the legacy-wallet warning fired on legacy_wallet alone while the decision it narrates also depends on DIG_WALLET_BASE, so with an operator-set base and a stale seed it named the legacy path as the file the service "keeps opening" when it opens the operator's base instead. Gated on the same conjunction; a pure test pins the state it mis-narrated. - dig-wallet resolve_wallet_base returned the untrimmed value, so a padded DIG_WALLET_BASE yielded a path with leading or trailing spaces. It now returns the trimmed value, with a whitespace-PADDED test beside the whitespace-only one. Co-Authored-By: Claude <noreply@anthropic.com>
Merge (never rebase) of origin/main adf03d8 into loop/491-wallet-seed-path. Only conflict is the version in Cargo.toml / Cargo.lock; resolved to 0.248.0, which is above main's 0.240.0 and above this branch's prior 0.241.0. Co-Authored-By: Claude <noreply@anthropic.com>
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 <noreply@anthropic.com>
origin/main advanced to 33ab082 (#497, the peer-local refusal fix). Only the workspace version and Cargo.lock conflicted; the wallet-base change touches no file that commit does. The pre-assigned 0.248.0 is kept and the lock re-resolved from main's, so dig-node-service tracks 0.248.0 rather than main's 0.245.0. Co-Authored-By: Claude <noreply@anthropic.com>
|
Lane progress — head
Verifying next: |
…e wallet-env prose Merge (never rebase) of origin/main f1170d0 into loop/491-wallet-seed-path. Conflicts resolved: - Cargo.toml / Cargo.lock: keep 0.241.0, which is ahead of main's 0.239.0. - dig-wallet autoseed::user_base: keep main's `pub` visibility, which dig_node_service::wallet_env needs to compile, together with this branch's delegation to crate::wallet_base. Both doc rationales are load-bearing and are merged; "The body is unchanged" is deleted because the merge makes it false. Prose corrections in dig-node-service::wallet_env, which arrives with the merge. Once the wallet base is anchored, a stock .deb service resolves wallet_base under the state dir and node_base under the account home, so split_of returns an Ambient split on every start. Ambient means Proceed, so this is not a functional regression - only the text describing it had become false: - the module doc's resolver chain now names DIG_WALLET_BASE first; - AMBIENT_SPLIT_ROOTS no longer claims the resolver "fell back to the working directory", and no longer prescribes setting HOME, which DIG_WALLET_BASE outranks; - the ambient test fixture and its doc use an anchored wallet base. The concat! form is preserved deliberately, and the contains guards are strengthened to name DIG_WALLET_BASE and to reject the inert "set HOME" remedy, so a rewording cannot keep them green while saying something else. Security findings from the gate round: - state.rs: the legacy-wallet warning fired on legacy_wallet alone while the decision it narrates also depends on DIG_WALLET_BASE, so with an operator-set base and a stale seed it named the legacy path as the file the service "keeps opening" when it opens the operator's base instead. Gated on the same conjunction; a pure test pins the state it mis-narrated. - dig-wallet resolve_wallet_base returned the untrimmed value, so a padded DIG_WALLET_BASE yielded a path with leading or trailing spaces. It now returns the trimmed value, with a whitespace-PADDED test beside the whitespace-only one. Co-Authored-By: Claude <noreply@anthropic.com>
Merge (never rebase) of origin/main adf03d8 into loop/491-wallet-seed-path. Only conflict is the version in Cargo.toml / Cargo.lock; resolved to 0.248.0, which is above main's 0.240.0 and above this branch's prior 0.241.0. Co-Authored-By: Claude <noreply@anthropic.com>
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 <noreply@anthropic.com>
origin/main advanced to 33ab082 (#497, the peer-local refusal fix). Only the workspace version and Cargo.lock conflicted; the wallet-base change touches no file that commit does. The pre-assigned 0.248.0 is kept and the lock re-resolved from main's, so dig-node-service tracks 0.248.0 rather than main's 0.245.0. Co-Authored-By: Claude <noreply@anthropic.com>
… a code span 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 <noreply@anthropic.com>
3f26616 to
5ada6d2
Compare
|
All six threads replied to and resolved — Suites at this tree (counts, not just exit codes):
The dig-node-service run is also the compile proof GATING-1 asked for: the crate cannot build against a private One thing the gate round did not catch, found by the merge-precondition check:
Rewrote all three merge subjects to Production code is untouched by this push: the only non-merge change since the gate round is the doc-link commit in Next: watch |
origin/main advanced to b9979d8 (#494, the shell error catalogue). Only the workspace version and Cargo.lock conflicted; #494 touches no file this branch does. The pre-assigned 0.248.0 is kept and the lock re-resolved from main's. Subject is `chore:` and not `merge:` deliberately: `merge` is not in this repo's commitlint type-enum, and the three earlier merge commits on this branch were rewritten for the same reason. Co-Authored-By: Claude <noreply@anthropic.com>
|
orchestrator bc2a88 / loop-reviewer scoped re-gate started on 82db50b at 2026-09-02T17:32Z. Scope: GATING-1 (autoseed base-resolution), GATING-2 (state.rs prose), N1 (fail-closed discriminating test), and re-verification of the six resolved threads. Not a full re-review. IN PROGRESS — not the verdict. First observation: |
…path # Conflicts: # Cargo.lock # Cargo.toml
loop-reviewer — scoped re-gate STARTED (IN PROGRESS — not the verdict)Fresh context, orchestrator e93b41. Head being audited: Scope of this pass (not a full re-review):
I do not merge, undraft, or edit code. Verdict posted as a separate comment when the pass returns. |
IN PROGRESS — not the verdict (item 3 + item 4)Item 3 — CLEAN. The 18:54Z merge commit Item 4 — NOT clean; the conflict RECURRED and the version is now BELOW main. So the branch's
This is a merge-precondition item, not a code finding; the correctness half of this pass continues. |
IN PROGRESS — not the verdict (items 1 and 2: all three prior findings + all eight threads re-verified at
|
loop-reviewer — VERDICT: PASS (correctness), MERGE BLOCKED on a precondition the orchestrator ownsHead audited: The change is correct and I would merge it as it stands. All three prior findings and all eight
Two things I checked that a reply-reading pass would have missed, and both came out clean: the Item 3 — the merge-main commit is CLEAN. Item 4 — NOT clean, and this is the one blocking thing. Note that What the orchestrator must do
Threads: 8 total, 8 resolved, 0 unresolved. I opened none — I found nothing at this head that Not covered by this pass, stated so it is not mistaken for cleared: this was scoped to the four I did not run the suite locally — CI's |
|
Addendum to the verdict — one correction, no change to the outcome. I did attempt a local The suite evidence in the verdict is therefore CI's |
|
orchestrator
Chain log: |
Co-Authored-By: Claude <noreply@anthropic.com>
DO NOT MERGE - gate round in progress.
Closes #491
What was wrong
dig_wallet::seed_path()resolvedLOCALAPPDATA->HOME->"."and joinedDigWallet/seed.bin.autoseed::user_base()used a byte-identical chain for the device key.On the shipped unit
packaging/linux/systemd/net.dignetwork.dig-node.service, every rung misses. Verified against systemd.exec(5) rather than inferred:$HOMEis unset. "$USERis set unconditionally, while$HOME,$LOGNAME, and$SHELLare only set for the units that haveUser=set andSetLoginEnvironment=unset or set to true", andSetLoginEnvironment="defaults to true ifUser=,DynamicUser=orPAMName=are set, false otherwise". The unit sets none of the four./. NoWorkingDirectory=; "If not set, defaults to the root directory when systemd is running as a system instance."/is writable and the service runs as root.ProtectSystem=fullmounts/usr,/boot,/efiand/etcread-only; onlystrictlocks/. The unit sets noUser=.So the base collapsed to
"."and the seed was created at/DigWallet/seed.bin, with the device key at/DigNode/device/device.key— and the write succeeded.This is case 1: created at the wrong path, silently. Not the "mint fails, node comes up wallet-less" the ticket predicted. That distinction is the reason it survived: an unwritable path fails visibly and gets investigated, while a successful write to
/leaves every install with a working wallet that nobody looks at. Both roots came from the same broken base, so seed and device key stayed consistent and the wallet opened normally.The fix
resolve_wallet_base(base_override, localappdata, home) -> Option<PathBuf>- pure, takes its inputs, and returnsNonewhen nothing resolves rather than collapsing to a relative path. Pure because the environment is process-global: an env-reading resolver can only be tested under the mutex that already serialises these tests, and the Windows arm is then unreachable on a Linux CI runner. CI here isubuntu-lateston all four jobs.DIG_WALLET_BASEnaming the BASE, never a wallet directory.DigWallet/and the device key'sDigNode/device/resolve from that single value, so they cannot be pointed at unrelated places.autoseed.rscalls that sibling relationship "the entire partial-exfiltration boundary"; a per-directory override would let an operator collapse it.state::service_data_dir_overridesthat already anchors identity and cache./var/lib/dig-nodeis created0700 root:rootby the.debpostinst.The condition on the anchor, which is the safety-critical half
An existing wallet is never re-rooted. The node's operator wallet holds real $DIG for mirror-coin collateral, so anchoring a host that already has one would leave the funded seed unreferenced and mint a fresh empty wallet beside it - turning this bug into a money loss.
legacy_wallet_present()is asked before the override is written, resolves the legacy chain ignoring the override, and treats an undeterminable answer as PRESENT - the direction that cannot strand a wallet. Windows is the live case:%LOCALAPPDATA%IS set for a LocalSystem service, so existing installs already hold a seed under the systemprofile path. Key material is never moved automatically; a deliberate migration verb is a follow-up.This is not the "derive the wallet base from the node's cache dir" change that #489's
wallet_env.rsargues against, and the difference is the whole design: an independent explicit base, adopted only where there is nothing to orphan.Tests
Red was produced against a stub preserving the old behaviour, so the failures are assertion failures (
left: Some("."), right: None;left: [], right: [("DIG_WALLET_BASE", "/var/lib/dig-node")]), not compile errors.Added:
a_service_environment_with_no_home_resolves_no_base_at_all,an_empty_value_is_treated_as_unset_at_every_rung,the_override_outranks_both_platform_variables,localappdata_still_outranks_home_when_no_override_is_set,one_base_override_moves_the_seed_and_the_device_key_together,a_service_with_no_wallet_yet_anchors_the_wallet_base_at_the_state_dir,a_service_that_already_has_a_wallet_keeps_opening_the_old_one,an_operator_set_wallet_base_outranks_the_anchor.Blast radius
gitnexus's dig-node index is stale and returns a false-safe
impactedCount: 0, so this was measured by grep and direct reads.seed_path()is crate-private with 5 call sites indig-wallet; viadefault_paths/default_seed_pathit reaches 5 production sites indig-node-serviceplus one example.sage::custody::legacy_seed_pathis a different path and is untouched.Deliberately not done here
"."fallback still exists for non-service runs. Making it fail closed changes the return type of two crate functions and the publicseed_export::default_seed_path(), cascading into 5 production call sites across 2 crates - larger than the fix, and it mixes an error-handling redesign into a path change. Named child ticket to follow.$HOME, the service does not). Real, and a money-surface concern since the CLI then reports on an empty wallet. Separate ticket.wallet_base= the state dir andnode_base=/root, so fix(wallet): say when a wallet-isolating override was not honoured #489's split detection classifies it as anAmbientsplit and would warn on every start, and itsAMBIENT_SPLIT_ROOTSprose ("fell back to the working directory") becomes false for that case. fix(wallet): say when a wallet-isolating override was not honoured #489's files are not touched here. To be sequenced.WalletPaths::resolve(seed)derivesdevice_keyfrom the env base rather than the passed seed's base, so a test handing it a tempdir seed writes a device key under the real user base. Pre-existing; ticket to follow.Gates
loop-reviewerandloop-securityrunning in fresh contexts against48e138f.