feat: add detailed SARIF findings to security scan summaries - #223
Conversation
📝 WalkthroughWalkthroughCompute EFFECTIVE_REF for remaps; CodeQL writes SARIF to a directory; Trivy renamed and grouped severities with per-finding collapsible summaries; added inline fail-on-critical/high security step and inline link-check failure; Dockerfiles add unzip/cleanup, Trivy annotations, and a non-root runner stage. Changes
Sequence Diagram(s)sequenceDiagram
actor Dev as Developer
participant GH as GitHub Actions
participant Checkout as Checkout/Remap
participant CodeQL as CodeQL Scanner
participant Trivy as Trivy Scanner
participant Lychee as Lychee (Link Checker)
participant Art as SARIF/Reports
Dev->>GH: push / open PR triggers workflow
GH->>Checkout: compute EFFECTIVE_REF
GH->>CodeQL: run CodeQL -> write SARIF to codeql-results/
GH->>Trivy: run Trivy -> generate findings (grouped severities)
GH->>Lychee: run link checks using EFFECTIVE_REF -> produce html-link-report.md
CodeQL->>Art: upload SARIF directory
Trivy->>Art: upload/report findings and summary blocks
GH->>GH: render collapsible summaries from findings
GH->>GH: run inline checks -> fail if critical/high findings or broken links
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (7)📓 Common learnings📚 Learning: 2024-12-07T23:06:33.954ZApplied to files:
📚 Learning: 2026-01-09T01:58:54.241ZApplied to files:
📚 Learning: 2026-01-07T04:40:01.060ZApplied to files:
📚 Learning: 2026-01-07T17:36:32.578ZApplied to files:
📚 Learning: 2025-12-17T13:27:43.679ZApplied to files:
📚 Learning: 2026-01-07T04:14:30.762ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (8)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/build.yml:
- Around line 182-185: The jq pipeline that writes Trivy SARIF to
$GITHUB_STEP_SUMMARY uses message text directly and can contain '|' which breaks
the Markdown table; update the jq expression in the command that reads
trivy-results.sarif (the jq -r '.runs[0].results[] | ... \(.message.text |
gsub("\n"; " ") | .[0:80]) |' ...' segment) to sanitize pipes by adding a gsub
to replace '|' with an escaped entity (for example append | gsub("\\|";
"|") so \(.message.text | gsub("\n"; " ") | gsub("\\|"; "|") |
.[0:80]) is used), ensuring table cells are not broken while keeping the rest of
the formatting the same.
- Around line 131-134: The table can break if message.text contains pipe
characters; update the jq pipeline that builds the table (the expression
starting with .runs[0].results[] | ... \(.message.text | gsub("\n"; " ") |
.[0:80]) ) to sanitize pipes before truncation, e.g. apply an additional gsub to
replace or escape "|" (for example gsub("\\|"; "|") or gsub("\\|";
"\\\\|")) so the output no longer injects literal "|" into the Markdown table
cell.
- Around line 197-202: The upload step "Upload CodeQL scan results to GitHub
Security tab" currently hardcodes sarif_file as "codeql-results/java.sarif",
which misses other languages; change the upload-sarif invocation (uses:
github/codeql-action/upload-sarif@...) so it uploads all SARIFs by pointing
sarif_file to the directory or glob (e.g., the "codeql-results" directory or
"codeql-results/*.sarif") or iterate over files similarly to the summary step,
keeping the conditional on env.UPLOAD_SCAN_SARIF == 'true' intact.
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
166-172: Variable nameCRITICALis misleading—it counts Critical+High combined.The comment correctly notes this maps to both CRITICAL and HIGH, but the variable name
CRITICALsuggests only critical findings. Consider renaming toCRITICAL_HIGHfor clarity and consistency with other references in this file (e.g., "critical/high" in the summary table label).✨ Suggested rename for clarity
- CRITICAL=$(jq -r '[.runs[0].results[] | select(.level == "error")] | length' trivy-results.sarif 2>/dev/null || echo "0") + CRITICAL_HIGH=$(jq -r '[.runs[0].results[] | select(.level == "error")] | length' trivy-results.sarif 2>/dev/null || echo "0") MEDIUM=$(jq -r '[.runs[0].results[] | select(.level == "warning")] | length' trivy-results.sarif 2>/dev/null || echo "0") LOW=$(jq -r '[.runs[0].results[] | select(.level == "note")] | length' trivy-results.sarif 2>/dev/null || echo "0") echo "| Severity | Count |" >> $GITHUB_STEP_SUMMARY echo "|----------|-------|" >> $GITHUB_STEP_SUMMARY - echo "| :red_circle: Critical/High | $CRITICAL |" >> $GITHUB_STEP_SUMMARY + echo "| :red_circle: Critical/High | $CRITICAL_HIGH |" >> $GITHUB_STEP_SUMMARY
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yml
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 0
File: :0-0
Timestamp: 2026-01-07T04:14:30.762Z
Learning: For Trivy security scanning in GitHub Actions workflows, the standard approach is to group findings by severity rather than by scanner type (vuln, secret, misconfig). This provides a practical security-focused summary.
Learnt from: david-waltermire
Repo: metaschema-framework/oscal-cli PR: 0
File: :0-0
Timestamp: 2026-01-07T17:36:32.578Z
Learning: In metaschema-framework repositories, nightly builds skip security scans (CodeQL and Trivy) because: (1) security scans run on every PR and push to main/develop/release branches providing continuous coverage, (2) GitHub Dependabot provides real-time alerts for dependency vulnerabilities between PRs, and (3) nightly builds focus on catching integration issues with upstream dependencies rather than security vulnerabilities. This is an intentional optimization that maintains security posture.
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 623
File: .github/workflows/build.yml:8-14
Timestamp: 2026-01-07T04:40:01.060Z
Learning: In the metaschema-java repository, the build workflow intentionally includes `main` in the `pull_request.branches` trigger even though CONTRIBUTING.md requires PRs to target `develop`. This is a defensive design to provide feedback if someone accidentally targets main, rather than silent failure. The UPLOAD_SCAN_SARIF logic also needs main for security scan results.
📚 Learning: 2026-01-07T17:36:32.578Z
Learnt from: david-waltermire
Repo: metaschema-framework/oscal-cli PR: 0
File: :0-0
Timestamp: 2026-01-07T17:36:32.578Z
Learning: In metaschema-framework repositories, nightly builds skip security scans (CodeQL and Trivy) because: (1) security scans run on every PR and push to main/develop/release branches providing continuous coverage, (2) GitHub Dependabot provides real-time alerts for dependency vulnerabilities between PRs, and (3) nightly builds focus on catching integration issues with upstream dependencies rather than security vulnerabilities. This is an intentional optimization that maintains security posture.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2026-01-07T04:40:01.060Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 623
File: .github/workflows/build.yml:8-14
Timestamp: 2026-01-07T04:40:01.060Z
Learning: In the metaschema-java repository, the build workflow intentionally includes `main` in the `pull_request.branches` trigger even though CONTRIBUTING.md requires PRs to target `develop`. This is a defensive design to provide feedback if someone accidentally targets main, rather than silent failure. The UPLOAD_SCAN_SARIF logic also needs main for security scan results.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2026-01-07T04:14:30.762Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 0
File: :0-0
Timestamp: 2026-01-07T04:14:30.762Z
Learning: For Trivy security scanning in GitHub Actions workflows, the standard approach is to group findings by severity rather than by scanner type (vuln, secret, misconfig). This provides a practical security-focused summary.
Applied to files:
.github/workflows/build.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Website
- GitHub Check: Code
🔇 Additional comments (4)
.github/workflows/build.yml (4)
41-42: LGTM!The
UPLOAD_SCAN_SARIFcondition correctly covers direct pushes to protected branches and PRs targeting them, ensuring security findings are visible in the GitHub Security tab. The inclusion ofmainaligns with the repository's defensive design for security scan uploads. Based on learnings, this is the expected behavior.
104-108: LGTM!Deferring the SARIF upload with
upload: 'never'and storing results locally allows the summary to be generated and uploads to complete before the build failure check—exactly as the PR intends.
151-159: LGTM!The Trivy configuration comprehensively scans for vulnerabilities, secrets, and misconfigurations. SARIF output enables GitHub Security tab integration, and including all severity levels ensures complete visibility. Based on learnings, this aligns with the standard approach for Trivy scanning.
209-235: LGTM!The fail-on-findings step correctly:
- Runs after SARIF uploads, ensuring results are captured before the build fails
- Checks both CodeQL and Trivy for error-level (critical/high) findings
- Accumulates failures using the
FAILEDflag pattern so both scanners are fully checked- Provides clear
::error::annotations indicating which scanner detected issuesThis meets the PR objective of failing the build on critical/high findings while ensuring uploads complete first.
cc4a94c to
1fc9441
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Dockerfile.ci (1)
6-9: Add explanatory comments to DS029 ignores for consistency with DS026.The two DS029 ignores are necessary (each applies to a different instruction due to Trivy's per-instruction ignore scope), but lack inline explanations for clarity. Add descriptive text matching the style of DS026 on line 2.
📝 Suggested improvement
-# trivy:ignore:DS029 +# trivy:ignore:DS029 - Builder stage requires root for system package installation FROM ${BUILDER_IMAGE} AS builder -# trivy:ignore:DS029 +# trivy:ignore:DS029 - Builder stage requires root for package installation RUN apt-get update && apt-get install -y unzip
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Dockerfile.ciDockerfile.local
✅ Files skipped from review due to trivial changes (1)
- Dockerfile.local
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 0
File: :0-0
Timestamp: 2026-01-07T04:14:30.762Z
Learning: For Trivy security scanning in GitHub Actions workflows, the standard approach is to group findings by severity rather than by scanner type (vuln, secret, misconfig). This provides a practical security-focused summary.
Learnt from: david-waltermire
Repo: metaschema-framework/oscal-cli PR: 0
File: :0-0
Timestamp: 2026-01-07T17:36:32.578Z
Learning: In metaschema-framework repositories, nightly builds skip security scans (CodeQL and Trivy) because: (1) security scans run on every PR and push to main/develop/release branches providing continuous coverage, (2) GitHub Dependabot provides real-time alerts for dependency vulnerabilities between PRs, and (3) nightly builds focus on catching integration issues with upstream dependencies rather than security vulnerabilities. This is an intentional optimization that maintains security posture.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Website
- GitHub Check: Code
🔇 Additional comments (1)
Dockerfile.ci (1)
2-2: LGTM: DS026 ignore is appropriately justified.CLI tools that execute and exit—like oscal-cli—have no ongoing health state to monitor, making HEALTHCHECK inapplicable. The inline ignore format is correct and properly documented.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
168-179: Consider increasing message truncation limit.The message text is truncated to 80 characters, which might cut off important context for security findings. Consider increasing to 120-150 characters for better readability while keeping the summary manageable.
📝 Suggested adjustment
- jq -r '.runs[0].results[] | "| \(.level // "warning") | \(.ruleId // "unknown") | `\(.locations[0].physicalLocation.artifactLocation.uri // "unknown"):\(.locations[0].physicalLocation.region.startLine // "?")` | \(.message.text | gsub("\n"; " ") | gsub("\\|"; "\\\\|") | .[0:80]) |"' "$sarif" >> $GITHUB_STEP_SUMMARY + jq -r '.runs[0].results[] | "| \(.level // "warning") | \(.ruleId // "unknown") | `\(.locations[0].physicalLocation.artifactLocation.uri // "unknown"):\(.locations[0].physicalLocation.region.startLine // "?")` | \(.message.text | gsub("\n"; " ") | gsub("\\|"; "\\\\|") | .[0:150]) |"' "$sarif" >> $GITHUB_STEP_SUMMARY
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yml
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 0
File: :0-0
Timestamp: 2026-01-07T04:14:30.762Z
Learning: For Trivy security scanning in GitHub Actions workflows, the standard approach is to group findings by severity rather than by scanner type (vuln, secret, misconfig). This provides a practical security-focused summary.
Learnt from: david-waltermire
Repo: metaschema-framework/oscal-cli PR: 0
File: :0-0
Timestamp: 2026-01-07T17:36:32.578Z
Learning: In metaschema-framework repositories, nightly builds skip security scans (CodeQL and Trivy) because: (1) security scans run on every PR and push to main/develop/release branches providing continuous coverage, (2) GitHub Dependabot provides real-time alerts for dependency vulnerabilities between PRs, and (3) nightly builds focus on catching integration issues with upstream dependencies rather than security vulnerabilities. This is an intentional optimization that maintains security posture.
📚 Learning: 2026-01-07T04:14:30.762Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 0
File: :0-0
Timestamp: 2026-01-07T04:14:30.762Z
Learning: For Trivy security scanning in GitHub Actions workflows, the standard approach is to group findings by severity rather than by scanner type (vuln, secret, misconfig). This provides a practical security-focused summary.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2026-01-07T17:36:32.578Z
Learnt from: david-waltermire
Repo: metaschema-framework/oscal-cli PR: 0
File: :0-0
Timestamp: 2026-01-07T17:36:32.578Z
Learning: In metaschema-framework repositories, nightly builds skip security scans (CodeQL and Trivy) because: (1) security scans run on every PR and push to main/develop/release branches providing continuous coverage, (2) GitHub Dependabot provides real-time alerts for dependency vulnerabilities between PRs, and (3) nightly builds focus on catching integration issues with upstream dependencies rather than security vulnerabilities. This is an intentional optimization that maintains security posture.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2024-12-07T23:06:33.954Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema PR: 54
File: .github/workflows/workflow-validate-repo-markdown.yml:31-31
Timestamp: 2024-12-07T23:06:33.954Z
Learning: In the `.github/workflows/workflow-validate-repo-markdown.yml` file, the command setting the `files` variable in the `Identify Markdown files` step does not require additional quoting, as the inputs prevent issues with word splitting and globbing.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2026-01-07T04:40:01.060Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 623
File: .github/workflows/build.yml:8-14
Timestamp: 2026-01-07T04:40:01.060Z
Learning: In the metaschema-java repository, the build workflow intentionally includes `main` in the `pull_request.branches` trigger even though CONTRIBUTING.md requires PRs to target `develop`. This is a defensive design to provide feedback if someone accidentally targets main, rather than silent failure. The UPLOAD_SCAN_SARIF logic also needs main for security scan results.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2025-12-17T13:27:43.679Z
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: All changes require PR review with CODEOWNERS enforcement
Applied to files:
.github/workflows/build.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Code
🔇 Additional comments (7)
.github/workflows/build.yml (7)
194-194: LGTM - Improved terminology.The step name now reflects the broader scope of Trivy scanning (vulnerabilities, secrets, and misconfigurations), which is more accurate than "vulnerability scanner" alone.
221-232: LGTM - Consistent implementation.The Trivy details section follows the same pattern as CodeQL, providing good consistency. The same message truncation consideration from the CodeQL section applies here as well.
352-352: LGTM - Dynamic branch reference improves link validation.The use of
${{ github.base_ref || github.ref_name }}correctly adapts to the workflow context: using the target branch for PRs and the current branch for direct pushes. This ensures links are validated against the correct branch version.
369-371: LGTM - Robust error handling for grep.The addition of
|| ERRORS=0and|| TIMEOUTS=0correctly handles the case when grep finds no matches (exit code 1), preventing the script from failing. The inline comment clearly explains this behavior.
406-415: LGTM - Simplified and consistent link check failure logic.The inline implementation is cleaner than a GitHub Script approach and maintains consistency with the summary step's error detection logic. The same robust grep error handling and clear error message improve the workflow's maintainability.
254-287: CodeQL security-severity threshold is correctly implemented.The
security-severity >= 7.0threshold properly captures both high (7.0-8.9) and critical (>9.0) severity levels according to GitHub's SARIF specification. Implementation aligns with PR objectives and requires no changes.
246-246: Theupload-sarifaction correctly supports directory paths for thesarif_fileparameter.The change from uploading a specific file (
codeql-results/java.sarif) to uploading the directory (codeql-results) is valid and enables multi-language SARIF uploads. When uploading a directory with multiple SARIF files, ensure each SARIF run has a uniquerunAutomationDetails.id, or use the action's optionalcategoryinput to differentiate them.
|
@coderabbitai Addressed nitpicks in commit 49b0122:
|
|
Tip For best results, initiate chat on the files or code changes. @david-waltermire Thank you for addressing those nitpicks! The changes look good:
🧠 Learnings used |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/build.yml:
- Around line 261-304: The jq expression used to compute CODEQL_CRITICAL can
fail when a rule's properties["security-severity"] is non-numeric; wrap the
tonumber call with a try/catch to safely coerce non-numeric values to 0. Update
the CODEQL_CRITICAL calculation (the jq string assigned to CODEQL_CRITICAL) to
replace (($severities[.ruleId] // "0") | tonumber >= 7.0) with
(($severities[.ruleId] // "0") | try tonumber catch 0) >= 7.0 so non-numeric
severities don't break the check.
- Line 253: The workflow currently sets sarif_file: codeql-results for the
upload-sarif action; ensure that any multiple .sarif files placed in the
codeql-results directory will not collide by either (1) verifying the generated
SARIF files include unique runAutomationDetails.id values per scan or (2)
splitting uploads so each upload-sarif step targets a single SARIF file or
directory and uses a distinct category (e.g., category: 'codeql' for CodeQL and
a different category per other runs) to guarantee uniqueness and avoid GitHub
SARIF upload collisions.
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
228-239: Inconsistent message truncation between scanners.CodeQL truncates messages at 120 characters (line 182) while Trivy truncates at 80 characters (line 236). Consider aligning these for consistent presentation.
♻️ Suggested fix
- jq -r '.runs[0].results[] | "| \(.level // "warning") | \(.ruleId // "unknown") | `\(.locations[0].physicalLocation.artifactLocation.uri // "unknown"):\(.locations[0].physicalLocation.region.startLine // "?")` | \(.message.text | gsub("\n"; " ") | gsub("\\|"; "\\\\|") | .[0:80]) |"' trivy-results.sarif >> $GITHUB_STEP_SUMMARY + jq -r '.runs[0].results[] | "| \(.level // "warning") | \(.ruleId // "unknown") | `\(.locations[0].physicalLocation.artifactLocation.uri // "unknown"):\(.locations[0].physicalLocation.region.startLine // "?")` | \(.message.text | gsub("\n"; " ") | gsub("\\|"; "\\\\|") | .[0:120]) |"' trivy-results.sarif >> $GITHUB_STEP_SUMMARY
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build.ymlDockerfile.ciDockerfile.local
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 0
File: :0-0
Timestamp: 2026-01-07T04:14:30.762Z
Learning: For Trivy security scanning in GitHub Actions workflows, the standard approach is to group findings by severity rather than by scanner type (vuln, secret, misconfig). This provides a practical security-focused summary.
Learnt from: david-waltermire
Repo: metaschema-framework/oscal-cli PR: 0
File: :0-0
Timestamp: 2026-01-07T17:36:32.578Z
Learning: In metaschema-framework repositories, nightly builds skip security scans (CodeQL and Trivy) because: (1) security scans run on every PR and push to main/develop/release branches providing continuous coverage, (2) GitHub Dependabot provides real-time alerts for dependency vulnerabilities between PRs, and (3) nightly builds focus on catching integration issues with upstream dependencies rather than security vulnerabilities. This is an intentional optimization that maintains security posture.
📚 Learning: 2026-01-07T04:14:30.762Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 0
File: :0-0
Timestamp: 2026-01-07T04:14:30.762Z
Learning: For Trivy security scanning in GitHub Actions workflows, the standard approach is to group findings by severity rather than by scanner type (vuln, secret, misconfig). This provides a practical security-focused summary.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2026-01-07T17:36:32.578Z
Learnt from: david-waltermire
Repo: metaschema-framework/oscal-cli PR: 0
File: :0-0
Timestamp: 2026-01-07T17:36:32.578Z
Learning: In metaschema-framework repositories, nightly builds skip security scans (CodeQL and Trivy) because: (1) security scans run on every PR and push to main/develop/release branches providing continuous coverage, (2) GitHub Dependabot provides real-time alerts for dependency vulnerabilities between PRs, and (3) nightly builds focus on catching integration issues with upstream dependencies rather than security vulnerabilities. This is an intentional optimization that maintains security posture.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2024-12-07T23:06:33.954Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema PR: 54
File: .github/workflows/workflow-validate-repo-markdown.yml:31-31
Timestamp: 2024-12-07T23:06:33.954Z
Learning: In the `.github/workflows/workflow-validate-repo-markdown.yml` file, the command setting the `files` variable in the `Identify Markdown files` step does not require additional quoting, as the inputs prevent issues with word splitting and globbing.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2026-01-07T04:40:01.060Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 623
File: .github/workflows/build.yml:8-14
Timestamp: 2026-01-07T04:40:01.060Z
Learning: In the metaschema-java repository, the build workflow intentionally includes `main` in the `pull_request.branches` trigger even though CONTRIBUTING.md requires PRs to target `develop`. This is a defensive design to provide feedback if someone accidentally targets main, rather than silent failure. The UPLOAD_SCAN_SARIF logic also needs main for security scan results.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2025-12-17T13:27:43.679Z
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: All changes require PR review with CODEOWNERS enforcement
Applied to files:
.github/workflows/build.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Code
🔇 Additional comments (14)
Dockerfile.local (4)
2-2: LGTM - Appropriate Trivy ignore annotation.The DS026 ignore is correctly documented with rationale. CLI tools that run and exit don't benefit from HEALTHCHECK.
6-6: LGTM - Helpful context comment.The comment clarifies the security model difference between stages, which aids maintainability.
11-11: LGTM - Correctly replaced ADD with COPY.COPY is preferred for local files as it's more explicit and doesn't have ADD's implicit tar extraction or remote URL fetching behaviors.
13-14: LGTM - Good hygiene improvements.The DS029 ignore is appropriately documented (builder legitimately needs root for package installation), and the apt cache cleanup reduces layer size.
Dockerfile.ci (4)
2-2: LGTM - Consistent Trivy ignore annotation.Same appropriate DS026 ignore as in Dockerfile.local for CLI tool without HEALTHCHECK.
6-9: LGTM - Good documentation and hygiene.The context comment and DS029 ignore with apt cleanup follow the same good patterns as Dockerfile.local.
15-25: LGTM - Solid non-root runtime setup.The runner stage correctly:
- Creates a dedicated system user/group
- Transfers ownership of the application directory
- Creates a writable working directory
- Switches to non-root user before execution
- Validates installation with version check
This follows container security best practices.
10-13: Fix wildcard COPY to avoid build failure with multiple matching files.The pattern
./target/*oscal-cli.zipwould cause a Docker build failure if multiple files match, since COPY with a wildcard can only copy to a directory destination, not a file. Docker will exit with "the destination path must be a directory when copying more than one source."Use a more specific pattern or adopt the approach in Dockerfile.local, which uses
findto locate and copy the file explicitly:RUN find ./target -iname '*oscal-cli.zip' -exec cp {} /tmp/oscal-cli.zip \;This ensures exactly one file is copied and the build succeeds reliably.
Likely an incorrect or invalid review comment.
.github/workflows/build.yml (6)
168-186: LGTM - Well-structured CodeQL findings table.The implementation correctly:
- Joins results with rules to retrieve
security-severityfrom rule definitions- Handles both driver and extension rules
- Escapes pipe characters and newlines for Markdown table safety
- Uses collapsible details for cleaner summaries
369-369: LGTM - Improved branch interpolation for remap URLs.Using
${{ github.base_ref || github.ref_name }}correctly handles both PR context (base_ref) and direct push context (ref_name) for the remap URL.
385-387: LGTM - Robust grep error handling.The pattern
$(command) || VAR=0correctly handles grep's exit code 1 when no matches are found, preventing false failures.
391-398: LGTM - Consistent collapsible details pattern.The broken links section now uses the same collapsible
<details>pattern as the security scan findings, providing a consistent UX.
426-435: LGTM - Clean inline failure check.Moving from GitHub Script to inline shell is simpler and maintains the same logic. The step correctly:
- Only runs when fail-on-error is enabled
- Uses the same grep pattern as the summary step
- Provides actionable error messaging
253-253: Directory input is properly supported by upload-sarif action.The
sarif_fileinput correctly accepts directories, and the action will automatically find and upload all.sariffiles withincodeql-results/. Ensure that any multiple SARIF runs have uniquerunAutomationDetails.idvalues or use distinct categories to avoid conflicts (GitHub requires this to prevent run combination issues).
c9dab3d to
d59568a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/build.yml:
- Line 369: The remap in the Lychee args currently uses `${{ github.base_ref ||
github.ref_name }}` which can mismatch when a workflow is invoked with
`inputs.ref`; update the workflow to compute a single effective ref variable
(e.g., `effective_ref`) that prefers `inputs.ref` when it is provided and looks
like a branch name, falling back to `github.base_ref || github.ref_name`
otherwise, and then use that variable in both remap entries instead of the
inline expression; ensure this logic is implemented in the workflow inputs/steps
so the `--remap "https://github.com/.../tree/${{ effective_ref }}/ ..."` strings
always match the checked-out content.
- Around line 261-304: Trivy SARIF handling is incorrect: HIGH and CRITICAL are
both mapped to .level == "error", so update the trivy check to count
high+critical correctly by changing TRIVY_HIGH (or replacing both TRIVY_* vars)
to query .level == "error" (e.g., make TRIVY_CRITICAL and TRIVY_HIGH both use jq
selection '.runs[0].results[] | select(.level == "error")' or combine into a
single TRIVY_ERRORS count) in the code that reads trivy-results.sarif so HIGH
findings are not missed; adjust the echo messages and FAILED flag use
accordingly (refer to variables TRIVY_CRITICAL, TRIVY_HIGH and the
trivy-results.sarif handling block).
🧹 Nitpick comments (3)
.github/workflows/build.yml (3)
168-186: Guard against oversized Step Summary output (CodeQL findings table).If
$RESULTSis large, dumping every row into$GITHUB_STEP_SUMMARYcan exceed GitHub’s summary limits and/or make the job sluggish. Consider capping rows (e.g., first N) and printing “+X more…” (or uploading a separate artifact with full details).Proposed tweak (cap rows)
- jq -r ' + MAX_ROWS=200 + jq -r --argjson max "$MAX_ROWS" ' (.runs[0].tool.driver.rules // []) as $driver_rules | ([.runs[0].tool.extensions[]?.rules // []] | add // []) as $ext_rules | ($driver_rules + $ext_rules | map({(.id): (.properties["security-severity"] // null)}) | add // {}) as $severities | - .runs[0].results[] | + (.runs[0].results[0:$max])[] | "| \(.level // "warning") | \($severities[.ruleId] // "N/A") | \(.ruleId // "unknown") | `\(.locations[0].physicalLocation.artifactLocation.uri // "unknown"):\(.locations[0].physicalLocation.region.startLine // "?")` | \(.message.text | gsub("\n"; " ") | gsub("\\|"; "\\\\|") | .[0:120]) |" ' "$sarif" >> $GITHUB_STEP_SUMMARY + REMAINING=$((RESULTS - MAX_ROWS)) + if [ "$REMAINING" -gt 0 ]; then + echo "" >> $GITHUB_STEP_SUMMARY + echo "_Showing first $MAX_ROWS findings; $REMAINING more omitted._" >> $GITHUB_STEP_SUMMARY + fi
228-239: Trivy details table shows.levelvalues; consider mapping to “Critical/High/…” for readability.Right now the “Severity” column will show
error|warning|note, which is less intuitive than the summary’s Critical/High labels. (Your severity grouping aligns with the repo’s preferred “group by severity” approach—nice.) Based on learnings, grouping by severity is preferred.Proposed tweak (map level to label)
- echo "| Severity | Rule | Location | Message |" >> $GITHUB_STEP_SUMMARY + echo "| Severity | Rule | Location | Message |" >> $GITHUB_STEP_SUMMARY echo "|----------|------|----------|---------|" >> $GITHUB_STEP_SUMMARY - jq -r '.runs[0].results[] | "| \(.level // "warning") | \(.ruleId // "unknown") | `\(.locations[0].physicalLocation.artifactLocation.uri // "unknown"):\(.locations[0].physicalLocation.region.startLine // "?")` | \(.message.text | gsub("\n"; " ") | gsub("\\|"; "\\\\|") | .[0:120]) |"' trivy-results.sarif >> $GITHUB_STEP_SUMMARY + jq -r ' + def sev: + if .level == "error" then "Critical" + elif .level == "warning" then "High" + elif .level == "note" then "Medium/Low" + else (.level // "Unknown") end; + .runs[0].results[] | + "| \(sev) | \(.ruleId // "unknown") | `\(.locations[0].physicalLocation.artifactLocation.uri // "unknown"):\(.locations[0].physicalLocation.region.startLine // "?")` | \(.message.text | gsub("\n"; " ") | gsub("\\|"; "\\\\|") | .[0:120]) |" + ' trivy-results.sarif >> $GITHUB_STEP_SUMMARY
380-398: Link checker summary + explicit failure step look good (nice handling ofgrep -cexit codes).
Small optional: you duplicate the broken-link regex in multiple steps; factoring it into one variable (or a small bash heredoc block) would reduce drift risk.Also applies to: 425-434
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yml
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 0
File: :0-0
Timestamp: 2026-01-07T04:14:30.762Z
Learning: For Trivy security scanning in GitHub Actions workflows, the standard approach is to group findings by severity rather than by scanner type (vuln, secret, misconfig). This provides a practical security-focused summary.
Learnt from: david-waltermire
Repo: metaschema-framework/oscal-cli PR: 0
File: :0-0
Timestamp: 2026-01-07T17:36:32.578Z
Learning: In metaschema-framework repositories, nightly builds skip security scans (CodeQL and Trivy) because: (1) security scans run on every PR and push to main/develop/release branches providing continuous coverage, (2) GitHub Dependabot provides real-time alerts for dependency vulnerabilities between PRs, and (3) nightly builds focus on catching integration issues with upstream dependencies rather than security vulnerabilities. This is an intentional optimization that maintains security posture.
📚 Learning: 2026-01-07T04:14:30.762Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 0
File: :0-0
Timestamp: 2026-01-07T04:14:30.762Z
Learning: For Trivy security scanning in GitHub Actions workflows, the standard approach is to group findings by severity rather than by scanner type (vuln, secret, misconfig). This provides a practical security-focused summary.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2026-01-07T17:36:32.578Z
Learnt from: david-waltermire
Repo: metaschema-framework/oscal-cli PR: 0
File: :0-0
Timestamp: 2026-01-07T17:36:32.578Z
Learning: In metaschema-framework repositories, nightly builds skip security scans (CodeQL and Trivy) because: (1) security scans run on every PR and push to main/develop/release branches providing continuous coverage, (2) GitHub Dependabot provides real-time alerts for dependency vulnerabilities between PRs, and (3) nightly builds focus on catching integration issues with upstream dependencies rather than security vulnerabilities. This is an intentional optimization that maintains security posture.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2024-12-07T23:06:33.954Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema PR: 54
File: .github/workflows/workflow-validate-repo-markdown.yml:31-31
Timestamp: 2024-12-07T23:06:33.954Z
Learning: In the `.github/workflows/workflow-validate-repo-markdown.yml` file, the command setting the `files` variable in the `Identify Markdown files` step does not require additional quoting, as the inputs prevent issues with word splitting and globbing.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2026-01-07T04:40:01.060Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 623
File: .github/workflows/build.yml:8-14
Timestamp: 2026-01-07T04:40:01.060Z
Learning: In the metaschema-java repository, the build workflow intentionally includes `main` in the `pull_request.branches` trigger even though CONTRIBUTING.md requires PRs to target `develop`. This is a defensive design to provide feedback if someone accidentally targets main, rather than silent failure. The UPLOAD_SCAN_SARIF logic also needs main for security scan results.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2025-12-17T13:27:43.679Z
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: All changes require PR review with CODEOWNERS enforcement
Applied to files:
.github/workflows/build.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Code
🔇 Additional comments (1)
.github/workflows/build.yml (1)
253-253: No changes needed. Theupload-sarifaction fully supports directory input for thesarif_fileparameter. The usage at line 253 (sarif_file: codeql-resultswithcategory: 'codeql') correctly follows the documented pattern—when uploading a directory, each SARIF file must have a unique identifier, which is satisfied here via the distinctcategoryinput.
d59568a to
8ee8b88
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/build.yml:
- Around line 75-76: The expression setting EFFECTIVE_REF uses a non-existent
length() function which breaks workflow evaluation; update the EFFECTIVE_REF
line to remove the length(inputs.ref) check and rely on the existing checks
(inputs.ref present, no '/' and not starting with 'v') to choose inputs.ref,
falling back to github.base_ref or github.ref_name (or, if you need strict SHA
detection, replace this single-expression logic with a separate step that
computes and exports EFFECTIVE_REF in bash where you can test string length).
- Around line 263-306: The Trivy SARIF severity comment and summary variable
names are inconsistent with the actual mapping; update the comment that precedes
the Trivy block to state "error → CRITICAL+HIGH, warning → MEDIUM, note →
LOW+UNKNOWN/INFO" and rename the summary variables to reflect SARIF levels
(e.g., change TRIVY_CRITICAL_HIGH to TRIVY_ERROR_LEVEL and TRIVY_MEDIUM to
TRIVY_WARNING_LEVEL) in the Trivy parsing block and in the subsequent messages;
adjust the echo messages to refer to "critical/high" when reporting
TRIVY_ERROR_LEVEL and "medium" when reporting TRIVY_WARNING_LEVEL so the
variable names, comments, and reported labels all match the SARIF mapping used
by the jq selects.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build.ymlDockerfile.ciDockerfile.local
🚧 Files skipped from review as they are similar to previous changes (1)
- Dockerfile.local
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 0
File: :0-0
Timestamp: 2026-01-07T04:14:30.762Z
Learning: For Trivy security scanning in GitHub Actions workflows, the standard approach is to group findings by severity rather than by scanner type (vuln, secret, misconfig). This provides a practical security-focused summary.
Learnt from: david-waltermire
Repo: metaschema-framework/oscal-cli PR: 0
File: :0-0
Timestamp: 2026-01-07T17:36:32.578Z
Learning: In metaschema-framework repositories, nightly builds skip security scans (CodeQL and Trivy) because: (1) security scans run on every PR and push to main/develop/release branches providing continuous coverage, (2) GitHub Dependabot provides real-time alerts for dependency vulnerabilities between PRs, and (3) nightly builds focus on catching integration issues with upstream dependencies rather than security vulnerabilities. This is an intentional optimization that maintains security posture.
Learnt from: david-waltermire
Repo: metaschema-framework/oscal-cli PR: 223
File: .github/workflows/build.yml:253-253
Timestamp: 2026-01-09T01:58:54.241Z
Learning: In metaschema-framework repositories, the CodeQL upload-sarif action intentionally uses a directory path (codeql-results) rather than a specific file path to support multi-language CodeQL scans where each language produces a separate .sarif file. The github/codeql-action/upload-sarif action accepts directory paths and uploads all .sarif files within, with distinct category values preventing upload collisions.
📚 Learning: 2024-12-07T23:06:33.954Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema PR: 54
File: .github/workflows/workflow-validate-repo-markdown.yml:31-31
Timestamp: 2024-12-07T23:06:33.954Z
Learning: In the `.github/workflows/workflow-validate-repo-markdown.yml` file, the command setting the `files` variable in the `Identify Markdown files` step does not require additional quoting, as the inputs prevent issues with word splitting and globbing.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2026-01-09T01:58:54.241Z
Learnt from: david-waltermire
Repo: metaschema-framework/oscal-cli PR: 223
File: .github/workflows/build.yml:253-253
Timestamp: 2026-01-09T01:58:54.241Z
Learning: In GitHub Actions workflow files (any .yml or .yaml under .github/workflows), configure CodeQL upload-sarif to point to a directory (e.g., codeql-results) rather than a single file. The github/codeql-action/upload-sarif action accepts a directory and will upload all SARIF files within, using distinct category values to avoid collisions. This approach supports multi-language CodeQL scans where each language may produce its own SARIF file.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2026-01-07T04:40:01.060Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 623
File: .github/workflows/build.yml:8-14
Timestamp: 2026-01-07T04:40:01.060Z
Learning: In the metaschema-java repository, the build workflow intentionally includes `main` in the `pull_request.branches` trigger even though CONTRIBUTING.md requires PRs to target `develop`. This is a defensive design to provide feedback if someone accidentally targets main, rather than silent failure. The UPLOAD_SCAN_SARIF logic also needs main for security scan results.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2026-01-07T17:36:32.578Z
Learnt from: david-waltermire
Repo: metaschema-framework/oscal-cli PR: 0
File: :0-0
Timestamp: 2026-01-07T17:36:32.578Z
Learning: In metaschema-framework repositories, nightly builds skip security scans (CodeQL and Trivy) because: (1) security scans run on every PR and push to main/develop/release branches providing continuous coverage, (2) GitHub Dependabot provides real-time alerts for dependency vulnerabilities between PRs, and (3) nightly builds focus on catching integration issues with upstream dependencies rather than security vulnerabilities. This is an intentional optimization that maintains security posture.
Applied to files:
.github/workflows/build.yml
📚 Learning: 2025-12-17T13:27:43.679Z
Learnt from: CR
Repo: metaschema-framework/metaschema-java PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:27:43.679Z
Learning: All changes require PR review with CODEOWNERS enforcement
Applied to files:
.github/workflows/build.yml
📚 Learning: 2026-01-07T04:14:30.762Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 0
File: :0-0
Timestamp: 2026-01-07T04:14:30.762Z
Learning: For Trivy security scanning in GitHub Actions workflows, the standard approach is to group findings by severity rather than by scanner type (vuln, secret, misconfig). This provides a practical security-focused summary.
Applied to files:
.github/workflows/build.yml
🪛 actionlint (1.7.10)
.github/workflows/build.yml
76-76: undefined function "length". available functions are "always", "cancelled", "contains", "endswith", "failure", "format", "fromjson", "hashfiles", "join", "startswith", "success", "tojson"
(expression)
🔇 Additional comments (11)
Dockerfile.ci (4)
2-2: LGTM: Appropriate Trivy annotation for CLI tools.The DS026 ignore is correctly applied since HEALTHCHECK is not applicable for CLI tools that execute commands and exit.
6-9: Excellent security documentation and cleanup.The explanatory comments clearly document the root vs. non-root user pattern across stages. The builder stage properly combines package installation and apt cache cleanup in a single RUN layer, following Docker best practices for image size optimization.
Based on PR objectives, these changes address the documented nitpicks in commit 49b0122.
13-13: LGTM: Binary made executable.Setting executable permissions on the oscal-cli binary ensures it can be invoked properly.
15-25: Excellent security implementation with non-root user.The runner stage properly implements the principle of least privilege:
- Creates a dedicated system user and group using
-rflag- Uses
-s /bin/falseto prevent shell access- Sets proper ownership of application directories
- Switches to non-root user before executing the application
- Validates the installation with a version check as the non-root user
This is a security best practice that reduces the attack surface of the container.
.github/workflows/build.yml (7)
170-188: LGTM! Excellent addition of per-finding details.The collapsible details section improves transparency by showing individual CodeQL findings with severity, rule, location, and message. The jq logic correctly joins results with rule definitions to extract security-severity values, and the message formatting (newline/pipe escaping, 120-char truncation) is appropriate.
203-203: LGTM! More accurate step name.The rename from "vulnerability scanner" to "security scanner" better reflects Trivy's multi-faceted scanning (vulnerabilities, secrets, misconfigurations).
230-241: LGTM! Consistent implementation of Trivy findings details.The collapsible details section for Trivy findings matches the CodeQL approach and correctly displays individual findings. The simpler jq query is appropriate since Trivy's SARIF format includes severity level directly on results.
Based on learnings, grouping findings by severity in the summary provides a practical security-focused view.
255-255: LGTM! Directory-based upload for multi-language support.The change to use a directory path (
codeql-results) rather than a specific file is correct. Theupload-sarifaction accepts directories and will upload all SARIF files within, using distinct category values to prevent collisions. This approach supports multi-language CodeQL scans where each language produces its own SARIF file.Based on learnings, this directory-based approach is the standard pattern for multi-language CodeQL scans.
371-371: Link remap logic depends on fixing EFFECTIVE_REF.The use of
${{ env.EFFECTIVE_REF }}in the remap URL is a good approach for dynamic branch-based link remapping. However, this depends on fixing the criticallength()function issue in the EFFECTIVE_REF definition at line 76.
382-409: LGTM! Improved error handling and user experience.The link checker summary improvements are solid:
- Better grep error handling prevents script failures when no matches are found
- Collapsible details section for broken links improves readability
- Consistent formatting with security scan summaries
428-437: LGTM! Clean inline failure logic.The inline approach to failing on broken links is cleaner and more explicit than relying on the lychee action's exit code. The logic correctly identifies actual broken links (ERROR, 4xx, 5xx) while excluding timeouts, and the error handling matches the summary section.
Add collapsible details sections showing individual findings from CodeQL and Trivy scans in the GitHub Actions Step Summary, with automatic build failure for high-severity security issues. CodeQL improvements: - Display findings table with level, security-severity, rule, location, message - Join results with rule definitions to get security-severity scores - Fail build on critical/high severity findings (security-severity >= 7.0) Trivy improvements: - Display findings table with level, rule, location, message - Fail build on critical/high severity findings Link Checker improvements: - Make results a visible heading with collapsible broken links list - Check report content for broken links instead of unreliable exit codes - Use dynamic branch ref in remap URL Dockerfile improvements: - Replace ADD with COPY (Docker best practice) - Add trivy:ignore comments with explanatory notes for builder stage - Clean up apt cache after package installation General fixes: - Truncate messages at 120 characters for readability - Include main branch in SARIF upload condition
8ee8b88 to
9bc6a3b
Compare
Replace complex heuristic (checking for '/' and 'v' prefix) with simple fallback chain: inputs.ref || github.base_ref || github.ref_name || 'develop' This is more predictable and handles all expected cases: - workflow_call with explicit ref: uses inputs.ref - PRs: uses github.base_ref (target branch) - Direct pushes: uses github.ref_name - Fallback: 'develop' as safe default
3d90de3
into
metaschema-framework:develop
1 participant
Summary
Test plan
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.