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)	☐
P1-B	W1-1, W1-3, W1-2, W1-4	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

• 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.)