Skip to content
This repository was archived by the owner on May 7, 2026. It is now read-only.

fix: handle SecurityError in IframeManager destroy and removeIframeById - #163

Merged
pauldambra merged 2 commits into
mainfrom
fix/iframe-manager-security-error-on-destroy
Apr 2, 2026
Merged

pauldambra merged 2 commits into
mainfrom
fix/iframe-manager-security-error-on-destroy

Conversation

@pauldambra

Copy link
Copy Markdown
Member

Summary

  • PR fix: catch all SecurityError variants in stop handler cleanup #156 fixed SecurityError handling in the stop handler's handlers.forEach loop, but iframeManager.destroy() and removeIframeById() also call removeEventListener on iframe contentWindow references that may have become cross-origin — these calls were unprotected
  • Wraps both contentWindow.removeEventListener call sites in iframe-manager.ts with the same DOMException/SecurityError catch pattern
  • Customer-reported error: "SecurityError: Failed to read a named property 'removeEventListener' from 'Window': Blocked a frame with origin"

Test plan

  • Parameterized tests verify destroy() and removeIframeById() swallow SecurityError from cross-origin contentWindow
  • Parameterized tests verify non-SecurityError exceptions still propagate
  • All 21 existing + new tests pass

🤖 Generated with Claude Code

PR #156 protected the stop handler's cleanup loop but
iframeManager.destroy() and removeIframeById() also call
removeEventListener on iframe contentWindow references that may have
become cross-origin, causing unhandled SecurityError.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@pauldambra
pauldambra requested review from a team, TueHaulund, fasyy612, ksvat and nicowaltz and removed request for a team April 2, 2026 12:21
…ling

Deduplicates the try-catch-rethrow pattern used in three places and
co-locates the explanatory comment with the helper.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@fasyy612 fasyy612 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ☀️

@pauldambra
pauldambra merged commit e06b822 into main Apr 2, 2026
11 checks passed
@pauldambra
pauldambra deleted the fix/iframe-manager-security-error-on-destroy branch April 2, 2026 14:39
pauldambra added a commit to PostHog/posthog-js that referenced this pull request Apr 8, 2026
## Summary

- Bumps `@posthog/rrweb-plugin-console-record`, `@posthog/rrweb-record`, and `@posthog/rrweb-types` from `0.0.53` → `0.0.56`
- Includes fixes from posthog-rrweb#157–159, #163, and #166

THIS SHOULD SHOW A BUNDLE SIZE REDUCTION BEFORE MERGING

### Changes included (0.0.54–0.0.56)
- PostHog/posthog-rrweb#157: fix: clear mutation buffer on iframe pagehide to prevent recording corruption
- PostHog/posthog-rrweb#158: fix: skip unchanged setAttribute calls to prevent replay flicker
- PostHog/posthog-rrweb#159: fix: prevent iframe leak in untainted prototype and avoid unnecessary iframe creation
- PostHog/posthog-rrweb#163: fix: handle SecurityError in IframeManager destroy and removeIframeById
- PostHog/posthog-rrweb#166: fix: remove postcss from @posthog/rrweb-record bundle (420KB → 170KB)

