From 2fce9a78604eb8969a314416de40b2cf0c8244d5 Mon Sep 17 00:00:00 2001 From: DanS Date: Mon, 12 Oct 2026 21:42:23 -0500 Subject: [PATCH] fix(security): reconcile the encryption-pending flag on every connect The wizard persists an "encryption requested" flag so a quit or crash before the wallet is actually encrypted is not silent. That flag was only reconciled on the slow-path getwalletinfo refresh, so it could be left stale, and the clear-and-warn logic lived inline in two places. - A single reconcileEncryptionPending now runs on every connect and on every encryption-state refresh, keyed on the parsed getwalletinfo reply. It clears the flag whenever the wallet is encrypted, however it got there (deferred request, Settings, another client), and warns once per session when encryption was requested but the wallet is not encrypted and nothing is pending or in flight. - A failed or warm-up (-28) RPC decides nothing, so a 15 s timeout that forces encrypted=false can no longer mis-clear or mis-warn. - The flag is also cleared at encryptwallet success and when the user removes encryption, so neither path leaves a stale flag that would warn on the next connect. No settings format change. Tests: the pure decision helper over the flag, RPC-ok, encrypted, deferred-pending, in-flight and already-warned combinations (clear when encrypted, warn once when lost, nothing on a failed RPC or no flag). Co-Authored-By: Claude Opus 4.8 --- src/app.h | 3 ++ src/app_network.cpp | 2 ++ src/app_security.cpp | 54 ++++++++++++++++++---------- src/app_wizard.cpp | 2 +- src/data/wallet_security_reconcile.h | 23 ++++++++++++ tests/test_phase4.cpp | 17 +++++++++ 6 files changed, 82 insertions(+), 19 deletions(-) create mode 100644 src/data/wallet_security_reconcile.h diff --git a/src/app.h b/src/app.h index b089714..e97154e 100644 --- a/src/app.h +++ b/src/app.h @@ -2043,6 +2043,9 @@ private: void refreshAddresses(); // Legacy: standalone address refresh void refreshPrice(); void refreshWalletEncryptionState(); + // C88: reconcile the persisted "encryption requested" flag against one parsed getwalletinfo result + // (connect-time ConnectionInit and every Encryption job). Main thread only. + void reconcileEncryptionPending(const services::NetworkRefreshService::WalletEncryptionResult& enc); void applyRefreshPolicy(ui::NavPage page); bool effectivelySyncing() const; // syncing, or within the post-sync settle window (hysteresis) bool currentPageNeedsWalletDataRefresh() const; diff --git a/src/app_network.cpp b/src/app_network.cpp index daac6d8..dde17eb 100644 --- a/src/app_network.cpp +++ b/src/app_network.cpp @@ -797,6 +797,7 @@ void App::onConnected() auto result = NetworkRefreshService::collectConnectionInitResult(refreshRpc, prefetchedInfo); return [this, result]() { NetworkRefreshService::applyConnectionInitResult(state_, result); + reconcileEncryptionPending(result.encryption); // C88: runs on every connect, keyed on the RPC reply if (state_.isLocked()) { resetTransactionHistoryCacheSession(); } else if (state_.transactions.empty()) { @@ -3032,6 +3033,7 @@ bool App::refreshEncryptionState() auto result = NetworkRefreshService::parseWalletEncryptionResult(walletInfo); return [this, result]() { NetworkRefreshService::applyWalletEncryptionResult(state_, result); + reconcileEncryptionPending(result); // C88: covers the slow-path fallback and the lock-state polls if (state_.isLocked()) { lockTransactionHistoryCacheSession(); } else if (state_.transactions.empty()) { diff --git a/src/app_security.cpp b/src/app_security.cpp index 896be28..657dc79 100644 --- a/src/app_security.cpp +++ b/src/app_security.cpp @@ -7,6 +7,7 @@ #include "app.h" #include "data/pin_vault_policy.h" +#include "data/wallet_security_reconcile.h" #include "rpc/rpc_client.h" #include "rpc/rpc_worker.h" #include "rpc/connection.h" @@ -282,6 +283,12 @@ void App::restartDaemonAfterEncryption(const char* taskName, bool announceRestar state_.locked = true; state_.unlocked_until = 0; state_.encryption_state_known = true; + // encryptwallet succeeded: the wizard's persisted request is satisfied. Clear it HERE (not only at the next + // connect) so a quit before the reconnect can't leave a stale flag (C88). + if (settings_ && settings_->getEncryptionPending()) { + settings_->setEncryptionPending(false); + settings_->save(); + } if (isUsingEmbeddedDaemon()) { if (announceRestartStatus) { @@ -659,12 +666,6 @@ void App::refreshWalletEncryptionState() { state_.unlocked_until = until; state_.locked = (until == 0); state_.encryption_state_known = true; - // Wallet is encrypted — any pending deferred-encryption request has now been - // satisfied (however it completed). Clear the persisted flag (W2-2). - if (settings_ && settings_->getEncryptionPending()) { - settings_->setEncryptionPending(false); - settings_->save(); - } if (state_.locked) { lockTransactionHistoryCacheSession(); } else if (state_.transactions.empty()) { @@ -677,18 +678,6 @@ void App::refreshWalletEncryptionState() { state_.locked = false; state_.unlocked_until = 0; state_.encryption_state_known = true; - // W2-2: encryption was requested (persisted flag) but the wallet is NOT encrypted, - // and no deferred encryption is pending/in-flight — it was lost to a quit/crash or a - // failed connect before it applied. Warn (once/session) instead of silently leaving - // an unencrypted wallet the user believes is protected. The flag stays set until the - // wallet is actually encrypted, so the warning recurs each launch until resolved. - if (settings_ && settings_->getEncryptionPending() && - !wallet_security_.hasDeferredEncryption() && !encrypt_in_progress_ && - !encryption_incomplete_warned_) { - encryption_incomplete_warned_ = true; - ui::Notifications::instance().warning( - TR("sec_encryption_did_not_complete"), 30.0f); - } if (state_.transactions.empty()) { loadTransactionHistoryCacheIfAvailable(); } else { @@ -712,6 +701,29 @@ void App::refreshWalletEncryptionState() { }); } +void App::reconcileEncryptionPending(const services::NetworkRefreshService::WalletEncryptionResult& enc) +{ + if (!settings_) return; + using Act = data::EncryptionPendingAction; + switch (data::decideEncryptionPending(settings_->getEncryptionPending(), enc.ok, enc.encrypted, + wallet_security_.hasDeferredEncryption(), encrypt_in_progress_, + encryption_incomplete_warned_)) { + case Act::Clear: + // The wallet is encrypted, however it got there (deferred, Settings, another client). + settings_->setEncryptionPending(false); + settings_->save(); + break; + case Act::Warn: + // Requested but NOT encrypted, and nothing pending/in flight: lost to a quit/crash or failed connect. + // The flag stays set, so this recurs each launch until the wallet is actually encrypted (W2-2). + encryption_incomplete_warned_ = true; + ui::Notifications::instance().warning(TR("sec_encryption_did_not_complete"), 30.0f); + break; + case Act::None: + break; + } +} + // =========================================================================== // Auto-lock on idle // =========================================================================== @@ -1872,6 +1884,12 @@ void App::renderDecryptWalletDialog() { state_.sync.rescanning = true; state_.sync.rescan_progress = 0.0f; + // The user chose to remove encryption: any wizard-era "encryption pending" request is moot + // (else the next connect would warn that encryption "did not complete") (C88). + if (settings_ && settings_->getEncryptionPending()) { + settings_->setEncryptionPending(false); + settings_->save(); + } // Clear encryption state early — vault/PIN removed now, // wallet file is already unencrypted services::WalletSecurityWorkflowExecutor::cleanupVaultAndPin([this]() { diff --git a/src/app_wizard.cpp b/src/app_wizard.cpp index 4b9d387..1fbd71e 100644 --- a/src/app_wizard.cpp +++ b/src/app_wizard.cpp @@ -1401,7 +1401,7 @@ void App::renderFirstRunWizard() { (pinEntered && pinOk) ? pinStr : std::string()); // Persist that encryption was requested (never the passphrase) so a quit/crash or // failed daemon connect before it applies isn't silent — reconciled on the next - // connect in refreshWalletEncryptionState (W2-2). Saved with the wizard state below. + // connect in App::reconcileEncryptionPending (W2-2). Saved with the wizard state below. settings_->setEncryptionPending(true); // Clear sensitive buffers diff --git a/src/data/wallet_security_reconcile.h b/src/data/wallet_security_reconcile.h new file mode 100644 index 0000000..54b7103 --- /dev/null +++ b/src/data/wallet_security_reconcile.h @@ -0,0 +1,23 @@ +#pragma once +#include + +namespace dragonx { +namespace data { + +enum class EncryptionPendingAction { None, Clear, Warn }; + +// What to do with the persisted "encryption requested in the wizard" flag, given ONE parsed getwalletinfo +// result. rpcOk must come from the parsed reply (a -28 warm-up failure or a throw is rpcOk=false => None); +// never derive it from WalletState (a 15 s timeout forces encrypted=false there). +inline EncryptionPendingAction decideEncryptionPending(bool flagSet, bool rpcOk, bool walletEncrypted, + bool deferredPending, bool encryptInProgress, + bool alreadyWarned) +{ + if (!flagSet || !rpcOk) return EncryptionPendingAction::None; + if (walletEncrypted) return EncryptionPendingAction::Clear; + if (deferredPending || encryptInProgress || alreadyWarned) return EncryptionPendingAction::None; + return EncryptionPendingAction::Warn; +} + +} // namespace data +} // namespace dragonx diff --git a/tests/test_phase4.cpp b/tests/test_phase4.cpp index 615111d..75d374e 100644 --- a/tests/test_phase4.cpp +++ b/tests/test_phase4.cpp @@ -71,6 +71,7 @@ #include "ui/notifications.h" #include "data/seed_migration_resume.h" #include "data/pin_vault_policy.h" +#include "data/wallet_security_reconcile.h" #include "util/address_validation.h" #include "util/seed_phrase.h" #include "util/local_time.h" @@ -1153,6 +1154,21 @@ void testIdleTracker() EXPECT_NEAR(idle.idleSeconds(t0 + std::chrono::seconds(10)), 0.0, 1e-9); } +void testEncryptionPendingPolicy() +{ + using namespace dragonx::data; + // flag, rpcOk, encrypted, deferred, inProgress, warned + EXPECT_TRUE(decideEncryptionPending(true, true, true, false, false, false) == EncryptionPendingAction::Clear); + EXPECT_TRUE(decideEncryptionPending(true, true, true, true, true, true ) == EncryptionPendingAction::Clear); // encrypted wins + EXPECT_TRUE(decideEncryptionPending(true, true, false, false, false, false) == EncryptionPendingAction::Warn); + EXPECT_TRUE(decideEncryptionPending(true, true, false, true, false, false) == EncryptionPendingAction::None); // deferred still queued + EXPECT_TRUE(decideEncryptionPending(true, true, false, false, true, false) == EncryptionPendingAction::None); // encryptwallet in flight + EXPECT_TRUE(decideEncryptionPending(true, true, false, false, false, true ) == EncryptionPendingAction::None); // once per session + EXPECT_TRUE(decideEncryptionPending(true, false, false, false, false, false) == EncryptionPendingAction::None); // -28 / failed RPC + EXPECT_TRUE(decideEncryptionPending(false, true, true, false, false, false) == EncryptionPendingAction::None); // nothing pending + EXPECT_TRUE(decideEncryptionPending(false, true, false, false, false, false) == EncryptionPendingAction::None); +} + void testRefreshSchedulerDeferFor() { using dragonx::services::RefreshScheduler; @@ -17497,6 +17513,7 @@ int main() testSpendableFiltering(); testRefreshScheduler(); testIdleTracker(); + testEncryptionPendingPolicy(); testRefreshSchedulerDeferFor(); testRpcBusyGate(); testIsLongWalletMethod();