From 11273081f3333f44692417fb6c9b521940614e0b Mon Sep 17 00:00:00 2001 From: Rigidity Date: Fri, 3 Jul 2026 18:44:03 -0400 Subject: [PATCH 1/2] Consume bridge approvals atomically and reject expired ones `process_after_approval` did a separate `get_pending_approval` then `remove_pending_approval`, each taking the lock independently. Two concurrent `bridgeApprovals.resolve` calls for the same approval id could both observe the record before either removed it, double-executing the gated action (e.g. wallet.sendXch / wallet.getSecretKey) from one user approval. Add `take_pending_approval`, which removes and returns the record under a single lock, and use it so an approval is consumed exactly once. Also reject resolution of an approval whose `expires_at_ms` has passed with an `approval_timeout` error, closing the window between expiry and the next tick of the background expiry loop. Remove the now-unused `get_pending_approval`/`find_pending_approval` helpers. --- crates/sage-apps/src/bridge/bridge_request.rs | 37 ++++++++++++------- crates/sage-apps/src/bridge/state.rs | 31 +++++++--------- 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/crates/sage-apps/src/bridge/bridge_request.rs b/crates/sage-apps/src/bridge/bridge_request.rs index b3c42f4ca..aa510ebf7 100644 --- a/crates/sage-apps/src/bridge/bridge_request.rs +++ b/crates/sage-apps/src/bridge/bridge_request.rs @@ -7,9 +7,10 @@ use crate::{ RustBridgeRequest, RustBridgeResponse, SharedSageApp, SystemBridgeCapability, UserBridgeCapability, assert_bridge_origin, emit_bridge_response_to_app, emit_system_runtime_event_to_listeners, ensure_app_is_enabled_for_scope, - ensure_approval_expiry_loop, get_pending_approval, get_system_capability_definition, - get_user_capability_definition, list_pending_approvals, remove_pending_approval, resolve_app, - start_bridge_approval_runtime, sync_bridge_approval_runtime, write_pending_approval, + ensure_approval_expiry_loop, get_system_capability_definition, + get_user_capability_definition, list_pending_approvals, resolve_app, + start_bridge_approval_runtime, sync_bridge_approval_runtime, take_pending_approval, + unix_timestamp_ms, write_pending_approval, }; pub(crate) async fn process( @@ -85,8 +86,9 @@ pub(crate) async fn process_after_approval( apps_state: &State<'_, AppsHostState>, args: ResolveBridgeApprovalArgs, ) -> Result<(), String> { - let pending = get_pending_approval(apps_state, &args.approval_id).await?; - remove_pending_approval(apps_state, &args.approval_id).await; + let pending = take_pending_approval(apps_state, &args.approval_id) + .await + .ok_or_else(|| format!("No pending approval with id {}", args.approval_id))?; sync_bridge_approval_runtime(app_handle, apps_state).await?; @@ -102,7 +104,23 @@ pub(crate) async fn process_after_approval( let origin = assert_bridge_origin(app_handle, &app.with_app(SharedSageApp::webview_label)).await?; - let invoke_result = if args.approved { + let invoke_result = if !args.approved { + RustBridgeInvokeResult::error( + &pending.request.id, + "user_denied", + args.reason + .unwrap_or_else(|| "User denied the request".to_string()), + ) + } else if unix_timestamp_ms() as u64 > pending.expires_at_ms { + // The approval expired before it was resolved. Reject rather than + // execute so a late `resolve(approved: true)` cannot run the gated + // action after its timeout window. + RustBridgeInvokeResult::error( + &pending.request.id, + "approval_timeout", + "Approval expired before it was resolved".to_string(), + ) + } else { process_shared( app_handle, app_state, @@ -112,13 +130,6 @@ pub(crate) async fn process_after_approval( true, ) .await? - } else { - RustBridgeInvokeResult::error( - &pending.request.id, - "user_denied", - args.reason - .unwrap_or_else(|| "User denied the request".to_string()), - ) }; emit_bridge_response_to_app(app_handle, &origin.app, &invoke_result.try_into()?).await?; diff --git a/crates/sage-apps/src/bridge/state.rs b/crates/sage-apps/src/bridge/state.rs index de89c9306..a1b054b92 100644 --- a/crates/sage-apps/src/bridge/state.rs +++ b/crates/sage-apps/src/bridge/state.rs @@ -46,23 +46,6 @@ pub(crate) async fn write_pending_approval( approval_id } -pub(crate) async fn find_pending_approval( - apps_state: &State<'_, AppsHostState>, - approval_id: &str, -) -> Option { - let pending = apps_state.bridge.pending_approvals.lock().await; - pending.get(approval_id).cloned() -} - -pub(crate) async fn get_pending_approval( - apps_state: &State<'_, AppsHostState>, - approval_id: &str, -) -> Result { - find_pending_approval(apps_state, approval_id) - .await - .ok_or_else(|| format!("No pending approval with id {approval_id}")) -} - pub(crate) async fn list_pending_approvals( apps_state: &State<'_, AppsHostState>, ) -> Vec { @@ -79,6 +62,20 @@ pub(crate) async fn remove_pending_approval( pending.remove(approval_id); } +/// Atomically removes and returns a pending approval under a single lock. +/// +/// This is the only correct way to consume an approval before executing it: +/// a separate `get` + `remove` allows two concurrent resolves of the same +/// `approval_id` to both observe the record and double-execute the gated +/// action (e.g. `wallet.sendXch` / `wallet.getSecretKey`). +pub(crate) async fn take_pending_approval( + apps_state: &State<'_, AppsHostState>, + approval_id: &str, +) -> Option { + let mut pending = apps_state.bridge.pending_approvals.lock().await; + pending.remove(approval_id) +} + pub(crate) async fn pending_approval_app_ids(apps_state: &State<'_, AppsHostState>) -> Vec { use std::collections::BTreeSet; From 78de31c5d6dde6f78c0ede3573b1f72556811577 Mon Sep 17 00:00:00 2001 From: Rigidity Date: Fri, 3 Jul 2026 18:54:17 -0400 Subject: [PATCH 2/2] Bind wallet.sendXch approval to the wallet active at request time A wallet.sendXch approval showed the user a transaction for the wallet that was active when the approval was requested, but execution ran against whatever wallet was active at resolve time. Switching wallets between request and approval therefore sent from a different wallet than the one the user reviewed. Record the active wallet fingerprint on the pending approval when it is created, and refuse to execute a sendXch approval if the active wallet no longer matches (error code wallet_changed). getSecretKey is unaffected because it targets an explicit fingerprint from its params. --- crates/sage-apps/src/bridge/bridge_request.rs | 64 ++++++++++++++++++- crates/sage-apps/src/bridge/state.rs | 2 + crates/sage-apps/src/bridge/types.rs | 5 ++ 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/crates/sage-apps/src/bridge/bridge_request.rs b/crates/sage-apps/src/bridge/bridge_request.rs index aa510ebf7..dd983ac5d 100644 --- a/crates/sage-apps/src/bridge/bridge_request.rs +++ b/crates/sage-apps/src/bridge/bridge_request.rs @@ -3,8 +3,9 @@ use tauri::{AppHandle, Manager, State, Webview}; use crate::{ AppState, AppsHostState, BridgeApprovalsChangedEvent, BridgeCapability, BridgeContext, BridgeMethod, BridgeMethodCapability, BridgeOrigin, BridgeRegistry, BridgeRegistryKind, - BridgeTools, ResolveBridgeApprovalArgs, RustBridgeApprovalRequest, RustBridgeInvokeResult, - RustBridgeRequest, RustBridgeResponse, SharedSageApp, SystemBridgeCapability, + BridgeTools, PendingBridgeApproval, ResolveBridgeApprovalArgs, RustBridgeApprovalBody, + RustBridgeApprovalRequest, RustBridgeInvokeResult, RustBridgeRequest, RustBridgeResponse, + SharedSageApp, SystemBridgeCapability, UserBridgeCapability, assert_bridge_origin, emit_bridge_response_to_app, emit_system_runtime_event_to_listeners, ensure_app_is_enabled_for_scope, ensure_approval_expiry_loop, get_system_capability_definition, @@ -120,6 +121,16 @@ pub(crate) async fn process_after_approval( "approval_timeout", "Approval expired before it was resolved".to_string(), ) + } else if wallet_binding_violated(app_state, &pending).await { + // Wallet-affecting approvals are bound to the wallet that was active + // (and shown to the user) when the approval was created. If the user + // switched wallets before resolving, refuse to execute rather than + // run the action against a different wallet. + RustBridgeInvokeResult::error( + &pending.request.id, + "wallet_changed", + "Active wallet changed since the approval was requested".to_string(), + ) } else { process_shared( app_handle, @@ -179,7 +190,15 @@ async fn process_shared( match method.approval_request(BridgeContext { app }, request) { Ok(Some(approval)) => { - request_approval(app_handle, app.id(), registry_kind, approval, request).await?; + request_approval( + app_handle, + app_state, + app.id(), + registry_kind, + approval, + request, + ) + .await?; Ok(RustBridgeInvokeResult::Pending {}) } Ok(None) => { @@ -233,20 +252,59 @@ async fn execute_bridge_request( } } +async fn active_wallet_fingerprint(app_state: &State<'_, AppState>) -> Option { + app_state + .lock() + .await + .wallet() + .map(|wallet| wallet.fingerprint) + .ok() +} + +/// Returns true if this approval is bound to a specific wallet and the active +/// wallet no longer matches it (or is no longer available). +async fn wallet_binding_violated( + app_state: &State<'_, AppState>, + pending: &PendingBridgeApproval, +) -> bool { + // Only methods that execute against the *active* wallet need binding. + // (`GetSecretKey` targets an explicit fingerprint from its params, so it is + // unaffected by wallet switches.) + let requires_wallet_binding = + matches!(pending.approval.body, RustBridgeApprovalBody::SendXch { .. }); + + if !requires_wallet_binding { + return false; + } + + let Some(approved_fingerprint) = pending.approved_fingerprint else { + return true; + }; + + active_wallet_fingerprint(app_state).await != Some(approved_fingerprint) +} + async fn request_approval( app_handle: &AppHandle, + app_state: &State<'_, AppState>, app_id: String, registry_kind: BridgeRegistryKind, approval: RustBridgeApprovalRequest, request: &RustBridgeRequest, ) -> Result<(), String> { let apps_state = app_handle.state::(); + + // Record which wallet was active when the approval was created, so + // wallet-affecting approvals can be bound to the wallet the user saw. + let approved_fingerprint = active_wallet_fingerprint(app_state).await; + write_pending_approval( &apps_state, app_id.clone(), registry_kind, &approval, request, + approved_fingerprint, ) .await; diff --git a/crates/sage-apps/src/bridge/state.rs b/crates/sage-apps/src/bridge/state.rs index a1b054b92..ba9664236 100644 --- a/crates/sage-apps/src/bridge/state.rs +++ b/crates/sage-apps/src/bridge/state.rs @@ -26,6 +26,7 @@ pub(crate) async fn write_pending_approval( registry_kind: BridgeRegistryKind, approval: &RustBridgeApprovalRequest, request: &RustBridgeRequest, + approved_fingerprint: Option, ) -> String { let approval_id = Uuid::new_v4().to_string(); let now = unix_timestamp_ms() as u64; @@ -40,6 +41,7 @@ pub(crate) async fn write_pending_approval( request: request.clone(), created_at_ms: now, expires_at_ms: now + BRIDGE_APPROVAL_TIMEOUT_MS, + approved_fingerprint, }, ); diff --git a/crates/sage-apps/src/bridge/types.rs b/crates/sage-apps/src/bridge/types.rs index bcb81fd80..8f046e3fc 100644 --- a/crates/sage-apps/src/bridge/types.rs +++ b/crates/sage-apps/src/bridge/types.rs @@ -105,6 +105,11 @@ pub(crate) struct PendingBridgeApproval { pub request: RustBridgeRequest, pub created_at_ms: u64, pub expires_at_ms: u64, + /// Fingerprint of the active wallet when this approval was created. Used to + /// bind wallet-affecting approvals (e.g. `wallet.sendXch`) to the wallet the + /// user actually saw, so switching wallets before resolving cannot redirect + /// the action to a different wallet. `None` if no wallet was active. + pub approved_fingerprint: Option, } impl RustBridgeInvokeResult {