Skip to content

fix(codecatalyst): report SARIF as static analysis, not SCA - #26

Open
bradAGI wants to merge 1 commit into
trustabl:mainfrom
bradAGI:fix/codecatalyst-sarif-report-format
Open

bradAGI wants to merge 1 commit into
trustabl:mainfrom
bradAGI:fix/codecatalyst-sarif-report-format

Conversation

@bradAGI

@bradAGI bradAGI commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

The workflow declares:

Reports:
  trustabl-sarif:
    Format: SARIFSCA

with a comment next to it asking for the enum to be verified:

NOTE: verify the SARIF report Format enum against current CodeCatalyst docs before relying on it (the schema evolves).

Verified. It is the wrong one.

SARIFSA vs SARIFSCA

CodeCatalyst offers SARIF under two distinct report types:

Format Report type
SARIFSA Static analysis
SARIFSCA Software composition analysis

trustabl is a static analyzer over agent source code. SCA is the dependency/vulnerability report type — and this workflow does not produce one at all: the scanner never passes --vuln-scan, so trustabl.sarif contains no dependency findings whatsoever.

AWS's own example for a static-analysis report, from Supported SARIF properties:

Reports:
  MySAReport:
    Format: SARIFSA
    IncludePaths:
      - output/sa_report.json
    SuccessCriteria:
      StaticAnalysisFinding:
        Number: 25
        Severity: HIGH

Note the SuccessCriteria that pairs with it — StaticAnalysisFinding. Filed as SARIFSCA, these findings sit under a report type whose criteria are Vulnerability-shaped, so anyone adding success criteria later would be reaching for the wrong ones.

The change

SARIFSCASARIFSA, and the comment now records why that is the right enum rather than asking the next reader to go check. codecatalyst/README.md's matching note is updated too.

One-word behavior change; no effect on the scanner or the CodePipeline path.

Sources: Supported SARIF properties — Amazon CodeCatalyst

The workflow declared Format: SARIFSCA, with a comment asking for the enum to
be verified against current docs. Verified: it is the wrong one.

CodeCatalyst offers SARIF under two report types — SARIFSA for static analysis
and SARIFSCA for software composition analysis. trustabl is a static analyzer
over agent source; SCA is the dependency/vulnerability report type, which this
workflow does not produce at all (the scanner never passes --vuln-scan, so
trustabl.sarif contains no dependency findings).

So the findings were being filed as a composition-analysis result they are not,
which is also why the SuccessCriteria that apply to them would be the SCA ones
(Vulnerability) rather than StaticAnalysisFinding.

AWS's own example for a static-analysis report:

  Reports:
    MySAReport:
      Format: SARIFSA
      IncludePaths:
        - output/sa_report.json

https://docs.aws.amazon.com/codecatalyst/latest/userguide/test.sarif.html

The comment is replaced with the reason for the choice rather than a request to
check it, and codecatalyst/README.md's note is updated to match.
@sairenchristianbuerano

Copy link
Copy Markdown
Collaborator

Thanks @bradAGI. Flagging that this is being read rather than sitting unlooked-at.

We are reviewing all 52 open PRs together instead of one at a time. 31 of them edit scan/trustabl-scan.sh, so merge order decides as much as any individual verdict — GitHub reports almost all of these as mergeable, but that is each PR against main in isolation, not against each other. The first merge makes most of the rest conflict.

Queued with the CodeCatalyst docs.

The order we are working to: the test harness in #1 first, since twelve PRs depend on it and nothing is verifiable without it, then the fixes that close fail-open paths, then behaviour changes, then docs. A verdict on this one follows once its cluster is read.

Apologies for the wait, and thanks for the contribution.

@sairenchristianbuerano

Copy link
Copy Markdown
Collaborator

Thanks @bradAGI — the change looks good, but it needs a rebase.

Several PRs touching scan/trustabl-scan.sh have landed on main since you opened this, so it no longer merges cleanly. Could you rebase and resolve?

bash test/run-tests.sh will confirm it — 19 passing on main right now.

Ping me when it's up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants