Merge branch 'main' into hardening/constant-time-comparisons

Author: prk-Jr

crates/js/lib/package-lock.json

@@ -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 <form> 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;
 

crates/js/lib/src/core/render.ts

@@ -269,6 +269,13 @@ export function installPrebidNpm(config?: Partial<PrebidNpmConfig>): 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

crates/js/lib/src/integrations/prebid/index.ts

@@ -27,12 +27,12 @@ describe('render', () => {
     expect(iframe.srcdoc).toContain('<span>ad</span>');
     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 () => {

crates/js/lib/test/core/render.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,13 +629,18 @@ 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
     expect(tsBid.params.bidderParams).toEqual({
       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({});

crates/js/lib/test/integrations/prebid/index.test.ts

@@ -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.
-                // - <script>, <iframe>, <object>, <embed>: direct execution vectors.
+                // - <script>, <object>, <embed>: direct execution vectors.
                 // - <base>: rewrites all relative URLs, undermining the proxy rewriter.
                 // - <meta>: can trigger redirects (http-equiv=refresh) or inject CSP.
-                // - <form>: action/formaction can exfiltrate data; stripped to match the
-                //   iframe sandbox which omits allow-forms.
+                // - <form>: action/formaction can exfiltrate data.
                 // - <link>: external stylesheet/resource loading.
                 // - <style>: CSS expressions, @import, and url() data exfiltration.
                 // - <noscript>: rendered when scripts are disabled (always the case
                 //   inside a sandbox without allow-scripts); strip to prevent parser
                 //   differential attacks.
                 element!(
-                    "script, iframe, object, embed, base, meta, form, link, style, noscript",
+                    "script, object, embed, base, meta, form, link, style, noscript",
                     |el| {
                         el.remove();
                         Ok(())
@@ -454,10 +453,9 @@ pub fn sanitize_creative_html(markup: &str) -> String {
                     // NOTE: This uses simple substring matching on the lowercased value,
                     // which does not handle CSS escape sequences (e.g. `\65xpression(`)
                     // or comments (e.g. `expr/**/ession(`). That is acceptable: CSS
-                    // expression() is IE6-8 only and the iframe sandbox's absence of
-                    // allow-scripts prevents execution even if an obfuscated value were
-                    // to slip through. The sandbox is the primary mitigation; this check
-                    // is defense-in-depth for obvious patterns.
+                    // expression() is IE6-8 only, and downstream rendering still happens
+                    // inside a sandboxed iframe. This check is defense-in-depth for
+                    // obvious patterns.
                     if let Some(style) = el.get_attribute("style") {
                         let lower = style.to_ascii_lowercase();
                         if lower.contains("expression(")
@@ -1504,10 +1502,14 @@ mod tests {
     }
 
     #[test]
-    fn sanitize_removes_iframe_element() {
+    fn sanitize_preserves_iframe_element_and_src() {
         let html = r#"<div>ad</div><iframe src="https://evil.example/"></iframe>"#;
         let out = sanitize_creative_html(html);
-        assert!(!out.contains("<iframe"), "should remove iframe element");
+        assert!(out.contains("<iframe"), "should preserve iframe element");
+        assert!(
+            out.contains("https://evil.example/"),
+            "should preserve safe iframe src"
+        );
         assert!(out.contains("ad"), "should preserve safe content");
     }