Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
125 changes: 52 additions & 73 deletions crates/trusted-server-core/src/integrations/aps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use crate::settings::{IntegrationConfig, Settings};

const APS_INTEGRATION_ID: &str = "aps";
const APS_RENDERER_ROUTE: &str = "/integrations/aps/renderer";
const APS_RENDERER_BOOTSTRAP_QUERY: &str = "mode=data-bootstrap";
const DEFAULT_CURRENCY: &str = "USD";
const APS_SDK_SOURCE: &str = "prebid";
const APS_SDK_VERSION: &str = "2.2.0";
Expand All @@ -47,72 +48,12 @@ const MAX_LANGUAGE_BYTES: usize = 8;
const MAX_PAGE_URL_BYTES: usize = 8192;
const MAX_RENDER_ENVELOPE_BYTES: usize = 256 * 1024;
const APS_RENDERER_CSP: &str = "default-src 'none'; sandbox allow-forms allow-pointer-lock allow-popups allow-popups-to-escape-sandbox allow-scripts allow-top-navigation-by-user-activation; script-src 'unsafe-inline' https:; connect-src https:; frame-src https:; img-src https: data:; media-src https: blob:; style-src 'unsafe-inline' https:; font-src https: data:;";
const APS_RENDERER_BOOTSTRAP_CSP: &str = "default-src 'none'; sandbox allow-forms allow-pointer-lock allow-popups allow-popups-to-escape-sandbox allow-same-origin allow-scripts allow-top-navigation-by-user-activation; script-src 'unsafe-inline' https:; connect-src https:; frame-src https: data:; img-src https: data:; media-src https: blob:; style-src 'unsafe-inline' https:; font-src https: data:;";

const APS_RENDERER_DOCUMENT: &str = r#"<!doctype html>
<meta charset="utf-8">
<script>
(function(){
'use strict';
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;
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){
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;
try{
var url=new URL(renderer.creativeUrl);
if(url.protocol!=='https:'||url.username||url.password)return false;
var binary=atob(renderer.aaxResponse);
if(binary.length>262144||btoa(binary)!==renderer.aaxResponse)return false;
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;
var seat=decoded.seatbid[0];
if(!keys(seat,['bid'])||!Array.isArray(seat.bid)||seat.bid.length!==1)return false;
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&&
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;}
}
function receive(event){
if(event.source!==parent)return;
var message=event.data;
if(!keys(message,['nonce','renderer'])||message.nonce!==expected||!validRenderer(message.renderer))return;
removeEventListener('message',receive);
var acceptedNonce=expected;
expected='';
var renderer=message.renderer;
window._aps=window._aps instanceof Map?window._aps:new Map();
var account=window._aps.get(renderer.accountId);
if(!account){
account={queue:[],store:new Map([['listeners',new Map()]])};
window._aps.set(renderer.accountId,account);
}
account.queue.push(new CustomEvent('prebid/creative/render',{detail:{aaxResponse:renderer.aaxResponse,seatBidId:renderer.bidId}}));
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},'*');};
document.head.appendChild(script);
}
addEventListener('message',receive);
})();
</script>
"#;
const APS_RENDERER_DOCUMENT: &str =
include_str!("../../../trusted-server-js/lib/src/integrations/aps/renderer.html");

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.

📝 note — Core now embeds a file from the JS crate's source tree (not dist/). This works and include_str! tracks it for rebuilds, so no change needed — but the cross-crate source coupling is easy to misread as a generated artifact. A one-line comment saying the HTML is hand-written source shared with the TS bundle would save the next reader a detour.

const APS_RENDERER_BOOTSTRAP_DOCUMENT: &str =
include_str!("../../../trusted-server-js/lib/src/integrations/aps/renderer-bootstrap.html");

