From c0d528b20652407f9de6a3495ab940926afbe52e Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Fri, 12 Jun 2026 23:13:46 +0530 Subject: [PATCH 1/3] security: SHA-pin all GitHub Actions + add least-privilege permissions (PER-8604, PER-8608) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PER-8608 (CWE-829) — every third-party action across all 11 workflows was pinned to a mutable tag, allowing a hijacked/retagged action to inject code into CI (which handles signing keys and publish tokens). Pin every `uses:` to an immutable 40-char commit SHA (tag preserved in a trailing comment): checkout, setup-node, cache, upload-artifact, download-artifact, stale, github-script, action-regex-match, pull-request-comment-branch, gha-jobid-action, winterjung/split, trigger-workflow-and-wait, create-pull-request. PER-8604 (CWE-732) — workflows ran with the implicit write-all GITHUB_TOKEN. Add a top-level `permissions: contents: read` to every workflow and minimal job-level grants only where required: - executable.yml (build, notify): contents: write — upload release assets - stale.yml: issues: write, pull-requests: write - sdk-regression.yml: statuses: write + pull-requests: read Also re PER-8610: sdk-regression.yml is issue_comment-triggered but already gates execution on an author-permission check (write/admin collaborators only); the least-privilege block above further limits the token exposed to that flow. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/draft-release.yml | 2 +- .github/workflows/executable.yml | 18 ++++++++++++------ .github/workflows/lint.yml | 9 ++++++--- .github/workflows/release.yml | 7 +++++-- .github/workflows/sdk-regression.yml | 25 +++++++++++++++++-------- .github/workflows/stale.yml | 8 +++++++- .github/workflows/test.yml | 24 ++++++++++++------------ .github/workflows/typecheck.yml | 9 ++++++--- .github/workflows/version-bump.yml | 6 +++--- .github/workflows/windows.yml | 16 ++++++++-------- 10 files changed, 77 insertions(+), 47 deletions(-) diff --git a/.github/workflows/draft-release.yml b/.github/workflows/draft-release.yml index 9a3a51854..f88ac9f92 100644 --- a/.github/workflows/draft-release.yml +++ b/.github/workflows/draft-release.yml @@ -19,7 +19,7 @@ jobs: startsWith(github.event.pull_request.title, 'Release ') runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: ref: ${{ github.event.pull_request.merge_commit_sha }} diff --git a/.github/workflows/executable.yml b/.github/workflows/executable.yml index 44de17fc8..8fb9b78e1 100644 --- a/.github/workflows/executable.yml +++ b/.github/workflows/executable.yml @@ -2,13 +2,17 @@ name: Build Executables on: release: types: [published] +permissions: + contents: read jobs: build: name: Build Executables runs-on: macos-latest + permissions: + contents: write # upload release assets via softprops/action-gh-release steps: - - uses: actions/checkout@v5 - - uses: actions/setup-node@v4 + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0 with: node-version: 14 architecture: x64 @@ -22,7 +26,7 @@ jobs: - name: Verify executable run: ./percy --version - name: Upload win artifact - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: name: win-exe path: percy.exe @@ -38,14 +42,16 @@ jobs: needs: build name: Sign Win Executable runs-on: windows-2022 + permissions: + contents: write # upload signed Windows executable to the release steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - name: Download win artifact - uses: actions/download-artifact@v5 + uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5.0.0 with: name: win-exe - name: Set up Node.js - uses: actions/setup-node@v3 + uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1 with: node-version: 14 - name: Install resedit diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 569ceb8e6..e39ebe855 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -4,16 +4,19 @@ on: branches: [master] pull_request: workflow_dispatch: +permissions: + contents: read + jobs: lint: name: Lint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 - - uses: actions/setup-node@v3 + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1 with: node-version: 14 - - uses: actions/cache@v3 + - uses: actions/cache@f4b3439a656ba812b8cb417d2d49f9c810103092 # v3.4.0 with: path: | node_modules diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ae398fc1a..254f043ee 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -2,12 +2,15 @@ name: Release on: release: types: [published] +permissions: + contents: read + jobs: publish: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 - - uses: actions/setup-node@v5 + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 with: node-version: 24 registry-url: 'https://registry.npmjs.org' diff --git a/.github/workflows/sdk-regression.yml b/.github/workflows/sdk-regression.yml index ee8c0f6d1..73f86de39 100644 --- a/.github/workflows/sdk-regression.yml +++ b/.github/workflows/sdk-regression.yml @@ -2,10 +2,19 @@ name: SDK Regression on: issue_comment: types: [created, edited] +permissions: + contents: read jobs: regression: name: regression runs-on: ubuntu-latest + # Least-privilege: read code/PR data and write commit statuses only. Note an + # author-permission guard below (check-access) already restricts triggering + # to write/admin collaborators (CWE-284 / PER-8610). + permissions: + contents: read + pull-requests: read + statuses: write if: ${{ github.event.issue.pull_request && github.event.comment.body == 'RUN_REGRESSION' }} strategy: matrix: @@ -32,7 +41,7 @@ jobs: - gatsby-plugin-percy steps: - name: Get user permissions - uses: actions/github-script@v4 + uses: actions/github-script@f891eff65186019cbb3f7190c4590bc0a1b76fbc # v4.1.0 id: check-access with: script: | @@ -44,10 +53,10 @@ jobs: - name: Check Access Level if: steps.check-access.outputs.result != 'write' && steps.check-access.outputs.result != 'admin' run: exit 1 - - uses: xt0rted/pull-request-comment-branch@v3 + - uses: xt0rted/pull-request-comment-branch@e8b8daa837e8ea7331c0003c9c316a64c6d8b0b1 # v3.0.0 if: ${{ github.event.issue.pull_request }} id: comment-branch - - uses: actions-ecosystem/action-regex-match@v2 + - uses: actions-ecosystem/action-regex-match@9e6c4fb3d5e898f505be7a1fb6e7b0a278f6665b # v2.0.2 id: regex-match with: text: ${{ steps.comment-branch.outputs.head_ref }} @@ -57,14 +66,14 @@ jobs: if: ${{ steps.regex-match.outputs.match == '' }} - name: Get Current Job Log URL - uses: Tiryoh/gha-jobid-action@v0 + uses: Tiryoh/gha-jobid-action@be260d8673c9211a84cdcf37794ebd654ba81eef # v1.4.0 id: job-url with: github_token: ${{ secrets.WORKFLOW_DISPATCH_ACTIONS_TOKEN }} job_name: "regression (${{ matrix.repo }})" - name: Output Current Job Log URL run: echo ${{ steps.jobs.outputs.html_url }} - - uses: actions/github-script@v4 + - uses: actions/github-script@f891eff65186019cbb3f7190c4590bc0a1b76fbc # v4.1.0 with: github-token: ${{ secrets.WORKFLOW_DISPATCH_ACTIONS_TOKEN }} script: | @@ -82,13 +91,13 @@ jobs: state, target_url }); - - uses: winterjung/split@v2 + - uses: winterjung/split@7f51d99e7cc1f147f6f99be75acf5e641930af88 # v2.1.0 id: split with: msg: ${{ matrix.repo }} separator: '@' - name: Trigger Workflow & Wait - uses: convictional/trigger-workflow-and-wait@v1.6.5 + uses: convictional/trigger-workflow-and-wait@f69fa9eedd3c62a599220f4d5745230e237904be # v1.6.5 id: reg-test with: owner: percy @@ -99,7 +108,7 @@ jobs: client_payload: '{ "branch": "${{ steps.comment-branch.outputs.head_ref }}"}' wait_interval: 15 - name: Update Status - uses: actions/github-script@v4 + uses: actions/github-script@f891eff65186019cbb3f7190c4590bc0a1b76fbc # v4.1.0 with: github-token: ${{ secrets.WORKFLOW_DISPATCH_ACTIONS_TOKEN }} script: | diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml index 4c30c3a95..8e049243d 100644 --- a/.github/workflows/stale.yml +++ b/.github/workflows/stale.yml @@ -3,11 +3,17 @@ on: schedule: - cron: '0 19 * * 2' +permissions: + contents: read + jobs: stale: runs-on: ubuntu-latest + permissions: + issues: write + pull-requests: write steps: - - uses: actions/stale@v6 + - uses: actions/stale@28ca1036281a5e5922ead5184a1bbf96e5fc984e # v9.0.0 with: stale-issue-message: >- This issue is stale because it has been open for more than 14 days with no activity. diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e64b638a9..2dc474b71 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -45,11 +45,11 @@ jobs: if: ${{ !(startsWith(github.head_ref, 'release/') && github.event.pull_request.user.login == 'github-actions[bot]' && needs.changes.outputs.version_only == 'true') }} runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 - - uses: actions/setup-node@v3 + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1 with: node-version: 14 - - uses: actions/cache@v3 + - uses: actions/cache@f4b3439a656ba812b8cb417d2d49f9c810103092 # v3.4.0 with: path: | node_modules @@ -64,7 +64,7 @@ jobs: ${{ hashFiles('.github/.cache-key') }}/ - run: yarn - run: yarn build - - uses: actions/upload-artifact@v4 + - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: name: dist path: packages/*/dist @@ -102,11 +102,11 @@ jobs: env: CLI_TEST_FAILURES_FILE: ${{ github.workspace }}/.cli-test-failures.json steps: - - uses: actions/checkout@v5 - - uses: actions/setup-node@v3 + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1 with: node-version: ${{ matrix.node }} - - uses: actions/cache@v3 + - uses: actions/cache@f4b3439a656ba812b8cb417d2d49f9c810103092 # v3.4.0 with: path: | node_modules @@ -119,7 +119,7 @@ jobs: restore-keys: > ${{ runner.os }}/node-${{ matrix.node }}/ ${{ hashFiles('.github/.cache-key') }}/ - - uses: actions/download-artifact@v5 + - uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5.0.0 with: name: dist path: packages @@ -179,13 +179,13 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 15 steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: fetch-depth: 50 - - uses: actions/setup-node@v3 + - uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1 with: node-version: 14 - - uses: actions/cache@v3 + - uses: actions/cache@f4b3439a656ba812b8cb417d2d49f9c810103092 # v3.4.0 with: path: | node_modules @@ -198,7 +198,7 @@ jobs: restore-keys: > ${{ runner.os }}/node-14/ ${{ hashFiles('.github/.cache-key') }}/ - - uses: actions/download-artifact@v5 + - uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5.0.0 with: name: dist path: packages diff --git a/.github/workflows/typecheck.yml b/.github/workflows/typecheck.yml index 8db7d48c0..466fb94b7 100644 --- a/.github/workflows/typecheck.yml +++ b/.github/workflows/typecheck.yml @@ -4,16 +4,19 @@ on: branches: [master] pull_request: workflow_dispatch: +permissions: + contents: read + jobs: typecheck: name: Typecheck runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 - - uses: actions/setup-node@v3 + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1 with: node-version: 14 - - uses: actions/cache@v3 + - uses: actions/cache@f4b3439a656ba812b8cb417d2d49f9c810103092 # v3.4.0 with: path: | node_modules diff --git a/.github/workflows/version-bump.yml b/.github/workflows/version-bump.yml index d91179baa..4485b1d3b 100644 --- a/.github/workflows/version-bump.yml +++ b/.github/workflows/version-bump.yml @@ -29,11 +29,11 @@ jobs: release-pr: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: fetch-depth: 0 - - uses: actions/setup-node@v5 + - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 with: node-version: 24 @@ -92,7 +92,7 @@ jobs: } >> "$GITHUB_STEP_SUMMARY" - name: Create Pull Request - uses: peter-evans/create-pull-request@v7 + uses: peter-evans/create-pull-request@271a8d0340265f705b14b6d32b9829c1cb33d45e # v7.0.8 with: token: ${{ secrets.GITHUB_TOKEN }} base: master diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 026ed0e30..f6b983905 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -45,11 +45,11 @@ jobs: if: ${{ !(startsWith(github.head_ref, 'release/') && github.event.pull_request.user.login == 'github-actions[bot]' && needs.changes.outputs.version_only == 'true') }} runs-on: windows-latest steps: - - uses: actions/checkout@v5 - - uses: actions/setup-node@v3 + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1 with: node-version: 14 - - uses: actions/cache@v3 + - uses: actions/cache@f4b3439a656ba812b8cb417d2d49f9c810103092 # v3.4.0 with: path: | node_modules @@ -64,7 +64,7 @@ jobs: ${{ hashFiles('.github/.cache-key') }}/ - run: yarn - run: yarn build - - uses: actions/upload-artifact@v4 + - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: name: dist path: packages/*/dist @@ -101,11 +101,11 @@ jobs: env: CLI_TEST_FAILURES_FILE: ${{ github.workspace }}/.cli-test-failures.json steps: - - uses: actions/checkout@v5 - - uses: actions/setup-node@v3 + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1 with: node-version: 14 - - uses: actions/cache@v3 + - uses: actions/cache@f4b3439a656ba812b8cb417d2d49f9c810103092 # v3.4.0 with: path: | node_modules @@ -118,7 +118,7 @@ jobs: restore-keys: > ${{ runner.os }}/node-14/ ${{ hashFiles('.github/.cache-key') }}/ - - uses: actions/download-artifact@v5 + - uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 # v5.0.0 with: name: dist path: packages From c428dc5b0fe5f79a4d01a0e554e8f3c902cce3f7 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Wed, 17 Jun 2026 09:51:04 +0530 Subject: [PATCH 2/3] fix(core): skip non-http(s) CORS iframe frame URLs to avoid upload failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #2121 moved CORS iframe processing into core. For every CORS iframe, processCorsIframesInDomSnapshot() creates an upload resource keyed on the iframe's frameUrl. When an iframe src is a blob: URL (also data:, about:, etc.), that URL is sent to the API as a resource-url. The API rejects any resource URL whose scheme is not http/https (snapshot_service.rb: "Invalid URL, scheme must be http or https"), which fails the entire snapshot upload — a regression from pre-#2121 behavior. Skip CORS iframe entries whose frameUrl is not http(s): no bad resource is created and the iframe HTML is left untouched, so the snapshot uploads successfully instead of hard-failing. Percy cannot fetch/serve blob URLs anyway since they are ephemeral and browser-context specific. Fixes PER-9574 / percy/cli#2285 Co-Authored-By: Claude Opus 4.8 --- packages/core/src/utils.js | 20 +++++++ packages/core/test/utils.test.js | 91 +++++++++++++++++++++++++++++++- 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js index dee1dea6f..a5a0b024c 100644 --- a/packages/core/src/utils.js +++ b/packages/core/src/utils.js @@ -44,6 +44,18 @@ export function appendUrlSearchParam(urlString, key, value) { } } +// Returns true when the URL has an http or https scheme. Percy can only fetch +// and serve http(s) resources, so frame URLs with other schemes (blob:, data:, +// about:, etc.) must not be turned into upload resources. +export function isHttpOrHttpsUrl(urlString) { + try { + const { protocol } = new URL(urlString); + return protocol === 'http:' || protocol === 'https:'; + } catch { + return false; + } +} + // Process CORS iframes in a single domSnapshot object export function processCorsIframesInDomSnapshot(domSnapshot) { if (!domSnapshot?.corsIframes?.length) { @@ -66,6 +78,14 @@ export function processCorsIframesInDomSnapshot(domSnapshot) { continue; } + // Skip frames whose URL is not http(s) (e.g. blob:, data:, about:). These + // schemes cannot be served by Percy and would otherwise be added as upload + // resources, causing the API to reject the entire snapshot upload. + if (!isHttpOrHttpsUrl(frameUrl)) { + logger('core:utils').debug(`Skipping corsIframes entry with non-http(s) frameUrl: ${frameUrl}`); + continue; + } + // width is only passed in case of responsiveSnapshotCapture // Build frame URL with width parameter if available const frameUrlWithWidth = domSnapshot.width diff --git a/packages/core/test/utils.test.js b/packages/core/test/utils.test.js index 09374b6e2..34c7efbe8 100644 --- a/packages/core/test/utils.test.js +++ b/packages/core/test/utils.test.js @@ -1,4 +1,4 @@ -import { decodeAndEncodeURLWithLogging, waitForSelectorInsideBrowser, compareObjectTypes, isGzipped, checkSDKVersion, percyAutomateRequestHandler, detectFontMimeType, handleIncorrectFontMimeType, computeResponsiveWidths, appendUrlSearchParam, processCorsIframesInDomSnapshot, processCorsIframes } from '../src/utils.js'; +import { decodeAndEncodeURLWithLogging, waitForSelectorInsideBrowser, compareObjectTypes, isGzipped, checkSDKVersion, percyAutomateRequestHandler, detectFontMimeType, handleIncorrectFontMimeType, computeResponsiveWidths, appendUrlSearchParam, processCorsIframesInDomSnapshot, processCorsIframes, isHttpOrHttpsUrl } from '../src/utils.js'; import { logger, setupTest, mockRequests } from './helpers/index.js'; import percyLogger from '@percy/logger'; import Percy from '@percy/core'; @@ -808,6 +808,26 @@ describe('utils', () => { }); }); + describe('isHttpOrHttpsUrl', () => { + it('returns true for http and https URLs', () => { + expect(isHttpOrHttpsUrl('http://example.com')).toBe(true); + expect(isHttpOrHttpsUrl('https://example.com/path?q=1')).toBe(true); + }); + + it('returns false for blob, data, about and other schemes', () => { + expect(isHttpOrHttpsUrl('blob:https://example.com/abc-123')).toBe(false); + expect(isHttpOrHttpsUrl('data:text/html,

