diff --git a/.changeset/idempotent-write-retries.md b/.changeset/idempotent-write-retries.md new file mode 100644 index 0000000..d956572 --- /dev/null +++ b/.changeset/idempotent-write-retries.md @@ -0,0 +1,21 @@ +--- +"@understudy/protocol": minor +"@understudy/connector": minor +--- + +Idempotent write retries and a single write-classification source of truth. + +- `@understudy/protocol` now exports `WRITE_COMMAND_TYPES` (and its + `WriteCommandType` union) as the one classification downstream layers derive + from, and reclassifies `scroll` / `switch_tab` as writes — so + `isWriteCommand` returns `true` for them. They are user-visible side effects: + a `dryRun` must simulate (not perform) them and an idempotent retry must + replay (not repeat) them, so a relative-`dy` `scroll` never double-scrolls. + No schema change. +- `@understudy/connector`'s `act` / `fillCredential` derive the wire + `commandId` from the breakwater idempotency key (`ik_`) instead of a + random UUID, so a retry after a lost or unparseable response replays the + service's recorded write Event instead of executing the write twice. + Dry-runs keep random ids. The `act` union is now pinned at compile time to + the protocol's write class minus `fill_secret` (no divergence to reconcile, + now that `scroll`/`switch_tab` are protocol writes). diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9ecb432..fdc3660 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,12 +17,14 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 15 steps: - - uses: actions/checkout@v4 + # v5/v6 majors run on the node24 action runtime; the v4 majors ran on + # node20, which GitHub now flags with a deprecation annotation. + - uses: actions/checkout@v5 # Reads the pnpm version from package.json "packageManager". - - uses: pnpm/action-setup@v4 + - uses: pnpm/action-setup@v6 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@v5 with: node-version: 22 cache: pnpm diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 22b4bca..2f04dcf 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -32,15 +32,18 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 steps: - - uses: actions/checkout@v4 + # v5/v6 majors run on the node24 action runtime (changesets/action@v1 + # already does); the v4 majors ran on node20, which GitHub now flags + # with a deprecation annotation. + - uses: actions/checkout@v5 with: # Changesets reads history to decide what changed since the last tag. fetch-depth: 0 # Reads the pnpm version from package.json "packageManager". - - uses: pnpm/action-setup@v4 + - uses: pnpm/action-setup@v6 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@v5 with: node-version: 22 cache: pnpm diff --git a/.gitignore b/.gitignore index 67bf4c2..5325746 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,10 @@ dist/ # Cloudflare / Wrangler (added at M3) .wrangler/ .dev.vars +# Operator-local secret backups (the deployed worker is canonical; these are +# recoverable copies of values Cloudflare never shows again). NEVER commit. +# Specific prefix so the tracked .dev.vars.example template is unaffected. +.secrets* # WXT extension build (added at M2) .wxt/ diff --git a/apps/backend/.dev.vars.example b/apps/backend/.dev.vars.example index 6706623..61649cc 100644 --- a/apps/backend/.dev.vars.example +++ b/apps/backend/.dev.vars.example @@ -17,3 +17,10 @@ CALLER_TOKENS={"dev-caller-token":{"actor":"stub-consumer","tenantId":"dev-tenan # prints (which embeds this token) into the M2 extension's connection # settings. EXTENSION_TOKENS={"dev-ext-token":"dev-tenant"} + +# base64url-encoded 32-byte AES-256-GCM key that envelope-encrypts every +# vault value (src/vault.ts). This dev value decodes to the literal +# "dev-vault-master-key-0123456789!". Generate a real one with: +# node -e "console.log(require('node:crypto').randomBytes(32).toString('base64url'))" +# Seed dev secrets with: node scripts/vault-put.mjs 'vault://ref' --local +VAULT_MASTER_KEY=ZGV2LXZhdWx0LW1hc3Rlci1rZXktMDEyMzQ1Njc4OSE diff --git a/apps/backend/CLAUDE.md b/apps/backend/CLAUDE.md index 3c59207..ef70f21 100644 --- a/apps/backend/CLAUDE.md +++ b/apps/backend/CLAUDE.md @@ -14,19 +14,24 @@ Cloudflare Worker (Hono) + one Agents-SDK Durable Object per session (`SessionAg | `tsconfig.json` | TS config (workers-types, bundler resolution, strict) | Adjusting compiler options | | `vitest.config.ts` | Workers-pool test config (`@cloudflare/vitest-pool-workers`); imports `./test/tokens` | Adjusting test runner/pool config | | `.dev.vars.example` | Template for local `wrangler dev` secrets; matches `stub-consumer.mjs` defaults | Setting up local dev | -| `src/index.ts` | Worker entry: Hono app (`/v1/sessions*`, `/health`) + `routeAgentRequest` for the session WS; re-exports `SessionAgent` | Adding/changing a route, changing auth order | -| `src/session.ts` | `SessionAgent` — the per-session Durable Object: WS auth, event routing, `dispatch`/`fillSecret` RPCs | Changing session lifecycle, dryRun behavior, fill_secret dispatch | -| `src/coordinator.ts` | `SessionCoordinator` — portable command↔event correlation interface, no Cloudflare imports | Understanding the portable seam, swapping the CF impl | -| `src/coordinator-cf.ts` | `CfSessionCoordinator` — CF impl: pending map + persisted awaiting-marker + hibernation reconciliation | Debugging a stuck/timed-out command, hibernation edge cases | +| `.secrets.production.env` | Operator-local backup of the DEPLOYED worker's secrets (gitignored via `.secrets*`; absent in a fresh clone; the worker is canonical) | Recovering/rotating prod secrets — see README "Secrets" | +| `src/index.ts` | Worker entry: Hono app (`/v1/sessions*`, `/health`) + `routeAgentRequest` with pre-accept WS/HTTP auth hooks (`gateAgentRequest`) + `DispatchOutcome`→status mapping; re-exports `SessionAgent` | Adding/changing a route, changing auth order, changing failure statuses | +| `src/session.ts` | `SessionAgent` — the per-session Durable Object: WS auth, event routing, `dispatch`/`fillSecret` RPCs (typed `DispatchOutcome`, write-replay cache) | Changing session lifecycle, dryRun behavior, fill_secret dispatch, idempotent replay | +| `src/coordinator.ts` | `SessionCoordinator` — portable command↔event correlation interface + failure-prefix constants, no Cloudflare imports | Understanding the portable seam, swapping the CF impl | +| `src/coordinator-cf.ts` | `CfSessionCoordinator` — CF impl: pending map (+ duplicate-in-flight guard) + persisted awaiting-marker + hibernation reconciliation | Debugging a stuck/timed-out command, hibernation edge cases | | `src/auth.ts` | Caller bearer-token auth, sessionId mint/scope (HMAC), extension token verify | Changing auth, adding a token type, debugging 401/404 | | `src/secrets.ts` | `resolveSecret` — vault lookup only, no dispatch | Changing the vault backend, debugging secret resolution failures | -| `src/types.ts` | Shared `Env`, `SessionState`, `SessionStatus`, `VaultBinding` | Adding a binding, changing DO state shape | +| `src/vault.ts` | AES-256-GCM envelope codec + `EncryptedKvVault`/`createVault` — KV holds ciphertext only | Changing the envelope format/key handling (mirror `scripts/vault-put.mjs`) | +| `src/base64url.ts` | base64url codec shared by auth.ts and vault.ts | Rarely — codec changes | +| `src/types.ts` | Shared `Env`, `SessionState` (incl. `completedWrites`), `SessionStatus`, `VaultBinding`, `DispatchOutcome` | Adding a binding, changing DO state shape, changing the RPC outcome union | | `scripts/stub-consumer.mjs` | Throwaway Node runbook harness (not a workspace member) driving the API against a real extension | Running the attended M3 end-to-end verification | -| `test/service.test.ts` | Hono route tests: auth, tenant scoping, dryRun, fill_secret routing | Verifying/extending the command API | -| `test/session.test.ts` | `SessionAgent`/coordinator tests: WS auth, resolve correlation, timeout, hibernation-resume | Verifying/extending DO behavior | +| `scripts/vault-put.mjs` | Seeds one vault secret as an envelope via `wrangler kv key put` (plaintext from stdin; `--local` for dev) | Seeding/rotating vault values (never raw `kv key put`) | +| `test/service.test.ts` | Hono route tests: auth, tenant scoping, dryRun, fill_secret routing, pre-accept WS gate, idempotent write replay | Verifying/extending the command API | +| `test/session.test.ts` | `SessionAgent`/coordinator tests: in-DO WS auth (defense in depth), onClose stamping, resolve correlation, hibernation-resume | Verifying/extending DO behavior | | `test/auth.test.ts` | Auth module unit tests | Verifying/extending auth.ts | -| `test/coordinator.test.ts` | Coordinator unit tests | Verifying/extending coordinator-cf.ts | +| `test/coordinator.test.ts` | Coordinator unit tests (timeout, duplicate guard, abandon, no-leak logging) | Verifying/extending coordinator-cf.ts | | `test/secrets.test.ts` | Vault resolution unit tests | Verifying/extending secrets.ts | +| `test/vault.test.ts` | Envelope round-trip/tamper/wrong-key + `EncryptedKvVault` fail-closed tests | Verifying/extending vault.ts | | `test/helpers.ts` | Workers-runtime test helpers (session stub, WS extraction) | Writing a new Workers-pool test | | `test/tokens.ts` | Shared test-only token constants (used by vitest.config.ts and suites) | Adding a test caller/extension identity | | `test/tsconfig.json` | Test typecheck project (extends root config, includes `test/**`) | Adjusting test typecheck scope | diff --git a/apps/backend/README.md b/apps/backend/README.md index 58a4148..d68fb19 100644 --- a/apps/backend/README.md +++ b/apps/backend/README.md @@ -27,20 +27,35 @@ sessionId's embedded tenant matches the caller's, 404 on mismatch) → `safePars → the correlated Event is returned as JSON. This order is load-bearing: an unauthenticated or cross-tenant request never reaches parsing or dispatch. -Dispatch failures map to status codes by error-message prefix (the only signal -that survives the DO RPC boundary; the constants live in `src/coordinator.ts`): +Expected dispatch failures cross the DO RPC boundary as a typed +`DispatchOutcome` (`src/types.ts`), never as a rejected RPC promise — workerd +logs every server-side RPC rejection as an uncaught exception even when the +caller handles it, and a typed reason beats message-prefix parsing at the +route (internally the coordinator still rejects with the `src/coordinator.ts` +prefix constants; `SessionAgent.dispatchFailure` maps them in-isolate). The +route maps reasons to statuses: **503** `{error: "extension not connected"}` when the session has no live, onConnect-authorized extension socket — the gate consults that delivery -predicate directly (not the persisted `status` scalar, which a late close from -a replaced socket can leave stale), fails fast instead of burning the 30s -timeout, and answers non-2xx deliberately: a 200 `ok:false` Event would be -cached by a consumer's idempotency store and replayed after reconnect; -**504** `{error: "command timed out"}` when a connected extension never -answered; anything else is a uniform JSON **500** via `app.onError`. A real -`fill_secret` checks the same predicate *before* touching the vault, so no -plaintext is ever resolved for a command that cannot dispatch. Exception: -a ref-less dryRun write (e.g. navigate) still short-circuits to simulated -`ok:true` without touching the wire — it was never a liveness signal. +predicate directly (not the persisted `status` scalar), fails fast instead of +burning the 30s timeout, and answers non-2xx deliberately: a 200 `ok:false` +Event would be cached by a consumer's idempotency store and replayed after +reconnect; **503** `{error: "session resynced mid-command"}` when a fresh +`hello` abandoned the in-flight command (the extension reconnected — same +retryable family, its own honest reason); **504** `{error: "command timed +out"}` when a connected extension never answered; **409** `{error: "command +already in flight"}` for a concurrent duplicate of a still-pending write +commandId; anything else is a genuine bug and remains a uniform JSON **500** +via `app.onError`. A real `fill_secret` checks the liveness predicate +*before* touching the vault, so no plaintext is ever resolved for a command +that cannot dispatch. Exception: a ref-less dryRun write (e.g. navigate) +still short-circuits to simulated `ok:true` without touching the wire — it +was never a liveness signal. + +Completed **writes** are additionally recorded per commandId +(`SessionState.completedWrites`, capped at 100): a retry under the same +commandId — the connector derives it from the breakwater idempotency key — +replays the recorded Event instead of executing twice, closing the +write-performed-but-response-lost retry gap. Reads never replay. Inside the DO, `SessionAgent` holds a `CfSessionCoordinator` (the Cloudflare implementation of the portable `SessionCoordinator` interface). `send(cmd)` writes @@ -60,11 +75,22 @@ command API (`index.ts`, `session.ts`) is unaffected. ## WebSocket security model -`onConnect` runs an async token check (`verifyExtensionToken`) before marking a -connection `connection.setState({ authorized: true })`. The Agents SDK accepts a -socket — and admits it to the connection set `getConnections()` returns — before -that async check resolves, so an unauthenticated or wrong-tenant socket can sit in -the connection set during that window. Four things close this gap: +The first gate is at the **Worker edge**: `routeAgentRequest`'s +`onBeforeConnect`/`onBeforeRequest` hooks (`index.ts::gateAgentRequest`) +verify the extension token and tenant scope before the Durable Object ever +accepts the socket (or serves the SDK's HTTP surface). A bad token is a +plain 401 and a cross-tenant sessionId a 404 (no existence oracle, matching +the /v1 discipline) — the socket never enters the DO's connection set at +all. + +The in-DO gate remains as defense in depth for any path that reaches the DO +without that router: `onConnect` runs the same async checks +(`verifyExtensionToken` + `scopeSession`) before marking a connection +`connection.setState({ authorized: true })`, closing with 1008 otherwise. +The Agents SDK accepts a socket — and admits it to the connection set +`getConnections()` returns — before that async check resolves, so an +unauthenticated or wrong-tenant socket could sit in the connection set +during that window. Four things close this gap: - `sendToExtension` (the coordinator's outbound path) iterates `getConnections()` but filters to `isAuthorizedConnection`, so a command is @@ -122,15 +148,19 @@ the connection set during that window. Four things close this gap: secret. A dry-run `ok` guarantees *resolvability* (the ref maps to a live node in the current generation) — not *executability* of the eventual dispatch (e.g. box-model availability), which only the real command proves. -- **The vault binding (`Env.VAULT`) is a KV namespace, not CF Secrets/Secrets - Store**: `fill_secret`'s `secretRef` is chosen per-call at runtime, and CF's - Secrets/Secrets Store bindings are static — one binding per fixed secret name — - which cannot address an arbitrary runtime-chosen key. KV's `get(key)` can. The - `wrangler.jsonc` `VAULT` KV namespace id is a dev-only placeholder - (`REPLACE_WITH_VAULT_KV_NAMESPACE_ID`); because KV values are readable back at - rest, a stronger backend (per-tenant KMS, or a Secrets-Store-via-API binding) - must replace it behind the same `VaultBinding.get` seam (`types.ts`) before any - real credential is stored — this is a pre-production gate, not yet done. +- **The vault binding (`Env.VAULT`) is a KV namespace holding only AES-256-GCM + envelopes, not CF Secrets/Secrets Store**: `fill_secret`'s `secretRef` is + chosen per-call at runtime, and CF's Secrets/Secrets Store bindings are + static — one binding per fixed secret name — which cannot address an + arbitrary runtime-chosen key. KV's `get(key)` can. Because raw KV values are + readable back at rest, every value is envelope-encrypted (`src/vault.ts`, + format `v1..`, fresh IV per value) under `VAULT_MASTER_KEY` — a + Worker secret that never touches KV or wrangler.jsonc — and decrypted only + inside the DO via `createVault(env)`. Seed values with + `scripts/vault-put.mjs` (same envelope, plain Node), never a raw + `wrangler kv key put`; a legacy plaintext value fails closed at read time + ("not a recognized envelope"). A per-tenant external KMS remains a possible + future swap behind the same `VaultBinding.get` seam (`types.ts`). - **Hibernation cannot lose an in-flight command; only shutdown/restart can**: verified against the Cloudflare Durable Objects docs (2026-07-14) — hibernation requires no pending timer, no in-progress awaited fetch, no active WS use, and no @@ -161,11 +191,22 @@ the connection set during that window. Four things close this gap: - Every *successfully dispatched* `Command` produces exactly one `Event` bearing its `commandId`, and the command route returns exactly that Event; a command that cannot dispatch or never resolves maps to a non-2xx JSON - error instead (503/504/500 — see the failure mapping above). + error instead (503/504/409/500 — see the failure mapping above). +- A **write** commandId executes at most once within a session's last 100 + writes: a repeat of a completed write replays its recorded Event + (`completedWrites`, cap 100), a repeat of a still-pending write is refused + 409, and the extension keeps a matching 100-entry replay + in-flight record + for the case where the service times out while the extension is still + executing (a duplicate is dropped, not re-run). A retry delayed beyond 100 + intervening writes degrades to re-execution. Reads never replay. - `fill_secret` plaintext never enters `setState`, logs, the Event response, or an - error string; the coordinator logs only `{commandId, type}`. + error string; the coordinator logs only `{commandId, type}`. A replayed + `fill_secret` touches neither the vault nor the wire. +- The vault at rest holds only `v1..` AES-256-GCM envelopes; a value + that does not decrypt under `VAULT_MASTER_KEY` is refused, never served. - One Durable Object per `sessionId`; a sessionId whose embedded tenant disagrees - with the authenticated caller is refused with 404, never 403. + with the authenticated caller is refused with 404, never 403 — on the /v1 + API and on the agent WS/HTTP path alike. - A mid-command DO hibernation cannot happen (see above); an interrupting shutdown/restart is bounded by the per-command timeout, and the persisted awaiting-marker reconciles any orphaned late result rather than mis-resolving it. @@ -174,5 +215,68 @@ the connection set during that window. Four things close this gap: itself imports nothing Cloudflare-specific. - `dryRun` never dispatches a mutating command and never resolves a secret; it always returns a simulated `action_result` from a read-only ref check. -- An accepted-but-not-yet-authorized WebSocket connection can neither receive a - command nor have its inbound messages processed (see WebSocket security model). +- An unauthorized WS upgrade is refused at the Worker edge (401/404) before + the DO accepts it; any socket that still reaches the DO unauthorized can + neither receive a command, have its inbound messages processed, nor change + the session's status by closing (see WebSocket security model). +- Expected delivery failures never cross the DO RPC boundary as rejections + (no workerd "Uncaught (in promise)" noise); only genuine bugs throw. + +## Deploy + +First deployed 2026-07-17 to `https://understudy-backend.gcharang.workers.dev` +(account `056cbaa6f5c3d8ff5584f1aa84bbe050`). The account id is deliberately +NOT pinned in `wrangler.jsonc` (public repo, two local accounts): pass it per +command. Runbook, from `apps/backend`: + +```sh +export CLOUDFLARE_ACCOUNT_ID=056cbaa6f5c3d8ff5584f1aa84bbe050 + +# One-time: the ciphertext store (id goes into wrangler.jsonc kv_namespaces) +pnpm exec wrangler kv namespace create VAULT + +# Secrets - all four are required; `wrangler deploy` refuses to ship without +# them (wrangler.jsonc `secrets.required`). Mint strong values: +openssl rand -hex 32 | pnpm exec wrangler secret put AUTH_HMAC_SECRET +printf '%s' '{"":{"actor":"","tenantId":""}}' | pnpm exec wrangler secret put CALLER_TOKENS +printf '%s' '{"":""}' | pnpm exec wrangler secret put EXTENSION_TOKENS +openssl rand 32 | basenc --base64url | tr -d '=' | pnpm exec wrangler secret put VAULT_MASTER_KEY + +pnpm exec wrangler deploy +curl -s https://understudy-backend.gcharang.workers.dev/health # {"ok":true} + +# Seed a vault secret (encrypts locally; KV never sees plaintext): +printf '%s' 'the-secret' | VAULT_MASTER_KEY= node scripts/vault-put.mjs 'vault://tenant/ref' +``` + +The extension connects to +`wss://understudy-backend.gcharang.workers.dev/agents/session/?token=`. + +### Secrets + +All four are **required** — `wrangler deploy` refuses to ship without them +(`wrangler.jsonc` `secrets.required`, which is also the `.dev.vars` allowlist +for local dev). Cloudflare stores them encrypted and **never shows a value +again** after `wrangler secret put`, so the deployed worker is the canonical +copy and the only readable backup is the operator-local, gitignored +`apps/backend/.secrets.production.env` (created out of band, never committed — +`.secrets*` in the root `.gitignore`). Lose that file and a secret can only be +*rotated*, not recovered. + +| secret | what it is | regenerate | rotation impact | +|---|---|---|---| +| `AUTH_HMAC_SECRET` | HMAC-SHA256 key signing minted sessionIds (stateless tenant scoping) | `openssl rand -hex 32` | invalidates every outstanding sessionId — consumers must re-mint | +| `CALLER_TOKENS` | JSON map `bearer token → {actor, tenantId}`; a consumer sends the raw token as `Authorization: Bearer …` | token: `printf 'uk_caller_%s\n' "$(openssl rand -hex 24)"` | the affected consumer swaps its `UNDERSTUDY_TOKEN` | +| `EXTENSION_TOKENS` | JSON map `WS token → tenantId`; the extension sends the raw token as `?token=…` | token: `printf 'uk_ext_%s\n' "$(openssl rand -hex 24)"` | that user pastes the new WS URL into the extension panel | +| `VAULT_MASTER_KEY` | base64url 32-byte AES-256-GCM key envelope-encrypting every vault value | `openssl rand 32 \| basenc --base64url \| tr -d '='` | **every stored vault value must be re-sealed** (`vault-put.mjs`) — old envelopes become undecryptable | + +**When to rotate:** on suspected exposure of that specific secret, when +offboarding a tenant/user (edit the relevant JSON map and re-put), or on a +periodic schedule for the two keys. `CALLER_TOKENS`/`EXTENSION_TOKENS` are +add/remove-an-entry edits — rotating one caller/extension does not disturb the +others. + +**Re-push after editing the backup file** (from `apps/backend`) — one at a +time with `wrangler secret put `, or all four via the bulk endpoint (the +`.secrets.production.env` header carries a ready-made env→JSON one-liner that +pipes into `wrangler secret bulk`). diff --git a/apps/backend/scripts/vault-put.mjs b/apps/backend/scripts/vault-put.mjs new file mode 100644 index 0000000..3dde346 --- /dev/null +++ b/apps/backend/scripts/vault-put.mjs @@ -0,0 +1,87 @@ +#!/usr/bin/env node +/** + * Seed one vault secret as an AES-256-GCM envelope (never plaintext): + * + * VAULT_MASTER_KEY= node scripts/vault-put.mjs [--local] + * + * Reads the plaintext from stdin (so it never lands in shell history or + * `ps`), encrypts it with the same v1.. envelope format as + * src/vault.ts (the two must change together; vault.test.ts pins the + * format), and writes it via `wrangler kv key put --binding VAULT`. + * `--local` targets the miniflare dev KV that `wrangler dev` reads; + * otherwise the write goes to the real remote namespace in wrangler.jsonc. + * Falls back to VAULT_MASTER_KEY from .dev.vars when --local and the env + * var is unset. + */ +import { spawnSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { webcrypto } from "node:crypto"; + +const ENVELOPE_VERSION = "v1"; +const IV_BYTES = 12; +const MASTER_KEY_BYTES = 32; + +function fail(message) { + console.error(`vault-put: ${message}`); + process.exit(1); +} + +const args = process.argv.slice(2); +const local = args.includes("--local"); +const secretRef = args.find((a) => !a.startsWith("--")); +if (!secretRef) fail("usage: node scripts/vault-put.mjs [--local]"); + +const backendDir = join(dirname(fileURLToPath(import.meta.url)), ".."); + +function masterKeyFromDevVars() { + try { + const devVars = readFileSync(join(backendDir, ".dev.vars"), "utf8"); + const line = devVars.split("\n").find((l) => l.startsWith("VAULT_MASTER_KEY=")); + return line?.slice("VAULT_MASTER_KEY=".length).trim(); + } catch { + return undefined; + } +} + +const masterKeyB64 = process.env.VAULT_MASTER_KEY ?? (local ? masterKeyFromDevVars() : undefined); +if (!masterKeyB64) { + fail( + local + ? "set VAULT_MASTER_KEY (or put it in .dev.vars)" + : "set VAULT_MASTER_KEY to the deployed worker's key", + ); +} +const rawKey = Buffer.from(masterKeyB64, "base64url"); +if (rawKey.length !== MASTER_KEY_BYTES) fail(`VAULT_MASTER_KEY must decode to ${MASTER_KEY_BYTES} bytes`); + +const plaintext = readFileSync(0, "utf8").replace(/\n$/, ""); +if (!plaintext) fail("no plaintext on stdin (pipe or type the secret, then EOF)"); + +const key = await webcrypto.subtle.importKey("raw", rawKey, { name: "AES-GCM" }, false, ["encrypt"]); +const iv = webcrypto.getRandomValues(new Uint8Array(IV_BYTES)); +const ciphertext = await webcrypto.subtle.encrypt( + { name: "AES-GCM", iv }, + key, + new TextEncoder().encode(plaintext), +); +const envelope = `${ENVELOPE_VERSION}.${Buffer.from(iv).toString("base64url")}.${Buffer.from( + new Uint8Array(ciphertext), +).toString("base64url")}`; + +const wranglerArgs = [ + "exec", + "wrangler", + "kv", + "key", + "put", + secretRef, + envelope, + "--binding", + "VAULT", + local ? "--local" : "--remote", +]; +const result = spawnSync("pnpm", wranglerArgs, { cwd: backendDir, stdio: "inherit" }); +if (result.status !== 0) fail(`wrangler kv key put exited ${result.status ?? "on a signal"}`); +console.log(`vault-put: sealed ${secretRef} (${local ? "local" : "remote"})`); diff --git a/apps/backend/src/auth.ts b/apps/backend/src/auth.ts index 7e3c08b..fbbf1be 100644 --- a/apps/backend/src/auth.ts +++ b/apps/backend/src/auth.ts @@ -10,6 +10,7 @@ * connection independently of caller auth, so no caller credential is ever * sent to (or trusted from) the browser extension. */ +import { base64urlDecode, base64urlEncode } from "./base64url"; import type { Env } from "./types"; export interface Actor { @@ -133,21 +134,6 @@ async function importHmacKey(secret: string): Promise { ); } -function base64urlEncode(bytes: Uint8Array): string { - let binary = ""; - for (const byte of bytes) binary += String.fromCharCode(byte); - return btoa(binary).replace(/\+/g, "-").replace(/\//g, "_").replace(/=+$/, ""); -} - -function base64urlDecode(value: string): Uint8Array { - const base64 = value.replace(/-/g, "+").replace(/_/g, "/"); - const padded = base64 + "=".repeat((4 - (base64.length % 4)) % 4); - const binary = atob(padded); - const bytes = new Uint8Array(binary.length); - for (let i = 0; i < binary.length; i++) bytes[i] = binary.charCodeAt(i); - return bytes; -} - function toHex(bytes: Uint8Array): string { return Array.from(bytes, (byte) => byte.toString(16).padStart(2, "0")).join(""); } diff --git a/apps/backend/src/base64url.ts b/apps/backend/src/base64url.ts new file mode 100644 index 0000000..396593a --- /dev/null +++ b/apps/backend/src/base64url.ts @@ -0,0 +1,20 @@ +/** + * base64url codec shared by sessionId minting (auth.ts) and the vault + * envelope format (vault.ts). Workers have btoa/atob but no Buffer, hence + * the manual binary-string bridging. + */ + +export function base64urlEncode(bytes: Uint8Array): string { + let binary = ""; + for (const byte of bytes) binary += String.fromCharCode(byte); + return btoa(binary).replace(/\+/g, "-").replace(/\//g, "_").replace(/=+$/, ""); +} + +export function base64urlDecode(value: string): Uint8Array { + const base64 = value.replace(/-/g, "+").replace(/_/g, "/"); + const padded = base64 + "=".repeat((4 - (base64.length % 4)) % 4); + const binary = atob(padded); + const bytes = new Uint8Array(binary.length); + for (let i = 0; i < binary.length; i++) bytes[i] = binary.charCodeAt(i); + return bytes; +} diff --git a/apps/backend/src/coordinator-cf.ts b/apps/backend/src/coordinator-cf.ts index e20cbda..e22e5cc 100644 --- a/apps/backend/src/coordinator-cf.ts +++ b/apps/backend/src/coordinator-cf.ts @@ -9,7 +9,7 @@ */ import type { Command, Event } from "@understudy/protocol"; -import { COMMAND_TIMED_OUT, SESSION_NOT_CONNECTED } from "./coordinator"; +import { COMMAND_TIMED_OUT, DUPLICATE_COMMAND, SESSION_NOT_CONNECTED } from "./coordinator"; import type { PendingCommand, PendingMap, SessionCoordinator } from "./coordinator"; import type { SessionStatus } from "./types"; @@ -27,8 +27,9 @@ export interface CoordinatorHost { * The delivery predicate: does a live, onConnect-authorized extension * socket exist right now? This is the exact precondition sendToExtension * relies on - NOT the persisted SessionState.status scalar, which is a - * lossy last-writer-wins echo of it (a late onClose from a replaced - * socket can stamp "detached" over a healthy reconnected session). + * last-writer-wins echo maintained by lifecycle hooks (onClose guards the + * known stamping races, but the scalar is still eventually-consistent + * bookkeeping, not the delivery truth). */ hasAuthorizedConnection(): boolean; /** Reads the persisted awaiting-commandId marker (SessionState.awaitingCommandIds). */ @@ -84,6 +85,15 @@ export class CfSessionCoordinator implements SessionCoordinator { new Error(`${SESSION_NOT_CONNECTED}: no authorized extension connection`), ); } + // Stable, consumer-derived commandIds (the idempotent-retry contract) + // make a concurrent duplicate possible; letting it in would overwrite + // the first send's pending-map entry and strand its resolver until the + // orphaned timer fires. Refuse the second send instead. + if (this.pending.has(cmd.commandId)) { + return Promise.reject( + new Error(`${DUPLICATE_COMMAND}: ${cmd.commandId} is already awaiting its event`), + ); + } return new Promise((resolve, reject) => { const timer = setTimeout(() => { reject( diff --git a/apps/backend/src/coordinator.ts b/apps/backend/src/coordinator.ts index f0ec1d0..b561c93 100644 --- a/apps/backend/src/coordinator.ts +++ b/apps/backend/src/coordinator.ts @@ -13,14 +13,19 @@ import type { Command, Event } from "@understudy/protocol"; import type { SessionStatus } from "./types"; /** - * send()'s delivery-failure vocabulary. A message prefix is the only failure - * signal that survives an RPC boundary (custom Error subclasses do not), so - * every throw site and every host mapping failures onto its transport (the - * Cloudflare Worker maps these to 503/504 in index.ts) shares these - * constants instead of free-typing the strings. + * send()'s delivery-failure vocabulary. These prefixes never cross the RPC + * boundary as rejections: SessionAgent.dispatch/fillSecret catch coordinator + * rejections IN-ISOLATE and map each prefix to a typed DispatchOutcome + * (types.ts), which the Worker maps to 503/504/409 (index.ts). Keeping the + * rejection inside the Durable Object matters twice over - a structured + * outcome instead of message-prefix parsing at the route, and no + * "Uncaught (in promise)" instrumentation noise from workerd, which reports + * every RPC promise that rejects server-side even when the caller handles it. */ export const SESSION_NOT_CONNECTED = "session not connected"; export const COMMAND_TIMED_OUT = "command timed out"; +export const SESSION_RESYNCED = "session resynced"; +export const DUPLICATE_COMMAND = "duplicate command in flight"; /** One outstanding `send(cmd)` call, awaiting its correlated Event. */ export interface PendingCommand { diff --git a/apps/backend/src/index.ts b/apps/backend/src/index.ts index 4ce6454..87f4be3 100644 --- a/apps/backend/src/index.ts +++ b/apps/backend/src/index.ts @@ -13,9 +13,8 @@ import { Hono } from "hono"; import { getAgentByName, routeAgentRequest } from "agents"; import { safeParseCommand } from "@understudy/protocol"; -import { authenticate, mintSessionId, scopeSession } from "./auth"; -import { COMMAND_TIMED_OUT, SESSION_NOT_CONNECTED } from "./coordinator"; -import type { Env } from "./types"; +import { authenticate, mintSessionId, scopeSession, verifyExtensionToken } from "./auth"; +import type { DispatchOutcome, Env } from "./types"; import type { SessionAgent } from "./session"; export { SessionAgent } from "./session"; @@ -78,29 +77,45 @@ app.post("/v1/sessions/:sessionId/commands", async (c) => { // carrying a secret through this route (DL-004). const dryRun = body?.dryRun === true; const stub = await getSessionStub(c.env, sessionId); - try { - const event = - parsed.data.type === "fill_secret" - ? await stub.fillSecret(parsed.data, dryRun) - : await stub.dispatch(parsed.data, dryRun); - return c.json(event); - } catch (err) { - // The DO's throw crosses the RPC boundary as a plain Error; its message - // prefix is the contract (coordinator.ts). "Not connected" is 503 - a - // retryable infrastructure state - deliberately NOT a 200 ok:false - // Event, which a consumer's idempotency store would cache and replay - // even after the extension reconnects. - const message = err instanceof Error ? err.message : String(err); - if (message.startsWith(SESSION_NOT_CONNECTED)) { + const outcome: DispatchOutcome = + parsed.data.type === "fill_secret" + ? await stub.fillSecret(parsed.data, dryRun) + : await stub.dispatch(parsed.data, dryRun); + if (outcome.ok) return c.json(outcome.event); + + // Expected delivery failures arrive as typed outcomes, never as RPC + // rejections (types.ts::DispatchOutcome). All of these are deliberately + // non-2xx rather than a 200 ok:false Event, which a consumer's + // idempotency store would cache and replay even after the extension + // reconnects. Only a genuine bug still throws (-> the uniform 500). + // + // The honest reason is logged for observability; by DL-004 construction it + // carries only {commandId, type}-level detail, never a command payload. + console.warn("command dispatch failed", outcome.message); + switch (outcome.reason) { + case "not_connected": + // Retryable infrastructure state: no live, authorized extension socket. return c.json({ error: "extension not connected" }, 503); - } - if (message.startsWith(COMMAND_TIMED_OUT)) { + case "timed_out": return c.json({ error: "command timed out" }, 504); - } - throw err; + case "resynced": + // The extension reconnected mid-command, abandoning whatever it had in + // flight. Retryable, like not_connected - the session itself is healthy. + return c.json({ error: "session resynced mid-command" }, 503); + case "duplicate_in_flight": + return c.json({ error: "command already in flight" }, 409); + default: + // A new DispatchOutcome.reason without a route mapping fails the build + // here (assertNever) instead of silently returning undefined. + return assertNever(outcome.reason); } }); +// Compile-time exhaustiveness backstop for the DispatchOutcome.reason switch. +function assertNever(value: never): never { + throw new Error(`unhandled dispatch outcome reason: ${String(value)}`); +} + // Anything a route throws (or rethrows above) becomes a uniform JSON 500 // instead of workerd's opaque non-JSON error page. Command payloads never // enter error messages by construction (DL-004), so logging the message @@ -110,9 +125,36 @@ app.onError((err, c) => { return c.json({ error: "internal error" }, 500); }); +/** + * Worker-level gate on every request routeAgentRequest matches, BEFORE the + * Durable Object ever accepts the socket (or serves an SDK HTTP surface). + * Defense-in-depth with SessionAgent.onConnect: the in-DO gate stays (it + * covers any path that reaches the DO without this router), but an + * unauthorized upgrade is now refused at the edge instead of being + * accepted-but-inert. Failure statuses mirror the /v1 discipline: bad token + * 401; a sessionId whose tenant disagrees with the token collapses to 404, + * never 403 (DL-008: no existence oracle). `lobby.name` is the raw + * `:sessionId` path segment routeAgentRequest extracted. + */ +async function gateAgentRequest( + req: Request, + lobby: { name: string }, + env: Env, +): Promise { + const token = new URL(req.url).searchParams.get("token") ?? ""; + const verified = await verifyExtensionToken(token, env); + if (verified === null) return new Response("invalid extension token", { status: 401 }); + const scope = await scopeSession(lobby.name, verified.tenantId, env); + if (scope !== "ok") return new Response("not found", { status: 404 }); + return undefined; +} + export default { async fetch(request: Request, env: Env, ctx: ExecutionContext): Promise { - const agentResponse = await routeAgentRequest(request, env); + const agentResponse = await routeAgentRequest(request, env, { + onBeforeConnect: (req, lobby) => gateAgentRequest(req, lobby, env), + onBeforeRequest: (req, lobby) => gateAgentRequest(req, lobby, env), + }); if (agentResponse) return agentResponse; return app.fetch(request, env, ctx); }, diff --git a/apps/backend/src/secrets.ts b/apps/backend/src/secrets.ts index 2402a8b..5cf7a1a 100644 --- a/apps/backend/src/secrets.ts +++ b/apps/backend/src/secrets.ts @@ -5,18 +5,17 @@ * plaintext and returns it - nothing else. It performs no dispatch (imports * neither session.ts nor the coordinator) and writes the plaintext to no * log, no state, and no error string. The only caller is SessionAgent. - * fillSecret (M-004, DO-side): it awaits resolveSecret(this.env.VAULT, + * fillSecret (M-004, DO-side): it awaits resolveSecret(createVault(this.env), * cmd.secretRef) and immediately dispatches the resulting keystrokes via the * coordinator, so plaintext exists only transiently inside that Durable * Object and never reaches the Worker/route, the model, or any durable * surface (setState, audit, Event response). * - * Pre-production security gate: the concrete VAULT binding wired in - * wrangler.jsonc is a KV namespace (see types.ts's VaultBinding doc), and KV - * values are readable back at rest. That is acceptable for dev, where no - * real credential is stored, but a stronger backend (per-tenant KMS, or a - * Secrets-Store-via-API binding) MUST be swapped in behind this same - * VaultBinding.get seam before any real credential is stored. + * At-rest posture: the VaultBinding handed in is vault.ts's decrypting + * layer over the KV namespace - KV itself holds only AES-256-GCM envelopes + * (see vault.ts), so a KV read-back at rest yields ciphertext without the + * VAULT_MASTER_KEY Worker secret. A per-tenant external KMS remains a + * possible future swap behind this same VaultBinding.get seam. */ import type { VaultBinding } from "./types"; diff --git a/apps/backend/src/session.ts b/apps/backend/src/session.ts index 91715bb..0b5a63e 100644 --- a/apps/backend/src/session.ts +++ b/apps/backend/src/session.ts @@ -3,13 +3,24 @@ import type { AgentContext, Connection, ConnectionContext, WSMessage } from "age import { isWriteCommand, safeParseEvent } from "@understudy/protocol"; import type { Command, Event } from "@understudy/protocol"; import { scopeSession, verifyExtensionToken } from "./auth"; -import { SESSION_NOT_CONNECTED } from "./coordinator"; +import { + COMMAND_TIMED_OUT, + DUPLICATE_COMMAND, + SESSION_NOT_CONNECTED, + SESSION_RESYNCED, +} from "./coordinator"; import { CfSessionCoordinator } from "./coordinator-cf"; import { resolveSecret } from "./secrets"; -import type { Env, SessionState, SessionStatus } from "./types"; +import { createVault } from "./vault"; +import type { DispatchOutcome, Env, SessionState, SessionStatus } from "./types"; type FillSecretCommand = Extract; +// Bounds SessionState.completedWrites (the idempotent-retry replay record). +// 100 write results at ~100 bytes each is well under any DO state budget +// while covering far more retries than a consumer's per-case write count. +const COMPLETED_WRITES_CAP = 100; + export class SessionAgent extends Agent { initialState: SessionState = { browser: null, @@ -18,6 +29,7 @@ export class SessionAgent extends Agent { generation: 0, awaitingCommandIds: [], status: "pending", + completedWrites: [], }; private readonly coordinator: CfSessionCoordinator; @@ -111,7 +123,7 @@ export class SessionAgent extends Agent { this.coordinator.resolvePending(ev); return; case "hello": - this.coordinator.abandonInFlight("session resynced: hello"); + this.coordinator.abandonInFlight(`${SESSION_RESYNCED}: hello`); this.setState({ ...this.state, browser: { browser: ev.browser, extVersion: ev.extVersion }, @@ -127,6 +139,11 @@ export class SessionAgent extends Agent { } async onClose(connection: Connection, code: number, reason: string, wasClean: boolean): Promise { + // A socket that never passed onConnect's auth check never contributed + // to the session's status, so its close must not change it either - a + // rejected/never-authorized socket closing on a fresh session would + // otherwise stamp "pending" over with "detached". + if (!this.isAuthorizedConnection(connection)) return; // Only the LAST authorized socket's close detaches the session: a late // close event from a replaced socket must not stamp "detached" over a // healthy reconnect (the closing connection is excluded explicitly, in @@ -137,49 +154,128 @@ export class SessionAgent extends Agent { if (!stillLive) this.setState({ ...this.state, status: "detached" }); } - async dispatch(command: Command, dryRun?: boolean): Promise { - if (!dryRun) { - return this.coordinator.send(command); - } - if (!isWriteCommand(command)) { - return this.coordinator.send(command); - } - - const probe = await this.checkRefResolves(this.commandRef(command)); - return this.simulatedResult(command.commandId, probe); - } + async dispatch(command: Command, dryRun?: boolean): Promise { + try { + if (dryRun === true && isWriteCommand(command)) { + const probe = await this.checkRefResolves(this.commandRef(command)); + return { ok: true, event: this.simulatedResult(command.commandId, probe) }; + } - async fillSecret(cmd: FillSecretCommand, dryRun?: boolean): Promise { - if (dryRun) { - return this.simulatedResult(cmd.commandId, await this.checkRefResolves(cmd.ref)); - } + // Real dispatch (a dry-run READ also lands here: it executes for real). + // A write whose Event was already recorded replays it instead of + // executing twice - the consumer retries under the same commandId when + // its previous attempt's response was lost or unparseable. The + // completedWrite helpers no-op for reads (incl. a dry-run read), so no + // dryRun guard is needed here: a dry-run write already returned above. + const replayed = this.completedWriteEvent(command); + if (replayed !== undefined) return { ok: true, event: replayed }; - // Gate BEFORE the vault: resolving a secret for a command that cannot - // dispatch would materialize plaintext (and emit a vault access) for - // nothing - fail-fast matters most exactly here (DL-004). - if (!this.hasAuthorizedConnection()) { - throw new Error(`${SESSION_NOT_CONNECTED}: no authorized extension connection`); + const event = await this.coordinator.send(command); + this.rememberCompletedWrite(command, event); + return { ok: true, event }; + } catch (err) { + return this.dispatchFailure(err); } + } - let secret: string; + async fillSecret(cmd: FillSecretCommand, dryRun?: boolean): Promise { try { - secret = await resolveSecret(this.env.VAULT, cmd.secretRef); - } catch { - return { - type: "action_result", + if (dryRun === true) { + return { + ok: true, + event: this.simulatedResult(cmd.commandId, await this.checkRefResolves(cmd.ref)), + }; + } + + // Replay BEFORE the connection gate and the vault: a retry of an + // already-performed fill needs neither liveness nor plaintext. + const replayed = this.completedWriteEvent(cmd); + if (replayed !== undefined) return { ok: true, event: replayed }; + + // Gate BEFORE the vault: resolving a secret for a command that cannot + // dispatch would materialize plaintext (and emit a vault access) for + // nothing - fail-fast matters most exactly here (DL-004). + if (!this.hasAuthorizedConnection()) { + return { + ok: false, + reason: "not_connected", + message: `${SESSION_NOT_CONNECTED}: no authorized extension connection`, + }; + } + + let secret: string; + try { + secret = await resolveSecret(createVault(this.env), cmd.secretRef); + } catch { + return { + ok: true, + event: { + type: "action_result", + commandId: cmd.commandId, + ok: false, + error: "fill_secret: secret could not be resolved", + }, + }; + } + + const event = await this.coordinator.send({ + type: "type", commandId: cmd.commandId, - ok: false, - error: "fill_secret: secret could not be resolved", - }; + ref: cmd.ref, + text: secret, + submit: cmd.submit, + }); + this.rememberCompletedWrite(cmd, event); + return { ok: true, event }; + } catch (err) { + return this.dispatchFailure(err); } + } - return this.coordinator.send({ - type: "type", - commandId: cmd.commandId, - ref: cmd.ref, - text: secret, - submit: cmd.submit, - }); + /** + * Maps the coordinator's prefixed rejections to the typed outcome union + * IN-ISOLATE, so no expected failure ever crosses the RPC boundary as a + * rejected promise (workerd logs those as uncaught exceptions even when + * the Worker-side caller handles them). Anything unrecognized rethrows - + * that is a genuine bug and deserves both the noise and the 500. + */ + private dispatchFailure(err: unknown): DispatchOutcome { + const message = err instanceof Error ? err.message : String(err); + if (message.startsWith(SESSION_NOT_CONNECTED)) { + return { ok: false, reason: "not_connected", message }; + } + if (message.startsWith(COMMAND_TIMED_OUT)) { + return { ok: false, reason: "timed_out", message }; + } + if (message.startsWith(SESSION_RESYNCED)) { + return { ok: false, reason: "resynced", message }; + } + if (message.startsWith(DUPLICATE_COMMAND)) { + return { ok: false, reason: "duplicate_in_flight", message }; + } + throw err; + } + + /** The recorded Event for an already-completed write commandId, if any. */ + private completedWriteEvent(command: Command): Event | undefined { + if (!isWriteCommand(command)) return undefined; + return this.completedWrites().find((entry) => entry.commandId === command.commandId)?.event; + } + + private rememberCompletedWrite(command: Command, event: Event): void { + if (!isWriteCommand(command)) return; + const next = [ + ...this.completedWrites().filter((entry) => entry.commandId !== command.commandId), + { commandId: command.commandId, event }, + ]; + while (next.length > COMPLETED_WRITES_CAP) next.shift(); + this.setState({ ...this.state, completedWrites: next }); + } + + // Persisted before this field existed, a session's state can lack it; + // initialState only seeds brand-new DOs. + private completedWrites(): SessionState["completedWrites"] { + return this.state.completedWrites ?? []; } async getStatus(): Promise<{ @@ -240,6 +336,10 @@ export class SessionAgent extends Agent { case "type": case "fill_secret": case "key": + case "scroll": + // scroll.ref is optional (undefined => a window scroll): a ref-bearing + // dry-run probes it, a ref-less one simulates ok:true with no wire hop + // (like navigate/switch_tab) - it was never a liveness signal. return command.ref; default: return undefined; @@ -252,7 +352,8 @@ export class SessionAgent extends Agent { // The delivery predicate the coordinator's fail-fast gate consults: the // same precondition sendToExtension relies on, NOT the persisted status - // scalar (which a late onClose from a replaced socket can leave stale). + // scalar (onClose guards the stamping races, but the scalar remains an + // eventually-consistent echo, not the delivery truth). private hasAuthorizedConnection(): boolean { for (const connection of this.getConnections()) { if (this.isAuthorizedConnection(connection)) return true; diff --git a/apps/backend/src/types.ts b/apps/backend/src/types.ts index 43ee451..f049aa4 100644 --- a/apps/backend/src/types.ts +++ b/apps/backend/src/types.ts @@ -24,8 +24,14 @@ type HelloBrowserInfo = Pick, "browser" | "ext * agnostic: the concrete Cloudflare binding is a KV namespace (see * wrangler.jsonc's VAULT binding), because an arbitrary per-fill secretRef * needs a dynamic keyed lookup that CF's Secrets / Secrets Store bindings - - * one static binding per fixed secret name - cannot address. A per-tenant - * external KMS is a possible future swap behind this same seam. + * one static binding per fixed secret name - cannot address. + * + * Two layers implement this same interface: Env.VAULT (the raw KV namespace, + * which stores only AES-256-GCM envelopes - never plaintext at rest) and + * vault.ts's EncryptedKvVault (which wraps it and decrypts with + * VAULT_MASTER_KEY). resolveSecret always goes through the decrypting layer + * via vault.ts's createVault(env). A per-tenant external KMS remains a + * possible future swap behind this seam. */ export interface VaultBinding { get(secretRef: string): Promise; @@ -48,6 +54,13 @@ export interface Env { CALLER_TOKENS: string; /** Extension per-user token(s) (JSON), verified independently of caller auth. Required via `secrets.required`, like CALLER_TOKENS. */ EXTENSION_TOKENS: string; + /** + * base64url-encoded 32-byte AES-256-GCM key that envelope-encrypts every + * vault value (vault.ts). KV holds only ciphertext; without this secret a + * KV read-back at rest yields nothing usable. Required via + * `secrets.required`, like the token maps. + */ + VAULT_MASTER_KEY: string; } /** @@ -70,4 +83,29 @@ export interface SessionState { generation: number; awaitingCommandIds: string[]; status: SessionStatus; + /** + * Completed WRITE commands' Events, oldest first, capped in session.ts - + * the service half of the idempotent-retry contract. A consumer retrying + * a write under the same commandId (the connector derives it from the + * breakwater idempotency key) gets the recorded Event back instead of a + * second execution, closing the write-performed-but-response-lost gap. + * Only ever holds action_results for writes: small, and plaintext-free by + * the DL-004 construction (fill_secret results carry ok/error only). + */ + completedWrites: { commandId: string; event: Event }[]; } + +/** + * What dispatch/fillSecret return across the DO RPC boundary. Expected + * delivery failures travel as data, not exceptions: a rejected RPC promise + * is logged by workerd as an uncaught exception even when the Worker-side + * caller handles it, and a typed reason beats message-prefix parsing at the + * route. Unknown errors still throw - those are genuine 500s. + */ +export type DispatchOutcome = + | { ok: true; event: Event } + | { + ok: false; + reason: "not_connected" | "timed_out" | "resynced" | "duplicate_in_flight"; + message: string; + }; diff --git a/apps/backend/src/vault.ts b/apps/backend/src/vault.ts new file mode 100644 index 0000000..87efc7f --- /dev/null +++ b/apps/backend/src/vault.ts @@ -0,0 +1,96 @@ +/** + * Envelope encryption for the credential vault (the pre-production gate the + * M3 README called out): KV stores only AES-256-GCM ciphertext envelopes, + * so a KV read-back at rest yields nothing usable without VAULT_MASTER_KEY + * (a Worker secret that never lives in KV or wrangler.jsonc). + * + * Envelope wire format: `v1..` with a + * random 96-bit IV per value. GCM authenticates, so tampering (or the wrong + * key) fails decryption outright - fail closed, never garbage plaintext. + * + * scripts/vault-put.mjs mirrors this format in plain Node for seeding; the + * two must change together (the format test in vault.test.ts pins it). + */ + +import { base64urlDecode, base64urlEncode } from "./base64url"; +import type { Env, VaultBinding } from "./types"; + +const ENVELOPE_VERSION = "v1"; +const IV_BYTES = 12; +const MASTER_KEY_BYTES = 32; + +async function importMasterKey(masterKey: string, usage: "encrypt" | "decrypt"): Promise { + let raw: Uint8Array; + try { + raw = base64urlDecode(masterKey); + } catch { + throw new Error("vault master key is not valid base64url"); + } + if (raw.length !== MASTER_KEY_BYTES) { + throw new Error(`vault master key must be ${MASTER_KEY_BYTES} bytes`); + } + return crypto.subtle.importKey("raw", raw as BufferSource, { name: "AES-GCM" }, false, [usage]); +} + +/** Encrypts one secret value into the versioned envelope format. */ +export async function encryptSecret(masterKey: string, plaintext: string): Promise { + const key = await importMasterKey(masterKey, "encrypt"); + const iv = crypto.getRandomValues(new Uint8Array(IV_BYTES)); + const ciphertext = await crypto.subtle.encrypt( + { name: "AES-GCM", iv: iv as BufferSource }, + key, + new TextEncoder().encode(plaintext), + ); + return `${ENVELOPE_VERSION}.${base64urlEncode(iv)}.${base64urlEncode(new Uint8Array(ciphertext))}`; +} + +/** + * Decrypts one envelope. Throws on any malformed envelope, wrong key, or + * tampered ciphertext - with a message that names the failure class only, + * never envelope or plaintext material (DL-004). + */ +export async function decryptSecret(masterKey: string, envelope: string): Promise { + const parts = envelope.split("."); + if (parts.length !== 3 || parts[0] !== ENVELOPE_VERSION || !parts[1] || !parts[2]) { + throw new Error("vault value is not a recognized envelope"); + } + const key = await importMasterKey(masterKey, "decrypt"); + let plaintext: ArrayBuffer; + try { + plaintext = await crypto.subtle.decrypt( + { name: "AES-GCM", iv: base64urlDecode(parts[1]) as BufferSource }, + key, + base64urlDecode(parts[2]) as BufferSource, + ); + } catch { + // GCM auth failure and base64 garbage collapse to one scrubbed message: + // distinguishing them would leak nothing useful and costs a code path. + throw new Error("vault envelope failed to decrypt"); + } + return new TextDecoder().decode(plaintext); +} + +/** + * The decrypting VaultBinding layer over the raw ciphertext store. get() + * returns plaintext for a present envelope, null for an absent key, and + * throws (fail closed) for an envelope it cannot decrypt - the caller + * (SessionAgent.fillSecret via resolveSecret) already maps every throw to a + * scrubbed ok:false result. + */ +export class EncryptedKvVault implements VaultBinding { + constructor( + private readonly store: VaultBinding, + private readonly masterKey: string, + ) {} + + async get(secretRef: string): Promise { + const envelope = await this.store.get(secretRef); + if (envelope === null) return null; + return decryptSecret(this.masterKey, envelope); + } +} + +/** The one production wiring: Env.VAULT ciphertext + VAULT_MASTER_KEY. */ +export function createVault(env: Env): VaultBinding { + return new EncryptedKvVault(env.VAULT, env.VAULT_MASTER_KEY); +} diff --git a/apps/backend/test/auth.test.ts b/apps/backend/test/auth.test.ts index 37cf9c1..2dd4d1e 100644 --- a/apps/backend/test/auth.test.ts +++ b/apps/backend/test/auth.test.ts @@ -25,6 +25,7 @@ function makeEnv(overrides: Partial = {}): Env { AUTH_HMAC_SECRET: "test-hmac-secret-do-not-use-in-prod", CALLER_TOKENS: JSON.stringify(CALLER_TOKENS), EXTENSION_TOKENS: JSON.stringify(EXTENSION_TOKENS), + VAULT_MASTER_KEY: "unused-by-auth-tests", ...overrides, }; } diff --git a/apps/backend/test/coordinator.test.ts b/apps/backend/test/coordinator.test.ts index a132ad9..8f98451 100644 --- a/apps/backend/test/coordinator.test.ts +++ b/apps/backend/test/coordinator.test.ts @@ -175,4 +175,27 @@ describe("CfSessionCoordinator", () => { await expect(promiseB).rejects.toThrow("session resynced: hello received"); expect(host.getAwaitingCommandIds()).toEqual([]); }); + + it("refuses a second send for a commandId already in flight, leaving the first undisturbed", async () => { + // #given a command parked and awaiting its event + const host = createFakeHost(); + const coordinator = new CfSessionCoordinator(host); + const cmd: Command = { type: "click", commandId: "c-dup", ref: "r1" }; + const first = coordinator.send(cmd); + + // #when the same commandId is sent again mid-flight (stable consumer- + // derived ids make this reachable) + const err = await coordinator.send(cmd).catch((e: unknown) => e); + + // #then the duplicate is refused with the mappable prefix, nothing extra + // hit the wire, and the original still resolves normally + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toBe("duplicate command in flight: c-dup is already awaiting its event"); + expect(host.sent).toHaveLength(1); + + const event: Event = { type: "action_result", commandId: "c-dup", ok: true }; + coordinator.resolvePending(event); + await expect(first).resolves.toEqual(event); + expect(host.getAwaitingCommandIds()).toEqual([]); + }); }); diff --git a/apps/backend/test/service.test.ts b/apps/backend/test/service.test.ts index 8acdaea..5df5bcd 100644 --- a/apps/backend/test/service.test.ts +++ b/apps/backend/test/service.test.ts @@ -5,7 +5,14 @@ import { safeParseCommand, safeParseEvent } from "@understudy/protocol"; import type { Command } from "@understudy/protocol"; import type { SessionAgent } from "../src/session"; import type { SessionStatus } from "../src/types"; -import { CALLER_TOKEN_A, CALLER_TOKEN_B, EXTENSION_TOKEN_A } from "./tokens"; +import { encryptSecret } from "../src/vault"; +import { + CALLER_TOKEN_A, + CALLER_TOKEN_B, + EXTENSION_TOKEN_A, + EXTENSION_TOKEN_B, + TEST_VAULT_MASTER_KEY, +} from "./tokens"; import { BASE, getSessionStub, getWebSocket } from "./helpers"; /** @@ -13,12 +20,14 @@ import { BASE, getSessionStub, getWebSocket } from "./helpers"; * production code can never write through it. The real binding is a KV * namespace (wrangler.jsonc), which does support `put` - tests need that to * seed fixtures, so this narrow, test-only widening stays local to this file - * rather than loosening the production-facing type. + * rather than loosening the production-facing type. Values are sealed with + * the same envelope the production seeder writes (scripts/vault-put.mjs): + * KV never holds plaintext, in tests either. */ -function seedVault(secretRef: string, plaintext: string): Promise { +async function seedVault(secretRef: string, plaintext: string): Promise { return (env.VAULT as unknown as { put(key: string, value: string): Promise }).put( secretRef, - plaintext, + await encryptSecret(TEST_VAULT_MASTER_KEY, plaintext), ); } @@ -388,6 +397,43 @@ describe("fill_secret", () => { socket.close(1000, "done"); } }); + + it("fails closed with a scrubbed ok:false when a stored vault value is not a valid envelope", async () => { + // #given a RAW (non-envelope) value written straight to KV, as a legacy + // plaintext row or a value sealed under a rotated key would look at rest + await (env.VAULT as unknown as { put(key: string, value: string): Promise }).put( + "vault://legacy-raw", + "legacy-plaintext-not-an-envelope", + ); + const sessionId = await openSession(CALLER_TOKEN_A); + const socket = await connectFakeExtension(sessionId); + const received = collectCommands(socket); + + try { + // #when a consumer fill_secrets that ref + const res = await postCommand(sessionId, CALLER_TOKEN_A, { + type: "fill_secret", + commandId: "c-legacy", + ref: "s1e1", + secretRef: "vault://legacy-raw", + }); + + // #then EncryptedKvVault refuses to decrypt it -> the DO's catch returns + // the same scrubbed ok:false as any resolution failure (no envelope + // material, no key material, no 500), and nothing is typed + const event = await res.json(); + expect(event).toEqual({ + type: "action_result", + commandId: "c-legacy", + ok: false, + error: "fill_secret: secret could not be resolved", + }); + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(received).toEqual([]); + } finally { + socket.close(1000, "done"); + } + }); }); describe("dryRun (DL-011: fail-safe, never dispatches a mutation or resolves a secret)", () => { @@ -498,6 +544,69 @@ describe("dryRun (DL-011: fail-safe, never dispatches a mutation or resolves a s } }); + it("dryRun switch_tab simulates ok:true and never switches the tab (a write, not a read)", async () => { + // #given a connected fake extension + const sessionId = await openSession(CALLER_TOKEN_A); + const socket = await connectFakeExtension(sessionId); + const received = collectCommands(socket); + + try { + // #when a consumer dry-runs switch_tab (reclassified as a write in protocol v0.4.0) + const res = await postCommand( + sessionId, + CALLER_TOKEN_A, + { type: "switch_tab", commandId: "c-swt-dry", tabId: 3 }, + true, + ); + + // #then it simulates ok:true (no ref to probe, zero wire) rather than + // actually switching the user's tab - the dry-run-safety fix + const event = await res.json(); + expect(event).toEqual({ + type: "action_result", + commandId: "c-swt-dry", + ok: true, + simulated: true, + }); + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(received).toEqual([]); + } finally { + socket.close(1000, "done"); + } + }); + + it("dryRun scroll probes its ref and simulates, never scrolling", async () => { + // #given a connected fake extension whose ref map resolves the target + const sessionId = await openSession(CALLER_TOKEN_A); + const socket = await connectFakeExtension(sessionId); + const messages = answerResolveRefsWith(socket, ["s1e4"]); + + try { + // #when a consumer dry-runs a ref-bearing scroll + const res = await postCommand( + sessionId, + CALLER_TOKEN_A, + { type: "scroll", commandId: "c-scr-dry", ref: "s1e4", dy: 100 }, + true, + ); + + // #then only the read-only probe hits the wire - never the scroll itself + const event = await res.json(); + expect(event).toEqual({ + type: "action_result", + commandId: "c-scr-dry", + ok: true, + simulated: true, + }); + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(messages).toEqual([ + { type: "resolve_ref", commandId: expect.any(String), ref: "s1e4" }, + ]); + } finally { + socket.close(1000, "done"); + } + }); + it("dispatches a READ command for real - only WRITE commands are simulated", async () => { // #given a connected fake extension const sessionId = await openSession(CALLER_TOKEN_A); @@ -722,6 +831,26 @@ describe("extension liveness fail-fast", () => { replacement.close(1000, "done"); } }); + + it("detaches once the LAST authorized socket closes, after a replacement briefly coexisted", async () => { + // #given two authorized sockets, the old one replaced by a newer one + const sessionId = await openSession(CALLER_TOKEN_A); + const old = await connectFakeExtension(sessionId); + const replacement = await connectFakeExtension(sessionId); + + // #when the old one closes first - the replacement keeps the session live + old.close(1000, "replaced"); + await new Promise((resolve) => setTimeout(resolve, 150)); + const stub = await getSessionStub(sessionId); + expect((await stub.getStatus()).status).toBe("connected"); + + // #when the replacement then also closes - now nothing authorized remains + replacement.close(1000, "gone"); + await waitForStatus(sessionId, "detached"); + + // #then the session finally detaches (the full replaced-then-both-closed order) + expect((await stub.getStatus()).status).toBe("detached"); + }); }); describe("error taxonomy (route mapping)", () => { @@ -743,7 +872,7 @@ describe("error taxonomy (route mapping)", () => { expect(await res.json()).toEqual({ error: "invalid body" }); }); - it("maps an in-flight command abandoned by a hello resync to the uniform JSON 500", async () => { + it("maps an in-flight command abandoned by a hello resync to a retryable 503", async () => { // #given a connected extension holding a command in flight const sessionId = await openSession(CALLER_TOKEN_A); const socket = await connectFakeExtension(sessionId); @@ -761,11 +890,12 @@ describe("error taxonomy (route mapping)", () => { JSON.stringify({ type: "hello", browser: "chrome", extVersion: "1.0.0", tabs: [] }), ); - // #then the abandoned dispatch surfaces as the uniform JSON 500 - - // app.onError's shape, with no stack or internals in the body + // #then the abandoned dispatch surfaces as a retryable 503 with its + // own honest reason - the extension is alive (it just resynced), so + // this is infrastructure weather, not the masked 500 it once was const res = await resPromise; - expect(res.status).toBe(500); - expect(await res.json()).toEqual({ error: "internal error" }); + expect(res.status).toBe(503); + expect(await res.json()).toEqual({ error: "session resynced mid-command" }); } finally { socket.close(1000, "done"); } @@ -805,3 +935,299 @@ describe("error taxonomy (route mapping)", () => { 40_000, ); }); + +describe("WS upgrade gate (pre-accept, index.ts onBeforeConnect/onBeforeRequest)", () => { + it("refuses a bad-token upgrade with 401 before the DO ever accepts a socket", async () => { + // #given a session and an upgrade carrying an unknown extension token + const sessionId = await openSession(CALLER_TOKEN_A); + + // #when the upgrade hits the worker router + const res = await exports.default.fetch( + new Request(`${BASE}/agents/session/${sessionId}?token=not-a-real-token`, { + headers: { Upgrade: "websocket" }, + }), + ); + + // #then it is rejected at the edge - a plain 401, no accepted-then-closed + // socket, so an attacker never enters the DO's connection set at all + expect(res.status).toBe(401); + expect(res.webSocket ?? null).toBeNull(); + }); + + it("refuses a cross-tenant upgrade with 404, not 403 - no existence oracle (DL-008)", async () => { + // #given a session owned by tenantA and tenantB's valid extension token + const sessionId = await openSession(CALLER_TOKEN_A); + + // #when tenantB's token attempts the upgrade + const res = await exports.default.fetch( + new Request(`${BASE}/agents/session/${sessionId}?token=${EXTENSION_TOKEN_B}`, { + headers: { Upgrade: "websocket" }, + }), + ); + + // #then it collapses to the same 404 a malformed sessionId gets + expect(res.status).toBe(404); + expect(res.webSocket ?? null).toBeNull(); + }); + + it("gates non-WebSocket requests on the agent path too (onBeforeRequest)", async () => { + // #given a plain HTTP request (no Upgrade) to the agent route with no token + const sessionId = await openSession(CALLER_TOKEN_A); + + // #when it hits the worker router + const res = await exports.default.fetch( + new Request(`${BASE}/agents/session/${sessionId}`), + ); + + // #then the SDK's HTTP surface on the DO is unreachable without a token + expect(res.status).toBe(401); + }); + + it("a rejected upgrade never disturbs the session's status", async () => { + // #given a fresh (pending) session + const sessionId = await openSession(CALLER_TOKEN_A); + + // #when a bad-token upgrade is refused + await exports.default.fetch( + new Request(`${BASE}/agents/session/${sessionId}?token=nope`, { + headers: { Upgrade: "websocket" }, + }), + ); + + // #then the session still reads pending - nothing was accepted, nothing + // closed, nothing stamped + const stub = await getSessionStub(sessionId); + expect((await stub.getStatus()).status).toBe("pending"); + }); +}); + +describe("idempotent write replay (stable commandId contract)", () => { + it("replays a completed write's Event for a retry under the same commandId without re-executing", async () => { + // #given a connected extension that answered a click once + const sessionId = await openSession(CALLER_TOKEN_A); + const socket = await connectFakeExtension(sessionId); + const received = collectCommands(socket); + + try { + const incoming = waitForCommand(socket); + const firstRes = postCommand(sessionId, CALLER_TOKEN_A, { + type: "click", + commandId: "ik_case1:step1:click", + ref: "s1e1", + }); + await incoming; + socket.send( + JSON.stringify({ + type: "action_result", + commandId: "ik_case1:step1:click", + ok: true, + url: "https://portal.example/done", + }), + ); + const first = await (await firstRes).json(); + + // #when the consumer retries the SAME commandId (its previous response + // was lost or unparseable - the connector derives the id from the + // breakwater idempotency key, so a retry reuses it) + const retryRes = await postCommand(sessionId, CALLER_TOKEN_A, { + type: "click", + commandId: "ik_case1:step1:click", + ref: "s1e1", + }); + + // #then the recorded Event is replayed byte-for-byte and the extension + // never saw a second click - the write executed exactly once + expect(retryRes.status).toBe(200); + expect(await retryRes.json()).toEqual(first); + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(received.filter((cmd) => cmd.type === "click")).toHaveLength(1); + } finally { + socket.close(1000, "done"); + } + }); + + it("replays a completed write even with no extension connected - a replay needs no liveness", async () => { + // #given a write completed while an extension was attached, which then dropped + const sessionId = await openSession(CALLER_TOKEN_A); + const socket = await connectFakeExtension(sessionId); + const incoming = waitForCommand(socket); + const firstRes = postCommand(sessionId, CALLER_TOKEN_A, { + type: "navigate", + commandId: "ik_case1:step2:navigate", + url: "https://example.com/", + }); + await incoming; + socket.send( + JSON.stringify({ type: "action_result", commandId: "ik_case1:step2:navigate", ok: true }), + ); + const first = await (await firstRes).json(); + socket.close(1000, "gone"); + + // #when the consumer retries after the extension detached + const retryRes = await postCommand(sessionId, CALLER_TOKEN_A, { + type: "navigate", + commandId: "ik_case1:step2:navigate", + url: "https://example.com/", + }); + + // #then the recorded outcome is served instead of a 503 - the work + // already happened; only NEW work needs the wire + expect(retryRes.status).toBe(200); + expect(await retryRes.json()).toEqual(first); + }); + + it("refuses a concurrent duplicate write commandId as 409 while the first is still in flight", async () => { + // #given a write parked in the coordinator, not yet answered + const sessionId = await openSession(CALLER_TOKEN_A); + const socket = await connectFakeExtension(sessionId); + + try { + const incoming = waitForCommand(socket); + const firstRes = postCommand(sessionId, CALLER_TOKEN_A, { + type: "click", + commandId: "ik_case1:step3:click", + ref: "s1e1", + }); + await incoming; + + // #when the same commandId is posted again mid-flight + const dupRes = await postCommand(sessionId, CALLER_TOKEN_A, { + type: "click", + commandId: "ik_case1:step3:click", + ref: "s1e1", + }); + + // #then the duplicate is refused without disturbing the original... + expect(dupRes.status).toBe(409); + expect(await dupRes.json()).toEqual({ error: "command already in flight" }); + + // ...which still resolves normally when the extension answers + socket.send( + JSON.stringify({ type: "action_result", commandId: "ik_case1:step3:click", ok: true }), + ); + expect((await firstRes).status).toBe(200); + } finally { + socket.close(1000, "done"); + } + }); + + it("a retried fill_secret replays the recorded result without touching the vault again", async () => { + // #given a fill_secret that completed once + await seedVault("vault://replay-pw", "hunter2-replay"); + const sessionId = await openSession(CALLER_TOKEN_A); + const socket = await connectFakeExtension(sessionId); + + try { + const incoming = waitForCommand(socket); + const firstRes = postCommand(sessionId, CALLER_TOKEN_A, { + type: "fill_secret", + commandId: "ik_case1:login:fill", + ref: "s1e1", + secretRef: "vault://replay-pw", + }); + await incoming; + socket.send( + JSON.stringify({ type: "action_result", commandId: "ik_case1:login:fill", ok: true }), + ); + const first = await (await firstRes).json(); + + // #when the consumer retries the same commandId + const vaultGetSpy = vi.spyOn(env.VAULT, "get"); + try { + const retryRes = await postCommand(sessionId, CALLER_TOKEN_A, { + type: "fill_secret", + commandId: "ik_case1:login:fill", + ref: "s1e1", + secretRef: "vault://replay-pw", + }); + + // #then the recorded result is replayed with zero vault access and + // zero re-typing - no second plaintext materialization (DL-004) + expect(retryRes.status).toBe(200); + expect(await retryRes.json()).toEqual(first); + expect(vaultGetSpy).not.toHaveBeenCalled(); + } finally { + vaultGetSpy.mockRestore(); + } + } finally { + socket.close(1000, "done"); + } + }); + + it("replays a completed write across a hello resync (completedWrites survives the resync)", async () => { + // #given a write that completed on a connected extension + const sessionId = await openSession(CALLER_TOKEN_A); + const socket = await connectFakeExtension(sessionId); + + try { + const incoming = waitForCommand(socket); + const firstRes = postCommand(sessionId, CALLER_TOKEN_A, { + type: "click", + commandId: "ik_resync:click", + ref: "s1e1", + }); + await incoming; + socket.send( + JSON.stringify({ + type: "action_result", + commandId: "ik_resync:click", + ok: true, + url: "https://portal.example/after", + }), + ); + const first = await (await firstRes).json(); + + // #when the extension resyncs (hello bumps generation, abandons in-flight) + socket.send( + JSON.stringify({ type: "hello", browser: "chrome", extVersion: "1.0.0", tabs: [] }), + ); + await new Promise((resolve) => setTimeout(resolve, 50)); + + // #then a retry of the same commandId still replays the recorded Event + // unchanged - completedWrites is untouched by the resync, and replay is + // keyed by commandId alone (no generation dependence) + const retryRes = await postCommand(sessionId, CALLER_TOKEN_A, { + type: "click", + commandId: "ik_resync:click", + ref: "s1e1", + }); + expect(retryRes.status).toBe(200); + expect(await retryRes.json()).toEqual(first); + } finally { + socket.close(1000, "done"); + } + }); + + it("does NOT replay reads - the same get_tabs commandId re-executes freely", async () => { + // #given a read that completed once + const sessionId = await openSession(CALLER_TOKEN_A); + const socket = await connectFakeExtension(sessionId); + const received = collectCommands(socket); + + try { + expect((await roundTripGetTabs(socket, sessionId, "read-1")).status).toBe(200); + + // #when the same read commandId is posted again + // #then it round-trips to the extension again - reads are free to + // re-execute; only writes carry the exactly-once contract + expect((await roundTripGetTabs(socket, sessionId, "read-1")).status).toBe(200); + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(received.filter((cmd) => cmd.type === "get_tabs")).toHaveLength(2); + } finally { + socket.close(1000, "done"); + } + }); + + /** One full get_tabs round-trip (duplicated from the liveness suite's roundTrip, which is scoped there). */ + async function roundTripGetTabs( + socket: WebSocket, + sessionId: string, + commandId: string, + ): Promise { + const incoming = waitForCommand(socket); + const resPromise = postCommand(sessionId, CALLER_TOKEN_A, { type: "get_tabs", commandId }); + const received = await incoming; + socket.send(JSON.stringify({ type: "tabs_result", commandId: received.commandId, tabs: [] })); + return resPromise; + } +}); diff --git a/apps/backend/test/session.test.ts b/apps/backend/test/session.test.ts index 540cbae..f648ed2 100644 --- a/apps/backend/test/session.test.ts +++ b/apps/backend/test/session.test.ts @@ -1,51 +1,70 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, vi } from "vitest"; import { env, exports } from "cloudflare:workers"; import { runInDurableObject, evictDurableObject } from "cloudflare:test"; -import type { Connection } from "agents"; +import type { Connection, ConnectionContext } from "agents"; import type { Command } from "@understudy/protocol"; import { mintSessionId } from "../src/auth"; import type { SessionAgent } from "../src/session"; import { EXTENSION_TOKEN_A, EXTENSION_TOKEN_B } from "./tokens"; import { BASE, getSessionStub, getWebSocket } from "./helpers"; -function waitForClose(socket: WebSocket): Promise<{ code: number }> { - return new Promise((resolve, reject) => { - const timeout = setTimeout( - () => reject(new Error("timed out waiting for a WebSocket close")), - 10_000, - ); - socket.addEventListener("close", (event) => { - clearTimeout(timeout); - resolve(event as unknown as { code: number }); - }); +/** onMessage only reads `connection.state.authorized` (the onConnect auth gate) - see src/session.ts. */ +const FAKE_CONNECTION = { state: { authorized: true } } as unknown as Connection; + +/** + * A mutable stand-in for a Connection at the onConnect/onClose surface. + * The spies are returned alongside the connection (not only as its + * properties) so assertions hold direct references; setState mirrors onto + * .state the way the SDK does, so isAuthorizedConnection() reads what + * onConnect wrote. + */ +function fakeConnection(initialState: { authorized?: boolean } | null = null): { + connection: Connection; + close: ReturnType; + setState: ReturnType; +} { + const close = vi.fn(); + const holder: { state: { authorized?: boolean } | null } = { state: initialState }; + const setState = vi.fn((next: unknown) => { + holder.state = next as { authorized?: boolean } | null; }); + const connection = Object.assign(holder, { close, setState }) as unknown as Connection; + return { connection, close, setState }; } -/** onMessage only reads `connection.state.authorized` (the onConnect auth gate) - see src/session.ts. */ -const FAKE_CONNECTION = { state: { authorized: true } } as unknown as Connection; +function upgradeContextFor(sessionId: string, token: string): ConnectionContext { + return { + request: new Request(`${BASE}/agents/session/${sessionId}?token=${token}`), + } as ConnectionContext; +} -describe("onConnect token verification", () => { +/** + * The worker-level gate (index.ts onBeforeConnect) now refuses bad upgrades + * before the DO accepts anything - service.test.ts covers that layer. These + * tests drive onConnect DIRECTLY, proving the in-DO gate stands on its own + * (defense in depth): any path that reached the DO without the router would + * still be refused. + */ +describe("onConnect token verification (in-DO defense in depth)", () => { it("closes the connection with 1008 for a bad extension token", async () => { - // #given a WS upgrade request carrying an unknown token + // #given a DO instance and a connection whose upgrade carried an unknown token const sessionId = crypto.randomUUID(); - const res = await exports.default.fetch( - new Request(`${BASE}/agents/session/${sessionId}?token=not-a-real-token`, { - headers: { Upgrade: "websocket" }, - }), - ); - const socket = getWebSocket(res); - const closed = waitForClose(socket); + const stub = await getSessionStub(sessionId); + const { connection, close, setState } = fakeConnection(); - // #when the client accepts the upgrade - socket.accept(); + // #when onConnect runs its auth check + await runInDurableObject(stub, (instance: SessionAgent) => + instance.onConnect(connection, upgradeContextFor(sessionId, "not-a-real-token")), + ); - // #then the server closes it with 1008 - const event = await closed; - expect(event.code).toBe(1008); + // #then the socket is closed 1008 and never marked authorized + expect(close).toHaveBeenCalledWith(1008, "invalid extension token"); + expect(setState).not.toHaveBeenCalled(); }); it("binds the session to its tenant and stays connected for a good extension token", async () => { - // #given a WS upgrade request carrying a valid extension token for the session's own tenant + // #given a WS upgrade request carrying a valid extension token for the + // session's own tenant, through the REAL worker route (both gates pass) const sessionId = await mintSessionId("tenantA", env); const res = await exports.default.fetch( new Request(`${BASE}/agents/session/${sessionId}?token=${EXTENSION_TOKEN_A}`, { @@ -70,20 +89,53 @@ describe("onConnect token verification", () => { it("closes the connection with 1008 for a cross-tenant extension token", async () => { // #given a session owned by tenantA (its sessionId HMAC-embeds tenantA) const sessionId = await mintSessionId("tenantA", env); - const res = await exports.default.fetch( - new Request(`${BASE}/agents/session/${sessionId}?token=${EXTENSION_TOKEN_B}`, { - headers: { Upgrade: "websocket" }, - }), + const stub = await getSessionStub(sessionId); + const { connection, close, setState } = fakeConnection(); + + // #when a tenantB extension token reaches onConnect directly + await runInDurableObject(stub, (instance: SessionAgent) => + instance.onConnect(connection, upgradeContextFor(sessionId, EXTENSION_TOKEN_B)), ); - const socket = getWebSocket(res); - const closed = waitForClose(socket); - // #when a tenantB extension token attempts to attach to tenantA's session - socket.accept(); + // #then it is closed 1008 instead of binding a foreign tenant + expect(close).toHaveBeenCalledWith(1008, "tenant mismatch"); + expect(setState).not.toHaveBeenCalled(); + }); +}); - // #then the server closes it with 1008 instead of binding a foreign tenant - const event = await closed; - expect(event.code).toBe(1008); +describe("onClose status stamping", () => { + it("a never-authorized socket's close does not stamp a pending session detached", async () => { + // #given a fresh (pending) session and a connection that never passed auth + const sessionId = crypto.randomUUID(); + const stub = await getSessionStub(sessionId); + const { connection: unauthorized } = fakeConnection(null); + + // #when that connection closes (e.g. right after onConnect's 1008) + await runInDurableObject(stub, async (instance: SessionAgent) => { + expect(instance.state.status).toBe("pending"); + await instance.onClose(unauthorized, 1008, "invalid extension token", true); + }); + + // #then the session still reads pending - a socket that never + // contributed to the status cannot change it + expect((await stub.getStatus()).status).toBe("pending"); + }); + + it("an authorized socket's close still detaches when it was the last one", async () => { + // #given a session an authorized socket connected to (status: connected) + const sessionId = crypto.randomUUID(); + const stub = await getSessionStub(sessionId); + const { connection: authorized } = fakeConnection({ authorized: true }); + + await runInDurableObject(stub, async (instance: SessionAgent) => { + instance.setState({ ...instance.state, status: "connected" }); + + // #when it closes with no surviving authorized socket + await instance.onClose(authorized, 1000, "gone", true); + }); + + // #then the session reads detached, as before + expect((await stub.getStatus()).status).toBe("detached"); }); }); @@ -119,8 +171,8 @@ describe("dispatch / resolvePending", () => { return dispatchPromise; }); - // #then dispatch resolves with that event and the marker is cleared - expect(result).toEqual({ type: "tabs_result", commandId: "s1", tabs: [] }); + // #then dispatch resolves with that event (as an ok outcome) and the marker is cleared + expect(result).toEqual({ ok: true, event: { type: "tabs_result", commandId: "s1", tabs: [] } }); await runInDurableObject(stub, (instance: SessionAgent) => { expect(instance.state.awaitingCommandIds).toEqual([]); }); @@ -179,29 +231,22 @@ describe("DO eviction resilience (DL-007)", () => { describe("hello resync", () => { it("bumps generation, records browser/tabs, sets connected, and abandons in-flight commands", async () => { - // #given a dispatched command still awaiting its result, later - // abandoned (rejected asynchronously, from the separate onMessage call - // below). A rejection handler is attached in the SAME synchronous tick - // as the dispatch() call (converting it into an always-resolving - // outcome captured in this closure) rather than awaited later: - // attaching `.rejects` only once the promise is retrieved later leaves - // a window where nothing has claimed the rejection yet, which surfaces - // as an unhandled rejection - and fails the process exit code - even - // though this test's own assertion on it passes (verified empirically). + // #given a dispatched command still awaiting its result, later abandoned + // by the separate onMessage call below. dispatch() converts the + // coordinator's abandon-rejection into a resolved outcome in-isolate + // (types.ts::DispatchOutcome), so nothing here can surface as an + // unhandled rejection. const sessionId = crypto.randomUUID(); const stub = await getSessionStub(sessionId); const cmd: Command = { type: "get_tabs", commandId: "resync-1" }; - let outcome!: Promise<{ settled: "resolved" | "rejected"; value: unknown }>; + let outcome!: ReturnType; await runInDurableObject(stub, (instance: SessionAgent) => { // Stand in for a live authorized socket (see "resolves a dispatched - // command's promise" above), or the fail-fast gate rejects before the + // command's promise" above), or the fail-fast gate refuses before the // marker is ever parked and there is nothing for the resync to abandon. Object.assign(instance, { hasAuthorizedConnection: () => true }); - outcome = instance.dispatch(cmd).then( - (value) => ({ settled: "resolved", value }) as const, - (error: unknown) => ({ settled: "rejected", value: error }) as const, - ); + outcome = instance.dispatch(cmd); expect(instance.state.awaitingCommandIds).toContain("resync-1"); }); @@ -218,10 +263,12 @@ describe("hello resync", () => { ), ); - // #then the in-flight dispatch is abandoned (rejected) - const result = await outcome; - expect(result.settled).toBe("rejected"); - expect(result.value).toBeInstanceOf(Error); + // #then the in-flight dispatch resolves to the typed abandoned outcome + await expect(outcome).resolves.toEqual({ + ok: false, + reason: "resynced", + message: "session resynced: hello", + }); // #then the session state reflects the resync await runInDurableObject(stub, (instance: SessionAgent) => { diff --git a/apps/backend/test/tokens.ts b/apps/backend/test/tokens.ts index 70968e1..21fb982 100644 --- a/apps/backend/test/tokens.ts +++ b/apps/backend/test/tokens.ts @@ -21,3 +21,7 @@ export const EXTENSION_TOKENS = { [EXTENSION_TOKEN_A]: "tenantA", [EXTENSION_TOKEN_B]: "tenantB", }; + +// base64url of the 32-byte literal "test-vault-master-key-abcdefghij" - +// the AES-256-GCM key vault.ts envelopes test secrets with (src/vault.ts). +export const TEST_VAULT_MASTER_KEY = "dGVzdC12YXVsdC1tYXN0ZXIta2V5LWFiY2RlZmdoaWo"; diff --git a/apps/backend/test/vault.test.ts b/apps/backend/test/vault.test.ts new file mode 100644 index 0000000..c8a8b86 --- /dev/null +++ b/apps/backend/test/vault.test.ts @@ -0,0 +1,96 @@ +import { describe, it, expect } from "vitest"; +import { base64urlEncode } from "../src/base64url"; +import { decryptSecret, encryptSecret, EncryptedKvVault } from "../src/vault"; +import type { VaultBinding } from "../src/types"; +import { TEST_VAULT_MASTER_KEY } from "./tokens"; + +const ENVELOPE_RE = /^v1\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+$/; + +function randomKey(): string { + return base64urlEncode(crypto.getRandomValues(new Uint8Array(32))); +} + +describe("vault envelope encryption", () => { + it("round-trips a secret through the v1 envelope format", async () => { + // #given a plaintext sealed with the master key + const envelope = await encryptSecret(TEST_VAULT_MASTER_KEY, "hunter2"); + + // #then the envelope is the pinned wire format (scripts/vault-put.mjs + // mirrors it in Node; this test is what keeps the two in lockstep) + // and carries no plaintext + expect(envelope).toMatch(ENVELOPE_RE); + expect(envelope).not.toContain("hunter2"); + + // #when it is decrypted with the same key + // #then the original plaintext comes back + await expect(decryptSecret(TEST_VAULT_MASTER_KEY, envelope)).resolves.toBe("hunter2"); + }); + + it("seals the same plaintext to different envelopes (fresh IV per value)", async () => { + const first = await encryptSecret(TEST_VAULT_MASTER_KEY, "hunter2"); + const second = await encryptSecret(TEST_VAULT_MASTER_KEY, "hunter2"); + expect(first).not.toBe(second); + }); + + it("fails closed on the wrong key, with a scrubbed message", async () => { + // #given an envelope sealed under one key and read under another + const envelope = await encryptSecret(TEST_VAULT_MASTER_KEY, "hunter2"); + const failure = decryptSecret(randomKey(), envelope); + + // #then GCM authentication refuses it and the message names only the + // failure class - no plaintext, no envelope material + await expect(failure).rejects.toThrow("vault envelope failed to decrypt"); + await expect(failure).rejects.not.toThrow(/hunter2/); + }); + + it("fails closed on a tampered ciphertext", async () => { + const envelope = await encryptSecret(TEST_VAULT_MASTER_KEY, "hunter2"); + const [version, iv, ct] = envelope.split(".") as [string, string, string]; + const flipped = ct.startsWith("A") ? `B${ct.slice(1)}` : `A${ct.slice(1)}`; + + await expect( + decryptSecret(TEST_VAULT_MASTER_KEY, `${version}.${iv}.${flipped}`), + ).rejects.toThrow("vault envelope failed to decrypt"); + }); + + it("rejects values that are not v1 envelopes - a legacy plaintext KV value can never be served", async () => { + for (const notAnEnvelope of ["hunter2", "v2.a.b", "v1.onlyone", "v1..", ""]) { + await expect(decryptSecret(TEST_VAULT_MASTER_KEY, notAnEnvelope)).rejects.toThrow( + "vault value is not a recognized envelope", + ); + } + }); + + it("rejects a master key of the wrong length before touching the value", async () => { + const short = base64urlEncode(crypto.getRandomValues(new Uint8Array(16))); + await expect(encryptSecret(short, "x")).rejects.toThrow("vault master key must be 32 bytes"); + }); +}); + +describe("EncryptedKvVault", () => { + function storeWith(entries: Record): VaultBinding { + return { get: async (ref) => entries[ref] ?? null }; + } + + it("returns the decrypted plaintext for a present envelope", async () => { + const envelope = await encryptSecret(TEST_VAULT_MASTER_KEY, "hunter2"); + const vault = new EncryptedKvVault(storeWith({ "vault://pw": envelope }), TEST_VAULT_MASTER_KEY); + + await expect(vault.get("vault://pw")).resolves.toBe("hunter2"); + }); + + it("passes through null for an absent secretRef", async () => { + const vault = new EncryptedKvVault(storeWith({}), TEST_VAULT_MASTER_KEY); + await expect(vault.get("vault://missing")).resolves.toBeNull(); + }); + + it("fails closed (throws) rather than serving a value it cannot authenticate", async () => { + const vault = new EncryptedKvVault( + storeWith({ "vault://legacy": "raw-plaintext-from-before-envelopes" }), + TEST_VAULT_MASTER_KEY, + ); + await expect(vault.get("vault://legacy")).rejects.toThrow( + "vault value is not a recognized envelope", + ); + }); +}); diff --git a/apps/backend/vitest.config.ts b/apps/backend/vitest.config.ts index 415e9c2..1976cb0 100644 --- a/apps/backend/vitest.config.ts +++ b/apps/backend/vitest.config.ts @@ -1,6 +1,6 @@ import { cloudflareTest } from "@cloudflare/vitest-pool-workers"; import { defineConfig } from "vitest/config"; -import { CALLER_TOKENS, EXTENSION_TOKENS } from "./test/tokens"; +import { CALLER_TOKENS, EXTENSION_TOKENS, TEST_VAULT_MASTER_KEY } from "./test/tokens"; export default defineConfig({ plugins: [ @@ -16,6 +16,7 @@ export default defineConfig({ AUTH_HMAC_SECRET: "test-hmac-secret-do-not-use-in-prod", CALLER_TOKENS: JSON.stringify(CALLER_TOKENS), EXTENSION_TOKENS: JSON.stringify(EXTENSION_TOKENS), + VAULT_MASTER_KEY: TEST_VAULT_MASTER_KEY, }, }, }), diff --git a/apps/backend/wrangler.jsonc b/apps/backend/wrangler.jsonc index 0ad9272..48fdbc4 100644 --- a/apps/backend/wrangler.jsonc +++ b/apps/backend/wrangler.jsonc @@ -21,28 +21,34 @@ ], "kv_namespaces": [ { - // Credential vault: fill_secret's secretRef -> plaintext, resolved - // service-side by secrets.ts (M-007) and never logged or written to DO - // state (DL-004). A KV namespace gives the dynamic get-by-key lookup - // an arbitrary per-fill secretRef needs; CF's Secrets / Secrets Store - // bindings are static (one binding per fixed secret name) and cannot - // address a runtime-chosen key. A per-tenant external KMS is a - // possible future swap behind the same Env.VAULT seam (src/types.ts). + // Credential vault ciphertext store: fill_secret's secretRef -> + // AES-256-GCM envelope (src/vault.ts; seed with scripts/vault-put.mjs + // - never `wrangler kv key put` a raw plaintext). Resolution is + // service-side (secrets.ts via vault.ts) and the plaintext is never + // logged or written to DO state (DL-004). A KV namespace gives the + // dynamic get-by-key lookup an arbitrary per-fill secretRef needs; + // CF's Secrets / Secrets Store bindings are static (one binding per + // fixed secret name) and cannot address a runtime-chosen key. A + // per-tenant external KMS remains a possible future swap behind the + // same Env.VAULT seam (src/types.ts). "binding": "VAULT", - // Replace with a real namespace id (`wrangler kv namespace create - // VAULT`) before any dev/deploy that touches this binding. - "id": "REPLACE_WITH_VAULT_KV_NAMESPACE_ID" + // Created 2026-07-17 via `wrangler kv namespace create VAULT` on the + // production account (see README "Deploy"). Namespace ids are not + // secrets; the values inside are ciphertext envelopes regardless. + "id": "b6168345442644e3ac24ef4c43dd7f1e" } ], "secrets": { // AUTH_HMAC_SECRET signs/verifies server-minted sessionIds so // scopeSession can verify tenant ownership statelessly (M-006, DL-008). + // VAULT_MASTER_KEY (base64url, 32 bytes) envelope-encrypts every vault + // value; without it a KV read-back at rest yields only ciphertext. // CALLER_TOKENS / EXTENSION_TOKENS must be listed as well: when `secrets` // is declared, wrangler loads ONLY the listed keys from .dev.vars and // excludes the rest (no "optional" tier exists), which silently breaks // dev auth. They are operationally required anyway - without them the // service 401s every caller and rejects the extension WS. - "required": ["AUTH_HMAC_SECRET", "CALLER_TOKENS", "EXTENSION_TOKENS"] + "required": ["AUTH_HMAC_SECRET", "CALLER_TOKENS", "EXTENSION_TOKENS", "VAULT_MASTER_KEY"] }, "observability": { "enabled": true diff --git a/apps/cdp-spike/README.md b/apps/cdp-spike/README.md index becc77c..5d77b3d 100644 --- a/apps/cdp-spike/README.md +++ b/apps/cdp-spike/README.md @@ -37,3 +37,33 @@ The probe reports OK / FAIL for each command the plan depends on: The **A11y tree sample** button shows the pruned `{ref, role, name}` shape the backend would feed to the LLM — eyeball it on a real page to sanity-check that the actionable elements are captured with usable names. + +## OOPIF probe (M5 follow-up — ~5 minutes, attended) + +Answers the sub-question M0 deferred: does `Target.setAutoAttach{flatten:true}` +work under `chrome.debugger` (it is a *different* `Target` method than the +blocked `getTargets`), announcing cross-origin iframes as attached targets that +session-scoped `sendCommand({tabId, sessionId}, …)` (Chrome 125+) can drive? +That mechanism is everything cross-frame targeting in `apps/extension` would +build on — nothing is implemented there until this probe is green **and** a +consumer actually needs cross-origin-iframe targeting. + +1. Load/reload the unpacked extension (the probe shipped after M0; reload picks it up). +2. Open `apps/cdp-spike/oopif-test.html` **as a file** (drag it into a tab). Its + `https://example.org` iframe is cross-site, so site isolation makes it an OOPIF. +3. Side panel → **Attach to active tab** → **OOPIF probe**. + +**Reading the outcome:** + +- `OOPIF auto-attach WORKS …` — the deferred unknown is retired. An iframe + target attached, and both `Runtime.evaluate` and `Accessibility.getFullAXTree` + ran inside it via its `sessionId` (the evaluate row should print the + *iframe's* URL, and the iframe's a11y rows must NOT contain the top frame's + "top-frame button"). Record the result in `docs/technical-plan.md` ("M0 + findings" — OOPIF paragraph) before building on it. +- `iframe target(s) attached but session-scoped commands FAILED` — auto-attach + is permitted but session routing is not; cross-frame work would need a + per-frame `chrome.debugger.attach` fallback instead. +- `no OOPIF targets announced` — either the test page isn't in the attached + tab, or `setAutoAttach` is restricted like `getTargets`; the FAIL row for + `Target.setAutoAttach` distinguishes the two. diff --git a/apps/cdp-spike/background.js b/apps/cdp-spike/background.js index f8eccb6..beece32 100644 --- a/apps/cdp-spike/background.js +++ b/apps/cdp-spike/background.js @@ -124,7 +124,93 @@ async function probe() { return { summary: `${passed}/${results.length} commands OK`, results }; } -const HANDLERS = { attach, detach, a11y: getA11y, screenshot, probe }; +// ── OOPIF probe (M5 follow-up to the M0 findings) ──────────────────────────── +// Answers the sub-question M0 deferred: does Target.setAutoAttach{flatten:true} +// work under chrome.debugger, delivering attachedToTarget events with +// sessionIds that session-scoped sendCommand (Chrome 125+) can drive? That is +// the entire mechanism cross-origin-iframe (OOPIF) targeting would build on. + +/** Target.attachedToTarget params observed since the last OOPIF probe reset. */ +let attachedTargetEvents = []; + +chrome.debugger.onEvent.addListener((source, method, params) => { + if (!attached || source.tabId !== attached.tabId) return; + if (method === "Target.attachedToTarget") attachedTargetEvents.push(params); +}); + +async function probeOopif() { + if (!attached) throw new Error("Not attached — click Attach first"); + const tabId = attached.tabId; + const results = []; + const step = async (label, fn) => { + try { + const detail = await fn(); + results.push({ label, ok: true, detail: detail ?? "" }); + } catch (e) { + results.push({ label, ok: false, detail: String((e && e.message) || e) }); + } + }; + const inSession = (sessionId, method, params) => + chrome.debugger.sendCommand({ tabId, sessionId }, method, params); + + attachedTargetEvents = []; + await step("Target.setAutoAttach {flatten:true}", () => + cdp("Target.setAutoAttach", { autoAttach: true, waitForDebuggerOnStart: false, flatten: true }), + ); + // Auto-attach announces existing OOPIFs asynchronously; give it a beat. + await new Promise((resolve) => setTimeout(resolve, 1500)); + + const frames = attachedTargetEvents.filter((p) => p.targetInfo && p.targetInfo.type === "iframe"); + results.push({ + label: "Target.attachedToTarget events", + ok: attachedTargetEvents.length > 0, + detail: + attachedTargetEvents + .map((p) => `${p.targetInfo.type} ${String(p.targetInfo.url || "").slice(0, 60)}`) + .join(" | ") || "none received", + }); + + // Drive each auto-attached target through ITS OWN session: proves the + // sessionId routing the real driver (apps/extension) would need. + for (const p of attachedTargetEvents) { + const sessionId = p.sessionId; + const tag = `[${String(p.targetInfo.url || p.targetInfo.type).slice(0, 40)}]`; + await step(`${tag} Runtime.evaluate via sessionId`, async () => { + await inSession(sessionId, "Runtime.enable"); + const { result } = await inSession(sessionId, "Runtime.evaluate", { + expression: "location.href", + returnByValue: true, + }); + return String(result.value).slice(0, 70); + }); + await step(`${tag} Accessibility.getFullAXTree via sessionId`, async () => { + await inSession(sessionId, "Accessibility.enable"); + const { nodes } = await inSession(sessionId, "Accessibility.getFullAXTree"); + const pruned = pruneAxTree(nodes); + const sample = pruned + .slice(0, 3) + .map((n) => `${n.role}:${n.name}`) + .join(", "); + return `${nodes.length} nodes, ${pruned.length} actionable${sample ? ` (${sample})` : ""}`; + }); + } + + await step("Target.setAutoAttach off (cleanup)", () => + cdp("Target.setAutoAttach", { autoAttach: false, waitForDebuggerOnStart: false, flatten: true }), + ); + + const sessionSteps = results.filter((r) => r.label.includes("via sessionId")); + const sessionsOk = sessionSteps.length > 0 && sessionSteps.every((r) => r.ok); + const summary = + frames.length > 0 && sessionsOk + ? `OOPIF auto-attach WORKS: ${frames.length} iframe target(s) attached and driven via session-scoped commands` + : frames.length > 0 + ? "iframe target(s) attached but session-scoped commands FAILED — see rows above" + : "no OOPIF targets announced — is oopif-test.html (cross-origin iframe) open in the attached tab?"; + return { summary, results }; +} + +const HANDLERS = { attach, detach, a11y: getA11y, screenshot, probe, oopif: probeOopif }; chrome.runtime.onMessage.addListener((msg, _sender, sendResponse) => { const handler = msg && HANDLERS[msg.cmd]; diff --git a/apps/cdp-spike/manifest.json b/apps/cdp-spike/manifest.json index 26f22b6..2827489 100644 --- a/apps/cdp-spike/manifest.json +++ b/apps/cdp-spike/manifest.json @@ -3,7 +3,7 @@ "name": "CDP Spike (M0) — understudy", "version": "0.0.1", "description": "Throwaway M0 harness: verifies the chrome.debugger CDP command surface the plan depends on.", - "minimum_chrome_version": "114", + "minimum_chrome_version": "125", "permissions": ["debugger", "tabs", "activeTab", "storage", "sidePanel"], "host_permissions": [""], "background": { "service_worker": "background.js" }, diff --git a/apps/cdp-spike/oopif-test.html b/apps/cdp-spike/oopif-test.html new file mode 100644 index 0000000..80ece48 --- /dev/null +++ b/apps/cdp-spike/oopif-test.html @@ -0,0 +1,25 @@ + + + + + understudy OOPIF probe page + + + +

