feat(ads): let a video unit measure itself, and harden the field that made that risky

app/ad.js/route.ts

@@ -100,8 +100,24 @@ ${VISITOR_SNIPPET}
           iframe.setAttribute('scrolling', 'no');
           iframe.setAttribute('frameborder', '0');
           iframe.setAttribute('loading', 'lazy');
-          // Sandbox: allow the ad's click link to open a new tab, nothing else.
-          iframe.setAttribute('sandbox', 'allow-popups allow-popups-to-escape-sandbox allow-top-navigation-by-user-activation');
+          // Sandbox: allow the ad's click link to open a new tab, and nothing
+          // else — except on a fill that has something to measure.
+          //
+          // allow-scripts is granted ONLY to video and audio, the two media
+          // that can report playback, and for the same reason the autoplay
+          // permission below is: a tag that hands out a capability on every
+          // fill is handing out more than it uses. A static banner still runs
+          // nothing at all.
+          //
+          // allow-same-origin is NEVER granted, and the two together are why.
+          // A frame with both can reach its own frameElement and delete the
+          // sandbox attribute, which is not a sandbox at all. Without it this
+          // document has an opaque origin: it cannot read the publisher's DOM,
+          // cookies or storage, and the worst a compromised creative gets is
+          // its own inert box. Keep them apart.
+          var sandbox = 'allow-popups allow-popups-to-escape-sandbox allow-top-navigation-by-user-activation';
+          if (res.media === 'video' || res.media === 'audio') sandbox += ' allow-scripts';
+          iframe.setAttribute('sandbox', sandbox);
           // A muted <video> still needs the frame to hold the autoplay
           // permission, or Chrome refuses to start it and the unit is a poster
           // that never moves. Granted only on the fill that actually has a video

app/api/ads/serve/route.ts

@@ -5,6 +5,10 @@
 
 import { NextRequest, NextResponse } from "next/server";
 import { serveAd, isAdFormat } from "@/lib/ads/serve";
+import { serviceClient } from "@/lib/supabase/service";
+import { recordDecision } from "@/lib/ads/video/decisions";
+import { bannerBeaconScript, injectBannerBeacon } from "@/lib/ads/video/bannerBeacon";
+import { env } from "@/lib/env";
 import { clientIpFromHeaders, lookupGeo } from "@/lib/tracker/geo";
 import { parseDevice } from "@/lib/tracker/device";
 
