From 207a05d3c99e43e360b524d4e3c2e633cc72e895 Mon Sep 17 00:00:00 2001 From: "Claude (Carey Janecka)" Date: Thu, 22 Oct 2026 14:39:08 -0700 Subject: [PATCH 1/6] Test that redaction tags do not carry over to a new document A cross-site load gets a new renderer process, which numbers its nodes from the start again. The same form then gets the same element uids on a new document. Redaction tags are keyed by document, so the new fields are not redacted until a new fill. Add an e2e test for this case. Do not clear tags on navigation. Back can restore the old document from the back/forward cache with its filled values, and those must stay redacted. Refs EMG-293, #33. Co-Authored-By: Claude Opus 5.5 --- src/browser/session.ts | 4 ++++ test/e2e.test.ts | 52 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/src/browser/session.ts b/src/browser/session.ts index eea2d1a..e1ebe59 100644 --- a/src/browser/session.ts +++ b/src/browser/session.ts @@ -58,6 +58,10 @@ export class BrowserSession { this.page = pages[0] ?? (await this.browser.newPage()); this.cdp = await this.page.createCDPSession(); // Element uids are backendNodeId-based and die with the document. + // Redaction tags stay: they are keyed by document, so a new document + // that reuses node ids never matches them. Dropping them here would + // unredact a filled field that Back restores from the back/forward + // cache, which keeps the document and its values. this.page.on("framenavigated", (frame) => { if (frame === this.page?.mainFrame()) this.clearElements(); }); diff --git a/test/e2e.test.ts b/test/e2e.test.ts index 36a057d..a5c6bd5 100644 --- a/test/e2e.test.ts +++ b/test/e2e.test.ts @@ -10,6 +10,8 @@ import { McpStdioClient } from "./mcp-client.js"; */ const DEMO_PLAINTEXT = "mock-demo-p@ssw0rd-1234"; // mock-secrets.ts seed +/** The fixture server on another site, for a cross-site navigation. */ +const OTHER_SITE = FIXTURE_ORIGIN.replace("localhost", "127.0.0.1"); let fixture: { stop: () => void }; let client: McpStdioClient; @@ -161,6 +163,56 @@ test("fills a JSON card secret into several fields with one call", async () => { expect(client.transcript).not.toContain("4242424242424242"); }); +test("a new document does not inherit the old document's redaction tags", async () => { + const refs = (await client.callTool("list_secret_refs")).body as { + refs: { secretId: string; name?: string }[]; + }; + const card = refs.refs.find((r) => r.name === "demo-card"); + type Snap = { elements: { uid: string; name?: string; value?: string }[] }; + const cardFields = ["cardNumber", "cardExpiry", "cardCvc"]; + const checkout = async (origin: string) => { + await client.callTool("navigate", { url: `${origin}/checkout` }); + return (await client.callTool("snapshot")).body as Snap; + }; + const uids = (snap: Snap) => + cardFields.map((name) => snap.elements.find((e) => e.name === name)!.uid); + const fillCard = (snap: Snap) => { + const [number, expiry, cvc] = uids(snap); + return client.callTool("fill_secret", { + secret_id: card!.secretId, + fields: [ + { key: "number", element_uid: number }, + { key: "expiry", element_uid: expiry }, + { key: "cvc", element_uid: cvc }, + ], + }); + }; + const values = (snap: Snap) => + cardFields.map((name) => snap.elements.find((e) => e.name === name)?.value); + const redacted = cardFields.map(() => "[REDACTED:secret-filled-field]"); + + // A cross-site load gets a new renderer process, and a new process + // numbers its nodes from the start again. So the fixture form gets the + // same uids each time it loads after a cross-site hop. + await checkout(OTHER_SITE); + const first = await checkout(FIXTURE_ORIGIN); + expect((await fillCard(first)).isError).toBe(false); + expect(values((await client.callTool("snapshot")).body as Snap)).toEqual( + redacted, + ); + + await checkout(OTHER_SITE); + const second = await checkout(FIXTURE_ORIGIN); + expect(uids(second)).toEqual(uids(first)); + expect(values(second)).toEqual(["", "", ""]); + + // A new fill tags the fields again. + expect((await fillCard(second)).isError).toBe(false); + expect(values((await client.callTool("snapshot")).body as Snap)).toEqual( + redacted, + ); +}); + test("redacts a card number the page reformats, re-renders and echoes", async () => { const refs = (await client.callTool("list_secret_refs")).body as { refs: { secretId: string; name?: string }[]; From 910aa6ea162c26ce74905096a186ca0d885ea4f5 Mon Sep 17 00:00:00 2001 From: "Claude (Carey Janecka)" Date: Thu, 22 Oct 2026 14:43:43 -0700 Subject: [PATCH 2/6] Report the field state after fill_secret writes Masked or script-controlled inputs can drop a value, and the result still said filled: true. Now the broker reads each field back over CDP after the write and compares it to the value in broker memory. The result gives, per field, the value length, whether it matches the secret when spaces and punctuation are ignored, and validity.valid. It also gives confirmed, which is true when every field matches. It never gives the value or any part of it. await_fill returns the same result. A field that holds fewer characters than were written no longer fails the fill. The result reports it instead. Add a fixture page that clears the card number on input and rejects the expiry format, and an e2e test for it. Refs EMG-294, #34. Co-Authored-By: Claude Opus 5.5 --- docs/DESIGN.md | 2 +- site/docs.html | 10 ++- site/docs.md | 2 +- site/reference.html | 5 +- site/reference.md | 2 +- skills/secure-browser/SKILL.md | 2 +- skills/secure-browser/references/tools.md | 36 ++++++++- src/browser/inject.ts | 95 +++++++++++++++++++---- src/tools/await-fill.ts | 9 +-- src/tools/fill-common.ts | 43 ++++++++-- src/tools/fill-secret.ts | 11 +-- test/e2e.test.ts | 80 ++++++++++++++++++- test/fixtures/checkout-clearing.html | 34 ++++++++ test/fixtures/serve.ts | 6 ++ 14 files changed, 290 insertions(+), 47 deletions(-) create mode 100644 test/fixtures/checkout-clearing.html diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 9c3d1ef..ceb6560 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -16,7 +16,7 @@ Turnkey's export flow fits this exactly. `exportSecret` re-encrypts the secret t 4. `SecretsClient.exportSecret(ref)` — plaintext lands in broker memory. 5. Register the value with the `RedactionRegistry` **before** injection. 6. For each target: observe and authorize again, require the same destination (`isSameDestination`), register the target, then inject over CDP `Input.insertText`. Never through `Runtime.evaluate`. -7. Drop the value. Return outcome only: `{ filled: true }`. +7. Read each field back over CDP and compare it to the value in the broker. Drop the value. Return the outcome and non-secret facts per field only: `{ filled: true, confirmed, fields: [{ length, matches, valid }] }`. Never the value or any part of it. ## Destination binding diff --git a/site/docs.html b/site/docs.html index c3f4c93..ca1e72d 100644 --- a/site/docs.html +++ b/site/docs.html @@ -47,9 +47,13 @@

