fix(status): apply the KICS verdict first and fail when the results cannot be read

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({

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);

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')
     })

test/unit/commenter.test.js

@@ -8,17 +8,13 @@ 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
 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 },