fix(core): re-host non-http(s) CORS iframe URLs instead of failing upload

[Shivanshu-07]

Problem

Percy CLI fails snapshot upload when a page includes an iframe whose src is a blob: URL:

[percy:core] Error: Invalid URL, scheme must be http or https: blob:https://www.example.com

Regression introduced by #2121, which moved CORS iframe processing into @percy/core. Reported in #2285 (works on 1.31.7, fails on 1.31.14+).

Root cause

processCorsIframesInDomSnapshot() in packages/core/src/utils.js creates an upload resource for every CORS iframe, keyed on the iframe's frameUrl:

const iframeResource = { url: frameUrlWithWidth, content: iframeSnapshot.html, mimetype: 'text/html' };

A blob: URL is an in-memory handle that only exists in the originating browser context — there is no endpoint to fetch, it is scoped to the creating document, and it is revoked on unload. Percy's renderer (a separate browser) can never resolve it. When the iframe src is blob: (also data:, about:), that URL is sent to the API as a resource-url; the API rejects any non-http(s) scheme (snapshot_service.rb → Invalid URL, scheme must be http or https), failing the entire snapshot upload.

Fix

@percy/dom already handles blob URLs correctly for stylesheets (serialize-cssom.js): it extracts the content and re-hosts it under a synthetic /__serialized__/ http(s) URL via resourceFromText. The CORS iframe path never applied that pattern — this PR brings it to parity.

For a non-http(s) frameUrl, buildSyntheticFrameResourceUrl() mints a synthetic http(s) URL from the captured iframe HTML:

• resolved against the page origin (options.url), with localhost/127.0.0.1 rewritten to render.percy.local — matching how serialized DOM resources are hosted;
• path /__serialized__/cors-iframe-<percyElementId>.html (percyElementId URL-encoded);
• used for both the upload resource url and the rewritten iframe src, so the renderer serves the captured content exactly as it does an ordinary cross-origin iframe.

The blob URL never reaches the API, the snapshot uploads successfully, and the iframe content is preserved and renderable. Frames without a percyElementId (where the src cannot be rewritten to the synthetic URL) are skipped so no resource is orphaned. http(s) frames are completely unchanged.

Tests

packages/core/test/utils.test.js:

• isHttpOrHttpsUrl, rewriteLocalhostURL, buildSyntheticFrameResourceUrl helpers (origin resolution, localhost rewrite, render.percy.local fallback, id encoding, null when no percyElementId)
• processCorsIframesInDomSnapshot: re-hosts a blob frameUrl (resource + src rewritten, no blob: left), re-hosts data/about against render.percy.local fallback, skips a non-http(s) frame with no percyElementId, mixed http+blob array

All 121 utils specs and all Snapshot › CORS iframe processing integration specs pass; lint clean; unreachable defensive catch marked with istanbul ignore.

Fixes PER-9574 / closes #2285

🤖 Generated with Claude Code

[prklm10]

Have we tested it? Please attach the logs of the build ran. As I want to know if the resource we are getting fetched properly. As this schema logic is written in snapshot_service. from there this error is coming.

[Shivanshu-07]

@prklm10 Tested end-to-end against prod by driving @percy/core with the exact SDK payload that triggers the bug — a domSnapshot whose corsIframes entry has a blob: frameUrl. Ran it twice against the same project: once on pre-fix master, once with this branch. Logs below.

