Add OSS license and vulnerability report workflow#802
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds an OSS license and vulnerability reporting workflow triggered by pushes, pull requests, and manual dispatch. A Python collector scans dependency manifests and emits inventory reports. A Bash orchestrator runs REUSE and Trivy with fallback artifacts, generates Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant run_oss_license_vulnerability_report.sh
participant collect_oss_dependencies.py
participant REUSE
participant Trivy
participant ReportArtifact
GitHubActions->>run_oss_license_vulnerability_report.sh: run OSS report
run_oss_license_vulnerability_report.sh->>collect_oss_dependencies.py: collect dependency inventory
run_oss_license_vulnerability_report.sh->>REUSE: generate lint and SPDX outputs
run_oss_license_vulnerability_report.sh->>Trivy: generate vulnerability and license scans
run_oss_license_vulnerability_report.sh-->>GitHubActions: generate summary.md
GitHubActions->>ReportArtifact: upload oss-report
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Ready for review as a report-only workflow. @qingsi-at-nv @aristarkhovNV, please review the implementation and its evidence boundaries. The PR intentionally does not set enforcement thresholds or claim repository-wide vulnerability compliance. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/oss-license-vulnerability-report.yml (1)
38-39: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winTrivy binary version is unpinned.
setup-trivyis pinned tov0.2.6, but without aversion:input it installs the latest Trivy binary, so scan/report results can drift between runs without any workflow change — undermining the reproducibility of the evidence this report is meant to provide.Confirmed via web search that `aquasecurity/setup-trivy` requires an explicit `version:` input to pin the installed Trivy binary; flagged by zizmor's `unpinned-tools` rule.♻️ Proposed fix
- name: Install Trivy uses: aquasecurity/setup-trivy@v0.2.6 + with: + version: v0.72.0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/oss-license-vulnerability-report.yml around lines 38 - 39, Update the “Install Trivy” step using aquasecurity/setup-trivy to provide an explicit version input for the Trivy binary, selecting and retaining a fixed version so workflow scans remain reproducible.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/oss-license-vulnerability-report.yml:
- Around line 25-26: Update the “Checkout code” actions/checkout step to disable
credential persistence, ensuring the checkout token is not stored on disk for
subsequently installed third-party tools.
In `@scripts/collect_oss_dependencies.py`:
- Around line 213-234: Update image parsing in _parse_dockerfile to split the
image reference at the final tag separator, preserving registry host ports in
name while extracting only the image version. Keep digest removal and entry
construction unchanged.
In `@scripts/run_oss_license_vulnerability_report.sh`:
- Around line 11-21: Separate the exit statuses for the two REUSE operations:
track the result of `reuse lint` in a dedicated `reuse_lint_status` while
retaining the SPDX export status independently. Update the downstream Python
heredoc, summary reporting, and strict-mode gating to use `reuse_lint_status`
for compliance decisions, while preserving SPDX failure reporting separately.
---
Nitpick comments:
In @.github/workflows/oss-license-vulnerability-report.yml:
- Around line 38-39: Update the “Install Trivy” step using
aquasecurity/setup-trivy to provide an explicit version input for the Trivy
binary, selecting and retaining a fixed version so workflow scans remain
reproducible.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0559cee0-1854-4e0a-9696-a959c326bb29
📒 Files selected for processing (5)
.github/workflows/oss-license-vulnerability-report.ymldocs/source/index.rstdocs/source/references/oss_license_vulnerability_report.rstscripts/collect_oss_dependencies.pyscripts/run_oss_license_vulnerability_report.sh
| - name: Checkout code | ||
| uses: actions/checkout@v6 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Checkout persists credentials unnecessarily.
This job doesn't need to push or authenticate further; persisting credentials to disk widens the blast radius if a subsequently-installed third-party tool (pip install reuse, aquasecurity/setup-trivy) is compromised.
🔒 Proposed fix
- name: Checkout code
uses: actions/checkout@v6
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Checkout code | |
| uses: actions/checkout@v6 | |
| - name: Checkout code | |
| uses: actions/checkout@v6 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 25-26: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/oss-license-vulnerability-report.yml around lines 25 - 26,
Update the “Checkout code” actions/checkout step to disable credential
persistence, ensuring the checkout token is not stored on disk for subsequently
installed third-party tools.
Source: Linters/SAST tools
| def _parse_dockerfile(path: Path, repo_root: Path) -> list[dict[str, Any]]: | ||
| entries: list[dict[str, Any]] = [] | ||
| for line_number, raw_line in enumerate( | ||
| path.read_text(encoding="utf-8", errors="ignore").splitlines(), 1 | ||
| ): | ||
| match = DOCKER_FROM_RE.match(raw_line) | ||
| if not match: | ||
| continue | ||
| image = match.group("image").split("@", maxsplit=1)[0] | ||
| name, _, version = image.partition(":") | ||
| entries.append( | ||
| _entry( | ||
| ecosystem="container", | ||
| manifest_type="Dockerfile", | ||
| name=name, | ||
| version=version, | ||
| path=path, | ||
| repo_root=repo_root, | ||
| line=line_number, | ||
| ) | ||
| ) | ||
| return entries |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Docker image parsing breaks on registries with an explicit port.
image.partition(":") (Line 222) splits on the first colon. For myregistry.io:5000/app:1.2.3, this yields name="myregistry.io" and version="5000/app:1.2.3" instead of name="myregistry.io:5000/app", version="1.2.3". Any private registry with a port in the host will produce garbage inventory entries.
🐛 Proposed fix
- image = match.group("image").split("@", maxsplit=1)[0]
- name, _, version = image.partition(":")
+ image = match.group("image").split("@", maxsplit=1)[0]
+ repo, _, tag_or_path = image.rpartition("/")
+ if ":" in tag_or_path:
+ last_segment, _, version = tag_or_path.rpartition(":")
+ name = f"{repo}/{last_segment}" if repo else last_segment
+ else:
+ name, version = image, ""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _parse_dockerfile(path: Path, repo_root: Path) -> list[dict[str, Any]]: | |
| entries: list[dict[str, Any]] = [] | |
| for line_number, raw_line in enumerate( | |
| path.read_text(encoding="utf-8", errors="ignore").splitlines(), 1 | |
| ): | |
| match = DOCKER_FROM_RE.match(raw_line) | |
| if not match: | |
| continue | |
| image = match.group("image").split("@", maxsplit=1)[0] | |
| name, _, version = image.partition(":") | |
| entries.append( | |
| _entry( | |
| ecosystem="container", | |
| manifest_type="Dockerfile", | |
| name=name, | |
| version=version, | |
| path=path, | |
| repo_root=repo_root, | |
| line=line_number, | |
| ) | |
| ) | |
| return entries | |
| def _parse_dockerfile(path: Path, repo_root: Path) -> list[dict[str, Any]]: | |
| entries: list[dict[str, Any]] = [] | |
| for line_number, raw_line in enumerate( | |
| path.read_text(encoding="utf-8", errors="ignore").splitlines(), 1 | |
| ): | |
| match = DOCKER_FROM_RE.match(raw_line) | |
| if not match: | |
| continue | |
| image = match.group("image").split("@", maxsplit=1)[0] | |
| repo, _, tag_or_path = image.rpartition("/") | |
| if ":" in tag_or_path: | |
| last_segment, _, version = tag_or_path.rpartition(":") | |
| name = f"{repo}/{last_segment}" if repo else last_segment | |
| else: | |
| name, version = image, "" | |
| entries.append( | |
| _entry( | |
| ecosystem="container", | |
| manifest_type="Dockerfile", | |
| name=name, | |
| version=version, | |
| path=path, | |
| repo_root=repo_root, | |
| line=line_number, | |
| ) | |
| ) | |
| return entries |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/collect_oss_dependencies.py` around lines 213 - 234, Update image
parsing in _parse_dockerfile to split the image reference at the final tag
separator, preserving registry host ports in name while extracting only the
image version. Keep digest removal and entry construction unchanged.
| reuse_status=0 | ||
| if command -v reuse >/dev/null 2>&1; then | ||
| # Run before generating artifacts so the report does not flag its own output. | ||
| reuse lint > "${output_dir}/reuse-lint.txt" 2>&1 || reuse_status=$? | ||
| reuse spdx -o "${output_dir}/reuse.spdx" || reuse_status=$? | ||
| else | ||
| reuse_status=127 | ||
| cat > "${output_dir}/reuse-lint.txt" <<'TXT' | ||
| reuse not installed | ||
| TXT | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
reuse_status conflates lint and spdx exit codes.
Both reuse lint and reuse spdx write into the same reuse_status. If lint passes but the spdx export fails, reuse_status becomes the spdx failure code, and the summary (Lines 137-140) will report "REUSE compliance: fail" even though the actual license/copyright lint passed. Strict mode (Lines 172-174) would also gate on the wrong signal.
🐛 Proposed fix
-reuse_status=0
+reuse_lint_status=0
+reuse_spdx_status=0
if command -v reuse >/dev/null 2>&1; then
# Run before generating artifacts so the report does not flag its own output.
- reuse lint > "${output_dir}/reuse-lint.txt" 2>&1 || reuse_status=$?
- reuse spdx -o "${output_dir}/reuse.spdx" || reuse_status=$?
+ reuse lint > "${output_dir}/reuse-lint.txt" 2>&1 || reuse_lint_status=$?
+ reuse spdx -o "${output_dir}/reuse.spdx" || reuse_spdx_status=$?
+ reuse_status=$reuse_lint_status
else
reuse_status=127
+ reuse_lint_status=127
+ reuse_spdx_status=127📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| reuse_status=0 | |
| if command -v reuse >/dev/null 2>&1; then | |
| # Run before generating artifacts so the report does not flag its own output. | |
| reuse lint > "${output_dir}/reuse-lint.txt" 2>&1 || reuse_status=$? | |
| reuse spdx -o "${output_dir}/reuse.spdx" || reuse_status=$? | |
| else | |
| reuse_status=127 | |
| cat > "${output_dir}/reuse-lint.txt" <<'TXT' | |
| reuse not installed | |
| TXT | |
| fi | |
| reuse_lint_status=0 | |
| reuse_spdx_status=0 | |
| if command -v reuse >/dev/null 2>&1; then | |
| # Run before generating artifacts so the report does not flag its own output. | |
| reuse lint > "${output_dir}/reuse-lint.txt" 2>&1 || reuse_lint_status=$? | |
| reuse spdx -o "${output_dir}/reuse.spdx" || reuse_spdx_status=$? | |
| reuse_status=$reuse_lint_status | |
| else | |
| reuse_status=127 | |
| reuse_lint_status=127 | |
| reuse_spdx_status=127 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/run_oss_license_vulnerability_report.sh` around lines 11 - 21,
Separate the exit statuses for the two REUSE operations: track the result of
`reuse lint` in a dedicated `reuse_lint_status` while retaining the SPDX export
status independently. Update the downstream Python heredoc, summary reporting,
and strict-mode gating to use `reuse_lint_status` for compliance decisions,
while preserving SPDX failure reporting separately.
|
Looking at the report summary, out of 206 discovered dependencies only 5 listed as Scanner-resolved? (what does it mean btw?) Trivy seems to only pickup |
|
@aristarkhovNV Yes: the 206 figure is the broad inventory across supported dependency declarations, while Trivy resolved only five concrete packages from docs/requirements.txt; only those five are claimed as vulnerability-scanned. The remaining references are inventory-only because they are source URLs, image/action references, ranges, variables, or manifest forms that Trivy did not resolve to concrete packages. Commit 1b84605 makes this distinction explicit by renaming the metric to Trivy-resolved concrete packages and listing the resolved target paths in the summary. It also addresses the other review findings: registry-port image parsing, separate REUSE lint/SPDX statuses, pinned Trivy, and non-persistent checkout credentials. The full checks are running. |
|
Fresh report evidence on 1b84605 is now terminal: run 29960121459 and artifact 8545637912 report 207 discovered references, 0 parse errors, 5 Trivy-resolved concrete packages from exactly docs/requirements.txt, and no vulnerability findings among those five. REUSE lint remains nonzero (938/995 license metadata; 939/995 copyright metadata) while SPDX export succeeds. The PR description now carries these current, report-only numbers; my earlier 206 count referenced the prior artifact. |
|
Current head 1b84605 is terminal green: 44 checks succeeded, 3 expected checks were skipped, and 0 failed. The requested reviewers remain Andrei and Qingsi; the report-only scope and exact resolved target are documented in the PR. |
|
@arussell-nvidia We need to resolve all dependencies including those introduced through CMake/FetchContent, npm, and vcpkg, into a canonical CycloneDX or SPDX SBOM, run Trivy against that SBOM, and report resolved, scanned, unresolved, and explicitly excluded dependencies separately. CMake coverage should come from configured build profiles, not only static regex parsing, and we need to configure a "max dependency" case for the coverage. Capture expanded FetchContent declarations including nested ones, and then generated vcpkg metadata, with every dependency either resolved or explicitly excluded. Also the coverage should fail closed, meaning that new manifests, build options, populated dependencies, or unresolved expressions must not be silently omitted, even while vulnerability and license findings remain advisory. collect_oss_dependencies.py will need substantial change to address the above and then the workflow yaml to consume its output artifacts. |
Configure the maximum public CMake profile, resolve npm/Python/vcpkg inputs, and fail closed when dependency declarations are omitted. Generate one canonical CycloneDX SBOM and scan that SBOM with Trivy. Signed-off-by: Andrew Russell <arussell@nvidia.com>
|
@qingsi-at-nv Implemented the requested dependency-resolution and coverage model in
Local validation passed: 3/3 focused unit tests, Ruff, workflow YAML parsing, and diff checks. The network-heavy resolver path is now running in CI. |
Keep compact CMake and vcpkg evidence while preventing Trivy from recursively resolving fetched dependency build trees. Extend fail-closed manifest detection to Conda, Pipenv, and Gradle inputs. Signed-off-by: Andrew Russell <arussell@nvidia.com>
Enable the maximum OGLO CMake profile so nlohmann_json is traced and pinned. Resolve Docker ARG defaults into concrete Python and ROS base image versions, with focused coverage. Signed-off-by: Andrew Russell <arussell@nvidia.com>
|
Resolver follow-up is green on current
Trivy reports two report-only findings against the same transitive package: The rest of the PR matrix is still running with no failures so far. |
Summary
CI evidence
OSS report workflow run 29960121459 completed successfully on commit
1b84605cand published artifact8545637912.The current report records:
docs/requirements.txtPolicy scope
This workflow is report-only by default. A successful job means evidence was generated; it does not certify that every inventoried reference was vulnerability-scanned or that the repository satisfies a final license policy. Strict thresholds remain a follow-up policy decision.
Local validation
Review status
Ready for implementation review as a report-only workflow. Final enforcement thresholds, policy ownership, and any broader compliance claim remain out of scope for this PR.
Summary by CodeRabbit