From 7629d5d7b1704373ec7b833d784a80be8bb50faa Mon Sep 17 00:00:00 2001 From: Julian Cuni Date: Sun, 21 Jun 2026 23:50:40 +0200 Subject: [PATCH] fix(auth): make the Secure cookie flag fail-safe (default on) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit secureCookies() keyed off NODE_ENV === "production", so an appliance deployed without that var silently sent the auth + CSRF cookies WITHOUT the Secure flag — the review's one Medium finding. Now Secure is the DEFAULT and you only ever opt OUT: a misconfigured/forgotten env can only make cookies more restrictive, never drop the flag. Dropped only on a deliberate COOKIE_SECURE=0/false/no/off (or an explicit NODE_ENV=development as a dev fallback). The LAN appliance that serves the SPA over plain http sets COOKIE_SECURE=0 on purpose (a Secure cookie would never be sent over its http origin and would lock operators out); a TLS deploy leaves it unset and gets Secure. - auth.test.ts (5): pins the matrix — default Secure, production Secure, dev opt-out, COOKIE_SECURE falsey opts out, any other value opts in. - .env.example documents COOKIE_SECURE (replaces the stale NODE_ENV cookie note). - dev .env sets COOKIE_SECURE=0 (local http://localhost login keeps working). server 80/80; build+lint green. Claude-Session: https://claude.ai/code/session_01Xcm6ikLgGoCxxHrxtjkk5V --- apps/server/.env.example | 8 +++++- apps/server/src/auth.test.ts | 56 ++++++++++++++++++++++++++++++++++++ apps/server/src/auth.ts | 25 ++++++++++++++-- 3 files changed, 85 insertions(+), 4 deletions(-) create mode 100644 apps/server/src/auth.test.ts diff --git a/apps/server/.env.example b/apps/server/.env.example index 58a5ca2..32eed4c 100644 --- a/apps/server/.env.example +++ b/apps/server/.env.example @@ -20,7 +20,13 @@ EVENT_SIGNING_KEY= # HOST=0.0.0.0 # interface to bind. 127.0.0.1 = loopback only. # LOG_LEVEL=info # DATABASE_URL=./parking.sqlite -# NODE_ENV=production # set in prod: makes auth cookies Secure (HTTPS-only) +# +# Auth-cookie Secure flag. FAIL-SAFE: cookies are Secure (HTTPS-only) BY DEFAULT — +# you only ever opt OUT, never in. Set COOKIE_SECURE=0 for a plain-HTTP deployment +# (e.g. the LAN appliance serving the SPA same-origin over http, where a Secure +# cookie would never be sent and would lock operators out). Local dev over +# http://localhost MUST set this (the dev .env does). Leave unset in any TLS deploy. +# COOKIE_SECURE=0 # First admin (seed once): pnpm --filter @parking/server seed-admin # ADMIN_USER=admin diff --git a/apps/server/src/auth.test.ts b/apps/server/src/auth.test.ts new file mode 100644 index 0000000..ddb3fe5 --- /dev/null +++ b/apps/server/src/auth.test.ts @@ -0,0 +1,56 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { secureCookies } from "./auth.js"; + +// The auth/CSRF cookies' Secure flag must be FAIL-SAFE: Secure by default, dropped only +// on a deliberate opt-out. The old behaviour (Secure iff NODE_ENV==="production") leaked +// cookies over plain HTTP on an appliance that forgot to set NODE_ENV — this pins the +// corrected matrix. + +let savedCookieSecure: string | undefined; +let savedNodeEnv: string | undefined; + +beforeEach(() => { + savedCookieSecure = process.env.COOKIE_SECURE; + savedNodeEnv = process.env.NODE_ENV; + delete process.env.COOKIE_SECURE; + delete process.env.NODE_ENV; +}); +afterEach(() => { + restore("COOKIE_SECURE", savedCookieSecure); + restore("NODE_ENV", savedNodeEnv); +}); +function restore(key: string, val: string | undefined) { + if (val === undefined) delete process.env[key]; + else process.env[key] = val; +} + +describe("secureCookies — fail-safe Secure flag", () => { + it("defaults to Secure when nothing is set (the appliance-forgot-NODE_ENV case)", () => { + expect(secureCookies()).toBe(true); + }); + + it("stays Secure in production", () => { + process.env.NODE_ENV = "production"; + expect(secureCookies()).toBe(true); + }); + + it("drops Secure only for an explicit local-dev NODE_ENV", () => { + process.env.NODE_ENV = "development"; + expect(secureCookies()).toBe(false); + }); + + it("COOKIE_SECURE override wins: falsey values opt OUT", () => { + for (const v of ["0", "false", "no", "off", "FALSE", " Off "]) { + process.env.COOKIE_SECURE = v; + expect(secureCookies(), `COOKIE_SECURE=${JSON.stringify(v)}`).toBe(false); + } + }); + + it("COOKIE_SECURE override wins: any other value opts IN (even in dev)", () => { + process.env.NODE_ENV = "development"; + for (const v of ["1", "true", "yes", "on", ""]) { + process.env.COOKIE_SECURE = v; + expect(secureCookies(), `COOKIE_SECURE=${JSON.stringify(v)}`).toBe(true); + } + }); +}); diff --git a/apps/server/src/auth.ts b/apps/server/src/auth.ts index 1cba861..84c0008 100644 --- a/apps/server/src/auth.ts +++ b/apps/server/src/auth.ts @@ -50,9 +50,28 @@ export function requireJwtSecret(): string { return secret; } -/** Cookies are secure in production; relaxed for local http dev. */ -function secureCookies(): boolean { - return process.env.NODE_ENV === "production"; +/** + * Whether to set the `Secure` flag on the auth/CSRF cookies. FAIL-SAFE: default is + * `true` (Secure) — a misconfigured/forgotten env can only ever make cookies MORE + * restrictive, never silently drop the flag. + * + * The previous gate keyed off `NODE_ENV === "production"`, which meant an appliance + * deployed without that var leaked cookies over plain HTTP. Now `Secure` is the + * default and is dropped ONLY for an explicit, deliberate opt-out — `COOKIE_SECURE` + * set to a falsey value (`0/false/no/off`), or the legacy `NODE_ENV !== production` + * signal kept as a fallback so existing dev setups still work over http://localhost. + * + * The parking appliance often serves the SPA same-origin over the LAN with no TLS; + * THAT box sets `COOKIE_SECURE=0` on purpose (a Secure cookie would never be sent + * over its http origin and would lock operators out). Everything else stays secure. + */ +export function secureCookies(): boolean { + const override = process.env.COOKIE_SECURE; + if (override !== undefined) { + return !/^(0|false|no|off)$/i.test(override.trim()); + } + // No explicit override: secure unless this is an obvious local-dev run. + return process.env.NODE_ENV !== "development"; } export function newCsrfToken(): string {