Allow AnalysisKind-specific configuration sections

src/config-utils.test.ts

@@ -12,7 +12,7 @@ import { AnalysisKind, supportedAnalysisKinds } from "./analyses";
 import * as api from "./api-client";
 import { CachingKind } from "./caching-utils";
 import { createStubCodeQL } from "./codeql";
-import { UserConfig } from "./config/db-config";
+import { UserConfig, UserConfigWithActionExtensions } from "./config/db-config";
 import * as file from "./config/file";
 import * as configUtils from "./config-utils";
 import * as errorMessages from "./error-messages";
@@ -2544,6 +2544,7 @@ test("loadUserConfig - loads local configuration files", async (t) => {
       ) =>
         configUtils.loadUserConfig(
           actionState,
+          [AnalysisKind.CodeScanning],
           filePath,
           workspaceDir,
           SAMPLE_DOTCOM_API_DETAILS,
@@ -2587,7 +2588,13 @@ test.serial("loadUserConfig - loads remote configuration files", async (t) => {
 
     const remoteAddress = "owner/repo/file@ref";
     await callee(configUtils.loadUserConfig)
-      .withArgs(remoteAddress, tmpDir, SAMPLE_DOTCOM_API_DETAILS, tmpDir)
+      .withArgs(
+        [AnalysisKind.CodeScanning],
+        remoteAddress,
+        tmpDir,
+        SAMPLE_DOTCOM_API_DETAILS,
+        tmpDir,
+      )
       .passes(t.deepEqual, {});
 
     t.true(
@@ -2665,6 +2672,7 @@ test.serial(
 
         // Prepare the test call to `loadUserConfig`.
         const targetWithArgs = target.withArgs(
+          [AnalysisKind.CodeScanning],
           address,
           tmpDir,
           SAMPLE_DOTCOM_API_DETAILS,
@@ -2705,3 +2713,57 @@ test.serial(
     });
   },
 );
+
+test("loadUserConfig - applies analysis-specific settings", async (t) => {
+  await withTmpDir(async (workspaceDir) => {
+    await withTmpDir(async (tmpDir) => {
+      // Construct the test target.
+      const loadUserConfig = (
+        actionState: ActionState<["Logger", "Env", "FeatureFlags"]>,
+        filePath: string,
+        analysisKind: AnalysisKind,
+      ) =>
+        configUtils.loadUserConfig(
+          actionState,
+          [analysisKind],
+          filePath,
+          workspaceDir,
+          SAMPLE_DOTCOM_API_DETAILS,
+          tmpDir,
+        );
+      const target = callee(loadUserConfig);
+
+      for (const analysisKind of Object.values(AnalysisKind)) {
+        const analysisSpecificConfig: UserConfig = {
+          paths: ["custom"],
+          queries: [{ uses: "foo" }],
+        };
+        const baseConfig: UserConfig = { "threat-models": ["remote"] };
+
+        // Write a local configuration file with an `analysisKind`-specific section.
+        const configPath = path.join(workspaceDir, "codeql.yml");
+        fs.writeFileSync(
+          configPath,
+          JSON.stringify({
+            ...baseConfig,
+            [analysisKind]: analysisSpecificConfig,
+          } satisfies UserConfigWithActionExtensions),
+          "utf8",
+        );
+
+        // The resulting configuration should be the `baseConfig` with the `analysisSpecificConfig`
+        // merged into it if the FF is enabled.
+        await target
+          .withArgs(configPath, analysisKind)
+          .withFeatures([Feature.ScopedConfigurations])
+          .passes(t.deepEqual, { ...baseConfig, ...analysisSpecificConfig });
+
+        // Without the FF, it should be the `baseConfig`. This also validates that the
+        // `AnalysisKind`-specific sections are removed even if the FF is off.
+        await target
+          .withArgs(configPath, analysisKind)
+          .passes(t.deepEqual, baseConfig);
+      }
+    });
+  });
+});

src/config-utils.ts

@@ -24,11 +24,13 @@ import { getCachingKind } from "./caching-utils";
 import { type CodeQL } from "./codeql";
 import { type Config } from "./config/action-config";
 import {
+  applyAnalysisKindConfig,
   calculateAugmentation,
   ExcludeQueryFilter,
   generateCodeScanningConfig,
   mergeDefaultSetupAndUserConfigs,
   parseUserConfig,
+  removeAnalysisKindConfigs,
   UserConfig,
 } from "./config/db-config";
 import {
@@ -484,11 +486,14 @@ async function downloadCacheWithTime(
  */
 export async function loadUserConfig(
   actionState: ActionState<["Logger", "Env", "FeatureFlags"]>,
+  analysisKinds: AnalysisKind[],
   configFile: string,
   workspacePath: string,
   apiDetails: api.GitHubApiCombinedDetails,
   tempDir: string,
 ): Promise<UserConfig> {
+  let config: UserConfig;
+
   if (isLocal(configFile)) {
     if (configFile !== userConfigFromActionPath(tempDir)) {
       // If the config file is not generated by the Action, it should be relative to the workspace.
@@ -503,16 +508,37 @@ export async function loadUserConfig(
     const validateConfig = await actionState.features.getValue(
       Feature.ValidateDbConfig,
     );
-    return getLocalConfig(actionState.logger, configFile, validateConfig);
+    config = getLocalConfig(actionState.logger, configFile, validateConfig);
   } else {
     // Drop the explicit prefix if it is present. Since `REMOTE_PATH_PREFIX` is chosen
     // to not conflict with permissible characters in "owner" or "repo" components,
     // this does not risk removing valid parts of either component by accident.
     if (isExplicitRemotePath(configFile)) {
       configFile = configFile.substring(REMOTE_PATH_PREFIX.length);
     }
-    return await getRemoteConfig(actionState, configFile, apiDetails);
+    config = await getRemoteConfig(actionState, configFile, apiDetails);
   }
+
+  // If the `ScopedConfigurations` feature is enabled, we allow options specific to the
+  // current `AnalysisKind` to be applied on top of a base configuration.
+  const allowScopedConfig = await actionState.features.getValue(
+    Feature.ScopedConfigurations,
+  );
+
+  if (allowScopedConfig && analysisKinds.length === 1) {
+    // Apply the `analysisKind`-specific configuration.
+    return applyAnalysisKindConfig(analysisKinds[0], config);
+  } else if (allowScopedConfig) {
+    actionState.logger.info(
+      `Ignoring '${Feature.ScopedConfigurations}' feature, because multiple analysis kinds are enabled.`,
+    );
+  }
+
+  // Remove `AnalysisKind`-specific sections from `config` even if the FF is not enabled,
+  // because the CLI will warn about unknown keys in the configuration. This is to guard
+  // against the case where we ship the feature, enable the FF, start using the new keys,
+  // and then need to roll the FF back.
+  return removeAnalysisKindConfigs(config);
 }
 
 /**
@@ -1071,6 +1097,7 @@ export async function determineUserConfig(
       );
       const fromConfigFile = await loadUserConfig(
         action,
+        inputs.analysisKinds,
         inputs.configFile,
         inputs.workspacePath,
         inputs.apiDetails,
@@ -1118,6 +1145,7 @@ export async function determineUserConfig(
     action.logger.debug(`Using configuration file: ${inputs.configFile}`);
     return await loadUserConfig(
       action,
+      inputs.analysisKinds,
       inputs.configFile,
       inputs.workspacePath,
       inputs.apiDetails,

src/config/db-config.test.ts

@@ -1,5 +1,6 @@
 import test, { ExecutionContext } from "ava";
 
+import { AnalysisKind } from "../analyses";
 import { RepositoryProperties } from "../feature-flags/properties";
 import { BuiltInLanguage, Language } from "../languages";
 import { getRunnerLogger } from "../logging";
@@ -14,6 +15,117 @@ import { ConfigurationError, prettyPrintPack } from "../util";
 
 import * as dbConfig from "./db-config";
 
+function makeUserConfigWithActionExtensions(options?: {
+  analysisConfig?: dbConfig.UserConfig;
+  analysisKinds?: AnalysisKind[];
+  baseConfig?: dbConfig.UserConfig;
+}) {
+  const testConfig = { ...options?.baseConfig };
+
+  const allAnalysisKinds = Object.values(AnalysisKind);
+  const analysisKinds = options?.analysisKinds ?? allAnalysisKinds;
+  for (const analysisKind of analysisKinds) {
+    testConfig[analysisKind] = { ...options?.analysisConfig };
+  }
+
+  return testConfig as dbConfig.UserConfigWithActionExtensions;
+}
+
+test("hasAnalysisKindKey - finds `AnalysisKind` sections", async (t) => {
+  const testConfig = makeUserConfigWithActionExtensions();
+
+  for (const analysisKind of Object.values(AnalysisKind)) {
+    // Check that it finds the `analysisKind`-specific section among multiple.
+    t.true(dbConfig.hasAnalysisKindKey(analysisKind, testConfig));
+    // And for just the `analysisKind`-specific section.
+    t.true(dbConfig.hasAnalysisKindKey(analysisKind, { [analysisKind]: {} }));
+  }
+});
+
+test("hasAnalysisKindKey - returns false if there is no relevant section", async (t) => {
+  const testConfig = makeUserConfigWithActionExtensions();
+
+  for (const analysisKind of Object.values(AnalysisKind)) {
+    // Clone the `testConfig` and remove the `analysisKind`-specific key.
+    const testConfigClone = { ...testConfig };
+    delete testConfigClone[analysisKind];
+
+    // Check that it doesn't find an `analysisKind`-specific section.
+    t.false(dbConfig.hasAnalysisKindKey(analysisKind, testConfigClone));
+  }
+});
+
+test("applyAnalysisKindConfig - applies analysis-specific settings", async (t) => {
+  for (const analysisKind of Object.values(AnalysisKind)) {
+    const testConfig = makeUserConfigWithActionExtensions({
+      analysisKinds: [analysisKind],
+      analysisConfig: {
+        "paths-ignore": [],
+      },
+      baseConfig: {
+        "paths-ignore": ["a", "b", "c"],
+        "threat-models": ["local"],
+      },
+    });
+    const clonedTestConfig = { ...testConfig };
+
+    const result = dbConfig.applyAnalysisKindConfig(analysisKind, testConfig);
+
+    // The `analysisKind`-specific section should have been removed.
+    // `threat-models` should be unchanged from the `baseConfig`.
+    // `paths-ignore` should have been overwritten with the `analysisKind`-specific config.
+    t.deepEqual(result, {
+      "threat-models": ["local"],
+      "paths-ignore": [],
+    });
+
+    // The input `testConfig` should not have changed.
+    t.deepEqual(testConfig, clonedTestConfig);
+  }
+});
+
+test("applyAnalysisKindConfig - removes all `AnalysisKind`-specific keys", async (t) => {
+  for (const analysisKind of Object.values(AnalysisKind)) {
+    const testConfig = makeUserConfigWithActionExtensions({
+      analysisConfig: {
+        "paths-ignore": ["a", "b", "c"],
+      },
+      baseConfig: {},
+    });
+
+    const result = dbConfig.applyAnalysisKindConfig(analysisKind, testConfig);
+
+    // All `AnalysisKind`-specific sections should have been removed.
+    // `paths-ignore` should have been overwritten with the analysis-specific config.
+    t.deepEqual(result, {
+      "paths-ignore": ["a", "b", "c"],
+    });
+  }
+});
+
+test("applyAnalysisKindConfig - returns the `baseConfig` if there is no `analysisKind`-specific key", async (t) => {
+  const baseConfig = {
+    "disable-default-queries": true,
+    "paths-ignore": ["a", "b", "c"],
+    queries: [{ name: "foo", uses: "something" }],
+    packs: { test: ["bar"] },
+    "default-setup": { org: { "model-packs": ["pack"] } },
+  } satisfies dbConfig.UserConfig;
+
+  const testConfig = makeUserConfigWithActionExtensions({
+    analysisKinds: [],
+    baseConfig,
+  });
+
+  const result = dbConfig.applyAnalysisKindConfig(
+    AnalysisKind.CodeScanning,
+    testConfig,
+  );
+
+  // The result should be the same as the `baseConfig`.
+  t.deepEqual(result, baseConfig);
+});
+
 /**
  * Test macro for ensuring the packs block is valid
  */