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.

[ChristianPavilonis]

🔧 P1: Wire the new APS reasons through the real diagnostics consumers

The runtime validator in gpt_diagnostics/store.ts:168-174 still accepts only the four original failure values, so every new aps_* reason is discarded by recordTrustedServerCreativeFailure(). The overlay switch in gpt_diagnostics/overlay.ts:185-194 also has no APS cases. As a result, ts_console and exported snapshots will continue showing no APS creative failures. The new tests mock the recorder and therefore do not exercise either consumer.

Update the store allowlist and overlay labels for every new category, make the presentation switch exhaustive, and add a test that passes an APS reason through the actual recorder/store and verifies the snapshot and overlay output.

[ChristianPavilonis]

🔧 P1: Do not let a foreign sender terminate rendering

A shaped message from a non-parent source now emits source_mismatch without a nonce. The Universal Creative wrapper accepts nonce-less failure messages from this iframe at render.ts:623, calls fail(), removes the iframe, and rejects the render. A sibling or ancestor window that obtains the iframe's WindowProxy can therefore race the valid parent message and suppress APS delivery. Previously this traffic was ignored, and the PR describes reporting as unable to influence delivery.

Keep event.source !== parent silent, or route this observation through a path that cannot trigger terminal renderer failure. Please add a test where a foreign shaped message arrives before the valid nonce-bound parent handshake and confirm the render still succeeds.

[ChristianPavilonis]

🔧 P1: Keep the attempt writable for downstream renderer failures

This marks the attempt completed as soon as port.postMessage returns. The store then clears its cycle at gpt_diagnostics/store.ts:529-531, and recordTrustedServerCreativeFailure() drops all completed attempts at line 549. Renderer timeouts, descriptor failures, Amazon script failures, and consumed-ID replays necessarily arrive after this response, so they cannot be retained even after the APS allowlist is fixed. The existing store test at store.test.ts:1707 explicitly verifies that post-completion failures are ignored.

Keep the cycle association writable for the 30-second mutation window after recording the independent response timestamp, or add a separate post-response failure channel. Cover response sent followed by a frame failure and a replay using the real diagnostics store.

[ChristianPavilonis]

🔧 P2: Do not create next-request evidence while associating the current APS attempt

This handshake occurs after the current GPT request, but recordTrustedServerOpportunity() explicitly records evidence for the slot's next request (gpt_diagnostics/store.ts:334 and :925-953). The creative attempt attaches to the latest existing cycle while this pending intent survives. A refresh within five seconds can therefore mislabel the next cycle as trusted_server_direct with a renderable candidate, or as competing when its real Prebid or publisher marker is also present.

Use a separate operation that associates the APS ad-unit code with the current GPT slot without creating next-request intent, or add an APS-specific current-cycle attempt method. Add a two-cycle test proving the following refresh receives only its own request-path evidence.

