feat(sdk-utils): export canonical iframe-capture helpers (single source of truth)

[aryanku-dev]

Summary

Part of PER-7292. The CORS-iframe SDK family (Capybara, Cypress, Nightwatch, Playwright, Puppeteer, Selenium-*) each hand-copied the iframe scheme list + resolver helpers, which drifted into 4 divergent scheme lists and 3 depth defaults (flagged in the cross-SDK parity review). This promotes them to @percy/sdk-utils as the single source of truth so every SDK consumes one canonical implementation and can drop its local _iframe_shim.js / inlined copies.

New exports (packages/sdk-utils/src/index.js)

• UNSUPPORTED_IFRAME_SRCS — canonical 15-prefix scheme list (about:, data:, javascript:, blob:, vbscript:, file:, ws:/wss:/ftp:, chrome*, devtools:, edge:, opera:, view-source:).
• isUnsupportedIframeSrc(src) — startsWith, case-insensitive; missing/empty src → unsupported.
• normalizeIgnoreSelectors(value) — array | string | unset → clean string[].
• resolveMaxFrameDepth(options) — per-snapshot maxIframeDepth → global percy.config.snapshot.maxIframeDepth → clampIframeDepth (reuses the existing canonical 3 / 10 bounds, so it stays aligned with @percy/dom).
• resolveIgnoreSelectors(options) — per-snapshot wins, else global config; normalized.

Tests

New iframe capture helpers block in packages/sdk-utils/test/index.test.js covering the scheme list, case-insensitivity, normalization, depth clamp + global-config fallback, and selector resolution. Node suite: 169/169 specs pass.

Follow-up (separate PRs)

Once this ships in a published @percy/sdk-utils, the JS SDKs can delete their local _iframe_shim.js and inlined resolvers and import these instead.

🤖 Generated with Claude Code

[aryanku-dev]

Claude Code Review (automated) — 2 inline finding(s). Full report in the PR comment below. Verdict: Passed.

[aryanku-dev]

[Medium] resolveIgnoreSelectors ?? drops the legacy alias when the canonical key is an empty array

options.ignoreIframeSelectors ?? options.ignoreSelectors only falls through on null/undefined. For { ignoreIframeSelectors: [], ignoreSelectors: '.ad' }, the empty array is kept, ignoreSelectors is never consulted, and .ad is silently dropped (the resolver then falls through to global config). Narrow trigger and arguably intentional ?? semantics — non-blocking.

Suggestion: branch on === undefined if the legacy alias should win when the canonical key is empty, and add a test for the { ignoreIframeSelectors: [], ignoreSelectors: '.ad' } case; otherwise add a one-line comment noting the behavior is intentional.

Reviewer: stack:code-reviewer

[aryanku-dev]

[Low] s.length reads as a truthy check rather than non-empty-string intent

Suggested change

	if (Array.isArray(value)) return value.filter(s => typeof s === 'string' && s.length);
	if (Array.isArray(value)) return value.filter(s => typeof s === 'string' && s.length > 0);

Reviewer: stack:code-reviewer

[aryanku-dev]

Claude Code PR Review

PR: #2319 • Head: 4a490b0 • Reviewers: stack:code-reviewer

Summary

Promotes the iframe-capture helpers (UNSUPPORTED_IFRAME_SRCS, isUnsupportedIframeSrc, normalizeIgnoreSelectors, resolveMaxFrameDepth, resolveIgnoreSelectors) into @percy/sdk-utils as a single source of truth, replacing four divergent per-SDK copies. Adds a thorough iframe capture helpers test block covering scheme matching, selector normalization, depth clamping, and per-snapshot/global config precedence.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	No secrets; pure helper functions
High	Security	Authentication/authorization checks present	N/A	No auth surface
High	Security	Input validation and sanitization	Pass	isUnsupportedIframeSrc coerces/lowercases src; operates on DOM-normalized scheme strings
High	Security	No IDOR — resource ownership validated	N/A	No resource access
High	Security	No SQL injection (parameterized queries)	N/A	No DB access
High	Correctness	Logic is correct, handles edge cases	Pass	One narrow edge case in resolveIgnoreSelectors (Medium, below); core paths well-tested
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	Pure functions, defensive null/type guards
High	Correctness	No race conditions or concurrency issues	N/A	Synchronous pure helpers
Medium	Testing	New code has corresponding tests	Pass	All five exports covered, incl. clamp/default/global-fallback branches
Medium	Testing	Error paths and edge cases tested	Pass	Empty/null/wrong-type inputs covered; missing the {ignoreIframeSelectors:[], ignoreSelectors} combo
Medium	Testing	Existing tests still pass (no regressions)	Pass	Additive only; no existing behavior changed
Medium	Performance	No N+1 queries or unbounded data fetching	N/A	No I/O
Medium	Performance	Long-running tasks use background jobs	N/A	N/A
Medium	Quality	Follows existing codebase patterns	Pass	Mirrors existing clampIframeDepth/depth-constant conventions in the same file
Medium	Quality	Changes are focused (single concern)	Pass	One concern: centralize iframe helpers
Low	Quality	Meaningful names, no dead code	Pass	Minor: s.length truthy check (below)
Low	Quality	Comments explain why, not what	Pass	"Single source of truth" history comment is excellent; a couple of per-fn comments narrate what
Low	Quality	No unnecessary dependencies added	Pass	No new deps

Findings

• File: packages/sdk-utils/src/index.js:84

• Severity: Medium

• Reviewer: stack:code-reviewer

• Issue: options.ignoreIframeSelectors ?? options.ignoreSelectors uses ??, which only falls through on null/undefined. If a caller passes { ignoreIframeSelectors: [], ignoreSelectors: '.ad' }, the empty array is kept and the legacy ignoreSelectors alias is never consulted — .ad is silently dropped and the resolver falls through to global config instead. Narrow trigger (caller must set both keys, canonical one empty), non-crashing, and arguably intentional ?? semantics — hence Medium, not gating.

• Suggestion: If the legacy alias should win when the canonical key is empty/absent, branch on === undefined instead of ??, and add a test for the { ignoreIframeSelectors: [], ignoreSelectors: '.ad' } case. If the current behavior is intentional, a one-line comment would prevent future "is this a bug?" churn.

• File: packages/sdk-utils/src/index.js:65

• Severity: Low

• Reviewer: stack:code-reviewer

• Issue: value.filter(s => typeof s === 'string' && s.length) uses s.length as a truthy check; the intent ("non-empty string") reads more clearly as s.length > 0.

• Suggestion: value.filter(s => typeof s === 'string' && s.length > 0).

Raised by other reviewers (not independently confirmed)

• URL-encoded scheme bypass of isUnsupportedIframeSrc (src/index.js:53, raised by stack:code-reviewer as High). Not confirmed: browsers do not decode javascript%3A... into the javascript: scheme (the encoded colon means it is not parsed as a scheme, so nothing executes), and iframe src values read from the DOM (element.src) are already scheme-normalized. Adding decodeURIComponent would risk throwing and producing false matches. Treated as non-gating.
• Missing mailto:/tel: schemes (raised as Medium). A completeness suggestion, not a defect; the list mirrors the consolidated existing SDK copies. Worth a follow-up only if a downstream SDK actually encounters these in iframe src.
• Test relies on live percy binding (raised as Medium). The tests mutate utils.percy.config and rely on the module exporting a live reference; this works and afterEach resets it. Minor robustness note, not a defect.

---

Verdict: PASS — clean, well-tested centralization. One Medium edge case in resolveIgnoreSelectors alias precedence and a couple of Low nits are worth folding into a follow-up, but none are blocking.