Name the guard that stops an APS creative render

[jevansnyc]

Why

Investigating blank ads on a live publisher page, APS bids were winning the
auction and Ad Manager was filling the slot, yet the creative never drew. The
tester framework reported the slot as "filled" at 1x1, which is exactly what a
successful universal-creative render looks like before it resizes. Nothing
downstream distinguished the two.

Two blind spots made this close to undiagnosable from the outside:

• Every guard on the APS render path returned silently. A frame that rejected
  the descriptor, one that never received it, and one that timed out all looked
  identical: an iframe that loaded and did nothing.
• The APS capability handshake never reported to GPT diagnostics, so ts_console
  showed delivery: unknown on every request cycle and zero creative failures,
  while APS bids were rendering blank.

On the page under investigation that was 24 of 24 cycles unattributed.

What changed

The sandboxed renderer document (aps.rs) reports which guard stopped it,
on the existing failure message: bad_hash, source_mismatch,
nonce_mismatch, descriptor_keys, descriptor_fields,
descriptor_envelope, amazon_script_error.

The Universal Creative source (render.ts) labels its own frame_timeout
and frame_load_error and relays whichever reason it holds to the top window.

The GPT bridge (gpt/index.ts) records a creative attempt around the
handshake and names each silent return: aps_consumed_tombstone,
aps_source_not_in_ad_unit, aps_descriptor_fields, aps_tombstone_capacity,
aps_missing_renderer_url. A successful post records a response, so this path
reports trusted_server_response_sent rather than unknown.

Notes for review

The APS path runs on the publisher's own Prebid ad units, which never pass
through Trusted Server slot mapping. No creative opportunity exists for them, so
the store rejected the attempt as creative_request_without_slot. The bridge now
resolves the GPT slot by element ID and records the opportunity first. That is
the least obvious part of the change and the part most worth a look.

Security posture, since a reason crosses an origin boundary:

• Reasons are fixed categories. A rejected descriptor is never echoed back.
• Reporting from the frame is one-shot and answers through parent, never the
  sender, so an unrelated sender cannot consume the report or learn from it.
  Traffic not shaped like the handshake stays silent, as before.
• Relayed reasons resolve through a null-prototype allowlist, so a hostile
  __proto__, constructor, or toString resolves to undefined.
• The attempt is taken from our own tombstone, never from the message.
• The relay is recording only and never influences delivery.

This is instrumentation. It does not attempt a fix, because the root cause is
still unknown: the evidence says the handshake completes and the renderer frame
loads, then the chain dies before Amazon's prebid-creative.js is requested.
These reason codes are what will name it on the next occurrence.

Testing

• render.test.ts: relay of a frame reason, frame timeout, and the allowlist
  including inherited-key and non-string rejection
• ad_init.test.ts: creative attempt recorded for a registered APS renderer,
  and the tombstone reason on a replayed ad ID
• aps.rs: every guard reports a reason, reporting is one-shot, no descriptor
  echo, and the frame never answers the sender

Gates run: cargo fmt --check, clippy (fastly target, -D warnings),
cargo test -p trusted-server-core (2143 passed), JS build, JS format, and the
full vitest suite (850 passed).

Two caveats. The vitest suite has 27 pre-existing failures in
sourcepoint/index.test.ts and permutive/segments.test.ts, all one
localStorage error from running Node 26 against a repo pinned to 24.12.0; the
count is unchanged by this branch. And the remaining adapter gates (axum,
cloudflare, spin, parity) were not run locally, because a .cargo/config.toml
alias conflict with a stale sibling checkout meant cargo had to be invoked from
outside the repo without workspace aliases. CI covers those.

[aram356]

@aram356 Will test in staging before merging in #1019

[aram356]

@ChristianPavilonis to understand if belongs in #1019

[ChristianPavilonis]

Summary

Reviewed cb1de4777cdd5efe2a32892c03d00c1b071795f6. Requesting changes because the new reporting path can alter APS delivery and the diagnostics pipeline does not currently retain the APS failure evidence this PR introduces. Four actionable findings are posted inline.

[prk-Jr]

Summary

Instrumentation for the APS Universal Creative render path: each silent guard now names itself, the creative frame relays its reason to the top window through a null-prototype allowlist, and the GPT bridge opens a diagnostics attempt around the capability handshake. The security reasoning in the description is unusually careful and the allowlist is the right shape for a cross-origin relay.

Three defects block it, all verified with runnable probes rather than read off the diff. The headline one is that none of the fifteen new reason codes can reach the store: isCreativeFailure in store.ts still allowlists only the original four, so every recordTrustedServerCreativeFailure(attemptId, 'aps_*') call added here is a no-op. ts_console will still report zero creative failures on the APS path, which is the exact blind spot the PR exists to close. Separately, adding a reason key to the renderer document's failure message breaks the direct (non-Prebid) render path, which gates on an exact two-key match.

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 files or lines outside this diff and can't be auto-applied.

Blocking

🔧 wrench

• Every new aps_* reason is dropped before it reaches the store — see inline at crates/trusted-server-js/lib/src/core/types.ts:213
• creativeFailureFact is non-exhaustive; ts_console will render undefined — see inline at crates/trusted-server-js/lib/src/core/types.ts:229
• Direct APS render path stops tearing down on renderer-failed — see inline at crates/trusted-server-core/src/integrations/aps.rs:60
• The new bridge tests mock the recorder, so they cannot catch the store gap — see inline at crates/trusted-server-js/lib/test/integrations/gpt/ad_init.test.ts:3379
• The relay branch has no test — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1725

