Adjust severity - #19
Conversation
…fix junk when gen sarif
📝 WalkthroughWalkthroughThis PR refactors SARIF enrichment to use an effective-severity model that reclassifies indirect dependencies as notes instead of warnings, extends workflow SARIF output validation via ChangesSARIF Enrichment and Output Pipeline
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/submit_deps.yml:
- Around line 119-120: The current jq check (the line invoking "jq type
../cabal-audit.sarif") only validates JSON and will not fail for non‑SARIF
shapes; replace that invocation with a jq -e structural predicate that asserts
the top-level value is an object and that it contains a "runs" key whose value
is an array so the workflow fails on invalid SARIF shape.
In `@src/Hgs/Sarif/Enrich.hs`:
- Around line 360-363: The current serialization uses
problemSeverity/explanation (which maps to "recommendation") for the "effective
severity" text and cabal-plan-submit.effective-severity property; change those
to serialize the actual EffectiveSeverity value instead: keep problem.severity
as recommendation for GitHub ranking, but where the code builds the effective
severity string (the expressions around problemSeverity, explanation, and
cvssText) and where the cabal-plan-submit.effective-severity property is set,
replace the use of problemSeverity/recommendation with a serializer/function
that renders the EffectiveSeverity enum/value (e.g. use the existing
EffectiveSeverity -> Text renderer) so the message body and property reflect the
real EffectiveSeverity; apply the same change in the other occurrences
referenced (the analogous blocks around lines 380-383 and 415-416).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 963b834b-2ebe-4298-8fcf-05e2051f9407
📒 Files selected for processing (2)
.github/workflows/submit_deps.ymlsrc/Hgs/Sarif/Enrich.hs
| "$CABAL_AUDIT" --sarif --to-file ../cabal-audit.sarif | ||
| jq type ../cabal-audit.sarif |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -u
tmp="$(mktemp)"
printf '[]\n' > "$tmp"
jq type "$tmp" >/dev/null
echo "jq type exit code: $?" # expected: 0
set +e
jq -e 'type == "object" and (.runs | type == "array")' "$tmp" >/dev/null
echo "strict predicate exit code: $?" # expected: non-zeroRepository: dancewithheart/cabal-plan-submit
Length of output: 2015
🏁 Script executed:
#!/bin/bash
set -u
tmp="$(mktemp)"
printf '[]\n' > "$tmp"
jq type "$tmp" >/dev/null
echo "jq type exit code: $?" # expected: 0
set +e
jq -e 'type == "object" and (.runs | type == "array")' "$tmp" >/dev/null
echo "strict predicate exit code: $?" # expected: non-zeroRepository: dancewithheart/cabal-plan-submit
Length of output: 1986
🌐 Web query:
enrich-sarif expects top-level runs array SARIF
💡 Result:
The requirement that a SARIF file must contain a top-level runs array is a fundamental aspect of the SARIF (Static Analysis Results Interchange Format) specification [1][2]. Because enrich-sarif and similar utility tools operate on the SARIF object model, they expect valid SARIF files that conform to this schema [3][4]. In the SARIF object model: - A SARIF log is a JSON file whose top-level object is the sarifLog object [2]. - The sarifLog object must contain a property named runs, which is an array of one or more run objects [1][2]. - Each run object represents a single invocation of a single analysis tool [1][5]. Tools like enrich-sarif often parse this structure to iterate through the runs array to add data, augment findings, or enhance metadata [4][6]. If your tool or process is failing with an error indicating it expects a top-level runs array, it is likely because the input file is missing this structure, is not formatted as valid SARIF, or has been incorrectly transformed (e.g., flattened or stripped of the outer container) [2][4]. To resolve this: 1. Verify that your input file follows the standard SARIF JSON structure, which starts with an object containing at least the version and the runs array [1][7]. 2. If you are generating this file programmatically, ensure the root object includes the "runs": [...] key [2]. 3. Check that you are not accidentally passing a single run object (the content inside the array) directly to the tool instead of the full SARIF file [3].
Citations:
- 1: https://github.com/microsoft/sarif-tutorials/blob/e9d5168c43cd180141708656d4fd994a23007fba/docs/2-Basics.md
- 2: https://github.com/microsoft/sarif-tutorials/blob/8b56ef54d4473b91b229a97942f7e3814c84352d/docs/2-Basics.md
- 3: https://github.com/microsoft/sarif-tools/blob/390d6084/sarif/sarif_file.py
- 4: https://github.com/secdim/sandbox-sarif-parser
- 5: https://github.com/microsoft/sarif-tutorials/blob/52e1d553c03cd8294940a09c7368800b8515e229/docs/3-Beyond-basics.md
- 6: https://github.com/microsoft/sarif-tools/blob/390d6084/README.md
- 7: Enrich messages of SARIF results astral-sh/ruff#13179
🏁 Script executed:
#!/bin/bash
set -u
tmp="$(mktemp)"
printf '[]\n' > "$tmp"
jq type "$tmp" >/dev/null
echo "jq type exit code: $?" # expected: 0
set +e
jq -e 'type == "object" and (.runs | type == "array" and length > 0)' "$tmp" >/dev/null
echo "strict predicate exit code: $?" # expected: non-zeroRepository: dancewithheart/cabal-plan-submit
Length of output: 1986
Make the jq check fail fast on non-SARIF (lines 119-120)
jq type ../cabal-audit.sarif succeeds for any valid JSON, so it won’t catch SARIF shape issues (top-level object with a runs array). Switch to a jq -e structural predicate so the workflow fails at validation time.
Suggested fix
- jq type ../cabal-audit.sarif
+ jq -e 'type == "object" and (.runs | type == "array")' ../cabal-audit.sarif > /dev/null📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "$CABAL_AUDIT" --sarif --to-file ../cabal-audit.sarif | |
| jq type ../cabal-audit.sarif | |
| "$CABAL_AUDIT" --sarif --to-file ../cabal-audit.sarif | |
| jq -e 'type == "object" and (.runs | type == "array")' ../cabal-audit.sarif > /dev/null |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/submit_deps.yml around lines 119 - 120, The current jq
check (the line invoking "jq type ../cabal-audit.sarif") only validates JSON and
will not fail for non‑SARIF shapes; replace that invocation with a jq -e
structural predicate that asserts the top-level value is an object and that it
contains a "runs" key whose value is an array so the workflow fails on invalid
SARIF shape.
| <> " effective severity: " | ||
| <> problemSeverity explanation | ||
| <> "\n" | ||
| <> foldMap (\cvss -> " advisory CVSS: " <> cvss <> "\n") cvssText |
There was a problem hiding this comment.
Serialize effective-severity with one vocabulary.
Indirect findings now emit level = note and effective-severity-note, but the message body and cabal-plan-submit.effective-severity property serialize the same concept as recommendation. That makes the new effective-severity field disagree with the SARIF level/tags this PR just introduced.
Keep problem.severity as recommendation if you want GitHub’s ranking text, but render effective severity from the actual EffectiveSeverity value.
Suggested fix
+renderEffectiveSeverity :: EffectiveSeverity -> Text
+renderEffectiveSeverity = sarifLevel
+
appendExplanationText :: Maybe Text -> Text -> FindingExplanation -> Text
appendExplanationText cvssText oldText explanation =
Text.stripEnd oldText
@@
<> explainedRelationship explanation
<> "\n"
<> " effective severity: "
- <> problemSeverity explanation
+ <> renderEffectiveSeverity (effectiveSeverity explanation)
<> "\n"
@@
<> explainedRelationship explanation
<> "`\n"
<> "* effective severity: `"
- <> problemSeverity explanation
+ <> renderEffectiveSeverity (effectiveSeverity explanation)
<> "`\n"
@@
, ("precision", String "medium")
, ("problem.severity", String (problemSeverity explanation))
- , ("cabal-plan-submit.effective-severity", String (problemSeverity explanation))
+ , ( "cabal-plan-submit.effective-severity"
+ , String (renderEffectiveSeverity (effectiveSeverity explanation))
+ )
])Also applies to: 380-383, 415-416
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Hgs/Sarif/Enrich.hs` around lines 360 - 363, The current serialization
uses problemSeverity/explanation (which maps to "recommendation") for the
"effective severity" text and cabal-plan-submit.effective-severity property;
change those to serialize the actual EffectiveSeverity value instead: keep
problem.severity as recommendation for GitHub ranking, but where the code builds
the effective severity string (the expressions around problemSeverity,
explanation, and cvssText) and where the cabal-plan-submit.effective-severity
property is set, replace the use of problemSeverity/recommendation with a
serializer/function that renders the EffectiveSeverity enum/value (e.g. use the
existing EffectiveSeverity -> Text renderer) so the message body and property
reflect the real EffectiveSeverity; apply the same change in the other
occurrences referenced (the analogous blocks around lines 380-383 and 415-416).
1 participant
tool.driver.rulesand CVSS score MangoIV/cabal-audit#75--to-file