M5 hardening: cross-tenant vault scoping + dialog handling breadth

.changeset/dialog-handling.md

@@ -0,0 +1,11 @@
+---
+"@understudy/protocol": minor
+"@understudy/connector": minor
+---
+
+Add JavaScript-dialog handling breadth.
+
+- **protocol**: new `dialog` Event (`{ type, tabId, dialogType: alert | confirm | prompt | beforeunload, message, url, defaultPrompt?, disposition: accept | dismiss }`) plus `DialogTypeSchema` / `DialogDispositionSchema` exports. Emitted unsolicited (like `page_event`) after the extension locally handles a page dialog, so a consumer learns what the page said and how it was answered.
+- **connector**: `browser.observe` gains a `get_dialogs` read returning the session's recent dialogs (`ObserveOutput.dialogs`), read from `GET /v1/sessions/:id`.
+
+The extension now applies a type-aware local disposition (alert/beforeunload accept, confirm/prompt dismiss) instead of blindly dismissing every dialog — a `beforeunload` dismiss previously cancelled navigations. Dispositions are decided synchronously extension-side because an open dialog blocks the single CDP channel; the consumer is notified, never in the response path.

apps/backend/.dev.vars.example

@@ -22,5 +22,7 @@ EXTENSION_TOKENS={"dev-ext-token":"dev-tenant"}
 # 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
+# Seed dev secrets with (key MUST be vault://<tenantId>/<name>; fillSecret
+# refuses refs outside the session's own tenant, and the dev tenant is
+# "dev-tenant" above): node scripts/vault-put.mjs 'vault://dev-tenant/ref' --local
 VAULT_MASTER_KEY=ZGV2LXZhdWx0LW1hc3Rlci1rZXktMDEyMzQ1Njc4OSE

apps/backend/README.md

@@ -129,7 +129,16 @@ during that window. Four things close this gap:
   `{commandId, type}`, never the command body), and never written to `setState`,
   never included in the Event response, and never appears in an error string. It
   exists only transiently inside this one Durable Object, for the duration of the
-  one service→extension WS hop.
+  one service→extension WS hop. Resolution is **tenant-scoped**: `fillSecret`
+  derives the session's authoritative tenant from its HMAC-signed `sessionId`
+  (`this.name`, via `auth.ts::tenantOf` — never a caller claim) and refuses any
+  `secretRef` outside that tenant's `vault://<tenantId>/…` namespace *before* any
+  vault read, so a consumer authenticated as tenant B, driving its own session,
+  can never resolve tenant A's secret. A cross-tenant *or* tenant-less ref returns
+  the same scrubbed `ok:false` an absent secret does (no existence oracle). Because
+  understudy owns one shared vault across every tenant, this check lives server-side
+  here — it is not delegated to a consumer-side breakwater, which can only govern
+  that consumer's own agent, never isolate one tenant from another.
 - **`dryRun` is a service-API parameter (`{command, dryRun?}`), not a `Command`
   union field**: adding it to every Command variant would churn the shared,
   published protocol for a cross-cutting concern. On a dryRun WRITE command,
@@ -142,8 +151,11 @@ during that window. Four things close this gap:
   always refuse — and it invalidates every outstanding ref, breaking the
   approved command that follows the simulation (the original M3 dry-run bug,
   caught by the attended e2e). `fillSecret` does the same ref-only check on
