diff --git a/canvas/src/__tests__/csp-nonce.test.ts b/canvas/src/__tests__/csp-nonce.test.ts index 317a3a46..5327a1b9 100644 --- a/canvas/src/__tests__/csp-nonce.test.ts +++ b/canvas/src/__tests__/csp-nonce.test.ts @@ -6,8 +6,8 @@ * * Fix: nonce-based script-src in production; permissive only in dev. */ -import { describe, it, expect } from "vitest"; -import { buildCsp } from "../middleware"; +import { describe, it, expect, afterEach } from "vitest"; +import { buildCsp, buildImgSrc } from "../middleware"; const TEST_NONCE = "dGVzdC1ub25jZQ=="; // base64("test-nonce") @@ -147,5 +147,52 @@ describe("buildCsp — format invariants", () => { expect(imgSrc).toContain("blob:"); expect(imgSrc).toContain("data:"); }); + + it(`[${label}] allows the generated-image R2 host in img-src`, () => { + const imgSrc = csp.match(/img-src[^;]*/)?.[0] ?? ""; + expect(imgSrc).toContain("r2.cloudflarestorage.com"); + }); } }); + +// --------------------------------------------------------------------------- +// buildImgSrc — generated-image (image-gen socket / R2) host handling +// --------------------------------------------------------------------------- +describe("buildImgSrc — generated-image R2 host", () => { + afterEach(() => { + delete process.env.NEXT_PUBLIC_IMAGE_GEN_R2_HOST; + }); + + it("keeps self/blob:/data: for the canvas's own assets", () => { + const imgSrc = buildImgSrc(); + expect(imgSrc.startsWith("img-src ")).toBe(true); + expect(imgSrc).toContain("'self'"); + expect(imgSrc).toContain("blob:"); + expect(imgSrc).toContain("data:"); + }); + + it("defaults to the documented R2 wildcard when no host is pinned", () => { + delete process.env.NEXT_PUBLIC_IMAGE_GEN_R2_HOST; + expect(buildImgSrc()).toContain("https://*.r2.cloudflarestorage.com"); + }); + + it("uses NEXT_PUBLIC_IMAGE_GEN_R2_HOST when pinned (tightest policy)", () => { + const pinned = + "https://molecule-workspace-data.deadbeef.r2.cloudflarestorage.com"; + process.env.NEXT_PUBLIC_IMAGE_GEN_R2_HOST = pinned; + const imgSrc = buildImgSrc(); + expect(imgSrc).toContain(pinned); + expect(imgSrc).not.toContain("*.r2.cloudflarestorage.com"); + }); +}); + +// --------------------------------------------------------------------------- +// Security invariant: connect-src must NOT be widened to R2 (no exfil channel) +// --------------------------------------------------------------------------- +describe("buildCsp — connect-src stays narrow (R2 only displayable, not fetchable)", () => { + it("does not include any r2.cloudflarestorage.com host in connect-src", () => { + const csp = buildCsp(TEST_NONCE, false); + const connectSrc = csp.match(/connect-src[^;]*/)?.[0] ?? ""; + expect(connectSrc).not.toContain("r2.cloudflarestorage.com"); + }); +}); diff --git a/canvas/src/middleware.ts b/canvas/src/middleware.ts index f904a645..3e731434 100644 --- a/canvas/src/middleware.ts +++ b/canvas/src/middleware.ts @@ -14,6 +14,9 @@ import type { NextRequest } from "next/server"; * is significantly lower risk than script injection and is acceptable here. * • object-src locked to 'none'; frame-src allows self + blob: for * browser-native PDF previews backed by authenticated Blob URLs. + * • img-src allows the generated-image blob host (Cloudflare R2) so the + * chat can render images returned by the image-gen capability socket as + * presigned R2 URLs. See buildImgSrc() for the security rationale. * • base-uri / frame-ancestors locked to 'self'/'none'. * • upgrade-insecure-requests forces HTTPS on mixed-content. * @@ -24,13 +27,49 @@ import type { NextRequest } from "next/server"; * * Exported for unit testing. */ +/** + * Build the img-src directive value. + * + * Beyond the canvas's own assets ('self' + blob:/data: for avatars, + * thumbnails, and ObjectURL-wrapped authenticated attachments) we must allow + * the host that serves GENERATED images. The image-gen capability socket + * (RFC #3105) stores each generated image in Cloudflare R2 and returns it to + * the agent as a time-boxed, SigV4-presigned GET URL on + * `..r2.cloudflarestorage.com`. The chat renders that + * URL directly as ``, so the + * host must be in img-src or the browser blocks the image (broken thumbnail). + * + * Which host? + * • The bucket name (MOLECULE_IMAGE_GEN_BUCKET) and the CF R2 account hash + * (MOLECULE_IMAGE_GEN_ENDPOINT) are CONTROL-PLANE deploy config, not known + * to this canvas build. So we cannot hardcode the exact host here. + * • A deploy MAY pin the exact origin via NEXT_PUBLIC_IMAGE_GEN_R2_HOST + * (e.g. "https://molecule-workspace-data..r2.cloudflarestorage.com") + * — preferred when known, since it is the tightest policy. + * • Otherwise we fall back to the documented wildcard + * `https://*.r2.cloudflarestorage.com`. + * + * Security rationale for the wildcard fallback: this ONLY widens img-src + * (image *display*). connect-src is left unchanged, so fetch()/XHR to R2 stays + * blocked — there is no data-exfiltration channel via this directive. The R2 + * URLs the browser loads are time-boxed, SigV4-presigned GETs scoped to a + * single object key that the agent already legitimately holds; permitting the + * `` to render them grants no new capability beyond viewing an image the + * user's own agent produced. + */ +export function buildImgSrc(): string { + const pinned = process.env.NEXT_PUBLIC_IMAGE_GEN_R2_HOST ?? ""; + const r2Host = pinned || "https://*.r2.cloudflarestorage.com"; + return `img-src 'self' blob: data: ${r2Host}`; +} + export function buildCsp(nonce: string, isDev: boolean): string { if (isDev) { return [ "default-src 'self'", "script-src 'self' 'unsafe-inline' 'unsafe-eval'", "style-src 'self' 'unsafe-inline'", - "img-src 'self' blob: data:", + buildImgSrc(), "font-src 'self'", "connect-src *", "worker-src 'self' blob:", @@ -60,7 +99,7 @@ export function buildCsp(nonce: string, isDev: boolean): string { `script-src 'self' 'nonce-${nonce}' 'strict-dynamic'`, // unsafe-inline kept for inline style="" attributes used by React Flow. "style-src 'self' 'unsafe-inline'", - "img-src 'self' blob: data:", + buildImgSrc(), "font-src 'self'", "object-src 'none'", "frame-src 'self' blob:", diff --git a/workspace-server/internal/middleware/securityheaders.go b/workspace-server/internal/middleware/securityheaders.go index c9aa1470..dd693ae2 100644 --- a/workspace-server/internal/middleware/securityheaders.go +++ b/workspace-server/internal/middleware/securityheaders.go @@ -1,11 +1,44 @@ package middleware import ( + "os" "strings" "github.com/gin-gonic/gin" ) +// generatedImageImgSrc returns the img-src directive for canvas (HTML) routes. +// +// Beyond the canvas's own assets ('self' + data:/blob:) we must allow the host +// that serves GENERATED images. The image-gen capability socket (RFC #3105) +// stores each generated image in Cloudflare R2 and hands the agent a time-boxed, +// SigV4-presigned GET URL on `..r2.cloudflarestorage.com`. +// The chat renders that URL directly as +// ``, so the host must be in +// img-src or the browser blocks display (broken thumbnail). +// +// The exact bucket + CF account hash are control-plane deploy config, so we +// cannot hardcode the origin. A deploy MAY pin it via MOLECULE_IMAGE_GEN_R2_HOST +// (tightest policy); otherwise we use the documented wildcard +// `https://*.r2.cloudflarestorage.com`. +// +// This must stay in sync with canvas/src/middleware.ts buildImgSrc(): the +// combined tenant image returns BOTH this Go header and the proxied Next.js +// CSP, and browsers enforce the INTERSECTION of multiple CSP headers — so the +// generated-image host has to be present in both or it is still blocked. +// +// Security: this widens only image *display* (img-src). connect-src is left +// unchanged, so fetch()/XHR to R2 stays blocked — no exfiltration channel. The +// URLs are time-boxed, signed GETs of a single object key the agent already +// holds; rendering them grants no capability beyond viewing the produced image. +func generatedImageImgSrc() string { + r2Host := os.Getenv("MOLECULE_IMAGE_GEN_R2_HOST") + if r2Host == "" { + r2Host = "https://*.r2.cloudflarestorage.com" + } + return "img-src 'self' data: blob: " + r2Host + "; " +} + // apiPrefixes lists the URL path prefixes that are served by Go platform // handlers (JSON/binary responses). Canvas-proxied routes (Next.js HTML) are // everything not in this list — they require 'unsafe-inline' for hydration. @@ -94,7 +127,10 @@ func SecurityHeaders() gin.HandlerFunc { "default-src 'self'; "+ "script-src 'self' 'unsafe-inline'; "+ "style-src 'self' 'unsafe-inline'; "+ - "img-src 'self' data: blob:; "+ + // img-src includes the generated-image R2 host so the chat + // can render image-gen results (presigned R2 URLs). See + // generatedImageImgSrc() for the security rationale. + generatedImageImgSrc()+ "frame-src 'self' blob:; "+ "connect-src 'self' ws: wss:; "+ "font-src 'self' data:") diff --git a/workspace-server/internal/middleware/securityheaders_test.go b/workspace-server/internal/middleware/securityheaders_test.go index 2f3e6d4b..17ce4bfd 100644 --- a/workspace-server/internal/middleware/securityheaders_test.go +++ b/workspace-server/internal/middleware/securityheaders_test.go @@ -56,7 +56,9 @@ func TestSecurityHeaders(t *testing.T) { "default-src 'self'", "script-src 'self' 'unsafe-inline'", "style-src 'self' 'unsafe-inline'", - "img-src 'self' data: blob:", + // img-src must allow the generated-image R2 host (default wildcard) + // in addition to self/data:/blob: — see generatedImageImgSrc(). + "img-src 'self' data: blob: https://*.r2.cloudflarestorage.com", "frame-src 'self' blob:", "connect-src 'self' ws: wss:", "font-src 'self' data:", @@ -203,6 +205,88 @@ func TestCSPCanvasRoutesGetPermissivePolicy(t *testing.T) { } } +// TestCSPGeneratedImageHost verifies the canvas CSP allows the generated-image +// R2 host in img-src (so image-gen results render) WITHOUT widening connect-src +// (no exfiltration channel). Covers both the default wildcard and a pinned host +// via MOLECULE_IMAGE_GEN_R2_HOST. +func TestCSPGeneratedImageHost(t *testing.T) { + t.Run("default wildcard", func(t *testing.T) { + t.Setenv("MOLECULE_IMAGE_GEN_R2_HOST", "") + r := gin.New() + r.Use(SecurityHeaders()) + r.GET("/", func(c *gin.Context) { c.String(http.StatusOK, "") }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/", nil) + r.ServeHTTP(w, req) + + csp := w.Header().Get("Content-Security-Policy") + imgSrc := cspDirective(csp, "img-src") + if !strings.Contains(imgSrc, "https://*.r2.cloudflarestorage.com") { + t.Errorf("img-src should allow R2 wildcard for generated images, got %q", imgSrc) + } + // img-src must still keep self/data:/blob: for canvas's own assets. + for _, want := range []string{"'self'", "data:", "blob:"} { + if !strings.Contains(imgSrc, want) { + t.Errorf("img-src missing %q, got %q", want, imgSrc) + } + } + // connect-src must NOT be widened to R2 — fetch()/XHR to R2 stays blocked. + connectSrc := cspDirective(csp, "connect-src") + if strings.Contains(connectSrc, "r2.cloudflarestorage.com") { + t.Errorf("connect-src must NOT include R2 (no exfil channel), got %q", connectSrc) + } + }) + + t.Run("pinned host", func(t *testing.T) { + const pinned = "https://molecule-workspace-data.deadbeef.r2.cloudflarestorage.com" + t.Setenv("MOLECULE_IMAGE_GEN_R2_HOST", pinned) + r := gin.New() + r.Use(SecurityHeaders()) + r.GET("/", func(c *gin.Context) { c.String(http.StatusOK, "") }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/", nil) + r.ServeHTTP(w, req) + + imgSrc := cspDirective(w.Header().Get("Content-Security-Policy"), "img-src") + if !strings.Contains(imgSrc, pinned) { + t.Errorf("img-src should use the pinned R2 host, got %q", imgSrc) + } + if strings.Contains(imgSrc, "*.r2.cloudflarestorage.com") { + t.Errorf("img-src should NOT fall back to wildcard when pinned, got %q", imgSrc) + } + }) + + t.Run("api routes stay strict", func(t *testing.T) { + t.Setenv("MOLECULE_IMAGE_GEN_R2_HOST", "") + r := gin.New() + r.Use(SecurityHeaders()) + r.GET("/workspaces/abc", func(c *gin.Context) { c.JSON(http.StatusOK, nil) }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/workspaces/abc", nil) + r.ServeHTTP(w, req) + + csp := w.Header().Get("Content-Security-Policy") + if csp != "default-src 'self'" { + t.Errorf("API route must keep strict CSP (no R2 host), got %q", csp) + } + }) +} + +// cspDirective extracts the single directive (e.g. "img-src ...") from a CSP +// header value, or "" if absent. +func cspDirective(csp, name string) string { + for _, d := range strings.Split(csp, ";") { + d = strings.TrimSpace(d) + if strings.HasPrefix(d, name+" ") || d == name { + return d + } + } + return "" +} + // TestSecurityHeaders_614_NosniffOnSSEAndAPIEndpoints is the acceptance test for // issue #614 — verifies X-Content-Type-Options: nosniff and X-Frame-Options: DENY // are present on API and SSE paths. SecurityHeaders() was already wired globally