OOPIF probe page (top frame)

+

+ Open this file directly (file://) in the tab you attach the CDP spike to, + then click OOPIF probe in the side panel. The iframe + below is https://example.org — a different site, so Chrome's + site isolation renders it in its own process (an out-of-process iframe). + If Target.setAutoAttach works under + chrome.debugger, the probe reports an attached iframe target + and reads its accessibility tree through a session-scoped command. +

+ +

+ + diff --git a/apps/cdp-spike/sidepanel.html b/apps/cdp-spike/sidepanel.html index be8813a..01b1122 100644 --- a/apps/cdp-spike/sidepanel.html +++ b/apps/cdp-spike/sidepanel.html @@ -21,6 +21,7 @@

CDP Spike (M0)

Open a tab you're logged into, then Attach and Probe.

+ diff --git a/apps/cdp-spike/sidepanel.js b/apps/cdp-spike/sidepanel.js index 4df9c00..ccb97be 100644 --- a/apps/cdp-spike/sidepanel.js +++ b/apps/cdp-spike/sidepanel.js @@ -34,6 +34,13 @@ bind("probe", "probe", (d) => { } }); +bind("oopif", "oopif", (d) => { + log(`— ${d.summary} —`, d.summary.includes("WORKS") ? "ok" : undefined); + for (const r of d.results) { + log(`${r.ok ? "OK " : "FAIL"} ${r.label}${r.detail ? " — " + r.detail : ""}`, r.ok ? "ok" : "fail"); + } +}); + bind("a11y", "a11y", (d) => log(`a11y: ${d.actionable}/${d.total} actionable\n` + JSON.stringify(d.sample, null, 2)), ); diff --git a/apps/extension/CLAUDE.md b/apps/extension/CLAUDE.md index 0319046..418aa8f 100644 --- a/apps/extension/CLAUDE.md +++ b/apps/extension/CLAUDE.md @@ -28,7 +28,9 @@ WXT + React MV3 extension driving protocol Commands into a real Chromium tab via | `src/core/ws-client.ts` | `ReconnectingWs` — WebSocket with backoff reconnect and self-driven pong heartbeat | Changing reconnect/backoff/heartbeat behavior | | `src/core/router.ts` | `routeCommand` — dispatches a parsed `Command` to a `CdpSession` executor or tab handler | Adding a new protocol Command type | | `src/core/router.test.ts` | Unit tests for `routeCommand` (one Event per Command, error paths) | Verifying command routing | -| `src/entrypoints/background.ts` | MV3 service worker: WS lifecycle, CDP session ownership, wake-time reattachment, alarm/heartbeat keepalive, panel `Port` host | Debugging SW eviction/reconnect, attach/detach flow, keepalive | +| `src/core/dedupe.ts` | `WriteDedupe` — `claim()` (execute / replay a completed write / drop an in-flight duplicate) + `remember`/`release`/`clear`; idempotent-retry contract, storage.session-mirrored, cap 100 | Changing write replay/dedupe/in-flight behavior | +| `src/core/dedupe.test.ts` | Unit tests for `WriteDedupe` (claim decisions, concurrent-claim atomicity, in-flight drop, cap, eviction survival, clear-on-session-change) | Verifying dedupe behavior | +| `src/entrypoints/background.ts` | MV3 service worker: WS lifecycle, CDP session ownership, write dedupe, wake-time reattachment, alarm/heartbeat keepalive, panel `Port` host | Debugging SW eviction/reconnect, attach/detach flow, keepalive | | `src/entrypoints/sidepanel/index.html` | Sidepanel HTML entry | Changing the sidepanel document shell | | `src/entrypoints/sidepanel/main.tsx` | React root mount for the sidepanel | Changing sidepanel bootstrap | | `src/entrypoints/sidepanel/App.tsx` | Sidepanel UI: WS status, WS URL field, Attach/Detach, live log, `Port` reconnect on SW eviction | Changing sidepanel UI or panel↔SW messaging | diff --git a/apps/extension/src/core/dedupe.test.ts b/apps/extension/src/core/dedupe.test.ts new file mode 100644 index 0000000..b7b5cd3 --- /dev/null +++ b/apps/extension/src/core/dedupe.test.ts @@ -0,0 +1,176 @@ +import { describe, it, expect } from "vitest"; +import type { Command, Event } from "@understudy/protocol"; +import { WriteDedupe, type SessionStorageArea } from "./dedupe"; + +function fakeStorage(): SessionStorageArea & { data: Map } { + const data = new Map(); + return { + data, + get: async (key) => (data.has(key) ? { [key]: data.get(key) } : {}), + set: async (items) => { + for (const [key, value] of Object.entries(items)) data.set(key, value); + }, + remove: async (key) => { + data.delete(key); + }, + }; +} + +const CLICK: Command = { type: "click", commandId: "ik_case1:step1:click", ref: "s1e1" }; +const CLICK_RESULT: Event = { type: "action_result", commandId: "ik_case1:step1:click", ok: true }; + +describe("WriteDedupe.claim", () => { + it("returns execute for a fresh write and marks it in-flight", async () => { + // #given a fresh dedupe + const dedupe = new WriteDedupe(fakeStorage()); + + // #when a write is claimed + const first = await dedupe.claim(CLICK); + + // #then it executes, and a concurrent duplicate is now dropped (in-flight) + expect(first).toEqual({ kind: "execute" }); + expect(await dedupe.claim(CLICK)).toEqual({ kind: "drop" }); + }); + + it("drops a duplicate while the original is still in-flight (the timeout race)", async () => { + // #given a claimed-but-not-yet-remembered write (still executing) + const dedupe = new WriteDedupe(fakeStorage()); + expect(await dedupe.claim(CLICK)).toEqual({ kind: "execute" }); + + // #when the same commandId is claimed again (service timed out, consumer retried) + // #then the duplicate is dropped - it must NOT execute a second time + expect(await dedupe.claim(CLICK)).toEqual({ kind: "drop" }); + }); + + it("resolves two concurrent claims for one commandId to exactly one execute", async () => { + // #given two claims issued before either resolves (both racing the first hydrate) + const dedupe = new WriteDedupe(fakeStorage()); + const [a, b] = await Promise.all([dedupe.claim(CLICK), dedupe.claim(CLICK)]); + + // #then exactly one is told to execute and the other is dropped + expect([a.kind, b.kind].sort()).toEqual(["drop", "execute"]); + }); + + it("replays a recorded write after it completes", async () => { + // #given a write that executed and was remembered + const dedupe = new WriteDedupe(fakeStorage()); + await dedupe.claim(CLICK); + await dedupe.remember(CLICK, CLICK_RESULT); + + // #when the same commandId arrives again + // #then the recorded Event is replayed, not re-executed + expect(await dedupe.claim(CLICK)).toEqual({ kind: "replay", event: CLICK_RESULT }); + }); + + it("re-executes after a release without a record (a failed/aborted execution)", async () => { + // #given a claimed write whose execution was released without recording + const dedupe = new WriteDedupe(fakeStorage()); + await dedupe.claim(CLICK); + dedupe.release(CLICK); + + // #then the same commandId is claimable again (the execution never happened) + expect(await dedupe.claim(CLICK)).toEqual({ kind: "execute" }); + }); + + it("never tracks reads - they always execute", async () => { + // #given a read command + const dedupe = new WriteDedupe(fakeStorage()); + const read: Command = { type: "get_tabs", commandId: "r1" }; + + // #then it executes, and even a repeat executes (reads re-run freely) + expect(await dedupe.claim(read)).toEqual({ kind: "execute" }); + await dedupe.remember(read, { type: "tabs_result", commandId: "r1", tabs: [] }); + expect(await dedupe.claim(read)).toEqual({ kind: "execute" }); + }); + + it("tracks scroll and switch_tab as writes (protocol reclassified them)", async () => { + // #given a scroll claimed and completed + const dedupe = new WriteDedupe(fakeStorage()); + const scroll: Command = { type: "scroll", commandId: "ik_s:scroll", dy: 120 }; + const scrollResult: Event = { type: "action_result", commandId: "ik_s:scroll", ok: true }; + await dedupe.claim(scroll); + await dedupe.remember(scroll, scrollResult); + + // #then a retry replays instead of double-scrolling (relative dy) + expect(await dedupe.claim(scroll)).toEqual({ kind: "replay", event: scrollResult }); + }); +}); + +describe("WriteDedupe persistence + lifecycle", () => { + it("survives a service-worker eviction via storage.session", async () => { + // #given a result recorded by one SW life + const storage = fakeStorage(); + const first = new WriteDedupe(storage); + await first.claim(CLICK); + await first.remember(CLICK, CLICK_RESULT); + + // #when the SW is evicted and a fresh instance hydrates from storage + const revived = new WriteDedupe(storage); + + // #then the recorded result still replays + expect(await revived.claim(CLICK)).toEqual({ kind: "replay", event: CLICK_RESULT }); + }); + + it("clear() drops the record so a new session cannot replay the old one's writes", async () => { + // #given a recorded write and a session change + const storage = fakeStorage(); + const dedupe = new WriteDedupe(storage); + await dedupe.claim(CLICK); + await dedupe.remember(CLICK, CLICK_RESULT); + + // #when the WS target session changes + await dedupe.clear(); + + // #then the commandId is fresh again (memory AND the storage mirror cleared) + expect(await dedupe.claim(CLICK)).toEqual({ kind: "execute" }); + expect(await new WriteDedupe(storage).claim(CLICK)).toEqual({ kind: "execute" }); + }); + + it("does NOT record a straggler whose session changed (clear) mid-execution", async () => { + // #given a write claimed under the old session, still executing when the + // WS target session changes (setWsUrl calls clear()) + const dedupe = new WriteDedupe(fakeStorage()); + expect(await dedupe.claim(CLICK)).toEqual({ kind: "execute" }); + await dedupe.clear(); + + // #when the old execution finishes and tries to record its result + await dedupe.remember(CLICK, CLICK_RESULT); + + // #then nothing is recorded into the new session's record - a reused + // commandId in the new session executes fresh, never replaying the + // old session's foreign Event + expect(await dedupe.claim(CLICK)).toEqual({ kind: "execute" }); + }); + + it("evicts the oldest entries beyond the cap (100)", async () => { + // #given 101 distinct writes claimed + recorded (cap is 100) + const dedupe = new WriteDedupe(fakeStorage()); + for (let i = 0; i <= 100; i++) { + const cmd: Command = { type: "click", commandId: `w${i}`, ref: "s1e1" }; + await dedupe.claim(cmd); + await dedupe.remember(cmd, { type: "action_result", commandId: `w${i}`, ok: true }); + } + + // #then the oldest fell out and the newest remains + expect(await dedupe.claim({ type: "click", commandId: "w0", ref: "s1e1" })).toEqual({ + kind: "execute", + }); + expect(await dedupe.claim({ type: "click", commandId: "w100", ref: "s1e1" })).toMatchObject({ + kind: "replay", + }); + }); + + it("treats a broken storage read as an empty record set (fail open to re-execution)", async () => { + // #given storage that throws on read + const dedupe = new WriteDedupe({ + get: async () => { + throw new Error("storage unavailable"); + }, + set: async () => {}, + remove: async () => {}, + }); + + // #then claims execute (re-execution - the pre-dedupe behavior) instead of throwing + expect(await dedupe.claim(CLICK)).toEqual({ kind: "execute" }); + }); +}); diff --git a/apps/extension/src/core/dedupe.ts b/apps/extension/src/core/dedupe.ts new file mode 100644 index 0000000..a4a911b --- /dev/null +++ b/apps/extension/src/core/dedupe.ts @@ -0,0 +1,145 @@ +import { isWriteCommand } from "@understudy/protocol"; +import type { Command, Event } from "@understudy/protocol"; + +// browser.storage.session's surface, narrowed to what WriteDedupe uses so +// tests can inject a plain in-memory fake. +export interface SessionStorageArea { + get(key: string): Promise>; + set(items: Record): Promise; + remove(key: string): Promise; +} + +const STORAGE_KEY = "understudy:completedWrites"; +// Matches the service's SessionState.completedWrites cap so the two replay +// windows are symmetric (a retry protected on one layer is protected on both). +const CAP = 100; + +interface CompletedWrite { + commandId: string; + event: Event; +} + +/** What claim() decided for a command: run it, replay a recorded result, or drop a duplicate. */ +export type ClaimDecision = + | { kind: "execute" } + | { kind: "replay"; event: Event } + | { kind: "drop" }; + +/** + * Extension half of the idempotent-retry contract (the service DO keeps the + * other half). Two retry hazards, both closed here for WRITE commands: + * + * 1. The write already COMPLETED and the service asks for it again (the + * service timed out after this extension responded, so its own + * completedWrites is empty): claim() returns the recorded Event to replay - + * no second execution. + * 2. The write is STILL EXECUTING when the service times out (deleting its + * pending entry) and the consumer retries the same commandId, so the same + * command arrives twice: claim() marks the first in-flight and DROPS the + * second. The one running execution's response resolves the service's + * (retry's) parked promise, so the consumer still gets a real result - once. + * + * Reads are never tracked: re-executing them is free and their results go + * stale by design. + * + * The completed record is memory-resident and mirrored to storage.session + * (survives SW eviction, not a browser restart - matching the session's own + * lifetime); the in-flight set is memory-only (an execution that dies with the + * SW SHOULD re-execute on wake). Persistence is best-effort: a lost mirror + * write degrades a future replay into a re-execution, the pre-dedupe behavior. + */ +export class WriteDedupe { + private entries: CompletedWrite[] | null = null; + // Cached so concurrent claims share ONE storage read; a null entries with a + // live hydration means "a read is in flight, await it". + private hydration: Promise | null = null; + private readonly inFlight = new Set(); + + constructor(private readonly storage: SessionStorageArea) {} + + /** + * Atomically classify a command. Reads always execute (untracked). For a + * write: replay a completed one, drop a still-in-flight duplicate, or mark a + * fresh one in-flight and tell the caller to execute it. The post-hydration + * body runs with no `await`, so two concurrent claims for one commandId + * cannot both pass the in-flight check - exactly one gets `execute`. + */ + async claim(cmd: Command): Promise { + if (!isWriteCommand(cmd)) return { kind: "execute" }; + await this.hydrate(); + const entries = this.entries ?? []; + const recorded = entries.find((entry) => entry.commandId === cmd.commandId); + if (recorded) return { kind: "replay", event: recorded.event }; + if (this.inFlight.has(cmd.commandId)) return { kind: "drop" }; + this.inFlight.add(cmd.commandId); + return { kind: "execute" }; + } + + /** Records a claimed write's Event and clears its in-flight mark; capped FIFO. */ + async remember(cmd: Command, event: Event): Promise { + if (!isWriteCommand(cmd)) return; + // Only record a write still marked in-flight from its own claim(). A + // clear() (WS session change) or release() since the claim means this + // execution spans a session boundary - its result belongs to the OLD + // session and must not enter the new session's record, or a reused + // commandId could replay a foreign session's Event. + if (!this.inFlight.has(cmd.commandId)) return; + await this.hydrate(); + const entries = this.entries ?? []; + const next = [ + ...entries.filter((entry) => entry.commandId !== cmd.commandId), + { commandId: cmd.commandId, event }, + ]; + while (next.length > CAP) next.shift(); + this.entries = next; + this.inFlight.delete(cmd.commandId); + try { + await this.storage.set({ [STORAGE_KEY]: next }); + } catch { + // Memory copy still serves this SW life; see the class doc. + } + } + + /** + * Releases an in-flight mark without recording - for an execution that ended + * without a stored result (an unexpected throw before remember). No-op once + * remember() has already cleared it, so a finally-block release is safe. + */ + release(cmd: Command): void { + this.inFlight.delete(cmd.commandId); + } + + /** + * Drops the whole record - memory, in-flight, and the storage mirror. Called + * when the WS target session changes (the sessionId lives in the WS URL): a + * new session must not inherit the previous one's replay entries, or a reused + * idempotency key could replay the wrong session's Event. + */ + async clear(): Promise { + this.entries = []; + this.inFlight.clear(); + try { + await this.storage.remove(STORAGE_KEY); + } catch { + // Best-effort; the in-memory reset above is authoritative for this SW life. + } + } + + // Storage is read at most once per SW life; the cached promise coalesces + // concurrent first-callers onto that single read. + private hydrate(): Promise { + if (this.entries !== null) return Promise.resolve(); + if (this.hydration === null) { + this.hydration = (async () => { + try { + const stored = await this.storage.get(STORAGE_KEY); + const value = stored[STORAGE_KEY]; + this.entries = Array.isArray(value) ? (value as CompletedWrite[]) : []; + } catch { + this.entries = []; + } + })(); + } + return this.hydration; + } +} diff --git a/apps/extension/src/entrypoints/background.ts b/apps/extension/src/entrypoints/background.ts index 70086b8..7a9774f 100644 --- a/apps/extension/src/entrypoints/background.ts +++ b/apps/extension/src/entrypoints/background.ts @@ -1,6 +1,7 @@ import { safeParseCommand } from "@understudy/protocol"; import type { Browser } from "wxt/browser"; import { ReconnectingWs } from "../core/ws-client"; +import { WriteDedupe } from "../core/dedupe"; import { routeCommand } from "../core/router"; import { CdpSession } from "../driver/cdp"; import { classifyCdpEvent } from "../driver/cdp-events"; @@ -42,6 +43,10 @@ let wsUrlEpoch = 0; let session: CdpSession | null = null; let attachedTitle: string | undefined; +// Write-replay record (idempotent-retry contract); hydrates lazily from +// storage.session, so rebuilding it each wake loses nothing. +const dedupe = new WriteDedupe(browser.storage.session); + const logBuffer: LogEntry[] = []; const ports = new Set(); @@ -153,8 +158,34 @@ async function handleCommand(raw: unknown): Promise { } return; } - const ev = await routeCommand(parsed.data, session); - ws?.send(ev); + + // Idempotent-retry gate for WRITE commands (reads always execute). A retry + // under the same commandId either replays a recorded result, or - if the + // original is still executing (the service timed out and the consumer + // retried) - is dropped so the write runs exactly once; the running + // execution's response resolves the service's parked promise. + const decision = await dedupe.claim(parsed.data); + if (decision.kind === "replay") { + log(`replayed recorded result for duplicate write ${parsed.data.commandId}`); + ws?.send(decision.event); + return; + } + if (decision.kind === "drop") { + log(`dropped duplicate in-flight write ${parsed.data.commandId}; the original execution will answer`); + return; + } + + try { + const ev = await routeCommand(parsed.data, session); + // Record before sending: once the write executed, a crash between the two + // must leave the record (a replayable result), not a re-executable gap. + await dedupe.remember(parsed.data, ev); + ws?.send(ev); + } finally { + // No-op once remember() cleared the mark; guarantees a thrown execution + // still frees its in-flight slot so a later retry can re-run it. + dedupe.release(parsed.data); + } } function extractCommandId(raw: unknown): string | null { @@ -331,6 +362,10 @@ async function persistAttachedTabId(tabId: number): Promise { } async function setWsUrl(url: string): Promise { + // A different WS URL means a different session (the sessionId is in the URL + // path): drop the write-replay record so a reused idempotency key can't + // replay the previous session's result. + if (url !== currentWsUrl) await dedupe.clear(); currentWsUrl = url; wsUrlHydrated = true; wsUrlEpoch += 1; diff --git a/docs/technical-plan.md b/docs/technical-plan.md index e73309e..30c2915 100644 --- a/docs/technical-plan.md +++ b/docs/technical-plan.md @@ -213,8 +213,20 @@ Design notes: - `tabs_result` answers `get_tabs` with a `commandId`-bearing event (added at M2). - **Published runtime exports** (what consumer connectors import): `parseCommand` / `parseEvent` (throwing) and `safeParseCommand` / `safeParseEvent`; `CommandSchema` / `EventSchema`, - `A11yNodeSchema`, `TabInfoSchema`, `SnapshotModeSchema`; `isWriteCommand`; and the types - `Command` / `Event` / `A11yNode` / `TabInfo` / `SnapshotMode`. + `A11yNodeSchema`, `TabInfoSchema`, `SnapshotModeSchema`; `isWriteCommand` + + `WRITE_COMMAND_TYPES` (the single write-classification source of truth downstream layers + derive from — the connector pins its gated union to it at compile time); and the types + `Command` / `Event` / `WriteCommandType` / `A11yNode` / `TabInfo` / `SnapshotMode`. +- **Write retries are idempotent end to end** (M5): the connector derives a write's `commandId` + from the breakwater idempotency key (`ik_`); the service records completed write Events + per session (`completedWrites`, cap 100) and replays a repeated commandId instead of + re-dispatching (409 for a concurrent duplicate still in flight); the extension keeps its own + replay + in-flight record (cap 100, storage.session) covering BOTH the service-timed-out- + after-execution case (replay) and the service-timed-out-mid-execution case (drop the duplicate, + never re-run). This closed the documented "write performed, response unparseable → retry + re-executes" gap. The write class here is the protocol's `WRITE_COMMAND_TYPES` — which now + includes `scroll`/`switch_tab` (user-visible side effects: a dry-run simulates them and a + retry replays them, so a relative-`dy` scroll never double-scrolls). ## Element targeting (a11y `ref` model) — `apps/extension/src/driver/cdp.ts` @@ -274,10 +286,16 @@ feeds its LLM. D7 confirmed end-to-end. only their extension-side *implementation* is pinned here. - v1 scope is a single designated tab/session anyway (see "Out of scope"). -**One sub-question deferred to M2:** cross-origin / out-of-process iframe (OOPIF) traversal via -`Target.setAutoAttach{ flatten: true }` (a *different* `Target` method than the blocked `getTargets`, -generally permitted under `chrome.debugger`). Not exercised by the probe. Add a focused sub-probe at M2 -if cross-frame targeting is needed — the single-frame path is proven. +**One sub-question deferred at M0, probe SHIPPED at M5 (2026-07-17):** cross-origin / +out-of-process iframe (OOPIF) traversal via `Target.setAutoAttach{ flatten: true }` (a *different* +`Target` method than the blocked `getTargets`, generally permitted under `chrome.debugger`). +`apps/cdp-spike` now carries a focused **OOPIF probe** (side-panel button + bundled +`oopif-test.html` cross-origin-iframe page + runbook in its README): it runs `setAutoAttach`, +collects `Target.attachedToTarget` events, and drives each attached target through session-scoped +`sendCommand({tabId, sessionId}, …)` (`Runtime.evaluate` + `Accessibility.getFullAXTree`). The +probe is attended (~5 min, real Chromium ≥125) and has not been run yet; record its verdict here +when it runs. Driver implementation in `apps/extension` stays gated on a green probe AND a +consumer actually needing cross-origin-iframe targeting — the single-frame path is proven. ## Consumer integration — the governed connector (breakwater + flowsafe) @@ -410,9 +428,18 @@ auth, local `fill_secret` shim) and carries stale pre-Topology-1 prose; prefer t flow is wired (`.github/workflows/release.yml`, single-branch master — first push publishes 0.3.0 / 0.1.0 with no changeset needed); publishing waits on the npm `understudy` org + `NPM_TOKEN` secret. The consumer-side Mastra+flowsafe e2e remains open. -- **M5 — Substrate hardening.** Session/tenant isolation verified with two tenants; credential vault + - D-SEC audit invariant; dialog handling; error/timeout paths on every command; session/GIF logging for - audit. (Approval/RBAC/policy live in the consumer, not here.) +- **M5 — Substrate hardening. LARGELY LANDED (2026-07-17, the deferred-items sweep):** + pre-accept WS/HTTP auth at the Worker edge (`onBeforeConnect`/`onBeforeRequest` — unauthorized + upgrades are 401/404 before the DO accepts, in-DO gate kept as defense in depth); credential + vault hardened to AES-256-GCM envelopes over KV under a `VAULT_MASTER_KEY` Worker secret + (`src/vault.ts` + `scripts/vault-put.mjs`; plaintext never at rest, legacy plaintext fails + closed); typed `DispatchOutcome` error taxonomy across the DO RPC boundary (503 not-connected / + 503 resynced / 504 timeout / 409 duplicate; no more RPC-rejection noise); idempotent write + replay (see above); onClose status stamping gated on authorization; **first real deploy** to + `https://understudy-backend.gcharang.workers.dev` with real minted secrets (runbook in + `apps/backend/README.md` "Deploy") — live smoke: health, 401, mint, fail-fast 503, WS-gate 401, + encrypted vault seed all verified. Still open under M5: two-tenant isolation e2e, dialog + handling breadth, session/GIF audit logging. - **M6 — Ops.** Rate/quotas at the service edge, observability, unattended-session seam scoping. ## Verification diff --git a/packages/connector/README.md b/packages/connector/README.md index c2a1cd7..ed79ccb 100644 --- a/packages/connector/README.md +++ b/packages/connector/README.md @@ -21,10 +21,11 @@ limits, audit). | `act` | `browser.act` | write — approval-gated | `click`, `type`, `navigate`, `key`, `scroll`, `switch_tab` | | `fillCredential` | `browser.fill_credential` | vaulted write — approval-gated | `fill_secret` | -`scroll` and `switch_tab` are non-writes in the protocol's own -`isWriteCommand` classification, but this package deliberately routes them -through the gated `act` connector: both change what the user's real browser -shows, and the conservative direction is to gate them. +`scroll` and `switch_tab` route through the gated `act` connector because both +change what the user's real browser shows. The protocol classifies them as +writes for exactly this reason (`isWriteCommand` returns `true`), so `act` +gates precisely the protocol's write class minus `fill_secret` — no divergence +to keep in sync. The protocol's `resolve_ref` command is deliberately unreachable from here — it is an internal service↔extension probe. Dry-run intent is expressed via the @@ -37,7 +38,10 @@ pnpm add @understudy/connector @understudy/protocol @proofoftech/breakwater @mas ``` `@proofoftech/breakwater` and `@mastra/core` are peer dependencies — the -connectors must share the consumer's Mastra runtime. +connectors must share the consumer's Mastra runtime. The package declares +`engines.node >= 22`, mirroring its breakwater peer (unlike +`@understudy/protocol`, which is runtime-agnostic and deliberately declares +no engines constraint). ## Quick start @@ -109,9 +113,18 @@ plain vars, in production. - **Idempotency.** Writes require a caller-supplied key (`callBrowserWrite`'s last argument). Replays return the stored result without re-executing, so DO hibernation/retry cannot double-submit after a - *known* outcome. The one inherent gap: if the service performed the write - but the response never parsed, the key stays retryable and a retry - re-executes — ack-based at-most-once, conservative in the retry direction. + *known* outcome. The once-inherent gap — write performed but the response + lost/unparseable, so the key stays retryable and the retry re-executes — + is closed end to end: the connector derives the wire `commandId` from the + idempotency key (`ik_`), the understudy service replays a recorded + write Event for a repeated commandId instead of re-dispatching (and refuses + a still-in-flight duplicate outright), and the extension both replays its + own recorded result if the service timed out *after* it responded and drops + a duplicate that arrives while it is *still* executing — so the write runs + exactly once even under a timeout race. Remaining boundary: the replay + records are bounded (100 writes each, service- and extension-side) and + scoped to the session — a retry beyond that bound degrades to the old + conservative re-execution. - **Dry-run.** `callBrowserDryRun` runs the connector's simulation: understudy checks the `ref` still resolves (a pure ref-map lookup — it never re-mints refs, so outstanding refs survive the simulation) and returns a diff --git a/packages/connector/package.json b/packages/connector/package.json index 862a1be..d57919e 100644 --- a/packages/connector/package.json +++ b/packages/connector/package.json @@ -14,6 +14,7 @@ "files": [ "dist", "README.md", + "CHANGELOG.md", "LICENSE" ], "sideEffects": false, diff --git a/packages/connector/src/index.test.ts b/packages/connector/src/index.test.ts index acf356a..6ab6cb3 100644 --- a/packages/connector/src/index.test.ts +++ b/packages/connector/src/index.test.ts @@ -6,11 +6,14 @@ import { InMemoryIdempotencyStore, InMemoryRateLimitStore, } from "@proofoftech/breakwater/connector-sdk"; +import { WRITE_COMMAND_TYPES, type WriteCommandType } from "@understudy/protocol"; import { afterEach, describe, expect, it, vi } from "vitest"; import { + actInput, BROWSER_ACT_CONNECTOR, BROWSER_FILL_CREDENTIAL_CONNECTOR, BROWSER_WRITE_CONNECTOR_IDS, + type ActInput, type ActOutput, type ConnectorStores, type FillCredentialOutput, @@ -123,9 +126,12 @@ describe("act", () => { expect(headers.authorization).toBe("Bearer caller-token-1"); expect(body).toMatchObject({ dryRun: false, - command: { type: "click", ref: "s1e2" }, + // The commandId is DERIVED from the idempotency key ("ik_" + key), not + // random: a retry after a lost/unparseable response re-runs execute() + // under the same key, and only a stable id lets the service replay the + // recorded Event instead of executing the write twice. + command: { type: "click", ref: "s1e2", commandId: "ik_case1:step1:click" }, }); - expect((body as { command: { commandId: unknown } }).command.commandId).toBeTypeOf("string"); }); it("dry-run needs no grant, sends dryRun:true, and marks the output simulated", async () => { @@ -141,6 +147,10 @@ describe("act", () => { expect(preview.ok).toBe(true); const { body } = sentRequest(fetchSpy); expect(body).toMatchObject({ dryRun: true, command: { type: "click" } }); + // No idempotency key on a simulation - the commandId stays random, so a + // dry-run can never collide with (or replay) the real write's record. + const dryId = (body as { command: { commandId: string } }).command.commandId; + expect(dryId.startsWith("ik_")).toBe(false); }); }); @@ -363,3 +373,49 @@ describe("service bridge hardening", () => { ); }); }); + +describe("write-classification sync with @understudy/protocol", () => { + // Compile-time pin: act gates exactly the protocol write class minus + // fill_secret (which fill_credential carries). Since the protocol + // reclassified scroll/switch_tab as writes, that is the whole relationship - + // no extras. If WRITE_COMMAND_TYPES changes upstream, this assignment stops + // compiling until actInput is deliberately revisited. + type GatedActionType = ActInput["action"]["type"]; + type ExpectedGatedActionType = Exclude; + type MutuallyAssignable = [A] extends [B] ? ([B] extends [A] ? true : false) : false; + const actGateMatchesProtocol: MutuallyAssignable = true; + + it("act's runtime schema gates the same set the types promise", () => { + // #given the literal action types the act schema actually accepts + const gated = new Set(actInput.shape.action.options.map((option) => option.shape.type.value)); + + // #then they are exactly protocol WRITE_COMMAND_TYPES minus fill_secret + const expected = new Set( + WRITE_COMMAND_TYPES.filter((type) => type !== "fill_secret"), + ); + expect(gated).toEqual(expected); + expect(actGateMatchesProtocol).toBe(true); + }); + + it("fill_credential carries the remaining protocol write with a key-derived commandId", async () => { + // #given a granted fill_credential call under an idempotency key + const fetchSpy = vi.fn().mockResolvedValue( + eventResponse({ type: "action_result", commandId: "c1", ok: true }), + ); + vi.stubGlobal("fetch", fetchSpy); + const { fillCredential } = createBrowserConnectors(ENV, stores()); + + // #when it executes + await callBrowserWrite( + fillCredential, + { sessionId: "s-1", ref: "s1e9", secretRef: "vault://x" }, + grantFor(BROWSER_FILL_CREDENTIAL_CONNECTOR), + "case1:login:fill", + ); + + // #then the wire carries fill_secret under the derived stable id + expect(sentRequest(fetchSpy).body).toMatchObject({ + command: { type: "fill_secret", commandId: "ik_case1:login:fill" }, + }); + }); +}); diff --git a/packages/connector/src/index.ts b/packages/connector/src/index.ts index 7a1ba17..18b890a 100644 --- a/packages/connector/src/index.ts +++ b/packages/connector/src/index.ts @@ -144,10 +144,12 @@ export type ObserveOutput = z.infer; // agent boundary can additionally reject high-entropy strings leaking into // type.text. // -// Stricter than the protocol's isWriteCommand on purpose: scroll and -// switch_tab are protocol non-writes, but both change what the user's real -// browser shows, so they sit behind the approval gate here. Keep this union -// in sync with the protocol's WRITE_COMMANDS when it changes. +// This union is exactly the protocol's write class minus fill_secret (which +// fill_credential carries): click/type/navigate/key/scroll/switch_tab. Since +// the protocol reclassified scroll/switch_tab as writes (they are user-visible +// side effects), there is no longer any divergence to reconcile - the +// relationship is pinned at compile time AND runtime in index.test.ts, so a +// protocol classification change breaks the build instead of drifting. export const actInput = z.object({ sessionId: z.string(), action: z.discriminatedUnion("type", [ @@ -225,12 +227,28 @@ async function callUnderstudy( return parseEvent(await res.json()); } -// crypto.randomUUID() is non-deterministic, which is safe here: breakwater's -// idempotency layer replays a completed key WITHOUT re-running execute(), so -// a command is built at most once per logical action even across DO -// hibernation/retry. -function toCommand(fields: object): Command { - return parseCommand({ ...fields, commandId: crypto.randomUUID() }); +// Writes pass a stable commandId derived from the breakwater idempotency +// key; everything else gets a random UUID. breakwater replays a COMPLETED +// key without re-running execute(), but a failed attempt (response lost or +// unparseable after the service performed the write) releases the key and +// the retry re-runs execute() - under a random id the service could not +// recognize the retry, and the write would run twice. Under the derived id +// the service replays the recorded Event instead (its completedWrites +// cache), making write retries exactly-once end to end. +function toCommand(fields: object, commandId?: string): Command { + return parseCommand({ ...fields, commandId: commandId ?? crypto.randomUUID() }); +} + +// The wrapper hands config.execute the same ToolExecutionContext it read the +// idempotency key from, so the derived commandId and breakwater's replay +// store always key off the same value. Structural access: Mastra's context +// type does not surface requestContext.get on every version. +function idempotencyCommandId(ctx: ToolExecutionContext): string | undefined { + const requestContext = ( + ctx as { requestContext?: { get?: (key: string) => unknown } } + ).requestContext; + const key = requestContext?.get?.(IDEMPOTENCY_KEY_CONTEXT_KEY); + return typeof key === "string" && key.length > 0 ? `ik_${key}` : undefined; } // -- Connectors ------------------------------------------------------------------ @@ -310,8 +328,9 @@ export function createBrowserConnectors( dryRun: true, rateLimit: "60/min", }, - execute: async (input, _ctx, runtime): Promise => { - const ev = await callUnderstudy(runtime, env, input.sessionId, toCommand(input.action)); + execute: async (input, ctx, runtime): Promise => { + const command = toCommand(input.action, idempotencyCommandId(ctx)); + const ev = await callUnderstudy(runtime, env, input.sessionId, command); if (ev.type === "action_result") return { ok: ev.ok, url: ev.url, error: ev.error }; throw new Error(`act: unexpected event '${ev.type}'`); }, @@ -350,13 +369,16 @@ export function createBrowserConnectors( dryRun: true, rateLimit: "30/min", }, - execute: async (input, _ctx, runtime): Promise => { - const command = toCommand({ - type: "fill_secret", - ref: input.ref, - secretRef: input.secretRef, // opaque handle; resolved service-side - ...(input.submit !== undefined ? { submit: input.submit } : {}), - }); + execute: async (input, ctx, runtime): Promise => { + const command = toCommand( + { + type: "fill_secret", + ref: input.ref, + secretRef: input.secretRef, // opaque handle; resolved service-side + ...(input.submit !== undefined ? { submit: input.submit } : {}), + }, + idempotencyCommandId(ctx), + ); const ev = await callUnderstudy(runtime, env, input.sessionId, command); if (ev.type === "action_result") return { ok: ev.ok, filled: ev.ok, error: ev.error }; throw new Error(`fillCredential: unexpected event '${ev.type}'`); diff --git a/packages/protocol/README.md b/packages/protocol/README.md index d2513e5..8c1fd93 100644 --- a/packages/protocol/README.md +++ b/packages/protocol/README.md @@ -30,13 +30,22 @@ import { // parsers parseCommand, parseEvent, safeParseCommand, safeParseEvent, // classification - isWriteCommand, + isWriteCommand, WRITE_COMMAND_TYPES, // types - type Command, type CommandType, type Event, + type Command, type CommandType, type Event, type WriteCommandType, type A11yNode, type TabInfo, type SnapshotMode, } from "@understudy/protocol"; ``` +### Runtime support + +Pure zod-4 schemas — no platform APIs. Runs anywhere zod does: Workers, +browsers (the extension bundles it), and Node. Deliberately no `engines` +field: constraining Node would only misstate that portability +([`@understudy/connector`](https://github.com/ProofOfTechOrg/understudy/tree/master/packages/connector) +declares `node >= 22`, mirroring its breakwater peer — that constraint lives +there, where it is real). + ## Commands (consumer/service → extension) | type | fields | class | @@ -44,17 +53,23 @@ import { | `snapshot` | `mode: "a11y" \| "dom" \| "screenshot"`, `tabId?` | read | | `get_tabs` | — | read | | `wait` | `for: "load" \| "idle" \| "ms"`, `value?` | read | -| `scroll` | `ref?`, `dy` | read | | `resolve_ref` | `ref` | read (internal — see below) | -| `switch_tab` | `tabId` | read | | `click` | `ref` | **write** | | `type` | `ref`, `text`, `submit?` | **write** | | `key` | `keys`, `ref?` | **write** | | `navigate` | `url`, `tabId?` | **write** | +| `scroll` | `ref?`, `dy` | **write** | +| `switch_tab` | `tabId` | **write** | | `fill_secret` | `ref`, `secretRef`, `submit?` | **write** (vaulted) | -`isWriteCommand` returns `true` for the write class. Two commands deserve a -warning: +`isWriteCommand` / `WRITE_COMMAND_TYPES` classify the write class in the +**operational** sense this system enforces: a command with a user-visible side +effect, which must be gated on approval (D8), simulated (never performed) on a +`dryRun`, and replayed (never repeated) on an idempotent retry. `scroll` and +`switch_tab` are writes here even though they don't mutate the DOM — both change +what the user's browser shows, so a dry-run must not perform them and a +lost-response retry of `scroll` (a *relative* `dy`) must not double-scroll. Two +commands deserve a warning: - **`fill_secret`** carries an opaque `secretRef` (e.g. `vault://…`), never a plaintext secret. The understudy *service* resolves it against the vault and @@ -94,6 +109,11 @@ generation that produced them; a stale ref returns ## Versioning +- **0.4.0** — exports `WRITE_COMMAND_TYPES` / `WriteCommandType` (the single + write-classification source downstream layers derive from) and reclassifies + `scroll` / `switch_tab` as writes, so `isWriteCommand` now returns `true` for + them (they are user-visible side effects: dry-run must simulate, retry must + replay). No schema change. - **0.3.0** — adds the internal `resolve_ref` probe (fixes dry-run: a snapshot probe re-mints refs and would invalidate the consumer's outstanding refs). - **0.2.0** — adds `fill_secret` and the optional `action_result.simulated` diff --git a/packages/protocol/package.json b/packages/protocol/package.json index 178b910..b56c2cb 100644 --- a/packages/protocol/package.json +++ b/packages/protocol/package.json @@ -14,6 +14,7 @@ "files": [ "dist", "README.md", + "CHANGELOG.md", "LICENSE" ], "sideEffects": false, diff --git a/packages/protocol/src/index.test.ts b/packages/protocol/src/index.test.ts index cddbe16..9ea8395 100644 --- a/packages/protocol/src/index.test.ts +++ b/packages/protocol/src/index.test.ts @@ -7,6 +7,7 @@ import { parseCommand, safeParseCommand, safeParseEvent, + WRITE_COMMAND_TYPES, type Command, } from "./index"; @@ -84,6 +85,42 @@ describe("isWriteCommand", () => { const probe: Command = { type: "resolve_ref", commandId: "c1", ref: "s1e2" }; expect(isWriteCommand(probe)).toBe(false); }); + + it("classifies scroll and switch_tab as writes - user-visible side effects, not DOM reads", () => { + // Both change what the user's real browser shows, so a dry-run must + // simulate (not perform) them and a retry must replay (not repeat) them - + // scroll's relative dy would otherwise double-scroll on a lost-response retry. + const scroll: Command = { type: "scroll", commandId: "c1", dy: 100 }; + const switchTab: Command = { type: "switch_tab", commandId: "c2", tabId: 3 }; + expect(isWriteCommand(scroll)).toBe(true); + expect(isWriteCommand(switchTab)).toBe(true); + }); + + it("agrees with the exported WRITE_COMMAND_TYPES tuple for every command type", () => { + // #given every command type in the union, as a representative command + const representatives: Command[] = [ + { type: "snapshot", commandId: "c", mode: "a11y" }, + { type: "navigate", commandId: "c", url: "https://example.com/" }, + { type: "click", commandId: "c", ref: "r" }, + { type: "type", commandId: "c", ref: "r", text: "t" }, + { type: "fill_secret", commandId: "c", ref: "r", secretRef: "vault://x" }, + { type: "key", commandId: "c", keys: "Enter" }, + { type: "scroll", commandId: "c", dy: 10 }, + { type: "wait", commandId: "c", for: "load" }, + { type: "resolve_ref", commandId: "c", ref: "r" }, + { type: "get_tabs", commandId: "c" }, + { type: "switch_tab", commandId: "c", tabId: 1 }, + ]; + + // #then the predicate and the tuple classify identically - the tuple is + // the published source of truth downstream layers build on + for (const cmd of representatives) { + expect(isWriteCommand(cmd)).toBe( + (WRITE_COMMAND_TYPES as readonly string[]).includes(cmd.type), + ); + } + expect(new Set(WRITE_COMMAND_TYPES).size).toBe(WRITE_COMMAND_TYPES.length); + }); }); describe("EventSchema", () => { diff --git a/packages/protocol/src/index.ts b/packages/protocol/src/index.ts index 8826fe1..4c97e1b 100644 --- a/packages/protocol/src/index.ts +++ b/packages/protocol/src/index.ts @@ -100,8 +100,31 @@ export const CommandSchema = z.discriminatedUnion("type", [ export type Command = z.infer; export type CommandType = Command["type"]; -// State-changing commands gate on human approval (D8). Reads run freely. -const WRITE_COMMANDS = new Set(["click", "type", "key", "navigate", "fill_secret"]); +// The single source of truth for the "write" class, in the OPERATIONAL sense +// this system enforces: a command with a user-visible side effect, which must +// be gated on approval (D8), SIMULATED (never performed) on a dry-run, and +// REPLAYED (never repeated) on an idempotent retry. Downstream layers (the +// service's dry-run gate + write-replay cache, the extension's dedupe, the +// connector's `act` union) all derive from this tuple instead of hand-copying +// the list. +// +// scroll and switch_tab are included even though they don't mutate the DOM: +// both change what the user's real browser shows, so a dry-run of one must not +// actually scroll/switch, and a lost-response retry of `scroll` (a RELATIVE dy) +// must not double-scroll. The genuine reads — snapshot / get_tabs / wait / +// resolve_ref — carry no side effect and stay out (they dispatch freely on a +// dry-run and re-execute freely on a retry). +export const WRITE_COMMAND_TYPES = [ + "click", + "type", + "key", + "navigate", + "fill_secret", + "scroll", + "switch_tab", +] as const satisfies readonly CommandType[]; +export type WriteCommandType = (typeof WRITE_COMMAND_TYPES)[number]; +const WRITE_COMMANDS = new Set(WRITE_COMMAND_TYPES); export const isWriteCommand = (c: Command): boolean => WRITE_COMMANDS.has(c.type); // ── Events: extension → backend ────────────────────────────────────────────── diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 9874d5d..3dfd9b5 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -45,3 +45,16 @@ allowBuilds: # WXT's vite to 8.1.3 (already vetted by the quarantine for @wxt-dev/module-react). overrides: wxt>vite: 8.1.3 + +# Declared peer compatibility for upstream ranges that lag what we install; +# each is verified working by this repo's own suites, so the install-time +# warnings were noise. Drop an entry when the upstream range catches up. +peerDependencyRules: + allowedVersions: + # agents' partyserver (and wrangler's optional peer) still declare + # workers-types ^4; v5 is the same auto-generated surface, newer date. + "partyserver>@cloudflare/workers-types": "5" + "wrangler>@cloudflare/workers-types": "5" + # agents' legacy ai-sdk v4-era deps peer zod ^3; the workspace is zod 4. + "@ai-sdk/provider-utils>zod": "4" + "@ai-sdk/ui-utils>zod": "4"