ObsidianDragon/docs/wallet-hardening.md

Wallet Loading & Management — Hardening Plan

Prioritized, grouped remediation for the wallet loading/management audit (33 verified findings + diagnosability QoL). Companion to the findings artifact. Line references are against dev.

• Provenance: 7 parallel subsystem finders, each finding adversarially verified against the code; the 3 highest-impact confirmed findings re-checked by hand. 32 confirmed, 1 refuted (W1-5), 1 raised (W5-3 Low→Med).
• Severity: 8 High · 12 Medium · 13 Low.

Status legend: ☐ not started · ◐ in progress · ☑ landed & verified

---

Roadmap (ordered by risk; shared fixes grouped)

Phase	Findings	Theme	Status
P0-A	W7-1, W2-1, W4-1, W4-3, W2-3, W4-5, W5-3 ✓	Secret hardening (console redaction + delete-export + memzero + lite encrypt-at-create)	☑ 7/7
P0-B	W2-2/W4-2, W2-4	Encryption integrity (never silently unencrypted)	☑
P1-A	W3-1, W3-2, W3-4 ✓ · W3-3 ⚑	Migrate-to-seed correctness (fund-adjacent)	◐ 3/4
P1-B	W1-1, W1-2, W1-3, W1-4 ✓ + startup guard	Missing/wrong wallet-file safety	☑
P2	W6-2, W5-1, W5-2, W6-1, W6-3	Stale state & lite save-failure surfacing	☐
F	W7-2, W7-3, W7-4, QoL	Diagnostics foundation + QoL bundle	☐

---

P0-A — Secret hardening

Shared fix: a SecureString RAII buffer (zeroes on destruction) retrofitted onto the un-scrubbed key/passphrase paths, plus console redaction and deleting the plaintext export.

• W7-1 (High) console_tab.cpp:1419 — RPC console echoes/stores/clipboards raw secrets. Fix: an allowlist of secret-bearing first-tokens (walletpassphrase, walletpassphrasechange, encryptwallet, importprivkey, importwallet, z_importkey, z_importviewingkey, signrawtransaction, magicrecoverkey, lite equivalents); echo > walletpassphrase **** and keep the raw text out of command_history_. Extract a pure redactConsoleCommand(cmd) helper for unit testing. ← implementing first (self-contained + testable).
• W2-1 (High) wallet_security_workflow.cpp:66 — delete the obsidiandecryptexport<ts> plaintext key dump after z_importwallet succeeds (overwrite-then-unlink).
• W4-3 (High) app_network.cpp:4481 — sodium_memzero the concatenated all-keys string in exportAllKeys; write the backup 0600. (Also unify with ExportAllKeysDialog — QoL.)
• W4-1 (High) app_network.cpp:3801 — zero the key copies in importPrivateKey/sweepPrivateKey (local + worker-lambda copies).
• W2-3 (Med) app_security.cpp:1481 — zero the passphrase threaded through the decrypt lambda chain.
• W4-5 (Med) app.cpp:3577 — the seed-backup .txt is a permanent predictable cleartext seed; at minimum warn + offer to delete, ideally discourage file save in favor of the on-screen phrase.
• W5-3 (Med) lite_wallet_lifecycle_service.cpp:322 — remove the dead passphrase field from the lite create/open/restore requests (unused; a secret copied for nothing).

P0-B — Encryption integrity

• W2-2 / W4-2 (High) wallet_security_controller.h:89 — the wizard's deferred encryption is in-memory only and silently lost if the daemon doesn't connect or the app quits/crashes first, so a wallet the user believes is encrypted stays plaintext. Fix: persist a lightweight encryption_requested_but_incomplete settings flag (NEVER the passphrase) when beginDeferredEncryption is called; surface a persistent warning banner while it's set; clear it only on confirmed encryptwallet success; on next connect, if set, re-prompt for the passphrase to complete it.
• W2-4 (Med) app_security.cpp:480 — lockWallet only sets locked on RPC success; log the failure and notify (currently a silent no-op that can leave the wallet unlocked).

P1-A — Migrate-to-seed correctness (fund-adjacent; verify carefully)

