Skip to content

fix(wallet): anchor the service wallet seed instead of resolving it relative to cwd (#491) - #499

Merged
MichaelTaylor3d merged 10 commits into
mainfrom
loop/491-wallet-seed-path
Sep 2, 2026
Merged

fix(wallet): anchor the service wallet seed instead of resolving it relative to cwd (#491)#499
MichaelTaylor3d merged 10 commits into
mainfrom
loop/491-wallet-seed-path

Conversation

@MichaelTaylor3d

@MichaelTaylor3d MichaelTaylor3d commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

DO NOT MERGE - gate round in progress.

Closes #491

What was wrong

dig_wallet::seed_path() resolved LOCALAPPDATA -> HOME -> "." and joined DigWallet/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:

  • $HOME is unset. "$USER is set unconditionally, while $HOME, $LOGNAME, and $SHELL are only set for the units that have User= set and SetLoginEnvironment= unset or set to true", and SetLoginEnvironment= "defaults to true if User=, DynamicUser= or PAMName= are set, false otherwise". The unit sets none of the four.
  • The working directory is /. No WorkingDirectory=; "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=full mounts /usr, /boot, /efi and /etc read-only; only strict locks /. The unit sets no User=.

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 returns None when 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 is ubuntu-latest on all four jobs.
  • One DIG_WALLET_BASE naming the BASE, never a wallet directory. DigWallet/ and the device key's DigNode/device/ resolve from that single value, so they cannot be pointed at unrelated places. autoseed.rs calls that sibling relationship "the entire partial-exfiltration boundary"; a per-directory override would let an operator collapse it.
  • Anchored to the machine state dir for a service run, through the existing state::service_data_dir_overrides that already anchors identity and cache. /var/lib/dig-node is created 0700 root:root by the .deb postinst.

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.rs argues 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.

RED   dig-wallet:       3 passed; 2 failed; 764 filtered out
RED   dig-node-service: 36 passed; 1 failed; 717 filtered out
GREEN dig-wallet:       768 passed; 0 failed; 1 ignored; 0 filtered out
GREEN dig-node-service: 754 passed; 0 failed; 0 ignored; 0 filtered out

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 in dig-wallet; via default_paths/default_seed_path it reaches 5 production sites in dig-node-service plus one example. sage::custody::legacy_seed_path is a different path and is untouched.

Deliberately not done here

  • The "." fallback still exists for non-service runs. Making it fail closed changes the return type of two crate functions and the public seed_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.
  • The CLI and the service still resolve different wallets on Linux (the CLI has $HOME, the service does not). Real, and a money-surface concern since the CLI then reports on an empty wallet. Separate ticket.
  • Interaction with fix(wallet): say when a wallet-isolating override was not honoured #489, unresolved by design. Once this anchors, a stock Linux service has wallet_base = the state dir and node_base = /root, so fix(wallet): say when a wallet-isolating override was not honoured #489's split detection classifies it as an Ambient split and would warn on every start, and its AMBIENT_SPLIT_ROOTS prose ("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) derives device_key from 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-reviewer and loop-security running in fresh contexts against 48e138f.

Salvage anchor for the wallet-seed-path lane. Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Sequencing constraint against #489 — read before resuming this lane

PR #489 (fix(wallet): say when a wallet-isolating override was not honoured, the #392 fix) is ready, mergeable, and will very likely land before this PR. It matters here in three ways.

1. Textual overlap. #489 edits crates/dig-wallet/src/autoseed.rs, which this PR also edits, plus Cargo.toml / Cargo.lock / SPEC.md. When it lands, reconcile with git merge origin/main, never a rebase — a rebase whose patch already merged prints dropping <sha> ... patch contents already upstream, exits 0, and silently takes the version bump with it.

2. #489 argues against re-rooting the wallet, and it is right about the hazard but not about this fix. Its new crates/dig-node-service/src/wallet_env.rs states: "Making either resolver defer to the other is the obvious fix and it is the destructive one... deriving the wallet base from the node's cache dir would move the seed off ...systemprofile\AppData\Local\DigWallet\seed.bin, find nothing there, and mint a FRESH wallet on every existing install - orphaning the operator wallet and any $DIG in it."

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 LOCALAPPDATA is set for a LocalSystem service, so existing Windows service installs already hold a seed at ...systemprofile\AppData\Local\DigWallet\seed.bin, and those wallets may hold $DIG. The fallback-chain miss described on #491 is a Linux phenomenon. The guard must be evaluated against the legacy resolved path on the actual platform, with the platform branch as a pure-function parameter so the Windows arm runs on the Linux-only CI runners.

Also from #489: the service anchor is conditional on DIG_NODE_RUN_CONTEXT, and a Windows service whose registered environment lacks it does not anchor at all. The anchor does not fire everywhere.

4. A new interaction this PR must not create silently. Once the wallet base is anchored, a stock Linux service has wallet_base = the state dir while node_base stays /root (the getpwuid_r fallback), so #489's split detection would classify every stock Linux install as a split and warn on every start, forever. Either the anchored case is recognised as intentional and not reported as a split, or the split prose is made accurate for it. This is tracked as part of this lane, not deferred.

…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>
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

loop-security — IN PROGRESS, not the verdict

Auditing head 48e138f (resolved from gh pr view 499 --json headRefOid, matches the dispatch brief). Read-only; no writes to the lane's worktree.

Diff scope confirmed: 6 files, +342/-26 — crates/dig-wallet/src/lib.rs, crates/dig-wallet/src/autoseed.rs, crates/dig-node-service/src/state.rs, SPEC.md, Cargo.toml, Cargo.lock (version 0.236.0 -> 0.241.0, already bumped before the lane).

Method note, stated up front: gitnexus's dig-node index is stale and returns a false-safe impactedCount: 0, so every blast-radius claim below is from grep + direct reads, not from impact.

Interim findings so far (none gating yet):

  1. No key material is added to any log, format or serialize path in the diff. The one new tracing::warn! at crates/dig-node-service/src/state.rs:264-270 emits two fields, seed = %dig_wallet::legacy_seed_path().display() and state_dir = %dir.display() — both Path::display(), no contents read. No Debug/Display/Serialize derive is added anywhere in the diff.

  2. The one-base shape holds structurally. autoseed::user_base() (crates/dig-wallet/src/autoseed.rs:113-115) now delegates to crate::wallet_base() — the second copy of the chain is DELETED rather than kept in sync. There is exactly one DigWallet join (lib.rs:246) and one DigNode/device join (autoseed.rs:88-91), both off the same wallet_base(). Verified there is no per-directory override: WALLET_BASE_ENV is the only new env var and it names the base.

Still to do: presence/TOCTOU on legacy_wallet_present, directory modes along the new path, DIG_WALLET_BASE as a redirection primitive vs. shipped DIG_IDENTITY_DIR/DIG_NODE_CACHE precedent, §5.1 format, §908.

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.service declares no User= and no WorkingDirectory=, sets ProtectSystem=full, and runs as root. ProtectSystem=full remounts /usr, /boot, /efi, /etc read-only and leaves / writable, so /DigWallet/seed.bin is a successful write. None of the new prose in lib.rs, state.rs or SPEC.md is born false on this point.
  • The do-not-orphan guard survives adversarial reading. legacy_wallet_present() is evaluated at state.rs:263, before the set_var loop; and independently of that ordering, legacy_wallet_base() passes None for 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), in run() before Config::from_env() and before dispatch. The Windows SCM path reaches win_service::run only via Command::RunService, i.e. through the same run(), so the SCM entry is covered.
  • The Windows arm is a pure case runnable on the Linux runners. CI is ubuntu-latest on all jobs, so this matters.
  • The sibling boundary holds. autoseed::user_base() now delegates to crate::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. No Debug/Display/Serialize added to DeviceKey.
  • 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 via autoseed::default_paths() at control.rs:2364, server.rs:2723, wallet_bootstrap.rs:27, dig-wallet/examples/operator_address.rs:8, and via seed_export::default_seed_path at seed_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:

  1. "the wallet's env-first resolver fell back to the working directory" — it did not; it resolved the anchored state dir.
  2. "set HOME in the service environment if you want the two halves to share one root" — DIG_WALLET_BASE outranks HOME in 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's an_unreadable_seed_path_is_an_error_not_an_absence is 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_all asserts 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 in state.rs: delete !legacy_wallet_present and a_service_that_already_has_a_wallet_keeps_opening_the_old_one goes red; delete the push and a_service_with_no_wallet_yet_anchors_the_wallet_base_at_the_state_dir goes red. The PR body should say that, rather than implying the resolver test is the regression proof.
  • N3 — crate::wallet_env at state.rs:219 names 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) and dig_wallet::sage::custody::legacy_seed_path (custody.rs:316) mean different things — the pre-#491 operator seed vs the Sage wallet-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.

