From 859ac622c2991250ed28fbdcee4b6d2a8efea6ff Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 30 Aug 2026 13:00:14 -0700 Subject: [PATCH] fix(player): report and fail closed on runtime delivery errors (#3472) * feat(player): report runtime data application * fix(player): fail closed on runtime data delivery * fix(player): close runtime delivery and sandbox gaps * fix(player): satisfy runtime contract and CodeQL * refactor(player): simplify runtime tag scanner * style(player): apply repository formatter * fix(player): type runtime tag boundaries * fix(core): mint guest-local runtime-data ids in a separate space from host ids --- .github/workflows/player-perf.yml | 7 + packages/core/src/runtime/bridge.test.ts | 10 +- packages/core/src/runtime/bridge.ts | 9 +- packages/core/src/runtime/init.ts | 18 +- packages/core/src/runtime/runtimeData.test.ts | 60 ++++++- packages/core/src/runtime/runtimeData.ts | 78 ++++++-- packages/core/src/runtime/types.ts | 10 ++ packages/core/src/runtime/window.d.ts | 2 +- packages/player/README.md | 33 +++- packages/player/package.json | 1 + .../player/src/hyperframes-player.test.ts | 133 +++++++++++++- packages/player/src/hyperframes-player.ts | 168 ++++++++++++++++-- packages/player/src/runtime-in-srcdoc.test.ts | 10 ++ packages/player/src/runtime-in-srcdoc.ts | 34 +++- .../src/runtime-message-handler.test.ts | 26 ++- .../player/src/runtime-message-handler.ts | 13 +- .../player/tests/browser/sandbox-origin.ts | 53 ++++++ .../perf/fixtures/sandbox-probe/index.html | 18 ++ packages/player/tests/perf/runner.ts | 1 + packages/player/tests/perf/server.ts | 6 + 20 files changed, 632 insertions(+), 58 deletions(-) create mode 100644 packages/player/tests/browser/sandbox-origin.ts create mode 100644 packages/player/tests/perf/fixtures/sandbox-probe/index.html diff --git a/.github/workflows/player-perf.yml b/.github/workflows/player-perf.yml index 757f531fb..7bcfd1aa7 100644 --- a/.github/workflows/player-perf.yml +++ b/.github/workflows/player-perf.yml @@ -107,6 +107,13 @@ jobs: if: matrix.shard == 'parity' uses: ./.github/actions/install-ffmpeg-linux + - name: Verify sandbox origin boundary (load shard only) + if: matrix.shard == 'load' + working-directory: packages/player + env: + PUPPETEER_EXECUTABLE_PATH: ${{ steps.setup-chrome.outputs.chrome-path }} + run: bun run test:browser-security + - name: Run player perf — ${{ matrix.shard }} (measure mode) working-directory: packages/player env: diff --git a/packages/core/src/runtime/bridge.test.ts b/packages/core/src/runtime/bridge.test.ts index 60cbd042d..ae94404c7 100644 --- a/packages/core/src/runtime/bridge.test.ts +++ b/packages/core/src/runtime/bridge.test.ts @@ -50,10 +50,12 @@ describe("installRuntimeControlBridge", () => { const deps = createMockDeps(); const handler = installRuntimeControlBridge(deps); const payload = { version: 3, segments: [] }; - handler(makeControlMessage("set-runtime-data", { channel: "captions", payload })); - handler(makeControlMessage("clear-runtime-data", { channel: "captions" })); - expect(deps.onSetRuntimeData).toHaveBeenCalledWith("captions", payload); - expect(deps.onClearRuntimeData).toHaveBeenCalledWith("captions"); + handler( + makeControlMessage("set-runtime-data", { channel: "captions", payload, requestId: 41 }), + ); + handler(makeControlMessage("clear-runtime-data", { channel: "captions", requestId: 42 })); + expect(deps.onSetRuntimeData).toHaveBeenCalledWith("captions", payload, 41); + expect(deps.onClearRuntimeData).toHaveBeenCalledWith("captions", 42); }); it("dispatches stop-media command", () => { diff --git a/packages/core/src/runtime/bridge.ts b/packages/core/src/runtime/bridge.ts index 804bde053..4de5ff306 100644 --- a/packages/core/src/runtime/bridge.ts +++ b/packages/core/src/runtime/bridge.ts @@ -28,8 +28,8 @@ type BridgeDeps = { ) => void; onEnablePickMode: () => void; onDisablePickMode: () => void; - onSetRuntimeData?: (channel: string, payload: unknown) => void; - onClearRuntimeData?: (channel: string) => void; + onSetRuntimeData?: (channel: string, payload: unknown, requestId?: number) => void; + onClearRuntimeData?: (channel: string, requestId?: number) => void; getCanonicalFps: () => number; }; @@ -78,10 +78,11 @@ const CONTROL_HANDLERS: Record = { "disable-pick-mode": (_d, deps) => deps.onDisablePickMode(), "flash-elements": (data) => handleFlashElements(data), "set-runtime-data": (data, deps) => { - if (typeof data.channel === "string") deps.onSetRuntimeData?.(data.channel, data.payload); + if (typeof data.channel === "string") + deps.onSetRuntimeData?.(data.channel, data.payload, data.requestId); }, "clear-runtime-data": (data, deps) => { - if (typeof data.channel === "string") deps.onClearRuntimeData?.(data.channel); + if (typeof data.channel === "string") deps.onClearRuntimeData?.(data.channel, data.requestId); }, }; diff --git a/packages/core/src/runtime/init.ts b/packages/core/src/runtime/init.ts index 880c9692b..20f76e189 100644 --- a/packages/core/src/runtime/init.ts +++ b/packages/core/src/runtime/init.ts @@ -67,7 +67,12 @@ import { shouldAttemptPeriodicTimelineBind } from "./timelineRebindPolicy"; import { installStudioCustomEase } from "./customEase"; import { parseNumeric } from "./startExpression"; import { parseStrictFiniteTimingNumber } from "./playbackRate"; -import { clearRuntimeData, setRuntimeData, setRuntimeDataErrorReporter } from "./runtimeData"; +import { + clearRuntimeData, + setRuntimeData, + setRuntimeDataAppliedReporter, + setRuntimeDataErrorReporter, +} from "./runtimeData"; const AUTHORED_DURATION_ATTR = "data-hf-authored-duration"; const AUTHORED_END_ATTR = "data-hf-authored-end"; @@ -133,14 +138,23 @@ export function initSandboxRuntimeModular(): void { // Own the analytics bridge before any best-effort runtime installation so // early failures are observable instead of disappearing before player setup. initRuntimeAnalytics(postRuntimeMessage as (payload: unknown) => void); - setRuntimeDataErrorReporter((channel, error) => { + setRuntimeDataErrorReporter((channel, requestId, error) => { postRuntimeMessage({ source: "hf-preview", type: "runtime-data-error", channel, + requestId, message: error instanceof Error ? error.message : String(error), }); }); + setRuntimeDataAppliedReporter((channel, requestId) => { + postRuntimeMessage({ + source: "hf-preview", + type: "runtime-data-applied", + channel, + requestId, + }); + }); // SDK moveElement edits must render even when no usable GSAP timeline ever // binds (CSS/WAAPI-animated or fully static compositions) — apply at init. // This runs at DOMContentLoaded, after inline composition scripts have diff --git a/packages/core/src/runtime/runtimeData.test.ts b/packages/core/src/runtime/runtimeData.test.ts index 60efc255e..856bcda7d 100644 --- a/packages/core/src/runtime/runtimeData.test.ts +++ b/packages/core/src/runtime/runtimeData.test.ts @@ -4,6 +4,7 @@ import { registerRuntimeDataHandler, resetRuntimeDataForTests, setRuntimeData, + setRuntimeDataAppliedReporter, setRuntimeDataErrorReporter, } from "./runtimeData"; @@ -48,6 +49,63 @@ describe("runtime data registry", () => { throw new Error("attach failed"); }); expect(() => setRuntimeData("captions", {})).not.toThrow(); - expect(reporter).toHaveBeenCalledWith("captions", expect.any(Error)); + expect(reporter).toHaveBeenCalledWith("captions", expect.any(Number), expect.any(Error)); + }); + + it("reports asynchronous completion and rejection", async () => { + const applied = vi.fn(); + const failed = vi.fn(); + setRuntimeDataAppliedReporter(applied); + setRuntimeDataErrorReporter(failed); + registerRuntimeDataHandler("captions", async (payload) => { + await Promise.resolve(); + if (payload === "bad") throw new Error("async attach failed"); + }); + + setRuntimeData("captions", "good"); + await vi.waitFor(() => expect(applied).toHaveBeenCalledWith("captions", expect.any(Number))); + setRuntimeData("captions", "bad"); + await vi.waitFor(() => + expect(failed).toHaveBeenCalledWith("captions", expect.any(Number), expect.any(Error)), + ); + }); + + it("reports only the latest concurrent delivery on a channel", async () => { + const applied = vi.fn(); + setRuntimeDataAppliedReporter(applied); + const resolvers: Array<() => void> = []; + registerRuntimeDataHandler( + "captions", + () => new Promise((resolve) => resolvers.push(resolve)), + ); + + setRuntimeData("captions", "first", 101); + setRuntimeData("captions", "latest", 102); + resolvers[1]?.(); + await vi.waitFor(() => expect(applied).toHaveBeenCalledWith("captions", 102)); + resolvers[0]?.(); + await Promise.resolve(); + + expect(applied).toHaveBeenCalledTimes(1); + }); + + it("never reports a composition-side delivery under a pending host request id", async () => { + const applied = vi.fn(); + setRuntimeDataAppliedReporter(applied); + const resolvers: Array<() => void> = []; + registerRuntimeDataHandler( + "captions", + () => new Promise((resolve) => resolvers.push(resolve)), + ); + + // The host mints id 1 and waits on it; the composition then calls the two-argument + // public form, which mints an id of its own. + setRuntimeData("captions", "first", 1); + setRuntimeData("captions", "latest"); + resolvers[1]?.(); + await vi.waitFor(() => expect(applied).toHaveBeenCalledTimes(1)); + + const [, reportedId] = applied.mock.calls[0] ?? []; + expect(reportedId).not.toBe(1); }); }); diff --git a/packages/core/src/runtime/runtimeData.ts b/packages/core/src/runtime/runtimeData.ts index d088bd8be..75b716f7b 100644 --- a/packages/core/src/runtime/runtimeData.ts +++ b/packages/core/src/runtime/runtimeData.ts @@ -1,21 +1,56 @@ -export type RuntimeDataHandler = (payload: unknown) => void; -export type RuntimeDataErrorReporter = (channel: string, error: unknown) => void; +export type RuntimeDataHandler = (payload: unknown) => void | Promise; +export type RuntimeDataErrorReporter = (channel: string, requestId: number, error: unknown) => void; +export type RuntimeDataAppliedReporter = (channel: string, requestId: number) => void; -const retained = new Map(); +type RetainedRuntimeData = { + payload: unknown; + requestId: number; + generation: number; +}; + +const retained = new Map(); const handlers = new Map(); +const generations = new Map(); let reportError: RuntimeDataErrorReporter = () => undefined; +let reportApplied: RuntimeDataAppliedReporter = () => undefined; +let localRequestId = 0; function validChannel(channel: string): boolean { return /^[a-z][a-z0-9-]{0,63}$/.test(channel); } -function deliver(channel: string, payload: unknown): void { +function nextGeneration(channel: string): number { + const generation = (generations.get(channel) ?? 0) + 1; + generations.set(channel, generation); + return generation; +} + +function resolveRequestId(requestId: number | undefined): number { + if (typeof requestId === "number" && Number.isSafeInteger(requestId) && requestId > 0) + return requestId; + // Guest-local ids count down so they can never collide with a host id, which is + // required above to be positive. A shared id space lets a composition-side call + // report `applied` under a host request's id while that host payload is still in flight. + localRequestId -= 1; + return localRequestId; +} + +function deliver(channel: string, retainedData: RetainedRuntimeData): void { const handler = handlers.get(channel); if (!handler) return; + const isCurrent = () => + generations.get(channel) === retainedData.generation && handlers.get(channel) === handler; try { - handler(payload); + void Promise.resolve(handler(retainedData.payload)).then( + () => { + if (isCurrent()) reportApplied(channel, retainedData.requestId); + }, + (error) => { + if (isCurrent()) reportError(channel, retainedData.requestId, error); + }, + ); } catch (error) { - reportError(channel, error); + if (isCurrent()) reportError(channel, retainedData.requestId, error); } } @@ -23,16 +58,29 @@ export function setRuntimeDataErrorReporter(reporter: RuntimeDataErrorReporter): reportError = reporter; } -export function setRuntimeData(channel: string, payload: unknown): void { - if (!validChannel(channel)) return; - retained.set(channel, payload); - deliver(channel, payload); +export function setRuntimeDataAppliedReporter(reporter: RuntimeDataAppliedReporter): void { + reportApplied = reporter; } -export function clearRuntimeData(channel: string): void { +export function setRuntimeData(channel: string, payload: unknown, requestId?: number): void { + if (!validChannel(channel)) return; + const retainedData = { + payload, + requestId: resolveRequestId(requestId), + generation: nextGeneration(channel), + }; + retained.set(channel, retainedData); + deliver(channel, retainedData); +} + +export function clearRuntimeData(channel: string, requestId?: number): void { if (!validChannel(channel)) return; retained.delete(channel); - deliver(channel, undefined); + deliver(channel, { + payload: undefined, + requestId: resolveRequestId(requestId), + generation: nextGeneration(channel), + }); } export function registerRuntimeDataHandler( @@ -42,7 +90,8 @@ export function registerRuntimeDataHandler( if (!validChannel(channel)) throw new Error(`Invalid HyperFrames runtime-data channel: ${channel}`); handlers.set(channel, handler); - if (retained.has(channel)) deliver(channel, retained.get(channel)); + const retainedData = retained.get(channel); + if (retainedData) deliver(channel, retainedData); return () => { if (handlers.get(channel) === handler) handlers.delete(channel); }; @@ -51,5 +100,8 @@ export function registerRuntimeDataHandler( export function resetRuntimeDataForTests(): void { retained.clear(); handlers.clear(); + generations.clear(); + localRequestId = 0; reportError = () => undefined; + reportApplied = () => undefined; } diff --git a/packages/core/src/runtime/types.ts b/packages/core/src/runtime/types.ts index 5e98822fa..edfd52050 100644 --- a/packages/core/src/runtime/types.ts +++ b/packages/core/src/runtime/types.ts @@ -175,9 +175,17 @@ export type RuntimeDataErrorMessage = { source: "hf-preview"; type: "runtime-data-error"; channel: string; + requestId: number; message: string; }; +export type RuntimeDataAppliedMessage = { + source: "hf-preview"; + type: "runtime-data-applied"; + channel: string; + requestId: number; +}; + /** * Analytics events emitted by the runtime. * @@ -229,6 +237,7 @@ export type RuntimeOutboundMessage = | RuntimeMediaAutoplayBlockedMessage | RuntimeReadyMessage | RuntimeDataErrorMessage + | RuntimeDataAppliedMessage | RuntimeAnalyticsMessage | RuntimePerformanceMessage | RuntimeGroupLevelsMessage; @@ -332,6 +341,7 @@ export type RuntimeGsapSetVars = Record void, ) => () => void; - setRuntimeData?: (channel: string, payload: unknown) => void; + setRuntimeData?: (channel: string, payload: unknown, requestId?: number) => void; clearRuntimeData?: (channel: string) => void; [key: string]: unknown; }; diff --git a/packages/player/README.md b/packages/player/README.md index 0627e4e07..0928213c2 100644 --- a/packages/player/README.md +++ b/packages/player/README.md @@ -132,9 +132,40 @@ player.shaderLoading; // "composition" | "player" | "none" (read/write) player.iframeElement; // HTMLIFrameElement (read-only) ``` +## Runtime data delivery + +`setRuntimeData(channel, payload)` clones and retains the payload, then delivers it after the +composition runtime is ready. Invalid channels and non-cloneable payloads throw synchronously. +Failures after the call returns are reported with `runtimedataerror`; successful application is +reported with `runtimedataapplied`. Both events include `{ channel, requestId }`, and errors also +include `message`. Listen for both outcomes when delivery matters: + +```js +player.addEventListener("runtimedataapplied", ({ detail }) => { + console.log("applied", detail.channel, detail.requestId); +}); +player.addEventListener("runtimedataerror", ({ detail }) => { + console.error("not applied", detail.channel, detail.requestId, detail.message); +}); +player.setRuntimeData("captions", captionData); +``` + +Only the latest in-flight update for a channel can emit a completion. A missing runtime response, +iframe teardown, or bridge delivery failure emits `runtimedataerror` instead of remaining pending +indefinitely. + ## Advanced: iframe access -The composition runs inside a sandboxed `