• W3-1 (High) app_network.cpp:4327 — adopt hardcodes datadir + "/wallet.dat"; use settings_->getActiveWalletFile() so migrating a non-default active wallet swaps the right file.
• W3-2 (High) seed_wallet_creator.cpp:57 — remove_all(<config>/seed-migrate) unconditionally at Phase-1 start; refuse to wipe if a temp DRAGONX/wallet.dat already exists (a prior un-adopted swept wallet) and surface it, so swept funds in the temp wallet can't be destroyed by re-entry.
• W3-4 (Med) app_network.cpp:1124 — block wallet switching while a migration is pending (getSeedMigrationPending()), not only while the dialog is open.
• W3-3 (Med) app_network.cpp:4231 — persist the sweep opid so an app-close mid-Sweeping can resume/re-poll it instead of silently dropping the txid.

P1-B — Missing/wrong wallet-file safety

• W1-1 (High) app_network.cpp:1109 — fs::exists()-check the target wallet file in switchToWallet() and before the first daemon launch at startup; if missing, block with an explicit "Wallet file not found — moved or deleted?" dialog (browse / create-new) instead of letting the daemon fabricate an empty wallet.
• W1-3 (Med) app_network.cpp:1095 — defer the syncedHere=true stamp to the first successful address/balance readback (idHash non-empty), not bare onConnected().
• W1-2 (Med) app_network.cpp:198 — split DB_CORRUPT-specific strings from the generic "Error loading wallet" fallback; give DB_TOO_NEW its own message/action (not a salvage offer).
• W1-4 (Low) wallets_dialog.h:393 — re-fs::exists() the in-datadir row before switching (match the out-of-datadir path).

P2 — State & lite persistence

• W6-2 (Med) network_refresh_service.cpp:1183 — record a per-field last-success timestamp / a "refresh failed" flag so the UI can show a staleness badge instead of last-good-as-current.
• W5-1 / W5-2 (Med) lite_wallet_controller.cpp:78,603 — liteLog() the failed save and bubble a one-shot UI warning (both call sites currently discard the bool).
• W6-1 (Med) wallet_state.h:313 — reset mining/pool_mining in clear() (or comment why not).
• W6-3 (Low) address_book.cpp:46 — per-entry try/catch: skip + count malformed entries instead of discarding the whole list.

F — Diagnostics foundation + QoL

Land W7-2 first — it unblocks the rest.

• W7-2 (Med) logger.cpp:31 — call Logger::instance().init(<config>/dragonx-debug.log) early in main() on all platforms; add an "Open log folder" action.
• W7-3 (Med) main.cpp:144 — add a sigaction-based crash handler writing dragonx-crash.log on POSIX (mirror the Windows SEH path).
• W7-4 (Low) logger.cpp:39 — size-cap/rotate the log on init().
• QoL — "Copy diagnostics for support" bundle; persistent alert history; daemon/RPC error banner; refresh-staleness badge; multi-wallet diagnostic panel; refresh-diagnostics panel; structured switch/migration audit logging; restore-from-seed entry point (W4-4, effort L).

---

Progress log

• P1-B / W1-3 + startup wallet-existence guard — ☑ landed:

  • W1-3 (Med): syncedHere was stamped in the markOpened block at bare connect (idHash still empty), letting a freshly-restored wallet skip its needed rescan. It's now stamped only once the identity is verified (idHash non-empty), so it takes effect at the post-address-refresh index update (updateWalletIndexForActiveWallet after addresses load), while lastOpenedEpoch still records at open.
  • Startup guard (the W1-1 launch counterpart): App::init now exists()-checks the recorded active wallet before the daemon is configured; a non-default active wallet that was moved/deleted between sessions falls back to the default wallet.dat with a warning, instead of the daemon silently auto-creating an empty wallet under the missing name. Runs before the PIN-vault init so the vault is scoped to the wallet actually opened. Build-clean; ctest 1/1.

