Skip to content

Add publisher-native rendering for selected APS bids - #1042

Merged
ChristianPavilonis merged 9 commits into
mainfrom
experiment/aps-native-rendering
Aug 25, 2026
Merged

Add publisher-native rendering for selected APS bids#1042
ChristianPavilonis merged 9 commits into
mainfrom
experiment/aps-native-rendering

Conversation

@ChristianPavilonis

@ChristianPavilonis ChristianPavilonis commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Adds an opt-in publisher_native mode for rendering Trusted Server-selected APS bids through APS's fixed Prebid creative runner in a publisher-origin friendly iframe.
  • Keeps the existing opaque Trusted Server renderer as the default and disables its static route only when rendering_mode = "publisher_native" is explicitly configured.
  • Keeps the change self-contained: Trusted Server injects the runner bootstrap, so publishers do not need to install custom JavaScript.

The existing opaque renderer can leave APS creatives blank when their nested HTTPS frames require their real origin. A controlled browser test through the publisher setup confirmed that the new friendly-frame path renders a real selected APS creative, while apstag.renderImp(document, bidId) cannot render the server-selected bid because it is absent from APS's browser-auction state.

The working path reuses the existing prebid/creative/render runner contract inside a friendly frame. Trusted Server does not call apstag.fetchBids(), call apstag.setDisplayBids(), mutate the publisher's APS SDK, or start another auction. The runner contract is observed vendor behavior rather than a documented external-response API, so APS account-team validation is still required before production use.

Changes

File Change
crates/trusted-server-core/src/integrations/aps.rs Adds the typed rendering mode, emits a CSP-safe native-mode marker, and omits the static renderer route in native mode.
crates/trusted-server-integration-tests/browser/tests/shared/aps-renderer.spec.ts Adds a real-browser proof that native mode queues the selected response, loads the fixed runner, and creates no opaque renderer frame.
crates/trusted-server-js/lib/src/core/request.ts Routes direct auction APS winners through the shared rendering-owner dispatcher.
crates/trusted-server-js/lib/src/integrations/aps/render.ts Validates descriptors and implements the friendly-frame runner with slot resolution, replacement, timeout, failure, and no-fallback behavior.
crates/trusted-server-js/lib/src/integrations/gpt/index.ts Applies the same rendering owner to server and Prebid APS paths while preserving one-shot ownership.
crates/trusted-server-js/lib/test/core/request.test.ts Covers default opaque rendering and direct native-runner rendering.
crates/trusted-server-js/lib/test/integrations/aps/render.test.ts Covers queue shape, runner load/failure, logical GPT slots, replacement, stale completion, timeout, and publisher-state errors.
crates/trusted-server-js/lib/test/integrations/gpt/ad_init.test.ts Covers accepted and failed native-runner handoffs for server and Prebid APS capabilities.
docs/guide/integrations/aps.md Documents configuration, implementation limits, CSP and security trade-offs, duplicate-demand risk, and rollout requirements.
trusted-server.example.toml Shows the default rendering mode and controlled native experiment opt-in.

Scope

This PR is intentionally limited to configuration, bid ownership, browser rendering, focused tests, and operator documentation. It does not add infrastructure, change bid selection, call publisher apstag methods, or claim a publicly supported APS external-response API.

The Rust and TypeScript changes are both required: the mode must be selected and exposed server-side, then enforced consistently across direct auctions and Google Ad Manager/Prebid rendering paths. Native mode deliberately uses a friendly iframe without the default opaque-origin sandbox, so it has a larger security surface and must remain isolated during testing.

Closes

Related to #999.

Test plan

  • cargo test-fastly && cargo test-axum
  • cargo clippy-fastly && cargo clippy-axum
  • cargo fmt --all -- --check
  • JS tests: cd crates/trusted-server-js/lib && npx vitest run — 44 files, 842 tests
  • JS format: cd crates/trusted-server-js/lib && npm run format
  • Docs format: cd docs && npm run format
  • WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
  • Manual testing via fastly compute serve
  • Other: Cloudflare and Spin tests; all six target-specific Clippy commands; adapter parity; JS lint and bundle build
  • Other: APS Playwright suite — 4 tests, including the publisher-native friendly-frame proof
  • Other: controlled browser test through ts dev proxy rendered a real selected APS creative with no /integrations/aps/renderer request

Checklist

  • Changes follow CLAUDE.md conventions
  • No unwrap() in production code — use expect("should ...")
  • Uses project logging macros (not println!)
  • New code has tests
  • No secrets or credentials committed