@@ -64,11 +68,33 @@ export async function GET(request: NextRequest) {
 
     if (!fill) return NextResponse.json({ ok: false }, { status: 200, headers });
 
+    // Playback measurement, on the two media that have any. The tag grants this
+    // frame allow-scripts for exactly these, so the beacon has somewhere to run;
+    // every other medium gets the markup untouched and a script-free sandbox.
+    let html = fill.html;
+    if (fill.media === "video" || fill.media === "audio") {
+      const decisionId = await recordDecision(serviceClient(), {
+        slotId,
+        // A display unit is drawn once and a reload is a genuinely new
+        // impression, so the fill is its own session.
+        sessionId: crypto.randomUUID(),
+        placement: "in_banner",
+        kind: fill.media === "audio" ? "audio" : "video",
+        surface: "web",
+        fill,
+        assetRevision: null,
+      });
+      // Unmeasurable is not unservable: no decision, no script, same ad.
+      if (decisionId) {
+        html = injectBannerBeacon(html, bannerBeaconScript(decisionId, env.siteUrl));
+      }
+    }
+
     return NextResponse.json(
       {
         ok: true,
         impressionId: fill.impressionId,
-        html: fill.html,
+        html,
         clickUrl: fill.clickUrl,
         // What was actually served, which is not always what was asked for —
         // the tag sizes its iframe from this.

lib/ads/campaigns.ts

@@ -1,3 +1,4 @@
+import { safeFontFamily } from "./creative";
 // Campaigns created from outside the dashboard.
 //
 // The dashboard's saveCampaign (app/actions/ads.ts) takes creatives the person
@@ -70,7 +71,7 @@ function creativeRow(campaignId: string, ownerId: string, c: AdCreative) {
     light_bg_color: c.lightBgColor ?? null,
     light_fg_color: c.lightFgColor ?? null,
     light_accent_color: c.lightAccentColor ?? null,
-    font_family: (c.fontFamily ?? "system-ui, sans-serif").slice(0, 200),
+    font_family: safeFontFamily(c.fontFamily),
   };
 }
 

lib/ads/creative.ts

@@ -540,6 +540,37 @@ function esc(s: string): string {
     .replace(/"/g, """);
 }
 
+/** What a creative gets when its stored font is not something we will emit. */
+export const DEFAULT_FONT_STACK = "system-ui, sans-serif";
+
+/**
+ * A font stack, or the default.
+ *
+ * `font_family` is the one advertiser-derived value that lands in a CSS
+ * context rather than a text node, and `esc()` is the wrong tool for that: it
+ * escapes for HTML, and inside a `<style>` block `&quot;` is not a quote and
+ * `}` is still a closing brace. An unfiltered value could close the rule and
+ * open its own — a defacement today, and worse once the unit is allowed to run
+ * scripts.
+ *
+ * So this is an allow-list of shapes rather than an escape: letters, digits,
+ * spaces, commas, hyphens and underscores, which is every stack we actually
+ * ship (`system-ui, -apple-system, Segoe UI, Roboto, sans-serif` and
+ * `system-ui, sans-serif` are the only two in production). Quotes are refused
+ * outright — `Helvetica Neue` is valid CSS unquoted, so the quoted form buys
+ * nothing and costs the one character most useful for breaking out.
+ *
+ * Applied where the value is interpolated, not only where it is saved: that is
+ * what makes it true for the 2,978 creatives already stored.
+ */
+const SAFE_FONT_STACK = /^[A-Za-z0-9 _,-]+$/;
+
+export function safeFontFamily(v: string | null | undefined): string {
+  const raw = (v ?? "").trim();
+  if (!raw || raw.length > 120) return DEFAULT_FONT_STACK;
+  return SAFE_FONT_STACK.test(raw) ? raw : DEFAULT_FONT_STACK;
+}
+
 // The brand mark: a real <img> logo when we have one, otherwise an accent-tinted
 // monogram tile. Never renders empty. Sandboxed served ads can't run JS, so we
 // only show the <img> when the URL was verified at generation time.
@@ -628,7 +659,7 @@ export function renderCreativeHtml(
   if (creative.format === "feed_item") {
     return `<!doctype html><html><head><meta charset="utf-8"><style>${vars}
       body{margin:0;padding:12px;background:${cssVar("bg")};color:${cssVar("fg")};
-        font-family:${creative.fontFamily};font-size:14px;line-height:1.45}
+        font-family:${safeFontFamily(creative.fontFamily)};font-size:14px;line-height:1.45}
       a{color:${cssVar("accent")}}
       pre{overflow:auto;font:12px/1.35 ui-monospace,SFMono-Regular,Menlo,Consolas,monospace}
       hr{border:0;border-top:1px solid ${cssVar("edge")}}
@@ -655,7 +686,7 @@ export function renderCreativeHtml(
       *{box-sizing:border-box;margin:0}
       a{text-decoration:none;display:block}
       .cp-ad{display:flex;align-items:center;gap:8px;width:100%;height:${h}px;
-        background:${cssVar("bg")};font-family:${creative.fontFamily};font-size:13px;
+        background:${cssVar("bg")};font-family:${safeFontFamily(creative.fontFamily)};font-size:13px;
         padding:0 12px;overflow:hidden;border-radius:0;
         border:1px solid ${cssVar("edge")};border-left:3px solid ${cssVar("accent")}}
       .cp-head{color:${cssVar("fg")};flex:0 1 auto;min-width:0;white-space:nowrap;
@@ -730,7 +761,7 @@ export function renderCreativeHtml(
     *{box-sizing:border-box;margin:0}
     a{text-decoration:none;display:block}
     .cp-ad{position:relative;width:${w}px;height:${h}px;background:${cssVar("bg")};
-      font-family:${creative.fontFamily};overflow:hidden;border-radius:0;
+      font-family:${safeFontFamily(creative.fontFamily)};overflow:hidden;border-radius:0;
       border:1px solid ${cssVar("edge")};display:flex;flex-direction:column}
     .cp-stage{width:100%;height:${stage}px;flex:0 0 auto;background:${cssVar("solidBg")};
       display:block;object-fit:cover}
@@ -847,7 +878,7 @@ export function renderCreativeHtml(
     *{box-sizing:border-box;margin:0}
     a{text-decoration:none;display:block}
     .cp-wrap{position:relative;width:${w}px;height:${h}px;overflow:hidden}
-    .cp-ad{position:relative;width:${w}px;height:${h}px;background:${bg};font-family:${creative.fontFamily};
+    .cp-ad{position:relative;width:${w}px;height:${h}px;background:${bg};font-family:${safeFontFamily(creative.fontFamily)};
       border-radius:0;padding:${isMobile ? "8px 10px" : "14px"};overflow:hidden;
       border:1px solid ${cssVar("edge")}}
   </style></head><body>

tests/ads-font-family-safety.test.ts

@@ -0,0 +1,44 @@
+import { describe, expect, it } from "vitest";
+import { DEFAULT_FONT_STACK, safeFontFamily } from "@/lib/ads/creative";
+
+describe("safeFontFamily", () => {
+  it("keeps both stacks that actually ship", () => {
+    // The only two values in production, across 2,978 creatives.
+    expect(safeFontFamily("system-ui, -apple-system, Segoe UI, Roboto, sans-serif")).toBe(
+      "system-ui, -apple-system, Segoe UI, Roboto, sans-serif",
+    );
+    expect(safeFontFamily("system-ui, sans-serif")).toBe("system-ui, sans-serif");
+  });
+
+  it("refuses a value that could close the CSS rule it sits in", () => {
+    // The whole point: this lands inside a <style> block, where esc() does
+    // nothing useful — `}` is still a closing brace after HTML-escaping.
+    expect(safeFontFamily("x} .cp-ad{background:red} y{")).toBe(DEFAULT_FONT_STACK);
+    expect(safeFontFamily("serif;color:red")).toBe(DEFAULT_FONT_STACK);
+  });
+
+  it("refuses anything that could pull in a remote resource", () => {
+    // CSS can exfiltrate through a URL even with scripts disabled.
+    expect(safeFontFamily("serif} *{background:url(https://evil.test/x)}")).toBe(DEFAULT_FONT_STACK);
+    expect(safeFontFamily("url(https://evil.test/x)")).toBe(DEFAULT_FONT_STACK);
+  });
+
+  it("refuses quotes, angle brackets and backslashes outright", () => {
+    for (const bad of ['"Helvetica"', "'Helvetica'", "a<b", "a>b", "a\\b", "a/*x*/b"]) {
+      expect(safeFontFamily(bad)).toBe(DEFAULT_FONT_STACK);
+    }
+  });
+
+  it("falls back on empty, missing and over-long values", () => {
+    expect(safeFontFamily("")).toBe(DEFAULT_FONT_STACK);
+    expect(safeFontFamily("   ")).toBe(DEFAULT_FONT_STACK);
+    expect(safeFontFamily(null)).toBe(DEFAULT_FONT_STACK);
+    expect(safeFontFamily(undefined)).toBe(DEFAULT_FONT_STACK);
+    // Long but otherwise legal: still refused, because nothing we ship is.
+    expect(safeFontFamily("a".repeat(121))).toBe(DEFAULT_FONT_STACK);
+  });
+
+  it("trims rather than rejecting incidental whitespace", () => {
+    expect(safeFontFamily("  Roboto, sans-serif  ")).toBe("Roboto, sans-serif");
+  });
+});

tests/ads-sandbox-scripts.test.ts

@@ -0,0 +1,40 @@
+import { describe, expect, it } from "vitest";
+import { GET } from "@/app/ad.js/route";
+
+async function tag(): Promise<string> {
+  const res = await GET();
+  return await res.text();
+}
+
+describe("ad.js sandbox", () => {
+  it("never grants allow-same-origin", async () => {
+    // With allow-scripts AND allow-same-origin together, the framed document
+    // can reach frameElement and delete its own sandbox attribute — which is
+    // not a sandbox. This assertion is the standing rule, in executable form.
+    //
+    // Comments are stripped first: the tag explains in prose why the pair is
+    // never granted, and the rule is about the value, not the file.
+    const js = (await tag()).replace(/^\s*\/\/.*$/gm, "");
+    expect(js).not.toContain("allow-same-origin");
+  });
+
+  it("grants scripts only to the media that can report playback", async () => {
+    const js = await tag();
+    expect(js).toContain("if (res.media === 'video' || res.media === 'audio') sandbox += ' allow-scripts';");
+    // The base sandbox, used for every other fill, stays script-free.
+    expect(js).toContain(
+      "var sandbox = 'allow-popups allow-popups-to-escape-sandbox allow-top-navigation-by-user-activation';",
+    );
+  });
+
+  it("still keeps the click-out permissions it always had", async () => {
+    const js = await tag();
+    expect(js).toContain("allow-popups");
+    expect(js).toContain("allow-top-navigation-by-user-activation");
+  });
+
+  it("grants autoplay only to video, as before", async () => {
+    const js = await tag();
+    expect(js).toContain("if (res.media === 'video') iframe.setAttribute('allow', 'autoplay');");
+  });
+});