From 020856ca4e4f8bd905d8627cbc659ccf9a3c6ac2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93scar=20San=20Jos=C3=A9?= Date: Tue, 14 Apr 2026 15:10:54 +0200 Subject: [PATCH] Address comments on the PR --- src/actions-util.ts | 29 ++++++++++++++ src/codeql.ts | 2 +- src/feature-flags/properties.test.ts | 27 +++---------- src/init-action.ts | 18 ++++++--- src/init.ts | 2 +- src/setup-codeql-action.ts | 30 ++++++++++++++- src/setup-codeql.test.ts | 16 ++++++-- src/setup-codeql.ts | 57 ++++++++++++++++------------ 8 files changed, 121 insertions(+), 60 deletions(-) diff --git a/src/actions-util.ts b/src/actions-util.ts index e1a7adb8f..296b19615 100644 --- a/src/actions-util.ts +++ b/src/actions-util.ts @@ -46,6 +46,35 @@ export const getOptionalInput = function (name: string): string | undefined { return value.length > 0 ? value : undefined; }; +/** + * Resolves the effective tools input by combining workflow input and repository properties. + * The explicit `tools` workflow input takes precedence. If none is provided, + * fall back to the repository property (if set). + * + * @param repositoryProperties - The loaded repository properties object + * @param toolsPropertyName - The name of the tools property to look up + * @returns An object containing the effective tools input and whether it came from repository property + */ +export function resolveToolsInput( + repositoryProperties: Record, + toolsPropertyName: string, +): { + effectiveToolsInput: string | undefined; + toolsInputFromRepositoryProperty: boolean; +} { + const toolsWorkflowInput = getOptionalInput("tools"); + const toolsPropertyValue: string | undefined = + repositoryProperties[toolsPropertyName]; + const effectiveToolsInput = toolsWorkflowInput ?? toolsPropertyValue; + const toolsInputFromRepositoryProperty = + toolsWorkflowInput === undefined && toolsPropertyValue !== undefined; + + return { + effectiveToolsInput, + toolsInputFromRepositoryProperty, + }; +} + export function getTemporaryDirectory(): string { const value = process.env["CODEQL_ACTION_TEMP"]; return value !== undefined && value !== "" diff --git a/src/codeql.ts b/src/codeql.ts index dad77466d..4978de66f 100644 --- a/src/codeql.ts +++ b/src/codeql.ts @@ -323,7 +323,7 @@ export async function setupCodeQL( features: FeatureEnablement, logger: Logger, checkVersion: boolean, - toolsInputFromRepositoryProperty = false, + toolsInputFromRepositoryProperty: boolean, ): Promise<{ codeql: CodeQL; toolsDownloadStatusReport?: ToolsDownloadStatusReport; diff --git a/src/feature-flags/properties.test.ts b/src/feature-flags/properties.test.ts index 912535efd..2bb9e10e4 100644 --- a/src/feature-flags/properties.test.ts +++ b/src/feature-flags/properties.test.ts @@ -78,6 +78,7 @@ test.serial("loadPropertiesFromApi loads known properties", async (t) => { url: "", data: [ { property_name: "github-codeql-extra-queries", value: "+queries" }, + { property_name: "github-codeql-tools", value: "toolcache" }, { property_name: "unknown-property", value: "something" }, ] satisfies properties.GitHubPropertiesResponse, }); @@ -87,30 +88,12 @@ test.serial("loadPropertiesFromApi loads known properties", async (t) => { logger, mockRepositoryNwo, ); - t.deepEqual(response, { "github-codeql-extra-queries": "+queries" }); + t.deepEqual(response, { + "github-codeql-extra-queries": "+queries", + "github-codeql-tools": "toolcache", + }); }); -test.serial( - "loadPropertiesFromApi loads github-codeql-tools property", - async (t) => { - sinon.stub(api, "getRepositoryProperties").resolves({ - headers: {}, - status: 200, - url: "", - data: [ - { property_name: "github-codeql-tools", value: "toolcache" }, - ] satisfies properties.GitHubPropertiesResponse, - }); - const logger = getRunnerLogger(true); - const mockRepositoryNwo = parseRepositoryNwo("owner/repo"); - const response = await properties.loadPropertiesFromApi( - logger, - mockRepositoryNwo, - ); - t.deepEqual(response, { "github-codeql-tools": "toolcache" }); - }, -); - test.serial("loadPropertiesFromApi parses true boolean property", async (t) => { sinon.stub(api, "getRepositoryProperties").resolves({ headers: {}, diff --git a/src/init-action.ts b/src/init-action.ts index ce705d3dd..9a878363e 100644 --- a/src/init-action.ts +++ b/src/init-action.ts @@ -15,6 +15,7 @@ import { getRequiredInput, getTemporaryDirectory, persistInputs, + resolveToolsInput, } from "./actions-util"; import { AnalysisKind, getAnalysisKinds } from "./analyses"; import { getGitHubVersion, GitHubApiCombinedDetails } from "./api-client"; @@ -141,6 +142,7 @@ async function sendCompletedStatusReport( toolsFeatureFlagsValid: boolean | undefined, toolsSource: ToolsSource, toolsVersion: string, + effectiveToolsInput: string | undefined, overlayBaseDatabaseStats: OverlayBaseDatabaseDownloadStats | undefined, dependencyCachingResults: DependencyCacheRestoreStatusReport | undefined, logger: Logger, @@ -165,7 +167,7 @@ async function sendCompletedStatusReport( const initStatusReport: InitStatusReport = { ...statusReportBase, - tools_input: getOptionalInput("tools") || "", + tools_input: effectiveToolsInput || "", tools_resolved_version: toolsVersion, tools_source: toolsSource || ToolsSource.Unknown, workflow_languages: workflowLanguages || "", @@ -220,6 +222,7 @@ async function run(startedAt: Date) { let toolsSource: ToolsSource; let toolsVersion: string; let zstdAvailability: ZstdAvailability | undefined; + let effectiveToolsInput: string | undefined; try { initializeEnvironment(getActionVersion()); @@ -302,12 +305,13 @@ async function run(startedAt: Date) { // Determine the effective tools input. // The explicit `tools` workflow input takes precedence. If none is provided, // fall back to the 'github-codeql-tools' repository property (if set). - const toolsWorkflowInput = getOptionalInput("tools"); - const toolsPropertyValue: string | undefined = - repositoryPropertiesResult.orElse({})[RepositoryPropertyName.TOOLS]; - const effectiveToolsInput = toolsWorkflowInput ?? toolsPropertyValue; + const resolvedToolsInput = resolveToolsInput( + repositoryPropertiesResult.orElse({}), + RepositoryPropertyName.TOOLS, + ); + effectiveToolsInput = resolvedToolsInput.effectiveToolsInput; const toolsInputFromRepositoryProperty = - toolsWorkflowInput === undefined && toolsPropertyValue !== undefined; + resolvedToolsInput.toolsInputFromRepositoryProperty; const initCodeQLResult = await initCodeQL( effectiveToolsInput, @@ -791,6 +795,7 @@ async function run(startedAt: Date) { toolsFeatureFlagsValid, toolsSource, toolsVersion, + effectiveToolsInput, overlayBaseDatabaseStats, dependencyCachingStatus, logger, @@ -808,6 +813,7 @@ async function run(startedAt: Date) { toolsFeatureFlagsValid, toolsSource, toolsVersion, + effectiveToolsInput, overlayBaseDatabaseStats, dependencyCachingStatus, logger, diff --git a/src/init.ts b/src/init.ts index 376425861..6519ec361 100644 --- a/src/init.ts +++ b/src/init.ts @@ -41,7 +41,7 @@ export async function initCodeQL( defaultCliVersion: CodeQLDefaultVersionInfo, features: FeatureEnablement, logger: Logger, - toolsInputFromRepositoryProperty = false, + toolsInputFromRepositoryProperty: boolean, ): Promise<{ codeql: CodeQL; toolsDownloadStatusReport?: ToolsDownloadStatusReport; diff --git a/src/setup-codeql-action.ts b/src/setup-codeql-action.ts index bd504f3fd..619da8959 100644 --- a/src/setup-codeql-action.ts +++ b/src/setup-codeql-action.ts @@ -6,11 +6,16 @@ import { getOptionalInput, getRequiredInput, getTemporaryDirectory, + resolveToolsInput, } from "./actions-util"; import { getGitHubVersion } from "./api-client"; import { CodeQL } from "./codeql"; import { EnvVar } from "./environment"; import { initFeatures } from "./feature-flags"; +import { + loadPropertiesFromApi, + RepositoryPropertyName, +} from "./feature-flags/properties"; import { initCodeQL } from "./init"; import { getActionsLogger, Logger } from "./logging"; import { getRepositoryNwo } from "./repository"; @@ -46,6 +51,7 @@ async function sendCompletedStatusReport( toolsFeatureFlagsValid: boolean | undefined, toolsSource: ToolsSource, toolsVersion: string, + effectiveToolsInput: string | undefined, logger: Logger, error?: Error, ): Promise { @@ -66,7 +72,7 @@ async function sendCompletedStatusReport( const initStatusReport: InitStatusReport = { ...statusReportBase, - tools_input: getOptionalInput("tools") || "", + tools_input: effectiveToolsInput || "", tools_resolved_version: toolsVersion, tools_source: toolsSource || ToolsSource.Unknown, workflow_languages: "", @@ -97,6 +103,7 @@ async function run(startedAt: Date): Promise { let toolsFeatureFlagsValid: boolean | undefined; let toolsSource: ToolsSource; let toolsVersion: string; + let effectiveToolsInput: string | undefined; try { initializeEnvironment(getActionVersion()); @@ -121,6 +128,11 @@ async function run(startedAt: Date): Promise { logger, ); + const repositoryPropertiesResult = await loadPropertiesFromApi( + logger, + repositoryNwo, + ); + const jobRunUuid = uuidV4(); logger.info(`Job run UUID is ${jobRunUuid}.`); core.exportVariable(EnvVar.JOB_RUN_UUID, jobRunUuid); @@ -140,14 +152,27 @@ async function run(startedAt: Date): Promise { gitHubVersion.type, ); toolsFeatureFlagsValid = codeQLDefaultVersionInfo.toolsFeatureFlagsValid; + + // Determine the effective tools input. + // The explicit `tools` workflow input takes precedence. If none is provided, + // fall back to the 'github-codeql-tools' repository property (if set). + const resolvedToolsInput = resolveToolsInput( + repositoryPropertiesResult, + RepositoryPropertyName.TOOLS, + ); + effectiveToolsInput = resolvedToolsInput.effectiveToolsInput; + const toolsInputFromRepositoryProperty = + resolvedToolsInput.toolsInputFromRepositoryProperty; + const initCodeQLResult = await initCodeQL( - getOptionalInput("tools"), + effectiveToolsInput, apiDetails, getTemporaryDirectory(), gitHubVersion.type, codeQLDefaultVersionInfo, features, logger, + toolsInputFromRepositoryProperty, ); codeql = initCodeQLResult.codeql; toolsDownloadStatusReport = initCodeQLResult.toolsDownloadStatusReport; @@ -183,6 +208,7 @@ async function run(startedAt: Date): Promise { toolsFeatureFlagsValid, toolsSource, toolsVersion, + effectiveToolsInput, logger, ); } diff --git a/src/setup-codeql.test.ts b/src/setup-codeql.test.ts index 4c249fcfb..72a53cbcb 100644 --- a/src/setup-codeql.test.ts +++ b/src/setup-codeql.test.ts @@ -8,6 +8,7 @@ import * as sinon from "sinon"; import * as actionsUtil from "./actions-util"; import * as api from "./api-client"; import { Feature, FeatureEnablement } from "./feature-flags"; +import { RepositoryPropertyName } from "./feature-flags/properties"; import { getRunnerLogger } from "./logging"; import * as setupCodeql from "./setup-codeql"; import * as tar from "./tar"; @@ -112,6 +113,7 @@ test.serial( false, features, getRunnerLogger(true), + false, ); t.is(source.sourceType, "download"); @@ -135,6 +137,7 @@ test.serial( false, features, getRunnerLogger(true), + false, ); t.is(source.toolsVersion, LINKED_CLI_VERSION.cliVersion); @@ -160,6 +163,7 @@ test.serial( false, features, logger, + false, ); // First, ensure that the CLI version is the linked version, so that backwards @@ -213,6 +217,7 @@ test.serial( SAMPLE_DEFAULT_CLI_VERSION, features, logger, + false, ); // Basic sanity check that the version we got back is indeed @@ -268,6 +273,7 @@ test.serial( SAMPLE_DEFAULT_CLI_VERSION, features, logger, + false, ); // Basic sanity check that the version we got back is indeed the version that the @@ -322,6 +328,7 @@ test.serial( false, features, logger, + false, ); // Check that the `CodeQLToolsSource` object matches our expectations. @@ -383,6 +390,7 @@ test.serial( false, features, logger, + false, ); // Check that the `CodeQLToolsSource` object matches our expectations. @@ -437,6 +445,7 @@ test.serial( false, features, logger, + false, ); // Check that the toolcache functions were called with the expected arguments @@ -504,6 +513,7 @@ const toolcacheInputFallbackMacro = test.macro({ false, features, logger, + false, ); // Check that the toolcache functions were called with the expected arguments @@ -612,7 +622,7 @@ test.serial( t.is(source.toolsVersion, latestToolcacheVersion); const expectedMessages: string[] = [ - `Attempting to use the latest CodeQL CLI version in the toolcache, as requested by the 'github-codeql-tools' repository property.`, + `Attempting to use the latest CodeQL CLI version in the toolcache, as requested by the '${RepositoryPropertyName.TOOLS}' repository property.`, `CLI version ${latestToolcacheVersion} is the latest version in the toolcache.`, `Using CodeQL CLI version ${latestToolcacheVersion} from toolcache at ${latestVersionPath}`, ]; @@ -657,8 +667,8 @@ test.serial( t.is(source.toolsVersion, SAMPLE_DEFAULT_CLI_VERSION.cliVersion); const expectedMessages: string[] = [ - `Attempting to use the latest CodeQL CLI version in the toolcache, as requested by the 'github-codeql-tools' repository property.`, - `Found no CodeQL CLI in the toolcache, ignoring the 'github-codeql-tools' repository property...`, + `Attempting to use the latest CodeQL CLI version in the toolcache, as requested by the '${RepositoryPropertyName.TOOLS}' repository property.`, + `Found no CodeQL CLI in the toolcache, ignoring the '${RepositoryPropertyName.TOOLS}' repository property...`, ]; for (const expectedMessage of expectedMessages) { t.assert( diff --git a/src/setup-codeql.ts b/src/setup-codeql.ts index 6c0ecf4e8..b564161aa 100644 --- a/src/setup-codeql.ts +++ b/src/setup-codeql.ts @@ -17,6 +17,7 @@ import { Feature, FeatureEnablement, } from "./feature-flags"; +import { RepositoryPropertyName } from "./feature-flags/properties"; import { Logger } from "./logging"; import * as tar from "./tar"; import { @@ -264,6 +265,24 @@ async function findOverridingToolsInCache( return undefined; } +/** + * Creates a user-friendly description of the tools input origin for log messages. + * + * @param toolsInput The tools input value + * @param toolsInputFromRepositoryProperty Whether the tools input came from repository property + * @returns A description like "'tools: toolcache'" or "the 'github-codeql-tools' repository property" + */ +function getToolsInputOriginDescription( + toolsInput: string | undefined, + toolsInputFromRepositoryProperty: boolean, +): string { + if (toolsInputFromRepositoryProperty) { + return `the '${RepositoryPropertyName.TOOLS}' repository property`; + } else { + return `'tools: ${toolsInput}'`; + } +} + /** * Determines where the CodeQL CLI we want to use comes from. This can be from a local file, * the Actions toolcache, or a download. @@ -286,7 +305,7 @@ export async function getCodeQLSource( tarSupportsZstd: boolean, features: FeatureEnablement, logger: Logger, - toolsInputFromRepositoryProperty = false, + toolsInputFromRepositoryProperty: boolean, ): Promise { // If there is an explicit `tools` input, it's not one of the reserved values, and it doesn't appear // to point to a URL, then we assume it is a local path and use the CLI from there. @@ -361,7 +380,7 @@ export async function getCodeQLSource( ); } else { logger.info( - `Using the latest CodeQL CLI nightly, as requested by 'tools: ${toolsInput}'.`, + `Using the latest CodeQL CLI nightly, as requested by ${getToolsInputOriginDescription(toolsInput, toolsInputFromRepositoryProperty)}.`, ); } toolsInput = await getNightlyToolsUrl(logger); @@ -386,7 +405,7 @@ export async function getCodeQLSource( tagName = defaults.bundleVersion; logger.info( - `'tools: ${toolsInput}' was requested, so using CodeQL version ${cliVersion}, the version shipped with the Action.`, + `${getToolsInputOriginDescription(toolsInput, toolsInputFromRepositoryProperty)} was requested, so using CodeQL version ${cliVersion}, the version shipped with the Action.`, ); if (toolsInput === "latest") { @@ -414,15 +433,9 @@ export async function getCodeQLSource( // If `toolsInput === "toolcache"`, try to find the latest version of the CLI that's available in the toolcache // and use that. We perform this check here since we can set `cliVersion` directly and don't want to default to // the linked version. - if (toolsInputFromRepositoryProperty) { - logger.info( - `Attempting to use the latest CodeQL CLI version in the toolcache, as requested by the 'github-codeql-tools' repository property.`, - ); - } else { - logger.info( - `Attempting to use the latest CodeQL CLI version in the toolcache, as requested by 'tools: ${toolsInput}'.`, - ); - } + logger.info( + `Attempting to use the latest CodeQL CLI version in the toolcache, as requested by ${getToolsInputOriginDescription(toolsInput, toolsInputFromRepositoryProperty)}.`, + ); latestToolcacheVersion = getLatestToolcacheVersion(logger); if (latestToolcacheVersion) { @@ -432,23 +445,17 @@ export async function getCodeQLSource( if (latestToolcacheVersion === undefined) { if (allowToolcacheValue) { - if (toolsInputFromRepositoryProperty) { - logger.info( - `Found no CodeQL CLI in the toolcache, ignoring the 'github-codeql-tools' repository property...`, - ); - } else { - logger.info( - `Found no CodeQL CLI in the toolcache, ignoring 'tools: ${toolsInput}'...`, - ); - } + logger.info( + `Found no CodeQL CLI in the toolcache, ignoring ${getToolsInputOriginDescription(toolsInput, toolsInputFromRepositoryProperty)}...`, + ); } else { if (allowToolcacheValueFF) { logger.warning( - `Ignoring 'tools: ${toolsInput}' because the workflow was not triggered dynamically.`, + `Ignoring ${getToolsInputOriginDescription(toolsInput, toolsInputFromRepositoryProperty)} because the workflow was not triggered dynamically.`, ); } else { - logger.info( - `Ignoring 'tools: ${toolsInput}' because the feature is not enabled.`, + logger.warning( + `Ignoring ${getToolsInputOriginDescription(toolsInput, toolsInputFromRepositoryProperty)} because the feature is not enabled.`, ); } } @@ -808,7 +815,7 @@ export async function setupCodeQLBundle( defaultCliVersion: CodeQLDefaultVersionInfo, features: FeatureEnablement, logger: Logger, - toolsInputFromRepositoryProperty = false, + toolsInputFromRepositoryProperty: boolean, ): Promise { if (!(await util.isBinaryAccessible("tar", logger))) { throw new util.ConfigurationError(