From f2547e0e8374552ee8c23aedba4752d85792cc91 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Tue, 13 Oct 2026 18:39:27 +0100 Subject: [PATCH] Use instance in `DownloadSource` and pass it on to `getApiClient` --- lib/entry-points.js | 45 +++++++++++++++++++++++----------- src/codeql.test.ts | 17 +++---------- src/setup-codeql.test.ts | 13 ++++++++++ src/setup/repository.ts | 51 +++++++++++++++++++------------------- src/start-proxy.test.ts | 53 +++++++++++++++++++++++++--------------- src/testing-utils.ts | 18 ++++++++++++++ 6 files changed, 125 insertions(+), 72 deletions(-) diff --git a/lib/entry-points.js b/lib/entry-points.js index a0e269a8e..9707f1c66 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -170632,6 +170632,7 @@ var import_undici = __toESM(require_undici()); // src/github.ts var GITHUB_DOTCOM_URL = "https://github.com"; +var GITHUB_DOTCOM_API_URL = "https://api.github.com"; function parseGitHubUrl(inputUrl) { const originalUrl = inputUrl; if (inputUrl.indexOf("://") === -1) { @@ -170666,6 +170667,12 @@ function getCurrentInstance(env) { apiURL: env.getRequired("GITHUB_API_URL" /* GITHUB_API_URL */) }; } +function getDotcomInstance() { + return { + url: GITHUB_DOTCOM_URL, + apiURL: GITHUB_DOTCOM_API_URL + }; +} // src/repository.ts function isRepositoryNwoStr(val) { @@ -170761,6 +170768,16 @@ function getApiDetails(env = getEnv()) { function getApiClient(env = getEnv(), apiDetails) { return createApiClientWithDetails(apiDetails ?? getApiDetails(env)); } +function getDotcomApiDetails(env) { + if (env.getRequired("GITHUB_SERVER_URL" /* GITHUB_SERVER_URL */) === GITHUB_DOTCOM_URL) { + return getApiDetails(env); + } else { + return { + auth: void 0, + ...getDotcomInstance() + }; + } +} function getApiClientWithExternalAuth(apiDetails, proxy) { return createApiClientWithDetails(apiDetails, { allowExternal: true, proxy }); } @@ -175781,38 +175798,38 @@ function getCodeQLActionRepository(action) { `The value of '${"GITHUB_ACTION_REPOSITORY" /* GITHUB_ACTION_REPOSITORY */}' is '${actionRepository}', which does not match the expected 'owner/repo' format.` ); } -var DEFAULT_DOWNLOAD_SOURCE = [ - GITHUB_DOTCOM_URL, - CODEQL_DEFAULT_ACTION_REPOSITORY -]; -var isSameDownloadSource = ([srcUrl, srcRepo], [otherUrl, otherRepo]) => { - return srcUrl === otherUrl && srcRepo === otherRepo; +var isSameDownloadSource = ([srcInstance, srcRepo], [otherInstance, otherRepo]) => { + return srcInstance.url === otherInstance.url && srcRepo === otherRepo; }; function getDefaultDotcomDownloadURL(tagName, assetName) { return `${GITHUB_DOTCOM_URL}/${CODEQL_DEFAULT_ACTION_REPOSITORY}/releases/download/${tagName}/${assetName}`; } async function getCodeQLAssetDownloadURL(action, apiDetails, tagName, assetName, assetKind = "CodeQL bundle") { const codeQLActionRepository = getCodeQLActionRepository(action); + const defaultDownloadSource = [ + getDotcomApiDetails(action.env), + CODEQL_DEFAULT_ACTION_REPOSITORY + ]; const potentialDownloadSources = [ // This GitHub instance, and this Action. - [apiDetails.url, codeQLActionRepository], + [apiDetails, codeQLActionRepository], // This GitHub instance, and the canonical Action. - [apiDetails.url, CODEQL_DEFAULT_ACTION_REPOSITORY], + [apiDetails, CODEQL_DEFAULT_ACTION_REPOSITORY], // GitHub.com, and the canonical Action. - DEFAULT_DOWNLOAD_SOURCE + defaultDownloadSource ]; const uniqueDownloadSources = potentialDownloadSources.filter( (source, index2, self2) => { return !self2.slice(0, index2).some((other) => isSameDownloadSource(source, other)); } ); - for (const [instanceURL, repository] of uniqueDownloadSources) { - if (isSameDownloadSource(DEFAULT_DOWNLOAD_SOURCE, [instanceURL, repository])) { + for (const [instance8, repository] of uniqueDownloadSources) { + if (isSameDownloadSource(defaultDownloadSource, [instance8, repository])) { break; } const { owner: repositoryOwner, repo: repositoryName } = parseRepositoryNwo(repository); try { - const release2 = await getApiClient().rest.repos.getReleaseByTag({ + const release2 = await getApiClient(action.env, instance8).rest.repos.getReleaseByTag({ owner: repositoryOwner, repo: repositoryName, tag: tagName @@ -175820,14 +175837,14 @@ async function getCodeQLAssetDownloadURL(action, apiDetails, tagName, assetName, for (const asset of release2.data.assets) { if (asset.name === assetName) { action.logger.info( - `Found ${assetKind} ${assetName} in ${repository} on ${instanceURL} with URL ${asset.url}.` + `Found ${assetKind} ${assetName} in ${repository} on ${instance8.apiURL} with URL ${asset.url}.` ); return asset.url; } } } catch (e) { action.logger.info( - `Looked for ${assetKind} ${assetName} in ${repository} on ${instanceURL} but got error ${e}.` + `Looked for ${assetKind} ${assetName} in ${repository} on ${instance8.apiURL} but got error ${e}.` ); } } diff --git a/src/codeql.test.ts b/src/codeql.test.ts index e70dcd99f..ff0a4d5b6 100644 --- a/src/codeql.test.ts +++ b/src/codeql.test.ts @@ -34,6 +34,7 @@ import { makeVersionInfo, createTestConfig, makeMacro, + mockApiDetails, } from "./testing-utils"; import { ToolsDownloadStatusReport } from "./tools-download"; import * as util from "./util"; @@ -126,19 +127,6 @@ function mockReleaseApi({ }); } -function mockApiDetails(apiDetails: Required) { - // This is a workaround to mock `api.getApiDetails()` since it doesn't seem to be possible to - // mock this directly. The difficulty is that `getApiDetails()` is called locally in - // `api-client.ts`, but `sinon.stub(api, "getApiDetails")` only affects calls to - // `getApiDetails()` via an imported `api` module. - sinon - .stub(actionsUtil, "getRequiredInput") - .withArgs("token") - .returns(apiDetails.auth); - process.env["GITHUB_SERVER_URL"] = apiDetails.url; - process.env["GITHUB_API_URL"] = apiDetails.apiURL || ""; -} - async function stubCodeql(): Promise { const codeqlObject = await codeql.getCodeQLForTesting(); sinon.stub(codeqlObject, "getVersion").resolves(makeVersionInfo("2.17.6")); @@ -290,6 +278,7 @@ for (const { cliVersion, tagName, expectedToolcacheVersion } of [ async (t) => { await util.withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); + mockApiDetails(SAMPLE_DOTCOM_API_DETAILS); const result = await installIntoToolcache({ cliVersion, @@ -470,6 +459,7 @@ test.serial( await util.withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); + mockApiDetails(SAMPLE_DOTCOM_API_DETAILS); await installIntoToolcache({ tagName: "codeql-bundle-20200601", @@ -517,6 +507,7 @@ test.serial( await util.withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); + mockApiDetails(SAMPLE_DOTCOM_API_DETAILS); await installIntoToolcache({ tagName: "codeql-bundle-20200601", diff --git a/src/setup-codeql.test.ts b/src/setup-codeql.test.ts index 6bdb29f46..9ad2c6773 100644 --- a/src/setup-codeql.test.ts +++ b/src/setup-codeql.test.ts @@ -32,6 +32,7 @@ import { getTestEnv, initAllState, makeMacro, + mockApiDetails, mockBundleDownloadApi, setupActionsVars, setupTests, @@ -221,6 +222,7 @@ for (const { await withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); + mockApiDetails(SAMPLE_DOTCOM_API_DETAILS); const source = await setupCodeql.getCodeQLSource( "linked", SAMPLE_DEFAULT_CLI_VERSION, @@ -255,6 +257,7 @@ test.serial( await withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); + mockApiDetails(SAMPLE_DOTCOM_API_DETAILS); const source = await setupCodeql.getCodeQLSource( "latest", SAMPLE_DEFAULT_CLI_VERSION, @@ -298,6 +301,7 @@ test.serial( await withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); + mockApiDetails(SAMPLE_DOTCOM_API_DETAILS); const result = await setupCodeql.setupCodeQLBundle( "linked", SAMPLE_DOTCOM_API_DETAILS, @@ -479,6 +483,7 @@ test.serial( await withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir, { GITHUB_EVENT_NAME: "dynamic" }); + mockApiDetails(SAMPLE_DOTCOM_API_DETAILS); const source = await setupCodeql.getCodeQLSource( undefined, @@ -624,6 +629,7 @@ for (const toolsInput of ["nightly", "nightly-latest"]) { await withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); + mockApiDetails(SAMPLE_DOTCOM_API_DETAILS); const source = await setupCodeql.getCodeQLSource( toolsInput, SAMPLE_DEFAULT_CLI_VERSION, @@ -945,6 +951,7 @@ const toolcacheInputFallbackMacro = makeMacro({ await withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); + mockApiDetails(SAMPLE_DOTCOM_API_DETAILS); for (const [k, v] of Object.entries(environment)) { process.env[k] = v; @@ -1165,6 +1172,7 @@ test.serial( await withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); + mockApiDetails(SAMPLE_DOTCOM_API_DETAILS); const source = await setupCodeql.getCodeQLSource( undefined, PER_LANGUAGE_CLI_VERSION, @@ -1207,6 +1215,7 @@ test.serial( await withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); + mockApiDetails(SAMPLE_DOTCOM_API_DETAILS); for (const { otherLanguagePacksReason, features } of [ // The per-language feature is disabled. { otherLanguagePacksReason: undefined, features: createFeatures([]) }, @@ -1286,6 +1295,7 @@ for (const fallback of [false, true]) { setupActionsVars(tmpDir, tmpDir, { GITHUB_ACTION_REPOSITORY: "codeql-testing/action-fork", }); + mockApiDetails(SAMPLE_DOTCOM_API_DETAILS); const result = await setupCodeql.setupCodeQLBundle( undefined, SAMPLE_DOTCOM_API_DETAILS, @@ -1553,6 +1563,7 @@ for (const error of [ await withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); + mockApiDetails(SAMPLE_DOTCOM_API_DETAILS); await t.throwsAsync( setupCodeql.setupCodeQLBundle( undefined, @@ -2259,6 +2270,7 @@ test.serial( async (t) => { await withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); + mockApiDetails(SAMPLE_DOTCOM_API_DETAILS); delete process.env[EnvVar.HAS_SET_UP_CODEQL]; stubDownloadAndExtract(); @@ -2294,6 +2306,7 @@ test.serial( await withTmpDir(async (tmpDir) => { setupActionsVars(tmpDir, tmpDir); + mockApiDetails(SAMPLE_DOTCOM_API_DETAILS); process.env[ActionsEnvVars.RUNNER_ENVIRONMENT] = "github-hosted"; delete process.env[EnvVar.HAS_SET_UP_CODEQL]; const staleDirectory = createToolcacheEntry( diff --git a/src/setup/repository.ts b/src/setup/repository.ts index 341a27875..fa8b566f2 100644 --- a/src/setup/repository.ts +++ b/src/setup/repository.ts @@ -8,7 +8,6 @@ import { parseRepositoryNwo, type RepositoryNwoStr, } from "../repository"; -import { type InstanceURL } from "../util"; /** The NWO of the standard CodeQL Action repository. */ export const CODEQL_DEFAULT_ACTION_REPOSITORY = "github/codeql-action"; @@ -48,20 +47,14 @@ export function getCodeQLActionRepository( } /**A download source is a pair of strings. */ -export type DownloadSource = [InstanceURL, RepositoryNwoStr]; - -/** GitHub.com, and the canonical Action. */ -export const DEFAULT_DOWNLOAD_SOURCE: DownloadSource = [ - GITHUB_DOTCOM_URL, - CODEQL_DEFAULT_ACTION_REPOSITORY, -]; +export type DownloadSource = [api.GitHubApiDetails, RepositoryNwoStr]; /** Decides if the two provided download sources are the same. */ const isSameDownloadSource = ( - [srcUrl, srcRepo]: DownloadSource, - [otherUrl, otherRepo]: DownloadSource, + [srcInstance, srcRepo]: DownloadSource, + [otherInstance, otherRepo]: DownloadSource, ) => { - return srcUrl === otherUrl && srcRepo === otherRepo; + return srcInstance.url === otherInstance.url && srcRepo === otherRepo; }; /** A download URL is represented as a string. */ @@ -104,13 +97,21 @@ export async function getCodeQLAssetDownloadURL( ): Promise { const codeQLActionRepository = getCodeQLActionRepository(action); + // We construct the default GitHub.com source here, because `getDotcomApiDetails` + // may retrieve the `token` input if we are currently running on GitHub.com and + // we want to avoid doing that statically. + const defaultDownloadSource: DownloadSource = [ + api.getDotcomApiDetails(action.env), + CODEQL_DEFAULT_ACTION_REPOSITORY, + ]; + const potentialDownloadSources: DownloadSource[] = [ // This GitHub instance, and this Action. - [apiDetails.url, codeQLActionRepository], + [apiDetails, codeQLActionRepository], // This GitHub instance, and the canonical Action. - [apiDetails.url, CODEQL_DEFAULT_ACTION_REPOSITORY], + [apiDetails, CODEQL_DEFAULT_ACTION_REPOSITORY], // GitHub.com, and the canonical Action. - DEFAULT_DOWNLOAD_SOURCE, + defaultDownloadSource, ]; // We now filter out any duplicates. @@ -123,34 +124,34 @@ export async function getCodeQLAssetDownloadURL( }, ); - for (const [instanceURL, repository] of uniqueDownloadSources) { + for (const [instance, repository] of uniqueDownloadSources) { // If we've reached the final case, short-circuit the API check since we know the asset exists and is public. - if ( - isSameDownloadSource(DEFAULT_DOWNLOAD_SOURCE, [instanceURL, repository]) - ) { + if (isSameDownloadSource(defaultDownloadSource, [instance, repository])) { break; } const { owner: repositoryOwner, repo: repositoryName } = parseRepositoryNwo(repository); try { - const release = await api.getApiClient().rest.repos.getReleaseByTag({ - owner: repositoryOwner, - repo: repositoryName, - tag: tagName, - }); + const release = await api + .getApiClient(action.env, instance) + .rest.repos.getReleaseByTag({ + owner: repositoryOwner, + repo: repositoryName, + tag: tagName, + }); for (const asset of release.data.assets) { if (asset.name === assetName) { action.logger.info( - `Found ${assetKind} ${assetName} in ${repository} on ${instanceURL} with URL ${asset.url}.`, + `Found ${assetKind} ${assetName} in ${repository} on ${instance.apiURL} with URL ${asset.url}.`, ); return asset.url; } } } catch (e) { action.logger.info( - `Looked for ${assetKind} ${assetName} in ${repository} on ${instanceURL} but got error ${e}.`, + `Looked for ${assetKind} ${assetName} in ${repository} on ${instance.apiURL} but got error ${e}.`, ); } } diff --git a/src/start-proxy.test.ts b/src/start-proxy.test.ts index d01543e70..f432b9b16 100644 --- a/src/start-proxy.test.ts +++ b/src/start-proxy.test.ts @@ -9,7 +9,7 @@ import { ActionState } from "./action-common"; import * as actionsUtil from "./actions-util"; import * as apiClient from "./api-client"; import * as defaults from "./defaults.json"; -import { ActionsEnvVars } from "./environment"; +import { ActionsEnvVars, type Env } from "./environment"; import { setUpFeatureFlagTests } from "./feature-flags/testing-util"; import { UnvalidatedObject, validateSchema } from "./json"; import { makeFromSchema } from "./json/testing-util"; @@ -27,6 +27,7 @@ import { initAllState, makeMacro, makeTestToken, + mockApiDetails, RecordingLogger, SAMPLE_DOTCOM_API_DETAILS, setupTests, @@ -699,18 +700,23 @@ test("getProxyPackage - includes platform in name", (t) => { } }); -function mockGetApiClient(endpoints: any) { - return ( - sinon - .stub(apiClient, "getApiClient") - // eslint-disable-next-line @typescript-eslint/no-unsafe-argument - .returns({ rest: endpoints } as any) - ); +function mockGetApiClient(env: Env, endpoints: any) { + const getRequiredInput = mockApiDetails(SAMPLE_DOTCOM_API_DETAILS, env); + const getApiClient = sinon + .stub(apiClient, "getApiClient") + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + .returns({ rest: endpoints } as any); + + return { getRequiredInput, getApiClient }; } type ReleaseAssets = Array<{ name: string; url?: string }>; -function mockGetReleaseByTag(expectedTag?: string, assets?: ReleaseAssets) { +function mockGetReleaseByTag( + env: Env, + expectedTag?: string, + assets?: ReleaseAssets, +) { const getReleaseByTag = assets === undefined ? sinon.stub().rejects() @@ -731,7 +737,7 @@ function mockGetReleaseByTag(expectedTag?: string, assets?: ReleaseAssets) { return { getReleaseByTag, - getApiClient: mockGetApiClient({ repos: { getReleaseByTag } }), + ...mockGetApiClient(env, { repos: { getReleaseByTag } }), }; } @@ -757,7 +763,6 @@ test.serial( "getDownloadUrl logs unknown platforms/arch and defaults to linux64", async (t) => { const logger = new RecordingLogger(); - mockGetReleaseByTag(); await withTmpDir(async (tempDir) => { const features = mockOfflineFeatures(tempDir, logger); @@ -767,6 +772,7 @@ test.serial( logger, features, }); + mockGetReleaseByTag(state.env); // Stub this to always return `true` so we get the default repo for the tests. sinon.stub(actionsUtil, "isRunningLocalAction").returns(true); @@ -796,11 +802,11 @@ test.serial( "getDownloadUrl returns default bundle URL for linked version", async (t) => { const logger = new RecordingLogger(); - mockGetReleaseByTag(); await withTmpDir(async (tempDir) => { const features = mockOfflineFeatures(tempDir, logger); const state = initAllState({ logger, features }); + mockGetReleaseByTag(state.env); // Stub this to always return `true` so we get the default repo for the tests. sinon.stub(actionsUtil, "isRunningLocalAction").returns(true); @@ -836,7 +842,11 @@ test.serial( sinon.stub(actionsUtil, "isRunningLocalAction").returns(true); for (const assets of testAssets) { - const stubs = mockGetReleaseByTag(defaults.bundleVersion, assets); + const stubs = mockGetReleaseByTag( + state.env, + defaults.bundleVersion, + assets, + ); const info = await startProxyExports.getDownloadUrl( state, SAMPLE_DOTCOM_API_DETAILS, @@ -854,6 +864,7 @@ test.serial( // mockGetReleaseByTag stubs the global getApiClient; since we are calling // that in a loop, we need to manually restore it here stubs.getApiClient.restore(); + stubs.getRequiredInput.restore(); } }); }, @@ -869,7 +880,7 @@ test.serial("getDownloadUrl returns matching release asset", async (t) => { url: "url-we-want", }, ]; - const stubs = mockGetReleaseByTag(defaults.bundleVersion, assets); + const stubs = mockGetReleaseByTag(state.env, defaults.bundleVersion, assets); await withTmpDir(async (tempDir) => { state.env.set(ActionsEnvVars.RUNNER_TEMP, tempDir); @@ -1011,7 +1022,6 @@ test.serial( "getProxyBinaryPath - returns path from tool cache if available", async (t) => { const logger = new RecordingLogger(); - mockGetReleaseByTag(); await withTmpDir(async (tempDir) => { const toolcachePath = "/path/to/proxy/dir"; @@ -1021,8 +1031,11 @@ test.serial( sinon.stub(actionsUtil, "isRunningLocalAction").returns(true); const features = mockOfflineFeatures(tempDir, logger); + const state = initAllState({ logger, features }); + mockGetReleaseByTag(state.env); + const path = await startProxyExports.getProxyBinaryPath( - initAllState({ logger, features }), + state, SAMPLE_DOTCOM_API_DETAILS, ); @@ -1041,7 +1054,7 @@ test.serial( const logger = new RecordingLogger(); const state = initAllState({ logger }); const downloadUrl = "url-we-want"; - const stubs = mockGetReleaseByTag(defaults.bundleVersion, [ + const stubs = mockGetReleaseByTag(state.env, defaults.bundleVersion, [ { name: startProxyExports.getProxyPackage(getTestPlatform(state)), url: downloadUrl, @@ -1091,7 +1104,7 @@ test.serial( ); checkExpectedLogMessages(t, logger.messages, [ - `Found private registry proxy ${startProxyExports.getProxyPackage(getTestPlatform(state))} in test/repo on https://github.com with URL ${downloadUrl}`, + `Found private registry proxy ${startProxyExports.getProxyPackage(getTestPlatform(state))} in test/repo on https://api.github.com with URL ${downloadUrl}`, ]); }, ); @@ -1122,7 +1135,7 @@ test.serial( headers: {}, url: "GET /repos/:owner/:repo/releases/tags/:tag", }); - mockGetApiClient({ repos: { getReleaseByTag } }); + mockGetApiClient(state.env, { repos: { getReleaseByTag } }); await withTmpDir(async (tempDir) => { const toolcachePath = "/path/to/proxy/dir"; @@ -1187,7 +1200,7 @@ test.serial( }); checkExpectedLogMessages(t, logger.messages, [ - `Found private registry proxy ${startProxyExports.getProxyPackage(getTestPlatform(state))} in test/repo on https://github.com with URL url-we-want.`, + `Found private registry proxy ${startProxyExports.getProxyPackage(getTestPlatform(state))} in test/repo on https://api.github.com with URL url-we-want.`, ]); }, ); diff --git a/src/testing-utils.ts b/src/testing-utils.ts index 8c38187c4..2b320b346 100644 --- a/src/testing-utils.ts +++ b/src/testing-utils.ts @@ -14,6 +14,7 @@ import * as sinon from "sinon"; import { ActionState, StateFeature } from "./action-common"; import { ActionsEnv, getActionVersion } from "./actions-util"; +import * as actionsUtil from "./actions-util"; import { AnalysisKind } from "./analyses"; import * as apiClient from "./api-client"; import { GitHubApiDetails } from "./api-client"; @@ -52,6 +53,23 @@ export const SAMPLE_DOTCOM_API_DETAILS = { apiURL: "https://api.github.com", }; +export function mockApiDetails( + apiDetails: Required, + env: Env = getEnv(), +) { + // This is a workaround to mock `api.getApiDetails()` since it doesn't seem to be possible to + // mock this directly. The difficulty is that `getApiDetails()` is called locally in + // `api-client.ts`, but `sinon.stub(api, "getApiDetails")` only affects calls to + // `getApiDetails()` via an imported `api` module. + const getRequiredInput = sinon.stub(actionsUtil, "getRequiredInput"); + getRequiredInput.withArgs("token").returns(apiDetails.auth); + + env.set("GITHUB_SERVER_URL", apiDetails.url); + env.set("GITHUB_API_URL", apiDetails.apiURL ?? ""); + + return getRequiredInput; +} + export const LINKED_CLI_VERSION = { cliVersion: defaults.cliVersion, tagName: defaults.bundleVersion,