fix(server): add app-wide browser security headers (#1541)

packages/server/src/__tests__/security-headers.test.ts

@@ -0,0 +1,156 @@
+import { mkdtemp, rm, writeFile } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import Fastify, { type FastifyInstance } from "fastify";
+import { describe, expect, it } from "vitest";
+import { buildApp } from "../app.js";
+import type { Config } from "../config.js";
+import { CONTENT_SECURITY_POLICY, SECURITY_HEADERS, securityHeadersHook } from "../security-headers.js";
+
+/**
+ * Issue #1541 / audit SEC-041: the repo must carry an app-wide browser
+ * security header layer that is visible in code and testable — SPA shell,
+ * static assets, and API JSON all included.
+ */
+
+const baseConfig: Config = {
+  channel: "dev",
+  growth: {
+    landingPagesEnabled: false,
+    landingCampaignMaxAgentTurns: 1,
+    landingCampaignMaxEstimatedTokens: 120_000,
+    landingCampaignMaxTrialsPerUserPer24Hours: 5,
+  },
+  docs: { enabled: false },
+  database: { url: process.env.DATABASE_URL ?? "", provider: "external" },
+  server: { port: 0, host: "127.0.0.1", publicUrl: undefined },
+  workspace: { root: "/tmp/first-tree-test-workspaces" },
+  secrets: {
+    jwtSecret: "test-jwt-secret-key-for-vitest",
+    encryptionKey: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
+  },
+  auth: { accessTokenExpiry: "30m", refreshTokenExpiry: "30d", connectTokenExpiry: "10m" },
+  trustProxy: false,
+  connectBootstrap: {
+    portableDownloadBaseUrl: "https://download.first-tree.ai/releases",
+  },
+  observability: { logging: { level: "error", format: "json", bridgeToSpanLevel: "off" } },
+  runtime: {
+    agentHttpTokenEnforcement: false,
+    runtimeSwitchFaultInjection: false,
+    pollingIntervalSeconds: 5,
+    presenceCleanupSeconds: 60,
+    archiveSweepIntervalSeconds: 0,
+    archiveMappedIdleSeconds: 60 * 60,
+    notificationWebhookUrl: undefined,
+  },
+  update: {
+    commandVersion: "test.version",
+    pollIntervalMinutes: 1440,
+    registryUrl: "https://localhost.invalid",
+  },
+  instanceId: "test-instance",
+};
+
+async function safeClose(app: FastifyInstance | undefined) {
+  if (app) await app.close();
+}
+
+function expectSecurityHeaders(headers: Record<string, unknown>) {
+  for (const [name, value] of Object.entries(SECURITY_HEADERS)) {
+    expect(headers[name], `missing ${name}`).toBe(value);
+  }
+}
+
+describe("security header policy", () => {
+  it("covers every header the audit listed as the minimum", () => {
+    expect(Object.keys(SECURITY_HEADERS).sort()).toEqual(
+      [
+        "content-security-policy",
+        "strict-transport-security",
+        "x-content-type-options",
+        "referrer-policy",
+        "x-frame-options",
+        "permissions-policy",
+      ].sort(),
+    );
+    expect(SECURITY_HEADERS["x-content-type-options"]).toBe("nosniff");
+    expect(SECURITY_HEADERS["strict-transport-security"]).toContain("max-age=");
+    expect(SECURITY_HEADERS["x-frame-options"]).toBe("DENY");
+  });
+
+  it("builds a CSP that disallows framing but keeps the SPA functional", () => {
+    const directives = Object.fromEntries(
+      CONTENT_SECURITY_POLICY.split("; ").map((d) => {
+        const [name, ...rest] = d.split(" ");
+        return [name, rest.join(" ")];
+      }),
+    );
+    // The audit's core asks: no framing, no plugins/objects, locked base/URL form targets.
+    expect(directives["default-src"]).toBe("'self'");
+    expect(directives["frame-ancestors"]).toBe("'none'");
+    expect(directives["object-src"]).toBe("'none'");
+    expect(directives["base-uri"]).toBe("'self'");
+    expect(directives["form-action"]).toBe("'self'");
+    // The SPA's reality: inline bootstrap scripts in index.html plus the
+    // GA4 / Clarity tags they inject must keep working (nonce hardening is
+    // future work — see security-headers.ts).
+    expect(directives["script-src"]).toContain("'unsafe-inline'");
+    expect(directives["script-src"]).toContain("https://www.googletagmanager.com");
+    expect(directives["script-src"]).toContain("https://www.clarity.ms");
+    // Same-origin API + WebSocket, analytics collect, optional Sentry.
+    expect(directives["connect-src"]).toContain("'self'");
+    expect(directives["connect-src"]).toContain("wss:");
+    expect(directives["connect-src"]).toContain("https://*.google-analytics.com");
+    expect(directives["connect-src"]).toContain("https://*.clarity.ms");
+    expect(directives["connect-src"]).toContain("https://*.ingest.sentry.io");
+  });
+});
+
+describe("securityHeadersHook", () => {
+  it("does not override a header a route set itself", async () => {
+    const app = Fastify();
+    try {
+      app.addHook("onSend", securityHeadersHook);
+      app.get("/custom", (_req, reply) => {
+        void reply.header("x-frame-options", "SAMEORIGIN");
+        return { ok: true };
+      });
+      const res = await app.inject({ method: "GET", url: "/custom" });
+      expect(res.statusCode).toBe(200);
+      expect(res.headers["x-frame-options"]).toBe("SAMEORIGIN");
+      // …while the rest of the set is still filled in around it.
+      expect(res.headers["content-security-policy"]).toBe(SECURITY_HEADERS["content-security-policy"]);
+      expect(res.headers["x-content-type-options"]).toBe("nosniff");
+    } finally {
+      await app.close();
+    }
+  });
+});
+
+describe("buildApp — app-wide security headers", () => {
+  it("stamps the full set on the SPA shell, static misses, and API responses", async () => {
+    const webRoot = await mkdtemp(join(tmpdir(), "first-tree-web-"));
+    await writeFile(join(webRoot, "index.html"), "<!doctype html><html><body>App shell</body></html>", "utf8");
+
+    let app: FastifyInstance | undefined;
+    try {
+      app = await buildApp({ ...baseConfig, webDistPath: webRoot });
+
+      const spa = await app.inject({ method: "GET", url: "/workspace/deep-link" });
+      expect(spa.statusCode).toBe(200);
+      expectSecurityHeaders(spa.headers);
+
+      const apiMiss = await app.inject({ method: "GET", url: "/api/missing" });
+      expect(apiMiss.statusCode).toBe(404);
+      expectSecurityHeaders(apiMiss.headers);
+
+      const healthz = await app.inject({ method: "GET", url: "/healthz" });
+      expect(healthz.statusCode).toBe(200);
+      expectSecurityHeaders(healthz.headers);
+    } finally {
+      await safeClose(app);
+      await rm(webRoot, { recursive: true, force: true });
+    }
+  });
+});

packages/server/src/app.ts

@@ -92,6 +92,7 @@ import {
   reportErrorToRoot,
   rootLogger,
 } from "./observability/index.js";
+import { securityHeadersHook } from "./security-headers.js";
 import { broadcastToAdmins } from "./services/admin-broadcast.js";
 import { expiryToSeconds } from "./services/auth.js";
 import { type BackgroundTasks, createBackgroundTasks } from "./services/background-tasks.js";
@@ -370,6 +371,12 @@ export async function buildApp(config: Config) {
     credentials: true,
   });
 
