From 746392d99fe70888640ff6549e5121078db8d5eb Mon Sep 17 00:00:00 2001 From: Shifra Williams Date: Wed, 5 Aug 2026 15:44:21 -0700 Subject: [PATCH] fix(serve): sanitize upstream error text before logging it (#11) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL flagged three findings on the proxy added in #2804 (alerts 937-939). All three are ours, none are stale. Log injection at the two `/api/*` failure paths: `err.code` and `err.message` reach `console.warn`/`console.error` raw. A control character in either would forge a log line. Nothing attacker-controlled can currently get there — Node's HTTP parser rejects CR/LF in the request line and in header names, so a client cannot steer the socket error text — but the file already has `sanitizeForLog` for exactly this, and it was applied to operator input in `validHttpUrl` and nowhere else. Applied at both flagged sites plus the handler-crash path, which is the same class and was only unflagged by luck. Incomplete URL substring sanitization in the test: the `RENDER_EXTERNAL_URL` case asserted `res.body.includes(url)`. CodeQL reads a substring check against a URL as a host check, and here it is a weak assertion for the same reason — it would pass if the URL appeared in a comment. Now it parses `__GITNEXUS_CONFIG__` and compares `backendUrl`. 55/55 docker-server tests pass. Co-authored-by: Claude Opus 5 --- docker-server.mjs | 6 +++--- docker-server.test.mjs | 7 +++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/docker-server.mjs b/docker-server.mjs index ed5e112d7..8b3edbf1f 100644 --- a/docker-server.mjs +++ b/docker-server.mjs @@ -408,7 +408,7 @@ async function proxyToUpstream(req, res) { if (retryEligible && !res.headersSent && n < proxyRetryAttempts && retryableError) { const delay = 250 * 2 ** (n - 1); // 250ms, 500ms, ... console.warn( - `[gitnexus-web] upstream ${err.code}; retry ${n}/${proxyRetryAttempts - 1} in ${delay}ms`, + `[gitnexus-web] upstream ${sanitizeForLog(err.code)}; retry ${n}/${proxyRetryAttempts - 1} in ${delay}ms`, ); setTimeout(() => { // The client may have aborted during the backoff window; don't fire a @@ -418,7 +418,7 @@ async function proxyToUpstream(req, res) { }, delay); return; } - console.error('[gitnexus-web] upstream proxy error:', err.message); + console.error('[gitnexus-web] upstream proxy error:', sanitizeForLog(err.message)); failGateway(res, 502, 'Bad gateway'); }); if (proxyTimeoutMs > 0) { @@ -487,7 +487,7 @@ const server = createServer(async (req, res) => { } // Fire-and-forget, so guard the boundary against unhandledRejection. proxyToUpstream(req, res).catch((err) => { - console.error('[gitnexus-web] proxy handler crashed:', err?.message ?? err); + console.error('[gitnexus-web] proxy handler crashed:', sanitizeForLog(err?.message ?? err)); failGateway(res, 502, 'Bad gateway'); }); return; diff --git a/docker-server.test.mjs b/docker-server.test.mjs index 51ac32742..80e742f7e 100644 --- a/docker-server.test.mjs +++ b/docker-server.test.mjs @@ -621,8 +621,11 @@ it('serves RENDER_EXTERNAL_URL as the backend origin when GITNEXUS_BACKEND_URL i async (port) => { const res = await rawGet(port, '/'); assert.equal(res.status, 200); - assert.ok(res.body.includes('window.__GITNEXUS_CONFIG__')); - assert.ok(res.body.includes('https://gitnexus-web.onrender.com')); + // Assert on the parsed value, not a substring of the page: a bare + // includes() would also pass if the URL appeared in a comment. + const injected = /window\.__GITNEXUS_CONFIG__=(\{.*?\});/.exec(res.body)?.[1]; + assert.ok(injected, 'Expected __GITNEXUS_CONFIG__ in response body'); + assert.equal(JSON.parse(injected).backendUrl, 'https://gitnexus-web.onrender.com'); }, ); });