Merge pull request #45 from github/suite_syntax

introduce new syntax for built-in query suites

.github/codeql/codeql-config.yml

@@ -2,5 +2,12 @@ name: "CodeQL config"
 queries: 
   - name: Run custom queries
     uses: ./queries
+  # Run all extra query suites, both because we want to
+  # and because it'll act as extra testing. This is why
+  # we include both even though one is a superset of the
+  # other, because we're testing the parsing logic and
+  # that the suites exist in the codeql bundle.
+  - uses: security-extended
+  - uses: security-and-quality
 paths-ignore:
   - tests

.github/workflows/integration-testing.yml

@@ -23,7 +23,9 @@ jobs:
         TEST_MODE: true
     - run: |
         cd "$CODEQL_ACTION_DATABASE_DIR"
-        if [ "$(ls | wc -l)" != 6 ] || \
+        # List all directories as there will be precisely one directory per database
+        # but there may be other files in this directory such as query suites.
+        if [ "$(ls -d */ | wc -l)" != 6 ] || \
            [[ ! -d cpp ]] || \
            [[ ! -d csharp ]] || \
            [[ ! -d go ]] || \

init/action.yml

@@ -5,7 +5,7 @@ inputs:
   tools:
     description: URL of CodeQL tools
     required: false
-    default: https://github.com/github/codeql-action/releases/download/codeql-bundle-20200427/codeql-bundle.tar.gz
+    default: https://github.com/github/codeql-action/releases/download/codeql-bundle-20200601/codeql-bundle.tar.gz
   languages:
     description: The languages to be analysed
     required: false

lib/config-utils.js

@@ -21,12 +21,15 @@ class ExternalQuery {
     }
 }
 exports.ExternalQuery = ExternalQuery;
