You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This change aligns this repository with the ROCm GitHub Organization Security Baseline and enables consistent security controls across ROCm repositories.
For more details, refer to the internal ROCm GitHub Org Security Controls Baseline Architecture documentation.
Support/questions: ROCm Security Discussions Teams channel.
⚠️ This PR modifies the Claude-review CI security perimeter
The following files in this PR control whether and how the claude-review workflow protects its LLM Gateway secrets at
runtime (see .github/workflows/CLAUDE_AUTO_REVIEW.md):
.github/workflows/pr-security-scan.yml
.github/workflows/weekly-security-scan.yml
Before applying the claude-review label on this PR, please:
Audit the diff in these paths line-by-line. A malicious or
accidental change could disable the --allowedTools
restriction, the overlay step, the sanitizer, or the
review/post job split -- any of which would expose the
secrets in env to the PR-modified workflow.
If the changes are legitimate and you want a Claude review,
do NOT apply the claude-review label. Instead, run via Actions → Claude Auto Review → Run workflow and enter
this PR's number. The dispatch path runs from the trusted,
code-owner-approved version of the workflow on develop, so a malicious PR-side
modification cannot affect the run.
(This banner is automated. The modifies-ci-paths label
will be removed automatically if a future push removes the
perimeter modifications. The Layer-3 in-workflow guard will
additionally fail the claude-review label-triggered run
on this PR if the label is applied while perimeter changes
are present.)
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.
Scope the security scanners to first-party code, harden the GitHub Actions workflows, replace unsafe subprocess usage, and resolve the Trivy dependency and Dockerfile findings.
Assisted-by: Cursor
Co-authored-by: Cursor <cursoragent@cursor.com>
The reason will be displayed to describe this comment to others. Learn more.
Verdict: REQUEST_CHANGES -- submitted as COMMENT (automated reviews are advisory) · Findings: 7 (0 Critical, 5 Major, 2 Minor)
Scope
Adds the ROCm security-scanning baseline (pr-security-scan.yml, weekly-security-scan.yml, dependabot.yml, four tool configs under .github/scan_tools_configs/) and folds in the fixes those scanners surfaced: SHA-pinned actions and $GITHUB_* env refs in ci.yml, scoped App-token permissions in the Claude auto-review workflows, --no-install-recommends + non-root USER in the CI Dockerfiles, shell=True removal in premerge-checks.py, and a GitPython bump. Mostly mechanical and well-commented; the remediations are where the risk is.
Findings
.github/scan_tools_configs/gitleaks.toml:19 — the (?i)\bsecret\s*= allowlist is repo-wide and matches the single most common real-credential variable name, materially weakening the gate this PR is adding.
.github/workflows/ci.yml:245 — lookup-only skips the download, so PR runs stop restoring build/ entirely; it also does not reliably suppress the save on a cache miss, so neither half of the adjacent comment holds.
mlir/utils/buildbot/mlir-rocm-mi200/Dockerfile:19 — derives from rocm/mlir but never switches to USER root, unlike Dockerfile.migraphx-ci which was correctly updated in this PR.
mlir/utils/jenkins/static-checks/premerge-checks.py:33 — the new git diff subprocess return code is unchecked; a git failure makes the clang-format gate pass silently.
.github/workflows/pr-security-scan.yml:2 — SPDX-License-Identifier: MIT plus a per-file copyright line conflicts with this repo's Apache-2.0 WITH LLVM-exception licensing and the no-per-file-copyright convention (same in the other four new files).
.github/scan_tools_configs/zizmor.yml:19 and .github/workflows/pr-security-scan.yml:5 — two minor accuracy/maintainability nits.
Notes
Spot-checked and confirmed present: claude_auto_review_perimeter_banner.yml and claude_auto_review_fork_notify.yml (both named in the dangerous-triggers ignore list), and ci.yml:240:9 is exactly the actions/cacheuses: line the cache-poisoning ignore targets.
dependabot.yml:14 lists /.github/actions/*; I could not find a composite action in that directory. If none exists yet, a glob matching nothing can fail the Dependabot job — worth confirming.
.gitignore:121 adds /external/*/.github. Those paths are almost certainly already tracked via the LLVM subtree, so .gitignore will not untrack them; it only affects traversal-based tools like zizmor. That is the stated intent, but future git add on those paths will now need -f.
The PR description links to internal AMD Confluence and Teams resources. Per the checklist's Critical bullet on internal-only hyperlinks in metadata, please reword those to plain-text references before merge.
CI status
No failing or cancelled checks; all four security-scan jobs are green and the remaining premerge jobs were still in progress at review time.
CodeQL scans vendored external tree without exclusion config
.github/workflows/pr-security-scan.yml:34
No codeql_config_path is supplied, so the pinned baseline analyzes the whole repository even in changed mode (that mode only limits the language matrix). Unlike the other scanner configs, this leaves the large vendored external/ tree in scope on every relevant PR, adding upstream findings and substantial analysis cost. Add a CodeQL config that excludes vendored paths and pass it here.
Weekly CodeQL scan includes vendored external tree
.github/workflows/weekly-security-scan.yml:36
The weekly all scan also omits codeql_config_path, so CodeQL analyzes the vendored external/ tree while every other scanner explicitly excludes it. This can turn the scheduled first-party scan into a large upstream LLVM analysis and surface findings this repository does not own. Pass the same CodeQL config excluding vendored paths here.
The reason will be displayed to describe this comment to others. Learn more.
Verdict: REQUEST_CHANGES -- submitted as COMMENT (automated reviews are advisory) · New findings: 2 (0 Critical, 0 Major, 2 Minor)
Scope
Imports the ROCm security-scanning baseline (pr-security-scan.yml, weekly-security-scan.yml, Dependabot, and per-tool configs under .github/scan_tools_configs/), plus a set of hardening fixes the scanners surfaced: SHA-pinned actions in ci.yml, ${{ }}-to-env-var conversion in run: blocks, narrowed App-token permission scopes, --no-install-recommends + non-root USER in the Jenkins/buildbot Dockerfiles, and shell=True removal in premerge-checks.py.
Findings
Five of the seven issues from the previous review round are fixed in this revision and their threads have been marked Resolved:
ci.yml build cache is now a correct actions/cache/restore + conditional actions/cache/save split, replacing the lookup-only approach; the brittle ci.yml:240:9 Zizmor cache-poisoning suppression is gone entirely as a result.
premerge-checks.py now inspects returncode on both git diff invocations and fails loudly instead of reporting a silent pass.
mlir/utils/buildbot/mlir-rocm-mi200/Dockerfile gained the USER root prologue it needed once the base image started ending on USER ubuntu.
The pre-commit.yml reference in pr-security-scan.yml now points at ci.yml.
CLAUDE_AUTO_REVIEW.md §15 gained the two new sync-point rows for the companion-filename and dummy-key couplings.
Two previously-raised issues are still open and remain the reason for this verdict — the existing threads are still visible on the diff, so they are not re-posted here:
.github/scan_tools_configs/gitleaks.toml:57 — the unscoped (?i)\bsecret\s*= allowlist still suppresses real credentials repository-wide.
.github/workflows/pr-security-scan.yml:2 — the SPDX-License-Identifier: MIT header (also in weekly-security-scan.yml and dependabot.yml) still conflicts with the repo's Apache-2.0 WITH LLVM-exception licensing.
Two new Minor findings are posted inline.
Notes
mlir/utils/jenkins/Dockerfile ending on USER ubuntu is a breaking change for anything deriving from rocm/mlir:*. Both in-repo derived images were updated; worth confirming no out-of-tree Dockerfile or Jenkins job assumes a root default.
CI status
No failing checks. Security scan / zizmor, gitleaks, bandit, trivy, and the codeql sub-jobs are green; the remaining premerge jobs and Security scan / codeql / complete were still in progress at review time.
The reason will be displayed to describe this comment to others. Learn more.
Verdict: REQUEST_CHANGES -- submitted as COMMENT (automated reviews are advisory) · New findings: 0 (0 Critical, 0 Major, 0 Minor)
Scope
This PR imports the ROCm organization security-scanning baseline (pr-security-scan.yml, weekly-security-scan.yml, dependabot.yml, and per-tool configs for Bandit/Gitleaks/Trivy/Zizmor) and, across review iterations, has grown to include CI hardening: SHA-pinned actions in ci.yml, untrusted-input handling via $GITHUB_BASE_REF, a requirements.txt for the premerge linters, a restore/save cache split, non-root USER lines in the three Dockerfiles, and subprocess return-code checks in premerge-checks.py.
Findings
One Major finding from the previous review is still open: the Gitleaks allowlist in .github/scan_tools_configs/gitleaks.toml still contains the repository-wide (?i)\bsecret\s*= pattern with no paths restriction and no condition = "AND", so it suppresses that match everywhere rather than only in test fixtures. In a PR whose purpose is to add security gates, this measurably weakens one of them. The existing inline thread is the place to continue that discussion — it has not been re-posted here.
Six previously-flagged items are confirmed fixed at this revision: the lookup-only cache misuse, the missing USER root in the MI200 Dockerfile, the ignored git diff return code, the brittle line-anchored zizmor ignore, the stale pre-commit.yml reference, and the duplicated pip pins. The MIT SPDX-header comment is withdrawn — the checklist's license-header rule covers new .cpp/.h/.py sources, not workflow YAML, and a maintainer confirmed it does not apply here.
Notes
Spot-checked the cache split: cache-primary-key is a documented actions/cache/restore output and the save gate (non-PR event plus cache-hit != 'true') is correct.
ubuntu:24.04 does ship a default ubuntu uid-1000 account, so USER ubuntu in mlir/utils/jenkins/Dockerfile resolves. Worth confirming that Jenkins jobs consuming rocm/mlir:*-latest pass docker run -u, since the image no longer defaults to root.
Both workflow filenames referenced by the zizmor dangerous-triggers ignore exist in the tree.
Dependabot is scoped to github-actions only, so the new Python pins stay manual. Grouped daily SHA bumps will also interact with the "re-verify internals on bump" sync row in CLAUDE_AUTO_REVIEW.md §15 — advisory, not blocking.
CI status
No failing or cancelled checks. The completed security scans (bandit, gitleaks, trivy, zizmor) all pass; CodeQL and the Jenkins premerge jobs are still in progress.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
modifies-ci-pathsPR modifies the Claude review CI security perimeter; audit before applying claude-review
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
rocmlir-pr-reviewer
Bot
added
the
modifies-ci-paths
![rocmlir-pr-reviewer[bot]](https://avatars.githubusercontent.com/u/7440191?s=60&v=4){kind=small}
Security Scanning Workflows Enablement
This PR adds standardized security scanning workflows and configurations from
ROCm/rocm-repo-template.Security Controls
Files added / updated
.github/workflows/pr-security-scan.yml— scans every pull request.github/workflows/weekly-security-scan.yml— scheduled weekly scan.github/dependabot.yml— automated dependency updates.github/scan_tools_configs/bandit.yml— Python static analysis config.github/scan_tools_configs/gitleaks.toml— secret detection config.github/scan_tools_configs/trivy.yml— container/dependency vulnerability config.github/scan_tools_configs/zizmor.yml— GitHub Actions security configWhy
This change aligns this repository with the ROCm GitHub Organization Security Baseline and enables consistent security controls across ROCm repositories.
For more details, refer to the internal ROCm GitHub Org Security Controls Baseline Architecture documentation.
Support/questions: ROCm Security Discussions Teams channel.