Merge branch 'main' into update-bundle/codeql-bundle-v2.18.0

src/analyze-action.ts

@@ -20,7 +20,7 @@ import { getCodeQL } from "./codeql";
 import { Config, getConfig } from "./config-utils";
 import { uploadDatabases } from "./database-upload";
 import { EnvVar } from "./environment";
-import { FeatureEnablement, Features } from "./feature-flags";
+import { Features } from "./feature-flags";
 import { Language } from "./languages";
 import { getActionsLogger, Logger } from "./logging";
 import { parseRepositoryNwo } from "./repository";
@@ -147,11 +147,7 @@ function doesGoExtractionOutputExist(config: Config): boolean {
  * - We approximate whether manual build steps are present by looking at
  * whether any extraction output already exists for Go.
  */
-async function runAutobuildIfLegacyGoWorkflow(
-  config: Config,
-  features: FeatureEnablement,
-  logger: Logger,
-) {
+async function runAutobuildIfLegacyGoWorkflow(config: Config, logger: Logger) {
   if (!config.languages.includes(Language.go)) {
     return;
   }
@@ -188,7 +184,7 @@ async function runAutobuildIfLegacyGoWorkflow(
   logger.debug(
     "Running Go autobuild because extraction output (TRAP files) for Go code has not been found.",
   );
-  await runAutobuild(config, Language.go, features, logger);
+  await runAutobuild(config, Language.go, logger);
 }
 
 async function run() {
@@ -260,7 +256,7 @@ async function run() {
     );
 
     await warnIfGoInstalledAfterInit(config, logger);
-    await runAutobuildIfLegacyGoWorkflow(config, features, logger);
+    await runAutobuildIfLegacyGoWorkflow(config, logger);
 
     dbCreationTimings = await runFinalize(
       outputDir,
@@ -268,7 +264,6 @@ async function run() {
       memory,
       codeql,
       config,
-      features,
       logger,
     );
 
@@ -301,10 +296,11 @@ async function run() {
     core.setOutput("sarif-output", path.resolve(outputDir));
     const uploadInput = actionsUtil.getOptionalInput("upload");
     if (runStats && actionsUtil.getUploadValue(uploadInput) === "always") {
-      uploadResult = await uploadLib.uploadFromActions(
+      uploadResult = await uploadLib.uploadFiles(
         outputDir,
         actionsUtil.getRequiredInput("checkout_path"),
         actionsUtil.getOptionalInput("category"),
+        features,
         logger,
       );
       core.setOutput("sarif-id", uploadResult.sarifID);

src/analyze.ts

@@ -383,7 +383,6 @@ export async function runFinalize(
   memoryFlag: string,
   codeql: CodeQL,
   config: configUtils.Config,
-  features: FeatureEnablement,
   logger: Logger,
 ): Promise<DatabaseCreationTimings> {
   try {
@@ -405,7 +404,7 @@ export async function runFinalize(
 
   // If we didn't already end tracing in the autobuild Action, end it now.
   if (process.env[EnvVar.AUTOBUILD_DID_COMPLETE_SUCCESSFULLY] !== "true") {
-    await endTracingForCluster(codeql, config, logger, features);
+    await endTracingForCluster(codeql, config, logger);
   }
   return timings;
 }

src/autobuild-action.ts

@@ -10,10 +10,8 @@ import { determineAutobuildLanguages, runAutobuild } from "./autobuild";
 import { getCodeQL } from "./codeql";
 import { Config, getConfig } from "./config-utils";
 import { EnvVar } from "./environment";
-import { Features } from "./feature-flags";
 import { Language } from "./languages";
 import { Logger, getActionsLogger } from "./logging";
-import { parseRepositoryNwo } from "./repository";
 import {
   StatusReportBase,
   getActionsStatus,
@@ -26,7 +24,6 @@ import {
   checkActionVersion,
   checkDiskUsage,
   checkGitHubVersionInRange,
-  getRequiredEnvParam,
   initializeEnvironment,
   wrapError,
 } from "./util";
@@ -92,17 +89,6 @@ async function run() {
     checkGitHubVersionInRange(gitHubVersion, logger);
     checkActionVersion(getActionVersion(), gitHubVersion);
 
-    const repositoryNwo = parseRepositoryNwo(
-      getRequiredEnvParam("GITHUB_REPOSITORY"),
-    );
-
-    const features = new Features(
-      gitHubVersion,
-      repositoryNwo,
-      getTemporaryDirectory(),
-      logger,
-    );
-
     config = await getConfig(getTemporaryDirectory(), logger);
     if (config === undefined) {
       throw new Error(
@@ -123,13 +109,13 @@ async function run() {
       }
       for (const language of languages) {
         currentLanguage = language;
-        await runAutobuild(config, language, features, logger);
+        await runAutobuild(config, language, logger);
       }
     }
 
     // End tracing early to avoid tracing analyze. This improves the performance and reliability of
     // the analyze step.
-    await endTracingForCluster(codeql, config, logger, features);
+    await endTracingForCluster(codeql, config, logger);
   } catch (unwrappedError) {
     const error = wrapError(unwrappedError);
     core.setFailed(

src/autobuild.ts

@@ -5,12 +5,7 @@ import { getGitHubVersion } from "./api-client";
 import { CodeQL, getCodeQL } from "./codeql";
 import * as configUtils from "./config-utils";
 import { EnvVar } from "./environment";
-import {
-  Feature,
-  featureConfig,
-  FeatureEnablement,
-  Features,
-} from "./feature-flags";
+import { Feature, featureConfig, Features } from "./feature-flags";
 import { isTracedLanguage, Language } from "./languages";
 import { Logger } from "./logging";
 import { parseRepositoryNwo } from "./repository";
@@ -159,7 +154,6 @@ export async function setupCppAutobuild(codeql: CodeQL, logger: Logger) {
 export async function runAutobuild(
   config: configUtils.Config,
   language: Language,
-  features: FeatureEnablement,
   logger: Logger,
 ) {
   logger.startGroup(`Attempting to automatically build ${language} code`);
@@ -169,7 +163,7 @@ export async function runAutobuild(
   }
   if (
     config.buildMode &&
-    (await features.getValue(Feature.AutobuildDirectTracing, codeQL))
+    (await codeQL.supportsFeature(ToolsFeature.TraceCommandUseBuildMode))
   ) {
     await codeQL.extractUsingBuildMode(config, language);
   } else {

src/codeql.test.ts

@@ -482,7 +482,6 @@ const injectedConfigMacro = test.macro({
         "",
         undefined,
         undefined,
-        createFeatures([]),
         getRunnerLogger(true),
       );
 
@@ -696,7 +695,6 @@ test("passes a code scanning config AND qlconfig to the CLI", async (t: Executio
       "",
       undefined,
       "/path/to/qlconfig.yml",
-      createFeatures([]),
       getRunnerLogger(true),
     );
 
@@ -726,7 +724,6 @@ test("does not pass a qlconfig to the CLI when it is undefined", async (t: Execu
       "",
       undefined,
       undefined, // undefined qlconfigFile
-      createFeatures([]),
       getRunnerLogger(true),
     );
 
@@ -1016,7 +1013,6 @@ test("Avoids duplicating --overwrite flag if specified in CODEQL_ACTION_EXTRA_OP
     "sourceRoot",
     undefined,
     undefined,
-    createFeatures([]),
     getRunnerLogger(false),
   );
 

src/codeql.ts

@@ -82,7 +82,6 @@ export interface CodeQL {
     sourceRoot: string,
     processName: string | undefined,
     qlconfigFile: string | undefined,
-    features: FeatureEnablement,
     logger: Logger,
   ): Promise<void>;
   /**
@@ -558,13 +557,12 @@ export async function getCodeQLForCmd(
       sourceRoot: string,
       processName: string | undefined,
       qlconfigFile: string | undefined,
-      features: FeatureEnablement,
       logger: Logger,
     ) {
       const extraArgs = config.languages.map(
         (language) => `--language=${language}`,
       );
-      if (await shouldEnableIndirectTracing(codeql, config, features)) {
+      if (await shouldEnableIndirectTracing(codeql, config)) {
         extraArgs.push("--begin-tracing");
         extraArgs.push(...(await getTrapCachingExtractorConfigArgs(config)));
         extraArgs.push(`--trace-process-name=${processName}`);

src/feature-flags.test.ts

@@ -285,30 +285,6 @@ for (const feature of Object.keys(featureConfig)) {
   }
 }
 
-// If we ever run into a situation where we no longer have any features that
-// specify a minimum version or required tools feature, then we will have a
-// bunch of code no longer being tested. This is unlikely, and this test will
-// fail if that happens.
-// If we do end up in that situation, then we should consider adding a synthetic
-// feature with a minimum version that is only used for tests.
-test("At least one feature has a minimum version specified", (t) => {
-  t.assert(
-    Object.values(featureConfig).some((f) => f.minimumVersion !== undefined),
-    "At least one feature should have a minimum version specified",
-  );
-
-  t.assert(
-    Object.values(featureConfig).some((f) => f.toolsFeature !== undefined),
-    "At least one feature should have a required tools feature specified",
-  );
-
-  // An even less likely scenario is that we no longer have any features.
-  t.assert(
-    Object.values(featureConfig).length > 0,
-    "There should be at least one feature",
-  );
-});
-
 test("Feature flags are saved to disk", async (t) => {
   await withTmpDir(async (tmpDir) => {
     const features = setUpFeatureFlagTests(tmpDir);

src/feature-flags.ts

@@ -45,7 +45,6 @@ export interface FeatureEnablement {
  * Legacy features should end with `_enabled`.
  */
 export enum Feature {
-  AutobuildDirectTracing = "autobuild_direct_tracing_v2",
   CleanupTrapCaches = "cleanup_trap_caches",
   CppDependencyInstallation = "cpp_dependency_installation_enabled",
   DisableCsharpBuildless = "disable_csharp_buildless",
@@ -86,12 +85,6 @@ export const featureConfig: Record<
     toolsFeature?: ToolsFeature;
   }
 > = {
-  [Feature.AutobuildDirectTracing]: {
-    defaultValue: false,
-    envVar: "CODEQL_ACTION_AUTOBUILD_BUILD_MODE_DIRECT_TRACING",
-    minimumVersion: undefined,
-    toolsFeature: ToolsFeature.TraceCommandUseBuildMode,
-  },
   [Feature.CleanupTrapCaches]: {
     defaultValue: false,
     envVar: "CODEQL_ACTION_CLEANUP_TRAP_CACHES",

src/init-action-post-helper.test.ts

@@ -368,8 +368,8 @@ async function testFailedSarifUpload(
 
   sinon.stub(workflow, "getWorkflow").resolves(actionsWorkflow);
 
-  const uploadFromActions = sinon.stub(uploadLib, "uploadFromActions");
-  uploadFromActions.resolves({
+  const uploadFiles = sinon.stub(uploadLib, "uploadFiles");
+  uploadFiles.resolves({
     sarifID: "42",
     statusReport: { raw_upload_size_bytes: 20, zipped_upload_size_bytes: 10 },
   } as uploadLib.UploadResult);
@@ -414,13 +414,14 @@ async function testFailedSarifUpload(
       );
     }
     t.true(
-      uploadFromActions.calledOnceWith(
+      uploadFiles.calledOnceWith(
         sinon.match.string,
         sinon.match.string,
         category,
         sinon.match.any,
+        sinon.match.any,
       ),
-      `Actual args were: ${uploadFromActions.args}`,
+      `Actual args were: ${uploadFiles.args}`,
     );
     t.true(
       waitForProcessing.calledOnceWith(sinon.match.any, "42", sinon.match.any, {
@@ -429,7 +430,7 @@ async function testFailedSarifUpload(
     );
   } else {
     t.true(diagnosticsExportStub.notCalled);
-    t.true(uploadFromActions.notCalled);
+    t.true(uploadFiles.notCalled);
     t.true(waitForProcessing.notCalled);
   }
   return result;

src/init-action-post-helper.ts

@@ -104,10 +104,11 @@ async function maybeUploadFailedSarif(
   }
 
   logger.info(`Uploading failed SARIF file ${sarifFile}`);
-  const uploadResult = await uploadLib.uploadFromActions(
+  const uploadResult = await uploadLib.uploadFiles(
     sarifFile,
     checkoutPath,
     category,
+    features,
     logger,
   );
   await uploadLib.waitForProcessing(

src/init-action.ts

@@ -541,7 +541,6 @@ async function run() {
       "Runner.Worker.exe",
       getOptionalInput("registries"),
       apiDetails,
-      features,
       logger,
     );
     if (tracerConfig !== undefined) {

src/init.ts

@@ -9,7 +9,7 @@ import { getOptionalInput, isSelfHostedRunner } from "./actions-util";
 import { GitHubApiCombinedDetails, GitHubApiDetails } from "./api-client";
 import { CodeQL, setupCodeQL } from "./codeql";
 import * as configUtils from "./config-utils";
-import { CodeQLDefaultVersionInfo, FeatureEnablement } from "./feature-flags";
+import { CodeQLDefaultVersionInfo } from "./feature-flags";
 import { Language, isScannedLanguage } from "./languages";
 import { Logger } from "./logging";
 import { ToolsSource } from "./setup-codeql";
@@ -71,7 +71,6 @@ export async function runInit(
   processName: string | undefined,
   registriesInput: string | undefined,
   apiDetails: GitHubApiCombinedDetails,
-  features: FeatureEnablement,
   logger: Logger,
 ): Promise<TracerConfig | undefined> {
   fs.mkdirSync(config.dbLocation, { recursive: true });
@@ -95,11 +94,10 @@ export async function runInit(
         sourceRoot,
         processName,
         qlconfigFile,
-        features,
         logger,
       ),
   );
-  return await getCombinedTracerConfig(codeql, config, features);
+  return await getCombinedTracerConfig(codeql, config);
 }
 
 export function printPathFiltersWarning(

src/tracer-config.test.ts

@@ -6,7 +6,6 @@ import test from "ava";
 import * as configUtils from "./config-utils";
 import { Language } from "./languages";
 import {
-  createFeatures,
   createTestConfig,
   mockCodeQLVersion,
   setupTests,
@@ -30,11 +29,7 @@ test("getCombinedTracerConfig - return undefined when no languages are traced la
     // No traced languages
     config.languages = [Language.javascript, Language.python];
     t.deepEqual(
-      await getCombinedTracerConfig(
-        mockCodeQLVersion("1.0.0"),
-        config,
-        createFeatures([]),
-      ),
+      await getCombinedTracerConfig(mockCodeQLVersion("1.0.0"), config),
       undefined,
     );
   });
@@ -72,7 +67,6 @@ test("getCombinedTracerConfig - with start-tracing.json environment file", async
     const result = await getCombinedTracerConfig(
       mockCodeQLVersion("1.0.0"),
       config,
-      createFeatures([]),
     );
     t.notDeepEqual(result, undefined);
 
@@ -133,7 +127,6 @@ test("getCombinedTracerConfig - with SetsCodeqlRunnerEnvVar feature enabled in C
     const result = await getCombinedTracerConfig(
       mockCodeQLVersion("1.0.0", { setsCodeqlRunnerEnvVar: true }),
       config,
-      createFeatures([]),
     );
     t.notDeepEqual(result, undefined);
 

src/tracer-config.ts

@@ -3,7 +3,6 @@ import * as path from "path";
 
 import { type CodeQL } from "./codeql";
 import { type Config } from "./config-utils";
-import { Feature, FeatureEnablement } from "./feature-flags";
 import { isTracedLanguage } from "./languages";
 import { Logger } from "./logging";
 import { ToolsFeature } from "./tools-features";
@@ -16,14 +15,23 @@ export type TracerConfig = {
 export async function shouldEnableIndirectTracing(
   codeql: CodeQL,
   config: Config,
-  features: FeatureEnablement,
 ): Promise<boolean> {
-  return (
-    (!config.buildMode ||
-      config.buildMode === BuildMode.Manual ||
-      !(await features.getValue(Feature.AutobuildDirectTracing, codeql))) &&
-    config.languages.some((l) => isTracedLanguage(l))
-  );
+  // We don't need to trace build mode none, or languages which unconditionally don't need tracing.
+  if (config.buildMode === BuildMode.None) {
+    return false;
+  }
+
+  // If the CLI supports `trace-command` with a `--build-mode`, we'll use direct tracing instead of
+  // indirect tracing.
+  if (
+    config.buildMode === BuildMode.Autobuild &&
+    (await codeql.supportsFeature(ToolsFeature.TraceCommandUseBuildMode))
+  ) {
+    return false;
+  }
+
+  // Otherwise, use direct tracing if any of the languages need to be traced.
+  return config.languages.some((l) => isTracedLanguage(l));
 }
 
 /**
@@ -39,9 +47,8 @@ export async function endTracingForCluster(
   codeql: CodeQL,
   config: Config,
   logger: Logger,
-  features: FeatureEnablement,
 ): Promise<void> {
-  if (!(await shouldEnableIndirectTracing(codeql, config, features))) return;
+  if (!(await shouldEnableIndirectTracing(codeql, config))) return;
 
   logger.info(
     "Unsetting build tracing environment variables. Subsequent steps of this job will not be traced.",
@@ -94,10 +101,10 @@ export async function getTracerConfigForCluster(
 export async function getCombinedTracerConfig(
   codeql: CodeQL,
   config: Config,
-  features: FeatureEnablement,
 ): Promise<TracerConfig | undefined> {
-  if (!(await shouldEnableIndirectTracing(codeql, config, features)))
+  if (!(await shouldEnableIndirectTracing(codeql, config))) {
     return undefined;
+  }
 
   const mainTracerConfig = await getTracerConfigForCluster(config);
 

src/upload-lib.ts

@@ -15,7 +15,7 @@ import { getGitHubVersion, wrapApiConfigurationError } from "./api-client";
 import { CodeQL, getCodeQL } from "./codeql";
 import { getConfig } from "./config-utils";
 import { EnvVar } from "./environment";
-import { FeatureEnablement, Features } from "./feature-flags";
+import { FeatureEnablement } from "./feature-flags";
 import * as fingerprints from "./fingerprints";
 import { initCodeQL } from "./init";
 import { Logger } from "./logging";
@@ -391,32 +391,6 @@ export function findSarifFilesInDir(sarifPath: string): string[] {
   return sarifFiles;
 }
 
-/**
- * Uploads a single SARIF file or a directory of SARIF files depending on what `sarifPath` refers
- * to.
- */
-export async function uploadFromActions(
-  sarifPath: string,
-  checkoutPath: string,
-  category: string | undefined,
-  logger: Logger,
-): Promise<UploadResult> {
-  return await uploadFiles(
-    getSarifFilePaths(sarifPath),
-    parseRepositoryNwo(util.getRequiredEnvParam("GITHUB_REPOSITORY")),
-    await actionsUtil.getCommitOid(checkoutPath),
-    await actionsUtil.getRef(),
-    await api.getAnalysisKey(),
-    category,
-    util.getRequiredEnvParam("GITHUB_WORKFLOW"),
-    actionsUtil.getWorkflowRunID(),
-    actionsUtil.getWorkflowRunAttempt(),
-    checkoutPath,
-    actionsUtil.getRequiredInput("matrix"),
-    logger,
-  );
-}
-
 function getSarifFilePaths(sarifPath: string) {
   if (!fs.existsSync(sarifPath)) {
     // This is always a configuration error, even for first-party runs.
@@ -563,32 +537,23 @@ export function buildPayload(
   return payloadObj;
 }
 
-// Uploads the given set of sarif files.
-// Returns true iff the upload occurred and succeeded
-async function uploadFiles(
-  sarifFiles: string[],
-  repositoryNwo: RepositoryNwo,
-  commitOid: string,
-  ref: string,
-  analysisKey: string,
+/**
+ * Uploads a single SARIF file or a directory of SARIF files depending on what `sarifPath` refers
+ * to.
+ */
+export async function uploadFiles(
+  sarifPath: string,
+  checkoutPath: string,
   category: string | undefined,
-  analysisName: string | undefined,
-  workflowRunID: number,
-  workflowRunAttempt: number,
-  sourceRoot: string,
-  environment: string | undefined,
+  features: FeatureEnablement,
   logger: Logger,
 ): Promise<UploadResult> {
+  const sarifFiles = getSarifFilePaths(sarifPath);
+
   logger.startGroup("Uploading results");
   logger.info(`Processing sarif files: ${JSON.stringify(sarifFiles)}`);
 
   const gitHubVersion = await getGitHubVersion();
-  const features = new Features(
-    gitHubVersion,
-    repositoryNwo,
-    actionsUtil.getTemporaryDirectory(),
-    logger,
-  );
 
   // Validate that the files we were asked to upload are all valid SARIF files
   for (const file of sarifFiles) {
@@ -601,8 +566,10 @@ async function uploadFiles(
     features,
     logger,
   );
-  sarif = await fingerprints.addFingerprints(sarif, sourceRoot, logger);
+  sarif = await fingerprints.addFingerprints(sarif, checkoutPath, logger);
 
+  const analysisKey = await api.getAnalysisKey();
+  const environment = actionsUtil.getRequiredInput("matrix");
   sarif = populateRunAutomationDetails(
     sarif,
     category,
@@ -618,16 +585,16 @@ async function uploadFiles(
   const sarifPayload = JSON.stringify(sarif);
   logger.debug(`Compressing serialized SARIF`);
   const zippedSarif = zlib.gzipSync(sarifPayload).toString("base64");
-  const checkoutURI = fileUrl(sourceRoot);
+  const checkoutURI = fileUrl(checkoutPath);
 
   const payload = buildPayload(
-    commitOid,
-    ref,
+    await actionsUtil.getCommitOid(checkoutPath),
+    await actionsUtil.getRef(),
     analysisKey,
-    analysisName,
+    util.getRequiredEnvParam("GITHUB_WORKFLOW"),
     zippedSarif,
-    workflowRunID,
-    workflowRunAttempt,
+    actionsUtil.getWorkflowRunID(),
+    actionsUtil.getWorkflowRunAttempt(),
     checkoutURI,
     environment,
     toolNames,
@@ -643,7 +610,11 @@ async function uploadFiles(
   logger.debug(`Number of results in upload: ${numResultInSarif}`);
 
   // Make the upload
-  const sarifID = await uploadPayload(payload, repositoryNwo, logger);
+  const sarifID = await uploadPayload(
+    payload,
+    parseRepositoryNwo(util.getRequiredEnvParam("GITHUB_REPOSITORY")),
+    logger,
+  );
 
   logger.endGroup();
 

src/upload-sarif-action.ts

@@ -1,8 +1,9 @@
 import * as core from "@actions/core";
 
 import * as actionsUtil from "./actions-util";
-import { getActionVersion } from "./actions-util";
+import { getActionVersion, getTemporaryDirectory } from "./actions-util";
 import { getGitHubVersion } from "./api-client";
+import { Features } from "./feature-flags";
 import { Logger, getActionsLogger } from "./logging";
 import { parseRepositoryNwo } from "./repository";
 import {
@@ -58,6 +59,16 @@ async function run() {
   const gitHubVersion = await getGitHubVersion();
   checkActionVersion(getActionVersion(), gitHubVersion);
 
+  const repositoryNwo = parseRepositoryNwo(
+    getRequiredEnvParam("GITHUB_REPOSITORY"),
+  );
+  const features = new Features(
+    gitHubVersion,
+    repositoryNwo,
+    getTemporaryDirectory(),
+    logger,
+  );
+
   const startingStatusReportBase = await createStatusReportBase(
     ActionName.UploadSarif,
     "starting",
@@ -71,10 +82,11 @@ async function run() {
   }
 
   try {
-    const uploadResult = await upload_lib.uploadFromActions(
+    const uploadResult = await upload_lib.uploadFiles(
       actionsUtil.getRequiredInput("sarif_file"),
       actionsUtil.getRequiredInput("checkout_path"),
       actionsUtil.getOptionalInput("category"),
+      features,
       logger,
     );
     core.setOutput("sarif-id", uploadResult.sarifID);