The safe fill workflow

For JSON payloads, pass its declared field keys in one call.
  • - Verify and submit. A follow-up snapshot shows a - structural redaction marker instead of a value. Click the submit - control, then snapshot until the page changes. + Verify and submit. The + fill_secret result says if each field kept its value + (confirmed, and matches, + length and valid per field), never the value + itself. A follow-up snapshot shows a structural redaction marker + instead of a value. Click the submit control, then snapshot until the + page changes.

    • Destination bindings

    diff --git a/site/docs.md b/site/docs.md index 54dc76f..708907e 100644 --- a/site/docs.md +++ b/site/docs.md @@ -12,7 +12,7 @@ The agent drives a browser but never reads secret material. Treat every secret a 2. **Navigate and inspect.** Use `navigate`, then `snapshot`. Snapshot element IDs become invalid after navigation, so take a fresh snapshot whenever the page changes. 3. **Fill visible values normally.** Use `type_text` only for non-secret values such as email or ZIP code. 4. **Fill credentials by reference.** Use `fill_secret` with the secret ID and current element ID. For JSON payloads, pass declared field keys in one call. -5. **Verify and submit.** A follow-up snapshot shows a structural redaction marker instead of a value. Click submit, then snapshot until the page changes. +5. **Verify and submit.** The `fill_secret` result says if each field kept its value (`confirmed`, and `matches`, `length` and `valid` per field), never the value itself. A follow-up snapshot shows a structural redaction marker instead of a value. Click submit, then snapshot until the page changes. ## Destination bindings diff --git a/site/reference.html b/site/reference.html index 27d7eee..0bbad9e 100644 --- a/site/reference.html +++ b/site/reference.html @@ -76,8 +76,9 @@

    MCP operations

    fields - Checks the binding, requests export, then injects via CDP. May - return pending_approval. + Checks the binding, requests export, injects via CDP, then reports + per field if the page kept the value (never the value). May return + pending_approval. diff --git a/site/reference.md b/site/reference.md index d392548..abab9cd 100644 --- a/site/reference.md +++ b/site/reference.md @@ -15,7 +15,7 @@ The tool surface is intentionally small. All results pass through the server's r | `snapshot` | None | Returns interactive element IDs. Filled values are redacted. | | `click` | `element_uid` | Clicks an element from the most recent snapshot. | | `type_text` | `element_uid`, `text` | Types visible, non-secret text. Never use it for a credential. | -| `fill_secret` | `secret_id` plus `element_uid`, or `fields` | Checks the binding, requests export, then injects via CDP. May return `pending_approval`. | +| `fill_secret` | `secret_id` plus `element_uid`, or `fields` | Injects via CDP if the binding holds; says if each field kept it. May return `pending_approval`. | | `await_fill` | `fill_id`, optional `timeout_seconds` (1–120) | Waits for approval, revalidates the target, and completes a pending fill; it can remain pending. | | `list_network_requests` | None | Reserved scaffold; currently returns `not_implemented`. | diff --git a/skills/secure-browser/SKILL.md b/skills/secure-browser/SKILL.md index 8bd8ecf..bd209f9 100644 --- a/skills/secure-browser/SKILL.md +++ b/skills/secure-browser/SKILL.md @@ -35,7 +35,7 @@ Snapshots and fills target the main frame only. Cross-origin iframe inputs, incl 3. `snapshot` — returns interactive elements with `uid`s. Element uids are invalidated by navigation; re-snapshot after every navigation. 4. Fill non-secret fields (email, name, ZIP) with `type_text`. These are visible in your transcript, which is fine — they are not secrets. 5. Fill secret fields with `fill_secret`. Single-value secrets take one `element_uid`. A secret whose binding lists `sbm:fields` holds a JSON payload (e.g. a card's number/expiry/cvc): pass `fields: [{key, element_uid}, …]` to fill all of them in one call — one export, one approval. -6. `snapshot` to verify: the filled field's value reads `[REDACTED:secret-filled-field]`. +6. Check the `fill_secret` result: `confirmed: true` means every field holds its value. If a field has `matches: false`, `length: 0`, or `valid: false`, the page dropped or rejected the value; report it and do not submit. A later `snapshot` shows filled fields as `[REDACTED:secret-filled-field]`. 7. `click` the submit button, then `snapshot` (repeat with short waits) until the page moves on. ## Browser handles diff --git a/skills/secure-browser/references/tools.md b/skills/secure-browser/references/tools.md index 2eda217..ba1d894 100644 --- a/skills/secure-browser/references/tools.md +++ b/skills/secure-browser/references/tools.md @@ -59,7 +59,39 @@ Field values are redacted: `[REDACTED:secret-filled-field]` after a fill, `[MASK ## fill_secret -`{ secret_id: string, element_uid?: string, fields?: [{key, element_uid}] }` → outcome only (no value). Steps performed broker-side: resolve ref → resolve elements → enforce binding (origin, URL pattern, selector) → export from Turnkey → inject via CDP → tag fields for redaction. Throws a binding-violation error if the live page or an element doesn't match the secret's binding; do not retry elsewhere. +`{ secret_id: string, element_uid?: string, fields?: [{key, element_uid}] }` → outcome and read-back facts (never the value). Steps performed broker-side: resolve ref → resolve elements → enforce binding (origin, URL pattern, selector) → export from Turnkey → inject via CDP → tag fields for redaction. Throws a binding-violation error if the live page or an element doesn't match the secret's binding; do not retry elsewhere. + +On success it returns what each field holds after the write, read back by the broker. These are facts about the field, never its value or any part of it: + +```jsonc +{ + "filled": true, + "confirmed": false, // true when every field holds its value + "secret_id": "…", + "element_uids": ["e12", "e13"], + "fields": [ + // length: characters in the field now; matches: equals the secret (or its part), + // ignoring spaces and punctuation the page adds; valid: the field's + // validity.valid, when it has one + { + "element_uid": "e12", + "key": "number", + "length": 0, + "matches": false, + "valid": true, + }, + { + "element_uid": "e13", + "key": "expiry", + "length": 4, + "matches": true, + "valid": false, + }, + ], +} +``` + +`matches: false` or `length: 0` means the page dropped or changed the value (some masked or script-controlled inputs do). `valid: false` means the page rejects the value's format. Report this to the user; do not submit the form as if the fill worked. A field the page removed during the fill has no `length` and `matches: false`. Single-value secrets take `element_uid`. A JSON-payload secret — its binding declares `sbm:fields`, a map from payload key to required selector — takes `fields` instead, filling several inputs in ONE call: one export, one approval. Example: a card secret with keys `number`/`expiry`/`cvc` fills all three checkout fields at once. Such a secret refuses a plain `element_uid` fill, so its whole payload never lands in one field. @@ -67,7 +99,7 @@ When the export needs multi-party approval, returns `{ filled: false, status: "p ## await_fill -`{ fill_id: string, timeout_seconds?: number (default 30, max 120), element_uid?: string, fields?: { key, element_uid }[] }`. Completes a pending fill once approvers reach quorum: waits up to the timeout, re-validates the destination against the live page, injects. Returns `pending_approval` again (same fields, including `approval_url`) if the timeout passes first — the fill stays valid, call again. Fails permanently if the export is rejected. If the target element left the page (or a hosted broker restarted), the fill stays valid: navigate back, snapshot, and call `await_fill` again with the same `fill_id` and the new `element_uid` or `fields`. +`{ fill_id: string, timeout_seconds?: number (default 30, max 120), element_uid?: string, fields?: { key, element_uid }[] }`. Completes a pending fill once approvers reach quorum: waits up to the timeout, re-validates the destination against the live page, injects, and returns the same result as a completed `fill_secret`. Returns `pending_approval` again (same fields, including `approval_url`) if the timeout passes first — the fill stays valid, call again. Fails permanently if the export is rejected. If the target element left the page (or a hosted broker restarted), the fill stays valid: navigate back, snapshot, and call `await_fill` again with the same `fill_id` and the new `element_uid` or `fields`. ## list_network_requests (unavailable) diff --git a/src/browser/inject.ts b/src/browser/inject.ts index 0b369ae..347900b 100644 --- a/src/browser/inject.ts +++ b/src/browser/inject.ts @@ -1,6 +1,23 @@ +import type { CDPSession } from "puppeteer-core"; + import type { ExportedSecret } from "../broker/types.js"; import type { BrowserSession } from "./session.js"; +/** + * Non-secret facts about a field after a fill, so the agent can tell a fill + * the page kept from one it dropped. Never the value or any part of it. + */ +export type FieldCheck = { + /** Length of the field's current value; absent when the field could not + * be read back (the page removed it). */ + length?: number; + /** Whether the current value equals the secret, ignoring spaces and + * punctuation the page may add (a card number grouped in fours). */ + matches: boolean; + /** The field's `validity.valid`, for elements that have one. */ + valid?: boolean; +}; + /** * The one function that handles plaintext: focuses the bound element and * types the secret via CDP `Input.insertText` (raw CDP, not page JS — the @@ -11,12 +28,15 @@ import type { BrowserSession } from "./session.js"; * the redaction registry BEFORE calling this, so no snapshot/network read * can race ahead of redaction (see `injectResolved` in * src/tools/fill-common.ts). + * + * Returns what the field holds after the write (see `FieldCheck`). A page + * that drops or rewrites the value is reported, not treated as an error. */ export async function injectSecret( session: BrowserSession, elementUid: string, secret: ExportedSecret, -): Promise { +): Promise { try { const element = session.resolveElement(elementUid); const cdp = session.cdpSession(); @@ -33,25 +53,70 @@ export async function injectSecret( await cdp.send("DOM.focus", { backendNodeId: element.backendNodeId }); await cdp.send("Input.insertText", { text: secret.value }); - // Verify acceptance by length only — never echo the value. The field may - // hold MORE characters than we inserted (auto-formatters add separators: - // "4242424242424242" renders as "4242 4242 4242 4242"), so only an empty - // or clearly truncated field is a failure. - const acceptedLength = await element.handle.evaluate((el) => { - const input = el as HTMLInputElement; - return "value" in el ? input.value.length : -1; - }); - if (acceptedLength >= 0 && acceptedLength < secret.value.length) { - throw new Error( - `Fill verification failed: field holds ${acceptedLength} chars, ` + - `expected at least ${secret.value.length}`, - ); - } // Commit for frameworks listening on change/blur. await element.handle.evaluate((el) => { el.dispatchEvent(new Event("change", { bubbles: true })); }); + return await checkField(cdp, element.backendNodeId, secret.value); } finally { secret.release(); } } + +/** Letters and digits only, so formatting the page adds does not count. */ +const significant = (s: string) => + s.normalize("NFKC").replace(/[^\p{L}\p{N}]/gu, ""); + +/** Runs with `this` bound to the field. Returns its value and validity. */ +const READ_FIELD = `function () { + const value = typeof this.value === "string" ? this.value : (this.textContent ?? ""); + const valid = this.validity && typeof this.validity.valid === "boolean" ? this.validity.valid : null; + return { value, valid }; +}`; + +/** + * Reads the field back and compares it in the broker. The value comes out + * of the page, is compared, and is dropped here: only the facts leave this + * function. It reads by backendNodeId over CDP, not through the element + * handle, so it still works when the change event made the page navigate + * within the document (which disposes the handles). The facts are only as + * true as the page lets them be, since page script can change what a field + * reports. A field that cannot be read back reports no length and no match. + */ +async function checkField( + cdp: CDPSession, + backendNodeId: number, + secret: string, +): Promise { + let objectId: string | undefined; + let field: { value: string; valid: boolean | null } | undefined; + try { + ({ + object: { objectId }, + } = await cdp.send("DOM.resolveNode", { backendNodeId })); + if (objectId) { + const { result } = await cdp.send("Runtime.callFunctionOn", { + objectId, + functionDeclaration: READ_FIELD, + returnByValue: true, + }); + field = result.value as typeof field; + } + } catch { + // The node is gone: nothing to report. + } finally { + if (objectId) { + void cdp.send("Runtime.releaseObject", { objectId }).catch(() => {}); + } + } + if (typeof field?.value !== "string") return { matches: false }; + const loose = significant(secret); + const check: FieldCheck = { + length: field.value.length, + matches: + field.value === secret || + (loose.length > 0 && significant(field.value) === loose), + }; + if (typeof field.valid === "boolean") check.valid = field.valid; + return check; +} diff --git a/src/tools/await-fill.ts b/src/tools/await-fill.ts index 965c05c..855b85f 100644 --- a/src/tools/await-fill.ts +++ b/src/tools/await-fill.ts @@ -4,6 +4,7 @@ import { ConsensusPendingError } from "../broker/secrets-client.js"; import { assertSameDestination, FillDeniedError, + filledResult, injectResolved, specsFromArgs, targetArgs, @@ -99,13 +100,9 @@ export const awaitFill = defineTool({ throw err; } - await injectResolved(ctx, exported, validated); + const checks = await injectResolved(ctx, exported, validated); await ctx.pendingFills.delete(fill.fillId); - return { - filled: true, - secret_id: ref.secretId, - element_uids: fill.targets.map((t) => t.elementUid), - }; + return filledResult(ref.secretId, fill.targets, checks); }, }); diff --git a/src/tools/fill-common.ts b/src/tools/fill-common.ts index 4c22444..a4d0537 100644 --- a/src/tools/fill-common.ts +++ b/src/tools/fill-common.ts @@ -23,7 +23,7 @@ import { import { z } from "zod"; import type { ExportedSecret, SecretRef } from "../broker/types.js"; -import { injectSecret } from "../browser/inject.js"; +import { injectSecret, type FieldCheck } from "../browser/inject.js"; import { observeTargets } from "../browser/observe.js"; import type { ToolContext } from "./tool.js"; @@ -217,12 +217,15 @@ async function recheckTarget( * * Parts shorter than the registry's minimum length (a 3-digit CVC) are not * text-scanned; the registered target still redacts the field itself. + * + * Returns what each field holds after its write, in `fill.specs` order. */ export async function injectResolved( ctx: ToolContext, exported: ExportedSecret, fill: ValidatedFill, -): Promise { +): Promise { + const checks: FieldCheck[] = []; try { ctx.registry.registerValues(redactionEntries(exported.ref, exported.value)); let payload: Record | undefined; @@ -246,13 +249,39 @@ export async function injectResolved( } const target = await recheckTarget(ctx, fill, index); ctx.registry.registerTarget(target, exported.ref.secretId); - await injectSecret(ctx.session, spec.elementUid, { - ref: exported.ref, - value, - release: () => {}, - }); + checks.push( + await injectSecret(ctx.session, spec.elementUid, { + ref: exported.ref, + value, + release: () => {}, + }), + ); } + return checks; } finally { exported.release(); } } + +/** + * The result of a completed fill: the outcome and, for each field, the + * non-secret facts read back after the write. `confirmed` is true when + * every field holds its value. + */ +export function filledResult( + secretId: string, + specs: FillFieldSpec[], + checks: FieldCheck[], +) { + return { + filled: true, + confirmed: checks.every((c) => c.matches), + secret_id: secretId, + element_uids: specs.map((s) => s.elementUid), + fields: specs.map((s, i) => ({ + element_uid: s.elementUid, + ...(s.key !== undefined ? { key: s.key } : {}), + ...checks[i]!, + })), + }; +} diff --git a/src/tools/fill-secret.ts b/src/tools/fill-secret.ts index 2f05fe7..7c54c9e 100644 --- a/src/tools/fill-secret.ts +++ b/src/tools/fill-secret.ts @@ -4,6 +4,7 @@ import { z } from "zod"; import { ConsensusNeededError } from "../broker/secrets-client.js"; import { + filledResult, injectResolved, specsFromArgs, targetArgs, @@ -110,13 +111,9 @@ export const fillSecret = defineTool({ // write and must still be the destination checked in step 3; each // part registers with the redaction registry BEFORE the value // touches the page, then the value is dropped. - await injectResolved(ctx, exported, validated); + const checks = await injectResolved(ctx, exported, validated); - // Outcome only — never the value. - return { - filled: true, - secret_id: ref.secretId, - element_uids: specs.map((s) => s.elementUid), - }; + // Outcome and read-back facts only — never the value. + return filledResult(ref.secretId, specs, checks); }, }); diff --git a/test/e2e.test.ts b/test/e2e.test.ts index a5c6bd5..96737ed 100644 --- a/test/e2e.test.ts +++ b/test/e2e.test.ts @@ -75,7 +75,20 @@ test("fills a secret without ever exposing it", async () => { element_uid: password!.uid, }); expect(fill.isError).toBe(false); - expect(fill.body).toMatchObject({ filled: true, secret_id: demo!.secretId }); + expect(fill.body).toEqual({ + filled: true, + confirmed: true, + secret_id: demo!.secretId, + element_uids: [password!.uid], + fields: [ + { + element_uid: password!.uid, + length: DEMO_PLAINTEXT.length, + matches: true, + valid: true, + }, + ], + }); // The post-fill snapshot shows the field structurally redacted. const snap2 = (await client.callTool("snapshot")).body as { @@ -236,6 +249,11 @@ test("redacts a card number the page reformats, re-renders and echoes", async () ], }); expect(fill.isError).toBe(false); + // The page grouped the number in fours; it still holds the secret. + expect(fill.body).toMatchObject({ + confirmed: true, + fields: [{ key: "number", length: 19, matches: true }, {}, {}], + }); const snap2 = (await client.callTool("snapshot")).body as { elements: { uid: string; name?: string; text?: string; value?: string }[]; @@ -340,6 +358,66 @@ test("a browser_id names a browser of its own, beside the session's", async () = expect(gone.text).toContain("Unknown browser_id"); }); +test("reports a field the page cleared or rejects after a fill", async () => { + const refs = (await client.callTool("list_secret_refs")).body as { + refs: { secretId: string; name?: string }[]; + }; + const card = refs.refs.find((r) => r.name === "demo-card"); + + await client.callTool("navigate", { + url: `${FIXTURE_ORIGIN}/checkout-clearing`, + }); + const snap = (await client.callTool("snapshot")).body as { + elements: { uid: string; name?: string }[]; + }; + const byName = (name: string) => + snap.elements.find((e) => e.name === name)!.uid; + const fill = await client.callTool("fill_secret", { + secret_id: card!.secretId, + fields: [ + { key: "number", element_uid: byName("cardNumber") }, + { key: "expiry", element_uid: byName("cardExpiry") }, + { key: "cvc", element_uid: byName("cardCvc") }, + ], + }); + expect(fill.isError).toBe(false); + expect(fill.body).toEqual({ + filled: true, + confirmed: false, + secret_id: card!.secretId, + element_uids: [ + byName("cardNumber"), + byName("cardExpiry"), + byName("cardCvc"), + ], + fields: [ + { + element_uid: byName("cardNumber"), + key: "number", + length: 0, + matches: false, + valid: true, + }, + { + element_uid: byName("cardExpiry"), + key: "expiry", + length: 4, + matches: true, + valid: false, + }, + { + element_uid: byName("cardCvc"), + key: "cvc", + length: 3, + matches: true, + valid: true, + }, + ], + }); + // The facts hold no part of any value. + expect(fill.text).not.toMatch(/4242|"1234"|"123"/); +}); + test("the plaintext never appeared anywhere in server output", () => { expect(client.transcript.length).toBeGreaterThan(0); expect(client.transcript).not.toContain(DEMO_PLAINTEXT); diff --git a/test/fixtures/checkout-clearing.html b/test/fixtures/checkout-clearing.html new file mode 100644 index 0000000..da4e747 --- /dev/null +++ b/test/fixtures/checkout-clearing.html @@ -0,0 +1,34 @@ + + + + + Checkout (drops the card number) + + + +
    + + + + + + + +
    + + + diff --git a/test/fixtures/serve.ts b/test/fixtures/serve.ts index 680f479..968fe8b 100644 --- a/test/fixtures/serve.ts +++ b/test/fixtures/serve.ts @@ -14,6 +14,9 @@ export function startFixtureServer(): { stop: () => void } { new URL("./checkout-formatted.html", import.meta.url), ); const splitHtml = Bun.file(new URL("./checkout-split.html", import.meta.url)); + const clearingHtml = Bun.file( + new URL("./checkout-clearing.html", import.meta.url), + ); const receiptHtml = Bun.file(new URL("./receipt.html", import.meta.url)); const server = Bun.serve({ port: FIXTURE_PORT, @@ -32,6 +35,9 @@ export function startFixtureServer(): { stop: () => void } { return new Response(formattedHtml); } if (pathname === "/checkout-split") return new Response(splitHtml); + if (pathname === "/checkout-clearing") { + return new Response(clearingHtml); + } if (pathname === "/pay" && req.method === "POST") { return new Response(receiptHtml); } From 324406eb9a1b309a152400c378b3426cff2e7965 Mon Sep 17 00:00:00 2001 From: "Claude (Carey Janecka)" Date: Thu, 22 Oct 2026 14:46:18 -0700 Subject: [PATCH 3/6] Add headings and alert text to the snapshot An agent could not read a success message or an error, because the snapshot listed only interactive elements. Now the snapshot has a text field with the page's visible headings and its alert, status and live regions. It has at most 20 headings and 10 alerts, and each entry has at most 200 characters. Each entry goes through the value scan before it is cut, so a cut cannot leave part of a secret. innerText leaves out field values. The receipt fixture marks its confirmation as a status region. The formatted checkout fixture shows the card number in a status line, which the snapshot shows redacted. Refs EMG-295, #35. Co-Authored-By: Claude Opus 5.5 --- docs/DEMO-STRIPE.md | 2 +- site/reference.html | 3 +- site/reference.md | 2 +- skills/secure-browser/SKILL.md | 2 +- skills/secure-browser/references/tools.md | 6 +- src/browser/page-text.ts | 85 +++++++++++++++++++++++ src/browser/snapshot.ts | 10 ++- src/tools/snapshot.ts | 5 +- test/e2e.test.ts | 32 +++++++++ test/fixtures/checkout-formatted.html | 5 +- test/fixtures/receipt.html | 4 +- test/page-text.test.ts | 66 ++++++++++++++++++ 12 files changed, 212 insertions(+), 10 deletions(-) create mode 100644 src/browser/page-text.ts create mode 100644 test/page-text.test.ts diff --git a/docs/DEMO-STRIPE.md b/docs/DEMO-STRIPE.md index 87dbf9c..c4292ba 100644 --- a/docs/DEMO-STRIPE.md +++ b/docs/DEMO-STRIPE.md @@ -46,7 +46,7 @@ The script is a plain MCP client: it uses only the server's tools, exactly as an 5. `type_text` the email, name, and ZIP. These are not secrets; they round-trip visibly. 6. `fill_secret` three times: card number, expiry, CVC. 7. `snapshot` — all three card fields show `[REDACTED:secret-filled-field]`. -8. `click` Pay; `snapshot` until the receipt page replaces the form. +8. `click` Pay; `snapshot` until the receipt page replaces the form. The snapshot `text` shows the confirmation message. The result: a completed purchase in the Stripe test dashboard, and a transcript in which the card number never appears. diff --git a/site/reference.html b/site/reference.html index 0bbad9e..8a54112 100644 --- a/site/reference.html +++ b/site/reference.html @@ -54,7 +54,8 @@

    MCP operations

    None Returns interactive elements with IDs for subsequent - click/type/fill operations. Filled values are redacted. + click/type/fill operations, and the page's headings and alert or + status text. Filled values are redacted. diff --git a/site/reference.md b/site/reference.md index abab9cd..4986761 100644 --- a/site/reference.md +++ b/site/reference.md @@ -12,7 +12,7 @@ The tool surface is intentionally small. All results pass through the server's r | ----------------------- | --------------------------------------------- | ------------------------------------------------------------------------------------------------ | | `list_secret_refs` | None | Opaque IDs, names, and destination bindings. Never returns secret values. | | `navigate` | `url` | Navigates the broker-owned browser and returns its final URL and title. | -| `snapshot` | None | Returns interactive element IDs. Filled values are redacted. | +| `snapshot` | None | Returns interactive element IDs, headings, and alert text. Filled values are redacted. | | `click` | `element_uid` | Clicks an element from the most recent snapshot. | | `type_text` | `element_uid`, `text` | Types visible, non-secret text. Never use it for a credential. | | `fill_secret` | `secret_id` plus `element_uid`, or `fields` | Injects via CDP if the binding holds; says if each field kept it. May return `pending_approval`. | diff --git a/skills/secure-browser/SKILL.md b/skills/secure-browser/SKILL.md index bd209f9..0bb24b8 100644 --- a/skills/secure-browser/SKILL.md +++ b/skills/secure-browser/SKILL.md @@ -36,7 +36,7 @@ Snapshots and fills target the main frame only. Cross-origin iframe inputs, incl 4. Fill non-secret fields (email, name, ZIP) with `type_text`. These are visible in your transcript, which is fine — they are not secrets. 5. Fill secret fields with `fill_secret`. Single-value secrets take one `element_uid`. A secret whose binding lists `sbm:fields` holds a JSON payload (e.g. a card's number/expiry/cvc): pass `fields: [{key, element_uid}, …]` to fill all of them in one call — one export, one approval. 6. Check the `fill_secret` result: `confirmed: true` means every field holds its value. If a field has `matches: false`, `length: 0`, or `valid: false`, the page dropped or rejected the value; report it and do not submit. A later `snapshot` shows filled fields as `[REDACTED:secret-filled-field]`. -7. `click` the submit button, then `snapshot` (repeat with short waits) until the page moves on. +7. `click` the submit button, then `snapshot` (repeat with short waits) until the page moves on. Read the snapshot's `text` (headings and alert or status messages) to confirm success or see the error. ## Browser handles diff --git a/skills/secure-browser/references/tools.md b/skills/secure-browser/references/tools.md index ba1d894..da5f25f 100644 --- a/skills/secure-browser/references/tools.md +++ b/skills/secure-browser/references/tools.md @@ -42,7 +42,11 @@ No tool ever returns a secret's value. ## snapshot -No arguments. Returns `{ url, title, elements: [...] }` where each element has: +No arguments. Returns `{ url, title, text: { headings, alerts }, elements: [...] }`. + +`text` is a short summary of what the page says, for reading results and errors: `headings` holds up to 20 visible headings, and `alerts` up to 10 visible alert, status, and live regions (`role=alert`, `role=status`, `aria-live`, ``). Each entry is at most 200 characters. It is not the full page text. A secret the page shows as text is replaced with `[REDACTED:]`. + +Each element has: - `uid` — targeting handle for click/type_text/fill_secret; valid until the next navigation - `tag`, `type`, `name`, `label`, `text`, `value` as available diff --git a/src/browser/page-text.ts b/src/browser/page-text.ts new file mode 100644 index 0000000..a5ce8f8 --- /dev/null +++ b/src/browser/page-text.ts @@ -0,0 +1,85 @@ +import type { RedactionRegistry } from "@turnkey/browser-secrets"; +import type { Page } from "puppeteer-core"; + +/** + * A short text summary of the page, so an agent can read a result or an + * error ("Payment complete", "Your card was declined") that is not an + * interactive element. Only headings and alert, status and live regions, + * capped in count and length: not the page's full text. + */ +export type PageText = { + headings: string[]; + alerts: string[]; +}; + +const HEADINGS = "h1, h2, h3, h4, h5, h6, [role=heading]"; +const ALERTS = [ + "[role=alert]", + "[role=alertdialog]", + "[role=status]", + "[aria-live]:not([aria-live=off])", + "output", +].join(", "); + +const MAX_HEADINGS = 20; +const MAX_ALERTS = 10; +/** Characters per entry the agent sees. */ +const MAX_ENTRY = 200; +/** Characters per entry read from the page. Larger than MAX_ENTRY so the + * text is scrubbed before it is cut: a cut first could leave part of a + * secret that the value scan no longer recognizes. */ +const MAX_RAW_ENTRY = 4 * MAX_ENTRY; + +/** + * Reads headings and alert regions of the top-level document, in document + * order. Each entry goes through the value scan before it is cut to size. + * innerText leaves out form field values, so a filled field's value never + * gets here; a secret the page shows as text is replaced with its marker. + */ +export async function capturePageText( + page: Page, + registry: RedactionRegistry, +): Promise { + const raw = await page.evaluate( + (headings, alerts, maxHeadings, maxAlerts, maxRaw) => { + const visible = (el: Element) => { + const rect = el.getBoundingClientRect(); + return rect.width > 0 && rect.height > 0; + }; + const read = (selector: string, max: number) => { + const out: string[] = []; + for (const el of Array.from(document.querySelectorAll(selector))) { + if (out.length >= max) break; + // A region inside another one is already in its text. + if (el.parentElement?.closest(selector)) continue; + if (!visible(el)) continue; + const text = (el as HTMLElement).innerText + ?.replace(/\s+/g, " ") + .trim() + .slice(0, maxRaw); + if (text) out.push(text); + } + return out; + }; + return { + headings: read(headings, maxHeadings), + alerts: read(alerts, maxAlerts), + }; + }, + HEADINGS, + ALERTS, + MAX_HEADINGS, + MAX_ALERTS, + MAX_RAW_ENTRY, + ); + const clean = (text: string) => { + const scrubbed = registry.scrubText(text); + return scrubbed.length > MAX_ENTRY + ? `${scrubbed.slice(0, MAX_ENTRY - 1)}…` + : scrubbed; + }; + return { + headings: raw.headings.map(clean), + alerts: raw.alerts.map(clean), + }; +} diff --git a/src/browser/snapshot.ts b/src/browser/snapshot.ts index 7308c94..6b9acef 100644 --- a/src/browser/snapshot.ts +++ b/src/browser/snapshot.ts @@ -6,6 +6,7 @@ import { topLevelDocument, type TopLevelDocument, } from "./observe.js"; +import { capturePageText, type PageText } from "./page-text.js"; import type { BrowserSession, ResolvedElement } from "./session.js"; /** Agent-visible description of one interactive element. */ @@ -24,6 +25,8 @@ export type SnapshotElement = { export type PageSnapshot = { url: string; title: string; + /** Headings and alert or status text (see src/browser/page-text.ts). */ + text: PageText; elements: SnapshotElement[]; }; @@ -124,7 +127,12 @@ export async function captureSnapshot( tagSplitCopies(registry, top, shown); session.setElements(resolved); - return { url: page.url(), title: await page.title(), elements }; + return { + url: page.url(), + title: await page.title(), + text: await capturePageText(page, registry), + elements, + }; } /** diff --git a/src/tools/snapshot.ts b/src/tools/snapshot.ts index 653bd33..34e17de 100644 --- a/src/tools/snapshot.ts +++ b/src/tools/snapshot.ts @@ -7,8 +7,9 @@ export const snapshot = defineTool({ browser: true, description: "Snapshot of the current page's interactive elements with stable " + - "element uids for click/type/fill targeting. Values of fields that " + - "received a secret are structurally redacted.", + "element uids for click/type/fill targeting, plus the page's headings " + + "and alert or status text, to read results and errors. Values of " + + "fields that received a secret are structurally redacted.", inputSchema: {}, handler: async (ctx) => { // Structural redaction (tagged fields, password inputs) happens inside diff --git a/test/e2e.test.ts b/test/e2e.test.ts index 96737ed..b2a5a93 100644 --- a/test/e2e.test.ts +++ b/test/e2e.test.ts @@ -226,6 +226,33 @@ test("a new document does not inherit the old document's redaction tags", async ); }); +test("snapshot shows the receipt page's confirmation text", async () => { + // The previous test left a filled checkout form on the page. + const form = (await client.callTool("snapshot")).body as { + url: string; + text: { headings: string[] }; + elements: { uid: string; text?: string }[]; + }; + expect(form.url).toBe(`${FIXTURE_ORIGIN}/checkout`); + expect(form.text.headings).toEqual(["Checkout"]); + const pay = form.elements.find((e) => e.text === "Pay $9.99")!; + expect( + (await client.callTool("click", { element_uid: pay.uid })).isError, + ).toBe(false); + + type Snap = { url: string; text: { headings: string[]; alerts: string[] } }; + let receipt: Snap | undefined; + for (let i = 0; i < 50 && !receipt?.url.endsWith("/pay"); i++) { + await Bun.sleep(100); + receipt = (await client.callTool("snapshot")).body as Snap; + } + expect(receipt?.url).toBe(`${FIXTURE_ORIGIN}/pay`); + expect(receipt?.text).toEqual({ + headings: ["Payment complete"], + alerts: ["Your demo order is confirmed. Nothing was charged."], + }); +}); + test("redacts a card number the page reformats, re-renders and echoes", async () => { const refs = (await client.callTool("list_secret_refs")).body as { refs: { secretId: string; name?: string }[]; @@ -256,6 +283,7 @@ test("redacts a card number the page reformats, re-renders and echoes", async () }); const snap2 = (await client.callTool("snapshot")).body as { + text: { alerts: string[] }; elements: { uid: string; name?: string; text?: string; value?: string }[]; }; // The page replaced the number input, so it has a fresh, untagged uid. @@ -266,6 +294,10 @@ test("redacts a card number the page reformats, re-renders and echoes", async () expect(summary.value).toBe("[REDACTED:secret-filled-field]"); const pay = snap2.elements.find((e) => e.text?.startsWith("Pay"))!; expect(pay.text).toBe(`Pay $9.99 with [REDACTED:${card!.secretId}]`); + // The page also shows the number as status text. + expect(snap2.text.alerts).toEqual([ + `Card [REDACTED:${card!.secretId}] saved`, + ]); expect(client.transcript).not.toContain("4242 4242 4242 4242"); expect(client.transcript).not.toMatch(/4242\D?4242\D?4242\D?4242/); diff --git a/test/fixtures/checkout-formatted.html b/test/fixtures/checkout-formatted.html index 64c0b05..587af45 100644 --- a/test/fixtures/checkout-formatted.html +++ b/test/fixtures/checkout-formatted.html @@ -8,7 +8,7 @@
    @@ -20,6 +20,7 @@ +

    diff --git a/test/fixtures/checkout-formatted.html b/test/fixtures/checkout-formatted.html index 587af45..08adfdd 100644 --- a/test/fixtures/checkout-formatted.html +++ b/test/fixtures/checkout-formatted.html @@ -7,7 +7,7 @@
    @@ -32,7 +32,7 @@ input.addEventListener("input", () => { input.value = group(input.value); }); - input.addEventListener("change", () => { + input.addEventListener("blur", () => { const fresh = input.cloneNode(); fresh.value = input.value; input.replaceWith(fresh); diff --git a/test/inject.test.ts b/test/inject.test.ts new file mode 100644 index 0000000..4db7ab0 --- /dev/null +++ b/test/inject.test.ts @@ -0,0 +1,54 @@ +import { expect, test } from "bun:test"; + +import { valueMatches } from "../src/browser/inject.js"; + +/** How a fill's read-back decides that a field holds the secret. */ + +test("matches the exact value", () => { + expect(valueMatches("synthetic-password!", "synthetic-password!", true)).toBe( + true, + ); + expect(valueMatches("4242424242424242", "4242424242424242", false)).toBe( + true, + ); +}); + +test("a password the page changed does not match", () => { + // The page stripped the punctuation: a different password. + expect(valueMatches("synthetic-password!", "syntheticpassword", true)).toBe( + false, + ); + // Not even in a field that is not a password input. + expect(valueMatches("synthetic-password!", "syntheticpassword", false)).toBe( + false, + ); + expect(valueMatches("Pässword1", "Pässword1", false)).toBe(false); +}); + +test("a number the page grouped matches", () => { + expect(valueMatches("4242424242424242", "4242 4242 4242 4242", false)).toBe( + true, + ); + expect(valueMatches("4242424242424242", "4242-4242-4242-4242", false)).toBe( + true, + ); + expect(valueMatches("1228", "12/28", false)).toBe(true); + expect(valueMatches("4242 4242 4242 4242", "4242424242424242", false)).toBe( + true, + ); +}); + +test("grouping tolerance has limits", () => { + // Not in a password field. + expect(valueMatches("123456", "123 456", true)).toBe(false); + // Only separators may be added, not other characters. + expect(valueMatches("4242424242424242", "4242a4242b4242c4242", false)).toBe( + false, + ); + // Digits that differ do not match. + expect(valueMatches("4242424242424242", "4242 4242 4242 4241", false)).toBe( + false, + ); + expect(valueMatches("", "", false)).toBe(true); + expect(valueMatches("", " ", false)).toBe(false); +});