Provide an opt-in publisher hook for testing whether a Trusted Server-selected APS bid can use publisher-owned rendering without silently starting another auction or falling back to the custom renderer. Keep the existing opaque renderer as the default while real publisher and APS compatibility is validated.\n\nSee also: #999
The test publisher cannot install a custom rendering hook. Reuse the existing APS Prebid creative runner contract in a publisher-origin friendly frame so the experiment remains self-contained while preserving the selected server bid and avoiding a second auction.\n\nDocument the larger security surface and retain the opaque renderer as the default.\n\nSee also: #999
@ChristianPavilonis ChristianPavilonis changed the title Test publisher-owned rendering for selected APS bids Test publisher-origin rendering for selected APS bids Aug 19, 2026
@ChristianPavilonis ChristianPavilonis changed the title Test publisher-origin rendering for selected APS bids Add publisher-native rendering for selected APS bids Aug 19, 2026
@aram356 aram356 linked an issue Aug 20, 2026 that may be closed by this pull request
@aram356
aram356 marked this pull request as ready for review August 20, 2026 15:54
@aram356
aram356 requested review from aram356 and prk-Jr and removed request for aram356 August 20, 2026 15:54

@prk-Jr prk-Jr 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.

Summary

An opt-in publisher_native APS rendering mode that hands the server-selected bid to APS's fixed Prebid creative runner inside a publisher-origin friendly frame, keeping the opaque static renderer as the default. The ownership discipline is careful and well tested; the blocking concerns are slot-container resolution on real GAM pages and an unpinned security-relevant default.

1 of the inline comments below carries a one-click GitHub suggestion — use Commit suggestion to apply it as a commit on the PR branch. The remaining comments describe the fix in prose because the change touches multiple files, spans several hunks, or would not survive cargo fmt as a single contiguous replacement.

Blocking

wrench

  • findApsContainer bypasses the codebase's slot-root resolution — see inline at crates/trusted-server-js/lib/src/integrations/aps/render.ts:54
  • No test pins the default mode to an empty head insert — see inline at crates/trusted-server-core/src/integrations/aps.rs:2432

Non-blocking

thinking / refactor / note

  • Native mode leaves the Universal Creative port unanswered and deletes GAM's own creative iframe — see inline at crates/trusted-server-js/lib/src/integrations/aps/render.ts:455
  • prepareApsRunnerDocument reimplements the Rust document's one-line CSS — see inline at crates/trusted-server-js/lib/src/integrations/aps/render.ts:385
  • publisher_native + allow_script_creatives has no guard or operator signal — see inline at crates/trusted-server-core/src/integrations/aps.rs:1255
  • GAM doc paragraph's trailing sentence is still unconditional — see inline at docs/guide/integrations/aps.md:198
  • Prebid path now consumes the capability before apsRendererUrl() is checked — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1701

Cross-cutting / body-level findings

  • Friendly-frame document.open()/write()/close() is proven on Chromium only. crates/trusted-server-integration-tests/browser/playwright.config.* declares a single project, chromium. The premise of this PR is observed vendor behavior in a real browser, and synchronously writing into a freshly appended src-less iframe is exactly where Safari and Firefox have historically diverged on document-replacement timing — the initial about:blank navigation can land after the write. Worth running the new native spec against webkit and firefox, even if only locally, before the cohort test. This does not block the merge; it blocks trusting the result.

  • The one-shot ownership discipline is genuinely careful. The dispatch Symbol plus the releaseNativeDispatch stale-completion guard, the "an invalid replacement still cancels the older pending frame" invariant, deferring markUsed() until the runner actually loads, and the decline-never-falls-back-to-the-opaque-renderer rule each have a matching contract test. expect(portMessages).toEqual([]) is the right way to pin the no-Universal-Creative-response contract — it asserts the absence that the whole mode depends on, rather than only asserting the happy path.

CI Status

  • browser integration tests: PASS
  • integration tests (Fastly EC lifecycle): PASS
  • integration tests: PASS
  • CodeQL: PASS
  • format-typescript: PASS (required)
  • Analyze (actions): PASS
  • cargo test (ts CLI, native): PASS
  • cargo test (cross-adapter parity): PASS
  • cargo test: PASS (required)
  • cargo test (axum native): PASS
  • Analyze (rust): PASS
  • format-docs: PASS (required)
  • cargo check (cloudflare native + wasm32-unknown-unknown): PASS
  • Analyze (javascript-typescript): PASS
  • cargo check/build/test (spin native + wasm32-wasip1): PASS
  • cargo fmt: PASS (required)
  • vitest: PASS
  • prepare integration artifacts: PASS

