Repository navigation
Cross-origin iframe recording: origin-scoped collectFromOrigins / emitToOrigin - #1991
eoghanmurray wants to merge 5 commits into
Conversation
🦋 Changeset detectedLatest commit: d253958 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@rrweb/all
@rrweb/browser-client
@rrweb/packer
@rrweb/record
@rrweb/replay
rrdom
rrdom-nodejs
rrvideo
rrweb
rrweb-player
rrweb-snapshot
@rrweb/types
@rrweb/utils
@rrweb/rrweb-plugin-canvas-webrtc-record
@rrweb/rrweb-plugin-canvas-webrtc-replay
@rrweb/rrweb-plugin-console-record
@rrweb/rrweb-plugin-console-replay
@rrweb/rrweb-plugin-network-record
@rrweb/rrweb-plugin-network-replay
@rrweb/rrweb-plugin-sequential-id-record
@rrweb/rrweb-plugin-sequential-id-replay
commit: |
f2ccc6c to
d35a4c4
Compare
The single `recordCrossOriginIframes` switch conflated two independent, position-inferred roles and gave no control over trust: a collector accepted forwarded events from *any* embedded iframe (ads, untrusted widgets could inject fabricated events), and a child forwarded to `*` (any embedder could harvest the recording). Split it into two origin-scoped options, each self-enabling by presence: - `collectFromOrigins: string[]` (parent side) — accept and trust forwarded events only from child iframes on these origins; drop everything else in IframeManager before it enters the recording. `['*']` opts back into accept-any. - `emitToOrigin: string` (child side) — forward this frame's events to its parent using this value as the postMessage targetOrigin, so the browser only delivers to a matching parent. A middle frame in a nested chain sets both; targeting is per-immediate-parent, not the root. Supersedes the unreleased `crossOriginIframeParentOrigin` from this PR's base, which is removed outright (it only hardened the send half). `recordCrossOriginIframes` is deprecated but still supported, resolving to `collectFromOrigins: ['*']` plus forward-to-any — the legacy auto-role path (incl. the same-origin-child no-op) is kept intact and still passes its 22 existing tests. Docs (recipe + guide tables, en + zh_CN) rewritten around the two options with a migration section. New tests cover the send-side targetOrigin gate and the receive-side origin allow-list. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
d35a4c4 to
d253958
Compare
|
re. versioning: The semver.org FAQ explicitly says to ship a deprecation as a minor release: update the docs, mark it deprecated, and cut a minor so consumers get a signal on their next minor upgrade. The reasoning is that the public API surface has meaningfully changed (something is now on notice), even though nothing breaks and nothing is removed. So by the book, this PR had two separate pulls toward minor:
The counter-argument for patch is that it's a convention/recommendation, not a hard rule: a deprecation changes zero runtime behavior (the option still works identically), and here it's just a JSDoc @deprecated tag plus a doc note — no runtime warning. Plenty of projects fold that into a patch. |
| > **Security warning:** Cross-origin iframe recording is not recommended unless | ||
| > you [strictly restrict](https://stackoverflow.com/a/21629575) which sites can | ||
| > embed your page and trust those sites with its recorded contents. The | ||
| > browser's cross-origin restrictions protect sensitive information. Accepting | ||
| > events with `collectFromOrigins: ['*']`, or forwarding them with | ||
| > `emitToOrigin: '*'`, removes that protection — a malicious page can then | ||
| > embed your page and collect its recorded contents, or inject fabricated | ||
| > events into your recording. Name specific origins on both sides. |
There was a problem hiding this comment.
Some mixed messages above.
For completeness, here is my understanding of all this:
It is safe to have emitToOrigin: '*' as long as you follow the guidelines here: https://stackoverflow.com/a/21629575 and strictly restrict who can embed your webpage in an iframe.
On the parent side, if you are iframing a malicious website, which is probably most relevant when a domain expires and is bought by a malicious actor, you run the risk of them filling your recordings with junk if you have collectFromOrigins: ['*'], and there might be a risk of them corrupting your recordings. And an unknown risk of them breaking out of the sandbox on replay side in the case of canvas replay being on which is more risky.
Alternative to #1679. The first four commits here are #1679's own commits (by @econte-sprig and @Juice10), cherry-picked onto current
mainso this branch is reviewable standalone; the final commit builds the alternative design on top. If this direction is preferred, those commits can be cherry-picked back onto #1679.Motivation
#1679 adds
crossOriginIframeParentOriginto harden the send side of cross-origin iframe recording (the child'spostMessagetarget origin). While reviewing it, a broader issue surfaced: the singlerecordCrossOriginIframesswitch conflates two independent roles whose correct trust boundary differs, and it offers no control over trust in either direction:*, so any embedder can harvest the recording.crossOriginIframeParentOriginonly addresses the second. This PR addresses both, symmetrically.Change
Replace the single switch with two origin-scoped options, each self-enabling by its presence:
collectFromOrigins: string[]IframeManager.['*']opts back into accept-any.emitToOrigin: stringpostMessagetarget origin so the browser only delivers to a matching parent.A frame in the middle of a nested chain sets both — targeting is per-immediate-parent, not the root, matching how events hop one level at a time.
This supersedes
crossOriginIframeParentOrigin(removed here, since #1679 is unreleased) and deprecatesrecordCrossOriginIframes, which still works and resolves tocollectFromOrigins: ['*']plus forward-to-any. The legacy auto-role path (including the same-origin-child no-op) is kept intact.Docs & tests
docs/recipes/cross-origin-iframes.md) and theguide.mdoptions table rewritten around the two options, with a nested-iframe example and arecordCrossOriginIframesmigration section (en + zh_CN).cross-origin-iframe-emit-to-origin.test.ts(send-side target-origin gate) andcross-origin-iframe-collect-from-origins.test.ts(receive-side origin allow-list).cross-origin-iframes.test.ts(22 tests) still passes unchanged, confirming the deprecated path is preserved.🤖 Generated with Claude Code