From 8c32ebe976fb93f0224d3bd2e4e6685618af7ef5 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Wed, 14 Oct 2026 12:02:00 +0100 Subject: [PATCH] Add `InstanceApiClient` and incorporate `getAuthorizationHeaderFor` into it --- lib/entry-points.js | 56 ++++++++++++++++--------- src/api-client.test.ts | 87 ++++++++++++++------------------------ src/api-client.ts | 91 ++++++++++++++++++++++++---------------- src/setup-codeql.test.ts | 13 ++++-- src/setup-codeql.ts | 7 +--- src/start-proxy.test.ts | 25 ++++++++--- src/start-proxy.ts | 8 ++-- 7 files changed, 157 insertions(+), 130 deletions(-) diff --git a/lib/entry-points.js b/lib/entry-points.js index c5e917b98..3bef1190c 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -170732,6 +170732,37 @@ function makeProxyRequestOptions(dispatcher) { } }; } +var InstanceApiClient = class { + client; + apiDetails; + authorizationHeader; + constructor(apiDetails, clientOptions = {}) { + this.apiDetails = apiDetails; + this.client = createApiClientWithDetails(apiDetails, clientOptions); + const token = this.apiDetails.auth?.trim(); + if (token !== void 0 && token !== "") { + this.authorizationHeader = `token ${token}`; + } + } + /** + * Gets a value for the `Authorization` header for a request to `url`; or `undefined` if the + * `Authorization` header should not be set for `url`. + * + * @param logger The logger to use for debugging messages. + * @param url The URL for which we want to add an `Authorization` header. + */ + getAuthorizationHeaderFor(logger2, url2) { + if (this.authorizationHeader === void 0) { + return void 0; + } + if (url2.startsWith(`${this.apiDetails.url}/`) || this.apiDetails.apiURL && url2.startsWith(`${this.apiDetails.apiURL}/`)) { + logger2.debug(`Providing an authorization token.`); + return this.authorizationHeader; + } + logger2.debug(`Not using an authorization token.`); + return void 0; + } +}; function createApiClientWithDetails(apiDetails, { allowExternal = false, proxy = void 0 } = {}) { const auth2 = allowExternal && apiDetails.externalRepoAuth !== void 0 ? apiDetails.externalRepoAuth : apiDetails.auth; const retryingOctokit = githubUtils.GitHub.plugin(retry); @@ -170775,18 +170806,6 @@ function getApiClient(env = getEnv(), apiDetails) { function getApiClientWithExternalAuth(apiDetails, proxy) { return createApiClientWithDetails(apiDetails, { allowExternal: true, proxy }); } -function getAuthorizationHeaderFor(logger2, apiDetails, url2) { - const token = apiDetails.auth?.trim(); - if (token === void 0 || token === "") { - return void 0; - } - if (url2.startsWith(`${apiDetails.url}/`) || apiDetails.apiURL && url2.startsWith(`${apiDetails.apiURL}/`)) { - logger2.debug(`Providing an authorization token.`); - return `token ${token}`; - } - logger2.debug(`Not using an authorization token.`); - return void 0; -} var cachedGitHubVersion = void 0; async function getGitHubVersionFromApi(apiClient, apiDetails) { if (parseGitHubUrl(apiDetails.url) === GITHUB_DOTCOM_URL) { @@ -176668,11 +176687,8 @@ var downloadCodeQL = async function(source, tarVersion, tempDir, logger2) { if (searchParams.has("token")) { logger2.debug("CodeQL tools URL contains an authorization token."); } else { - authorization = getAuthorizationHeaderFor( - logger2, - source.apiDetails, - codeqlURL - ); + const client = new InstanceApiClient(source.apiDetails); + authorization = client.getAuthorizationHeaderFor(logger2, codeqlURL); } const toolcacheDestination = getToolcacheDestination({ logger: logger2 }, source); const extractedBundlePath = toolcacheDestination.orElse( @@ -187879,14 +187895,14 @@ function getProxyFilename() { } async function getProxyBinaryPath(action) { const logger2 = action.logger; + const apiDetails = getApiDetails(); + const client = new InstanceApiClient(apiDetails); const proxyFileName = getProxyFilename(); const proxyInfo = await getDownloadUrl(action); let proxyBin = toolcache4.find(proxyFileName, proxyInfo.version); if (!proxyBin) { - const apiDetails = getApiDetails(); - const authorization = getAuthorizationHeaderFor( + const authorization = client.getAuthorizationHeaderFor( logger2, - apiDetails, proxyInfo.url ); const temp = await downloadProxy(logger2, proxyInfo.url, authorization); diff --git a/src/api-client.test.ts b/src/api-client.test.ts index 8ec5a9501..e2b37cefd 100644 --- a/src/api-client.test.ts +++ b/src/api-client.test.ts @@ -309,70 +309,45 @@ test.serial( test("getAuthorizationHeaderFor - returns undefined if there's no token", (t) => { const logger = new RecordingLogger(); - t.is( - api.getAuthorizationHeaderFor( - logger, - { ...SAMPLE_DOTCOM_API_DETAILS, auth: undefined }, - `${GITHUB_DOTCOM_URL}/github`, - ), - undefined, - ); - t.is( - api.getAuthorizationHeaderFor( - logger, - { ...SAMPLE_DOTCOM_API_DETAILS, auth: "" }, - `${GITHUB_DOTCOM_URL}/github`, - ), - undefined, - ); - t.is( - api.getAuthorizationHeaderFor( - logger, - { ...SAMPLE_DOTCOM_API_DETAILS, auth: " " }, - `${GITHUB_DOTCOM_URL}/github`, - ), - undefined, - ); + for (const authValue of [undefined, "", " "]) { + const client = new api.InstanceApiClient({ + ...SAMPLE_DOTCOM_API_DETAILS, + auth: authValue, + }); + t.is( + client.getAuthorizationHeaderFor(logger, `${GITHUB_DOTCOM_URL}/github`), + undefined, + ); + } }); test("getAuthorizationHeaderFor - returns header value with token if url is allowed", (t) => { const logger = new RecordingLogger(); + const client = new api.InstanceApiClient({ + ...SAMPLE_DOTCOM_API_DETAILS, + }); - t.is( - api.getAuthorizationHeaderFor( - logger, - SAMPLE_DOTCOM_API_DETAILS, - `${GITHUB_DOTCOM_URL}/github`, - ), - `token ${SAMPLE_DOTCOM_API_DETAILS.auth}`, - ); - t.is( - api.getAuthorizationHeaderFor( - logger, - SAMPLE_DOTCOM_API_DETAILS, - `${GITHUB_DOTCOM_API_URL}/github`, - ), - `token ${SAMPLE_DOTCOM_API_DETAILS.auth}`, - ); + for (const url of [ + `${GITHUB_DOTCOM_URL}/github`, + `${GITHUB_DOTCOM_API_URL}/github`, + ]) { + t.is( + client.getAuthorizationHeaderFor(logger, url), + `token ${SAMPLE_DOTCOM_API_DETAILS.auth}`, + ); + } }); test("getAuthorizationHeaderFor - returns undefined if url is not allowed", (t) => { const logger = new RecordingLogger(); + const client = new api.InstanceApiClient({ + ...SAMPLE_DOTCOM_API_DETAILS, + }); - t.is( - api.getAuthorizationHeaderFor( - logger, - SAMPLE_DOTCOM_API_DETAILS, - `https://localhost/github`, - ), - undefined, - ); - t.is( - api.getAuthorizationHeaderFor( - logger, - SAMPLE_DOTCOM_API_DETAILS, - `https://api.localhost/github`, - ), - undefined, - ); + for (const url of [ + `https://localhost/github`, + `https://api.localhost/github`, + ]) { + t.is(client.getAuthorizationHeaderFor(logger, url), undefined); + } }); diff --git a/src/api-client.ts b/src/api-client.ts index bd9ac339c..2a6d2eb13 100644 --- a/src/api-client.ts +++ b/src/api-client.ts @@ -122,6 +122,61 @@ export function makeProxyRequestOptions( /** The type of GitHub API client we use. */ export type ApiClient = InstanceType; +/** + * Encapsulates a {@link GitHubApiCombinedDetails} along with operations which ensure that any credentials + * are only used with the GitHub instance that is represented by the {@link GitHubApiCombinedDetails} value. + */ +export class InstanceApiClient { + private readonly client: ApiClient; + private readonly apiDetails: GitHubApiCombinedDetails; + private readonly authorizationHeader: string | undefined; + + constructor( + apiDetails: GitHubApiCombinedDetails, + clientOptions: CreateApiClientOptions = {}, + ) { + this.apiDetails = apiDetails; + this.client = createApiClientWithDetails(apiDetails, clientOptions); + + // Compute the Authorization header value if we have a token. + const token = this.apiDetails.auth?.trim(); + if (token !== undefined && token !== "") { + this.authorizationHeader = `token ${token}`; + } + } + + /** + * Gets a value for the `Authorization` header for a request to `url`; or `undefined` if the + * `Authorization` header should not be set for `url`. + * + * @param logger The logger to use for debugging messages. + * @param url The URL for which we want to add an `Authorization` header. + */ + public getAuthorizationHeaderFor( + logger: Logger, + url: string, + ): string | undefined { + // We don't have to check the `url` if we don't have a token to begin with. + if (this.authorizationHeader === undefined) { + return undefined; + } + + // We only want to provide an authorization header if we are downloading + // from the same GitHub instance the Action is running on. + // This avoids leaking Enterprise tokens to dotcom. + if ( + url.startsWith(`${this.apiDetails.url}/`) || + (this.apiDetails.apiURL && url.startsWith(`${this.apiDetails.apiURL}/`)) + ) { + logger.debug(`Providing an authorization token.`); + return this.authorizationHeader; + } + + logger.debug(`Not using an authorization token.`); + return undefined; + } +} + /** Options for `createApiClientWithDetails`. */ interface CreateApiClientOptions { allowExternal?: boolean; @@ -207,42 +262,6 @@ export function getApiClientWithExternalAuth( return createApiClientWithDetails(apiDetails, { allowExternal: true, proxy }); } -/** - * Gets a value for the `Authorization` header for a request to `url`; or `undefined` if the - * `Authorization` header should not be set for `url`. - * - * @param logger The logger to use for debugging messages. - * @param apiDetails Details of the GitHub API we are using. - * @param url The URL for which we want to add an `Authorization` header. - * - * @returns The value for the `Authorization` header or `undefined` if it shouldn't be populated. - */ -export function getAuthorizationHeaderFor( - logger: Logger, - apiDetails: GitHubApiDetails, - url: string, -): string | undefined { - // We don't have to check the `url` if we don't have a token to begin with. - const token = apiDetails.auth?.trim(); - if (token === undefined || token === "") { - return undefined; - } - - // We only want to provide an authorization header if we are downloading - // from the same GitHub instance the Action is running on. - // This avoids leaking Enterprise tokens to dotcom. - if ( - url.startsWith(`${apiDetails.url}/`) || - (apiDetails.apiURL && url.startsWith(`${apiDetails.apiURL}/`)) - ) { - logger.debug(`Providing an authorization token.`); - return `token ${token}`; - } - - logger.debug(`Not using an authorization token.`); - return undefined; -} - let cachedGitHubVersion: GitHubVersion | undefined = undefined; export async function getGitHubVersionFromApi( diff --git a/src/setup-codeql.test.ts b/src/setup-codeql.test.ts index d8c6f32d6..875cbcbf1 100644 --- a/src/setup-codeql.test.ts +++ b/src/setup-codeql.test.ts @@ -1290,7 +1290,14 @@ for (const fallback of [false, true]) { request: { fetch: fetchRelease }, }); sinon.stub(api, "getApiClient").value(() => client); - const authorizationSpy = sinon.spy(api, "getAuthorizationHeaderFor"); + + const spyClient = new api.InstanceApiClient(SAMPLE_DOTCOM_API_DETAILS); + const authorizationSpy = sinon.spy( + spyClient, + "getAuthorizationHeaderFor", + ); + sinon.stub(api, "InstanceApiClient").returns(spyClient); + const extractStub = stubDownloadAndExtract(); if (fallback) { extractStub.onFirstCall().rejects(new HTTPError("Not Found", 404)); @@ -1323,9 +1330,9 @@ for (const fallback of [false, true]) { t.is(extractStub.firstCall.args[0], assetURL); t.is(extractStub.lastCall.args[0], fallback ? combinedURL : assetURL); t.is(authorizationSpy.callCount, extractStub.callCount); - t.is(authorizationSpy.firstCall.args[2], assetURL); + t.is(authorizationSpy.firstCall.args[1], assetURL); t.is( - authorizationSpy.lastCall.args[2], + authorizationSpy.lastCall.args[1], fallback ? combinedURL : assetURL, ); t.is(extractStub.lastCall.args[3], "token token"); diff --git a/src/setup-codeql.ts b/src/setup-codeql.ts index 6668ef1e7..5186405fc 100644 --- a/src/setup-codeql.ts +++ b/src/setup-codeql.ts @@ -767,11 +767,8 @@ export const downloadCodeQL = async function ( if (searchParams.has("token")) { logger.debug("CodeQL tools URL contains an authorization token."); } else { - authorization = api.getAuthorizationHeaderFor( - logger, - source.apiDetails, - codeqlURL, - ); + const client = new api.InstanceApiClient(source.apiDetails); + authorization = client.getAuthorizationHeaderFor(logger, codeqlURL); } const toolcacheDestination = getToolcacheDestination({ logger }, source); diff --git a/src/start-proxy.test.ts b/src/start-proxy.test.ts index bd2489713..96cbe83a0 100644 --- a/src/start-proxy.test.ts +++ b/src/start-proxy.test.ts @@ -24,6 +24,7 @@ import { makeMacro, makeTestToken, RecordingLogger, + SAMPLE_DOTCOM_API_DETAILS, setupTests, withRecordingLoggerAsync, } from "./testing-utils"; @@ -961,6 +962,8 @@ test.serial( const logger = new RecordingLogger(); mockGetReleaseByTag(); + sinon.stub(apiClient, "getApiDetails").returns(SAMPLE_DOTCOM_API_DETAILS); + await withTmpDir(async (tempDir) => { const toolcachePath = "/path/to/proxy/dir"; sinon.stub(toolcache, "find").returns(toolcachePath); @@ -994,14 +997,19 @@ test.serial( const toolcachePath = "/path/to/proxy/dir"; const find = sinon.stub(toolcache, "find").returns(""); - const getApiDetails = sinon.stub(apiClient, "getApiDetails").returns({ + const apiDetails = { auth: "", url: "", apiURL: "", - }); + } satisfies apiClient.GitHubApiDetails; + const getApiDetails = sinon + .stub(apiClient, "getApiDetails") + .returns(apiDetails); + const client = new apiClient.InstanceApiClient(apiDetails); const getAuthorizationHeaderFor = sinon - .stub(apiClient, "getAuthorizationHeaderFor") + .stub(client, "getAuthorizationHeaderFor") .returns(undefined); + sinon.stub(apiClient, "InstanceApiClient").returns(client); const archivePath = "/path/to/archive"; const downloadTool = sinon .stub(toolcache, "downloadTool") @@ -1066,14 +1074,19 @@ test.serial( await withTmpDir(async (tempDir) => { const toolcachePath = "/path/to/proxy/dir"; const find = sinon.stub(toolcache, "find").returns(""); - const getApiDetails = sinon.stub(apiClient, "getApiDetails").returns({ + const apiDetails = { auth: "", url: "", apiURL: "", - }); + } satisfies apiClient.GitHubApiDetails; + const getApiDetails = sinon + .stub(apiClient, "getApiDetails") + .returns(apiDetails); + const client = new apiClient.InstanceApiClient(apiDetails); const getAuthorizationHeaderFor = sinon - .stub(apiClient, "getAuthorizationHeaderFor") + .stub(client, "getAuthorizationHeaderFor") .returns(undefined); + sinon.stub(apiClient, "InstanceApiClient").returns(client); const archivePath = "/path/to/archive"; const downloadTool = sinon .stub(toolcache, "downloadTool") diff --git a/src/start-proxy.ts b/src/start-proxy.ts index 6221c9940..f4ea47fd9 100644 --- a/src/start-proxy.ts +++ b/src/start-proxy.ts @@ -7,8 +7,8 @@ import { ActionState } from "./action-common"; import { getApiClient, getApiDetails, - getAuthorizationHeaderFor, getGitHubVersion, + InstanceApiClient, } from "./api-client"; import * as artifactScanner from "./artifact-scanner"; import { Config } from "./config-utils"; @@ -572,15 +572,15 @@ export async function getProxyBinaryPath( action: ActionState<["Base", "Logger", "FeatureFlags"]>, ): Promise { const logger = action.logger; + const apiDetails = getApiDetails(); + const client = new InstanceApiClient(apiDetails); const proxyFileName = getProxyFilename(); const proxyInfo = await getDownloadUrl(action); let proxyBin = toolcache.find(proxyFileName, proxyInfo.version); if (!proxyBin) { - const apiDetails = getApiDetails(); - const authorization = getAuthorizationHeaderFor( + const authorization = client.getAuthorizationHeaderFor( logger, - apiDetails, proxyInfo.url, ); const temp = await downloadProxy(logger, proxyInfo.url, authorization);