-  dryRun and never calls `resolveSecret` or dispatches a `type` command. This
-  is fail-safe by construction: a governance simulation (called *before* an
+  dryRun and never calls `resolveSecret` or dispatches a `type` command; a
+  `secretRef` outside the session's own tenant short-circuits to a simulated
+  `ok:false` (the refusal the real call gives) before even the ref probe, so a
+  governance preview is honest on the tenant axis too and still reads no vault.
+  This is fail-safe by construction: a governance simulation (called *before* an
   approval grant exists) can never actually mutate the page or resolve a
   secret. A dry-run `ok` guarantees *resolvability* (the ref maps to a live
   node in the current generation) — not *executability* of the eventual
@@ -159,8 +171,11 @@ during that window. Four things close this gap:
   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`).
+  ("not a recognized envelope"). A per-tenant external KMS (per-tenant
+  *encryption* at rest) remains a possible future swap behind the same
+  `VaultBinding.get` seam (`types.ts`); tenant *authorization* does not wait on
+  it — it is already enforced above the seam by the `vault://<tenantId>/…`
+  namespace check in the `fill_secret` bullet above.
 - **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
@@ -202,6 +217,11 @@ during that window. Four things close this gap:
 - `fill_secret` plaintext never enters `setState`, logs, the Event response, or an
   error string; the coordinator logs only `{commandId, type}`. A replayed
   `fill_secret` touches neither the vault nor the wire.
+- A `secretRef` resolves only within its session's own tenant: `fillSecret`
+  requires `vault://<tenantId>/…` (tenant derived from the signed sessionId) and
+  refuses a cross-tenant or tenant-less ref with the same scrubbed `ok:false` an
+  absent secret gets, before any vault read — no cross-tenant plaintext, no
+  existence oracle.
 - The vault at rest holds only `v1.<iv>.<ct>` 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
@@ -214,7 +234,9 @@ during that window. Four things close this gap:
 - `SessionCoordinator` is the only Cloudflare-coupling seam; `coordinator.ts`
   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.
+  returns a simulated `action_result` from a read-only ref check - or, for a
+  `secretRef` outside the session's tenant, a simulated `ok:false` refusal that
+  matches what the real call would return, with no vault read.
 - 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
@@ -245,8 +267,12 @@ openssl rand 32 | basenc --base64url | tr -d '=' | pnpm exec wrangler secret put
 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=<key> node scripts/vault-put.mjs 'vault://tenant/ref'
+# Seed a vault secret (encrypts locally; KV never sees plaintext). The key
+# MUST be vault://<tenantId>/<name> where <tenantId> matches the tenant in
+# CALLER_TOKENS/EXTENSION_TOKENS: fillSecret refuses any ref outside the
+# requesting session's own tenant namespace, so a tenant-less key (vault://ref)
+# is unreadable by design.
+printf '%s' 'the-secret' | VAULT_MASTER_KEY=<key> node scripts/vault-put.mjs 'vault://<tenantId>/ref'
 ```
 
 The extension connects to

apps/backend/scripts/stub-consumer.mjs

@@ -27,10 +27,12 @@
  *
  * `type` and `click` target the first ref found in the snapshot's a11y tree
  * (a naive "first element" heuristic - fine for a demo/runbook, not real
- * ref-targeting logic). `fill_secret` deliberately uses a fake vault://
- * secretRef that no vault will ever resolve, so it is expected to return
- * ok:false - demonstrating the scrubbed-error path, not a broken script. A
- * stale/unresolvable ref for any command is likewise expected to return
+ * ref-targeting logic). `fill_secret` deliberately uses a fake, tenant-scoped
+ * secretRef (`vault://dev-tenant/…`) the vault has no value for, so it is
+ * expected to return ok:false - demonstrating the scrubbed-error path, not a
+ * broken script. (fillSecret enforces `vault://<tenantId>/…` scoping: a ref
+ * outside the caller's own tenant is refused with the same scrubbed ok:false.)
+ * A stale/unresolvable ref for any command is likewise expected to return
  * ok:false, not to fail the run.
  *
  * Env vars / flags (all optional; flags win, then env vars, then defaults):
@@ -139,8 +141,10 @@ async function main() {
     type: "fill_secret",
     commandId: "stub-4",
     ref: targetRef,
-    // Deliberately fake and unresolvable - see the header comment.
-    secretRef: "vault://stub-consumer-fake-secret",
+    // Deliberately fake and unresolvable - see the header comment. Scoped to
+    // the default dev tenant so it exercises the vault-miss path; a ref outside
+    // the caller's tenant is refused with the same scrubbed ok:false.
+    secretRef: "vault://dev-tenant/stub-consumer-fake-secret",
   });
 
   console.log("\ndone.");

apps/backend/src/auth.ts

@@ -53,12 +53,28 @@ export async function authenticate(req: Request, env: Env): Promise<Actor | null
   return new StaticTokenVerifier(tokens).verify(token);
 }
 
