Add FF for config validation

2026-01-01 04:00:24 +08:00 · 2025-10-17 14:05:57 +01:00
parent d7a8ae5fdd
commit 2c8f4891d1
19 changed files with 1477 additions and 39 deletions
--- a/src/config-utils.test.ts
+++ b/src/config-utils.test.ts
@@ -148,6 +148,7 @@ test("load empty config", async (t) => {
    });

    const config = await configUtils.initConfig(
+      createFeatures([]),
      createTestInitConfigInputs({
        languagesInput: languages,
        repository: { owner: "github", repo: "example" },
@@ -187,6 +188,7 @@ test("load code quality config", async (t) => {
    });

    const config = await configUtils.initConfig(
+      createFeatures([]),
      createTestInitConfigInputs({
        analysisKinds: [AnalysisKind.CodeQuality],
        languagesInput: languages,
@@ -271,6 +273,7 @@ test("initActionState doesn't throw if there are queries configured in the repos

    await t.notThrowsAsync(async () => {
      const config = await configUtils.initConfig(
+        createFeatures([]),
        createTestInitConfigInputs({
          analysisKinds: [AnalysisKind.CodeQuality],
          languagesInput: languages,
@@ -309,6 +312,7 @@ test("loading a saved config produces the same config", async (t) => {
    t.deepEqual(await configUtils.getConfig(tempDir, logger), undefined);

    const config1 = await configUtils.initConfig(
+      createFeatures([]),
      createTestInitConfigInputs({
        languagesInput: "javascript,python",
        tempDir,
@@ -360,6 +364,7 @@ test("loading config with version mismatch throws", async (t) => {
      .returns("does-not-exist");

    const config = await configUtils.initConfig(
+      createFeatures([]),
      createTestInitConfigInputs({
        languagesInput: "javascript,python",
        tempDir,
@@ -388,6 +393,7 @@ test("load input outside of workspace", async (t) => {
  return await withTmpDir(async (tempDir) => {
    try {
      await configUtils.initConfig(
+        createFeatures([]),
        createTestInitConfigInputs({
          configFile: "../input",
          tempDir,
@@ -415,6 +421,7 @@ test("load non-local input with invalid repo syntax", async (t) => {

    try {
      await configUtils.initConfig(
+        createFeatures([]),
        createTestInitConfigInputs({
          configFile,
          tempDir,
@@ -443,6 +450,7 @@ test("load non-existent input", async (t) => {

    try {
      await configUtils.initConfig(
+        createFeatures([]),
        createTestInitConfigInputs({
          languagesInput,
          configFile,
@@ -526,6 +534,7 @@ test("load non-empty input", async (t) => {
    const configFilePath = createConfigFile(inputFileContents, tempDir);

    const actualConfig = await configUtils.initConfig(
+      createFeatures([]),
      createTestInitConfigInputs({
        languagesInput,
        buildModeInput: "none",
@@ -582,6 +591,7 @@ test("Using config input and file together, config input should be used.", async
    const languagesInput = "javascript";

    const config = await configUtils.initConfig(
+      createFeatures([]),
      createTestInitConfigInputs({
        languagesInput,
        configFile: configFilePath,
@@ -632,6 +642,7 @@ test("API client used when reading remote config", async (t) => {
    const languagesInput = "javascript";

    await configUtils.initConfig(
+      createFeatures([]),
      createTestInitConfigInputs({
        languagesInput,
        configFile,
@@ -652,6 +663,7 @@ test("Remote config handles the case where a directory is provided", async (t) =
    const repoReference = "octo-org/codeql-config/config.yaml@main";
    try {
      await configUtils.initConfig(
+        createFeatures([]),
        createTestInitConfigInputs({
          configFile: repoReference,
          tempDir,
@@ -680,6 +692,7 @@ test("Invalid format of remote config handled correctly", async (t) => {
    const repoReference = "octo-org/codeql-config/config.yaml@main";
    try {
      await configUtils.initConfig(
+        createFeatures([]),
        createTestInitConfigInputs({
          configFile: repoReference,
          tempDir,
@@ -709,6 +722,7 @@ test("No detected languages", async (t) => {

    try {
      await configUtils.initConfig(
+        createFeatures([]),
        createTestInitConfigInputs({
          tempDir,
          codeql,
@@ -731,6 +745,7 @@ test("Unknown languages", async (t) => {

    try {
      await configUtils.initConfig(
+        createFeatures([]),
        createTestInitConfigInputs({
          languagesInput,
          tempDir,
--- a/src/config-utils.ts
+++ b/src/config-utils.ts
@@ -531,6 +531,7 @@ async function loadUserConfig(
  workspacePath: string,
  apiDetails: api.GitHubApiCombinedDetails,
  tempDir: string,
+  validateConfig: boolean,
 ): Promise<UserConfig> {
  if (isLocal(configFile)) {
    if (configFile !== userConfigFromActionPath(tempDir)) {
@@ -543,9 +544,14 @@ async function loadUserConfig(
        );
      }
    }
-    return getLocalConfig(logger, configFile);
+    return getLocalConfig(logger, configFile, validateConfig);
  } else {
-    return await getRemoteConfig(logger, configFile, apiDetails);
+    return await getRemoteConfig(
+      logger,
+      configFile,
+      apiDetails,
+      validateConfig,
+    );
  }
 }

@@ -781,7 +787,10 @@ function hasQueryCustomisation(userConfig: UserConfig): boolean {
 * This will parse the config from the user input if present, or generate
 * a default config. The parsed config is then stored to a known location.
 */
-export async function initConfig(inputs: InitConfigInputs): Promise<Config> {
+export async function initConfig(
+  features: FeatureEnablement,
+  inputs: InitConfigInputs,
+): Promise<Config> {
  const { logger, tempDir } = inputs;

  // if configInput is set, it takes precedence over configFile
@@ -801,12 +810,14 @@ export async function initConfig(inputs: InitConfigInputs): Promise<Config> {
    logger.debug("No configuration file was provided");
  } else {
    logger.debug(`Using configuration file: ${inputs.configFile}`);
+    const validateConfig = await features.getValue(Feature.ValidateDbConfig);
    userConfig = await loadUserConfig(
      logger,
      inputs.configFile,
      inputs.workspacePath,
      inputs.apiDetails,
      tempDir,
+      validateConfig,
    );
  }

@@ -900,7 +911,11 @@ function isLocal(configPath: string): boolean {
  return configPath.indexOf("@") === -1;
 }

-function getLocalConfig(logger: Logger, configFile: string): UserConfig {
+function getLocalConfig(
+  logger: Logger,
+  configFile: string,
+  validateConfig: boolean,
+): UserConfig {
  // Error if the file does not exist
  if (!fs.existsSync(configFile)) {
    throw new ConfigurationError(
@@ -912,6 +927,7 @@ function getLocalConfig(logger: Logger, configFile: string): UserConfig {
    logger,
    configFile,
    fs.readFileSync(configFile, "utf-8"),
+    validateConfig,
  );
 }

@@ -919,6 +935,7 @@ async function getRemoteConfig(
  logger: Logger,
  configFile: string,
  apiDetails: api.GitHubApiCombinedDetails,
+  validateConfig: boolean,
 ): Promise<UserConfig> {
  // retrieve the various parts of the config location, and ensure they're present
  const format = new RegExp(
@@ -958,6 +975,7 @@ async function getRemoteConfig(
    logger,
    configFile,
    Buffer.from(fileContents, "base64").toString("binary"),
+    validateConfig,
  );
 }

--- a/src/config/db-config.test.ts
+++ b/src/config/db-config.test.ts
@@ -409,6 +409,7 @@ test("parseUserConfig - successfully parses valid YAML", (t) => {
      - uses: foo
    some-unknown-option: true
    `,
+    true,
  );
  t.truthy(result);
  if (t.truthy(result["paths-ignore"])) {
@@ -433,6 +434,7 @@ test("parseUserConfig - throws a ConfigurationError if the file is not valid YAM
         queries:
         - foo
        `,
+        true,
      ),
    {
      instanceOf: ConfigurationError,
@@ -454,6 +456,7 @@ test("parseUserConfig - throws a ConfigurationError if validation fails", (t) =>
         - "some/path"
        queries: true
        `,
+        true,
      ),
    {
      instanceOf: ConfigurationError,
@@ -465,3 +468,21 @@ test("parseUserConfig - throws a ConfigurationError if validation fails", (t) =>
  const expectedMessages = ["instance.queries is not of a type(s) array"];
  checkExpectedLogMessages(t, loggedMessages, expectedMessages);
 });
+
+test("parseUserConfig - throws no ConfigurationError if validation should fail, but feature is disabled", (t) => {
+  const loggedMessages: LoggedMessage[] = [];
+  const logger = getRecordingLogger(loggedMessages);
+
+  t.notThrows(() =>
+    dbConfig.parseUserConfig(
+      logger,
+      "test",
+      `
+        paths-ignore:
+         - "some/path"
+        queries: true
+        `,
+      false,
+    ),
+  );
+});
--- a/src/config/db-config.ts
+++ b/src/config/db-config.ts
@@ -483,6 +483,7 @@ export function generateCodeScanningConfig(
 * @param logger The logger to use.
 * @param pathInput The path to the file where `contents` was obtained from, for use in error messages.
 * @param contents The string contents of a YAML file to try and parse as a `UserConfig`.
+ * @param validateConfig Whether to validate the configuration file against the schema.
 * @returns The `UserConfig` corresponding to `contents`, if parsing was successful.
 * @throws A `ConfigurationError` if parsing failed.
 */
@@ -490,6 +491,7 @@ export function parseUserConfig(
  logger: Logger,
  pathInput: string,
  contents: string,
+  validateConfig: boolean,
 ): UserConfig {
  try {
    const schema =
@@ -497,18 +499,21 @@ export function parseUserConfig(
      require("../../src/db-config-schema.json") as jsonschema.Schema;

    const doc = yaml.load(contents);
-    const result = new jsonschema.Validator().validate(doc, schema);

-    if (result.errors.length > 0) {
-      for (const error of result.errors) {
-        logger.error(error.stack);
+    if (validateConfig) {
+      const result = new jsonschema.Validator().validate(doc, schema);
+
+      if (result.errors.length > 0) {
+        for (const error of result.errors) {
+          logger.error(error.stack);
+        }
+        throw new ConfigurationError(
+          errorMessages.getInvalidConfigFileMessage(
+            pathInput,
+            result.errors.map((e) => e.stack),
+          ),
+        );
      }
-      throw new ConfigurationError(
-        errorMessages.getInvalidConfigFileMessage(
-          pathInput,
-          result.errors.map((e) => e.stack),
-        ),
-      );
    }

    return doc as UserConfig;
--- a/src/feature-flags.ts
+++ b/src/feature-flags.ts
@@ -77,6 +77,7 @@ export enum Feature {
  QaTelemetryEnabled = "qa_telemetry_enabled",
  ResolveSupportedLanguagesUsingCli = "resolve_supported_languages_using_cli",
  UseRepositoryProperties = "use_repository_properties",
+  ValidateDbConfig = "validate_db_config",
 }

 export const featureConfig: Record<
@@ -287,6 +288,11 @@ export const featureConfig: Record<
    envVar: "CODEQL_ACTION_JAVA_MINIMIZE_DEPENDENCY_JARS",
    minimumVersion: "2.23.0",
  },
+  [Feature.ValidateDbConfig]: {
+    defaultValue: false,
+    envVar: "CODEQL_ACTION_VALIDATE_DB_CONFIG",
+    minimumVersion: undefined,
+  },
 };

 /**
--- a/src/init-action.ts
+++ b/src/init-action.ts
@@ -325,7 +325,7 @@ async function run() {
    }

    analysisKinds = await getAnalysisKinds(logger);
-    config = await initConfig({
+    config = await initConfig(features, {
      analysisKinds,
      languagesInput: getOptionalInput("languages"),
      queriesInput: getOptionalInput("queries"),
--- a/src/init.ts
+++ b/src/init.ts
@@ -61,10 +61,11 @@ export async function initCodeQL(
 }

 export async function initConfig(
+  features: FeatureEnablement,
  inputs: configUtils.InitConfigInputs,
 ): Promise<configUtils.Config> {
  return await withGroupAsync("Load language configuration", async () => {
-    return await configUtils.initConfig(inputs);
+    return await configUtils.initConfig(features, inputs);
  });
 }