Comment thread crates/trusted-server-js/lib/src/integrations/aps/render.ts Outdated
Comment thread crates/trusted-server-core/src/integrations/aps.rs
Comment thread crates/trusted-server-js/lib/src/integrations/aps/render.ts
Comment thread crates/trusted-server-js/lib/src/integrations/aps/render.ts
Comment thread crates/trusted-server-core/src/integrations/aps.rs
Comment thread docs/guide/integrations/aps.md Outdated
Comment thread crates/trusted-server-js/lib/src/integrations/gpt/index.ts

@aram356 aram356 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.

Summary

Opt-in publisher_native rendering mode for selected APS bids: config enum + head marker on the Rust side, a shared rendering-owner dispatcher across the direct, server/GPT, and Prebid paths, and a friendly-frame runner with fail-closed, no-fallback semantics. The envelope validation, one-shot ownership (capability consume + tombstone before dispatch), and preserved publisher content on failure are consistent across all three paths, and coverage is thorough (unit contract tests plus a real Playwright friendly-frame proof). Requesting changes for the mode-switch trust model and the missing config guard below; the remaining findings are non-blocking.

Blocking

🔧 wrench

  • Rendering-mode switch is a content-injectable DOM marker: isPublisherNativeApsRendering() trusts any matching <meta> in document.head, so injected markup can flip a default-mode deployment into the unsandboxed friendly frame (crates/trusted-server-js/lib/src/integrations/aps/render.ts:316).
  • No config guard for publisher_native + allow_script_creatives = true: nothing in code enforces the rollout ordering the docs mandate for the highest-risk combination (crates/trusted-server-core/src/integrations/aps.rs:158).

Non-blocking

♻️ refactor

  • Spurious "ignored stale completion" warning on every supersede: the normal replacement path always logs it (crates/trusted-server-js/lib/src/integrations/aps/render.ts:372).
  • Rust test gap: no assertion that TrustedServer mode emits no head marker (crates/trusted-server-core/src/integrations/aps.rs:2374).

📝 note

  • Mode flip strands live sessions and cached HTML: switching trusted_serverpublisher_native removes /integrations/aps/renderer while already-served pages (no marker) still target it — those slots fail closed after the 10s ready timeout until reload. Fail-closed is the right behavior, but a sentence in the Rollout or Troubleshooting section of docs/guide/integrations/aps.md about in-flight sessions and HTML caches would save an operator a confusing debugging session.

⛏ nitpick

  • Unused exports: renderApsPublisherNative / RenderApsPublisherNativeOptions (crates/trusted-server-js/lib/src/integrations/aps/render.ts:409).
  • Renderer route gated in two places: routes() and register() must stay in sync (crates/trusted-server-core/src/integrations/aps.rs:1213).

CI Status

  • fmt: PASS
  • clippy (all six targets): PASS
  • rust tests (fastly/axum/cloudflare/spin/CLI/parity): PASS
  • js tests (vitest): PASS
  • browser integration tests: PASS
  • CodeQL / docs + TS format: PASS

Comment thread crates/trusted-server-js/lib/src/integrations/aps/render.ts Outdated
Comment thread crates/trusted-server-core/src/integrations/aps.rs
Comment thread crates/trusted-server-js/lib/src/integrations/aps/render.ts
Comment thread crates/trusted-server-core/src/integrations/aps.rs
Comment thread crates/trusted-server-js/lib/src/integrations/aps/render.ts Outdated
Comment thread crates/trusted-server-core/src/integrations/aps.rs
ChristianPavilonis added a commit that referenced this pull request Aug 21, 2026

@prk-Jr prk-Jr 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.

Staff-level review pass. Reviewed in a clean detached worktree at head 13a6ecf3, with every claim below scratch-verified before posting.

What this PR does, mechanically

Adds a second, opt-in rendering owner for server-selected APS bids. Rust side: a strict ApsRenderingMode enum (trusted_server default / publisher_native) on ApsConfig; in native mode register() drops the /integrations/aps/renderer proxy route entirely and instead registers ApsRendererIntegration as an IntegrationHeadInjector whose only job is to stamp data-ts-aps-rendering-mode="publisher_native" onto the server-generated TSJS bundle <script> tag (head_inserts deliberately returns empty — the earlier forgeable <meta> marker was removed in 69b30c0d). JS side: render.ts gains a dispatchApsRendering() fan-out that reads that attribute once at module-eval time via document.currentScript, then routes each validated descriptor to exactly one owner — the existing opaque-sandbox renderApsCreative, or the new renderApsPublisherNative() which builds an unsandboxed same-origin friendly iframe, seeds the frame's account-keyed _aps queue with a prebid/creative/render CustomEvent, and loads Amazon's prebid-creative.js. All three call sites are rewired through the dispatcher; in native mode the GPT paths send no Universal Creative response and never fall back.

