ci: add security scanners to Quartz by cgoea · Pull Request #90 · ROCm/Quartz

cgoea · 2026-08-31T16:49:05Z

Motivation

Add security scanners to Quartz:

gitleaks — secrets and credentials in tracked files and git history
bandit — unsafe patterns in our Python scripts
zizmor — GitHub Actions workflow vulnerabilities
trivy — dependency vulnerabilities and misconfigurations
CodeQL — semantic code analysis of our Python

Add a config file for each scanner (bandit.yml, gitleaks.toml, trivy.yml, zizmor.yml at the repo root; CodeQL uses the org-wide default, no local override) so Quartz's exclusions (.venv, release-nightly, prerelease, nightly, etc.) replace the scanner's org-wide defaults instead of stacking on top of them.

JIRA ID: ROCM-28051

Technical Details

Adds two workflows that both call ROCm/rocm-security-gh/.github/workflows/security-baseline.yml, pinned to a fixed commit (not a mutable tag), and differ only in when they run and how they report:

security_scan_pr.yml — runs on every pull request, scoped to what that PR changed (scan_mode: changed, the reusable workflow's default). Reports as a job summary + build artifact (report_formats: human).
security_scan_weekly.yml — runs on a schedule (Saturdays, 10:00 UTC) plus workflow_dispatch, scoped to the whole repo (scan_mode: all), and uploads SARIF (report_formats: sarif) to the repository's Security tab.

Also included:

Scoped the create-github-app-token calls in notify_quartz/action.yml, receive_therock_data.yml, and sync_develop_to_main.yml to the specific permissions they actually use (permission-actions: write / permission-contents: write), fixing zizmor's "dangerous use of GitHub App tokens" high findings instead of inheriting the app's full installation permissions.
Registered the two new workflows in therock_workflow_registry_test.py's _EXCLUDED_WORKFLOWS set — they're Quartz's own CI plumbing.
Documented the scanners in SECURITY.md (what each one looks for, how the PR vs. weekly workflows differ, "a finding is not a vulnerability report") and in CONTRIBUTING.md (copy-paste commands to run every scanner locally against the same config CI uses).

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

github-advanced-security · 2026-09-09T13:27:10Z

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

HereThereBeDragons

can you add a little bit more details to the pr description? how often the scanner run and on what files etc. or somewhere else? could also become a new readme. maybe also how to update them or if they are automatically updated by dependabot.

other than the the real fix needed is the rename of release-nightly/ -> nightly and prerealeases/ -> prerelease/

HereThereBeDragons · 2026-09-09T14:10:46Z

+  pull_request:
+
+permissions:
+  contents: read


concurrency block?

HereThereBeDragons · 2026-09-09T14:14:58Z

yaml vs yml?

HereThereBeDragons · 2026-09-09T14:20:49Z

+# anything it doesn't list. See the PR-time counterpart
+# (security_scan_pr.yml) for the human-readable, non-elevated variant.
+
+name: Weekly security scan


one comment but i guess this is what we want.

claude:
2. No per-merge SARIF refresh (Major tradeoff — worth a question)
This is the real behavioral regression. #36 ran bandit/zizmor/codeql/gitleaks on push: [main, develop], so the Security tab refreshed on every merge. #90 only has PR-time (report_formats: human, no SARIF) + weekly (scan_mode: all, SARIF). Net effect: code-scanning results on main update only once a week. A vuln fixed and merged Monday still shows "open" on the Security tab until Saturday's run; a newly-introduced issue isn't recorded until then either. Is weekly-only intentional, or do you want a push-to-main SARIF job (or moving the scheduled one to also trigger on push to main) to keep the Security tab current?

Minor/low-priority: #36's CodeQL skipped draft PRs (pull_request.draft == false) and path-filtered to /*.py + .github/; #90 relies on scan_mode: changed instead, which is a reasonable substitute — not worth raising unless draft-PR scan noise bothers the team.

Keeping it repo agnostic. Monorepos can't run post merge the scanners, so we went for weekly

Co-authored-by: Laura Promberger <laura.promberger@amd.com>

…artz into users/cgoea/sec_scanners

…anners

- Strip trailing whitespace in bandit.yml (pre-commit failure). - Merging develop pulled in #100's stricter workflow-partition test; register the new security_scan_pr.yml / security_scan_weekly.yml workflows in _EXCLUDED_WORKFLOWS since they are Quartz-local CI plumbing, not TheRock/rockrel producer workflows that report to notify_quartz.

HereThereBeDragons

please fix naming problem and clarify more way the description talks about code ql but i dont see it here anywhere in the code

otherwise lgtm

HereThereBeDragons · 2026-09-10T11:22:21Z

+# online are picked up here without touching this file.

-name: Security scan (PR)
+name: PR security scan


please keep the naming uniform between pr and and weekly? now the naming just got reversed

Security scan (PR) -> PR security scan
Weekly security scan -> Security scan (Weekly)

HereThereBeDragons · 2026-09-10T11:24:11Z

+
+```bash
+# Secrets, over the full git history (installed separately, see gitleaks docs).
+gitleaks detect --source . --config gitleaks.toml --redact --verbose --no-banner


maybe switch this in the order with the preferred scanner? maybe put "recommended" or something in the front of the comment

HereThereBeDragons · 2026-09-10T11:24:47Z

+> in particular reports pre-existing findings that the pull request check does
+> not.
+
+CodeQL is not in the list above: it runs in CI only, against the org-wide


we dont have any codeql yet?

we don't. it will come with the scanners.

5c809d0, ci: add security scanners to Quartz (#90), Ciprian Goea (ciprian.goea@amd.com), Thu Sep 10 15:35:30 2026 +0300

cgoea added 2 commits August 31, 2026 19:22

enable security scanners from rocm-security

b9201ee

use v1.0.0 tag

b4921ec

cgoea requested a review from a team August 31, 2026 16:49

cgoea mentioned this pull request Aug 31, 2026

Add security scanners #36

Closed

cgoea requested a review from HereThereBeDragons August 31, 2026 16:50

cgoea added 3 commits September 1, 2026 15:09

update scanners commit id and add config files

6e6f581

bumpo sec scan commit id

9357f74

Merge branch 'develop' into users/cgoea/sec_scanners

51909c1

cgoea added 2 commits September 9, 2026 16:40

ci: scope GitHub App tokens to fix zizmor high findings

f5f757c

remove not needed comments

338821e

HereThereBeDragons reviewed Sep 9, 2026

View reviewed changes

cgoea and others added 4 commits September 9, 2026 18:00

Apply suggestion from @HereThereBeDragons

3cf13a9

Co-authored-by: Laura Promberger <laura.promberger@amd.com>

rename extension for all configs

31fd8b7

Merge branch 'users/cgoea/sec_scanners' of https://github.com/ROCm/Qu…

23b94ca

…artz into users/cgoea/sec_scanners

add missing concurrency group

f149097

cgoea requested a review from HereThereBeDragons September 9, 2026 15:12

cgoea added 4 commits September 9, 2026 18:13

fix uses call

acbcc86

Merge remote-tracking branch 'origin/develop' into users/cgoea/sec_sc…

a898078

…anners

docs: document security scanners and how to run them locally

2d02a29

cgoea force-pushed the users/cgoea/sec_scanners branch from 8a0990e to 2d02a29 Compare September 10, 2026 09:41

HereThereBeDragons approved these changes Sep 10, 2026

View reviewed changes

address comments from reviewer

b578e15

cgoea merged commit 5c809d0 into develop Sep 10, 2026
13 checks passed

cgoea deleted the users/cgoea/sec_scanners branch September 10, 2026 12:35

quartz-sync-github-app Bot pushed a commit that referenced this pull request Sep 10, 2026

Merge develop (5c809d0) into main

fc3677e

5c809d0, ci: add security scanners to Quartz (#90), Ciprian Goea (ciprian.goea@amd.com), Thu Sep 10 15:35:30 2026 +0300

Conversation

cgoea commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

Submission Checklist

Uh oh!

github-advanced-security AI commented Sep 9, 2026

What Enabling Code Scanning Means:

Uh oh!

HereThereBeDragons left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HereThereBeDragons left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cgoea commented Aug 31, 2026 •

edited

Loading