Accept webhooks up to GitHub's 25 MB

app.js

@@ -1,6 +1,5 @@
 import config from './lib/config-loader.js';
 import express from 'express';
-import bodyParser from 'body-parser';
 import expressSession from 'express-session';
 import authManager from './lib/authentication.js';
 import socketAuthenticator from './lib/socket-auth.js';
@@ -13,6 +12,7 @@ import { createRefreshAll } from './lib/refresh-all.js';
 import { createPacer } from './lib/pacer.js';
 import mainController from './controllers/main.js';
 import hooksController from './controllers/githubHooks.js';
+import { useBodyParsers } from './lib/body-parsers.js';
 import statsController from './controllers/stats.js';
 import userNamesController from './controllers/user-names.js';
 import projectsController from './controllers/projects.js';
@@ -52,7 +52,6 @@ const refreshAll = createRefreshAll({
 
 const app = express();
 const httpServer = createServer(app);
-const maxPostSize = 1024 * 1024;
 
 const __filename = fileURLToPath(import.meta.url);
 const __dirname = dirname(__filename);
@@ -63,8 +62,7 @@ app.set('view engine', 'html');
  * Middleware
  */
 app.use('/public', express.static(__dirname + '/public'));
-app.use(bodyParser.urlencoded({ limit: maxPostSize, extended: false }));
-app.use(bodyParser.json({ limit: maxPostSize }));
+useBodyParsers(app, config.github.hook_secret);
 app.use(
    expressSession({
       secret: config.session.secret,

lib/body-parsers.js

@@ -0,0 +1,27 @@
+import bodyParser from 'body-parser';
+
+// Everything the board and the API post is small.
+const MAX_POST_SIZE = 1024 * 1024;
+// GitHub sends webhook payloads up to 25 MB, and a big PR (a long body, many
+// labels or a large push) passes 1 MB. Rejected with a 413, that update was
+// lost until the hourly reconcile, about a dozen a day on prod.
+const MAX_HOOK_SIZE = 25 * 1024 * 1024;
+
+/**
+ * Parse request bodies: the webhook route with GitHub's limit, everything
+ * else with the small one. The webhook parsers run first; body-parser skips a
+ * body that's already parsed, so the global ones leave hook requests alone.
+ * The route is public (GitHub has to reach it), so the secret in its query is
+ * checked before reading up to 25 MB, not after.
+ */
+export function useBodyParsers(app, hookSecret) {
+   app.use(
+      '/hooks',
+      (req, res, next) =>
+         req.query.secret === hookSecret ? next() : res.status(401).send('Invalid POST'),
+      bodyParser.urlencoded({ limit: MAX_HOOK_SIZE, extended: false }),
+      bodyParser.json({ limit: MAX_HOOK_SIZE })
+   );
+   app.use(bodyParser.urlencoded({ limit: MAX_POST_SIZE, extended: false }));
+   app.use(bodyParser.json({ limit: MAX_POST_SIZE }));
+}

test/body-parsers.test.js

@@ -0,0 +1,52 @@
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import express from 'express';
+import { useBodyParsers } from '../lib/body-parsers.js';
+
+// a JSON body just past the general 1 MB limit, the size of a big webhook
+const big = JSON.stringify({ action: 'edited', body: 'x'.repeat(2 * 1024 * 1024) });
+
+async function withServer(t) {
+   const app = express();
+   useBodyParsers(app, 's3cret');
+   app.post('/hooks/main', (req, res) => res.json({ got: req.body.body.length }));
+   app.post('/roadmap', (req, res) => res.json({ got: req.body.body.length }));
+   const server = await new Promise(resolve => {
+      const s = app.listen(0, () => resolve(s));
+   });
+   t.after(() => server.close());
+   return `http://127.0.0.1:${server.address().port}`;
+}
+
+const post = (url, body, type) =>
+   fetch(url, { method: 'POST', headers: { 'content-type': type }, body });
+
+test('a webhook past 1 MB is parsed', async t => {
+   const base = await withServer(t);
+   const res = await post(`${base}/hooks/main?secret=s3cret`, big, 'application/json');
+   assert.equal(res.status, 200);
+   assert.equal((await res.json()).got, 2 * 1024 * 1024);
+});
+
+test('a form-encoded webhook past 1 MB is parsed', async t => {
+   const base = await withServer(t);
+   const form = new URLSearchParams({ body: 'x'.repeat(2 * 1024 * 1024) }).toString();
+   const res = await post(
+      `${base}/hooks/main?secret=s3cret`,
+      form,
+      'application/x-www-form-urlencoded'
+   );
+   assert.equal(res.status, 200);
+});
+
+test('other routes keep the 1 MB limit', async t => {
+   const base = await withServer(t);
+   const res = await post(`${base}/roadmap`, big, 'application/json');
+   assert.equal(res.status, 413);
+});
+
+test('a webhook without the secret is refused before its body is read', async t => {
+   const base = await withServer(t);
+   const res = await post(`${base}/hooks/main?secret=wrong`, big, 'application/json');
+   assert.equal(res.status, 401);
+});