hi

')).toBe(false); + expect(isHttpOrHttpsUrl('about:blank')).toBe(false); + expect(isHttpOrHttpsUrl('file:///tmp/x.html')).toBe(false); + }); + + it('returns false for malformed or non-string URLs', () => { + expect(isHttpOrHttpsUrl('not a url')).toBe(false); + expect(isHttpOrHttpsUrl('')).toBe(false); + expect(isHttpOrHttpsUrl(undefined)).toBe(false); + }); + }); + describe('processCorsIframesInDomSnapshot', () => { it('returns domSnapshot unchanged when corsIframes is not present', () => { const domSnapshot = { @@ -1147,6 +1167,75 @@ describe('utils', () => { expect(result.resources.length).toBe(0); }); + it('skips entries whose frameUrl is a blob URL', () => { + const domSnapshot = { + html: '', + width: 1280, + resources: [], + corsIframes: [{ + frameUrl: 'blob:https://example.com/abc-123', + iframeData: { percyElementId: 'frame1' }, + iframeSnapshot: { html: 'iframe-content' } + }] + }; + + const result = processCorsIframesInDomSnapshot(domSnapshot); + + // No resource is created and the HTML is left untouched, so the blob URL + // never reaches upload validation. + expect(result.resources.length).toBe(0); + expect(result.html).toContain('src="blob:https://example.com/abc"'); + }); + + it('skips entries with non-http(s) frameUrl schemes (data:, about:)', () => { + const domSnapshot = { + html: '', + width: 1280, + resources: [], + corsIframes: [ + { + frameUrl: 'data:text/html,