• P1-A / W3-3 (sweep opid persistence) — ⚑ deferred for careful, adversarially-reviewed work (not rushed). trackOperation only enqueues the opid for a background poller; a resume that re-tracks a persisted opid is only safe if the poller times out a stale opid (daemon restarted → op gone) rather than polling forever — otherwise a resume would hang the migration permanently, worse than today's re-sweep. Verifying that (and the double-sweep interactions) is exactly the "two rounds of adversarial review + a live mainnet run" the migration code mandates. The existing safety gates (adopt requires the legacy balance ~0 AND the sweep tx mined) already prevent fund loss on a mid-sweep interruption; W3-3 is a stuck-state robustness improvement, so it can wait for a dedicated pass.

• P1-B / W1-1 (+ W1-4) · W1-2 (wallet-file safety) — ☑ landed:

  • W1-1 (High): switchToWallet never checked the target wallet file exists, so a moved/deleted file "opened" as a fresh empty wallet (dragonxd auto-creates for a missing -wallet=), looking exactly like fund loss. It now std::filesystem::exists-checks datadir + "/" + walletFile before switching and blocks with a "not found (moved or deleted?)" warning. Placed before the daemon-stop prompt, and — since the check runs no matter how switchToWallet is invoked — it also closes W1-4 (the stale-switcher-row TOCTOU).
  • W1-2 (Med): walletOutputLooksCorrupt matched the generic "Error loading wallet" string, so a DB_TOO_NEW (newer-version) wallet was offered a -salvagewallet repair that can't fix it. Now the generic match is excluded when the output also contains "newer version". Build-clean; ctest 1/1. Remaining P1-B: W1-3 (defer the syncedHere stamp to a verified readback) + the startup-path existence check (app.cpp hands getActiveWalletFile() to the daemon with no exists() check — same silent-empty-wallet risk as W1-1 but at launch).

