From 9aca69303495b3806949d080b207fd7e71b6e6ad Mon Sep 17 00:00:00 2001 From: codaMW Date: Wed, 12 Aug 2026 10:30:37 +0200 Subject: [PATCH 1/2] fix(#273): clear trades, messages and sessions on identity deletion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generating a new user rotated the identity but left the previous user's data behind: delete_identity() cleared the identity row and trade-key mappings but not the trades table, the messages table, or the in-memory sessions, so the new identity inherited the old one's My Trades list and chats — a privacy issue, and dead state (the trade keys were already cleared). Add clear_trades / clear_messages to the DB trait (SQLite implemented; IndexedDB stubbed alongside the existing clear_trade_keys pending #233) and a SessionManager::clear_all(). Call them from delete_identity() — messages before trades for the FK — and empty the in-memory sessions. On the Dart side, invalidate rawTradesProvider after regenerate() so My Trades (and the chat rooms derived from it) reflect the clean slate immediately. Verified on a physical device (Nokia C31): after Generate New User, My Trades and chats are empty. Adds a SQLite test that the clears empty both tables. --- .../account/screens/account_screen.dart | 7 ++ rust/src/api/identity.rs | 20 ++++++ rust/src/db/indexeddb.rs | 8 +++ rust/src/db/mod.rs | 10 +++ rust/src/db/sqlite.rs | 65 +++++++++++++++++++ rust/src/mostro/session.rs | 10 +++ 6 files changed, 120 insertions(+) diff --git a/lib/features/account/screens/account_screen.dart b/lib/features/account/screens/account_screen.dart index 41b9dcd8..4b301faa 100644 --- a/lib/features/account/screens/account_screen.dart +++ b/lib/features/account/screens/account_screen.dart @@ -7,6 +7,7 @@ import 'package:mostro/core/app_routes.dart'; import 'package:mostro/core/app_theme.dart'; import 'package:mostro/core/services/identity_service.dart'; import 'package:mostro/features/account/providers/backup_reminder_provider.dart'; +import 'package:mostro/features/trades/providers/trades_providers.dart'; import 'package:mostro/features/account/providers/privacy_mode_provider.dart'; import 'package:mostro/features/account/widgets/backup_trigger_sheet.dart'; import 'package:mostro/l10n/app_localizations.dart'; @@ -386,6 +387,12 @@ class _AccountScreenState extends ConsumerState { // where the user is left without a valid identity. await IdentityService.regenerate(); ref.read(sessionProvider.notifier).clearSession(); + // The new identity starts with an empty trade DB (the Rust + // side clears trades, messages and sessions on regenerate, + // issue #273); drop the cached list so My Trades reflects + // the clean slate immediately instead of showing the + // previous identity's orders until the next refresh. + ref.invalidate(rawTradesProvider); await ref .read(backupReminderProvider.notifier) .showBackupReminder(); diff --git a/rust/src/api/identity.rs b/rust/src/api/identity.rs index b7e4f600..6d53fa70 100644 --- a/rust/src/api/identity.rs +++ b/rust/src/api/identity.rs @@ -304,6 +304,26 @@ pub async fn delete_identity() -> Result<()> { if let Err(e) = db.clear_trade_keys().await { log::warn!("[identity] failed to clear trade key mappings: {e}"); } + // Messages before trades: messages.trade_id is an FK onto trades(id). + // Both belong to the deleted identity — their trade keys were just + // cleared, so the rows are dead state a new identity must not inherit + // (privacy: a fresh user must not see the previous one's My Trades or + // chats). See issue #273. + if let Err(e) = db.clear_messages().await { + log::warn!("[identity] failed to clear messages: {e}"); + } + if let Err(e) = db.clear_trades().await { + log::warn!("[identity] failed to clear trades: {e}"); + } + } + + // Empty the in-memory sessions too: they key decryption for the deleted + // identity's trades and must not carry into the new one. + let dropped = crate::mostro::session::session_manager().clear_all().await; + if dropped > 0 { + log::debug!( + "[identity] cleared {dropped} in-memory session(s) on identity deletion" + ); } // Last, so the cleanup warnings above are dropped too: buffered lines name diff --git a/rust/src/db/indexeddb.rs b/rust/src/db/indexeddb.rs index b7dccc35..f604d79e 100644 --- a/rust/src/db/indexeddb.rs +++ b/rust/src/db/indexeddb.rs @@ -240,6 +240,14 @@ impl Storage for IndexedDbStorage { Ok(()) // IndexedDB not yet implemented (#233) } + async fn clear_trades(&self) -> Result<()> { + Ok(()) // IndexedDB not yet implemented (#233) + } + + async fn clear_messages(&self) -> Result<()> { + Ok(()) // IndexedDB not yet implemented (#233) + } + // ── Settings KV — fully implemented (chat cursor + preferences, #246) ─── async fn get_setting(&self, key: &str) -> Result> { diff --git a/rust/src/db/mod.rs b/rust/src/db/mod.rs index daa439df..a13d7603 100644 --- a/rust/src/db/mod.rs +++ b/rust/src/db/mod.rs @@ -130,6 +130,16 @@ pub trait Storage: Send + Sync { /// order→index mappings belong to the removed identity's derivation tree. async fn clear_trade_keys(&self) -> Result<()>; + /// Delete ALL trade rows. Used on identity deletion: the recovered trade + /// history belongs to the removed identity, and its trade keys are cleared + /// alongside, so the rows are dead state a new identity must not inherit. + async fn clear_trades(&self) -> Result<()>; + + /// Delete ALL chat message rows. Used on identity deletion: the peer and + /// admin conversations belong to the removed identity and must not leak + /// into the fresh one. + async fn clear_messages(&self) -> Result<()>; + // ── Settings / Mostro node ──────────────────────────────────────────────── /// Read a value from the generic key-value settings store, or `None` when diff --git a/rust/src/db/sqlite.rs b/rust/src/db/sqlite.rs index 42eb8f55..d347a459 100644 --- a/rust/src/db/sqlite.rs +++ b/rust/src/db/sqlite.rs @@ -456,6 +456,20 @@ impl Storage for SqliteStorage { Ok(()) } + async fn clear_trades(&self) -> Result<()> { + sqlx::query("DELETE FROM trades") + .execute(&self.pool) + .await?; + Ok(()) + } + + async fn clear_messages(&self) -> Result<()> { + sqlx::query("DELETE FROM messages") + .execute(&self.pool) + .await?; + Ok(()) + } + async fn get_setting(&self, key: &str) -> Result> { let row: Option<(String,)> = sqlx::query_as("SELECT value FROM settings WHERE key = ?") @@ -1064,4 +1078,55 @@ mod tests { drop(storage); let _ = std::fs::remove_file(&path); } + + #[tokio::test] + async fn clear_trades_and_messages_empty_both_tables() { + // Identity deletion must wipe the previous user's trade history and + // chats (privacy, issue #273), not just the keys. Insert one trade and + // one message, clear both tables, and confirm nothing survives. + let path = temp_db_path(); + let storage = SqliteStorage::open(path.to_str().unwrap()).await.unwrap(); + sqlx::query("INSERT INTO trades VALUES ('t1', '{}', 'Active', 1, NULL)") + .execute(&storage.pool) + .await + .unwrap(); + sqlx::query( + "INSERT INTO messages (id, trade_id, data, is_read, created_at) \ + VALUES ('m1', 't1', '{}', 0, 1)", + ) + .execute(&storage.pool) + .await + .unwrap(); + + // Precondition: one row in each table. + let trades: (i64,) = sqlx::query_as("SELECT COUNT(*) FROM trades") + .fetch_one(&storage.pool) + .await + .unwrap(); + assert_eq!(trades.0, 1); + assert!(storage.message_exists("m1").await.unwrap()); + + storage.clear_messages().await.unwrap(); + storage.clear_trades().await.unwrap(); + + // Both tables are empty afterwards. + let trades: (i64,) = sqlx::query_as("SELECT COUNT(*) FROM trades") + .fetch_one(&storage.pool) + .await + .unwrap(); + let messages: (i64,) = sqlx::query_as("SELECT COUNT(*) FROM messages") + .fetch_one(&storage.pool) + .await + .unwrap(); + assert_eq!(trades.0, 0); + assert_eq!(messages.0, 0); + assert!(!storage.message_exists("m1").await.unwrap()); + + // Clearing again on empty tables is a no-op, not an error. + storage.clear_messages().await.unwrap(); + storage.clear_trades().await.unwrap(); + + drop(storage); + let _ = std::fs::remove_file(&path); + } } diff --git a/rust/src/mostro/session.rs b/rust/src/mostro/session.rs index 467c634c..41e88575 100644 --- a/rust/src/mostro/session.rs +++ b/rust/src/mostro/session.rs @@ -169,6 +169,16 @@ impl SessionManager { }); before - sessions.len() } + + /// Drop every in-memory session. Used on identity deletion: the sessions + /// belong to the removed identity's trades, and a new identity must not + /// inherit them. Returns the number of sessions dropped. + pub async fn clear_all(&self) -> usize { + let mut sessions = self.sessions.write().await; + let dropped = sessions.len(); + sessions.clear(); + dropped + } } // ── Global singleton ──────────────────────────────────────────────────────── From 6a8bbbb7fc2658d756e4a13afce20c9c824ef7be Mon Sep 17 00:00:00 2001 From: codaMW Date: Wed, 12 Aug 2026 11:11:02 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix(#273):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20propagate=20cleanup=20errors,=20clear=20web=20messages,=20te?= =?UTF-8?q?st=20invalidation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - delete_identity() now propagates clear_messages/clear_trades errors instead of logging and returning Ok. These tables are not identity-scoped and have no reconcile fallback, so a silent failure would leak the previous identity's history; propagating aborts regenerate/importAndStore before the replacement identity is created (deleteIdentity runs before the new identity exists). - IndexedDB clear_messages now clears MESSAGES_STORE in a read-write transaction rather than no-op'ing: messages are persisted on web (save_message), so identity deletion must actually wipe them. clear_trades stays a no-op (no web trades store yet, #233). - Add a provider test: invalidating rawTradesProvider after the DB is cleared yields an empty list (the cache reset the account screen relies on). --- rust/src/api/identity.rs | 21 +++++++++++----- rust/src/db/indexeddb.rs | 15 +++++++++-- .../trades/filtered_trades_provider_test.dart | 25 +++++++++++++++++++ 3 files changed, 53 insertions(+), 8 deletions(-) diff --git a/rust/src/api/identity.rs b/rust/src/api/identity.rs index 6d53fa70..517409bb 100644 --- a/rust/src/api/identity.rs +++ b/rust/src/api/identity.rs @@ -309,12 +309,21 @@ pub async fn delete_identity() -> Result<()> { // cleared, so the rows are dead state a new identity must not inherit // (privacy: a fresh user must not see the previous one's My Trades or // chats). See issue #273. - if let Err(e) = db.clear_messages().await { - log::warn!("[identity] failed to clear messages: {e}"); - } - if let Err(e) = db.clear_trades().await { - log::warn!("[identity] failed to clear trades: {e}"); - } + // + // Unlike the identity row and trade-key mappings above (which have the + // `reconcile_trade_key_index` pubkey guard as a fallback), the trades + // and messages tables are NOT identity-scoped and have no such guard: + // a silent failure here would leak the previous identity's history into + // the next one. Propagate the error so the caller (regenerate / + // importAndStore) aborts before creating the replacement identity — + // delete_identity runs before the new identity exists, so there is no + // half-rotated state to unwind. + db.clear_messages() + .await + .map_err(|e| anyhow!("failed to clear messages on identity deletion: {e}"))?; + db.clear_trades() + .await + .map_err(|e| anyhow!("failed to clear trades on identity deletion: {e}"))?; } // Empty the in-memory sessions too: they key decryption for the deleted diff --git a/rust/src/db/indexeddb.rs b/rust/src/db/indexeddb.rs index f604d79e..cd5e7d4f 100644 --- a/rust/src/db/indexeddb.rs +++ b/rust/src/db/indexeddb.rs @@ -241,11 +241,22 @@ impl Storage for IndexedDbStorage { } async fn clear_trades(&self) -> Result<()> { - Ok(()) // IndexedDB not yet implemented (#233) + Ok(()) // No trades store on web yet (#233); nothing to clear. } async fn clear_messages(&self) -> Result<()> { - Ok(()) // IndexedDB not yet implemented (#233) + // Messages ARE persisted on web (save_message writes MESSAGES_STORE), + // so identity deletion must actually wipe them, not no-op (#273). + let db = self.open_db().await?; + let tx = db + .transaction_on_one_with_mode(MESSAGES_STORE, IdbTransactionMode::Readwrite) + .map_err(|e| js_err("tx open", e))?; + let store = tx + .object_store(MESSAGES_STORE) + .map_err(|e| js_err("store open", e))?; + store.clear().map_err(|e| js_err("clear", e))?; + tx.await.into_result().map_err(|e| js_err("tx commit", e))?; + Ok(()) } // ── Settings KV — fully implemented (chat cursor + preferences, #246) ─── diff --git a/test/features/trades/filtered_trades_provider_test.dart b/test/features/trades/filtered_trades_provider_test.dart index e7bf4116..098be204 100644 --- a/test/features/trades/filtered_trades_provider_test.dart +++ b/test/features/trades/filtered_trades_provider_test.dart @@ -95,4 +95,29 @@ void main() { }); }); }); + + group('rawTradesProvider invalidation (regenerate clean slate)', () { + test('invalidating after the DB is cleared yields an empty list', () async { + // The account screen calls ref.invalidate(rawTradesProvider) after + // IdentityService.regenerate(); the Rust side has cleared the trades DB + // by then (issue #273). Model that: a fetcher that first returns the old + // identity's trades, then — once "cleared" — returns nothing. + var trades = [ + fakeTrade(id: 'old', status: OrderStatus.active), + ]; + final container = createContainer(overrides: [ + rawTradesProvider.overrideWith((ref) async => trades), + ]); + + // Before regenerate: My Trades shows the previous identity's trades. + expect(await container.read(rawTradesProvider.future), isNotEmpty); + + // delete_identity() cleared the DB; the account screen invalidates. + trades = []; + container.invalidate(rawTradesProvider); + + // After: the cached list is dropped and the empty DB is reflected. + expect(await container.read(rawTradesProvider.future), isEmpty); + }); + }); }