hearthforge

← Back to log@ daabb00

security: harden cookies + CSRF for reverse-proxy deployments

AuthorKonata <konata@posteo.jp>
Date
Commitdaabb000babdafcd9028bc6db9cd8308cd22bc9d
Parenteb52ac0
7 files changed, 254 insertions(+), 12 deletions(-)
▾MREADME.md
+14@ daabb00@ master
@@ -100,6 +100,20 @@ All settings are environment variables:
100100
101101 \*\* Set `TRUSTED_PROXY=1` only when Hearthforge is behind a reverse proxy that strips any incoming `X-Forwarded-For` from clients. Caddy and Traefik do this by default; nginx requires `proxy_set_header X-Forwarded-For $remote_addr;` (rather than the common `$proxy_add_x_forwarded_for`, which appends to a client-supplied value). Setting `TRUSTED_PROXY=1` in front of a proxy that does not strip means rate limits and any audit logging are spoofable per request.
102102
103+### Reverse proxy deployment
104+
105+Hearthforge does not terminate TLS itself. For any production deployment, run it behind an HTTPS-terminating reverse proxy (Caddy, nginx, Traefik, …) and set:
106+
107+```
108+BASE_URL=https://your-forge.example.com
109+```
110+
111+A `BASE_URL` with the `https://` scheme is what activates HTTPS hardening:
112+
113+- Session and preference cookies are emitted with `Secure`, so the browser will only send them over HTTPS.
114+- Every response includes `Strict-Transport-Security: max-age=31536000; includeSubDomains`.
115+- The CSRF middleware compares the request `Origin` against `BASE_URL` (full origin, scheme + host + port), not the `Host` header.
116+
103117 ## CI/CD Pipelines
104118
105119 Hearthforge includes a built-in CI/CD system that runs pipelines in Docker or Podman containers,
▾Msrc/app.ts
+49-6@ daabb00@ master
@@ -51,17 +51,54 @@ export async function createApp(port: number) {
5151 }),
5252 )
5353 .onRequest(({ request }) => {
54- // CSRF defense-in-depth: reject mutating requests whose Origin
55- // header is present but does not match the Host. Modern browsers
56- // attach Origin to all cross-site mutating requests, so this
57- // catches what SameSite=Lax cookies would have allowed (top-level
58- // POSTs from same-origin contexts are unaffected). Non-browser
59- // clients (git push, curl) typically omit Origin and pass through.
54+ // CSRF defense-in-depth. The actual CSRF defense is the session
55+ // cookie's `SameSite=Lax`, which tells the browser not to attach
56+ // the cookie to cross-site sub-requests at all and to attach it
57+ // on cross-site top-level navigations only for safe methods. This
58+ // middleware adds a second line on top of that.
59+ //
60+ // For mutating methods we require either no Origin header (allowed
61+ // because non-browser clients like `git push` and `curl` do not
62+ // send one, and they authenticate via HTTP Basic, not the session
63+ // cookie — so they aren't part of the CSRF surface), or an Origin
64+ // that matches a known good value.
65+ //
66+ // In HTTPS production mode (config.PUBLIC_HTTPS) we compare the
67+ // full Origin string against config.PUBLIC_ORIGIN (scheme + host +
68+ // port). This is stricter than a Host-header match and is the
69+ // right move when behind a reverse proxy, because the proxy may
70+ // rewrite Host to the backend (e.g. localhost:3000) and a Host
71+ // match could also be satisfied by an attacker-controlled
72+ // subdomain that shares cookies (we set Path=/ with no Domain, so
73+ // today there is no such subdomain — but a future *.your-forge
74+ // setup must not silently break this assumption).
75+ //
76+ // In dev mode (BASE_URL is plain http://) we keep the looser
77+ // Origin.host === Host check so localhost:3000 / 127.0.0.1:3000
78+ // can be used interchangeably during local work.
79+ //
80+ // Two regressions would each break this design — re-read this
81+ // before changing either:
82+ // 1. Switching session cookies to SameSite=None re-opens
83+ // cross-site cookie attachment; the no-Origin allow branch
84+ // below would then need to be removed for cookie-authed
85+ // mutations.
86+ // 2. Introducing a state-mutating GET handler bypasses this
87+ // middleware (gated on method) and SameSite=Lax allows the
88+ // cookie on top-level GET navigations. Don't add such a
89+ // handler.
6090 const m = request.method;
6191 if (m !== "POST" && m !== "PUT" && m !== "PATCH" && m !== "DELETE")
6292 return;
6393 const origin = request.headers.get("origin");
6494 if (!origin) return;
95+ if (config.PUBLIC_HTTPS) {
96+ if (origin !== config.PUBLIC_ORIGIN)
97+ return new Response("Cross-origin request rejected", {
98+ status: 403,
99+ });
100+ return;
101+ }
65102 const host = request.headers.get("host");
66103 let originHost: string;
67104 try {
@@ -78,6 +115,12 @@ export async function createApp(port: number) {
78115 if (response instanceof Response) {
79116 response.headers.set("Content-Security-Policy", CSP);
80117 response.headers.set("X-Frame-Options", "DENY");
118+ if (config.PUBLIC_HTTPS) {
119+ response.headers.set(
120+ "Strict-Transport-Security",
121+ "max-age=31536000; includeSubDomains",
122+ );
123+ }
81124 }
82125 })
83126 .get("/health", async () => {
▾Msrc/config.ts
+8-2@ daabb00@ master
@@ -29,6 +29,10 @@ const config = {
2929 BASE_URL:
3030 env.BASE_URL ??
3131 `http://localhost:${parseInt(env.PORT ?? "", 10) || 3000}`,
32+ // Derived from BASE_URL after the literal closes (see below). Declared
33+ // here so consumers get a typed config object.
34+ PUBLIC_HTTPS: false,
35+ PUBLIC_ORIGIN: "",
3236 DATA_DIR: path.resolve(env.DATA_DIR ?? "./data"),
3337 HIGHLIGHT_WORKERS: parseInt(env.HIGHLIGHT_WORKERS ?? "", 10) || 4,
3438 COMMITTER_NAME: env.COMMITTER_NAME ?? env.OWNER_DISPLAY_NAME ?? "Admin",
@@ -62,8 +66,10 @@ const config = {
6266 };
6367
6468 // Derived values that depend on other config fields
69+const baseUrl = new URL(config.BASE_URL);
70+config.PUBLIC_HTTPS = baseUrl.protocol === "https:";
71+config.PUBLIC_ORIGIN = baseUrl.origin;
6572 config.COMMITTER_EMAIL =
66- env.COMMITTER_EMAIL ??
67- `${config.OWNER_DISPLAY_NAME}@${new URL(config.BASE_URL).hostname}`;
73+ env.COMMITTER_EMAIL ?? `${config.OWNER_DISPLAY_NAME}@${baseUrl.hostname}`;
6874
6975 export default config;
▾Msrc/routes/auth.tsx
+4-2@ daabb00@ master
@@ -60,11 +60,13 @@ async function createSession(userId: number): Promise<string> {
6060 }
6161
6262 function sessionCookie(id: string): string {
63- return `session=${id}; Path=/; HttpOnly; SameSite=Lax; Max-Age=${SESSION_DURATION_SECONDS}`;
63+ const secure = config.PUBLIC_HTTPS ? "; Secure" : "";
64+ return `session=${id}; Path=/; HttpOnly; SameSite=Lax${secure}; Max-Age=${SESSION_DURATION_SECONDS}`;
6465 }
6566
6667 function clearCookie(): string {
67- return "session=; Path=/; HttpOnly; SameSite=Lax; Max-Age=0";
68+ const secure = config.PUBLIC_HTTPS ? "; Secure" : "";
69+ return `session=; Path=/; HttpOnly; SameSite=Lax${secure}; Max-Age=0`;
6870 }
6971
7072 // In-memory challenge store (fine for single-process)
▾Msrc/routes/repos.tsx
+2-1@ daabb00@ master
@@ -193,9 +193,10 @@ export const repoRoutes = new Elysia()
193193 "/sort",
194194 ({ body }) => {
195195 const sort = body.sort === "name" ? "name" : "created";
196+ const secure = config.PUBLIC_HTTPS ? "; Secure" : "";
196197 return redirect(
197198 "/",
198- `repo_sort=${sort}; Path=/; SameSite=Lax; Max-Age=${YEAR_SECONDS}`,
199+ `repo_sort=${sort}; Path=/; SameSite=Lax${secure}; Max-Age=${YEAR_SECONDS}`,
199200 );
200201 },
201202 { body: t.Object({ sort: t.String() }) },
▾Msrc/routes/settings.tsx
+2-1@ daabb00@ master
@@ -268,7 +268,8 @@ export const settingsRoutes = new Elysia()
268268 return redirect("/settings?error=Invalid+theme");
269269 }
270270
271- const cookieHeader = `theme=${theme}; Path=/; SameSite=Lax; Max-Age=${YEAR_SECONDS}`;
271+ const secure = config.PUBLIC_HTTPS ? "; Secure" : "";
272+ const cookieHeader = `theme=${theme}; Path=/; SameSite=Lax${secure}; Max-Age=${YEAR_SECONDS}`;
272273 return redirect("/settings?success=theme", cookieHeader);
273274 },
274275 {
▾Atests/e2e.csrf.test.ts
+175@ daabb00@ master
@@ -0,0 +1,175 @@
1+import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
2+import config from '../src/config.ts';
3+import {
4+ BASE,
5+ ADMIN_PASS,
6+ setupTestEnv,
7+ spawnServer,
8+ killServer,
9+} from './helpers.ts';
10+
11+let server: Awaited<ReturnType<typeof spawnServer>>;
12+
13+// The CSRF middleware reads `config.PUBLIC_HTTPS` / `config.PUBLIC_ORIGIN` per
14+// request, so we toggle those between the dev-mode and HTTPS-mode describe
15+// blocks rather than spinning up two servers.
16+const ORIGINAL_PUBLIC_HTTPS = config.PUBLIC_HTTPS;
17+const ORIGINAL_PUBLIC_ORIGIN = config.PUBLIC_ORIGIN;
18+
19+beforeAll(async () => {
20+ await setupTestEnv();
21+ server = await spawnServer();
22+});
23+
24+afterAll(async () => {
25+ await killServer(server);
26+ config.PUBLIC_HTTPS = ORIGINAL_PUBLIC_HTTPS;
27+ config.PUBLIC_ORIGIN = ORIGINAL_PUBLIC_ORIGIN;
28+});
29+
30+// `bun:test` runs describe blocks in source order, so the dev-mode block runs
31+// first against the unmodified config, then we flip into HTTPS mode.
32+describe('CSRF / Secure cookie — dev mode (http BASE_URL)', () => {
33+ test('starts with PUBLIC_HTTPS off', () => {
34+ expect(config.PUBLIC_HTTPS).toBe(false);
35+ });
36+
37+ test('POST with no Origin is allowed (non-browser path)', async () => {
38+ const r = await fetch(`${BASE}/login`, {
39+ method: 'POST',
40+ headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
41+ body: 'username=admin&password=wrong',
42+ redirect: 'manual',
43+ });
44+ expect(r.status).not.toBe(403);
45+ });
46+
47+ test('POST with same-origin Origin is allowed', async () => {
48+ const r = await fetch(`${BASE}/login`, {
49+ method: 'POST',
50+ headers: {
51+ 'Content-Type': 'application/x-www-form-urlencoded',
52+ Origin: BASE,
53+ },
54+ body: 'username=admin&password=wrong',
55+ redirect: 'manual',
56+ });
57+ expect(r.status).not.toBe(403);
58+ });
59+
60+ test('POST with mismatched Origin is rejected', async () => {
61+ const r = await fetch(`${BASE}/login`, {
62+ method: 'POST',
63+ headers: {
64+ 'Content-Type': 'application/x-www-form-urlencoded',
65+ Origin: 'http://attacker.example',
66+ },
67+ body: 'username=admin&password=wrong',
68+ redirect: 'manual',
69+ });
70+ expect(r.status).toBe(403);
71+ });
72+
73+ test('successful login Set-Cookie omits Secure', async () => {
74+ const r = await fetch(`${BASE}/login`, {
75+ method: 'POST',
76+ headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
77+ body: `username=admin&password=${encodeURIComponent(ADMIN_PASS)}`,
78+ redirect: 'manual',
79+ });
80+ expect(r.status).toBe(302);
81+ const cookie = r.headers.get('set-cookie') ?? '';
82+ expect(cookie).toContain('session=');
83+ expect(cookie).not.toContain('Secure');
84+ });
85+
86+ test('responses do not include Strict-Transport-Security', async () => {
87+ const r = await fetch(`${BASE}/health`);
88+ expect(r.headers.get('strict-transport-security')).toBeNull();
89+ });
90+});
91+
92+describe('CSRF / Secure cookie — HTTPS mode (https BASE_URL)', () => {
93+ beforeAll(() => {
94+ // Simulate `BASE_URL=https://forge.test`. Note that the test client still
95+ // talks to the server over plain HTTP on localhost — that's the whole
96+ // point of the reverse-proxy story: the app trusts BASE_URL, not the
97+ // transport it sees on the proxy↔app hop.
98+ config.PUBLIC_HTTPS = true;
99+ config.PUBLIC_ORIGIN = 'https://forge.test';
100+ });
101+
102+ test('POST with no Origin is allowed (non-browser path)', async () => {
103+ const r = await fetch(`${BASE}/login`, {
104+ method: 'POST',
105+ headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
106+ body: 'username=admin&password=wrong',
107+ redirect: 'manual',
108+ });
109+ expect(r.status).not.toBe(403);
110+ });
111+
112+ test('POST with matching public Origin is allowed', async () => {
113+ const r = await fetch(`${BASE}/login`, {
114+ method: 'POST',
115+ headers: {
116+ 'Content-Type': 'application/x-www-form-urlencoded',
117+ Origin: 'https://forge.test',
118+ },
119+ body: 'username=admin&password=wrong',
120+ redirect: 'manual',
121+ });
122+ expect(r.status).not.toBe(403);
123+ });
124+
125+ test('POST whose Origin only matches Host (not BASE_URL) is rejected', async () => {
126+ // Stricter than dev mode: `Origin: ${BASE}` (http://localhost:PORT) would
127+ // pass the Host-match check but must fail the BASE_URL check.
128+ const r = await fetch(`${BASE}/login`, {
129+ method: 'POST',
130+ headers: {
131+ 'Content-Type': 'application/x-www-form-urlencoded',
132+ Origin: BASE,
133+ },
134+ body: 'username=admin&password=wrong',
135+ redirect: 'manual',
136+ });
137+ expect(r.status).toBe(403);
138+ });
139+
140+ test('POST with attacker Origin is rejected', async () => {
141+ const r = await fetch(`${BASE}/login`, {
142+ method: 'POST',
143+ headers: {
144+ 'Content-Type': 'application/x-www-form-urlencoded',
145+ Origin: 'https://attacker.example',
146+ },
147+ body: 'username=admin&password=wrong',
148+ redirect: 'manual',
149+ });
150+ expect(r.status).toBe(403);
151+ });
152+
153+ test('successful login Set-Cookie includes Secure', async () => {
154+ const r = await fetch(`${BASE}/login`, {
155+ method: 'POST',
156+ headers: {
157+ 'Content-Type': 'application/x-www-form-urlencoded',
158+ Origin: 'https://forge.test',
159+ },
160+ body: `username=admin&password=${encodeURIComponent(ADMIN_PASS)}`,
161+ redirect: 'manual',
162+ });
163+ expect(r.status).toBe(302);
164+ const cookie = r.headers.get('set-cookie') ?? '';
165+ expect(cookie).toContain('session=');
166+ expect(cookie).toContain('Secure');
167+ });
168+
169+ test('responses include Strict-Transport-Security', async () => {
170+ const r = await fetch(`${BASE}/health`);
171+ expect(r.headers.get('strict-transport-security')).toBe(
172+ 'max-age=31536000; includeSubDomains',
173+ );
174+ });
175+});