From c9c47bc38617d0cc4dab27eccbac1471428f740a Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 23 Mar 2026 15:47:09 -0500 Subject: [PATCH] Fix Prebid bidder routing and restore iframe creative rendering --- crates/js/lib/src/core/render.ts | 13 ++-- .../js/lib/src/integrations/prebid/index.ts | 7 ++ crates/js/lib/test/core/render.test.ts | 6 +- .../test/integrations/prebid/index.test.ts | 67 ++++++++++++------- crates/trusted-server-core/src/creative.rs | 24 ++++--- 5 files changed, 73 insertions(+), 44 deletions(-) diff --git a/crates/js/lib/src/core/render.ts b/crates/js/lib/src/core/render.ts index da223851..ee08ef28 100644 --- a/crates/js/lib/src/core/render.ts +++ b/crates/js/lib/src/core/render.ts @@ -7,15 +7,16 @@ import NORMALIZE_CSS from './styles/normalize.css?inline'; import IFRAME_TEMPLATE from './templates/iframe.html?raw'; // Sandbox permissions granted to creative iframes. -// Notably absent: -// allow-scripts, allow-same-origin — prevent JS execution and same-origin -// access, which are the primary attack vectors for malicious creatives. -// allow-forms — server-side sanitization strips
elements, so form -// submission from creatives is not a supported use case. Omitting this token -// is consistent with that server-side policy and reduces the attack surface. +// Ad creatives routinely contain scripts for tracking, click handling, and +// viewability measurement, so allow-scripts and allow-same-origin are required +// for creatives to render correctly. Server-side sanitization is the primary +// defense against malicious markup; the sandbox provides defense-in-depth. const CREATIVE_SANDBOX_TOKENS = [ + 'allow-forms', 'allow-popups', 'allow-popups-to-escape-sandbox', + 'allow-same-origin', + 'allow-scripts', 'allow-top-navigation-by-user-activation', ] as const; diff --git a/crates/js/lib/src/integrations/prebid/index.ts b/crates/js/lib/src/integrations/prebid/index.ts index 63282513..f2a815b7 100644 --- a/crates/js/lib/src/integrations/prebid/index.ts +++ b/crates/js/lib/src/integrations/prebid/index.ts @@ -269,6 +269,13 @@ export function installPrebidNpm(config?: Partial): typeof pbjs } else { unit.bids.push({ bidder: ADAPTER_CODE, params: tsParams }); } + + // Remove server-side bidder entries — they are now handled via the + // trustedServer adapter. Only keep client-side bidders (which run via + // their native Prebid.js adapters) and the trustedServer bid itself. + unit.bids = unit.bids.filter( + (b) => b.bidder === ADAPTER_CODE || clientSideBidders.has(b.bidder ?? '') + ); } // Ensure the trustedServer adapter is allowed to return bids under any diff --git a/crates/js/lib/test/core/render.test.ts b/crates/js/lib/test/core/render.test.ts index 5bdb3a81..a81486cf 100644 --- a/crates/js/lib/test/core/render.test.ts +++ b/crates/js/lib/test/core/render.test.ts @@ -27,12 +27,12 @@ describe('render', () => { expect(iframe.srcdoc).toContain('ad'); expect(div.querySelector('iframe')).toBe(iframe); const sandbox = iframe.getAttribute('sandbox') ?? ''; - expect(sandbox).not.toContain('allow-forms'); + expect(sandbox).toContain('allow-forms'); expect(sandbox).toContain('allow-popups'); expect(sandbox).toContain('allow-popups-to-escape-sandbox'); expect(sandbox).toContain('allow-top-navigation-by-user-activation'); - expect(sandbox).not.toContain('allow-same-origin'); - expect(sandbox).not.toContain('allow-scripts'); + expect(sandbox).toContain('allow-same-origin'); + expect(sandbox).toContain('allow-scripts'); }); it('preserves dollar sequences when building the creative document', async () => { diff --git a/crates/js/lib/test/integrations/prebid/index.test.ts b/crates/js/lib/test/integrations/prebid/index.test.ts index 961b7abc..a68150cb 100644 --- a/crates/js/lib/test/integrations/prebid/index.test.ts +++ b/crates/js/lib/test/integrations/prebid/index.test.ts @@ -411,14 +411,13 @@ describe('prebid/installPrebidNpm', () => { ]; pbjs.requestBids({ adUnits } as any); - // Each ad unit should have trustedServer added + // Each ad unit should only have trustedServer — original bidders are absorbed for (const unit of adUnits) { - const hasTsBidder = unit.bids.some((b: any) => b.bidder === 'trustedServer'); - expect(hasTsBidder).toBe(true); + expect(unit.bids).toHaveLength(1); + expect(unit.bids[0].bidder).toBe('trustedServer'); } - const trustedServerBid = adUnits[0].bids.find((b: any) => b.bidder === 'trustedServer'); - expect(trustedServerBid.params.bidderParams).toEqual({ appnexus: {} }); + expect(adUnits[0].bids[0].params.bidderParams).toEqual({ appnexus: {} }); // Should call through to original requestBids expect(mockRequestBids).toHaveBeenCalled(); @@ -434,7 +433,7 @@ describe('prebid/installPrebidNpm', () => { expect(tsCount).toBe(1); }); - it('captures per-bidder params on trustedServer bid', () => { + it('captures per-bidder params on trustedServer bid and removes originals', () => { const pbjs = installPrebidNpm(); const adUnits = [ @@ -447,8 +446,10 @@ describe('prebid/installPrebidNpm', () => { ]; pbjs.requestBids({ adUnits } as any); - const trustedServerBid = adUnits[0].bids.find((b: any) => b.bidder === 'trustedServer'); - expect(trustedServerBid).toBeDefined(); + // Only trustedServer should remain — original bidders are absorbed + expect(adUnits[0].bids).toHaveLength(1); + const trustedServerBid = adUnits[0].bids[0] as any; + expect(trustedServerBid.bidder).toBe('trustedServer'); expect(trustedServerBid.params.bidderParams).toEqual({ appnexus: { placementId: 123 }, rubicon: { accountId: 'abc' }, @@ -482,11 +483,12 @@ describe('prebid/installPrebidNpm', () => { ]; pbjs.requestBids({ adUnits } as any); - const tsBid0 = adUnits[0].bids.find((b: any) => b.bidder === 'trustedServer') as any; - expect(tsBid0.params.zone).toBe('header'); + // Original kargo bids should be removed, only trustedServer remains + expect(adUnits[0].bids).toHaveLength(1); + expect(adUnits[0].bids[0].params.zone).toBe('header'); - const tsBid1 = adUnits[1].bids.find((b: any) => b.bidder === 'trustedServer') as any; - expect(tsBid1.params.zone).toBe('fixed_bottom'); + expect(adUnits[1].bids).toHaveLength(1); + expect(adUnits[1].bids[0].params.zone).toBe('fixed_bottom'); }); it('omits zone when mediaTypes.banner.name is not set', () => { @@ -501,8 +503,8 @@ describe('prebid/installPrebidNpm', () => { ]; pbjs.requestBids({ adUnits } as any); - const tsBid = adUnits[0].bids.find((b: any) => b.bidder === 'trustedServer') as any; - expect(tsBid.params.zone).toBeUndefined(); + expect(adUnits[0].bids).toHaveLength(1); + expect(adUnits[0].bids[0].params.zone).toBeUndefined(); }); it('omits zone when ad unit has no mediaTypes', () => { @@ -511,8 +513,8 @@ describe('prebid/installPrebidNpm', () => { const adUnits = [{ bids: [{ bidder: 'rubicon', params: {} }] }]; pbjs.requestBids({ adUnits } as any); - const tsBid = adUnits[0].bids.find((b: any) => b.bidder === 'trustedServer') as any; - expect(tsBid.params.zone).toBeUndefined(); + expect(adUnits[0].bids).toHaveLength(1); + expect(adUnits[0].bids[0].params.zone).toBeUndefined(); }); it('clears stale zone when existing trustedServer bid is reused', () => { @@ -549,10 +551,10 @@ describe('prebid/installPrebidNpm', () => { mockPbjs.adUnits = [{ bids: [{ bidder: 'openx', params: {} }] }] as any[]; pbjs.requestBids({} as any); - const hasTsBidder = (mockPbjs.adUnits[0] as any).bids.some( - (b: any) => b.bidder === 'trustedServer' - ); - expect(hasTsBidder).toBe(true); + // Original openx bid should be removed, only trustedServer remains + const unit = mockPbjs.adUnits[0] as any; + expect(unit.bids).toHaveLength(1); + expect(unit.bids[0].bidder).toBe('trustedServer'); }); }); }); @@ -611,7 +613,7 @@ describe('prebid/client-side bidders', () => { delete (window as any).__tsjs_prebid; }); - it('excludes client-side bidders from trustedServer bidderParams', () => { + it('excludes client-side bidders from trustedServer bidderParams and removes server-side bids', () => { (window as any).__tsjs_prebid = { clientSideBidders: ['rubicon'] }; const pbjs = installPrebidNpm(); @@ -627,6 +629,8 @@ describe('prebid/client-side bidders', () => { ]; pbjs.requestBids({ adUnits } as any); + // Only rubicon (client-side) and trustedServer should remain + expect(adUnits[0].bids).toHaveLength(2); const tsBid = adUnits[0].bids.find((b: any) => b.bidder === 'trustedServer') as any; expect(tsBid).toBeDefined(); // rubicon should NOT be in bidderParams — it runs client-side @@ -634,6 +638,9 @@ describe('prebid/client-side bidders', () => { appnexus: { placementId: 123 }, kargo: { placementId: 'k1' }, }); + // appnexus and kargo should be removed (absorbed into trustedServer) + expect(adUnits[0].bids.find((b: any) => b.bidder === 'appnexus')).toBeUndefined(); + expect(adUnits[0].bids.find((b: any) => b.bidder === 'kargo')).toBeUndefined(); }); it('preserves client-side bidder bids as standalone entries', () => { @@ -673,6 +680,8 @@ describe('prebid/client-side bidders', () => { ]; pbjs.requestBids({ adUnits } as any); + // 3 bids: rubicon (client-side), openx (client-side), trustedServer + expect(adUnits[0].bids).toHaveLength(3); const tsBid = adUnits[0].bids.find((b: any) => b.bidder === 'trustedServer') as any; // Only appnexus should be in bidderParams expect(tsBid.params.bidderParams).toEqual({ @@ -682,6 +691,9 @@ describe('prebid/client-side bidders', () => { // Both client-side bidders should remain expect(adUnits[0].bids.find((b: any) => b.bidder === 'rubicon')).toBeDefined(); expect(adUnits[0].bids.find((b: any) => b.bidder === 'openx')).toBeDefined(); + + // Server-side bidder should be removed + expect(adUnits[0].bids.find((b: any) => b.bidder === 'appnexus')).toBeUndefined(); }); it('behaves normally when no client-side bidders are configured', () => { @@ -698,7 +710,10 @@ describe('prebid/client-side bidders', () => { ]; pbjs.requestBids({ adUnits } as any); - const tsBid = adUnits[0].bids.find((b: any) => b.bidder === 'trustedServer') as any; + // All original bidders should be removed, only trustedServer remains + expect(adUnits[0].bids).toHaveLength(1); + const tsBid = adUnits[0].bids[0] as any; + expect(tsBid.bidder).toBe('trustedServer'); expect(tsBid.params.bidderParams).toEqual({ appnexus: { placementId: 123 }, rubicon: { accountId: 'abc' }, @@ -720,7 +735,10 @@ describe('prebid/client-side bidders', () => { ]; pbjs.requestBids({ adUnits } as any); - const tsBid = adUnits[0].bids.find((b: any) => b.bidder === 'trustedServer') as any; + // All original bidders should be removed, only trustedServer remains + expect(adUnits[0].bids).toHaveLength(1); + const tsBid = adUnits[0].bids[0] as any; + expect(tsBid.bidder).toBe('trustedServer'); expect(tsBid.params.bidderParams).toEqual({ appnexus: { placementId: 123 }, rubicon: { accountId: 'abc' }, @@ -742,7 +760,8 @@ describe('prebid/client-side bidders', () => { ]; pbjs.requestBids({ adUnits } as any); - // trustedServer should still be present (even with empty bidderParams) + // All 3 should be present: rubicon, appnexus (both client-side), and trustedServer + expect(adUnits[0].bids).toHaveLength(3); const tsBid = adUnits[0].bids.find((b: any) => b.bidder === 'trustedServer') as any; expect(tsBid).toBeDefined(); expect(tsBid.params.bidderParams).toEqual({}); diff --git a/crates/trusted-server-core/src/creative.rs b/crates/trusted-server-core/src/creative.rs index 45162d8c..245af0e1 100644 --- a/crates/trusted-server-core/src/creative.rs +++ b/crates/trusted-server-core/src/creative.rs @@ -329,7 +329,7 @@ fn is_safe_data_uri(lower: &str) -> bool { /// Strip dangerous elements and attributes from ad creative HTML. /// -/// Removes elements that can execute code or exfiltrate data (`script`, `iframe`, +/// Removes elements that can execute code or exfiltrate data (`script`, /// `object`, `embed`, `base`, `meta`, `form`, `link`, `style`, `noscript`) and strips `on*` event-handler /// attributes and dangerous URI schemes from all remaining elements: /// - `javascript:`, `vbscript:` @@ -361,18 +361,17 @@ pub fn sanitize_creative_html(markup: &str) -> String { HtmlSettings { element_content_handlers: vec![ // Remove executable/dangerous elements along with their inner content. - // -