fix: bookmarklet postMessage rejected by origin check#136
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a regression where the Press This popup rejected all bookmarklet postMessage payloads due to an overly strict same-origin check, leaving the editor empty in the normal bookmarklet flow. The handler now validates the message source window (the opener) rather than requiring same-origin, and adds focused unit tests to prevent the regression.
Changes:
- Update
App’smessageevent handler to accept cross-origin bookmarklet messages by requiringevent.source === window.opener. - Add a unit test suite covering: cross-origin opener acceptance, non-opener rejection, and wrong-type rejection.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/App.js |
Replaces origin-based filtering with opener/source identity gating for bookmarklet postMessage. |
tests/components/app-postmessage.test.js |
Adds regression tests for the postMessage receive path and spoofing protection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Fixes #135. The bookmarklet's
postMessageto the Press This popup was silently dropped, leaving the editor empty (no title, no content, no media, no selection) whenever the bookmarklet was the source of data — i.e. the normal flow.Root cause
src/App.js's message handler required the message origin to equal the popup's own origin:The bookmarklet runs on whatever third-party page the user is on (
ma.tt,youtube.com, …), soevent.originis always cross-origin to the WordPress site. Every legitimate bookmarklet message was rejected.The check came in as a hardening pass in PR #100 (commit
a55f050, "Add event.origin check on the postMessage listener"). The intent was right (don't let arbitrary pages forgepress-this-datamessages); the implementation was wrong for this transport.The bug landed in trunk on Apr 3, after the 2.0.2 release on Mar 25. Stable users on 2.0.2 are unaffected — this only impacts trunk and 2.1.0-beta.
Fix
Validate the message source instead of its origin:
Only the window that opened this popup (the bookmarklet's window) holds a
Windowreference to it viawindow.open(), so identity-checking againstwindow.openerkeeps the spoofing protection without rejecting cross-origin senders.Tests
Added
tests/components/app-postmessage.test.jswith three cases that mount<App />with stubbed children and dispatchMessageEvents:press-this-datapayload → handler runs (asserted via thevalidate-embedsfetch firing). This test fails on trunk and passes after the fix — exactly the regression to guard against.Window) with otherwise-valid payload → rejected. Locks in spoofing protection so a future change can't drop the source check.event.data.type→ rejected.Test plan
npm run test:unit— all 364 tests passnpm run lint— cleanpm=1URL still works when proxy is enabled (the auto-scan path is independent and was working).wn=1window.name fallback (popup-blocked / mobile) is unaffected — it doesn't go through the postMessage handler.