Verification run locally

Check Result
npx vitest run PASS44 files, 863 tests, no type errors
npm run format (JS) / (docs) PASS / PASS
cargo fmt --all -- --check PASS
clippy-fastly / -axum / -cloudflare / -cloudflare-wasm / -spin-native / -spin-wasm PASS ×6
cargo test -p trusted-server-core --target aarch64-apple-darwin PASS1982 passed, + 2 doc-tests
node build-all.mjs PASS — 13 modules
GitHub CI (gh pr checks) 19/19 pass, incl. cross-adapter parity, browser integration, CodeQL

cargo test-fastly (Viceroy) was not run — all Rust changes are in trusted-server-core, which was exercised natively instead, and GitHub's cargo test job is green.

Verdict

The experiment itself is well-built: descriptor validation, capability-consumption ordering, supersede semantics and fail-closed paths are unusually careful, the security tradeoff is documented honestly rather than buried, and native-mode coverage is strong (unit + jsdom + a real Playwright fixture with a publisher CSP). Rust-side changes are minimal and parity-safe.

Requesting changes on one issue: an experiment behind a non-default config flag must not change the default path, and this one does. dispatchApsRendering moves the pending-frame cancel ahead of validation for all operators, including everyone still on trusted_server — I proved by scratch test that a rejected descriptor now destroys an in-flight valid render that main would have completed. Blank-slot regression on the shipping path, no test covers it, one-line fix that keeps all 863 tests green. The two other suggestion-block findings (inherited referrer policy, stale docs bullet) are mechanical and both verified — worth riding along.

The three design findings (per-bundle rather than per-page slot ownership; commit-on-script-load destroying the GAM iframe; publisher_native + allow_script_creatives guarded only by a log line) need not block the merge, but all three should be answered before any traffic is pointed at publisher_native — particularly commit-on-script-load, which is the decision most likely to cost impressions in a live cohort.

