fix(ci): rampart-gate.js severity precedence — metadata.rampart must win over raw semgrep severity - #25
Merged
Conversation
… raw semgrep severity Custom rules under rules/rampart/*.yml are all authored with `severity: ERROR` at the semgrep level (deliberate convention so semgrep never filters them out), carrying the real business severity separately in `metadata.rampart` (e.g. "HIGH", "MEDIUM"). The old precedence in the semgrep branch checked `rampartMeta === 'CRITICAL' || rawSev === 'ERROR'` first, so the `||` let the raw semgrep severity win before metadata.rampart was ever consulted — every custom-rule finding (all severity:ERROR by convention) got force-promoted to CRITICAL regardless of its documented rampart severity. Since CRITICAL always blocks (even in warn/ENFORCE=warn mode), this could hard-fail the Rampart PR Gate on any PR tripping a HIGH/MEDIUM custom rule. Fix: metadata.rampart now wins whenever present; raw semgrep severity is only used as a fallback for third-party ruleset findings that carry no rampart metadata. Verified against a real semgrep-report.json artifact (CI run 28683658719): the no-localstorage-token finding (rampartMeta: HIGH) moved from critical to high, dropping critical 1->0 and flipping the gate result under warn mode. Same bug + fix as ummeco/praycalc#58.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
🟢 Rampart Security Gate — CLEAN
Totals: 0 critical · 0 high · 0 medium · 0 low Mode: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
.github/scripts/rampart-gate.js'ssemgrepbranch had a precedence bug:All custom rules under
rules/rampart/*.ymlare deliberately authored withseverity: ERRORat the semgrep level (so semgrep itself never filters them out), carrying the real business severity separately inmetadata.rampart(e.g."HIGH","MEDIUM"). Because the||checksrawSev === 'ERROR'in the same condition asrampartMeta === 'CRITICAL', the raw semgrep severity wins beforemetadata.rampartis ever consulted. Every custom-rule finding — allseverity: ERRORby convention — gets force-promoted toCRITICALregardless of what it's actually documented as.Since CRITICAL always blocks (even in
RAMPART_ENFORCE=warnmode), this can hard-fail the Rampart PR Gate on any PR that trips a custom rule documented as HIGH or MEDIUM, when it should only warn (or not block at all in warn mode).Fix
metadata.rampartnow wins whenever present. Raw semgrep severity is only used as a fallback for third-party ruleset findings (p/typescript,p/react,p/owasp-top-ten,p/secrets) that carry no rampart metadata. Change is scoped entirely to theif (tool === 'semgrep')block — nothing else in the file touched.Evidence — real CI artifact (run 28683658719,
semgrep-reportartifact)That report contains a real finding:
rules.rampart.no-localstorage-tokenonweb/src/islands/SettingsIsland.tsx, with raw semgrepseverity: ERRORandmetadata.rampart: "HIGH".Ran
node .github/scripts/rampart-gate.js semgrep-report.json --tool semgrep(same invocation as.github/workflows/rampart-pr.yml'ssastjob) before and after the fix, against that same downloaded artifact:The
no-localstorage-tokenfinding correctly moves from the critical bucket to the high bucket, critical count drops 1→0, and the gate flips from blocking to passing underRAMPART_ENFORCE=warn. (Undererrormode it would still correctly block on the HIGH count — only the misclassification is fixed, not the finding's visibility.) No synthetic data was needed; this repo's real report already contained a triggering finding.Same bug + fix as ummeco/praycalc#58.