From b0624357cea1624d1b7229db85697cd12a86becd Mon Sep 17 00:00:00 2001 From: james2177 Date: Sun, 30 Aug 2026 18:49:03 +0100 Subject: [PATCH] feat(signing): one-shot signing protocol with durable state machine Replaces the page-message/runtime-callback signing flow with a versioned protocol: the background now generates the sole authoritative request id (never trusting the page/bridge's own correlation token), persists pending-request identity/binding/deadline to chrome.storage.session before opening any UI, and binds every decision to its originating tab/frame/ document and to the popup's own window. Closes #124. See docs/adr/0004-one-shot-signing-protocol.md for the full design. Claude-Session: https://claude.ai/code/session_01EpN7pg6WCWSV8iwYrjFJtB --- docs/adr/0004-one-shot-signing-protocol.md | 88 +++ docs/threat-model.md | 32 +- e2e/signTransaction.spec.ts | 69 ++- scripts/build-extension.mjs | 13 +- src/background/background.test.ts | 595 +++++++++++++++++---- src/background/background.ts | 447 +++++++++++++--- src/background/messageValidation.test.ts | 128 +++-- src/background/messageValidation.ts | 48 +- src/diagnostics/diagnostics.ts | 3 + src/intercept/albedoMainWorldEntry.ts | 22 +- src/intercept/awaitOutcome.test.ts | 106 ++++ src/intercept/awaitOutcome.ts | 49 ++ src/intercept/bridgeEntry.ts | 73 ++- src/intercept/mainWorldEntry.ts | 21 +- src/intercept/protocol.ts | 52 +- src/intercept/requestOutcome.test.ts | 85 +++ src/intercept/requestOutcome.ts | 63 +++ src/intercept/resolveOutcome.ts | 34 +- src/popup/App.test.tsx | 184 ++++--- src/popup/App.tsx | 94 ++-- src/signing/pendingRequestState.test.ts | 242 +++++++++ src/signing/pendingRequestState.ts | 191 +++++++ 22 files changed, 2226 insertions(+), 413 deletions(-) create mode 100644 docs/adr/0004-one-shot-signing-protocol.md create mode 100644 src/intercept/awaitOutcome.test.ts create mode 100644 src/intercept/awaitOutcome.ts create mode 100644 src/intercept/requestOutcome.test.ts create mode 100644 src/intercept/requestOutcome.ts create mode 100644 src/signing/pendingRequestState.test.ts create mode 100644 src/signing/pendingRequestState.ts diff --git a/docs/adr/0004-one-shot-signing-protocol.md b/docs/adr/0004-one-shot-signing-protocol.md new file mode 100644 index 0000000..d20aa8f --- /dev/null +++ b/docs/adr/0004-one-shot-signing-protocol.md @@ -0,0 +1,88 @@ +# ADR-0004: One-shot signing protocol with a durable state machine + +- **Status:** Accepted +- **Date:** 2026-08-30 +- **Decision owners:** Gryd Lock maintainers +- **Related issue/PR:** epic — replace the best-effort page-message/runtime-callback signing flow +- **Supersedes:** None (extends ADR-0003, which explicitly deferred this work: "Browser interception retains its existing request/decision contract") +- **Superseded by:** None + +## Context + +Before this change, the request/decision contract had five structural gaps, all confirmed against the code, not just the original audit: + +1. `mainWorldEntry.ts`/`albedoMainWorldEntry.ts` generated `localId = crypto.randomUUID()` and `bridgeEntry.ts` forwarded it to the background as `requestId`. The background used that page-adjacent id as the sole key for `pendingDecisions` and embedded it directly in the popup URL — a page-controlled identifier was the authoritative capability end to end. +2. `DECISION_MADE` resolved purely by `pendingDecisions.get(requestId)?.resolve(...)` — no sender, tab, frame, or window check at all. +3. `pendingDecisions` was an in-memory `Map`; a worker restart mid-review silently dropped it, and `DEFAULT_TIMEOUT_MS` was defined but never referenced. +4. There was no `chrome.windows.onRemoved` handler, so closing the popup hung the dApp's promise indefinitely, and `chrome.windows.create` was called without a callback, so a creation failure was unobservable. +5. Nothing bounded concurrent pending requests, so a malicious page could open unbounded popups. + +Two duplicate `requestOutcome()` implementations (Freighter and Albedo) also had no page-side deadline, so a missing/stale bridge (e.g. after an extension update) could hang the page's promise forever. + +## Decision drivers + +- A decision must be bound to one transaction digest, tab, frame, document, adapter, and review window — not a bare id. +- A page-controlled identifier must never be an authoritative capability. +- The first valid terminal transition wins; replay must be a no-op, not an error and not a re-release. +- A worker restart must not release a cancelled request or lose a completed decision, and must not need to re-run any work that already committed. +- Popup close, popup-creation failure, tab navigation/closure, and timeout must all settle deterministically and quickly. +- Keep the existing message names/shapes where they already work (`DECISION_MADE`, `GET_REVIEW`) and change *authority*, not vocabulary, to keep the diff reviewable and avoid touching the page-visible Freighter/Albedo protocols at all. + +## Considered options + +### Option A: Patch the existing map (add sender checks, add a timeout, add `windows.onRemoved`) + +This closes individual gaps but leaves the root problem: the background still has no source of truth beyond a live JS closure. A worker restart during review is still unrecoverable, because the fix would still be "hold the whole review open behind one message port," which is exactly what makes worker suspension dangerous in the first place. + +### Option B: Versioned protocol + durable state machine, with an opaque extension-generated capability doubling as the request id + +Separate the page/bridge correlation id (`localId`, never forwarded past the bridge) from a background-generated `requestId` that is the only authoritative identifier. Persist a serializable projection of pending-request state to `chrome.storage.session` (memory-only, cleared on browser close — same mechanism already used for protection state) *before* opening any UI. Replace the single blocking `SIGN_REQUEST` → decision round trip with two phases: an immediate `SIGN_ACK`/`SIGN_REJECTED` that closes the first message port quickly, followed by a `AWAIT_OUTCOME` resume/status handshake the bridge can retry indefinitely (with backoff) against a durable answer, so a worker restart mid-review is invisible to the bridge beyond one retry. + +### Option C: A separate opaque capability token distinct from the request id + +Scope C ("pass only an opaque capability in the review URL") could be read as requiring a third identifier beyond `requestId`. We chose not to introduce one: `requestId` is already background-generated, high-entropy, never sent to the page, and length-bounded — it already satisfies "opaque, not page-controlled." Adding a second token would double the state to track for no additional guarantee. This is a deliberate simplification, called out explicitly rather than left implicit. + +## Decision + +Use option B, with the option C simplification. + +**Versioned protocol** (`src/intercept/protocol.ts`, `src/background/messageValidation.ts`): every runtime message carries `protocolVersion`; a mismatch gets a typed `SIGN_REJECTED { reason: 'protocol-incompatible' }`, never a silent bypass. `SIGN_REQUEST` has no `requestId` field at all — the validator rejects a message that includes one, so a page cannot even attempt to smuggle an authoritative id in. + +**State machine** (`src/signing/pendingRequestState.ts`, pure, mirrors `src/protection/protectionState.ts`'s existing split of pure state from `chrome.*` wiring): `received → validating → assessing → awaiting_review → {proceed | cancel | expired | failed}`. Terminal states are sticky — `applyTransition` treats a repeat of the *same* terminal state as an idempotent no-op (replay is safe) and rejects any *different* terminal transition once settled (first valid terminal transition wins). Terminal records are kept as tombstones for `TOMBSTONE_TTL_MS` so a retried `AWAIT_OUTCOME` or a duplicate `DECISION_MADE` resolves from cache instead of hitting "unknown request." + +**Durable pending repository**: `background.ts` persists the record (not the resolver — resolvers cannot survive suspension) to `chrome.storage.session` before any decode/scoring/UI work. On restart, `restorePendingRequests()` reloads it, settling anything already past its deadline. `handleAwaitOutcome` is the resume handshake: it always answers from durable state, so it behaves identically whether the record was created this worker lifetime or restored from a previous one — no special-casing needed. + +**Trusted review capability**: the popup URL carries only `requestId`. `windowId` is bound on first legitimate contact (TOFU) from `sender.tab.windowId` — safe because `requestId` is the actual secret (unguessable, never page-visible), so only a page holding the correct popup URL can reach that code path at all. Every later `GET_REVIEW`/`DECISION_MADE` must match the bound `windowId` exactly; a mismatch is a silent no-op (no review content, no decision) rather than an error that would confirm the request's existence to a prober. + +**Failure and concurrency semantics**: `chrome.windows.onRemoved` settles an open request as `cancel`. `chrome.windows.create`'s callback is now used (previously ignored) to detect creation failure (`chrome.runtime.lastError`) and settle `failed`. The existing `chrome.tabs.onUpdated`/`onRemoved` listeners (already used for protection-state invalidation) are extended to also cancel pending signing requests for that tab. `admissionCheck` enforces `MAX_PENDING_PER_FRAME` (5) and `MAX_PENDING_GLOBAL` (50) *before* a popup is created. Deadlines are layered — background's `REVIEW_DEADLINE_MS` (90s) is authoritative and checked eagerly at every touchpoint, not solely via a timer; the bridge's `BRIDGE_FALLBACK_DEADLINE_MS` (100s) covers a missing/unresponsive background; the page's `PAGE_DEADLINE_MS` (120s) covers a missing/unresponsive bridge. + +**Compatibility**: a version mismatch produces a typed rejection. Missing `sender.documentId` (older Chrome) is recorded as `documentBound: false` and falls back to tab/frame-only binding — a reduced-but-still-checked guarantee, not silent bypass. + +## Consequences + +### Positive + +- Every non-negotiable invariant in the source issue is now enforced by code, not by convention: digest/tab/frame/document/adapter/window binding, replay-as-no-op, restart survival, deterministic settlement, bounded state. +- `mainWorldEntry.ts`'s and `albedoMainWorldEntry.ts`'s duplicated `requestOutcome()` helper is now shared (`src/intercept/requestOutcome.ts`), removing ~40 lines of duplication and adding the page-side deadline once instead of twice. +- `DEFAULT_TIMEOUT_MS` is now the real, wired-up deadline. + +### Negative + +- Review content (XDR-derived facts) is intentionally *not* persisted to `chrome.storage.session` — only the pending-request record is. A worker restart during an open review therefore preserves the request's identity/binding/deadline (so it settles safely) but not necessarily its displayed content if the popup needs to re-fetch. See Known Gaps in the PR description. +- Content-script bundle budgets (`scripts/build-extension.mjs`) were raised (mainWorld.js 5.0→5.5 KB, albedoMainWorld.js 6.0→6.5 KB, bridge.js 4.0→5.5 KB) to accommodate the shared helper and the resume/retry loop. Confirmed via `esbuild`'s bundle output that the growth is real code (esbuild strips comments during bundling regardless of the `minify` setting), not incidental bloat. + +### Neutral or follow-up work + +- Full adversarial coverage (forced service-worker termination via CDP, 1,000-concurrent-request load test, non-Chromium `documentId`-less browser matrix) is out of scope for this PR; see Known Gaps. + +## Security and privacy considerations + +XDR and review content remain extension-internal; the popup URL still carries only an opaque, background-generated id (never review content, never a page-supplied value). `chrome.storage.session` is memory-only and cleared on browser close, consistent with the existing protection-state persistence and ADR-0003's XDR-out-of-URL decision. Fixed alongside this change: `InterceptView` previously fell back to a URL-supplied `score=0` ("low risk") when review data failed to load — a real, if narrow, "predictable degradation" violation (threat-model.md objective 7) that a restart-survival feature would otherwise make easier to hit. It now fails closed to a reject-only state. + +## Validation + +Unit: `src/signing/pendingRequestState.test.ts` (state machine, admission control, tombstone pruning, adversarial storage-deserialization), `src/background/background.test.ts` (26 tests covering the full protocol including wrong-window rejection, popup-close, popup-creation-failure, worker-restart resume, replay-is-a-no-op, admission limits, tab invalidation), `src/intercept/awaitOutcome.test.ts` (resume/backoff/deadline logic, DI-based, no real timers), `src/intercept/requestOutcome.test.ts`, `src/background/messageValidation.test.ts`. E2E (`e2e/signTransaction.spec.ts`, real Chromium): all 6 pre-existing scenarios plus 2 new ones — popup close settles cancel without hanging the dApp, and two concurrent requests from the same page open two popups and settle independently without colliding. `npm run lint`, `npm run typecheck`, `npm run test:coverage` (92.25%/83.67%/90.96%/92.25%, threshold 90/80/80/90), `npm run validate:manifest`, and `npm run build` all pass on a clean `npm ci`. + +## Revisit criteria + +Revisit when: a live/CDP-based forced-worker-termination test harness becomes available in CI (close the "worker restart preserves review content" gap for real, not just at the identity/binding layer); a load-testing harness for the 1,000-concurrent-request and 50-global-admission acceptance criteria is added; a browser without `sender.documentId` support needs to be supported in practice (currently only unit-tested as a fallback path); or an external security review of this ADR identifies a binding this design does not cover. diff --git a/docs/threat-model.md b/docs/threat-model.md index b3f2296..27d93a7 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -2,7 +2,7 @@ - **Status:** Living document - **Applies to:** extension version `0.1.0` and current `main` -- **Last reviewed:** 2026-08-23 +- **Last reviewed:** 2026-08-30 This document describes the security properties of the current Freighter-first implementation. It separates intended guarantees from assumptions, known limitations, and hardening work that is still open. @@ -12,7 +12,9 @@ Gryd Lock attempts to intercept a Freighter `SUBMIT_TRANSACTION` request before The extension is an advisory layer, not a wallet, signer, transaction firewall, oracle, antivirus product, or guarantee that a destination is safe. It does not hold private keys and cannot secure a compromised wallet, browser, operating system, dApp, or risk-data source. -The current build also has known hardening gaps. In particular, the page-visible internal `postMessage` protocol is not yet origin- or session-authenticated, interception is registration-order dependent, pending decisions are stored only in memory, and the manifest currently injects scripts on ``. These gaps are tracked in public issues and are not presented as solved controls. +The current build also has known hardening gaps. In particular, the page-visible internal `postMessage` protocol is not yet origin-authenticated, interception is registration-order dependent, and the manifest currently injects scripts on ``. These gaps are tracked in public issues and are not presented as solved controls. + +As of the one-shot signing protocol (ADR-0004), pending sign-decision state is durable (`chrome.storage.session`, memory-only) rather than in-memory-only, and the background — not the page-visible `postMessage` correlation id — generates the sole authoritative request identifier. See ADR-0004 and the risk register below for what changed and what remains open. ## Security objectives @@ -79,19 +81,19 @@ The page and `mainWorldEntry.ts` share the same JavaScript world and `window` me ### Boundary 2: MAIN world ↔ isolated-world bridge -The interceptor posts `{ type, requestId, xdr }` to `window`; `bridgeEntry.ts` relays it through `chrome.runtime.sendMessage`. The response returns over the same page-visible bus. +The interceptor posts `{ type, protocolVersion, localId, xdr, adapter }` to `window` (`WINDOW_REQUEST_TYPE`, target origin `'*'`); `bridgeEntry.ts` relays the XDR through `chrome.runtime.sendMessage` as a `SIGN_REQUEST`. `localId` is a same-window postMessage correlation token only: as of ADR-0004, `bridgeEntry.ts` never forwards it to the background, and the background never sees or trusts it. The background generates its own `requestId` (returned in `SIGN_ACK`) and that is the only identifier ever treated as authoritative for pending state or popup URLs. -Current messages are not cryptographically authenticated or bound to an isolated-world session secret. Outbound internal messages also use `'*'` in some paths. Origin validation and session binding are tracked in issues `#1` and `#4`. +Messages are still not cryptographically authenticated or bound to an isolated-world session secret, and outbound internal messages still use `'*'` for the initial request (the response leg uses the resolved page origin). A page script sharing `window` can still observe this traffic and could, at most, forge a *request* (which only causes an unwanted extra review popup, gated by the per-frame/global admission limits in ADR-0004) — it cannot forge a *decision*, since decisions are bound to the background-generated `requestId` and the popup's own `windowId`, neither of which the page ever sees. Origin authentication for the request leg itself remains open and is tracked in issue `#1`. ### Boundary 3: isolated world → MV3 background service worker -`chrome.runtime` provides an extension-controlled channel, but the background listener currently trusts the typed shape of incoming messages and does not use sender metadata to enforce an expected content script, extension page, tab, or frame. A future hardening pass should validate message fields and sender context before creating state or resolving decisions. +`chrome.runtime` provides an extension-controlled channel. As of ADR-0004, the background validates message shape *and* protocol version (a mismatch gets a typed `SIGN_REJECTED`, not a silent bypass) and uses `sender.tab.id`/`sender.frameId`/`sender.documentId` to bind every pending sign request to its originating tab/frame/document; a tab closing or navigating away invalidates its pending requests (`chrome.tabs.onUpdated`/`onRemoved`). Per-frame and global admission limits (`MAX_PENDING_PER_FRAME`, `MAX_PENDING_GLOBAL` in `src/signing/pendingRequestState.ts`) bound how many concurrent requests a single frame or the whole extension will admit before opening a popup. ### Boundary 4: background service worker → warning popup -The background worker creates an extension popup URL containing only `mode=intercept` and an opaque `requestId`. The popup requests the worker-resident review over extension runtime messaging. React renders it as text and sends a `DECISION_MADE` runtime message. +The background worker creates an extension popup URL containing only `mode=intercept` and an opaque, background-generated `requestId`. The popup requests the worker-resident review over extension runtime messaging (`GET_REVIEW`) and sends a `DECISION_MADE` runtime message to resolve it. -React's default escaping reduces direct HTML injection risk. XDR, memos, amounts, sources, targets, scores, and findings are absent from the URL; bounded rendering limits apply to XDR, operations, facts, rendered values, and Soroban authorization traversal. Request authenticity, stale popup, and decision-binding risks remain. +React's default escaping reduces direct HTML injection risk. XDR, memos, amounts, sources, targets, scores, and findings are absent from the URL; bounded rendering limits apply to XDR, operations, facts, rendered values, and Soroban authorization traversal. As of ADR-0004: the popup's own `windowId` (from `chrome.windows.create`) is bound to the pending request on first legitimate contact and checked on every subsequent `GET_REVIEW`/`DECISION_MADE`, so a copied review URL opened in a different window reveals no review content and grants no decision authority; a decision is only honored while the request is in the `awaiting_review` state, so a replayed or premature `DECISION_MADE` is a no-op; closing the popup (`chrome.windows.onRemoved`) or a popup-creation failure both settle the request deterministically (cancelled/failed) instead of hanging; and pending-request identity/binding/deadline state is durable (`chrome.storage.session`) across a worker restart, so a restart mid-review does not lose the ability to resolve exactly once. Review *content* itself is not persisted to storage — only the pending-request record is — so a worker restart can still leave a reopened/reloaded popup unable to redisplay the review (see ADR-0004 Known Gaps); the extension fails closed (a reject-only state) rather than falling back to a misleading default in that case. ### Boundary 5: background service worker → oracle adapter @@ -148,20 +150,20 @@ Gryd Lock does not currently protect against: | Threat | Current control | Residual risk / planned mitigation | | --------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------- | -| Page script forges or races internal messages | Message type checks, `event.source === window`, random request ID | Same-page scripts share `window`; add origin policy and isolated session binding (`#1`, `#4`). | +| Page script forges or races internal messages | Message type checks, `event.source === window`, background-generated (never page-supplied) request ID (ADR-0004) | Same-page scripts share `window` and can still forge a request, gated by admission limits; cannot forge a decision. Origin policy for the request leg remains open (`#1`). | | Freighter receives request before Gryd Lock | MAIN-world listener uses capture mode at `document_start` and stops immediate propagation | Chrome does not guarantee cross-extension order; add runtime self-test and degraded-protection warning (`#8`). | -| User closes review popup | Decision map waits for `DECISION_MADE` | Request may hang forever; handle window removal and timeout (`#2`). | -| MV3 worker terminates during review | None beyond browser runtime behavior | In-memory resolver is lost; persist recoverable metadata and fail safely (`#3`). | -| Malicious dApp floods signing requests | Each request receives a random ID and separate popup | Unbounded windows and map growth permit client-side denial of service; add queue and concurrency limits (`#7`). | +| User closes review popup | `chrome.windows.onRemoved` settles the request as cancelled (ADR-0004) | Resolved. | +| MV3 worker terminates during review | Pending-request identity/binding/deadline persisted to `chrome.storage.session` before UI opens; `AWAIT_OUTCOME` resume handshake reads durable state (ADR-0004) | Review content itself is not persisted, only the pending-request record; a restart can still leave a reopened popup unable to redisplay the review (fails closed). | +| Malicious dApp floods signing requests | Per-frame and global admission limits reject beyond bound before any popup opens (ADR-0004) | Limits are unit-tested at the boundary; a full 1,000-concurrent-request load test is not yet part of CI. | | Malformed or adversarial XDR crashes, hangs, or becomes green | Bounded XDR/operation/fact/Soroban traversal parsing; malformed or over-limit review cancels; fuzz/property tests | SDK parser and static review still cannot prove runtime effects. | | Wrong Stellar network used for decoding | The incoming alias or custom passphrase is resolved once, retained in every typed target and the displayed review, and used in the network-bound digest | The extension relies on the wallet/dApp-supplied passphrase; it cannot independently prove the intended network. | -| Attacker-controlled review data exhausts popup or leaks through a URL | Worker-only review storage, request-ID-only popup URL, control-character stripping, and display bounds | Request authenticity and stale-popup binding remain open risks. | +| Attacker-controlled review data exhausts popup or leaks through a URL | Worker-only review storage, opaque background-generated-ID-only popup URL, control-character stripping, and display bounds | Request authenticity for the initial page request remains open (Boundary 2); stale/copied-popup risk is resolved by window binding (ADR-0004). | | Overbroad page access increases attack surface | MV3 isolation | Scripts and host permission currently use ``; reduce or justify access (`#5`). | | Oracle stalls signing | Current local stub resolves quickly | Remote adapter needs cancellation, timeout, and explicit fallback (`#12`). | | Oracle gives a wrong or malicious score | Tier mapping displays supplied score | Authenticate source, define freshness/provenance, monitor quality, and never describe score as a guarantee. | -| Dependency or build compromise | Lockfile, lint/type/test/build CI | Automate dependency updates and keep CI-gated review (`#46`); protect release credentials and provenance. | -| Popup or decision is not bound to the originating tab/frame/request | Request ID map | Add sender/tab/frame validation, session binding, stale-popup rejection, and one-shot decision semantics. | -| User sees false assurance while extension is inactive | README documents limitations | Add runtime health/self-test UI and explicit degraded states (`#8`). | +| Dependency or build compromise | Lockfile, lint/type/test/build CI | Automate dependency updates and keep CI-gated review (`#46`); protect release credentials and provenance. | +| Popup or decision is not bound to the originating tab/frame/request | Request bound to originating tab/frame/document (invalidated on navigation/close) and to the popup's own windowId; first valid terminal transition wins, replay is a no-op (ADR-0004) | Resolved for identity/binding/replay in scope; see ADR-0004 for what remains deferred. | +| User sees false assurance while extension is inactive | README documents limitations | Add runtime health/self-test UI and explicit degraded states (`#8`). | Issue references identify planned work; they are not evidence that the mitigation is already deployed. diff --git a/e2e/signTransaction.spec.ts b/e2e/signTransaction.spec.ts index f81494e..42c8c20 100644 --- a/e2e/signTransaction.spec.ts +++ b/e2e/signTransaction.spec.ts @@ -107,14 +107,19 @@ async function launchExtension(url: string): Promise { } } -async function submitTransaction(page: Page, xdr: string) { +async function submitTransaction(page: Page, xdr: string, messageId = 38) { return page.evaluate( - ({ freighterRequestSource, freighterResponseSource, submitTransactionType, transactionXdr }) => + ({ + freighterRequestSource, + freighterResponseSource, + submitTransactionType, + transactionXdr, + requestMessageId, + }) => new Promise<{ freighterSawReviewedRequest: boolean response: Record }>((resolve) => { - const messageId = 38 let freighterSawReviewedRequest = false window.addEventListener('message', (event) => { @@ -124,14 +129,14 @@ async function submitTransaction(page: Page, xdr: string) { if ( data.source === freighterRequestSource && data.type === submitTransactionType && - data.messageId === messageId && + data.messageId === requestMessageId && data.__grydlockReviewed === true ) { freighterSawReviewedRequest = true window.postMessage( { source: freighterResponseSource, - messageId, + messageId: requestMessageId, signedTransaction: 'signed-by-freighter', signerAddress: 'GBROWSERTESTSIGNER', }, @@ -139,7 +144,9 @@ async function submitTransaction(page: Page, xdr: string) { ) } - if (data.source === freighterResponseSource) { + // messageId disambiguates concurrent submitTransaction calls sharing one page: + // each call's own listener must only resolve on ITS OWN request's response. + if (data.source === freighterResponseSource && data.messageId === requestMessageId) { resolve({ freighterSawReviewedRequest, response: data, @@ -150,7 +157,7 @@ async function submitTransaction(page: Page, xdr: string) { window.postMessage( { source: freighterRequestSource, - messageId, + messageId: requestMessageId, type: submitTransactionType, transactionXdr, networkPassphrase: 'Test SDF Network ; September 2015', @@ -163,6 +170,7 @@ async function submitTransaction(page: Page, xdr: string) { freighterResponseSource: FREIGHTER_RESPONSE_SOURCE, submitTransactionType: SUBMIT_TRANSACTION_TYPE, transactionXdr: xdr, + requestMessageId: messageId, }, ) } @@ -298,6 +306,53 @@ test.describe('Freighter signTransaction interception', () => { await toolbar.close() }) + test('closing the review popup settles the request as cancelled instead of hanging the dApp', async () => { + const popupPromise = harness!.context.waitForEvent('page') + const responsePromise = submitTransaction(harness!.page, buildPaymentXdr()) + + const popup = await popupPromise + await expect(popup.getByRole('heading', { name: /risk/i })).toBeVisible() + await popup.close() + + await expect(responsePromise).resolves.toMatchObject({ + freighterSawReviewedRequest: false, + response: { + source: FREIGHTER_RESPONSE_SOURCE, + signedTransaction: '', + apiError: { code: -4 }, + }, + }) + }) + + test('two concurrent requests from the same page open two popups and settle independently, without colliding', async () => { + const firstPopupPromise = harness!.context.waitForEvent('page') + const firstResponsePromise = submitTransaction(harness!.page, buildPaymentXdr(), 101) + const firstPopup = await firstPopupPromise + await expect(firstPopup.getByRole('heading', { name: /risk/i })).toBeVisible() + + const secondPopupPromise = harness!.context.waitForEvent('page') + const secondResponsePromise = submitTransaction(harness!.page, buildPaymentXdr(), 102) + const secondPopup = await secondPopupPromise + await expect(secondPopup.getByRole('heading', { name: /risk/i })).toBeVisible() + + expect(firstPopup).not.toBe(secondPopup) + + // Decide them in reverse order: if either request's state collided with the + // other's (e.g. sharing one map entry), this would resolve the wrong promise + // with the wrong outcome instead of each settling independently. + await makeDecision(Promise.resolve(secondPopup), 'Cancel') + await makeDecision(Promise.resolve(firstPopup), 'Proceed') + + await expect(firstResponsePromise).resolves.toMatchObject({ + freighterSawReviewedRequest: true, + response: { messageId: 101, signedTransaction: 'signed-by-freighter' }, + }) + await expect(secondResponsePromise).resolves.toMatchObject({ + freighterSawReviewedRequest: false, + response: { messageId: 102, signedTransaction: '' }, + }) + }) + test('surfaces an incompatible protocol observed in a child frame', async () => { const toolbar = await openToolbar(harness!.context) const childFrame = harness!.page.waitForEvent('frameattached') diff --git a/scripts/build-extension.mjs b/scripts/build-extension.mjs index 603516a..4211fbb 100644 --- a/scripts/build-extension.mjs +++ b/scripts/build-extension.mjs @@ -9,13 +9,20 @@ const entries = [ ] const sizeBudgets = [ - { outfile: 'dist/mainWorld.js', budgetBytes: 5 * 1024, label: 'mainWorld.js' }, + // Raised from 5.00 KB alongside the one-shot signing protocol: mainWorld.js now + // shares requestOutcome.ts (adapter tag, page-side absolute deadline) instead of + // an inlined helper. Still tight enough to catch an accidental SDK/page-library pull-in. + { outfile: 'dist/mainWorld.js', budgetBytes: 5.5 * 1024, label: 'mainWorld.js' }, // The Albedo entry includes the popup-specific interception implementation plus the // closed-set protection heartbeat. Keep the budget tight enough to detect SDK or // page-library regressions; the Stellar SDK must remain background-only. - { outfile: 'dist/albedoMainWorld.js', budgetBytes: 6 * 1024, label: 'albedoMainWorld.js' }, + // Raised from 6.00 KB for the same shared requestOutcome.ts helper as mainWorld.js. + { outfile: 'dist/albedoMainWorld.js', budgetBytes: 6.5 * 1024, label: 'albedoMainWorld.js' }, // The isolated bridge owns the versioned handshake and strict message validation. - { outfile: 'dist/bridge.js', budgetBytes: 4 * 1024, label: 'bridge.js' }, + // Raised from 4.00 KB: the bridge now runs the SIGN_REQUEST -> SIGN_ACK -> AWAIT_OUTCOME + // resume/retry loop (awaitOutcome.ts) instead of a single blocking sendMessage callback, + // so it can survive a worker restart mid-review. Still well under mainWorld's budget. + { outfile: 'dist/bridge.js', budgetBytes: 5.5 * 1024, label: 'bridge.js' }, ] function formatBytes(bytes) { diff --git a/src/background/background.test.ts b/src/background/background.test.ts index 5aa5c0b..0b2a899 100644 --- a/src/background/background.test.ts +++ b/src/background/background.test.ts @@ -1,14 +1,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import * as resolveModule from '../intercept/resolveOutcome' -import { - MAX_NETWORK_PASSPHRASE_LENGTH, - MAX_REQUEST_ID_LENGTH, - MAX_XDR_LENGTH, -} from './messageValidation' +import { MAX_NETWORK_PASSPHRASE_LENGTH, MAX_XDR_LENGTH } from './messageValidation' +import { MAX_PENDING_PER_FRAME } from '../signing/pendingRequestState' +import type { AggregatedReview } from '../review/model' const mockAddListener = vi.fn() const mockGetURL = vi.fn((path: string) => `chrome-extension://test-id/${path}`) const mockWindowsCreate = vi.fn() +const mockWindowsRemoved = vi.fn() const mockSetBadgeText = vi.fn() const mockSetBadgeBackgroundColor = vi.fn() const mockLocalGet = vi.fn() @@ -26,6 +24,42 @@ const originalChrome = globalThis.chrome const flushPromises = () => new Promise((resolve) => setTimeout(resolve, 0)) +const FAKE_REVIEW: AggregatedReview = { + review: { + schemaVersion: 1, + policyVersion: 1, + networkPassphrase: 'test', + xdrDigest: 'digest-abc', + envelope: { type: 'transaction', source: 'GDEST', operationCount: 1 }, + operations: [], + findings: [], + }, + evidence: [], + findings: [], + severity: 'warning', +} + +const DAPP_SENDER = { + tab: { id: 9, url: 'https://dapp.example/' }, + frameId: 0, + url: 'https://dapp.example/', +} + +function popupSender(windowId: number, popupTabId = 200) { + return { + tab: { id: popupTabId, windowId, url: 'chrome-extension://test-id/src/popup/index.html' }, + frameId: 0, + url: 'chrome-extension://test-id/src/popup/index.html', + } +} + +const SIGN_REQUEST_MESSAGE = { + type: 'SIGN_REQUEST', + protocolVersion: 1, + xdr: 'test', + adapter: 'freighter', +} as const + describe('background message listener', () => { beforeEach(() => { vi.restoreAllMocks() @@ -36,12 +70,18 @@ describe('background message listener', () => { mockSessionGet.mockResolvedValue({}) mockSessionSet.mockResolvedValue(undefined) mockPermissionContains.mockResolvedValue(true) + mockWindowsCreate.mockImplementation( + (_options: unknown, callback: (window: { id: number }) => void) => callback({ id: 100 }), + ) globalThis.chrome = { runtime: { onMessage: { addListener: mockAddListener }, getURL: mockGetURL, }, - windows: { create: mockWindowsCreate }, + windows: { + create: mockWindowsCreate, + onRemoved: { addListener: mockWindowsRemoved }, + }, action: { setBadgeText: mockSetBadgeText, setBadgeBackgroundColor: mockSetBadgeBackgroundColor, @@ -65,177 +105,512 @@ describe('background message listener', () => { afterEach(() => { globalThis.chrome = originalChrome - vi.resetModules() // clear the internal pendingDecisions map for the next test + vi.resetModules() // clear the internal pendingSignRequests map for the next test }) - it('handles SIGN_REQUEST to SIGN_OUTCOME round trip and explicitly tests pendingDecisions lifecycle', async () => { - // Intercept resolveOutcome to control when it finishes and observe requestDecision - vi.spyOn(resolveModule, 'resolveReviewOutcome').mockImplementation(async (_xdr, deps) => { - // We must await it to test the round trip! - const decision = await deps.requestDecision({ - review: { - schemaVersion: 1, policyVersion: 1, networkPassphrase: 'test', xdrDigest: 'digest', - envelope: { type: 'transaction', source: 'GDEST', operationCount: 1 }, operations: [], findings: [], - }, - evidence: [], findings: [], severity: 'warning', - }) - return decision === 'proceed' ? 'allow' : 'cancel' + // resolveOutcome must be imported dynamically (not via a static top-level + // import) because vi.resetModules() in afterEach clears the module + // registry: a stale static reference would spy on a module instance + // background.ts's own re-import no longer resolves to, silently falling + // through to the REAL buildAggregatedReview (which fails closed on the + // fixture's non-XDR 'test' string) for every test after the first. + async function importBackgroundWithFakeReview(review: AggregatedReview | null = FAKE_REVIEW) { + const resolveModule = await import('../intercept/resolveOutcome') + vi.spyOn(resolveModule, 'buildAggregatedReview').mockResolvedValue(review) + await import('./background') + return mockAddListener.mock.calls[0][0] + } + + it('handles the full SIGN_REQUEST -> SIGN_ACK -> AWAIT_OUTCOME -> DECISION_MADE -> SIGN_OUTCOME round trip', async () => { + const listener = await importBackgroundWithFakeReview() + + const signResponse = vi.fn() + expect(listener(SIGN_REQUEST_MESSAGE, DAPP_SENDER, signResponse)).toBe(true) + await flushPromises() + await flushPromises() + + expect(signResponse).toHaveBeenCalledTimes(1) + const ack = signResponse.mock.calls[0][0] + expect(ack.type).toBe('SIGN_ACK') + expect(typeof ack.requestId).toBe('string') + expect(ack.requestId.length).toBeGreaterThan(0) + expect(typeof ack.deadlineAt).toBe('number') + + const popupUrl = mockWindowsCreate.mock.calls[0][0].url as string + expect(popupUrl).toContain('mode=intercept') + expect(popupUrl).toContain(`requestId=${ack.requestId}`) + expect(mockSetBadgeText).toHaveBeenCalledWith({ text: '!' }) + + const awaitResponse = vi.fn() + expect( + listener({ type: 'AWAIT_OUTCOME', requestId: ack.requestId }, {}, awaitResponse), + ).toBe(true) + await flushPromises() + expect(awaitResponse).not.toHaveBeenCalled() // still awaiting_review: the port stays open + + const decisionSender = popupSender(100) + listener( + { type: 'DECISION_MADE', protocolVersion: 1, requestId: ack.requestId, decision: 'proceed' }, + decisionSender, + vi.fn(), + ) + await flushPromises() + + expect(awaitResponse).toHaveBeenCalledWith({ + type: 'SIGN_OUTCOME', + requestId: ack.requestId, + outcome: 'proceed', }) + expect(mockSetBadgeText).toHaveBeenCalledWith({ text: '' }) - await import('./background') + // A replayed AWAIT_OUTCOME after settlement resolves immediately from the tombstone. + const replayAwait = vi.fn() + listener({ type: 'AWAIT_OUTCOME', requestId: ack.requestId }, {}, replayAwait) + await flushPromises() + expect(replayAwait).toHaveBeenCalledWith({ + type: 'SIGN_OUTCOME', + requestId: ack.requestId, + outcome: 'proceed', + }) - const listener = mockAddListener.mock.calls[0][0] + // A replayed DECISION_MADE (e.g. a double-submit) is a no-op: it must not crash and must + // not re-fire diagnostics/badge logic for an already-settled request. + expect(() => + listener( + { type: 'DECISION_MADE', protocolVersion: 1, requestId: ack.requestId, decision: 'cancel' }, + decisionSender, + vi.fn(), + ), + ).not.toThrow() + await flushPromises() + const replayAwait2 = vi.fn() + listener({ type: 'AWAIT_OUTCOME', requestId: ack.requestId }, {}, replayAwait2) + await flushPromises() + // Still 'proceed' — the replayed cancel never overwrote the first terminal decision. + expect(replayAwait2).toHaveBeenCalledWith( + expect.objectContaining({ outcome: 'proceed' }), + ) + }) + + it('never trusts a page-supplied requestId: two concurrent SIGN_REQUESTs get two distinct, extension-generated ids', async () => { + const listener = await importBackgroundWithFakeReview() + + const responseA = vi.fn() + const responseB = vi.fn() + listener(SIGN_REQUEST_MESSAGE, { ...DAPP_SENDER, frameId: 0 }, responseA) + await flushPromises() + listener(SIGN_REQUEST_MESSAGE, { ...DAPP_SENDER, frameId: 1 }, responseB) + await flushPromises() + await flushPromises() + + const idA = responseA.mock.calls[0][0].requestId + const idB = responseB.mock.calls[0][0].requestId + expect(idA).not.toBe(idB) + expect(idA).not.toBe('page-supplied-id') + }) + + it('rejects a SIGN_REQUEST carrying an unrecognized field (e.g. a page-supplied requestId) as malformed', async () => { + const listener = await importBackgroundWithFakeReview() const sendResponse = vi.fn() + expect( + listener({ ...SIGN_REQUEST_MESSAGE, requestId: 'attacker-chosen' }, DAPP_SENDER, sendResponse), + ).toBeUndefined() + expect(sendResponse).not.toHaveBeenCalled() + expect(mockWindowsCreate).not.toHaveBeenCalled() + }) - // 1. Send SIGN_REQUEST - const returnsTrue = listener( - { type: 'SIGN_REQUEST', requestId: 'req-1', xdr: 'test' }, - {}, - sendResponse, + it('responds SIGN_REJECTED for a mismatched protocol version without any side effect', async () => { + const listener = await importBackgroundWithFakeReview() + const sendResponse = vi.fn() + expect( + listener({ ...SIGN_REQUEST_MESSAGE, protocolVersion: 2 }, DAPP_SENDER, sendResponse), + ).toBe(true) + await flushPromises() + expect(sendResponse).toHaveBeenCalledWith({ type: 'SIGN_REJECTED', reason: 'protocol-incompatible' }) + expect(mockWindowsCreate).not.toHaveBeenCalled() + }) + + it('responds SIGN_REJECTED sender-unbound when the sender has no bindable tab', async () => { + const listener = await importBackgroundWithFakeReview() + const sendResponse = vi.fn() + expect(listener(SIGN_REQUEST_MESSAGE, {}, sendResponse)).toBe(true) + await flushPromises() + expect(sendResponse).toHaveBeenCalledWith({ type: 'SIGN_REJECTED', reason: 'sender-unbound' }) + expect(mockWindowsCreate).not.toHaveBeenCalled() + }) + + it('admits a request even when sender.documentId is absent (older-Chrome compatibility fallback)', async () => { + const listener = await importBackgroundWithFakeReview() + const sendResponse = vi.fn() + listener(SIGN_REQUEST_MESSAGE, DAPP_SENDER, sendResponse) + await flushPromises() + await flushPromises() + expect(sendResponse).toHaveBeenCalledWith( + expect.objectContaining({ type: 'SIGN_ACK' }), ) - expect(returnsTrue).toBe(true) + expect(mockWindowsCreate).toHaveBeenCalledTimes(1) + }) + + it('rejects the (MAX_PENDING_PER_FRAME + 1)th concurrent request from the same frame without opening a window', async () => { + const listener = await importBackgroundWithFakeReview() + + for (let index = 0; index < MAX_PENDING_PER_FRAME; index += 1) { + const sendResponse = vi.fn() + listener(SIGN_REQUEST_MESSAGE, DAPP_SENDER, sendResponse) + await flushPromises() + await flushPromises() + expect(sendResponse).toHaveBeenCalledWith(expect.objectContaining({ type: 'SIGN_ACK' })) + } + expect(mockWindowsCreate).toHaveBeenCalledTimes(MAX_PENDING_PER_FRAME) - // Wait for resolveOutcome to get called and hit `requestDecision` + const overflowResponse = vi.fn() + listener(SIGN_REQUEST_MESSAGE, DAPP_SENDER, overflowResponse) await flushPromises() + expect(overflowResponse).toHaveBeenCalledWith({ type: 'SIGN_REJECTED', reason: 'frame-limit' }) + expect(mockWindowsCreate).toHaveBeenCalledTimes(MAX_PENDING_PER_FRAME) + }) - // Verify it called chrome.windows.create with the URL - const popupUrl = mockWindowsCreate.mock.calls[0][0].url as string - expect(popupUrl).toContain('mode=intercept') - expect(popupUrl).toContain('requestId=req-1') - expect(popupUrl).not.toContain('destination=') - expect(popupUrl).not.toContain('score=') + it('a decision from the wrong popup window is rejected; the request stays open for the correct one', async () => { + const listener = await importBackgroundWithFakeReview() + const sendResponse = vi.fn() + listener(SIGN_REQUEST_MESSAGE, DAPP_SENDER, sendResponse) + await flushPromises() + await flushPromises() + const requestId = sendResponse.mock.calls[0][0].requestId - // Review severity, not a score URL parameter, controls the badge. - expect(mockSetBadgeText).toHaveBeenCalledWith({ text: '!' }) - expect(mockSetBadgeBackgroundColor).toHaveBeenCalledWith({ color: '#a86300' }) + // The first legitimate contact binds the review window (TOFU) — establish it. + const getReview = vi.fn() + listener( + { type: 'GET_REVIEW', protocolVersion: 1, requestId }, + popupSender(100), + getReview, + ) + await flushPromises() + expect(getReview).toHaveBeenCalledWith( + expect.objectContaining({ review: expect.objectContaining({ severity: 'warning' }) }), + ) - // At this point, pendingDecisions has 'req-1'. - // We send a DECISION_MADE message to resolve it. - listener({ type: 'DECISION_MADE', requestId: 'req-1', decision: 'proceed' }, {}, vi.fn()) + // A decision from a different window (e.g. a copied review URL opened elsewhere) is a no-op. + listener( + { type: 'DECISION_MADE', protocolVersion: 1, requestId, decision: 'proceed' }, + popupSender(999, 201), + vi.fn(), + ) + await flushPromises() - // Wait for the Promise chain to resolve + const awaitResponse = vi.fn() + listener({ type: 'AWAIT_OUTCOME', requestId }, {}, awaitResponse) await flushPromises() + expect(awaitResponse).not.toHaveBeenCalled() // still pending: the wrong-window decision did not settle it - // Verify round trip completion - expect(sendResponse).toHaveBeenCalledWith({ - type: 'SIGN_OUTCOME', - requestId: 'req-1', - outcome: 'allow', + // The correct window's decision still works. + listener( + { type: 'DECISION_MADE', protocolVersion: 1, requestId, decision: 'cancel' }, + popupSender(100), + vi.fn(), + ) + await flushPromises() + expect(awaitResponse).toHaveBeenCalledWith({ type: 'SIGN_OUTCOME', requestId, outcome: 'cancel' }) + }) + + it('a copied review URL opened in the wrong window reveals no review content', async () => { + const listener = await importBackgroundWithFakeReview() + const sendResponse = vi.fn() + listener(SIGN_REQUEST_MESSAGE, DAPP_SENDER, sendResponse) + await flushPromises() + await flushPromises() + const requestId = sendResponse.mock.calls[0][0].requestId + + listener({ type: 'GET_REVIEW', protocolVersion: 1, requestId }, popupSender(100), vi.fn()) + await flushPromises() + + const wrongWindowRead = vi.fn() + listener( + { type: 'GET_REVIEW', protocolVersion: 1, requestId }, + popupSender(999, 201), + wrongWindowRead, + ) + await flushPromises() + expect(wrongWindowRead).toHaveBeenCalledWith({ type: 'REVIEW_DATA', requestId }) + }) + + it('closing the review popup (windows.onRemoved) settles the request as cancelled', async () => { + const listener = await importBackgroundWithFakeReview() + const sendResponse = vi.fn() + listener(SIGN_REQUEST_MESSAGE, DAPP_SENDER, sendResponse) + await flushPromises() + await flushPromises() + const requestId = sendResponse.mock.calls[0][0].requestId + + const awaitResponse = vi.fn() + listener({ type: 'AWAIT_OUTCOME', requestId }, {}, awaitResponse) + await flushPromises() + expect(awaitResponse).not.toHaveBeenCalled() + + const onWindowRemoved = mockWindowsRemoved.mock.calls[0][0] + onWindowRemoved(100) // the windowId chrome.windows.create resolved to + await flushPromises() + + expect(awaitResponse).toHaveBeenCalledWith({ type: 'SIGN_OUTCOME', requestId, outcome: 'cancel' }) + }) + + it('a popup creation failure settles the request as failed (outcome cancel) and clears the badge', async () => { + mockWindowsCreate.mockImplementation((_options: unknown, callback: (w?: undefined) => void) => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ;(globalThis.chrome.runtime as any).lastError = { message: 'popup creation failed' } + callback(undefined) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + delete (globalThis.chrome.runtime as any).lastError }) + const listener = await importBackgroundWithFakeReview() + const sendResponse = vi.fn() + listener(SIGN_REQUEST_MESSAGE, DAPP_SENDER, sendResponse) + await flushPromises() + await flushPromises() + const requestId = sendResponse.mock.calls[0][0].requestId - // Verify badge text cleared when pending decisions are resolved + const awaitResponse = vi.fn() + listener({ type: 'AWAIT_OUTCOME', requestId }, {}, awaitResponse) + await flushPromises() + expect(awaitResponse).toHaveBeenCalledWith({ type: 'SIGN_OUTCOME', requestId, outcome: 'cancel' }) expect(mockSetBadgeText).toHaveBeenCalledWith({ text: '' }) + }) + + it('originating tab navigation invalidates the pending request', async () => { + const listener = await importBackgroundWithFakeReview() + const sendResponse = vi.fn() + listener(SIGN_REQUEST_MESSAGE, DAPP_SENDER, sendResponse) + await flushPromises() + await flushPromises() + const requestId = sendResponse.mock.calls[0][0].requestId + + const onTabUpdated = mockTabUpdated.mock.calls[0][0] + onTabUpdated(DAPP_SENDER.tab.id, { status: 'loading' }) + await flushPromises() + + const awaitResponse = vi.fn() + listener({ type: 'AWAIT_OUTCOME', requestId }, {}, awaitResponse) + await flushPromises() + expect(awaitResponse).toHaveBeenCalledWith({ type: 'SIGN_OUTCOME', requestId, outcome: 'cancel' }) + }) + + it('originating tab closure invalidates the pending request', async () => { + const listener = await importBackgroundWithFakeReview() + const sendResponse = vi.fn() + listener(SIGN_REQUEST_MESSAGE, DAPP_SENDER, sendResponse) + await flushPromises() + await flushPromises() + const requestId = sendResponse.mock.calls[0][0].requestId + + const onTabRemoved = mockTabRemoved.mock.calls[0][0] + onTabRemoved(DAPP_SENDER.tab.id) + await flushPromises() + + const awaitResponse = vi.fn() + listener({ type: 'AWAIT_OUTCOME', requestId }, {}, awaitResponse) + await flushPromises() + expect(awaitResponse).toHaveBeenCalledWith({ type: 'SIGN_OUTCOME', requestId, outcome: 'cancel' }) + }) + + it('a malformed/unreviewable XDR settles failed (outcome cancel) instead of hanging', async () => { + const listener = await importBackgroundWithFakeReview(null) + + const sendResponse = vi.fn() + listener(SIGN_REQUEST_MESSAGE, DAPP_SENDER, sendResponse) + await flushPromises() + await flushPromises() + const requestId = sendResponse.mock.calls[0][0].requestId + + // No popup is opened for a request that never reaches awaiting_review. + expect(mockWindowsCreate).not.toHaveBeenCalled() + + const awaitResponse = vi.fn() + listener({ type: 'AWAIT_OUTCOME', requestId }, {}, awaitResponse) + await flushPromises() + expect(awaitResponse).toHaveBeenCalledWith({ type: 'SIGN_OUTCOME', requestId, outcome: 'cancel' }) + }) + + it('AWAIT_OUTCOME for an unknown/never-seen requestId fails closed to cancel', async () => { + const listener = await importBackgroundWithFakeReview() + const awaitResponse = vi.fn() + listener({ type: 'AWAIT_OUTCOME', requestId: 'never-existed' }, {}, awaitResponse) + await flushPromises() + expect(awaitResponse).toHaveBeenCalledWith({ + type: 'SIGN_OUTCOME', + requestId: 'never-existed', + outcome: 'cancel', + }) + }) - // Verify delete: sending another DECISION_MADE shouldn't crash or re-resolve anything - // If pendingDecisions was not deleted, it would try to resolve a completed promise (which is safe in JS, but we want to ensure no crash) - expect(() => { - listener({ type: 'DECISION_MADE', requestId: 'req-1', decision: 'cancel' }, {}, vi.fn()) - }).not.toThrow() + it('resumes a pending request restored from durable storage after a worker restart', async () => { + const restoredRequestId = 'restored-request-1' + mockSessionGet.mockImplementation((key: string) => { + if (key === 'pendingRequestStateV1') { + return Promise.resolve({ + pendingRequestStateV1: [ + { + requestId: restoredRequestId, + adapter: 'freighter', + tabId: 9, + frameId: 0, + documentBound: true, + state: 'awaiting_review', + protocolVersion: 1, + createdAt: Date.now(), + deadlineAt: Date.now() + 60_000, + windowId: 100, + }, + ], + }) + } + return Promise.resolve({}) + }) + + await import('./background') + const listener = mockAddListener.mock.calls[0][0] + + // The resolver was lost with the old worker lifetime; AWAIT_OUTCOME re-attaches a fresh one + // by reading the durable record, and the port stays open until a decision arrives. + const awaitResponse = vi.fn() + listener({ type: 'AWAIT_OUTCOME', requestId: restoredRequestId }, {}, awaitResponse) + await flushPromises() + expect(awaitResponse).not.toHaveBeenCalled() + + listener( + { + type: 'DECISION_MADE', + protocolVersion: 1, + requestId: restoredRequestId, + decision: 'proceed', + }, + popupSender(100), + vi.fn(), + ) + await flushPromises() + + expect(awaitResponse).toHaveBeenCalledWith({ + type: 'SIGN_OUTCOME', + requestId: restoredRequestId, + outcome: 'proceed', + }) }) - it('safely handles unknown/out-of-order DECISION_MADE messages', async () => { + it('a restored request whose deadline already passed settles expired (outcome cancel) immediately, with no dangling resolver', async () => { + const restoredRequestId = 'restored-expired-1' + mockSessionGet.mockImplementation((key: string) => { + if (key === 'pendingRequestStateV1') { + return Promise.resolve({ + pendingRequestStateV1: [ + { + requestId: restoredRequestId, + adapter: 'freighter', + tabId: 9, + frameId: 0, + documentBound: true, + state: 'awaiting_review', + protocolVersion: 1, + createdAt: Date.now() - 200_000, + deadlineAt: Date.now() - 1, + windowId: 100, + }, + ], + }) + } + return Promise.resolve({}) + }) + await import('./background') const listener = mockAddListener.mock.calls[0][0] - // Send DECISION_MADE without any pending SIGN_REQUEST - // It should silently no-op at `resolve?.(...)` - expect(() => { - listener({ type: 'DECISION_MADE', requestId: 'unknown-id', decision: 'proceed' }, {}, vi.fn()) - }).not.toThrow() + const awaitResponse = vi.fn() + listener({ type: 'AWAIT_OUTCOME', requestId: restoredRequestId }, {}, awaitResponse) + await flushPromises() + expect(awaitResponse).toHaveBeenCalledWith({ + type: 'SIGN_OUTCOME', + requestId: restoredRequestId, + outcome: 'cancel', + }) }) it('rejects malformed and oversized sign requests before any side effect', async () => { - const resolveOutcome = vi.spyOn(resolveModule, 'resolveReviewOutcome') - const { pendingDecisions } = await import('./background') + const resolveModule = await import('../intercept/resolveOutcome') + const buildReview = vi.spyOn(resolveModule, 'buildAggregatedReview') + await import('./background') const listener = mockAddListener.mock.calls[0][0] const sendResponse = vi.fn() const invalidMessages: unknown[] = [ null, 'SIGN_REQUEST', { type: 'SIGN_REQUEST' }, - { type: 'SIGN_REQUEST', requestId: 'req-1' }, - { type: 'SIGN_REQUEST', requestId: 1, xdr: 'AAAAAg==' }, - { type: 'SIGN_REQUEST', requestId: 'req-1', xdr: 1 }, + { type: 'SIGN_REQUEST', protocolVersion: 1 }, + { type: 'SIGN_REQUEST', protocolVersion: 1, xdr: 1, adapter: 'freighter' }, + { type: 'SIGN_REQUEST', protocolVersion: 1, xdr: 'AAAAAg==', adapter: 'metamask' }, { type: 'SIGN_REQUEST', - requestId: 'req-1', + protocolVersion: 1, xdr: 'AAAAAg==', + adapter: 'freighter', networkPassphrase: 1, }, { type: 'SIGN_REQUEST', - requestId: 'r'.repeat(MAX_REQUEST_ID_LENGTH + 1), - xdr: 'AAAAAg==', - }, - { - type: 'SIGN_REQUEST', - requestId: 'req-1', + protocolVersion: 1, xdr: 'A'.repeat(MAX_XDR_LENGTH + 1), + adapter: 'freighter', }, { type: 'SIGN_REQUEST', - requestId: 'req-1', + protocolVersion: 1, xdr: 'AAAAAg==', + adapter: 'freighter', networkPassphrase: 'n'.repeat(MAX_NETWORK_PASSPHRASE_LENGTH + 1), }, ] for (const message of invalidMessages) { - expect(listener(message, {}, sendResponse)).toBeUndefined() + expect(listener(message, DAPP_SENDER, sendResponse)).toBeUndefined() } - expect(resolveOutcome).not.toHaveBeenCalled() + expect(buildReview).not.toHaveBeenCalled() expect(mockWindowsCreate).not.toHaveBeenCalled() expect(mockSetBadgeText).not.toHaveBeenCalled() expect(mockSetBadgeBackgroundColor).not.toHaveBeenCalled() expect(sendResponse).not.toHaveBeenCalled() - expect(pendingDecisions.size).toBe(0) }) it('never resolves pending state for an invalid decision message', async () => { - const { pendingDecisions } = await import('./background') - const listener = mockAddListener.mock.calls[0][0] - const resolvePending = vi.fn() - pendingDecisions.set('req-1', { - resolve: resolvePending, - review: { - review: { - schemaVersion: 1, - policyVersion: 1, - networkPassphrase: 'test', - xdrDigest: 'digest', - envelope: { type: 'transaction', source: 'GTEST', operationCount: 0 }, - operations: [], - findings: [], - }, - evidence: [], - findings: [], - severity: 'info', - }, - }) + const listener = await importBackgroundWithFakeReview() + const sendResponse = vi.fn() + listener(SIGN_REQUEST_MESSAGE, DAPP_SENDER, sendResponse) + await flushPromises() + await flushPromises() + const requestId = sendResponse.mock.calls[0][0].requestId const invalidDecisions: unknown[] = [ - { type: 'DECISION_MADE', requestId: 'req-1' }, - { type: 'DECISION_MADE', requestId: 'req-1', decision: 'allow' }, - { type: 'DECISION_MADE', requestId: 'req-1', decision: 1 }, - { type: 'DECISION_MADE', requestId: 1, decision: 'proceed' }, - { - type: 'DECISION_MADE', - requestId: 'r'.repeat(MAX_REQUEST_ID_LENGTH + 1), - decision: 'cancel', - }, + { type: 'DECISION_MADE', protocolVersion: 1, requestId }, + { type: 'DECISION_MADE', protocolVersion: 1, requestId, decision: 'allow' }, + { type: 'DECISION_MADE', protocolVersion: 1, requestId, decision: 1 }, + { type: 'DECISION_MADE', protocolVersion: 1, requestId: 1, decision: 'proceed' }, + { type: 'DECISION_MADE', protocolVersion: 2, requestId, decision: 'proceed' }, ] for (const message of invalidDecisions) { - expect(listener(message, {}, vi.fn())).toBeUndefined() + expect(listener(message, popupSender(100), vi.fn())).toBeUndefined() } + await flushPromises() - expect(resolvePending).not.toHaveBeenCalled() - expect(pendingDecisions.has('req-1')).toBe(true) + const awaitResponse = vi.fn() + listener({ type: 'AWAIT_OUTCOME', requestId }, {}, awaitResponse) + await flushPromises() + expect(awaitResponse).not.toHaveBeenCalled() // still pending: none of the invalid messages resolved it - listener({ type: 'DECISION_MADE', requestId: 'req-1', decision: 'cancel' }, {}, vi.fn()) - expect(resolvePending).toHaveBeenCalledOnce() - expect(resolvePending).toHaveBeenCalledWith('cancel') + listener( + { type: 'DECISION_MADE', protocolVersion: 1, requestId, decision: 'cancel' }, + popupSender(100), + vi.fn(), + ) + await flushPromises() + expect(awaitResponse).toHaveBeenCalledWith({ type: 'SIGN_OUTCOME', requestId, outcome: 'cancel' }) }) it('requires a fresh successful non-financial handshake before reporting protected', async () => { diff --git a/src/background/background.ts b/src/background/background.ts index 66a3771..b7ecf60 100644 --- a/src/background/background.ts +++ b/src/background/background.ts @@ -4,18 +4,25 @@ import { exportDiagnostics, recordDiagnosticEvent, } from '../diagnostics/diagnosticsStore' -import { resolveReviewOutcome } from '../intercept/resolveOutcome' +import { buildAggregatedReview } from '../intercept/resolveOutcome' import type { Decision, + Outcome, + RuntimeAwaitOutcomeMessage, + RuntimeDecisionMadeMessage, RuntimeReviewRequestMessage, RuntimeReviewResponseMessage, + RuntimeSignAckMessage, RuntimeSignOutcomeMessage, + RuntimeSignRejectedMessage, + RuntimeSignRequestMessage, RuntimeProtectionStatusQueryMessage, } from '../intercept/protocol' import type { AggregatedReview } from '../review/model' import { recordDecision } from '../lib/history' import { tierForScore } from '../lib/tiers' import { + isRuntimeAwaitOutcomeMessage, isRuntimeDecisionMadeMessage, isRuntimeProtectionAdapterStatusMessage, isRuntimeProtectionBridgeOnlineMessage, @@ -34,19 +41,42 @@ import { type ProtectionRecord, type ProtectionSnapshot, } from '../protection/protectionState' - -export const DEFAULT_TIMEOUT_MS = 60_000 +import { + SIGN_PROTOCOL_VERSION, + REVIEW_DEADLINE_MS, + admissionCheck, + applyTransition, + isExpired, + isTerminal, + outcomeForState, + shouldPrune, + validatePendingRequestRecord, + type PendingRequestRecord, + type PendingState, +} from '../signing/pendingRequestState' + +export const DEFAULT_TIMEOUT_MS = REVIEW_DEADLINE_MS export const PROTECTION_STORAGE_KEY = 'protectionStateV1' +export const PENDING_REQUEST_STORAGE_KEY = 'pendingRequestStateV1' const HANDSHAKE_TTL_MS = 5_000 const MAX_PENDING_HANDSHAKES = 100 -interface PendingReview { - resolve: (decision: Decision) => void - review: AggregatedReview +interface PendingSignEntry { + record: PendingRequestRecord + review?: AggregatedReview + resolvers: Array<(outcome: Outcome) => void> } -/** Review data stays in extension memory and is never placed in the popup URL. */ -export const pendingDecisions = new Map() +/** + * Resolver functions never survive worker suspension, so they live only in + * this in-memory map, keyed by the background-generated `requestId` — the + * only identifier ever treated as authoritative. `entry.record` is the + * serializable projection persisted to chrome.storage.session (see + * persistPendingRequests/restorePendingRequests below); review content + * never reaches the popup URL and the page/bridge boundary's own + * correlation id never reaches this map at all. + */ +export const pendingSignRequests = new Map() const protectionRecords = new Map() const bridgeContexts = new Map() @@ -283,8 +313,130 @@ function handleProtectionHandshakeAck( const protectionRestore = restoreProtectionRecords() +function persistPendingRequests(): void { + const session = chrome.storage?.session + if (!session) return + const records = [...pendingSignRequests.values()].map((entry) => entry.record) + void session.set({ [PENDING_REQUEST_STORAGE_KEY]: records }).catch(() => { + void recordDiagnosticEvent('storage.write_failure').catch(() => {}) + }) +} + +async function restorePendingRequests(): Promise { + const session = chrome.storage?.session + if (!session) return + try { + const stored = await session.get(PENDING_REQUEST_STORAGE_KEY) + const values = Array.isArray(stored[PENDING_REQUEST_STORAGE_KEY]) + ? stored[PENDING_REQUEST_STORAGE_KEY] + : [] + const now = Date.now() + for (const raw of values) { + const record = validatePendingRequestRecord(raw) + if (!record || shouldPrune(record, now)) continue + // A worker restart never silently loses a deadline: any record whose + // absolute deadline already passed is settled 'expired' on restore, + // not left dangling for a resume attempt that could never succeed. + const effective = isExpired(record, now) + ? (applyTransition(record, 'expired', now) ?? record) + : record + pendingSignRequests.set(effective.requestId, { record: effective, resolvers: [] }) + } + persistPendingRequests() + } catch { + void recordDiagnosticEvent('storage.write_failure').catch(() => {}) + } +} + +const pendingRequestsRestore = restorePendingRequests() + +function pruneStalePendingRequests(): void { + const now = Date.now() + let changed = false + for (const [requestId, entry] of pendingSignRequests) { + if (isExpired(entry.record, now)) { + settlePendingRequest(requestId, 'expired') + changed = true + continue + } + if (shouldPrune(entry.record, now)) { + pendingSignRequests.delete(requestId) + changed = true + } + } + if (changed) persistPendingRequests() +} + +function transitionPendingRequest( + requestId: string, + next: PendingState, + patch: Partial = {}, +): PendingRequestRecord | undefined { + const entry = pendingSignRequests.get(requestId) + if (!entry) return undefined + const updated = applyTransition(entry.record, next, Date.now(), patch) + if (!updated) return undefined + entry.record = updated + persistPendingRequests() + return updated +} + +/** The first valid terminal transition wins; every later call for the same requestId is a no-op against an already-settled record. */ +function settlePendingRequest(requestId: string, state: PendingState): void { + const entry = pendingSignRequests.get(requestId) + if (!entry) return + const alreadySettled = entry.record.state === state + const updated = applyTransition(entry.record, state, Date.now()) + if (!updated) return + entry.record = updated + persistPendingRequests() + if (!alreadySettled && (state === 'proceed' || state === 'cancel') && entry.review) { + recordFirstDecision(entry.review, state) + } + const outcome = outcomeForState(state) + const resolvers = entry.resolvers + entry.resolvers = [] + for (const resolve of resolvers) resolve(outcome) + clearBadgeIfIdle() +} + +function createPopupWindow(requestId: string): Promise { + const params = new URLSearchParams({ mode: 'intercept', requestId }) + return new Promise((resolve, reject) => { + try { + chrome.windows.create( + { + url: chrome.runtime.getURL(`src/popup/index.html?${params.toString()}`), + type: 'popup', + width: 440, + height: 680, + }, + (createdWindow) => { + if (chrome.runtime.lastError || !createdWindow) { + reject(chrome.runtime.lastError ?? new Error('popup creation failed')) + return + } + resolve(createdWindow) + }, + ) + } catch (error) { + reject(error) + } + }) +} + +/** windowId binds a pending request to its exact review popup ("review window"). First legitimate contact binds it (TOFU) — safe because requestId is an unguessable, never-page-visible secret, so only the correct popup can reach this at all; every later message must match exactly. */ +function bindPopupWindow(requestId: string, windowId: number): void { + const entry = pendingSignRequests.get(requestId) + if (!entry || entry.record.windowId !== undefined) return + entry.record = { ...entry.record, windowId } + persistPendingRequests() +} + +/** Terminal (tombstoned) entries are kept around for replay/idempotency, so "idle" means no non-terminal entry remains — not an empty map. */ function clearBadgeIfIdle() { - if (pendingDecisions.size === 0) { + const hasActive = [...pendingSignRequests.values()].some((entry) => !isTerminal(entry.record.state)) + if (!hasActive) { chrome.action.setBadgeText({ text: '' }) } } @@ -323,79 +475,233 @@ function recordFirstDecision(review: AggregatedReview, decision: Decision) { }).catch(() => {}) } -export function requestDecision(requestId: string, review: AggregatedReview): Promise { - const score = scoreForSeverity(review.severity) - const tierInfo = tierForScore(score) - chrome.action.setBadgeText({ text: '!' }) - chrome.action.setBadgeBackgroundColor({ color: tierInfo.colour }) - - return new Promise((resolve) => { - pendingDecisions.set(requestId, { - resolve: (decision) => { - pendingDecisions.delete(requestId) - recordFirstDecision(review, decision) - resolve(decision) - clearBadgeIfIdle() - }, - review, - }) +/** + * Admits a SIGN_REQUEST: validates protocol/sender, enforces admission + * limits, then persists durable pending state *before* building the review + * or opening the popup. Responds with SIGN_ACK immediately once the request + * is durably recorded — it does not wait for a user decision, so this + * message port closes quickly and the worker is not required to stay alive + * for the whole review. The caller resumes/awaits the eventual decision via + * AWAIT_OUTCOME (handleAwaitOutcome), which is safe to call before, during, + * or after a worker restart because it only ever reads durable state. + */ +function handleSignRequest( + message: RuntimeSignRequestMessage, + sender: chrome.runtime.MessageSender, + sendResponse: (response: RuntimeSignAckMessage | RuntimeSignRejectedMessage) => void, +): void { + let responded = false + const respondOnce = (response: RuntimeSignAckMessage | RuntimeSignRejectedMessage) => { + if (responded) return + responded = true + sendResponse(response) + } - const params = new URLSearchParams({ mode: 'intercept', requestId }) + if (message.protocolVersion !== SIGN_PROTOCOL_VERSION) { + respondOnce({ type: 'SIGN_REJECTED', reason: 'protocol-incompatible' }) + return + } + const location = senderLocation(sender) + if (!location) { + respondOnce({ type: 'SIGN_REJECTED', reason: 'sender-unbound' }) + return + } - chrome.windows.create({ - url: chrome.runtime.getURL(`src/popup/index.html?${params.toString()}`), - type: 'popup', - width: 440, - height: 680, + void pendingRequestsRestore + .then(async () => { + pruneStalePendingRequests() + + const activeRecords = [...pendingSignRequests.values()] + .map((entry) => entry.record) + .filter((record) => !isTerminal(record.state)) + const admission = admissionCheck(activeRecords, location.tabId, location.frameId) + if (admission !== 'ok') { + void recordDiagnosticEvent('signing.request.admission_rejected').catch(() => {}) + respondOnce({ + type: 'SIGN_REJECTED', + reason: admission === 'frame-limit' ? 'frame-limit' : 'global-limit', + }) + return + } + + const now = Date.now() + const requestId = crypto.randomUUID() + const record: PendingRequestRecord = { + requestId, + adapter: message.adapter, + tabId: location.tabId, + frameId: location.frameId, + documentId: sender.documentId, + documentBound: sender.documentId !== undefined, + networkPassphrase: message.networkPassphrase, + state: 'received', + protocolVersion: SIGN_PROTOCOL_VERSION, + createdAt: now, + deadlineAt: now + DEFAULT_TIMEOUT_MS, + } + pendingSignRequests.set(requestId, { record, resolvers: [] }) + // Durable before any decode/scoring/UI work — a worker restart mid-decode + // still knows this request exists even if the review itself is lost. + persistPendingRequests() + + respondOnce({ type: 'SIGN_ACK', requestId, deadlineAt: record.deadlineAt }) + + transitionPendingRequest(requestId, 'validating') + const review = await buildAggregatedReview( + message.xdr, + // The current local adapter only accepts account strings. Keep that + // projection here, after the review engine has enforced a typed, + // network-scoped account target. + { getScore: (target) => getScore(target.value) }, + message.networkPassphrase, + ) + + if (!review) { + settlePendingRequest(requestId, 'failed') + return + } + + transitionPendingRequest(requestId, 'assessing') + const withDigest = transitionPendingRequest(requestId, 'awaiting_review', { + xdrDigest: review.review.xdrDigest, + }) + if (!withDigest) return // already settled (e.g. tab closed/navigated while decoding) + + const entry = pendingSignRequests.get(requestId) + if (entry) entry.review = review + + const score = scoreForSeverity(review.severity) + const tierInfo = tierForScore(score) + chrome.action.setBadgeText({ text: '!' }) + chrome.action.setBadgeBackgroundColor({ color: tierInfo.colour }) + + try { + const popupWindow = await createPopupWindow(requestId) + if (typeof popupWindow.id === 'number') bindPopupWindow(requestId, popupWindow.id) + } catch { + settlePendingRequest(requestId, 'failed') + } }) + .catch(() => { + respondOnce({ type: 'SIGN_REJECTED', reason: 'sender-unbound' }) + }) +} + +/** The bridge's resume/status handshake. Safe to call repeatedly and safe to retry after a worker restart: the answer always comes from durable state, never from a resolver that could not have survived suspension. */ +function handleAwaitOutcome( + message: RuntimeAwaitOutcomeMessage, + sendResponse: (response: RuntimeSignOutcomeMessage) => void, +): void { + void pendingRequestsRestore.then(() => { + pruneStalePendingRequests() + const entry = pendingSignRequests.get(message.requestId) + if (!entry) { + sendResponse({ type: 'SIGN_OUTCOME', requestId: message.requestId, outcome: 'cancel' }) + return + } + if (isTerminal(entry.record.state)) { + sendResponse({ + type: 'SIGN_OUTCOME', + requestId: message.requestId, + outcome: outcomeForState(entry.record.state), + }) + return + } + entry.resolvers.push((outcome) => + sendResponse({ type: 'SIGN_OUTCOME', requestId: message.requestId, outcome }), + ) }) } -function reviewResponse(message: RuntimeReviewRequestMessage): RuntimeReviewResponseMessage { - const pending = pendingDecisions.get(message.requestId) - return { type: 'REVIEW_DATA', requestId: message.requestId, review: pending?.review } +/** + * Popup-originated read. Binds (or checks) the review window the same way + * handleDecisionMade does, so copying a review URL into another window + * reveals no transaction: a windowId mismatch gets an empty response, not + * review content. + */ +function handleGetReview( + message: RuntimeReviewRequestMessage, + sender: chrome.runtime.MessageSender, + sendResponse: (response: RuntimeReviewResponseMessage) => void, +): void { + void pendingRequestsRestore.then(() => { + const entry = pendingSignRequests.get(message.requestId) + const senderWindowId = sender.tab?.windowId + if (!entry || typeof senderWindowId !== 'number') { + sendResponse({ type: 'REVIEW_DATA', requestId: message.requestId }) + return + } + if (entry.record.windowId === undefined) { + bindPopupWindow(message.requestId, senderWindowId) + } else if (entry.record.windowId !== senderWindowId) { + void recordDiagnosticEvent('signing.review.window_mismatch').catch(() => {}) + sendResponse({ type: 'REVIEW_DATA', requestId: message.requestId }) + return + } + sendResponse({ type: 'REVIEW_DATA', requestId: message.requestId, review: entry.review }) + }) +} + +/** + * Popup-originated decision. A decision is only honored when: the record is + * still 'awaiting_review' (not already terminal, not yet ready — replay and + * premature decisions are both no-ops), and the sender's own window matches + * the bound review window (or is the first legitimate contact). Nothing + * about the dApp tab/frame/document is re-checked here — that binding is + * enforced by tab close/navigation invalidation below, which settles the + * request the moment the originating context goes away. + */ +function handleDecisionMade( + message: RuntimeDecisionMadeMessage, + sender: chrome.runtime.MessageSender, +): void { + void pendingRequestsRestore.then(() => { + pruneStalePendingRequests() + const entry = pendingSignRequests.get(message.requestId) + if (!entry || entry.record.state !== 'awaiting_review') return + + const senderWindowId = sender.tab?.windowId + if (typeof senderWindowId !== 'number') return + if (entry.record.windowId === undefined) { + bindPopupWindow(message.requestId, senderWindowId) + } else if (entry.record.windowId !== senderWindowId) { + void recordDiagnosticEvent('signing.decision.window_mismatch').catch(() => {}) + return + } + + settlePendingRequest(message.requestId, message.decision) + }) +} + +/** Any pending request bound to this tab/frame is invalidated: an originating tab closing or navigating away can no longer be released or approved. */ +function invalidatePendingRequestsForTab(tabId: number): void { + void pendingRequestsRestore.then(() => { + for (const [requestId, entry] of pendingSignRequests) { + if (entry.record.tabId === tabId && !isTerminal(entry.record.state)) { + settlePendingRequest(requestId, 'cancel') + } + } + }) } chrome.runtime.onMessage.addListener((message: unknown, _sender, sendResponse) => { if (isRuntimeSignRequestMessage(message)) { - resolveReviewOutcome( - message.xdr, - { - // The current local adapter only accepts account strings. Keep that - // projection here, after the review engine has enforced a typed, - // network-scoped account target. - getScore: (target) => getScore(target.value), - requestDecision: (review) => requestDecision(message.requestId, review), - }, - message.networkPassphrase, - ) - .then((outcome) => { - const response: RuntimeSignOutcomeMessage = { - type: 'SIGN_OUTCOME', - requestId: message.requestId, - outcome, - } - sendResponse(response) - }) - .catch(() => { - const response: RuntimeSignOutcomeMessage = { - type: 'SIGN_OUTCOME', - requestId: message.requestId, - outcome: 'cancel', - } - sendResponse(response) - }) + handleSignRequest(message, _sender, sendResponse) + return true + } + if (isRuntimeAwaitOutcomeMessage(message)) { + handleAwaitOutcome(message, sendResponse) return true } if (isRuntimeReviewRequestMessage(message)) { - sendResponse(reviewResponse(message)) - return undefined + handleGetReview(message, _sender, sendResponse) + return true } if (isRuntimeDecisionMadeMessage(message)) { - pendingDecisions.get(message.requestId)?.resolve(message.decision) + handleDecisionMade(message, _sender) } if (isRuntimeProtectionHandshakeMessage(message)) { @@ -503,6 +809,7 @@ chrome.tabs?.onUpdated.addListener((tabId, changeInfo) => { } persistProtectionRecords() }) + invalidatePendingRequestsForTab(tabId) }) chrome.tabs?.onRemoved.addListener((tabId) => { void protectionRestore.then(() => { @@ -513,4 +820,16 @@ chrome.tabs?.onRemoved.addListener((tabId) => { } persistProtectionRecords() }) + invalidatePendingRequestsForTab(tabId) +}) + +/** Closing the review popup settles the request as a cancellation — the user never saw a decision the request could still honor. */ +chrome.windows.onRemoved?.addListener((windowId) => { + void pendingRequestsRestore.then(() => { + for (const [requestId, entry] of pendingSignRequests) { + if (entry.record.windowId === windowId && !isTerminal(entry.record.state)) { + settlePendingRequest(requestId, 'cancel') + } + } + }) }) diff --git a/src/background/messageValidation.test.ts b/src/background/messageValidation.test.ts index cfd177e..2c87afa 100644 --- a/src/background/messageValidation.test.ts +++ b/src/background/messageValidation.test.ts @@ -3,6 +3,7 @@ import { MAX_NETWORK_PASSPHRASE_LENGTH, MAX_REQUEST_ID_LENGTH, MAX_XDR_LENGTH, + isRuntimeAwaitOutcomeMessage, isRuntimeDecisionMadeMessage, isRuntimeProtectionAdapterStatusMessage, isRuntimeProtectionBridgeOnlineMessage, @@ -17,27 +18,42 @@ describe('isRuntimeSignRequestMessage', () => { expect( isRuntimeSignRequestMessage({ type: 'SIGN_REQUEST', - requestId: 'req-1', + protocolVersion: 1, xdr: 'AAAAAg==', + adapter: 'freighter', }), ).toBe(true) expect( isRuntimeSignRequestMessage({ type: 'SIGN_REQUEST', - requestId: 'req-2', + protocolVersion: 1, xdr: 'AAAAAg==', + adapter: 'albedo-popup', networkPassphrase: 'Test SDF Network ; September 2015', }), ).toBe(true) }) + it('has no requestId field at all — the page/bridge boundary correlation id never reaches this message', () => { + expect( + isRuntimeSignRequestMessage({ + type: 'SIGN_REQUEST', + protocolVersion: 1, + xdr: 'AAAAAg==', + adapter: 'freighter', + requestId: 'page-supplied-id', + }), + ).toBe(false) + }) + it('accepts values exactly at each explicit size limit', () => { expect( isRuntimeSignRequestMessage({ type: 'SIGN_REQUEST', - requestId: 'r'.repeat(MAX_REQUEST_ID_LENGTH), + protocolVersion: 1, xdr: 'A'.repeat(MAX_XDR_LENGTH), + adapter: 'freighter', networkPassphrase: 'n'.repeat(MAX_NETWORK_PASSPHRASE_LENGTH), }), ).toBe(true) @@ -50,22 +66,40 @@ describe('isRuntimeSignRequestMessage', () => { [], {}, { type: 'SIGN_REQUEST' }, - { type: 'SIGN_REQUEST', requestId: 'req-1' }, - { type: 'SIGN_REQUEST', requestId: 1, xdr: 'AAAAAg==' }, - { type: 'SIGN_REQUEST', requestId: 'req-1', xdr: 1 }, - { type: 'SIGN_REQUEST', requestId: 'req-1', xdr: 'AAAAAg==', networkPassphrase: 1 }, - { type: 'OTHER', requestId: 'req-1', xdr: 'AAAAAg==' }, + { type: 'SIGN_REQUEST', protocolVersion: 1 }, + { type: 'SIGN_REQUEST', protocolVersion: '1', xdr: 'AAAAAg==', adapter: 'freighter' }, + { type: 'SIGN_REQUEST', protocolVersion: 1, xdr: 1, adapter: 'freighter' }, + { type: 'SIGN_REQUEST', protocolVersion: 1, xdr: 'AAAAAg==', adapter: 'metamask' }, + { type: 'SIGN_REQUEST', protocolVersion: 1, xdr: 'AAAAAg==', adapter: undefined }, + { + type: 'SIGN_REQUEST', + protocolVersion: 1, + xdr: 'AAAAAg==', + adapter: 'freighter', + networkPassphrase: 1, + }, + { type: 'OTHER', protocolVersion: 1, xdr: 'AAAAAg==', adapter: 'freighter' }, ])('rejects malformed values %#', (message) => { expect(isRuntimeSignRequestMessage(message)).toBe(false) }) it.each([ - { type: 'SIGN_REQUEST', requestId: '', xdr: 'AAAAAg==' }, - { type: 'SIGN_REQUEST', requestId: ' ', xdr: 'AAAAAg==' }, - { type: 'SIGN_REQUEST', requestId: 'req-1', xdr: '' }, - { type: 'SIGN_REQUEST', requestId: 'req-1', xdr: ' ' }, - { type: 'SIGN_REQUEST', requestId: 'req-1', xdr: 'AAAAAg==', networkPassphrase: '' }, - { type: 'SIGN_REQUEST', requestId: 'req-1', xdr: 'AAAAAg==', networkPassphrase: ' ' }, + { type: 'SIGN_REQUEST', protocolVersion: 1, xdr: '', adapter: 'freighter' }, + { type: 'SIGN_REQUEST', protocolVersion: 1, xdr: ' ', adapter: 'freighter' }, + { + type: 'SIGN_REQUEST', + protocolVersion: 1, + xdr: 'AAAAAg==', + adapter: 'freighter', + networkPassphrase: '', + }, + { + type: 'SIGN_REQUEST', + protocolVersion: 1, + xdr: 'AAAAAg==', + adapter: 'freighter', + networkPassphrase: ' ', + }, ])('rejects empty required or supplied strings %#', (message) => { expect(isRuntimeSignRequestMessage(message)).toBe(false) }) @@ -73,18 +107,15 @@ describe('isRuntimeSignRequestMessage', () => { it.each([ { type: 'SIGN_REQUEST', - requestId: 'r'.repeat(MAX_REQUEST_ID_LENGTH + 1), - xdr: 'AAAAAg==', - }, - { - type: 'SIGN_REQUEST', - requestId: 'req-1', + protocolVersion: 1, xdr: 'A'.repeat(MAX_XDR_LENGTH + 1), + adapter: 'freighter', }, { type: 'SIGN_REQUEST', - requestId: 'req-1', + protocolVersion: 1, xdr: 'AAAAAg==', + adapter: 'freighter', networkPassphrase: 'n'.repeat(MAX_NETWORK_PASSPHRASE_LENGTH + 1), }, ])('rejects values over each explicit size limit %#', (message) => { @@ -92,6 +123,25 @@ describe('isRuntimeSignRequestMessage', () => { }) }) +describe('isRuntimeAwaitOutcomeMessage', () => { + it('accepts a bounded, closed-set resume/status request', () => { + expect(isRuntimeAwaitOutcomeMessage({ type: 'AWAIT_OUTCOME', requestId: 'req-1' })).toBe(true) + }) + + it.each([ + null, + undefined, + {}, + { type: 'AWAIT_OUTCOME' }, + { type: 'AWAIT_OUTCOME', requestId: '' }, + { type: 'AWAIT_OUTCOME', requestId: 1 }, + { type: 'AWAIT_OUTCOME', requestId: 'req-1', xdr: 'extra' }, + { type: 'AWAIT_OUTCOME', requestId: 'r'.repeat(MAX_REQUEST_ID_LENGTH + 1) }, + ])('rejects malformed values %#', (message) => { + expect(isRuntimeAwaitOutcomeMessage(message)).toBe(false) + }) +}) + describe('protection health message validation', () => { const health = { type: 'PROTECTION_HANDSHAKE', @@ -161,6 +211,7 @@ describe('isRuntimeDecisionMadeMessage', () => { expect( isRuntimeDecisionMadeMessage({ type: 'DECISION_MADE', + protocolVersion: 1, requestId: 'req-1', decision, }), @@ -172,14 +223,16 @@ describe('isRuntimeDecisionMadeMessage', () => { undefined, [], {}, - { type: 'DECISION_MADE' }, - { type: 'DECISION_MADE', requestId: 'req-1' }, - { type: 'DECISION_MADE', requestId: 1, decision: 'proceed' }, - { type: 'DECISION_MADE', requestId: '', decision: 'proceed' }, - { type: 'DECISION_MADE', requestId: 'req-1', decision: 'allow' }, - { type: 'DECISION_MADE', requestId: 'req-1', decision: 1 }, + { type: 'DECISION_MADE', protocolVersion: 1 }, + { type: 'DECISION_MADE', protocolVersion: 1, requestId: 'req-1' }, + { type: 'DECISION_MADE', protocolVersion: 2, requestId: 'req-1', decision: 'proceed' }, + { type: 'DECISION_MADE', protocolVersion: 1, requestId: 1, decision: 'proceed' }, + { type: 'DECISION_MADE', protocolVersion: 1, requestId: '', decision: 'proceed' }, + { type: 'DECISION_MADE', protocolVersion: 1, requestId: 'req-1', decision: 'allow' }, + { type: 'DECISION_MADE', protocolVersion: 1, requestId: 'req-1', decision: 1 }, { type: 'DECISION_MADE', + protocolVersion: 1, requestId: 'r'.repeat(MAX_REQUEST_ID_LENGTH + 1), decision: 'cancel', }, @@ -190,12 +243,27 @@ describe('isRuntimeDecisionMadeMessage', () => { describe('isRuntimeReviewRequestMessage', () => { it('accepts only the bounded, closed-set popup review request', () => { - expect(isRuntimeReviewRequestMessage({ type: 'GET_REVIEW', requestId: 'req-1' })).toBe(true) - expect(isRuntimeReviewRequestMessage({ type: 'GET_REVIEW', requestId: 'req-1', xdr: 'secret' })).toBe(false) - expect(isRuntimeReviewRequestMessage({ type: 'GET_REVIEW', requestId: '' })).toBe(false) + expect( + isRuntimeReviewRequestMessage({ type: 'GET_REVIEW', protocolVersion: 1, requestId: 'req-1' }), + ).toBe(true) + expect( + isRuntimeReviewRequestMessage({ + type: 'GET_REVIEW', + protocolVersion: 1, + requestId: 'req-1', + xdr: 'secret', + }), + ).toBe(false) + expect( + isRuntimeReviewRequestMessage({ type: 'GET_REVIEW', protocolVersion: 2, requestId: 'req-1' }), + ).toBe(false) + expect( + isRuntimeReviewRequestMessage({ type: 'GET_REVIEW', protocolVersion: 1, requestId: '' }), + ).toBe(false) expect( isRuntimeReviewRequestMessage({ type: 'GET_REVIEW', + protocolVersion: 1, requestId: 'r'.repeat(MAX_REQUEST_ID_LENGTH + 1), }), ).toBe(false) diff --git a/src/background/messageValidation.ts b/src/background/messageValidation.ts index 5b5c0ca..54602b1 100644 --- a/src/background/messageValidation.ts +++ b/src/background/messageValidation.ts @@ -1,4 +1,5 @@ import type { + RuntimeAwaitOutcomeMessage, RuntimeDecisionMadeMessage, RuntimeProtectionAdapterStatusMessage, RuntimeProtectionBridgeOnlineMessage, @@ -7,6 +8,7 @@ import type { RuntimeReviewRequestMessage, RuntimeSignRequestMessage, } from '../intercept/protocol' +import { SIGN_PROTOCOL_VERSION } from '../signing/pendingRequestState' import { PROTECTION_PROTOCOL_VERSION } from '../protection/protectionState' // UUID request IDs are currently 36 characters. The larger bound preserves @@ -40,14 +42,34 @@ function isNonEmptyBoundedString(value: unknown, maxLength: number): value is st ) } -/** Validate an untrusted runtime value before XDR decoding or popup creation. */ +function isSigningAdapter(value: unknown): value is 'freighter' | 'albedo-popup' { + return value === 'freighter' || value === 'albedo-popup' +} + +/** + * Validate an untrusted runtime value before XDR decoding or popup + * creation. There is deliberately no `requestId` field: the page/bridge + * boundary's correlation id never crosses into this message, and the + * background is the sole generator of the authoritative request id + * (returned in SIGN_ACK). A mismatched protocolVersion is checked by the + * caller and answered with a typed SIGN_REJECTED, not silently ignored. + */ export function isRuntimeSignRequestMessage( message: unknown, ): message is RuntimeSignRequestMessage { - if (!isRecord(message) || message.type !== 'SIGN_REQUEST') return false - - if (!isNonEmptyBoundedString(message.requestId, MAX_REQUEST_ID_LENGTH)) return false + if ( + !isRecord(message) || + !hasOnlyKeys(message, ['type', 'protocolVersion', 'xdr', 'networkPassphrase', 'adapter']) || + message.type !== 'SIGN_REQUEST' + ) { + return false + } + + if (typeof message.protocolVersion !== 'number' || !Number.isInteger(message.protocolVersion)) { + return false + } if (!isNonEmptyBoundedString(message.xdr, MAX_XDR_LENGTH)) return false + if (!isSigningAdapter(message.adapter)) return false return ( message.networkPassphrase === undefined || @@ -55,14 +77,27 @@ export function isRuntimeSignRequestMessage( ) } +/** Validate the bridge's resume/status handshake before reading durable pending state. */ +export function isRuntimeAwaitOutcomeMessage( + message: unknown, +): message is RuntimeAwaitOutcomeMessage { + return ( + isRecord(message) && + hasOnlyKeys(message, ['type', 'requestId']) && + message.type === 'AWAIT_OUTCOME' && + isNonEmptyBoundedString(message.requestId, MAX_REQUEST_ID_LENGTH) + ) +} + /** Validate a popup decision before looking up or resolving pending state. */ export function isRuntimeDecisionMadeMessage( message: unknown, ): message is RuntimeDecisionMadeMessage { return ( isRecord(message) && - hasOnlyKeys(message, ['type', 'requestId', 'decision']) && + hasOnlyKeys(message, ['type', 'protocolVersion', 'requestId', 'decision']) && message.type === 'DECISION_MADE' && + message.protocolVersion === SIGN_PROTOCOL_VERSION && isNonEmptyBoundedString(message.requestId, MAX_REQUEST_ID_LENGTH) && (message.decision === 'proceed' || message.decision === 'cancel') ) @@ -74,8 +109,9 @@ export function isRuntimeReviewRequestMessage( ): message is RuntimeReviewRequestMessage { return ( isRecord(message) && - hasOnlyKeys(message, ['type', 'requestId']) && + hasOnlyKeys(message, ['type', 'protocolVersion', 'requestId']) && message.type === 'GET_REVIEW' && + message.protocolVersion === SIGN_PROTOCOL_VERSION && isNonEmptyBoundedString(message.requestId, MAX_REQUEST_ID_LENGTH) ) } diff --git a/src/diagnostics/diagnostics.ts b/src/diagnostics/diagnostics.ts index 57fae62..d66a921 100644 --- a/src/diagnostics/diagnostics.ts +++ b/src/diagnostics/diagnostics.ts @@ -56,6 +56,9 @@ export const DIAGNOSTIC_EVENTS = [ 'protection.handshake.success', 'protection.handshake.failure', 'protection.permission.changed', + 'signing.request.admission_rejected', + 'signing.review.window_mismatch', + 'signing.decision.window_mismatch', ] as const export type DiagnosticEvent = (typeof DIAGNOSTIC_EVENTS)[number] diff --git a/src/intercept/albedoMainWorldEntry.ts b/src/intercept/albedoMainWorldEntry.ts index 3c8503e..c1091bc 100644 --- a/src/intercept/albedoMainWorldEntry.ts +++ b/src/intercept/albedoMainWorldEntry.ts @@ -48,8 +48,8 @@ * dApp code, so the patch is in place before albedo-intent is imported. */ -import { WINDOW_REQUEST_TYPE, WINDOW_RESPONSE_TYPE, type Outcome } from './protocol' import { reportAdapterStatus, startProtectionHeartbeat } from './protectionHandshake' +import { requestOutcome } from './requestOutcome' const ALBEDO_CONFIRM_ORIGIN = 'https://albedo.link' const ALBEDO_WINDOW_NAME = 'auth.albedo.link' @@ -63,24 +63,6 @@ startProtectionHeartbeat('albedo-popup') */ const TX_INTENTS = new Set(['tx', 'pay']) -/** Forward a sign-outcome request to the bridge via the shared protocol. */ -function requestOutcome(xdr: string, networkPassphrase?: string): Promise { - const localId = crypto.randomUUID() - - return new Promise((resolve) => { - function onMessage(event: MessageEvent) { - if (event.source !== window) return - const data = event.data as { type?: string; localId?: string; outcome?: string } | undefined - if (data?.type !== WINDOW_RESPONSE_TYPE || data.localId !== localId) return - window.removeEventListener('message', onMessage) - const outcome = data.outcome - resolve(outcome === 'proceed' || outcome === 'allow' ? outcome : 'cancel') - } - window.addEventListener('message', onMessage) - window.postMessage({ type: WINDOW_REQUEST_TYPE, localId, xdr, networkPassphrase }, '*') - }) -} - /** * Build a synthetic albedoIntentResult rejection that looks identical to the * "action rejected by user" error that Albedo normally posts back. @@ -162,7 +144,7 @@ window.open = function grydlockOpen( } // Don't block the thread — run scoring async. - requestOutcome(xdr, network).then((outcome) => { + requestOutcome(xdr, network, 'albedo-popup').then((outcome) => { if (outcome === 'cancel') { // Close the real popup and synthesize a rejection. try { diff --git a/src/intercept/awaitOutcome.test.ts b/src/intercept/awaitOutcome.test.ts new file mode 100644 index 0000000..7c7e9c3 --- /dev/null +++ b/src/intercept/awaitOutcome.test.ts @@ -0,0 +1,106 @@ +import { describe, expect, it, vi } from 'vitest' +import { awaitOutcome, type AwaitOutcomeResponse } from './awaitOutcome' + +// A fake clock/sleep pair: `sleep` advances the fake clock by exactly the +// requested amount instead of waiting on a real timer, per the CI guidance +// to keep non-deterministic/timing-sensitive tests off real timers. +function fakeClock(startAt: number) { + let now = startAt + return { + now: () => now, + sleep: (ms: number) => + new Promise((resolve) => { + now += ms + resolve() + }), + } +} + +describe('awaitOutcome', () => { + it('returns the outcome from the first matching SIGN_OUTCOME response', async () => { + const clock = fakeClock(0) + const sendMessage = vi.fn( + async (): Promise => ({ + type: 'SIGN_OUTCOME', + requestId: 'req-1', + outcome: 'proceed', + }), + ) + + const outcome = await awaitOutcome('req-1', 10_000, { + sendMessage, + now: clock.now, + sleep: clock.sleep, + }) + + expect(outcome).toBe('proceed') + expect(sendMessage).toHaveBeenCalledTimes(1) + expect(sendMessage).toHaveBeenCalledWith('req-1') + }) + + it('retries on a disconnected/undefined response (simulating a dead or restarting worker), then resolves once it reconnects', async () => { + const clock = fakeClock(0) + const sendMessage = vi + .fn<(id: string) => Promise>() + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce({ type: 'SIGN_OUTCOME', requestId: 'req-1', outcome: 'cancel' }) + + const outcome = await awaitOutcome('req-1', 60_000, { + sendMessage, + now: clock.now, + sleep: clock.sleep, + }) + + expect(outcome).toBe('cancel') + expect(sendMessage).toHaveBeenCalledTimes(3) + }) + + it('ignores a response for a different requestId (defense in depth) and keeps retrying', async () => { + const clock = fakeClock(0) + const sendMessage = vi + .fn<(id: string) => Promise>() + .mockResolvedValueOnce({ type: 'SIGN_OUTCOME', requestId: 'someone-elses-request', outcome: 'proceed' }) + .mockResolvedValueOnce({ type: 'SIGN_OUTCOME', requestId: 'req-1', outcome: 'proceed' }) + + const outcome = await awaitOutcome('req-1', 60_000, { + sendMessage, + now: clock.now, + sleep: clock.sleep, + }) + + expect(outcome).toBe('proceed') + expect(sendMessage).toHaveBeenCalledTimes(2) + }) + + it('resolves cancel once the absolute deadline passes without ever needing the background to answer', async () => { + const clock = fakeClock(0) + const sendMessage = vi.fn(async (): Promise => undefined) + + const outcome = await awaitOutcome('req-1', 1_000, { + sendMessage, + now: clock.now, + sleep: clock.sleep, + backoffMs: () => 400, + }) + + expect(outcome).toBe('cancel') + // Deadline is enforced without depending on the background ever responding. + expect(sendMessage.mock.calls.length).toBeGreaterThan(0) + expect(clock.now()).toBeGreaterThanOrEqual(1_000) + }) + + it('never calls sendMessage once the deadline has already passed', async () => { + const clock = fakeClock(5_000) + const sendMessage = vi.fn() + + const outcome = await awaitOutcome('req-1', 1_000, { + sendMessage, + now: clock.now, + sleep: clock.sleep, + }) + + expect(outcome).toBe('cancel') + expect(sendMessage).not.toHaveBeenCalled() + }) +}) diff --git a/src/intercept/awaitOutcome.ts b/src/intercept/awaitOutcome.ts new file mode 100644 index 0000000..629fac0 --- /dev/null +++ b/src/intercept/awaitOutcome.ts @@ -0,0 +1,49 @@ +import type { Outcome } from './protocol' + +export interface AwaitOutcomeResponse { + type: 'SIGN_OUTCOME' + requestId: string + outcome: Outcome +} + +export interface AwaitOutcomeDeps { + /** Resolves `undefined` on any failure (disconnected port, dead/restarting worker) rather than rejecting — a rejection would abort the retry loop instead of triggering a retry. */ + sendMessage: (requestId: string) => Promise + now?: () => number + sleep?: (ms: number) => Promise + backoffMs?: (attempt: number) => number +} + +const DEFAULT_BACKOFF_MS = (attempt: number) => Math.min(250 * attempt, 2_000) + +/** + * The bridge's resume/status handshake loop. Every attempt is a fresh, + * independent round trip — there is no callback held open across a worker + * restart — so a background service worker that suspends or is killed and + * restarted mid-review is transparently reconnected to on the next attempt: + * the answer always comes from the worker's durable state, not from a + * resolver that could not have survived suspension. Bounded by an absolute + * deadline so a missing/unresponsive background can never hang this loop + * past that point; it resolves 'cancel' once the deadline passes. + */ +export async function awaitOutcome( + requestId: string, + deadlineAt: number, + deps: AwaitOutcomeDeps, +): Promise { + const now = deps.now ?? Date.now + const sleep = deps.sleep ?? ((ms: number) => new Promise((resolve) => setTimeout(resolve, ms))) + const backoffMs = deps.backoffMs ?? DEFAULT_BACKOFF_MS + + let attempt = 0 + while (now() < deadlineAt) { + const response = await deps.sendMessage(requestId) + if (response?.type === 'SIGN_OUTCOME' && response.requestId === requestId) { + return response.outcome + } + attempt += 1 + await sleep(backoffMs(attempt)) + } + + return 'cancel' +} diff --git a/src/intercept/bridgeEntry.ts b/src/intercept/bridgeEntry.ts index 033da0e..36c5a57 100644 --- a/src/intercept/bridgeEntry.ts +++ b/src/intercept/bridgeEntry.ts @@ -1,4 +1,6 @@ import { + BRIDGE_FALLBACK_DEADLINE_MS, + SIGN_PROTOCOL_VERSION, WINDOW_PROTECTION_ACK_TYPE, WINDOW_PROTECTION_ADAPTER_STATUS_TYPE, WINDOW_PROTECTION_PROBE_TYPE, @@ -9,13 +11,33 @@ import { type RuntimeProtectionBridgeOnlineMessage, type RuntimeProtectionHandshakeAckMessage, type RuntimeProtectionHandshakeMessage, - type RuntimeSignOutcomeMessage, + type RuntimeSignAckMessage, + type RuntimeSignRejectedMessage, type RuntimeSignRequestMessage, } from './protocol' import { PROTECTION_PROTOCOL_VERSION } from '../protection/protectionState' +import { awaitOutcome, type AwaitOutcomeResponse } from './awaitOutcome' +import type { SigningAdapter } from '../signing/pendingRequestState' const MAX_NONCE_LENGTH = 128 +function isSigningAdapter(value: unknown): value is SigningAdapter { + return value === 'freighter' || value === 'albedo-popup' +} + +/** Resolves `undefined` on any failure (disconnected port, dead/restarting worker) instead of rejecting, so a caller can retry rather than abort. */ +function sendRuntimeMessage(message: unknown): Promise { + return new Promise((resolve) => { + chrome.runtime.sendMessage(message, (response: TResponse | undefined) => { + if (chrome.runtime.lastError) { + resolve(undefined) + return + } + resolve(response) + }) + }) +} + function isProtectionAdapter(value: unknown): value is 'freighter' | 'albedo-popup' { return value === 'freighter' || value === 'albedo-popup' } @@ -39,24 +61,49 @@ window.addEventListener('message', (event) => { requestId?: string xdr?: string networkPassphrase?: string + adapter?: unknown } | undefined + // `localId`/`requestId` here is only the page/bridge boundary's own + // correlation token for matching this response to the caller's promise — + // it is never forwarded to the background and never treated as an + // authoritative capability. The background generates its own requestId + // (returned in SIGN_ACK below) and that is the only id used for pending + // state, popup URLs, or decision binding. const localId = data?.localId ?? data?.requestId - if (data?.type !== WINDOW_REQUEST_TYPE || !localId || !data.xdr) return - - const message: RuntimeSignRequestMessage = { - type: 'SIGN_REQUEST', - requestId: localId, - xdr: data.xdr, - networkPassphrase: data.networkPassphrase, + if (data?.type !== WINDOW_REQUEST_TYPE || !localId || !data.xdr || !isSigningAdapter(data.adapter)) { + return } - chrome.runtime.sendMessage(message, (response: RuntimeSignOutcomeMessage | undefined) => { - window.postMessage( - { type: WINDOW_RESPONSE_TYPE, localId, outcome: response?.outcome ?? 'cancel' }, - '*', + void (async () => { + const signRequest: RuntimeSignRequestMessage = { + type: 'SIGN_REQUEST', + protocolVersion: SIGN_PROTOCOL_VERSION, + xdr: data.xdr as string, + networkPassphrase: data.networkPassphrase, + adapter: data.adapter as SigningAdapter, + } + + const ackOrRejection = await sendRuntimeMessage( + signRequest, ) - }) + if (!ackOrRejection || ackOrRejection.type !== 'SIGN_ACK') { + window.postMessage({ type: WINDOW_RESPONSE_TYPE, localId, outcome: 'cancel' }, '*') + return + } + + // Bounded by whichever deadline is sooner: the worker's own authoritative + // deadline, or this bridge-side fallback that fires even if the worker + // never answers another AWAIT_OUTCOME call again (e.g. uninstalled). + const deadlineAt = Math.min(ackOrRejection.deadlineAt, Date.now() + BRIDGE_FALLBACK_DEADLINE_MS) + const requestId = ackOrRejection.requestId + const outcome = await awaitOutcome(requestId, deadlineAt, { + sendMessage: (id) => + sendRuntimeMessage({ type: 'AWAIT_OUTCOME', requestId: id }), + }) + + window.postMessage({ type: WINDOW_RESPONSE_TYPE, localId, outcome }, '*') + })() }) window.addEventListener('message', (event) => { diff --git a/src/intercept/mainWorldEntry.ts b/src/intercept/mainWorldEntry.ts index 9bffbc5..a520ea9 100644 --- a/src/intercept/mainWorldEntry.ts +++ b/src/intercept/mainWorldEntry.ts @@ -1,5 +1,5 @@ -import { WINDOW_REQUEST_TYPE, WINDOW_RESPONSE_TYPE, type Outcome } from './protocol' import { reportAdapterStatus, startProtectionHeartbeat } from './protectionHandshake' +import { requestOutcome } from './requestOutcome' /** * Real Freighter (`@stellar/freighter-api`) does not expose a callable @@ -25,23 +25,6 @@ interface FreighterSubmitTransactionRequest { __grydlockReviewed?: boolean } -function requestOutcome(xdr: string, networkPassphrase?: string): Promise { - const localId = crypto.randomUUID() - - return new Promise((resolve) => { - function onMessage(event: MessageEvent) { - if (event.source !== window) return - const data = event.data as { type?: string; localId?: string; outcome?: string } | undefined - if (data?.type !== WINDOW_RESPONSE_TYPE || data.localId !== localId) return - window.removeEventListener('message', onMessage) - const outcome = data.outcome - resolve(outcome === 'proceed' || outcome === 'allow' ? outcome : 'cancel') - } - window.addEventListener('message', onMessage) - window.postMessage({ type: WINDOW_REQUEST_TYPE, localId, xdr, networkPassphrase }, '*') - }) -} - /** * Grabs an outgoing SUBMIT_TRANSACTION request before Freighter's own * content-script listener sees it (registration-order dependent — this @@ -76,7 +59,7 @@ window.addEventListener( const request = data as FreighterSubmitTransactionRequest const networkPassphrase = request.networkPassphrase ?? request.network - requestOutcome(request.transactionXdr, networkPassphrase).then((outcome) => { + requestOutcome(request.transactionXdr, networkPassphrase, 'freighter').then((outcome) => { if (outcome === 'cancel') { window.postMessage( { diff --git a/src/intercept/protocol.ts b/src/intercept/protocol.ts index b636c06..1e5d5f8 100644 --- a/src/intercept/protocol.ts +++ b/src/intercept/protocol.ts @@ -1,6 +1,21 @@ +import { SIGN_PROTOCOL_VERSION, type SigningAdapter } from '../signing/pendingRequestState' + export type Decision = 'proceed' | 'cancel' export type Outcome = 'allow' | Decision +export { SIGN_PROTOCOL_VERSION } + +/** + * Deadlines are layered so a lower layer's fail-closed cancel arrives before + * a higher layer gives up unilaterally: background's REVIEW_DEADLINE_MS + * (src/signing/pendingRequestState.ts, 90s) is authoritative; the bridge's + * fallback covers a missing/unresponsive background; the page's covers a + * missing/unresponsive bridge (e.g. a stale content script after an + * extension update). Each is a final backstop, not the common path. + */ +export const BRIDGE_FALLBACK_DEADLINE_MS = 100_000 +export const PAGE_DEADLINE_MS = 120_000 + export const WINDOW_REQUEST_TYPE = 'GRYDLOCK_REQUEST_OUTCOME' export const WINDOW_RESPONSE_TYPE = 'GRYDLOCK_OUTCOME_RESPONSE' export const WINDOW_PROTECTION_PROBE_TYPE = 'GRYDLOCK_PROTECTION_PROBE' @@ -49,11 +64,44 @@ export interface RuntimeExportDiagnosticsMessage { type: 'EXPORT_DIAGNOSTICS' } +/** + * The page/bridge boundary's own correlation id (`localId` in + * mainWorldEntry.ts/albedoMainWorldEntry.ts) never crosses into this + * message: it is not a parameter here and the background never sees it. The + * background generates its own `requestId` in SIGN_ACK and that id — never + * anything page-supplied — is the only identifier treated as authoritative + * for pending state, popup URLs, or decision binding. + */ export interface RuntimeSignRequestMessage { type: 'SIGN_REQUEST' - requestId: string + protocolVersion: number xdr: string networkPassphrase?: string + adapter: SigningAdapter +} + +/** Closes the SIGN_REQUEST message port immediately; the worker is not required to stay alive for the rest of the review. */ +export interface RuntimeSignAckMessage { + type: 'SIGN_ACK' + requestId: string + deadlineAt: number +} + +/** A typed, explicit rejection — never a silent bypass — for an incompatible protocol version, unbindable sender, or admission-control limit. */ +export interface RuntimeSignRejectedMessage { + type: 'SIGN_REJECTED' + reason: 'protocol-incompatible' | 'sender-unbound' | 'frame-limit' | 'global-limit' +} + +/** + * The bridge's resume/status handshake: safe to call repeatedly (idempotent + * against a terminal record) and safe to retry after a worker restart, + * since the answer is read from durable state, not from a resolver that + * could not have survived suspension. + */ +export interface RuntimeAwaitOutcomeMessage { + type: 'AWAIT_OUTCOME' + requestId: string } export interface RuntimeSignOutcomeMessage { @@ -64,12 +112,14 @@ export interface RuntimeSignOutcomeMessage { export interface RuntimeDecisionMadeMessage { type: 'DECISION_MADE' + protocolVersion: number requestId: string decision: Decision } export interface RuntimeReviewRequestMessage { type: 'GET_REVIEW' + protocolVersion: number requestId: string } diff --git a/src/intercept/requestOutcome.test.ts b/src/intercept/requestOutcome.test.ts new file mode 100644 index 0000000..0ec6bc3 --- /dev/null +++ b/src/intercept/requestOutcome.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, it, vi } from 'vitest' +import { requestOutcome } from './requestOutcome' +import { PAGE_DEADLINE_MS, SIGN_PROTOCOL_VERSION, WINDOW_REQUEST_TYPE } from './protocol' + +interface CapturedRequest { + type: string + protocolVersion: number + localId: string + xdr: string + networkPassphrase?: string + adapter: string +} + +function captureRequest(): Promise { + return new Promise((resolve) => { + function onMessage(event: MessageEvent) { + const data = event.data as Partial | undefined + if (data?.type !== WINDOW_REQUEST_TYPE) return + window.removeEventListener('message', onMessage) + resolve(data as CapturedRequest) + } + window.addEventListener('message', onMessage) + }) +} + +/** + * jsdom does not set `event.source` for a same-window postMessage (it comes + * through as something other than `window`), so requestOutcome's own + * `event.source !== window` guard — a deliberate, load-bearing check that + * rejects responses from other frames/windows — can never be satisfied here. + * That makes the request/response round trip untestable in this + * environment; it's the same reason mainWorldEntry.ts/bridgeEntry.ts have + * never had direct unit tests and are excluded from the coverage gate (see + * vite.config.ts). The round trip, including the source check, is covered + * by e2e/signTransaction.spec.ts against a real Chromium instance instead. + * What jsdom *can* verify: the outgoing request's shape, and the page-side + * deadline, which settles via a timer rather than a message. + */ +describe('requestOutcome', () => { + it('posts a versioned, adapter-tagged window request carrying a fresh correlation id', async () => { + const captured = captureRequest() + void requestOutcome('xdr-payload', 'Test SDF Network ; September 2015', 'freighter') + const posted = await captured + + expect(posted.type).toBe(WINDOW_REQUEST_TYPE) + expect(posted.protocolVersion).toBe(SIGN_PROTOCOL_VERSION) + expect(posted.xdr).toBe('xdr-payload') + expect(posted.networkPassphrase).toBe('Test SDF Network ; September 2015') + expect(posted.adapter).toBe('freighter') + expect(typeof posted.localId).toBe('string') + expect(posted.localId.length).toBeGreaterThan(0) + }) + + it('tags the outgoing request with the caller-supplied adapter and omits an absent network passphrase', async () => { + const captured = captureRequest() + void requestOutcome('xdr-payload', undefined, 'albedo-popup') + const posted = await captured + + expect(posted.adapter).toBe('albedo-popup') + expect(posted.networkPassphrase).toBeUndefined() + }) + + it('two concurrent calls get two distinct, unguessable correlation ids', async () => { + const first = captureRequest() + void requestOutcome('xdr-a', undefined, 'freighter') + const postedFirst = await first + + const second = captureRequest() + void requestOutcome('xdr-b', undefined, 'freighter') + const postedSecond = await second + + expect(postedFirst.localId).not.toBe(postedSecond.localId) + }) + + it('resolves cancel once the page-side absolute deadline elapses without any response — the bridge/background may be gone entirely', async () => { + vi.useFakeTimers() + try { + const promise = requestOutcome('xdr-payload', undefined, 'freighter') + await vi.advanceTimersByTimeAsync(PAGE_DEADLINE_MS) + await expect(promise).resolves.toBe('cancel') + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/src/intercept/requestOutcome.ts b/src/intercept/requestOutcome.ts new file mode 100644 index 0000000..be5f0e8 --- /dev/null +++ b/src/intercept/requestOutcome.ts @@ -0,0 +1,63 @@ +import { + PAGE_DEADLINE_MS, + SIGN_PROTOCOL_VERSION, + WINDOW_REQUEST_TYPE, + WINDOW_RESPONSE_TYPE, + type Outcome, +} from './protocol' +import type { SigningAdapter } from '../signing/pendingRequestState' + +/** + * Shared by mainWorldEntry.ts (Freighter) and albedoMainWorldEntry.ts + * (Albedo) — previously duplicated verbatim in both files. `localId` is a + * same-window postMessage correlation token only; it never crosses the + * bridge into the background worker and is never treated as an + * authoritative capability anywhere downstream. + * + * A page-side absolute deadline is required because the bridge content + * script can be stale or missing entirely (e.g. after an extension update + * replaces bridge.js but this MAIN-world script, injected into an + * already-open tab, keeps running the old build) — without it, a request + * whose response never arrives would hang the dApp's promise forever. + */ +export function requestOutcome( + xdr: string, + networkPassphrase: string | undefined, + adapter: SigningAdapter, +): Promise { + const localId = crypto.randomUUID() + + return new Promise((resolve) => { + let settled = false + + function settle(outcome: Outcome) { + if (settled) return + settled = true + window.removeEventListener('message', onMessage) + window.clearTimeout(deadline) + resolve(outcome) + } + + function onMessage(event: MessageEvent) { + if (event.source !== window) return + const data = event.data as { type?: string; localId?: string; outcome?: string } | undefined + if (data?.type !== WINDOW_RESPONSE_TYPE || data.localId !== localId) return + const outcome = data.outcome + settle(outcome === 'proceed' || outcome === 'allow' ? outcome : 'cancel') + } + + window.addEventListener('message', onMessage) + const deadline = window.setTimeout(() => settle('cancel'), PAGE_DEADLINE_MS) + window.postMessage( + { + type: WINDOW_REQUEST_TYPE, + protocolVersion: SIGN_PROTOCOL_VERSION, + localId, + xdr, + networkPassphrase, + adapter, + }, + '*', + ) + }) +} diff --git a/src/intercept/resolveOutcome.ts b/src/intercept/resolveOutcome.ts index b5d3071..806ca39 100644 --- a/src/intercept/resolveOutcome.ts +++ b/src/intercept/resolveOutcome.ts @@ -25,17 +25,24 @@ export interface ResolveReviewOutcomeDeps { } /** - * Production review path. A malformed envelope is deliberately presented as - * an incomplete review instead of being allowed; opaque effects always reach - * the user and only account targets are eligible for destination assessment. + * Builds the bounded, scored review without waiting on a user decision. A + * malformed envelope deliberately produces `null` instead of an implicit + * allow; opaque effects always reach the user and only account targets are + * eligible for destination assessment. + * + * Split out from resolveReviewOutcome so a caller (the background worker's + * signing state machine) can persist durable pending-request state and + * respond to its caller before a user decision exists, rather than holding + * a message port open for the full review-plus-popup duration — a worker + * kept alive only by an open port is not resilient to MV3 suspension. */ -export async function resolveReviewOutcome( +export async function buildAggregatedReview( xdr: string, - deps: ResolveReviewOutcomeDeps, + deps: Pick, networkPassphrase?: string, -): Promise { +): Promise { const review = (deps.extractReview ?? extractTransactionReview)(xdr, networkPassphrase) - if (!review) return 'cancel' + if (!review) return null const evidence: TargetEvidence[] = await Promise.all( scoreableTargets(review).map(async (target) => { @@ -50,7 +57,18 @@ export async function resolveReviewOutcome( }), ) - return deps.requestDecision(aggregateReview(review, evidence)) + return aggregateReview(review, evidence) +} + +/** Production review path used where a single Promise spanning the whole review-plus-decision is acceptable (e.g. tests, the legacy non-durable caller). */ +export async function resolveReviewOutcome( + xdr: string, + deps: ResolveReviewOutcomeDeps, + networkPassphrase?: string, +): Promise { + const review = await buildAggregatedReview(xdr, deps, networkPassphrase) + if (!review) return 'cancel' + return deps.requestDecision(review) } function tierForScore(score: number): 'low' | 'elevated' | 'high' | 'critical' { diff --git a/src/popup/App.test.tsx b/src/popup/App.test.tsx index 44def43..7a3dd08 100644 --- a/src/popup/App.test.tsx +++ b/src/popup/App.test.tsx @@ -134,68 +134,35 @@ describe('App', () => { describe('App in intercept mode', () => { const originalChrome = globalThis.chrome - beforeEach(() => { - vi.restoreAllMocks() - // @ts-expect-error test-only stub of the chrome extension API - globalThis.chrome = { runtime: { sendMessage: vi.fn() } } - }) - - afterEach(() => { - globalThis.chrome = originalChrome - window.history.pushState(null, '', '/') - }) - - it('renders the tier from URL params without calling the adapter', async () => { - const getScoreSpy = vi.spyOn(adapter, 'getScore') - window.history.pushState(null, '', '?mode=intercept&requestId=req-1&destination=GDEST&score=85') - const { container } = render() - expect(screen.getByText(/critical risk/i)).toBeInTheDocument() - expect(screen.getByText('GDEST')).toBeInTheDocument() - expect(screen.getByText(/critical risk/i).closest('.popup')).toHaveAttribute( - 'data-tier', - 'critical', - ) - expect(getScoreSpy).not.toHaveBeenCalled() - - // a11y check - const results = await axe(container) - expect(results).toHaveNoViolations() - }) - - it('loads and renders worker-resident review data without placing it in the URL', async () => { - const review = { - severity: 'high' as const, + function reviewFixture(severity: 'info' | 'warning' | 'high' | 'critical') { + return { + severity, evidence: [], - findings: [ - { - code: 'authority-change', - severity: 'high' as const, - title: 'Account authority change', - detail: 'Signer changed.', - operationIndex: 0, - }, - ], + findings: + severity === 'high' + ? [ + { + code: 'authority-change', + severity: 'high' as const, + title: 'Account authority change', + detail: 'Signer changed.', + operationIndex: 0, + }, + ] + : [], review: { schemaVersion: 1 as const, policyVersion: 1 as const, networkPassphrase: 'Custom network', xdrDigest: 'a'.repeat(64), envelope: { type: 'transaction' as const, source: 'GSOURCE', operationCount: 1 }, - operations: [ - { - index: 0, - type: 'setOptions', - source: 'GSOURCE', - coverage: 'understood' as const, - summary: 'Change account options', - facts: [], - targets: [], - findings: [], - }, - ], + operations: [], findings: [], }, } + } + + function mockReviewResponse(review: ReturnType | undefined) { vi.mocked(chrome.runtime.sendMessage).mockImplementation((message, callback) => { if ( (message as { type?: string }).type === 'GET_REVIEW' && @@ -204,36 +171,88 @@ describe('App in intercept mode', () => { callback({ review }) } }) + } + + beforeEach(() => { + vi.restoreAllMocks() + // @ts-expect-error test-only stub of the chrome extension API + globalThis.chrome = { runtime: { sendMessage: vi.fn() } } + }) + + afterEach(() => { + globalThis.chrome = originalChrome + window.history.pushState(null, '', '/') + }) + + it('requests the review with the versioned protocol and the opaque requestId only — nothing else from the URL', async () => { + mockReviewResponse(reviewFixture('info')) window.history.pushState(null, '', '?mode=intercept&requestId=req-review') render() - expect(await screen.findByText('Custom network')).toBeInTheDocument() - expect(screen.getByRole('alert')).toHaveTextContent(/account authority change/i) + await screen.findByText(/low risk/i) expect(chrome.runtime.sendMessage).toHaveBeenCalledWith( - { type: 'GET_REVIEW', requestId: 'req-review' }, + { type: 'GET_REVIEW', protocolVersion: 1, requestId: 'req-review' }, expect.any(Function), ) + }) + + it('loads and renders worker-resident review data without placing it in the URL', async () => { + mockReviewResponse(reviewFixture('high')) + window.history.pushState(null, '', '?mode=intercept&requestId=req-review') + + const { container } = render() + + expect(await screen.findByText('Custom network')).toBeInTheDocument() + expect(screen.getByRole('alert')).toHaveTextContent(/account authority change/i) expect(window.location.search).not.toContain('digest') + + // a11y check + const results = await axe(container) + expect(results).toHaveNoViolations() }) - it('sends the decision and closes on Proceed', () => { - window.history.pushState(null, '', '?mode=intercept&requestId=req-1&destination=GDEST&score=10') + it('fails closed — shows a reject-only state, not a reassuring low-risk default — when no review can be loaded', async () => { + mockReviewResponse(undefined) + window.history.pushState(null, '', '?mode=intercept&requestId=req-1') const closeSpy = vi.spyOn(window, 'close').mockImplementation(() => {}) + render() - fireEvent.click(screen.getByText('Proceed')) + + expect(await screen.findByText(/could not be loaded for review/i)).toBeInTheDocument() + expect(screen.queryByText(/low risk/i)).not.toBeInTheDocument() + expect(screen.queryByText('Proceed')).not.toBeInTheDocument() + + fireEvent.click(screen.getByText('Reject')) expect(chrome.runtime.sendMessage).toHaveBeenCalledWith({ type: 'DECISION_MADE', + protocolVersion: 1, + requestId: 'req-1', + decision: 'cancel', + }) + expect(closeSpy).toHaveBeenCalled() + }) + + it('sends the decision and closes on Proceed', async () => { + mockReviewResponse(reviewFixture('info')) + window.history.pushState(null, '', '?mode=intercept&requestId=req-1') + const closeSpy = vi.spyOn(window, 'close').mockImplementation(() => {}) + render() + fireEvent.click(await screen.findByText('Proceed')) + expect(chrome.runtime.sendMessage).toHaveBeenCalledWith({ + type: 'DECISION_MADE', + protocolVersion: 1, requestId: 'req-1', decision: 'proceed', }) expect(closeSpy).toHaveBeenCalled() }) - it('blocks high-risk proceed until the user confirms', () => { - window.history.pushState(null, '', '?mode=intercept&requestId=req-1&destination=GDEST&score=60') + it('blocks high-risk proceed until the user confirms', async () => { + mockReviewResponse(reviewFixture('high')) + window.history.pushState(null, '', '?mode=intercept&requestId=req-1') render() - const proceedButton = screen.getByText('Proceed') + const proceedButton = await screen.findByText('Proceed') expect(proceedButton).toBeDisabled() fireEvent.click( screen.getByLabelText(/i understand this destination shows strong risk signals/i), @@ -241,24 +260,28 @@ describe('App in intercept mode', () => { expect(proceedButton).toBeEnabled() }) - it('blocks critical-risk proceed until the user types the confirmation phrase', () => { - window.history.pushState(null, '', '?mode=intercept&requestId=req-1&destination=GDEST&score=85') + it('blocks critical-risk proceed until the user types the confirmation phrase', async () => { + mockReviewResponse(reviewFixture('critical')) + window.history.pushState(null, '', '?mode=intercept&requestId=req-1') render() - const proceedButton = screen.getByText('Proceed') + const proceedButton = await screen.findByText('Proceed') + const input = screen.getByLabelText(/type critical to enable proceed/i) expect(proceedButton).toBeDisabled() - fireEvent.change(screen.getByLabelText(/type critical to enable proceed/i), { - target: { value: 'critical' }, - }) + fireEvent.change(input, { target: { value: 'high' } }) + expect(proceedButton).toBeDisabled() + fireEvent.change(input, { target: { value: 'critical' } }) expect(proceedButton).toBeEnabled() }) - it('sends cancel and closes on Cancel', () => { - window.history.pushState(null, '', '?mode=intercept&requestId=req-1&destination=GDEST&score=10') + it('sends cancel and closes on Cancel', async () => { + mockReviewResponse(reviewFixture('info')) + window.history.pushState(null, '', '?mode=intercept&requestId=req-1') const closeSpy = vi.spyOn(window, 'close').mockImplementation(() => {}) render() - fireEvent.click(screen.getByText('Cancel')) + fireEvent.click(await screen.findByText('Cancel')) expect(chrome.runtime.sendMessage).toHaveBeenCalledWith({ type: 'DECISION_MADE', + protocolVersion: 1, requestId: 'req-1', decision: 'cancel', }) @@ -266,33 +289,28 @@ describe('App in intercept mode', () => { }) it('sends cancel and closes when Escape is pressed', async () => { - window.history.pushState(null, '', '?mode=intercept&requestId=req-1&destination=GDEST&score=85') + mockReviewResponse(reviewFixture('critical')) + window.history.pushState(null, '', '?mode=intercept&requestId=req-1') const closeSpy = vi.spyOn(window, 'close').mockImplementation(() => {}) render() + await screen.findByText('Proceed') await userEvent.setup().keyboard('{Escape}') expect(chrome.runtime.sendMessage).toHaveBeenCalledWith({ type: 'DECISION_MADE', + protocolVersion: 1, requestId: 'req-1', decision: 'cancel', }) expect(closeSpy).toHaveBeenCalled() }) - it('focuses Cancel so a critical warning can be dismissed immediately', () => { - window.history.pushState(null, '', '?mode=intercept&requestId=req-1&destination=GDEST&score=85') + it('focuses Cancel so a critical warning can be dismissed immediately', async () => { + mockReviewResponse(reviewFixture('critical')) + window.history.pushState(null, '', '?mode=intercept&requestId=req-1') render() + await screen.findByText('Proceed') expect(screen.getByRole('button', { name: 'Cancel' })).toHaveFocus() }) - - it('falls back to an empty destination list when destinations JSON is malformed', () => { - window.history.pushState( - null, - '', - '?mode=intercept&requestId=req-1&destinations=%7Bbad-json&score=10', - ) - render() - expect(screen.getByText(/low risk/i)).toBeInTheDocument() - }) }) diff --git a/src/popup/App.tsx b/src/popup/App.tsx index 79bac96..da09b3d 100644 --- a/src/popup/App.tsx +++ b/src/popup/App.tsx @@ -5,7 +5,7 @@ import DevScoreSlider from './DevScoreSlider' import TierWarning from './TierWarning' import TrustedAddressesManager from './TrustedAddressesManager' import ProtectionStatusPanel from './ProtectionStatus' -import type { RuntimeDecisionMadeMessage } from '../intercept/protocol' +import { SIGN_PROTOCOL_VERSION, type RuntimeDecisionMadeMessage } from '../intercept/protocol' import type { AggregatedReview } from '../review/model' import { tierForReviewSeverity } from '../review/policy' import './App.css' @@ -24,12 +24,6 @@ type PreviewState = | 'dev-slider' | 'review' -interface DestinationRow { - destination: string - asset?: string - score: number -} - const REVIEW_PREVIEW: AggregatedReview = { severity: 'high', evidence: [], @@ -147,46 +141,78 @@ function PreviewView({ preview }: { preview: PreviewState }) { ) } +type ReviewLoadState = 'loading' | 'unavailable' | { review: AggregatedReview } + +/** + * Fails closed when review data cannot be loaded (e.g. a worker restart + * mid-review lost the in-memory content, or this popup's window doesn't + * match the request's bound review window) instead of falling back to a + * URL-supplied score. That fallback used to default to a reassuring 'low' + * tier via a always-empty destinations/score URL param, silently + * misrepresenting an indeterminate review as low risk — the one thing this + * warning exists to never do. + */ function InterceptView({ params }: { params: URLSearchParams }) { const requestId = params.get('requestId') ?? '' - const destinationsJson = params.get('destinations') - const score = Number(params.get('score') ?? '0') - const tier = tierForScore(score) - const [review, setReview] = useState() + const canFetchReview = Boolean(requestId && chrome?.runtime?.sendMessage) + const [reviewState, setReviewState] = useState( + canFetchReview ? 'loading' : 'unavailable', + ) useEffect(() => { - if (!requestId || !chrome?.runtime?.sendMessage) return - chrome.runtime.sendMessage({ type: 'GET_REVIEW', requestId }, (response: { review?: AggregatedReview } | undefined) => { - if (response?.review) setReview(response.review) - }) - }, [requestId]) - - let destinations: DestinationRow[] = [] - if (destinationsJson) { - try { - destinations = JSON.parse(destinationsJson) - } catch { - destinations = [] - } - } else { - const destination = params.get('destination') ?? '' - const asset = params.get('asset') ?? undefined - if (destination) { - destinations = [{ destination, asset, score }] + if (!canFetchReview) return + let cancelled = false + chrome.runtime.sendMessage( + { type: 'GET_REVIEW', protocolVersion: SIGN_PROTOCOL_VERSION, requestId }, + (response: { review?: AggregatedReview } | undefined) => { + if (cancelled) return + setReviewState(response?.review ? { review: response.review } : 'unavailable') + }, + ) + return () => { + cancelled = true } - } + }, [canFetchReview, requestId]) function respond(decision: 'proceed' | 'cancel') { - const message: RuntimeDecisionMadeMessage = { type: 'DECISION_MADE', requestId, decision } + const message: RuntimeDecisionMadeMessage = { + type: 'DECISION_MADE', + protocolVersion: SIGN_PROTOCOL_VERSION, + requestId, + decision, + } chrome.runtime.sendMessage(message) window.close() } + if (reviewState === 'loading') { + return
Checking destination…
+ } + + if (reviewState === 'unavailable') { + return ( +
+

+ This request could not be loaded for review. For your safety, reject it and retry from + the dApp. +

+ +
+ ) + } + + const { review } = reviewState + const tier = tierForScore( + { low: 10, elevated: 35, high: 60, critical: 85 }[tierForReviewSeverity(review.severity)], + ) + const score = { info: 10, warning: 35, high: 60, critical: 85 }[review.severity] + return ( respond('cancel')} onProceed={() => respond('proceed')} diff --git a/src/signing/pendingRequestState.test.ts b/src/signing/pendingRequestState.test.ts new file mode 100644 index 0000000..07668ae --- /dev/null +++ b/src/signing/pendingRequestState.test.ts @@ -0,0 +1,242 @@ +import { describe, expect, it } from 'vitest' +import { + MAX_PENDING_GLOBAL, + MAX_PENDING_PER_FRAME, + REVIEW_DEADLINE_MS, + TOMBSTONE_TTL_MS, + admissionCheck, + applyTransition, + canTransition, + isExpired, + isTerminal, + outcomeForState, + shouldPrune, + validatePendingRequestRecord, + type PendingRequestRecord, + type PendingState, +} from './pendingRequestState' + +const NOW = 1_000_000 + +function makeRecord(overrides: Partial = {}): PendingRequestRecord { + return { + requestId: 'req-1', + adapter: 'freighter', + tabId: 1, + frameId: 0, + documentBound: true, + state: 'received', + protocolVersion: 1, + createdAt: NOW, + deadlineAt: NOW + REVIEW_DEADLINE_MS, + ...overrides, + } +} + +describe('canTransition / applyTransition', () => { + it('allows the full happy path in order', () => { + const path: PendingState[] = [ + 'received', + 'validating', + 'assessing', + 'awaiting_review', + 'proceed', + ] + for (let index = 0; index < path.length - 1; index += 1) { + expect(canTransition(path[index], path[index + 1])).toBe(true) + } + }) + + it('rejects skipping states', () => { + expect(canTransition('received', 'awaiting_review')).toBe(false) + expect(canTransition('received', 'assessing')).toBe(false) + expect(canTransition('validating', 'awaiting_review')).toBe(false) + }) + + it('rejects any transition out of a terminal state', () => { + for (const terminal of ['proceed', 'cancel', 'expired', 'failed'] as const) { + for (const next of ['received', 'validating', 'assessing', 'awaiting_review'] as const) { + expect(canTransition(terminal, next)).toBe(false) + } + } + }) + + it('every state can reach a terminal state directly except the states already terminal', () => { + for (const state of ['received', 'validating', 'assessing'] as const) { + expect(canTransition(state, 'failed')).toBe(true) + expect(canTransition(state, 'expired')).toBe(true) + } + expect(canTransition('awaiting_review', 'proceed')).toBe(true) + expect(canTransition('awaiting_review', 'cancel')).toBe(true) + expect(canTransition('awaiting_review', 'expired')).toBe(true) + expect(canTransition('awaiting_review', 'failed')).toBe(true) + }) + + it('applyTransition returns null (no-op) for an invalid transition', () => { + const record = makeRecord({ state: 'received' }) + expect(applyTransition(record, 'awaiting_review', NOW)).toBeNull() + }) + + it('the first valid terminal transition wins; replaying the SAME terminal state is an idempotent no-op that returns the unchanged record', () => { + const record = makeRecord({ state: 'awaiting_review' }) + const settled = applyTransition(record, 'proceed', NOW) + expect(settled?.state).toBe('proceed') + expect(settled?.settledAt).toBe(NOW) + + const replay = applyTransition(settled as PendingRequestRecord, 'proceed', NOW + 5_000) + expect(replay).toBe(settled) // same reference: no mutation, no re-settlement + }) + + it('rejects a DIFFERENT terminal transition once already terminal (replay cannot flip proceed to cancel)', () => { + const record = makeRecord({ state: 'awaiting_review' }) + const settled = applyTransition(record, 'proceed', NOW) as PendingRequestRecord + expect(applyTransition(settled, 'cancel', NOW + 1)).toBeNull() + }) + + it('merges a patch (e.g. binding the xdrDigest) on a successful transition', () => { + const record = makeRecord({ state: 'assessing' }) + const updated = applyTransition(record, 'awaiting_review', NOW, { xdrDigest: 'abc123' }) + expect(updated?.xdrDigest).toBe('abc123') + expect(updated?.state).toBe('awaiting_review') + }) +}) + +describe('isExpired / isTerminal / outcomeForState', () => { + it('a non-terminal record past its deadline is expired', () => { + const record = makeRecord({ state: 'awaiting_review', deadlineAt: NOW - 1 }) + expect(isExpired(record, NOW)).toBe(true) + }) + + it('a terminal record is never "expired" even past its deadline', () => { + const record = makeRecord({ state: 'proceed', deadlineAt: NOW - 1, settledAt: NOW - 1 }) + expect(isExpired(record, NOW)).toBe(false) + }) + + it('isTerminal is true only for proceed/cancel/expired/failed', () => { + expect(isTerminal('proceed')).toBe(true) + expect(isTerminal('cancel')).toBe(true) + expect(isTerminal('expired')).toBe(true) + expect(isTerminal('failed')).toBe(true) + expect(isTerminal('awaiting_review')).toBe(false) + expect(isTerminal('received')).toBe(false) + }) + + it('outcomeForState maps proceed to proceed and everything else to cancel', () => { + expect(outcomeForState('proceed')).toBe('proceed') + expect(outcomeForState('cancel')).toBe('cancel') + expect(outcomeForState('expired')).toBe('cancel') + expect(outcomeForState('failed')).toBe('cancel') + }) +}) + +describe('shouldPrune (tombstone TTL)', () => { + it('keeps a terminal record before the tombstone TTL elapses', () => { + const record = makeRecord({ state: 'cancel', settledAt: NOW }) + expect(shouldPrune(record, NOW + TOMBSTONE_TTL_MS - 1)).toBe(false) + }) + + it('prunes a terminal record once the tombstone TTL elapses', () => { + const record = makeRecord({ state: 'cancel', settledAt: NOW }) + expect(shouldPrune(record, NOW + TOMBSTONE_TTL_MS + 1)).toBe(true) + }) + + it('never prunes a non-terminal record regardless of age', () => { + const record = makeRecord({ state: 'awaiting_review', createdAt: NOW - 10_000_000 }) + expect(shouldPrune(record, NOW)).toBe(false) + }) +}) + +describe('admissionCheck', () => { + it('admits when under both bounds', () => { + expect(admissionCheck([], 1, 0)).toBe('ok') + }) + + it('rejects once a single frame reaches MAX_PENDING_PER_FRAME active requests', () => { + const active = Array.from({ length: MAX_PENDING_PER_FRAME }, (_, index) => + makeRecord({ requestId: `r${index}`, tabId: 1, frameId: 0, state: 'awaiting_review' }), + ) + expect(admissionCheck(active, 1, 0)).toBe('frame-limit') + }) + + it('a busy frame does not block a DIFFERENT frame in the same tab', () => { + const active = Array.from({ length: MAX_PENDING_PER_FRAME }, (_, index) => + makeRecord({ requestId: `r${index}`, tabId: 1, frameId: 0, state: 'awaiting_review' }), + ) + expect(admissionCheck(active, 1, 1)).toBe('ok') + }) + + it('rejects once the global bound is reached even across many distinct frames', () => { + const active = Array.from({ length: MAX_PENDING_GLOBAL }, (_, index) => + makeRecord({ requestId: `r${index}`, tabId: index, frameId: 0, state: 'awaiting_review' }), + ) + expect(admissionCheck(active, 999, 0)).toBe('global-limit') + }) + + it('admits right up to the boundary and rejects the very next request', () => { + const active = Array.from({ length: MAX_PENDING_PER_FRAME - 1 }, (_, index) => + makeRecord({ requestId: `r${index}`, tabId: 1, frameId: 0, state: 'awaiting_review' }), + ) + expect(admissionCheck(active, 1, 0)).toBe('ok') + active.push(makeRecord({ requestId: 'boundary', tabId: 1, frameId: 0, state: 'awaiting_review' })) + expect(admissionCheck(active, 1, 0)).toBe('frame-limit') + }) +}) + +describe('validatePendingRequestRecord (adversarial storage deserialization)', () => { + const valid = makeRecord() + + it('accepts a well-formed record', () => { + expect(validatePendingRequestRecord(valid)).toEqual(valid) + }) + + it.each([ + null, + undefined, + 'a-string', + 42, + [], + {}, + { ...valid, requestId: '' }, + { ...valid, requestId: 123 }, + { ...valid, requestId: 'x'.repeat(129) }, + { ...valid, adapter: 'metamask' }, + { ...valid, adapter: undefined }, + { ...valid, tabId: '1' }, + { ...valid, tabId: 1.5 }, + { ...valid, frameId: undefined }, + { ...valid, documentBound: 'true' }, + { ...valid, state: 'reviewing' }, + { ...valid, state: undefined }, + { ...valid, protocolVersion: '1' }, + { ...valid, protocolVersion: 1.5 }, + { ...valid, createdAt: 'now' }, + { ...valid, deadlineAt: NaN }, + { ...valid, documentId: 42 }, + { ...valid, networkPassphrase: 42 }, + { ...valid, xdrDigest: 42 }, + { ...valid, windowId: 1.5 }, + { ...valid, settledAt: 'now' }, + ])('rejects malformed input %#', (input) => { + expect(validatePendingRequestRecord(input)).toBeNull() + }) + + it('accepts optional fields when present and well-formed', () => { + const withOptionals = makeRecord({ + documentId: 'doc-1', + networkPassphrase: 'Test SDF Network ; September 2015', + xdrDigest: 'a'.repeat(64), + windowId: 7, + settledAt: NOW, + state: 'proceed', + }) + expect(validatePendingRequestRecord(withOptionals)).toEqual(withOptionals) + }) + + it('never copies an unrecognized field from untrusted storage input onto the returned record', () => { + const withExtra = { ...valid, injected: 'field' } + const result = validatePendingRequestRecord(withExtra) as unknown as Record + expect(result).not.toBeNull() + expect(result.injected).toBeUndefined() + expect(Object.keys(result)).not.toContain('injected') + }) +}) diff --git a/src/signing/pendingRequestState.ts b/src/signing/pendingRequestState.ts new file mode 100644 index 0000000..d41ce15 --- /dev/null +++ b/src/signing/pendingRequestState.ts @@ -0,0 +1,191 @@ +/** + * Pure state for the one-shot signing protocol. Mirrors the split used by + * src/protection/protectionState.ts: this module holds no chrome.* calls and + * no resolver/callback functions (those cannot survive MV3 worker + * suspension), only the serializable record shape, transition rules, and + * admission/pruning policy that src/background/background.ts wires up. + */ +export const SIGN_PROTOCOL_VERSION = 1 + +// The worker is the deadline's source of truth: it is checked eagerly on +// every touchpoint (AWAIT_OUTCOME, DECISION_MADE, restore-after-restart) +// rather than relied on solely via a fire-and-forget timer, so correctness +// does not depend on a setTimeout surviving a worker restart. +export const REVIEW_DEADLINE_MS = 90_000 + +// How long a terminal record is kept after settlement so a replayed +// DECISION_MADE or a retried AWAIT_OUTCOME resolves idempotently instead of +// hitting "unknown request" once storage/memory has pruned it. +export const TOMBSTONE_TTL_MS = 120_000 + +export const MAX_PENDING_PER_FRAME = 5 +export const MAX_PENDING_GLOBAL = 50 + +export type PendingState = + | 'received' + | 'validating' + | 'assessing' + | 'awaiting_review' + | 'proceed' + | 'cancel' + | 'expired' + | 'failed' + +export type SigningAdapter = 'freighter' | 'albedo-popup' + +export const TERMINAL_STATES: ReadonlySet = new Set([ + 'proceed', + 'cancel', + 'expired', + 'failed', +]) + +export function isTerminal(state: PendingState): boolean { + return TERMINAL_STATES.has(state) +} + +export interface PendingRequestRecord { + requestId: string + adapter: SigningAdapter + tabId: number + frameId: number + documentId?: string + documentBound: boolean + networkPassphrase?: string + xdrDigest?: string + windowId?: number + state: PendingState + protocolVersion: number + createdAt: number + deadlineAt: number + settledAt?: number +} + +const ALLOWED_TRANSITIONS: Record = { + received: ['validating', 'failed', 'expired'], + validating: ['assessing', 'failed', 'expired'], + assessing: ['awaiting_review', 'failed', 'expired'], + awaiting_review: ['proceed', 'cancel', 'expired', 'failed'], + proceed: [], + cancel: [], + expired: [], + failed: [], +} + +export function canTransition(from: PendingState, to: PendingState): boolean { + return ALLOWED_TRANSITIONS[from].includes(to) +} + +/** + * The first valid terminal transition wins; replay is a no-op. A repeat of + * the *same* terminal state the record already holds is treated as an + * idempotent replay (returns the unchanged record) rather than a rejected + * transition, so a retried AWAIT_OUTCOME or duplicate DECISION_MADE never + * needs special-casing by the caller. Any other transition attempted from a + * terminal state, or a transition not present in the table, is rejected + * (returns null) and must be treated as a no-op by the caller. + */ +export function applyTransition( + record: PendingRequestRecord, + to: PendingState, + now: number, + patch: Partial = {}, +): PendingRequestRecord | null { + if (record.state === to) return record + if (!canTransition(record.state, to)) return null + return { + ...record, + ...patch, + state: to, + settledAt: isTerminal(to) ? now : record.settledAt, + } +} + +export function isExpired(record: PendingRequestRecord, now: number): boolean { + return !isTerminal(record.state) && now > record.deadlineAt +} + +export function shouldPrune(record: PendingRequestRecord, now: number): boolean { + return isTerminal(record.state) && record.settledAt !== undefined + ? now - record.settledAt > TOMBSTONE_TTL_MS + : false +} + +export type AdmissionResult = 'ok' | 'frame-limit' | 'global-limit' + +/** Rejects immediately (no popup opened) once bounds are hit; active records only, terminal tombstones don't count against the cap. */ +export function admissionCheck( + activeRecords: readonly PendingRequestRecord[], + tabId: number, + frameId: number, +): AdmissionResult { + if (activeRecords.length >= MAX_PENDING_GLOBAL) return 'global-limit' + const frameActive = activeRecords.filter( + (record) => record.tabId === tabId && record.frameId === frameId, + ) + if (frameActive.length >= MAX_PENDING_PER_FRAME) return 'frame-limit' + return 'ok' +} + +export function outcomeForState(state: PendingState): 'proceed' | 'cancel' { + return state === 'proceed' ? 'proceed' : 'cancel' +} + +const SIGNING_ADAPTERS: readonly SigningAdapter[] = ['freighter', 'albedo-popup'] +const PENDING_STATES: readonly PendingState[] = [ + 'received', + 'validating', + 'assessing', + 'awaiting_review', + 'proceed', + 'cancel', + 'expired', + 'failed', +] + +/** Hand-rolled runtime guard for chrome.storage.session deserialization, mirroring validateProtectionRecord. */ +export function validatePendingRequestRecord(value: unknown): PendingRequestRecord | null { + if (!value || typeof value !== 'object') return null + const record = value as Partial + if ( + typeof record.requestId !== 'string' || + record.requestId.length === 0 || + record.requestId.length > 128 + ) { + return null + } + if (!SIGNING_ADAPTERS.includes(record.adapter as SigningAdapter)) return null + if (typeof record.tabId !== 'number' || !Number.isInteger(record.tabId)) return null + if (typeof record.frameId !== 'number' || !Number.isInteger(record.frameId)) return null + if (typeof record.documentBound !== 'boolean') return null + if (!PENDING_STATES.includes(record.state as PendingState)) return null + if (typeof record.protocolVersion !== 'number' || !Number.isInteger(record.protocolVersion)) { + return null + } + if (typeof record.createdAt !== 'number' || !Number.isFinite(record.createdAt)) return null + if (typeof record.deadlineAt !== 'number' || !Number.isFinite(record.deadlineAt)) return null + if (record.documentId !== undefined && typeof record.documentId !== 'string') return null + if (record.networkPassphrase !== undefined && typeof record.networkPassphrase !== 'string') { + return null + } + if (record.xdrDigest !== undefined && typeof record.xdrDigest !== 'string') return null + if (record.windowId !== undefined && !Number.isInteger(record.windowId)) return null + if (record.settledAt !== undefined && typeof record.settledAt !== 'number') return null + + return { + requestId: record.requestId, + adapter: record.adapter as SigningAdapter, + tabId: record.tabId, + frameId: record.frameId, + documentId: record.documentId, + documentBound: record.documentBound, + networkPassphrase: record.networkPassphrase, + xdrDigest: record.xdrDigest, + windowId: record.windowId, + state: record.state as PendingState, + protocolVersion: record.protocolVersion, + createdAt: record.createdAt, + deadlineAt: record.deadlineAt, + settledAt: record.settledAt, + } +}