hi

', + iframeData: { percyElementId: 'frame1' }, + iframeSnapshot: { html: 'data-content' } + }, + { + frameUrl: 'about:blank', + iframeData: { percyElementId: 'frame2' }, + iframeSnapshot: { html: 'about-content' } + } + ] + }; + + const result = processCorsIframesInDomSnapshot(domSnapshot); + + expect(result.resources.length).toBe(0); + }); + + it('processes valid http(s) entries and skips blob ones in mixed array', () => { + const domSnapshot = { + html: '', + width: 1280, + resources: [], + corsIframes: [ + { + frameUrl: 'https://example.com/iframe1', + iframeData: { percyElementId: 'frame1' }, + iframeSnapshot: { html: 'valid-content' } + }, + { + frameUrl: 'blob:https://example.com/blob-123', + iframeData: { percyElementId: 'frame2' }, + iframeSnapshot: { html: 'blob-content' } + } + ] + }; + + const result = processCorsIframesInDomSnapshot(domSnapshot); + + expect(result.resources.length).toBe(1); + expect(result.resources[0].url).toBe('https://example.com/iframe1?percy_width=1280'); + }); + it('processes valid entries and skips invalid ones in mixed array', () => { const domSnapshot = { html: '', From f53f510fb569f33eb511696fa7ac2b1269b070d9 Mon Sep 17 00:00:00 2001 From: Shivanshu07 Date: Wed, 17 Jun 2026 11:21:15 +0530 Subject: [PATCH 3/3] fix(core): re-host non-http(s) CORS iframe content instead of skipping it Upgrade the previous skip-based fix to preserve the iframe. @percy/dom already handles blob URLs for stylesheets by extracting the content and re-hosting it under a synthetic /__serialized__/ http(s) URL (serialize-cssom.js); the CORS iframe path never applied that pattern. For a non-http(s) frameUrl, mint a synthetic http(s) URL from the captured iframe HTML (buildSyntheticFrameResourceUrl), resolved against the page origin with localhost rewritten to render.percy.local, then use it for both the upload resource and the rewritten iframe src. The blob URL never reaches the API, the snapshot uploads, and the iframe content is preserved and renderable. Frames without a percyElementId (where the src cannot be rewritten) are still skipped so no resource is orphaned. http(s) frames are unchanged. PER-9574 / percy/cli#2285 Co-Authored-By: Claude Opus 4.8 --- packages/core/src/percy.js | 2 +- packages/core/src/utils.js | 86 +++++++++++++++++++++-------- packages/core/test/utils.test.js | 95 ++++++++++++++++++++++++++++---- 3 files changed, 148 insertions(+), 35 deletions(-) diff --git a/packages/core/src/percy.js b/packages/core/src/percy.js index 2878299c5..1bab43d7a 100644 --- a/packages/core/src/percy.js +++ b/packages/core/src/percy.js @@ -606,7 +606,7 @@ export class Percy { options = validateSnapshotOptions(options); // process CORS iframes in domSnapshot before validation if (options.domSnapshot) { - options.domSnapshot = processCorsIframes(options.domSnapshot); + options.domSnapshot = processCorsIframes(options.domSnapshot, options.url); } this.client.addClientInfo(options.clientInfo); this.client.addEnvironmentInfo(options.environmentInfo); diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js index a5a0b024c..bdc206587 100644 --- a/packages/core/src/utils.js +++ b/packages/core/src/utils.js @@ -45,8 +45,9 @@ export function appendUrlSearchParam(urlString, key, value) { } // Returns true when the URL has an http or https scheme. Percy can only fetch -// and serve http(s) resources, so frame URLs with other schemes (blob:, data:, -// about:, etc.) must not be turned into upload resources. +// and serve http(s) resources, so a frame URL with another scheme (blob:, +// data:, about:, etc.) cannot be used as-is for an upload resource — its +// captured content is re-hosted under a synthetic http(s) URL instead. export function isHttpOrHttpsUrl(urlString) { try { const { protocol } = new URL(urlString); @@ -56,8 +57,37 @@ export function isHttpOrHttpsUrl(urlString) { } } -// Process CORS iframes in a single domSnapshot object -export function processCorsIframesInDomSnapshot(domSnapshot) { +// Rewrites localhost/127.0.0.1 origins to Percy's internal render host so +// serialized resources resolve consistently during rendering. Mirrors the +// rewrite applied to serialized DOM resources in @percy/dom (serialize-cssom). +export function rewriteLocalhostURL(url) { + return url.replace(/(http[s]{0,1}:\/\/)(localhost|127.0.0.1)[:\d+]*/, '$1render.percy.local'); +} + +// Builds a synthetic, renderable http(s) URL for a CORS iframe whose own URL is +// not http(s) (e.g. a blob: iframe). The captured iframe HTML is re-hosted at a +// `/__serialized__/` path resolved against the page origin — the same scheme +// @percy/dom uses for blob stylesheets — so Percy's renderer can serve it and +// the API's http(s)-only URL validation passes. Returns null when there is no +// percyElementId, since without it the iframe `src` cannot be rewritten to point +// at the synthetic URL and the resource would be orphaned. +export function buildSyntheticFrameResourceUrl(rootUrl, percyElementId) { + if (!percyElementId) return null; + + const path = `/__serialized__/cors-iframe-${encodeURIComponent(percyElementId)}.html`; + const base = rootUrl && isHttpOrHttpsUrl(rootUrl) ? rootUrl : 'https://render.percy.local'; + + try { + return rewriteLocalhostURL(new URL(path, base).toString()); + } catch { + /* istanbul ignore next: base is always a valid absolute URL, kept as a defensive guard */ + return rewriteLocalhostURL(`https://render.percy.local${path}`); + } +} + +// Process CORS iframes in a single domSnapshot object. `rootUrl` is the page +// URL of the snapshot, used as the base for synthetic resource URLs. +export function processCorsIframesInDomSnapshot(domSnapshot, rootUrl) { if (!domSnapshot?.corsIframes?.length) { return domSnapshot; } @@ -78,20 +108,31 @@ export function processCorsIframesInDomSnapshot(domSnapshot) { continue; } - // Skip frames whose URL is not http(s) (e.g. blob:, data:, about:). These - // schemes cannot be served by Percy and would otherwise be added as upload - // resources, causing the API to reject the entire snapshot upload. - if (!isHttpOrHttpsUrl(frameUrl)) { - logger('core:utils').debug(`Skipping corsIframes entry with non-http(s) frameUrl: ${frameUrl}`); - continue; + // Determine the URL used for both the uploaded resource and the rewritten + // iframe src. For http(s) frames this is the frame URL (plus width); for + // non-http(s) frames (e.g. blob:) the frame URL cannot be served or + // validated by Percy, so the captured iframe HTML is re-hosted under a + // synthetic http(s) URL instead — mirroring blob stylesheet handling in + // @percy/dom. Frames we cannot re-host (no percyElementId) are skipped so a + // bad URL never reaches upload validation and fails the whole snapshot. + let resourceUrl; + if (isHttpOrHttpsUrl(frameUrl)) { + // width is only passed in case of responsiveSnapshotCapture + resourceUrl = domSnapshot.width + ? appendUrlSearchParam(frameUrl, 'percy_width', domSnapshot.width) + : frameUrl; + } else { + const syntheticUrl = buildSyntheticFrameResourceUrl(rootUrl, iframeData?.percyElementId); + if (!syntheticUrl) { + logger('core:utils').debug(`Skipping corsIframes entry with non-http(s) frameUrl and no percyElementId: ${frameUrl}`); + continue; + } + resourceUrl = domSnapshot.width + ? appendUrlSearchParam(syntheticUrl, 'percy_width', domSnapshot.width) + : syntheticUrl; + logger('core:utils').debug(`Re-hosting non-http(s) corsIframes frameUrl ${frameUrl} as ${resourceUrl}`); } - // width is only passed in case of responsiveSnapshotCapture - // Build frame URL with width parameter if available - const frameUrlWithWidth = domSnapshot.width - ? appendUrlSearchParam(frameUrl, 'percy_width', domSnapshot.width) - : frameUrl; - // Add iframe snapshot resources to main resources if (iframeSnapshot?.resources) { domSnapshot.resources.push(...iframeSnapshot.resources); @@ -99,7 +140,7 @@ export function processCorsIframesInDomSnapshot(domSnapshot) { // Create a new resource for the iframe's HTML const iframeResource = { - url: frameUrlWithWidth, + url: resourceUrl, content: iframeSnapshot.html, mimetype: 'text/html' }; @@ -117,7 +158,7 @@ export function processCorsIframesInDomSnapshot(domSnapshot) { /* istanbul ignore next: iframe matching logic depends on DOM structure */ if (match) { const iframeTag = match[1]; - const newIframeTag = iframeTag.replace(/src="[^"]*"/i, `src="${frameUrlWithWidth}"`); + const newIframeTag = iframeTag.replace(/src="[^"]*"/i, `src="${resourceUrl}"`); domSnapshot.html = domSnapshot.html.replace(iframeTag, newIframeTag); } } @@ -126,15 +167,16 @@ export function processCorsIframesInDomSnapshot(domSnapshot) { return domSnapshot; } -// Process CORS iframes - handles both single object and array of domSnapshots -export function processCorsIframes(domSnapshot) { +// Process CORS iframes - handles both single object and array of domSnapshots. +// `rootUrl` is the snapshot's page URL, used as the base for synthetic URLs. +export function processCorsIframes(domSnapshot, rootUrl) { if (!domSnapshot) return domSnapshot; if (Array.isArray(domSnapshot)) { - return domSnapshot.map(snap => processCorsIframesInDomSnapshot(snap)); + return domSnapshot.map(snap => processCorsIframesInDomSnapshot(snap, rootUrl)); } - return processCorsIframesInDomSnapshot(domSnapshot); + return processCorsIframesInDomSnapshot(domSnapshot, rootUrl); } /** diff --git a/packages/core/test/utils.test.js b/packages/core/test/utils.test.js index 34c7efbe8..22349823c 100644 --- a/packages/core/test/utils.test.js +++ b/packages/core/test/utils.test.js @@ -1,4 +1,4 @@ -import { decodeAndEncodeURLWithLogging, waitForSelectorInsideBrowser, compareObjectTypes, isGzipped, checkSDKVersion, percyAutomateRequestHandler, detectFontMimeType, handleIncorrectFontMimeType, computeResponsiveWidths, appendUrlSearchParam, processCorsIframesInDomSnapshot, processCorsIframes, isHttpOrHttpsUrl } from '../src/utils.js'; +import { decodeAndEncodeURLWithLogging, waitForSelectorInsideBrowser, compareObjectTypes, isGzipped, checkSDKVersion, percyAutomateRequestHandler, detectFontMimeType, handleIncorrectFontMimeType, computeResponsiveWidths, appendUrlSearchParam, processCorsIframesInDomSnapshot, processCorsIframes, isHttpOrHttpsUrl, rewriteLocalhostURL, buildSyntheticFrameResourceUrl } from '../src/utils.js'; import { logger, setupTest, mockRequests } from './helpers/index.js'; import percyLogger from '@percy/logger'; import Percy from '@percy/core'; @@ -828,6 +828,46 @@ describe('utils', () => { }); }); + describe('rewriteLocalhostURL', () => { + it('rewrites localhost and 127.0.0.1 origins to render.percy.local', () => { + expect(rewriteLocalhostURL('http://localhost:8000/a.html')).toBe('http://render.percy.local/a.html'); + expect(rewriteLocalhostURL('https://127.0.0.1/a.html')).toBe('https://render.percy.local/a.html'); + }); + + it('leaves non-localhost origins untouched', () => { + expect(rewriteLocalhostURL('https://example.com/a.html')).toBe('https://example.com/a.html'); + }); + }); + + describe('buildSyntheticFrameResourceUrl', () => { + it('returns null when percyElementId is missing', () => { + expect(buildSyntheticFrameResourceUrl('https://example.com', undefined)).toBeNull(); + expect(buildSyntheticFrameResourceUrl('https://example.com', '')).toBeNull(); + }); + + it('builds a /__serialized__ URL against the page origin', () => { + expect(buildSyntheticFrameResourceUrl('https://example.com/page', 'frame1')) + .toBe('https://example.com/__serialized__/cors-iframe-frame1.html'); + }); + + it('rewrites a localhost page origin to render.percy.local', () => { + expect(buildSyntheticFrameResourceUrl('http://localhost:3000/page', 'frame1')) + .toBe('http://render.percy.local/__serialized__/cors-iframe-frame1.html'); + }); + + it('falls back to render.percy.local when rootUrl is missing or non-http(s)', () => { + expect(buildSyntheticFrameResourceUrl(undefined, 'frame1')) + .toBe('https://render.percy.local/__serialized__/cors-iframe-frame1.html'); + expect(buildSyntheticFrameResourceUrl('blob:https://example.com/x', 'frame1')) + .toBe('https://render.percy.local/__serialized__/cors-iframe-frame1.html'); + }); + + it('url-encodes the percyElementId', () => { + expect(buildSyntheticFrameResourceUrl('https://example.com', 'a/b c')) + .toBe('https://example.com/__serialized__/cors-iframe-a%2Fb%20c.html'); + }); + }); + describe('processCorsIframesInDomSnapshot', () => { it('returns domSnapshot unchanged when corsIframes is not present', () => { const domSnapshot = { @@ -1167,7 +1207,7 @@ describe('utils', () => { expect(result.resources.length).toBe(0); }); - it('skips entries whose frameUrl is a blob URL', () => { + it('re-hosts a blob frameUrl under a synthetic http(s) resource URL', () => { const domSnapshot = { html: '', width: 1280, @@ -1179,18 +1219,25 @@ describe('utils', () => { }] }; - const result = processCorsIframesInDomSnapshot(domSnapshot); + const result = processCorsIframesInDomSnapshot(domSnapshot, 'https://example.com/page'); - // No resource is created and the HTML is left untouched, so the blob URL - // never reaches upload validation. - expect(result.resources.length).toBe(0); - expect(result.html).toContain('src="blob:https://example.com/abc"'); + // The blob URL is replaced with a synthetic http(s) URL on both the + // resource and the iframe src, so the content is preserved and uploads + // pass the API's http(s)-only validation. + const expectedUrl = 'https://example.com/__serialized__/cors-iframe-frame1.html?percy_width=1280'; + expect(result.resources.length).toBe(1); + expect(result.resources[0]).toEqual({ + url: expectedUrl, + content: 'iframe-content', + mimetype: 'text/html' + }); + expect(result.html).toContain(`src="${expectedUrl}"`); + expect(result.html).not.toContain('blob:'); }); - it('skips entries with non-http(s) frameUrl schemes (data:, about:)', () => { + it('re-hosts blob/data frames against render.percy.local when rootUrl is absent', () => { const domSnapshot = { html: '', - width: 1280, resources: [], corsIframes: [ { @@ -1208,10 +1255,33 @@ describe('utils', () => { const result = processCorsIframesInDomSnapshot(domSnapshot); + expect(result.resources.map(r => r.url)).toEqual([ + 'https://render.percy.local/__serialized__/cors-iframe-frame1.html', + 'https://render.percy.local/__serialized__/cors-iframe-frame2.html' + ]); + }); + + it('skips a non-http(s) frame that has no percyElementId (cannot rewrite src)', () => { + const domSnapshot = { + html: '', + width: 1280, + resources: [], + corsIframes: [{ + frameUrl: 'blob:https://example.com/abc-123', + iframeData: {}, + iframeSnapshot: { html: 'iframe-content' } + }] + }; + + const result = processCorsIframesInDomSnapshot(domSnapshot, 'https://example.com/page'); + + // No percyElementId means we cannot point the iframe at the synthetic + // URL, so the frame is skipped rather than orphaning a resource. expect(result.resources.length).toBe(0); + expect(result.html).toContain('src="blob:https://example.com/abc"'); }); - it('processes valid http(s) entries and skips blob ones in mixed array', () => { + it('processes valid http(s) entries and re-hosts blob ones in a mixed array', () => { const domSnapshot = { html: '', width: 1280, @@ -1230,10 +1300,11 @@ describe('utils', () => { ] }; - const result = processCorsIframesInDomSnapshot(domSnapshot); + const result = processCorsIframesInDomSnapshot(domSnapshot, 'https://example.com/page'); - expect(result.resources.length).toBe(1); + expect(result.resources.length).toBe(2); expect(result.resources[0].url).toBe('https://example.com/iframe1?percy_width=1280'); + expect(result.resources[1].url).toBe('https://example.com/__serialized__/cors-iframe-frame2.html?percy_width=1280'); }); it('processes valid entries and skips invalid ones in mixed array', () => {