From 2789712b422ac12cb21edce665a075976d613c9f Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 28 Apr 2020 11:29:10 -0700 Subject: [PATCH 01/24] Send tool names as parameter to upload endpoint --- lib/upload-lib.js | 4 +++- lib/util.js | 18 +++++++++++++++ src/testdata/tool-names.sarif | 41 +++++++++++++++++++++++++++++++++++ src/upload-lib.ts | 5 ++++- src/util.ts | 20 +++++++++++++++++ 5 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 src/testdata/tool-names.sarif diff --git a/lib/upload-lib.js b/lib/upload-lib.js index 7896d419e..f28f085a4 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -100,6 +100,7 @@ async function uploadFiles(sarifFiles) { if (matrix === "null" || matrix === "") { matrix = undefined; } + const toolNames = util.getToolNames(sarifPayload); const payload = JSON.stringify({ "commit_oid": commitOid, "ref": ref, @@ -108,7 +109,8 @@ async function uploadFiles(sarifFiles) { "workflow_run_id": workflowRunID, "checkout_uri": checkoutURI, "environment": matrix, - "started_at": startedAt + "started_at": startedAt, + "tool_names": toolNames, }); core.info('Uploading results'); const githubToken = core.getInput('token'); diff --git a/lib/util.js b/lib/util.js index 0612c1268..d12a91044 100644 --- a/lib/util.js +++ b/lib/util.js @@ -262,3 +262,21 @@ async function reportActionSucceeded(action) { await sendStatusReport(await createStatusReport(action, 'success')); } exports.reportActionSucceeded = reportActionSucceeded; +/** + * Get the array of all the tool names contained in the given sarif contents. + * + * Returns an array of unique string tool names. + */ +function getToolNames(sarifContents) { + const sarif = JSON.parse(sarifContents); + const toolNames = {}; + for (const run of sarif.runs || []) { + const tool = run.tool || {}; + const driver = tool.driver || {}; + if (typeof driver.name === "string" && driver.name.length > 0) { + toolNames[driver.name] = true; + } + } + return Object.keys(toolNames); +} +exports.getToolNames = getToolNames; diff --git a/src/testdata/tool-names.sarif b/src/testdata/tool-names.sarif new file mode 100644 index 000000000..ee6cd8cd7 --- /dev/null +++ b/src/testdata/tool-names.sarif @@ -0,0 +1,41 @@ +{ + "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "CodeQL command-line toolchain" + } + } + }, + { + "tool": { + "driver": { + "name": "CodeQL command-line toolchain" + } + } + }, + { + "tool": { + "driver": { + "name": "ESLint" + } + } + }, + { + "tool": { + "driver": { + "name": "" + } + } + }, + { + "tool": { + "driver": { + "name": null + } + } + } + ] +} diff --git a/src/upload-lib.ts b/src/upload-lib.ts index 1d35b0b2a..8c6a31e4e 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -98,6 +98,8 @@ async function uploadFiles(sarifFiles: string[]) { matrix = undefined; } + const toolNames = util.getToolNames(sarifPayload); + const payload = JSON.stringify({ "commit_oid": commitOid, "ref": ref, @@ -106,7 +108,8 @@ async function uploadFiles(sarifFiles: string[]) { "workflow_run_id": workflowRunID, "checkout_uri": checkoutURI, "environment": matrix, - "started_at": startedAt + "started_at": startedAt, + "tool_names": toolNames, }); core.info('Uploading results'); diff --git a/src/util.ts b/src/util.ts index 7bb3ec0a5..cfdd2419c 100644 --- a/src/util.ts +++ b/src/util.ts @@ -293,3 +293,23 @@ export async function reportActionFailed(action: string, cause?: string, excepti export async function reportActionSucceeded(action: string) { await sendStatusReport(await createStatusReport(action, 'success')); } + +/** + * Get the array of all the tool names contained in the given sarif contents. + * + * Returns an array of unique string tool names. + */ +export function getToolNames(sarifContents: string): string[] { + const sarif = JSON.parse(sarifContents); + const toolNames = {}; + + for (const run of sarif.runs || []) { + const tool = run.tool || {}; + const driver = tool.driver || {}; + if (typeof driver.name === "string" && driver.name.length > 0) { + toolNames[driver.name] = true; + } + } + + return Object.keys(toolNames); +} From b419ff6d7b5eaa802c5ac487d43f81187a272c23 Mon Sep 17 00:00:00 2001 From: David Verdeguer Date: Wed, 29 Apr 2020 11:43:39 +0200 Subject: [PATCH 02/24] Error on queries with missing/multiple languages --- lib/finalize-db.js | 4 ++-- src/finalize-db.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/finalize-db.js b/lib/finalize-db.js index 8679a7d0f..f73bb4951 100644 --- a/lib/finalize-db.js +++ b/lib/finalize-db.js @@ -73,12 +73,12 @@ async function resolveQueryLanguages(codeqlCmd, config) { const noDeclaredLanguage = resolveQueriesOutputObject.noDeclaredLanguage; const noDeclaredLanguageQueries = Object.keys(noDeclaredLanguage); if (noDeclaredLanguageQueries.length !== 0) { - core.warning('Some queries do not declare a language:\n' + noDeclaredLanguageQueries.join('\n')); + throw new Error('Some queries do not declare a language, their qlpack.yml file is missing or is invalid'); } const multipleDeclaredLanguages = resolveQueriesOutputObject.multipleDeclaredLanguages; const multipleDeclaredLanguagesQueries = Object.keys(multipleDeclaredLanguages); if (multipleDeclaredLanguagesQueries.length !== 0) { - core.warning('Some queries declare multiple languages:\n' + multipleDeclaredLanguagesQueries.join('\n')); + throw new Error('Some queries declare multiple languages, their qlpack.yml file is missing or is invalid'); } } return res; diff --git a/src/finalize-db.ts b/src/finalize-db.ts index a03e68a1b..b9605b0e2 100644 --- a/src/finalize-db.ts +++ b/src/finalize-db.ts @@ -82,13 +82,13 @@ async function resolveQueryLanguages(codeqlCmd: string, config: configUtils.Conf const noDeclaredLanguage = resolveQueriesOutputObject.noDeclaredLanguage; const noDeclaredLanguageQueries = Object.keys(noDeclaredLanguage); if (noDeclaredLanguageQueries.length !== 0) { - core.warning('Some queries do not declare a language:\n' + noDeclaredLanguageQueries.join('\n')); + throw new Error('Some queries do not declare a language, their qlpack.yml file is missing or is invalid'); } const multipleDeclaredLanguages = resolveQueriesOutputObject.multipleDeclaredLanguages; const multipleDeclaredLanguagesQueries = Object.keys(multipleDeclaredLanguages); if (multipleDeclaredLanguagesQueries.length !== 0) { - core.warning('Some queries declare multiple languages:\n' + multipleDeclaredLanguagesQueries.join('\n')); + throw new Error('Some queries declare multiple languages, their qlpack.yml file is missing or is invalid'); } } From 5c74b0f641d2a7814c866b545abdb60dcda04180 Mon Sep 17 00:00:00 2001 From: David Verdeguer Date: Wed, 29 Apr 2020 12:33:41 +0200 Subject: [PATCH 03/24] Parse ignoreDefaultQueries field --- lib/config-utils.js | 4 ++++ src/config-utils.ts | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/lib/config-utils.js b/lib/config-utils.js index ae4414430..ce032a2da 100644 --- a/lib/config-utils.js +++ b/lib/config-utils.js @@ -23,6 +23,7 @@ exports.ExternalQuery = ExternalQuery; class Config { constructor() { this.name = ""; + this.ignoreDefaultQueries = false; this.additionalQueries = []; this.externalQueries = []; this.pathsIgnore = []; @@ -75,6 +76,9 @@ function initConfig() { if (parsedYAML.name && typeof parsedYAML.name === "string") { config.name = parsedYAML.name; } + if (parsedYAML['ignore-default-queries'] && typeof parsedYAML['ignore-default-queries'] === "boolean") { + config.ignoreDefaultQueries = parsedYAML['ignore-default-queries']; + } const queries = parsedYAML.queries; if (queries && queries instanceof Array) { queries.forEach(query => { diff --git a/src/config-utils.ts b/src/config-utils.ts index 5221e5906..71d5edc12 100644 --- a/src/config-utils.ts +++ b/src/config-utils.ts @@ -17,6 +17,7 @@ export class ExternalQuery { export class Config { public name = ""; + public ignoreDefaultQueries = false; public additionalQueries: string[] = []; public externalQueries: ExternalQuery[] = []; public pathsIgnore: string[] = []; @@ -81,6 +82,10 @@ function initConfig(): Config { config.name = parsedYAML.name; } + if (parsedYAML['ignore-default-queries'] && typeof parsedYAML['ignore-default-queries'] === "boolean") { + config.ignoreDefaultQueries = parsedYAML['ignore-default-queries']; + } + const queries = parsedYAML.queries; if (queries && queries instanceof Array) { queries.forEach(query => { From 8bd6c1e5f0c025009c9a5576eb0df7a5fcf481ba Mon Sep 17 00:00:00 2001 From: David Verdeguer Date: Wed, 29 Apr 2020 12:41:28 +0200 Subject: [PATCH 04/24] Ignore default queries --- lib/finalize-db.js | 9 ++++++--- src/finalize-db.ts | 10 +++++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/finalize-db.js b/lib/finalize-db.js index 8679a7d0f..b2c607c5a 100644 --- a/lib/finalize-db.js +++ b/lib/finalize-db.js @@ -88,7 +88,11 @@ 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 additionalQueries = queriesPerLanguage[database] || []; + const queries = []; + if (!config.ignoreDefaultQueries) { + queries.push(database + '-code-scanning.qls'); + } + queries.push(...queriesPerLanguage[database]); const sarifFile = path.join(sarifFolder, database + '.sarif'); await exec.exec(codeqlCmd, [ 'database', @@ -97,8 +101,7 @@ async function runQueries(codeqlCmd, databaseFolder, sarifFolder, config) { '--format=sarif-latest', '--output=' + sarifFile, '--no-sarif-add-snippets', - database + '-code-scanning.qls', - ...additionalQueries, + ...queries ]); core.debug('SARIF results for database ' + database + ' created at "' + sarifFile + '"'); core.endGroup(); diff --git a/src/finalize-db.ts b/src/finalize-db.ts index a03e68a1b..e1cc1fc3e 100644 --- a/src/finalize-db.ts +++ b/src/finalize-db.ts @@ -102,7 +102,12 @@ async function runQueries(codeqlCmd: string, databaseFolder: string, sarifFolder for (let database of fs.readdirSync(databaseFolder)) { core.startGroup('Analyzing ' + database); - const additionalQueries = queriesPerLanguage[database] || []; + const queries: string[] = []; + if (!config.ignoreDefaultQueries) { + queries.push(database + '-code-scanning.qls'); + } + queries.push(...queriesPerLanguage[database]); + const sarifFile = path.join(sarifFolder, database + '.sarif'); await exec.exec(codeqlCmd, [ @@ -112,8 +117,7 @@ async function runQueries(codeqlCmd: string, databaseFolder: string, sarifFolder '--format=sarif-latest', '--output=' + sarifFile, '--no-sarif-add-snippets', - database + '-code-scanning.qls', - ...additionalQueries, + ...queries ]); core.debug('SARIF results for database ' + database + ' created at "' + sarifFile + '"'); From 32ced8c9013ff2ff6f9652ab98596f494ca4ac1a Mon Sep 17 00:00:00 2001 From: David Verdeguer Date: Wed, 29 Apr 2020 14:05:40 +0200 Subject: [PATCH 05/24] Update README --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 219864679..916cc8302 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,8 @@ The CodeQL action should be run on `push` events, and on a `schedule`. `Push` ev ### Configuration You may optionally specify additional queries for CodeQL to execute by using a config file. The queries must belong to a [QL pack](https://help.semmle.com/codeql/codeql-cli/reference/qlpack-overview.html) and can be in your repository or any public repository. You can choose a single .ql file, a folder containing multiple .ql files, a .qls [query suite](https://help.semmle.com/codeql/codeql-cli/procedures/query-suites.html) file, or any combination of the above. To use queries from other repositories use the same syntax as when [using an action](https://help.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idstepsuses). +You can disable the default queries using `ignore-default-queries: true`. + You can choose to ignore some files or folders from the analysis, or include additional files/folders for analysis. This *only* works for Javascript and Python analysis. Identifying potential files for extraction: - Scans each folder that's defined as `paths` in turn, traversing subfolders and looking for relevant files. @@ -98,6 +100,8 @@ A config file looks like this: ```yaml name: "My CodeQL config" +ignore-default-queries: true + queries: - name: In-repo queries (Runs the queries located in the my-queries folder of the repo) uses: ./my-queries From 6f11b5d2137e77ab676c0610b5a372fe06ef538e Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 29 Apr 2020 08:04:46 -0700 Subject: [PATCH 06/24] Add initial util test --- src/util.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 src/util.test.ts diff --git a/src/util.test.ts b/src/util.test.ts new file mode 100644 index 000000000..3dfd2f72d --- /dev/null +++ b/src/util.test.ts @@ -0,0 +1,9 @@ +import * as fs from 'fs'; + +import * as util from './util'; + +test('getToolNames', () => { + const input = fs.readFileSync(__dirname + '/testdata/tool-names.sarif', 'utf8') + const toolNames = util.getToolNames(input); + expect(toolNames).toStrictEqual(["CodeQL command-line toolchain", "ESLint"]) +}) From 62f756fc9dcd6e91482f05fbdc4b57d3a49d7957 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 29 Apr 2020 08:10:28 -0700 Subject: [PATCH 07/24] Fix other typo --- .github/pull_request_template.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index d6467871a..52ccfdb09 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,7 +1,7 @@ ### Merge / deployment checklist -- Run test builds as necessary. Can be on this repository or elsewhere as needed in order to test the change - please include links to tests in otehr repos! +- Run test builds as necessary. Can be on this repository or elsewhere as needed in order to test the change - please include links to tests in other repos! - [ ] CodeQL using init/finish actions - [ ] 3rd party tool using upload action - [ ] Confirm this change is backwards compatible with existing workflows. -- [ ] Confirm the [readme](https://github.com/github/codeql-action/blob/master/README.md) has been updated if necessary. \ No newline at end of file +- [ ] Confirm the [readme](https://github.com/github/codeql-action/blob/master/README.md) has been updated if necessary. From 20a0628ed73a0cbde47da41ab316fa9ba13a3fcc Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 29 Apr 2020 16:45:47 +0100 Subject: [PATCH 08/24] Fix typos / errors in PR template --- .github/pull_request_template.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index d6467871a..ca84204a3 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,7 +1,7 @@ ### Merge / deployment checklist -- Run test builds as necessary. Can be on this repository or elsewhere as needed in order to test the change - please include links to tests in otehr repos! - - [ ] CodeQL using init/finish actions +- Run test builds as necessary. Can be on this repository or elsewhere as needed in order to test the change - please include links to tests in other repos! + - [ ] CodeQL using init/analyze actions - [ ] 3rd party tool using upload action - [ ] Confirm this change is backwards compatible with existing workflows. -- [ ] Confirm the [readme](https://github.com/github/codeql-action/blob/master/README.md) has been updated if necessary. \ No newline at end of file +- [ ] Confirm the [readme](https://github.com/github/codeql-action/blob/master/README.md) has been updated if necessary. From 0cf8450c2400c10d330933a5102827114f907cbe Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 29 Apr 2020 18:03:56 +0000 Subject: [PATCH 09/24] Bump @actions/http-client from 1.0.4 to 1.0.8 Bumps [@actions/http-client](https://github.com/actions/http-client) from 1.0.4 to 1.0.8. - [Release notes](https://github.com/actions/http-client/releases) - [Changelog](https://github.com/actions/http-client/blob/master/RELEASES.md) - [Commits](https://github.com/actions/http-client/commits) Signed-off-by: dependabot[bot] --- package-lock.json | 6 +++--- package.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 63573714f..f3d9d22af 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,9 +15,9 @@ "integrity": "sha512-nvFkxwiicvpzNiCBF4wFBDfnBvi7xp/as7LE1hBxBxKG2L29+gkIPBiLKMVORL+Hg3JNf07AKRfl0V5djoypjQ==" }, "@actions/http-client": { - "version": "1.0.4", - "resolved": "https://registry.npmjs.org/@actions/http-client/-/http-client-1.0.4.tgz", - "integrity": "sha512-6EzXhqapKKtYr21ZnFQVBYwfrYPKPCivuSkUN/66/BDakkH2EPjUZH8tZ3MgHdI+gQIdcsY0ybbxw9ZEOmJB6g==", + "version": "1.0.8", + "resolved": "https://registry.npmjs.org/@actions/http-client/-/http-client-1.0.8.tgz", + "integrity": "sha512-G4JjJ6f9Hb3Zvejj+ewLLKLf99ZC+9v+yCxoYf9vSyH+WkzPLB2LuUtRMGNkooMqdugGBFStIKXOuvH1W+EctA==", "requires": { "tunnel": "0.0.6" }, diff --git a/package.json b/package.json index 922361cd6..00e36b072 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,7 @@ "dependencies": { "@actions/core": "^1.0.0", "@actions/exec": "^1.0.1", - "@actions/http-client": "^1.0.4", + "@actions/http-client": "^1.0.8", "@actions/io": "^1.0.1", "@actions/tool-cache": "^1.1.2", "@octokit/rest": "^17.1.0", From f237316c5ae1ee5b7e8096ac149a339ab1006c6f Mon Sep 17 00:00:00 2001 From: Max Veytsman Date: Wed, 29 Apr 2020 15:25:48 -0400 Subject: [PATCH 10/24] Improve errors & warnings in autobuild --- lib/autobuild.js | 8 ++++++-- src/autobuild.ts | 9 +++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/autobuild.js b/lib/autobuild.js index bce7f3a34..8f3dcecba 100644 --- a/lib/autobuild.js +++ b/lib/autobuild.js @@ -22,12 +22,16 @@ async function run() { // We want pick the dominant language in the repo from the ones we're able to build // The languages are sorted in order specified by user or by lines of code if we got // them from the GitHub API, so try to build the first language on the list. - const language = (_a = process.env[sharedEnv.CODEQL_ACTION_TRACED_LANGUAGES]) === null || _a === void 0 ? void 0 : _a.split(',')[0]; + const autobuildLanguages = ((_a = process.env[sharedEnv.CODEQL_ACTION_TRACED_LANGUAGES]) === null || _a === void 0 ? void 0 : _a.split(',')) || []; + const language = autobuildLanguages[0]; if (!language) { core.info("None of the languages in this project require extra build steps"); return; } core.debug(`Detected dominant traced language: ${language}`); + if (autobuildLanguages.length > 1) { + core.warning(`We will only automatically build ${language} code. If you wish to scan ${autobuildLanguages.slice(1).join(' and ')}, you must replace this block with custom build steps.`); + } core.startGroup(`Attempting to automatically build ${language} code`); // TODO: share config accross actions better via env variables const codeqlCmd = util.getRequiredEnvParam(sharedEnv.CODEQL_ACTION_CMD); @@ -51,6 +55,6 @@ async function run() { await util.reportActionSucceeded('autobuild'); } run().catch(e => { - core.setFailed("autobuild action failed: " + e); + core.setFailed("We were unable to automatically build your code. Please replace the call to the autobuild action with your custom build steps. codeql/autobuild action failed. " + e); console.log(e); }); diff --git a/src/autobuild.ts b/src/autobuild.ts index bd7b37f25..519ebb190 100644 --- a/src/autobuild.ts +++ b/src/autobuild.ts @@ -15,7 +15,8 @@ async function run() { // We want pick the dominant language in the repo from the ones we're able to build // The languages are sorted in order specified by user or by lines of code if we got // them from the GitHub API, so try to build the first language on the list. - const language = process.env[sharedEnv.CODEQL_ACTION_TRACED_LANGUAGES]?.split(',')[0]; + const autobuildLanguages = process.env[sharedEnv.CODEQL_ACTION_TRACED_LANGUAGES]?.split(',') || []; + const language = autobuildLanguages[0]; if (!language) { core.info("None of the languages in this project require extra build steps"); @@ -24,6 +25,10 @@ async function run() { core.debug(`Detected dominant traced language: ${language}`); + if (autobuildLanguages.length > 1) { + core.warning(`We will only automatically build ${language} code. If you wish to scan ${autobuildLanguages.slice(1).join(' and ')}, you must replace this block with custom build steps.`); + } + core.startGroup(`Attempting to automatically build ${language} code`); // TODO: share config accross actions better via env variables const codeqlCmd = util.getRequiredEnvParam(sharedEnv.CODEQL_ACTION_CMD); @@ -53,6 +58,6 @@ async function run() { } run().catch(e => { - core.setFailed("autobuild action failed: " + e); + core.setFailed("We were unable to automatically build your code. Please replace the call to the autobuild action with your custom build steps. codeql/autobuild action failed. " + e); console.log(e); }); From 34db3b0291dc246bf040e839a3cdd3f786dc4862 Mon Sep 17 00:00:00 2001 From: Justin Hutchings Date: Wed, 29 Apr 2020 14:09:20 -0700 Subject: [PATCH 11/24] Update README.md --- README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/README.md b/README.md index a296ea3d4..66f9ea2c5 100644 --- a/README.md +++ b/README.md @@ -2,8 +2,6 @@ This action runs GitHub's industry-leading static analysis engine, CodeQL, against a repository's source code to find security vulnerabilities. It then automatically uploads the results to GitHub so they can be displayed in the repository's security tab. CodeQL runs an extensible set of [queries](https://github.com/semmle/ql), which have been developed by the community and the [GitHub Security Lab](https://securitylab.github.com/) to find common vulnerabilities in your code. -[Sign up for the Advanced Security beta](https://github.com/features/security/advanced-security/signup) - ## Usage To get code scanning results from CodeQL analysis on your repo you can use the following workflow as a template: From 2809bdc3eef2c19ad883621be3c456c79672db60 Mon Sep 17 00:00:00 2001 From: David Verdeguer Date: Thu, 30 Apr 2020 09:37:04 +0200 Subject: [PATCH 12/24] ignore-default-queries -> disable-default-queries --- README.md | 2 +- lib/config-utils.js | 6 +++--- lib/finalize-db.js | 2 +- src/config-utils.ts | 6 +++--- src/finalize-db.ts | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 916cc8302..68030b5e1 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,7 @@ The CodeQL action should be run on `push` events, and on a `schedule`. `Push` ev ### Configuration You may optionally specify additional queries for CodeQL to execute by using a config file. The queries must belong to a [QL pack](https://help.semmle.com/codeql/codeql-cli/reference/qlpack-overview.html) and can be in your repository or any public repository. You can choose a single .ql file, a folder containing multiple .ql files, a .qls [query suite](https://help.semmle.com/codeql/codeql-cli/procedures/query-suites.html) file, or any combination of the above. To use queries from other repositories use the same syntax as when [using an action](https://help.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idstepsuses). -You can disable the default queries using `ignore-default-queries: true`. +You can disable the default queries using `disable-default-queries: true`. You can choose to ignore some files or folders from the analysis, or include additional files/folders for analysis. This *only* works for Javascript and Python analysis. Identifying potential files for extraction: diff --git a/lib/config-utils.js b/lib/config-utils.js index ce032a2da..df70c6283 100644 --- a/lib/config-utils.js +++ b/lib/config-utils.js @@ -23,7 +23,7 @@ exports.ExternalQuery = ExternalQuery; class Config { constructor() { this.name = ""; - this.ignoreDefaultQueries = false; + this.disableDefaultQueries = false; this.additionalQueries = []; this.externalQueries = []; this.pathsIgnore = []; @@ -76,8 +76,8 @@ function initConfig() { if (parsedYAML.name && typeof parsedYAML.name === "string") { config.name = parsedYAML.name; } - if (parsedYAML['ignore-default-queries'] && typeof parsedYAML['ignore-default-queries'] === "boolean") { - config.ignoreDefaultQueries = parsedYAML['ignore-default-queries']; + if (parsedYAML['disable-default-queries'] && typeof parsedYAML['disable-default-queries'] === "boolean") { + config.disableDefaultQueries = parsedYAML['disable-default-queries']; } const queries = parsedYAML.queries; if (queries && queries instanceof Array) { diff --git a/lib/finalize-db.js b/lib/finalize-db.js index b2c607c5a..b9a33f155 100644 --- a/lib/finalize-db.js +++ b/lib/finalize-db.js @@ -89,7 +89,7 @@ async function runQueries(codeqlCmd, databaseFolder, sarifFolder, config) { for (let database of fs.readdirSync(databaseFolder)) { core.startGroup('Analyzing ' + database); const queries = []; - if (!config.ignoreDefaultQueries) { + if (!config.disableDefaultQueries) { queries.push(database + '-code-scanning.qls'); } queries.push(...queriesPerLanguage[database]); diff --git a/src/config-utils.ts b/src/config-utils.ts index 71d5edc12..fb74c4228 100644 --- a/src/config-utils.ts +++ b/src/config-utils.ts @@ -17,7 +17,7 @@ export class ExternalQuery { export class Config { public name = ""; - public ignoreDefaultQueries = false; + public disableDefaultQueries = false; public additionalQueries: string[] = []; public externalQueries: ExternalQuery[] = []; public pathsIgnore: string[] = []; @@ -82,8 +82,8 @@ function initConfig(): Config { config.name = parsedYAML.name; } - if (parsedYAML['ignore-default-queries'] && typeof parsedYAML['ignore-default-queries'] === "boolean") { - config.ignoreDefaultQueries = parsedYAML['ignore-default-queries']; + if (parsedYAML['disable-default-queries'] && typeof parsedYAML['disable-default-queries'] === "boolean") { + config.disableDefaultQueries = parsedYAML['disable-default-queries']; } const queries = parsedYAML.queries; diff --git a/src/finalize-db.ts b/src/finalize-db.ts index e1cc1fc3e..f0f60e828 100644 --- a/src/finalize-db.ts +++ b/src/finalize-db.ts @@ -103,7 +103,7 @@ async function runQueries(codeqlCmd: string, databaseFolder: string, sarifFolder core.startGroup('Analyzing ' + database); const queries: string[] = []; - if (!config.ignoreDefaultQueries) { + if (!config.disableDefaultQueries) { queries.push(database + '-code-scanning.qls'); } queries.push(...queriesPerLanguage[database]); From 6997a2170d427e931505374441bd8b9a45f1d983 Mon Sep 17 00:00:00 2001 From: David Verdeguer Date: Thu, 30 Apr 2020 11:03:44 +0200 Subject: [PATCH 13/24] Address comments --- README.md | 2 +- lib/finalize-db.js | 2 +- src/finalize-db.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 68030b5e1..dd5aa2dea 100644 --- a/README.md +++ b/README.md @@ -100,7 +100,7 @@ A config file looks like this: ```yaml name: "My CodeQL config" -ignore-default-queries: true +disable-default-queries: true queries: - name: In-repo queries (Runs the queries located in the my-queries folder of the repo) diff --git a/lib/finalize-db.js b/lib/finalize-db.js index b9a33f155..0082161f4 100644 --- a/lib/finalize-db.js +++ b/lib/finalize-db.js @@ -92,7 +92,7 @@ async function runQueries(codeqlCmd, databaseFolder, sarifFolder, config) { if (!config.disableDefaultQueries) { queries.push(database + '-code-scanning.qls'); } - queries.push(...queriesPerLanguage[database]); + queries.push(...(queriesPerLanguage[database] || [])); const sarifFile = path.join(sarifFolder, database + '.sarif'); await exec.exec(codeqlCmd, [ 'database', diff --git a/src/finalize-db.ts b/src/finalize-db.ts index f0f60e828..c897f95c8 100644 --- a/src/finalize-db.ts +++ b/src/finalize-db.ts @@ -106,7 +106,7 @@ async function runQueries(codeqlCmd: string, databaseFolder: string, sarifFolder if (!config.disableDefaultQueries) { queries.push(database + '-code-scanning.qls'); } - queries.push(...queriesPerLanguage[database]); + queries.push(...(queriesPerLanguage[database] || [])); const sarifFile = path.join(sarifFolder, database + '.sarif'); From 7963db13d860f4ccdb4463e1ebf3e1b854516f5e Mon Sep 17 00:00:00 2001 From: Max Veytsman Date: Thu, 30 Apr 2020 09:11:50 -0400 Subject: [PATCH 14/24] Move error to correct catch block --- lib/autobuild.js | 4 ++-- src/autobuild.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/autobuild.js b/lib/autobuild.js index 8f3dcecba..6776b80ff 100644 --- a/lib/autobuild.js +++ b/lib/autobuild.js @@ -48,13 +48,13 @@ async function run() { core.endGroup(); } catch (error) { - core.setFailed(error.message); + core.setFailed("We were unable to automatically build your code. Please replace the call to the autobuild action with your custom build steps. " + error.message); await util.reportActionFailed('autobuild', error.message, error.stack); return; } await util.reportActionSucceeded('autobuild'); } run().catch(e => { - core.setFailed("We were unable to automatically build your code. Please replace the call to the autobuild action with your custom build steps. codeql/autobuild action failed. " + e); + core.setFailed("autobuild action failed. " + e); console.log(e); }); diff --git a/src/autobuild.ts b/src/autobuild.ts index 519ebb190..4dedb3343 100644 --- a/src/autobuild.ts +++ b/src/autobuild.ts @@ -49,7 +49,7 @@ async function run() { core.endGroup(); } catch (error) { - core.setFailed(error.message); + core.setFailed("We were unable to automatically build your code. Please replace the call to the autobuild action with your custom build steps. " + error.message); await util.reportActionFailed('autobuild', error.message, error.stack); return; } @@ -58,6 +58,6 @@ async function run() { } run().catch(e => { - core.setFailed("We were unable to automatically build your code. Please replace the call to the autobuild action with your custom build steps. codeql/autobuild action failed. " + e); + core.setFailed("autobuild action failed. " + e); console.log(e); }); From 26e955cfa321f8ee3150ca31cf77d693930ed1bb Mon Sep 17 00:00:00 2001 From: Joshua Hale Date: Thu, 30 Apr 2020 16:28:53 +0100 Subject: [PATCH 15/24] report status as failure if upload fails --- lib/finalize-db.js | 5 ++++- lib/upload-lib.js | 13 +++++++++---- lib/upload-sarif.js | 10 +++++++--- src/finalize-db.ts | 5 ++++- src/upload-lib.ts | 18 ++++++++++++------ src/upload-sarif.ts | 10 ++++++---- 6 files changed, 42 insertions(+), 19 deletions(-) diff --git a/lib/finalize-db.js b/lib/finalize-db.js index ec7e6f440..caaa4ce83 100644 --- a/lib/finalize-db.js +++ b/lib/finalize-db.js @@ -125,7 +125,10 @@ async function run() { core.info('Analyzing database'); await runQueries(codeqlCmd, databaseFolder, sarifFolder, config); if ('true' === core.getInput('upload')) { - await upload_lib.upload(sarifFolder); + if (!await upload_lib.upload(sarifFolder)) { + await util.reportActionFailed('failed', 'upload'); + return; + } } } catch (error) { diff --git a/lib/upload-lib.js b/lib/upload-lib.js index f28f085a4..74e8a300a 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -56,21 +56,24 @@ function combineSarifFiles(sarifFiles) { exports.combineSarifFiles = combineSarifFiles; // Uploads a single sarif file or a directory of sarif files // depending on what the path happens to refer to. +// Returns true iff the upload occurred and succeeded async function upload(input) { if (fs.lstatSync(input).isDirectory()) { const sarifFiles = fs.readdirSync(input) .filter(f => f.endsWith(".sarif")) .map(f => path.resolve(input, f)); - await uploadFiles(sarifFiles); + return await uploadFiles(sarifFiles); } else { - await uploadFiles([input]); + return await uploadFiles([input]); } } exports.upload = upload; // Uploads the given set of sarif files. +// Returns true iff the upload occurred and succeeded async function uploadFiles(sarifFiles) { core.startGroup("Uploading results"); + let succeeded = false; try { // Check if an upload has happened before. If so then abort. // This is intended to catch when the finish and upload-sarif actions @@ -78,7 +81,7 @@ async function uploadFiles(sarifFiles) { const sentinelFile = await getSentinelFilePath(); if (fs.existsSync(sentinelFile)) { core.info("Aborting as an upload has already happened from this job"); - return; + return false; } const commitOid = util.getRequiredEnvParam('GITHUB_SHA'); const workflowRunIDStr = util.getRequiredEnvParam('GITHUB_RUN_ID'); @@ -94,7 +97,7 @@ async function uploadFiles(sarifFiles) { const workflowRunID = parseInt(workflowRunIDStr, 10); if (Number.isNaN(workflowRunID)) { core.setFailed('GITHUB_RUN_ID must define a non NaN workflow run ID'); - return; + return false; } let matrix = core.getInput('matrix'); if (matrix === "null" || matrix === "") { @@ -131,6 +134,7 @@ async function uploadFiles(sarifFiles) { } else { core.info("Successfully uploaded results"); + succeeded = true; } // Mark that we have made an upload fs.writeFileSync(sentinelFile, ''); @@ -139,4 +143,5 @@ async function uploadFiles(sarifFiles) { core.setFailed(error.message); } core.endGroup(); + return succeeded; } diff --git a/lib/upload-sarif.js b/lib/upload-sarif.js index 5bd8e593d..b22d2ac62 100644 --- a/lib/upload-sarif.js +++ b/lib/upload-sarif.js @@ -15,16 +15,20 @@ async function run() { return; } try { - await upload_lib.upload(core.getInput('sarif_file')); + if (await upload_lib.upload(core.getInput('sarif_file'))) { + await util.reportActionSucceeded('upload-sarif'); + } + else { + await util.reportActionFailed('upload-sarif', 'upload'); + } } catch (error) { core.setFailed(error.message); await util.reportActionFailed('upload-sarif', error.message, error.stack); return; } - await util.reportActionSucceeded('upload-sarif'); } run().catch(e => { - core.setFailed("upload-sarif action failed: " + e); + core.setFailed("codeql/upload-sarif action failed: " + e); console.log(e); }); diff --git a/src/finalize-db.ts b/src/finalize-db.ts index a4165e97e..f58ab6c1c 100644 --- a/src/finalize-db.ts +++ b/src/finalize-db.ts @@ -150,7 +150,10 @@ async function run() { await runQueries(codeqlCmd, databaseFolder, sarifFolder, config); if ('true' === core.getInput('upload')) { - await upload_lib.upload(sarifFolder); + if (!await upload_lib.upload(sarifFolder)) { + await util.reportActionFailed('failed', 'upload'); + return; + } } } catch (error) { diff --git a/src/upload-lib.ts b/src/upload-lib.ts index 8c6a31e4e..c7965555d 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -49,20 +49,23 @@ export function combineSarifFiles(sarifFiles: string[]): string { // Uploads a single sarif file or a directory of sarif files // depending on what the path happens to refer to. -export async function upload(input: string) { +// Returns true iff the upload occurred and succeeded +export async function upload(input: string): Promise { if (fs.lstatSync(input).isDirectory()) { const sarifFiles = fs.readdirSync(input) .filter(f => f.endsWith(".sarif")) .map(f => path.resolve(input, f)); - await uploadFiles(sarifFiles); + return await uploadFiles(sarifFiles); } else { - await uploadFiles([input]); + return await uploadFiles([input]); } } // Uploads the given set of sarif files. -async function uploadFiles(sarifFiles: string[]) { +// Returns true iff the upload occurred and succeeded +async function uploadFiles(sarifFiles: string[]): Promise { core.startGroup("Uploading results"); + let succeeded = false; try { // Check if an upload has happened before. If so then abort. // This is intended to catch when the finish and upload-sarif actions @@ -70,7 +73,7 @@ async function uploadFiles(sarifFiles: string[]) { const sentinelFile = await getSentinelFilePath(); if (fs.existsSync(sentinelFile)) { core.info("Aborting as an upload has already happened from this job"); - return; + return false; } const commitOid = util.getRequiredEnvParam('GITHUB_SHA'); @@ -90,7 +93,7 @@ async function uploadFiles(sarifFiles: string[]) { if (Number.isNaN(workflowRunID)) { core.setFailed('GITHUB_RUN_ID must define a non NaN workflow run ID'); - return; + return false; } let matrix: string | undefined = core.getInput('matrix'); @@ -130,6 +133,7 @@ async function uploadFiles(sarifFiles: string[]) { core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); } else { core.info("Successfully uploaded results"); + succeeded = true; } // Mark that we have made an upload @@ -139,4 +143,6 @@ async function uploadFiles(sarifFiles: string[]) { core.setFailed(error.message); } core.endGroup(); + + return succeeded; } diff --git a/src/upload-sarif.ts b/src/upload-sarif.ts index b0aedce5a..418769c27 100644 --- a/src/upload-sarif.ts +++ b/src/upload-sarif.ts @@ -9,17 +9,19 @@ async function run() { } try { - await upload_lib.upload(core.getInput('sarif_file')); + if (await upload_lib.upload(core.getInput('sarif_file'))) { + await util.reportActionSucceeded('upload-sarif'); + } else { + await util.reportActionFailed('upload-sarif', 'upload'); + } } catch (error) { core.setFailed(error.message); await util.reportActionFailed('upload-sarif', error.message, error.stack); return; } - - await util.reportActionSucceeded('upload-sarif'); } run().catch(e => { - core.setFailed("upload-sarif action failed: " + e); + core.setFailed("codeql/upload-sarif action failed: " + e); console.log(e); }); From 1da651c2196959a0403d7f49689ac6aabac909a9 Mon Sep 17 00:00:00 2001 From: Robert Brignull Date: Fri, 1 May 2020 10:39:23 +0100 Subject: [PATCH 16/24] Add retries to the upload --- lib/upload-lib.js | 56 ++++++++++++++++++++++++++--------------- src/upload-lib.ts | 63 +++++++++++++++++++++++++++++++++-------------- 2 files changed, 80 insertions(+), 39 deletions(-) diff --git a/lib/upload-lib.js b/lib/upload-lib.js index f28f085a4..62d49a065 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -54,6 +54,40 @@ function combineSarifFiles(sarifFiles) { return JSON.stringify(combinedSarif); } exports.combineSarifFiles = combineSarifFiles; +// Upload the given payload. +// If the request fails then this will be retry a small number of times. +async function uploadPayload(payload) { + core.info('Uploading results'); + const githubToken = core.getInput('token'); + const ph = new auth.BearerCredentialHandler(githubToken); + const client = new http.HttpClient('Code Scanning : Upload SARIF', [ph]); + const url = 'https://api.github.com/repos/' + process.env['GITHUB_REPOSITORY'] + '/code-scanning/analysis'; + // Make up to 4 attempts to upload, and sleep for these + // number of seconds between each attempt. + // We don't want to backoff too much to avoid wasting action + // minutes, but just waiting a little bit could maybe help. + const backoffPeriods = [1, 5, 15]; + for (let attempt = 0; attempt <= backoffPeriods.length; attempt++) { + const res = await client.put(url, payload); + core.debug('response status: ' + res.message.statusCode); + if (res.message.statusCode === 202) { + core.info("Successfully uploaded results"); + return; + } + const requestID = res.message.headers["x-github-request-id"]; + if (attempt < backoffPeriods.length) { + // Log the failure as a warning but don't mark the action as failed yet + core.warning('Upload attempt (' + (attempt + 1) + ' of ' + (backoffPeriods.length + 1) + + ') failed (' + requestID + '). Retrying in ' + backoffPeriods[attempt] + ' seconds: ' + + await res.readBody()); + // Sleep for the backoff period + await new Promise(r => setTimeout(r, backoffPeriods[attempt] * 1000)); + } + else { + core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); + } + } +} // Uploads a single sarif file or a directory of sarif files // depending on what the path happens to refer to. async function upload(input) { @@ -112,26 +146,8 @@ async function uploadFiles(sarifFiles) { "started_at": startedAt, "tool_names": toolNames, }); - core.info('Uploading results'); - const githubToken = core.getInput('token'); - const ph = new auth.BearerCredentialHandler(githubToken); - const client = new http.HttpClient('Code Scanning : Upload SARIF', [ph]); - const url = 'https://api.github.com/repos/' + process.env['GITHUB_REPOSITORY'] + '/code-scanning/analysis'; - const res = await client.put(url, payload); - const requestID = res.message.headers["x-github-request-id"]; - core.debug('response status: ' + res.message.statusCode); - if (res.message.statusCode === 500) { - // If the upload fails with 500 then we assume it is a temporary problem - // with turbo-scan and not an error that the user has caused or can fix. - // We avoid marking the job as failed to avoid breaking CI workflows. - core.error('Upload failed (' + requestID + '): ' + await res.readBody()); - } - else if (res.message.statusCode !== 202) { - core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); - } - else { - core.info("Successfully uploaded results"); - } + // Make the upload + await uploadPayload(payload); // Mark that we have made an upload fs.writeFileSync(sentinelFile, ''); } diff --git a/src/upload-lib.ts b/src/upload-lib.ts index 8c6a31e4e..c9b69dfcb 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -47,6 +47,48 @@ export function combineSarifFiles(sarifFiles: string[]): string { return JSON.stringify(combinedSarif); } +// Upload the given payload. +// If the request fails then this will be retry a small number of times. +async function uploadPayload(payload) { + core.info('Uploading results'); + + const githubToken = core.getInput('token'); + const ph: auth.BearerCredentialHandler = new auth.BearerCredentialHandler(githubToken); + const client = new http.HttpClient('Code Scanning : Upload SARIF', [ph]); + const url = 'https://api.github.com/repos/' + process.env['GITHUB_REPOSITORY'] + '/code-scanning/analysis'; + + // Make up to 4 attempts to upload, and sleep for these + // number of seconds between each attempt. + // We don't want to backoff too much to avoid wasting action + // minutes, but just waiting a little bit could maybe help. + const backoffPeriods = [1, 5, 15]; + + for (let attempt = 0; attempt <= backoffPeriods.length; attempt++) { + + const res: http.HttpClientResponse = await client.put(url, payload); + core.debug('response status: ' + res.message.statusCode); + + if (res.message.statusCode === 202) { + core.info("Successfully uploaded results"); + return; + } + + const requestID = res.message.headers["x-github-request-id"]; + + if (attempt < backoffPeriods.length) { + // Log the failure as a warning but don't mark the action as failed yet + core.warning('Upload attempt (' + (attempt + 1) + ' of ' + (backoffPeriods.length + 1) + + ') failed (' + requestID + '). Retrying in ' + backoffPeriods[attempt] + ' seconds: ' + + await res.readBody()); + // Sleep for the backoff period + await new Promise(r => setTimeout(r, backoffPeriods[attempt] * 1000)); + + } else { + core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); + } + } +} + // Uploads a single sarif file or a directory of sarif files // depending on what the path happens to refer to. export async function upload(input: string) { @@ -112,25 +154,8 @@ async function uploadFiles(sarifFiles: string[]) { "tool_names": toolNames, }); - core.info('Uploading results'); - const githubToken = core.getInput('token'); - const ph: auth.BearerCredentialHandler = new auth.BearerCredentialHandler(githubToken); - const client = new http.HttpClient('Code Scanning : Upload SARIF', [ph]); - const url = 'https://api.github.com/repos/' + process.env['GITHUB_REPOSITORY'] + '/code-scanning/analysis'; - const res: http.HttpClientResponse = await client.put(url, payload); - const requestID = res.message.headers["x-github-request-id"]; - - core.debug('response status: ' + res.message.statusCode); - if (res.message.statusCode === 500) { - // If the upload fails with 500 then we assume it is a temporary problem - // with turbo-scan and not an error that the user has caused or can fix. - // We avoid marking the job as failed to avoid breaking CI workflows. - core.error('Upload failed (' + requestID + '): ' + await res.readBody()); - } else if (res.message.statusCode !== 202) { - core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); - } else { - core.info("Successfully uploaded results"); - } + // Make the upload + await uploadPayload(payload); // Mark that we have made an upload fs.writeFileSync(sentinelFile, ''); From 5d2700f9cbcdfd67a172405ed36c95850ccf35bf Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Fri, 1 May 2020 11:07:58 +0100 Subject: [PATCH 17/24] Increase the log level of the message showing what SARIF files were uploaded. --- lib/upload-lib.js | 2 +- src/upload-lib.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/upload-lib.js b/lib/upload-lib.js index f28f085a4..2136fee71 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -85,7 +85,7 @@ async function uploadFiles(sarifFiles) { const ref = util.getRequiredEnvParam('GITHUB_REF'); // it's in the form "refs/heads/master" const analysisName = util.getRequiredEnvParam('GITHUB_WORKFLOW'); const startedAt = process.env[sharedEnv.CODEQL_ACTION_STARTED_AT]; - core.debug("Uploading sarif files: " + JSON.stringify(sarifFiles)); + core.info("Uploading sarif files: " + JSON.stringify(sarifFiles)); let sarifPayload = combineSarifFiles(sarifFiles); sarifPayload = fingerprints.addFingerprints(sarifPayload); const zipped_sarif = zlib_1.default.gzipSync(sarifPayload).toString('base64'); diff --git a/src/upload-lib.ts b/src/upload-lib.ts index 8c6a31e4e..105e440b1 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -79,7 +79,7 @@ async function uploadFiles(sarifFiles: string[]) { const analysisName = util.getRequiredEnvParam('GITHUB_WORKFLOW'); const startedAt = process.env[sharedEnv.CODEQL_ACTION_STARTED_AT]; - core.debug("Uploading sarif files: " + JSON.stringify(sarifFiles)); + core.info("Uploading sarif files: " + JSON.stringify(sarifFiles)); let sarifPayload = combineSarifFiles(sarifFiles); sarifPayload = fingerprints.addFingerprints(sarifPayload); From cffc0f7b4e89a6b33851dd8f9033f870784b54ed Mon Sep 17 00:00:00 2001 From: Robert Brignull Date: Fri, 1 May 2020 11:19:39 +0100 Subject: [PATCH 18/24] fix typo --- lib/upload-lib.js | 2 +- src/upload-lib.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/upload-lib.js b/lib/upload-lib.js index 62d49a065..668f5f8cc 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -55,7 +55,7 @@ function combineSarifFiles(sarifFiles) { } exports.combineSarifFiles = combineSarifFiles; // Upload the given payload. -// If the request fails then this will be retry a small number of times. +// If the request fails then this will retry a small number of times. async function uploadPayload(payload) { core.info('Uploading results'); const githubToken = core.getInput('token'); diff --git a/src/upload-lib.ts b/src/upload-lib.ts index c9b69dfcb..18d43f861 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -48,7 +48,7 @@ export function combineSarifFiles(sarifFiles: string[]): string { } // Upload the given payload. -// If the request fails then this will be retry a small number of times. +// If the request fails then this will retry a small number of times. async function uploadPayload(payload) { core.info('Uploading results'); From e52e34ba17846a8305514ca2459c4102b88a10cf Mon Sep 17 00:00:00 2001 From: Robert Brignull Date: Fri, 1 May 2020 11:20:21 +0100 Subject: [PATCH 19/24] remove change to behaviour on 500 errors --- lib/upload-lib.js | 6 ++++++ src/upload-lib.ts | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/lib/upload-lib.js b/lib/upload-lib.js index 668f5f8cc..d8f4639d9 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -83,6 +83,12 @@ async function uploadPayload(payload) { // Sleep for the backoff period await new Promise(r => setTimeout(r, backoffPeriods[attempt] * 1000)); } + else if (res.message.statusCode === 500) { + // If the upload fails with 500 then we assume it is a temporary problem + // with turbo-scan and not an error that the user has caused or can fix. + // We avoid marking the job as failed to avoid breaking CI workflows. + core.error('Upload failed (' + requestID + '): ' + await res.readBody()); + } else { core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); } diff --git a/src/upload-lib.ts b/src/upload-lib.ts index 18d43f861..1342ccfa8 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -83,6 +83,12 @@ async function uploadPayload(payload) { // Sleep for the backoff period await new Promise(r => setTimeout(r, backoffPeriods[attempt] * 1000)); + } else if (res.message.statusCode === 500) { + // If the upload fails with 500 then we assume it is a temporary problem + // with turbo-scan and not an error that the user has caused or can fix. + // We avoid marking the job as failed to avoid breaking CI workflows. + core.error('Upload failed (' + requestID + '): ' + await res.readBody()); + } else { core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); } From b6a0306228047c89bc6638afbf4606fca113562b Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Fri, 1 May 2020 11:12:15 +0100 Subject: [PATCH 20/24] Fail the upload action if uploading a folder with no SARIF files in. --- lib/upload-lib.js | 3 +++ src/upload-lib.ts | 3 +++ 2 files changed, 6 insertions(+) diff --git a/lib/upload-lib.js b/lib/upload-lib.js index 2136fee71..568d2934f 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -61,6 +61,9 @@ async function upload(input) { const sarifFiles = fs.readdirSync(input) .filter(f => f.endsWith(".sarif")) .map(f => path.resolve(input, f)); + if (sarifFiles.length === 0) { + core.setFailed("No SARIF files found to upload in \"" + input + "\"."); + } await uploadFiles(sarifFiles); } else { diff --git a/src/upload-lib.ts b/src/upload-lib.ts index 105e440b1..a4959ae36 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -54,6 +54,9 @@ export async function upload(input: string) { const sarifFiles = fs.readdirSync(input) .filter(f => f.endsWith(".sarif")) .map(f => path.resolve(input, f)); + if (sarifFiles.length === 0) { + core.setFailed("No SARIF files found to upload in \"" + input + "\"."); + } await uploadFiles(sarifFiles); } else { await uploadFiles([input]); From 0c4fc16b490c4da59b8fa673963871f640c36ba5 Mon Sep 17 00:00:00 2001 From: Robert Brignull Date: Fri, 1 May 2020 11:32:09 +0100 Subject: [PATCH 21/24] only retry on 5xx status codes --- lib/upload-lib.js | 18 ++++++++++++------ src/upload-lib.ts | 19 +++++++++++++------ 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/lib/upload-lib.js b/lib/upload-lib.js index d8f4639d9..a23f54c4b 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -70,11 +70,18 @@ async function uploadPayload(payload) { for (let attempt = 0; attempt <= backoffPeriods.length; attempt++) { const res = await client.put(url, payload); core.debug('response status: ' + res.message.statusCode); - if (res.message.statusCode === 202) { + const statusCode = res.message.statusCode; + if (statusCode === 202) { core.info("Successfully uploaded results"); return; } const requestID = res.message.headers["x-github-request-id"]; + // On any other status code that's not 5xx mark the upload as failed + if (!statusCode || statusCode < 500 || statusCode >= 600) { + core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); + return; + } + // On a 5xx status code we may retry the request if (attempt < backoffPeriods.length) { // Log the failure as a warning but don't mark the action as failed yet core.warning('Upload attempt (' + (attempt + 1) + ' of ' + (backoffPeriods.length + 1) + @@ -82,15 +89,14 @@ async function uploadPayload(payload) { await res.readBody()); // Sleep for the backoff period await new Promise(r => setTimeout(r, backoffPeriods[attempt] * 1000)); + continue; } - else if (res.message.statusCode === 500) { - // If the upload fails with 500 then we assume it is a temporary problem + else { + // If the upload fails with 5xx then we assume it is a temporary problem // with turbo-scan and not an error that the user has caused or can fix. // We avoid marking the job as failed to avoid breaking CI workflows. core.error('Upload failed (' + requestID + '): ' + await res.readBody()); - } - else { - core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); + return; } } } diff --git a/src/upload-lib.ts b/src/upload-lib.ts index 1342ccfa8..d404b2685 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -68,13 +68,21 @@ async function uploadPayload(payload) { const res: http.HttpClientResponse = await client.put(url, payload); core.debug('response status: ' + res.message.statusCode); - if (res.message.statusCode === 202) { + const statusCode = res.message.statusCode; + if (statusCode === 202) { core.info("Successfully uploaded results"); return; } const requestID = res.message.headers["x-github-request-id"]; + // On any other status code that's not 5xx mark the upload as failed + if (!statusCode || statusCode < 500 || statusCode >= 600) { + core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); + return; + } + + // On a 5xx status code we may retry the request if (attempt < backoffPeriods.length) { // Log the failure as a warning but don't mark the action as failed yet core.warning('Upload attempt (' + (attempt + 1) + ' of ' + (backoffPeriods.length + 1) + @@ -82,15 +90,14 @@ async function uploadPayload(payload) { await res.readBody()); // Sleep for the backoff period await new Promise(r => setTimeout(r, backoffPeriods[attempt] * 1000)); + continue; - } else if (res.message.statusCode === 500) { - // If the upload fails with 500 then we assume it is a temporary problem + } else { + // If the upload fails with 5xx then we assume it is a temporary problem // with turbo-scan and not an error that the user has caused or can fix. // We avoid marking the job as failed to avoid breaking CI workflows. core.error('Upload failed (' + requestID + '): ' + await res.readBody()); - - } else { - core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); + return; } } } From a23cb1d61aa40e508980e1112d465e92f44d7c76 Mon Sep 17 00:00:00 2001 From: Robert Brignull Date: Fri, 1 May 2020 11:45:00 +0100 Subject: [PATCH 22/24] include status code is error message --- lib/upload-lib.js | 8 ++++---- src/upload-lib.ts | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/upload-lib.js b/lib/upload-lib.js index a23f54c4b..e2dd3b32d 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -78,15 +78,15 @@ async function uploadPayload(payload) { const requestID = res.message.headers["x-github-request-id"]; // On any other status code that's not 5xx mark the upload as failed if (!statusCode || statusCode < 500 || statusCode >= 600) { - core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); + core.setFailed('Upload failed (' + requestID + '): (' + statusCode + ') ' + await res.readBody()); return; } // On a 5xx status code we may retry the request if (attempt < backoffPeriods.length) { // Log the failure as a warning but don't mark the action as failed yet core.warning('Upload attempt (' + (attempt + 1) + ' of ' + (backoffPeriods.length + 1) + - ') failed (' + requestID + '). Retrying in ' + backoffPeriods[attempt] + ' seconds: ' + - await res.readBody()); + ') failed (' + requestID + '). Retrying in ' + backoffPeriods[attempt] + + ' seconds: (' + statusCode + ') ' + await res.readBody()); // Sleep for the backoff period await new Promise(r => setTimeout(r, backoffPeriods[attempt] * 1000)); continue; @@ -95,7 +95,7 @@ async function uploadPayload(payload) { // If the upload fails with 5xx then we assume it is a temporary problem // with turbo-scan and not an error that the user has caused or can fix. // We avoid marking the job as failed to avoid breaking CI workflows. - core.error('Upload failed (' + requestID + '): ' + await res.readBody()); + core.error('Upload failed (' + requestID + '): (' + statusCode + ') ' + await res.readBody()); return; } } diff --git a/src/upload-lib.ts b/src/upload-lib.ts index d404b2685..bcb6056dd 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -78,7 +78,7 @@ async function uploadPayload(payload) { // On any other status code that's not 5xx mark the upload as failed if (!statusCode || statusCode < 500 || statusCode >= 600) { - core.setFailed('Upload failed (' + requestID + '): ' + await res.readBody()); + core.setFailed('Upload failed (' + requestID + '): (' + statusCode + ') ' + await res.readBody()); return; } @@ -86,8 +86,8 @@ async function uploadPayload(payload) { if (attempt < backoffPeriods.length) { // Log the failure as a warning but don't mark the action as failed yet core.warning('Upload attempt (' + (attempt + 1) + ' of ' + (backoffPeriods.length + 1) + - ') failed (' + requestID + '). Retrying in ' + backoffPeriods[attempt] + ' seconds: ' + - await res.readBody()); + ') failed (' + requestID + '). Retrying in ' + backoffPeriods[attempt] + + ' seconds: (' + statusCode + ') ' + await res.readBody()); // Sleep for the backoff period await new Promise(r => setTimeout(r, backoffPeriods[attempt] * 1000)); continue; @@ -96,7 +96,7 @@ async function uploadPayload(payload) { // If the upload fails with 5xx then we assume it is a temporary problem // with turbo-scan and not an error that the user has caused or can fix. // We avoid marking the job as failed to avoid breaking CI workflows. - core.error('Upload failed (' + requestID + '): ' + await res.readBody()); + core.error('Upload failed (' + requestID + '): (' + statusCode + ') ' + await res.readBody()); return; } } From 129ce28897c35736adad3bd570afe34aa4ba9e05 Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 1 May 2020 11:59:44 +0100 Subject: [PATCH 23/24] Update upload-lib.ts --- lib/upload-lib.js | 2 +- src/upload-lib.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/upload-lib.js b/lib/upload-lib.js index e2dd3b32d..631e8fe39 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -93,7 +93,7 @@ async function uploadPayload(payload) { } else { // If the upload fails with 5xx then we assume it is a temporary problem - // with turbo-scan and not an error that the user has caused or can fix. + // and not an error that the user has caused or can fix. // We avoid marking the job as failed to avoid breaking CI workflows. core.error('Upload failed (' + requestID + '): (' + statusCode + ') ' + await res.readBody()); return; diff --git a/src/upload-lib.ts b/src/upload-lib.ts index bcb6056dd..2e8139991 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -94,7 +94,7 @@ async function uploadPayload(payload) { } else { // If the upload fails with 5xx then we assume it is a temporary problem - // with turbo-scan and not an error that the user has caused or can fix. + // and not an error that the user has caused or can fix. // We avoid marking the job as failed to avoid breaking CI workflows. core.error('Upload failed (' + requestID + '): (' + statusCode + ') ' + await res.readBody()); return; From 4e9886ad2bf3b89c655a3c6c66a849ba06ae5b1e Mon Sep 17 00:00:00 2001 From: Chris Gavin Date: Fri, 1 May 2020 15:27:32 +0100 Subject: [PATCH 24/24] Stop the upload action early if no files will be uploaded. --- lib/upload-lib.js | 1 + src/upload-lib.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/upload-lib.js b/lib/upload-lib.js index 568d2934f..a9abfb900 100644 --- a/lib/upload-lib.js +++ b/lib/upload-lib.js @@ -63,6 +63,7 @@ async function upload(input) { .map(f => path.resolve(input, f)); if (sarifFiles.length === 0) { core.setFailed("No SARIF files found to upload in \"" + input + "\"."); + return; } await uploadFiles(sarifFiles); } diff --git a/src/upload-lib.ts b/src/upload-lib.ts index a4959ae36..443a95e86 100644 --- a/src/upload-lib.ts +++ b/src/upload-lib.ts @@ -56,6 +56,7 @@ export async function upload(input: string) { .map(f => path.resolve(input, f)); if (sarifFiles.length === 0) { core.setFailed("No SARIF files found to upload in \"" + input + "\"."); + return; } await uploadFiles(sarifFiles); } else {