Checked and found clean

  • Cross-adapter parity. No stream_response, no fastly::*, no platform-gated API. tsjs_script_tag_attributes is an existing adapter-agnostic core mechanism (registry.rs:1063, consumed at html_processor.rs:359). All six clippy targets and the parity job green.
  • The third-render-context URL-rewrite hazard does not apply. Native mode touches neither rewrite_creative_html nor rewrite_inline_creative_html — no adm HTML is injected. The runner receives the base64 aaxResponse envelope and loads renderer.creativeUrl, which validCreativeUrl already requires to be absolute HTTPS, credential-free, and non-publisher-origin. No relative-vs-absolute trap.
  • hb_adid non-uniqueness. The GPT server path still resolves via slotIdForMessageSource(e.source) and rejects on matchedBid.hb_adid !== adId; requesting-slot-first ordering and its comment untouched. The Prebid path still gates on messageSourceBelongsToAdUnit. The new source plumb-through into findApsContainer reuses the existing contentWindow === source idiom.
  • sourceMatchedCandidates fallback-to-all-candidates. Tried to turn this into a wrong-slot-render finding and could not substantiate it — both native call sites validate the source upstream, and uniqueSlotCandidate requires exactly one match, so ambiguity falls through to the next strategy rather than guessing. Dropped.
  • No publisher-side changes required. The attribute lands on TS's own generated <script id="trustedserver-js">.
  • Mode-attribute forgeability. Captured once at IIFE eval from document.currentScript, and the immediate bundle tag is neither async nor defer (tsjs.rs:43-46), so later-injected markup cannot flip the mode — covered by test.
  • No real operator values in docs, config or tests (example-account, publisher.example, creative.example, fictional throughout). One nit noted inline.
  • Serde contract. snake_case, #[default] TrustedServer, #[serde(default)], unknown values reject — covered by test.
  • Project conventions. No unwrap() in prod code, no anyhow, no thiserror, no println!, no wildcard imports, comments above code, tests colocated. JS tests use vi.spyOn/vi.resetModules (no vi.mock() factory, so vi.hoisted() doesn't apply).
  • TDZ / cleanup / leak audit of renderApsPublisherNative. cleanup() only ever runs after the timeoutId assignment, settled guards re-entry, the timeout is the guaranteed terminal path so nativeDispatches entries always release, and activeFrames.set correctly follows the previous frame's cancel. The un-disconnect()ed MutationObserver dies with the frame's browsing context on iframe.remove() — not a leak. Dropped.
  • Cache/rollback semantics of a mode change. The route-disappears-under-cached-HTML hazard is documented in both the Migration and Rollout sections — good catch by the author.

Pre-existing, not this PR

The APS renderer branch never calls safelyRecordCreativeRequest in either mode, so APS renders are invisible to gptDiagnosticsRecordermain returns early before that point the same way. Native-mode renders inherit the blind spot rather than creating it. Worth a follow-up issue if the cohort needs render-rate telemetry, since blank-slot triage here has historically leaned on tsjs.renders vs tsjs.bids.

Comment thread crates/trusted-server-js/lib/src/integrations/aps/render.ts Outdated
Comment thread crates/trusted-server-js/lib/src/integrations/aps/render.ts
Comment thread docs/guide/integrations/aps.md Outdated
Comment thread crates/trusted-server-js/lib/src/integrations/aps/render.ts
Comment thread crates/trusted-server-js/lib/src/integrations/aps/render.ts
Comment thread crates/trusted-server-core/src/integrations/aps.rs
Comment thread crates/trusted-server-integration-tests/browser/tests/shared/aps-renderer.spec.ts Outdated
Comment thread crates/trusted-server-js/lib/src/integrations/gpt/index.ts

@aram356 aram356 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.

Summary

Re-review at head 07dfc189f. Every finding from the previous review is resolved: the rendering-mode signal moved from the injectable head <meta> marker to the server-owned data-ts-aps-rendering-mode bundle-tag attribute captured once at module init, with a regression test proving post-init injected meta tags and spoofed script tags are ignored, and the Playwright proof now runs under a CSP without 'unsafe-inline'. The script-creatives combination is settled with a warn-level startup log plus a test documenting the controlled-validation intent. The stale-completion warning, default-mode marker assertions, mode-flip/cached-HTML docs, and module-private renderer internals are all in place. The findApsContainer rework (requesting-frame disambiguation, -container inner resolution, unique-candidate dynamic prefixes) is conservative where candidates are ambiguous and is covered by new unit tests on both the render and bridge sides.

The findings below are non-blocking.

Non-blocking

❓ question

  • Foreign-author "probe" commit on the branch (d97bda69f, authored by prk-Jr): a 1.1MB commit duplicating main's content (ESI designs, template cache) under the message "probe", later netted out by the 07dfc189f main merge. Verified it leaks nothing into the PR — the diff vs main is exactly the 10 expected APS files (+1324/−59) — and squash-merge keeps it out of main's history. Was this push intentional? Dropping it with a rebase would keep the branch history honest; otherwise a quick confirmation here is enough.

🌱 seedling

  • Deferred-bundle copy of render.ts cannot see the mode attribute: the deferred tsjs-prebid.js bundles its own copy of this module, and deferred tags from tsjs_deferred_script_tags() do not carry tsjs_script_tag_attributes() (crates/trusted-server-js/lib/src/integrations/aps/render.ts:47).

⛏ nitpick

  • Renderer route gated in two places: routes() and register() both gate on the mode and must stay in sync (crates/trusted-server-core/src/integrations/aps.rs:1213).

CI Status

  • fmt: PASS
  • clippy (all six targets): PASS
  • rust tests (fastly/axum/cloudflare/spin/CLI/parity): PASS
  • js tests (vitest): PASS
  • integration + browser integration tests (including the APS publisher-native Playwright proof): PASS
  • CodeQL / docs + TS format: PASS

Comment thread crates/trusted-server-js/lib/src/integrations/aps/render.ts
Comment thread crates/trusted-server-core/src/integrations/aps.rs
Invalid descriptors in default mode could cancel valid in-flight opaque
renderer frames because the shared dispatcher cancelled before validation.
Restrict early supersession to native mode so default behavior remains
stable.

Preserve the no-referrer policy in friendly frames and align browser
coverage and operator guidance with the actual script-tag mode signal.
@ChristianPavilonis
ChristianPavilonis merged commit 616b740 into main Aug 25, 2026
@ChristianPavilonis
ChristianPavilonis deleted the experiment/aps-native-rendering branch August 25, 2026 15:51
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.

APS creatives requiring nested same-origin semantics render blank

3 participants