Skip to content

ci(security): gate release findings through reviewed, expiring dispositions - #3682

Draft
purp wants to merge 4 commits into
NVIDIA:mainfrom
purp:3671-security-dispositions/purp
Draft

purp wants to merge 4 commits into
NVIDIA:mainfrom
purp:3671-security-dispositions/purp

Conversation

@purp

@purp purp commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Draft for discussion. The release Security Scan fails on any HIGH/CRITICAL CodeQL, Trivy, or Zizmor finding. This PR lets those findings pass only when a reviewed, expiring entry in security-dispositions.toml covers them. The table and its evaluator come from the workflow revision, not from the scanned tag.

Related Issue

Closes #3671 (part of #3559)

Changes

  • tasks/scripts/security_dispositions.py: a stdlib-only evaluator. It extracts findings from CodeQL SARIF, Zizmor JSON, and Trivy JSON, then matches them on scanner + rule + path. Deferrals expire; rejected entries don't. Exit codes: 0 = covered or not enforcing, 10 = failing findings, 2 = invalid input. It writes a step summary listing matched, failing, stale, and soon-to-expire entries.
  • security-dispositions.toml: 10 entries for the v0.1.0-pre.11 backlog (8 deferred, 2 rejected). The deferrals carry an sla_exception that cites ci(security): stage release finding enforcement #3552 and feat(ci): record time-limited security finding dispositions for the release gate #3671. The file is added to CODEOWNERS.
  • codeql.yml, trivy-scan.yml, workflow-security.yml: each scanner job first does a sparse checkout of the workflow revision. It copies the evaluator and the table to $RUNNER_TEMP, then deletes the checkout, all before the candidate checkout. The enforce steps now call the evaluator instead of failing on the finding count.
  • trivy-scan.sh reports: prints the report set the table gate reads (consolidated config plus the image and packaged-chart reports), so both gates evaluate the same data.
  • CI.md: SLA windows (7/14/30/90 days), the severity mapping, and how to add, extend, or remove entries. architecture/build.md: one paragraph on the gate.

Open questions for review

  • Per-path cap. An entry covers at most as many findings at a path as it lists alerts for that path. A new instance of the same rule in an already-covered file then still fails. It works, but it's clumsy. Alternatives are welcome.
  • SLAs in CI.md, not SECURITY.md. SECURITY.md is the standard vulnerability-reporting text, so I left it alone.
  • Alert fix(ci): remove legacy wheel publishing machinery #331 (a new zizmor/template-injection at release-canary.yml:405) is deliberately left uncovered. It should be fixed rather than deferred, so the gate still fails on it.
  • The table uses GitHub handles as approvers. Code-owner review of the file is the only control that the approver actually approved.

Testing

  • 52 evaluator unit tests (mise run test:security-dispositions, now part of mise run test)
  • tasks/scripts/trivy-scan-test.sh passes, including a new check that reports lists the same files gate reads
  • Ran the evaluator against the artifacts from Release Tag run 35997548356 (v0.1.0-pre.11):
    • Dated 2026-09-24, every finding is covered: CodeQL 8/8, Zizmor 27/27, Trivy 2/2. All exit 0.
    • Dated 2026-11-18, after the deferrals expire, every scanner exits 10. The 3 rejected CodeQL findings still pass.
  • actionlint and zizmor (--min-severity=high) are clean on the three edited workflows
  • mise run pre-commit passes: every check passes except rust:lockfiles:check, which can't run sccache in my local sandbox. This PR changes no Cargo files.
  • Not yet run: a Security Scan dispatch against v0.1.0-pre.11. The dispatch needs this branch on NVIDIA/OpenShell.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

🤖 Generated with Claude Code

Validate security-dispositions.toml, extract HIGH/CRITICAL findings from
CodeQL SARIF, Zizmor JSON, and Trivy JSON reports, and classify each as
covered, expired, over-cap, or uncovered. Exit 10 on failing findings when
enforcement is on and 2 when the file or a report is invalid.

Signed-off-by: Jim Meyer <jimeyer@nvidia.com>
Refs NVIDIA#3671

Signed-off-by: Jim Meyer <jimeyer@nvidia.com>
CodeQL, Zizmor, and Trivy now pass their HIGH/CRITICAL findings to the
disposition evaluator instead of failing on any count. The evaluator and
table are staged from the workflow revision before the candidate checkout,
so a scanned candidate cannot approve its own findings.

Refs NVIDIA#3671

Signed-off-by: Jim Meyer <jimeyer@nvidia.com>
Refs NVIDIA#3671

Signed-off-by: Jim Meyer <jimeyer@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Sep 24, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@alangou

alangou commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

The overall direction makes sense, but I think these two points need addressing before this becomes a release gate:

  1. Scanning an existing tag fails on the new Trivy reports command. After checking out candidate_ref, the workflow calls tasks/scripts/trivy-scan.sh reports from that candidate checkout (line 203). That subcommand is introduced by this PR, so tags such as v0.1.0-pre.11 do not have it. Running that tag's script with reports returns exit 2, which would fail the reporting step before dispositions are evaluated, including with finding enforcement disabled. Previously, this step used the existing gate command. Please stage the report-selection logic and any dependencies from the workflow revision too, and validate a dispatch against an existing tag.

  2. The per-path quota can silently cover an unrelated new finding. The evaluator matches only scanner + rule + path and uses alert numbers as a count, rather than checking finding identity (matching logic). If an old finding disappears and a different finding of the same rule appears elsewhere in the same file, the replacement inherits the exception as long as the count stays within the cap. Before this change, that new HIGH/CRITICAL finding would block the release. I reproduced this with the repository's permanent codeql/rust/cleartext-logging/198 rejection: a synthetic replacement finding at provider.rs:9999 is classified as covered even with the evaluation date set to 2099. Ordinary line drift should remain supported, but permanent rejections in particular need a stable finding identity rather than a file-level allowance. Please investigate scanner fingerprints or an equivalent stable identity, and add coverage for both harmless line shifts and replacement findings.

This branch has not been deployed

No deployments
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.

feat(ci): record time-limited security finding dispositions for the release gate

2 participants