Skip to content

Name the guard that stops an APS creative render #1052

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jevansnyc wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Name the guard that stops an APS creative render #1052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dc34278
Report a reason when an APS creative render fails
jevansnyc Aug 20, 2026
fae9e35
Attribute APS creative delivery in GPT diagnostics
jevansnyc Aug 20, 2026
4d3e02d
Merge branch 'main' into aps-renderer-failure-diagnostics
aram356 Sep 7, 2026
6b36f92
Merge branch 'main' into aps-renderer-failure-diagnostics
aram356 Sep 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 69 additions & 20 deletions crates/trusted-server-core/src/integrations/aps.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,42 +57,54 @@ const APS_RENDERER_DOCUMENT: &str = r#"<!doctype html>
var match=/^#tsaps=([A-Za-z0-9_-]{22,128})$/.exec(location.hash);
var expected=match&&match[1];
try{history.replaceState(null,'',location.pathname+location.search);}catch(_error){}
if(!expected)return;
var reported=false;
function report(reason,nonce){
if(reported)return;
reported=true;
try{parent.postMessage({message:'trusted-server/aps/renderer-failed',nonce:nonce,reason:reason},'*');}catch(_error){}
Comment on lines +61 to +64

@prk-Jr prk-Jr Aug 28, 2026

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.

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

}
if(!expected){report('bad_hash');return;}
function keys(value,expectedKeys){
if(!value||typeof value!=='object'||Array.isArray(value))return false;
var actual=Object.keys(value).sort();
return actual.length===expectedKeys.length&&actual.every(function(key,index){return key===expectedKeys[index];});
}
function validRenderer(renderer){
function rendererProblem(renderer){
if(!keys(renderer,['aaxResponse','accountId','bidId','creativeId','creativeUrl','height','tagType','type','version','width'])&&
!keys(renderer,['aaxResponse','accountId','bidId','creativeUrl','height','tagType','type','version','width']))return false;
if(renderer.type!=='aps'||renderer.version!==1||typeof renderer.accountId!=='string'||!renderer.accountId||new TextEncoder().encode(renderer.accountId).length>1024)return false;
if(typeof renderer.bidId!=='string'||!renderer.bidId||!Number.isInteger(renderer.width)||renderer.width<=0||!Number.isInteger(renderer.height)||renderer.height<=0)return false;
if(Object.prototype.hasOwnProperty.call(renderer,'creativeId')&&(typeof renderer.creativeId!=='string'||!renderer.creativeId||new TextEncoder().encode(renderer.creativeId).length>1024))return false;
if(renderer.tagType!=='iframe'&&renderer.tagType!=='script')return false;
if(typeof renderer.creativeUrl!=='string'||new TextEncoder().encode(renderer.creativeUrl).length>4096)return false;
if(typeof renderer.aaxResponse!=='string'||!renderer.aaxResponse||renderer.aaxResponse.length>349528)return false;
!keys(renderer,['aaxResponse','accountId','bidId','creativeUrl','height','tagType','type','version','width']))return 'descriptor_keys';
if(renderer.type!=='aps'||renderer.version!==1||typeof renderer.accountId!=='string'||!renderer.accountId||new TextEncoder().encode(renderer.accountId).length>1024)return 'descriptor_fields';
if(typeof renderer.bidId!=='string'||!renderer.bidId||!Number.isInteger(renderer.width)||renderer.width<=0||!Number.isInteger(renderer.height)||renderer.height<=0)return 'descriptor_fields';
if(Object.prototype.hasOwnProperty.call(renderer,'creativeId')&&(typeof renderer.creativeId!=='string'||!renderer.creativeId||new TextEncoder().encode(renderer.creativeId).length>1024))return 'descriptor_fields';
if(renderer.tagType!=='iframe'&&renderer.tagType!=='script')return 'descriptor_fields';
if(typeof renderer.creativeUrl!=='string'||new TextEncoder().encode(renderer.creativeUrl).length>4096)return 'descriptor_fields';
if(typeof renderer.aaxResponse!=='string'||!renderer.aaxResponse||renderer.aaxResponse.length>349528)return 'descriptor_fields';
try{
var url=new URL(renderer.creativeUrl);
if(url.protocol!=='https:'||url.username||url.password)return false;
if(url.protocol!=='https:'||url.username||url.password)return 'descriptor_envelope';
var binary=atob(renderer.aaxResponse);
if(binary.length>262144||btoa(binary)!==renderer.aaxResponse)return false;
if(binary.length>262144||btoa(binary)!==renderer.aaxResponse)return 'descriptor_envelope';
var bytes=Uint8Array.from(binary,function(character){return character.charCodeAt(0);});
var decoded=JSON.parse(new TextDecoder('utf-8',{fatal:true}).decode(bytes));
if(!keys(decoded,['seatbid'])||!Array.isArray(decoded.seatbid)||decoded.seatbid.length!==1)return false;
if(!keys(decoded,['seatbid'])||!Array.isArray(decoded.seatbid)||decoded.seatbid.length!==1)return 'descriptor_envelope';
var seat=decoded.seatbid[0];
if(!keys(seat,['bid'])||!Array.isArray(seat.bid)||seat.bid.length!==1)return false;
if(!keys(seat,['bid'])||!Array.isArray(seat.bid)||seat.bid.length!==1)return 'descriptor_envelope';
var bid=seat.bid[0];
if(!keys(bid,['ext','h','id','price','w'])||!keys(bid.ext,['creativeurl','tagtype']))return false;
return bid.id===renderer.bidId&&bid.w===renderer.width&&bid.h===renderer.height&&
if(!keys(bid,['ext','h','id','price','w'])||!keys(bid.ext,['creativeurl','tagtype']))return 'descriptor_envelope';
if(bid.id===renderer.bidId&&bid.w===renderer.width&&bid.h===renderer.height&&
bid.ext.creativeurl===renderer.creativeUrl&&bid.ext.tagtype===renderer.tagType&&
typeof bid.price==='number'&&Number.isFinite(bid.price)&&bid.price>=0;
}catch(_error){return false;}
typeof bid.price==='number'&&Number.isFinite(bid.price)&&bid.price>=0)return undefined;
return 'descriptor_envelope';
}catch(_error){return 'descriptor_envelope';}
}
function receive(event){
if(event.source!==parent)return;
var message=event.data;
if(!keys(message,['nonce','renderer'])||message.nonce!==expected||!validRenderer(message.renderer))return;
// 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(event.source!==parent){report('source_mismatch');return;}

@ChristianPavilonis ChristianPavilonis Aug 27, 2026

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.

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

@prk-Jr prk-Jr Aug 28, 2026

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.

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

@aram356 aram356 Sep 4, 2026

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.

wrench - Moving the source check below the shape check lets a co-resident script cancel a render that would have succeeded.

Two things changed together here: event.source!==parent moved from the first line of receive to third place, and it now calls report(...), which latches the one-shot reported flag (aps.rs:60-65).

The comment just above says the shape check means "an unrelated sender cannot consume this frame's single report." That holds only against unshaped noise. An adversary who has read these two lines sends {nonce:'x', renderer:{}}, which passes keys(message,['nonce','renderer']), fails the source check, and burns the report permanently. reported is a single var with no reset, so the later, genuine amazon_script_error at aps.rs:122 returns without posting anything.

The more serious half is on the consumer side. The Universal Creative handler treats a renderer-failed message as terminal: fail() sets done=true and calls f.remove() (render.ts:772). So a forged pre-handshake message does not merely burn a diagnostic, it removes the renderer frame of a render that was going to succeed. The direct path is accidentally shielded from this today only by the hasExactKeys mismatch in the finding above, which means fixing that finding without fixing this one would extend the exposure to both paths.

On reachability: the frame is never named and cross-origin siblings cannot enumerate it, so this is not reachable from an arbitrary third party. It is reachable from anything running in the same document as the frame's parent, which on the Universal Creative path is Amazon/GAM-served creative code. That is a real population for an ad-serving surface, even if it is not the open internet.

Proposed fix (apply manually - restoring the guard's position and dropping the report is a two-part edit across the reordered block):

function receive(event){
 if(event.source!==parent)return;
 var message=event.data;
 if(!keys(message,['nonce','renderer']))return;
 if(message.nonce!==expected){report('nonce_mismatch');return;}
 var problem=rendererProblem(message.renderer);
 if(problem){report(problem,message.nonce);return;}

A message from a non-parent source is by definition not attributable to the real render, so reporting it buys no diagnostic value and costs the one-shot budget. That also lets source_mismatch drop out of the reason list, the types.ts union, and the allowlist. If you would rather keep the reason for its own sake, the alternative is to scope reported per phase so a pre-acceptance rejection cannot consume the post-acceptance budget, but not reporting at all is both simpler and strictly safer here.

if(message.nonce!==expected){report('nonce_mismatch');return;}
var problem=rendererProblem(message.renderer);
if(problem){report(problem,message.nonce);return;}
removeEventListener('message',receive);
var acceptedNonce=expected;
expected='';
Expand All @@ -107,7 +119,7 @@ function receive(event){
var script=document.createElement('script');
script.src='https://client.aps.amazon-adsystem.com/prebid-creative.js';
script.onload=function(){parent.postMessage({message:'trusted-server/aps/renderer-ready',nonce:acceptedNonce},'*');};
script.onerror=function(){parent.postMessage({message:'trusted-server/aps/renderer-failed',nonce:acceptedNonce},'*');};
script.onerror=function(){report('amazon_script_error',acceptedNonce);};
document.head.appendChild(script);
}
addEventListener('message',receive);
Expand Down Expand Up @@ -2613,4 +2625,41 @@ mod tests {
assert!(APS_RENDERER_CSP.contains("sandbox allow-forms"));
assert!(!APS_RENDERER_CSP.contains("allow-same-origin"));
}

#[test]

@aram356 aram356 Sep 4, 2026

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.

nitpick - This test is close to a tautology and would not have caught either regression in this PR.

It searches a const &str defined in the same file for substrings the test author also wrote, which limits what it can detect.

Concretely: descriptor_fields appears at six sites in the document and descriptor_envelope at seven. Reverting any five of the six descriptor_fields returns back to return false leaves the string present and the test green. Neutering the body of report entirely to return; also leaves every listed substring intact and the test green. What it does catch is the outright deletion of one of the five singly-occurring reasons, which is real but narrow.

Two assertions are also coupled to minified spelling rather than behaviour: contains("reason:reason") and contains("if(reported)return") both break if someone adds a space after a colon, with no behavioural change. That makes them a source of false failures rather than a safety net.

The negative assertions (!contains("JSON.stringify(renderer)"), !contains("event.source.postMessage")) have more value as guardrails against a specific future mistake, though var s=event.source;s.postMessage(...) would slip past.

None of this is a blocker, and I recognise the constraint: the renderer is a JS program embedded in a Rust string with no JS-side harness, so substring matching is nearly the only tool available in mod tests. Two options that would give real coverage, if it is worth the effort:

  • Assert the reason list against the TypeScript allowlist keys, ideally from a shared source of truth. That is the seam where finding 4 lives, and a test of that shape would have caught it.
  • Or move APS_RENDERER_DOCUMENT somewhere it can be evaluated under vitest with a fake parent, which is exactly the technique render.test.ts:830-874 already uses successfully for APS_UNIVERSAL_CREATIVE_RENDERER. That would let the guards be tested by behaviour instead of by spelling.

fn renderer_document_reports_a_reason_for_every_silent_guard() {
for reason in [
"bad_hash",
"source_mismatch",
"nonce_mismatch",
"descriptor_keys",
"descriptor_fields",
"descriptor_envelope",
"amazon_script_error",
] {
assert!(
APS_RENDERER_DOCUMENT.contains(reason),
"renderer document should report a `{reason}` reason instead of returning silently"
);
}

// Reasons travel on the existing failure message rather than a new channel.
assert!(
APS_RENDERER_DOCUMENT.contains("reason:reason"),
"should attach the reason to the failure message"
);

// A reason is a fixed category, never a copy of the rejected descriptor.
assert!(!APS_RENDERER_DOCUMENT.contains("JSON.stringify(renderer)"));
assert!(!APS_RENDERER_DOCUMENT.contains("reason:message"));

// Reporting is one-shot so a hostile sender cannot flood the parent.
assert!(
APS_RENDERER_DOCUMENT.contains("if(reported)return"),
"should report at most one reason per frame"
);

// A foreign sender is answered through the parent, never the sender.
assert!(!APS_RENDERER_DOCUMENT.contains("event.source.postMessage"));
}
}
29 changes: 27 additions & 2 deletions crates/trusted-server-js/lib/src/core/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,37 @@ export type GptDiagnosticsTrustedServerOpportunity =
| 'unrenderable_candidate'
| 'no_candidate';

/** A safe failure category observed while obtaining or posting creative markup. */
/**
* A safe failure category observed while obtaining or posting creative markup.
*
* The `aps_` members cover the APS Universal Creative render path, where a
* blank slot is otherwise indistinguishable from a filled one: Ad Manager
* reports a non-empty 1x1 render whether or not the creative ever drew. Each
* member names the exact guard that stopped the render.
*/
export type GptDiagnosticsCreativeFailure =
| 'missing_render_source'
| 'cache_fetch_failed'
| 'invalid_cache_payload'
| 'response_post_failed';
| 'response_post_failed'
// Reported by the sandboxed renderer document and relayed by the creative.

@aram356 aram356 Sep 4, 2026

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.

wrench - The store rejects all 15 new aps_* reasons, so nothing this PR adds is ever recorded.

This union is the compile-time contract, but the runtime contract is isCreativeFailure in crates/trusted-server-js/lib/src/integrations/gpt_diagnostics/store.ts:168-175, which still lists only the original four:

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 gates on it at store.ts:540 and returns immediately for anything unlisted. Widening the union did not widen the guard.

I confirmed this against a real GptDiagnosticsStore (not a mock): with a live attempt, before any response is recorded,

LIVE attempt, aps_ reason    -> undefined            (dropped)
LIVE attempt, legacy reason  -> ["response_post_failed"]

So every reason this PR introduces is silently discarded: the relayed frame reasons and the five handshake reasons (aps_consumed_tombstone, aps_source_not_in_ad_unit, aps_descriptor_fields, aps_tombstone_capacity, aps_missing_renderer_url) alike. The feature is inert end to end, and delivery stays unknown on exactly the cycles the PR set out to attribute.

The reason the suite does not catch this is that ad_init.test.ts stubs gptDiagnosticsRecorder with vi.fn()s, so the assertions confirm the bridge called the recorder, never that the store accepted the value.

Proposed fix (apply manually - store.ts is not part of this diff, so it cannot be a suggestion):

const CREATIVE_FAILURES: ReadonlySet<GptDiagnosticsCreativeFailure> = new Set([
  'missing_render_source',
  'cache_fetch_failed',
  'invalid_cache_payload',
  'response_post_failed',
  'aps_bad_hash',
  'aps_nonce_mismatch',
  'aps_source_mismatch',
  'aps_descriptor_keys',
  'aps_descriptor_fields',
  'aps_descriptor_envelope',
  'aps_runner_script_error',
  'aps_frame_timeout',
  'aps_frame_load_error',
  'aps_frame_reported_failure',
  'aps_unknown',
  'aps_consumed_tombstone',
  'aps_source_not_in_ad_unit',
  'aps_missing_renderer_url',
  'aps_tombstone_capacity',
]);

function isCreativeFailure(reason: unknown): reason is GptDiagnosticsCreativeFailure {
  return (
    typeof reason === 'string' &&
    CREATIVE_FAILURES.has(reason as GptDiagnosticsCreativeFailure)
  );
}

Worth pairing that with a type-level exhaustiveness assertion, so a future widening of this union cannot again leave the guard behind silently. Something like a const _exhaustive: Record<GptDiagnosticsCreativeFailure, true> keyed off the same list would make the divergence a compile error rather than a runtime no-op. There is currently no test tying the two together, which is what let them drift.

| 'aps_bad_hash'

@ChristianPavilonis ChristianPavilonis Aug 27, 2026

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.

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

@prk-Jr prk-Jr Aug 28, 2026

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.

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

| 'aps_nonce_mismatch'
| 'aps_source_mismatch'
| 'aps_descriptor_keys'
| 'aps_descriptor_fields'
| 'aps_descriptor_envelope'
| 'aps_runner_script_error'
// Observed by the Universal Creative source around its renderer frame.
| 'aps_frame_timeout'
| 'aps_frame_load_error'
| 'aps_frame_reported_failure'
| 'aps_unknown'
// Observed on the Trusted Server side of the capability handshake.
| 'aps_consumed_tombstone'
| 'aps_source_not_in_ad_unit'
| 'aps_missing_renderer_url'
| 'aps_tombstone_capacity';

@prk-Jr prk-Jr Aug 28, 2026

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.

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


/** Delivery evidence derived for a GPT request cycle. */
export type GptDiagnosticsDelivery =
Expand Down
70 changes: 64 additions & 6 deletions crates/trusted-server-js/lib/src/integrations/aps/render.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { log } from '../../core/log';
import { findSlot } from '../../core/render';
import type { ApsPrebidRendererEntry, ApsRendererV1, TsjsApi } from '../../core/types';
import type {
ApsPrebidRendererEntry,
ApsRendererV1,
GptDiagnosticsCreativeFailure,
TsjsApi,
} from '../../core/types';

export const APS_RENDERER_PATH = '/integrations/aps/renderer';
export const APS_RENDERING_MODE_ATTRIBUTE_NAME = 'data-ts-aps-rendering-mode';
Expand Down Expand Up @@ -32,6 +37,58 @@ const activeFrames = new WeakMap<HTMLElement, HTMLIFrameElement>();
const pendingFrameCancels = new WeakMap<HTMLElement, () => void>();
const RENDERER_READY_MESSAGE = 'trusted-server/aps/renderer-ready';
const RENDERER_FAILED_MESSAGE = 'trusted-server/aps/renderer-failed';
/**
* Message the Universal Creative frame relays to the top window when an APS
* render never completes.
*
* The creative frame is cross-origin, so the top-window listener treats every
* field as untrusted and validates the reason against
* [`APS_RENDER_FAILURE_REASONS`] before recording it. The relay is
* diagnostics-only and never influences creative delivery.
*/
export const APS_RENDER_FAILED_MESSAGE = 'trusted-server/aps/render-failed';

/**
* Wire reasons the render path can emit, mapped onto safe diagnostic categories.
*
* Built on a null prototype so a hostile `__proto__`, `constructor`, or
* `toString` relayed by the cross-origin creative frame resolves to `undefined`
* rather than an inherited member.
*/
const APS_RENDER_FAILURE_REASONS: Readonly<Record<string, GptDiagnosticsCreativeFailure>> =

@prk-Jr prk-Jr Aug 28, 2026

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.

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

Object.freeze(
Object.assign(
Object.create(null) as Record<string, GptDiagnosticsCreativeFailure>,
{
bad_hash: 'aps_bad_hash',
nonce_mismatch: 'aps_nonce_mismatch',
source_mismatch: 'aps_source_mismatch',
descriptor_keys: 'aps_descriptor_keys',
descriptor_fields: 'aps_descriptor_fields',
descriptor_envelope: 'aps_descriptor_envelope',
amazon_script_error: 'aps_runner_script_error',
frame_timeout: 'aps_frame_timeout',
frame_load_error: 'aps_frame_load_error',
frame_reported_failure: 'aps_frame_reported_failure',
unknown: 'aps_unknown',
} as const
)
);

/**
* Resolve a relayed render failure reason to a safe diagnostic category.
*
* Returns `undefined` for anything not on the allowlist, so an unrecognized or
* hostile value from the cross-origin creative frame is dropped instead of
* being recorded.
*
* @example
* apsRenderFailureReason('frame_timeout'); // 'aps_frame_timeout'
* apsRenderFailureReason('__proto__'); // undefined
*/
export function apsRenderFailureReason(value: unknown): GptDiagnosticsCreativeFailure | undefined {
return typeof value === 'string' ? APS_RENDER_FAILURE_REASONS[value] : undefined;
}
const RENDERER_READY_TIMEOUT_MS = 10_000;
const MAX_PREBID_RENDERER_ENTRIES = 256;
const DEFAULT_PREBID_RENDERER_TTL_SECONDS = 300;
Expand Down Expand Up @@ -711,12 +768,13 @@ var b=new Uint8Array(16);c.getRandomValues(b);var s="";for(var i=0;i<b.length;i+
var n=w.btoa(s).replace(/\+/g,"-").replace(/\//g,"_").replace(/=+$/g,"");
var f=w.document.createElement("iframe"),done=false,t;
function clean(){w.removeEventListener("message",receive);if(t)w.clearTimeout(t);}
function fail(){if(done)return;done=true;clean();f.remove();reject(new Error("APS renderer frame failed"));}
function receive(e){var m=e.data;if(e.source!==f.contentWindow||!m||m.nonce!==n)return;
if(m.message==="${RENDERER_READY_MESSAGE}"){done=true;clean();resolve();}
else if(m.message==="${RENDERER_FAILED_MESSAGE}")fail();}
function report(x){try{(w.top||w).postMessage({message:"${APS_RENDER_FAILED_MESSAGE}",adId:(d&&typeof d.adId==="string")?d.adId:"",reason:x},"*");}catch(_e){}}

@aram356 aram356 Sep 4, 2026

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.

wrench - The renderer document's new 3-key failure message is silently ignored by the direct render path, which is a behavioural regression, not a diagnostics gap.

report() in aps.rs:61-65 now posts three keys:

parent.postMessage({message:'trusted-server/aps/renderer-failed',nonce:nonce,reason:reason},'*');

There are two consumers of that message and only one was updated. The Universal Creative source on the line below matches on m.message with no key-count check, so it is fine. The direct/native consumer in renderApsCreative is not (render.ts:725):

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

hasExactKeys (render.ts:194-205) requires the key set to match exactly, so a third key fails it and the handler returns before ever reaching the RENDERER_FAILED_MESSAGE branch. This path is live: core/request.ts:59 wires renderApsCreative as the trustedServer branch of dispatchApsRendering.

I confirmed it by driving renderApsCreative end to end and posting both shapes from the frame:

AFTER 3-KEY (new) frame still connected: true     <- failure ignored
AFTER 2-KEY (old) frame still connected: false    <- pre-PR behaviour

So on this path an amazon_script_error no longer tears the frame down. Before this PR, script.onerror posted a 2-key message and the frame was cancelled immediately; now the slot sits through the full 10s RENDERER_READY_TIMEOUT_MS before cleanup. The same applies to nonce_mismatch, bad_hash, and the descriptor_* reasons, which are newly emitted here but all dropped.

The new Rust test cannot catch this: it asserts on substrings of APS_RENDERER_DOCUMENT and never touches the TypeScript consumer, and the message shape is precisely the seam between them.

Proposed fix (apply manually - render.ts:725 is outside this diff's hunks, so it cannot be expressed as a suggestion):

function receive(event: MessageEvent): void {
  if (
    event.source !== iframe.contentWindow ||
    !(
      hasExactKeys(event.data, ['message', 'nonce']) ||
      hasExactKeys(event.data, ['message', 'nonce', 'reason'])
    )
  ) {
    return;
  }
  if (event.data.nonce !== nonce) return;
  if (event.data.message === RENDERER_READY_MESSAGE) commit();
  else if (event.data.message === RENDERER_FAILED_MESSAGE) fail();
}

A regression test that posts the 3-key shape and asserts the frame is torn down would keep the two consumers honest with each other.

function fail(x){if(done)return;done=true;clean();f.remove();report(x||"unknown");reject(new Error("APS renderer frame failed"));}
function receive(e){var m=e.data;if(e.source!==f.contentWindow||!m)return;
if(m.message==="${RENDERER_READY_MESSAGE}"&&m.nonce===n){done=true;clean();resolve();}
else if(m.message==="${RENDERER_FAILED_MESSAGE}"&&(m.nonce===n||m.nonce===undefined))fail(typeof m.reason==="string"?m.reason:"frame_reported_failure");}

@aram356 aram356 Sep 4, 2026

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.

thinking - Accepting m.nonce===undefined widens what can cancel a pending render.

main required m.nonce===n on both branches. This line keeps the strict check for renderer-ready but relaxes it for renderer-failed, accepting a message with no nonce at all.

The reason is clear enough from the other side: report('bad_hash') and report('nonce_mismatch') deliberately post without a nonce, correctly, since echoing an attacker-supplied nonce back would be worse. So the relaxation is what makes those two reasons observable, and the intent is sound.

The cost is that a nonce is no longer required to trigger fail(), and fail() is terminal: it removes the frame and rejects the render. The e.source!==f.contentWindow check above still constrains this to the renderer frame itself, so the exposure is narrow and largely overlaps the source_mismatch finding in aps.rs rather than adding much on top of it.

Mostly worth a comment on this line explaining why the unnonced case is accepted, so the asymmetry between the two branches does not read as an oversight to the next reader. If the source_mismatch report is dropped per the finding above, it would also be worth re-checking whether the unnonced case still needs to be terminal, or whether those two reasons could be relayed for diagnostics without cancelling the render.

f.width=String(r.width);f.height=String(r.height);f.style.border="0";
f.setAttribute("sandbox","${APS_RENDERER_SANDBOX}");
f.src=p.href+"#tsaps="+n;f.onload=function(){if(!done&&f.contentWindow)f.contentWindow.postMessage({nonce:n,renderer:r},"*");};
f.onerror=fail;w.addEventListener("message",receive);t=w.setTimeout(fail,${RENDERER_READY_TIMEOUT_MS});w.document.body.appendChild(f);
f.onerror=function(){fail("frame_load_error");};w.addEventListener("message",receive);t=w.setTimeout(function(){fail("frame_timeout");},${RENDERER_READY_TIMEOUT_MS});w.document.body.appendChild(f);
}catch(e){reject(e);}});};})();`;
Loading
Loading