From 5a7bb76ff1c337546b919eaaafee29a02ee19d48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Sun, 24 May 2026 11:37:20 +0200 Subject: [PATCH 1/3] fix(journal): fail loud in production when secrets are unset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `requireSecret(name, devFallback)` in lib/config.server.ts and `getDatabaseUrl()` in @trails-cool/db. Both: - return the env var when set, - fall back to the dev default in non-production, - throw at boot in production if the env var is missing OR matches the known dev fallback (which would otherwise silently ship a public secret / point at localhost). Applied to: - JWT_SECRET (lib/jwt.server.ts) — was `?? "dev-jwt-secret-change-in-production"` - SESSION_SECRET (lib/auth/session.server.ts) — was `?? "dev-secret-change-in-production"` - DATABASE_URL (server.ts health + boss; packages/db migrate-data) — was `?? "postgres://trails:trails@localhost:5432/trails"` Why: these strings are in the repo and known to attackers. A misconfigured prod deploy that forgot to set them would either run with guessable signing keys (full session/JWT forgery) or connect to a non-existent localhost DB. Better to refuse to start than to silently operate insecurely. Tests: - packages/db/src/get-database-url.test.ts (5 cases) - lib/config.server.test.ts gains `requireSecret` cases (4 new) Full repo: pnpm typecheck, pnpm lint, pnpm test all green. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/journal/app/lib/auth/session.server.ts | 3 +- apps/journal/app/lib/config.server.test.ts | 35 ++++++++++++++++ apps/journal/app/lib/config.server.ts | 25 ++++++++++++ apps/journal/app/lib/jwt.server.ts | 4 +- apps/journal/server.ts | 5 ++- packages/db/src/get-database-url.test.ts | 44 +++++++++++++++++++++ packages/db/src/index.ts | 28 +++++++++++-- packages/db/src/migrate-data.ts | 4 +- 8 files changed, 138 insertions(+), 10 deletions(-) create mode 100644 packages/db/src/get-database-url.test.ts diff --git a/apps/journal/app/lib/auth/session.server.ts b/apps/journal/app/lib/auth/session.server.ts index 2230bee..5c6edbd 100644 --- a/apps/journal/app/lib/auth/session.server.ts +++ b/apps/journal/app/lib/auth/session.server.ts @@ -9,8 +9,9 @@ import { createCookieSessionStorage, redirect } from "react-router"; import { eq } from "drizzle-orm"; import { users } from "@trails-cool/db/schema/journal"; import { getDb } from "../db.ts"; +import { requireSecret } from "../config.server.ts"; -const sessionSecret = process.env.SESSION_SECRET ?? "dev-secret-change-in-production"; +const sessionSecret = requireSecret("SESSION_SECRET", "dev-secret-change-in-production"); export const sessionStorage = createCookieSessionStorage({ cookie: { diff --git a/apps/journal/app/lib/config.server.test.ts b/apps/journal/app/lib/config.server.test.ts index 7c3993f..37b9d11 100644 --- a/apps/journal/app/lib/config.server.test.ts +++ b/apps/journal/app/lib/config.server.test.ts @@ -17,3 +17,38 @@ describe("getOrigin", () => { expect(getOrigin()).toBe("http://localhost:3000"); }); }); + +describe("requireSecret", () => { + beforeEach(() => { + vi.resetModules(); + vi.unstubAllEnvs(); + }); + + it("returns the env value when set in any environment", async () => { + vi.stubEnv("NODE_ENV", "production"); + vi.stubEnv("MY_SECRET", "real-secret"); + const { requireSecret } = await import("./config.server.ts"); + expect(requireSecret("MY_SECRET", "dev-fallback")).toBe("real-secret"); + }); + + it("returns the dev fallback when unset in development", async () => { + vi.stubEnv("NODE_ENV", "development"); + delete process.env.MY_SECRET; + const { requireSecret } = await import("./config.server.ts"); + expect(requireSecret("MY_SECRET", "dev-fallback")).toBe("dev-fallback"); + }); + + it("throws in production when the secret is unset", async () => { + vi.stubEnv("NODE_ENV", "production"); + delete process.env.MY_SECRET; + const { requireSecret } = await import("./config.server.ts"); + expect(() => requireSecret("MY_SECRET", "dev-fallback")).toThrow(/MY_SECRET/); + }); + + it("throws in production when the secret matches the dev fallback", async () => { + vi.stubEnv("NODE_ENV", "production"); + vi.stubEnv("MY_SECRET", "dev-fallback"); + const { requireSecret } = await import("./config.server.ts"); + expect(() => requireSecret("MY_SECRET", "dev-fallback")).toThrow(/dev fallback/); + }); +}); diff --git a/apps/journal/app/lib/config.server.ts b/apps/journal/app/lib/config.server.ts index 3796b8e..df7c5ed 100644 --- a/apps/journal/app/lib/config.server.ts +++ b/apps/journal/app/lib/config.server.ts @@ -5,3 +5,28 @@ export function getOrigin(): string { return process.env.ORIGIN ?? "http://localhost:3000"; } + +/** + * Read a required secret from the environment. Returns the env value when + * set. In production, throws if the env var is missing or matches the + * known-public dev fallback — silently shipping a default secret to prod + * is a credential leak. In dev/test, returns the supplied fallback so the + * local loop keeps working without ceremony. + * + * Use this for any value where a leaked default would be a security + * incident: signing keys, session secrets, database credentials. + */ +export function requireSecret(name: string, devFallback: string): string { + const value = process.env[name]; + const isProd = process.env.NODE_ENV === "production"; + if (isProd) { + if (!value || value === devFallback) { + throw new Error( + `Refusing to start: ${name} is unset or matches the known-public dev fallback. ` + + `Set ${name} to a strong, unique value in production.`, + ); + } + return value; + } + return value ?? devFallback; +} diff --git a/apps/journal/app/lib/jwt.server.ts b/apps/journal/app/lib/jwt.server.ts index c49e79c..333b8fb 100644 --- a/apps/journal/app/lib/jwt.server.ts +++ b/apps/journal/app/lib/jwt.server.ts @@ -1,8 +1,8 @@ import { SignJWT, jwtVerify } from "jose"; -import { getOrigin } from "./config.server.ts"; +import { getOrigin, requireSecret } from "./config.server.ts"; const JWT_SECRET = new TextEncoder().encode( - process.env.JWT_SECRET ?? "dev-jwt-secret-change-in-production", + requireSecret("JWT_SECRET", "dev-jwt-secret-change-in-production"), ); const ISSUER = getOrigin(); diff --git a/apps/journal/server.ts b/apps/journal/server.ts index 05a50c2..5e4f980 100644 --- a/apps/journal/server.ts +++ b/apps/journal/server.ts @@ -7,6 +7,7 @@ import { join, extname, resolve } from "node:path"; import { logger } from "./app/lib/logger.server.ts"; import { httpRequestDuration, registry } from "./app/lib/metrics.server.ts"; import { createBoss, startWorker } from "@trails-cool/jobs"; +import { getDatabaseUrl } from "@trails-cool/db"; import postgres from "postgres"; Sentry.init({ @@ -66,7 +67,7 @@ async function handleMetrics(_req: IncomingMessage, res: ServerResponse): Promis const version = process.env.SENTRY_RELEASE ?? "dev"; async function handleHealth(_req: IncomingMessage, res: ServerResponse): Promise { - const client = postgres(process.env.DATABASE_URL ?? "postgres://trails:trails@localhost:5432/trails", { max: 1 }); + const client = postgres(getDatabaseUrl(), { max: 1 }); try { await client`SELECT 1`; res.writeHead(200, { "Content-Type": "application/json" }); @@ -149,7 +150,7 @@ server.listen(port, async () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any jobs.push(notificationsFanoutJob, notificationsPurgeJob, komootBulkImportJob as any, importBatchesSweepJob, sendWelcomeEmailJob); - const boss = createBoss(process.env.DATABASE_URL ?? "postgres://trails:trails@localhost:5432/trails"); + const boss = createBoss(getDatabaseUrl()); await startWorker(boss, jobs); // Register the started boss so feature code can enqueue jobs against // the same instance via getBoss() / enqueueOptional(). diff --git a/packages/db/src/get-database-url.test.ts b/packages/db/src/get-database-url.test.ts new file mode 100644 index 0000000..a456ed0 --- /dev/null +++ b/packages/db/src/get-database-url.test.ts @@ -0,0 +1,44 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const DEV = "postgres://trails:trails@localhost:5432/trails"; + +describe("getDatabaseUrl", () => { + beforeEach(() => { + vi.resetModules(); + vi.unstubAllEnvs(); + }); + + it("uses the override argument when provided", async () => { + vi.stubEnv("NODE_ENV", "production"); + const { getDatabaseUrl } = await import("./index.ts"); + expect(getDatabaseUrl("postgres://override/db")).toBe("postgres://override/db"); + }); + + it("returns DATABASE_URL when set", async () => { + vi.stubEnv("NODE_ENV", "production"); + vi.stubEnv("DATABASE_URL", "postgres://real-prod/db"); + const { getDatabaseUrl } = await import("./index.ts"); + expect(getDatabaseUrl()).toBe("postgres://real-prod/db"); + }); + + it("falls back to the dev URL in development", async () => { + vi.stubEnv("NODE_ENV", "development"); + delete process.env.DATABASE_URL; + const { getDatabaseUrl } = await import("./index.ts"); + expect(getDatabaseUrl()).toBe(DEV); + }); + + it("throws in production when DATABASE_URL is unset", async () => { + vi.stubEnv("NODE_ENV", "production"); + delete process.env.DATABASE_URL; + const { getDatabaseUrl } = await import("./index.ts"); + expect(() => getDatabaseUrl()).toThrow(/DATABASE_URL/); + }); + + it("throws in production when DATABASE_URL matches the dev default", async () => { + vi.stubEnv("NODE_ENV", "production"); + vi.stubEnv("DATABASE_URL", DEV); + const { getDatabaseUrl } = await import("./index.ts"); + expect(() => getDatabaseUrl()).toThrow(/dev default/); + }); +}); diff --git a/packages/db/src/index.ts b/packages/db/src/index.ts index 8141386..2c6ee2b 100644 --- a/packages/db/src/index.ts +++ b/packages/db/src/index.ts @@ -3,10 +3,32 @@ import postgres from "postgres"; import * as plannerSchema from "./schema/planner.ts"; import * as journalSchema from "./schema/journal.ts"; +const DEV_DB_URL = "postgres://trails:trails@localhost:5432/trails"; + +/** + * Resolve the database URL with fail-loud semantics in production. + * In dev/test we silently fall back to the local Compose URL so the + * loop keeps working; in prod we refuse to start rather than + * silently pointing at localhost (which either won't resolve, or + * worse, will connect to an unintended database on the host). + */ +export function getDatabaseUrl(override?: string): string { + if (override) return override; + const url = process.env.DATABASE_URL; + if (process.env.NODE_ENV === "production") { + if (!url || url === DEV_DB_URL) { + throw new Error( + "Refusing to start: DATABASE_URL is unset or matches the dev default. " + + "Set DATABASE_URL to the production connection string.", + ); + } + return url; + } + return url ?? DEV_DB_URL; +} + export function createDb(connectionString?: string) { - const client = postgres( - connectionString ?? process.env.DATABASE_URL ?? "postgres://trails:trails@localhost:5432/trails", - ); + const client = postgres(getDatabaseUrl(connectionString)); return drizzle(client, { schema: { ...plannerSchema, ...journalSchema }, }); diff --git a/packages/db/src/migrate-data.ts b/packages/db/src/migrate-data.ts index d8299ec..b482fe5 100644 --- a/packages/db/src/migrate-data.ts +++ b/packages/db/src/migrate-data.ts @@ -10,13 +10,13 @@ import { readdirSync, readFileSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; import postgres from "postgres"; +import { getDatabaseUrl } from "./index.ts"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const migrationsDir = path.resolve(__dirname, "..", "migrations"); async function main() { - const url = process.env.DATABASE_URL ?? "postgres://trails:trails@localhost:5432/trails"; - const sql = postgres(url); + const sql = postgres(getDatabaseUrl()); try { const files = readdirSync(migrationsDir) .filter((f) => f.endsWith(".sql")) From 9d48d26a6e1c12c37e15e1beb56a8a6c161ffcf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Sun, 24 May 2026 11:40:24 +0200 Subject: [PATCH 2/3] ci: supply throwaway JWT/SESSION secrets for E2E (NODE_ENV=production) pnpm test:e2e runs via react-router serve which boots with NODE_ENV=production; the new requireSecret() guard refuses to start without explicit values. CI now supplies CI-only throwaway secrets so the guard still bites in real prod deploys without breaking the test job. --- .github/workflows/ci.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1237944..1e8edee 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -257,6 +257,12 @@ jobs: BROUTER_URL: http://localhost:17777 E2E: "true" INTEGRATION_SECRET: ${{ secrets.INTEGRATION_SECRET }} + # pnpm test:e2e starts the server via `react-router serve`, which + # boots with NODE_ENV=production. requireSecret() refuses to start + # in production without these set — supply random throwaway values + # for CI so the fail-loud guard still bites in real prod deploys. + JWT_SECRET: ci-jwt-secret-only-for-e2e-do-not-reuse + SESSION_SECRET: ci-session-secret-only-for-e2e-do-not-reuse - name: Playwright job summary if: ${{ !cancelled() }} From ebedfa257b250dc2f22b050e509f53cc3998e7f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Sun, 24 May 2026 11:45:09 +0200 Subject: [PATCH 3/3] fix: add E2E opt-out for fail-loud secret/DB-URL guards Playwright runs the server via `react-router serve` with NODE_ENV=production but against a local dev Postgres and local cookie secrets. The guards added in 5a7bb76 refused to start under that configuration. `E2E=true` (already set by the CI E2E job) is now the explicit opt-out: in real production this env var is never set, so the guard still bites. --- .github/workflows/ci.yml | 10 ++++------ apps/journal/app/lib/config.server.ts | 5 ++++- packages/db/src/index.ts | 6 +++++- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1e8edee..5c5ae58 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -255,14 +255,12 @@ jobs: run: pnpm test:e2e env: BROUTER_URL: http://localhost:17777 + # E2E=true is the explicit opt-out from the fail-loud + # requireSecret() / getDatabaseUrl() guards — playwright boots + # the server via `react-router serve` (NODE_ENV=production) but + # against the local dev Postgres + local cookie secrets. E2E: "true" INTEGRATION_SECRET: ${{ secrets.INTEGRATION_SECRET }} - # pnpm test:e2e starts the server via `react-router serve`, which - # boots with NODE_ENV=production. requireSecret() refuses to start - # in production without these set — supply random throwaway values - # for CI so the fail-loud guard still bites in real prod deploys. - JWT_SECRET: ci-jwt-secret-only-for-e2e-do-not-reuse - SESSION_SECRET: ci-session-secret-only-for-e2e-do-not-reuse - name: Playwright job summary if: ${{ !cancelled() }} diff --git a/apps/journal/app/lib/config.server.ts b/apps/journal/app/lib/config.server.ts index df7c5ed..c4222bc 100644 --- a/apps/journal/app/lib/config.server.ts +++ b/apps/journal/app/lib/config.server.ts @@ -18,7 +18,10 @@ export function getOrigin(): string { */ export function requireSecret(name: string, devFallback: string): string { const value = process.env[name]; - const isProd = process.env.NODE_ENV === "production"; + // Playwright runs `react-router serve` (NODE_ENV=production) against a + // local stack. E2E=true is the explicit opt-out so the guard still + // bites in real prod deploys. + const isProd = process.env.NODE_ENV === "production" && process.env.E2E !== "true"; if (isProd) { if (!value || value === devFallback) { throw new Error( diff --git a/packages/db/src/index.ts b/packages/db/src/index.ts index 2c6ee2b..bac4b53 100644 --- a/packages/db/src/index.ts +++ b/packages/db/src/index.ts @@ -15,7 +15,11 @@ const DEV_DB_URL = "postgres://trails:trails@localhost:5432/trails"; export function getDatabaseUrl(override?: string): string { if (override) return override; const url = process.env.DATABASE_URL; - if (process.env.NODE_ENV === "production") { + // Playwright runs `react-router serve` which boots with + // NODE_ENV=production, but the CI E2E suite legitimately points at a + // local Postgres using the dev URL. E2E=true is the explicit opt-out. + const isProd = process.env.NODE_ENV === "production" && process.env.E2E !== "true"; + if (isProd) { if (!url || url === DEV_DB_URL) { throw new Error( "Refusing to start: DATABASE_URL is unset or matches the dev default. " +