Before the fix (pre-#2294 utils.js) — fails

[percy:core] Encountered an error uploading snapshot: PR2294 blob iframe repro
[percy:core] Error: Invalid URL, scheme must be http or https: blob:http://localhost:8000/3f1c0a2e-9b7d-4e21-9c44-blob-iframe-demo
[percy:core] Failure: Upload snapshot failure
[percy:core] Failure Reason: Some snapshot or component uploads failed.
[percy:core] Finalized build #918: .../builds/51128638

Build state (admin API): state: failed, total-snapshots: 0, failure-reason: no_snapshots. This is the exact error from #2285 — the blob: URL is sent to the API as a resource-url and snapshot_service.rb rejects the non-http(s) scheme, failing the whole snapshot.

With the fix — re-hosted, fetched, and accepted

[percy:core:utils]  Re-hosting non-http(s) corsIframes frameUrl blob:http://localhost:8000/3f1c0a2e-... as http://render.percy.local/__serialized__/cors-iframe-percy-blob-iframe-1.html
[percy:core:discovery] Handling request: http://render.percy.local/__serialized__/cors-iframe-percy-blob-iframe-1.html
[percy:client] Uploading 312B resource: http://render.percy.local/__serialized__/cors-iframe-percy-blob-iframe-1.html
[percy:client] Uploaded resource http://render.percy.local/__serialized__/cors-iframe-percy-blob-iframe-1.html
[percy:client] Resources uploaded: PR2294 blob iframe repro...
[percy:client] Finalized snapshot: PR2294 blob iframe repro...
[percy:core] Finalized build #919: .../builds/51128663

Build state (admin API): state: finished, total-snapshots: 1, failure-reason: none. No Invalid URL error.

To your specific question — yes, the resource is fetched properly: the [percy:core:discovery] Handling request: …/__serialized__/cors-iframe-…html line shows discovery serving the re-hosted iframe content, and [percy:client] Uploaded resource …/__serialized__/…html confirms it passed snapshot_service's http(s) scheme validation. The blob: URL never reaches the API; the captured iframe HTML is preserved and renderable under the synthetic /__serialized__/ http URL (same pattern @percy/dom uses for blob stylesheets).

[Shivanshu-07]

Claude Code PR Review

PR: #2294 • Head: f53f510 • Reviewers: correctness/logic + testing/quality (independent fallback reviewers; no in-scope project reviewers found)

Summary

Fixes a regression (#2285) where a CORS iframe with a non-http(s) src (blob:, data:, about:) caused the entire snapshot upload to fail with Invalid URL, scheme must be http or https. The fix re-hosts the captured iframe HTML under a synthetic http(s) /__serialized__/cors-iframe-<id>.html URL — mirroring @percy/dom's blob-stylesheet handling — and rewrites the iframe src to match. Also bundles a repo-wide CI security hardening commit (SHA-pin all GitHub Actions + least-privilege permissions: blocks, PER-8604/PER-8608).

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	Workflows use secrets.GITHUB_TOKEN/NPM_TOKEN; none hardcoded.
High	Security	Authentication/authorization checks present	N/A	No auth surface in scope.
High	Security	Input validation and sanitization	Pass	isHttpOrHttpsUrl validates scheme; non-http(s) frame URLs never reach the API. SHA-pins reduce supply-chain risk.
High	Security	No IDOR — resource ownership validated	N/A	Not applicable.
High	Security	No SQL injection (parameterized queries)	N/A	No DB access.
High	Correctness	Logic is correct, handles edge cases	Pass	Missing percyElementId/iframeData, URL parse failures, responsive width arrays, mixed http+blob arrays all handled. Verified live: blob iframe re-hosts and uploads.
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	URL parse failure → safe false; defensive catch is istanbul ignore with a working fallback.
High	Correctness	No race conditions or concurrency issues	Pass	Synchronous; mutates only the local domSnapshot.
Medium	Testing	New code has corresponding tests	Pass	All four new helpers + processCorsIframesInDomSnapshot covered with exact toEqual/URL assertions.
Medium	Testing	Error paths and edge cases tested	Pass	no-percyElementId skip (asserts blob: left intact), data/about fallback, mixed array, html has no blob:. Minor gaps noted below.
Medium	Testing	Existing tests still pass (no regressions)	Pass	Full @percy/core suite run against head — green.
Medium	Performance	No N+1 queries or unbounded data fetching	N/A	Not applicable.
Medium	Performance	Long-running tasks use background jobs	N/A	Not applicable.
Medium	Quality	Follows existing codebase patterns	Pass	Reuses @percy/dom's /__serialized__/ + rewriteLocalhostURL conventions.
Medium	Quality	Changes are focused (single concern)	Fail	Repo-wide CI security hardening (~11 workflows) bundled into a one-function bugfix; complicates bisect/revert. Low severity.
Low	Quality	Meaningful names, no dead code	Pass	buildSyntheticFrameResourceUrl, isHttpOrHttpsUrl are intention-revealing.
Low	Quality	Comments explain why, not what	Pass	Re-hosting rationale + skip decision well documented, cross-referencing @percy/dom.
Low	Quality	No unnecessary dependencies added	Pass	No new deps.

Findings

• File: packages/core/src/utils.js:63 (rewriteLocalhostURL)

• Severity: Medium

• Reviewer: testing/quality + correctness

• Issue: The regex /(http[s]{0,1}:\/\/)(localhost|127.0.0.1)[:\d+]*/ has unescaped dots (so 127x0y0z1 matches) and is unanchored at the host boundary (so localhost.evil.com → render.percy.local.evil.com). [:\d+]* is a character class, not ":+digits". Verified with node.

• Suggestion: /(https?:\/\/)(localhost|127\.0\.0\.1)(:\d+)?(?=[/?#]|$)/. Note: this is copied verbatim from @percy/dom/src/utils.js (the JSDoc says "Mirrors…"), so it is a pre-existing upstream quirk, not a regression, and the input here is Percy-generated or the customer's own origin — limited blast radius. Worth a "keep in sync with @percy/dom" comment or a shared helper rather than a blocker.

• File: packages/core/test/utils.test.js (skip-case fixture)

• Severity: Medium (test coverage)

• Reviewer: correctness

• Issue: The "skips a non-http(s) frame with no percyElementId" test uses an iframeSnapshot without .resources, so it doesn't lock in that child resources are also dropped when the frame is skipped.

• Suggestion: Add iframeSnapshot.resources: [...] to that fixture and assert they're not pushed — cheap insurance against a future refactor orphaning child resources.

• File: packages/core/test/utils.test.js

• Severity: Low

• Reviewer: testing/quality

• Issue: No negative tests for the regex over-match cases (localhost.evil.com, 127x0y0z1).

• Suggestion: Add them to pin intended behavior if the regex is tightened.

• File: PR scope

• Severity: Low

• Reviewer: testing/quality

• Issue: Repo-wide CI security hardening (PER-8604/PER-8608) is bundled with the PER-9574 bugfix.

• Suggestion: Split into two PRs, or at minimum keep them as the two clearly-separated commits they already are. Not blocking.

Note: A reviewer flagged version-bump.yml's create-PR job as possibly missing a permissions: block. Verified false — the file declares top-level permissions: contents: write + pull-requests: write at head, so the job retains its grants and is not broken by this PR.

---

Verdict: PASS