Add publisher-native rendering for selected APS bids - #1042
Conversation
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
prk-Jr
left a comment
There was a problem hiding this comment.
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 survivecargo fmtas a single contiguous replacement.
Blocking
wrench
findApsContainerbypasses the codebase's slot-root resolution — see inline atcrates/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 prepareApsRunnerDocumentreimplements the Rust document's one-line CSS — see inline atcrates/trusted-server-js/lib/src/integrations/aps/render.ts:385publisher_native+allow_script_creativeshas no guard or operator signal — see inline atcrates/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 atcrates/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 initialabout:blanknavigation can land after the write. Worth running the new native spec againstwebkitandfirefox, 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
Symbolplus thereleaseNativeDispatchstale-completion guard, the "an invalid replacement still cancels the older pending frame" invariant, deferringmarkUsed()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
aram356
left a comment
There was a problem hiding this comment.
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>indocument.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
TrustedServermode emits no head marker (crates/trusted-server-core/src/integrations/aps.rs:2374).
📝 note
- Mode flip strands live sessions and cached HTML: switching
trusted_server→publisher_nativeremoves/integrations/aps/rendererwhile 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 ofdocs/guide/integrations/aps.mdabout 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()andregister()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
prk-Jr
left a comment
There was a problem hiding this comment.
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 |
PASS — 44 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 |
PASS — 1982 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, nofastly::*, no platform-gated API.tsjs_script_tag_attributesis an existing adapter-agnostic core mechanism (registry.rs:1063, consumed athtml_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_htmlnorrewrite_inline_creative_html— noadmHTML is injected. The runner receives the base64aaxResponseenvelope and loadsrenderer.creativeUrl, whichvalidCreativeUrlalready requires to be absolute HTTPS, credential-free, and non-publisher-origin. No relative-vs-absolute trap. hb_adidnon-uniqueness. The GPT server path still resolves viaslotIdForMessageSource(e.source)and rejects onmatchedBid.hb_adid !== adId; requesting-slot-first ordering and its comment untouched. The Prebid path still gates onmessageSourceBelongsToAdUnit. The newsourceplumb-through intofindApsContainerreuses the existingcontentWindow === sourceidiom.sourceMatchedCandidatesfallback-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, anduniqueSlotCandidaterequires 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 neitherasyncnordefer(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, noanyhow, no thiserror, noprintln!, no wildcard imports, comments above code, tests colocated. JS tests usevi.spyOn/vi.resetModules(novi.mock()factory, sovi.hoisted()doesn't apply). - TDZ / cleanup / leak audit of
renderApsPublisherNative.cleanup()only ever runs after thetimeoutIdassignment,settledguards re-entry, the timeout is the guaranteed terminal path sonativeDispatchesentries always release, andactiveFrames.setcorrectly follows the previous frame's cancel. The un-disconnect()edMutationObserverdies with the frame's browsing context oniframe.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 gptDiagnosticsRecorder — main 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.
aram356
left a comment
There was a problem hiding this comment.
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 byprk-Jr): a 1.1MB commit duplicating main's content (ESI designs, template cache) under the message "probe", later netted out by the07dfc189fmain 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.tscannot see the mode attribute: the deferredtsjs-prebid.jsbundles its own copy of this module, and deferred tags fromtsjs_deferred_script_tags()do not carrytsjs_script_tag_attributes()(crates/trusted-server-js/lib/src/integrations/aps/render.ts:47).
⛏ nitpick
- Renderer route gated in two places:
routes()andregister()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
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.
Summary
publisher_nativemode for rendering Trusted Server-selected APS bids through APS's fixed Prebid creative runner in a publisher-origin friendly iframe.rendering_mode = "publisher_native"is explicitly configured.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/renderrunner contract inside a friendly frame. Trusted Server does not callapstag.fetchBids(), callapstag.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
crates/trusted-server-core/src/integrations/aps.rscrates/trusted-server-integration-tests/browser/tests/shared/aps-renderer.spec.tscrates/trusted-server-js/lib/src/core/request.tscrates/trusted-server-js/lib/src/integrations/aps/render.tscrates/trusted-server-js/lib/src/integrations/gpt/index.tscrates/trusted-server-js/lib/test/core/request.test.tscrates/trusted-server-js/lib/test/integrations/aps/render.test.tscrates/trusted-server-js/lib/test/integrations/gpt/ad_init.test.tsdocs/guide/integrations/aps.mdtrusted-server.example.tomlScope
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
apstagmethods, 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-axumcargo clippy-fastly && cargo clippy-axumcargo fmt --all -- --checkcd crates/trusted-server-js/lib && npx vitest run— 44 files, 842 testscd crates/trusted-server-js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute servets dev proxyrendered a real selected APS creative with no/integrations/aps/rendererrequestChecklist
unwrap()in production code — useexpect("should ...")println!)