Close caller-data re-injection via page snapshot + tighten fill_user_data gate (#607)#608
Merged
Merged
Conversation
…gate Follow-up to #596 (tracked in #607). #596 hides caller `data` from the prompt and the tool result, but two channels remained open: 1. Re-injection via the page snapshot. After fill_user_data places a value, the DOM element.value is serialized back into the model context by the next snapshot (ariaSnapshot.ts). ARIA refs are regenerated every snapshot, so per-ref masking is unviable — instead redact known caller-data values from the serialized snapshot string in addPageSnapshot (and from extract's page markdown), before they reach the model. Robust to ref churn and React reconciliation. Adds collectSensitiveValues + redactSensitiveValues. 2. fill_user_data reused assessFill, whose operational-field exemption let a caller secret be placed into (and, via operationalRefs, submitted from) an operational field on an untrusted host. Gate it with a dedicated assessDataFill (trusted-host only, no operational exemption) and stop recording operationalRefs. Known residual: sub-6-char values are not redacted (length threshold, to avoid false-positive redaction of common short page text). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens pilo-core against re-introducing caller-provided data values into the model context after they’ve been placed into the DOM, and tightens the fill_user_data firewall gate so caller data can only be filled on caller-trusted hosts.
Changes:
- Redacts known caller-data values from page snapshots (
WebAgent.addPageSnapshot) and fromextract’s page markdown before either reaches the model. - Introduces
assessDataFilland updatesfill_user_datato use it (trusted-host-only, no operational-field exemption; does not recordoperationalRefs). - Adds/updates unit tests covering the redaction helpers, the new gate, and
extractredaction behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/security/actionFirewall.ts | Adds assessDataFill, plus collectSensitiveValues/redactSensitiveValues helpers and a new block reason string. |
| packages/core/src/webAgent.ts | Redacts caller-data values from serialized page snapshots before compression/model-context insertion. |
| packages/core/src/tools/webActionTools.ts | Redacts extract markdown and tightens fill_user_data gating to trusted hosts only; stops recording operationalRefs. |
| packages/core/test/security/actionFirewall.test.ts | Adds unit tests for assessDataFill and the sensitive-value collection/redaction helpers. |
| packages/core/test/tools/webActionTools.test.ts | Adds tests ensuring extract prompt input is redacted and fill_user_data is blocked on untrusted operational fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+282
to
+306
| /** | ||
| * 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. Input is a parsed JSON request body, so | ||
| * no cycle handling is needed. | ||
| */ | ||
| export function collectSensitiveValues(data: unknown): Set<string> { | ||
| const values = new Set<string>(); | ||
| 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 (Array.isArray(node)) { | ||
| for (const item of node) visit(item); | ||
| } else if (typeof node === "object") { | ||
| for (const value of Object.values(node as Record<string, unknown>)) visit(value); | ||
| } | ||
| }; | ||
| visit(data); | ||
| return values; | ||
| } |
Comment on lines
+1023
to
+1027
| if (this.data) { | ||
| currentPageSnapshot = redactSensitiveValues( | ||
| currentPageSnapshot, | ||
| collectSensitiveValues(this.data), | ||
| ); |
lmorchard
marked this pull request as draft
July 20, 2026 19:19
Collaborator
Author
|
Calling this a draft for now. Going to address copilot review and then give this at least one run through evals |
- collectSensitiveValues: guard against cyclic `data` (typed `any`, may be built programmatically) with a WeakSet, so redaction fails safe instead of stack-overflowing the agent loop. Adds a cyclic-input test. - Compute the sensitive-value set once (WebAgent.sensitiveDataValues, set when `data` is assigned, cleared in clearInternalState) instead of re-walking `data` on every snapshot. Thread it into the tool context so `extract` reuses it too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Addressed both Copilot comments in 7db5b91:
Full pilo-core suite green (944), typecheck + prettier clean, no schema drift. |
srbiv
approved these changes
Jul 21, 2026
lmorchard
marked this pull request as ready for review
July 21, 2026 16:38
Collaborator
Author
|
80% partial eval pass, nothing obvious broken 
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #596, closes #607. #596 hides caller
datafrom the prompt and tool results, but two channels remained open for a value once it's placed viafill_user_data:element.valueis serialized back into the model context by the next page snapshot (ariaSnapshot.ts). ARIA refs regenerate every snapshot, so per-ref masking is unviable — this redacts known caller-data values from the serialized snapshot string (inaddPageSnapshot, before compression) and from theextracttool's page markdown, before either reaches the model. Robust to ref churn and React reconciliation. AddscollectSensitiveValues+redactSensitiveValues.fill_user_datareusedassessFill, whose operational-field exemption let a secret be placed into (and, viaoperationalRefs, submitted from) an operational field on an untrusted host. Replaced with a dedicatedassessDataFill(trusted-host-only, no operational exemption); no longer recordsoperationalRefs.Scope / known residuals
This narrows, but does not fully close, the re-injection surface:
Testing
assessDataFill/collectSensitiveValues/redactSensitiveValuesunit tests.fill_user_data: blocked on untrusted host including an operational field (noperformAction, nooperationalRefs, no leak); trusted-host fill unchanged.extract: page markdown redacted before the extractor sees it.🤖 Generated with Claude Code