From 084672c41cc4461c4e8b16497b9bb89100d71e45 Mon Sep 17 00:00:00 2001 From: DashCaddy Loop Date: Tue, 18 Aug 2026 04:15:48 -0700 Subject: [PATCH] fix(csrf): tag browser auto-retry as [CSRF-debug], keep [CSRF] for real probes (DC-058) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Background: every time dashcaddy-api restarts, the first POST from a dashboard browser tab hits the missing-CSRF-cookie branch. The status/js/globals.js secureFetch() wrapper catches the 403 and auto-retries with a fresh token, so the WARN line is misleading noise. Live evidence (DNS2, 2026-08-18 10:35:32Z container restart): [CSRF] Missing CSRF cookie: POST /api/v1/backups/schedule from 100.121.150.22 [CSRF] Missing CSRF cookie: POST /api/v1/backups/schedule from 100.121.150.22 [CSRF] Missing CSRF cookie: POST /api/v1/backups/schedule from 172.17.0.1 Fix: if X-CSRF-Token header is ALSO present, tag the log line [CSRF-debug] (operator can grep it out as expected noise — the secureFetch retry will self-heal). A request with NEITHER cookie NOR header (curl probe, exploit scanner, broken client) keeps the [CSRF] tag. Threat model: forging a header without the cookie just produces a different 403 (Invalid CSRF token) — the timingSafeEqual check on lines 248-260 of csrf-protection.js is unchanged. This is a log-only fix. Tests: 4 new in __tests__/csrf-protection.test.js under 'DC-058: browser-auto-retry vs real-probe log tagging'. Full suite 1982/1982 green on DNS2 worktree. [self-grade=pending] --- .../__tests__/csrf-protection.test.js | 83 +++++++++++++++++++ dashcaddy-api/src/security/csrf-protection.js | 20 ++++- 2 files changed, 101 insertions(+), 2 deletions(-) diff --git a/dashcaddy-api/__tests__/csrf-protection.test.js b/dashcaddy-api/__tests__/csrf-protection.test.js index b039f57..022e9ad 100644 --- a/dashcaddy-api/__tests__/csrf-protection.test.js +++ b/dashcaddy-api/__tests__/csrf-protection.test.js @@ -322,6 +322,89 @@ describe('CSRF Protection', () => { process.env.NODE_ENV = origEnv; }); + + // DC-058: differentiate "browser auto-retry" from "real probe" by the + // presence of the X-CSRF-Token header. The 403 response is identical in + // both branches; only the stderr log tag changes. + describe('DC-058: browser-auto-retry vs real-probe log tagging', () => { + let stderrSpy; + let origEnv; + + beforeEach(() => { + origEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'production'; + stderrSpy = jest.spyOn(process.stderr, 'write').mockImplementation(() => true); + }); + + afterEach(() => { + process.env.NODE_ENV = origEnv; + stderrSpy.mockRestore(); + }); + + it('tags missing-cookie with [CSRF-debug] when X-CSRF-Token header also present (browser auto-retry)', () => { + const { req, res, next } = createMockReqRes({ + method: 'POST', path: '/api/v1/backups/schedule', + headers: { cookie: '', 'x-csrf-token': 'some-signature-attempt' } + }); + csrfValidationMiddleware(req, res, next); + + // 403 response unchanged + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: expect.stringContaining('DC-100') }) + ); + // Log tag is [CSRF-debug] + expect(stderrSpy).toHaveBeenCalled(); + const lastWrite = stderrSpy.mock.calls[stderrSpy.mock.calls.length - 1][0]; + expect(lastWrite).toContain('[CSRF-debug]'); + expect(lastWrite).toContain('browser auto-retry'); + expect(lastWrite).not.toMatch(/^\[CSRF\][^-]/); // not bare [CSRF] + }); + + it('tags missing-cookie with [CSRF] when no X-CSRF-Token header present (real probe)', () => { + const { req, res, next } = createMockReqRes({ + method: 'POST', path: '/api/v1/backups/schedule', + headers: { cookie: '' } + }); + csrfValidationMiddleware(req, res, next); + + expect(res.status).toHaveBeenCalledWith(403); + expect(stderrSpy).toHaveBeenCalled(); + const lastWrite = stderrSpy.mock.calls[stderrSpy.mock.calls.length - 1][0]; + expect(lastWrite).toContain('[CSRF]'); + expect(lastWrite).not.toContain('[CSRF-debug]'); + expect(lastWrite).not.toContain('browser auto-retry'); + }); + + it('keeps [CSRF] tag when cookie present but header missing (curl probe with manual cookie)', () => { + const nonce = generateToken(); + const { req, res, next } = createMockReqRes({ + method: 'POST', path: '/api/v1/backups/schedule', + headers: { cookie: `${CSRF_COOKIE_NAME}=${nonce}` } + }); + csrfValidationMiddleware(req, res, next); + + expect(res.status).toHaveBeenCalledWith(403); + expect(stderrSpy).toHaveBeenCalled(); + const lastWrite = stderrSpy.mock.calls[stderrSpy.mock.calls.length - 1][0]; + expect(lastWrite).toContain('[CSRF]'); + expect(lastWrite).not.toContain('[CSRF-debug]'); + }); + + it('headerToken is read from x-csrf-token (lowercased by Express) — lowercase header triggers [CSRF-debug]', () => { + // Express/Node lowercases all incoming header keys, so production code + // only ever sees lowercase. We test the exact code path here. + const { req, res, next } = createMockReqRes({ + method: 'POST', path: '/api/v1/backups/schedule', + headers: { cookie: '', 'x-csrf-token': 'some-signature-attempt' } + }); + csrfValidationMiddleware(req, res, next); + + expect(res.status).toHaveBeenCalledWith(403); + const lastWrite = stderrSpy.mock.calls[stderrSpy.mock.calls.length - 1][0]; + expect(lastWrite).toContain('[CSRF-debug]'); + }); + }); }); describe('renewCSRFToken', () => { diff --git a/dashcaddy-api/src/security/csrf-protection.js b/dashcaddy-api/src/security/csrf-protection.js index 97cc7f4..ad7581f 100644 --- a/dashcaddy-api/src/security/csrf-protection.js +++ b/dashcaddy-api/src/security/csrf-protection.js @@ -214,14 +214,30 @@ function csrfValidationMiddleware(req, res, next) { return next(); } - // Validate both values exist + // DC-058: differentiate "browser auto-retry" from "real probe" using the + // X-CSRF-Token header as a signal. The dashboard JS in status/js/globals.js + // secureFetch() pre-fetches /api/v1/csrf-token (which sets the CSRF cookie + // via csrfCookieMiddleware) before posting; if the GET raced with container + // restart OR the user cleared cookies mid-session, the POST can arrive with + // a header but no cookie. secureFetch catches the 403 and auto-retries + // with a fresh token (lines 225-238 of globals.js). For these "has header + // but no cookie" misses, tag the log line [CSRF-debug] — operators can + // grep them out as expected noise. A request with NEITHER cookie NOR + // header (curl probe, exploit scanner, broken client) keeps the louder + // [CSRF] tag. if (!cookieNonce) { - process.stderr.write(`[CSRF] Missing CSRF cookie: ${method} ${req.path} from ${req.ip}\n`); + const isLikelyBrowserAutoRetry = !!headerToken; + const tag = isLikelyBrowserAutoRetry ? '[CSRF-debug]' : '[CSRF]'; + process.stderr.write(`${tag} Missing CSRF cookie: ${method} ${req.path} from ${req.ip}` + + (isLikelyBrowserAutoRetry ? ' (browser auto-retry — header present, expect self-heal)' : '') + '\n'); return errorResponse(res, 403, '[DC-100] CSRF token missing', { message: 'CSRF cookie not found. Please refresh the page (Ctrl+Shift+R) and try again.' }); } + // Cookie present but no header — a real browser POST always sends both, so + // header-less is suspicious (curl probe with manual cookie, misconfigured + // client). Keep WARN level. if (!headerToken) { process.stderr.write(`[CSRF] Missing CSRF header: ${method} ${req.path} from ${req.ip}\n`); return errorResponse(res, 403, '[DC-100] CSRF token missing', {