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 01/48] 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 02/48] 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 03/48] 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(); From b776ff7e5abcc58d875d7d3b439edfde51a80c7f Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Sun, 27 Sep 2026 13:50:20 +0000 Subject: [PATCH 04/48] Add bounded authenticated review intent HTTPS consumer --- docs/review-intent-client.md | 20 + src/server/service/review-intent-client.ts | 412 +++++++++++++++++++++ test/fixtures/review-intent-client.js | 46 +++ test/review-intent-client.test.js | 361 ++++++++++++++++++ 4 files changed, 839 insertions(+) create mode 100644 docs/review-intent-client.md create mode 100644 src/server/service/review-intent-client.ts create mode 100644 test/fixtures/review-intent-client.js create mode 100644 test/review-intent-client.test.js diff --git a/docs/review-intent-client.md b/docs/review-intent-client.md new file mode 100644 index 0000000..8c2fcd6 --- /dev/null +++ b/docs/review-intent-client.md @@ -0,0 +1,20 @@ +# Review intent consumption client + +`src/server/service/review-intent-client.ts` consumes a saved review-side link intent over authenticated HTTPS. The caller supplies a trusted, exact HTTPS origin, a client-bound service credential with a validity window, and an explicit request ID. The returned policy and opaque identifiers come from the authenticated review response. Browser-supplied access, retention and callback fields are rejected. + +The factory returns only `consume`; it copies credential values into a closure. A call makes one POST to `/service/v1/artifact-intents/consume`, validates the peer certificate with Node's trust store, and sends no browser cookies or origin. Redirects and automatic retries are disabled. An uncertain result requires the caller to retain the same request ID for an explicit retry. + +Limits are four active attempts, a ten-second total deadline, five-second inactivity timeout, 16 KiB response headers and 64 KiB body. An AbortSignal cancels the attempt. Invalid credentials or payloads fail before I/O. Responses must match the configured client and requested intent, use the v1 closed schema and valid future timestamps, and omit cookies and content encoding. Duplicate JSON keys, including escaped spellings, invalid UTF-8 and unknown fields fail. Errors expose only a fixed category; remote bodies and credentials are not included. + +This module is not wired into a route or configured in production. It does not authenticate an artifact owner, obtain consent, create a binding, exchange a completion code, or establish browser callback authority. Those are separate pending tasks. Credential distribution and upstream deployment also remain pending. + +## Validation + +Local tests use a disposable certificate and loopback HTTPS server. A child process loads that certificate through `NODE_EXTRA_CA_CERTS` at startup; an independent child with ordinary trust rejects it before sending credentials. No TLS bypass is used. Tests cover successful policy reads, copied configuration, request-ID replay, rejection before I/O, status classification without retry, malformed and oversized responses, duplicate keys, four-slot saturation and recovery, cancellation, credential expiry, inactivity and slow-stream total deadlines. + +- `npx mocha test/review-intent-client.test.js`: 11 behavioral cases; one performance case is opt-in. +- `npm test`: 712 pass, 50 pending opt-in or environment-dependent cases. +- Full TypeScript check and targeted ESLint pass. +- Opt-in 100-request performance run: fresh loopback TLS connections, mean 12.36 ms, median 12 ms, p95 15 ms. These are local synthetic timings, not an external-service latency promise. + +Run the measured test with `TEST_REVIEW_INTENT_PERF=1 TEST_REVIEW_INTENT_PERF_REPORT=/tmp/review-intent-perf.json npx mocha test/review-intent-client.test.js`. diff --git a/src/server/service/review-intent-client.ts b/src/server/service/review-intent-client.ts new file mode 100644 index 0000000..4e918a2 --- /dev/null +++ b/src/server/service/review-intent-client.ts @@ -0,0 +1,412 @@ +import * as https from "https"; +import { ClientRequest } from "http"; + +const contract = "4open.artifacts/1", + limit = 65536; +const opaque = /^[a-f0-9]{32}$/, + secret = /^[a-f0-9]{64}$/; +export type IntentClientFailure = + | "invalid" + | "busy" + | "unavailable" + | "rejected" + | "conflict" + | "expired" + | "not-found" + | "protocol"; +export class IntentClientError extends Error { + constructor(public readonly kind: IntentClientFailure) { + super("Review intent consumption " + kind); + this.name = "IntentClientError"; + } +} +export interface ReviewIntentClientConfig { + origin: string; + clientId: string; + keyId: string; + token: string; + notBefore: string; + notAfter: string; +} +export interface IntentConsumption { + contract: string; + clientId: string; + intentId: string; + token: string; + requestId: string; +} +export type ConsumedReviewIntent = Readonly<{ + contract: string; + clientId: string; + intentId: string; + submissionRef: string; + callbackId: string; + entitlementId: string; + expiresAt: string; + policy: Readonly<{ + version: number; + access: "anonymous-link" | "restricted-review"; + retainUntil: string; + }>; +}>; +function fail(kind: IntentClientFailure): never { + throw new IntentClientError(kind); +} +function object(value: unknown, fields: string[]): Record { + if (!value || typeof value !== "object" || Array.isArray(value)) + fail("protocol"); + const out = value as Record; + if ( + Object.keys(out).length !== fields.length || + fields.some((k) => !Object.prototype.hasOwnProperty.call(out, k)) + ) + fail("protocol"); + return out; +} +function instant(value: unknown): number { + if ( + typeof value !== "string" || + !/^2[01][0-9]{2}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$/.test(value) + ) + fail("protocol"); + const time = Date.parse(value); + if ( + !Number.isFinite(time) || + new Date(time).toISOString() !== value.replace("Z", ".000Z") + ) + fail("protocol"); + return time; +} +function id(value: unknown): value is string { + return typeof value === "string" && opaque.test(value); +} + +// JSON.parse checks syntax; this bounded walk additionally rejects duplicate +// decoded keys, including escaped spellings, before typed field validation. +function decode(bytes: Buffer): unknown { + if (bytes.length > limit) fail("protocol"); + const text = bytes.toString("utf8"); + if (!Buffer.from(text, "utf8").equals(bytes)) fail("protocol"); + let parsed: unknown; + try { + parsed = JSON.parse(text); + } catch { + fail("protocol"); + } + let at = 0; + const space = () => { + while (/\s/.test(text[at] || "") && at < text.length) at++; + }; + const string = (): string => { + const start = at++; + while (at < text.length) { + if (text[at] === "\\") { + at += 2; + continue; + } + if (text[at++] === '"') return JSON.parse(text.slice(start, at)); + } + return fail("protocol"); + }; + const walk = (depth: number): void => { + if (depth > 8) fail("protocol"); + space(); + if (text[at] === '"') { + string(); + return; + } + if (text[at] === "{" || text[at] === "[") { + const isObject = text[at++] === "{", + end = isObject ? "}" : "]", + keys = new Set(); + space(); + if (text[at] === end) { + at++; + return; + } + for (;;) { + space(); + if (isObject) { + const key = string(); + if (keys.has(key)) fail("protocol"); + keys.add(key); + space(); + at++; // colon; syntax already checked + } + walk(depth + 1); + space(); + if (text[at++] === end) return; + } + } + while (at < text.length && !/[\s,}\]]/.test(text[at])) at++; + }; + walk(0); + space(); + if (at !== text.length) fail("protocol"); + return parsed; +} +function intent( + bytes: Buffer, + clientId: string, + intentId: string, +): ConsumedReviewIntent { + const p = object(decode(bytes), [ + "contract", + "clientId", + "intentId", + "submissionRef", + "callbackId", + "policy", + "expiresAt", + "entitlementId", + ]); + const policy = object(p.policy, ["version", "access", "retainUntil"]), + now = Date.now(); + if ( + p.contract !== contract || + p.clientId !== clientId || + p.intentId !== intentId || + !id(p.submissionRef) || + !id(p.callbackId) || + !id(p.entitlementId) + ) + fail("protocol"); + if ( + !Number.isSafeInteger(policy.version) || + (policy.version as number) < 1 || + (policy.access !== "anonymous-link" && + policy.access !== "restricted-review") || + instant(policy.retainUntil) <= now + ) + fail("protocol"); + const expires = instant(p.expiresAt); + if (expires <= now || expires > now + 600000) fail("protocol"); + return Object.freeze({ + contract, + clientId, + intentId, + submissionRef: p.submissionRef, + callbackId: p.callbackId, + entitlementId: p.entitlementId, + expiresAt: p.expiresAt as string, + policy: Object.freeze({ + version: policy.version as number, + access: policy.access as "anonymous-link" | "restricted-review", + retainUntil: policy.retainUntil as string, + }), + }); +} + +/** Trusted server configuration only. One attempt per call; the caller retains + * its request ID for an explicit retry. No browser callback or owner authority + * is established by consuming an intent. No credentials are exposed on the + * returned object, through serialization, or in transport errors. */ +export function createReviewIntentConsumer(config: ReviewIntentClientConfig) { + let origin: string, + clientId: string, + keyId: string, + token: string, + from: number, + until: number; + try { + const c = object(config, [ + "origin", + "clientId", + "keyId", + "token", + "notBefore", + "notAfter", + ]); + if ( + typeof c.origin !== "string" || + c.origin.length > 2048 || + !id(c.clientId) || + typeof c.keyId !== "string" || + !/^[A-Za-z0-9_-]{1,64}$/.test(c.keyId) || + typeof c.token !== "string" || + !secret.test(c.token) + ) + fail("invalid"); + const url = new URL(c.origin); + if ( + url.protocol !== "https:" || + url.origin !== c.origin || + url.username || + url.password + ) + fail("invalid"); + origin = c.origin; + clientId = c.clientId; + keyId = c.keyId; + token = c.token; + from = instant(c.notBefore); + until = instant(c.notAfter); + if (until <= from || Date.now() < from || Date.now() >= until) + fail("invalid"); + } catch { + return fail("invalid"); + } + let active = 0; + return Object.freeze({ + consume( + input: IntentConsumption, + signal?: AbortSignal, + ): Promise { + let body: Buffer, expectedIntent: string; + try { + const p = object(input, [ + "contract", + "clientId", + "intentId", + "token", + "requestId", + ]); + if ( + p.contract !== contract || + p.clientId !== clientId || + !id(p.intentId) || + !id(p.requestId) || + typeof p.token !== "string" || + !secret.test(p.token) || + Date.now() < from || + Date.now() >= until + ) + fail("invalid"); + expectedIntent = p.intentId; + body = Buffer.from( + JSON.stringify({ + contract, + clientId, + intentId: expectedIntent, + token: p.token, + requestId: p.requestId, + }), + ); + } catch { + return Promise.reject(new IntentClientError("invalid")); + } + if (signal?.aborted) + return Promise.reject(new IntentClientError("unavailable")); + if (active >= 4) return Promise.reject(new IntentClientError("busy")); + active++; + return new Promise((resolve, reject) => { + let request: ClientRequest | undefined, + settled = false; + const abort = () => finish("unavailable"); + const timer = setTimeout(abort, 10000); + function finish( + error?: IntentClientFailure, + value?: ConsumedReviewIntent, + ) { + if (settled) return; + settled = true; + clearTimeout(timer); + signal?.removeEventListener("abort", abort); + active--; + if (error) { + request?.destroy(); + reject(new IntentClientError(error)); + } else resolve(value!); + } + signal?.addEventListener("abort", abort, { once: true }); + try { + request = https.request( + origin + "/service/v1/artifact-intents/consume", + { + method: "POST", + agent: false, + rejectUnauthorized: true, + minVersion: "TLSv1.2", + maxHeaderSize: 16384, + headers: { + Authorization: "Bearer " + token, + "X-4open-Artifact-Client-Id": clientId, + "X-4open-Artifact-Service-Key-Id": keyId, + "Content-Type": "application/json", + Accept: "application/json", + "Accept-Encoding": "identity", + "Content-Length": body.length, + }, + }, + (response) => { + response.once("aborted", abort); + response.once("error", abort); + const status = response.statusCode || 0; + if (status !== 200) { + const kind: IntentClientFailure = + status === 404 + ? "not-found" + : status === 409 + ? "conflict" + : status === 410 + ? "expired" + : [400, 401, 403, 422].includes(status) + ? "rejected" + : [408, 429, 500, 502, 503, 504].includes(status) + ? "unavailable" + : "protocol"; + finish(kind); + return; + } + const types = response.rawHeaders.filter( + (_v, i) => + i % 2 === 0 && + response.rawHeaders[i].toLowerCase() === "content-type", + ).length; + const length = response.headers["content-length"]; + if ( + types !== 1 || + !/^application\/json(?:\s*;\s*charset=(?:utf-8|"utf-8"))?\s*$/i.test( + response.headers["content-type"] || "", + ) || + response.headers["content-encoding"] !== undefined || + response.headers["set-cookie"] !== undefined || + (length !== undefined && + (!/^[0-9]+$/.test(length) || Number(length) > limit)) + ) { + finish("protocol"); + return; + } + let size = 0; + const chunks: Buffer[] = []; + response.on("data", (chunk: Buffer) => { + size += chunk.length; + if (size > limit) finish("protocol"); + else if (!settled) chunks.push(chunk); + }); + response.once("end", () => { + if (settled) return; + if ( + signal?.aborted || + Date.now() < from || + Date.now() >= until + ) { + finish("unavailable"); + return; + } + try { + finish( + undefined, + intent( + Buffer.concat(chunks, size), + clientId, + expectedIntent, + ), + ); + } catch { + finish("protocol"); + } + }); + }, + ); + request.once("error", abort); + request.setTimeout(5000, abort); + if (signal?.aborted) abort(); + if (!settled) request.end(body); + } catch { + finish("unavailable"); + } + }); + }, + }); +} diff --git a/test/fixtures/review-intent-client.js b/test/fixtures/review-intent-client.js new file mode 100644 index 0000000..d15216b --- /dev/null +++ b/test/fixtures/review-intent-client.js @@ -0,0 +1,46 @@ +require("ts-node/register/transpile-only"); +const process = require("process"); +const { setTimeout } = require("timers"); +const { AbortController } = globalThis; +const { + createReviewIntentConsumer, +} = require("../../src/server/service/review-intent-client"); + +process.on("message", async ({ id, config, input, mode }) => { + try { + const client = createReviewIntentConsumer(config); + const controller = new AbortController(); + if (mode === "preabort") controller.abort(); + if (mode === "abort") setTimeout(() => controller.abort(), 100); + if (mode === "mutate") { + config.token = "invalid"; + config.origin = "https://invalid.example"; + } + if (mode === "parallel") { + const pending = Array.from({ length: 5 }, () => + client.consume(input, controller.signal), + ); + const results = await Promise.allSettled(pending); + const followup = await client.consume(input); + process.send({ + id, + results: results.map((r) => + r.status === "fulfilled" ? "ok" : r.reason.kind, + ), + followup, + }); + return; + } + const started = Date.now(); + const result = await client.consume(input, controller.signal); + process.send({ + id, + result, + elapsedMs: Date.now() - started, + frozen: Object.isFrozen(result) && Object.isFrozen(result.policy), + serialized: JSON.stringify(client), + }); + } catch (error) { + process.send({ id, kind: error.kind, message: error.message }); + } +}); diff --git a/test/review-intent-client.test.js b/test/review-intent-client.test.js new file mode 100644 index 0000000..d7a0571 --- /dev/null +++ b/test/review-intent-client.test.js @@ -0,0 +1,361 @@ +const { expect } = require("chai"); +const https = require("https"); +const fs = require("fs"); +const os = require("os"); +const path = require("path"); +const process = require("process"); +const { fork, execFileSync } = require("child_process"); +const { + setTimeout, + clearTimeout, + setInterval, + clearInterval, +} = require("timers"); +const clientId = "a".repeat(32), + intentId = "c".repeat(32); +const at = (offset) => + new Date(Math.floor((Date.now() + offset) / 1000) * 1000) + .toISOString() + .replace(".000Z", "Z"); +const input = () => ({ + contract: "4open.artifacts/1", + clientId, + intentId, + token: "d".repeat(64), + requestId: "e".repeat(32), +}); +const intent = () => ({ + contract: "4open.artifacts/1", + clientId, + intentId, + submissionRef: "f".repeat(32), + callbackId: "1".repeat(32), + entitlementId: "2".repeat(32), + expiresAt: at(120000), + policy: { + version: 1, + access: "restricted-review", + retainUntil: at(86400000), + }, +}); + +describe("review intent HTTPS client", function () { + this.timeout(20000); + let server, + child, + folder, + origin, + handler, + calls, + serial = 0; + const pending = new Map(); + const config = () => ({ + origin, + clientId, + keyId: "rotation-1", + token: "b".repeat(64), + notBefore: at(-60000), + notAfter: at(3600000), + }); + function call(options = {}) { + const id = ++serial; + return new Promise((resolve, reject) => { + const timer = setTimeout(() => { + pending.delete(id); + reject(new Error("child test timed out")); + }, 18000); + pending.set(id, (value) => { + clearTimeout(timer); + resolve(value); + }); + child.send({ id, config: config(), input: input(), ...options }); + }); + } + before(async function () { + folder = fs.mkdtempSync(path.join(os.tmpdir(), "review-intent-tls-")); + execFileSync( + "openssl", + [ + "req", + "-x509", + "-newkey", + "rsa:2048", + "-nodes", + "-days", + "1", + "-subj", + "/CN=localhost", + "-addext", + "subjectAltName=IP:127.0.0.1,DNS:localhost", + "-keyout", + path.join(folder, "key"), + "-out", + path.join(folder, "cert"), + ], + { stdio: "ignore" }, + ); + server = https.createServer( + { + key: fs.readFileSync(path.join(folder, "key")), + cert: fs.readFileSync(path.join(folder, "cert")), + }, + (req, res) => { + const chunks = []; + req.on("data", (chunk) => chunks.push(chunk)); + req.on("end", () => { + calls.push({ + method: req.method, + url: req.url, + headers: req.headers, + body: JSON.parse(Buffer.concat(chunks).toString()), + }); + handler(req, res); + }); + }, + ); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + origin = "https://127.0.0.1:" + server.address().port; + child = fork(path.join(__dirname, "fixtures/review-intent-client.js"), { + env: { ...process.env, NODE_EXTRA_CA_CERTS: path.join(folder, "cert") }, + stdio: ["ignore", "ignore", "pipe", "ipc"], + }); + child.on("message", (message) => { + const done = pending.get(message.id); + pending.delete(message.id); + if (done) done(message); + }); + }); + after(async function () { + child?.kill(); + server?.closeAllConnections(); + if (server) await new Promise((resolve) => server.close(resolve)); + if (folder) fs.rmSync(folder, { recursive: true, force: true }); + }); + beforeEach(() => { + calls = []; + handler = (_req, res) => { + res.setHeader("Content-Type", "application/json"); + res.end(JSON.stringify(intent())); + }; + }); + it("uses authenticated TLS, the saved policy, and the caller's retry ID", async () => { + const result = await call({ mode: "mutate" }); + expect(result.result.policy.access).to.equal("restricted-review"); + expect(result.frozen).to.equal(true); + expect(result.serialized).to.equal("{}"); + expect(calls).to.have.length(1); + expect(calls[0].body).to.deep.equal(input()); + expect(calls[0].method).to.equal("POST"); + expect(calls[0].url).to.equal("/service/v1/artifact-intents/consume"); + expect(calls[0].headers.authorization).to.equal("Bearer " + "b".repeat(64)); + expect(calls[0].headers["x-4open-artifact-client-id"]).to.equal(clientId); + expect(calls[0].headers["x-4open-artifact-service-key-id"]).to.equal( + "rotation-1", + ); + for (const key of ["cookie", "origin", "referer"]) + expect(calls[0].headers[key]).to.equal(undefined); + await call(); + expect(calls[1].body.requestId).to.equal(calls[0].body.requestId); + }); + it("rejects invalid config and caller data before making a request", async () => { + for (const patch of [ + { origin: origin + "/" }, + { origin: "http://127.0.0.1" }, + { origin: origin + "/path" }, + { token: "bad" }, + { notAfter: at(-1000) }, + { notBefore: at(30000) }, + { clientId: "bad" }, + { extra: "ignored?" }, + ]) + expect((await call({ config: { ...config(), ...patch } })).kind).to.equal( + "invalid", + ); + for (const patch of [ + { clientId: "0".repeat(32) }, + { token: "bad" }, + { requestId: "bad" }, + { policy: {} }, + { contract: "other" }, + ]) + expect((await call({ input: { ...input(), ...patch } })).kind).to.equal( + "invalid", + ); + expect((await call({ mode: "preabort" })).kind).to.equal("unavailable"); + expect(calls).to.have.length(0); + }); + it("classifies failures without following redirects, retrying, or exposing response bodies", async () => { + for (const [status, kind] of [ + [301, "protocol"], + [400, "rejected"], + [401, "rejected"], + [403, "rejected"], + [404, "not-found"], + [409, "conflict"], + [410, "expired"], + [422, "rejected"], + [429, "unavailable"], + [503, "unavailable"], + ]) { + handler = (_req, res) => { + res.writeHead(status, { + Location: origin + "/leak", + "Content-Length": "10000000", + }); + res.write("private response and token"); + }; + const result = await call(); + expect(result.kind).to.equal(kind); + expect(result.message).to.equal("Review intent consumption " + kind); + } + expect(calls).to.have.length(10); + }); + it("rejects mismatched identity, policy, expiry, unknown fields and malformed JSON", async () => { + const good = intent(); + const values = [ + { ...good, clientId: "0".repeat(32) }, + { ...good, intentId: "0".repeat(32) }, + { ...good, callbackId: "bad" }, + { ...good, entitlementId: "bad" }, + { ...good, expiresAt: at(-1000) }, + { ...good, expiresAt: at(700000) }, + { ...good, policy: { ...good.policy, access: "public" } }, + { ...good, policy: { ...good.policy, version: 1.5 } }, + { ...good, policy: { ...good.policy, retainUntil: at(-1000) } }, + { ...good, owner: "browser" }, + ]; + const raw = [ + ...values.map((v) => JSON.stringify(v)), + "null", + "[]", + "{", + JSON.stringify(good).replace( + '"clientId":', + '"clientId":"' + clientId + '","cl\\u0069entId":', + ), + JSON.stringify(good).replace('"version":1', '"version":2,"version":1'), + Buffer.from([0xff]), + " ".repeat(65537), + ]; + for (const body of raw) { + handler = (_req, res) => { + res.setHeader("Content-Type", "application/json"); + res.end(body); + }; + expect((await call()).kind).to.equal("protocol"); + } + }); + it("rejects cookies, encoding, ambiguous content types and oversized declared bodies", async () => { + for (const headers of [ + { "Set-Cookie": "identity=secret" }, + { "Content-Encoding": "gzip" }, + { "Content-Type": "text/html" }, + { "Content-Type": ["application/json", "application/json"] }, + { "Content-Length": "65537" }, + ]) { + handler = (_req, res) => { + res.writeHead(200, { "Content-Type": "application/json", ...headers }); + res.write(JSON.stringify(intent())); + }; + expect((await call()).kind).to.equal("protocol"); + } + }); + it("bounds concurrent attempts and releases capacity after completion", async () => { + handler = (_req, res) => + setTimeout(() => { + res.setHeader("Content-Type", "application/json"); + res.end(JSON.stringify(intent())); + }, 150); + const result = await call({ mode: "parallel" }); + expect(result.results).to.deep.equal(["ok", "ok", "ok", "ok", "busy"]); + expect(result.followup.intentId).to.equal(intentId); + expect(calls).to.have.length(5); + }); + it("cancels an unfinished response without retrying", async () => { + handler = (_req, res) => { + res.writeHead(200, { "Content-Type": "application/json" }); + res.write("{"); + }; + expect((await call({ mode: "abort" })).kind).to.equal("unavailable"); + expect(calls).to.have.length(1); + }); + it("times out an inactive response within its request deadline", async () => { + handler = (_req, res) => { + res.writeHead(200, { "Content-Type": "application/json" }); + res.write("{"); + }; + const start = Date.now(); + expect((await call()).kind).to.equal("unavailable"); + expect(Date.now() - start).to.be.within(4500, 12000); + expect(calls).to.have.length(1); + }); + it("rejects an untrusted TLS certificate before sending credentials", async () => { + const env = { + ...process.env, + NODE_EXTRA_CA_CERTS: "", + NODE_TLS_REJECT_UNAUTHORIZED: "1", + }; + delete env.NODE_OPTIONS; + const untrusted = fork( + path.join(__dirname, "fixtures/review-intent-client.js"), + { env, stdio: ["ignore", "ignore", "pipe", "ipc"] }, + ); + try { + const response = new Promise((resolve) => + untrusted.once("message", resolve), + ); + untrusted.send({ id: 1, config: config(), input: input() }); + expect((await response).kind).to.equal("unavailable"); + expect(calls).to.have.length(0); + } finally { + untrusted.kill(); + } + }); + it("rejects a response received after the credential expires", async () => { + handler = (_req, res) => + setTimeout(() => { + res.setHeader("Content-Type", "application/json"); + res.end(JSON.stringify(intent())); + }, 2100); + expect( + (await call({ config: { ...config(), notAfter: at(2000) } })).kind, + ).to.equal("unavailable"); + expect(calls).to.have.length(1); + }); + it("enforces the total deadline even while the peer streams data", async () => { + handler = (_req, res) => { + res.writeHead(200, { "Content-Type": "application/json" }); + res.write(" "); + const timer = setInterval(() => res.write(" "), 200); + res.once("close", () => clearInterval(timer)); + }; + const start = Date.now(); + expect((await call()).kind).to.equal("unavailable"); + expect(Date.now() - start).to.be.within(9500, 15000); + expect(calls).to.have.length(1); + }); + it("measures repeated fresh TLS connections when explicitly requested", async function () { + if (!process.env.TEST_REVIEW_INTENT_PERF) this.skip(); + const times = []; + for (let n = 0; n < 100; n++) { + const result = await call(); + expect(result.result.intentId).to.equal(intentId); + times.push(result.elapsedMs); + } + times.sort((a, b) => a - b); + const receipt = { + calls: calls.length, + transport: "fresh loopback HTTPS connections", + synthetic: true, + meanMs: times.reduce((a, b) => a + b, 0) / times.length, + medianMs: times[50], + p95Ms: times[94], + }; + expect(calls).to.have.length(100); + if (process.env.TEST_REVIEW_INTENT_PERF_REPORT) + fs.writeFileSync( + process.env.TEST_REVIEW_INTENT_PERF_REPORT, + JSON.stringify(receipt, null, 2) + "\n", + ); + }); +}); From 5485f57e0292421e2279f68c6582b5a5cf35ab3b Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Sun, 27 Sep 2026 14:16:57 +0000 Subject: [PATCH 05/48] Retain session-bound artifact owner consent under current authority --- .github/workflows/ci.yml | 38 ++ docs/review-owner-consent.md | 23 + .../anonymizedRepositories.schema.ts | 1 + src/core/model/users/users.schema.ts | 1 + src/server/service/review-owner-consent.ts | 326 +++++++++++ test/review-owner-consent.test.js | 509 ++++++++++++++++++ 6 files changed, 898 insertions(+) create mode 100644 docs/review-owner-consent.md create mode 100644 src/server/service/review-owner-consent.ts create mode 100644 test/review-owner-consent.test.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 46a2a9d..be4c47a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,3 +30,41 @@ jobs: - run: npm ci - run: npm run build:ui - run: npm test + + review-consent: + name: Review consent transactions + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: npm + - run: npm ci + - run: npx tsc --noEmit + - name: Start disposable MongoDB replica set + run: | + docker run -d --name review-consent-test --network host --memory=512m --cpus=1 --tmpfs /data/db:rw,noexec,nosuid,size=256m mongo:7.0@sha256:0e145625e78b94224d16222ff2609c4621ff6e2c390300e4e6bf698305596792 mongod --bind_ip 127.0.0.1 --port 28743 --replSet review-consent-test --oplogSize 16 + for attempt in $(seq 1 40); do + if docker exec review-consent-test mongosh --quiet --port 28743 --eval 'db.adminCommand({ping:1})' > /dev/null 2>&1; then break; fi + sleep 1 + done + docker exec review-consent-test mongosh --quiet --port 28743 --eval 'rs.initiate({_id:"review-consent-test",members:[{_id:0,host:"127.0.0.1:28743"}]})' + for attempt in $(seq 1 40); do + if docker exec review-consent-test mongosh --quiet --port 28743 --eval 'if (!db.hello().isWritablePrimary) quit(1)' > /dev/null 2>&1; then exit 0; fi + sleep 1 + done + exit 1 + - name: Verify consent authority, concurrency and replay cost + env: + TEST_REVIEW_CONSENT_MONGO: mongodb://127.0.0.1:28743/?replicaSet=review-consent-test + TEST_REVIEW_CONSENT_PERF_REPORT: review-consent-perf.json + run: npx mocha test/review-owner-consent.test.js + - uses: actions/upload-artifact@v4 + with: + name: review-consent-performance + path: review-consent-perf.json + - name: Remove disposable database + if: always() + run: docker rm -f review-consent-test || true diff --git a/docs/review-owner-consent.md b/docs/review-owner-consent.md new file mode 100644 index 0000000..7dc9105 --- /dev/null +++ b/docs/review-owner-consent.md @@ -0,0 +1,23 @@ +# Review owner consent backend + +`createReviewOwnerConsent` prepares a signed policy preview from the authenticated review intent consumer and records explicit access/retention acceptance in MongoDB. It is an unwired backend. Browser routes, CSRF protection, UI, completion-code exchange and activation remain unfinished. Capabilities continue to advertise no available integration features. + +The caller must provide an account ID and session ID derived from a current authenticated browser session. Never populate this principal from request JSON or treat a service credential as an owner's identity. The factory also needs the configured intent consumer, a dedicated random 32-byte signing key held outside Git, and a connected MongoDB replica set. Configure bounded server-selection and socket timeouts on that connection. Tests use MongoDB 7 with a three-second server-selection timeout and ten-second socket timeout. + +Preview checks the current account and repository before consuming the review intent, then checks them again afterward. The network request holds no MongoDB transaction or authority lock. The account must be active or have the legacy unset active status; the repository must be ready and unexpired. Ownership uses the current account ObjectId. Coauthors must match the account's current numeric GitHub ID. Legacy username-only coauthors and unrelated administrators cannot provide consent. + +The signed preview binds the saved intent and exact policy to the account, hashed session ID and repository ObjectId. Changing the account, session, repository or policy invalidates confirmation. The signing context is specific to this protocol; key rotation invalidates outstanding previews. Confirmation requires both access and retention acceptance and an explicit 32-hex request ID. Preview and replay expire with the intent or retention policy. + +Confirmation writes one immutable `review_owner_consents` record per client/intent. It records the account, repository, authority role, exact policy, request ID, confirmation time and a hash of the signed preview. It stores no raw session ID, intent token, service credential or preview ticket. Explicit retries return the same receipt only after current authority checks; another request ID or repository conflicts. No binding is created by accepting a policy. + +Transactions touch the current account and repository before writing or reading the receipt. Ownership transfer, coauthor removal and account disablement therefore serialize with confirmation. The private `reviewConsentRevision` fields are excluded from ordinary Mongoose reads. Four transactions may be active per factory. Database commands and commits have limits; transactions are not automatically retried. A conflict or uncertain commit returns a fixed `unavailable` error, and the caller must retain the same ticket and request ID for explicit recovery. + +The final binding flow must still revalidate the initiating review session and saved intent scope with the review service. This backend proves current upstream owner/coauthor consent, not continuing review-side author authority, immutable snapshots, restricted file access or preservation. Receipt retention/export and the HTTP adapter remain separate work; this change enables no provider or public route. + +## Validation + +The opt-in suite runs against a disposable MongoDB replica set. It covers literal acceptance, immutable replay, stable-ID coauthors, administrator/username rejection, ticket tampering, account/session binding, disabled accounts, ownership changes, removed coauthors, archived/expired repositories, permission changes during upstream I/O, concurrent confirmations, expiry, second-repository conflicts and authority-field privacy. A held ownership-change transaction forces a real write conflict; the explicit retry is denied and no receipt is created. + +Run `TEST_REVIEW_CONSENT_MONGO='mongodb://127.0.0.1:28743/?replicaSet=review-consent-test' npx mocha test/review-owner-consent.test.js`. Set `TEST_REVIEW_CONSENT_PERF_REPORT=/tmp/review-consent-perf.json` to include 100 durable receipt replays. The CI job starts and removes its own pinned MongoDB image and preserves the performance report. Local timings are synthetic and are not a production latency guarantee. + +Local validation passes 12 targeted cases with the benchmark enabled, 723 full-suite cases with 51 opt-in/environment-dependent cases pending, the full TypeScript check and targeted lint. The final 100-replay run measured mean 12.85 ms, median 12.82 ms and p95 14.89 ms against a one-CPU, 512 MiB disposable database. An earlier performance attempt encountered a database conflict while the privacy fixture automatically built indexes; the final fixture disables automatic schema/index creation and creates its collections explicitly. diff --git a/src/core/model/anonymizedRepositories/anonymizedRepositories.schema.ts b/src/core/model/anonymizedRepositories/anonymizedRepositories.schema.ts index e7ac0c3..5be6492 100644 --- a/src/core/model/anonymizedRepositories/anonymizedRepositories.schema.ts +++ b/src/core/model/anonymizedRepositories/anonymizedRepositories.schema.ts @@ -2,6 +2,7 @@ import { repositoryAccessSchema } from "../repository-access.schema"; import { Schema } from "mongoose"; const AnonymizedRepositorySchema = new Schema({ + reviewConsentRevision: { type: Number, select: false }, repoId: { type: String, index: { unique: true, collation: { locale: "en", strength: 2 } }, diff --git a/src/core/model/users/users.schema.ts b/src/core/model/users/users.schema.ts index be84b78..cc852e0 100644 --- a/src/core/model/users/users.schema.ts +++ b/src/core/model/users/users.schema.ts @@ -1,6 +1,7 @@ import { Schema } from "mongoose"; const UserSchema = new Schema({ + reviewConsentRevision: { type: Number, select: false }, accessTokens: { github: { type: String, select: false }, }, diff --git a/src/server/service/review-owner-consent.ts b/src/server/service/review-owner-consent.ts new file mode 100644 index 0000000..4897fe8 --- /dev/null +++ b/src/server/service/review-owner-consent.ts @@ -0,0 +1,326 @@ +import { createHash, createHmac, timingSafeEqual } from "crypto"; +import { Connection, ClientSession, Types } from "mongoose"; +import { + ConsumedReviewIntent, + IntentConsumption, + createReviewIntentConsumer, +} from "./review-intent-client"; + +type Principal = { accountId: string; sessionId: string }; +type Authority = { + accountId: string; + repositoryId: string; + repositoryName: string; + role: "owner" | "coauthor"; +}; +type Quote = { + version: 1; + accountId: string; + sessionHash: string; + repositoryId: string; + intent: ConsumedReviewIntent; +}; +export type ConsentReceipt = Readonly<{ + clientId: string; + intentId: string; + accountId: string; + repositoryId: string; + role: "owner" | "coauthor"; + policy: ConsumedReviewIntent["policy"]; + requestId: string; + confirmedAt: string; +}>; +type SavedConsent = { + _id: string; + accountId: string; + repositoryId: string; + requestId: string; + ticketHash: string; + receipt: ConsentReceipt; +}; +export class ReviewConsentError extends Error { + constructor( + public readonly kind: + "invalid" | "forbidden" | "expired" | "conflict" | "unavailable", + ) { + super("Review owner consent " + kind); + this.name = "ReviewConsentError"; + } +} +const deny = (kind: ReviewConsentError["kind"]): never => { + throw new ReviewConsentError(kind); +}; +const hash = (value: string) => + createHash("sha256").update(value).digest("hex"); +const objectId = (value: unknown): value is string => + typeof value === "string" && /^[a-f0-9]{24}$/.test(value); +function principal(value: Principal): { + accountId: string; + sessionHash: string; +} { + if ( + !value || + !objectId(value.accountId) || + typeof value.sessionId !== "string" || + !/^[A-Za-z0-9_-]{24,256}$/.test(value.sessionId) + ) + deny("invalid"); + return { accountId: value.accountId, sessionHash: hash(value.sessionId) }; +} + +/** Server-side consent storage. Principals must come from current authenticated + * browser sessions, never request JSON. A future HTTP adapter must apply its + * own same-origin/CSRF checks. No route or provider is enabled by this factory. */ +export function createReviewOwnerConsent( + connection: Connection, + consumer: ReturnType, + signingKey: Buffer, +) { + if (!Buffer.isBuffer(signingKey) || signingKey.length !== 32) deny("invalid"); + const key = Buffer.from(signingKey); + let activeTransactions = 0; + const signature = (body: string) => + createHmac("sha256", key) + .update("4open.review-consent/1." + body) + .digest(); + function decode(ticket: string, who: ReturnType): Quote { + if ( + typeof ticket !== "string" || + ticket.length > 8192 || + !/^[A-Za-z0-9_-]+\.[A-Za-z0-9_-]{43}$/.test(ticket) + ) + deny("invalid"); + const [body, encoded] = ticket.split("."); + const supplied = Buffer.from(encoded, "base64url"); + if ( + supplied.length !== 32 || + supplied.toString("base64url") !== encoded || + !timingSafeEqual(supplied, signature(body)) + ) + deny("invalid"); + let quote: Quote; + try { + quote = JSON.parse(Buffer.from(body, "base64url").toString("utf8")); + } catch { + return deny("invalid"); + } + if ( + quote.version !== 1 || + quote.accountId !== who.accountId || + quote.sessionHash !== who.sessionHash || + !objectId(quote.repositoryId) + ) + deny("forbidden"); + if ( + Date.parse(quote.intent.expiresAt) <= Date.now() || + Date.parse(quote.intent.policy.retainUntil) <= Date.now() + ) + deny("expired"); + return quote; + } + async function transaction( + who: ReturnType, + repository: { repoId: string } | { _id: Types.ObjectId }, + work: (authority: Authority, session: ClientSession) => Promise, + ): Promise { + if (connection.readyState !== 1 || activeTransactions >= 4) + return deny("unavailable"); + activeTransactions++; + const session = await connection.startSession().catch(() => { + activeTransactions--; + return deny("unavailable"); + }); + try { + session.startTransaction({ + readConcern: { level: "snapshot" }, + writeConcern: { w: "majority", wtimeoutMS: 3000 }, + maxCommitTimeMS: 5000, + }); + // Touch the authority documents in the same transaction as the receipt. + // Concurrent disablement, ownership changes and coauthor removal then + // conflict with this transaction instead of producing a stale consent. + const user = ( + await connection.db.collection("users").findOneAndUpdate( + { + _id: new Types.ObjectId(who.accountId), + $or: [{ status: "active" }, { status: { $exists: false } }], + }, + { $inc: { reviewConsentRevision: 1 } }, + { + session, + returnDocument: "after", + maxTimeMS: 3000, + projection: { externalIDs: 1 }, + }, + ) + ).value; + if (!user) return deny("forbidden"); + const githubId = user.externalIDs?.github; + const roles: Record[] = [ + { owner: new Types.ObjectId(who.accountId) }, + ]; + if (typeof githubId === "string" && /^[1-9][0-9]{0,19}$/.test(githubId)) + roles.push({ "coauthors.githubId": githubId }); + const repo = ( + await connection.db + .collection("anonymizedrepositories") + .findOneAndUpdate( + { ...repository, status: "ready", $or: roles }, + { $inc: { reviewConsentRevision: 1 } }, + { + session, + returnDocument: "after", + maxTimeMS: 3000, + projection: { repoId: 1, owner: 1, options: 1 }, + }, + ) + ).value; + if (!repo) return deny("forbidden"); + if ( + repo.options?.expirationMode !== "never" && + repo.options?.expirationDate && + (!Number.isFinite(new Date(repo.options.expirationDate).getTime()) || + new Date(repo.options.expirationDate).getTime() <= Date.now()) + ) + return deny("expired"); + const authority: Authority = { + accountId: who.accountId, + repositoryId: String(repo._id), + repositoryName: repo.repoId, + role: String(repo.owner) === who.accountId ? "owner" : "coauthor", + }; + const result = await work(authority, session); + await session.commitTransaction(); + return result; + } catch (error) { + if (session.inTransaction()) + await session.abortTransaction().catch(() => undefined); + if (error instanceof ReviewConsentError) throw error; + return deny("unavailable"); + } finally { + activeTransactions--; + await session.endSession().catch(() => undefined); + } + } + return Object.freeze({ + async preview( + actor: Principal, + repositoryName: string, + input: IntentConsumption, + signal?: AbortSignal, + ) { + const who = principal(actor); + const command = { ...input }; + if ( + typeof repositoryName !== "string" || + !/^[A-Za-z0-9_-]{3,128}$/.test(repositoryName) + ) + return deny("invalid"); + const before = await transaction( + who, + { repoId: repositoryName }, + async (authority) => authority, + ); + // The provider call holds no database transaction or authority lock. + const intent = await consumer.consume(command, signal); + const current = await transaction( + who, + { _id: new Types.ObjectId(before.repositoryId) }, + async (authority) => authority, + ); + if ( + signal?.aborted || + Date.parse(intent.expiresAt) <= Date.now() || + Date.parse(intent.policy.retainUntil) <= Date.now() + ) + return deny("expired"); + const quote: Quote = { + version: 1, + accountId: who.accountId, + sessionHash: who.sessionHash, + repositoryId: current.repositoryId, + intent, + }; + const body = Buffer.from(JSON.stringify(quote)).toString("base64url"); + return Object.freeze({ + ticket: body + "." + signature(body).toString("base64url"), + repositoryName: current.repositoryName, + policy: intent.policy, + expiresAt: intent.expiresAt, + }); + }, + async confirm( + actor: Principal, + ticket: string, + input: { + requestId: string; + acceptAccess: boolean; + acceptRetention: boolean; + }, + ): Promise { + const who = principal(actor); + if ( + !input || + Object.keys(input).sort().join(",") !== + "acceptAccess,acceptRetention,requestId" || + input.acceptAccess !== true || + input.acceptRetention !== true || + typeof input.requestId !== "string" || + !/^[a-f0-9]{32}$/.test(input.requestId) + ) + return deny("invalid"); + const quote = decode(ticket, who), + requestId = input.requestId; + return transaction( + who, + { _id: new Types.ObjectId(quote.repositoryId) }, + async (authority, session) => { + decode(ticket, who); // Recheck expiry after waiting for authority locks. + const collection = connection.db.collection( + "review_owner_consents", + ); + const id = quote.intent.clientId + ":" + quote.intent.intentId; + const ticketHash = hash(ticket); + // A string _id binds each intent to at most one accepted repository and + // session. No raw session ID, intent token or service key is persisted. + const existing = await collection.findOne( + { _id: id }, + { session, maxTimeMS: 3000 }, + ); + if (existing) { + if ( + existing.ticketHash !== ticketHash || + existing.requestId !== requestId || + existing.accountId !== who.accountId || + existing.repositoryId !== authority.repositoryId + ) + return deny("conflict"); + return Object.freeze(existing.receipt as ConsentReceipt); + } + const receipt: ConsentReceipt = Object.freeze({ + clientId: quote.intent.clientId, + intentId: quote.intent.intentId, + accountId: who.accountId, + repositoryId: authority.repositoryId, + role: authority.role, + policy: quote.intent.policy, + requestId, + confirmedAt: new Date().toISOString(), + }); + await collection.insertOne( + { + _id: id, + accountId: who.accountId, + repositoryId: authority.repositoryId, + requestId, + ticketHash, + receipt, + }, + { session, maxTimeMS: 3000 }, + ); + return receipt; + }, + ); + }, + }); +} diff --git a/test/review-owner-consent.test.js b/test/review-owner-consent.test.js new file mode 100644 index 0000000..eb45004 --- /dev/null +++ b/test/review-owner-consent.test.js @@ -0,0 +1,509 @@ +require("ts-node/register/transpile-only"); +const { expect } = require("chai"); +const mongoose = require("mongoose"); +const { randomBytes } = require("crypto"); +const process = require("process"); +const { + createReviewOwnerConsent, +} = require("../src/server/service/review-owner-consent"); +const actorId = new mongoose.Types.ObjectId(); +const coauthorId = new mongoose.Types.ObjectId(); +const otherId = new mongoose.Types.ObjectId(); +const repoId = new mongoose.Types.ObjectId(); +const actor = (accountId = actorId, sessionId = "s".repeat(32)) => ({ + accountId: String(accountId), + sessionId, +}); +const request = () => ({ + contract: "4open.artifacts/1", + clientId: "1".repeat(32), + intentId: "2".repeat(32), + token: "3".repeat(64), + requestId: "4".repeat(32), +}); +const intent = () => ({ + contract: "4open.artifacts/1", + clientId: "1".repeat(32), + intentId: "2".repeat(32), + submissionRef: "5".repeat(32), + callbackId: "6".repeat(32), + entitlementId: "7".repeat(32), + expiresAt: new Date(Date.now() + 120000) + .toISOString() + .replace(/\.\d{3}Z$/, "Z"), + policy: { + version: 1, + access: "restricted-review", + retainUntil: "2099-01-01T00:00:00Z", + }, +}); +const confirmation = () => ({ + requestId: "8".repeat(32), + acceptAccess: true, + acceptRetention: true, +}); +async function rejected(promise, kind) { + try { + await promise; + expect.fail("operation unexpectedly succeeded"); + } catch (error) { + expect(error.kind).to.equal(kind); + expect(error.message).to.equal("Review owner consent " + kind); + } +} + +describe("review owner consent transactions", function () { + this.timeout(15000); + let connection, store, calls, upstream; + const key = randomBytes(32); + before(async function () { + if (!process.env.TEST_REVIEW_CONSENT_MONGO) this.skip(); + connection = await mongoose + .createConnection(process.env.TEST_REVIEW_CONSENT_MONGO, { + dbName: "review_consent_test_" + randomBytes(8).toString("hex"), + serverSelectionTimeoutMS: 3000, + socketTimeoutMS: 10000, + monitorCommands: true, + autoIndex: false, + autoCreate: false, + maxPoolSize: 10, + }) + .asPromise(); + await connection.db.createCollection("review_owner_consents"); + }); + after(async () => { + if (connection) { + await connection.dropDatabase(); + await connection.close(); + } + }); + beforeEach(async () => { + await Promise.all( + ["users", "anonymizedrepositories", "review_owner_consents"].map((name) => + connection.db.collection(name).deleteMany({}), + ), + ); + await connection.db.collection("users").insertMany([ + { + _id: actorId, + username: "owner", + externalIDs: { github: "101" }, + status: "active", + }, + { + _id: coauthorId, + username: "coauthor", + externalIDs: { github: "102" }, + status: "active", + }, + { + _id: otherId, + username: "other", + externalIDs: { github: "103" }, + isAdmin: true, + status: "active", + }, + ]); + await connection.db.collection("anonymizedrepositories").insertOne({ + _id: repoId, + repoId: "synthetic-repository", + owner: actorId, + status: "ready", + coauthors: [{ username: "coauthor", githubId: "102" }], + options: { expirationMode: "never" }, + }); + calls = 0; + upstream = async () => intent(); + store = createReviewOwnerConsent( + connection, + { + consume: async (...args) => { + calls++; + return upstream(...args); + }, + }, + key, + ); + }); + it("requires explicit policy acceptance and retains one immutable receipt on retries", async () => { + const preview = await store.preview( + actor(), + "synthetic-repository", + request(), + ); + expect(preview.policy).to.deep.equal(intent().policy); + expect(preview.repositoryName).to.equal("synthetic-repository"); + expect(calls).to.equal(1); + for (const patch of [ + { acceptAccess: false }, + { acceptRetention: false }, + { acceptAccess: "true" }, + { policy: {} }, + ]) + await rejected( + store.confirm(actor(), preview.ticket, { ...confirmation(), ...patch }), + "invalid", + ); + const first = await store.confirm(actor(), preview.ticket, confirmation()); + expect(first.role).to.equal("owner"); + expect(first.policy).to.deep.equal(preview.policy); + const replay = await store.confirm(actor(), preview.ticket, confirmation()); + expect(replay).to.deep.equal(first); + await rejected( + store.confirm(actor(), preview.ticket, { + ...confirmation(), + requestId: "9".repeat(32), + }), + "conflict", + ); + const rows = await connection.db + .collection("review_owner_consents") + .find({}) + .toArray(); + expect(rows).to.have.length(1); + const stored = JSON.stringify(rows); + for (const secret of [actor().sessionId, request().token, preview.ticket]) + expect(stored).not.to.include(secret); + expect(calls).to.equal(1); + }); + it("accepts a stable-ID coauthor but never an unrelated administrator or matching legacy username", async () => { + const quote = await store.preview( + actor(coauthorId), + "synthetic-repository", + request(), + ); + expect( + (await store.confirm(actor(coauthorId), quote.ticket, confirmation())) + .role, + ).to.equal("coauthor"); + await rejected( + store.preview(actor(otherId), "synthetic-repository", request()), + "forbidden", + ); + await connection.db + .collection("anonymizedrepositories") + .updateOne( + { _id: repoId }, + { $set: { coauthors: [{ username: "coauthor" }] } }, + ); + await rejected( + store.preview(actor(coauthorId), "synthetic-repository", request()), + "forbidden", + ); + expect(calls).to.equal(1); + }); + it("binds signed previews to the account, session and exact policy", async () => { + const preview = await store.preview( + actor(), + "synthetic-repository", + request(), + ); + await rejected( + store.confirm(actor(otherId), preview.ticket, confirmation()), + "forbidden", + ); + await rejected( + store.confirm( + actor(actorId, "n".repeat(32)), + preview.ticket, + confirmation(), + ), + "forbidden", + ); + const [body, signature] = preview.ticket.split("."); + const modified = JSON.parse(Buffer.from(body, "base64url").toString()); + modified.intent.policy.access = "anonymous-link"; + await rejected( + store.confirm( + actor(), + Buffer.from(JSON.stringify(modified)).toString("base64url") + + "." + + signature, + confirmation(), + ), + "invalid", + ); + await rejected( + store.confirm(actor(), "x".repeat(8193), confirmation()), + "invalid", + ); + expect( + await connection.db.collection("review_owner_consents").countDocuments(), + ).to.equal(0); + }); + it("rechecks owner status and revocation when confirming or replaying", async () => { + const quote = await store.preview( + actor(), + "synthetic-repository", + request(), + ); + await store.confirm(actor(), quote.ticket, confirmation()); + await connection.db + .collection("users") + .updateOne({ _id: actorId }, { $set: { status: "banned" } }); + await rejected( + store.confirm(actor(), quote.ticket, confirmation()), + "forbidden", + ); + await connection.db + .collection("users") + .updateOne({ _id: actorId }, { $set: { status: "active" } }); + await connection.db + .collection("anonymizedrepositories") + .updateOne({ _id: repoId }, { $set: { owner: otherId } }); + await rejected( + store.confirm(actor(), quote.ticket, confirmation()), + "forbidden", + ); + expect( + await connection.db.collection("review_owner_consents").countDocuments(), + ).to.equal(1); + }); + it("rejects removed coauthors and archived or expired repositories", async () => { + const quote = await store.preview( + actor(coauthorId), + "synthetic-repository", + request(), + ); + await connection.db + .collection("anonymizedrepositories") + .updateOne({ _id: repoId }, { $set: { coauthors: [] } }); + await rejected( + store.confirm(actor(coauthorId), quote.ticket, confirmation()), + "forbidden", + ); + await connection.db + .collection("anonymizedrepositories") + .updateOne({ _id: repoId }, { $set: { status: "archived" } }); + await rejected( + store.preview(actor(), "synthetic-repository", request()), + "forbidden", + ); + await connection.db.collection("anonymizedrepositories").updateOne( + { _id: repoId }, + { + $set: { + status: "ready", + options: { expirationMode: "remove", expirationDate: new Date(0) }, + }, + }, + ); + await rejected( + store.preview(actor(), "synthetic-repository", request()), + "expired", + ); + expect(calls).to.equal(1); + }); + it("does not hold database authority locks across the upstream request and checks removal afterward", async () => { + let ready, release; + const arrived = new Promise((resolve) => { + ready = resolve; + }); + const gate = new Promise((resolve) => { + release = resolve; + }); + upstream = async () => { + ready(); + await gate; + return intent(); + }; + const pending = store.preview( + actor(coauthorId), + "synthetic-repository", + request(), + ); + await arrived; + try { + await connection.db + .collection("anonymizedrepositories") + .updateOne( + { _id: repoId }, + { $set: { coauthors: [] } }, + { maxTimeMS: 1000 }, + ); + } finally { + release(); + } + await rejected(pending, "forbidden"); + expect( + await connection.db.collection("review_owner_consents").countDocuments(), + ).to.equal(0); + }); + it("keeps one receipt across concurrent confirmations and permits explicit recovery", async () => { + const quote = await store.preview( + actor(), + "synthetic-repository", + request(), + ); + const results = await Promise.allSettled([ + store.confirm(actor(), quote.ticket, confirmation()), + store.confirm(actor(), quote.ticket, confirmation()), + ]); + expect(results.some((r) => r.status === "fulfilled")).to.equal(true); + for (const result of results) + if (result.status === "rejected") + expect(result.reason.kind).to.equal("unavailable"); + const retry = await store.confirm(actor(), quote.ticket, confirmation()); + for (const result of results) + if (result.status === "fulfilled") + expect(result.value).to.deep.equal(retry); + expect( + await connection.db.collection("review_owner_consents").countDocuments(), + ).to.equal(1); + }); + it("rejects expired previews without recording consent", async () => { + upstream = async () => ({ + ...intent(), + expiresAt: new Date(Date.now() + 2000).toISOString(), + }); + const quote = await store.preview( + actor(), + "synthetic-repository", + request(), + ); + const { setTimeout } = require("timers/promises"); + await setTimeout(2100); + await rejected( + store.confirm(actor(), quote.ticket, confirmation()), + "expired", + ); + expect( + await connection.db.collection("review_owner_consents").countDocuments(), + ).to.equal(0); + }); + it("cannot confirm a quote for a second repository under the same intent", async () => { + const first = await store.preview( + actor(), + "synthetic-repository", + request(), + ); + await store.confirm(actor(), first.ticket, confirmation()); + await connection.db.collection("anonymizedrepositories").insertOne({ + repoId: "second-repository", + owner: actorId, + status: "ready", + options: { expirationMode: "never" }, + }); + const second = await store.preview(actor(), "second-repository", request()); + await rejected( + store.confirm(actor(), second.ticket, confirmation()), + "conflict", + ); + expect( + await connection.db.collection("review_owner_consents").countDocuments(), + ).to.equal(1); + }); + it("conflicts with a concurrent ownership change and rejects its explicit retry", async () => { + const quote = await store.preview( + actor(), + "synthetic-repository", + request(), + ); + const session = await connection.startSession(); + session.startTransaction(); + await connection.db + .collection("anonymizedrepositories") + .updateOne({ _id: repoId }, { $set: { owner: otherId } }, { session }); + let arrived; + const ready = new Promise((resolve) => { + arrived = resolve; + }); + const listener = (event) => { + if ( + event.commandName === "findAndModify" && + event.command.findAndModify === "anonymizedrepositories" + ) + arrived(); + }; + connection.getClient().on("commandStarted", listener); + const pending = store.confirm(actor(), quote.ticket, confirmation()).then( + () => "ok", + (error) => error.kind, + ); + try { + await ready; + await session.commitTransaction(); + expect(await pending).to.equal("unavailable"); + await rejected( + store.confirm(actor(), quote.ticket, confirmation()), + "forbidden", + ); + expect( + await connection.db + .collection("review_owner_consents") + .countDocuments(), + ).to.equal(0); + } finally { + connection.getClient().off("commandStarted", listener); + if (session.inTransaction()) await session.abortTransaction(); + await session.endSession(); + } + }); + it("keeps authority serialization fields out of ordinary user and repository reads", async () => { + await store.preview(actor(), "synthetic-repository", request()); + const users = connection.model( + "ConsentPrivacyUser", + require("../src/core/model/users/users.schema").default, + "users", + ); + const repos = connection.model( + "ConsentPrivacyRepo", + require("../src/core/model/anonymizedRepositories/anonymizedRepositories.schema") + .default, + "anonymizedrepositories", + ); + expect( + (await users.findById(actorId).lean()).reviewConsentRevision, + ).to.equal(undefined); + expect( + (await repos.findById(repoId).lean()).reviewConsentRevision, + ).to.equal(undefined); + expect( + (await connection.db.collection("users").findOne({ _id: actorId })) + .reviewConsentRevision, + ).to.equal(2); + expect( + ( + await connection.db + .collection("anonymizedrepositories") + .findOne({ _id: repoId }) + ).reviewConsentRevision, + ).to.equal(2); + }); + it("measures durable receipt replay when explicitly requested", async function () { + if (!process.env.TEST_REVIEW_CONSENT_PERF_REPORT) this.skip(); + const quote = await store.preview( + actor(), + "synthetic-repository", + request(), + ); + const original = await store.confirm(actor(), quote.ticket, confirmation()); + const times = []; + for (let n = 0; n < 100; n++) { + const start = process.hrtime.bigint(); + expect( + await store.confirm(actor(), quote.ticket, confirmation()), + ).to.deep.equal(original); + times.push(Number(process.hrtime.bigint() - start) / 1e6); + } + times.sort((a, b) => a - b); + expect( + await connection.db.collection("review_owner_consents").countDocuments(), + ).to.equal(1); + require("fs").writeFileSync( + process.env.TEST_REVIEW_CONSENT_PERF_REPORT, + JSON.stringify( + { + calls: times.length, + database: "disposable loopback MongoDB replica set", + synthetic: true, + meanMs: times.reduce((a, b) => a + b, 0) / times.length, + medianMs: times[50], + p95Ms: times[94], + }, + null, + 2, + ) + "\n", + ); + }); +}); From 7b60979c960483ceeaf7416431b42bf99b6db5c9 Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Sun, 27 Sep 2026 14:36:46 +0000 Subject: [PATCH 06/48] Protect artifact consent HTTP requests with current sessions and CSRF --- .github/workflows/ci.yml | 7 +- docs/review-consent-http.md | 21 + docs/review-owner-consent.md | 2 +- src/server/service/review-consent-http.ts | 308 +++++++++++++ test/review-consent-http.test.js | 503 ++++++++++++++++++++++ 5 files changed, 838 insertions(+), 3 deletions(-) create mode 100644 docs/review-consent-http.md create mode 100644 src/server/service/review-consent-http.ts create mode 100644 test/review-consent-http.test.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index be4c47a..4cabbf0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,11 +60,14 @@ jobs: env: TEST_REVIEW_CONSENT_MONGO: mongodb://127.0.0.1:28743/?replicaSet=review-consent-test TEST_REVIEW_CONSENT_PERF_REPORT: review-consent-perf.json - run: npx mocha test/review-owner-consent.test.js + TEST_REVIEW_CONSENT_HTTP_PERF: review-consent-http-perf.json + run: npx mocha test/review-owner-consent.test.js test/review-consent-http.test.js - uses: actions/upload-artifact@v4 with: name: review-consent-performance - path: review-consent-perf.json + path: | + review-consent-perf.json + review-consent-http-perf.json - name: Remove disposable database if: always() run: docker rm -f review-consent-test || true diff --git a/docs/review-consent-http.md b/docs/review-consent-http.md new file mode 100644 index 0000000..3415727 --- /dev/null +++ b/docs/review-consent-http.md @@ -0,0 +1,21 @@ +# Browser transport for review consent + +`createReviewConsentRouter` exposes the consent backend through three routes when explicitly mounted at `/api/review-consent`: `GET /csrf`, `POST /preview` and `POST /confirm`. Application startup does not mount this router. Browser UI, private configuration, startup ordering and activation remain unfinished. + +The factory requires a canonical HTTPS origin, the consent backend and a dedicated private 32-byte CSRF key shared by serving processes. Mount it after Passport/session authentication and before any JSON parser. The current startup ordering must change before integration; mounting after a parser fails closed. Transport must be HTTPS directly or through the application's explicitly trusted proxy configuration. + +Identity comes from Passport's authenticated account and a fresh lookup in the session store. Neither request JSON nor a bearer credential can supply it. POSTs require the exact configured Origin and a constant-time-checked CSRF token bound to the account and session. CSRF tokens are derived with a protocol-specific HMAC; issuing one never saves a session. A late CSRF response therefore cannot recreate a session destroyed by a concurrent logout. Authority is reloaded after backend work before returning a preview or confirmation response. + +Requests use exact paths without queries, strict methods, closed JSON fields and literal access/retention acceptance. Cross-origin or cross-site context, bearer authentication, duplicate security headers, content encoding and unexpected content types are rejected. The CSRF GET accepts no body. JSON is limited to 12 KiB; tickets to 8 KiB. A 15-second total deadline closes incomplete requests, and browser disconnects cancel pending preview consumption. Authenticated requests are limited to 60 per account per minute with at most 1,024 live counters. + +Responses disable caching and referrers. Confirmations include only the request ID, accepted policy and confirmation time. Internal account/repository identifiers and provider/database errors are omitted. A confirmation already accepted by the backend can commit despite a lost response; the UI must preserve the same ticket and request ID for explicit recovery. This transport does not create artifact bindings or bypass final review-side authority checks. + +## Validation + +The tests use real Express and express-session middleware with a disposable in-memory session store, a synthetic backend and loopback HTTP representing the trusted TLS proxy boundary. They do not measure real MongoDB or provider latency. TLS verification of the service client and MongoDB authority transactions are tested separately. + +Coverage includes session-derived identity, CSRF replay across sessions, cross-origin/bearer rejection, bounded closed payloads, exact routes, session revocation during preview, no session resurrection during a CSRF/logout race, response redaction, fixed errors, rate limits, unauthorized incomplete uploads, browser cancellation and the authenticated 15-second slow-upload deadline. The optional benchmark performs 50 fresh loopback HTTP preview requests. The CI database job runs these HTTP tests alongside the consent transaction tests and retains both performance reports. + +The final local targeted run passes 12 cases in 16 seconds. Transport-only timings are mean 4.94 ms, median 4.51 ms and p95 7.26 ms. TypeScript and targeted lint pass. + +The final local broader suite passes 723 cases with 63 opt-in/environment-dependent cases pending. The separate CI replica-set job exercises the otherwise skipped consent database tests as well as this transport suite. diff --git a/docs/review-owner-consent.md b/docs/review-owner-consent.md index 7dc9105..d323a8a 100644 --- a/docs/review-owner-consent.md +++ b/docs/review-owner-consent.md @@ -20,4 +20,4 @@ The opt-in suite runs against a disposable MongoDB replica set. It covers litera Run `TEST_REVIEW_CONSENT_MONGO='mongodb://127.0.0.1:28743/?replicaSet=review-consent-test' npx mocha test/review-owner-consent.test.js`. Set `TEST_REVIEW_CONSENT_PERF_REPORT=/tmp/review-consent-perf.json` to include 100 durable receipt replays. The CI job starts and removes its own pinned MongoDB image and preserves the performance report. Local timings are synthetic and are not a production latency guarantee. -Local validation passes 12 targeted cases with the benchmark enabled, 723 full-suite cases with 51 opt-in/environment-dependent cases pending, the full TypeScript check and targeted lint. The final 100-replay run measured mean 12.85 ms, median 12.82 ms and p95 14.89 ms against a one-CPU, 512 MiB disposable database. An earlier performance attempt encountered a database conflict while the privacy fixture automatically built indexes; the final fixture disables automatic schema/index creation and creates its collections explicitly. +Local validation passes 12 targeted cases with the benchmark enabled, 723 full-suite cases with 51 opt-in/environment-dependent cases pending, the full TypeScript check and targeted lint. The final 100-replay run measured mean 12.85 ms, median 12.82 ms and p95 14.89 ms against a one-CPU, 512 MiB disposable database. An earlier performance attempt returned `unavailable` while the privacy fixture was automatically building indexes. The log does not establish the underlying database error. The final fixture disables automatic schema/index creation and creates its collections explicitly. diff --git a/src/server/service/review-consent-http.ts b/src/server/service/review-consent-http.ts new file mode 100644 index 0000000..262a067 --- /dev/null +++ b/src/server/service/review-consent-http.ts @@ -0,0 +1,308 @@ +import { createHmac, timingSafeEqual } from "crypto"; +import * as express from "express"; +import { Request, Response } from "express"; +import { createReviewOwnerConsent } from "./review-owner-consent"; + +type BrowserSession = { passport?: { user?: unknown } }; +type Context = { accountId: string; sessionId: string }; +const opaque = /^[a-f0-9]{32}$/; +const secret = /^[a-f0-9]{64}$/; +function fields( + value: unknown, + names: string[], +): value is Record { + return ( + !!value && + typeof value === "object" && + !Array.isArray(value) && + Object.keys(value).sort().join(",") === names.sort().join(",") + ); +} +function reject(res: Response, status: number, code: string) { + if (!res.headersSent && !res.destroyed) res.setHeader("Connection", "close"); + if (!res.headersSent && !res.destroyed) + res.status(status).json({ error: { code } }); +} +async function current(req: Request): Promise { + const id = (req.user as { user?: { _id?: unknown } } | undefined)?.user?._id; + const accountId = id === undefined ? "" : String(id); + if ( + !req.isAuthenticated?.() || + !/^[a-f0-9]{24}$/.test(accountId) || + !/^[A-Za-z0-9_-]{24,256}$/.test(req.sessionID || "") + ) + return undefined; + const session = await new Promise( + (resolve, fail) => + req.sessionStore.get(req.sessionID, (error, value) => + error + ? fail(new Error("Session unavailable")) + : resolve(value as BrowserSession | undefined), + ), + ); + if (session?.passport?.user !== accountId) return undefined; + return { accountId, sessionId: req.sessionID }; +} + +/** Mount after current Passport/session authentication and BEFORE any JSON + * parser. This factory is not registered by application startup. TLS must be + * established directly or identified through the configured trusted proxy. */ +export function createReviewConsentRouter( + origin: string, + backend: ReturnType, + csrfKey: Buffer, +) { + try { + if (new URL(origin).origin !== origin || !origin.startsWith("https://")) + throw new Error(); + } catch { + throw new Error("Invalid review consent origin"); + } + if (!Buffer.isBuffer(csrfKey) || csrfKey.length !== 32) + throw new Error("Invalid review consent CSRF key"); + const key = Buffer.from(csrfKey); + const csrf = (actor: Context) => + createHmac("sha256", key) + .update( + "4open.review-consent-csrf/1." + + actor.accountId + + "." + + actor.sessionId, + ) + .digest(); + const router = express.Router({ strict: true, caseSensitive: true }); + const rates = new Map(); + router.use((req, res, next) => { + res.setHeader("Cache-Control", "no-store"); + res.setHeader("Referrer-Policy", "no-referrer"); + res.setHeader("X-Content-Type-Options", "nosniff"); + res.removeHeader("Access-Control-Allow-Origin"); + if (!["/csrf", "/preview", "/confirm"].includes(req.url)) + return reject(res, 404, "not-found"); + if (req.method !== (req.url === "/csrf" ? "GET" : "POST")) + return reject(res, 405, "invalid-request"); + if ( + req.url === "/csrf" && + (req.headers["transfer-encoding"] !== undefined || + (req.headers["content-length"] !== undefined && + req.headers["content-length"] !== "0")) + ) + return reject(res, 400, "invalid-request"); + if ( + !req.secure || + req.headers.authorization !== undefined || + req.headers["content-encoding"] !== undefined + ) + return reject(res, 403, "forbidden"); + for (const name of [ + "origin", + "content-type", + "x-review-csrf", + "sec-fetch-site", + ]) { + if ( + req.rawHeaders.filter((v, i) => i % 2 === 0 && v.toLowerCase() === name) + .length > 1 + ) + return reject(res, 403, "forbidden"); + } + if ( + (req.headers.origin !== undefined && req.headers.origin !== origin) || + (req.method === "POST" && req.headers.origin !== origin) || + (req.headers["sec-fetch-site"] !== undefined && + req.headers["sec-fetch-site"] !== "same-origin") + ) + return reject(res, 403, "forbidden"); + if ( + req.method === "POST" && + !/^application\/json(?:\s*;\s*charset=utf-8)?$/i.test( + req.headers["content-type"] || "", + ) + ) + return reject(res, 415, "invalid-request"); + // Fail closed if mounted after a body parser that bypassed our byte limit. + if (req.body !== undefined) return reject(res, 503, "unavailable"); + const controller = new AbortController(); + res.locals.consentSignal = controller.signal; + const timer = setTimeout(() => { + controller.abort(); + reject(res, 408, "expired"); + req.destroy(); + }, 15000); + const done = () => { + clearTimeout(timer); + controller.abort(); + }; + res.once("close", done); + res.once("finish", done); + next(); + }); + router.use(async (req, res, next) => { + try { + const actor = await current(req); + if (res.destroyed || res.headersSent || res.locals.consentSignal.aborted) + return; + if (!actor) return reject(res, 401, "unauthorized"); + if (req.method === "POST") { + const supplied = req.headers["x-review-csrf"]; + if ( + typeof supplied !== "string" || + !secret.test(supplied) || + !timingSafeEqual(Buffer.from(supplied, "hex"), csrf(actor)) + ) + return reject(res, 403, "forbidden"); + } + const now = Date.now(); + for (const [id, rate] of rates) if (rate.until <= now) rates.delete(id); + const rate = rates.get(actor.accountId) || { + until: now + 60000, + count: 0, + }; + if ( + rate.count >= 60 || + (!rates.has(actor.accountId) && rates.size >= 1024) + ) { + res.setHeader("Retry-After", "60"); + return reject(res, 429, "rate-limited"); + } + rate.count++; + rates.set(actor.accountId, rate); + res.locals.consentActor = actor; + next(); + } catch { + reject(res, 503, "unavailable"); + } + }); + router.get("/csrf", async (req, res) => { + try { + const fresh = await current(req); + if (!fresh || fresh.accountId !== res.locals.consentActor.accountId) + return reject(res, 401, "unauthorized"); + // Derive without saving the session: a late CSRF response must never + // resurrect a session that a concurrent logout has destroyed. + res.json({ csrf: csrf(fresh).toString("hex") }); + } catch { + reject(res, 503, "unavailable"); + } + }); + router.use(express.json({ limit: 12288, strict: true, inflate: false })); + router.post(["/preview", "/confirm"], async (req, res) => { + const actor = res.locals.consentActor as Context; + try { + let result: unknown; + if (req.url === "/preview") { + if ( + !fields(req.body, ["repositoryId", "intent"]) || + typeof req.body.repositoryId !== "string" || + !/^[A-Za-z0-9_-]{3,128}$/.test(req.body.repositoryId) || + !fields(req.body.intent, [ + "contract", + "clientId", + "intentId", + "token", + "requestId", + ]) + ) + return reject(res, 422, "invalid-request"); + const input = req.body.intent; + if ( + input.contract !== "4open.artifacts/1" || + ![input.clientId, input.intentId, input.requestId].every( + (v) => typeof v === "string" && opaque.test(v), + ) || + typeof input.token !== "string" || + !secret.test(input.token) + ) + return reject(res, 422, "invalid-request"); + const preview = await backend.preview( + actor, + req.body.repositoryId, + { + contract: input.contract, + clientId: input.clientId as string, + intentId: input.intentId as string, + requestId: input.requestId as string, + token: input.token, + }, + res.locals.consentSignal, + ); + result = { + ticket: preview.ticket, + repositoryName: preview.repositoryName, + policy: { + version: preview.policy.version, + access: preview.policy.access, + retainUntil: preview.policy.retainUntil, + }, + expiresAt: preview.expiresAt, + }; + } else { + if ( + !fields(req.body, [ + "ticket", + "requestId", + "acceptAccess", + "acceptRetention", + ]) || + typeof req.body.ticket !== "string" || + req.body.ticket.length > 8192 || + typeof req.body.requestId !== "string" || + !opaque.test(req.body.requestId) || + req.body.acceptAccess !== true || + req.body.acceptRetention !== true + ) + return reject(res, 422, "invalid-request"); + const saved = await backend.confirm(actor, req.body.ticket, { + requestId: req.body.requestId, + acceptAccess: true, + acceptRetention: true, + }); + result = { + requestId: saved.requestId, + policy: { + version: saved.policy.version, + access: saved.policy.access, + retainUntil: saved.policy.retainUntil, + }, + confirmedAt: saved.confirmedAt, + }; + } + const fresh = await current(req); + if ( + !fresh || + fresh.accountId !== actor.accountId || + fresh.sessionId !== actor.sessionId + ) + return reject(res, 401, "unauthorized"); + if (!res.headersSent && !res.destroyed) res.json(result); + } catch (error) { + const kind = (error as { kind?: string })?.kind; + const status = + kind === "invalid" || kind === "protocol" + ? 422 + : kind === "forbidden" || kind === "rejected" + ? 403 + : kind === "expired" + ? 410 + : kind === "conflict" + ? 409 + : 503; + reject( + res, + status, + status === 503 + ? "unavailable" + : status === 422 + ? "invalid-request" + : kind!, + ); + } + }); + router.use(((error, _req, res, _next) => + reject( + res, + error?.status === 413 ? 413 : 400, + "invalid-request", + )) as express.ErrorRequestHandler); + return router; +} diff --git a/test/review-consent-http.test.js b/test/review-consent-http.test.js new file mode 100644 index 0000000..896b348 --- /dev/null +++ b/test/review-consent-http.test.js @@ -0,0 +1,503 @@ +require("ts-node/register/transpile-only"); +const { expect } = require("chai"); +const express = require("express"); +const session = require("express-session"); +const http = require("http"); +const { randomBytes } = require("crypto"); +const { + createReviewConsentRouter, +} = require("../src/server/service/review-consent-http"); +const origin = "https://anonymous.example.test"; +const owner = "a".repeat(24); +const intent = { + contract: "4open.artifacts/1", + clientId: "1".repeat(32), + intentId: "2".repeat(32), + token: "3".repeat(64), + requestId: "4".repeat(32), +}; +const previewInput = () => ({ repositoryId: "synthetic-repository", intent }); +const confirmInput = () => ({ + ticket: "synthetic-signed-ticket", + requestId: "5".repeat(32), + acceptAccess: true, + acceptRetention: true, +}); +const policy = { + version: 1, + access: "restricted-review", + retainUntil: "2099-01-01T00:00:00Z", +}; + +describe("review consent browser HTTP transport", function () { + this.timeout(20000); + let server, store, cookie, sid, csrf, backend, calls; + function request(path, options = {}) { + const body = + options.raw === undefined + ? options.body === undefined + ? undefined + : JSON.stringify(options.body) + : options.raw; + return new Promise((resolve, reject) => { + const req = http.request( + { + host: "127.0.0.1", + port: server.address().port, + path, + method: options.method || (body === undefined ? "GET" : "POST"), + headers: { + "X-Forwarded-Proto": "https", + ...(cookie ? { Cookie: cookie } : {}), + ...(body === undefined + ? {} + : { + Origin: origin, + "Content-Type": "application/json", + "Content-Length": Buffer.byteLength(body), + "X-Review-CSRF": csrf || "", + }), + ...options.headers, + }, + }, + (res) => { + const chunks = []; + res.on("data", (chunk) => chunks.push(chunk)); + res.on("end", () => { + const raw = Buffer.concat(chunks).toString(); + resolve({ + status: res.statusCode, + headers: res.headers, + body: raw ? JSON.parse(raw) : null, + }); + }); + }, + ); + req.on("error", reject); + if (body !== undefined) req.write(body); + req.end(); + }); + } + beforeEach(async () => { + cookie = ""; + sid = ""; + csrf = ""; + calls = []; + store = new session.MemoryStore(); + backend = { + preview: async (actor, repository, input) => { + calls.push({ actor, repository, input }); + return { + ticket: "synthetic-signed-ticket", + repositoryName: repository, + policy, + expiresAt: "2098-01-01T00:00:00Z", + }; + }, + confirm: async (actor, ticket, input) => { + calls.push({ actor, ticket, input }); + return { + clientId: intent.clientId, + intentId: intent.intentId, + accountId: actor.accountId, + repositoryId: "b".repeat(24), + role: "owner", + requestId: input.requestId, + policy, + confirmedAt: "2026-01-01T00:00:00Z", + }; + }, + }; + const app = express(); + // Model a TLS-terminating trusted loopback proxy, never trust arbitrary peers. + app.set("trust proxy", "loopback"); + app.use( + session({ + secret: "synthetic-local-session-secret", + resave: false, + saveUninitialized: false, + store, + }), + ); + // Test-only login stands in for Passport's existing authenticated session. + app.get("/test-login", (req, res) => { + req.session.passport = { user: owner }; + req.session.save((error) => + error ? res.sendStatus(500) : res.json({ sid: req.sessionID }), + ); + }); + app.use((req, _res, next) => { + const id = req.session.passport?.user; + if (id) req.user = { user: { _id: id } }; + req.isAuthenticated = () => !!id; + next(); + }); + app.use( + "/api/review-consent", + createReviewConsentRouter(origin, backend, randomBytes(32)), + ); + server = http.createServer(app); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const login = await request("/test-login"); + cookie = login.headers["set-cookie"][0].split(";")[0]; + sid = login.body.sid; + const issued = await request("/api/review-consent/csrf"); + expect(issued.status).to.equal(200); + csrf = issued.body.csrf; + }); + afterEach(async () => { + server.closeAllConnections(); + await new Promise((resolve) => server.close(resolve)); + store.clear(); + }); + it("binds CSRF to the current Passport session and omits owner identifiers from confirmation responses", async () => { + const preview = await request("/api/review-consent/preview", { + body: previewInput(), + }); + expect(preview.status).to.equal(200); + expect(calls[0].actor).to.deep.equal({ accountId: owner, sessionId: sid }); + expect(calls[0].input).to.deep.equal(intent); + expect(preview.headers["cache-control"]).to.equal("no-store"); + expect(preview.headers["referrer-policy"]).to.equal("no-referrer"); + expect(preview.headers["access-control-allow-origin"]).to.equal(undefined); + const result = await request("/api/review-consent/confirm", { + body: confirmInput(), + }); + expect(result.status).to.equal(200); + expect(result.body).to.deep.equal({ + requestId: "5".repeat(32), + policy, + confirmedAt: "2026-01-01T00:00:00Z", + }); + expect(JSON.stringify(result.body)).not.to.include(owner); + }); + it("rejects insecure, cross-origin, bearer and missing-CSRF requests before backend work", async () => { + for (const headers of [ + { "X-Forwarded-Proto": "http" }, + { Origin: "https://attacker.example" }, + { Origin: "null" }, + { Authorization: "Bearer synthetic" }, + { "X-Review-CSRF": "0".repeat(64) }, + { "X-Review-CSRF": "" }, + { "Sec-Fetch-Site": "same-site" }, + { "Content-Encoding": "gzip" }, + ]) + expect( + ( + await request("/api/review-consent/preview", { + body: previewInput(), + headers, + }) + ).status, + ).to.equal(403); + expect( + ( + await request("/api/review-consent/preview", { + body: previewInput(), + headers: { Cookie: "" }, + }) + ).status, + ).to.equal(401); + expect(calls).to.have.length(0); + }); + it("accepts only bounded JSON with the exact operation fields and explicit acceptance", async () => { + for (const input of [ + { ...previewInput(), accountId: owner }, + { ...previewInput(), policy }, + { ...previewInput(), repositoryId: "../other" }, + { ...previewInput(), intent: { ...intent, requestId: "bad" } }, + ]) + expect( + (await request("/api/review-consent/preview", { body: input })).status, + ).to.equal(422); + for (const input of [ + { ...confirmInput(), acceptAccess: false }, + { ...confirmInput(), acceptRetention: "true" }, + { ...confirmInput(), policy }, + ]) + expect( + (await request("/api/review-consent/confirm", { body: input })).status, + ).to.equal(422); + expect( + (await request("/api/review-consent/preview", { raw: "{" })).status, + ).to.equal(400); + expect( + (await request("/api/review-consent/preview", { raw: " ".repeat(13000) })) + .status, + ).to.equal(413); + expect( + ( + await request("/api/review-consent/preview", { + body: previewInput(), + headers: { "Content-Type": "text/plain" }, + }) + ).status, + ).to.equal(415); + for (const path of ["/csrf?x=1", "/csrf/", "/CSRF", "/%63srf", "/other"]) + expect((await request("/api/review-consent" + path)).status).to.equal( + 404, + ); + expect((await request("/api/review-consent/preview")).status).to.equal(405); + expect( + (await request("/api/review-consent/csrf", { method: "GET", body: {} })) + .status, + ).to.equal(400); + expect(calls).to.have.length(0); + }); + it("rejects a token copied from another session of the same account", async () => { + const old = csrf; + cookie = ""; + const login = await request("/test-login"); + cookie = login.headers["set-cookie"][0].split(";")[0]; + expect( + ( + await request("/api/review-consent/preview", { + body: previewInput(), + headers: { "X-Review-CSRF": old }, + }) + ).status, + ).to.equal(403); + expect(calls).to.have.length(0); + }); + it("reloads session authority before returning a pending policy preview", async () => { + let arrived, release; + const ready = new Promise((resolve) => { + arrived = resolve; + }); + const gate = new Promise((resolve) => { + release = resolve; + }); + backend.preview = async () => { + arrived(); + await gate; + return { + ticket: "private-policy-preview", + policy, + repositoryName: "synthetic-repository", + expiresAt: "2098-01-01T00:00:00Z", + }; + }; + const pending = request("/api/review-consent/preview", { + body: previewInput(), + }); + await ready; + await new Promise((resolve) => store.destroy(sid, resolve)); + release(); + const result = await pending; + expect(result.status).to.equal(401); + expect(JSON.stringify(result.body)).not.to.include( + "private-policy-preview", + ); + }); + it("never rewrites or resurrects a session when a CSRF read races with logout", async () => { + const original = store.get.bind(store); + let reads = 0, + arrived, + release; + const ready = new Promise((resolve) => { + arrived = resolve; + }); + const gate = new Promise((resolve) => { + release = resolve; + }); + store.get = (id, callback) => + original(id, (error, value) => { + if (++reads === 3) { + arrived(); + gate.then(() => callback(error, value)); + } else callback(error, value); + }); + let writes = 0; + const set = store.set.bind(store); + store.set = (...args) => { + writes++; + return set(...args); + }; + const pending = request("/api/review-consent/csrf"); + await ready; + await new Promise((resolve) => store.destroy(sid, resolve)); + release(); + await pending; + expect(writes).to.equal(0); + expect( + await new Promise((resolve) => + original(sid, (_error, value) => resolve(value)), + ), + ).to.equal(undefined); + expect( + (await request("/api/review-consent/preview", { body: previewInput() })) + .status, + ).to.equal(401); + expect(calls).to.have.length(0); + }); + it("returns fixed failure codes without exposing provider or database messages", async () => { + for (const [kind, status] of [ + ["forbidden", 403], + ["conflict", 409], + ["expired", 410], + ["invalid", 422], + ["unavailable", 503], + [undefined, 503], + ]) { + backend.preview = async () => { + const error = new Error("private credential and database details"); + error.kind = kind; + throw error; + }; + const result = await request("/api/review-consent/preview", { + body: previewInput(), + }); + expect(result.status).to.equal(status); + expect(JSON.stringify(result.body)).not.to.include("private"); + } + }); + it("bounds authenticated request rates without accepting a different client identity", async () => { + for (let n = 0; n < 59; n++) + expect((await request("/api/review-consent/csrf")).status).to.equal(200); + const response = await request("/api/review-consent/csrf"); + expect(response.status).to.equal(429); + expect(response.headers["retry-after"]).to.equal("60"); + expect(calls).to.have.length(0); + }); + it("closes an unauthorized incomplete upload without waiting for its declared body", async () => { + const started = Date.now(); + let pending; + try { + const result = await new Promise((resolve, reject) => { + pending = http.request( + { + host: "127.0.0.1", + port: server.address().port, + path: "/api/review-consent/preview", + method: "POST", + headers: { + "X-Forwarded-Proto": "https", + Origin: origin, + "Content-Type": "application/json", + "Content-Length": 10000000, + }, + }, + (response) => { + response.resume(); + response.on("end", () => + resolve({ + status: response.statusCode, + connection: response.headers.connection, + }), + ); + }, + ); + pending.on("error", reject); + pending.write("{"); + }); + expect(result).to.deep.equal({ status: 401, connection: "close" }); + expect(Date.now() - started).to.be.lessThan(2000); + expect(calls).to.have.length(0); + } finally { + pending?.destroy(); + } + }); + it("cancels a pending preview when its browser connection closes", async () => { + let entered, aborted; + const ready = new Promise((resolve) => { + entered = resolve; + }); + const cancelled = new Promise((resolve) => { + aborted = resolve; + }); + backend.preview = async (_actor, _repo, _input, signal) => { + entered(); + await new Promise((resolve) => + signal.addEventListener("abort", resolve, { once: true }), + ); + aborted(); + throw new Error("private pending request cancelled"); + }; + const body = JSON.stringify(previewInput()); + const pending = http.request({ + host: "127.0.0.1", + port: server.address().port, + path: "/api/review-consent/preview", + method: "POST", + headers: { + Cookie: cookie, + "X-Forwarded-Proto": "https", + Origin: origin, + "Content-Type": "application/json", + "X-Review-CSRF": csrf, + "Content-Length": Buffer.byteLength(body), + }, + }); + pending.on("error", () => undefined); + pending.end(body); + await ready; + pending.destroy(); + await cancelled; + }); + it("expires an authenticated incomplete upload at the total request deadline", async () => { + const started = Date.now(); + let pending; + try { + const status = await new Promise((resolve) => { + pending = http.request( + { + host: "127.0.0.1", + port: server.address().port, + path: "/api/review-consent/preview", + method: "POST", + headers: { + Cookie: cookie, + "X-Forwarded-Proto": "https", + Origin: origin, + "Content-Type": "application/json", + "Content-Length": 1000, + "X-Review-CSRF": csrf, + }, + }, + (response) => { + response.resume(); + response.on("end", () => resolve(response.statusCode)); + }, + ); + pending.on("error", () => resolve("closed")); + pending.write("{"); + }); + expect([408, "closed"]).to.include(status); + expect(Date.now() - started).to.be.within(14000, 19000); + expect(calls).to.have.length(0); + } finally { + pending?.destroy(); + } + }); + it("measures authenticated HTTP preview overhead when explicitly requested", async function () { + const process = require("process"); + if (!process.env.TEST_REVIEW_CONSENT_HTTP_PERF) this.skip(); + const times = []; + for (let n = 0; n < 50; n++) { + const start = process.hrtime.bigint(); + expect( + (await request("/api/review-consent/preview", { body: previewInput() })) + .status, + ).to.equal(200); + times.push(Number(process.hrtime.bigint() - start) / 1e6); + } + times.sort((a, b) => a - b); + expect(calls).to.have.length(50); + require("fs").writeFileSync( + process.env.TEST_REVIEW_CONSENT_HTTP_PERF, + JSON.stringify( + { + calls: times.length, + transport: "loopback HTTP with synthetic trusted-proxy TLS metadata", + sessionStore: "in-memory test store", + backend: "synthetic response, no MongoDB or provider timing", + meanMs: times.reduce((a, b) => a + b, 0) / times.length, + medianMs: times[25], + p95Ms: times[47], + }, + null, + 2, + ) + "\n", + ); + }); +}); From e4334bc9a7fc86b3a079439eee918ee5c39b9a44 Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Sun, 27 Sep 2026 14:56:06 +0000 Subject: [PATCH 07/48] Add isolated artifact owner consent page --- docs/review-consent-ui.md | 13 +++ public/partials/reviewConsent.htm | 24 ++++++ public/script/main.js | 14 +++- public/script/review-consent.js | 72 +++++++++++++++++ public/script/routes.js | 2 + public/script/templates.js | 2 + src/server/index.ts | 4 + src/server/review-consent-page.ts | 58 +++++++++++++ test/review-consent-page.test.js | 127 +++++++++++++++++++++++++++++ test/review-consent-ui.test.js | 130 ++++++++++++++++++++++++++++++ 10 files changed, 443 insertions(+), 3 deletions(-) create mode 100644 docs/review-consent-ui.md create mode 100644 public/partials/reviewConsent.htm create mode 100644 public/script/review-consent.js create mode 100644 src/server/review-consent-page.ts create mode 100644 test/review-consent-page.test.js create mode 100644 test/review-consent-ui.test.js diff --git a/docs/review-consent-ui.md b/docs/review-consent-ui.md new file mode 100644 index 0000000..7de25eb --- /dev/null +++ b/docs/review-consent-ui.md @@ -0,0 +1,13 @@ +# Artifact consent page + +`/review-link` serves a separate page without the ordinary site's third-party widget or profile header. Its content security policy allows only first-party scripts and connections. The page rejects URL queries and noncanonical aliases, disables caching and referrers, and accepts only filename-shaped manifest entries. + +The handoff fragment contains exactly `clientId`, `intentId` and `token`. The application removes it from the current history entry before starting application requests or the router. Intent credentials and signed preview tickets stay in a controller closure. They are not rendered or written to browser storage. Reloading loses the handoff; users must reopen a new link from their submission. Signed-out users sign in before reopening the link. + +Owners enter the anonymous repository ID and request a preview. The backend supplies the repository name, access policy, retention deadline and expiry. Confirmation requires two separate unchecked approvals. A failed confirmation keeps the same signed ticket and random request ID for an explicit retry. Repository changes are disabled while that confirmation is uncertain. Account changes and page disposal cancel requests and discard late results. Success says that consent is recorded and that artifact linking remains incomplete. + +The page does not mount the consent API or advertise available integration capabilities. Startup configuration, final binding exchange, provider activation and a review-side link launcher remain unfinished. The consent HTTP router still needs to be mounted after authentication and before body parsing. + +Validation uses the compiled frontend bundles. Targeted tests cover fragment removal, storage, approval flags, replay identity, departure during CSRF, account changes during preview, invalid handoffs and policies, endpoint unavailability, expiry, sign-in and mismatched receipts. Server tests check canonical paths, headers and unsafe manifests. Chromium checks at 1280 and 320 pixels exercise the full synthetic form flow with CSP enabled and check overflow, external requests, script errors and WCAG A/AA rules. No live database or provider request is involved. + +Run `npm run build:ui`, then `npx mocha test/review-consent-page.test.js test/review-consent-ui.test.js`. Set `TEST_REVIEW_PAGE_PERF_REPORT` to a private output path to measure 100 isolated HTML responses over loopback HTTP. This benchmark excludes browser rendering, database operations and provider latency. diff --git a/public/partials/reviewConsent.htm b/public/partials/reviewConsent.htm new file mode 100644 index 0000000..a7f61ff --- /dev/null +++ b/public/partials/reviewConsent.htm @@ -0,0 +1,24 @@ +
+ Anonymous GitHub +

