diff --git a/src/commenter.js b/src/commenter.js index e782a16..3525e73 100644 --- a/src/commenter.js +++ b/src/commenter.js @@ -138,17 +138,13 @@ async function postPRComment(results, repo, prNumber, octokit, commentWithQuerie message = createComment(results) + "_The queries table was omitted: it exceeds the GitHub comment size limit._\n"; } - const {data: comments} = await octokit.rest.issues.listComments({ + const comments = await octokit.paginate(octokit.rest.issues.listComments, { ...repo, issue_number: prNumber, + per_page: 100, }); - const comment = comments.find((comment) => { - return ( - comment.user.login === "github-actions[bot]" && - comment.body.startsWith("![kics-logo](") - ); - }); + const comment = comments.find((comment) => comment.body?.startsWith("![kics-logo](")); if (comment) { await octokit.rest.issues.updateComment({ diff --git a/src/main.js b/src/main.js index 38892fb..8352947 100644 --- a/src/main.js +++ b/src/main.js @@ -60,6 +60,9 @@ async function main() { const outputFormats = process.env.INPUT_OUTPUT_FORMATS; const exitCode = process.env.KICS_EXIT_CODE + // the scan verdict comes first so a reporting failure can never hide it + setWorkflowStatus(exitCode); + try { const octokit = github.getOctokit(githubToken); let context = {}; @@ -80,7 +83,13 @@ async function main() { enableComments = enableComments ? enableComments : "false" enableJobsSummary = enableJobsSummary ? enableJobsSummary : "false" - const parsedResults = readJSON(outputPath.resultsJSONFile); + let parsedResults; + try { + parsedResults = readJSON(outputPath.resultsJSONFile); + } catch (e) { + core.setFailed(`Could not read the KICS results: ${e.message}`); + return; + } if (enableAnnotations.toLocaleLowerCase() === "true") { annotator.annotateChangesWithResults(parsedResults); } @@ -91,7 +100,6 @@ async function main() { await commenter.postJobSummary(parsedResults, commentsWithQueries.toLocaleLowerCase() === "true", excludedColumnsForCommentsWithQueries); } - setWorkflowStatus(exitCode); await cleanupOutput(outputPath.resultsJSONFile, outputFormats); } catch (e) { console.error(e); diff --git a/test/e2e/action.test.js b/test/e2e/action.test.js index fa36f2a..dfb76fb 100644 --- a/test/e2e/action.test.js +++ b/test/e2e/action.test.js @@ -5,13 +5,9 @@ const assert = require('node:assert/strict') const { runAction } = require('../helpers/run-action') const { startFakeGitHub, humanComment } = require('../helpers/fake-github') const { kicsResults, query, finding } = require('../helpers/fixtures') -const { knownBug } = require('../helpers/known-bug') const KICS_FOUND_RESULTS = '50' -const SILENT_FAILURE = - 'main() swallows every exception, so a KICS failure is never reported when GitHub or results.json cannot be used' - test('pull request comment', async (t) => { await t.test('is not posted unless enable_comments is on (the default)', async (t) => { const fake = await startFakeGitHub(t) @@ -171,19 +167,19 @@ test('workflow status follows the KICS exit code', async (t) => { assert.equal(run.code, 0) }) - await t.test('fails when KICS fails and the PR comment cannot be posted', knownBug(SILENT_FAILURE), async (t) => { + await t.test('fails when KICS fails and the PR comment cannot be posted', async (t) => { const fake = await startFakeGitHub(t, { failWith: 500 }) const run = await runAction(t, { results: kicsResults(), kicsExitCode: KICS_FOUND_RESULTS, inputs: { enable_comments: 'true' }, apiUrl: fake.url }) assert.equal(run.code, 1, 'a GitHub API outage must not turn a failed scan into a green build') }) - await t.test('fails when KICS failed and wrote no results.json', knownBug(SILENT_FAILURE), async (t) => { + await t.test('fails when KICS failed and wrote no results.json', async (t) => { const run = await runAction(t, { kicsExitCode: '126' }) assert.equal(run.code, 1, run.stdout + run.stderr) assert.ok(run.errors.length > 0) }) - await t.test('reports a missing results.json instead of passing silently', knownBug(SILENT_FAILURE), async (t) => { + await t.test('reports a missing results.json instead of passing silently', async (t) => { const run = await runAction(t, { kicsExitCode: '0', inputs: { enable_comments: 'true' } }) assert.notEqual(run.code, 0, 'the scan produced no report, which is not a success') }) diff --git a/test/unit/commenter.test.js b/test/unit/commenter.test.js index d9101d4..891edab 100644 --- a/test/unit/commenter.test.js +++ b/test/unit/commenter.test.js @@ -8,7 +8,6 @@ const path = require('node:path') const { postPRComment, postJobSummary } = require('../../src/commenter') const { startFakeGitHub, humanComment, MAX_COMMENT_LENGTH } = require('../helpers/fake-github') const { kicsResults, query, finding } = require('../helpers/fixtures') -const { knownBug } = require('../helpers/known-bug') const repo = { owner: 'o', repo: 'r' } const PR = 1 @@ -16,9 +15,6 @@ const COMMENTS_PATH = '/repos/o/r/issues/1/comments' const kicsBody = '![kics-logo](https://example.com/logo.png)\nold report' const noExclusions = [] -// Pending fix: PR #163 (fix-duplicated-comment). Remove the `knownBug` markers once it is merged. -const PR_163 = 'PR #163: comment lookup must paginate and match on the KICS marker, not the author' - async function post(fake, results = kicsResults(), { withQueries = false, excluded = noExclusions, prNumber = PR } = {}) { await postPRComment(results, repo, prNumber, fake.octokit, withQueries, excluded) } @@ -81,7 +77,7 @@ test('PR comment lifecycle', async (t) => { }) test('finding the existing KICS comment in a busy thread', async (t) => { - await t.test('updates it when buried past the first 30 comments', knownBug(PR_163), async (t) => { + await t.test('updates it when buried past the first 30 comments', async (t) => { const comments = Array.from({ length: 40 }, (_, i) => humanComment(i)) comments.push({ id: 1000, user: { login: 'github-actions[bot]' }, body: kicsBody }) const fake = await startFakeGitHub(t, { comments }) @@ -92,7 +88,7 @@ test('finding the existing KICS comment in a busy thread', async (t) => { assert.ok(!fake.calls().some((c) => c.startsWith('POST')), 'must not create a duplicate') }) - await t.test('updates it when it is on page 3 of 100+ comments per page', knownBug(PR_163), async (t) => { + await t.test('updates it when it is on page 3 of 100+ comments per page', async (t) => { const comments = Array.from({ length: 250 }, (_, i) => humanComment(i)) comments.push({ id: 1000, user: { login: 'github-actions[bot]' }, body: kicsBody }) const fake = await startFakeGitHub(t, { comments }) @@ -104,13 +100,13 @@ test('finding the existing KICS comment in a busy thread', async (t) => { assert.ok(fake.requests.filter((r) => r.method === 'GET').length <= 3, 'lists 100 comments per page, not 30') }) - await t.test('updates it when it was posted by an author other than github-actions[bot]', knownBug(PR_163), async (t) => { + await t.test('updates it when it was posted by an author other than github-actions[bot]', async (t) => { const fake = await startFakeGitHub(t, { comments: [{ id: 5, user: { login: 'my-app[bot]' }, body: kicsBody }] }) await post(fake) assert.deepEqual(fake.writes().map((r) => `${r.method} ${r.path}`), ['PATCH /repos/o/r/issues/comments/5']) }) - await t.test('updates the oldest KICS comment when duplicates already exist', knownBug(PR_163), async (t) => { + await t.test('updates the oldest KICS comment when duplicates already exist', async (t) => { const fake = await startFakeGitHub(t, { comments: [ { id: 10, user: { login: 'someone' }, body: kicsBody },