Non-blocking

🤔 thinking / ♻️ refactor / 📝 note

• source_mismatch burns the one-shot report and is attacker-triggerable — see inline at crates/trusted-server-core/src/integrations/aps.rs:103
• The opportunity lands on the slot's next request cycle, mislabeling it — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1619
• beginApsCreativeAttempt runs before the source check — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1760
• window.googletag untyped access — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1616 (carries a suggestion)

👍 praise

• Null-prototype allowlist, and tests that actually prove it — see inline at crates/trusted-server-js/lib/src/integrations/aps/render.ts:56

Cross-cutting / body-level findings

• 📝 New type errors introduced, though tsc is not a gate here. A tsc --noEmit delta between the merge-base and this head shows 8 new errors: the overlay.ts TS2366 and the gpt/index.ts TS2339 covered inline, plus 6 × TS2532 (Object is possibly 'undefined') in ad_init.test.ts. The base already carries 289 errors so this is not a regression in gate terms, but the first two are pointing at real defects — worth noting that the type system did flag both of the JS-side bugs found in this review, and nothing was listening.

CI Status

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

All 14 checks green. Every finding below survives green CI — noted in each comment where the existing tests structurally cannot catch the defect.

[aram356]

Summary

This is careful, well-documented instrumentation, and the reasoning in the PR description about the security posture of the relay is sound. But the change does not currently deliver the outcome it is written for: I verified against a real GptDiagnosticsStore that every one of the 15 new aps_* reasons is discarded before it reaches a diagnostics record, so a blank APS render still reports delivery: unknown. Two further defects sit behind that one, and one of them is a behavioural regression on the direct render path rather than a diagnostics-only concern.

The full JS suite passes on this branch (899/899). That is precisely the problem: the new tests mock gptDiagnosticsRecorder with vi.fn()s and never exercise the store, so the gap between the widened type union and the store's runtime guard is invisible to them.

A note on the PR description: the 27 vitest failures you reported are not reproducible here. On the repo-pinned Node 24.12.0 the suite is fully green, which matches your own diagnosis that they were a Node 26 artifact.

None of the inline comments below carry a one-click suggestion. Each fix either lands in a file outside this diff (store.ts), touches lines outside a diff hunk, or needs a design decision, so all of them describe the change in prose instead.

Blocking

wrench

• The store rejects all 15 new aps_* reasons; nothing is ever recorded - see inline at crates/trusted-server-js/lib/src/core/types.ts:197
• Relayed frame failures land on a completed attempt and are dropped - see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1809
• beginApsCreativeAttempt corrupts attribution for the next request cycle - see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1616
• The new 3-key failure message breaks the direct render path - see inline at crates/trusted-server-js/lib/src/integrations/aps/render.ts:771
• Reordering the source check lets a co-resident script cancel a successful render - see inline at crates/trusted-server-core/src/integrations/aps.rs:104

Non-blocking

thinking / nitpick

• renderable_candidate is semantically wrong on this path - see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1618
• The relay handler applies no source validation - see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1723
• m.nonce===undefined widens what can cancel a pending render - see inline at crates/trusted-server-js/lib/src/integrations/aps/render.ts:775
• The guard-coverage test is close to a tautology - see inline at crates/trusted-server-core/src/integrations/aps.rs:2629
• The server-side APS path gets no instrumentation - see the cross-cutting section below

Cross-cutting / body-level findings

• thinking - The server-side APS render path is left uninstrumented. installTsRenderBridge has two APS paths. This PR instruments the Prebid-capability path thoroughly, but the server-side path at gpt/index.ts:1848-1875 (reached when matchedBid.renderer !== undefined) still has the same silent returns the PR is written to eliminate: if (!renderer) return; and if (!rendererUrl) return false;, with no safelyRecordCreativeFailure on either. Given the PR's framing, a reader will assume APS render failures are now attributable in general, and on this path they are not. If the omission is deliberate because the live investigation only involved the publisher-Prebid path, that is a reasonable scope decision, but it is worth a sentence in the PR description so the next person debugging a blank server-APS slot is not misled by the reason codes' apparent coverage.

• note - How these findings were verified. Findings 1, 2, and 3 were each confirmed by running throwaway tests against the real GptDiagnosticsStore rather than by reading alone, and finding 4 by driving renderApsCreative end to end. The scratch tests were discarded; the observed outputs are quoted in the relevant inline comments. Gates run locally on this head: cargo fmt --check PASS, cargo clippy-fastly PASS, cargo test -p trusted-server-core 2255 passed / 0 failed, npx vitest run 899 passed / 0 failed, npm run format PASS.

CI Status

• No GitHub checks reported on aps-renderer-failure-diagnostics (gh pr checks returns "no checks reported on the branch"); not run remotely. Locally on this head: cargo fmt --check PASS, cargo clippy-fastly PASS, cargo test -p trusted-server-core PASS (2255), npx vitest run PASS (899), npm run format PASS. The remaining adapter gates (axum, cloudflare, spin, parity) were not run.