feat: add detailed SARIF findings to security scan summaries #237
Conversation
📝 WalkthroughWalkthroughIntroduces EFFECTIVE_REF env; changes CodeQL SARIF output to Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as GH Actions Runner
participant Checkout as Checkout (determine EFFECTIVE_REF)
participant CodeQL as CodeQL Action
participant Trivy as Trivy Action
participant LinkChecker as Link Checker Action
participant Parser as SARIF/Report Parser
participant Summary as Actions Summary
Runner->>Checkout: checkout repo (compute EFFECTIVE_REF)
Runner->>CodeQL: run CodeQL scan (write `codeql-results/`)
Runner->>Trivy: run Trivy scan (write `trivy-results.sarif`)
Runner->>LinkChecker: run link checks (write `html-link-report.md`)
Runner->>Parser: parse SARIFs and link report
Parser->>Summary: provide findings (counts, severities, per-finding details)
Summary->>Runner: render summaries and conditional collapsible details
alt critical/high findings present
Runner->>Summary: emit ::error and fail workflow
else link errors present
Runner->>Summary: emit ::error and fail workflow
else no blocking findings
Runner->>Summary: upload SARIFs and continue
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
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: 0
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
175-177: Pipe characters in messages may break table formatting.If the message text contains
|characters, the Markdown table will be malformed. Consider escaping pipes in the message: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"; " ") | .[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:80]) |"' "$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 (3)
📓 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
⏰ 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: Code
- GitHub Check: Website
🔇 Additional comments (2)
.github/workflows/build.yml (2)
214-235: LGTM!The severity grouping approach (Critical/High, Medium, Low) aligns with the standard practice for Trivy scanning. The collapsible details table provides useful visibility into individual findings without cluttering the summary.
Note: The same pipe character escaping consideration from the CodeQL section applies to line 232.
257-284: LGTM!The enforcement step is correctly positioned after the SARIF upload steps (lines 245-256), ensuring scan results are uploaded to GitHub Security tab before the build fails. This matches the PR objective to "run after summaries and SARIF uploads complete."
The error handling with conditional file/directory checks and jq fallbacks (
|| echo "0") is robust.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/build.yml:
- Around line 214-222: The SARIF severity mapping and displayed labels are
wrong: variable CRITICAL (from select(.level == "error")) maps only to CRITICAL,
MEDIUM (select(.level == "warning")) maps to HIGH, and LOW (select(.level ==
"note")) contains MEDIUM+LOW; update the inline comment and the echo labels to
reflect the true mapping (e.g., label CRITICAL as "Critical", rename MEDIUM
variable/label to "High", and split or reclassify note-level results into
distinct MEDIUM and LOW counts or create NOTE_MEDIUM/NOTE_LOW variables), adjust
the jq selectors or add new ones to accurately count note-level MEDIUM vs LOW if
you need both categories, and ensure any downstream failure/threshold logic that
uses CRITICAL/MEDIUM/LOW is updated to match the corrected severity grouping.
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
169-180: Consider adding a severity mapping comment for CodeQL.For consistency with the Trivy section (line 214), consider adding a comment explaining the CodeQL SARIF level mapping (e.g., which levels correspond to critical/high/medium/low severity). This would help readers understand what the
.levelfield represents in the table.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yml
🧰 Additional context used
🧠 Learnings (3)
📓 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
⏰ 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: Code
- GitHub Check: Website
🔇 Additional comments (3)
.github/workflows/build.yml (3)
224-235: LGTM!The Trivy details section implementation is consistent with the CodeQL section and correctly formats the findings table.
374-374: Good fix for GitHub Actions output typing.Correctly changed to string comparison (
== '2') to match GitHub Actions output typing, wheresteps.*.outputs.*values are strings, not integers.
257-284: The failure enforcement logic is well-structured and correct.The code properly checks
level == "error"for both CodeQL and Trivy, which correctly maps to critical/high severity findings per GitHub's SARIF standard. The defensive checks for file/directory existence are appropriate, error messages are clear, and the logic runs after SARIF uploads complete to ensure results are available in the Security tab.
david-waltermire
force-pushed
the
feature/sarif-summary-details
branch
from
512f210 to
c3a85af
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/build.yml:
- Around line 257-289: The CODEQL check only counts SARIF results with level ==
"error" (CODEQL_CRITICAL) and misses security findings that set
properties.security-severity; update the jq used when computing CODEQL_CRITICAL
(for files matching codeql-results/*.sarif) to count any result where .level ==
"error" OR where .properties["security-severity"] exists and its numeric value
maps to high/critical (e.g., tonumber >= 7.0) so both high and critical security
severities are detected; adjust the jq expression assigned to CODEQL_CRITICAL
accordingly and keep the rest of the failure logic unchanged.
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
169-180: Consider escaping additional markdown special characters in finding messages.The current implementation escapes pipe characters to prevent table corruption, which is good. However, other markdown special characters (backticks, asterisks, underscores, square brackets) in finding messages could still break table formatting or create unintended markdown rendering.
♻️ Enhanced escaping for robustness
- 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("\\|"; "\\\\|") | gsub("`"; "\\\\`") | gsub("\\*"; "\\\\*") | gsub("_"; "\\\\_") | .[0:80]) |"' "$sarif" >> $GITHUB_STEP_SUMMARYNote: This escapes backticks, asterisks, and underscores in addition to pipes. Apply the same change to the Trivy details section on line 232.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yml
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
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
⏰ 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)
224-235: LGTM with same escaping consideration as CodeQL section.This Trivy details section follows the same pattern as the CodeQL section above. If you implement the enhanced markdown character escaping suggested for CodeQL (lines 169-180), apply the same change here on line 232.
348-348: LGTM!Good formatting improvement to make the link checker header more prominent.
379-390: LGTM! Correct string comparison for GitHub Actions.The exit code comparisons correctly use string type (
'2') to match GitHub Actions output typing. The logic appropriately distinguishes between timeouts (exit code 1) and actual broken links (exit code 2), only triggering issues and failures for broken links.
249-249: Confirm SARIF files have uniquerunAutomationDetails.idvalues.The
upload-sarifaction does support directory input. However, when uploading multiple SARIF files from a directory, each file must include a uniquerunAutomationDetails.idto ensure results receive separate categories. Verify that the multi-language scans in this workflow generate SARIF files with distinct IDs.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/build.yml (2)
169-180: Consider UTF-8 safety in message truncation.The jq string truncation
.[0:80]operates at the character level but doesn't account for multi-byte UTF-8 sequences that could be split mid-character. While this is unlikely to cause failures (markdown rendering is usually forgiving), it could produce garbled output for messages containing emoji or non-ASCII characters.Additionally, 80 characters may be too short for meaningful context in security findings.
♻️ Optional improvements
Consider increasing the truncation limit to 120-150 characters for better context, or using jq's
@htmlor@urifor safer escaping:- 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
224-235: Same UTF-8 truncation concern applies here.The Trivy findings table uses the same jq truncation approach as the CodeQL section, so the same UTF-8 safety and message length considerations apply.
📜 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: 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.
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.
📚 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: 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
⏰ 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 (5)
.github/workflows/build.yml (5)
336-336: LGTM: Dynamic ref mapping handles both PR and push contexts.The use of
github.base_ref || github.ref_namecorrectly provides the target branch name in PR contexts and the current branch name in push contexts. This ensures the link checker can properly remap GitHub tree URLs to local workspace files in both scenarios.
351-361: LGTM: Improved error handling in link report extraction.The updated grep logic with
|| ERRORS=0and|| TIMEOUTS=0correctly handles the case where grep finds no matches (which would normally exit with code 1). This prevents spurious failures and makes the summary more robust.The regex pattern
^\[ERROR\]|^\[[45][0-9]{2}\]appropriately captures lychee's error formats for broken links.
381-381: LGTM: Correct string comparison for exit codes.GitHub Actions step outputs are strings, so the string comparison
== '2'is correct. The logic appropriately distinguishes between broken links (exit code 2, should fail/create issue) and timeouts (exit code 1, should not fail/create issue).Also applies to: 392-392
249-249: Theupload-sarifaction supports directory input as documented in the official GitHub documentation. When a directory path is provided tosarif_file, the action uploads all.sariffiles in that directory. This change from a specific file (codeql-results/java.sarif) to the directory (codeql-results) is valid and aligns with the CodeQL analyze action's output structure.
257-289: No issues found. CodeQL and Trivy correctly map critical/high severity findings tolevel: "error"(andlevel: "warning"for Trivy high severity) in their SARIF output, so the filtering logic in lines 265 and 275–276 is accurate.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/build.yml:
- Around line 265-266: The current jq expression calculating CODEQL_CRITICAL
reads .properties["security-severity"] from each result, but SARIF places
security-severity on rule definitions, not results; update the jq pipeline to
build a mapping from runs[0].tool.driver.rules[] keyed by ruleId to extract
properties["security-severity"] for each result (falling back to 0) and then
count results where level == "error" OR the looked-up security-severity (as
number) >= 7.0; target the existing variable/expr (CODEQL_CRITICAL and the jq
expression) and adjust it to join results with runs[0].tool.driver.rules via
ruleId to perform the proper severity-based filtering.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yml
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
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.
📚 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
⏰ 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 (7)
.github/workflows/build.yml (7)
169-180: LGTM!The collapsible details section for CodeQL findings is well-implemented. The jq expression correctly handles edge cases with default values, escapes pipe characters to preserve table formatting, and truncates long messages appropriately.
224-235: LGTM!The Trivy details section mirrors the CodeQL implementation for consistency. Based on learnings, grouping findings by severity is the standard approach for Trivy scanning, and this implementation provides both the severity summary table and individual finding details.
249-249: LGTM!Uploading the
codeql-resultsdirectory instead of a single file correctly handles scenarios where CodeQL generates multiple SARIF files (e.g., for different languages). Theupload-sarifaction supports both file and directory inputs.
337-337: LGTM!The remap URL correctly uses
github.base_reffor pull requests (which provides the target branch) and falls back togithub.ref_namefor push events. This ensures the link checker can validate links against local files regardless of the trigger type.
349-361: LGTM!The updated error detection logic correctly handles Lychee's output format with
[ERROR],[4xx], and[5xx]prefixes. The|| ERRORS=0pattern properly handlesgrep's exit code 1 when no matches are found, preventing the step from failing prematurely.
382-382: LGTM!Using string comparison
'2'for exit code is correct since GitHub Actions step outputs are strings. This appropriately limits issue creation to actual broken links (exit code 2) rather than timeouts (exit code 1).
391-399: LGTM!The shell-based approach directly inspects the link report for broken links, providing consistent behavior with the summary step. This is more robust than relying solely on exit codes and correctly handles edge cases like missing report files.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/build.yml (2)
169-180: Consider displaying security-severity in the summary table.The summary displays the SARIF
levelfield (warning/error), but the failure check at lines 267-276 correctly usessecurity-severityfrom rule definitions to determine criticality. This creates an inconsistency where a finding shown as "warning" in the summary might actually trigger a build failure due to high security-severity (≥7.0). Consider joining with rule definitions here as well to display the actual security-severity score alongside or instead of the level.Example: Display security-severity in summary
The jq query could be enhanced to show security-severity similar to the failure check logic:
- 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].tool.driver.rules // []) as $driver_rules | + ([.runs[0].tool.extensions[]?.rules // []] | add // []) as $ext_rules | + ($driver_rules + $ext_rules | map({(.id): (.properties["security-severity"] // "0")}) | add // {}) as $severities | + .runs[0].results[] | + "| \(.level // "warning") (severity: \($severities[.ruleId] // "N/A")) | \(.ruleId // "unknown") | `\(.locations[0].physicalLocation.artifactLocation.uri // "unknown"):\(.locations[0].physicalLocation.region.startLine // "?")` | \(.message.text | gsub("\n"; " ") | gsub("\\|"; "\\\\|") | .[0:80]) |" + ' "$sarif" >> $GITHUB_STEP_SUMMARY
404-413: Consider reducing code duplication with summary section.The error checking logic here (lines 408-409) duplicates the grep pattern from the summary section (line 364). While the current implementation is correct, consider extracting the error count to an environment variable in the summary step to avoid repeating the grep logic.
Example: Extract error count to environment variable
In the summary step, add after line 365:
TIMEOUTS=$(grep -c "^\[TIMEOUT\]" html-link-report.md 2>/dev/null) || TIMEOUTS=0 + echo "LINK_CHECK_ERRORS=$ERRORS" >> $GITHUB_ENVThen simplify the failure check:
- name: Fail on link check error if: ${{ !inputs.skip_linkcheck && !cancelled() && env.INPUT_FAIL_ON_ERROR == 'true' }} run: | - if [ -f "html-link-report.md" ]; then - ERRORS=$(grep -cE "^\[ERROR\]|^\[[45][0-9]{2}\]" html-link-report.md 2>/dev/null) || ERRORS=0 - if [ "$ERRORS" -gt 0 ]; then - echo "::error::Link checker found $ERRORS broken link(s). See report for details." - exit 1 - fi + if [ "${LINK_CHECK_ERRORS:-0}" -gt 0 ]; then + echo "::error::Link checker found $LINK_CHECK_ERRORS broken link(s). See report for details." + exit 1 fi
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yml
🧰 Additional context used
🧠 Learnings (3)
📚 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: 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
⏰ 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: Code
- GitHub Check: Website
🔇 Additional comments (6)
.github/workflows/build.yml (6)
224-235: LGTM: Trivy details section mirrors CodeQL structure.The collapsible details section is consistent with the CodeQL summary. Unlike CodeQL, Trivy's
levelfield directly maps to severity (error=CRITICAL, warning=HIGH, note=MEDIUM/LOW), so displaying the level is more appropriate here.
257-300: LGTM: Well-implemented security finding failure check.The jq query correctly joins CodeQL results with rule definitions to extract security-severity, addressing the issue mentioned in the commit message. The logic appropriately:
- Collects security-severity from both driver and extension rules
- Maps ruleId to severity values
- Filters for
level=="error"ORsecurity-severity >= 7.0- Handles missing files gracefully with
|| echo "0"The step runs unconditionally (no dependency on previous step success), ensuring the build fails fast on critical/high findings even if summaries or uploads encounter issues.
347-347: Verify branch reference in remap URL is correct.The remap URL uses
github.base_ref || github.ref_name, which points to the target branch for PRs (e.g., "develop") rather than the source branch being checked out (line 93). This means:
- For PRs: validates links against the target branch
- For pushes: validates links against the current branch
Ensure this is intentional. If the generated website content should be validated against the source branch's file structure, use
github.head_ref || github.ref_nameinstead.Based on learnings, could you clarify whether the remap should point to the PR's target branch (base_ref) or source branch (head_ref)?
358-376: LGTM: Improved error extraction and formatting.The updated logic correctly:
- Uses
grep -cEwith pattern^\[ERROR\]|^\[[45][0-9]{2}\]to match lychee's error formats- Handles grep's exit code 1 (no matches) by defaulting to 0
- Wraps broken links in a collapsible details section for better UX
The heading change from bold markdown to plain heading is also appropriate for GitHub Action summaries.
395-395: LGTM: Correct string comparison for GitHub Actions output.The change to string comparison (
== '2') is correct, as GitHub Actions step outputs are strings. This ensures proper comparison of the exit_code value from the lychee-action step.
249-249: No issue found — theupload-sarifaction supports directory paths.The
sarif_fileparameter accepts both individual file paths and directories. When given a directory, the action recursively processes all.sariffiles within it. The change tocodeql-resultsis supported by the action and will not break SARIF uploads.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/build.yml (2)
169-187: Consider clearer column header and verify message truncation.The table column "Security" would be clearer as "Security Severity" or "Sec-Severity" to indicate it shows the security-severity score from rule definitions.
Additionally, truncating messages to 80 characters (
.[0:80]) may cut off important context that would help developers understand the finding. Consider increasing to 120-150 characters or making the entire details section scrollable without per-message truncation.♻️ Proposed improvement
- echo "| Level | Security | Rule | Location | Message |" >> $GITHUB_STEP_SUMMARY - echo "|-------|----------|------|----------|---------|" >> $GITHUB_STEP_SUMMARY + echo "| Level | Sec-Severity | Rule | Location | Message |" >> $GITHUB_STEP_SUMMARY + echo "|-------|--------------|------|----------|---------|" >> $GITHUB_STEP_SUMMARY # Join results with rules to get security-severity (which is on rule definitions, not results) jq -r ' (.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[] | - "| \(.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:80]) |" + "| \(.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:150]) |" ' "$sarif" >> $GITHUB_STEP_SUMMARY
231-242: Table structure inconsistency with CodeQL findings.The Trivy details table has 4 columns (Severity, Rule, Location, Message), while the CodeQL table has 5 columns (Level, Security, Rule, Location, Message). This inconsistency may confuse users reviewing both summaries.
Consider whether Trivy findings should include a similar security-severity column for consistency, or if the difference is intentional due to how Trivy reports severity differently.
As per learnings, severity-grouped findings are the standard approach for Trivy, which you've already implemented in the summary counts. The details table structure difference may be acceptable if Trivy doesn't provide security-severity scores in the same way CodeQL does.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yml
🧰 Additional context used
🧠 Learnings (3)
📚 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: 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
⏰ 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 (6)
.github/workflows/build.yml (6)
354-354: LGTM: Link remap correctly handles PR and push contexts.The use of
github.base_ref || github.ref_nameappropriately handles both pull request contexts (wherebase_refis the target branch) and push contexts (whereref_nameis the current branch). This ensures the remap URL construction works correctly across different workflow trigger types.
365-383: LGTM: Improved error detection and formatting.The updated link checker summary correctly:
- Uses extended regex (
-E) to match ERROR and HTTP 4xx/5xx patterns- Handles grep exit codes gracefully with
|| ERRORS=0pattern- Wraps broken links in a collapsible details section for better readability
The implementation is clean and well-structured.
402-402: LGTM: Correct string comparison for exit code.The string comparison
exit_code == '2'is the correct approach in GitHub Actions, as step outputs are always strings. This properly distinguishes broken links (exit code 2) from timeouts (exit code 1) when deciding whether to create an issue.
410-420: LGTM: Robust error detection by inspecting report content.The new approach of directly inspecting
html-link-report.mdfor broken links is more robust than relying solely on the lychee-action step's exit code. This provides consistent error detection logic between the summary and failure steps, and gives clearer error messages with specific error counts.
264-307: Security-severity threshold is correct.The implementation correctly uses
security-severity >= 7.0to identify high and critical CodeQL findings. CodeQL officially maps severity scores as: High (7.0–8.9) and Critical (9.0–10.0), so this threshold properly captures both severity levels. The logic for failing the build after SARIF uploads complete is sound and ensures findings appear in the Security tab even on failure.
256-256: Directory paths are supported by the upload-sarif action. Thegithub/codeql-action/upload-sarifaction accepts a directory path and will recursively find and upload all.sariffiles within it. Ensure each uploaded SARIF file has a uniquecategory(or uniquerunAutomationDetails.id) to avoid upload failures, as GitHub rejects uploads combining multiple runs with the same tool+category combination.
|
@coderabbitai Addressed nitpick in commit 83b21c6:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 4945 @david-waltermire Thank you for adding the security-severity column! I can see in commit 83b21c6 that you've:
This provides excellent visibility into which findings will trigger build failures based on the security-severity threshold (>= 7.0). The implementation correctly handles the fact that security-severity is stored in rule definitions rather than on individual results. 🧠 Learnings used |
|
@coderabbitai Addressed nitpicks in commit bf3310b:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 3618 @david-waltermire Perfect! I can see in commit bf3310b that you've:
These improvements enhance the readability of the CodeQL findings table, making it easier to quickly scan severity scores while still providing sufficient detail in the message column. Great refinements! 🎯 🧠 Learnings used |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/build.yml (2)
231-242: Consider consistent message truncation across scanners.The Trivy findings table truncates messages at 80 characters (line 239), while CodeQL uses 120 characters (line 183). For consistency and a uniform user experience, consider using the same truncation length for both scanners unless there's a specific reason for the difference.
♻️ Align Trivy message truncation with CodeQL
- 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
264-307: Well-structured failure gate with one documentation suggestion.The implementation correctly:
- Runs after SARIF uploads to ensure results are visible before failure
- Uses the same jq join pattern as the summary to extract
security-severityfrom rule definitions- Checks both
level == "error"andsecurity-severity >= 7.0for CodeQL- Properly maps Trivy severity levels (error=CRITICAL, warning=HIGH)
- Accumulates failures before exiting
Consider adding an inline comment documenting that the 7.0 threshold represents GitHub's high/critical security-severity boundary for clarity.
📝 Add documentation for security-severity threshold
# Check for error level OR security-severity >= 7.0 (high/critical) + # Note: GitHub security-severity scale: 0.0-3.9=Low, 4.0-6.9=Medium, 7.0-8.9=High, 9.0-10.0=Critical # Note: security-severity is on rule definitions, not results, so we join via ruleId
📜 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/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: 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
⏰ 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: Code
- GitHub Check: Website
🔇 Additional comments (5)
.github/workflows/build.yml (5)
169-187: LGTM! Well-implemented findings details.The collapsible details section provides valuable visibility into individual CodeQL findings. The jq query correctly joins results with rule definitions to extract
security-severityfrom the proper location in the SARIF structure, handles missing values gracefully, and applies appropriate markdown escaping.
256-256: Verify directory-based SARIF upload works as expected.The
sarif_fileparameter now points to thecodeql-resultsdirectory instead of a specific file. While theupload-sarifaction should support directory uploads and process all.sariffiles within it, please confirm this works correctly in your testing, especially if you plan to support multiple languages in the future.
354-354: LGTM! Context-aware URL remapping.The use of
github.base_ref || github.ref_namecorrectly adapts the remap URL for different contexts: using the target branch for PRs and the current branch for push events. This ensures link checking validates against the appropriate tree reference.
365-383: LGTM! Improved error detection and reporting.The updated regex pattern correctly matches ERROR, 4xx, and 5xx status codes, and the defensive handling of
grep -cexit codes (with|| ERRORS=0fallback) prevents script failures when no matches are found. The collapsible details block improves summary readability.
402-420: LGTM! Robust link checker failure handling.The dual approach of checking both exit code (line 402) and report content (lines 411-420) provides robust failure detection. Both checks correctly distinguish between actual broken links (ERROR/4xx/5xx) and timeouts, respecting the workflow inputs for issue creation and failure behavior. The content-based validation in the new step is particularly defensive.
david-waltermire
force-pushed
the
feature/sarif-summary-details
branch
from
83d79d8 to
25a0e16
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
411-420: Consider consistency with issue creation condition.Line 402 uses
steps.linkchecker.outputs.exit_code == '2'while this step inspects the report content with grep. Both approaches should produce the same result, but the inconsistency could cause divergence if lychee's exit code semantics change.Consider using the same detection method (either exit_code or report inspection) in both locations for maintainability.
Option: Align with exit_code approach
- name: Fail on link check error - # Check report for actual broken links (ERROR, 4xx, 5xx), not timeouts - if: ${{ !inputs.skip_linkcheck && !cancelled() && env.INPUT_FAIL_ON_ERROR == 'true' }} - run: | - if [ -f "html-link-report.md" ]; then - ERRORS=$(grep -cE "^\[ERROR\]|^\[[45][0-9]{2}\]" html-link-report.md 2>/dev/null) || ERRORS=0 - if [ "$ERRORS" -gt 0 ]; then - echo "::error::Link checker found $ERRORS broken link(s). See report for details." - exit 1 - fi - fi + if: ${{ !inputs.skip_linkcheck && !cancelled() && steps.linkchecker.outputs.exit_code == '2' && env.INPUT_FAIL_ON_ERROR == 'true' }} + run: | + echo "::error::Link checker found broken links. See report for details." + exit 1
📜 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/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: 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/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-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: 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
⏰ 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: Code
- GitHub Check: Website
🔇 Additional comments (7)
.github/workflows/build.yml (7)
169-187: LGTM! Well-implemented details section with security-severity join.The jq query correctly:
- Builds a lookup map from both driver and extension rules
- Joins results with rule definitions via
ruleId- Handles missing security-severity gracefully with "N/A"
- Escapes pipes and newlines in messages
- Truncates messages safely to 120 characters
231-242: LGTM! Consistent implementation with CodeQL details.The Trivy details section follows the same pattern as CodeQL with appropriate simplifications (no security-severity join needed since Trivy uses direct level mapping).
264-307: LGTM! Proper fail-fast implementation with correct threshold logic.The step correctly:
- Runs after summaries and SARIF uploads complete
- Uses security-severity ≥ 7.0 threshold (aligns with CVSS HIGH/CRITICAL boundary)
- Joins CodeQL results with rule definitions for security-severity lookup
- Handles missing SARIF files gracefully with fallback to "0"
- Accumulates failures across all files before exiting
- Provides clear error messages per scanner and severity
The tonumber conversion and null-coalescing pattern safely handles missing or non-numeric security-severity values.
354-354: LGTM! Dynamic remap improves link checking accuracy.The interpolation
${{ github.base_ref || github.ref_name }}correctly:
- Uses the PR target branch when available (pull_request events)
- Falls back to the current branch for push events
- Ensures links are validated against the correct tree reference
365-392: LGTM! Improved error detection and summary formatting.The changes correctly:
- Use robust regex patterns for error detection (
^\[ERROR\]|^\[[45][0-9]{2}\])- Separate timeouts from actual broken links
- Handle grep's exit code 1 (no matches) with
|| ERRORS=0- Wrap broken link details in collapsible sections for better readability
402-402: LGTM! Correct exit code handling for issue creation.The condition properly distinguishes actual broken links (exit code 2) from timeouts (exit code 1), ensuring issues are only created for actionable problems.
256-256: No action needed — thegithub/codeql-action/upload-sarifaction fully supports directory paths for thesarif_fileparameter. It will recursively process all.sariffiles in the directory, which is the correct approach for multi-language CodeQL analysis.
david-waltermire
force-pushed
the
feature/sarif-summary-details
branch
from
25a0e16 to
acf13e3
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 defining EFFECTIVE_REF uses the non-existent
GitHub Actions function length(inputs.ref) which will fail at runtime; remove
the length check and either 1) simplify the expression to use only inputs.ref
with !contains(inputs.ref, '/') and !startsWith(inputs.ref, 'v') (i.e., set
EFFECTIVE_REF to the existing inline expression without length), or 2) move the
logic into a separate step that computes the effective ref (e.g., a shell step
that inspects inputs.ref and falls back to github.base_ref or github.ref_name)
and then reference that step's output where EFFECTIVE_REF is used.
- Around line 266-309: Update the incorrect Trivy SARIF mapping comment (the
comment above the Trivy check that currently misstates level mappings) so it
matches the actual logic used for TRIVY_CRITICAL_HIGH and TRIVY_MEDIUM: state
that error = CRITICAL/HIGH, warning = MEDIUM, note = LOW; keep the Trivy check
code (variables TRIVY_CRITICAL_HIGH and TRIVY_MEDIUM and their jq selectors)
unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yml
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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: 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/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: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-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 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.
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
🪛 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 (5)
.github/workflows/build.yml (5)
171-189: LGTM: Clean implementation of findings details table.The collapsible details section properly joins security-severity from rule definitions (driver + extensions) and presents findings in a clear table format. The jq query correctly:
- Aggregates rules from both driver and extension sources
- Maps rule IDs to their security-severity values
- Handles missing security-severity with "N/A" fallback
- Escapes pipe characters and newlines in messages
- Truncates messages to 120 characters as intended
233-244: LGTM: Trivy findings details properly formatted.The collapsible details table follows the same clean pattern as CodeQL, with appropriate message escaping and 120-character truncation. The implementation correctly leverages Trivy's SARIF structure where severity level is directly on results.
258-258: LGTM: Directory path supports multi-language scans.The
codeql-resultsdirectory path correctly enables uploading multiple SARIF files for multi-language CodeQL scans, where each language produces a separate .sarif file.Based on learnings, this is the standard pattern for CodeQL uploads in metaschema-framework repositories.
367-394: LGTM: Improved Link Checker summary with clear error reporting.The refactored summary section cleanly extracts broken links using grep with proper exit code handling. The collapsible details section for broken links provides a focused view of failures, and the three-way condition (errors/timeouts/success) gives clear status feedback.
414-422: LGTM: Dual failure detection provides robustness.The new grep-based failure detection complements the exit_code check at line 404. Using the same error pattern as the summary section (lines 373, 417) ensures consistency, and checking the report directly provides a reliable backup to exit codes.
The separation of concerns is clear:
- Line 404: Uses exit_code == '2' for issue creation
- Lines 414-422: Uses grep for build failure
- Both correctly exclude timeouts from failure conditions
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 - Upload entire codeql-results directory for multi-language support - Fail build on critical/high severity findings (security-severity >= 7.0) Trivy improvements: - Display findings table with level, rule, location, message - Correct SARIF severity mapping (error=CRITICAL, warning=HIGH, note=MEDIUM/LOW) - 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 General fixes: - Escape pipe characters in messages to prevent markdown table corruption - Use valid HTML structure in summary elements - Use string comparison for exit code checks - Truncate messages at 120 characters for readability
david-waltermire
force-pushed
the
feature/sarif-summary-details
branch
from
acf13e3 to
5c2896c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/build.yml:
- Around line 75-77: The EFFECTIVE_REF heuristic wrongly excludes valid feature
branches because it treats any ref containing '/' as non-branch; change the
predicate so we accept inputs.ref unless it actually looks like a version tag
(e.g., semver starting with "v" and containing dots) — update the condition that
builds EFFECTIVE_REF to prefer inputs.ref when provided and not matching a tag
pattern (use inputs.ref && !(startsWith(inputs.ref, 'v') && contains(inputs.ref,
'.'))) then fall back to github.base_ref or github.ref_name; keep references to
EFFECTIVE_REF, inputs.ref, github.base_ref, and github.ref_name when making the
change.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build.yml
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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: 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/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-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: 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 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.
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
🔇 Additional comments (6)
.github/workflows/build.yml (6)
224-245: LGTM! Consistent implementation of per-finding details.The Trivy summary follows the same collapsible details pattern as CodeQL, providing good consistency. The inline comment documenting Trivy's SARIF level mapping is helpful for maintainability.
Based on learnings, grouping findings by severity is the standard approach for Trivy scanning in this repository.
259-259: Correct directory-based SARIF upload path.The change to use a directory path (
codeql-results) correctly supports multi-language CodeQL scans where each language produces a separate SARIF file. Theupload-sarifaction handles directory paths and uploads all contained.sariffiles.Based on learnings, this is the intentional approach for multi-language CodeQL scanning in metaschema-framework repositories.
357-357: Link checker remap now uses dynamic EFFECTIVE_REF.The remap correctly uses
env.EFFECTIVE_REFto resolve repository links against the appropriate branch reference, rather than hardcodingmain. This ensures link checking works correctly for different branch contexts.Note: This depends on the
EFFECTIVE_REFheuristic working correctly for all branch types (see comment on lines 75-77).
368-423: Improved link checker summary and failure detection.The refactored summary and failure logic provides better clarity:
Summary rendering: The collapsible details section for broken links improves readability when failures occur.
Failure detection: Inspecting the report content (lines 417-422) is more reliable than relying solely on step exit codes, especially given
continue-on-error: trueon the lychee-action step.Error handling: The grep pattern
|| ERRORS=0correctly handles cases where grep finds no matches (which would otherwise exit with code 1).The implementation correctly distinguishes between actual broken links (
[ERROR],[4xx],[5xx]) and timeouts, aligning with the workflow's intent to fail on errors but tolerate timeouts.
172-190: Well-implemented per-finding details with security-severity join.The collapsible details section provides valuable visibility into individual findings. The jq query correctly joins results with rule definitions from both
driver.rulesandextensions.rulesto extract security-severity values, and the fallback handling (// null,// [],// "N/A") ensures robustness when data is missing. Message escaping properly handles newlines and pipe characters, with truncation to 120 characters as intended.
267-310: The security-severity threshold and Trivy handling are correct.The
>= 7.0threshold correctly aligns with CVSS v3.1 standards (High: 7.0–8.9, Critical: 9.0–10.0), ensuring the build fails on both high and critical severity findings. The jq logic properly extracts and compares security-severity values, handling missing values safely.Trivy correctly warns on medium findings without failing the build, following standard practice for severity-based grouping. GitHub Actions executes steps sequentially, so the positioning after upload steps is sufficient; no explicit dependencies are needed.
The implementation is sound.
.github/workflows/build.yml
Outdated
| # Effective ref for link checker remap: prefer inputs.ref if it looks like a branch name, else fall back to base_ref/ref_name | ||
| # Heuristic: use inputs.ref if provided, doesn't contain '/', and doesn't start with 'v' (likely a tag) | ||
| EFFECTIVE_REF: ${{ (inputs.ref && !contains(inputs.ref, '/') && !startsWith(inputs.ref, 'v')) && inputs.ref || github.base_ref || github.ref_name }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Test the EFFECTIVE_REF heuristic with various branch names
test_ref() {
local ref="$1"
local has_slash="false"
local starts_v="false"
if [[ "$ref" == */* ]]; then
has_slash="true"
fi
if [[ "$ref" == v* ]]; then
starts_v="true"
fi
echo "Testing ref: $ref"
echo " Contains '/': $has_slash"
echo " Starts with 'v': $starts_v"
# GitHub Actions condition: inputs.ref && !contains(inputs.ref, '/') && !startsWith(inputs.ref, 'v')
if [[ -n "$ref" && "$has_slash" == "false" && "$starts_v" == "false" ]]; then
echo " ✓ Would use inputs.ref"
else
echo " ✗ Would fall back to base_ref/ref_name"
fi
echo ""
}
# Test cases
test_ref "develop"
test_ref "main"
test_ref "feature/sarif-summary-details"
test_ref "release/v1.0.0"
test_ref "v1.2.3"
test_ref "abc123def"Repository: metaschema-framework/liboscal-java
Length of output: 713
🏁 Script executed:
# Check the workflow file around line 357 to see how EFFECTIVE_REF is used
sed -n '355,365p' .github/workflows/build.ymlRepository: metaschema-framework/liboscal-java
Length of output: 699
🏁 Script executed:
# Also check if there's any documentation or comments about the intent of this heuristic
sed -n '70,80p' .github/workflows/build.ymlRepository: metaschema-framework/liboscal-java
Length of output: 770
🏁 Script executed:
# Find the workflow input definition for 'ref'
grep -A 10 -B 5 "inputs:" .github/workflows/build.yml | head -40Repository: metaschema-framework/liboscal-java
Length of output: 933
🏁 Script executed:
# Check if there's any documentation about when inputs.ref is used
grep -n "inputs.ref" .github/workflows/build.ymlRepository: metaschema-framework/liboscal-java
Length of output: 568
Feature branches (containing /) cannot be used with the EFFECTIVE_REF heuristic.
The condition !contains(inputs.ref, '/') incorrectly excludes valid branch names like feature/sarif-summary-details. When such a branch is explicitly provided via inputs.ref, the heuristic falls back to github.base_ref or github.ref_name instead. This creates inconsistent behavior: the workflow accepts feature branch refs (per the input description) but the link checker ignores them, using the base branch reference instead for remapping URLs.
Consider allowing feature branches by checking if the ref is actually a known tag format (e.g., semver tags like v1.2.3) rather than assuming any ref with / is not a branch name.
🤖 Prompt for AI Agents
In @.github/workflows/build.yml around lines 75 - 77, The EFFECTIVE_REF
heuristic wrongly excludes valid feature branches because it treats any ref
containing '/' as non-branch; change the predicate so we accept inputs.ref
unless it actually looks like a version tag (e.g., semver starting with "v" and
containing dots) — update the condition that builds EFFECTIVE_REF to prefer
inputs.ref when provided and not matching a tag pattern (use inputs.ref &&
!(startsWith(inputs.ref, 'v') && contains(inputs.ref, '.'))) then fall back to
github.base_ref or github.ref_name; keep references to EFFECTIVE_REF,
inputs.ref, github.base_ref, and github.ref_name when making the change.
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
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
223-231: Consider clarifying UNKNOWN severity handling.The comment indicates that UNKNOWN severity maps to "note" level (grouped with LOW), but the table label only shows "Low". While functionally correct, the label could be more explicit.
Optional clarification
- echo "| :yellow_circle: Low | $LOW |" >> $GITHUB_STEP_SUMMARY + echo "| :yellow_circle: Low/Unknown | $LOW |" >> $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 (5)
📓 Common learnings
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: 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/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.
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: 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: 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 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.
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
⏰ 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: Code
- GitHub Check: Website
🔇 Additional comments (8)
.github/workflows/build.yml (8)
75-76: LGTM! Clean fallback logic for effective ref.The fallback chain correctly handles all workflow trigger types (PR, push, manual, called) and provides a sensible default.
171-189: Excellent implementation of detailed CodeQL findings.The jq query correctly joins results with rule definitions to extract security-severity values, handles missing data gracefully with fallbacks, and properly escapes markdown special characters. The collapsible details section provides good UX.
233-244: LGTM! Trivy findings details well-implemented.The collapsible findings table uses consistent severity mapping and message formatting. The jq query correctly handles edge cases with the else clause.
258-258: Correct use of directory path for multi-language support.The directory path allows CodeQL to upload results from multiple languages, each producing a separate .sarif file.
Based on learnings, the upload-sarif action handles directory paths correctly with distinct category values preventing collisions.
356-356: Good use of EFFECTIVE_REF for consistent link remapping.The remap URL now correctly uses the effective ref variable, ensuring link checking works properly across different branches and workflow triggers.
367-394: Well-structured link checker summary with proper error handling.The summary correctly:
- Handles grep exit codes for zero matches
- Uses regex patterns to identify errors vs timeouts
- Provides collapsible details for broken links
- Shows appropriate status icons and messages
404-404: Dual detection methods work correctly but introduce minor fragility.The workflow uses two different methods to detect broken links:
- Line 404: Uses lychee step's
exit_code == '2'output for issue creation- Lines 414-422: Uses grep on report content for build failure
While both should agree in practice (exit code 2 corresponds to ERROR/4xx/5xx in the report), this dual approach could become inconsistent if lychee's behavior or report format changes.
Also applies to: 414-422
266-309: Security findings failure logic is correctly implemented.The step properly:
- Runs after SARIF uploads complete (per PR objectives)
- Fails on CodeQL error level OR security-severity ≥ 7.0 (the official threshold where CodeQL High severity begins, per GitHub documentation)
- Fails on Trivy critical/high findings
- Warns on Trivy medium findings without failing
- Provides clear error messages with counts
david-waltermire
merged commit 75a6c49
into
metaschema-framework:develop
Addressed code example issues (applied from metaschema-java PR #633 feedback)Fixes applied in commit 7109d39: validating-with-constraints.md.vm
reading-writing-data.md.vm
executing-metapath.md.vm
|
1 participant
Summary
Test plan
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.