diff --git a/src/core/AnonymizedFile.ts b/src/core/AnonymizedFile.ts index 19703ce..2a7e81f 100644 --- a/src/core/AnonymizedFile.ts +++ b/src/core/AnonymizedFile.ts @@ -362,7 +362,16 @@ export default class AnonymizedFile { return true; } + /** Tree entries have no size; zero-byte files are still valid files. */ + async assertContentFile(): Promise { + const file = await this.getFileInfo(); + if (file.size == null || file.submodule) { + throw new AnonymousError("folder_not_supported", { httpStatus: 404 }); + } + } + async content(): Promise { + await this.assertContentFile(); if (this.anonymizedPath.includes(config.ANONYMIZATION_MASK)) { await this.originalPath(); } @@ -388,6 +397,7 @@ export default class AnonymizedFile { } async anonymizedContent() { + await this.assertContentFile(); const anonymizer = this.repository.generateAnonymizeTransformer( await this.originalPath() ); @@ -439,6 +449,7 @@ export default class AnonymizedFile { } async send(res: Response): Promise { + await this.assertContentFile(); const anonymizer = this.repository.generateAnonymizeTransformer( await this.originalPath() ); diff --git a/src/core/source/GitHubStream.ts b/src/core/source/GitHubStream.ts index b53f6d8..f9de7ae 100644 --- a/src/core/source/GitHubStream.ts +++ b/src/core/source/GitHubStream.ts @@ -139,16 +139,30 @@ export default class GitHubStream extends GitHubBase { filePath ); logger.debug("downloading via raw URL (LFS)", { url }); + const assertDownloadUrl = (value: URL) => { + if (value.protocol !== "https:" || value.username || value.password || value.port || + !["raw.githubusercontent.com", "media.githubusercontent.com"].includes(value.hostname)) { + throw new AnonymousError("upstream_error", { httpStatus: 502 }); + } + }; const data = this.data; const cancellation = new AbortController(); let active: stream.Readable | undefined; const output = stream.Readable.from((async function* () { try { active = got.stream(url, { - hooks: { beforeRequest: [async () => { await githubTokenContext(token)?.renew(); }] }, + hooks: { + beforeRequest: [async () => { await githubTokenContext(token)?.renew(); }], + beforeRedirect: [options => { assertDownloadUrl(options.url); }], + }, headers: !token || githubTokenContext(token)?.publicRepository ? {} : { authorization: `token ${token}` }, followRedirect: true, }); + active.once("response", response => { + if (response.statusCode !== 200) return; + try { assertDownloadUrl(new URL(response.url)); } + catch (error) { active?.destroy(error as Error); } + }); for await (const chunk of active) yield chunk; return; } catch (error) { @@ -173,12 +187,6 @@ export default class GitHubStream extends GitHubBase { if (cancellation.signal.aborted) return; const metadata = response.data as { type?: string; download_url?: string }; if (metadata.type !== "file" || !metadata.download_url) throw error; - const assertDownloadUrl = (value: URL) => { - if (value.protocol !== "https:" || value.username || value.password || value.port || - !["raw.githubusercontent.com", "media.githubusercontent.com"].includes(value.hostname)) { - throw new AnonymousError("upstream_error", { httpStatus: 502 }); - } - }; assertDownloadUrl(new URL(metadata.download_url)); // The API URL supplies its own authorization. Never forward the // owner's token to the download host or a redirect destination. diff --git a/src/server/routes/file.ts b/src/server/routes/file.ts index 67da556..180436b 100644 --- a/src/server/routes/file.ts +++ b/src/server/routes/file.ts @@ -115,6 +115,7 @@ router.get( anonymizedPath, }); const originalPath = await f.originalPath(); + await f.assertContentFile(); if (!f.isFileSupported()) { throw new AnonymousError("file_not_supported", { httpStatus: 403, diff --git a/test/folder-file-security.test.js b/test/folder-file-security.test.js new file mode 100644 index 0000000..bf2b7e5 --- /dev/null +++ b/test/folder-file-security.test.js @@ -0,0 +1,86 @@ +const { expect } = require("chai"); +const express = require("express"); +const http = require("http"); +const { Readable } = require("stream"); +const { URL } = require("url"); +const got = require("got"); +require("ts-node/register/transpile-only"); +const AnonymizedFile = require("../src/core/AnonymizedFile").default; +const GitHubStream = require("../src/core/source/GitHubStream").default; +const utils = require("../src/server/routes/route-utils"); +const router = require("../src/server/routes/file").default; + +async function collect(input) { const chunks = []; for await (const chunk of input) chunks.push(Buffer.from(chunk)); return Buffer.concat(chunks).toString(); } +async function listen(app) { return new Promise(resolve => { const server = app.listen(0, "127.0.0.1", () => resolve(server)); }); } +async function close(server) { server.closeAllConnections(); await new Promise(resolve => server.close(resolve)); } + +describe("folder file endpoint disclosure", function () { + this.timeout(10000); + const restores = []; + function stub(object, key, value) { const old = object[key]; object[key] = value; restores.push(() => { object[key] = old; }); } + afterEach(() => restores.splice(0).reverse().forEach(restore => restore())); + + for (const size of [undefined, null]) { + it(`rejects folder metadata with size ${size} before any cached or upstream content`, async () => { + const f = new AnonymizedFile({ repository: { options: { terms: [] }, model: { source: { type: "GitHubStream" } } }, anonymizedPath: "strategy1/.idea" }); + f._file = { name: ".idea", path: "strategy1", sha: "tree-sha", size }; + for (const call of [() => f.content(), () => f.anonymizedContent(), () => f.send({})]) { + const error = await call().catch(error => error); + expect(error.message).to.equal("folder_not_supported"); expect(error.httpStatus).to.equal(404); + } + }); + } + it("keeps zero-byte files and repository HTML downloadable", async () => { + for (const [name, size] of [["empty.txt", 0], ["index.html", 123]]) { + const f = new AnonymizedFile({ repository: { options: { terms: [] }, model: { source: { type: "GitHubStream" } } }, anonymizedPath: name }); + f._file = { name, path: "", size, sha: "blob-sha" }; + await f.assertContentFile(); + } + }); + it("rejects GET, HEAD and conditional folder requests without trailing slash", async () => { + stub(utils, "getRepo", async () => ({ options: { terms: [] }, isReady: async () => true })); + stub(AnonymizedFile.prototype, "getFileInfo", async () => ({ name: ".idea", path: "strategy1", sha: "tree-sha" })); + stub(AnonymizedFile.prototype, "send", async () => { throw Error("must not serve cached content"); }); + const app = express(); app.use("/api/repo", router); const server = await listen(app); + try { + for (const method of ["GET", "HEAD"]) { + const response = await got(`http://127.0.0.1:${server.address().port}/api/repo/test/file/strategy1/.idea`, { method, headers: { "if-none-match": "*" }, throwHttpErrors: false }); + expect(response.statusCode).to.equal(404); + if (method === "GET") expect(JSON.parse(response.body).error).to.equal("folder_not_supported"); + } + } finally { await close(server); } + }); + it("blocks a real HTTP redirect to a GitHub tree page before fetching its HTML", async () => { + const server = await listen(http.createServer((_req, res) => { res.writeHead(302, { location: "https://github.com/example/private-repo/tree/main/folder" }); res.end(); })); + const original = got.stream; + stub(got, "stream", (_url, options) => original(`http://127.0.0.1:${server.address().port}/raw`, options)); + const source = new GitHubStream({ repoId: "test", organization: "example", repoName: "repo", commit: "main", getToken: () => "" }); + try { + const error = await collect(await source.downloadWithFallback("", "tree-sha", "folder")).catch(error => error); + expect(error.message).to.equal("upstream_error"); + } finally { await close(server); } + }); + it("rejects a successful web-page response before emitting any bytes", async () => { + const server = await listen(http.createServer((_req, res) => { res.writeHead(200, { "content-type": "text/html" }); res.end("owner metadata"); })); + const original = got.stream; + stub(got, "stream", (_url, options) => original(`http://127.0.0.1:${server.address().port}/raw`, options)); + const source = new GitHubStream({ repoId: "test", organization: "example", repoName: "repo", commit: "main", getToken: () => "" }); + let bytes = 0; + try { + const stream = await source.downloadWithFallback("", "tree-sha", "folder"); + stream.on("data", chunk => { bytes += chunk.length; }); + const error = await collect(stream).catch(error => error); + expect(error.message).to.equal("upstream_error"); expect(bytes).to.equal(0); + } finally { await close(server); } + }); + it("allows raw/LFS redirects and legitimate HTML file bodies", async () => { + stub(got, "stream", (_url, options) => { + for (const host of ["raw.githubusercontent.com", "media.githubusercontent.com"]) { + options.hooks.beforeRedirect[0]({ url: new URL(`https://${host}/owner/repo/main/index.html`) }); + } + return Readable.from(["repository file"]); + }); + const source = new GitHubStream({ repoId: "test", organization: "example", repoName: "repo", commit: "main", getToken: () => "" }); + expect(await collect(await source.downloadWithFallback("", "blob-sha", "index.html"))).to.equal("repository file"); + }); +}); diff --git a/test/owner-notifications.test.js b/test/owner-notifications.test.js index 49756cc..26aa1ad 100644 --- a/test/owner-notifications.test.js +++ b/test/owner-notifications.test.js @@ -484,6 +484,7 @@ describe("streamed file access alerts", () => { repository = { owner: { id: "saved-owner" }, options: { terms: [] }, model: { source: {} }, repoId: "anon-id", status: "ready", source: {}, getToken: async () => "token", generateAnonymizeTransformer: () => new PassThrough() }; file = new File({ repository, anonymizedPath: "README.md" }); + file._file = { name: "README.md", path: "", sha: "sha", size: 10 }; }); afterEach(() => { while (restores.length) restores.pop()(); }); for (const status of [403, 404, 503]) it(`handles local file download failure ${status}`, async () => { diff --git a/test/production-regressions.test.js b/test/production-regressions.test.js index d5b1c5c..d3c72c8 100644 --- a/test/production-regressions.test.js +++ b/test/production-regressions.test.js @@ -226,7 +226,7 @@ describe("production regressions", function () { const utils = require("../src/server/routes/route-utils"); const repo = { options: { pdf: false, image: false, terms: [] }, model: { source: { commit: "commit-1" }, options: {} }, isReady: async () => true, countView: async () => {} }; stub(utils, "getRepo", async () => repo); - stub(File.prototype, "originalPath", async function () { this._file = { name: originalName, path: "" }; return originalName; }); + stub(File.prototype, "originalPath", async function () { this._file = { name: originalName, path: "", size: 10 }; return originalName; }); stub(File.prototype, "sha", async () => "sha"); stub(File.prototype, "send", async () => {}); const handler = require("../src/server/routes/file").default.stack[0].route.stack[0].handle;