• P1-A / W3-1 · W3-2 · W3-4 (migrate-to-seed correctness) — ☑ landed (fund-adjacent — reviewed carefully):

  • W3-1 (High): beginAdoptSeedWallet hardcoded datadir + "/wallet.dat" as the file to swap. With a non-default active wallet (e.g. wallet-2.dat), that installed the swept seed wallet into an unloaded wallet.dat and left the daemon reloading the emptied legacy — funds only recoverable via the seed phrase. Now swaps datadir + "/" + getActiveWalletFile() (captured on the main thread; switching is blocked during migration so it can't race).
  • W3-2 (High): SeedWalletCreator::create did remove_all(<config>/seed-migrate) unconditionally at the start. A prior migration that swept funds into the temp wallet but was abandoned/crashed before adopting would have that fund-bearing wallet destroyed. It now refuses (with a clear message) when DRAGONX/wallet.dat already exists — a completed migration removes the dir on adopt, so a leftover means an unfinished one.
  • W3-4 (Med): switchToWallet only blocked switching while the migration dialog was open; closing it via "Later" mid-migration dropped the guard. Now also blocks while getSeedMigrationPending(). Build-clean; ctest 1/1. Remaining P1-A: W3-3 (persist the sweep opid so an app-close mid-sweep can resume/re-poll instead of silently dropping the txid).

• P0-B / W2-2 (deferred encryption silently lost) + W2-4 (auto-lock silent-fail) — ☑ landed:

  • W2-2: the wizard's deferred encryption was stored only in memory, so a quit/crash or a failed daemon connect before it applied left the wallet unencrypted with no record it was ever requested — the user believing it was encrypted. Now a persisted encryption_pending settings flag is set the moment encryption is requested (never the passphrase — only the fact). refreshWalletEncryptionState() reconciles it on every connect: wallet observed encrypted → clear the flag; wallet not encrypted while the flag is set and no deferred encryption is pending/in-flight → a once-per-session "your wallet is NOT encrypted — open Settings to finish" warning (the flag stays set, so it recurs each launch until resolved). We deliberately don't persist the passphrase to auto-complete — surfacing it is the secure choice.
  • W2-4: lockWallet()'s continuation only handled success — a failed walletlock silently left the wallet unlocked (an unfulfilled auto-lock). It now logs and warns once (reset on the next successful lock), so a failing auto-lock is visible instead of leaving the wallet exposed. Touches settings.{h,cpp}, app_wizard.cpp, app_security.cpp, app.h. Not unit-testable at this layer (RPC/connect-driven state machine); build-clean, ctest 1/1.

• P0-A / W5-3 (lite create-time passphrase) — ☑ landed (chose option (b) wire it up). The lite create/open/restore passphrase was collected but never consumed by the backend — a "passphrase" field that did nothing. It now has a real meaning for all three operations, in LiteWalletController: create/restore → encryptWallet(passphrase) (the backend encrypts + locks + saves the brand-new wallet); open → unlockWallet(passphrase), but only when encryptionStatus() reports the existing wallet is actually encrypted+locked (skips a spurious unlock otherwise). Encrypt/unlock take their own copy and wipe it; a post-create encrypt failure is liteLog'd (the wallet still exists — the create isn't failed). Six existing lite-controller tests carried an incidental hunter2 create passphrase from the dead-field era; removed (they test non-encryption flows and want an unencrypted wallet), and added testLiteWalletControllerCreateEncryptsWithPassphrase to prove the new behavior. Build-clean; ctest 1/1. (Follow-up UX polish: settings_page could show the passphrase field's meaning per operation — "encrypt" for create/restore vs "unlock" for open.)

• P0-A / W4-5 (seed-backup file) — ☑ landed (proportionate): the seed "Save" already wrote 0600 + zeroed the in-memory buffer, but the success message was a bare "Saved to ". It now reads "Saved an UNENCRYPTED seed file — move it to secure offline storage and delete this copy: ", so the plaintext-on-disk risk is called out. i18n.cpp (English source; res/lang back-fill of this changed key is deferred to the batch i18n pass). A stronger fix (pre-save confirmation, or dropping the file-save in favor of on-screen + Copy) is a follow-up UX decision.

• P0-A / W4-1 · W4-3 · W2-3 (memzero cluster) — ☑ landed, using the file's established sodium_memzero pattern (matching the existing lambda-capture scrub at app_network.cpp:2885 and JSON scrub at :4025) rather than a new type, since this is fund-moving code:

  • W4-1 importPrivateKey/sweepPrivateKey: the spending/viewing key is now scrubbed on all paths — the calling-frame copy (after the worker post), the worker-lambda's captured copy (lambda made mutable, zeroed after the request is sent), and the JSON request params copy.
  • W4-3 exportAllKeys/backupWallet: the concatenated all-keys buffer is zeroed after the consumer uses it, and the backup file is now written via Platform::writeFileAtomically(..., restrictPermissions=true) (atomic + 0600) instead of a umask-default ofstream.
  • W2-3 decrypt-wallet passphrase: std::move-captured into the worker lambda (so no plaintext copy is left in the calling frame) and sodium_memzero'd right after unlockWallet (its only use). Not unit-testable (the scrubbing has no observable RPC effect — the key value sent to the daemon is unchanged; only post-use memory zeroing is added). Build-clean; ctest 1/1 (no regression). Remaining in P0-A: W5-3 (remove the dead lite passphrase field), W4-5 (predictable plaintext seed-backup file).

• P0-A / W2-1 — ☑ landed: the decrypt-wallet flow now scrubs (best-effort in-place zero-overwrite) and removes the plaintext key export (obsidiandecryptexport…) as soon as the z_importwallet attempt resolves — success or failure — so a full cleartext dump of every private key is no longer left on disk forever. Recovery remains the encrypted backup (wallet.dat.encrypted.bak). app_security.cpp (after the import call). Not unit-testable (fs I/O in a deep lambda); build-clean, ctest 1/1 (no regression).

• P0-A / W7-1 — ☑ landed: RedactConsoleCommand/ConsoleCommandCarriesSecret in console_tab_helpers redact secret-bearing commands (an allowlist of 13 first-tokens: walletpassphrase, encryptwallet, z_importkey, …) to > walletpassphrase **** before they hit the console echo AND the recall history; the real command still executes unredacted. Wired into submitConsoleCommand (console_tab.cpp). New testConsoleSecretRedaction (11 assertions). Clean build; ctest 1/1. (Output-secret commands like z_exportkey — result redaction — remain a follow-up.)