+/**
+ * A tenantId must be a flat, non-empty slug. The credential vault namespaces
+ * keys as `vault://<tenantId>/<name>` and SessionAgent.fillSecret isolates
+ * tenants with a `vault://<tenantId>/` prefix check, so a tenantId that is
+ * empty or contains `/` would let one tenant's prefix straddle another's
+ * namespace (tenant "acme" reaching "acme/eu"'s keys). Enforced at mint
+ * (fail-closed at session creation) and re-checked in tenantOf, so no signed
+ * id can carry an unsafe tenant into that prefix check.
+ */
+export function isValidTenantId(tenantId: string): boolean {
+  return tenantId.length > 0 && !tenantId.includes("/");
+}
+
 /**
  * Mints a sessionId with the owning tenant embedded and HMAC-signed, so
  * scopeSession can verify ownership statelessly - no lookup table maps
  * sessionId -> tenant; the id carries its own proof (DL-008).
  */
 export async function mintSessionId(tenantId: string, env: Env): Promise<string> {
+  if (!isValidTenantId(tenantId)) {
+    throw new Error("invalid tenantId: must be non-empty and contain no '/'");
+  }
   const nonce = toHex(crypto.getRandomValues(new Uint8Array(16)));
   const payloadBytes = new TextEncoder().encode(JSON.stringify({ t: tenantId, n: nonce }));
 
@@ -69,37 +85,53 @@ export async function mintSessionId(tenantId: string, env: Env): Promise<string>
 }
 
 /**
- * Verifies sessionId's HMAC signature and that its embedded tenant matches
- * tenantId. Every failure path - bad shape, bad signature, wrong tenant,
- * decode error - collapses to the same "not-found" the caller surfaces as
- * 404, so no response shape distinguishes "malformed id" from "someone
- * else's session" (DL-008: no existence oracle).
+ * Verifies sessionId's HMAC signature and returns the tenant embedded in it,
+ * or null for any malformed / forged / undecodable id. This is a session's
+ * AUTHORITATIVE tenant - it comes from the signed id itself, never a
+ * caller-supplied claim - so a Durable Object can trust `tenantOf(this.name)`
+ * to scope a resource it owns (e.g. the credential vault) to that session's
+ * owner. scopeSession is the boolean "does this id belong to tenantId?"
+ * wrapper over it.
  */
-export async function scopeSession(
-  sessionId: string,
-  tenantId: string,
-  env: Env,
-): Promise<"ok" | "not-found"> {
+export async function tenantOf(sessionId: string, env: Env): Promise<string | null> {
   try {
     const parts = sessionId.split(".");
-    if (parts.length !== 2) return "not-found";
+    if (parts.length !== 2) return null;
     const [payloadB64, sigB64] = parts;
-    if (!payloadB64 || !sigB64) return "not-found";
+    if (!payloadB64 || !sigB64) return null;
 
     const payloadBytes = base64urlDecode(payloadB64);
     const sigBytes = base64urlDecode(sigB64);
 
     const key = await importHmacKey(env.AUTH_HMAC_SECRET);
     const verified = await crypto.subtle.verify("HMAC", key, sigBytes, payloadBytes);
-    if (!verified) return "not-found";
+    if (!verified) return null;
 
     const payload = JSON.parse(new TextDecoder().decode(payloadBytes)) as { t?: unknown };
-    return payload.t === tenantId ? "ok" : "not-found";
+    // Re-check the shape a valid mint enforces: a signed id must never carry an
+    // empty or slash-bearing tenant into fillSecret's vault-namespace prefix
+    // check (defense in depth against a token minted before this rule existed).
+    return typeof payload.t === "string" && isValidTenantId(payload.t) ? payload.t : null;
   } catch {
-    return "not-found";
+    return null;
   }
 }
 
+/**
+ * Whether sessionId is a valid id minted for tenantId. Every failure path -
+ * bad shape, bad signature, wrong tenant, decode error - collapses to the
+ * same "not-found" the caller surfaces as 404, so no response shape
+ * distinguishes "malformed id" from "someone else's session" (DL-008: no
+ * existence oracle).
+ */
+export async function scopeSession(
+  sessionId: string,
+  tenantId: string,
+  env: Env,
+): Promise<"ok" | "not-found"> {
+  return (await tenantOf(sessionId, env)) === tenantId ? "ok" : "not-found";
+}
+
 /**
  * Verifies the extension's own per-user token (SessionAgent.onConnect),
  * independent of the caller-auth path above - the browser extension and the