Commit4127ecfunknown_key
fix(security+ux): CSRF same-origin defence + theme/date consistency
fix(security+ux): CSRF same-origin defence + theme/date consistency Three parallel audits (security / dead-code / UX) surfaced a small number of concrete bugs alongside ~25 false positives. The verified issues: SECURITY - middleware/csrf.ts — the global `csrfProtect` middleware required a matching `_csrf` token on every state-changing request, but ~40+ web-UI forms across the codebase don't pass one. In production this meant every authenticated form submission returned `403 CSRF token invalid` — settings, issues, PRs, repo create, fork, sponsor, 2FA, etc. all broken. There was zero test coverage of CSRF so the regression had never been caught. Fix: layered defence following OWASP guidance. Primary check is now Origin/Referer same-origin verification (every modern browser sends Origin on cross-origin POSTs, and a forged cross-site request always carries the attacker's Origin). The legacy double-submit cookie token is kept as a fallback for clients that strip both headers. A request is accepted iff EITHER check passes. No form code had to change. - middleware/rate-limit.ts — the test-env bypass was gated on `NODE_ENV === "test" || BUN_ENV === "test"`. If a misconfigured production container leaks one of those (common in container reuse), rate limits would silently turn off — exposing copilot completions, git, and auth endpoints to brute force. Hardened with a `NODE_ENV === "production"` guard that refuses to honour the bypass in prod regardless of other flags. - src/__tests__/csrf.test.ts (new) — 17 tests covering: method gating (GET/HEAD/OPTIONS), skip paths (anonymous, /api/*, Bearer), the Origin/Referer same-origin defence (matching host accepts; attacker origin rejects; malformed origin rejects; missing both with no cookie rejects), the double-submit token fallback (matching header accepts; mismatched rejects; matching form-body accepts), and the cookie setter (first request sets cookie; subsequent requests don't overwrite). UX consistency - routes/gates.tsx — replaced hard-coded `#bc8cff` (off-theme purple) on the "Repaired" stat with `var(--accent)`. - routes/workflows.tsx, routes/packages.tsx, views/log-tail.tsx — workflow logs and npm install snippets used hard-coded `#0b0d0f` background + `#c7ccd1` text. In light-mode users got unreadable white-on-white. Now `var(--bg-tertiary)` + `var(--text)`. - routes/import.tsx — bulk-import callout used hard-coded `#3fb950` / `#f0b429` border colors instead of `var(--green)` / `var(--yellow)`. - routes/mirrors.tsx, routes/migrations.tsx, routes/dep-updater.tsx — these three pages rendered timestamps via `.toLocaleString()` while every other page on the platform uses `formatRelative()`. Standardised on `formatRelative()` so users see "2 hours ago" everywhere. Verified false positives from the audits (no action taken): - "Repo-settings template/archive/transfer authorisation gap" — the endpoints DO check `owner.id !== user.id` after `requireRepoAccess`, see repo-settings.tsx:306,347,422,458. - "Path traversal in git getBlob/getTree" — git's tree resolver is content-addressed and cannot escape the tree object, regardless of `..` sequences in user paths. - "Bearer header case-sensitivity" — `slice(7)` is correct for any case variant of "Bearer " since the prefix is always 7 chars. - "Webhook secrets stored plaintext" — webhook secrets must be retrievable to sign outgoing requests; hashing them at rest is incompatible with the HMAC-sign-outbound design. Post-change: bun test → 1273 pass / 0 fail (was 1256). bunx tsc → 0 errors. https://claude.ai/code/session_01RzRiNtuiTJYnSCP8r6Xj5z
11 files changed+301−204127ecfdfe35ab1460125d356a8bcf61da97fb1f
11 changed files+301−20
Addedsrc/__tests__/csrf.test.ts+227−0View fileUnifiedSplit
@@ -0,0 +1,227 @@
1/**
2 * CSRF middleware — same-origin (Origin/Referer) defence + double-submit
3 * cookie token fallback.
4 *
5 * The platform was previously broken in production for logged-in users:
6 * the global `csrfProtect` middleware required a token on every POST but
7 * ~40+ web-UI forms didn't include one, so every authenticated form
8 * submission returned 403. The middleware now accepts a request iff EITHER:
9 *
10 * 1) Origin/Referer header matches the request host (same-origin), OR
11 * 2) The legacy double-submit cookie token check passes.
12 *
13 * These tests lock in that behaviour so a future refactor cannot silently
14 * regress it.
15 */
16
17import { describe, it, expect } from "bun:test";
18import { Hono } from "hono";
19import { csrfToken, csrfProtect } from "../middleware/csrf";
20
21function buildApp() {
22 const app = new Hono();
23 app.use("*", csrfToken);
24 app.use("*", csrfProtect);
25 app.get("/echo", (c) => c.text("ok"));
26 app.post("/echo", (c) => c.text("ok"));
27 return app;
28}
29
30describe("csrfProtect — request-method gating", () => {
31 it("lets GET requests through unconditionally", async () => {
32 const app = buildApp();
33 const res = await app.request("/echo");
34 expect(res.status).toBe(200);
35 });
36
37 it("lets HEAD/OPTIONS pass the csrf gate (does not 403)", async () => {
38 const app = buildApp();
39 const head = await app.request("/echo", { method: "HEAD" });
40 const opts = await app.request("/echo", { method: "OPTIONS" });
41 // The middleware does not return 403 — any non-200 status here means
42 // the route handler didn't accept the method, not that CSRF rejected.
43 expect(head.status).not.toBe(403);
44 expect(opts.status).not.toBe(403);
45 });
46});
47
48describe("csrfProtect — skip paths", () => {
49 it("skips when no session cookie is present (anonymous user)", async () => {
50 const app = buildApp();
51 // No `session` cookie → anonymous → not a CSRF target
52 const res = await app.request("/echo", { method: "POST" });
53 expect(res.status).toBe(200);
54 });
55
56 it("skips API routes (/api/*) regardless of method", async () => {
57 const app = new Hono();
58 app.use("*", csrfToken);
59 app.use("*", csrfProtect);
60 app.post("/api/anything", (c) => c.text("ok"));
61
62 const res = await app.request("/api/anything", {
63 method: "POST",
64 headers: { cookie: "session=fake-session-cookie" },
65 });
66 expect(res.status).toBe(200);
67 });
68
69 it("skips Bearer-authenticated requests", async () => {
70 const app = buildApp();
71 const res = await app.request("/echo", {
72 method: "POST",
73 headers: {
74 cookie: "session=fake",
75 authorization: "Bearer some-token-here",
76 },
77 });
78 expect(res.status).toBe(200);
79 });
80});
81
82describe("csrfProtect — same-origin defence (Origin/Referer)", () => {
83 it("accepts POST when Origin matches request host", async () => {
84 const app = buildApp();
85 const res = await app.request("/echo", {
86 method: "POST",
87 headers: {
88 cookie: "session=fake",
89 host: "gluecron.example.com",
90 origin: "https://gluecron.example.com",
91 },
92 });
93 expect(res.status).toBe(200);
94 });
95
96 it("accepts POST when Origin matches host with a non-default port", async () => {
97 const app = buildApp();
98 const res = await app.request("/echo", {
99 method: "POST",
100 headers: {
101 cookie: "session=fake",
102 host: "localhost:3000",
103 origin: "http://localhost:3000",
104 },
105 });
106 expect(res.status).toBe(200);
107 });
108
109 it("accepts POST when Referer matches host (browser may strip Origin)", async () => {
110 const app = buildApp();
111 const res = await app.request("/echo", {
112 method: "POST",
113 headers: {
114 cookie: "session=fake",
115 host: "gluecron.example.com",
116 referer: "https://gluecron.example.com/some/page",
117 },
118 });
119 expect(res.status).toBe(200);
120 });
121
122 it("rejects POST when Origin is an attacker site", async () => {
123 const app = buildApp();
124 const res = await app.request("/echo", {
125 method: "POST",
126 headers: {
127 cookie: "session=fake",
128 host: "gluecron.example.com",
129 origin: "https://evil.example.com",
130 },
131 });
132 expect(res.status).toBe(403);
133 });
134
135 it("rejects POST when Referer is an attacker site", async () => {
136 const app = buildApp();
137 const res = await app.request("/echo", {
138 method: "POST",
139 headers: {
140 cookie: "session=fake",
141 host: "gluecron.example.com",
142 referer: "https://evil.example.com/page",
143 },
144 });
145 expect(res.status).toBe(403);
146 });
147
148 it("rejects POST when Origin and Referer are both absent and no token cookie", async () => {
149 const app = buildApp();
150 const res = await app.request("/echo", {
151 method: "POST",
152 headers: { cookie: "session=fake" },
153 });
154 expect(res.status).toBe(403);
155 });
156
157 it("rejects POST when Origin is a malformed URL", async () => {
158 const app = buildApp();
159 const res = await app.request("/echo", {
160 method: "POST",
161 headers: {
162 cookie: "session=fake",
163 host: "gluecron.example.com",
164 origin: "::::not a url::::",
165 },
166 });
167 expect(res.status).toBe(403);
168 });
169});
170
171describe("csrfProtect — double-submit token fallback", () => {
172 it("accepts POST with matching token in header even when Origin is missing", async () => {
173 const app = buildApp();
174 const res = await app.request("/echo", {
175 method: "POST",
176 headers: {
177 cookie: "session=fake; csrf_token=abc123",
178 "x-csrf-token": "abc123",
179 },
180 });
181 expect(res.status).toBe(200);
182 });
183
184 it("rejects POST with mismatched token", async () => {
185 const app = buildApp();
186 const res = await app.request("/echo", {
187 method: "POST",
188 headers: {
189 cookie: "session=fake; csrf_token=abc123",
190 "x-csrf-token": "wrong-token",
191 },
192 });
193 expect(res.status).toBe(403);
194 });
195
196 it("accepts POST with matching token in form body", async () => {
197 const app = buildApp();
198 const body = new URLSearchParams({ _csrf: "abc123", other: "field" });
199 const res = await app.request("/echo", {
200 method: "POST",
201 headers: {
202 cookie: "session=fake; csrf_token=abc123",
203 "content-type": "application/x-www-form-urlencoded",
204 },
205 body: body.toString(),
206 });
207 expect(res.status).toBe(200);
208 });
209});
210
211describe("csrfToken — cookie setter", () => {
212 it("sets a csrf_token cookie on first request when none exists", async () => {
213 const app = buildApp();
214 const res = await app.request("/echo");
215 const setCookie = res.headers.get("set-cookie") || "";
216 expect(setCookie).toContain("csrf_token=");
217 });
218
219 it("does not overwrite an existing csrf_token cookie", async () => {
220 const app = buildApp();
221 const res = await app.request("/echo", {
222 headers: { cookie: "csrf_token=existing-value" },
223 });
224 const setCookie = res.headers.get("set-cookie") || "";
225 expect(setCookie).not.toContain("csrf_token=");
226 });
227});
Modifiedsrc/middleware/csrf.ts+48−3View fileUnifiedSplit
@@ -39,8 +39,23 @@ export const csrfToken = createMiddleware(async (c, next) => {
3939});
4040
4141/**
42 * Validates CSRF token on mutating requests (POST, PUT, DELETE, PATCH).
43 * Checks form body field '_csrf' or header 'x-csrf-token' against cookie.
42 * Validates CSRF on mutating requests (POST, PUT, DELETE, PATCH).
43 *
44 * Defence-in-depth, in order:
45 * 1. Same-origin check via Origin/Referer header (OWASP-recommended primary
46 * defence against CSRF — a cross-site forged request from a victim's
47 * browser always carries the attacker's Origin, never the app's host).
48 * 2. Double-submit cookie token (the `_csrf` form field or `X-CSRF-Token`
49 * header must equal the `csrf_token` cookie). Optional — used as a
50 * belt-and-braces fallback when present.
51 *
52 * The Origin check alone is sufficient for modern browsers (all major
53 * browsers send Origin on cross-origin POSTs). The token check is retained
54 * because some legacy clients strip Origin/Referer, and because it gives
55 * forms an explicit "I came from a real page" signal.
56 *
57 * A request is accepted iff EITHER the Origin/Referer matches the request
58 * host OR the token check passes.
4459 */
4560export const csrfProtect = createMiddleware(async (c, next) => {
4661 const method = c.req.method.toUpperCase();
@@ -74,9 +89,39 @@ export const csrfProtect = createMiddleware(async (c, next) => {
7489 return next();
7590 }
7691
92 // ---- 1) Same-origin check (Origin / Referer header) -----------------
93 // A genuine same-origin request from our own pages will carry an Origin
94 // (always for cross-origin POSTs, usually for same-origin too) or a
95 // Referer that matches the request host. Cross-site forged requests
96 // either carry the attacker's origin or are stripped — in both cases
97 // they fail this check.
98 const host = c.req.header("host");
99 const origin = c.req.header("origin");
100 const referer = c.req.header("referer");
101 let originOk = false;
102 if (host) {
103 if (origin) {
104 try {
105 originOk = new URL(origin).host === host;
106 } catch {
107 originOk = false;
108 }
109 } else if (referer) {
110 try {
111 originOk = new URL(referer).host === host;
112 } catch {
113 originOk = false;
114 }
115 }
116 }
117 if (originOk) {
118 return next();
119 }
120
121 // ---- 2) Double-submit cookie token (fallback) ------------------------
77122 const cookieToken = getCookie(c, CSRF_COOKIE);
78123 if (!cookieToken) {
79 return c.text("CSRF token missing", 403);
124 return c.text("CSRF check failed: no Origin/Referer header and no token cookie", 403);
80125 }
81126
82127 // Check header first, then form body
Modifiedsrc/middleware/rate-limit.ts+9−1View fileUnifiedSplit
@@ -39,7 +39,15 @@ export function rateLimit(
3939 // In test env, expose informational rate-limit headers but do not actually
4040 // enforce limits — the shared in-memory store leaks across test files and
4141 // would push requests into 429 once accumulated.
42 if (process.env.NODE_ENV === "test" || process.env.BUN_ENV === "test") {
42 //
43 // Belt-and-braces: refuse to disable enforcement when NODE_ENV=production
44 // even if BUN_ENV/NODE_ENV are also somehow set to "test". A misconfigured
45 // production container with a leaked test env var must not silently drop
46 // rate limiting.
47 const isTestEnv =
48 process.env.NODE_ENV !== "production" &&
49 (process.env.NODE_ENV === "test" || process.env.BUN_ENV === "test");
50 if (isTestEnv) {
4351 c.header("X-RateLimit-Limit", String(maxRequests));
4452 c.header("X-RateLimit-Remaining", String(maxRequests));
4553 c.header("X-RateLimit-Reset", String(Math.ceil((Date.now() + windowMs) / 1000)));
Modifiedsrc/routes/dep-updater.tsx+2−1View fileUnifiedSplit
@@ -13,6 +13,7 @@ import { db } from "../db";
1313import { depUpdateRuns, repositories, users } from "../db/schema";
1414import { Layout } from "../views/layout";
1515import { RepoHeader } from "../views/components";
16import { formatRelative } from "../views/ui";
1617import { IssueNav } from "./issues";
1718import { softAuth, requireAuth } from "../middleware/auth";
1819import type { AuthEnv } from "../middleware/auth";
@@ -170,7 +171,7 @@ depUpdater.get(
170171 const applied = safeParseBumps(r.appliedBumps);
171172 const attempted = safeParseBumps(r.attemptedBumps);
172173 const bumps = applied.length > 0 ? applied : attempted;
173 const when = new Date(r.createdAt).toLocaleString();
174 const when = formatRelative(r.createdAt as unknown as string);
174175 return (
175176 <div
176177 class="issue-row"
Modifiedsrc/routes/gates.tsx+2−2View fileUnifiedSplit
@@ -114,7 +114,7 @@ gates.get("/:owner/:repo/gates", async (c) => {
114114 <div style="font-size: 11px; color: var(--text-muted); text-transform: uppercase">Passed</div>
115115 </div>
116116 <div class="panel" style="padding: 12px; text-align: center">
117 <div style="font-size: 22px; font-weight: 700; color: #bc8cff">{repaired}</div>
117 <div style="font-size: 22px; font-weight: 700; color: var(--accent)">{repaired}</div>
118118 <div style="font-size: 11px; color: var(--text-muted); text-transform: uppercase">Repaired</div>
119119 </div>
120120 <div class="panel" style="padding: 12px; text-align: center">
@@ -154,7 +154,7 @@ gates.get("/:owner/:repo/gates", async (c) => {
154154 </div>
155155 )}
156156 {r.repairCommitSha && (
157 <div style="font-size: 12px; color: #bc8cff; margin-top: 2px">
157 <div style="font-size: 12px; color: var(--accent); margin-top: 2px">
158158 Auto-repaired in{" "}
159159 <a href={`/${owner}/${repo}/commit/${r.repairCommitSha}`}>
160160 {r.repairCommitSha.slice(0, 7)}
Modifiedsrc/routes/import.tsx+2−2View fileUnifiedSplit
@@ -85,7 +85,7 @@ importRoutes.get("/import", requireAuth, requireAdmin, async (c) => {
8585 </p>
8686 <a
8787 href="/import/bulk"
88 style="display: block; background: var(--bg-secondary); border: 1px solid var(--border); border-left: 3px solid #3fb950; border-radius: var(--radius); padding: 14px 16px; margin-bottom: 20px; font-size: 14px; text-decoration: none; color: inherit"
88 style="display: block; background: var(--bg-secondary); border: 1px solid var(--border); border-left: 3px solid var(--green); border-radius: var(--radius); padding: 14px 16px; margin-bottom: 20px; font-size: 14px; text-decoration: none; color: inherit"
8989 >
9090 <strong>Migrating a whole org? Try the bulk importer →</strong>
9191 <div style="color: var(--text-muted); margin-top: 4px">
@@ -116,7 +116,7 @@ importRoutes.get("/import", requireAuth, requireAdmin, async (c) => {
116116 id="import-progress"
117117 role="status"
118118 aria-live="polite"
119 style="display: none; background: var(--bg-secondary); border: 1px solid var(--border); border-left: 3px solid #f0b429; border-radius: var(--radius); padding: 14px 16px; margin-bottom: 20px; font-size: 14px"
119 style="display: none; background: var(--bg-secondary); border: 1px solid var(--border); border-left: 3px solid var(--yellow); border-radius: var(--radius); padding: 14px 16px; margin-bottom: 20px; font-size: 14px"
120120 >
121121 <strong>Import in progress…</strong>
122122 <div style="color: var(--text-muted); margin-top: 4px">
Modifiedsrc/routes/migrations.tsx+2−3View fileUnifiedSplit
@@ -20,6 +20,7 @@ import { desc, eq } from "drizzle-orm";
2020import { db } from "../db";
2121import { repositories } from "../db/schema";
2222import { Layout } from "../views/layout";
23import { formatRelative } from "../views/ui";
2324import { requireAuth } from "../middleware/auth";
2425import type { AuthEnv } from "../middleware/auth";
2526
@@ -154,9 +155,7 @@ migrations.get("/migrations", async (c) => {
154155 —
155156 </div>
156157 <div style="flex:1;min-width:0;font-size:12px;color:var(--text-muted)">
157 {r.createdAt
158 ? new Date(r.createdAt).toLocaleString()
159 : "—"}
158 {r.createdAt ? formatRelative(r.createdAt as unknown as string) : "—"}
160159 </div>
161160 <div style="width:120px;text-align:right">
162161 <a
Modifiedsrc/routes/mirrors.tsx+5−4View fileUnifiedSplit
@@ -14,6 +14,7 @@ import { db } from "../db";
1414import { repositories, users } from "../db/schema";
1515import { Layout } from "../views/layout";
1616import { RepoHeader } from "../views/components";
17import { formatRelative } from "../views/ui";
1718import { softAuth, requireAuth } from "../middleware/auth";
1819import type { AuthEnv } from "../middleware/auth";
1920import { isSiteAdmin } from "../lib/admin";
@@ -180,9 +181,9 @@ mirrors.get("/:owner/:repo/settings/mirror", requireAuth, async (c) => {
180181 style="font-size:12px;color:var(--text-muted);text-transform:uppercase"
181182 >
182183 {mirror.lastStatus === "ok" ? "Success" : "Error"} —{" "}
183 {new Date(
184 {formatRelative(
184185 mirror.lastSyncedAt as unknown as string
185 ).toLocaleString()}
186 )}
186187 </div>
187188 {mirror.lastError && (
188189 <pre
@@ -222,9 +223,9 @@ mirrors.get("/:owner/:repo/settings/mirror", requireAuth, async (c) => {
222223 <span
223224 style="font-size:12px;color:var(--text-muted);font-family:var(--font-mono)"
224225 >
225 {new Date(
226 {formatRelative(
226227 r.startedAt as unknown as string
227 ).toLocaleString()}
228 )}
228229 </span>
229230 </div>
230231 {r.message && (
Modifiedsrc/routes/packages.tsx+2−2View fileUnifiedSplit
@@ -163,7 +163,7 @@ ui.get("/:owner/:repo/packages", async (c) => {
163163 </li>
164164 <li>
165165 Add to your <code>.npmrc</code>:
166 <pre style="background: #0b0d0f; color: #c7ccd1; padding: 8px 12px; border-radius: 4px; font-size: 12px; margin: 6px 0">
166 <pre style="background: var(--bg-tertiary); color: var(--text); padding: 8px 12px; border-radius: 4px; font-size: 12px; margin: 6px 0">
167167 registry={registryUrl}
168168 {"\n"}
169169 //{host}/npm/:_authToken=YOUR_PAT
@@ -315,7 +315,7 @@ ui.get("/:owner/:repo/packages/:pkgName{.+}", async (c) => {
315315 <h3 style="font-size: 14px; text-transform: uppercase; letter-spacing: 0.5px; color: var(--text-muted); margin: 0 0 8px 0">
316316 Install
317317 </h3>
318 <pre style="background: #0b0d0f; color: #c7ccd1; padding: 10px 14px; border-radius: 6px; font-size: 13px; overflow-x: auto">
318 <pre style="background: var(--bg-tertiary); color: var(--text); padding: 10px 14px; border-radius: 6px; font-size: 13px; overflow-x: auto">
319319 npm install {fullName}
320320 {latestVersion ? `@${latestVersion.version}` : ""}
321321 </pre>
Modifiedsrc/routes/workflows.tsx+1−1View fileUnifiedSplit
@@ -493,7 +493,7 @@ actions.get("/:owner/:repo/actions/runs/:runId", async (c) => {
493493 if (j.logs && j.logs.length > 0) {
494494 return (
495495 <pre
496 style="margin: 0; padding: 12px 14px; background: #0b0d0f; color: #c7ccd1; font-size: 12px; line-height: 1.45; overflow-x: auto; max-height: 480px; border-top: 1px solid var(--border)"
496 style="margin: 0; padding: 12px 14px; background: var(--bg-tertiary); color: var(--text); font-size: 12px; line-height: 1.45; overflow-x: auto; max-height: 480px; border-top: 1px solid var(--border)"
497497 >
498498 {j.logs}
499499 </pre>
Modifiedsrc/views/log-tail.tsx+1−1View fileUnifiedSplit
@@ -40,7 +40,7 @@ export function LogTail(props: {
4040 </div>
4141 <pre
4242 id={elementId}
43 style={`margin: 0; padding: 12px 14px; background: #0b0d0f; color: #c7ccd1; font-size: 12px; line-height: 1.45; overflow: auto; max-height: ${props.height || "480px"}; border-bottom-left-radius: 6px; border-bottom-right-radius: 6px`}
43 style={`margin: 0; padding: 12px 14px; background: var(--bg-tertiary); color: var(--text); font-size: 12px; line-height: 1.45; overflow: auto; max-height: ${props.height || "480px"}; border-bottom-left-radius: 6px; border-bottom-right-radius: 6px`}
4444 >
4545 {props.fallbackLogs || ""}
4646 </pre>
4747