Comment thread crates/dig-wallet/src/autoseed.rs Outdated
Comment thread crates/dig-node-service/src/state.rs
Comment thread crates/dig-wallet/src/lib.rs
Comment thread crates/dig-wallet/src/lib.rs
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

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 f1170d0origin/main is now that commit at version 0.239.0, and 48e138f is no longer a descendant of it. Caught by the correctness gate, not by me.

0.241.0 on this branch is still above 0.239.0, so the version gate stays satisfied. Re-read the version from Cargo.toml on disk after the merge anyway — never from the commit log.

Use git merge origin/main. Never rebase. A rebase whose patch already merged prints dropping <sha> ... patch contents already upstream, exits 0, and silently takes the version bump with it.

Conflict 1 — crates/dig-wallet/src/autoseed.rs, user_base()

Both sides touched it, in different ways:

  • main (fix(wallet): say when a wallet-isolating override was not honoured #489) made it pub fn user_base() and rewrote its doc to explain why: dig_node_service::wallet_env calls it to compare the two resolvers. Its doc ends "The body is unchanged."
  • this branch changed the BODY to crate::wallet_base() and rewrote the doc to explain the delegation, leaving it private.

Resolution: keep pub (dig-node-service will not compile otherwise), keep this branch's body, and merge both doc comments — main's reason for the visibility and this branch's reason for the delegation are both load-bearing. Delete "The body is unchanged."; the merge makes it false.

Conflict 2 — crates/dig-node-service/src/wallet_env.rs prose is now false in three places

The interaction is present, not future. On a stock .deb service with no wallet, the anchor gives wallet_base=/var/lib/dig-node vs node_base=/root, so split_of returns Some(cause: Ambient) on every start and logs AMBIENT_SPLIT_ROOTS.

This is not a functional regressionAmbient means Proceed, and a fresh Windows service also classifies Ambient, so there is no RefuseSplitRoot and no wallet-less install. What is wrong is the text:

  • line ~6 module doc spells the chain LOCALAPPDATA -> HOME -> ".". It is now DIG_WALLET_BASE -> LOCALAPPDATA -> HOME -> ".".
  • line ~134 AMBIENT_SPLIT_ROOTS: "the wallet's env-first resolver fell back to the working directory" — false once the base is anchored.
  • line ~137 AMBIENT_SPLIT_ROOTS: "set HOME in the service environment if you ..." — DIG_WALLET_BASE now outranks HOME, so this is the wrong remedy.
  • line ~364 test doc: "no LOCALAPPDATA, no HOME, so the wallet half collapses to ..." — stale for the anchored case.

Do NOT suppress the announcement and do NOT make the anchor conditional on the split. Correct the prose so it describes what actually happens.

Watch the guards at wallet_env.rs:534-540: they are AMBIENT_SPLIT_ROOTS.contains("HOME") / contains("LOCALAPPDATA") assertions, and a contains check on an identifier is satisfied by any superstring — so a reworded constant can keep them green while saying something different. Keep them meaningful.

Then

Re-run cargo test -p dig-wallet --lib and -p dig-node-service --lib and read the counts, not the exit status. Push. Stay DRAFT.

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-node is created mode 0700 by the postinst (packaging/linux/build-deb.sh:114); ensure_dir_restricted sets 0700 on fresh create (crates/dig-node-service/src/state.rs:457-474); harden_unix_dir enforces 0700, reads it back, and fails closed by removing the dir if it cannot (state.rs:827-843).
  • Both secret FILES are 0600 at open time either way — the mode is set on the OpenOptions before open, 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/ and DigNode/device/ are themselves created by fs::create_dir_all with 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 by service.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() uses Path::try_exists, which returns Err and not Ok(false) when existence cannot be determined (crates/dig-node-core/src/shared/at_rest.rs:45-51), and legacy_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 at crates/dig-node-service/src/entrypoint.rs:857 — after arg parsing, before the tokio runtime is built, and before wallet_bootstrap::ensure_wallet_seed() at entrypoint.rs:1425. The Windows SCM path is covered because dig-node install writes the service run-context into the service environment (service.rs:871), so running_as_service() is already true at line 857 and the anchor is live before win_service.rs:126. std::env::set_var is 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 are Path::display(); neither file is read.
  • DeviceKey is untouched by the diff and still carries no Debug, Display or Serialize (crates/dig-wallet/src/autoseed.rs:140-144). Zeroizing usage is untouched — hex(), as_password() and password_str are all unchanged.
  • No new error type carries bytes: legacy_wallet_present() returns bool, legacy_seed_path() returns a PathBuf, 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: twolegacy_seed_path() (lib.rs:226) and seed_path() (lib.rs:245)
  • production join of the device dir: oneautoseed.rs:88-92
  • callers of user_base(): oneautoseed.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:

  1. Operator sets DIG_WALLET_BASE=/srv/dig-wallet in /etc/dig-node/dig-node.env.
  2. The host also carries a stale /DigWallet/seed.bin from a pre-fix build.
  3. On start the node logs seed=/DigWallet/seed.bin together with "this service keeps opening it rather than the machine state dir" — while wallet_base() resolves /srv/dig-wallet and 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:219 references crate::wallet_env, which does not exist in this tree — there is no crates/dig-node-service/src/wallet_env.rs at 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_base trims 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.

Comment thread crates/dig-node-service/src/state.rs
Comment thread crates/dig-node-service/src/state.rs Outdated
Comment thread crates/dig-wallet/src/lib.rs
Comment thread crates/dig-wallet/src/autoseed.rs
MichaelTaylor3d added a commit that referenced this pull request Sep 2, 2026
…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>
MichaelTaylor3d added a commit that referenced this pull request Sep 2, 2026
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>
MichaelTaylor3d added a commit that referenced this pull request Sep 2, 2026
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>
MichaelTaylor3d added a commit that referenced this pull request Sep 2, 2026
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>
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Lane progress — head 2f6aa499 (pushed).

Verifying next: cargo test -p dig-wallet --lib then -p dig-node-service --lib, checking the test COUNT and not only the exit status. Then replies + resolves on all six threads.

…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>
@MichaelTaylor3d
MichaelTaylor3d force-pushed the loop/491-wallet-seed-path branch from 3f26616 to 5ada6d2 Compare September 2, 2026 10:47
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

All six threads replied to and resolved — unresolved=0 confirmed from the GraphQL API, not from the six successful calls. Head is now 5ada6d23.

Suites at this tree (counts, not just exit codes):

  • cargo test -p dig-wallet --lib — 777 run, 776 passed, 0 failed, 1 ignored, 0 filtered out
  • cargo test -p dig-node-service --lib778 passed, 0 failed, 0 filtered out

The dig-node-service run is also the compile proof GATING-1 asked for: the crate cannot build against a private user_base().

One thing the gate round did not catch, found by the merge-precondition check: Lint commit messages was RED.

merge: is not in this repo's type-enum (commitlint.config.mjs allows feat|fix|docs|style|refactor|perf|test|build|ci|chore|revert), so my merge commit failed type-enum. The two earlier merge: commits on this branch are the same violation and had only escaped because the action lints the push range, not the full PR range — 65d39e34 passed as a push head while carrying the identical defect. A force-push that re-lints from the base would have failed on all three, so I fixed the class rather than the one instance.

Rewrote all three merge subjects to chore: with git commit-tree, which preserves the trees exactly: git diff 3f266164 5ada6d23 is empty, and each rewritten commit's tree hash was asserted equal to its original before the branch moved. Moved the ref with reset --soft (cannot touch a file) and pushed with --force-with-lease=…:3f266164. Author identity is unchanged and inherited — Michael Taylor <michael@michaeltaylor.dev>; nothing was passed to -c user.*.

Production code is untouched by this push: the only non-merge change since the gate round is the doc-link commit in state.rs.

Next: watch Lint commit messages + Test + coverage to terminal on 5ada6d23. PR stays DRAFT.

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>
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

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: gh pr view --json mergeable reports CONFLICTING at this head.

@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review September 2, 2026 18:52
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

loop-reviewer — scoped re-gate STARTED (IN PROGRESS — not the verdict)

Fresh context, orchestrator e93b41. Head being audited: 3cffd0b9a222c5c745a819ff88a90801fb50ca0c (resolved from the remote, not from the brief).

Scope of this pass (not a full re-review):

  1. Re-verify the prior verdict's GATING-1 (autoseed base resolution), GATING-2 (state.rs prose) and N1 (fail-closed discriminating test) at the tree.
  2. Re-verify each of the six threads resolved at 10:48Z against the code — a resolved thread is not a fixed defect.
  3. Confirm the 18:54Z merge-main commit adds nothing beyond the Cargo.toml/Cargo.lock version-line resolution.
  4. Confirm mergeability is clean now (was CONFLICTING at 82db50b).

I do not merge, undraft, or edit code. Verdict posted as a separate comment when the pass returns.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

IN PROGRESS — not the verdict (item 3 + item 4)

Item 3 — CLEAN. The 18:54Z merge commit 3cffd0b adds nothing beyond resolving the version line.
Evidence, at the tree: git diff --stat 3cffd0b origin/main@bfaa79d differs in exactly the PR's own
seven files, and by the exact inverse of the PR delta (git diff --stat b9979d8 82db50b):
Cargo.lock, Cargo.toml, SPEC.md, crates/dig-node-service/src/state.rs,
crates/dig-node-service/src/wallet_env.rs, crates/dig-wallet/src/autoseed.rs,
crates/dig-wallet/src/lib.rs — 464 insertions one way, 464 deletions the other. Main's own SPEC.md
and dig-node-core/dig-wallet changes from that window are preserved intact. Cargo.toml on disk at
3cffd0b reads version = "0.248.0".

Item 4 — NOT clean; the conflict RECURRED and the version is now BELOW main.
mergeable: CONFLICTING, mergeStateStatus: DIRTY at 3cffd0b, re-read from the API in this pass.
origin/main has moved on twice since the merge-main was taken: it is now 8089a1f1
(#507, merged 12:28 local) with bfaa79de (#500) beneath it, and main's version is 0.252.0.
git merge-tree --write-tree 3cffd0b origin/main conflicts on Cargo.toml and Cargo.lock.

So the branch's 0.248.0 is four minors BEHIND main rather than ahead of it. Two consequences the
orchestrator must handle, and the second is the one that hides:

  • the version-increment gate cannot pass from here; and
  • re-read the version from Cargo.toml on disk after the next rebase, never from the commit log
    a rebase whose bump patch collides with one already upstream reports RC=0, zero conflicts, and a
    single dropping <sha> line, leaving the branch at main's version with no bump commit left to notice.

This is a merge-precondition item, not a code finding; the correctness half of this pass continues.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

IN PROGRESS — not the verdict (items 1 and 2: all three prior findings + all eight threads re-verified at 3cffd0b)

I re-read the code rather than the replies. Every one is genuinely discharged at the tree.

GATING-1 (autoseed base resolution) — FIXED. crates/dig-wallet/src/autoseed.rs:122
pub fn user_base() -> PathBuf delegating to crate::wallet_base() at :123, with the #392
rationale preserved at :112-121. Both halves the finding demanded survived the merge: the pub
(without which dig-node-service would not compile — wallet_env.rs:6 documents it and
wallet_root_split calls it) and the delegation. A wrong version — taking the branch side wholesale —
would have dropped the pub and broken the build, or kept a second copy of the resolution chain,
which is the drift the delete exists to prevent: wallet_base() (lib.rs:205) is now the single
place the chain lives, and the seed/device-key sibling relationship cannot resolve from two bases.

GATING-2 (state.rs / AMBIENT_SPLIT_ROOTS prose) — FIXED, in the right direction. Both false
claims are gone from crates/dig-node-service/src/wallet_env.rs:131-140: "fell back to the working
directory" (which the anchored base makes untrue) and "set HOME in the service environment" (an
inert remedy, since DIG_WALLET_BASE outranks it). The module doc at :6-7 now lists
DIG_WALLET_BASE as the first rung. Critically, the guard test moved with the prose
(wallet_env.rs:541-553): contains(WALLET_BASE_ENV) plus a NEGATIVE !contains("set HOME"), so
re-introducing the inert advice goes red. And the ambient() fixture at :368-376 was updated from
Path::new(".") to /var/lib/dig-node — without that, the fixture would have kept asserting the
pre-fix shape and the whole ambient suite would have passed for a state production can no longer
reach. A wrong version fixes the behaviour and leaves the constant, shipping a start-up warning that
tells operators to set a variable that does nothing.

N1 (fail-closed discriminating test) — FIXED, and made STRUCTURAL rather than only asserted.
crates/dig-wallet/src/lib.rs:251 extracts
fn presence_counts_as_present(probe: std::io::Result<autoseed::Presence>) -> bool, and
an_unreadable_legacy_seed_path_counts_as_present (lib.rs:1252-1268) asserts both directions: the
control (Ok(Absent) → false, Ok(Present) → true) and the property (PermissionDenied,
InvalidInput, Other → true). This is the guard-specificity shape done right — stated over the
error CLASS by iterating kinds, not over one attacker-chosen error. The stronger part is that the
io::Result ARGUMENT cannot survive the refactor the finding feared: a change to Path::exists()
collapses every metadata error to false and cannot keep this signature, so it fails to COMPILE
rather than silently stranding a funded wallet. The test's own doc states honestly why it asserts the
mapping rather than a real unreadable path (an interior-NUL base cannot be delivered through
set_var on Windows) — a bounded limitation, not a benign fixture.

Thread A (loop-security, warn narrates a decision it does not share) — FIXED as specified.
state.rs:273 is now if legacy_wallet && !env_is_set(WALLET_BASE_ENV), the same conjunction the
decision at state.rs:245 uses (!wallet_base_set && !legacy_wallet_present), with the reason at
:267-272. Covered by an_operator_set_wallet_base_wins_even_with_a_legacy_wallet_on_disk
(state.rs:1120). I checked the polarity trap this invites and it is clean:
env_is_set (state.rs:302-307) trims before the emptiness test, matching
resolve_wallet_base's .map(|v| v.trim()).find(|v| !v.is_empty()) — so a whitespace-only
DIG_WALLET_BASE is "unset" to BOTH, and there is no window where the service declines to anchor
because the variable looks set while the wallet crate ignores it.

Trim thread (padded DIG_WALLET_BASE) — FIXED, with the fixture the old test could not see.
lib.rs:199-201 trims BEFORE the emptiness test and returns the trimmed value, so the padded case
cannot survive; a_padded_value_resolves_to_the_path_the_operator_wrote (lib.rs:1224-1239) asserts
both a winning padded override and a padded lower rung. Note this covers the three-state gap
properly: whitespace-only (unset), padded (trimmed), and empty-at-every-rung (None) are three
distinct assertions, not one.

crate::wallet_env reference thread — FIXED. state.rs:220 is now a real intra-doc link, and
the module it names exists in this tree (crates/dig-node-service/src/wallet_env.rs, arrived with
#489 via the merge). state.rs:224 states the reason for the link form: rustdoc fails if the module
moves. So it can no longer become a permanently false code span.

N2 (PR body cited a non-discriminating test as the proof) — ADDRESSED. The body's Tests section
now reports the red run against a behaviour-preserving stub with the actual assertion failures
(left: Some("."), right: None), which is the discriminating evidence; the resolver tests are listed
as coverage rather than as the regression proof.

The autoseed structural observation — still holds at this head. Re-checked rather than assumed:
user_base() has exactly one caller (autoseed.rs:89), the wallet-dir joins are lib.rs:230 and
lib.rs:257, the device-dir join is autoseed.rs:89-92, and all of them hang off the one
wallet_base()/legacy_wallet_base() pair. There is no operator-reachable configuration that puts
the device key inside the wallet directory.

SPEC.md — coherent. §7.3a gains the wallet-base anchoring rule INCLUDING its condition in
normative voice ("MUST be adopted ONLY when no wallet is present…", "Presence that cannot be
DETERMINED MUST count as present", "Key material MUST NOT be moved or copied automatically"), and the
§16.4 layout table gains the <user_base> resolution sentence cross-referencing §7.3a. The old rule's
phrasing does not survive anywhere I could find it — I swept for "working directory" and "set HOME"
across the crate rather than spot-checking the lines the diff touched.

Suite result and the verdict follow.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

loop-reviewer — VERDICT: PASS (correctness), MERGE BLOCKED on a precondition the orchestrator owns

Head audited: 3cffd0b9a222c5c745a819ff88a90801fb50ca0c, resolved from the remote at the start of
this pass. It did not move while I worked. Scoped re-gate, not a full re-review: the prior verdict's
three findings, the eight resolved threads, the merge-main commit's content, and mergeability.

The change is correct and I would merge it as it stands. All three prior findings and all eight
review threads are genuinely discharged AT THE TREE — I re-read the code rather than the replies, and
the per-finding evidence with file:line is in the interim comment above. The short form:

item state anchor
GATING-1 autoseed base resolution FIXED autoseed.rs:122 pub fn user_base()crate::wallet_base(); single chain at lib.rs:205
GATING-2 AMBIENT_SPLIT_ROOTS prose FIXED wallet_env.rs:131-140; guard test + negative assertion :541-553; ambient() fixture moved off "." at :368
N1 fail-closed test FIXED, made structural lib.rs:251 presence_counts_as_present(io::Result<..>); test lib.rs:1252 iterates the error CLASS
thread A: warn narrates a decision it does not share FIXED state.rs:273 conjunction matches state.rs:245; test state.rs:1120
thread: padded DIG_WALLET_BASE FIXED lib.rs:199-201; test lib.rs:1224
thread: crate::wallet_env reference FIXED state.rs:220 real intra-doc link; module exists via #489
N2: body cited a non-discriminating test ADDRESSED body's red run cites the stub's assertion failures
autoseed sibling-boundary observation HOLDS at this head one caller autoseed.rs:89; joins lib.rs:230, lib.rs:257, autoseed.rs:89-92

Two things I checked that a reply-reading pass would have missed, and both came out clean: the
ambient() FIXTURE was updated alongside the prose (without it the suite would have kept passing for
a state production can no longer reach — a false green by drift), and env_is_set
(state.rs:302-307) trims exactly as resolve_wallet_base does, so there is no window where the
service declines to anchor because a whitespace-only variable looks set to one side and unset to the
other. I also swept the two crates for the superseded phrasing rather than spot-checking the touched
lines: every surviving mention of "working directory" describes the DEFECT, and the only "set HOME"
left is the negative assertion that forbids its return.

Item 3 — the merge-main commit is CLEAN. 3cffd0b differs from bfaa79d in exactly the PR's own
seven files and by the exact inverse of the PR delta (464 insertions one way, 464 deletions the
other). Main's own SPEC.md, dig-node-core and dig-wallet changes from that window are preserved
intact. Cargo.toml on disk reads version = "0.248.0".

Item 4 — NOT clean, and this is the one blocking thing. mergeable: CONFLICTING,
mergeStateStatus: DIRTY. origin/main moved twice more after the merge-main was taken — it is now
8089a1f1 (#507) over bfaa79de (#500) — and main's version is 0.252.0, four minors ABOVE this
branch's 0.248.0
. git merge-tree --write-tree 3cffd0b origin/main conflicts on Cargo.toml and
Cargo.lock.

Note that Check version increment currently reads pass on this PR. That green was computed
against 0.247.1 and is now stale — it is a reassuring number that measured a base which no longer
exists, not evidence the version is acceptable.

What the orchestrator must do

  1. Rebase onto origin/main and RE-BUMP past 0.252.0, as one instruction. Both halves, or the
    version gate fails.
  2. Re-read the version from Cargo.toml on disk afterwards, never from the commit log. A rebase
    whose bump patch collides with one already upstream reports RC=0, zero conflicts, and a single
    dropping <sha> line — leaving the branch AT main's version with no bump commit left to notice.
    Scan the rebase output for dropping, not only for CONFLICT.
  3. Remove the DO-NOT-MERGE line from the body — this verdict is now posted, which is the
    condition that line names. I do not edit PR state.
  4. Re-run check-merge-preconditions.sh unpiped (script … >/tmp/out 2>&1; rc=$?) and merge only on
    -eq 0. The rebase moves the head, so the checks re-run; this verdict covers the code, and a
    rebase that only resolves the version line does not invalidate it.

Threads: 8 total, 8 resolved, 0 unresolved. I opened none — I found nothing at this head that
warrants an open thread, and the merge is already barred mechanically by the conflict, so an
artificial thread would only have to be resolved again after the rebase.

Not covered by this pass, stated so it is not mistaken for cleared: this was scoped to the four
items above. I did not re-audit the security leg, the money-surface reasoning in wallet_env, or the
four deliberately-deferred items the body names (the "." fallback for non-service runs, the
CLI/service wallet divergence on Linux, the #489 sequencing, and WalletPaths::resolve's device-key
derivation). Each needs its named child ticket to exist before this closes #491.

I did not run the suite locally — CI's Test + coverage passed in 19m17s on this exact head, along
with Clippy, Rustfmt, all four package builds and all three CodeQL analyses, which is stronger
evidence than a partial local run and costs nothing. Named rather than silently skipped.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Addendum to the verdict — one correction, no change to the outcome.

I did attempt a local cargo test -p dig-wallet -p dig-node-service --lib in my own detached
worktree; the verdict's phrasing "I did not run the suite locally" understates that. It did not
produce a result: it failed to COMPILE with crate 'anstyle_wincon' required to be available in rlib format, but was not found in this form and the same for strsim — a toolchain/target-dir artifact
on this machine, not a property of the code. Worth naming because the backgrounded run reported
exit code 0 while the log contained error: could not compile, so the status was the shell's and
not the command's; a caller reading only that code would have recorded a green run that never ran a
test. Test count matched: zero.

The suite evidence in the verdict is therefore CI's Test + coverage on 3cffd0b (19m17s, pass),
unchanged, and it is the stronger evidence either way. Verdict stands: PASS on correctness, merge
blocked on the rebase + re-bump above 0.252.0.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

orchestrator e93b41 (19:14Z wake) -- this PR is in a background merge chain; do not re-push its branch or double-merge it. All gates on it have RETURNED (see the verdict comments above). Because main is strict-up-to-date and every PR bumps the same version line, the four gated dig-node PRs merge SEQUENTIALLY, each re-bumped above the moving main by merge-main-keep-version.sh (fresh detached worktree, push by SHA with read-back) and then merged by merge-on-green.sh (required checks asserted BY NAME, undraft, squash):

order PR version at merge why patch
1 #499 0.252.1 fix
2 #498 0.252.2 fix
3 #506 0.252.3 fix
4 #504 0.252.4 test/docs

Chain log: C:/tmp/orch-e93b41/chain.log on the loop machine. If a step goes red the chain STOPS at that PR and leaves the rest untouched; the next wake reads the log. Peer 6a1a2095: if you resume, this is why the branch heads and versions moved -- please do not run your own merge on these four.

@MichaelTaylor3d
MichaelTaylor3d merged commit 0aaa7f7 into main Sep 2, 2026
15 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the loop/491-wallet-seed-path branch September 2, 2026 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A stock Linux .deb service resolves its wallet seed to ./DigWallet/seed.bin, relative to the service cwd

1 participant