+  // App-wide browser security headers (CSP / HSTS / nosniff / frame / referrer
+  // / permissions) on every response — SPA, static assets, API JSON, errors.
+  // The hook only fills headers a route has not set itself, so route-local
+  // policies keep winning. See security-headers.ts (issue #1541).
+  app.addHook("onSend", securityHeadersHook);
+
   // Rate limiting — single actor-aware global safety cap.
   // `hook: "preHandler"` runs the limiter after route-level onRequest hooks
   // (memberAuth, agentSelector) so the key generator can read

packages/server/src/security-headers.ts

@@ -0,0 +1,83 @@
+import type { onSendHookHandler } from "fastify";
+
+/**
+ * App-wide browser security headers (issue #1541 / audit SEC-041).
+ *
+ * The server serves both the JSON API and the SPA from one Fastify instance,
+ * but before this module the only security header anywhere was an
+ * attachment-specific `X-Content-Type-Options` (api/attachments.ts). Anything
+ * the edge (Cloudflare / CapRover) adds on top was invisible and untestable
+ * from the repo. This hook makes the baseline contract code: every response —
+ * SPA shell, static asset, API JSON, error page — carries the set below.
+ *
+ * The hook only fills headers the route has not set itself, so a route with a
+ * deliberately different policy (none today) keeps its own value.
+ */
+
+/**
+ * Content-Security-Policy for the SPA, built as an array so each directive
+ * can carry its own rationale. Audit findings this answers:
+ *
+ *  - `default-src 'self'` — anything not listed below comes from our origin.
+ *  - `script-src … 'unsafe-inline'` — packages/web/index.html ships three
+ *    inline bootstrap scripts (GA4 loader, Clarity loader, theme init) and
+ *    dynamically injects gtag.js / the Clarity tag from those two hosts.
+ *    `'unsafe-inline'` keeps them working; moving to nonces means rewriting
+ *    index.html per response and is deliberately out of scope here.
+ *  - `style-src … 'unsafe-inline'` — React renders dynamic `style` attributes
+ *    (style-src-attr falls back to style-src).
+ *  - `img-src … https:` — chat markdown renders user-pasted external images;
+ *    https-only blocks mixed content without breaking that content.
+ *  - `font-src 'self'` — Inter / JetBrains Mono are self-hosted (see the
+ *    <link rel="preload"> tags in index.html).
+ *  - `connect-src` — same-origin REST + WebSocket (`ws:`/`wss:` for browsers
+ *    that don't fold websockets into 'self'), GA4 collect, Clarity collect,
+ *    and the optional Sentry browser SDK (VITE_SENTRY_DSN).
+ *  - `frame-ancestors 'none'` — nothing legitimately frames the app (the
+ *    first-tree.ai link on the login page is a plain anchor), so the
+ *    authenticated dashboard must not be embeddable. Paired with
+ *    `X-Frame-Options: DENY` for pre-CSP2 browsers.
+ *  - `object-src 'none'` / `base-uri 'self'` / `form-action 'self'` — classic
+ *    XSS escalation lids; the app uses none of these surfaces.
+ */
+const CSP_DIRECTIVES = [
+  "default-src 'self'",
+  "script-src 'self' 'unsafe-inline' https://www.googletagmanager.com https://www.clarity.ms",
+  "style-src 'self' 'unsafe-inline'",
+  "img-src 'self' data: blob: https:",
+  "font-src 'self'",
+  "connect-src 'self' ws: wss: https://*.google-analytics.com https://www.googletagmanager.com https://*.clarity.ms https://*.ingest.sentry.io https://*.ingest.us.sentry.io",
+  "object-src 'none'",
+  "base-uri 'self'",
+  "form-action 'self'",
+  "frame-ancestors 'none'",
+];
+
+export const CONTENT_SECURITY_POLICY = CSP_DIRECTIVES.join("; ");
+
+export const SECURITY_HEADERS: Record<string, string> = {
+  "content-security-policy": CONTENT_SECURITY_POLICY,
+  // 2 years, subdomains included. No `preload`: list submission is a
+  // separate, deliberate step. Harmless over plain HTTP (browsers ignore it).
+  "strict-transport-security": "max-age=63072000; includeSubDomains",
+  "x-content-type-options": "nosniff",
+  // Matches the modern browser default, but pinned so it cannot drift with
+  // client defaults: cross-origin requests leak origin only, never full URL.
+  "referrer-policy": "strict-origin-when-cross-origin",
+  // Redundant with CSP frame-ancestors for modern browsers; kept for legacy.
+  "x-frame-options": "DENY",
+  // The dashboard uses no sensor APIs; deny the noisy ones outright.
+  "permissions-policy": "camera=(), microphone=(), geolocation=(), payment=(), usb=()",
+};
+
+/**
+ * onSend hook — runs for every response the instance emits, including error
+ * and not-found responses, which is exactly the "app-wide" coverage the
+ * audit asked for. Registered on the root instance in buildApp.
+ */
+export const securityHeadersHook: onSendHookHandler = (_request, reply, _payload, done) => {
+  for (const [name, value] of Object.entries(SECURITY_HEADERS)) {
+    if (!reply.hasHeader(name)) reply.header(name, value);
+  }
+  done();
+};