Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 85 additions & 16 deletions crates/sage-apps/src/bridge/bridge_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ 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_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(
Expand Down Expand Up @@ -85,8 +87,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?;

Expand All @@ -102,7 +105,33 @@ 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 {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expiry boundary mismatch on resolve

Medium Severity

The new resolve-time expiry guard rejects only when now is strictly greater than expires_at_ms, while the background expiry loop already treats an approval as expired when expires_at_ms <= now. At the exact expiry timestamp, a late resolve(approved: true) can still run gated actions such as wallet.sendXch even though the loop considers the approval expired.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 78de31c. Configure here.

// 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 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,
app_state,
Expand All @@ -112,13 +141,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?;
Expand Down Expand Up @@ -168,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) => {
Expand Down Expand Up @@ -222,20 +252,59 @@ async fn execute_bridge_request(
}
}

async fn active_wallet_fingerprint(app_state: &State<'_, AppState>) -> Option<u32> {
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::<AppsHostState>();

// 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;

Expand Down
33 changes: 16 additions & 17 deletions crates/sage-apps/src/bridge/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pub(crate) async fn write_pending_approval(
registry_kind: BridgeRegistryKind,
approval: &RustBridgeApprovalRequest,
request: &RustBridgeRequest,
approved_fingerprint: Option<u32>,
) -> String {
let approval_id = Uuid::new_v4().to_string();
let now = unix_timestamp_ms() as u64;
Expand All @@ -40,29 +41,13 @@ pub(crate) async fn write_pending_approval(
request: request.clone(),
created_at_ms: now,
expires_at_ms: now + BRIDGE_APPROVAL_TIMEOUT_MS,
approved_fingerprint,
},
);

approval_id
}

pub(crate) async fn find_pending_approval(
apps_state: &State<'_, AppsHostState>,
approval_id: &str,
) -> Option<PendingBridgeApproval> {
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<PendingBridgeApproval, String> {
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<PendingBridgeApproval> {
Expand All @@ -79,6 +64,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<PendingBridgeApproval> {
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<String> {
use std::collections::BTreeSet;

Expand Down
5 changes: 5 additions & 0 deletions crates/sage-apps/src/bridge/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u32>,
}

impl RustBridgeInvokeResult {
Expand Down
Loading