diff --git a/packages/core/src/security/actionFirewall.ts b/packages/core/src/security/actionFirewall.ts index 82fb01ee..f889e8b4 100644 --- a/packages/core/src/security/actionFirewall.ts +++ b/packages/core/src/security/actionFirewall.ts @@ -12,6 +12,9 @@ export const SECURITY_BLOCKED_CROSS_SITE_OPERATIONAL_SUBMIT = export const SECURITY_BLOCKED_UNTRUSTED_NAVIGATION = "Security policy blocked navigation to a host the caller did not name"; +export const SECURITY_BLOCKED_UNTRUSTED_DATA_FILL = + "Security policy blocked placing caller-provided data on a host the caller did not name"; + export type FillSource = "agent" | "user-approved"; export type ActionFirewallResult = @@ -246,6 +249,84 @@ function hasSensitiveAutocomplete(autocomplete: string | null): boolean { return tokens.some((token) => SENSITIVE_AUTOCOMPLETE_TOKENS.has(token)); } +/** + * Gate placing a caller-provided data value (via fill_user_data) into a field. + * + * Stricter than assessFill: caller data may be placed ONLY on a host the caller + * named (start host + trusted_hostnames; unsafe_mode overrides). Unlike assessFill + * there is NO operational-field exemption — an operational field (search box, + * combobox) on an untrusted host is still an attacker-readable sink for a secret, + * so caller data must not inherit that carve-out. A null host is never trusted, so + * it fails closed. + */ +export function assessDataFill(input: { + pageHostname: string | null; + firewall: FirewallConfig; +}): ActionFirewallResult { + if (input.firewall.unsafeMode) return { allowed: true }; + if (input.pageHostname !== null && input.firewall.trustedHostnames.has(input.pageHostname)) { + return { allowed: true }; + } + return { allowed: false, reason: SECURITY_BLOCKED_UNTRUSTED_DATA_FILL, isRecoverable: true }; +} + +/** + * Minimum length for a caller-data value to be treated as a secret worth redacting + * from page content re-entering the model context. Short values (country codes, + * single digits, "true") appear incidentally in legitimate page text and would cause + * false-positive redaction. This is a tuning knob; sub-threshold values are a known, + * documented gap. + */ +const MIN_SENSITIVE_VALUE_LENGTH = 6; + +/** + * Walk an arbitrary caller-supplied `data` object and collect the leaf values worth + * redacting from page content: string and numeric leaves at or above the length + * threshold. Booleans and null are ignored. `data` is typed `any` and may be built + * programmatically, so a WeakSet guards against cyclic references — redaction must + * fail safe, never stack-overflow the agent loop. + */ +export function collectSensitiveValues(data: unknown): Set { + const values = new Set(); + const seen = new WeakSet(); + const visit = (node: unknown): void => { + if (node === null || node === undefined) return; + if (typeof node === "string") { + const trimmed = node.trim(); + if (trimmed.length >= MIN_SENSITIVE_VALUE_LENGTH) values.add(trimmed); + } else if (typeof node === "number" || typeof node === "bigint") { + const asString = String(node); + if (asString.length >= MIN_SENSITIVE_VALUE_LENGTH) values.add(asString); + } else if (typeof node === "object") { + if (seen.has(node)) return; + seen.add(node); + if (Array.isArray(node)) { + for (const item of node) visit(item); + } else { + for (const value of Object.values(node as Record)) visit(value); + } + } + }; + visit(data); + return values; +} + +/** + * Replace every (case-insensitive) occurrence of each value with "[hidden]". + * Regex-special characters in a value are matched literally; empty values are ignored. + * Used to strip caller-data secrets from page content (snapshot, extracted markdown) + * before it re-enters the model context. + */ +export function redactSensitiveValues(text: string, values: Iterable): string { + let out = text; + for (const value of values) { + if (!value) continue; + const escaped = value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + out = out.replace(new RegExp(escaped, "gi"), "[hidden]"); + } + return out; +} + export class InvalidHostnameError extends Error { constructor(input: string, reason: string) { super(`Invalid hostname "${input}": ${reason}`); diff --git a/packages/core/src/tools/webActionTools.ts b/packages/core/src/tools/webActionTools.ts index f8c6ff51..0ee2e37d 100644 --- a/packages/core/src/tools/webActionTools.ts +++ b/packages/core/src/tools/webActionTools.ts @@ -24,6 +24,8 @@ import { assessFill, assessFormSubmission, assessNavigation, + assessDataFill, + redactSensitiveValues, extractHostname, type FirewallConfig, } from "../security/actionFirewall.js"; @@ -55,6 +57,12 @@ interface WebActionContext { * ever seeing the values — they are substituted directly at the browser sink. */ data?: Record; + /** + * Sensitive caller-data values (derived from `data`), precomputed by the agent + * and used to redact secrets from extracted page markdown before it reaches the + * model. Precomputed to avoid re-walking `data` on every extract. + */ + sensitiveValues?: ReadonlySet; } /** @@ -595,7 +603,14 @@ export function createWebActionTools(context: WebActionContext): ToolSet { }); // Get the page markdown content - const markdown = await context.browser.getMarkdown(); + let markdown = await context.browser.getMarkdown(); + + // Redact any caller-data secret that the page echoes back (e.g. a value + // just placed via fill_user_data), so it does not re-enter the model + // context through the extracted content. + if (context.sensitiveValues && context.sensitiveValues.size > 0) { + markdown = redactSensitiveValues(markdown, context.sensitiveValues); + } // Build extraction prompt const prompt = buildExtractionPrompt(description, markdown); @@ -704,15 +719,12 @@ export function createWebActionTools(context: WebActionContext): ToolSet { ]); const pageHostname = extractHostname(pageUrl); - // SAME firewall gate as the regular fill tool: filling caller data - // into a field on an untrusted host is blocked exactly like a - // freeform agent fill, so page-driven prompt injection cannot exfil. - const assessment = assessFill({ - field: metadata, - source: "agent", - pageHostname, - firewall: context.firewall, - }); + // Stricter than the regular fill gate: caller data may be placed only + // on a caller-trusted host, with NO operational-field exemption. An + // operational field (search box) on an untrusted host is still an + // attacker-readable sink for a secret, so caller data must not inherit + // assessFill's search-box carve-out. + const assessment = assessDataFill({ pageHostname, firewall: context.firewall }); if (!assessment.allowed) { emitNonInteractiveBlock(context, "freeform-fill", assessment.reason, pageHostname, []); return failedActionResult("fill_user_data", assessment.reason, context, ref); @@ -742,12 +754,11 @@ export function createWebActionTools(context: WebActionContext): ToolSet { }); // Record provenance so a following submit check can gate on it. Use - // agentFilledRefs/operationalRefs, NEVER approvedRefs — caller data is - // not user-approved per-field and must not bypass the submit gate. + // agentFilledRefs only — NEVER approvedRefs (caller data is not + // user-approved per-field) and NEVER operationalRefs (the operational + // exemption would let caller data submit cross-host; a secret is not + // operational input). context.agentFilledRefs.add(ref); - if (assessment.allowed && "operational" in assessment && assessment.operational) { - context.operationalRefs.add(ref); - } return { success: true, action: "fill_user_data", ref, key }; } catch (error) { diff --git a/packages/core/src/webAgent.ts b/packages/core/src/webAgent.ts index f7eb0c7b..80ba60bc 100644 --- a/packages/core/src/webAgent.ts +++ b/packages/core/src/webAgent.ts @@ -65,6 +65,8 @@ import { import { normalizeHostname, withTrustedStartHost, + collectSensitiveValues, + redactSensitiveValues, type FirewallConfig, } from "./security/actionFirewall.js"; @@ -270,6 +272,12 @@ export class WebAgent { private currentPage: { url: string; title: string } = { url: "", title: "" }; private currentIterationId: string = ""; private data: any = null; + /** + * Sensitive caller-data values to redact from page content (snapshot, extract + * markdown). Computed once from `data` — which is constant for a run — to avoid + * re-walking the payload on every snapshot. + */ + private sensitiveDataValues: ReadonlySet = new Set(); private abortSignal: AbortSignal | undefined = undefined; /** * The abort signal for the current iteration: the caller's abortSignal combined with @@ -579,6 +587,7 @@ export class WebAgent { allowFileUpload: this.allowFileUpload, llmProviderTimeoutMs: this.llmProviderTimeoutMs, data: this.data ?? undefined, + sensitiveValues: this.sensitiveDataValues, }); // Only include search tools if a search service was created @@ -1013,7 +1022,15 @@ export class WebAgent { this.truncateOldExternalContent(); const currentUrl = await this.browser.getUrl(); - const currentPageSnapshot = await this.browser.getTreeWithRefs(); + let currentPageSnapshot = await this.browser.getTreeWithRefs(); + // Redact caller-data secrets that the page echoes back (e.g. a value just + // placed via fill_user_data reflected in element.value): ARIA refs are + // regenerated every snapshot, so per-ref masking is unviable — redact the + // serialized string instead, before it enters the model context. The value + // set is precomputed (this.sensitiveDataValues) to avoid re-walking `data`. + if (this.sensitiveDataValues.size > 0) { + currentPageSnapshot = redactSensitiveValues(currentPageSnapshot, this.sensitiveDataValues); + } const compressedSnapshot = this.compressor.compress(currentPageSnapshot); if (this.debug) { @@ -1848,6 +1865,7 @@ export class WebAgent { private async initializeBrowserAndState(task: string, options: ExecuteOptions): Promise { this.clearInternalState(); this.data = options.data || null; + this.sensitiveDataValues = collectSensitiveValues(this.data); this.abortSignal = options.abortSignal || undefined; this.emit(WebAgentEventType.TASK_SETUP, { @@ -2031,6 +2049,7 @@ export class WebAgent { this.currentPage = { url: "", title: "" }; this.currentIterationId = ""; this.data = null; + this.sensitiveDataValues = new Set(); this.abortSignal = undefined; this.searchService = null; } diff --git a/packages/core/test/security/actionFirewall.test.ts b/packages/core/test/security/actionFirewall.test.ts index eec42745..fe9c4885 100644 --- a/packages/core/test/security/actionFirewall.test.ts +++ b/packages/core/test/security/actionFirewall.test.ts @@ -12,6 +12,10 @@ import { SECURITY_BLOCKED_UNAUTHORIZED_SUBMIT, SECURITY_BLOCKED_CROSS_SITE_OPERATIONAL_SUBMIT, SECURITY_BLOCKED_UNTRUSTED_NAVIGATION, + SECURITY_BLOCKED_UNTRUSTED_DATA_FILL, + assessDataFill, + collectSensitiveValues, + redactSensitiveValues, type FirewallConfig, } from "../../src/security/actionFirewall.js"; @@ -668,3 +672,109 @@ describe("assessFormSubmission same-site operational restriction", () => { expect(result.allowed).toBe(true); }); }); + +describe("assessDataFill", () => { + const untrusted: FirewallConfig = { trustedHostnames: new Set(), unsafeMode: false }; + + it("allows placing caller data on a trusted host", () => { + const r = assessDataFill({ + pageHostname: "example.com", + firewall: { trustedHostnames: new Set(["example.com"]), unsafeMode: false }, + }); + expect(r.allowed).toBe(true); + }); + + it("blocks placing caller data on an untrusted host", () => { + const r = assessDataFill({ pageHostname: "attacker.example", firewall: untrusted }); + expect(r.allowed).toBe(false); + if (r.allowed) throw new Error("expected block"); + expect(r.reason).toBe(SECURITY_BLOCKED_UNTRUSTED_DATA_FILL); + }); + + it("fails closed on a null host", () => { + const r = assessDataFill({ + pageHostname: null, + firewall: { trustedHostnames: new Set(["example.com"]), unsafeMode: false }, + }); + expect(r.allowed).toBe(false); + }); + + it("has no operational-field exemption (unlike assessFill): unaffected by field type", () => { + // assessDataFill takes no field — an operational field on an untrusted host is + // still blocked, which is the whole point vs. reusing assessFill. + const r = assessDataFill({ pageHostname: "attacker.example", firewall: untrusted }); + expect(r.allowed).toBe(false); + }); + + it("bypasses in unsafe mode", () => { + const r = assessDataFill({ + pageHostname: "attacker.example", + firewall: { trustedHostnames: new Set(), unsafeMode: true }, + }); + expect(r.allowed).toBe(true); + }); +}); + +describe("collectSensitiveValues", () => { + it("collects string/number leaves at or above the length threshold, recursing", () => { + const values = collectSensitiveValues({ + code: "CANARY-A1B2C3D9", + nested: { token: "supersecret" }, + list: ["item-one", 1234567], + }); + expect(values.has("CANARY-A1B2C3D9")).toBe(true); + expect(values.has("supersecret")).toBe(true); + expect(values.has("item-one")).toBe(true); + expect(values.has("1234567")).toBe(true); + }); + + it("ignores short values, booleans, and null", () => { + const values = collectSensitiveValues({ + a: "abc", + b: 42, + ok: true, + missing: null, + long: "abcdef", + }); + expect(values.has("abc")).toBe(false); + expect(values.has("42")).toBe(false); + expect(values.has("abcdef")).toBe(true); + }); + + it("returns an empty set for null/undefined", () => { + expect(collectSensitiveValues(null).size).toBe(0); + expect(collectSensitiveValues(undefined).size).toBe(0); + }); + + it("does not infinite-loop on a cyclic data structure (fails safe)", () => { + const cyclic: Record = { token: "supersecret" }; + cyclic.self = cyclic; + cyclic.list = [cyclic, "another-secret"]; + const values = collectSensitiveValues(cyclic); + expect(values.has("supersecret")).toBe(true); + expect(values.has("another-secret")).toBe(true); + }); +}); + +describe("redactSensitiveValues", () => { + it("replaces every occurrence with [hidden], case-insensitively", () => { + const out = redactSensitiveValues( + "code CANARY-A1B2C3D9 then canary-a1b2c3d9", + new Set(["CANARY-A1B2C3D9"]), + ); + expect(out).not.toMatch(/CANARY/i); + expect(out.match(/\[hidden\]/g)?.length).toBe(2); + }); + + it("leaves text unchanged when nothing matches", () => { + expect(redactSensitiveValues("nothing here", new Set(["SECRET123"]))).toBe("nothing here"); + }); + + it("treats regex-special characters literally", () => { + expect(redactSensitiveValues("a+b (c)", new Set(["a+b"]))).toBe("[hidden] (c)"); + }); + + it("ignores empty values", () => { + expect(redactSensitiveValues("keep", new Set([""]))).toBe("keep"); + }); +}); diff --git a/packages/core/test/tools/webActionTools.test.ts b/packages/core/test/tools/webActionTools.test.ts index 5263197a..7c94df9d 100644 --- a/packages/core/test/tools/webActionTools.test.ts +++ b/packages/core/test/tools/webActionTools.test.ts @@ -13,7 +13,7 @@ import { z } from "zod"; import { InvalidRefException, BrowserActionException } from "../../src/errors.js"; import { SECURITY_BLOCKED_UNTRUSTED_NAVIGATION } from "../../src/security/actionFirewall.js"; import { generateTextWithRetry } from "../../src/utils/retry.js"; -import { SECURITY_BLOCKED_UNAUTHORIZED_FILL } from "../../src/security/actionFirewall.js"; +import { SECURITY_BLOCKED_UNTRUSTED_DATA_FILL } from "../../src/security/actionFirewall.js"; // Mock the ai module vi.mock("ai", () => ({ @@ -1020,6 +1020,22 @@ describe("Web Action Tools", () => { expect((result as any).extractedData).toContain("Extracted data: Important info"); }); + it("redacts caller-data secrets from the page markdown before the extractor sees it", async () => { + const SECRET = "super-secret-membership-42"; + vi.spyOn(mockBrowser, "getMarkdown").mockResolvedValue(`Your code is ${SECRET} — thanks!`); + const dataTools: any = createWebActionTools({ + ...context, + sensitiveValues: new Set([SECRET]), + }); + mockGenerateTextWithRetry.mockResolvedValueOnce({ text: "ok" } as any); + + await dataTools.extract.execute({ description: "read the page" }); + + const promptArg = (mockGenerateTextWithRetry.mock.calls[0][0] as any).prompt as string; + expect(promptArg).not.toContain(SECRET); + expect(promptArg).toContain("[hidden]"); + }); + it('wraps extractedData in with safety warning', async () => { mockGenerateTextWithRetry.mockResolvedValueOnce({ text: "Hello from the page", @@ -1438,12 +1454,43 @@ describe("Web Action Tools", () => { const result = await udTools.fill_user_data.execute({ ref: "E12", key: "membership_code" }); expect(result.success).toBe(false); - expect(result.error).toBe(SECURITY_BLOCKED_UNAUTHORIZED_FILL); + expect(result.error).toBe(SECURITY_BLOCKED_UNTRUSTED_DATA_FILL); expect(performActionSpy).not.toHaveBeenCalled(); expect(context.agentFilledRefs.has("E12")).toBe(false); expect(JSON.stringify(result)).not.toContain(SECRET); }); + it("blocks placing caller data into an OPERATIONAL field on an untrusted host (no assessFill carve-out)", async () => { + // A search box would be allowed by assessFill's operational exemption, but + // caller data must not leak into an operational field on an untrusted host. + const searchField: FieldMetadata = { + ref: "E13", + tagName: "input", + inputType: "search", + role: "searchbox", + name: "q", + label: "Search", + placeholder: "Search", + autocomplete: null, + isContentEditable: false, + formId: "search", + formAction: "https://untrusted.com/search", + formMethod: "get", + }; + mockBrowser.url = "https://untrusted.com/search"; + mockBrowser.fieldMetadata.set("E13", searchField); + const performActionSpy = vi.spyOn(mockBrowser, "performAction"); + + const result = await udTools.fill_user_data.execute({ ref: "E13", key: "membership_code" }); + + expect(result.success).toBe(false); + expect(result.error).toBe(SECURITY_BLOCKED_UNTRUSTED_DATA_FILL); + expect(performActionSpy).not.toHaveBeenCalled(); + expect(context.agentFilledRefs.has("E13")).toBe(false); + expect(context.operationalRefs.has("E13")).toBe(false); + expect(JSON.stringify(result)).not.toContain(SECRET); + }); + it("fills on a trusted host, masks the value, and records provenance not approval", async () => { mockBrowser.url = "https://example.com/signup"; mockBrowser.fieldMetadata.set("E12", emailField);