From 2d8f85402b8da2ff3323e5b17805ad27396ce9b3 Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Sat, 14 May 2022 07:58:06 -0700 Subject: [PATCH 1/9] Use more realistic HTTP status codes in fake --- src/test/util/fake-github-actions-cache-server.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/test/util/fake-github-actions-cache-server.ts b/src/test/util/fake-github-actions-cache-server.ts index e753286b9..925628fe5 100644 --- a/src/test/util/fake-github-actions-cache-server.ts +++ b/src/test/util/fake-github-actions-cache-server.ts @@ -318,7 +318,11 @@ export class FakeGitHubActionsCacheServer { commited: false, tarballId, }); - this.#respond(response, 200, JSON.stringify({cacheId: entryId})); + this.#respond( + response, + /* Created */ 201, + JSON.stringify({cacheId: entryId}) + ); }); } @@ -370,7 +374,7 @@ export class FakeGitHubActionsCacheServer { }); request.on('end', () => { - this.#respond(response, 200); + this.#respond(response, /* No Content */ 204); }); } @@ -404,7 +408,7 @@ export class FakeGitHubActionsCacheServer { } entry.commited = true; - this.#respond(response, 200); + this.#respond(response, /* No Content */ 204); } /** From 1d8ab30a478ab40c3ff6a026319cf963d296b252 Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Sat, 14 May 2022 08:35:21 -0700 Subject: [PATCH 2/9] Add random base path to GitHub Cache fake --- src/test/cache-github.test.ts | 4 +- .../util/fake-github-actions-cache-server.ts | 84 +++++++++---------- 2 files changed, 41 insertions(+), 47 deletions(-) diff --git a/src/test/cache-github.test.ts b/src/test/cache-github.test.ts index d292e3bb1..473a8a41f 100644 --- a/src/test/cache-github.test.ts +++ b/src/test/cache-github.test.ts @@ -23,11 +23,11 @@ test.before.each(async (ctx) => { // suite) because we want fresh cache state for each test. const authToken = String(Math.random()).slice(2); ctx.server = new FakeGitHubActionsCacheServer(authToken); - await ctx.server.listen(); + const actionsCacheUrl = await ctx.server.listen(); ctx.rig = new WireitTestRig(); ctx.rig.env = { WIREIT_CACHE: 'github', - ACTIONS_CACHE_URL: `http://localhost:${ctx.server.port}/`, + ACTIONS_CACHE_URL: actionsCacheUrl, ACTIONS_RUNTIME_TOKEN: authToken, RUNNER_TEMP: pathlib.join(ctx.rig.temp, 'github-cache-temp'), }; diff --git a/src/test/util/fake-github-actions-cache-server.ts b/src/test/util/fake-github-actions-cache-server.ts index 925628fe5..9958bf3a4 100644 --- a/src/test/util/fake-github-actions-cache-server.ts +++ b/src/test/util/fake-github-actions-cache-server.ts @@ -75,6 +75,7 @@ type ApiName = 'check' | 'reserve' | 'upload' | 'commit' | 'download'; */ export class FakeGitHubActionsCacheServer { readonly #server: http.Server; + #url!: URL; /** * An authentication token which this server will require to be set in a @@ -116,18 +117,24 @@ export class FakeGitHubActionsCacheServer { }; } - async listen(): Promise { - return new Promise((resolve) => { - this.#server.listen( - { - host: 'localhost', - port: /* random free */ 0, - }, - () => { - resolve(); - } - ); + async listen(): Promise { + const host = 'localhost'; + await new Promise((resolve) => { + this.#server.listen({host, port: /* random free */ 0}, () => resolve()); }); + const address = this.#server.address(); + if (address === null || typeof address !== 'object') { + throw new Error( + `Expected server address to be ServerInfo object, ` + + `got ${JSON.stringify(address)}` + ); + } + // The real API includes a unique identifier as the base path. It's good to + // include this in the fake because it ensures the client is preserving the + // base path and not just using the origin. + const randomBasePath = Math.random().toString().slice(2); + this.#url = new URL(`http://${host}:${address.port}/${randomBasePath}/`); + return this.#url.href; } async close(): Promise { @@ -138,17 +145,6 @@ export class FakeGitHubActionsCacheServer { }); } - get port(): number { - const address = this.#server.address(); - if (address === null || typeof address !== 'object') { - throw new Error( - `Expected server address to be ServerInfo object, ` + - `got ${JSON.stringify(address)}` - ); - } - return address.port; - } - rateLimitNextRequest(apiName: ApiName): void { this.#rateLimitNextRequest.add(apiName); } @@ -187,34 +183,35 @@ export class FakeGitHubActionsCacheServer { if (!request.url) { return this.#respond(response, 404); } - // Request.url is only the pathname + query params. - const url = new URL(request.url, `http://localhost:${this.port}`); + // request.url is a string with pathname + query params. + const url = new URL(request.url, this.#url.origin); + if (!url.pathname.startsWith(this.#url.pathname)) { + // Missing the random base path. + return this.#respond(response, 404); + } + const api = url.pathname.slice(this.#url.pathname.length); - if ( - url.pathname === '/_apis/artifactcache/cache' && - request.method === 'GET' - ) { + if (api === '_apis/artifactcache/cache' && request.method === 'GET') { return this.#check(request, response, url); } - if ( - url.pathname === '/_apis/artifactcache/caches' && - request.method === 'POST' - ) { + if (api === '_apis/artifactcache/caches' && request.method === 'POST') { return this.#reserve(request, response); } - if (url.pathname.startsWith('/_apis/artifactcache/caches/')) { + if (api.startsWith('_apis/artifactcache/caches/')) { + const tail = api.slice('_apis/artifactcache/caches/'.length); if (request.method === 'PATCH') { - return this.#upload(request, response, url); + return this.#upload(request, response, tail); } if (request.method === 'POST') { - return this.#commit(request, response, url); + return this.#commit(request, response, tail); } } - if (url.pathname.startsWith('/tarballs/') && request.method === 'GET') { - return this.#download(request, response, url); + if (api.startsWith('tarballs/') && request.method === 'GET') { + const tail = api.slice('tarballs/'.length); + return this.#download(request, response, tail); } this.#respond(response, 404); @@ -276,7 +273,7 @@ export class FakeGitHubActionsCacheServer { response, 200, JSON.stringify({ - archiveLocation: `http://localhost:${this.port}/tarballs/${entry.tarballId}`, + archiveLocation: `${this.#url.href}tarballs/${entry.tarballId}`, cacheKey: keys, }) ); @@ -335,7 +332,7 @@ export class FakeGitHubActionsCacheServer { #upload( request: http.IncomingMessage, response: http.ServerResponse, - url: URL + idStr: string ): void { this.metrics.upload++; if (this.#rateLimitNextRequest.delete('upload')) { @@ -345,7 +342,6 @@ export class FakeGitHubActionsCacheServer { return; } - const idStr = url.pathname.slice('/_apis/artifactcache/caches/'.length); if (idStr.match(/\d+/) === null) { return this.#respond(response, 400, 'Cache ID was not an integer'); } @@ -387,7 +383,7 @@ export class FakeGitHubActionsCacheServer { #commit( request: http.IncomingMessage, response: http.ServerResponse, - url: URL + idStr: string ): void { this.metrics.commit++; if (this.#rateLimitNextRequest.delete('commit')) { @@ -397,7 +393,6 @@ export class FakeGitHubActionsCacheServer { return; } - const idStr = url.pathname.slice('/_apis/artifactcache/caches/'.length); if (idStr.match(/\d+/) === null) { return this.#respond(response, 400, 'Cache ID was not an integer'); } @@ -425,15 +420,14 @@ export class FakeGitHubActionsCacheServer { #download( _request: http.IncomingMessage, response: http.ServerResponse, - url: URL + tarballId: string ): void { this.metrics.download++; if (this.#rateLimitNextRequest.delete('download')) { return this.#rateLimit(response); } - const tarballId = url.pathname.slice('/tarballs/'.length) as TarballId; - const id = this.#tarballIdToEntryId.get(tarballId); + const id = this.#tarballIdToEntryId.get(tarballId as TarballId); if (id === undefined) { return this.#respond(response, 404, 'Tarball does not exist'); } From 6070ceccae3f7e381f4db46df884a75633c2cd78 Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Sat, 14 May 2022 08:48:14 -0700 Subject: [PATCH 3/9] Check that commit method expected size matches --- .../util/fake-github-actions-cache-server.ts | 106 +++++++++++------- 1 file changed, 66 insertions(+), 40 deletions(-) diff --git a/src/test/util/fake-github-actions-cache-server.ts b/src/test/util/fake-github-actions-cache-server.ts index 9958bf3a4..7a91d4919 100644 --- a/src/test/util/fake-github-actions-cache-server.ts +++ b/src/test/util/fake-github-actions-cache-server.ts @@ -149,6 +149,18 @@ export class FakeGitHubActionsCacheServer { this.#rateLimitNextRequest.add(apiName); } + #readBody(request: http.IncomingMessage): Promise { + const chunks: Buffer[] = []; + request.on('data', (chunk: Buffer) => { + chunks.push(chunk); + }); + return new Promise((resolve) => { + request.on('end', () => { + resolve(Buffer.concat(chunks)); + }); + }); + } + #respond( response: http.ServerResponse, status: number, @@ -196,16 +208,16 @@ export class FakeGitHubActionsCacheServer { } if (api === '_apis/artifactcache/caches' && request.method === 'POST') { - return this.#reserve(request, response); + return void this.#reserve(request, response); } if (api.startsWith('_apis/artifactcache/caches/')) { const tail = api.slice('_apis/artifactcache/caches/'.length); if (request.method === 'PATCH') { - return this.#upload(request, response, tail); + return void this.#upload(request, response, tail); } if (request.method === 'POST') { - return this.#commit(request, response, tail); + return void this.#commit(request, response, tail); } } @@ -286,7 +298,10 @@ export class FakeGitHubActionsCacheServer { * key + version. If so, returns a "409 Conflict" response. If not, returns a * new unique cache ID which can be used to upload the tarball. */ - #reserve(request: http.IncomingMessage, response: http.ServerResponse): void { + async #reserve( + request: http.IncomingMessage, + response: http.ServerResponse + ): Promise { this.metrics.reserve++; if (this.#rateLimitNextRequest.delete('reserve')) { return this.#rateLimit(response); @@ -295,32 +310,29 @@ export class FakeGitHubActionsCacheServer { return; } - let jsonStr = ''; - request.on('data', (chunk) => { - jsonStr += chunk; - }); - - request.on('end', () => { - const json = JSON.parse(jsonStr) as {key: string; version: string}; - const keyAndVersion = encodeKeyAndVersion(json.key, json.version); - if (this.#keyAndVersionToEntryId.has(keyAndVersion)) { - return this.#respond(response, /* Conflict */ 409); - } - const entryId = this.#nextEntryId++ as EntryId; - const tarballId = String(Math.random()).slice(2) as TarballId; - this.#keyAndVersionToEntryId.set(keyAndVersion, entryId); - this.#tarballIdToEntryId.set(tarballId, entryId); - this.#entryIdToEntry.set(entryId, { - chunks: [], - commited: false, - tarballId, - }); - this.#respond( - response, - /* Created */ 201, - JSON.stringify({cacheId: entryId}) - ); + const json = await this.#readBody(request); + const data = JSON.parse(json.toString()) as { + key: string; + version: string; + }; + const keyAndVersion = encodeKeyAndVersion(data.key, data.version); + if (this.#keyAndVersionToEntryId.has(keyAndVersion)) { + return this.#respond(response, /* Conflict */ 409); + } + const entryId = this.#nextEntryId++ as EntryId; + const tarballId = String(Math.random()).slice(2) as TarballId; + this.#keyAndVersionToEntryId.set(keyAndVersion, entryId); + this.#tarballIdToEntryId.set(tarballId, entryId); + this.#entryIdToEntry.set(entryId, { + chunks: [], + commited: false, + tarballId, }); + this.#respond( + response, + /* Created */ 201, + JSON.stringify({cacheId: entryId}) + ); } /** @@ -329,11 +341,11 @@ export class FakeGitHubActionsCacheServer { * This API receives a tarball (or a chunk of a tarball) and stores it using * the given key (as returned by the reserve cache API). */ - #upload( + async #upload( request: http.IncomingMessage, response: http.ServerResponse, idStr: string - ): void { + ): Promise { this.metrics.upload++; if (this.#rateLimitNextRequest.delete('upload')) { return this.#rateLimit(response); @@ -365,13 +377,9 @@ export class FakeGitHubActionsCacheServer { ); } - request.on('data', (chunk: unknown) => { - entry.chunks.push(chunk as Buffer); - }); - - request.on('end', () => { - this.#respond(response, /* No Content */ 204); - }); + const buffer = await this.#readBody(request); + entry.chunks.push(buffer); + this.#respond(response, /* No Content */ 204); } /** @@ -380,11 +388,11 @@ export class FakeGitHubActionsCacheServer { * This API marks a tarball uploaded by the onSaveCache API (which could be * sent in multiple chunks) as complete. */ - #commit( + async #commit( request: http.IncomingMessage, response: http.ServerResponse, idStr: string - ): void { + ): Promise { this.metrics.commit++; if (this.#rateLimitNextRequest.delete('commit')) { return this.#rateLimit(response); @@ -396,12 +404,30 @@ export class FakeGitHubActionsCacheServer { if (idStr.match(/\d+/) === null) { return this.#respond(response, 400, 'Cache ID was not an integer'); } + const id = Number(idStr) as EntryId; const entry = this.#entryIdToEntry.get(id); if (entry === undefined) { return this.#respond(response, 400, 'Cache entry did not exist'); } + const json = await this.#readBody(request); + const data = JSON.parse(json.toString()) as { + size: number; + }; + const expectedSize = data.size; + const actualSize = entry.chunks.reduce( + (sum, chunk) => sum + chunk.length, + 0 + ); + if (actualSize !== expectedSize) { + return this.#respond( + response, + 400, + 'Cache entry did not have expected size' + ); + } + entry.commited = true; this.#respond(response, /* No Content */ 204); } From 046510bd0ed4502ef8bdb6b700cebe1b659a9b04 Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Sat, 14 May 2022 10:32:13 -0700 Subject: [PATCH 4/9] Add multiple chunk support to fake and test --- src/test/cache-github.test.ts | 104 ++++++++++++++++++ .../util/fake-github-actions-cache-server.ts | 74 +++++++++---- 2 files changed, 154 insertions(+), 24 deletions(-) diff --git a/src/test/cache-github.test.ts b/src/test/cache-github.test.ts index 473a8a41f..884b5ebb1 100644 --- a/src/test/cache-github.test.ts +++ b/src/test/cache-github.test.ts @@ -6,6 +6,7 @@ import * as pathlib from 'path'; import * as assert from 'uvu/assert'; +import * as crypto from 'crypto'; import {suite} from 'uvu'; import {WireitTestRig} from './util/test-rig.js'; import {registerCommonCacheTests} from './cache-common.js'; @@ -279,4 +280,107 @@ test( }) ); +test( + 'uploads large tarball in multiple chunks', + timeout(async ({rig, server}) => { + const cmdA = await rig.newCommand(); + + await rig.write({ + 'package.json': { + scripts: { + a: 'wireit', + }, + wireit: { + a: { + command: cmdA.command, + files: ['input'], + output: ['output'], + }, + }, + }, + }); + + // Generate a random file which is big enough to exceed the maximum chunk + // size, so that it gets split into 2 separate upload requests. + // + // The maximum chunk size is defined here: + // https://github.com/actions/toolkit/blob/500d0b42fee2552ae9eeb5933091fe2fbf14e72d/packages/cache/src/options.ts#L59 + // + // This needs to be actually random data, not just arbitrary, because the + // tarball will be compressed, and we need a poor compression ratio in order + // to hit our target size. + const MB = 1024 * 1024; + const maxChunkBytes = 32 * MB; + const compressionHeadroomBytes = 8 * MB; // Found experimentally. + const totalBytes = maxChunkBytes + compressionHeadroomBytes; + const fileContent = crypto.randomBytes(totalBytes).toString(); + + // On the initial run a large file is created and should be cached. + { + await rig.write('input', 'v0'); + server.resetMetrics(); + + const exec = rig.exec('npm run a'); + const inv = await cmdA.nextInvocation(); + await rig.write('output', fileContent); + inv.exit(0); + + // Note here is when we are creating the compressed tarball, which is the + // slowest part of this test. + + assert.equal((await exec.exit).code, 0); + assert.equal(cmdA.numInvocations, 1); + assert.equal(server.metrics, { + check: 1, + reserve: 1, + // Since we had a file that was larger than the maximum chunk size, we + // should have 2 upload requests. + upload: 2, + commit: 1, + download: 0, + }); + } + + // Invalidate cache by changing input. + { + await rig.write('input', 'v1'); + server.resetMetrics(); + + const exec = rig.exec('npm run a'); + const inv = await cmdA.nextInvocation(); + assert.not(await rig.exists('output')); + inv.exit(0); + + assert.equal((await exec.exit).code, 0); + assert.equal(cmdA.numInvocations, 2); + assert.equal(server.metrics, { + check: 1, + reserve: 1, + upload: 1, + commit: 1, + download: 0, + }); + } + + // Change input back to v0. The large file should be restored from cache. + { + await rig.write('input', 'v0'); + server.resetMetrics(); + + const exec = rig.exec('npm run a'); + + assert.equal((await exec.exit).code, 0); + assert.equal(cmdA.numInvocations, 2); + assert.equal(server.metrics, { + check: 1, + reserve: 0, + upload: 0, + commit: 0, + download: 1, + }); + assert.equal(await rig.read('output'), fileContent); + } + }) +); + test.run(); diff --git a/src/test/util/fake-github-actions-cache-server.ts b/src/test/util/fake-github-actions-cache-server.ts index 7a91d4919..284572ce2 100644 --- a/src/test/util/fake-github-actions-cache-server.ts +++ b/src/test/util/fake-github-actions-cache-server.ts @@ -29,11 +29,17 @@ type KeyAndVersion = string & { }; interface CacheEntry { - chunks: Buffer[]; + chunks: ChunkRange[]; commited: boolean; tarballId: TarballId; } +interface ChunkRange { + start: number; + end: number; + buffer: Buffer; +} + const encodeKeyAndVersion = (key: string, version: string): KeyAndVersion => `${key}:${version}` as KeyAndVersion; @@ -338,8 +344,8 @@ export class FakeGitHubActionsCacheServer { /** * Handle the PATCH:/_apis/artifactcache/caches/ API. * - * This API receives a tarball (or a chunk of a tarball) and stores it using - * the given key (as returned by the reserve cache API). + * This API receives a chunk of a tarball defined by the Content-Range header, + * and stores it using the given key (as returned by the reserve cache API). */ async #upload( request: http.IncomingMessage, @@ -363,30 +369,36 @@ export class FakeGitHubActionsCacheServer { return this.#respond(response, 400, 'Cache entry did not exist'); } - if (entry.chunks.length > 0) { - // The real server supports multiple requests uploading different ranges - // of the same tarball distinguished using the Content-Range header, for - // large tarballs. However, our tests don't test this functionality, so we - // don't bother implementing it. - // - // TODO(aomarks) We probably should actually try to cover this case. + const contentRange = request.headers['content-range'] ?? ''; + const parsedContentRange = contentRange.match(/^bytes (\d+)-(\d+)\/\*$/); + if (parsedContentRange === null) { return this.#respond( response, - 501, - 'Multiple tarball upload requests not supported' + 400, + 'Missing or invalid Content-Range header' ); } + const start = Number(parsedContentRange[1]); + const end = Number(parsedContentRange[2]); + const expectedLength = end - start + 1; const buffer = await this.#readBody(request); - entry.chunks.push(buffer); + if (buffer.length !== expectedLength) { + return this.#respond( + response, + 400, + 'Chunk length did not match Content-Range header' + ); + } + entry.chunks.push({start, end, buffer}); this.#respond(response, /* No Content */ 204); } /** * Handle the POST:/_apis/artifactcache/caches/ API. * - * This API marks a tarball uploaded by the onSaveCache API (which could be - * sent in multiple chunks) as complete. + * This API marks an uploaded tarball (which can be sent in multiple chunks) + * as complete. */ async #commit( request: http.IncomingMessage, @@ -411,20 +423,34 @@ export class FakeGitHubActionsCacheServer { return this.#respond(response, 400, 'Cache entry did not exist'); } + // Sort the chunks according to range and validate that there are no missing + // or overlapping chunks. + entry.chunks.sort((a, b) => a.start - b.start); + let expectedNextStart = 0; + let totalLength = 0; + for (const chunk of entry.chunks) { + if (chunk.start !== expectedNextStart) { + return this.#respond( + response, + 400, + 'Cache entry chunks were not contiguous' + ); + } + expectedNextStart = chunk.end + 1; + totalLength += chunk.buffer.length; + } + + // Validate against the expected total length from this request. const json = await this.#readBody(request); const data = JSON.parse(json.toString()) as { size: number; }; - const expectedSize = data.size; - const actualSize = entry.chunks.reduce( - (sum, chunk) => sum + chunk.length, - 0 - ); - if (actualSize !== expectedSize) { + const expectedLength = data.size; + if (totalLength !== expectedLength) { return this.#respond( response, 400, - 'Cache entry did not have expected size' + 'Cache entry did not match expected length' ); } @@ -467,12 +493,12 @@ export class FakeGitHubActionsCacheServer { response.statusCode = 200; const contentLength = entry.chunks.reduce( - (acc, chunk) => acc + chunk.length, + (sum, chunk) => sum + chunk.buffer.length, 0 ); response.setHeader('Content-Length', contentLength); for (const chunk of entry.chunks) { - response.write(chunk); + response.write(chunk.buffer); } response.end(); } From 9164f75b58a230cb55fe16ddb49d1eb879d40ef5 Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Sat, 14 May 2022 11:00:11 -0700 Subject: [PATCH 5/9] Run fake GitHub Cache server on HTTPS with a self-signed cert --- package-lock.json | 37 +++++++++++++++++++ package.json | 1 + src/test/cache-github.test.ts | 28 +++++++++++++- .../util/fake-github-actions-cache-server.ts | 9 +++-- src/types/selfsigned.d.ts | 16 ++++++++ 5 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 src/types/selfsigned.d.ts diff --git a/package-lock.json b/package-lock.json index 94154fbcc..5f247d6a9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -36,6 +36,7 @@ "jsonschema": "^1.4.0", "pnpm": "^7.0.0", "prettier": "^2.6.2", + "selfsigned": "^2.0.1", "typescript": "^4.6.3", "uvu": "^0.5.3", "wireit": "^0.4.1", @@ -5393,6 +5394,15 @@ } } }, + "node_modules/node-forge": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/node-forge/-/node-forge-1.3.1.tgz", + "integrity": "sha512-dPEtOeMvF9VMcYV/1Wb8CPoVAXtp6MKMlcbAt4ddqmGqUJ6fQZFXkNZNkNlfevtNkGtaSoXf/vNNNSvgrdXwtA==", + "dev": true, + "engines": { + "node": ">= 6.13.0" + } + }, "node_modules/nopt": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/nopt/-/nopt-5.0.0.tgz", @@ -6480,6 +6490,18 @@ "node": ">=4" } }, + "node_modules/selfsigned": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/selfsigned/-/selfsigned-2.0.1.tgz", + "integrity": "sha512-LmME957M1zOsUhG+67rAjKfiWFox3SBxE/yymatMZsAx+oMrJ0YQ8AToOnyCm7xbeg2ep37IHLxdu0o2MavQOQ==", + "dev": true, + "dependencies": { + "node-forge": "^1" + }, + "engines": { + "node": ">=10" + } + }, "node_modules/semver": { "version": "6.3.0", "resolved": "https://registry.npmjs.org/semver/-/semver-6.3.0.tgz", @@ -12047,6 +12069,12 @@ "whatwg-url": "^5.0.0" } }, + "node-forge": { + "version": "1.3.1", + "resolved": "https://registry.npmjs.org/node-forge/-/node-forge-1.3.1.tgz", + "integrity": "sha512-dPEtOeMvF9VMcYV/1Wb8CPoVAXtp6MKMlcbAt4ddqmGqUJ6fQZFXkNZNkNlfevtNkGtaSoXf/vNNNSvgrdXwtA==", + "dev": true + }, "nopt": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/nopt/-/nopt-5.0.0.tgz", @@ -12882,6 +12910,15 @@ "kind-of": "^6.0.0" } }, + "selfsigned": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/selfsigned/-/selfsigned-2.0.1.tgz", + "integrity": "sha512-LmME957M1zOsUhG+67rAjKfiWFox3SBxE/yymatMZsAx+oMrJ0YQ8AToOnyCm7xbeg2ep37IHLxdu0o2MavQOQ==", + "dev": true, + "requires": { + "node-forge": "^1" + } + }, "semver": { "version": "6.3.0", "resolved": "https://registry.npmjs.org/semver/-/semver-6.3.0.tgz", diff --git a/package.json b/package.json index 2c2afba3d..4efd29d0c 100644 --- a/package.json +++ b/package.json @@ -282,6 +282,7 @@ "jsonschema": "^1.4.0", "pnpm": "^7.0.0", "prettier": "^2.6.2", + "selfsigned": "^2.0.1", "typescript": "^4.6.3", "uvu": "^0.5.3", "wireit": "^0.4.1", diff --git a/src/test/cache-github.test.ts b/src/test/cache-github.test.ts index 884b5ebb1..a3ca30c57 100644 --- a/src/test/cache-github.test.ts +++ b/src/test/cache-github.test.ts @@ -4,26 +4,50 @@ * SPDX-License-Identifier: Apache-2.0 */ +import * as fs from 'fs/promises'; import * as pathlib from 'path'; import * as assert from 'uvu/assert'; import * as crypto from 'crypto'; +import * as selfsigned from 'selfsigned'; import {suite} from 'uvu'; +import {fileURLToPath} from 'url'; import {WireitTestRig} from './util/test-rig.js'; import {registerCommonCacheTests} from './cache-common.js'; import {FakeGitHubActionsCacheServer} from './util/fake-github-actions-cache-server.js'; import {timeout} from './util/uvu-timeout.js'; +const __filename = fileURLToPath(import.meta.url); +const __dirname = pathlib.dirname(__filename); +const repoRoot = pathlib.resolve(__dirname, '..', '..'); + +const SELF_SIGNED_CERT = selfsigned.generate([ + {name: 'commonName', value: 'localhost'}, +]); +const SELF_SIGNED_CERT_PATH = pathlib.resolve( + repoRoot, + 'temp', + 'self-signed.cert' +); + const test = suite<{ rig: WireitTestRig; server: FakeGitHubActionsCacheServer; }>(); +test.before(async () => { + await fs.mkdir(pathlib.dirname(SELF_SIGNED_CERT_PATH), {recursive: true}); + await fs.writeFile(SELF_SIGNED_CERT_PATH, SELF_SIGNED_CERT.cert); +}); + test.before.each(async (ctx) => { try { // Set up the cache service for each test (as opposed to for the whole // suite) because we want fresh cache state for each test. const authToken = String(Math.random()).slice(2); - ctx.server = new FakeGitHubActionsCacheServer(authToken); + ctx.server = new FakeGitHubActionsCacheServer(authToken, { + cert: SELF_SIGNED_CERT.cert, + key: SELF_SIGNED_CERT.private, + }); const actionsCacheUrl = await ctx.server.listen(); ctx.rig = new WireitTestRig(); ctx.rig.env = { @@ -31,6 +55,8 @@ test.before.each(async (ctx) => { ACTIONS_CACHE_URL: actionsCacheUrl, ACTIONS_RUNTIME_TOKEN: authToken, RUNNER_TEMP: pathlib.join(ctx.rig.temp, 'github-cache-temp'), + // Tell Node to trust our self-signed certificate for HTTPS. + NODE_EXTRA_CA_CERTS: SELF_SIGNED_CERT_PATH, }; await ctx.rig.setup(); } catch (error) { diff --git a/src/test/util/fake-github-actions-cache-server.ts b/src/test/util/fake-github-actions-cache-server.ts index 284572ce2..b51bd52c5 100644 --- a/src/test/util/fake-github-actions-cache-server.ts +++ b/src/test/util/fake-github-actions-cache-server.ts @@ -4,7 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import * as http from 'http'; +import * as https from 'https'; +import type * as http from 'http'; /** * Numeric ID for a cache entry. @@ -107,9 +108,9 @@ export class FakeGitHubActionsCacheServer { readonly #keyAndVersionToEntryId = new Map(); readonly #tarballIdToEntryId = new Map(); - constructor(authToken: string) { + constructor(authToken: string, tlsCert: {cert: string; key: string}) { this.#authToken = authToken; - this.#server = http.createServer(this.#route); + this.#server = https.createServer(tlsCert, this.#route); this.resetMetrics(); } @@ -139,7 +140,7 @@ export class FakeGitHubActionsCacheServer { // include this in the fake because it ensures the client is preserving the // base path and not just using the origin. const randomBasePath = Math.random().toString().slice(2); - this.#url = new URL(`http://${host}:${address.port}/${randomBasePath}/`); + this.#url = new URL(`https://${host}:${address.port}/${randomBasePath}/`); return this.#url.href; } diff --git a/src/types/selfsigned.d.ts b/src/types/selfsigned.d.ts new file mode 100644 index 000000000..eb4a03e20 --- /dev/null +++ b/src/types/selfsigned.d.ts @@ -0,0 +1,16 @@ +/** + * @license + * Copyright 2022 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +// Typings for https://github.com/jfromaniello/selfsigned which are not +// available in DefinitelyTyped. + +declare module 'selfsigned' { + export function generate(attrs: Array<{name: string; value: string}>): { + cert: string; + public: string; + private: string; + }; +} From bf4a09fe456a69c678e588864a7b82a27e812075 Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Sat, 14 May 2022 12:29:02 -0700 Subject: [PATCH 6/9] Raise minimum timeout for long large file test --- src/test/cache-github.test.ts | 4 ++-- src/test/util/uvu-timeout.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/cache-github.test.ts b/src/test/cache-github.test.ts index a3ca30c57..51896629f 100644 --- a/src/test/cache-github.test.ts +++ b/src/test/cache-github.test.ts @@ -14,7 +14,7 @@ import {fileURLToPath} from 'url'; import {WireitTestRig} from './util/test-rig.js'; import {registerCommonCacheTests} from './cache-common.js'; import {FakeGitHubActionsCacheServer} from './util/fake-github-actions-cache-server.js'; -import {timeout} from './util/uvu-timeout.js'; +import {timeout, DEFAULT_UVU_TIMEOUT} from './util/uvu-timeout.js'; const __filename = fileURLToPath(import.meta.url); const __dirname = pathlib.dirname(__filename); @@ -406,7 +406,7 @@ test( }); assert.equal(await rig.read('output'), fileContent); } - }) + }, Math.max(DEFAULT_UVU_TIMEOUT, 15_000)) ); test.run(); diff --git a/src/test/util/uvu-timeout.ts b/src/test/util/uvu-timeout.ts index c72c02a01..f5ab2364c 100644 --- a/src/test/util/uvu-timeout.ts +++ b/src/test/util/uvu-timeout.ts @@ -6,7 +6,7 @@ import type * as uvu from 'uvu'; -const DEFAULT_TIMEOUT = Number(process.env.TEST_TIMEOUT ?? 60_000); +export const DEFAULT_UVU_TIMEOUT = Number(process.env.TEST_TIMEOUT ?? 60_000); /** * Returns a promise that resolves after the given period of time. @@ -24,7 +24,7 @@ export const wait = async (ms: number) => */ export const timeout = ( handler: uvu.Callback, - ms = DEFAULT_TIMEOUT + ms = DEFAULT_UVU_TIMEOUT ): uvu.Callback => { return (...args) => { let timerId: ReturnType; From c9b17068e24132868c3f43fe632c5aa8faecebf1 Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Sat, 14 May 2022 13:07:07 -0700 Subject: [PATCH 7/9] Check content-type, accept, user-agent headers --- .../util/fake-github-actions-cache-server.ts | 102 +++++++++++++++++- 1 file changed, 101 insertions(+), 1 deletion(-) diff --git a/src/test/util/fake-github-actions-cache-server.ts b/src/test/util/fake-github-actions-cache-server.ts index b51bd52c5..01ad1a672 100644 --- a/src/test/util/fake-github-actions-cache-server.ts +++ b/src/test/util/fake-github-actions-cache-server.ts @@ -46,6 +46,9 @@ const encodeKeyAndVersion = (key: string, version: string): KeyAndVersion => type ApiName = 'check' | 'reserve' | 'upload' | 'commit' | 'download'; +// https://github.com/actions/toolkit/blob/500d0b42fee2552ae9eeb5933091fe2fbf14e72d/packages/cache/src/internal/cacheHttpClient.ts#L55 +const JSON_RESPONSE_TYPE = 'application/json;api-version=6.0-preview.1'; + /** * A fake version of the GitHub Actions cache server. * @@ -195,6 +198,67 @@ export class FakeGitHubActionsCacheServer { return true; } + #checkUserAgent( + request: http.IncomingMessage, + response: http.ServerResponse + ): boolean { + // https://github.com/actions/toolkit/blob/500d0b42fee2552ae9eeb5933091fe2fbf14e72d/packages/cache/src/internal/cacheHttpClient.ts#L67 + const expected = 'actions/cache'; + const actual = request.headers['user-agent']; + if (actual !== expected) { + // The real server might not be this strict, but we want to be sure we're + // acting just like the official client library. + this.#respond( + response, + /* Bad Request */ 400, + `Expected user-agent ${JSON.stringify(expected)}. ` + + `Got ${JSON.stringify(actual)}.` + ); + return false; + } + return true; + } + + #checkContentType( + request: http.IncomingMessage, + response: http.ServerResponse, + expected: string | undefined + ): boolean { + const actual = request.headers['content-type']; + if (actual !== expected) { + // The real server might not be this strict, but we want to be sure we're + // acting just like the official client library. + this.#respond( + response, + /* Bad Request */ 400, + `Expected content-type ${JSON.stringify(expected)}. ` + + `Got ${JSON.stringify(actual)}.` + ); + return false; + } + return true; + } + + #checkAccept( + request: http.IncomingMessage, + response: http.ServerResponse, + expected: string | undefined + ): boolean { + const actual = request.headers['accept']; + if (actual !== expected) { + // The real server might not be this strict, but we want to be sure we're + // acting just like the official client library. + this.#respond( + response, + /* Bad Request */ 400, + `Expected accept ${JSON.stringify(expected)}. ` + + `Got ${JSON.stringify(actual)}.` + ); + return false; + } + return true; + } + #route = ( request: http.IncomingMessage, response: http.ServerResponse @@ -210,6 +274,10 @@ export class FakeGitHubActionsCacheServer { } const api = url.pathname.slice(this.#url.pathname.length); + if (!this.#checkUserAgent(request, response)) { + return; + } + if (api === '_apis/artifactcache/cache' && request.method === 'GET') { return this.#check(request, response, url); } @@ -255,6 +323,12 @@ export class FakeGitHubActionsCacheServer { if (!this.#checkAuthorization(request, response)) { return; } + if (!this.#checkContentType(request, response, undefined)) { + return; + } + if (!this.#checkAccept(request, response, JSON_RESPONSE_TYPE)) { + return; + } const keys = url.searchParams.get('keys'); if (!keys) { @@ -316,6 +390,12 @@ export class FakeGitHubActionsCacheServer { if (!this.#checkAuthorization(request, response)) { return; } + if (!this.#checkContentType(request, response, 'application/json')) { + return; + } + if (!this.#checkAccept(request, response, JSON_RESPONSE_TYPE)) { + return; + } const json = await this.#readBody(request); const data = JSON.parse(json.toString()) as { @@ -360,6 +440,14 @@ export class FakeGitHubActionsCacheServer { if (!this.#checkAuthorization(request, response)) { return; } + if ( + !this.#checkContentType(request, response, 'application/octet-stream') + ) { + return; + } + if (!this.#checkAccept(request, response, JSON_RESPONSE_TYPE)) { + return; + } if (idStr.match(/\d+/) === null) { return this.#respond(response, 400, 'Cache ID was not an integer'); @@ -413,6 +501,12 @@ export class FakeGitHubActionsCacheServer { if (!this.#checkAuthorization(request, response)) { return; } + if (!this.#checkContentType(request, response, 'application/json')) { + return; + } + if (!this.#checkAccept(request, response, JSON_RESPONSE_TYPE)) { + return; + } if (idStr.match(/\d+/) === null) { return this.#respond(response, 400, 'Cache ID was not an integer'); @@ -471,7 +565,7 @@ export class FakeGitHubActionsCacheServer { * unguessable. */ #download( - _request: http.IncomingMessage, + request: http.IncomingMessage, response: http.ServerResponse, tarballId: string ): void { @@ -479,6 +573,12 @@ export class FakeGitHubActionsCacheServer { if (this.#rateLimitNextRequest.delete('download')) { return this.#rateLimit(response); } + if (!this.#checkContentType(request, response, undefined)) { + return; + } + if (!this.#checkAccept(request, response, undefined)) { + return; + } const id = this.#tarballIdToEntryId.get(tarballId as TarballId); if (id === undefined) { From a3cf6901dbac717304eb85825079796f4c0558f7 Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Sat, 14 May 2022 15:33:37 -0700 Subject: [PATCH 8/9] Check that chunks are <= 32MB --- src/test/util/fake-github-actions-cache-server.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/test/util/fake-github-actions-cache-server.ts b/src/test/util/fake-github-actions-cache-server.ts index 01ad1a672..c26029754 100644 --- a/src/test/util/fake-github-actions-cache-server.ts +++ b/src/test/util/fake-github-actions-cache-server.ts @@ -471,6 +471,13 @@ export class FakeGitHubActionsCacheServer { const end = Number(parsedContentRange[2]); const expectedLength = end - start + 1; + // The real server might not be this strict, but we should make sure we + // aren't sending larger chunks than the official client library does. + // https://github.com/actions/toolkit/blob/500d0b42fee2552ae9eeb5933091fe2fbf14e72d/packages/cache/src/options.ts#L59 + if (expectedLength > 32 * 1024 * 1024) { + return this.#respond(response, 400, 'Upload chunk was > 32MB'); + } + const buffer = await this.#readBody(request); if (buffer.length !== expectedLength) { return this.#respond( From 3d60f2b82fc1a85c28edce56dd6ada2127b63e90 Mon Sep 17 00:00:00 2001 From: Alexander Marks Date: Sat, 14 May 2022 15:53:15 -0700 Subject: [PATCH 9/9] Check transfer-encoding header --- src/test/util/fake-github-actions-cache-server.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/test/util/fake-github-actions-cache-server.ts b/src/test/util/fake-github-actions-cache-server.ts index c26029754..197588ac7 100644 --- a/src/test/util/fake-github-actions-cache-server.ts +++ b/src/test/util/fake-github-actions-cache-server.ts @@ -448,6 +448,17 @@ export class FakeGitHubActionsCacheServer { if (!this.#checkAccept(request, response, JSON_RESPONSE_TYPE)) { return; } + const expectedTransferEncoding = 'chunked'; + const actualTransferEncoding = request.headers['transfer-encoding']; + if (actualTransferEncoding !== 'chunked') { + return this.#respond( + response, + /* Bad Request */ 400, + `Expected transfer-encoding ${JSON.stringify( + expectedTransferEncoding + )}. ` + `Got ${String(actualTransferEncoding)}.` + ); + } if (idStr.match(/\d+/) === null) { return this.#respond(response, 400, 'Cache ID was not an integer');