Skip to content

fix(registry): authenticate deck sound effect messages - #3819

Merged
jrusso1020 merged 1 commit into
mainfrom
fix/security-deck-audio-messages
Sep 9, 2026
Merged

fix(registry): authenticate deck sound effect messages#3819
jrusso1020 merged 1 commit into
mainfrom
fix/security-deck-audio-messages

Conversation

@jrusso1020

Copy link
Copy Markdown
Collaborator

The Airbnb deck's sound-effect handler accepts messages from unrelated windows and indexes an ordinary object using the supplied clip name. A message naming __proto__ can therefore assign currentTime to Object.prototype.

Require the sender to be the current deck player's non-null iframe window (#637), and require an own string clip name before lookup (#636). The four sound effects, gesture unlock, volume and slideshow mute behavior remain unchanged. The player is resolved for each message so a replaced iframe cannot keep controlling sound.

Verification: a regression executes the actual demo scripts in JSDOM with mocked Audio and covers all four clip names, unlock/mute/unmute, null/foreign/replaced senders, inherited names, coerced names and prototype integrity. It passes the fix and fails the original demo. A real Chromium check with the actual player iframe confirms valid sound messages work while self messages and prototype names are rejected. Full workspace build, core typechecks, lint/format and signed hooks pass; Fallow is clear.

Validation limitation: the repository CLI lint/check commands target the unchanged index.html composition, not the demo.html host page. They stop on its existing requestAnimationFrame lint finding (plus a file-size warning); no browser/layout pass is claimed from that command. The host-page behavior is covered by the separate real-browser check above.

Please verify both alert flows and compatibility. No dismissals; verify main CodeQL closure after merge. Test lives with the core JSDOM fixtures to use its existing dependency, with no new dependencies or production core changes.

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The two guards are correctly placed at the message boundary and preserve the demo’s existing sound contract.

  • registry/examples/airbnb-deck/demo.html:171-176 resolves the current player/iframe for every event and requires exact non-null contentWindow identity before reading the payload. That makes iframe replacement self-healing and rejects parent/self/sibling/null senders without relying on a fixed origin.
  • registry/examples/airbnb-deck/demo.html:176 requires a primitive string and an own key of the four-entry clip table before lookup. __proto__, constructor, coerced arrays, and unknown names cannot reach the currentTime assignment.
  • packages/core/src/runtime/deckAudio.test.ts:10-73 executes the shipped demo.html script rather than duplicating the handler. Its placement is reasonable: core already owns the JSDOM test dependency, no production core surface changes, and the witness covers valid sounds, gesture unlock, mute/unmute, current/foreign/null/replaced senders, inherited keys, and prototype integrity.

Independent verification at exact head de65a8eae923a630c12ee4abd11dabcae2c0be34:

  • Focused JSDOM witness: 1/1 passed.
  • Changed TypeScript: oxlint 0 warnings/errors; oxfmt check passed.
  • Player build passed and verified the runtime pin.
  • Local Chromium against the built real demo: the current iframe produced one valid play; same-frame __proto__, a foreign iframe, and a null-source event produced no additional plays; Object.prototype remained clean; no page errors.
  • All hosted checks are now green after the Google apt mirror recovered, including the full Test, required CLI smoke, preview parity, perf parity/fps, Windows, regression matrix, and CodeQL. The PR-ref open-alert set is empty, and no review threads/comments are open.

Separate sequencing note: main alert #648 from merged #3816 is not part of this diff. I independently classified it as a scoped false positive; landing #3819 remains subject to that prior-batch disposition being recorded as James stated.

— Magi

Verdict: APPROVE

Reasoning: Both source authentication and clip selection are fail-closed at the boundary, the actual shipped script is exercised, independent browser behavior matches the unit witness, and every native gate is green at the unchanged head.

@jrusso1020
jrusso1020 merged commit c6010f2 into main Sep 9, 2026
118 of 134 checks passed
@jrusso1020
jrusso1020 deleted the fix/security-deck-audio-messages branch September 9, 2026 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants