security: reject code-bearing snapshot fields (execute/domTransformation) from the local API (PER-8607, PER-8613)#2280
Open
Shivanshu-07 wants to merge 1 commit into
Conversation
…-8607, PER-8613) The local /percy/snapshot endpoint is unauthenticated, so accepting `execute` (run via page.eval) and `domTransformation` (passed to window.eval) from a network request body lets any local caller inject arbitrary JavaScript into the — possibly authenticated — page being snapshotted (CWE-94, CVSS 8.8). Add stripRemoteScriptFields(): the POST /percy/snapshot handler now strips `execute` and `domTransformation` (recursively, incl. additionalSnapshots[]) from the request body before it reaches percy.snapshot(), logging a warning when it does. The config-file / CLI path (`percy snapshot`) calls percy.snapshot() directly and never traverses this route, so legitimate config-sourced execute/domTransformation continue to work. Trusted programmatic callers can opt back in with PERCY_ALLOW_REMOTE_SCRIPTS=true. Mirrors the existing stripBlockedConfigFields control on /percy/config. Adds unit tests covering removal, nested removal, the opt-in override, and non-object bodies. NOTE: this changes default behaviour for any caller that POSTs a URL snapshot with execute/domTransformation to the local API expecting server-side execution — those fields are now ignored unless the opt-in env is set. Flagged for review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
Third focused percy-cli security PR — two High-severity code-injection findings (CVSS 8.8, deadline 2026-06-16).
domTransformationstring from POST body reacheswindow.evalin Chromiumexecutehooks)Root cause
The local
/percy/snapshotendpoint is unauthenticated.execute(run viapage.eval) anddomTransformation(passed towindow.evalin@percy/dom) flow straight fromreq.bodyinto the browser context of the page being snapshotted. Since Percy often snapshots authenticated/staging pages, an injected script can exfiltrate cookies/tokens or SSRF internal services.Fix
Added
stripRemoteScriptFields()and wired it into thePOST /percy/snapshothandler:executeanddomTransformationare removed (recursively, includingadditionalSnapshots[]) from the network request body before it reachespercy.snapshot(), with a warning logged.Why this is safe for legitimate use: the config-file / CLI path (
percy snapshot <url|sitemap>) callspercy.snapshot()directly and never passes through this HTTP route (confirmed inpackages/cli-snapshot/src/snapshot.js→percy.yield.snapshot(options)), so config-sourcedexecute/domTransformationare unaffected. This mirrors the existingstripBlockedConfigFieldscontrol already applied to/percy/config.Trusted programmatic callers that intentionally POST a URL +
execute/domTransformationto the local API can opt back in withPERCY_ALLOW_REMOTE_SCRIPTS=true.Verification
node --checkpasses; logic verified against the ticket's exploit payloads (top-level + nestedexecute/domTransformationremoved;url/name/additionalSnapshots[].namepreserved; original body not mutated; opt-in retains fields).api.test.js(describe('stripRemoteScriptFields')): removal, no-op + no-warn on clean bodies, opt-in override, non-object bodies.Closes PER-8607, PER-8613.
🤖 Generated with Claude Code