Merge pull request #2245 from github/henrymercer/ignore-already-specified-flags

Ensure `--overwrite` flag is only passed once

CHANGELOG.md

@@ -7,6 +7,7 @@ Note that the only difference between `v2` and `v3` of the CodeQL Action is the
 ## [UNRELEASED]
 
 - We are rolling out a feature in April/May 2024 that improves the reliability and performance of analyzing code when analyzing a compiled language with the `autobuild` [build mode](https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/codeql-code-scanning-for-compiled-languages#codeql-build-modes). [#2235](https://github.com/github/codeql-action/pull/2235)
+- Fix a bug where the `init` Action would fail if `--overwrite` was specified in `CODEQL_ACTION_EXTRA_OPTIONS`. [#2245](https://github.com/github/codeql-action/pull/2245)
 
 ## 3.25.0 - 15 Apr 2024
 

lib/codeql.js

@@ -292,7 +292,9 @@ async function getCodeQLForCmd(cmd, checkVersion) {
                 `--source-root=${sourceRoot}`,
                 ...(await getLanguageAliasingArguments(this)),
                 ...extraArgs,
-                ...getExtraOptionsFromEnv(["database", "init"]),
+                ...getExtraOptionsFromEnv(["database", "init"], {
+                    ignoringOptions: ["--overwrite"],
+                }),
             ], { stdin: externalRepositoryToken });
         },
         async runAutobuild(config, language, features) {
@@ -463,7 +465,9 @@ async function getCodeQLForCmd(cmd, checkVersion) {
                 "--expect-discarded-cache",
                 "--min-disk-free=1024", // Try to leave at least 1GB free
                 "-v",
-                ...getExtraOptionsFromEnv(["database", "run-queries"]),
+                ...getExtraOptionsFromEnv(["database", "run-queries"], {
+                    ignoringOptions: ["--expect-discarded-cache"],
+                }),
             ];
             if (await util.codeQlVersionAbove(this, feature_flags_1.CODEQL_VERSION_FINE_GRAINED_PARALLELISM)) {
                 codeqlArgs.push("--intra-layer-parallelism");
@@ -713,10 +717,13 @@ async function getCodeQLForCmd(cmd, checkVersion) {
 exports.getCodeQLForCmd = getCodeQLForCmd;
 /**
  * Gets the options for `path` of `options` as an array of extra option strings.
+ *
+ * @param ignoringOptions Options that should be ignored, for example because they have already
+ *                        been passed and it is an error to pass them more than once.
  */
-function getExtraOptionsFromEnv(paths) {
+function getExtraOptionsFromEnv(paths, { ignoringOptions } = {}) {
     const options = util.getExtraOptionsEnvParam();
-    return getExtraOptions(options, paths, []);
+    return getExtraOptions(options, paths, []).filter((option) => !ignoringOptions?.includes(option));
 }
 /**
  * Gets `options` as an array of extra option strings.

lib/codeql.test.js

@@ -613,6 +613,24 @@ for (const { codeqlVersion, flagPassed, githubVersion, negativeFlagPassed, } of
             "Exit code was 32 and last log line was: line5\\. See the logs for more details\\."),
     });
 });
+(0, ava_1.default)("Avoids duplicating --overwrite flag if specified in CODEQL_ACTION_EXTRA_OPTIONS", async (t) => {
+    const runnerConstructorStub = stubToolRunnerConstructor();
+    const codeqlObject = await codeql.getCodeQLForTesting();
+    sinon.stub(codeqlObject, "getVersion").resolves((0, testing_utils_1.makeVersionInfo)("2.12.6"));
+    // safeWhich throws because of the test CodeQL object.
+    sinon.stub(safeWhich, "safeWhich").resolves("");
+    process.env["CODEQL_ACTION_EXTRA_OPTIONS"] =
+        '{ "database": { "init": ["--overwrite"] } }';
+    await codeqlObject.databaseInitCluster(stubConfig, "sourceRoot", undefined, undefined, (0, testing_utils_1.createFeatures)([]), (0, logging_1.getRunnerLogger)(false));
+    t.true(runnerConstructorStub.calledOnce);
+    const args = runnerConstructorStub.firstCall.args[1];
+    t.is(args.filter((option) => option === "--overwrite").length, 1, "--overwrite should only be passed once");
+    // Clean up
+    const configArg = args.find((arg) => arg.startsWith("--codescanning-config="));
+    t.truthy(configArg, "Should have injected a codescanning config");
+    const configFile = configArg.split("=")[1];
+    await (0, del_1.default)(configFile, { force: true });
+});
 function stubToolRunnerConstructor(exitCode = 0, stderr) {
     const runnerObjectStub = sinon.createStubInstance(toolrunner.ToolRunner);
     const runnerConstructorStub = sinon.stub(toolrunner, "ToolRunner");

src/codeql.test.ts

@@ -989,6 +989,42 @@ test("runTool outputs last line of stderr if fatal error could not be found", as
   );
 });
 
+test("Avoids duplicating --overwrite flag if specified in CODEQL_ACTION_EXTRA_OPTIONS", async (t) => {
+  const runnerConstructorStub = stubToolRunnerConstructor();
+  const codeqlObject = await codeql.getCodeQLForTesting();
+  sinon.stub(codeqlObject, "getVersion").resolves(makeVersionInfo("2.12.6"));
+  // safeWhich throws because of the test CodeQL object.
+  sinon.stub(safeWhich, "safeWhich").resolves("");
+
+  process.env["CODEQL_ACTION_EXTRA_OPTIONS"] =
+    '{ "database": { "init": ["--overwrite"] } }';
+
+  await codeqlObject.databaseInitCluster(
+    stubConfig,
+    "sourceRoot",
+    undefined,
+    undefined,
+    createFeatures([]),
+    getRunnerLogger(false),
+  );
+
+  t.true(runnerConstructorStub.calledOnce);
+  const args = runnerConstructorStub.firstCall.args[1] as string[];
+  t.is(
+    args.filter((option: string) => option === "--overwrite").length,
+    1,
+    "--overwrite should only be passed once",
+  );
+
+  // Clean up
+  const configArg = args.find((arg: string) =>
+    arg.startsWith("--codescanning-config="),
+  );
+  t.truthy(configArg, "Should have injected a codescanning config");
+  const configFile = configArg!.split("=")[1];
+  await del(configFile, { force: true });
+});
+
 export function stubToolRunnerConstructor(
   exitCode: number = 0,
   stderr?: string,

src/codeql.ts

@@ -627,7 +627,9 @@ export async function getCodeQLForCmd(
           `--source-root=${sourceRoot}`,
           ...(await getLanguageAliasingArguments(this)),
           ...extraArgs,
-          ...getExtraOptionsFromEnv(["database", "init"]),
+          ...getExtraOptionsFromEnv(["database", "init"], {
+            ignoringOptions: ["--overwrite"],
+          }),
         ],
         { stdin: externalRepositoryToken },
       );
@@ -835,7 +837,9 @@ export async function getCodeQLForCmd(
         "--expect-discarded-cache",
         "--min-disk-free=1024", // Try to leave at least 1GB free
         "-v",
-        ...getExtraOptionsFromEnv(["database", "run-queries"]),
+        ...getExtraOptionsFromEnv(["database", "run-queries"], {
+          ignoringOptions: ["--expect-discarded-cache"],
+        }),
       ];
       if (
         await util.codeQlVersionAbove(
@@ -1174,10 +1178,18 @@ export async function getCodeQLForCmd(
 
 /**
  * Gets the options for `path` of `options` as an array of extra option strings.
+ *
+ * @param ignoringOptions Options that should be ignored, for example because they have already
+ *                        been passed and it is an error to pass them more than once.
  */
-function getExtraOptionsFromEnv(paths: string[]) {
+function getExtraOptionsFromEnv(
+  paths: string[],
+  { ignoringOptions }: { ignoringOptions?: string[] } = {},
+) {
   const options: ExtraOptions = util.getExtraOptionsEnvParam();
-  return getExtraOptions(options, paths, []);
+  return getExtraOptions(options, paths, []).filter(
+    (option) => !ignoringOptions?.includes(option),
+  );
 }
 
 /**