|
| 1 | +// How much a webhook is allowed to publish in one call. |
| 2 | +// |
| 3 | +// The publishing endpoint documents a ceiling of MAX_BATCH items, and for a |
| 4 | +// while that was a promise the server could not keep: body-parser's default |
| 5 | +// limit is 100kb, and MAX_BATCH items of the documented size weigh about ten |
| 6 | +// times that. A caller sending exactly what the docs described got a rejection |
| 7 | +// no field limit in moshpit-content.mjs explained. |
| 8 | +// |
| 9 | +// Worse, it did not even report as a rejection. The catch-all error handler |
| 10 | +// turned body-parser's 413 into the generic 500 HTML page, so "your batch is |
| 11 | +// too big" arrived as "a bug got in" and sent people reading server logs. |
| 12 | +// |
| 13 | +// These tests pin all three halves of the fix: the limit is derived from the |
| 14 | +// field limits rather than guessed, a batch far past the old default is |
| 15 | +// accepted, and one past the new ceiling is refused as JSON with the status |
| 16 | +// body-parser chose. |
| 17 | +import assert from "node:assert/strict"; |
| 18 | +import fs from "node:fs"; |
| 19 | +import { mkdtempSync } from "node:fs"; |
| 20 | +import { tmpdir } from "node:os"; |
| 21 | +import path from "node:path"; |
| 22 | +import { fileURLToPath } from "node:url"; |
| 23 | +import { createRequire } from "node:module"; |
| 24 | +import test from "node:test"; |
| 25 | + |
| 26 | +import { MAX_BATCH, MAX_BODY, MAX_PUBLISH_BYTES } from "../src/lib/moshpit-content.mjs"; |
| 27 | + |
| 28 | +const require = createRequire(import.meta.url); |
| 29 | +let deps = null; |
| 30 | +try { |
| 31 | + deps = { express: require("express"), cookieParser: require("cookie-parser") }; |
| 32 | +} catch { |
| 33 | + deps = null; |
| 34 | +} |
| 35 | + |
| 36 | +const workdir = mkdtempSync(path.join(tmpdir(), "moshcode-publish-limit-test-")); |
| 37 | +process.env.DATABASE_URL = `file:${path.join(workdir, "test.db")}`; |
| 38 | +process.env.SESSION_SECRET = "test-secret"; |
| 39 | + |
| 40 | +const SRC = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../src"); |
| 41 | + |
| 42 | +async function boot() { |
| 43 | + const { migrate } = await import("../src/migrate.mjs"); |
| 44 | + await migrate(); |
| 45 | + const { run, db } = await import("../src/db.mjs"); |
| 46 | + const { sessionMiddleware, csrfGuard } = await import("../src/lib/session.mjs"); |
| 47 | + const { moshpitRouter } = await import("../src/routes/moshpit.mjs"); |
| 48 | + const { createApiKey } = await import("../src/lib/apikey.mjs"); |
| 49 | + |
| 50 | + await run(`INSERT OR REPLACE INTO users (id, email, display_name, created_at) VALUES ('u1','a@b.c','one',1)`); |
| 51 | + await run(`INSERT INTO moshpit_tlds (tld,user_id,owner_email,created_at) VALUES ('eggs','u1','a@b.c',1)`); |
| 52 | + await run(`INSERT INTO moshpit_names (tld,label,user_id,created_at) VALUES ('eggs','blue','u1',1)`); |
| 53 | + |
| 54 | + const key = (await createApiKey("u1", "cli one")).plaintext; |
| 55 | + |
| 56 | + // Wired the way src/server.mjs wires it: the scoped parser first, the global |
| 57 | + // one after. That order is the fix — see the drift guard at the bottom. |
| 58 | + const app = deps.express(); |
| 59 | + const captureRaw = (req, _res, buf) => { req.rawBody = buf.toString("utf8"); }; |
| 60 | + app.use("/api/moshpit/sites", deps.express.json({ limit: MAX_PUBLISH_BYTES, verify: captureRaw })); |
| 61 | + app.use(deps.express.json({ verify: captureRaw })); |
| 62 | + app.use(deps.express.urlencoded({ extended: false })); |
| 63 | + app.use(deps.cookieParser()); |
| 64 | + app.use(sessionMiddleware); |
| 65 | + app.use(csrfGuard); |
| 66 | + app.use(moshpitRouter); |
| 67 | + |
| 68 | + // eslint-disable-next-line no-unused-vars |
| 69 | + app.use((err, req, res, _next) => { |
| 70 | + const status = Number(err?.status ?? err?.statusCode) || 500; |
| 71 | + if (status >= 400 && status < 500) { |
| 72 | + const detail = err?.type === "entity.too.large" ? "too large" : "unreadable"; |
| 73 | + if (req.path.startsWith("/api/")) return res.status(status).json({ error: detail }); |
| 74 | + return res.status(status).type("text").send(`${detail}\n`); |
| 75 | + } |
| 76 | + return res.status(500).type("html").send("<body>500</body>"); |
| 77 | + }); |
| 78 | + |
| 79 | + const server = await new Promise((resolve) => { |
| 80 | + const s = app.listen(0, "127.0.0.1", () => resolve(s)); |
| 81 | + }); |
| 82 | + const base = `http://127.0.0.1:${server.address().port}`; |
| 83 | + |
| 84 | + const call = async (method, p, body) => { |
| 85 | + const res = await fetch(`${base}${p}`, { |
| 86 | + method, |
| 87 | + headers: { "content-type": "application/json", authorization: `Bearer ${key}` }, |
| 88 | + body: body === undefined ? undefined : JSON.stringify(body), |
| 89 | + }); |
| 90 | + const text = await res.text(); |
| 91 | + let json = null; |
| 92 | + try { json = JSON.parse(text); } catch { /* HTML error page */ } |
| 93 | + return { status: res.status, json, text }; |
| 94 | + }; |
| 95 | + |
| 96 | + return { server, db, call }; |
| 97 | +} |
| 98 | + |
| 99 | +let booted = null; |
| 100 | +const app = () => (booted ||= boot()); |
| 101 | + |
| 102 | +test.after(() => { |
| 103 | + if (!booted) return; |
| 104 | + booted.then(({ server, db }) => { server.close(); db.close?.(); }) |
| 105 | + .finally(() => { try { fs.rmSync(workdir, { recursive: true, force: true }); } catch { /* noop */ } }); |
| 106 | +}); |
| 107 | + |
| 108 | +const skip = { skip: !deps && "apps/pwa deps not installed" }; |
| 109 | + |
| 110 | +test("publish limit: the ceiling is derived from the field limits, not guessed", () => { |
| 111 | + // The property that matters: a batch of the documented size must fit. If |
| 112 | + // MAX_BODY or MAX_BATCH grows and this is still a hardcoded "2mb", the two |
| 113 | + // drift apart silently and the endpoint starts refusing documented input. |
| 114 | + assert.ok( |
| 115 | + MAX_PUBLISH_BYTES >= MAX_BATCH * MAX_BODY, |
| 116 | + `${MAX_PUBLISH_BYTES} must hold ${MAX_BATCH} bodies of ${MAX_BODY}`, |
| 117 | + ); |
| 118 | + |
| 119 | + // And it must be meaningfully past the 100kb default, or nothing was fixed. |
| 120 | + assert.ok(MAX_PUBLISH_BYTES > 100 * 1024 * 4, "should be well past body-parser's 100kb default"); |
| 121 | +}); |
| 122 | + |
| 123 | +test("publish limit: a batch past the old 100kb default is accepted", skip, async () => { |
| 124 | + const { call } = await app(); |
| 125 | + |
| 126 | + // ~240kb of JSON: comfortably past the old default, comfortably under the |
| 127 | + // new ceiling. This is the request that used to fail. |
| 128 | + const items = Array.from({ length: 20 }, (_, i) => ({ |
| 129 | + kind: "text", |
| 130 | + title: `post ${i}`, |
| 131 | + slug: `post-${i}`, |
| 132 | + body: "x".repeat(12_000), |
| 133 | + })); |
| 134 | + const payload = JSON.stringify(items); |
| 135 | + assert.ok(payload.length > 100 * 1024, `fixture must exceed the old default, got ${payload.length}`); |
| 136 | + assert.ok(payload.length < MAX_PUBLISH_BYTES, "fixture must stay under the new ceiling"); |
| 137 | + |
| 138 | + const res = await call("POST", "/api/moshpit/sites/blue.eggs/content", items); |
| 139 | + // 201 when every item was created, 200 on a pure update, 207 when the batch |
| 140 | + // was partly valid. Any of the three means the body was read — which is the |
| 141 | + // thing under test. What must not come back is 413. |
| 142 | + assert.ok( |
| 143 | + [200, 201, 207].includes(res.status), |
| 144 | + `expected a published batch, got ${res.status} ${res.text.slice(0, 200)}`, |
| 145 | + ); |
| 146 | + assert.equal(res.json.results.length, 20); |
| 147 | + assert.ok(res.json.results.every((r) => r.ok), "every item in a valid batch should publish"); |
| 148 | +}); |
| 149 | + |
| 150 | +test("publish limit: past the ceiling it is a 413 in JSON, not a 500 in HTML", skip, async () => { |
| 151 | + const { call } = await app(); |
| 152 | + |
| 153 | + // One item whose body alone exceeds the whole-batch ceiling. |
| 154 | + const huge = [{ kind: "text", title: "too much", slug: "too-much", body: "x".repeat(MAX_PUBLISH_BYTES + 1024) }]; |
| 155 | + |
| 156 | + const res = await call("POST", "/api/moshpit/sites/blue.eggs/content", huge); |
| 157 | + |
| 158 | + // The status body-parser chose, not the catch-all 500 this used to become. |
| 159 | + assert.equal(res.status, 413, `expected 413, got ${res.status}`); |
| 160 | + // JSON, because /api/ is an API. An HTML error page here is unparseable by |
| 161 | + // the scripts this endpoint exists for. |
| 162 | + assert.ok(res.json, `expected a JSON body, got ${res.text.slice(0, 200)}`); |
| 163 | + assert.match(res.json.error, /too large/i); |
| 164 | +}); |
| 165 | + |
| 166 | +test("publish limit: the scoped parser is mounted before the global one", () => { |
| 167 | + // A drift guard rather than a behaviour test, because the wiring lives in |
| 168 | + // server.mjs and server.mjs listens on import — it cannot be imported here. |
| 169 | + // |
| 170 | + // Order is the whole fix. body-parser skips a request whose body another |
| 171 | + // parser already read, so if the global 100kb parser is mounted first it |
| 172 | + // wins and the scoped limit becomes decorative. That reversal is invisible: |
| 173 | + // every test above still passes, because they wire their own stack. |
| 174 | + const src = fs.readFileSync(path.join(SRC, "server.mjs"), "utf8"); |
| 175 | + |
| 176 | + const scoped = src.indexOf('app.use("/api/moshpit/sites", express.json('); |
| 177 | + const global = src.indexOf("app.use(express.json({ verify: captureRaw }))"); |
| 178 | + |
| 179 | + assert.ok(scoped !== -1, "server.mjs should mount a scoped JSON parser for /api/moshpit/sites"); |
| 180 | + assert.ok(global !== -1, "server.mjs should still mount a global JSON parser"); |
| 181 | + assert.ok(scoped < global, "the scoped parser must be mounted BEFORE the global one, or its limit never applies"); |
| 182 | + |
| 183 | + // And it must use the derived constant, not a literal that can drift. |
| 184 | + assert.match(src, /limit:\s*MAX_PUBLISH_BYTES/, "the scoped parser should use MAX_PUBLISH_BYTES"); |
| 185 | +}); |
0 commit comments