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); }); });