+// The set of acceptable values for built-in suites from the codeql bundle
+const builtinSuites = ['security-extended', 'security-and-quality'];
 class Config {
     constructor() {
         this.name = "";
         this.disableDefaultQueries = false;
         this.additionalQueries = [];
         this.externalQueries = [];
+        this.additionalSuites = [];
         this.pathsIgnore = [];
         this.paths = [];
     }
@@ -39,9 +42,33 @@ class Config {
         }
         // Check for the local path case before we start trying to parse the repository name
         if (queryUses.startsWith("./")) {
-            this.additionalQueries.push(queryUses.slice(2));
+            const localQueryPath = queryUses.slice(2);
+            // 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.
+            const workspacePath = util.getRequiredEnvParam('GITHUB_WORKSPACE');
+            const absoluteQueryPath = path.join(workspacePath, localQueryPath);
+            // Check the file exists
+            if (!fs.existsSync(absoluteQueryPath)) {
+                throw new Error(getLocalPathDoesNotExist(localQueryPath));
+            }
+            // Check the local path doesn't jump outside the repo using '..' or symlinks
+            if (!(fs.realpathSync(absoluteQueryPath) + path.sep).startsWith(workspacePath + path.sep)) {
+                throw new Error(getLocalPathOutsideOfRepository(localQueryPath));
+            }
+            this.additionalQueries.push(absoluteQueryPath);
             return;
         }
+        // Check for one of the builtin suites
+        if (queryUses.indexOf('/') === -1 && queryUses.indexOf('@') === -1) {
+            const suite = builtinSuites.find((suite) => suite === queryUses);
+            if (suite) {
+                this.additionalSuites.push(suite);
+                return;
+            }
+            else {
+                throw new Error(getQueryUsesIncorrect(queryUses));
+            }
+        }
         let tok = queryUses.split('@');
         if (tok.length !== 2) {
             throw new Error(getQueryUsesIncorrect(queryUses));
@@ -74,9 +101,21 @@ function getQueryUsesBlank() {
 }
 exports.getQueryUsesBlank = getQueryUsesBlank;
 function getQueryUsesIncorrect(queryUses) {
-    return '"uses" value for queries must be a path, or owner/repo@ref \n Found: ' + queryUses;
+    return '"uses" value for queries must be a built-in suite (' + builtinSuites.join(' or ') +
+        '), a relative path, or of the form owner/repo@ref\n' +
+        'Found: ' + queryUses;
 }
 exports.getQueryUsesIncorrect = getQueryUsesIncorrect;
+function getLocalPathOutsideOfRepository(localPath) {
+    return 'Unable to use queries from local path "' + localPath +
+        '" as it is outside of the repository';
+}
+exports.getLocalPathOutsideOfRepository = getLocalPathOutsideOfRepository;
+function getLocalPathDoesNotExist(localPath) {
+    return 'Unable to use queries from local path "' + localPath +
+        '" as the path does not exist in the repository';
+}
+exports.getLocalPathDoesNotExist = getLocalPathDoesNotExist;
 function getConfigFileOutsideWorkspaceErrorMessage(configFile) {
     return 'The configuration file "' + configFile + '" is outside of the workspace';
 }

lib/config-utils.test.js

@@ -87,6 +87,7 @@ ava_1.default("load non-empty input", async (t) => {
       name: my config
       disable-default-queries: true
       queries:
+        - uses: ./
         - uses: ./foo
         - uses: foo/bar@dev
       paths-ignore:
@@ -98,12 +99,14 @@ ava_1.default("load non-empty input", async (t) => {
         const expectedConfig = new configUtils.Config();
         expectedConfig.name = 'my config';
         expectedConfig.disableDefaultQueries = true;
-        expectedConfig.additionalQueries.push('foo');
+        expectedConfig.additionalQueries.push(tmpDir);
+        expectedConfig.additionalQueries.push(path.join(tmpDir, 'foo'));
         expectedConfig.externalQueries = [new configUtils.ExternalQuery('foo/bar', 'dev')];
         expectedConfig.pathsIgnore = ['a', 'b'];
         expectedConfig.paths = ['c/d'];
         fs.writeFileSync(path.join(tmpDir, 'input'), inputFileContents, 'utf8');
         setInput('config-file', 'input');
+        fs.mkdirSync(path.join(tmpDir, 'foo'));
         const actualConfig = await configUtils.loadConfig();
         // Should exactly equal the object we constructed earlier
         t.deepEqual(actualConfig, expectedConfig);
@@ -195,7 +198,9 @@ const testInputs = {
     "foo/bar": configUtils.getQueryUsesIncorrect("foo/bar"),
     "foo/bar@v1@v2": configUtils.getQueryUsesIncorrect("foo/bar@v1@v2"),
     "foo@master": configUtils.getQueryUsesIncorrect("foo@master"),
-    "https://github.com/foo/bar@master": configUtils.getQueryUsesIncorrect("https://github.com/foo/bar@master")
+    "https://github.com/foo/bar@master": configUtils.getQueryUsesIncorrect("https://github.com/foo/bar@master"),
+    "./foo": configUtils.getLocalPathDoesNotExist("foo"),
+    "./..": configUtils.getLocalPathOutsideOfRepository(".."),
 };
 for (const [input, result] of Object.entries(testInputs)) {
     ava_1.default("load invalid input - queries uses \"" + input + "\"", async (t) => {

lib/finalize-db.js

@@ -67,26 +67,50 @@ async function finalizeDatabaseCreation(codeqlCmd, databaseFolder) {
         core.endGroup();
     }
 }
+async function runResolveQueries(codeqlCmd, queries) {
+    let output = '';
+    const options = {
+        listeners: {
+            stdout: (data) => {
+                output += data.toString();
+            }
+        }
+    };
+    await exec.exec(codeqlCmd, [
+        'resolve',
+        'queries',
+        ...queries,
+        '--format=bylanguage'
+    ], options);
+    return JSON.parse(output);
+}
 async function resolveQueryLanguages(codeqlCmd, config) {
     let res = new Map();
-    if (config.additionalQueries.length !== 0) {
+    if (!config.disableDefaultQueries || config.additionalSuites.length !== 0) {
-        let resolveQueriesOutput = '';
+        const suites = [];
-        const options = {
+        for (const language of await util.getLanguages()) {
-            listeners: {
+            if (!config.disableDefaultQueries) {
-                stdout: (data) => {
+                suites.push(language + '-code-scanning.qls');
-                    resolveQueriesOutput += data.toString();
-                }
             }
-        };
+            for (const additionalSuite of config.additionalSuites) {
-        await exec.exec(codeqlCmd, [
+                suites.push(language + '-' + additionalSuite + '.qls');
-            'resolve',
+            }
-            'queries',
+        }
-            ...config.additionalQueries,
+        const resolveQueriesOutputObject = await runResolveQueries(codeqlCmd, suites);
-            '--format=bylanguage'
-        ], options);
-        const resolveQueriesOutputObject = JSON.parse(resolveQueriesOutput);
         for (const [language, queries] of Object.entries(resolveQueriesOutputObject.byLanguage)) {
-            res[language] = Object.keys(queries);
+            if (res[language] === undefined) {
+                res[language] = [];
+            }
+            res[language].push(...Object.keys(queries));
+        }
+    }
+    if (config.additionalQueries.length !== 0) {
+        const resolveQueriesOutputObject = await runResolveQueries(codeqlCmd, config.additionalQueries);
+        for (const [language, queries] of Object.entries(resolveQueriesOutputObject.byLanguage)) {
+            if (res[language] === undefined) {
+                res[language] = [];
+            }
+            res[language].push(...Object.keys(queries));
         }
         const noDeclaredLanguage = resolveQueriesOutputObject.noDeclaredLanguage;
         const noDeclaredLanguageQueries = Object.keys(noDeclaredLanguage);
@@ -106,11 +130,16 @@ async function runQueries(codeqlCmd, databaseFolder, sarifFolder, config) {
     const queriesPerLanguage = await resolveQueryLanguages(codeqlCmd, config);
     for (let database of fs.readdirSync(databaseFolder)) {
         core.startGroup('Analyzing ' + database);
-        const queries = [];
+        const queries = queriesPerLanguage[database] || [];
-        if (!config.disableDefaultQueries) {
+        if (queries.length === 0) {
-            queries.push(database + '-code-scanning.qls');
+            throw new Error('Unable to analyse ' + database + ' as no queries were selected for this language');
         }
-        queries.push(...(queriesPerLanguage[database] || []));
+        // Pass the queries to codeql using a file instead of using the command
+        // line to avoid command line length restrictions, particularly on windows.
+        const querySuite = path.join(databaseFolder, database + '-queries.qls');
+        const querySuiteContents = queries.map(q => '- query: ' + q).join('\n');
+        fs.writeFileSync(querySuite, querySuiteContents);
+        core.debug('Query suite file for ' + database + '...\n' + querySuiteContents);
         const sarifFile = path.join(sarifFolder, database + '.sarif');
         await exec.exec(codeqlCmd, [
             'database',
@@ -120,7 +149,7 @@ async function runQueries(codeqlCmd, databaseFolder, sarifFolder, config) {
             '--format=sarif-latest',
             '--output=' + sarifFile,
             '--no-sarif-add-snippets',
-            ...queries
+            querySuite
         ]);
         core.debug('SARIF results for database ' + database + ' created at "' + sarifFile + '"');
         core.endGroup();

src/config-utils.test.ts

@@ -91,6 +91,7 @@ test("load non-empty input", async t => {
       name: my config
       disable-default-queries: true
       queries:
+        - uses: ./
         - uses: ./foo
         - uses: foo/bar@dev
       paths-ignore:
@@ -103,7 +104,8 @@ test("load non-empty input", async t => {
     const expectedConfig = new configUtils.Config();
     expectedConfig.name = 'my config';
     expectedConfig.disableDefaultQueries = true;
-    expectedConfig.additionalQueries.push('foo');
+    expectedConfig.additionalQueries.push(tmpDir);
+    expectedConfig.additionalQueries.push(path.join(tmpDir, 'foo'));
     expectedConfig.externalQueries = [new configUtils.ExternalQuery('foo/bar', 'dev')];
     expectedConfig.pathsIgnore = ['a', 'b'];
     expectedConfig.paths = ['c/d'];
@@ -111,6 +113,8 @@ test("load non-empty input", async t => {
     fs.writeFileSync(path.join(tmpDir, 'input'), inputFileContents, 'utf8');
     setInput('config-file', 'input');
 
+    fs.mkdirSync(path.join(tmpDir, 'foo'));
+
     const actualConfig = await configUtils.loadConfig();
 
     // Should exactly equal the object we constructed earlier
@@ -222,7 +226,9 @@ const testInputs = {
   "foo/bar": configUtils.getQueryUsesIncorrect("foo/bar"),
   "foo/bar@v1@v2": configUtils.getQueryUsesIncorrect("foo/bar@v1@v2"),
   "foo@master": configUtils.getQueryUsesIncorrect("foo@master"),
-  "https://github.com/foo/bar@master": configUtils.getQueryUsesIncorrect("https://github.com/foo/bar@master")
+  "https://github.com/foo/bar@master": configUtils.getQueryUsesIncorrect("https://github.com/foo/bar@master"),
+  "./foo": configUtils.getLocalPathDoesNotExist("foo"),
+  "./..": configUtils.getLocalPathOutsideOfRepository(".."),
 };
 
 for (const [input, result] of Object.entries(testInputs)) {

src/config-utils.ts

@@ -17,11 +17,17 @@ export class ExternalQuery {
     }
 }
 
+// The set of acceptable values for built-in suites from the codeql bundle
+const builtinSuites = ['security-extended', 'security-and-quality'] as const;
+// Derive the union type from the array values
+type BuiltInSuite = typeof builtinSuites[number];
+
 export class Config {
     public name = "";
     public disableDefaultQueries = false;
     public additionalQueries: string[] = [];
     public externalQueries: ExternalQuery[] = [];
+    public additionalSuites: BuiltInSuite[] = [];
     public pathsIgnore: string[] = [];
     public paths: string[] = [];
 
@@ -35,10 +41,37 @@ export class Config {
 
         // Check for the local path case before we start trying to parse the repository name
         if (queryUses.startsWith("./")) {
-            this.additionalQueries.push(queryUses.slice(2));
+            const localQueryPath = queryUses.slice(2);
+            // 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.
+            const workspacePath = util.getRequiredEnvParam('GITHUB_WORKSPACE');
+            const absoluteQueryPath = path.join(workspacePath, localQueryPath);
+
+            // Check the file exists
+            if (!fs.existsSync(absoluteQueryPath)) {
+                throw new Error(getLocalPathDoesNotExist(localQueryPath));
+            }
+
+            // Check the local path doesn't jump outside the repo using '..' or symlinks
+            if (!(fs.realpathSync(absoluteQueryPath) + path.sep).startsWith(workspacePath + path.sep)) {
+                throw new Error(getLocalPathOutsideOfRepository(localQueryPath));
+            }
+
+            this.additionalQueries.push(absoluteQueryPath);
             return;
         }
 
+        // Check for one of the builtin suites
+        if (queryUses.indexOf('/') === -1 && queryUses.indexOf('@') === -1) {
+            const suite = builtinSuites.find((suite) => suite === queryUses);
+            if (suite) {
+                this.additionalSuites.push(suite);
+                return;
+            } else {
+                throw new Error(getQueryUsesIncorrect(queryUses));
+            }
+        }
+
         let tok = queryUses.split('@');
         if (tok.length !== 2) {
             throw new Error(getQueryUsesIncorrect(queryUses));
@@ -74,7 +107,19 @@ export function getQueryUsesBlank(): string {
 }
 
 export function getQueryUsesIncorrect(queryUses: string): string {
-    return '"uses" value for queries must be a path, or owner/repo@ref \n Found: ' + queryUses;
+    return '"uses" value for queries must be a built-in suite (' + builtinSuites.join(' or ') +
+        '), a relative path, or of the form owner/repo@ref\n' +
+        'Found: ' + queryUses;
+}
+
+export function getLocalPathOutsideOfRepository(localPath: string): string {
+    return 'Unable to use queries from local path "' + localPath +
+        '" as it is outside of the repository';
+}
+
+export function getLocalPathDoesNotExist(localPath: string): string {
+    return 'Unable to use queries from local path "' + localPath +
+        '" as the path does not exist in the repository';
 }
 
 export function getConfigFileOutsideWorkspaceErrorMessage(configFile: string): string {

src/finalize-db.ts

@@ -69,32 +69,74 @@ async function finalizeDatabaseCreation(codeqlCmd: string, databaseFolder: strin
   }
 }
 
+interface ResolveQueriesOutput {
+  byLanguage: {
+    [language: string]: {
+      [queryPath: string]: {}
+    }
+  };
+  noDeclaredLanguage: {
+    [queryPath: string]: {}
+  };
+  multipleDeclaredLanguages: {
+    [queryPath: string]: {}
+  };
+}
+
+async function runResolveQueries(codeqlCmd: string, queries: string[]): Promise<ResolveQueriesOutput> {
+  let output = '';
+  const options = {
+    listeners: {
+      stdout: (data: Buffer) => {
+        output += data.toString();
+      }
+    }
+  };
+
+  await exec.exec(
+    codeqlCmd, [
+      'resolve',
+      'queries',
+      ...queries,
+      '--format=bylanguage'
+    ],
+    options);
+
+  return JSON.parse(output);
+}
+
 async function resolveQueryLanguages(codeqlCmd: string, config: configUtils.Config): Promise<Map<string, string[]>> {
   let res = new Map();
 
-  if (config.additionalQueries.length !== 0) {
+  if (!config.disableDefaultQueries || config.additionalSuites.length !== 0) {
-    let resolveQueriesOutput = '';
+    const suites: string[] = [];
-    const options = {
+    for (const language of await util.getLanguages()) {
-      listeners: {
+      if (!config.disableDefaultQueries) {
-        stdout: (data: Buffer) => {
+        suites.push(language + '-code-scanning.qls');
-          resolveQueriesOutput += data.toString();
-        }
       }
-    };
+      for (const additionalSuite of config.additionalSuites) {
+        suites.push(language + '-' + additionalSuite + '.qls');
+      }
+    }
 
-    await exec.exec(
+    const resolveQueriesOutputObject = await runResolveQueries(codeqlCmd, suites);
-      codeqlCmd, [
-        'resolve',
-        'queries',
-        ...config.additionalQueries,
-        '--format=bylanguage'
-      ],
-      options);
-
-    const resolveQueriesOutputObject = JSON.parse(resolveQueriesOutput);
 
     for (const [language, queries] of Object.entries(resolveQueriesOutputObject.byLanguage)) {
-      res[language] = Object.keys(<any>queries);
+      if (res[language] === undefined) {
+        res[language] = [];
+      }
+      res[language].push(...Object.keys(<any>queries));
+    }
+  }
+
+  if (config.additionalQueries.length !== 0) {
+    const resolveQueriesOutputObject = await runResolveQueries(codeqlCmd, config.additionalQueries);
+
+    for (const [language, queries] of Object.entries(resolveQueriesOutputObject.byLanguage)) {
+      if (res[language] === undefined) {
+        res[language] = [];
+      }
+      res[language].push(...Object.keys(<any>queries));
     }
 
     const noDeclaredLanguage = resolveQueriesOutputObject.noDeclaredLanguage;
@@ -120,11 +162,17 @@ async function runQueries(codeqlCmd: string, databaseFolder: string, sarifFolder
   for (let database of fs.readdirSync(databaseFolder)) {
     core.startGroup('Analyzing ' + database);
 
-    const queries: string[] = [];
+    const queries = queriesPerLanguage[database] || [];
-    if (!config.disableDefaultQueries) {
+    if (queries.length === 0) {
-      queries.push(database + '-code-scanning.qls');
+      throw new Error('Unable to analyse ' + database + ' as no queries were selected for this language');
     }
-    queries.push(...(queriesPerLanguage[database] || []));
+
+    // Pass the queries to codeql using a file instead of using the command
+    // line to avoid command line length restrictions, particularly on windows.
+    const querySuite = path.join(databaseFolder, database + '-queries.qls');
+    const querySuiteContents = queries.map(q => '- query: ' + q).join('\n');
+    fs.writeFileSync(querySuite, querySuiteContents);
+    core.debug('Query suite file for ' + database + '...\n' + querySuiteContents);
 
     const sarifFile = path.join(sarifFolder, database + '.sarif');
 
@@ -136,7 +184,7 @@ async function runQueries(codeqlCmd: string, databaseFolder: string, sarifFolder
       '--format=sarif-latest',
       '--output=' + sarifFile,
       '--no-sarif-add-snippets',
-      ...queries
+      querySuite
     ]);
 
     core.debug('SARIF results for database ' + database + ' created at "' + sarifFile + '"');