Approve artifact access

+

This records your approval as the repository owner or an authorized coauthor.

+

Sign in first, then reopen the artifact link from your submission. Sign in

+

{{ error }}

+

Consent recorded. Artifact linking is not complete. Return to your submission to check its status.

+
+ + +

Use the ID after /r/ in the anonymous repository URL.

+ +
+
+

{{ preview.repositoryName }}

+

Access: {{ preview.policy.access === 'anonymous-link' ? 'Anyone with the anonymous link' : 'Restricted review access' }}.

+

Retain until {{ preview.policy.retainUntil }}. Policy version {{ preview.policy.version }}.

+

This preview expired. Open a new artifact link from your submission.

+
+
+ +
+

Waiting for the server…

+
diff --git a/public/script/main.js b/public/script/main.js index 89a1c6b..a757e3b 100644 --- a/public/script/main.js +++ b/public/script/main.js @@ -1,3 +1,4 @@ +import { takeReviewFragment } from "./review-consent.js"; import { loadLibrary } from "./lazy-assets.js"; import { createApp, h, watch, provide, inject, onBeforeUnmount } from "vue"; import { createRouter, createWebHistory, RouterView } from "vue-router"; @@ -30,6 +31,9 @@ export function safeUrl(value) { } export function mountApplication(target = "#app", options = {}) { + const isolatedConsent = !!document.querySelector('meta[name="review-consent-page"][content="1"]'); + let reviewHandoff = isolatedConsent && window.location.pathname === "/review-link" ? takeReviewFragment(window) : null; + const takeReviewHandoff = () => { const value = reviewHandoff; reviewHandoff = null; return value; }; // AngularJS links stored the repository route in the fragment. Normalize it // before web history reads the URL, replacing the entry so Back still works. if (!options.history && window.location.pathname === "/" && /^#!\/r\/[^/?#]+/.test(window.location.hash)) { @@ -50,7 +54,7 @@ export function mountApplication(target = "#app", options = {}) { const value = router.currentRoute.value.params[key]; return Array.isArray(value) ? value.join("/") : value; } }); - return { http, ...timers, location, params, window, promises, html: { trustAsHtml: value => value }, translate: (key, params) => Promise.resolve(translate(key, params)), quotaService: createQuotaService(http) }; + return { takeReviewHandoff, http, ...timers, location, params, window, promises, html: { trustAsHtml: value => value }, translate: (key, params) => Promise.resolve(translate(key, params)), quotaService: createQuotaService(http) }; } function pageComponent(definition) { return { @@ -92,7 +96,7 @@ export function mountApplication(target = "#app", options = {}) { const href = anchor.getAttribute("href"); if (href.startsWith("#")) return; const url = new URL(anchor.href, window.location.href); - if (url.origin !== window.location.origin || serverPaths.test(url.pathname)) return; + if (isolatedConsent || /^\/review-link\/?$/i.test(url.pathname) || url.origin !== window.location.origin || serverPaths.test(url.pathname)) return; event.preventDefault(); router.push(url.pathname + url.search + url.hash); $("#navbarSupportedContent.show").collapse("hide"); @@ -100,7 +104,7 @@ export function mountApplication(target = "#app", options = {}) { document.addEventListener("click", navigate); onBeforeUnmount(() => document.removeEventListener("click", navigate)); return () => [ - h("header", { class: "app-header" }, templates["partials/header.htm"](root, headerCache)), + isolatedConsent ? null : h("header", { class: "app-header" }, templates["partials/header.htm"](root, headerCache)), h("main", { class: "app-view align-items-stretch w-100" }, h(RouterView, null, { default: ({ Component, route }) => Component ? h(Component, { key: route.meta.preserveExplorer ? route.matched[0]?.path : route.path }) : null, })), @@ -123,6 +127,10 @@ export function mountApplication(target = "#app", options = {}) { app.directive("paper-scrollspy", paperScrollspy); app.use(router); router.beforeEach(async to => { + if ((to.path === "/review-link" && !isolatedConsent) || (isolatedConsent && to.path !== "/review-link")) { + window.location.assign(to.fullPath); + return false; + } root?.emit("routeLeave"); if (/^\/(r|repository|anonymize|pull-request-anonymize|gist-anonymize|pr|gist)(\/|$)/.test(to.path)) { await loadLibrary("markdown"); diff --git a/public/script/review-consent.js b/public/script/review-consent.js new file mode 100644 index 0000000..e58240a --- /dev/null +++ b/public/script/review-consent.js @@ -0,0 +1,72 @@ +// Read once, before the router or application requests can observe the fragment. +export function takeReviewFragment(browser) { + const fragment = browser.location.hash.slice(1); + browser.history.replaceState(null, "", "/review-link"); + if (fragment.length > 2048) return null; + const values = new URLSearchParams(fragment); + if ([...values.keys()].length !== 3 || [...values.keys()].some(key => !["clientId", "intentId", "token"].includes(key))) return null; + const clientId = values.get("clientId"), intentId = values.get("intentId"), token = values.get("token"); + if (!/^[a-f0-9]{32}$/.test(clientId || "") || !/^[a-f0-9]{32}$/.test(intentId || "") || !/^[a-f0-9]{64}$/.test(token || "")) return null; + return { contract: "4open.artifacts/1", clientId, intentId, token }; +} + +export function reviewConsentController(state, services) { + const { http, promises, window: browser } = services; + let handoff = services.takeReviewHandoff(); + let ticket, csrf, confirmId, consumeId, identity; + let active = true, expiryTimer; + const stop = promises.defer(); + const requestId = () => [...browser.crypto.getRandomValues(new Uint8Array(16))].map(byte => byte.toString(16).padStart(2, "0")).join(""); + Object.assign(state, { repositoryId: "", busy: false, preview: null, acceptAccess: false, acceptRetention: false, uncertain: false, saved: false, expired: false, error: handoff ? "" : "Open a new artifact link from your submission. This link is missing or invalid." }); + const current = () => active && state.user?.username === identity; + function dispose() { + active = false; handoff = ticket = csrf = undefined; + stop.resolve(); browser.clearTimeout(expiryTimer); + } + state.on("dispose", dispose); + state.on("routeLeave", dispose); + state.watch(() => state.user?.username, username => { + if (!identity && username) identity = username; + else if (identity && username !== identity) { + dispose(); state.preview = null; state.busy = false; + state.error = "Your account changed. Open a new artifact link from your submission."; + } + }); + const options = () => ({ timeout: stop.promise, headers: { "Content-Type": "application/json", "X-Review-CSRF": csrf } }); + const message = error => error?.status === 404 ? "Artifact linking is not available on this server." : error?.status === 401 ? "Sign in, then open a new artifact link from your submission." : error?.status === 403 ? "This account cannot approve this repository or this link." : "The request could not be completed. You can retry from this page."; + state.loadPreview = async () => { + if (!active || !handoff || state.busy || state.uncertain || state.saved || !state.user?.username) return; + identity = state.user.username; + const repositoryId = state.repositoryId.trim(); + if (!/^[A-Za-z0-9_-]{3,128}$/.test(repositoryId)) { state.error = "Enter the anonymous repository ID from its URL."; return; } + state.busy = true; state.error = ""; state.preview = null; ticket = undefined; + state.acceptAccess = state.acceptRetention = false; state.expired = false; + browser.clearTimeout(expiryTimer); + consumeId ||= requestId(); + try { + csrf = (await http.get("/api/review-consent/csrf", { timeout: stop.promise })).data.csrf; + if (!current()) return; + if (!/^[a-f0-9]{64}$/.test(csrf || "")) throw new Error("invalid_csrf"); + const result = (await http.post("/api/review-consent/preview", { repositoryId, intent: { ...handoff, requestId: consumeId } }, options())).data; + if (!current()) return; + if (!result || typeof result.ticket !== "string" || !result.ticket || result.ticket.length > 8192 || typeof result.repositoryName !== "string" || !["restricted-review", "anonymous-link"].includes(result.policy?.access) || !Number.isSafeInteger(result.policy.version) || !Number.isFinite(Date.parse(result.policy.retainUntil)) || !Number.isFinite(Date.parse(result.expiresAt)) || Date.parse(result.expiresAt) <= Date.now() || Date.parse(result.expiresAt) > Date.now() + 600000) throw new Error("invalid_preview"); + ticket = result.ticket; confirmId = requestId(); + state.preview = { repositoryName: result.repositoryName, policy: { ...result.policy }, expiresAt: result.expiresAt }; + expiryTimer = browser.setTimeout(() => { if (active) state.expired = true; }, Math.max(0, Date.parse(result.expiresAt) - Date.now())); + } catch (error) { if (current()) state.error = message(error); } + finally { if (current()) state.busy = false; } + }; + state.confirmConsent = async () => { + if (!current() || state.busy || state.saved || !ticket || !state.acceptAccess || !state.acceptRetention) return; + if (Date.parse(state.preview.expiresAt) <= Date.now()) { state.expired = true; return; } + state.busy = true; state.error = ""; + try { + const result = (await http.post("/api/review-consent/confirm", { ticket, requestId: confirmId, acceptAccess: true, acceptRetention: true }, options())).data; + if (!current()) return; + if (result?.requestId !== confirmId || !Number.isFinite(Date.parse(result.confirmedAt))) throw new Error("invalid_receipt"); + state.saved = true; state.uncertain = false; handoff = ticket = undefined; + } catch (error) { + if (current()) { state.uncertain = true; state.error = "The confirmation was not verified. Retry with the same approval, or return to your submission to check its status."; } + } finally { if (current()) state.busy = false; } + }; +} diff --git a/public/script/routes.js b/public/script/routes.js index 364fe48..c7ca3a9 100644 --- a/public/script/routes.js +++ b/public/script/routes.js @@ -1,7 +1,9 @@ +import { reviewConsentController } from "./review-consent.js"; import * as pages from "./app.js"; import * as admin from "./admin.js"; export const pageRoutes = [ +{path: "/review-link", template: "partials/reviewConsent.htm", title: "Artifact consent – Anonymous GitHub", setup: (state, services) => reviewConsentController(state, services)}, {path: "/connections", template: "partials/connections.htm", title: "GitHub connections – Anonymous GitHub", preserveExplorer: false, setup: (state, services) => pages.connectionsController(state, services.http)}, {path: "/signin", template: "partials/signin.htm", title: "Sign in – Anonymous GitHub", preserveExplorer: false, setup: (state, services) => pages.signinController(state, services.http)}, {path: "/", template: "partials/home.htm", title: "Anonymous GitHub – Share the code, not the author", preserveExplorer: false, setup: (state, services) => pages.homeController(state, services.http, services.location, services.window, services.timeout)}, diff --git a/public/script/templates.js b/public/script/templates.js index 6386558..f68d321 100644 --- a/public/script/templates.js +++ b/public/script/templates.js @@ -1,3 +1,4 @@ +import { render as reviewConsentTemplate } from "../partials/reviewConsent.htm"; import { render as connectionsTemplate } from "../partials/connections.htm"; import { render as signinTemplate } from "../partials/signin.htm"; import { render as template0 } from "../partials/404.htm"; @@ -27,6 +28,7 @@ import { render as template23 } from "../partials/profile.htm"; import { render as template24 } from "../partials/pullRequest.htm"; import { render as template25 } from "../partials/status.htm"; export const templates = { +"partials/reviewConsent.htm": reviewConsentTemplate, "partials/connections.htm": connectionsTemplate, "partials/signin.htm": signinTemplate, "partials/404.htm": template0, diff --git a/src/server/index.ts b/src/server/index.ts index 158b0e4..2c38fd3 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -40,6 +40,9 @@ import { createLogger, serializeError } from "../core/logger"; import { createReviewCapabilities } from "./service/review-capabilities"; +import { createReviewConsentPage, isReviewConsentPagePath } from "./review-consent-page"; + +const reviewConsentPage = createReviewConsentPage(); const logger = createLogger("server"); // Lazily build the templated index.html on first request so the server @@ -77,6 +80,7 @@ function getIndexHtml(): string { } function indexResponse(req: express.Request, res: express.Response) { + if (isReviewConsentPagePath(req.path)) return reviewConsentPage(req, res); if ( req.path.startsWith("/script") || req.path.startsWith("/css") || diff --git a/src/server/review-consent-page.ts b/src/server/review-consent-page.ts new file mode 100644 index 0000000..62e15c0 --- /dev/null +++ b/src/server/review-consent-page.ts @@ -0,0 +1,58 @@ +import { Request, Response } from "express"; +import { existsSync, readFileSync } from "fs"; +import { resolve } from "path"; + +// Reserve aliases too: they must never fall through to the ordinary page, +// which loads third-party scripts. Only the canonical URL serves this page. +export function isReviewConsentPagePath(path: string): boolean { + try { + return ( + decodeURIComponent(path).toLowerCase().replace(/\/+$/, "") === + "/review-link" + ); + } catch { + return false; + } +} + +export function createReviewConsentPage( + manifestPath = resolve("public", "asset-manifest.json"), +) { + return (req: Request, res: Response): void => { + res.set({ + "Cache-Control": "no-store", + "Referrer-Policy": "no-referrer", + "X-Content-Type-Options": "nosniff", + "X-Frame-Options": "DENY", + "Content-Security-Policy": + "default-src 'self'; script-src 'self'; connect-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; font-src 'self' data:; frame-src 'none'; object-src 'none'; base-uri 'none'; form-action 'self'; frame-ancestors 'none'", + }); + if ( + req.originalUrl !== "/review-link" || + (req.method !== "GET" && req.method !== "HEAD") + ) { + res.status(404).json({ error: "not_found" }); + return; + } + let manifest: Record = {}; + try { + if (existsSync(manifestPath)) { + const parsed: unknown = JSON.parse(readFileSync(manifestPath, "utf8")); + if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) + manifest = parsed as Record; + } + } catch { + // A missing or malformed manifest uses the ordinary unhashed assets. + } + const asset = (name: string): string => { + const value = manifest[name]; + const extension = name.endsWith(".js") ? "js" : "css"; + return typeof value === "string" && + new RegExp(`^[A-Za-z0-9][A-Za-z0-9_.-]*\\.${extension}$`).test(value) + ? value + : name; + }; + res.type("html").send(` +Artifact consent · Anonymous GitHub
`); + }; +} diff --git a/test/review-consent-page.test.js b/test/review-consent-page.test.js new file mode 100644 index 0000000..2dffd37 --- /dev/null +++ b/test/review-consent-page.test.js @@ -0,0 +1,127 @@ +require("ts-node/register/transpile-only"); +const { expect } = require("chai"); +const express = require("express"); +const http = require("http"); +const fs = require("fs"); +const os = require("os"); +const path = require("path"); +const { + createReviewConsentPage, + isReviewConsentPagePath, +} = require("../src/server/review-consent-page"); + +describe("isolated review consent page", function () { + let server, directory, manifest; + beforeEach(async function () { + directory = fs.mkdtempSync(path.join(os.tmpdir(), "review-page-")); + manifest = path.join(directory, "manifest.json"); + const app = express(); + const page = createReviewConsentPage(manifest); + app.use((req, res) => + isReviewConsentPagePath(req.path) + ? page(req, res) + : res.status(418).end(), + ); + server = await new Promise((resolve) => { + const s = app.listen(0, "127.0.0.1", () => resolve(s)); + }); + }); + afterEach(async function () { + await new Promise((resolve) => server.close(resolve)); + fs.rmSync(directory, { recursive: true, force: true }); + }); + function get(url, method = "GET") { + return new Promise((resolve, reject) => { + const req = http.request( + { + hostname: "127.0.0.1", + port: server.address().port, + path: url, + method, + }, + (res) => { + let body = ""; + res.on("data", (chunk) => { + body += chunk; + }); + res.on("end", () => + resolve({ status: res.statusCode, headers: res.headers, body }), + ); + }, + ); + req.on("error", reject); + req.end(); + }); + } + it("serves only first-party assets with no inline scripts or external frames", async function () { + const response = await get("/review-link"); + expect(response.status).equal(200); + expect(response.headers["cache-control"]).equal("no-store"); + expect(response.headers["referrer-policy"]).equal("no-referrer"); + expect(response.headers["x-frame-options"]).equal("DENY"); + expect(response.headers["content-security-policy"]) + .include("script-src 'self'") + .include("frame-ancestors 'none'"); + expect(response.body).include('name="review-consent-page"'); + expect(response.body).not.match(/https?:|kofi|ko-fi|/i); + expect(response.body.match(/`; res.contentType("text/html").send(html); } catch { // Raw Markdown must re-enter rendering when a transient failure clears. diff --git a/test/asset-build.test.js b/test/asset-build.test.js index 8b30d77..3fe4cdf 100644 --- a/test/asset-build.test.js +++ b/test/asset-build.test.js @@ -95,7 +95,7 @@ describe("asset build", function () { fs.writeFileSync(path.join(directory, groups.markdownFiles[0]), "function {"); const result = build(); expect(result.status).not.to.equal(0); - expect(result.stderr).to.include("uglify"); + expect(result.stderr).to.match(/uglify|ERROR/); expect(fs.existsSync(path.join(directory, "public/asset-manifest.json"))).to.equal(false); }); }); diff --git a/test/download-header-performance.test.js b/test/download-header-performance.test.js index 77d1bf6..2ccbf39 100644 --- a/test/download-header-performance.test.js +++ b/test/download-header-performance.test.js @@ -94,13 +94,13 @@ describe("download headers and website revalidation", function () { expect(second.headers["content-security-policy"]).to.include("sandbox"); }); it("renders Markdown again after a transient failure served the raw fallback", async function () { - const marked = require("marked").marked, render = marked.parse; + const markdown = require("../src/markdown"), render = markdown.renderMarkdown; let first; try { - marked.parse = () => { throw Error("transient rendering failure"); }; + markdown.renderMarkdown = () => { throw Error("transient rendering failure"); }; first = await fetch("/w/fixture/page.md"); expect(first.status).to.equal(200); expect(first.body).to.equal("fixture content"); - } finally { marked.parse = render; } + } finally { markdown.renderMarkdown = render; } const recovered = await fetch("/w/fixture/page.md", "GET", { "If-None-Match": first.headers.etag }); expect(recovered.status).to.equal(200); expect(recovered.body).to.include("Anonymized page"); expect(recovered.headers.etag).not.to.equal(first.headers.etag); diff --git a/test/fixtures/markdown/README.md b/test/fixtures/markdown/README.md new file mode 100644 index 0000000..e404f07 --- /dev/null +++ b/test/fixtures/markdown/README.md @@ -0,0 +1,5 @@ +# GFM extension fixtures + +`gfm-extensions.json` contains the tables, strikethrough, autolink and task-list examples from GitHub's [cmark-gfm extension tests](https://github.com/github/cmark-gfm/blob/27d942c8b0a62d192f616e5bf3578f4b6a89e180/test/extensions.txt), retrieved November 2, 2026. The examples are by Yuki Izumi and contributors and are licensed under [CC BY-SA 4.0](https://creativecommons.org/licenses/by-sa/4.0/). + +The Markdown and expected HTML are unchanged. They have been extracted into JSON with their section names. `` is upstream's crash-only test, not expected rendered HTML. The test normalizes HTML serialization and insignificant block whitespace, and sanitizes expected HTML with the production policy. Footnote links, alerts, math, media and diagrams have separate behavioral assertions in `markdown-compatibility.test.js`. diff --git a/test/fixtures/markdown/gfm-extensions.json b/test/fixtures/markdown/gfm-extensions.json new file mode 100644 index 0000000..f0461e4 --- /dev/null +++ b/test/fixtures/markdown/gfm-extensions.json @@ -0,0 +1,122 @@ +[ + { + "section": "Tables", + "markdown": "| abc | def |\n| --- | --- |\n| ghi | jkl |\n| mno | pqr |\n", + "html": "\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
abcdef
ghijkl
mnopqr
\n" + }, + { + "section": "Tables", + "markdown": "Hello!\n\n| _abc_ | セン |\n| ----- | ---- |\n| 1. Block elements inside cells don't work. | |\n| But _**inline elements do**_. | x |\n\nHi!\n", + "html": "

Hello!

\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
abcセン
1. Block elements inside cells don't work.
But inline elements do.x
\n

Hi!

\n" + }, + { + "section": "Tables", + "markdown": "| Not enough table | to be considered table |\n\n| Not enough table | to be considered table |\n| Not enough table | to be considered table |\n\n| Just enough table | to be considered table |\n| ----------------- | ---------------------- |\n\n| ---- | --- |\n\n|x|\n|-|\n\n| xyz |\n| --- |\n", + "html": "

| Not enough table | to be considered table |

\n

| Not enough table | to be considered table |\n| Not enough table | to be considered table |

\n\n\n\n\n\n\n\n
Just enough tableto be considered table
\n

| ---- | --- |

\n\n\n\n\n\n\n
x
\n\n\n\n\n\n\n
xyz
\n" + }, + { + "section": "Tables", + "markdown": "abc | def\n--- | ---\nxyz | ghi\n", + "html": "\n\n\n\n\n\n\n\n\n\n\n\n\n
abcdef
xyzghi
\n" + }, + { + "section": "Tables", + "markdown": "Hello!\n\n| _abc_ | セン |\n| ----- | ---- |\n| this row has a space at the end | | \n| But _**inline elements do**_. | x |\n\nHi!\n", + "html": "

Hello!

\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
abcセン
this row has a space at the end
But inline elements do.x
\n

Hi!

\n" + }, + { + "section": "Tables", + "markdown": "aaa | bbb | ccc | ddd | eee\n:-- | --- | :-: | --- | --:\nfff | ggg | hhh | iii | jjj\n", + "html": "\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
aaabbbcccdddeee
fffggghhhiiijjj
\n" + }, + { + "section": "Tables", + "markdown": "| a | b | c |\n| --- | --- |\n| this | isn't | okay |\n", + "html": "

| a | b | c |\n| --- | --- |\n| this | isn't | okay |

\n" + }, + { + "section": "Tables", + "markdown": "| a | b | c |\n| --- | --- | ---\n| x\n| a | b\n| 1 | 2 | 3 | 4 | 5 |\n", + "html": "\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
abc
x
ab
123
\n" + }, + { + "section": "Tables", + "markdown": "| a | b |\n| --- | --- |\n| Escaped pipes are \\|okay\\|. | Like \\| this. |\n| Within `\\|code\\| is okay` too. |\n| _**`c\\|`**_ \\| complex\n| don't **\\_reparse\\_**\n", + "html": "\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
ab
Escaped pipes are |okay|.Like | this.
Within |code| is okay too.
c| | complex
don't _reparse_
\n" + }, + { + "section": "Tables", + "markdown": "| a |\n--- |\n", + "html": "\n\n\n\n\n\n
a
\n" + }, + { + "section": "Tables", + "markdown": "| a | b |\n| --- | --- |\n| \\\\ | `\\\\` |\n| \\\\\\\\ | `\\\\\\\\` |\n| \\_ | `\\_` |\n| \\| | `\\|` |\n| \\a | `\\a` |\n\n\\\\ `\\\\`\n\n\\\\\\\\ `\\\\\\\\`\n\n\\_ `\\_`\n\n\\| `\\|`\n\n\\a `\\a`\n", + "html": "\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
ab
\\\\\\
\\\\\\\\\\\\
_\\_
||
\\a\\a
\n

\\ \\\\

\n

\\\\ \\\\\\\\

\n

_ \\_

\n

| \\|

\n

\\a \\a

\n" + }, + { + "section": "Tables", + "markdown": "| a |\n| --- |\n| hello |\n| ok
sure |\n", + "html": "\n\n\n\n\n\n\n\n\n\n\n\n\n\n
a
hello
ok
sure
\n" + }, + { + "section": "Tables", + "markdown": "Here's a link to [Freedom Planet 2][].\n\n| Here's a link to [Freedom Planet 2][] in a table header. |\n| --- |\n| Here's a link to [Freedom Planet 2][] in a table row. |\n\n[Freedom Planet 2]: http://www.freedomplanet2.com/\n", + "html": "

Here's a link to Freedom Planet 2.

\n\n\n\n\n\n\n\n\n\n\n\n
Here's a link to Freedom Planet 2 in a table header.
Here's a link to Freedom Planet 2 in a table row.
\n" + }, + { + "section": "Tables", + "markdown": "| a | b | c |\n| --- | --- | --- |\n| d || e |\n", + "html": "\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n
abc
de
\n" + }, + { + "section": "Tables", + "markdown": "| a | b |\n| --- | --- |\n|***(a)***|\n", + "html": "\n\n\n\n\n\n\n\n\n\n\n\n\n
ab
(a)
\n" + }, + { + "section": "Tables", + "markdown": "123\n456\n| a | b |\n| ---| --- |\nd | e\n", + "html": "

123\n456

\n\n\n\n\n\n\n\n\n\n\n\n\n\n
ab
de
\n" + }, + { + "section": "Strikethroughs", + "markdown": "A proper ~strikethrough~.\n", + "html": "

A proper strikethrough.

\n" + }, + { + "section": "Strikethroughs", + "markdown": "These are ~not strikethroughs.\n\nNo, they are not~\n\nThis ~is ~ legit~ isn't ~ legit.\n\nThis is not ~~~~~one~~~~~ huge strikethrough.\n\n~one~ ~~two~~ ~~~three~~~\n\nNo ~mismatch~~\n", + "html": "

These are ~not strikethroughs.

\n

No, they are not~

\n

This is ~ legit isn't ~ legit.

\n

This is not ~~~~~one~~~~~ huge strikethrough.

\n

one two ~~~three~~~

\n

No ~mismatch~~

\n" + }, + { + "section": "Autolinks", + "markdown": ": http://google.com https://google.com\n\n http://google.com/å\n\nscyther@pokemon.com\n\nscy.the_rbe-edr+ill@pokemon.com\n\nscyther@pokemon.com.\n\nscyther@pokemon.com/\n\nscyther@pokemon.com/beedrill@pokemon.com\n\nmailto:scyther@pokemon.com\n\nThis is a mailto:scyther@pokemon.com\n\nmailto:scyther@pokemon.com.\n\nmmmmailto:scyther@pokemon.com\n\nmailto:scyther@pokemon.com/\n\nmailto:scyther@pokemon.com/message\n\nmailto:scyther@pokemon.com/mailto:beedrill@pokemon.com\n\nxmpp:scyther@pokemon.com\n\nxmpp:scyther@pokemon.com.\n\nxmpp:scyther@pokemon.com/message\n\nxmpp:scyther@pokemon.com/message.\n\nEmail me at:scyther@pokemon.com\n\nwww.github.com www.github.com/á\n\nwww.google.com/a_b\n\nUnderscores not allowed in host name www.xxx.yyy._zzz\n\nUnderscores not allowed in host name www.xxx._yyy.zzz\n\nUnderscores allowed in domain name www._xxx.yyy.zzz\n\n**Autolink and http://inlines**\n\n![http://inline.com/image](http://inline.com/image)\n\na.w@b.c\n\nFull stop outside parens shouldn't be included http://google.com/ok.\n\n(Full stop inside parens shouldn't be included http://google.com/ok.)\n\n\"http://google.com\"\n\n'http://google.com'\n\nhttp://🍄.ga/ http://x🍄.ga/\n", + "html": "

: http://google.com https://google.com

\n

http://google.com/å http://google.com/å

\n

scyther@pokemon.com

\n

scy.the_rbe-edr+ill@pokemon.com

\n

scyther@pokemon.com.

\n

scyther@pokemon.com/

\n

scyther@pokemon.com/beedrill@pokemon.com

\n

mailto:scyther@pokemon.com

\n

This is a mailto:scyther@pokemon.com

\n

mailto:scyther@pokemon.com.

\n

mmmmailto:scyther@pokemon.com

\n

mailto:scyther@pokemon.com/

\n

mailto:scyther@pokemon.com/message

\n

mailto:scyther@pokemon.com/mailto:beedrill@pokemon.com

\n

xmpp:scyther@pokemon.com

\n

xmpp:scyther@pokemon.com.

\n

xmpp:scyther@pokemon.com/message

\n

xmpp:scyther@pokemon.com/message.

\n

Email me at:scyther@pokemon.com

\n

www.github.com www.github.com/á

\n

www.google.com/a_b

\n

Underscores not allowed in host name www.xxx.yyy._zzz

\n

Underscores not allowed in host name www.xxx._yyy.zzz

\n

Underscores allowed in domain name www._xxx.yyy.zzz

\n

Autolink and http://inlines

\n

\"http://inline.com/image\"

\n

a.w@b.c

\n

Full stop outside parens shouldn't be included http://google.com/ok.

\n

(Full stop inside parens shouldn't be included http://google.com/ok.)

\n

"http://google.com"

\n

'http://google.com'

\n

http://🍄.ga/ http://x🍄.ga/

\n" + }, + { + "section": "Autolinks", + "markdown": "This shouldn't crash everything: (_A_@_.A\n", + "html": "\n" + }, + { + "section": "Autolinks", + "markdown": "These should not link:\n\n* @a.b.c@. x\n* n@. b\n", + "html": "

These should not link:

\n
    \n
  • @a.b.c@. x
    • \n
  • n@. b
    • \n
\n" + }, + { + "section": "Task lists", + "markdown": "- [ ] foo\n- [x] bar\n", + "html": "
    \n
  • foo
    • \n
  • bar
    • \n
\n" + }, + { + "section": "Task lists", + "markdown": "- [x] foo\n - [ ] bar\n - [x] baz\n- [ ] bim\n\nShow a regular (non task) list to show that it has the same structure\n- [@] foo\n - [@] bar\n - [@] baz\n- [@] bim\n", + "html": "
    \n
  • foo\n
      \n
    • bar
      • \n
    • baz
      • \n
    \n
    • \n
  • bim
    • \n
\n

Show a regular (non task) list to show that it has the same structure

\n
    \n
  • [@] foo\n
      \n
    • [@] bar
      • \n
    • [@] baz
      • \n
    \n
    • \n
  • [@] bim
    • \n
\n" + }, + { + "section": "Task lists", + "markdown": "- [x] foo\n - [ ] bar\n - [x] baz\n- [ ] bim\n\nShow a regular (non task) list to show that it has the same structure\n- [@] foo\n - [@] bar\n - [@] baz\n- [@] bim\n", + "html": "
    \n
  • foo\n
      \n
    • bar
      • \n
    • baz
      • \n
    \n
    • \n
  • bim
    • \n
\n

Show a regular (non task) list to show that it has the same structure

\n
    \n
  • [@] foo\n
      \n
    • [@] bar
      • \n
    • [@] baz
      • \n
    \n
    • \n
  • [@] bim
    • \n
\n" + } +] diff --git a/test/markdown-compatibility.test.js b/test/markdown-compatibility.test.js new file mode 100644 index 0000000..f6df8b9 --- /dev/null +++ b/test/markdown-compatibility.test.js @@ -0,0 +1,239 @@ +const normalizeFootnotes = html => html.replace(/footnote-(?:[a-f0-9]+-){4}/g, "footnote-"); +const { expect } = require("chai"); +const { JSDOM } = require("jsdom"); +const fs = require("node:fs"); +const path = require("node:path"); +require("ts-node/register/transpile-only"); +const { renderMarkdown, sanitizeMarkdownHtml } = require("../src/markdown"); +const examples = require("./fixtures/markdown/gfm-extensions.json"); +const fragment = source => JSDOM.fragment(renderMarkdown(source)); +const stl = "solid triangle\nfacet normal 0 0 1\nouter loop\nvertex 0 0 0\nvertex 1 0 0\nvertex 0 1 0\nendloop\nendfacet\nendsolid triangle"; + +function normalize(html) { + const dom = JSDOM.fragment(html); + // Formatting whitespace between block elements and boolean-attribute spelling + // have no semantic effect; keep whitespace inside text and code intact. + for (const element of dom.querySelectorAll("*")) { + for (const node of [...element.childNodes]) { + if (node.nodeType === 3 && element.tagName !== "PRE" && element.tagName !== "CODE") { + if (/^\s*$/.test(node.textContent) && /^(TABLE|THEAD|TBODY|TR|UL|OL|SECTION|DIV)$/.test(element.tagName)) node.remove(); + else if (node.nextSibling?.nodeName === "UL") node.textContent = node.textContent.trimEnd(); + } + } + } + const container = dom.ownerDocument.createElement("div"); container.append(dom); + return container.innerHTML.trim(); +} + +describe("GitHub Markdown compatibility", function () { + for (const [index, example] of examples.entries()) { + it(`cmark-gfm ${example.section} example ${index + 1}`, function () { + if (example.html.trim() === "") { + expect(() => renderMarkdown(example.markdown)).not.to.throw(); + return; + } + expect(normalize(renderMarkdown(example.markdown))).to.equal(normalize(sanitizeMarkdownHtml(example.html))); + }); + } + + const cases = [ + ["setext headings", "Title\n=====\n\nSubtitle\n--------", "h1#title + h2#subtitle"], + ["nested emphasis", "***both*** and ~~deleted~~", "em strong, strong em"], + ["reference links", "[link][ref]\n\n[ref]: https://example.com \"Title\"", 'a[href="https://example.com"][title="Title"]'], + ["nested lists", "1. First\n - Nested\n - Deep", "ol ul ul li"], + ["ordered list starts", "42. Forty-two", 'ol[start="42"]'], + ["hard line breaks", "one \ntwo\\\nthree", "p br"], + ["autolink email", "", 'a[href="mailto:reader@example.com"]'], + ["disclosures", "
More\n\n**Details**\n\n
", "details[open] summary"], + ["keyboard text", "Ctrl + C", "kbd"], + ["subscript and superscript", "H2O and x2", "sub + sup, sub"], + ["custom anchors", '\n\n[Go](#custom)', 'a[name="custom"]'], + ["picture sources", 'Logo', 'picture source[media][srcset]'], + ["aligned HTML", '

Centered

', 'p[align="center"]'], + ["theme images", '![Dark](logo.png#gh-dark-mode-only)', 'img.gh-dark-mode-only'], + ["video links", '[Movie](clip.mp4?download=1)', 'video[controls][src="clip.mp4?download=1"]'], + ["image dimensions", '', 'img[width="120"][height="80"][align="right"]'], + ["table spans", '
Heading
Cell
', 'th[colspan="2"]'], + ["emoji", ":smile: :+1: :unknown_emoji:", 'img.emoji[alt=":smile:"]'], + ["current emoji shortcodes", ":melting_face: :saluting_face: :pink_heart:", 'img.emoji[alt=":melting_face:"]'], + ["syntax highlighting", "```javascript\nconst answer = 42;\n```", "code.language-javascript .hljs-keyword"], + ["inline math", "Value $x^2$.", ".katex math msup"], + ["backtick math", "Value $`x^2`$.", ".katex math msup"], + ["display math", "$$\nx^2\n$$", ".katex-display math"], + ["same-line display math", "$$x^2$$", ".katex-display math"], + ["math fences", "~~~math\nx^2\n~~~", ".katex-display math"], + ["Mermaid tilde fences", "~~~mermaid\ngraph TD; A-->B\n~~~", "pre.mermaid"], + ["Mermaid long fences", "````mermaid\ngraph TD; A-->B\n````", "pre.mermaid"], + ["GeoJSON", '```geojson\n{"type":"LineString","coordinates":[[0,0],[10,10]]}\n```', 'figure[data-diagram="geojson"] svg path'], + ["TopoJSON", '```topojson\n{"type":"Topology","objects":{"line":{"type":"LineString","arcs":[0]}},"arcs":[[[0,0],[10,10]]]}\n```', 'figure[data-diagram="topojson"] svg path'], + ["ASCII STL", `\`\`\`stl\n${stl}\n\`\`\``, 'figure[data-diagram="stl"] svg path'], + ]; + for (const [name, source, selector] of cases) it(`renders ${name}`, function () { + expect(fragment(source).querySelector(selector), name).not.to.equal(null); + }); + for (const type of ["NOTE", "TIP", "IMPORTANT", "WARNING", "CAUTION"]) it(`renders ${type} alerts`, function () { + const dom = fragment(`> [!${type}]\n> **Read** this.\n>\n> - Details`); + const alert = dom.querySelector(`.markdown-alert-${type.toLowerCase()}`); + expect(alert.querySelector("svg")).not.to.equal(null); + expect(alert.querySelector("strong").textContent).to.equal("Read"); + expect(alert.querySelector("li").textContent).to.equal("Details"); + }); + it("does not turn nested quotes into alerts", function () { + expect(fragment("> Outer\n>\n> > [!NOTE]\n> > Nested").querySelector(".markdown-alert")).to.equal(null); + }); + it("resolves all repeated footnote references and backrefs", function () { + const dom = fragment("One[^long] and two[^long].\n\n[^long]: First **paragraph**.\n\n Second paragraph."); + for (const link of dom.querySelectorAll('a[href^="#"]')) { + expect(dom.querySelector(`[id="${link.getAttribute("href").slice(1)}"]`), link.outerHTML).not.to.equal(null); + } + expect(dom.querySelectorAll("[data-footnote-backref]")).to.have.length(2); + expect(dom.querySelector(".footnotes").textContent).to.include("Second paragraph."); + }); + it("keeps footnote navigation and accessibility targets within each document", function () { + const source = "One[^a].\n\n[^a]: Details"; + const dom = JSDOM.fragment(`
${renderMarkdown(source)}
${renderMarkdown(source)}
`); + const ids = [...dom.querySelectorAll("[id]")].map(node => node.id); + expect(new Set(ids).size).to.equal(ids.length); + for (const article of dom.querySelectorAll("article")) { + for (const node of article.querySelectorAll("[href], [aria-describedby]")) { + const target = node.getAttribute("aria-describedby") || node.getAttribute("href").slice(1); + expect(article.querySelector(`[id="${target}"]`)).not.to.equal(null); + } + } + }); + for (const source of ["Costs $5 and $10 per run.", "Budget: $100-$200."]) it(`preserves currency prose: ${source}`, function () { + const dom = fragment(source); + expect(dom.querySelector(".katex")).to.equal(null); + expect(dom.textContent.trim()).to.equal(source); + }); + it("uses unique GitHub heading slugs including suffixed collisions and Unicode", function () { + const dom = fragment("# Hello\n# Hello\n# Hello-1\n# Hello\n# 中文 *标题*\n# constructor"); + expect([...dom.querySelectorAll("h1")].map(h => h.id)).to.deep.equal(["hello", "hello-1", "hello-1-1", "hello-2", "中文-标题", "constructor"]); + }); + it("keeps formatting syntax literal inside code and escaped text", function () { + const dom = fragment("`$x$ :smile: [^a]`\n\n```text\n> [!NOTE]\n$x$ :smile:\n```\n\n\\*literal\\* and \\$5"); + expect(dom.querySelector(".katex, .emoji, .markdown-alert")).to.equal(null); + expect(dom.textContent).to.include("*literal* and $5"); + }); + it("resolves Markdown and HTML links/images without rewriting code or fragment links", function () { + const dom = JSDOM.fragment(renderMarkdown('[Guide](../guide.md#top) [Local](#top)\n\n\n\n``', "/r/repo/docs/")); + expect(dom.querySelector("a").getAttribute("href")).to.equal("/r/repo/guide.md#top"); + expect(dom.querySelectorAll("a")[1].getAttribute("href")).to.equal("#top"); + expect(dom.querySelector("img").getAttribute("srcset")).to.equal("/r/repo/docs/x.png 1x, /r/repo/docs/x2.png 2x"); + expect(dom.querySelector("code").textContent).to.equal(''); + }); + it("renders GeoJSON polygons rather than their spherical complement", function () { + const source = '```geojson\n{"type":"Polygon","coordinates":[[[-90,35],[-90,30],[-85,30],[-85,35],[-90,35]]]}\n```'; + const path = fragment(source).querySelector("svg path").getAttribute("d"); + expect(path.match(/M/g)).to.have.length(1); + expect(path).not.to.match(/NaN|Infinity/); + }); + it("centers a lone GeoJSON feature at its real coordinates", function () { + const source = '```geojson\n{"type":"Feature","properties":{},"geometry":{"type":"Point","coordinates":[120,45]}}\n```'; + expect(fragment(source).querySelector("svg path").getAttribute("d")).to.include("M320,160"); + }); + it("does not leak references, slug counts, macros or base URLs between documents", function () { + renderMarkdown("# Title\n\n[ref]: a.md\n\n[^a]: note\n\n$\\gdef\\custom{hello}$", "/first/"); + const dom = JSDOM.fragment(renderMarkdown("# Title\n\n[ref] [^a] [file](b.md) $\\custom$", "/second/")); + expect(dom.querySelector("h1").id).to.equal("title"); + expect(dom.textContent).to.include("[ref] [^a]"); + expect(dom.querySelector('a[href="/second/b.md"]')).not.to.equal(null); + expect(dom.querySelector(".katex").textContent).not.to.include("hello"); + }); + for (const language of ["geojson", "topojson", "stl"]) it(`keeps invalid ${language} readable`, function () { + const dom = fragment(`\`\`\`${language}\nnot a diagram \n\`\`\``); + expect(dom.querySelector("pre code").textContent).to.include("not a diagram"); + expect(dom.querySelector("script")).to.equal(null); + }); + it("blocks active content through HTML, TeX, images and generated-content spoofing", function () { + const dom = fragment('\n\nSafe\n\n$\\href{javascript:alert(1)}{click}$'); + expect(dom.querySelector("script,svg,[onload],[autofocus]")).to.equal(null); + expect(dom.querySelector("input").disabled).to.equal(true); + expect(dom.querySelector("input").type).to.equal("checkbox"); + for (const el of dom.querySelectorAll("[href],[src],[srcset]")) expect(el.outerHTML).not.to.include("javascript:"); + }); + + it("serves the shared renderer through the project-page route", async function () { + this.timeout(10000); // Includes cold TypeScript compilation of the route dependencies. + const { Readable } = require("node:stream"); + const utils = require("../src/server/routes/route-utils"); + const File = require("../src/core/AnonymizedFile").default; + const restores = []; + const stub = (object, key, value) => { const previous = object[key]; restores.push(() => { object[key] = previous; }); object[key] = value; }; + try { + stub(utils, "getRepo", async () => ({ options: { terms: [], page: true, pageSource: { path: "/", branch: "main" } }, model: { source: { branch: "main" } } })); + stub(File.prototype, "getFileInfo", async () => ({ name: "README.md", path: "", size: 100 })); + stub(File.prototype, "originalPath", async () => "README.md"); + stub(File.prototype, "isFileSupported", () => true); + stub(File.prototype, "extension", () => "md"); + stub(File.prototype, "sha", async () => "sha"); + stub(File.prototype, "anonymizedContent", async () => Readable.from(["# Title\n\n> [!TIP]\n> Text with $x^2$ and note[^a].\n\n[^a]: Details"])); + stub(File.prototype, "send", () => { throw new Error("Unexpected raw fallback"); }); + stub(utils, "handleError", error => { throw error; }); + const headers = {}; + let html; + const res = { + header(name, value) { headers[name] = value; return this; }, + set(name, value) { headers[name] = value; return this; }, + contentType() { return this; }, send(value) { html = value; return this; }, + }; + const handler = require("../src/server/routes/webview").default.stack[0].route.stack[0].handle; + await handler({ path: "/repo/README.md", params: { repoId: "repo" }, headers: {}, method: "GET" }, res); + const page = JSDOM.fragment(html); + expect(page.querySelector("#title")).not.to.equal(null); + expect(page.querySelector(".markdown-alert-tip .katex")).not.to.equal(null); + expect(page.querySelector(".footnotes")).not.to.equal(null); + expect(html).to.include('/script/markdown.min.js'); + expect(headers["Content-Security-Policy"]).to.include("sandbox"); + expect(headers["Content-Security-Policy"]).not.to.include("allow-same-origin"); + } finally { restores.reverse().forEach(restore => restore()); } + }); + + describe("shipped browser bundle", function () { + let dom; + before(function () { + dom = new JSDOM("
", { runScripts: "outside-only", url: "https://viewer.test/" }); + dom.window.Object.hasOwn = undefined; + dom.window.eval(fs.readFileSync(path.join(__dirname, "../public/script/markdown.min.js"), "utf8")); + dom.window.eval(fs.readFileSync(path.join(__dirname, "../public/script/utils.js"), "utf8")); + }); + after(function () { dom.window.close(); }); + it("matches the server renderer across the feature matrix", function () { + for (const [name, source] of cases) { + expect(normalizeFootnotes(dom.window.renderMD(source, "/r/repo/")), name).to.equal(normalizeFootnotes(renderMarkdown(source, "/r/repo/"))); + } + }); + it("enhances Mermaid inserted while another diagram is rendering", async function () { + const { setTimeout: delay } = require("node:timers/promises"); + const sources = []; + let settings; + dom.window.mermaid = { + initialize(options) { settings = options; }, + async run({ nodes }) { + sources.push(nodes[0].textContent); + nodes[0].dataset.processed = "true"; + if (sources.length === 1) { + dom.window.document.querySelector("main").insertAdjacentHTML("beforeend", dom.window.renderMD("~~~mermaid\ngraph TD; B-->C\n~~~")); + await delay(10); + } + }, + }; + dom.window.document.querySelector("main").innerHTML = dom.window.renderMD("```mermaid\ngraph TD; A-->B\n```"); + await delay(80); + expect(settings.securityLevel).to.equal("strict"); + expect(sources).to.deep.equal(["graph TD; A-->B", "graph TD; B-->C"]); + }); + it("supports keyboard-activated diagram zoom and model rotation", function () { + dom.window.document.querySelector("main").innerHTML = dom.window.renderMD(`\`\`\`stl\n${stl}\n\`\`\``); + const figure = dom.window.document.querySelector("figure"); + figure.querySelector('[data-diagram-action="in"]').click(); + expect(figure.querySelector("g").getAttribute("transform")).to.include("scale(1.25)"); + const before = figure.querySelector("path").getAttribute("d"); + figure.querySelector('[data-diagram-action="rotate"]').click(); + expect(figure.querySelector("path").getAttribute("d")).not.to.equal(before); + figure.querySelector('[data-diagram-action="reset"]').click(); + expect(figure.querySelector("g").getAttribute("transform")).to.include("scale(1)"); + expect(figure.querySelector("path").getAttribute("d")).to.equal(before); + }); + }); +}); diff --git a/test/markdown-fonts.test.js b/test/markdown-fonts.test.js new file mode 100644 index 0000000..03ad026 --- /dev/null +++ b/test/markdown-fonts.test.js @@ -0,0 +1,35 @@ +const { expect } = require("chai"); +const express = require("express"); +const fs = require("node:fs"); +const path = require("node:path"); +const ts = require("typescript"); + +// Exercise the production static middleware without starting databases/workers. +function staticApp() { + const app = express(); + const source = fs.readFileSync(path.join(__dirname, "../src/server/index.ts"), "utf8"); + const start = source.indexOf(" app.use(\n express.static"); + const end = source.indexOf("\n\n app", start); + new Function("app", "express", "join", ts.transpileModule(source.slice(start, end), {}).outputText)(app, express, (...parts) => path.join(__dirname, "..", ...parts)); + return app; +} +module.exports = { staticApp }; + +if (typeof describe === "function") describe("sandboxed Markdown font requests", function () { + let server; + before(async function () { server = await new Promise(resolve => { const listener = staticApp().listen(0, "127.0.0.1", () => resolve(listener)); }); }); + after(async function () { if (server) await new Promise(resolve => server.close(resolve)); }); + it("allows anonymous opaque-origin font requests", async function () { + const response = await globalThis.fetch(`http://127.0.0.1:${server.address().port}/css/fonts/KaTeX_Main-Regular.woff2`, { headers: { Origin: "null" } }); + expect(response.status).to.equal(200); + expect(response.headers.get("access-control-allow-origin")).to.equal("*"); + expect(response.headers.get("access-control-allow-credentials")).to.equal(null); + await response.arrayBuffer(); + }); + it("does not enable CORS for unrelated assets", async function () { + const response = await globalThis.fetch(`http://127.0.0.1:${server.address().port}/script/markdown.min.js`, { headers: { Origin: "null" } }); + expect(response.status).to.equal(200); + expect(response.headers.get("access-control-allow-origin")).to.equal(null); + await response.arrayBuffer(); + }); +}); diff --git a/test/markdown-sanitization.test.js b/test/markdown-sanitization.test.js index 9cae728..7fcc735 100644 --- a/test/markdown-sanitization.test.js +++ b/test/markdown-sanitization.test.js @@ -1,42 +1,6 @@ const { expect } = require("chai"); -const { marked } = require("marked"); -const sanitizeHtml = require("sanitize-html"); - -const sanitizeOptions = { - allowedTags: sanitizeHtml.defaults.allowedTags.concat([ - "img", - "video", - "input", - "details", - "summary", - "del", - "ins", - "sup", - "sub", - ]), - allowedAttributes: { - ...sanitizeHtml.defaults.allowedAttributes, - img: ["src", "srcset", "alt", "title", "width", "height", "loading"], - video: ["src", "controls", "title"], - input: ["type", "checked", "disabled"], - code: ["class"], - span: ["class"], - div: ["class"], - pre: ["class"], - td: ["align"], - th: ["align"], - }, - allowedSchemes: ["http", "https", "mailto"], -}; - -/** - * Mirrors the server-side rendering pipeline in webview.ts: - * sanitizeHtml(marked(content, opts), sanitizeOptions) - */ -function renderAndSanitize(markdown) { - const raw = marked(markdown, { async: false }); - return sanitizeHtml(raw, sanitizeOptions); -} +require("ts-node/register/transpile-only"); +const { renderMarkdown: renderAndSanitize } = require("../src/markdown"); describe("Markdown sanitization", function () { // --------------------------------------------------------------- @@ -192,9 +156,9 @@ describe("Markdown sanitization", function () { describe("preserves safe markdown content", function () { it("preserves headings", function () { const html = renderAndSanitize("# Heading 1\n## Heading 2"); - expect(html).to.include("

"); + expect(html).to.match(/

/); expect(html).to.include("Heading 1"); - expect(html).to.include("

"); + expect(html).to.match(/

/); }); it("preserves paragraphs", function () { @@ -226,7 +190,7 @@ describe("Markdown sanitization", function () { it("preserves code blocks", function () { const html = renderAndSanitize("```js\nconsole.log('hi')\n```"); expect(html).to.include(" html.replace(/footnote-(?:[a-f0-9]+-){4}/g, "footnote-"); +const { expect } = require("chai"); +const fs = require("node:fs"); +const path = require("node:path"); +const vm = require("node:vm"); +const { JSDOM } = require("jsdom"); +require("ts-node/register/transpile-only"); +const { renderMarkdown } = require("../src/markdown"); +const { renderDiagram } = require("../src/markdown-diagrams"); +const root = path.resolve(__dirname, ".."); + +function checkHtml(html, allowMediaData = false) { + const dom = JSDOM.fragment(html); + expect(dom.querySelector("script,iframe,object,embed,form,base,meta,link")).to.equal(null); + for (const element of dom.querySelectorAll("*")) { + for (const attr of element.attributes) { + expect(attr.name).not.to.match(/^on/i); + if (["href", "xlink:href", "src", "srcset", "poster"].includes(attr.name)) { + // Strip URL control characters before checking executable schemes. + // eslint-disable-next-line no-control-regex + expect(attr.value.replace(/[\u0000-\u0020]/g, "")).not.to.match(allowMediaData && ["src", "srcset", "poster"].includes(attr.name) ? /(?:javascript|vbscript):/i : /(?:javascript|vbscript|data):/i); + } + } + } +} + +describe("Markdown security boundaries", function () { + const attacks = [ + '', + 'link', + '', + '', + '

', + '
More
', + '', + 'Text[^a" onclick="alert(1)]\n\n[^a" onclick="alert(1)]: Note', + '$\\href{javascript:alert(1)}{click}$', + '$\\url{javascript:alert(1)}$', + '$\\includegraphics{https://evil.test/tracker.svg}$', + '$\\htmlData{evil=1 onmouseover=alert(1)}{x}$', + '$\\htmlStyle{background:url(https://evil.test/tracker)}{x}$', + '', + '```javascript\n\n```', + '```mermaid\ngraph TD; A[""]\n```', + ]; + let browser; + before(function () { + browser = new JSDOM("", { runScripts: "outside-only", url: "https://viewer.test/" }); + vm.runInContext(fs.readFileSync(path.join(root, "public/script/markdown.min.js"), "utf8"), browser.getInternalVMContext()); + }); + after(function () { browser.window.close(); }); + for (const [index, source] of attacks.entries()) it(`rejects active content payload ${index + 1} on server and browser`, function () { + const server = renderMarkdown(source); + const client = browser.window.MarkdownRenderer.render(source); + checkHtml(server); checkHtml(client); + expect(normalizeFootnotes(client)).to.equal(normalizeFootnotes(server)); + }); + + it("sanitizes mutation payloads with the shipped DOMPurify", function () { + const dom = new JSDOM("", { runScripts: "outside-only" }); + try { + vm.runInContext(fs.readFileSync(path.join(root, "public/script/external/purify.min.js"), "utf8"), dom.getInternalVMContext()); + for (const source of attacks.slice(0, 7)) checkHtml(dom.window.DOMPurify.sanitize(source), true); + } finally { dom.window.close(); } + }); + + it("enforces strict Mermaid mode even when source HTML supplies an initialization flag", async function () { + const dom = new JSDOM('
graph TD; A-->B
', { runScripts: "outside-only" }); + let configuration; + try { + dom.window.mermaid = { + initialize(value) { configuration = value; }, + async run({ nodes }) { nodes.forEach(node => { node.dataset.processed = "true"; }); }, + }; + vm.runInContext(fs.readFileSync(path.join(root, "public/script/markdown.min.js"), "utf8"), dom.getInternalVMContext()); + await new Promise(resolve => require("node:timers").setTimeout(resolve, 0)); + expect(configuration.securityLevel).to.equal("strict"); + } finally { dom.window.close(); } + }); + + for (const [library, source, publicFile] of [ + ["Mermaid", "node_modules/mermaid/dist/mermaid.min.js", "public/script/external/mermaid.min.js"], + ["DOMPurify", "node_modules/dompurify/dist/purify.min.js", "public/script/external/purify.min.js"], + ["KaTeX", "node_modules/katex/dist/katex.min.js", "public/script/external/katex.min.js"], + ["KaTeX CSS", "node_modules/katex/dist/katex.min.css", "public/css/katex.min.css"], + ]) it(`ships the audited ${library} version without an outdated vendored copy`, function () { + expect(fs.readFileSync(path.join(root, publicFile)).equals(fs.readFileSync(path.join(root, source)))).to.equal(true); + }); + + it("rejects array-like objects before TopoJSON loops over their forged length", function () { + const topology = { type: "Topology", arcs: [{ 0: [0, 0], length: 1e12 }], objects: { line: { type: "LineString", arcs: [0] } } }; + expect(() => renderDiagram(JSON.stringify(topology), "topojson")).to.throw("array"); + }); + it("bounds repeated TopoJSON arc expansion before allocating coordinates", function () { + const topology = { type: "Topology", arcs: [Array.from({ length: 1000 }, (_, i) => [i / 100, 0])], objects: { line: { type: "LineString", arcs: Array(1000).fill(0) } } }; + expect(() => renderDiagram(JSON.stringify(topology), "topojson")).to.throw("expansion limit"); + }); + it("rejects deeply nested geometry before recursive library calls", function () { + let geometry = { type: "Point", coordinates: [0, 0] }; + for (let i = 0; i < 40; i++) geometry = { type: "GeometryCollection", geometries: [geometry] }; + expect(() => renderDiagram(JSON.stringify(geometry), "geojson")).to.throw("complexity limit"); + }); + it("rejects nonnumeric coordinate objects without invoking D3", function () { + expect(() => renderDiagram('{"type":"Point","coordinates":[{"valueOf":"attack"},0]}', "geojson")).to.throw("position"); + }); + it("bounds diagram input size and keeps rejected source readable", function () { + expect(() => renderDiagram(" ".repeat(1_000_001), "stl")).to.throw("source limit"); + const source = '```geojson\n{"type":"Point","coordinates":[null,0]}\n```'; + expect(JSDOM.fragment(renderMarkdown(source)).querySelector("pre code").textContent).to.include('"coordinates":[null,0]'); + }); +}); From 6099356e0e1a6b2c533bc548805784f4ddb7471c Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Mon, 2 Nov 2026 08:18:04 +0100 Subject: [PATCH 47/48] feat: add scoped repository maintenance access for LLM clients (#913) * feat: add scoped repository maintenance access for LLM clients * fix: translate maintenance API errors --- README.md | 10 + public/i18n/locale-en.json | 9 + public/llms.txt | 70 +++ public/maintenance-openapi.json | 608 ++++++++++++++++++++++++ src/core/model/users/users.schema.ts | 3 + src/core/model/users/users.types.ts | 3 + src/server/index.ts | 45 +- src/server/routes/connection.ts | 6 +- src/server/routes/maintenance-limits.ts | 28 ++ src/server/routes/maintenance-tokens.ts | 62 +++ src/server/routes/maintenance.ts | 59 +++ src/server/routes/token-auth.ts | 36 +- test/maintenance-api.test.js | 297 ++++++++++++ 13 files changed, 1219 insertions(+), 17 deletions(-) create mode 100644 public/llms.txt create mode 100644 public/maintenance-openapi.json create mode 100644 src/server/routes/maintenance-limits.ts create mode 100644 src/server/routes/maintenance-tokens.ts create mode 100644 src/server/routes/maintenance.ts create mode 100644 test/maintenance-api.test.js diff --git a/README.md b/README.md index 5df6034..0aeece3 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,16 @@ npm install -g @tdurieux/anonymous_github anonymous_github ``` +### LLM and automation clients + +Use expiring, repository-scoped maintenance tokens to inspect saved status, +edit anonymization settings, refresh snapshots, and extend expiration. +A signed-in owner or coauthor can issue and revoke tokens through +`/api/user/maintenance-tokens`. See the [client guide](public/llms.txt) and +[OpenAPI description](public/maintenance-openapi.json), also served at +`/llms.txt` and `/maintenance-openapi.json` on each instance. +These tokens cannot access GitHub previews, file downloads, or admin routes. + ## Self-hosting Local installation requires Node.js 22.15 or newer in the 22.x series, diff --git a/public/i18n/locale-en.json b/public/i18n/locale-en.json index a8284a5..dee6ad3 100644 --- a/public/i18n/locale-en.json +++ b/public/i18n/locale-en.json @@ -117,6 +117,15 @@ "gistId_not_specified": "A gist ID must be provided.", "gistId_already_used": "The gist ID is already used.", "missing_token": "An authentication token is required.", + "token_attempts_rate_limited": "Too many token authentication attempts. Wait before trying again.", + "refresh_rate_limited": "A refresh was already requested for this repository. Wait one minute before trying again.", + "session_required": "Sign in through your browser to manage maintenance tokens.", + "invalid_token_options": "Provide a token name, 1 to 20 repository IDs, and an expiration of 1 to 90 days.", + "token_limit_reached": "You have reached the limit of 50 API tokens. Revoke an unused token before creating another.", + "invalid_token_id": "The token ID is invalid.", + "invalid_settings": "Provide supported settings with boolean values, or a list of anonymization terms.", + "token_scope_forbidden": "This token does not allow this operation or repository.", + "authentication_unavailable": "Authentication is temporarily unavailable. Try again later.", "invalid_token": "The provided authentication token is invalid.", "login_failed": "Login failed. Please try again.", "server_error": "An unexpected server error occurred. Please try again later.", diff --git a/public/llms.txt b/public/llms.txt new file mode 100644 index 0000000..7291787 --- /dev/null +++ b/public/llms.txt @@ -0,0 +1,70 @@ +# Anonymous GitHub repository maintenance + +Anonymous GitHub publishes anonymized snapshots for peer review. Use the +maintenance API to manage existing snapshots you own or coauthor. It is not a +GitHub API proxy and does not edit the upstream GitHub repository. + +Machine-readable API description: /maintenance-openapi.json +All paths below are relative to this instance. Send JSON request bodies and +Authorization: Bearer . Keep tokens in a secret store; +never put them in URLs, prompts, source control, or logs. + +## Connect a client + +A person signs in with GitHub and creates the anonymized repository first. +In their signed-in browser session, POST /api/user/maintenance-tokens with: +{"name":"Paper maintenance","repoIds":["paper-123"],"expiresInDays":30} + +The response contains the token once, its id, selected repoIds and expiresAt. +Expiry is 1 to 90 days, default 30. Select 1 to 20 existing repository IDs. +There is a limit of 50 stored API tokens per account. A person can inspect +GET /api/user/maintenance-tokens and revoke one with +DELETE /api/user/maintenance-tokens/{id} using the same browser session. +These endpoints require a session, not a bearer token. Do not hand the client +an admin token or browser cookie. Maintenance tokens cannot log into the UI. + +## Maintain a repository + +1. GET /api/maintenance/repositories/{repoId} reads saved status, branch, + commit and options without querying GitHub or downloading content. +2. PATCH the same URL with only the settings to change, for example: + {"terms":["Alice Smith","Example University"],"image":false,"update":false} + Omitted settings stay unchanged. terms replaces the whole term list; + [] clears it. Supported boolean fields: update, image, pdf, notebook, link. + update controls automatic snapshot updates. Settings edits work in ready + or error state, and cannot change the upstream source or connection. +3. POST /api/repo/{repoId}/refresh with {} requests the latest snapshot of the + saved branch using the repository's existing connection. Poll the saved + status endpoint every 30 seconds until ready or error. Do not repeat + refresh while work is in progress. +4. POST /api/repo/{repoId}/extend with {} adds six months to the current + expiration, or to today if already expired. An expired snapshot is queued + for restoration. This operation is NOT idempotent. Read the expiration + date after an ambiguous network failure before deciding whether to retry. + +Refresh completion is asynchronous. A successful response does not mean the +snapshot is ready. Refresh uses the existing GitHub quota and preparation +limits. All maintenance tokens for an account share an API rate budget, +even across different client IPs. Each account can request one refresh per +repository per minute, shared across its tokens. Bearer authentication attempts +also have a limit of 120 per minute per IP, including invalid tokens. Both limits +return HTTP 429 with Retry-After. Do not use maintenance calls to crawl GitHub or retrieve source files. + +## Errors and access limits + +Errors normally contain {"error":"code"}. The instance rate limiter may +return text for HTTP 429; always check status and Retry-After first. +401: token expired, revoked or invalid. Ask the owner to replace it. +403: token_scope_forbidden or not_authorized. The token must name this +repository and its account must still be an owner, coauthor or administrator. +404: repository does not exist. +409: state changed or operation is unavailable. Read status before retrying. +429: back off until Retry-After or resetAt, when supplied. Never retry in a loop. +5xx: retry reads with backoff. Inspect state before retrying a write. + +Scoped tokens cannot create repositories, change source repositories, delete +snapshots, manage coauthors, search GitHub, preview arbitrary branches or files, +download archives, read file contents, access admin routes, or mint tokens. +Use the signed-in UI for those management tasks. Existing public review links +keep their usual access rules. Revocation and membership checks apply on each +maintenance request. Never publish private maintenance responses as review links. diff --git a/public/maintenance-openapi.json b/public/maintenance-openapi.json new file mode 100644 index 0000000..00c90d0 --- /dev/null +++ b/public/maintenance-openapi.json @@ -0,0 +1,608 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "Anonymous GitHub maintenance API", + "version": "1.0.0", + "description": "Maintain existing anonymized snapshots. Session-only token issuance is documented at /llms.txt. No GitHub proxy or file download access." + }, + "servers": [ + { + "url": "/" + } + ], + "security": [ + { + "maintenanceToken": [] + } + ], + "components": { + "securitySchemes": { + "maintenanceToken": { + "type": "http", + "scheme": "bearer" + } + }, + "schemas": { + "Error": { + "type": "object", + "required": [ + "error" + ], + "properties": { + "error": { + "type": "string" + }, + "resetAt": { + "type": "number", + "description": "Unix timestamp in milliseconds, when available." + } + } + }, + "RepositoryStatus": { + "type": "object", + "required": [ + "repoId", + "options" + ], + "properties": { + "repoId": { + "type": "string" + }, + "status": { + "type": "string" + }, + "statusMessage": { + "type": "string" + }, + "statusDate": { + "type": "string", + "format": "date-time" + }, + "settingsSavedAt": { + "type": "string", + "format": "date-time" + }, + "source": { + "type": "object", + "properties": { + "branch": { + "type": "string" + }, + "commit": { + "type": "string" + } + } + }, + "options": { + "type": "object", + "properties": { + "terms": { + "type": "array", + "maxItems": 1000, + "items": { + "type": "string", + "minLength": 1, + "maxLength": 1000 + } + }, + "update": { + "type": "boolean" + }, + "image": { + "type": "boolean" + }, + "pdf": { + "type": "boolean" + }, + "notebook": { + "type": "boolean" + }, + "link": { + "type": "boolean" + }, + "expirationDate": { + "type": "string", + "format": "date-time" + }, + "expirationMode": { + "type": "string", + "enum": [ + "never", + "redirect", + "remove" + ] + }, + "page": { + "type": "boolean" + }, + "pageSource": { + "type": "object", + "properties": { + "branch": { + "type": "string" + }, + "path": { + "type": "string" + } + } + } + } + } + } + } + } + }, + "paths": { + "/api/maintenance/repositories/{repoId}": { + "parameters": [ + { + "name": "repoId", + "in": "path", + "required": true, + "schema": { + "type": "string", + "pattern": "^[a-zA-Z0-9_-]+$" + }, + "description": "Existing anonymized repository ID selected when issuing the token." + } + ], + "get": { + "operationId": "getRepositoryMaintenanceStatus", + "summary": "Read saved status and settings without GitHub calls", + "responses": { + "200": { + "description": "Operation succeeded; refresh or restoration may still be running.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RepositoryStatus" + } + } + } + }, + "400": { + "description": "Invalid settings.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "401": { + "description": "Invalid or expired token.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "403": { + "description": "Scope or membership denied.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "404": { + "description": "Repository not found.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "409": { + "description": "Repository state changed or operation unavailable.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "429": { + "description": "Rate limited. Respect Retry-After.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + }, + "text/plain": { + "schema": { + "type": "string" + } + } + } + }, + "503": { + "description": "Service unavailable.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + } + } + }, + "patch": { + "operationId": "patchRepositorySettings", + "summary": "Change only supplied settings; terms replaces the entire list", + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "additionalProperties": false, + "minProperties": 1, + "properties": { + "terms": { + "type": "array", + "maxItems": 1000, + "items": { + "type": "string", + "minLength": 1, + "maxLength": 1000 + } + }, + "update": { + "type": "boolean" + }, + "image": { + "type": "boolean" + }, + "pdf": { + "type": "boolean" + }, + "notebook": { + "type": "boolean" + }, + "link": { + "type": "boolean" + } + } + } + } + } + }, + "responses": { + "200": { + "description": "Operation succeeded; refresh or restoration may still be running.", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "status": { + "type": "string" + }, + "repoId": { + "type": "string" + }, + "settingsSavedAt": { + "type": "string", + "format": "date-time" + } + } + } + } + } + }, + "400": { + "description": "Invalid settings.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "401": { + "description": "Invalid or expired token.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "403": { + "description": "Scope or membership denied.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "404": { + "description": "Repository not found.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "409": { + "description": "Repository state changed or operation unavailable.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "429": { + "description": "Rate limited. Respect Retry-After.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + }, + "text/plain": { + "schema": { + "type": "string" + } + } + } + }, + "503": { + "description": "Service unavailable.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + } + } + } + }, + "/api/repo/{repoId}/refresh": { + "parameters": [ + { + "name": "repoId", + "in": "path", + "required": true, + "schema": { + "type": "string", + "pattern": "^[a-zA-Z0-9_-]+$" + }, + "description": "Existing anonymized repository ID selected when issuing the token." + } + ], + "post": { + "operationId": "refreshRepository", + "summary": "Queue refresh of the saved branch. Poll status, not refresh.", + "responses": { + "200": { + "description": "Operation succeeded; refresh or restoration may still be running.", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "status": { + "type": "string" + } + } + } + } + } + }, + "400": { + "description": "Invalid settings.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "401": { + "description": "Invalid or expired token.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "403": { + "description": "Scope or membership denied.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "404": { + "description": "Repository not found.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "409": { + "description": "Repository state changed or operation unavailable.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "429": { + "description": "Rate limited. Respect Retry-After.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + }, + "text/plain": { + "schema": { + "type": "string" + } + } + } + }, + "503": { + "description": "Service unavailable.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + } + }, + "description": "One refresh per repository per minute per account, shared across tokens and IPs. HTTP 429 includes Retry-After." + } + }, + "/api/repo/{repoId}/extend": { + "parameters": [ + { + "name": "repoId", + "in": "path", + "required": true, + "schema": { + "type": "string", + "pattern": "^[a-zA-Z0-9_-]+$" + }, + "description": "Existing anonymized repository ID selected when issuing the token." + } + ], + "post": { + "operationId": "extendRepository", + "summary": "Extend expiration by six months. Not idempotent; inspect state before retrying.", + "responses": { + "200": { + "description": "Operation succeeded; refresh or restoration may still be running.", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "status": { + "type": "string" + }, + "expirationDate": { + "type": "string", + "format": "date-time" + } + } + } + } + } + }, + "400": { + "description": "Invalid settings.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "401": { + "description": "Invalid or expired token.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "403": { + "description": "Scope or membership denied.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "404": { + "description": "Repository not found.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "409": { + "description": "Repository state changed or operation unavailable.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "429": { + "description": "Rate limited. Respect Retry-After.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + }, + "text/plain": { + "schema": { + "type": "string" + } + } + } + }, + "503": { + "description": "Service unavailable.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + } + } + } + } + } +} diff --git a/src/core/model/users/users.schema.ts b/src/core/model/users/users.schema.ts index 348597d..aa27ec4 100644 --- a/src/core/model/users/users.schema.ts +++ b/src/core/model/users/users.schema.ts @@ -36,6 +36,9 @@ const UserSchema = new Schema({ name: { type: String }, createdAt: { type: Date, default: Date.now }, lastUsedAt: { type: Date }, + scope: { type: String, enum: ["repository-maintenance"] }, + repoIds: { type: [String], default: undefined }, + expiresAt: { type: Date }, }, ], photo: String, diff --git a/src/core/model/users/users.types.ts b/src/core/model/users/users.types.ts index fc0ada6..c2d23e9 100644 --- a/src/core/model/users/users.types.ts +++ b/src/core/model/users/users.types.ts @@ -19,6 +19,9 @@ export interface IUser { name?: string; createdAt?: Date; lastUsedAt?: Date; + scope?: "repository-maintenance"; + repoIds?: string[]; + expiresAt?: Date; }[]; notificationEmail?: string; emailPromptNever?: boolean; diff --git a/src/server/index.ts b/src/server/index.ts index 59e87d9..ace551c 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -16,7 +16,10 @@ import * as passport from "passport"; import { connect } from "./database"; import { startTemporaryStorageMaintenance } from "../core/temporary-storage"; import { initSession, router as connectionRouter } from "./routes/connection"; +import maintenanceRouter from "./routes/maintenance"; +import maintenanceTokens from "./routes/maintenance-tokens"; import { bearerTokenAuth } from "./routes/token-auth"; +import { tokenAttemptLimitOptions, maintenanceRefreshLimitOptions } from "./routes/maintenance-limits"; import router from "./routes"; import { conferenceStatusCheck, @@ -149,12 +152,6 @@ export default async function start() { // client-spoofable header. app.set("trust proxy", resolveTrustProxy(config.TRUST_PROXY)); - // handle session and connection - app.use(initSession()); - app.use(passport.initialize()); - app.use(passport.session()); - app.use(bearerTokenAuth); - startWorker(); const redisClient = createClient({ @@ -169,6 +166,32 @@ export default async function start() { await redisClient.connect(); + // Charge failed and forbidden bearer requests before their database lookup. + app.use(rateLimit({ + ...tokenAttemptLimitOptions, + store: new RedisStore({ + prefix: "token-attempt:", + sendCommand: (...args: string[]) => redisClient.sendCommand(args), + }), + })); + // Bearer attempts are limited before session deserialization as well. + app.use(initSession()); + app.use(passport.initialize()); + app.use(passport.session()); + app.use(bearerTokenAuth); + app.use(rateLimit({ + ...maintenanceRefreshLimitOptions, + store: new RedisStore({ + prefix: "maintenance-refresh:", + sendCommand: (...args: string[]) => redisClient.sendCommand(args), + }), + })); + + // All maintenance tokens for an account share a budget across client IPs. + const apiRateKey = (req: express.Request, res: express.Response) => + res.locals.maintenanceOwnerId + ? `maintenance:${res.locals.maintenanceOwnerId}` + : requestRateLimitKey(req); const rate = rateLimit({ store: new RedisStore({ sendCommand: (...args: string[]) => redisClient.sendCommand(args), @@ -177,7 +200,7 @@ export default async function start() { skip: async (request: express.Request, _response: express.Response) => { try { const user = await getUser(request); - if (user && user.isAdmin) return true; + if (user && user.isAdmin && !_response.locals.maintenanceToken) return true; } catch { // ignore: user not connected } @@ -193,7 +216,7 @@ export default async function start() { // if not logged in, limit to half the rate return config.RATE_LIMIT / 2; }, - keyGenerator: requestRateLimitKey, + keyGenerator: apiRateKey, standardHeaders: "draft-6", legacyHeaders: false, handler: sendRequestRateLimit, @@ -203,14 +226,14 @@ export default async function start() { delayAfter: 50, delayMs: () => 150, maxDelayMs: 5000, - keyGenerator: requestRateLimitKey, + keyGenerator: apiRateKey, }); const webViewSpeedLimiter = slowDown({ windowMs: 15 * 60 * 1000, // 15 minutes delayAfter: 200, delayMs: () => 150, maxDelayMs: 5000, - keyGenerator: requestRateLimitKey, + keyGenerator: apiRateKey, }); app.use("/github", rate, speedLimiter, githubAppRouter); @@ -220,6 +243,8 @@ export default async function start() { const apiRouter = express.Router(); app.use("/api", rate, apiRouter); + apiRouter.use("/maintenance", speedLimiter, maintenanceRouter); + apiRouter.use("/user/maintenance-tokens", maintenanceTokens); apiRouter.use("/admin", router.admin); apiRouter.use("/options", router.option); apiRouter.use("/conferences", router.conference); diff --git a/src/server/routes/connection.ts b/src/server/routes/connection.ts index 57339a1..c7ac330 100644 --- a/src/server/routes/connection.ts +++ b/src/server/routes/connection.ts @@ -252,7 +252,11 @@ router.post( const model = await UserModel.findOne({ "apiTokens.tokenHash": hashToken(token), }); - if (!model) return res.status(401).json({ error: "invalid_token" }); + const entry = model?.apiTokens?.find(t => t.tokenHash === hashToken(token)); + if (!model || !entry || entry.scope || + (entry.expiresAt && entry.expiresAt.getTime() <= Date.now())) { + return res.status(401).json({ error: "invalid_token" }); + } if (isDisabledAccount(model.status)) { return res.status(model.status === "banned" ? 403 : 401).json({ error: model.status === "banned" ? "user_banned" : "not_connected", diff --git a/src/server/routes/maintenance-limits.ts b/src/server/routes/maintenance-limits.ts new file mode 100644 index 0000000..a6cb115 --- /dev/null +++ b/src/server/routes/maintenance-limits.ts @@ -0,0 +1,28 @@ +import { Options } from "express-rate-limit"; +import { requestRateLimitKey } from "../rate-limit-key"; + +// Redis stores are supplied by the server. These policies also run in tests +// with the in-memory store, through real HTTP requests and the auth middleware. +export const tokenAttemptLimitOptions: Partial = { + windowMs: 60_000, + limit: 120, + skip: req => !req.headers.authorization, + keyGenerator: requestRateLimitKey, + standardHeaders: "draft-6", + legacyHeaders: false, + message: { error: "token_attempts_rate_limited" }, +}; + +export const maintenanceRefreshLimitOptions: Partial = { + windowMs: 60_000, + limit: 1, + skip: (req, res) => !res.locals.maintenanceToken || req.method !== "POST" || + !/^\/api\/repo\/[a-zA-Z0-9_-]+\/refresh\/?$/.test(req.path), + // Share the cooldown across this account's tokens and client IPs. Keep + // different accounts separate so a removed coauthor cannot block the owner. + keyGenerator: (req, res) => + `${res.locals.maintenanceOwnerId}:${req.path.split("/")[3].toLowerCase()}`, + standardHeaders: "draft-6", + legacyHeaders: false, + message: { error: "refresh_rate_limited" }, +}; diff --git a/src/server/routes/maintenance-tokens.ts b/src/server/routes/maintenance-tokens.ts new file mode 100644 index 0000000..f091f76 --- /dev/null +++ b/src/server/routes/maintenance-tokens.ts @@ -0,0 +1,62 @@ +import * as express from "express"; +import { Types } from "mongoose"; +import { getUser, handleError, isOwnerCoauthorOrAdmin } from "./route-utils"; +import { generateToken, hashToken } from "./token-auth"; +import UserModel from "../../core/model/users/users.model"; +import { getRepository } from "../database"; +import AnonymousError from "../../core/AnonymousError"; + +const router = express.Router(); +router.use((req, res, next) => { + res.set("Cache-Control", "no-store"); + // A restricted token must never mint credentials or become a session. + if (req.headers.authorization || !req.isAuthenticated()) { + return res.status(401).json({ error: "session_required" }); + } + next(); +}); + +router.get("/", async (req, res) => { + try { + const user = await getUser(req); + res.json((user.model.apiTokens || []).filter(t => t.scope === "repository-maintenance") + .map(t => ({ id: t._id, name: t.name, repoIds: t.repoIds, expiresAt: t.expiresAt, + createdAt: t.createdAt, lastUsedAt: t.lastUsedAt }))); + } catch (error) { handleError(error, res, req); } +}); + +router.post("/", async (req, res) => { + try { + const user = await getUser(req); + const { name, repoIds, expiresInDays = 30 } = req.body; + if (typeof name !== "string" || !name.trim() || name.length > 80 || + !Array.isArray(repoIds) || !repoIds.length || repoIds.length > 20 || + !repoIds.every(id => typeof id === "string" && /^[a-zA-Z0-9_-]{3,200}$/.test(id)) || + !Number.isInteger(expiresInDays) || expiresInDays < 1 || expiresInDays > 90) { + throw new AnonymousError("invalid_token_options", { httpStatus: 400 }); + } + for (const id of new Set(repoIds)) { + isOwnerCoauthorOrAdmin(await getRepository(id), user); + } + const token = generateToken(); + const entry = { _id: new Types.ObjectId(), name: name.trim(), scope: "repository-maintenance", + repoIds: [...new Set(repoIds)], tokenHash: hashToken(token), createdAt: new Date(), + expiresAt: new Date(Date.now() + expiresInDays * 86400_000) }; + const saved = await UserModel.updateOne({ _id: user.model._id, + "apiTokens.50": { $exists: false } }, { $push: { apiTokens: entry } }); + if (!saved.matchedCount) throw new AnonymousError("token_limit_reached", { httpStatus: 409 }); + res.status(201).json({ id: entry._id, name: entry.name, repoIds: entry.repoIds, + expiresAt: entry.expiresAt, token }); + } catch (error) { handleError(error, res, req); } +}); + +router.delete("/:id", async (req, res) => { + try { + const user = await getUser(req); + if (!Types.ObjectId.isValid(req.params.id)) throw new AnonymousError("invalid_token_id", { httpStatus: 400 }); + const result = await UserModel.updateOne({ _id: user.model._id }, + { $pull: { apiTokens: { _id: req.params.id, scope: "repository-maintenance" } } }); + res.json({ removed: result.modifiedCount }); + } catch (error) { handleError(error, res, req); } +}); +export default router; diff --git a/src/server/routes/maintenance.ts b/src/server/routes/maintenance.ts new file mode 100644 index 0000000..89f047d --- /dev/null +++ b/src/server/routes/maintenance.ts @@ -0,0 +1,59 @@ +import * as express from "express"; +import { getRepo, getUser, handleError, isOwnerCoauthorOrAdmin } from "./route-utils"; +import AnonymousError from "../../core/AnonymousError"; +import RepoModel from "../../core/model/anonymizedRepositories/anonymizedRepositories.model"; + +const router = express.Router(); +const toggles = ["update", "image", "pdf", "notebook", "link"]; + +/** Only in-place anonymization settings; never accept source or connection input. */ +export function maintenanceSettings(body: unknown): Record { + const invalid = () => new AnonymousError("invalid_settings", { httpStatus: 400 }); + if (!body || typeof body !== "object" || Array.isArray(body)) throw invalid(); + const entries = Object.entries(body); + if (!entries.length) throw invalid(); + const updates: Record = {}; + for (const [key, value] of entries) { + if (key === "terms") { + if (!Array.isArray(value) || value.length > 1000 || + !value.every(t => typeof t === "string" && t.length > 0 && t.length <= 1000)) throw invalid(); + } else if (!toggles.includes(key) || typeof value !== "boolean") throw invalid(); + updates[`options.${key}`] = value; + } + return updates; +} + +router.route("/repositories/:repoId") + .all((_req, res, next) => { res.set("Cache-Control", "no-store"); next(); }) + .get(async (req, res) => { + try { + const user = await getUser(req); + const repo = await getRepo(req, res, { nocheck: true }); + if (!repo) return; + isOwnerCoauthorOrAdmin(repo, user); + // Read saved metadata without refreshing GitHub credentials or content. + res.json({ repoId: repo.repoId, status: repo.status, statusMessage: repo.model.statusMessage, + statusDate: repo.model.statusDate, settingsSavedAt: repo.model.settingsSavedAt, + source: { branch: repo.model.source.branch, commit: repo.model.source.commit }, + options: repo.model.options }); + } catch (error) { handleError(error, res, req); } + }) + .patch(async (req, res) => { + try { + const user = await getUser(req); + const repo = await getRepo(req, res, { nocheck: true }); + if (!repo) return; + isOwnerCoauthorOrAdmin(repo, user); + if (!["ready", "error"].includes(repo.status || "")) throw new AnonymousError("invalid_status", { httpStatus: 409 }); + repo.assertNotArchived(); + const updates = maintenanceSettings(req.body); + const settingsSavedAt = new Date(); + const saved = await RepoModel.updateOne({ _id: repo.model._id, status: repo.model.status, + statusDate: repo.model.statusDate, + settingsSavedAt: repo.model.settingsSavedAt || { $exists: false } }, + { $set: { ...updates, settingsSavedAt } }).exec(); + if (!saved.matchedCount) throw new AnonymousError("invalid_status", { httpStatus: 409 }); + res.json({ repoId: repo.repoId, status: repo.status, settingsSavedAt }); + } catch (error) { handleError(error, res, req); } + }); +export default router; diff --git a/src/server/routes/token-auth.ts b/src/server/routes/token-auth.ts index 313d0ae..5dcb5a3 100644 --- a/src/server/routes/token-auth.ts +++ b/src/server/routes/token-auth.ts @@ -16,21 +16,35 @@ export function generateToken(): string { export async function bearerTokenAuth( req: express.Request, - _res: express.Response, + res: express.Response, next: express.NextFunction ): Promise { - if (req.user) return next(); - const header = req.headers["authorization"]; if (!header || typeof header !== "string") return next(); const match = header.match(/^Bearer\s+(.+)$/i); - if (!match) return next(); + if (!match || !match[1].trim() || match[1].length > 512) { + res.status(401).json({ error: "invalid_token" }); + return; + } const tokenHash = hashToken(match[1].trim()); try { const model = await UserModel.findOne({ "apiTokens.tokenHash": tokenHash }); - if (!model) return next(); - if (isDisabledAccount(model.status)) return next(); + const token = model?.apiTokens?.find(t => t.tokenHash === tokenHash); + if (!model || !token || isDisabledAccount(model.status) || + (token.expiresAt && token.expiresAt.getTime() <= Date.now()) || + (token.scope && (!token.expiresAt || token.scope !== "repository-maintenance"))) { + res.status(401).json({ error: "invalid_token" }); + return; + } + if (token.scope) { + if (!maintenanceTokenAllows(req.method, req.path, token.repoIds || [])) { + res.status(403).json({ error: "token_scope_forbidden" }); + return; + } + res.locals.maintenanceToken = true; + res.locals.maintenanceOwnerId = String(model._id); + } // Mirror the shape produced by passport's verify() in connection.ts // so existing getUser()/route code works unchanged. @@ -48,6 +62,16 @@ export async function bearerTokenAuth( ); } catch (err) { logger.error("lookup failed", serializeError(err)); + res.status(503).json({ error: "authentication_unavailable" }); + return; } return next(); } + +/** Exact paths keep scoped tokens out of GitHub proxy and session routes. */ +export function maintenanceTokenAllows(method: string, path: string, repoIds: string[]): boolean { + const readOrEdit = path.match(/^\/api\/maintenance\/repositories\/([a-zA-Z0-9_-]+)\/?$/); + if (readOrEdit && ["GET", "PATCH"].includes(method)) return repoIds.includes(readOrEdit[1]); + const action = path.match(/^\/api\/repo\/([a-zA-Z0-9_-]+)\/(refresh|extend)\/?$/); + return !!action && method === "POST" && repoIds.includes(action[1]); +} diff --git a/test/maintenance-api.test.js b/test/maintenance-api.test.js new file mode 100644 index 0000000..e893607 --- /dev/null +++ b/test/maintenance-api.test.js @@ -0,0 +1,297 @@ +const { expect } = require("chai"); +require("ts-node/register/transpile-only"); +const express = require("express"); +const UserModel = require("../src/core/model/users/users.model").default; +const { bearerTokenAuth, hashToken, maintenanceTokenAllows } = require("../src/server/routes/token-auth"); +const { maintenanceSettings } = require("../src/server/routes/maintenance"); +const { router: connection } = require("../src/server/routes/connection"); + +describe("repository maintenance tokens", () => { + let server, base, originalFind, originalUpdate, model, lookups, logins; + before(async () => { + originalFind = UserModel.findOne; + originalUpdate = UserModel.updateOne; + UserModel.findOne = async () => { lookups++; return model; }; + UserModel.updateOne = async () => ({ matchedCount: 1 }); + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + // An existing cookie must not override the supplied restricted token. + req.user = { username: "cookie-admin" }; + req.login = (_user, callback) => { logins++; callback(); }; + next(); + }); + app.use(bearerTokenAuth); + app.use("/github", connection); + app.use((_req, res) => res.json({ reached: true, scoped: !!res.locals.maintenanceToken })); + server = await new Promise(resolve => { const s = app.listen(0, "127.0.0.1", () => resolve(s)); }); + base = `http://127.0.0.1:${server.address().port}`; + }); + beforeEach(() => { + lookups = logins = 0; + model = { _id: "owner", username: "alice", status: "active", apiTokens: [{ + tokenHash: hashToken("secret"), scope: "repository-maintenance", repoIds: ["paper-123"], + expiresAt: new Date(Date.now() + 60_000), + }] }; + }); + after(async () => { + UserModel.findOne = originalFind; + UserModel.updateOne = originalUpdate; + await new Promise(resolve => server.close(resolve)); + }); + const request = (path, method = "GET") => globalThis.fetch(base + path, { + method, headers: { authorization: "Bearer secret" }, + }); + it("allows only the selected repository and explicit maintenance operations", async () => { + for (const [path, method] of [ + ["/api/maintenance/repositories/paper-123", "GET"], + ["/api/maintenance/repositories/paper-123", "PATCH"], + ["/api/repo/paper-123/refresh", "POST"], ["/api/repo/paper-123/extend", "POST"], + ]) { + const response = await request(path, method); + expect(response.status).to.equal(200); + expect((await response.json()).scoped).to.equal(true); + } + }); + it("blocks downloads, previews, source replacement, token minting, and other repos", async () => { + for (const [path, method] of [ + ["/api/repo/paper-123/file/README.md", "GET"], ["/api/repo/paper-123/zip", "GET"], + ["/api/repo/owner/repository/readme", "GET"], ["/api/repo/paper-123", "POST"], + ["/api/maintenance/repositories/another", "PATCH"], ["/api/repo/another/refresh", "POST"], + ["/api/admin", "GET"], ["/api/user/maintenance-tokens", "POST"], + ["/github/login-token", "POST"], ["/w/paper-123/", "GET"], + ]) expect((await request(path, method)).status, path).to.equal(403); + }); + it("rejects encoded scope bypasses and unexpected methods", () => { + for (const path of ["/api/repo/paper%2D123/refresh", "/api/repo/paper-123/refresh/extra", "/api/repo/paper-123//refresh"]) { + expect(maintenanceTokenAllows("POST", path, ["paper-123"])).to.equal(false); + } + expect(maintenanceTokenAllows("DELETE", "/api/maintenance/repositories/paper-123", ["paper-123"])).to.equal(false); + }); + it("rejects expired, revoked, banned, and malformed scoped tokens despite a session", async () => { + model.apiTokens[0].expiresAt = new Date(0); + expect((await request("/api/repo/paper-123/refresh", "POST")).status).to.equal(401); + model.apiTokens[0].expiresAt = undefined; + expect((await request("/api/repo/paper-123/refresh", "POST")).status).to.equal(401); + model.apiTokens[0].expiresAt = new Date(Date.now() + 60_000); + model.status = "banned"; + expect((await request("/api/repo/paper-123/refresh", "POST")).status).to.equal(401); + model = null; + expect((await request("/api/repo/paper-123/refresh", "POST")).status).to.equal(401); + expect(lookups).to.equal(4); + }); + it("cannot exchange a maintenance token in a body for an unrestricted session", async () => { + const response = await globalThis.fetch(base + "/github/login-token", { method: "POST", + headers: { "Content-Type": "application/json" }, body: JSON.stringify({ token: "secret" }) }); + expect(response.status).to.equal(401); + expect(logins).to.equal(0); + }); + it("rejects malformed authorization instead of falling back to a cookie", async () => { + for (const authorization of ["Bearer", "Basic anything", "Bearer " + "x".repeat(513)]) { + const response = await globalThis.fetch(base + "/api/admin", { headers: { authorization } }); + expect(response.status).to.equal(401); + } + expect(lookups).to.equal(0); + }); + it("fails closed when authentication storage is unavailable", async () => { + const find = UserModel.findOne; + UserModel.findOne = async () => { throw new Error("database offline"); }; + try { + const response = await request("/api/maintenance/repositories/paper-123"); + expect(response.status).to.equal(503); + expect(await response.json()).to.deep.equal({ error: "authentication_unavailable" }); + } finally { UserModel.findOne = find; } + }); + it("preserves legacy unrestricted admin tokens", async () => { + delete model.apiTokens[0].scope; + expect((await request("/api/admin")).status).to.equal(200); + }); +}); + +describe("maintenance settings patch", () => { + it("preserves omitted fields and accepts explicit false and empty terms", () => { + expect(maintenanceSettings({ terms: [], update: false })).to.deep.equal({ + "options.terms": [], "options.update": false, + }); + }); + it("rejects source changes, arbitrary option paths, invalid types and oversized terms", () => { + for (const body of [null, [], {}, { source: {} }, { "options.update": true }, + { image: "false" }, { terms: [1] }, { terms: [""] }, { terms: ["a".repeat(1001)] }, + { terms: Array(1001).fill("a") }, { pageSource: { branch: "other" } }]) { + expect(() => maintenanceSettings(body)).to.throw("invalid_settings"); + } + }); +}); + +describe("maintenance routes enforce current membership", () => { + const db = require("../src/server/database"); + const RepoModel = require("../src/core/model/anonymizedRepositories/anonymizedRepositories.model").default; + const maintenance = require("../src/server/routes/maintenance").default; + const tokens = require("../src/server/routes/maintenance-tokens").default; + let server, base, originals, user, repo, writes, tokenWrite; + before(async () => { + originals = [UserModel.findById, UserModel.updateOne, db.getRepository, RepoModel.updateOne]; + UserModel.findById = async () => user; + UserModel.updateOne = async (filter, update) => { + tokenWrite = { filter, update }; + return { matchedCount: 1, modifiedCount: 1 }; + }; + db.getRepository = async () => repo; + RepoModel.updateOne = (filter, update) => { + writes.push({ filter, update }); + return { exec: async () => ({ matchedCount: 1 }) }; + }; + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + req.user = { user }; + req.isAuthenticated = () => req.headers["x-session"] === "yes"; + next(); + }); + app.use("/api/maintenance", maintenance); + app.use("/api/user/maintenance-tokens", tokens); + server = await new Promise(resolve => { const s = app.listen(0, "127.0.0.1", () => resolve(s)); }); + base = `http://127.0.0.1:${server.address().port}`; + }); + beforeEach(() => { + user = new UserModel({ username: "alice", externalIDs: { github: "123" } }); + repo = { repoId: "paper-123", status: "ready", owner: { id: user.id }, + assertNotArchived() {}, model: { _id: "repo", status: "ready", statusDate: new Date(), + source: { branch: "main", commit: "abc", accessToken: "never-return" }, + options: { terms: ["Alice"], image: true, pdf: true }, coauthors: [], + githubAccess: { secret: "never-return" } } }; + writes = []; tokenWrite = null; + }); + after(async () => { + [UserModel.findById, UserModel.updateOne, db.getRepository, RepoModel.updateOne] = originals; + await new Promise(resolve => server.close(resolve)); + }); + function call(path, method = "GET", body, headers = {}) { + return globalThis.fetch(base + path, { method, + headers: { "Content-Type": "application/json", ...headers }, + body: body ? JSON.stringify(body) : undefined }); + } + it("returns saved state without credentials and marks it uncacheable", async () => { + const response = await call("/api/maintenance/repositories/paper-123"); + expect(response.status).to.equal(200); + expect(response.headers.get("cache-control")).to.equal("no-store"); + const body = await response.json(); + expect(body.source).to.deep.equal({ branch: "main", commit: "abc" }); + expect(JSON.stringify(body)).not.to.include("never-return"); + }); + it("denies reads and writes after membership removal", async () => { + repo.owner.id = "other-owner"; + for (const method of ["GET", "PATCH"]) { + expect((await call("/api/maintenance/repositories/paper-123", method, + method === "PATCH" ? { image: false } : undefined)).status).to.equal(403); + } + expect(writes).to.have.length(0); + }); + it("lets coauthors patch individual options without replacing the source", async () => { + repo.owner.id = "other-owner"; + repo.model.coauthors = [{ githubId: "123" }]; + const response = await call("/api/maintenance/repositories/paper-123", "PATCH", { image: false }); + expect(response.status).to.equal(200); + expect(writes[0].update.$set["options.image"]).to.equal(false); + expect(Object.keys(writes[0].update.$set)).to.deep.equal(["options.image", "settingsSavedAt"]); + expect(writes[0].filter.status).to.equal("ready"); + }); + it("rejects settings changes while removal is underway", async () => { + repo.status = "removing"; + expect((await call("/api/maintenance/repositories/paper-123", "PATCH", { image: false })).status).to.equal(409); + expect(writes).to.have.length(0); + }); + it("issues only hashed, bounded, scoped credentials from a session", async () => { + const response = await call("/api/user/maintenance-tokens", "POST", + { name: "client", repoIds: ["paper-123"] }, { "x-session": "yes" }); + expect(response.status).to.equal(201); + const body = await response.json(); + expect(body.token).to.match(/^[a-f0-9]{64}$/); + expect(tokenWrite.update.$push.apiTokens.tokenHash).to.equal(hashToken(body.token)); + expect(tokenWrite.update.$push.apiTokens.scope).to.equal("repository-maintenance"); + expect(tokenWrite.filter["apiTokens.50"]).to.deep.equal({ $exists: false }); + expect(body).not.to.have.property("tokenHash"); + }); + it("rejects token issuance without session, with bearer, or for unrelated repositories", async () => { + const body = { name: "client", repoIds: ["paper-123"] }; + expect((await call("/api/user/maintenance-tokens", "POST", body)).status).to.equal(401); + expect((await call("/api/user/maintenance-tokens", "POST", body, + { "x-session": "yes", authorization: "Bearer anything" })).status).to.equal(401); + repo.owner.id = "other-owner"; + expect((await call("/api/user/maintenance-tokens", "POST", body, { "x-session": "yes" })).status).to.equal(403); + expect(tokenWrite).to.equal(null); + }); + it("rejects token lifetimes above 90 days", async () => { + expect((await call("/api/user/maintenance-tokens", "POST", + { name: "client", repoIds: ["paper-123"], expiresInDays: 91 }, + { "x-session": "yes" })).status).to.equal(400); + expect(tokenWrite).to.equal(null); + }); + it("revokes only the signed-in account's maintenance token", async () => { + const id = "507f1f77bcf86cd799439011"; + expect((await call(`/api/user/maintenance-tokens/${id}`, "DELETE", undefined, + { "x-session": "yes" })).status).to.equal(200); + expect(String(tokenWrite.filter._id)).to.equal(user.id); + expect(tokenWrite.update.$pull.apiTokens).to.deep.equal({ _id: id, scope: "repository-maintenance" }); + }); +}); + +describe("maintenance abuse limits", () => { + const { rateLimit } = require("express-rate-limit"); + const { tokenAttemptLimitOptions, maintenanceRefreshLimitOptions } = require("../src/server/routes/maintenance-limits"); + let server, base, originals, lookups, model, reached; + beforeEach(async () => { + originals = [UserModel.findOne, UserModel.updateOne]; + UserModel.findOne = async () => { lookups++; return model; }; + UserModel.updateOne = async () => ({ matchedCount: 1 }); + const app = express(); + app.use(rateLimit({ ...tokenAttemptLimitOptions, limit: 8 })); + app.use(bearerTokenAuth); + app.use(rateLimit(maintenanceRefreshLimitOptions)); + app.use((_req, res) => { reached++; res.json({ ok: true }); }); + server = await new Promise(resolve => { const s = app.listen(0, "127.0.0.1", () => resolve(s)); }); + base = `http://127.0.0.1:${server.address().port}`; + model = { _id: "owner", status: "active", apiTokens: ["one", "two"].map(token => ({ + tokenHash: hashToken(token), scope: "repository-maintenance", repoIds: ["paper-123", "Paper-123"], + expiresAt: new Date(Date.now() + 60_000), + })) }; + lookups = reached = 0; + }); + afterEach(async () => { + [UserModel.findOne, UserModel.updateOne] = originals; + await new Promise(resolve => server.close(resolve)); + }); + it("shares refresh cooldown across tokens, casing and trailing slashes; allows polling", async () => { + const call = (path, token, method = "POST") => globalThis.fetch(base + path, { + method, headers: { authorization: `Bearer ${token}` }, + }); + expect((await call("/api/repo/paper-123/refresh", "one")).status).to.equal(200); + const denied = await call("/api/repo/Paper-123/refresh/", "two"); + expect(denied.status).to.equal(429); + expect(Number(denied.headers.get("retry-after"))).to.be.greaterThan(0); + expect(await denied.json()).to.deep.equal({ error: "refresh_rate_limited" }); + expect((await call("/api/maintenance/repositories/paper-123", "two", "GET")).status).to.equal(200); + // A removed coauthor's requests cannot consume another account's cooldown. + model._id = "another-owner"; + expect((await call("/api/repo/paper-123/refresh", "one")).status).to.equal(200); + }); + it("charges failed authentication before MongoDB and never reaches a route", async () => { + model = null; + const beforeRoutes = reached; + for (let i = 0; i < 8; i++) { + expect((await globalThis.fetch(base + "/api/admin", { + headers: { authorization: "Bearer invalid" }, + })).status).to.equal(401); + } + const beforeLookups = lookups; + const response = await globalThis.fetch(base + "/api/admin", { + headers: { authorization: "Bearer invalid" }, + }); + expect(response.status).to.equal(429); + expect(await response.json()).to.deep.equal({ error: "token_attempts_rate_limited" }); + expect(lookups).to.equal(beforeLookups); + expect(reached).to.equal(beforeRoutes); + // Anonymous review traffic does not use the bearer authentication budget. + expect((await globalThis.fetch(base + "/")).status).to.equal(200); + }); +}); From d0b217a448695445850bbbf88ee13ecdc526c5be Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Mon, 2 Nov 2026 09:00:02 +0100 Subject: [PATCH 48/48] feat: include public GitHub submodules in shared repositories (#914) * feat: include public GitHub submodules in shared repositories * fix: preserve submodule access across token and document contexts --- README.md | 14 ++ public/i18n/locale-en.json | 3 +- src/core/AnonymizedFile.ts | 21 +- src/core/GitHubUtils.ts | 2 +- src/core/Repository.ts | 19 +- src/core/model/anonymized-path.ts | 2 + src/core/model/files/files.schema.ts | 4 + src/core/model/files/files.types.ts | 10 + src/core/source/GitHubStream.ts | 44 +++- src/core/source/github-submodules.ts | 99 ++++++++ src/core/zipStream.ts | 143 +++++++----- src/server/routes/repository-public.ts | 13 +- src/streamer/route.ts | 3 +- test/github-submodules.test.js | 308 +++++++++++++++++++++++++ test/owner-notifications.test.js | 4 +- test/performance-integration.test.js | 13 ++ test/zip-proxy-errors.test.js | 2 +- 17 files changed, 617 insertions(+), 87 deletions(-) create mode 100644 src/core/source/github-submodules.ts create mode 100644 test/github-submodules.test.js diff --git a/README.md b/README.md index 0aeece3..0f9937b 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,20 @@ A free public instance is available at **[anonymous.4open.science](https://anony Just open **[anonymous.4open.science](https://anonymous.4open.science/)**, paste your repository URL, add the terms to hide, and share the generated link. +### Git submodules + +Direct public GitHub submodules are included in the file browser and ZIP downloads +at the commit recorded by the parent repository. Their paths and contents use the +parent's anonymization settings. `.gitmodules` files are omitted from shared GitHub +content because they contain source repository URLs. + +The service expands up to 20 direct submodules and 10,000 combined child tree entries. +Private, non-GitHub, nested, missing, oversized, and truncated submodules remain empty +folders with a warning. Child files are fetched anonymously on demand; child tree +lookups use GitHub's unauthenticated API quota. ZIP downloads fetch public child archives. +Existing cached repositories with `.gitmodules` rebuild their file trees on the next +listing or ZIP download. No manual database migration is required. + ### CLI Anonymize a repository locally and produce an anonymized zip: diff --git a/public/i18n/locale-en.json b/public/i18n/locale-en.json index dee6ad3..c0c1113 100644 --- a/public/i18n/locale-en.json +++ b/public/i18n/locale-en.json @@ -72,6 +72,7 @@ "invalid_request": "The request is missing required fields or is malformed.", "invalid_project_name": "Use a project name of up to 100 characters without line breaks or control characters.", "no_file_selected": "Please select a file.", + "submodule_not_available": "This public submodule file is no longer available at its recorded commit.", "file_not_found": "The requested file is not found.", "file_not_accessible": "The requested file is not accessible.", "file_not_supported": "The file type is not supported. Anonymous GitHub cannot handle it.", @@ -164,6 +165,6 @@ "page_branch_mismatch": "GitHub Pages on this repository is served from the '{{pageBranch}}' branch, but you selected '{{selectedBranch}}'. Switch the branch above to '{{pageBranch}}' to anonymize the Pages site.", "folder_truncated": "This folder has more than 10,000 entries; only a partial listing is shown.", "repo_truncated": "Some folders in this repository have too many files to be fully listed. Affected folders are marked with a warning icon.", - "submodules_not_included": "This repository uses git submodules. Submodule contents are not included in the anonymized repository and appear as empty folders." + "submodules_not_included": "Some submodule contents could not be included. Only direct public GitHub submodules within the size limits are supported. Private, external, nested, or unavailable submodules appear as empty folders." } } diff --git a/src/core/AnonymizedFile.ts b/src/core/AnonymizedFile.ts index 858a4bb..020d521 100644 --- a/src/core/AnonymizedFile.ts +++ b/src/core/AnonymizedFile.ts @@ -153,8 +153,18 @@ export default class AnonymizedFile { return this._file.size; } + private visibleFile(file: IFile): IFile { + if (this.repository.model?.source?.type !== "Zip" && file.name === ".gitmodules") { + throw new AnonymousError("file_not_found", { httpStatus: 404 }); + } + return file; + } + async getFileInfo(): Promise { - if (this._file) return this._file; + if (this.repository.model?.source?.type !== "Zip" && basename(this.anonymizedPath) === ".gitmodules") { + throw new AnonymousError("file_not_found", { httpStatus: 404 }); + } + if (this._file) return this.visibleFile(this._file); let fileDir = dirname(this.anonymizedPath); if (fileDir == ".") fileDir = ""; if (fileDir.endsWith("/")) fileDir = fileDir.slice(0, -1); @@ -180,7 +190,7 @@ export default class AnonymizedFile { if (exact) { if (exact.metadataPending) await this.repository.completeRecoveredFileMetadata(exact); this._file = exact; - return exact; + return this.visibleFile(exact); } const terms = this.repository.options.terms || []; @@ -213,7 +223,7 @@ export default class AnonymizedFile { const compiled = compileTerms(terms); if (typeof this.repository.findAnonymizedPath === "function") { const indexed = await this.repository.findAnonymizedPath(this.anonymizedPath); - if (indexed) { this._file = indexed; return indexed; } + if (indexed) { this._file = indexed; return this.visibleFile(indexed); } if (indexed === null) { if (anonymizePathCompiled(this.anonymizedPath, compiled) === this.anonymizedPath) { const recovered = await this.recoverTruncatedFile(fileDir); @@ -237,7 +247,7 @@ export default class AnonymizedFile { ) { if (candidate.metadataPending) await this.repository.completeRecoveredFileMetadata(candidate); this._file = candidate; - return candidate; + return this.visibleFile(candidate); } } @@ -312,6 +322,7 @@ export default class AnonymizedFile { if (!this._file) { this._file = await this.getFileInfo(); } + this.visibleFile(this._file); return join(this._file.path, this._file.name); } extension() { @@ -398,6 +409,7 @@ export default class AnonymizedFile { branch: this.repository.model.source.branch, repoId: this.repository.repoId, filePath: this.filePath, + upstream: (await this.getFileInfo()).upstream, sha: await this.sha(), size: await this.size(), anonymizerOptions: anonymizer.opt, @@ -446,6 +458,7 @@ export default class AnonymizedFile { json: { sha, size, + upstream: (await this.getFileInfo()).upstream, token: await githubTokenForStreamer(token, this.repository.model.source.repositoryName), repoFullName: this.repository.model.source.repositoryName, commit: this.repository.model.source.commit, diff --git a/src/core/GitHubUtils.ts b/src/core/GitHubUtils.ts index e0f562e..f644bd4 100644 --- a/src/core/GitHubUtils.ts +++ b/src/core/GitHubUtils.ts @@ -244,7 +244,7 @@ export function octokit(token: string) { const prefix = `/repos/${context.publicRepository.split("/").map(encodeURIComponent).join("/")}`.toLowerCase(); const path = url.pathname.toLowerCase(); const suffix = path.slice(prefix.length); - const readable = /^(?:\/?|\/branches(?:\/[^/]+)?|\/commits(?:\/[^/]+)?|\/readme|\/pages|\/zipball\/[^/]+|\/git\/(?:trees|blobs)\/[^/]+|\/pulls\/\d+|\/issues\/\d+\/comments)$/.test(suffix); + const readable = /^(?:\/?|\/branches(?:\/[^/]+)?|\/commits(?:\/[^/]+)?|\/contents(?:\/[^/]+)*|\/readme|\/pages|\/zipball\/[^/]+|\/git\/(?:trees|blobs)\/[^/]+|\/pulls\/\d+|\/issues\/\d+\/comments)$/.test(suffix); if (!readable || (options.method !== "GET" && !(options.method === "HEAD" && suffix.startsWith("/zipball/"))) || url.origin !== "https://api.github.com" || (path !== prefix && !path.startsWith(prefix + "/"))) { throw new AnonymousError("github_app_access_required", { httpStatus: 403 }); diff --git a/src/core/Repository.ts b/src/core/Repository.ts index a2efb18..2511c97 100644 --- a/src/core/Repository.ts +++ b/src/core/Repository.ts @@ -8,6 +8,7 @@ import { RepositoryStatus } from "./types"; import { Readable } from "stream"; import * as sha1 from "crypto-js/sha1"; import User from "./User"; +import { SubmoduleMount } from "./source/github-submodules"; import GitHubStream from "./source/GitHubStream"; import Zip from "./source/Zip"; import { @@ -43,14 +44,14 @@ async function buildPathIndex(repoId: string, key: string, terms: string[], gene const task = (async () => { const compiled = compileTerms(terms); const cursor = FileModel.find({ repoId, treeGeneration: generation || { $exists: false } }) - .select("name path sha size").sort({ path: 1, name: 1 }).lean().cursor(); + .select("name path sha size upstream").sort({ path: 1, name: 1 }).lean().cursor(); let batch: Parameters[0] = []; try { for await (const file of cursor) { const name = anonymizePathCompiled(file.name, compiled); const directory = anonymizePathCompiled(file.path, compiled); batch.push({ updateOne: { filter: { repoId, key, originalId: file._id }, update: { $set: { - repoId, key, originalId: file._id, name: file.name, path: file.path, sha: file.sha, size: file.size, + repoId, key, originalId: file._id, name: file.name, path: file.path, sha: file.sha, size: file.size, upstream: file.upstream, anonymousName: name, anonymousDirectory: directory, createdAt: new Date(), searchName: name.toLowerCase(), searchDirectory: directory.toLowerCase(), anonymousPath: anonymizePathCompiled(file.path ? `${file.path}/${file.name}` : file.name, compiled), @@ -202,6 +203,17 @@ export default class Repository { return await FileModel.find(query).exec(); } + async submoduleMounts(): Promise { + if (this.model.source.type === "Zip") return []; + await this.ensureFileTree(); + const mounts = await FileModel.find({ repoId: this.repoId, + treeGeneration: this.model.treeGeneration || { $exists: false }, submodule: "included", + }).select("name path upstream").lean().exec(); + return mounts.filter(file => file.upstream).map(file => ({ + ...file.upstream!, path: [file.path, file.name].filter(Boolean).join("/"), + })); + } + private async ensureFileTree(opt: { force?: boolean; progress?: (status: string) => void } = {}) { this.assertNotArchived(); let hasFile = this.hasEmptyTree() || await FileModel.exists({ repoId: this.repoId, treeGeneration: this.model.treeGeneration || { $exists: false } }).exec(); @@ -219,6 +231,9 @@ export default class Repository { hasFile = null; } } + if (hasFile && this.source instanceof GitHubStream && await FileModel.exists({ + repoId: this.repoId, treeGeneration: this.model.treeGeneration || { $exists: false }, name: ".gitmodules", + }).exec()) hasFile = null; if (!hasFile || opt.force) { const previousGeneration = this.model.treeGeneration; const previousPrefix = contentGenerationPrefix(`${previousGeneration || "legacy"}:${this.model.anonymizeDate?.toISOString() || ""}`); diff --git a/src/core/model/anonymized-path.ts b/src/core/model/anonymized-path.ts index d7ee41e..363f59a 100644 --- a/src/core/model/anonymized-path.ts +++ b/src/core/model/anonymized-path.ts @@ -1,7 +1,9 @@ +import { UpstreamFileSchema } from "./files/files.schema"; import { IFile } from "./files/files.types"; import { model, Schema } from "mongoose"; const schema = new Schema({ repoId: { type: String, required: true }, key: { type: String, required: true }, originalId: Schema.Types.ObjectId, name: { type: String, required: true }, path: { type: String, default: "", required: true }, sha: String, size: Number, + upstream: { type: UpstreamFileSchema, default: undefined }, anonymousName: String, anonymousDirectory: String, anonymousPath: String, searchName: String, searchDirectory: String, createdAt: { type: Date, default: Date.now } }); diff --git a/src/core/model/files/files.schema.ts b/src/core/model/files/files.schema.ts index 7838320..bb97e56 100644 --- a/src/core/model/files/files.schema.ts +++ b/src/core/model/files/files.schema.ts @@ -1,5 +1,7 @@ import { Schema } from "mongoose"; +export const UpstreamFileSchema = new Schema({ organization: String, repoName: String, commit: String, path: String }, { _id: false }); + const FileSchema = new Schema({ name: { type: String, index: true }, path: { type: String, index: true }, @@ -7,6 +9,8 @@ const FileSchema = new Schema({ treeGeneration: String, treeStaged: Boolean, metadataPending: Boolean, + upstream: { type: UpstreamFileSchema, default: undefined }, + submodule: { type: String, enum: ["included", "unsupported", "unavailable", "limit", "nested"] }, sha: { type: String, }, diff --git a/src/core/model/files/files.types.ts b/src/core/model/files/files.types.ts index f0fd8be..e6db347 100644 --- a/src/core/model/files/files.types.ts +++ b/src/core/model/files/files.types.ts @@ -1,6 +1,16 @@ import { Document, Model } from "mongoose"; +export interface UpstreamFile { + organization: string; + repoName: string; + commit: string; + path: string; +} + export interface IFile { + /** Internal source metadata, never returned by the public tree API. */ + upstream?: UpstreamFile; + submodule?: "included" | "unsupported" | "unavailable" | "limit" | "nested"; name: string; path: string; repoId: string; diff --git a/src/core/source/GitHubStream.ts b/src/core/source/GitHubStream.ts index 564b7aa..122e8c4 100644 --- a/src/core/source/GitHubStream.ts +++ b/src/core/source/GitHubStream.ts @@ -18,7 +18,8 @@ import * as stream from "stream"; import AnonymousError from "../AnonymousError"; import { octokit, waitForTokenGate, RateLimitDelayError } from "../GitHubUtils"; import FileModel from "../model/files/files.model"; -import { IFile } from "../model/files/files.types"; +import { expandSubmodules, validUpstream } from "./github-submodules"; +import { IFile, UpstreamFile } from "../model/files/files.types"; import { createLogger, serializeError } from "../logger"; import { measureStage, recordCacheHit } from "../request-monitoring"; import config from "../../config"; @@ -322,16 +323,27 @@ export default class GitHubStream extends GitHubBase { filePath: string, repoId: string, fileMeta: () => - | Promise<{ sha: string; size?: number }> - | { sha: string; size?: number } + | Promise<{ sha: string; size?: number; upstream?: UpstreamFile }> + | { sha: string; size?: number; upstream?: UpstreamFile } | Promise | string ) { const meta = await fileMeta(); - const expected: { sha: string; size?: number } = + const expected: { sha: string; size?: number; upstream?: UpstreamFile } = typeof meta === "string" ? { sha: meta } : meta; + if (expected.upstream && !validUpstream(expected.upstream)) throw new AnonymousError("invalid_request", { httpStatus: 400 }); + // Hydrated Mongoose subdocuments expose source fields through getters, + // so spreading them would retain the parent's repository coordinates. + const origin = expected.upstream ? new GitHubStream({ + ...this.data, + organization: expected.upstream.organization, + repoName: expected.upstream.repoName, + commit: expected.upstream.commit, + getToken: () => "", + }) : this; + const sourcePath = expected.upstream?.path || filePath; const generation = createHash("sha256").update(JSON.stringify([ - this.data.commit, expected.sha, this.data.cacheGeneration, this.data.cacheRevision, + this.data.commit, expected.sha, this.data.cacheGeneration, this.data.cacheRevision, ...(expected.upstream ? [expected.upstream] : []), ])).digest("hex"); const cachePath = this.data.cacheGeneration ? `${contentGenerationPrefix(this.data.cacheGeneration)}/${generation}/${filePath}` : filePath; const assertActive = async () => { @@ -383,8 +395,9 @@ export default class GitHubStream extends GitHubBase { if (cacheFills.size >= 64) throw new AnonymousError("cache_busy", { httpStatus: 503 }); filling = coordinatedFill(key, cached, async () => { await assertActive(); - const token = await this.data.getToken(); - const sourceMiss = cachedContentMiss(this.data, token); + const parentToken = await this.data.getToken(); + const token = expected.upstream ? "" : parentToken; + const sourceMiss = cachedContentMiss(this.data, parentToken); if (sourceMiss && sourceMiss !== "file_not_found") throw new AnonymousError(sourceMiss, { httpStatus: 404 }); const missKey = createHash("sha256").update(JSON.stringify([ key, this.data.organization, this.data.repoName, token, @@ -393,7 +406,7 @@ export default class GitHubStream extends GitHubBase { if (missed) throw new AnonymousError(missed, { httpStatus: 404 }); try { const fetchContent = () => measureStage("upstream", async () => { - const content = await this.downloadWithFallback(token, expected.sha, filePath) + const content = await origin.downloadWithFallback(token, expected.sha, sourcePath) .catch(error => { throw downloadError(error, filePath); }); let count = 0; const limited = new stream.Transform({ transform(chunk, encoding, callback) { @@ -413,9 +426,14 @@ export default class GitHubStream extends GitHubBase { finally { clearTimeout(deadline); limited.destroy(); content.destroy(); } return true; }); - return await this.confirmOwnerAccess(repoId, fetchContent); + return expected.upstream ? await fetchContent() : await this.confirmOwnerAccess(repoId, fetchContent); } catch (error) { - if (error instanceof AnonymousError && error.message === "file_not_found" && error.httpStatus === 404) { + if (expected.upstream && error instanceof AnonymousError && + ["file_not_found", "file_not_accessible"].includes(error.message)) { + // A removed public dependency is not a lost parent credential. + throw new AnonymousError("submodule_not_available", { httpStatus: 404, cause: error }); + } + if (!expected.upstream && error instanceof AnonymousError && error.message === "file_not_found" && error.httpStatus === 404) { const code = await classifyContentMiss(this.data, token); missingContent.set(missKey, code, 30_000); if (code !== "file_not_found") throw new AnonymousError(code, { httpStatus: 404, cause: error }); @@ -448,14 +466,14 @@ export default class GitHubStream extends GitHubBase { object: file, }); } - return { sha: fileSha, size: await file.size() }; + return { sha: fileSha, size: await file.size(), upstream: (await file.getFileInfo()).upstream }; } ); } async getFiles(progress?: (status: string) => void) { this._truncatedFolders = []; - return this.getTruncatedTree(this.data.commit, progress); + return expandSubmodules(await this.getTruncatedTree(this.data.commit, progress), this.data); } /** @@ -485,6 +503,7 @@ export default class GitHubStream extends GitHubBase { } private async fetchFileMetadata(filePath: string): Promise { + if (basename(filePath) === ".gitmodules") return null; const token = await this.data.getToken(); const oct = octokit(token); try { @@ -665,6 +684,7 @@ export default class GitHubStream extends GitHubBase { path: pathFile, repoId: this.data.repoId, size: elem.size, + ...(elem.type === "commit" ? { submodule: "unsupported" } : {}), sha: elem.sha, }); }); diff --git a/src/core/source/github-submodules.ts b/src/core/source/github-submodules.ts new file mode 100644 index 0000000..16477da --- /dev/null +++ b/src/core/source/github-submodules.ts @@ -0,0 +1,99 @@ +import { octokit, waitForTokenGate } from "../GitHubUtils"; +import AnonymousError from "../AnonymousError"; +import { IFile, UpstreamFile } from "../model/files/files.types"; +import { GitHubBaseData } from "./GitHubBase"; + +export const MAX_SUBMODULES = 20; +export const MAX_SUBMODULE_ENTRIES = 10000; +export interface SubmoduleMount extends Omit { path: string; } + +export function safeRepositoryPath(value: unknown): value is string { + return typeof value === "string" && value.length > 0 && + !value.includes("\\") && !value.includes("\0") && + value.split("/").every(part => part !== "" && part !== "." && part !== ".."); +} + +export function validUpstream(value: unknown): value is UpstreamFile { + if (!value || typeof value !== "object") return false; + const v = value as UpstreamFile; + return typeof v.organization === "string" && /^[a-zA-Z0-9-]+$/.test(v.organization) && + typeof v.repoName === "string" && /^[a-zA-Z0-9_.-]+$/.test(v.repoName) && ![".", ".."].includes(v.repoName) && + typeof v.commit === "string" && /^[a-f0-9]{40}$/.test(v.commit) && safeRepositoryPath(v.path); +} + +export function submoduleRepository(url: unknown, parent: GitHubBaseData): { organization: string; repoName: string } | null { + if (typeof url !== "string") return null; + const normalized = url.replace(/^git@github\.com:/, "https://github.com/").replace(/^ssh:\/\/git@github\.com\//, "https://github.com/").replace(/^git:\/\/github\.com\//, "https://github.com/"); + try { + const parsed = new URL(normalized, url.startsWith("./") || url.startsWith("../") + ? `https://github.com/${parent.organization}/${parent.repoName}/` : undefined); + if (parsed.protocol !== "https:" || parsed.hostname !== "github.com" || parsed.port || parsed.username || parsed.password || parsed.search || parsed.hash) return null; + const parts = parsed.pathname.replace(/\.git\/?$/, "").replace(/\/$/, "").split("/"); + if (parts.length !== 3) return null; + const candidate = { organization: parts[1], repoName: parts[2], commit: "a".repeat(40), path: "file" }; + return validUpstream(candidate) ? { organization: candidate.organization, repoName: candidate.repoName } : null; + } catch { return null; } +} + +/** Resolve only direct public GitHub dependencies. Never send the parent token to a child. */ +export async function expandSubmodules(files: IFile[], data: GitHubBaseData): Promise { + const output = files.filter(file => file.name !== ".gitmodules"); + const modules = output.filter(file => file.submodule); + let remaining = MAX_SUBMODULE_ENTRIES; + for (const [index, mount] of modules.entries()) { + if (index >= MAX_SUBMODULES) { mount.submodule = "limit"; continue; } + const mountPath = [mount.path, mount.name].filter(Boolean).join("/"); + if (!safeRepositoryPath(mountPath) || !mount.sha || !/^[a-f0-9]{40}$/.test(mount.sha)) continue; + const token = await data.getToken(); + await waitForTokenGate(token); + try { + const response = await octokit(token).repos.getContent({ + owner: data.organization, repo: data.repoName, path: mountPath, ref: data.commit, + }); + const metadata = response.data as { submodule_git_url?: string; sha?: string }; + const repository = submoduleRepository(metadata.submodule_git_url, data); + if (!repository || metadata.sha !== mount.sha) continue; + // An unauthenticated request proves public access, even for an owner whose + // parent token can also read private dependencies. No credentials cross repos. + await waitForTokenGate(""); + const child = await octokit("").git.getTree({ + owner: repository.organization, repo: repository.repoName, tree_sha: mount.sha, recursive: "1", + }); + if (child.data.truncated || child.data.tree.length > remaining) { mount.submodule = "limit"; continue; } + const entries = child.data.tree; + if (entries.some(entry => !safeRepositoryPath(entry.path))) { mount.submodule = "unsupported"; continue; } + remaining -= entries.length; + mount.submodule = "included"; + mount.upstream = { ...repository, commit: mount.sha, path: "" }; + for (const entry of entries) { + const relative = entry.path!; + if (relative.split("/").pop() === ".gitmodules") continue; + const fullPath = `${mountPath}/${relative}`; + const slash = fullPath.lastIndexOf("/"); + output.push({ + repoId: data.repoId, name: fullPath.slice(slash + 1), path: fullPath.slice(0, slash), + sha: entry.sha, size: entry.size, + ...(entry.type === "commit" ? { submodule: "nested" as const } : {}), + ...(entry.type === "blob" ? { upstream: { ...repository, commit: mount.sha, path: relative } } : {}), + }); + } + } catch (error) { + if ((error as { status?: number }).status === 404) { mount.submodule = "unavailable"; continue; } + // Do not persist a partial tree on rate limits, outages or credential errors. + throw error; + } + } + return output; +} + +export function validateMounts(value: unknown): SubmoduleMount[] { + if (value === undefined) return []; + if (!Array.isArray(value) || value.length > MAX_SUBMODULES || value.some(mount => !validUpstream(mount))) { + throw new AnonymousError("invalid_request", { httpStatus: 400 }); + } + const paths = value.map(mount => mount.path as string); + if (paths.some((path, i) => paths.some((other, j) => i !== j && (path === other || path.startsWith(other + "/"))))) { + throw new AnonymousError("invalid_request", { httpStatus: 400 }); + } + return value; +} diff --git a/src/core/zipStream.ts b/src/core/zipStream.ts index 9aa55a7..b088d05 100644 --- a/src/core/zipStream.ts +++ b/src/core/zipStream.ts @@ -1,4 +1,6 @@ import got from "got"; +import config from "../config"; +import { MAX_SUBMODULE_ENTRIES, SubmoduleMount, safeRepositoryPath, validateMounts } from "./source/github-submodules"; import { Readable, Transform, Writable } from "stream"; import { Parse } from "unzip-stream"; import archiver = require("archiver"); @@ -19,6 +21,7 @@ const logger = createLogger("zip-stream"); export interface StreamAnonymizedZipOptions { repoId: string; + submodules?: SubmoduleMount[]; organization: string; repoName: string; commit: string; @@ -78,6 +81,7 @@ export async function streamAnonymizedZip( on(event: string, listener: (...args: unknown[]) => void): unknown; } ): Promise { + const submodules = validateMounts(opt.submodules); let response; try { const token = await opt.getToken(); @@ -113,14 +117,10 @@ export async function streamAnonymizedZip( }, }); } - const upstreamDone = startStage("upstream"); - const downloadStream = got.stream(response.url); - downloadStream.once("end", () => upstreamDone()); - downloadStream.once("error", () => upstreamDone(true)); - downloadStream.once("close", () => upstreamDone(!downloadStream.readableEnded)); - + let downloadStream: Readable; const archive = archiver("zip", {}); - const parser = Parse() as Transform; + let parser: Transform; + let childEntries = 0; const activeStreams = new Set(); const compiledTerms = compileTerms(opt.anonymizerOptions.terms || []); let stopped = false; @@ -160,8 +160,8 @@ export async function streamAnonymizedZip( stopped = true; cleanupDrain?.(); cleanupDrain = undefined; - downloadStream.destroy(); - parser.destroy(); + downloadStream?.destroy(); + parser?.destroy(); for (const stream of activeStreams) stream.destroy(); activeStreams.clear(); output.destroy(); @@ -187,53 +187,82 @@ export async function streamAnonymizedZip( archive.on("error", fail); archive.pipe(output); - downloadStream - .on("error", fail) - .pipe(parser) - .on("entry", (entry: Readable & { type: string; path: string; autodrain: () => void }) => { - if (stopped) { - entry.destroy(); - return; - } - entry.on("error", fail); - activeStreams.add(entry); - entry.once("close", () => activeStreams.delete(entry)); - if (entry.type === "File") { - try { - const fileName = anonymizePathCompiled( - entry.path.substring(entry.path.indexOf("/") + 1), - compiledTerms - ); - if (!isEntryAllowed(entry.path, opt.contentOptions)) { - entry.autodrain(); - return; - } - // Pass filePath via the constructor — AnonymizeTransformer reads it - // there to decide whether the entry is text (and therefore should be - // anonymized) vs binary (passthrough). Assigning afterwards leaves - // isText=false for every file, so the zip ships unanonymized. - parser.pause(); - const onArchived = () => { entryWaiting = true; setImmediate(resumeParser); }; - archive.once("entry", onArchived); - const anonymizer = new AnonymizeTransformer({ - ...opt.anonymizerOptions, - filePath: entry.path, - }); - anonymizer.on("error", fail); - activeStreams.add(anonymizer); - anonymizer.once("close", () => activeStreams.delete(anonymizer)); - const st = entry.pipe(anonymizer); - archive.append(st, zipEntryOptions(fileName, entry.path)); - } catch (error) { - fail(error as Error); + const startArchive = (url: string, prefix: string, next: number) => { + if (stopped) return; + const upstreamDone = startStage("upstream"); + const input = got.stream(url); + downloadStream = input; + input.once("end", () => upstreamDone()); + input.once("error", () => upstreamDone(true)); + input.once("close", () => upstreamDone(!input.readableEnded)); + parser = Parse() as Transform; + downloadStream + .on("error", fail) + .pipe(parser) + .on("entry", (entry: Readable & { type: string; path: string; autodrain: () => void }) => { + if (stopped) { + entry.destroy(); + return; } - } else { - entry.autodrain(); - } - }) - .on("error", fail) - .on("end", () => { - if (stopped) return; - void archive.finalize().catch(fail); - }); + entry.on("error", fail); + activeStreams.add(entry); + entry.once("close", () => activeStreams.delete(entry)); + if (entry.type === "File") { + try { + const relativePath = entry.path.substring(entry.path.indexOf("/") + 1); + if (!safeRepositoryPath(relativePath)) throw new AnonymousError("invalid_file_path", { httpStatus: 502 }); + if (relativePath.split("/").pop() === ".gitmodules") { entry.autodrain(); return; } + if (prefix && ++childEntries > MAX_SUBMODULE_ENTRIES) throw new AnonymousError("file_too_big", { httpStatus: 413 }); + const mountedPath = prefix ? `${prefix}/${relativePath}` : relativePath; + const fileName = anonymizePathCompiled(mountedPath, compiledTerms); + if (!isEntryAllowed(entry.path, opt.contentOptions)) { + entry.autodrain(); + return; + } + // Pass filePath via the constructor — AnonymizeTransformer reads it + // there to decide whether the entry is text (and therefore should be + // anonymized) vs binary (passthrough). Assigning afterwards leaves + // isText=false for every file, so the zip ships unanonymized. + parser.pause(); + const onArchived = () => { entryWaiting = true; setImmediate(resumeParser); }; + archive.once("entry", onArchived); + const anonymizer = new AnonymizeTransformer({ + ...opt.anonymizerOptions, + filePath: mountedPath, + }); + anonymizer.on("error", fail); + activeStreams.add(anonymizer); + anonymizer.once("close", () => activeStreams.delete(anonymizer)); + let source: Readable = entry; + if (prefix) { + let bytes = 0; + const limited = new Transform({ transform(chunk, encoding, callback) { + bytes += chunk.length; + callback(bytes > config.MAX_FILE_SIZE ? new AnonymousError("file_too_big", { httpStatus: 413 }) : null, chunk); + } }); + limited.on("error", fail); + activeStreams.add(limited); + limited.once("close", () => activeStreams.delete(limited)); + source = entry.pipe(limited); + } + const st = source.pipe(anonymizer); + archive.append(st, zipEntryOptions(fileName, mountedPath)); + } catch (error) { + fail(error as Error); + } + } else { + entry.autodrain(); + } + }) + .on("error", fail) + .on("end", () => { + if (stopped) return; + const mount = submodules[next]; + if (mount) { + // Public child archives never receive parent credentials. + startArchive(`https://codeload.github.com/${encodeURIComponent(mount.organization)}/${encodeURIComponent(mount.repoName)}/zip/${mount.commit}`, mount.path, next + 1); + } else void archive.finalize().catch(fail); + }); + }; + startArchive(response.url, "", 0); } diff --git a/src/server/routes/repository-public.ts b/src/server/routes/repository-public.ts index ef1c8ad..e6388f1 100644 --- a/src/server/routes/repository-public.ts +++ b/src/server/routes/repository-public.ts @@ -57,6 +57,7 @@ router.get( res.attachment(`${repo.repoId}.zip`); if (req.method === "HEAD") { res.end(); return; } await repo.countView(); + const submodules = await repo.submoduleMounts(); if (config.STREAMER_ENTRYPOINT) { // use the streamer service @@ -75,6 +76,7 @@ router.get( branch: repo.model.source.branch, repoId: repo.repoId, anonymizerOptions: anonymizer.opt, + submodules, contentOptions: { image: repo.options.image, pdf: repo.options.pdf, @@ -169,6 +171,7 @@ router.get( commit: repo.model.source.commit || "HEAD", getToken: () => repo.getToken(), anonymizerOptions: anonymizer.opt, + submodules, contentOptions: { image: repo.options.image, pdf: repo.options.pdf, @@ -383,15 +386,15 @@ router.get( truncatedFolders: (repo.model.truncatedFolders || []).map((path) => anonymizePath(path, repo.options.terms || []) ), - // Submodule contents are not included in GitHub archives/trees, so - // they end up as empty folders in the anonymized repository. Surface - // a warning in the explorer when the repository uses submodules (#737). + // Legacy trees retain .gitmodules until their next tree refresh. hasSubmodules: (await FileModel.exists({ repoId: repo.repoId, treeGeneration: repo.model.treeGeneration || { $exists: false }, - name: ".gitmodules", - path: "", + $or: [ + { submodule: { $exists: true, $ne: "included" } }, + { name: ".gitmodules", path: "" }, + ], })) != null, }); } catch (error) { diff --git a/src/streamer/route.ts b/src/streamer/route.ts index e96abf3..447c759 100644 --- a/src/streamer/route.ts +++ b/src/streamer/route.ts @@ -34,6 +34,7 @@ router.post( getToken: () => token, anonymizerOptions: { ...anonymizerOptions, repoId }, contentOptions, + submodules: req.body.submodules, }, res ); @@ -89,7 +90,7 @@ router.post("/", async (req: express.Request, res: express.Response) => { const content = await source.getFileContentCache( filePath, repoId, - () => ({ sha: fileSha, size: fileSize }) + () => ({ sha: fileSha, size: fileSize, upstream: req.body.upstream }) ); if (res.destroyed || res.writableEnded) { content.destroy(); anonymizer.destroy(); return; } const mime = lookup(filePath); diff --git a/test/github-submodules.test.js b/test/github-submodules.test.js new file mode 100644 index 0000000..d011474 --- /dev/null +++ b/test/github-submodules.test.js @@ -0,0 +1,308 @@ +const { expect } = require("chai"); +const { Readable, PassThrough } = require("node:stream"); +const { once } = require("node:events"); +const { setImmediate } = require("node:timers"); +const archiver = require("archiver"); +const got = require("got"); +require("ts-node/register/transpile-only"); +const utils = require("../src/core/GitHubUtils"); +const { expandSubmodules, submoduleRepository, validateMounts, MAX_SUBMODULES, MAX_SUBMODULE_ENTRIES } = require("../src/core/source/github-submodules"); +const GitHubStream = require("../src/core/source/GitHubStream").default; +const FileModel = require("../src/core/model/files/files.model").default; +const storage = require("../src/core/storage").default; +const { streamAnonymizedZip } = require("../src/core/zipStream"); +const config = require("../src/config").default; +const sha = "a".repeat(40), blob = "b".repeat(40); +const parent = { repoId: "submodule-test", organization: "parent", repoName: "repo", commit: "c".repeat(40), getToken: () => "parent-private-token" }; +const upstream = { organization: "child", repoName: "public", commit: sha, path: "src/Alice.txt" }; +const mount = () => new FileModel({ repoId: parent.repoId, name: "dependency", path: "vendor", sha, submodule: "unsupported" }); + +async function zip(entries) { + const archive = archiver("zip"); const chunks = []; + archive.on("data", chunk => chunks.push(chunk)); const done = once(archive, "end"); + for (const [name, content] of Object.entries(entries)) archive.append(content, { name }); + await archive.finalize(); await done; return Buffer.concat(chunks); +} + +async function readZip(options) { + const response = new PassThrough(); const parser = require("unzip-stream").Parse(); + const entries = [], files = {}; + parser.on("entry", entry => { + const chunks = []; entry.on("data", chunk => chunks.push(chunk)); + entries.push(once(entry, "end").then(() => { files[entry.path] = Buffer.concat(chunks).toString(); })); + }); + response.pipe(parser); const done = once(parser, "finish"); + await streamAnonymizedZip(options, response); await done; await Promise.all(entries); + return files; +} + +const zipOptions = { ...parent, getToken: () => "", anonymizerOptions: { terms: ["Alice"], image: true, link: true }, + submodules: [{ organization: "child", repoName: "public", commit: sha, path: "vendor/dependency" }] }; + +describe("public GitHub submodules", function () { + this.timeout(10000); + const restore = []; + function stub(object, key, value) { const previous = object[key]; object[key] = value; restore.push(() => { object[key] = previous; }); } + afterEach(() => restore.splice(0).reverse().forEach(fn => fn())); + beforeEach(() => stub(utils, "waitForTokenGate", async () => {})); + + function api(tree = [{ path: "src/Alice.txt", type: "blob", sha: blob, size: 5 }], options = {}) { + const calls = []; + stub(utils, "octokit", token => ({ + repos: { getContent: async args => { + calls.push({ method: "contents", token, ...args }); + return { data: { submodule_git_url: options.url || "https://github.com/child/public.git", sha } }; + } }, + git: { getTree: async args => { + calls.push({ method: "tree", token, ...args }); + if (options.error) throw options.error; + return { data: { tree, truncated: options.truncated || false } }; + } }, + })); + return calls; + } + + it("mounts a pinned child tree without forwarding parent credentials or exposing .gitmodules", async () => { + const calls = api([ + { path: ".gitmodules", type: "blob", sha: blob, size: 100 }, + { path: "src", type: "tree", sha: blob }, + { path: "src/Alice.txt", type: "blob", sha: blob, size: 5 }, + { path: "nested", type: "commit", sha }, + ]); + const files = await expandSubmodules([mount(), { name: ".gitmodules", path: "" }], parent); + expect(files.map(file => file.name)).to.deep.equal(["dependency", "src", "Alice.txt", "nested"]); + expect(files[0].submodule).to.equal("included"); + expect(files[2]).to.include({ path: "vendor/dependency/src", sha: blob, size: 5 }); + expect(files[2].upstream).to.deep.equal(upstream); + expect(files[3].submodule).to.equal("nested"); + expect(calls).to.deep.equal([ + { method: "contents", token: "parent-private-token", owner: "parent", repo: "repo", path: "vendor/dependency", ref: parent.commit }, + { method: "tree", token: "", owner: "child", repo: "public", tree_sha: sha, recursive: "1" }, + ]); + expect(new FileModel(files[2]).toObject().upstream).to.deep.equal(upstream); + }); + + it("discovers submodules through scoped public-read contexts without authorizing child or write requests", async () => { + const { registerGitHubToken } = require("../src/core/github-token-context"); + const token = "public-read:submodule-discovery"; + registerGitHubToken(token, { publicRepository: "parent/repo", quotaKey: "submodule-test", + renew: async () => "parent-user-token" }); + const calls = []; + stub(globalThis, "fetch", async (url, options) => { + calls.push({ url: String(url), authorization: new globalThis.Headers(options.headers).get("authorization") }); + const result = String(url).includes("/contents/") + ? { submodule_git_url: "https://github.com/child/public.git", sha } + : { tree: [{ path: "src/Alice.txt", type: "blob", sha: blob, size: 5 }], truncated: false }; + return new globalThis.Response(JSON.stringify(result), { headers: { "content-type": "application/json" } }); + }); + const files = await expandSubmodules([mount()], { ...parent, getToken: () => token }); + expect(files[0].submodule).to.equal("included"); + expect(calls).to.deep.equal([ + { url: `https://api.github.com/repos/parent/repo/contents/vendor%2Fdependency?ref=${parent.commit}`, authorization: "token parent-user-token" }, + { url: `https://api.github.com/repos/child/public/git/trees/${sha}?recursive=1`, authorization: null }, + ]); + for (const request of [ + "GET /repos/child/public/contents/file.txt", "GET /repos/parent/repo-other/contents/file.txt", + "PUT /repos/parent/repo/contents/file.txt", "DELETE /repos/parent/repo/contents/file.txt", + ]) { + let caught; try { await utils.octokit(token).request(request); } catch (error) { caught = error; } + expect(caught?.message).to.equal("github_app_access_required"); + } + expect(calls).to.have.length(2); + }); + + it("recognizes gitlinks from the parent tree and leaves ordinary repositories at one request", async () => { + const source = new GitHubStream(parent); let calls = 0; + source.getGHTree = async () => { calls++; return { tree: [{ path: "README.md", type: "blob", sha: blob, size: 5 }] }; }; + expect((await source.getFiles()).map(file => file.name)).to.deep.equal(["README.md"]); expect(calls).to.equal(1); + api(); + source.getGHTree = async () => ({ tree: [{ path: "vendor/dependency", type: "commit", mode: "160000", sha }] }); + expect((await source.getFiles())[0].submodule).to.equal("included"); + }); + + it("supports relative and SSH GitHub URLs and rejects other hosts and unsafe paths", () => { + for (const url of ["../public.git", "git@github.com:parent/public.git", "ssh://git@github.com/parent/public.git", "https://github.com/parent/public.git"]) { + expect(submoduleRepository(url, parent)).to.deep.equal({ organization: "parent", repoName: "public" }); + } + for (const url of ["https://evil.test/child/repo", "file:///tmp/repo", "https://github.com@evil.test/a/b", "https://github.com/a/b?token=secret", "https://github.com/a%2fb/c"]) expect(submoduleRepository(url, parent)).to.equal(null); + for (const path of ["../escape", "/absolute", "a/../b", "a\\b", "a//b", "a\0b"]) expect(() => validateMounts([{ ...upstream, path }])).to.throw("invalid_request"); + expect(() => validateMounts([{ ...upstream, path: "lib" }, { ...upstream, path: "lib/nested" }])).to.throw("invalid_request"); + }); + + it("leaves private, missing and external children unsupported without blocking parent files", async () => { + api([], { error: Object.assign(new Error("not found"), { status: 404 }) }); + const result = await expandSubmodules([mount(), { name: "README.md", path: "", size: 3 }], parent); + expect(result[0].submodule).to.equal("unavailable"); expect(result[1].name).to.equal("README.md"); + const calls = api([], { url: "https://gitlab.com/child/repo.git" }); + expect((await expandSubmodules([mount()], parent))[0].submodule).to.equal("unsupported"); + expect(calls).to.have.length(1); + }); + + it("does not persist transient failures as permanently missing dependencies", async () => { + const error = Object.assign(new Error("rate limited"), { status: 403 }); api([], { error }); + let caught; try { await expandSubmodules([mount()], parent); } catch (err) { caught = err; } + expect(caught).to.equal(error); + }); + + it("bounds module count and rejects oversized or truncated child trees", async () => { + const calls = api(); + const mounts = Array.from({ length: MAX_SUBMODULES + 1 }, (_, i) => ({ ...mount().toObject(), name: `dependency${i}` })); + const result = await expandSubmodules(mounts, parent); + expect(result[MAX_SUBMODULES].submodule).to.equal("limit"); expect(calls).to.have.length(MAX_SUBMODULES * 2); + api(Array.from({ length: MAX_SUBMODULE_ENTRIES + 1 }, (_, i) => ({ path: `file${i}`, type: "blob", sha: blob, size: 1 }))); + expect((await expandSubmodules([mount()], parent))[0].submodule).to.equal("limit"); + api([], { truncated: true }); expect((await expandSubmodules([mount()], parent))[0].submodule).to.equal("limit"); + }); + + for (const hydrated of [false, true]) it(`fetches child content from ${hydrated ? "hydrated" : "plain"} metadata and caches under its parent mount`, async () => { + const fileOrigin = hydrated ? new FileModel({ upstream }).upstream : upstream; + const source = new GitHubStream({ ...parent, cacheGeneration: "tree:date" }); const cache = new Map(), requests = []; + stub(storage, "fileInfo", async (_id, path) => { if (!cache.has(path)) throw Object.assign(Error("missing"), { code: "ENOENT" }); return { size: cache.get(path).length }; }); + stub(storage, "write", async (id, path, input) => { expect(id).to.equal(parent.repoId); const chunks = []; for await (const chunk of input) chunks.push(Buffer.from(chunk)); cache.set(path, Buffer.concat(chunks)); }); + stub(storage, "read", async (_id, path) => Readable.from([cache.get(path)])); + stub(got, "stream", (url, options) => { requests.push({ url, options }); return Readable.from([Buffer.from("Alice")]); }); + for (let i = 0; i < 2; i++) { + const stream = await source.getFileContentCache("vendor/dependency/src/Alice.txt", parent.repoId, () => ({ sha: blob, size: 5, upstream: fileOrigin })); + for await (const chunk of stream) expect(chunk.toString()).to.equal("Alice"); + } + expect(requests).to.have.length(1); + expect(requests[0].url).to.equal(`https://github.com/child/public/raw/${sha}/src/Alice.txt`); + expect(requests[0].options.headers).to.deep.equal({}); + expect([...cache.keys()][0]).to.match(/\/vendor\/dependency\/src\/Alice\.txt$/); + const changed = { ...upstream, commit: "d".repeat(40) }; + const stream = await source.getFileContentCache("vendor/dependency/src/Alice.txt", parent.repoId, () => ({ sha: blob, size: 5, upstream: changed })); + stream.resume(); expect(requests).to.have.length(2); + }); + + it("does not report an unavailable public child as lost parent access", async () => { + const { isAccessFailure } = require("../src/core/owner-notifications"); + const source = new GitHubStream(parent); + stub(storage, "fileInfo", async () => { throw Object.assign(Error("missing"), { code: "ENOENT" }); }); + stub(storage, "write", async (_id, _path, input) => { for await (const _chunk of input) { /* consume */ } }); + stub(got, "stream", () => Readable.from((async function* () { + yield* []; + throw Object.assign(Error("not found"), { response: { statusCode: 404 } }); + })())); + let error; + try { await source.getFileContentCache("vendor/lib/file.txt", parent.repoId, () => ({ sha: blob, size: 5, upstream })); } + catch (caught) { error = caught; } + expect(error?.message).to.equal("submodule_not_available"); + expect(isAccessFailure(error)).to.equal(false); + }); + + it("includes pinned child archives with anonymized names and content, omitting manifests and disabled images", async () => { + const main = await zip({ "parent/README.md": "Alice parent", "parent/.gitmodules": "https://github.com/private/identity" }); + const child = await zip({ "child/src/Alice.txt": "Alice child", "child/.gitmodules": "identity", "child/photo.png": "image" }); + const calls = []; + stub(got, "stream", (url, options) => { calls.push({ url, options }); return Readable.from([calls.length === 1 ? main : child]); }); + expect(await readZip({ ...zipOptions, contentOptions: { image: false } })).to.deep.equal({ + "README.md": "XXXX-1 parent", "vendor/dependency/src/XXXX-1.txt": "XXXX-1 child", + }); + expect(calls.map(call => call.url)).to.deep.equal([ + `https://codeload.github.com/parent/repo/zip/${parent.commit}`, `https://codeload.github.com/child/public/zip/${sha}`, + ]); + expect(calls.every(call => call.options === undefined)).to.equal(true); + }); + + it("aborts the ZIP if a child exceeds the per-file limit", async () => { + const main = await zip({ "parent/README.md": "ok" }), child = await zip({ "child/large.txt": "too much data" }); + let calls = 0; stub(got, "stream", () => Readable.from([++calls === 1 ? main : child])); stub(config, "MAX_FILE_SIZE", 4); + const response = new PassThrough(); response.resume(); const errors = []; + response.on("error", error => errors.push(error)); const done = new Promise(resolve => response.once("close", resolve)); + await streamAnonymizedZip(zipOptions, response); await done; + expect(errors.map(error => error.message)).to.deep.equal(["file_too_big"]); + }); +}); + +describe("submodule transport and public metadata", function () { + this.timeout(10000); + it("does not expose upstream repository coordinates in the public tree", async () => { + const Repository = require("../src/core/Repository").default; + const files = await Repository.prototype.anonymizedFiles.call({ + _model: { options: { terms: ["Alice"] } }, + files: async () => [{ name: "Alice.txt", path: "vendor/dependency/src", sha: blob, size: 5, upstream, submodule: "included" }], + }, { includeSha: false }); + expect(files[0]).to.have.all.keys("name", "path", "sha", "size"); + expect(files[0].name).to.equal("XXXX-1.txt"); + expect(JSON.stringify(files)).not.to.include("public"); + }); + + for (const mode of ["send", "anonymizedContent"]) { + it(`preserves child origin through ${mode} and the real streamer route`, async () => { + const express = require("express"); + const File = require("../src/core/AnonymizedFile").default; + const { AnonymizeTransformer } = require("../src/core/anonymize-utils"); + const router = require("../src/streamer/route").router; + const previous = { endpoint: config.STREAMER_ENTRYPOINT, stream: got.stream, + info: storage.fileInfo, write: storage.write, read: storage.read }; + let server, payload; const cache = new Map(), requests = []; + try { + storage.fileInfo = async (_id, path) => { if (!cache.has(path)) throw Object.assign(Error("missing"), { code: "ENOENT" }); return { size: cache.get(path).length }; }; + storage.write = async (_id, path, stream) => { const chunks = []; for await (const chunk of stream) chunks.push(Buffer.from(chunk)); cache.set(path, Buffer.concat(chunks)); }; + storage.read = async (_id, path) => Readable.from([cache.get(path)]); + got.stream = (url, options) => { + if (!String(url).startsWith("https://github.com/")) return previous.stream(url, options); + requests.push({ url, options }); return Readable.from([Buffer.from("Alice")]); + }; + const app = express(); app.use(express.json()); + app.use((req, _res, next) => { payload = req.body; next(); }); app.use("/api", router); + server = await new Promise(resolve => { const listening = app.listen(0, "127.0.0.1", () => resolve(listening)); }); + config.STREAMER_ENTRYPOINT = `http://127.0.0.1:${server.address().port}/`; + const file = new File({ anonymizedPath: "vendor/dependency/src/XXXX-1.txt", repository: { + repoId: parent.repoId, options: { terms: ["Alice"] }, + model: { source: { repositoryName: "parent/repo", commit: parent.commit } }, + getToken: parent.getToken, owner: { id: "owner" }, + generateAnonymizeTransformer: filePath => new AnonymizeTransformer({ terms: ["Alice"], filePath, image: true, link: true }), + } }); + file._file = { name: "Alice.txt", path: "vendor/dependency/src", sha: blob, size: 5, upstream }; + let content = ""; + if (mode === "send") { + const response = new PassThrough(); response.setHeader = () => {}; + response.on("data", chunk => { content += chunk; }); + await file.send(response); + } else { + for await (const chunk of await file.anonymizedContent()) content += chunk; + } + expect(content).to.equal("XXXX-1"); + expect(payload.upstream).to.deep.equal(upstream); + expect(payload.filePath).to.equal("vendor/dependency/src/Alice.txt"); + expect(requests).to.have.length(1); + expect(requests[0].url).to.equal(`https://github.com/child/public/raw/${sha}/src/Alice.txt`); + expect(requests[0].options.headers).to.deep.equal({}); + } finally { + config.STREAMER_ENTRYPOINT = previous.endpoint; got.stream = previous.stream; + storage.fileInfo = previous.info; storage.write = previous.write; storage.read = previous.read; + if (server) { server.closeAllConnections(); await new Promise(resolve => server.close(resolve)); } + } + }); + } + + it("cancels the current child archive when the reader disconnects", async () => { + const main = await zip({ "parent/README.md": "ok" }); const child = new PassThrough(); + const previous = got.stream; let calls = 0, started; + const childStarted = new Promise(resolve => { started = resolve; }); + const response = new PassThrough(); response.resume(); + try { + got.stream = () => { if (++calls === 1) return Readable.from([main]); started(); return child; }; + await streamAnonymizedZip({ ...zipOptions, submodules: [...zipOptions.submodules, + { ...zipOptions.submodules[0], path: "other/dependency" }] }, response); + await childStarted; response.destroy(); + await new Promise(resolve => setImmediate(resolve)); + expect(child.destroyed).to.equal(true); expect(calls).to.equal(2); + } finally { got.stream = previous; child.destroy(); response.destroy(); } + }); +}); + +describe("legacy submodule manifests", function () { + it("rejects a cached .gitmodules file even when its public name was anonymized", async () => { + const File = require("../src/core/AnonymizedFile").default; + const file = new File({ anonymizedPath: ".XXXX-1", repository: { + options: { terms: ["gitmodules"] }, model: { source: { type: "GitHubStream" } }, + } }); + file._file = { name: ".gitmodules", path: "", sha: blob, size: 5 }; + for (const read of [() => file.getFileInfo(), () => file.originalPath()]) { + let error; try { await read(); } catch (caught) { error = caught; } + expect(error?.message).to.equal("file_not_found"); + } + }); +}); diff --git a/test/owner-notifications.test.js b/test/owner-notifications.test.js index 0343bee..49756cc 100644 --- a/test/owner-notifications.test.js +++ b/test/owner-notifications.test.js @@ -500,9 +500,7 @@ describe("streamed file access alerts", () => { stub(config, "STREAMER_ENTRYPOINT", "http://streamer/"); const stream = new PassThrough(); stub(got, "stream", () => stream); - file.originalPath = async () => "README.md"; - file.sha = async () => "sha"; - file.size = async () => 10; + file._file = { name: "README.md", path: "", sha: "sha", size: 10 }; const returned = await file.anonymizedContent(); expect(returned).to.equal(stream); const error = Object.assign(new Error("streamer failure"), { response: { statusCode: status, body: JSON.stringify({ error: status === 503 ? "upstream_error" : "file_not_accessible" }) } }); diff --git a/test/performance-integration.test.js b/test/performance-integration.test.js index c0bc500..8fa69d8 100644 --- a/test/performance-integration.test.js +++ b/test/performance-integration.test.js @@ -40,6 +40,19 @@ const uri = process.env.PERFORMANCE_MONGO_URI; }); beforeEach(async () => { await Promise.all([Repo, PR, Gist, File, Path, Name, ConferenceModel].map(model => model.deleteMany({}))); }); + it("preserves submodule origin through the persisted anonymized path index", async () => { + const row = await Repo.create({ repoId: "submodule-index", owner: user.model._id, status: "ready", treeGeneration: "generation", + source: { repositoryName: "parent/repo", commit: "a".repeat(40) }, options: { terms: ["Alice"] } }); + const upstream = { organization: "child", repoName: "public", commit: "b".repeat(40), path: "Alice.txt" }; + await File.create({ repoId: row.repoId, treeGeneration: "generation", name: "Alice.txt", path: "vendor/lib", sha: "c".repeat(40), size: 5, upstream }); + const repository = new Repository(row); + const file = new AnonymizedFile({ repository, anonymizedPath: "vendor/lib/XXXX-1.txt" }); + expect((await file.getFileInfo()).upstream).to.deep.equal(upstream); + const publicTree = await repository.anonymizedFiles({ includeSha: false }); + expect(publicTree[0]).not.to.have.property("upstream"); + expect(JSON.stringify(publicTree)).not.to.include("child"); + }); + it("uses membership indexes for owner, stable GitHub ID, and legacy username", async () => { const other = new mongoose.Types.ObjectId(); await Repo.insertMany(Array.from({ length: 2000 }, (_, i) => ({ repoId: `filler-${i}`, owner: other, coauthors: [{ username: `other-${i}`, githubId: String(i + 50000) }] }))); diff --git a/test/zip-proxy-errors.test.js b/test/zip-proxy-errors.test.js index 626e081..fb7ee80 100644 --- a/test/zip-proxy-errors.test.js +++ b/test/zip-proxy-errors.test.js @@ -25,7 +25,7 @@ describe("ZIP proxy failures", function () { utils.getUser = async () => { throw new Error("anonymous"); }; utils.getRepo = async () => ({ repoId: "test", options: {}, model: { source: { repositoryName: "owner/repo", commit: "abc" } }, - countView: async () => {}, getToken: async () => "", + countView: async () => {}, submoduleMounts: async () => [], getToken: async () => "", generateAnonymizeTransformer: () => ({ opt: {} }), }); upstream = new PassThrough();