/// Configuration for the APS `OpenRTB` integration.
#[derive(Debug, Clone, Deserialize, Serialize, Validate)]
Expand Down Expand Up @@ -1211,13 +1152,28 @@ impl IntegrationProxy for ApsRendererIntegration {
message: "Failed to build APS not-found response".to_string(),
});
}
let (renderer_document, renderer_csp) = match request.uri().query() {

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 — Two different documents, with two different CSPs, are now served from one path and distinguished only by the query string, with no Cache-Control and no Vary on either response.

Any cache that drops or normalizes the query serves the wrong document to the wrong client: the bootstrap URL would return the legacy renderer (which the fallback then cannot talk to), or the query-free URL would return the bootstrap (which no legacy client understands).

Fix: set an explicit Cache-Control on both responses and confirm the query string is part of the Fastly cache key.

None => (APS_RENDERER_DOCUMENT, APS_RENDERER_CSP),
Some(APS_RENDERER_BOOTSTRAP_QUERY) => {
(APS_RENDERER_BOOTSTRAP_DOCUMENT, APS_RENDERER_BOOTSTRAP_CSP)
}
Some(_) => {
return http::Response::builder()
.status(StatusCode::NOT_FOUND)
.body(EdgeBody::from("Not Found"))
.change_context(TrustedServerError::Integration {
integration: APS_INTEGRATION_ID.to_string(),
message: "Failed to build APS not-found response".to_string(),
});
}
};
http::Response::builder()
.status(StatusCode::OK)
.header(header::CONTENT_TYPE, "text/html; charset=utf-8")
.header("x-content-type-options", "nosniff")
.header("referrer-policy", "no-referrer")
.header(header::CONTENT_SECURITY_POLICY, APS_RENDERER_CSP)
.body(EdgeBody::from(APS_RENDERER_DOCUMENT))
.header(header::CONTENT_SECURITY_POLICY, renderer_csp)
.body(EdgeBody::from(renderer_document))
.change_context(TrustedServerError::Integration {
integration: APS_INTEGRATION_ID.to_string(),
message: "Failed to build APS renderer response".to_string(),
Expand Down Expand Up @@ -2318,7 +2274,7 @@ mod tests {
}

#[test]
fn registers_and_serves_only_static_renderer_route() {
fn registers_and_serves_static_renderer_and_data_bootstrap_modes() {
let integration = ApsRendererIntegration;
let routes = integration.routes();
assert_eq!(routes.len(), 1, "should register one route");
Expand Down Expand Up @@ -2347,6 +2303,25 @@ mod tests {
APS_RENDERER_CSP
);

let bootstrap = http::Request::builder()
.method(Method::GET)
.uri(format!(
"{APS_RENDERER_ROUTE}?{APS_RENDERER_BOOTSTRAP_QUERY}"
))
.body(EdgeBody::empty())
.expect("should build renderer bootstrap request");
let response =
futures::executor::block_on(integration.handle(&settings, &services, bootstrap))
.expect("should serve renderer bootstrap");
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(
response.headers()[header::CONTENT_SECURITY_POLICY],
APS_RENDERER_BOOTSTRAP_CSP
);
assert!(APS_RENDERER_BOOTSTRAP_CSP.contains("allow-same-origin"));
assert!(APS_RENDERER_BOOTSTRAP_CSP.contains("frame-src https: data:"));
assert!(APS_RENDERER_BOOTSTRAP_DOCUMENT.contains("trusted-server/aps/bootstrap-navigate"));

let post = http::Request::builder()
.method(Method::POST)
.uri(APS_RENDERER_ROUTE)
Expand Down Expand Up @@ -2374,6 +2349,7 @@ mod tests {

assert_eq!(registration.integration_id, APS_INTEGRATION_ID);
assert_eq!(registration.proxies.len(), 1);
assert!(registration.request_filters.is_empty());
assert!(registration.js_disabled);
}

Expand Down Expand Up @@ -2425,12 +2401,15 @@ mod tests {
#[test]
fn renderer_document_is_static_and_nonce_bound() {
assert!(APS_RENDERER_DOCUMENT.contains("^#tsaps="));
assert!(APS_RENDERER_DOCUMENT.contains("event.source!==parent"));
assert!(APS_RENDERER_DOCUMENT.contains("message.nonce!==expected"));
assert!(APS_RENDERER_DOCUMENT.contains("event.source !== parent"));
assert!(APS_RENDERER_DOCUMENT.contains("message.nonce !== expected"));
assert!(APS_RENDERER_DOCUMENT.contains("['nonce', 'publisherOrigin', 'renderer']"));

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 asserts on Prettier's exact spacing and wrapping of a file Prettier owns. A printWidth or formatting-config change reflows the array and breaks a core Rust test for a purely cosmetic reason.

Prefer markers that survive reformatting — for example asserting on publisherOrigin and validPublisherOrigin separately, rather than on the rendered array literal.

assert!(APS_RENDERER_DOCUMENT.contains("['nonce', 'renderer']"));
assert!(APS_RENDERER_DOCUMENT.contains("url.origin === publisherOrigin"));
assert!(APS_RENDERER_DOCUMENT.contains("prebid/creative/render"));
assert!(APS_RENDERER_DOCUMENT.contains("window._aps instanceof Map"));
assert!(APS_RENDERER_DOCUMENT.contains("store:new Map([['listeners',new Map()]])"));
assert!(APS_RENDERER_DOCUMENT.contains("account.queue.push(new CustomEvent"));
assert!(APS_RENDERER_DOCUMENT.contains("store: new Map([['listeners', new Map()]])"));
assert!(APS_RENDERER_DOCUMENT.contains("account.queue.push("));
assert!(
APS_RENDERER_DOCUMENT.contains("trusted-server/aps/renderer-ready")
&& APS_RENDERER_DOCUMENT.contains("trusted-server/aps/renderer-failed")
Expand All @@ -2442,7 +2421,7 @@ mod tests {
);
assert!(!APS_RENDERER_DOCUMENT.contains("<script src="));
let queue_index = APS_RENDERER_DOCUMENT
.find("account.queue.push(new CustomEvent")
.find("account.queue.push(")
.expect("should queue render event");
let runner_index = APS_RENDERER_DOCUMENT
.find("document.head.appendChild(script)")
Expand Down
64 changes: 64 additions & 0 deletions crates/trusted-server-core/src/response_privacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,33 @@ pub const CDN_CACHE_HEADERS: &[&str] = &[
"cloudflare-cdn-cache-control",
];

const APS_INTEGRATION_ID: &str = "aps";
const APS_PUBLISHER_FRAME_ANCESTORS_CSP: &str = "frame-ancestors 'self';";

fn enforce_aps_publisher_frame_ancestors(settings: &Settings, response: &mut Response) {

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.

🌱 seedling — This is a security-sensitive gate, but the only new coverage (aps_enabled_appends_an_independent_publisher_frame_policy) exercises the enabled path. There is no negative test asserting the header is not appended when APS is absent or enabled: false, so a regression that always-appends or always-skips would still pass CI. Consider adding a disabled-case test asserting no frame-ancestors header is added.

let aps_enabled = settings

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.

📝 note — This reads the raw integrations.aps.enabled bool directly (== Some(true)), while the rest of the codebase decides APS activation through integration_config::<ApsConfig>() / IntegrationConfig::is_enabled. They align today (the provider's is_enabled() is also just config.enabled), so behavior is correct — but it's a second source of truth for "is APS on" that could drift if APS ever gains an alternate enable path. A short comment noting the intentional coupling, or reusing the typed accessor, would guard against that.

.integrations
.get(APS_INTEGRATION_ID)

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.

♻️ refactor — This re-declares APS_INTEGRATION_ID and re-derives enablement from raw JSON. Settings::integration_config::<ApsConfig> and IntegrationConfig::is_enabled already exist and are the source of truth used by registration.

Using them keeps the two definitions from drifting if APS enablement ever gains another condition — otherwise a future change to ApsConfig::is_enabled leaves this header check silently out of sync with whether APS is actually running.

.and_then(|value| value.get("enabled"))
.and_then(serde_json::Value::as_bool)
== Some(true);
if !aps_enabled {
return;
}

let already_present = response
.headers()
.get_all(header::CONTENT_SECURITY_POLICY)
.iter()
.any(|value| value.as_bytes() == APS_PUBLISHER_FRAME_ANCESTORS_CSP.as_bytes());
if !already_present {
response.headers_mut().append(
header::CONTENT_SECURITY_POLICY,
HeaderValue::from_static(APS_PUBLISHER_FRAME_ANCESTORS_CSP),
);
}
}

fn strip_cdn_cache_headers(response: &mut Response) {
for name in CDN_CACHE_HEADERS {
response.headers_mut().remove(*name);
Expand Down Expand Up @@ -134,6 +161,12 @@ pub fn apply_response_headers_with_cache_privacy(settings: &Settings, response:
// the pre-apply pass could not see. Re-run the downgrade so the final
// response can never pair Set-Cookie with shared cacheability.
enforce_set_cookie_cache_privacy(response);

// APS creatives retain their HTTPS origin. A creative can therefore frame
// a publisher URL beneath the opaque renderer unless every publisher
// response rejects the cross-origin ancestor chain. Append this independent
// policy after operator headers so configuration cannot weaken it.
enforce_aps_publisher_frame_ancestors(settings, response);

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 — This appends frame-ancestors 'self' to every Trusted Server response whenever integrations.aps.enabled = true, which silently changes the publisher's site-wide embedding policy: AMP caches, partner embeds, in-app webviews, and same-brand cross-origin framing all stop working the moment APS is turned on.

The containment argument for it is sound and the docs are honest about the trade-off, so this is not a request to change the behavior — but it deserves an explicit release note, since the blast radius reaches every page rather than the ad slot.

📌 out of scope — the docs promise "a separately reviewed ancestor-allowlist feature before enabling APS". Worth a tracked follow-up issue so that promise does not go stale.

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 — The frame-ancestors 'self' policy is appended to every response type (JSON, images, the tsjs bundle), not just framed HTML. It's harmless (frame-ancestors is inert on non-framed resources) and the byte-exact dedup keeps it idempotent, but it's slightly broader than the stated "publisher document" intent. No change required — flagging for awareness.

}

#[cfg(test)]
Expand Down Expand Up @@ -315,6 +348,37 @@ mod tests {
);
}

#[test]
fn aps_enabled_appends_an_independent_publisher_frame_policy() {
let mut settings =
settings_with_response_headers(&[("content-security-policy", "default-src 'self'")]);
settings
.integrations
.insert_config(
APS_INTEGRATION_ID,
&serde_json::json!({"enabled": true, "account_id": "example-account"}),
)
.expect("should enable APS");
let mut response = response_builder()
.header(header::CONTENT_SECURITY_POLICY, "script-src 'self'")
.body(edgezero_core::body::Body::empty())
.expect("should build response");

apply_response_headers_with_cache_privacy(&settings, &mut response);
apply_response_headers_with_cache_privacy(&settings, &mut response);

let policies = response
.headers()
.get_all(header::CONTENT_SECURITY_POLICY)
.iter()
.map(|value| value.to_str().expect("should contain valid CSP"))
.collect::<Vec<_>>();
assert_eq!(
policies,
vec!["default-src 'self'", APS_PUBLISHER_FRAME_ANCESTORS_CSP]
);
}

#[test]
fn uncacheable_response_rejects_operator_cdn_cache_headers() {
let settings = settings_with_response_headers(&[
Expand Down
Loading
Loading