fix(serve): sanitize upstream error text before logging it (#11)

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 <noreply@anthropic.com>
This commit is contained in:
Shifra Williams 2026-08-05 15:44:21 -07:00 • committed by GitHub
parent 356032e2f6
commit 746392d99f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 8 additions and 5 deletions

View file

@ -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;

View file

@ -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');
},
);
});