From 63bb4699ea4397c31dee401717a6159b8ffde43c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 19:54:36 +0000 Subject: [PATCH] Answer unverified webhooks with 401 and stop calling next() after replying An unverified request to /github-hook fell through to Express's 404, which reads as a wrong URL in GitHub's delivery log rather than a signature mismatch. Answer it with a 401 instead. Both /github-hook and /config also called next() after sending their response. With no later route, that landed in finalhandler, which finds the headers already sent and destroys the socket. Neither handler needs next(), so drop it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Qq8TZHcEcvGppVjWSdt7ek --- docs/error-handling.md | 4 ++-- lib/app.js | 8 +++----- test/server.js | 4 ++-- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/docs/error-handling.md b/docs/error-handling.md index 1a902cc..f21bf0c 100644 --- a/docs/error-handling.md +++ b/docs/error-handling.md @@ -283,8 +283,8 @@ Only `opened`, `edited`, `reopened` and `synchronize` pull_request events are queued. The bot's own body update comes back as an `edited` event and is recognised by its sender. A PR already queued or running is not queued twice. Unverified requests in production are logged at `warn` as -`Unverified request: from
` and fall through to -Express's 404. +`Unverified request: from
` and answered with a +401, so a mismatched secret shows up as such in GitHub's delivery log. **Config tester** (`POST /config`). Validation errors (bad repo name, invalid JSON, schema violations, a repo with no PRs) are answered with a diff --git a/lib/app.js b/lib/app.js index 8df1556..3213de9 100644 --- a/lib/app.js +++ b/lib/app.js @@ -22,7 +22,7 @@ module.exports = function createApp(controller, config, log) { // Everything else is acknowledged and logged as ignored, with the same // status and reason fields as a processed job, so the log says what // arrived and what became of it. - app.post('/github-hook', function (req, res, next) { + app.post('/github-hook', function (req, res) { if (config.nodeEnv != 'production' || (typeof req.isXHubValid === 'function' && req.isXHubValid())) { res.send(new Date().toISOString()); const payload = req.body; @@ -63,8 +63,8 @@ module.exports = function createApp(controller, config, log) { const ip = req.headers["x-forwarded-for"] || req.socket.remoteAddress; log.warn({ method: req.method, url: req.originalUrl, ip }, `Unverified request: ${req.method} ${req.originalUrl} from ${ip}`); + res.sendStatus(401); } - next(); }); // Deployment health check. Clever Cloud calls the path named in @@ -82,7 +82,7 @@ module.exports = function createApp(controller, config, log) { }); }); - app.post('/config', bodyParser.urlencoded({ extended: false }), async function (req, res, next) { + app.post('/config', bodyParser.urlencoded({ extended: false }), async function (req, res) { try { let params = req.body; let url; @@ -95,8 +95,6 @@ module.exports = function createApp(controller, config, log) { } catch (err) { res.status(400).send({ error: err.message }); log.error({ err, url: req.originalUrl }, `${req.originalUrl}: request failed`); - } finally { - next(); } }); diff --git a/test/server.js b/test/server.js index 7662094..ebf3912 100644 --- a/test/server.js +++ b/test/server.js @@ -178,7 +178,7 @@ suite('Server signature verification', () => { .set('Content-Type', 'application/json') .set('X-Hub-Signature', sign('wrong-secret', BODY)) .send(BODY) - .expect(404) + .expect(401) .end(done); }); @@ -189,7 +189,7 @@ suite('Server signature verification', () => { .post('/github-hook') .set('Content-Type', 'application/json') .send(BODY) - .expect(404) + .expect(401) .end(done); }); });