---
*Created with [PostHog Code](https://posthog.com/code?ref=pr)*
mergify Bot pushed a commit to doxynix/doxynix that referenced this pull request Apr 11, 2026
This PR contains the following updates:

| Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) |
|---|---|---|---|
| [fast-xml-parser](https://redirect.github.com/NaturalIntelligence/fast-xml-parser) | [`5.5.10` → `5.5.11`](https://renovatebot.com/diffs/npm/fast-xml-parser/5.5.10/5.5.11) | ![age](https://developer.mend.io/api/mc/badges/age/npm/fast-xml-parser/5.5.11?slim=true) | ![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/fast-xml-parser/5.5.10/5.5.11?slim=true) |
| [posthog-js](https://posthog.com/docs/libraries/js) ([source](https://redirect.github.com/PostHog/posthog-js)) | [`1.365.1` → `1.365.3`](https://renovatebot.com/diffs/npm/posthog-js/1.365.1/1.365.3) | ![age](https://developer.mend.io/api/mc/badges/age/npm/posthog-js/1.365.3?slim=true) | ![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/posthog-js/1.365.1/1.365.3?slim=true) |

---

### Release Notes

<details>
<summary>NaturalIntelligence/fast-xml-parser (fast-xml-parser)</summary>

### [`v5.5.11`](https://redirect.github.com/NaturalIntelligence/fast-xml-parser/compare/v5.5.10...12b041ddeffa8b37ea44d572284c00a89148f2d3)

[Compare Source](https://redirect.github.com/NaturalIntelligence/fast-xml-parser/compare/v5.5.10...12b041ddeffa8b37ea44d572284c00a89148f2d3)

</details>

<details>
<summary>PostHog/posthog-js (posthog-js)</summary>

### [`v1.365.3`](https://redirect.github.com/PostHog/posthog-js/releases/tag/posthog-js%401.365.3)

[Compare Source](https://redirect.github.com/PostHog/posthog-js/compare/posthog-js@1.365.2...posthog-js@1.365.3)

#### 1.365.3

##### Patch Changes

- [#&#8203;3357](https://redirect.github.com/PostHog/posthog-js/pull/3357) [`dbdddca`](https://redirect.github.com/PostHog/posthog-js/commit/dbdddcad578adf282f620d2afcd5808600a9c287) Thanks [@&#8203;pauldambra](https://redirect.github.com/pauldambra)! - Bump [@&#8203;posthog/rrweb](https://redirect.github.com/posthog/rrweb) packages to 0.0.56, which includes:
  - [PostHog/posthog-rrweb#157](https://redirect.github.com/PostHog/posthog-rrweb/issues/157): fix: clear mutation buffer on iframe pagehide to prevent recording corruption
  - [PostHog/posthog-rrweb#158](https://redirect.github.com/PostHog/posthog-rrweb/issues/158): fix: skip unchanged setAttribute calls to prevent replay flicker
  - [PostHog/posthog-rrweb#159](https://redirect.github.com/PostHog/posthog-rrweb/issues/159): fix: prevent iframe leak in untainted prototype and avoid unnecessary iframe creation
  - [PostHog/posthog-rrweb#163](https://redirect.github.com/PostHog/posthog-rrweb/issues/163): fix: handle SecurityError in IframeManager destroy and removeIframeById
  - [PostHog/posthog-rrweb#166](https://redirect.github.com/PostHog/posthog-rrweb/issues/166): fix: remove postcss from [@&#8203;posthog/rrweb-record](https://redirect.github.com/posthog/rrweb-record) bundle (420KB → 170KB) (2026-04-08)
- Updated dependencies \[]:
  - [@&#8203;posthog/types](https://redirect.github.com/posthog/types)@&#8203;1.365.3

### [`v1.365.2`](https://redirect.github.com/PostHog/posthog-js/releases/tag/posthog-js%401.365.2)

[Compare Source](https://redirect.github.com/PostHog/posthog-js/compare/posthog-js@1.365.1...posthog-js@1.365.2)

#### 1.365.2

##### Patch Changes

- [#&#8203;3323](https://redirect.github.com/PostHog/posthog-js/pull/3323) [`c387f6d`](https://redirect.github.com/PostHog/posthog-js/commit/c387f6dc146c9c09640e471e66043ad832b0476e) Thanks [@&#8203;pauldambra](https://redirect.github.com/pauldambra)! - perf(replay): reduce memory and CPU cost of event compression by caching gzipped empty arrays and eliminating redundant JSON.stringify for size estimation
  (2026-04-08)
- Updated dependencies \[[`c387f6d`](https://redirect.github.com/PostHog/posthog-js/commit/c387f6dc146c9c09640e471e66043ad832b0476e)]:
  - [@&#8203;posthog/types](https://redirect.github.com/posthog/types)@&#8203;1.365.2

</details>

---

### Configuration

📅 **Schedule**: (in timezone Europe/Moscow)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://redirect.github.com/renovatebot/renovate/discussions) if that's undesired.

---

 - [ ] If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://redirect.github.com/renovatebot/renovate).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants