diff --git a/src/core/AnonymizedFile.ts b/src/core/AnonymizedFile.ts index 3217e95..8d1ef4f 100644 --- a/src/core/AnonymizedFile.ts +++ b/src/core/AnonymizedFile.ts @@ -508,7 +508,7 @@ export default class AnonymizedFile { resolve(); }); } catch (error) { - handleError(error, res); + reject(error); } }); } diff --git a/src/core/source/GitHubStream.ts b/src/core/source/GitHubStream.ts index 28c3a58..c2eabfa 100644 --- a/src/core/source/GitHubStream.ts +++ b/src/core/source/GitHubStream.ts @@ -105,7 +105,7 @@ export default class GitHubStream extends GitHubBase { const url = githubRawFileUrl( this.data.organization, this.data.repoName, - this.data.commit, + this.data.commit || "HEAD", filePath ); logger.debug("downloading via raw URL (LFS)", { url }); diff --git a/src/core/zipStream.ts b/src/core/zipStream.ts index 1ce912f..51e3baf 100644 --- a/src/core/zipStream.ts +++ b/src/core/zipStream.ts @@ -75,17 +75,23 @@ export async function streamAnonymizedZip( on(event: string, listener: (...args: unknown[]) => void): unknown; } ): Promise { - const source = new GitHubDownload({ - repoId: opt.repoId, - organization: opt.organization, - repoName: opt.repoName, - commit: opt.commit, - getToken: opt.getToken, - }); - let response; try { - response = await source.getZipUrl(); + const token = await opt.getToken(); + if (!token) { + // The API already checked public access. Codeload serves public archives + // directly, avoiding the streamer's shared unauthenticated REST quota. + response = { url: `https://codeload.github.com/${encodeURIComponent(opt.organization)}/${encodeURIComponent(opt.repoName)}/zip/${encodeURIComponent(opt.commit || "HEAD")}` }; + } else { + const source = new GitHubDownload({ + repoId: opt.repoId, + organization: opt.organization, + repoName: opt.repoName, + commit: opt.commit, + getToken: () => token, + }); + response = await source.getZipUrl(); + } } catch (error) { const code = await classifyGitHubMissError(error, { organization: opt.organization, diff --git a/test/streamer-public-access.test.js b/test/streamer-public-access.test.js index 09d1e1f..e7eafaf 100644 --- a/test/streamer-public-access.test.js +++ b/test/streamer-public-access.test.js @@ -1,7 +1,7 @@ const { expect } = require("chai"); const express = require("express"); const got = require("got"); -const { Readable } = require("stream"); +const { Readable, PassThrough } = require("stream"); require("ts-node/register/transpile-only"); const config = require("../src/config").default; const { registerGitHubToken, githubTokenForStreamer } = require("../src/core/github-token-context"); @@ -36,8 +36,31 @@ describe("public repository streamer handoff", function () { await rejects(githubTokenForStreamer("public-read:missing", "owner/public"), "Public repository access context expired"); }); - for (const mode of ["send", "anonymizedContent"]) { - it(`serves a public README through ${mode} and a separate HTTP streamer`, async function () { + it("rejects send when the access recheck fails before opening the streamer", async function () { + const endpoint = config.STREAMER_ENTRYPOINT; + config.STREAMER_ENTRYPOINT = "http://unused.test/"; + const response = new PassThrough(); + try { + registerGitHubToken("public-read:revoked-send", { + quotaKey: "test", publicRepository: "owner/public", renew: async () => { throw new Error("revoked"); }, + }); + const file = new File({ repository: { + options: { terms: [] }, model: { source: { repositoryName: "owner/public" } }, + getToken: async () => "public-read:revoked-send", + generateAnonymizeTransformer: filePath => new AnonymizeTransformer({ terms: [], filePath }), + }, anonymizedPath: "README.md" }); + file._file = { name: "README.md", path: "", sha: "sha", size: 25 }; + await rejects(file.send(response), "revoked"); + } finally { + config.STREAMER_ENTRYPOINT = endpoint; + response.destroy(); + } + }); + + for (const [mode, commit] of [ + ["send", "abc"], ["anonymizedContent", "abc"], ["send", undefined], ["send", ""], + ]) { + it(`serves a public README through ${mode} with commit ${JSON.stringify(commit)}`, async function () { const previous = { endpoint: config.STREAMER_ENTRYPOINT, stream: got.stream, cache: GitHubStream.prototype.getFileContentCache }; const servers = []; let payload; @@ -66,7 +89,7 @@ describe("public repository streamer handoff", function () { const options = { terms: ["Alice"], image: true, link: true }; const repo = { repoId: "test", options, - model: { source: { repositoryName: "owner/public", commit: "abc" } }, + model: { source: { repositoryName: "owner/public", commit } }, getToken: async () => "public-read:http-test", generateAnonymizeTransformer: path => new AnonymizeTransformer({ ...options, filePath: path }), }; @@ -86,7 +109,7 @@ describe("public repository streamer handoff", function () { expect(JSON.stringify(payload)).not.to.include("public-read:"); expect(JSON.stringify(payload)).not.to.include("private-owner-token"); expect(requests).to.have.length(1); - expect(requests[0].url).to.equal("https://github.com/owner/public/raw/abc/README.md"); + expect(requests[0].url).to.equal(`https://github.com/owner/public/raw/${commit || "HEAD"}/README.md`); expect(requests[0].options.headers).not.to.have.property("authorization"); } finally { got.stream = previous.stream; diff --git a/test/zip-stream-errors.test.js b/test/zip-stream-errors.test.js index 0bb1f05..2903f5c 100644 --- a/test/zip-stream-errors.test.js +++ b/test/zip-stream-errors.test.js @@ -22,7 +22,8 @@ async function fixture() { } describe("ZIP stream errors", function () { - it("finishes a valid ZIP with anonymized file content", async function () { + for (const commit of ["abc", undefined, ""]) { + it(`downloads a public ZIP without a REST lookup with commit ${JSON.stringify(commit)}`, async function () { const input = await fixture(); const previous = { stream: got.stream, zip: GitHubDownload.prototype.getZipUrl }; const response = new PassThrough(); @@ -37,10 +38,13 @@ describe("ZIP stream errors", function () { const finished = once(parser, "finish"); response.pipe(parser); try { - GitHubDownload.prototype.getZipUrl = async () => ({ url: "https://example.test/archive.zip" }); - got.stream = () => Readable.from([input]); + GitHubDownload.prototype.getZipUrl = async () => { throw new Error("must not use the anonymous REST quota"); }; + got.stream = url => { + expect(url).to.equal(`https://codeload.github.com/owner/public/zip/${commit || "HEAD"}`); + return Readable.from([input]); + }; await streamAnonymizedZip({ - repoId: "test", organization: "owner", repoName: "public", commit: "abc", + repoId: "test", organization: "owner", repoName: "public", commit, getToken: () => "", anonymizerOptions: { terms: ["private"], image: true, link: true }, }, response); await finished; @@ -52,6 +56,7 @@ describe("ZIP stream errors", function () { response.destroy(); } }); + } it("aborts a download on an asynchronous anonymization timeout without crashing", async function () { const input = await fixture(); @@ -73,7 +78,7 @@ describe("ZIP stream errors", function () { }; await streamAnonymizedZip({ repoId: "test", organization: "owner", repoName: "public", commit: "abc", - getToken: () => "", anonymizerOptions: { terms: ["private"], image: true, link: true }, + getToken: () => "private-token", anonymizerOptions: { terms: ["private"], image: true, link: true }, }, response); await closed; await new Promise(resolve => setImmediate(resolve));