From 83e8d451927bbafd1679e734fd1bd631adb6f540 Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Sun, 27 Sep 2026 13:13:04 +0000 Subject: [PATCH] fix: close rejected artifact service requests before body draining --- docs/review-service-capabilities.md | 10 ++++++-- src/server/service/review-capabilities.ts | 30 +++++++++++------------ test/review-capabilities.test.js | 28 +++++++++++++++++++++ 3 files changed, 50 insertions(+), 18 deletions(-) diff --git a/docs/review-service-capabilities.md b/docs/review-service-capabilities.md index e52b9c0..b2997c4 100644 --- a/docs/review-service-capabilities.md +++ b/docs/review-service-capabilities.md @@ -10,7 +10,8 @@ The endpoint is mounted before JSON parsing, browser sessions and passport. The entire `/service` namespace returns a typed 404 when configuration is absent. Only the exact GET path is accepted. Query strings, browser context headers, cookies, bodies, duplicate authentication headers and unknown paths are rejected. -Responses contain no credentials, owner identity, repository data, cookies or CORS +Rejections close the connection immediately, including requests whose declared +body has not arrived. Responses contain no credentials, owner identity, repository data, cookies or CORS permission; they have `no-store` and `no-referrer` headers. No database, queue or provider call is involved. @@ -51,7 +52,7 @@ session or repository-owner authority. ## Verification -`mocha --no-config --no-package test/review-capabilities.test.js` passes seven +`mocha --no-config --no-package test/review-capabilities.test.js` passes eight HTTP/configuration scenarios, including 60 concurrent reads, cross-client and wrong-key rejection, raw duplicate headers, exact path checks, browser context, expiry, overlapping keys and restart revocation. Add @@ -76,3 +77,8 @@ changes. Use the lockfile when reproducing these results. The endpoint is implemented and tested locally. Upstream deployment, private key distribution, ownership consent and the remaining artifact workflows are pending. + +A raw-socket regression first reproduced a rejected request waiting for its unread +10 MB body. Closing failed connections makes the same header-only request return +400 and close immediately; the complete eight-case endpoint suite and full +TypeScript checking pass. Full PR CI is repeated for this correction. diff --git a/src/server/service/review-capabilities.ts b/src/server/service/review-capabilities.ts index 477a021..173ef31 100644 --- a/src/server/service/review-capabilities.ts +++ b/src/server/service/review-capabilities.ts @@ -109,15 +109,15 @@ export function createReviewCapabilities( res.setHeader("Referrer-Policy", "no-referrer"); res.setHeader("X-Content-Type-Options", "nosniff"); const fail = (status: number, code: string, retry?: number) => { + // Do not keep a rejected request alive while Node drains an unread body. + res.setHeader("Connection", "close"); if (retry) res.setHeader("Retry-After", String(retry)); - res - .status(status) - .json({ - contract, - code, - requestId: randomBytes(16).toString("hex"), - ...(retry ? { retryAfterSeconds: retry } : {}), - }); + res.status(status).json({ + contract, + code, + requestId: randomBytes(16).toString("hex"), + ...(retry ? { retryAfterSeconds: retry } : {}), + }); }; if (!keys || req.originalUrl !== "/service/v1/capabilities") return fail(404, "not-found"); @@ -190,13 +190,11 @@ export function createReviewCapabilities( ); rate.count++; // No link, snapshot, protected-delivery or retention support exists yet. - res - .status(200) - .json({ - contract, - clientId: client, - contracts: [contract], - available: [], - }); + res.status(200).json({ + contract, + clientId: client, + contracts: [contract], + available: [], + }); }; } diff --git a/test/review-capabilities.test.js b/test/review-capabilities.test.js index 2c4e44d..a854907 100644 --- a/test/review-capabilities.test.js +++ b/test/review-capabilities.test.js @@ -253,6 +253,34 @@ describe("review service capabilities", function () { "Invalid REVIEW_SERVICE_KEYS configuration", ); }); + it("closes rejected incomplete bodies without waiting for upload completion", async function () { + this.timeout(5000); + await start(); + const result = await new Promise((resolve, reject) => { + const socket = require("net").createConnection({ + host: "127.0.0.1", + port, + }); + let received = ""; + socket.setTimeout(1500, () => { + socket.destroy(); + reject(new Error("rejected request still waiting for its unread body")); + }); + socket.on("error", reject); + socket.on("data", (data) => { + received += data.toString(); + }); + socket.on("close", () => resolve(received)); + socket.on("connect", () => + socket.write( + "GET /service/v1/capabilities HTTP/1.1\r\nHost: localhost\r\nContent-Length: 10000000\r\nConnection: keep-alive\r\n\r\n", + ), + ); + }); + expect(result).to.contain("HTTP/1.1 400"); + expect(result.toLowerCase()).to.contain("connection: close"); + expect(downstream).to.equal(0); + }); if (process.env.RUN_REVIEW_SERVICE_BENCHMARK === "1") it("records bounded loopback read performance", async () => { await start();