From eaa84e3af58256ed2622e9ef6178b0afaa57a080 Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Sun, 27 Sep 2026 12:51:28 +0000 Subject: [PATCH 1/3] Add optional authenticated review-service capabilities endpoint --- docs/review-service-capabilities.md | 74 ++++++ src/server/index.ts | 3 + src/server/service/review-capabilities.ts | 202 ++++++++++++++++ test/review-capabilities.test.js | 281 ++++++++++++++++++++++ 4 files changed, 560 insertions(+) create mode 100644 docs/review-service-capabilities.md create mode 100644 src/server/service/review-capabilities.ts create mode 100644 test/review-capabilities.test.js diff --git a/docs/review-service-capabilities.md b/docs/review-service-capabilities.md new file mode 100644 index 0000000..97bbc3d --- /dev/null +++ b/docs/review-service-capabilities.md @@ -0,0 +1,74 @@ +# Review service capabilities + +`GET /service/v1/capabilities` is an optional server-to-server endpoint for the +`4open.artifacts/1` contract. It returns the authenticated client ID, the supported +contract version, and an empty `available` list. Linking, snapshots, protected +files, retention, publication, entitlements and usage are not advertised. Clients +must decline those workflows until their required capabilities are implemented. + +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 +permission; they have `no-store` and `no-referrer` headers. No database, queue or +provider call is involved. + +## Private configuration + +Leave `REVIEW_SERVICE_KEYS` unset until activation is approved. If configured, +its value is a JSON object with exactly `version: 1` and `keys`. Each key has +exactly these fields: + +- `clientId`: 32 lowercase hexadecimal characters. +- `keyId`: 1–64 ASCII letters, digits, underscores or hyphens. +- `tokenSHA256`: SHA-256 of the **ASCII 64-character lowercase hexadecimal bearer + token**, stored as 64 lowercase hexadecimal characters. Do not hash the decoded + token bytes and do not store a plaintext token in this registration. +- `notBefore` and `notAfter`: UTC timestamps, `YYYY-MM-DDTHH:mm:ssZ`, in 2000–2099. + The start is inclusive and the end exclusive. + +Use private runtime configuration outside Git. The caller sends +`Authorization: Bearer `, `X-4open-Artifact-Client-Id` and +`X-4open-Artifact-Service-Key-Id`. Keys grant only capabilities reads. Generate a +separate token for this operation and environment; never reuse GitHub credentials, +webhook secrets or future exchange credentials. The Go review client requires +HTTPS with a trusted server certificate; a deployment must provide that transport. + +The registry is bounded to 64 KiB, 64 keys, 32 clients and 16 keys per client. +Duplicate client/key pairs and reused digests are rejected. Unknown fields, +plaintext secret fields, invalid timestamps and empty configuration fail startup +with a generic diagnostic. Configuration uses ordinary JSON object parsing. +Credentials are copied into a private registry and compared by constant-time +SHA-256 digest equality. Overlapping validity windows permit rotation. Remove a +key and restart to revoke it; this is not hot reloading. + +Authenticated clients share 60 reads per minute across their keys. Counters are +bounded by registered clients, reset lazily, and have no timer. Limits are local +to each server process; they are not a distributed quota. A 429 response supplies +a typed failure and a bounded retry delay. This endpoint does not accept browser +session or repository-owner authority. + +## Verification + +`mocha --no-config --no-package test/review-capabilities.test.js` passes seven +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 +`RUN_REVIEW_SERVICE_BENCHMARK=1` for a 150-request synthetic loopback measurement. +The recorded run averaged 1.708 ms, median 1.657 ms and p95 1.974 ms with fresh +HTTP connections. These are local tooling measurements, not production latency. + +A local HTTPS interoperability rehearsal uses the existing Go +`artifactclient.NewCapabilitiesReader` at review source `ee07d06`. It verifies +certificate trust, accepts the empty capability response, rejects both unsupported +access policies and rejects a wrong credential. No external provider is contacted. + +The isolated new module passes strict TypeScript checking. The repository-wide +check fails with the same diagnostics on unchanged base `083544b`: ES6 does not +support existing named regex groups, an existing Octokit response type is missing +`retryCount`, and the existing library target lacks `String.at`. These are retained +as baseline limitations; this change does not claim a passing full upstream build. + +The endpoint is implemented and tested locally. Upstream deployment, private key +distribution, ownership consent and the remaining artifact workflows are pending. diff --git a/src/server/index.ts b/src/server/index.ts index eac73e1..158b0e4 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -38,6 +38,8 @@ import config from "../config"; import { resolveTrustProxy, isCloudflareIP } from "./trustProxy"; import { createLogger, serializeError } from "../core/logger"; +import { createReviewCapabilities } from "./service/review-capabilities"; + const logger = createLogger("server"); // Lazily build the templated index.html on first request so the server @@ -101,6 +103,7 @@ function indexResponse(req: express.Request, res: express.Response) { export default async function start() { const app = express(); app.set("query parser", "extended"); + app.use("/service", createReviewCapabilities(process.env.REVIEW_SERVICE_KEYS)); app.use("/github/app/webhook", githubAppWebhook); app.use(express.json()); // Preserve the empty body used by API validation when no JSON was parsed. diff --git a/src/server/service/review-capabilities.ts b/src/server/service/review-capabilities.ts new file mode 100644 index 0000000..477a021 --- /dev/null +++ b/src/server/service/review-capabilities.ts @@ -0,0 +1,202 @@ +import { createHash, randomBytes, timingSafeEqual } from "crypto"; +import { RequestHandler } from "express"; + +const contract = "4open.artifacts/1"; +const clientPattern = /^[a-f0-9]{32}$/; +const keyPattern = /^[A-Za-z0-9_-]{1,64}$/; +const secretPattern = /^[a-f0-9]{64}$/; +type Key = { clientId: string; digest: Buffer; from: number; until: number }; + +function invalid(): never { + // Never include private configuration or request values in diagnostic errors. + throw new Error("Invalid REVIEW_SERVICE_KEYS configuration"); +} +function record(value: unknown, fields: string[]): Record { + if (!value || typeof value !== "object" || Array.isArray(value)) invalid(); + const data = value as Record; + if ( + Object.keys(data).length !== fields.length || + fields.some((k) => !Object.prototype.hasOwnProperty.call(data, k)) + ) + invalid(); + return data; +} +function timestamp(value: unknown): number { + if ( + typeof value !== "string" || + !/^20[0-9]{2}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$/.test(value) + ) + invalid(); + const time = Date.parse(value); + if ( + !Number.isFinite(time) || + new Date(time).toISOString() !== value.replace("Z", ".000Z") + ) + invalid(); + return time; +} +function registry(raw: string): Map { + if (Buffer.byteLength(raw, "utf8") > 65536) invalid(); + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + invalid(); + } + const config = record(parsed, ["version", "keys"]); + if ( + config.version !== 1 || + !Array.isArray(config.keys) || + config.keys.length < 1 || + config.keys.length > 64 + ) + invalid(); + const keys = new Map(), + digests = new Set(), + clients = new Map(); + for (const entry of config.keys) { + const k = record(entry, [ + "clientId", + "keyId", + "tokenSHA256", + "notBefore", + "notAfter", + ]); + if ( + typeof k.clientId !== "string" || + !clientPattern.test(k.clientId) || + typeof k.keyId !== "string" || + !keyPattern.test(k.keyId) || + typeof k.tokenSHA256 !== "string" || + !secretPattern.test(k.tokenSHA256) + ) + invalid(); + const from = timestamp(k.notBefore), + until = timestamp(k.notAfter); + const id = k.clientId + ":" + k.keyId; + const count = (clients.get(k.clientId) || 0) + 1; + if ( + until <= from || + keys.has(id) || + digests.has(k.tokenSHA256) || + count > 16 + ) + invalid(); + clients.set(k.clientId, count); + if (clients.size > 32) invalid(); + digests.add(k.tokenSHA256); + keys.set(id, { + clientId: k.clientId, + digest: Buffer.from(k.tokenSHA256, "hex"), + from, + until, + }); + } + return keys; +} + +/** Optional server-to-server read-only endpoint. No browser, owner or repository authority is inferred. */ +export function createReviewCapabilities( + raw?: string, + now: () => number = Date.now, +): RequestHandler { + const keys = raw === undefined ? null : registry(raw); + // One bounded counter per registered client, shared by its rotation keys. + const rates = new Map(); + const dummyDigest = Buffer.alloc(32); + return (req, res) => { + res.setHeader("Cache-Control", "no-store"); + res.setHeader("Referrer-Policy", "no-referrer"); + res.setHeader("X-Content-Type-Options", "nosniff"); + const fail = (status: number, code: string, retry?: number) => { + if (retry) res.setHeader("Retry-After", String(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"); + if (req.method !== "GET") return fail(405, "invalid-request"); + // Reject browser context and bodies before authentication; never read or log them. + if ( + [ + "cookie", + "origin", + "referer", + "sec-fetch-site", + "sec-fetch-mode", + "sec-fetch-dest", + "sec-fetch-user", + ].some((h) => req.headers[h] !== undefined) + ) + return fail(403, "forbidden"); + if ( + req.headers["transfer-encoding"] !== undefined || + req.headers["content-type"] !== undefined || + (req.headers["content-length"] !== undefined && + req.headers["content-length"] !== "0") + ) + return fail(400, "invalid-request"); + const one = (name: string): string | undefined => { + let count = 0; + for (let i = 0; i < req.rawHeaders.length; i += 2) + if (req.rawHeaders[i].toLowerCase() === name) count++; + const value = req.headers[name]; + return count === 1 && typeof value === "string" ? value : undefined; + }; + const client = one("x-4open-artifact-client-id"), + keyId = one("x-4open-artifact-service-key-id"), + authorization = one("authorization"); + if ( + !client || + !clientPattern.test(client) || + !keyId || + !keyPattern.test(keyId) || + !authorization || + !/^Bearer [a-f0-9]{64}$/i.test(authorization) || + !secretPattern.test(authorization.slice(7)) + ) + return fail(401, "unauthorized"); + const key = keys.get(client + ":" + keyId); + const digest = createHash("sha256") + .update(authorization.slice(7), "ascii") + .digest(); + const equal = timingSafeEqual(digest, key ? key.digest : dummyDigest); + const time = now(); + if ( + !key || + !equal || + !Number.isFinite(time) || + time < key.from || + time >= key.until + ) + return fail(401, "unauthorized"); + const minute = Math.floor(time / 60000); + let rate = rates.get(client); + if (!rate || rate.minute !== minute) { + rate = { minute, count: 0 }; + rates.set(client, rate); + } + if (rate.count >= 60) + return fail( + 429, + "rate-limited", + Math.max(1, Math.ceil((60000 - (time % 60000)) / 1000)), + ); + rate.count++; + // No link, snapshot, protected-delivery or retention support exists yet. + res + .status(200) + .json({ + contract, + clientId: client, + contracts: [contract], + available: [], + }); + }; +} diff --git a/test/review-capabilities.test.js b/test/review-capabilities.test.js new file mode 100644 index 0000000..2c4e44d --- /dev/null +++ b/test/review-capabilities.test.js @@ -0,0 +1,281 @@ +require("ts-node/register/transpile-only"); +const { expect } = require("chai"); +const { createHash } = require("crypto"); +const http = require("http"); +const process = require("process"); +const express = require("express"); +const { + createReviewCapabilities, +} = require("../src/server/service/review-capabilities"); +const client = "a".repeat(32), + token = "b".repeat(64); +const digest = (value) => + createHash("sha256").update(value, "ascii").digest("hex"); +const key = (overrides = {}) => ({ + clientId: client, + keyId: "rotation-1", + tokenSHA256: digest(token), + notBefore: "2026-01-01T00:00:00Z", + notAfter: "2027-01-01T00:00:00Z", + ...overrides, +}); +const config = (keys) => JSON.stringify({ version: 1, keys }); +const headers = { + "X-4open-Artifact-Client-Id": client, + "X-4open-Artifact-Service-Key-Id": "rotation-1", + Authorization: "Bearer " + token, +}; +describe("review service capabilities", function () { + let server, port, time, downstream; + async function start(raw = config([key()])) { + time = Date.parse("2026-06-01T00:00:00Z"); + downstream = 0; + const app = express(); + app.use( + "/service", + createReviewCapabilities(raw, () => time), + ); + app.use((_req, res) => { + downstream++; + res.setHeader("Set-Cookie", "browser=1"); + res.send("browser"); + }); + server = http.createServer(app); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + port = server.address().port; + } + function request(options = {}) { + return new Promise((resolve, reject) => { + const req = http.request( + { + hostname: "127.0.0.1", + port, + path: "/service/v1/capabilities", + method: "GET", + headers, + ...options, + }, + (res) => { + const parts = []; + res.on("data", (data) => parts.push(data)); + res.on("end", () => + resolve({ + status: res.statusCode, + headers: res.headers, + text: Buffer.concat(parts).toString(), + }), + ); + }, + ); + req.on("error", reject); + req.end(options.body); + }); + } + afterEach(async () => { + if (server) { + await new Promise((resolve) => server.close(resolve)); + server = undefined; + } + }); + it("is disabled without a registry and bypasses browser middleware", async () => { + // Explicitly omit configuration, rather than start()'s test default. + const app = express(); + app.use("/service", createReviewCapabilities()); + app.use((_q, r) => r.send("browser")); + server = app.listen(0, "127.0.0.1"); + await new Promise((resolve) => server.once("listening", resolve)); + port = server.address().port; + const result = await request(); + expect(result.status).to.equal(404); + expect(result.headers["set-cookie"]).to.equal(undefined); + }); + it("advertises only the supported contract, bound to the authenticated client", async () => { + await start(); + const result = await request(); + expect(result.status).to.equal(200); + expect(JSON.parse(result.text)).to.deep.equal({ + contract: "4open.artifacts/1", + clientId: client, + contracts: ["4open.artifacts/1"], + available: [], + }); + expect(result.headers["cache-control"]).to.equal("no-store"); + expect(result.headers["referrer-policy"]).to.equal("no-referrer"); + expect(result.headers["set-cookie"]).to.equal(undefined); + expect(result.headers["access-control-allow-origin"]).to.equal(undefined); + expect(downstream).to.equal(0); + }); + it("rejects missing, wrong, duplicated and cross-client credentials without reflecting secrets", async () => { + await start(); + for (const input of [ + {}, + { ...headers, Authorization: "Bearer " + "c".repeat(64) }, + { ...headers, "X-4open-Artifact-Client-Id": "d".repeat(32) }, + { ...headers, "X-4open-Artifact-Service-Key-Id": "unknown" }, + { + ...headers, + Authorization: [headers.Authorization, headers.Authorization], + }, + { ...headers, "X-4open-Artifact-Client-Id": [client, client] }, + { + ...headers, + "X-4open-Artifact-Service-Key-Id": ["rotation-1", "rotation-1"], + }, + ]) { + const r = await request({ headers: input }); + expect(r.status).to.equal(401); + const failure = JSON.parse(r.text); + expect(failure.code).to.equal("unauthorized"); + expect(failure.requestId).to.match(/^[a-f0-9]{32}$/); + expect(r.text).not.to.contain(token); + expect(r.text).not.to.contain(client); + } + expect(downstream).to.equal(0); + }); + it("has exact paths and rejects browser context, methods, queries and bodies", async () => { + await start(); + for (const path of [ + "/service/v1/capabilities/", + "/service/v1/capabilities?code=secret", + "/service/v1/%63apabilities", + "/service/v1/bindings", + "/service/V1/capabilities", + ]) + expect((await request({ path })).status, path).to.equal(404); + for (const method of ["POST", "PUT", "OPTIONS", "HEAD"]) + expect((await request({ method })).status).to.equal(405); + for (const name of [ + "Cookie", + "Origin", + "Referer", + "Sec-Fetch-Site", + "Sec-Fetch-Mode", + "Sec-Fetch-Dest", + "Sec-Fetch-User", + ]) + expect( + (await request({ headers: { ...headers, [name]: "browser" } })).status, + ).to.equal(403); + expect( + ( + await request({ + headers: { ...headers, "Content-Type": "application/json" }, + }) + ).status, + ).to.equal(400); + expect( + ( + await request({ + headers: { ...headers, "Content-Length": "2" }, + body: "{}", + }) + ).status, + ).to.equal(400); + expect(downstream).to.equal(0); + }); + it("supports overlapping keys with inclusive start, exclusive expiry and restart revocation", async () => { + const other = "c".repeat(64); + await start( + config([key(), key({ keyId: "rotation-2", tokenSHA256: digest(other) })]), + ); + time = Date.parse(key().notBefore) - 1; + expect((await request()).status).to.equal(401); + time++; + expect((await request()).status).to.equal(200); + expect( + ( + await request({ + headers: { + ...headers, + "X-4open-Artifact-Service-Key-Id": "rotation-2", + Authorization: "Bearer " + other, + }, + }) + ).status, + ).to.equal(200); + time = Date.parse(key().notAfter); + expect((await request()).status).to.equal(401); + await new Promise((resolve) => server.close(resolve)); + server = undefined; + await start( + config([key({ keyId: "rotation-2", tokenSHA256: digest(other) })]), + ); + expect((await request()).status).to.equal(401); + }); + it("limits reads per client across keys and resets without allocating anonymous counters", async () => { + const other = "c".repeat(64); + await start( + config([key(), key({ keyId: "rotation-2", tokenSHA256: digest(other) })]), + ); + const results = await Promise.all( + Array.from({ length: 60 }, () => request()), + ); + expect(results.every((r) => r.status === 200)).to.equal(true); + const r = await request({ + headers: { + ...headers, + "X-4open-Artifact-Service-Key-Id": "rotation-2", + Authorization: "Bearer " + other, + }, + }); + expect(r.status).to.equal(429); + expect(r.headers["retry-after"]).to.equal("60"); + expect(JSON.parse(r.text).retryAfterSeconds).to.equal(60); + time += 60000; + expect((await request()).status).to.equal(200); + }); + it("validates bounded digest-only closed configuration without leaking it", () => { + const bad = [ + "", + "{", + "x".repeat(65537), + "null", + "[]", + config([]), + JSON.stringify({ version: 2, keys: [key()] }), + config([key({ secret: token })]), + config([key({ tokenSHA256: token.toUpperCase() })]), + config([key(), key()]), + config([key(), key({ keyId: "other" })]), + config([key({ notAfter: key().notBefore })]), + config([key({ notBefore: "2026-02-30T00:00:00Z" })]), + config([key({ notBefore: "2026-01-01T01:00:00+01:00" })]), + config([key({ clientId: "../client" })]), + config([key({ keyId: "a:b" })]), + config( + Array.from({ length: 17 }, (_, i) => + key({ keyId: "k" + i, tokenSHA256: digest(String(i)) }), + ), + ), + ]; + for (const raw of bad) + expect(() => createReviewCapabilities(raw)).to.throw( + "Invalid REVIEW_SERVICE_KEYS configuration", + ); + }); + if (process.env.RUN_REVIEW_SERVICE_BENCHMARK === "1") + it("records bounded loopback read performance", async () => { + await start(); + const samples = []; + for (let i = 0; i < 150; i++) { + if (i % 50 === 0) time += 60000; + const begin = process.hrtime.bigint(); + const r = await request(); + expect(r.status).to.equal(200); + samples.push(Number(process.hrtime.bigint() - begin) / 1e6); + } + samples.sort((a, b) => a - b); + const report = { + requests: samples.length, + transport: "loopback HTTP, fresh connections, synthetic credentials", + meanMs: samples.reduce((a, b) => a + b, 0) / samples.length, + medianMs: samples[75], + p95Ms: samples[142], + providerCalls: false, + }; + require("fs").writeFileSync( + "/tmp/upstream-capabilities-perf.json", + JSON.stringify(report, null, 2) + "\n", + ); + }); +}); From 31dfe0f60431011dd3615c6ab4cfff95922f42a0 Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Sun, 27 Sep 2026 13:03:02 +0000 Subject: [PATCH 2/3] Record clean-install upstream build and full test results --- docs/review-service-capabilities.md | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/docs/review-service-capabilities.md b/docs/review-service-capabilities.md index 97bbc3d..e52b9c0 100644 --- a/docs/review-service-capabilities.md +++ b/docs/review-service-capabilities.md @@ -56,7 +56,7 @@ 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 `RUN_REVIEW_SERVICE_BENCHMARK=1` for a 150-request synthetic loopback measurement. -The recorded run averaged 1.708 ms, median 1.657 ms and p95 1.974 ms with fresh +The clean-install run averaged 2.035 ms, median 1.732 ms and p95 2.417 ms with fresh HTTP connections. These are local tooling measurements, not production latency. A local HTTPS interoperability rehearsal uses the existing Go @@ -64,11 +64,15 @@ A local HTTPS interoperability rehearsal uses the existing Go certificate trust, accepts the empty capability response, rejects both unsupported access policies and rejects a wrong credential. No external provider is contacted. -The isolated new module passes strict TypeScript checking. The repository-wide -check fails with the same diagnostics on unchanged base `083544b`: ES6 does not -support existing named regex groups, an existing Octokit response type is missing -`retryCount`, and the existing library target lacks `String.at`. These are retained -as baseline limitations; this change does not claim a passing full upstream build. +A clean `npm ci` install, the full TypeScript check and emitted build, UI build, +and targeted ESLint pass. The full test suite reports 700 passing in 36 seconds +and 49 pending opt-in cases; those skipped cases are not claimed as tested. +The capability suite separately passes eight cases including its benchmark. + +An initial check used dependencies copied from another checkout and produced +TypeScript diagnostics. Those installed versions differed from the lockfile; +reinstalling the locked dependencies resolved every diagnostic without source +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. 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 3/3] 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();