[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.

[prk-Jr]

🔧 Every new aps_* reason is dropped before it reaches the store.

The union grew fifteen members here, but the runtime allowlist it is validated against did not. store.ts:168 is unchanged by this PR:

function isCreativeFailure(reason: unknown): reason is GptDiagnosticsCreativeFailure {
  return (
    reason === 'missing_render_source' ||
    reason === 'cache_fetch_failed' ||
    reason === 'invalid_cache_payload' ||
    reason === 'response_post_failed'
  );
}

recordTrustedServerCreativeFailure returns early at store.ts:540 on !isCreativeFailure(reason), so every safelyRecordCreativeFailure(attemptId, 'aps_*') call added in this PR is a no-op.

Verified against the real GptDiagnosticsStore — both reasons recorded on one live attempt, then snapshotted:

has response_post_failed: true
has aps_frame_timeout:    false

"trustedServerCreativeFailures": ["response_post_failed"]

So ts_console will still show zero creative failures on the APS path. delivery: trusted_server_response_sent does work — I confirmed that separately — but the reason codes, which are the point of the change, never land.

Fix in store.ts:168-175: extend the guard with all fifteen members. Worth deriving both the type and the guard from one const array of literals so they can't drift apart again — that drift is the whole bug, and it will recur the next time a reason is added.

Not offered as a suggestion: the fix is in a file outside this diff.

[prk-Jr]

🔧 creativeFailureFact is non-exhaustive; ts_console will render undefined.

overlay.ts:182-195 switches over this union with no default and a declared : string return. The fifteen new members fall through and return undefined, which overlay.ts:268-270 pushes straight into the facts list:

for (const failure of new Set(cycle.trustedServerCreativeFailures ?? [])) {
  facts.push(creativeFailureFact(failure));
}

This is latent today only because the store gap above blocks every aps_* reason from ever reaching the overlay. Fix that one alone and ts_console starts printing undefined lines instead of failure reasons — so these two need to ship together.

TypeScript does flag it (overlay.ts(184,4): error TS2366: Function lacks ending return statement and return type does not include 'undefined'), confirmed new by diffing tsc --noEmit between the merge-base and this head. It isn't a CI gate here, since the base already carries 289 pre-existing errors.

Fix: add a case per new member in creativeFailureFact. Not offered as a suggestion — the fix is in a file outside this diff.

[prk-Jr]

🔧 This breaks the direct APS render path's failure teardown.

report() always posts three keys — message, nonce, reason — including when nonce is undefined. The direct (non-Prebid) render path still gates on an exact two-key match, at render.ts:573, unchanged by this PR:

if (event.source !== iframe.contentWindow || !hasExactKeys(event.data, ['message', 'nonce'])) {
  return;
}

hasExactKeys compares the full sorted key set, so every failure message from this document is now silently discarded there and fail() never runs. The frame is no longer torn down on an explicit failure — it lingers for the full RENDERER_READY_TIMEOUT_MS (10s) before the timeout restores publisher content.

Verified by driving renderApsCreative with both message shapes:

OLD shape {message,nonce}        -> frame removed: true
NEW shape {message,nonce,reason} -> frame removed: false

renderer-ready still posts exactly two keys, so success is unaffected — only the failure fast path is dead. CI stayed green because the nearest existing test (leaves existing slot content intact when validation or loading fails) exercises the iframe error event, never the message, and the Rust test added here is string-matching on APS_RENDERER_DOCUMENT so it structurally cannot see the mismatch.

Fix at render.ts:573, accepting both shapes:

if (
  event.source !== iframe.contentWindow ||
  (!hasExactKeys(event.data, ['message', 'nonce']) &&
    !hasExactKeys(event.data, ['message', 'nonce', 'reason']))
) {
  return;
}

Worth a regression test on that path too, since nothing currently covers the message-driven teardown. Not offered as a suggestion: the fix is in a file outside this diff.

[prk-Jr]

🤔 Moving the source check below the shape check lets a foreign sender consume the one-shot report.

Previously event.source !== parent returned first, so a non-parent sender could never reach report(). Now any window that can post a well-shaped {nonce, renderer} to this frame reaches this line, sets reported = true, and burns the frame's single report.

That is reachable in practice: indexed access on a cross-origin WindowProxy is allowed by spec, so any iframe on the page can traverse top.frames[...] down to this frame and postMessage to it. The consequence is precisely the failure mode this PR exists to fix — the frame's real later failure, amazon_script_error, is never reported, and a false aps_source_mismatch is recorded in its place, pointing the next investigation at the wrong guard.

The description says "an unrelated sender cannot consume the report or learn from it." The second half holds — answering through parent is right. The first half doesn't.

source_mismatch also has close to zero diagnostic value on its own: our Universal Creative source always posts via f.contentWindow.postMessage, so event.source is parent on every legitimate path. The only way this reason can fire is a foreign sender, which makes it a pure attack surface against the diagnostics rather than a signal.

Proposed — restore the ordering and drop the reason:

function receive(event){
 if(event.source!==parent)return;
 var message=event.data;
 // Stay silent for traffic that is not shaped like the render handshake, so an
 // unrelated sender cannot consume this frame's single report.
 if(!keys(message,['nonce','renderer']))return;
 if(message.nonce!==expected){report('nonce_mismatch');return;}

If you'd rather keep the reason, the alternative is to report it without setting reported, so a foreign sender can't suppress the real one.

Not offered as a one-click suggestion because either shape needs matching updates: renderer_document_reports_a_reason_for_every_silent_guard asserts the document contains source_mismatch, and types.ts carries the aps_source_mismatch member.

[prk-Jr]

♻️ Untyped window access — every other call site in this file casts to GptWindow.

See :958 ((window as GptWindow).googletag), :630, and :826. This one is bare, which produces a new TS2339: Property 'googletag' does not exist on type 'Window & typeof globalThis'.

Suggested change

	const pubads = window.googletag?.pubads?.();
	const pubads = (window as GptWindow).googletag?.pubads?.();

Scratch-verified in an isolated worktree at this head: removes the TS2339 with no new errors in the tsc --noEmit delta, eslint --max-warnings=0 clean, prettier --check clean, and all 124 ad_init.test.ts tests still pass.

[prk-Jr]

🤔 This stamps the slot's next request cycle as Trusted Server-driven, not the one being rendered.

recordTrustedServerOpportunity writes a pending intent via recordRequestIntentSource (store.ts:932), which attaches to the slot's next GPT request. But recordTrustedServerCreativeRequest, called immediately after, resolves against the slot's latest already-recorded cycle. The two land on different cycles.

Reproduced against the real store with the ordering the APS path actually produces (GPT request fires, then the UC handshake arrives):

cycle 1: requestPath "unattributed",
         delivery    "trusted_server_response_sent"          <- the actual APS render
cycle 2: requestPath "trusted_server_direct",
         trustedServerOpportunity "renderable_candidate"     <- phantom

The good news is the headline claim holds: cycle 1 reads trusted_server_response_sent, because delivery() keys off trustedServerCreativeResponseAtMs rather than requestPath. So this is not blocking.

The cost is that every APS handshake also labels the slot's next refresh as Trusted Server-driven with a renderable_candidate opportunity, when TS never participated in it. On a page with APS refreshes that is a steady stream of false trusted_server_direct cycles in the console this PR exists to sharpen.

All recordTrustedServerCreativeRequest actually needs from this call is the slot association (trustedServerSlots.set, store.ts:352). The pending intent is an unwanted side effect of borrowing recordTrustedServerOpportunity to establish it — a narrower recorder entry point that registers the association without writing an intent would avoid it.

[prk-Jr]

📝 Note: the attempt opens before the source is checked.

beginApsCreativeAttempt runs ahead of messageSourceBelongsToAdUnit, so a foreign iframe posting Prebid Request with a known TS ad ID opens a diagnostics attempt (and records the opportunity) before being rejected on the next line.

This looks deliberate given the fail-closed comment above, and the blast radius is bounded — attemptIdsByCycle reuses one live attempt per cycle, and recordTrustedServerCreativeFailure dedupes by reason (store.ts:563-565), so there's no unbounded growth. Recording it as diagnostics pollution rather than a defect, but worth naming since the reader of ts_console can't distinguish these attempts from real ones.

[prk-Jr]

🔧 This branch has no test.

It's the one place a cross-origin reason becomes a recorded attempt — the untrusted-input boundary of the whole feature — and nothing exercises it. render.test.ts covers the emission side (relay of a frame reason, frame timeout) and apsRenderFailureReason in isolation, but nothing covers consumption: the tombstone lookup, the attemptId resolution, or the drop when failedAdId matches no tombstone.

Worth covering at least:

• a relayed reason with a known tombstone ad ID recording against that tombstone's attemptId
• an unknown ad ID resolving to undefined and recording nothing
• a reason outside the allowlist being dropped

[prk-Jr]

🔧 These tests cannot catch the store gap, by construction.

Both new tests replace gptDiagnosticsRecorder wholesale with four vi.fn() spies and then assert the bridge called them. That verifies the call site and nothing downstream — which is exactly why the isCreativeFailure gap flagged on types.ts:213 shipped with green CI. The assertion expect(recordTrustedServerCreativeFailure).toHaveBeenCalledWith(11, 'aps_consumed_tombstone') passes whether or not the store accepts aps_consumed_tombstone, and it does not.

A single test driving the real GptDiagnosticsStore — record an opportunity and a creative request, feed one aps_* failure, assert it appears in snapshot().slots[].requests[].trustedServerCreativeFailures — would have failed immediately and caught it. That end-to-end assertion is the one worth adding here, since the reason codes reaching the console is the deliverable, not the recorder being called.

Same shape of gap on the Rust side: renderer_document_reports_a_reason_for_every_silent_guard string-matches APS_RENDERER_DOCUMENT rather than executing it, so it can't see the message-shape break flagged on aps.rs:60.

[prk-Jr]

👍 Right shape for a cross-origin relay, and the tests actually prove it.

Null-prototype backing object plus Object.freeze, with resolution through a typeof value === 'string' guard, is the correct defence here — and the tests don't just assert the happy path. __proto__, constructor, toString, a non-string, and { toString: () => 'frame_timeout' } are all covered, which is the set that usually gets missed. Keeping the rejected descriptor out of the reason entirely, and answering through parent rather than the sender, are both the right calls.