Merge branch 'main' into tracer-config

2025-12-30 03:00:13 +08:00 · 2020-08-25 15:05:15 +01:00
parent 8efabe9ec9 abddb8bf00
commit 2df51e04f7
13 changed files with 461 additions and 40 deletions
--- a/src/config-utils.test.ts
+++ b/src/config-utils.test.ts
@@ -273,6 +273,195 @@ test("default queries are used", async t => {
  });
 });

+test("Queries can be specified in config file", async t => {
+  return await util.withTmpDir(async tmpDir => {
+    process.env['RUNNER_TEMP'] = tmpDir;
+    process.env['GITHUB_WORKSPACE'] = tmpDir;
+
+    const inputFileContents = `
+      name: my config
+      queries:
+        - uses: ./foo`;
+
+    fs.writeFileSync(path.join(tmpDir, 'input'), inputFileContents, 'utf8');
+    setInput('config-file', 'input');
+
+    fs.mkdirSync(path.join(tmpDir, 'foo'));
+
+    const resolveQueriesArgs: {queries: string[], extraSearchPath: string | undefined}[] = [];
+    const codeQL = setCodeQL({
+      resolveQueries: async function(queries: string[], extraSearchPath: string | undefined) {
+        resolveQueriesArgs.push({queries, extraSearchPath});
+        // Return what we're given, just in the right format for a resolved query
+        // This way we can test by seeing which returned items are in the final
+        // configuration.
+        const dummyResolvedQueries = {};
+        queries.forEach(q => { dummyResolvedQueries[q] = {}; });
+        return {
+          byLanguage: {
+            'javascript': dummyResolvedQueries,
+          },
+          noDeclaredLanguage: {},
+          multipleDeclaredLanguages: {},
+        };
+      },
+    });
+
+    setInput('languages', 'javascript');
+
+    const config = await configUtils.initConfig(tmpDir, tmpDir, codeQL);
+
+    // Check resolveQueries was called correctly
+    // It'll be called once for the default queries
+    // and once for `./foo` from the config file.
+    t.deepEqual(resolveQueriesArgs.length, 2);
+    t.deepEqual(resolveQueriesArgs[1].queries.length, 1);
+    t.regex(resolveQueriesArgs[1].queries[0], /.*\/foo$/);
+
+    // Now check that the end result contains the default queries and the query from config
+    t.deepEqual(config.queries['javascript'].length, 2);
+    t.regex(config.queries['javascript'][0], /javascript-code-scanning.qls$/);
+    t.regex(config.queries['javascript'][1], /.*\/foo$/);
+  });
+});
+
+test("Queries from config file can be overridden in workflow file", async t => {
+  return await util.withTmpDir(async tmpDir => {
+    process.env['RUNNER_TEMP'] = tmpDir;
+    process.env['GITHUB_WORKSPACE'] = tmpDir;
+
+    const inputFileContents = `
+      name: my config
+      queries:
+        - uses: ./foo`;
+
+    fs.writeFileSync(path.join(tmpDir, 'input'), inputFileContents, 'utf8');
+    setInput('config-file', 'input');
+
+    // This config item should take precedence over the config file but shouldn't affect the default queries.
+    setInput('queries', './override');
+
+    fs.mkdirSync(path.join(tmpDir, 'foo'));
+    fs.mkdirSync(path.join(tmpDir, 'override'));
+
+    const resolveQueriesArgs: {queries: string[], extraSearchPath: string | undefined}[] = [];
+    const codeQL = setCodeQL({
+      resolveQueries: async function(queries: string[], extraSearchPath: string | undefined) {
+        resolveQueriesArgs.push({queries, extraSearchPath});
+        // Return what we're given, just in the right format for a resolved query
+        // This way we can test overriding by seeing which returned items are in
+        // the final configuration.
+        const dummyResolvedQueries = {};
+        queries.forEach(q => { dummyResolvedQueries[q] = {}; });
+        return {
+          byLanguage: {
+            'javascript': dummyResolvedQueries,
+          },
+          noDeclaredLanguage: {},
+          multipleDeclaredLanguages: {},
+        };
+      },
+    });
+
+    setInput('languages', 'javascript');
+
+    const config = await configUtils.initConfig(tmpDir, tmpDir, codeQL);
+
+    // Check resolveQueries was called correctly
+    // It'll be called once for the default queries and once for `./override`,
+    // but won't be called for './foo' from the config file.
+    t.deepEqual(resolveQueriesArgs.length, 2);
+    t.deepEqual(resolveQueriesArgs[1].queries.length, 1);
+    t.regex(resolveQueriesArgs[1].queries[0], /.*\/override$/);
+
+    // Now check that the end result contains only the default queries and the override query
+    t.deepEqual(config.queries['javascript'].length, 2);
+    t.regex(config.queries['javascript'][0], /javascript-code-scanning.qls$/);
+    t.regex(config.queries['javascript'][1], /.*\/override$/);
+  });
+});
+
+test("Multiple queries can be specified in workflow file, no config file required", async t => {
+  return await util.withTmpDir(async tmpDir => {
+    process.env['RUNNER_TEMP'] = tmpDir;
+    process.env['GITHUB_WORKSPACE'] = tmpDir;
+
+    fs.mkdirSync(path.join(tmpDir, 'override1'));
+    fs.mkdirSync(path.join(tmpDir, 'override2'));
+
+    setInput('queries', './override1,./override2');
+
+    const resolveQueriesArgs: {queries: string[], extraSearchPath: string | undefined}[] = [];
+    const codeQL = setCodeQL({
+      resolveQueries: async function(queries: string[], extraSearchPath: string | undefined) {
+        resolveQueriesArgs.push({queries, extraSearchPath});
+        // Return what we're given, just in the right format for a resolved query
+        // This way we can test overriding by seeing which returned items are in
+        // the final configuration.
+        const dummyResolvedQueries = {};
+        queries.forEach(q => { dummyResolvedQueries[q] = {}; });
+        return {
+          byLanguage: {
+            'javascript': dummyResolvedQueries,
+          },
+          noDeclaredLanguage: {},
+          multipleDeclaredLanguages: {},
+        };
+      },
+    });
+
+    setInput('languages', 'javascript');
+
+    const config = await configUtils.initConfig(tmpDir, tmpDir, codeQL);
+
+    // Check resolveQueries was called correctly:
+    // It'll be called once for the default queries,
+    // and then once for each of the two queries from the workflow
+    t.deepEqual(resolveQueriesArgs.length, 3);
+    t.deepEqual(resolveQueriesArgs[1].queries.length, 1);
+    t.deepEqual(resolveQueriesArgs[2].queries.length, 1);
+    t.regex(resolveQueriesArgs[1].queries[0], /.*\/override1$/);
+    t.regex(resolveQueriesArgs[2].queries[0], /.*\/override2$/);
+
+    // Now check that the end result contains both the queries from the workflow, as well as the defaults
+    t.deepEqual(config.queries['javascript'].length, 3);
+    t.regex(config.queries['javascript'][0], /javascript-code-scanning.qls$/);
+    t.regex(config.queries['javascript'][1], /.*\/override1$/);
+    t.regex(config.queries['javascript'][2], /.*\/override2$/);
+  });
+});
+
+test("Invalid queries in workflow file handled correctly", async t => {
+  return await util.withTmpDir(async tmpDir => {
+    process.env['RUNNER_TEMP'] = tmpDir;
+    process.env['GITHUB_WORKSPACE'] = tmpDir;
+
+    setInput('queries', 'foo/bar@v1@v3');
+    setInput('languages', 'javascript');
+
+    // This function just needs to be type-correct; it doesn't need to do anything,
+    // since we're deliberately passing in invalid data
+    const codeQL = setCodeQL({
+      resolveQueries: async function(_queries: string[], _extraSearchPath: string | undefined) {
+        return {
+          byLanguage: {
+            'javascript': {},
+          },
+          noDeclaredLanguage: {},
+          multipleDeclaredLanguages: {},
+        };
+      },
+    });
+
+    try {
+      await configUtils.initConfig(tmpDir, tmpDir, codeQL);
+      t.fail('initConfig did not throw error');
+    } catch (err) {
+      t.deepEqual(err, new Error(configUtils.getQueryUsesInvalid(undefined, "foo/bar@v1@v3")));
+    }
+  });
+});
+
 test("API client used when reading remote config", async t => {
  return await util.withTmpDir(async tmpDir => {
    process.env['RUNNER_TEMP'] = tmpDir;
@@ -310,7 +499,7 @@ test("API client used when reading remote config", async t => {
    const spyGetContents = mockGetContents(dummyResponse);

    // Create checkout directory for remote queries repository
-    fs.mkdirSync(path.join(tmpDir, 'foo/bar'), { recursive: true });
+    fs.mkdirSync(path.join(tmpDir, 'foo/bar/dev'), { recursive: true });

    setInput('config-file', 'octo-org/codeql-config/config.yaml@main');
    setInput('languages', 'javascript');
--- a/src/config-utils.ts
+++ b/src/config-utils.ts
@@ -160,11 +160,11 @@ const builtinSuites = ['security-extended', 'security-and-quality'] as const;
 * Throws an error if suiteName is not a valid builtin suite.
 */
 async function addBuiltinSuiteQueries(
-  configFile: string,
  languages: string[],
  codeQL: CodeQL,
  resultMap: { [language: string]: string[] },
-  suiteName: string) {
+  suiteName: string,
+  configFile?: string) {

  const suite = builtinSuites.find((suite) => suite === suiteName);
  if (!suite) {
@@ -179,10 +179,10 @@ async function addBuiltinSuiteQueries(
 * Retrieve the set of queries at localQueryPath and add them to resultMap.
 */
 async function addLocalQueries(
-  configFile: string,
  codeQL: CodeQL,
  resultMap: { [language: string]: string[] },
-  localQueryPath: string) {
+  localQueryPath: string,
+  configFile?: string) {

  // Resolve the local path against the workspace so that when this is
  // passed to codeql it resolves to exactly the path we expect it to resolve to.
@@ -212,11 +212,11 @@ async function addLocalQueries(
 * Retrieve the set of queries at the referenced remote repo and add them to resultMap.
 */
 async function addRemoteQueries(
-  configFile: string,
  codeQL: CodeQL,
  resultMap: { [language: string]: string[] },
  queryUses: string,
-  tempDir: string) {
+  tempDir: string,
+  configFile?: string) {

  let tok = queryUses.split('@');
  if (tok.length !== 2) {
@@ -257,12 +257,12 @@ async function addRemoteQueries(
 * a finite set of hardcoded terms for builtin suites.
 */
 async function parseQueryUses(
-  configFile: string,
  languages: string[],
  codeQL: CodeQL,
  resultMap: { [language: string]: string[] },
  queryUses: string,
-  tempDir: string) {
+  tempDir: string,
+  configFile?: string) {

  queryUses = queryUses.trim();
  if (queryUses === "") {
@@ -271,18 +271,18 @@ async function parseQueryUses(

  // Check for the local path case before we start trying to parse the repository name
  if (queryUses.startsWith("./")) {
-    await addLocalQueries(configFile, codeQL, resultMap, queryUses.slice(2));
+    await addLocalQueries(codeQL, resultMap, queryUses.slice(2), configFile);
    return;
  }

  // Check for one of the builtin suites
  if (queryUses.indexOf('/') === -1 && queryUses.indexOf('@') === -1) {
-    await addBuiltinSuiteQueries(configFile, languages, codeQL, resultMap, queryUses);
+    await addBuiltinSuiteQueries(languages, codeQL, resultMap, queryUses, configFile);
    return;
  }

  // Otherwise, must be a reference to another repo
-  await addRemoteQueries(configFile, codeQL, resultMap, queryUses, tempDir);
+  await addRemoteQueries(codeQL, resultMap, queryUses, tempDir, configFile);
 }

 // Regex validating stars in paths or paths-ignore entries.
@@ -356,6 +356,9 @@ export function validateAndSanitisePath(
  return path;
 }

+// An undefined configFile in some of these functions indicates that
+// the property was in a workflow file, not a config file
+
 export function getNameInvalid(configFile: string): string {
  return getConfigFilePropertyError(configFile, NAME_PROPERTY, 'must be a non-empty string');
 }
@@ -368,7 +371,7 @@ export function getQueriesInvalid(configFile: string): string {
  return getConfigFilePropertyError(configFile, QUERIES_PROPERTY, 'must be an array');
 }

-export function getQueryUsesInvalid(configFile: string, queryUses?: string): string {
+export function getQueryUsesInvalid(configFile: string | undefined, queryUses?: string): string {
  return getConfigFilePropertyError(
    configFile,
    QUERIES_PROPERTY + '.' + QUERIES_USES_PROPERTY,
@@ -385,14 +388,14 @@ export function getPathsInvalid(configFile: string): string {
  return getConfigFilePropertyError(configFile, PATHS_PROPERTY, 'must be an array of non-empty strings');
 }

-export function getLocalPathOutsideOfRepository(configFile: string, localPath: string): string {
+export function getLocalPathOutsideOfRepository(configFile: string | undefined, localPath: string): string {
  return getConfigFilePropertyError(
    configFile,
    QUERIES_PROPERTY + '.' + QUERIES_USES_PROPERTY,
    'is invalid as the local path "' + localPath + '" is outside of the repository');
 }

-export function getLocalPathDoesNotExist(configFile: string, localPath: string): string {
+export function getLocalPathDoesNotExist(configFile: string | undefined, localPath: string): string {
  return getConfigFilePropertyError(
    configFile,
    QUERIES_PROPERTY + '.' + QUERIES_USES_PROPERTY,
@@ -422,8 +425,12 @@ export function getConfigFileDirectoryGivenMessage(configFile: string): string {
  return 'The configuration file "' + configFile + '" looks like a directory, not a file';
 }

-function getConfigFilePropertyError(configFile: string, property: string, error: string): string {
-  return 'The configuration file "' + configFile + '" is invalid: property "' + property + '" ' + error;
+function getConfigFilePropertyError(configFile: string | undefined, property: string, error: string): string {
+  if (configFile === undefined) {
+    return 'The workflow property "' + property + '" is invalid: ' + error;
+  } else {
+    return 'The configuration file "' + configFile + '" is invalid: property "' + property + '" ' + error;
+  }
 }

 export function getNoLanguagesError(): string {
@@ -518,6 +525,27 @@ async function getLanguages(): Promise<Language[]> {
  return parsedLanguages;
 }

+/**
+ * Returns true if queries were provided in the workflow file
+ * (and thus added), otherwise false
+ */
+async function addQueriesFromWorkflowIfRequired(
+  codeQL: CodeQL,
+  languages: string[],
+  resultMap: { [language: string]: string[] },
+  tempDir: string
+): Promise<boolean> {
+  const queryUses = core.getInput('queries');
+  if (queryUses) {
+    for (const query of queryUses.split(',')) {
+      await parseQueryUses(languages, codeQL, resultMap, query, tempDir);
+    }
+    return true;
+  }
+
+  return false;
+}
+
 /**
 * Get the default config for when the user has not supplied one.
 */
@@ -525,6 +553,8 @@ export async function getDefaultConfig(tempDir: string, toolCacheDir: string, co
  const languages = await getLanguages();
  const queries = {};
  await addDefaultQueries(codeQL, languages, queries);
+  await addQueriesFromWorkflowIfRequired(codeQL, languages, queries, tempDir);
+
  return {
    languages: languages,
    queries: queries,
@@ -581,7 +611,10 @@ async function loadConfig(configFile: string, tempDir: string, toolCacheDir: str
    await addDefaultQueries(codeQL, languages, queries);
  }

-  if (QUERIES_PROPERTY in parsedYAML) {
+  // If queries were provided using `with` in the action configuration,
+  // they should take precedence over the queries in the config file
+  const addedQueriesFromAction = await addQueriesFromWorkflowIfRequired(codeQL, languages, queries, tempDir);
+  if (!addedQueriesFromAction && QUERIES_PROPERTY in parsedYAML) {
    if (!(parsedYAML[QUERIES_PROPERTY] instanceof Array)) {
      throw new Error(getQueriesInvalid(configFile));
    }
@@ -589,7 +622,7 @@ async function loadConfig(configFile: string, tempDir: string, toolCacheDir: str
      if (!(QUERIES_USES_PROPERTY in query) || typeof query[QUERIES_USES_PROPERTY] !== "string") {
        throw new Error(getQueryUsesInvalid(configFile));
      }
-      await parseQueryUses(configFile, languages, codeQL, queries, query[QUERIES_USES_PROPERTY], tempDir);
+      await parseQueryUses(languages, codeQL, queries, query[QUERIES_USES_PROPERTY], tempDir, configFile);
    }
  }

--- a/src/external-queries.test.ts
+++ b/src/external-queries.test.ts
@@ -10,12 +10,13 @@ setupTests(test);

 test("checkoutExternalQueries", async t => {
  await util.withTmpDir(async tmpDir => {
+    const ref = "df4c6869212341b601005567381944ed90906b6b";
    await externalQueries.checkoutExternalRepository(
      "github/codeql-go",
-      "df4c6869212341b601005567381944ed90906b6b",
+      ref,
      tmpDir);

    // COPYRIGHT file existed in df4c6869212341b601005567381944ed90906b6b but not in the default branch
-    t.true(fs.existsSync(path.join(tmpDir, "github", "codeql-go", "COPYRIGHT")));
+    t.true(fs.existsSync(path.join(tmpDir, "github", "codeql-go", ref, "COPYRIGHT")));
  });
 });
--- a/src/external-queries.ts
+++ b/src/external-queries.ts
@@ -9,7 +9,13 @@ import * as path from 'path';
 export async function checkoutExternalRepository(repository: string, ref: string, tempDir: string): Promise<string> {
  core.info('Checking out ' + repository);

-  const checkoutLocation = path.join(tempDir, repository);
+  const checkoutLocation = path.join(tempDir, repository, ref);
+
+  if (!checkoutLocation.startsWith(tempDir)) {
+    // this still permits locations that mess with sibling repositories in `tempDir`, but that is acceptable
+    throw new Error(`'${repository}@${ref}' is not a valid repository and reference.`);
+  }
+
  if (!fs.existsSync(checkoutLocation)) {
    const repoURL = 'https://github.com/' + repository + '.git';
    await exec.exec('git', ['clone', repoURL, checkoutLocation]);