feat(custody): offline seed-export CLI so a custodied wallet survives custody removal - #331
Conversation
…ystem#1701) WIP stub so the lane survives a cap. Co-Authored-By: Claude <noreply@anthropic.com>
… custody removal Node-side USER custody is being retired. For a user who migrated their seed into this node and kept no independent copy, the node's seed file is the only surviving copy of their spend key, so the custody surface cannot be deleted until that key has been handed back. `dig-node wallet export-seed` is that hand-back. The population was re-measured and is ONE real blob: 248 bytes, first byte 0x01 -- the LEGACY EncryptedSeed layout -- sealed under a user password, sitting under a base directory current builds no longer resolve. Both facts shape the design: * the export goes through `seed_store::decrypt_seed`, which dispatches on the leading magic and accepts either on-disk format. An export built on `opaque::open` alone would fail on 100% of the real population; `legacy_seed_file_exports` fails by name under exactly that mutation. * `--path` overrides the default location, because a resolver-only command cannot reach the one file it exists to rescue. No network surface is added: no RPC method, no control-plane verb, no loopback endpoint. `wallet_action` now returns `Option<ControlAction>` so the local verb is typed as reaching no control method rather than being mapped to a fake one. Running the command needs local filesystem access AND the wallet password -- the same two things an attacker would already need to open the file by hand. `--json` is refused: a recovery phrase must not be emitted as machine-readable output, the form most likely to be redirected into a file or a log. Read-only throughout -- nothing on disk is written, moved, zeroized or deleted -- and the mnemonic never reaches a log or an error string. Refs dig-node#329, dig_ecosystem#1701. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…g them Static analysis cannot tell a password-shaped test literal from a real credential, so CodeQL flagged all twelve. The values were always irrelevant to the assertions -- only same-label-same-password and different-labels-differ matter -- so they are now derived from a hashed label, matching the existing convention in this workspace. Re-proved after the change: the legacy test still fails by name under an `opaque::open` mutation. Refs dig-node#329. Co-Authored-By: Claude <noreply@anthropic.com>
# Conflicts: # Cargo.lock # Cargo.toml
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
CHANGES-REQUIRED (1 gating finding, one line) — full gate, head 009a7fd67090a8d1bc8373a020d11e5860ef9640
The head moved during this review (23670f7d → 009a7fd6: a merge of origin/main bringing PR#330's
module_reshare.rs + v0.144.1). git diff 23670f7d 009a7fd6 -- crates/dig-wallet crates/dig-node-service
is empty, so every finding below was re-resolved against 009a7fd6 and holds at that head.
The three questions, answered
1. Can the mnemonic reach any log, error string or panic? NO — verified across the whole surface, not just the test.
ExportError's three variants carry only a PathBuf and, for Unreadable, an io::Error string; the decrypt
error is discarded at seed_export.rs:108 (map_err(|_| ...)), so nothing derived from file contents survives.
Display (:64-78) and the derived Debug interpolate only paths. A grep of every output construct
(println!/eprintln!/print!/tracing::*/log::*/info!/debug!/warn!/error!/trace!/panic!/
unwrap/expect) across both non-test modules returns exactly one site that touches the phrase:
seed_export_cli.rs:64 println!("{}", *mnemonic), which is the intended stdout hand-back. There is no
tracing call on the path at all, and no unwrap/expect in non-test code, so no panic can carry it. The
phrase is Zeroizing<String> end to end. wrong_password_fails_without_leaking is a real per-WORD check and
its mutated failure output confirms the rendered error is path-only.
2. Does the opaque::open mutation fail by name with the honest control still passing? YES — reproduced.
Rebuilt export_mnemonic on dig_keystore::opaque::open in an isolated worktree (C:\tmp\worktrees\gate331,
now removed; no shared checkout mutated):
test seed_export::tests::current_format_seed_file_also_exports ... ok <-- honest control survives
failures:
seed_export::tests::legacy_seed_file_exports
seed_export::tests::explicit_path_reaches_a_non_default_location
seed_export::tests::export_leaves_the_file_byte_identical
test result: FAILED. 3 passed; 3 failed
legacy_seed_file_exports fails by name with Undecryptable(...), and the control passes — confirming
the claim that the control alone would have masked the trap entirely. This is load-bearing, not a value
assertion: the fixture is built by the actual legacy writer (digstore_chain::seed::encrypt_seed) and the
test asserts bytes[0] == 1 first, so it names the property (reads the legacy container) rather than an
outcome. current_format_seed_file_also_exports asserts bytes[0] != 1, so it cannot silently become a
second legacy test. Both formats reach crate::seed_store::decrypt_seed, whose magic dispatch is pre-existing
and separately proven by legacy_digstore_chain_seed_file_still_decrypts.
3. Is no-network-surface STRUCTURAL? YES — type-level, not test-level.
wallet_action returns Option<ControlAction> and WalletCommand::ExportSeed { .. } => return None
(entrypoint.rs:787); no ControlAction variant for export exists, so the verb cannot be expressed as a
control method. It is routed locally by a preceding match arm (entrypoint.rs:676-678) straight into
seed_export_cli::run. method_for_argv propagates the None via ?. A repo-wide grep for
export_seed|export-seed|exportSeed finds zero hits in rpc, the control interface, or any served handler —
only the CLI enum, the local route, and docs. The ticket's rejection of a served wallet.export_seed RPC is
honoured.
Also verified
- Nothing written or deleted.
export_mnemonicperforms exactlypath.exists()+std::fs::read. No
write/create/OpenOptions/truncate/rename/remove/lock file anywhere on the path, in either module.
export_leaves_the_file_byte_identicalpins it across both a success and a wrong-password call. --pathreasoning holds.explicit_path_reaches_a_non_default_locationasserts
path != default_seed_path()before the read, so it cannot pass while only ever reading the default.- Trailing-space password.
strip_line_ending(seed_export_cli.rs:97) removes only\nthen an
optional preceding\r.a_piped_password_keeps_its_own_spacesuses" pad ded "— first AND last chars
spaces — which is exactly the input that distinguishes it from the nearest wrong implementation (trim),
and covers LF, CRLF, no-terminator and a bare interior\r. TheIsTerminalfallback is correct for the
Windows non-TTY hang. - CodeQL fixture passwords: a genuine fix, not scanner-dodging.
password(label)is
hex(sha256(label)); no assertion anywhere depends on its value, only on same-label-same-value, so the
literal it replaced was genuinely not a credential and removing it loses no test power. (Citation caveat
below.) - Tests green unmutated: 6/6
dig-wallet::seed_export, 4/4dig-node-service::seed_export_cli. - Nothing overclaimed. Neither module nor
SPEC.mdclaims the export→import→address round trip; the
handling notice tells the user to confirm the derived address matches before relying on it, i.e. it
states the round trip as the user's step rather than as proven. No blob is zeroized anywhere in the diff. - dig-constants check. (1) Nothing here BELONGS in
dig-constants: the diff introduces no shared/
cross-repo constant — the two consts are local prose strings and the legacy version byte1and the
DIGVK1/DIGLW1/DIGOP1magics are pre-existing inseed_store.rs, unmoved. (2) Nothing here should be
USINGdig-constants: no hardcoded literal in the diff duplicates a valuedig-constantspublishes. Clear
on both questions.
Merge preconditions (asserted BY NAME, not from the rollup)
check-merge-preconditions.sh --repo DIG-Network/dig-node --pr 331 → BLOCKED (exit 1):
Lint commit messages SUCCESS, Check version increment SUCCESS, Rustfmt SUCCESS, Clippy SUCCESS,
Test + coverage IN_PROGRESS. Unresolved threads 0, draft=true. Do not pass on the pending check —
re-assert by name once it lands. Versions 0.145.0 / dig-wallet 0.31.0 are correct over the merged
v0.144.1.
The one gating finding
See the inline comment on crates/dig-node-service/src/lib.rs.
Non-gating notes (do NOT block merge; posted as a plain comment so no thread bars
|
Corrected the module doc-comment ordering so each doc block sits above the module it describes. Previously, the security module's doc comment appeared before seed_export_cli, leaving security undocumented and causing rustdoc to misattribute the trust-gate paragraph to the seed export module. - Moved seed_export_cli doc to precede its module - Placed security doc directly above security module - Each module now has correct rustdoc attribution Co-Authored-By: Claude <noreply@anthropic.com>
…t 135 The doc-placement fix in a6ce17b was correct -- my feature commit had inserted the export-seed doc BETWEEN the `security` doc block and its `pub mod security;`, so the combined block attached to `seed_export_cli` and left `security` undocumented. That fix also rewrote every line ending in the file from LF to CRLF, which is why a four-line change arrived as 135 insertions and 135 deletions. Every other file in this repo is LF, and a whole-file rewrite destroys `git blame` for the module list. Converted back to LF, keeping the corrected placement. The diff against main is now the four lines the feature actually adds. Refs dig-node#329. Co-Authored-By: Claude <noreply@anthropic.com>
Closes #329. Step 3 of https://github.com/DIG-Network/dig_ecosystem/issues/1701.
Why this exists
Node-side USER custody is being retired. For a user who migrated their seed into this node
and kept no independent copy, the node's seed file is the only surviving copy of their
spend key — so the custody surface cannot be deleted until that key has been handed back.
dig-node wallet export-seedis that hand-back, and it is deleted in step 4 with the codeit rescues.
Step 3 had been skipped on a population count of zero. That count was scoped to the wrong
store. The real population is one, and it is a user's wallet.
The measured blob, and how it shaped the design
0x01— the legacyEncryptedSeedlayoutTwo traps follow directly, and both are pinned by tests:
dig_keystore::opaque::openfails on 100% of the real population. The export goes through
seed_store::decrypt_seed, which dispatches on the leading magic.exists to rescue.
Trap 2 is not hypothetical here. Run with no
--pathon the measuring machine, the commandresolves
…\AppData\Local\DigWallet\seed.binand reports "did not decrypt" — that fileexists and is a different wallet (an operator identity). The default path finds the
wrong file, not no file.
What was built
export_mnemonic/default_seed_path/ExportErrorcrates/dig-wallet/src/seed_export.rs:93,:84,:46crates/dig-node-service/src/seed_export_cli.rs:45export-seedverbcrates/dig-node-service/src/entrypoint.rs:375crates/dig-node-service/src/entrypoint.rs:675SPEC.md:2431No network surface is added — stated explicitly
No RPC method, no control-plane verb, no loopback endpoint, no served handler.
dig_wallet::seed_exportis called in-process by the CLI against the local filesystem.Running it needs local filesystem access and the wallet password — the same two things
an attacker would already need to open the seed file by hand — so it grants nothing that
local access did not already grant.
A served
wallet.export_seedwas considered and REJECTED on the parent ticket. Thatruling stands and this PR does not revisit it.
To keep that structural rather than merely intended,
wallet_actionnow returnsOption<ControlAction>:export-seedis typed as reaching no control method, insteadof being mapped to a fake one.
method_for_argvcorrespondingly answersNonefor it.--jsonis refused with a message naming the working alternative. A recovery phrasemust not be emitted as machine-readable output — the form most likely to be redirected into
a file or a log.
Evidence
Legacy
0x01decrypts, proven by mutation. Rebuildingexport_mnemoniconopaque::openinstead ofdecrypt_seed:legacy_seed_file_exportsfails by name, with the exactUndecryptable(…)symptom the real population would produce. Note thatcurrent_format_seed_file_also_exportskept passing under the mutation — it is thehonest control, and on its own it would have masked the trap completely.
End to end, on the real binary, against a 248-byte
0x01fixture — byte-for-byte thesame size and format as the measured blob:
Failure paths, same binary: wrong password →
did not decrypt(exit 2); absent file →no seed file at …plus a--pathhint (exit 2);--json→ refused before the file isread (exit 2).
The mnemonic reaches no log and no error string.
ExportErrorcarries only a path anda failure class; the underlying decrypt error text is discarded at the boundary. The phrase
is returned in
Zeroizingand is printed at exactly one call site.wrong_password_fails_without_leakingchecks the rendered error against every word ofthe phrase, not just the whole phrase — a message spilling a single recovered word would
still be a leak, and a whole-phrase check could not see it.
Nothing was deleted or written. Read-only throughout.
export_leaves_the_file_byte_identicalpins it in code, and the e2e fixture's sha256 wasidentical before and after the successful and failed runs.
POST /api/exportis untouched— it remains the only other recovery path.
seed_path(),seed_storeandautoseedareunmodified; the five operator-identity blobs were neither read for content, exported, nor
counted as population.
Two defects found in review, both fixed on this branch
1. Misattributed rustdoc (
lib.rs). My feature commit inserted theexport-seeddoc lines byreplacing the string
pub mod security;— but that declaration sits after its own doc block, sothe insertion landed between
security's doc andsecurityitself. The combined block attached toseed_export_cli, leavingpub mod security;undocumented. Corrected ina6ce17b.2. A whole-file LF→CRLF flip inside that fix.
a6ce17balso rewrote every line ending, turning afour-line change into
135 insertions(+), 135 deletions(-)and destroyinggit blamefor the wholemodule list. Measured:
origin/mainCR=0, myf6b306dCR=0,a6ce17bCR=135. Every other fileon this branch is LF.
b46ef4bconverts it back, keeping the corrected placement —lib.rsis now a4-line diff again.
Blast radius checked
wallet_action— the one existing symbol whose signature changed. It is a private fnin
entrypoint.rswith 6 references, all in that same file (1 dispatch, 1 definition,1 test helper, 3 assertions); it is not
pub, so nothing outside the crate can reach it.Risk: LOW. Everything else in this PR is additive: two new modules, one new enum
variant, one new dependency.
dig_wallet::seed_exportis new public API on apath-dependency consumed only inside this workspace.
Measured by grep over the workspace rather than the gitnexus index — the index for this
worktree wrote to tracked files (
AGENTS.md,CLAUDE.md) and those writes were revertedrather than committed. Recording the fallback per §2.0 bound 2.
What is NOT proven here
The parent ticket's acceptance is export → import into dig-app → derived address
matches → only then the node blob is zeroized. That is a human-observed round trip across
two applications; nothing in this PR can stand in for it, and no unit test here claims to.
What is proven: the export produces the correct phrase from a fixture matching the real
blob's exact size and format, from the real binary. What is outstanding: a person running
it against the actual blob, importing the result into dig-app, and confirming the derived
address. No blob may be zeroized before that happens — and this PR zeroizes none.
Tests
dig-wallet: 6 new,cargo test -p dig-wallet seed_export→ 6 passed, 0 failed(629 filtered out — a non-zero match, so the filter is not vacuous)
dig-node-service: 4 new; full lib suite → 403 passed, 0 failed, including theexisting CLI/control-method drift gates that the
wallet_actionsignature change touchesVersion
0.144.1→0.145.0(minor — a new, backwards-compatible CLI capability).origin/mainmoved to0.144.1mid-review (#330 landed from a sibling lane), which conflictedon
Cargo.toml. Main is merged in and the conflict resolved in favour of0.145.0;Cargo.lockwas regenerated from the merged manifest rather than hand-resolved, and the union was compiled
and tested rather than assumed to build.
dig-wallet0.30.0→0.31.0(new public module).Cargo.lockupdated in the samecommit.