From 5ccef822447dad3d567a5d4908ba25494ca08714 Mon Sep 17 00:00:00 2001 From: Henry Mercer Date: Wed, 3 Jun 2026 18:31:11 +0100 Subject: [PATCH] Address review comments --- lib/entry-points.js | 13 ++++++-- src/testing-utils.ts | 5 +++ src/util.test.ts | 72 ++++++++++++++++++++++++++++++++------------ src/util.ts | 43 +++++++++++++++++++++++--- 4 files changed, 107 insertions(+), 26 deletions(-) diff --git a/lib/entry-points.js b/lib/entry-points.js index 03ee3761a..29a829a23 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -148062,6 +148062,14 @@ function asHTTPError(arg) { return void 0; } var cachedCodeQlVersion = void 0; +function isVersionInfo(x) { + const candidate = x; + return typeof candidate === "object" && candidate !== null && typeof candidate.version === "string" && (candidate.features === void 0 || typeof candidate.features === "object" && candidate.features !== null) && (candidate.overlayVersion === void 0 || typeof candidate.overlayVersion === "number"); +} +function isPersistedVersionInfo(x) { + const candidate = x; + return typeof candidate === "object" && candidate !== null && typeof candidate.cmd === "string" && isVersionInfo(candidate.version); +} function cacheCodeQlVersion(cmd, version) { if (cachedCodeQlVersion !== void 0) { throw new Error("cacheCodeQlVersion() should be called only once"); @@ -148086,10 +148094,11 @@ function getCachedCodeQlVersion(cmd) { } catch { return void 0; } - if (typeof persisted?.version?.version !== "string" || cmd !== void 0 && persisted.cmd !== cmd) { + if (!isPersistedVersionInfo(persisted) || cmd !== void 0 && persisted.cmd !== cmd) { return void 0; } - return persisted.version; + cachedCodeQlVersion = persisted.version; + return cachedCodeQlVersion; } async function codeQlVersionAtLeast(codeql, requiredVersion) { return semver.gte((await codeql.getVersion()).version, requiredVersion); diff --git a/src/testing-utils.ts b/src/testing-utils.ts index c3e1d5142..1702d6835 100644 --- a/src/testing-utils.ts +++ b/src/testing-utils.ts @@ -32,6 +32,7 @@ import { GitHubVariant, GitHubVersion, HTTPError, + resetCachedCodeQlVersion, } from "./util"; export const SAMPLE_DOTCOM_API_DETAILS = { @@ -101,6 +102,10 @@ export function setupTests(testFn: TestFn) { // unless the test explicitly sets one up. codeql.setCodeQL({}); + // Reset the in-process CodeQL version cache so that it doesn't leak between + // tests, which each represent a separate Actions step in production. + resetCachedCodeQlVersion(); + // Replace stdout and stderr so we can record output during tests t.context.testOutput = ""; const processStdoutWrite = process.stdout.write.bind(process.stdout); diff --git a/src/util.test.ts b/src/util.test.ts index 8760b77e2..3d27e952a 100644 --- a/src/util.test.ts +++ b/src/util.test.ts @@ -533,25 +533,57 @@ test("Failure.orElse returns the default value for a failure result", (t) => { t.is(result.orElse("default value"), "default value"); }); -test("getCachedCodeQlVersion reuses a version persisted by an earlier step", (t) => { - process.env[EnvVar.CODEQL_VERSION_INFO] = JSON.stringify({ - cmd: "/path/to/codeql", - version: { version: "2.20.0" }, - }); - t.deepEqual(util.getCachedCodeQlVersion("/path/to/codeql"), { - version: "2.20.0", - }); -}); +test.serial( + "getCachedCodeQlVersion reuses a version persisted by an earlier step", + (t) => { + process.env[EnvVar.CODEQL_VERSION_INFO] = JSON.stringify({ + cmd: "/path/to/codeql", + version: { version: "2.20.0" }, + }); + t.deepEqual(util.getCachedCodeQlVersion("/path/to/codeql"), { + version: "2.20.0", + }); + }, +); -test("getCachedCodeQlVersion ignores a persisted version from a different CLI", (t) => { - process.env[EnvVar.CODEQL_VERSION_INFO] = JSON.stringify({ - cmd: "/path/to/other-codeql", - version: { version: "2.20.0" }, - }); - t.is(util.getCachedCodeQlVersion("/path/to/codeql"), undefined); -}); +test.serial( + "getCachedCodeQlVersion ignores a persisted version from a different CLI", + (t) => { + process.env[EnvVar.CODEQL_VERSION_INFO] = JSON.stringify({ + cmd: "/path/to/other-codeql", + version: { version: "2.20.0" }, + }); + t.is(util.getCachedCodeQlVersion("/path/to/codeql"), undefined); + }, +); -test("getCachedCodeQlVersion ignores a malformed persisted value", (t) => { - process.env[EnvVar.CODEQL_VERSION_INFO] = "not valid json"; - t.is(util.getCachedCodeQlVersion("/path/to/codeql"), undefined); -}); +test.serial( + "getCachedCodeQlVersion ignores a malformed persisted value", + (t) => { + process.env[EnvVar.CODEQL_VERSION_INFO] = "not valid json"; + t.is(util.getCachedCodeQlVersion("/path/to/codeql"), undefined); + }, +); + +test.serial( + "getCachedCodeQlVersion ignores a persisted value with the wrong structure", + (t) => { + for (const value of [ + JSON.stringify({ cmd: "/path/to/codeql" }), + JSON.stringify({ cmd: "/path/to/codeql", version: {} }), + JSON.stringify({ cmd: "/path/to/codeql", version: { version: 2 } }), + JSON.stringify({ version: { version: "2.20.0" } }), + JSON.stringify({ + cmd: "/path/to/codeql", + version: { version: "2.20.0", overlayVersion: "1" }, + }), + JSON.stringify({ + cmd: "/path/to/codeql", + version: { version: "2.20.0", features: "nope" }, + }), + ]) { + process.env[EnvVar.CODEQL_VERSION_INFO] = value; + t.is(util.getCachedCodeQlVersion("/path/to/codeql"), undefined, value); + } + }, +); diff --git a/src/util.ts b/src/util.ts index c9a20b467..200d68d2c 100644 --- a/src/util.ts +++ b/src/util.ts @@ -619,12 +619,44 @@ export function asHTTPError(arg: any): HTTPError | undefined { let cachedCodeQlVersion: undefined | VersionInfo = undefined; +/** + * Resets the in-process cache of the CodeQL CLI version. Only for use in tests, + * which exercise multiple "steps" within a single process. + */ +export function resetCachedCodeQlVersion(): void { + cachedCodeQlVersion = undefined; +} + /** The persisted version together with the CLI path it was obtained from. */ interface PersistedVersionInfo { cmd: string; version: VersionInfo; } +function isVersionInfo(x: unknown): x is VersionInfo { + const candidate = x as Partial | null; + return ( + typeof candidate === "object" && + candidate !== null && + typeof candidate.version === "string" && + (candidate.features === undefined || + (typeof candidate.features === "object" && + candidate.features !== null)) && + (candidate.overlayVersion === undefined || + typeof candidate.overlayVersion === "number") + ); +} + +function isPersistedVersionInfo(x: unknown): x is PersistedVersionInfo { + const candidate = x as Partial | null; + return ( + typeof candidate === "object" && + candidate !== null && + typeof candidate.cmd === "string" && + isVersionInfo(candidate.version) + ); +} + export function cacheCodeQlVersion(cmd: string, version: VersionInfo): void { if (cachedCodeQlVersion !== undefined) { throw new Error("cacheCodeQlVersion() should be called only once"); @@ -651,19 +683,22 @@ export function getCachedCodeQlVersion(cmd?: string): undefined | VersionInfo { if (!serialized) { return undefined; } - let persisted: PersistedVersionInfo; + let persisted: unknown; try { - persisted = JSON.parse(serialized) as PersistedVersionInfo; + persisted = JSON.parse(serialized); } catch { return undefined; } if ( - typeof persisted?.version?.version !== "string" || + !isPersistedVersionInfo(persisted) || (cmd !== undefined && persisted.cmd !== cmd) ) { return undefined; } - return persisted.version; + // Memoize the parsed value so that subsequent calls in this process don't + // re-parse the environment variable. + cachedCodeQlVersion = persisted.version; + return cachedCodeQlVersion; } export async function codeQlVersionAtLeast(