From e5c1bc873cd6db84db5815a8915bb6075c654798 Mon Sep 17 00:00:00 2001 From: Mathew Goldsborough <1759329+mgoldsborough@users.noreply.github.com> Date: Thu, 16 Apr 2026 07:06:19 -1000 Subject: [PATCH 1/2] feat: set default HSTS and CSP headers for direct-exposure deployments (#14) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous stance of "HSTS/CSP belong on the reverse proxy" leaves self-hosted OSS deployments with no hardening when no proxy sits in front. Direct-exposure HTTPS is a supported topology, so the defaults should protect it. Add conservative defaults in securityHeaders(): - HSTS: max-age=31536000; includeSubDomains (no preload — that is a deliberate operator choice) - CSP: default-src 'none'; frame-ancestors 'none'; base-uri 'none' (no effect on JSON/SSE; bundle UI HTML is consumed via fetch+srcdoc so the response CSP does not break the iframe bridge, but does protect anyone opening that HTML directly) Operators who terminate TLS at a proxy that already emits these headers can disable them with NB_HSTS="" / NB_CSP="", or override to a stricter/looser value via env var or the middleware option. Env var takes precedence over option so ops can change the runtime policy without a code change. --- README.md | 2 + src/api/middleware/security-headers.ts | 63 +++++++++++++++++-- src/api/routes/proxy.ts | 5 ++ test/unit/api/security-headers.test.ts | 83 +++++++++++++++++++++----- 4 files changed, 135 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 7c115af8..f6eeb730 100644 --- a/README.md +++ b/README.md @@ -333,6 +333,8 @@ Run `nb --help` or `nb --help` for full usage. If you haven't run `bun | `NB_BUNDLE_START_CONCURRENCY` | Max bundle subprocesses spawned in parallel at boot (default: 4, set to 1 for sequential) | | `NB_TIMEZONE` | Default IANA timezone for time-aware features | | `NB_HOST_URL` | Public host URL for OAuth redirects | +| `NB_HSTS` | `Strict-Transport-Security` value (default: `max-age=31536000; includeSubDomains`). Set to `""` to disable — e.g., when a reverse proxy already emits this header | +| `NB_CSP` | `Content-Security-Policy` value (default: `default-src 'none'; frame-ancestors 'none'; base-uri 'none'`). Set to `""` to disable | **Identity & telemetry** diff --git a/src/api/middleware/security-headers.ts b/src/api/middleware/security-headers.ts index 08d84c4b..0a2de03d 100644 --- a/src/api/middleware/security-headers.ts +++ b/src/api/middleware/security-headers.ts @@ -1,16 +1,60 @@ import { createMiddleware } from "hono/factory"; /** - * Security headers middleware. - * Sets standard browser security headers on every response. - * Does NOT set HSTS or CSP — those belong on the reverse proxy. + * Default HSTS: 1-year, applies to subdomains. No `preload` — opting into the + * preload list is a deliberate operator choice. + */ +export const DEFAULT_HSTS = "max-age=31536000; includeSubDomains"; + +/** + * Default CSP: locks the API down to nothing. JSON and SSE responses are + * unaffected. Bundle UI HTML served from /v1/apps/... is consumed by the + * iframe bridge via fetch + srcdoc (where the response CSP does not apply), + * so a restrictive header actively protects against someone opening that + * HTML directly in a browser. + */ +export const DEFAULT_CSP = "default-src 'none'; frame-ancestors 'none'; base-uri 'none'"; + +export interface SecurityHeadersOptions { + /** + * Strict-Transport-Security value. `undefined` uses the default, empty + * string disables the header. `NB_HSTS` env var takes precedence. + */ + hsts?: string; + /** + * Content-Security-Policy value. `undefined` uses the default, empty string + * disables the header. `NB_CSP` env var takes precedence. + */ + csp?: string; +} + +/** + * Security headers middleware. Sets standard browser security headers on + * every response. + * + * HSTS and CSP are included with conservative defaults so direct-exposure + * self-hosted deployments are not left naked when no reverse proxy sits in + * front. Operators who terminate TLS at a proxy that already emits these + * headers can disable them by setting `NB_HSTS=""` / `NB_CSP=""`, or override + * to a stricter/looser value via env var or option. * * `X-Frame-Options` is set as a *default* (`DENY`) — routes that legitimately * serve framed content (e.g., the same-origin http-proxy bundles use to embed * their dev servers) override it explicitly to `SAMEORIGIN`. We use `set` only * when the route hasn't already provided a value, so route-level intent wins. + * + * The proxy route serves iframed bundle dev-server content, where the strict + * default CSP would block the bundle's own scripts/styles. Such routes set + * the internal `X-NB-Skip-Security-Defaults` response header to opt out of + * HSTS/CSP defaults; this middleware strips that header before egress. The + * parent shell's `frame-ancestors 'none'` is the real protection vector for + * those responses, not a CSP on the iframe content itself. */ -export function securityHeaders() { +export const SKIP_DEFAULTS_HEADER = "X-NB-Skip-Security-Defaults"; + +export function securityHeaders(options: SecurityHeadersOptions = {}) { + const hsts = process.env.NB_HSTS ?? options.hsts ?? DEFAULT_HSTS; + const csp = process.env.NB_CSP ?? options.csp ?? DEFAULT_CSP; return createMiddleware(async (c, next) => { await next(); c.res.headers.set("X-Content-Type-Options", "nosniff"); @@ -20,5 +64,16 @@ export function securityHeaders() { c.res.headers.set("Referrer-Policy", "strict-origin-when-cross-origin"); c.res.headers.set("X-XSS-Protection", "0"); c.res.headers.set("Permissions-Policy", "camera=(), microphone=(), geolocation=()"); + const skipDefaults = c.res.headers.has(SKIP_DEFAULTS_HEADER); + if (skipDefaults) { + c.res.headers.delete(SKIP_DEFAULTS_HEADER); + return; + } + if (hsts && !c.res.headers.has("Strict-Transport-Security")) { + c.res.headers.set("Strict-Transport-Security", hsts); + } + if (csp && !c.res.headers.has("Content-Security-Policy")) { + c.res.headers.set("Content-Security-Policy", csp); + } }); } diff --git a/src/api/routes/proxy.ts b/src/api/routes/proxy.ts index 049efae9..ef0234f5 100644 --- a/src/api/routes/proxy.ts +++ b/src/api/routes/proxy.ts @@ -3,6 +3,7 @@ import { log } from "../../cli/log.ts"; import { WORKSPACE_ID_RE } from "../auth-middleware.ts"; import { requireAuth } from "../middleware/auth.ts"; import { errorLog } from "../middleware/error-log.ts"; +import { SKIP_DEFAULTS_HEADER } from "../middleware/security-headers.ts"; import { type AppContext, type AppEnv, apiError } from "../types.ts"; /** @@ -172,6 +173,10 @@ export function proxyRoutes(ctx: AppContext) { // Same-origin embedding: the security-headers middleware respects this // when already set; cross-origin embedding stays denied. outHeaders.set("X-Frame-Options", "SAMEORIGIN"); + // Opt out of default HSTS/CSP — iframed bundle dev-server content needs + // its own (typically permissive) policy. The middleware strips this + // header before egress. + outHeaders.set(SKIP_DEFAULTS_HEADER, "1"); return new Response(upstream.body, { status: upstream.status, statusText: upstream.statusText, diff --git a/test/unit/api/security-headers.test.ts b/test/unit/api/security-headers.test.ts index ebb9da5f..d8d6dcb6 100644 --- a/test/unit/api/security-headers.test.ts +++ b/test/unit/api/security-headers.test.ts @@ -1,58 +1,113 @@ -import { describe, test, expect } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { Hono } from "hono"; -import { securityHeaders } from "../../../src/api/middleware/security-headers.ts"; +import { + DEFAULT_CSP, + DEFAULT_HSTS, + securityHeaders, +} from "../../../src/api/middleware/security-headers.ts"; -function createTestApp() { +function createTestApp(options?: Parameters[0]) { const app = new Hono(); - app.use("*", securityHeaders()); + app.use("*", securityHeaders(options)); app.get("/test", (c) => c.json({ ok: true })); app.post("/test", (c) => c.json({ ok: true })); return app; } describe("securityHeaders middleware", () => { - const app = createTestApp(); + // Env vars leak across modules in Bun; snapshot/restore to keep tests isolated. + let savedHsts: string | undefined; + let savedCsp: string | undefined; + + beforeEach(() => { + savedHsts = process.env.NB_HSTS; + savedCsp = process.env.NB_CSP; + delete process.env.NB_HSTS; + delete process.env.NB_CSP; + }); + + afterEach(() => { + if (savedHsts === undefined) delete process.env.NB_HSTS; + else process.env.NB_HSTS = savedHsts; + if (savedCsp === undefined) delete process.env.NB_CSP; + else process.env.NB_CSP = savedCsp; + }); test("sets X-Content-Type-Options: nosniff on all responses", async () => { - const res = await app.request("/test"); + const res = await createTestApp().request("/test"); expect(res.headers.get("X-Content-Type-Options")).toBe("nosniff"); }); test("sets X-Frame-Options: DENY", async () => { - const res = await app.request("/test"); + const res = await createTestApp().request("/test"); expect(res.headers.get("X-Frame-Options")).toBe("DENY"); }); test("sets Referrer-Policy: strict-origin-when-cross-origin", async () => { - const res = await app.request("/test"); + const res = await createTestApp().request("/test"); expect(res.headers.get("Referrer-Policy")).toBe("strict-origin-when-cross-origin"); }); test("sets X-XSS-Protection: 0", async () => { - const res = await app.request("/test"); + const res = await createTestApp().request("/test"); expect(res.headers.get("X-XSS-Protection")).toBe("0"); }); test("sets Permissions-Policy", async () => { - const res = await app.request("/test"); + const res = await createTestApp().request("/test"); expect(res.headers.get("Permissions-Policy")).toBe("camera=(), microphone=(), geolocation=()"); }); - test("does not set HSTS or CSP", async () => { - const res = await app.request("/test"); + test("sets HSTS and CSP defaults for direct-exposure deployments", async () => { + const res = await createTestApp().request("/test"); + expect(res.headers.get("Strict-Transport-Security")).toBe(DEFAULT_HSTS); + expect(res.headers.get("Content-Security-Policy")).toBe(DEFAULT_CSP); + }); + + test("option overrides default HSTS/CSP", async () => { + const res = await createTestApp({ + hsts: "max-age=60", + csp: "default-src 'self'", + }).request("/test"); + expect(res.headers.get("Strict-Transport-Security")).toBe("max-age=60"); + expect(res.headers.get("Content-Security-Policy")).toBe("default-src 'self'"); + }); + + test("empty-string option disables HSTS/CSP (delegates to reverse proxy)", async () => { + const res = await createTestApp({ hsts: "", csp: "" }).request("/test"); expect(res.headers.get("Strict-Transport-Security")).toBeNull(); expect(res.headers.get("Content-Security-Policy")).toBeNull(); }); + test("NB_HSTS env var overrides option", async () => { + process.env.NB_HSTS = "max-age=42"; + const res = await createTestApp({ hsts: "max-age=60" }).request("/test"); + expect(res.headers.get("Strict-Transport-Security")).toBe("max-age=42"); + }); + + test("NB_CSP env var overrides option", async () => { + process.env.NB_CSP = "default-src https:"; + const res = await createTestApp({ csp: "default-src 'self'" }).request("/test"); + expect(res.headers.get("Content-Security-Policy")).toBe("default-src https:"); + }); + + test("NB_HSTS='' env var disables HSTS even when option is set", async () => { + process.env.NB_HSTS = ""; + const res = await createTestApp({ hsts: "max-age=60" }).request("/test"); + expect(res.headers.get("Strict-Transport-Security")).toBeNull(); + }); + test("sets headers on POST responses too", async () => { - const res = await app.request("/test", { method: "POST" }); + const res = await createTestApp().request("/test", { method: "POST" }); expect(res.headers.get("X-Content-Type-Options")).toBe("nosniff"); expect(res.headers.get("X-Frame-Options")).toBe("DENY"); + expect(res.headers.get("Strict-Transport-Security")).toBe(DEFAULT_HSTS); }); test("sets headers on 404 responses", async () => { - const res = await app.request("/nonexistent"); + const res = await createTestApp().request("/nonexistent"); expect(res.headers.get("X-Content-Type-Options")).toBe("nosniff"); expect(res.headers.get("X-Frame-Options")).toBe("DENY"); + expect(res.headers.get("Content-Security-Policy")).toBe(DEFAULT_CSP); }); }); From 9ce48091407d204dbab5b22f9690e841df1dee9f Mon Sep 17 00:00:00 2001 From: Mathew Goldsborough <1759329+mgoldsborough@users.noreply.github.com> Date: Mon, 4 May 2026 16:47:43 -1000 Subject: [PATCH 2/2] test(security-headers): cover SKIP_DEFAULTS opt-out and route override - Strip `x-nb-skip-security-defaults` from upstream proxy responses so a bundle dev server can't disable platform HSTS/CSP for unrelated routes. - Add unit tests for the SKIP_DEFAULTS_HEADER opt-out path (HSTS/CSP skipped, header stripped from egress, other defaults still applied) and route-level HSTS/CSP override preservation. - CHANGELOG entry under Changed for the new defaults + NB_HSTS/NB_CSP override env vars. --- CHANGELOG.md | 1 + src/api/routes/proxy.ts | 4 ++++ test/unit/api/security-headers.test.ts | 29 ++++++++++++++++++++++++++ 3 files changed, 34 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19af97da..0815e3b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ - `manage_workspaces.list` now returns the requesting user's role within each workspace (`userRole?: "admin" | "member"`) so the web client can gate workspace-admin UI without an extra `list_members` round-trip. - `POST /v1/chat` validation error wording. Hand-rolled checks (`"metadata must be a JSON object"`, `"allowedTools must be an array of strings"`) replaced with TypeBox path-prefixed errors (`"/metadata: Expected object"`, `"/allowedTools: Expected array"`). HTTP status (400) and `error: "bad_request"` are unchanged. External callers asserting on the exact wording need to update; the field name still appears in the message. - Skill manifest writes now always include `metadata.keywords` and `metadata.triggers` as arrays (defaulting to `[]` when omitted by the caller). Previously a partial `metadata: { category: "X" }` could write a manifest with no `keywords`/`triggers` keys at all; the loader's domain type required them, so the divergence was a latent type lie. The on-disk JSON shape is now what the type always claimed. +- API responses now carry HSTS (`max-age=31536000; includeSubDomains`) and CSP (`default-src 'none'; frame-ancestors 'none'; base-uri 'none'`) by default so direct-exposure self-hosted deployments aren't naked. Operators terminating TLS at a reverse proxy that already emits these can disable via `NB_HSTS=""` / `NB_CSP=""`, or override to a custom value via env var or middleware option ([#20](https://github.com/NimbleBrainInc/nimblebrain/pull/20)). ### Fixed diff --git a/src/api/routes/proxy.ts b/src/api/routes/proxy.ts index ef0234f5..bfbccc6d 100644 --- a/src/api/routes/proxy.ts +++ b/src/api/routes/proxy.ts @@ -223,6 +223,9 @@ const REQUEST_HEADERS_STRIPPED = new Set([ * * - Set-Cookie / Set-Cookie2: see top-of-file trust model. * - X-Frame-Options / CSP: replaced with our own SAMEORIGIN. + * - X-NB-Skip-Security-Defaults: internal signal — this route sets it + * deliberately below. Stripping upstream copies prevents a bundle dev + * server from disabling platform HSTS/CSP for unrelated responses. */ const RESPONSE_HEADERS_STRIPPED = new Set([ "set-cookie", @@ -230,6 +233,7 @@ const RESPONSE_HEADERS_STRIPPED = new Set([ "x-frame-options", "content-security-policy", "content-security-policy-report-only", + "x-nb-skip-security-defaults", ]); const REQUEST_HAS_BODY = new Set(["POST", "PUT", "PATCH", "DELETE"]); diff --git a/test/unit/api/security-headers.test.ts b/test/unit/api/security-headers.test.ts index d8d6dcb6..ce5008aa 100644 --- a/test/unit/api/security-headers.test.ts +++ b/test/unit/api/security-headers.test.ts @@ -3,6 +3,7 @@ import { Hono } from "hono"; import { DEFAULT_CSP, DEFAULT_HSTS, + SKIP_DEFAULTS_HEADER, securityHeaders, } from "../../../src/api/middleware/security-headers.ts"; @@ -97,6 +98,34 @@ describe("securityHeaders middleware", () => { expect(res.headers.get("Strict-Transport-Security")).toBeNull(); }); + test(`${SKIP_DEFAULTS_HEADER} opts route out of HSTS/CSP and is stripped from egress`, async () => { + const app = new Hono(); + app.use("*", securityHeaders()); + app.get("/proxied", (c) => { + c.header(SKIP_DEFAULTS_HEADER, "1"); + return c.text("ok"); + }); + const res = await app.request("/proxied"); + expect(res.headers.get("Strict-Transport-Security")).toBeNull(); + expect(res.headers.get("Content-Security-Policy")).toBeNull(); + expect(res.headers.get(SKIP_DEFAULTS_HEADER)).toBeNull(); + // Other defaults still apply — the opt-out is HSTS/CSP-specific. + expect(res.headers.get("X-Content-Type-Options")).toBe("nosniff"); + }); + + test("preserves route-level HSTS/CSP overrides", async () => { + const app = new Hono(); + app.use("*", securityHeaders()); + app.get("/custom", (c) => { + c.header("Strict-Transport-Security", "max-age=600"); + c.header("Content-Security-Policy", "default-src 'self'"); + return c.text("ok"); + }); + const res = await app.request("/custom"); + expect(res.headers.get("Strict-Transport-Security")).toBe("max-age=600"); + expect(res.headers.get("Content-Security-Policy")).toBe("default-src 'self'"); + }); + test("sets headers on POST responses too", async () => { const res = await createTestApp().request("/test", { method: "POST" }); expect(res.headers.get("X-Content-Type-Options")).toBe("nosniff");