chore(#511): relax release-gated semgrep to fail-on-ERROR#521
Conversation
- Set SEMGREP_FAIL_SEVERITY default ERROR (was WARNING) in both the framework's .github/workflows/security-scan.yml and the adopter golden-path template golden-paths/pipelines/security.yml - WARNING (medium) findings stay advisory in the step summary; only ERROR fails - Stops a clean release reding on pre-existing low-severity bash lint (the 4 WARNING findings that red the v2.3.0 release scan); they remain trackable debt - Decision recorded in AgDR-0063-semgrep-severity-policy Closes #511 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
atlas-apex
left a comment
There was a problem hiding this comment.
Code Review: PR #521
Commit: 76196a91dcfa1963df484cb1c8f880bd926030f5
Summary
Flips the default SEMGREP_FAIL_SEVERITY from WARNING to ERROR in the framework's release-gated scan (.github/workflows/security-scan.yml) and the adopter golden-path template (golden-paths/pipelines/security.yml), documents the policy inline, and records the decision in AgDR-0063-semgrep-severity-policy.md. WARNING (medium) findings stay advisory in the step summary; ERROR (high) still hard-fails. Resolves the v2.3.0 release going red on 0 ERROR + 4 pre-existing WARNING findings (#511).
Checklist Results
- ✅ Architecture & Design: N/A (CI config + AgDR)
- ✅ Code Quality: Pass
- ✅ Testing: Pass
- ✅ Security: Pass
- ✅ Performance: N/A
- ✅ PR Description & Glossary: Pass
- ✅ Summary Bullet Narrative: Pass
- ✅ Technical Decisions (AgDR):Pass (AgDR-0063 ships in this PR)
- ✅ Adopter Handbooks: N/A (no handbook triggers in diff)
Issues Found
None blocking.
Threshold-change consistency (the key verification): Confirmed the existing two-branch fail-logic handles ERROR-only mode correctly in both files:
security-scan.yml:161-167—if FAIL_SEV=WARNINGfails onWARNING>0 OR ERROR>0;elif FAIL_SEV=ERRORfails only onERROR>0.security.yml:128-132— same structure: WARNING mode fails onWARNING+ERROR>0; ERROR mode fails only onERROR>0.
With 0 ERROR + 4 WARNING, the ERROR branch does not fire → job passes. The flip is consistent with the pre-existing logic; no logic change was needed, only the env default. Both the framework input override (fail_on_severity) and the template env override remain functional escape hatches for stricter gating.
Security posture: This relaxes the gate for medium severity only; high-severity (ERROR) findings still block. WARNING is surfaced, not silenced. The AgDR documents the accepted tradeoff. Sound default for a freshly-added release gate — agree with option (a) over (b).
Suggestions
- nit: the PR body uses "reding"/"red" as a verb in a couple of bullets — stylistic only, no change required.
- The 4 pre-existing
r/bashWARNINGs are correctly tracked as non-blocking debt for a follow-up; consider filing a tracking ticket so they don't drift.
Verdict
APPROVED
Pre-existing shellcheck/style warnings in untouched embedded script blocks were treated as out of scope for this PR, per the change's intent. The post-merge release-tag-green validation is the right call given a release can't be triggered from a PR.
🤖 Reviewed by Rex (Code Reviewer Agent)
📌 Reviewed commit: 76196a91dcfa1963df484cb1c8f880bd926030f5
Summary
r/bash, added in [Feature] Release-gated security scan on the framework repo itself (dog-food the Shield pipeline) #487) failed its first real run on the v2.3.0 tag with0 ERROR + 4 WARNING, becauseSEMGREP_FAIL_SEVERITYwasWARNING. It runs post-merge on the tag so it didn't block v2.3.0, but it would red every future release until the 4 findings are cleaned up.ERRORin both places — the framework's own.github/workflows/security-scan.ymland the adopter golden-path templategolden-paths/pipelines/security.yml. The existing fail logic already supported both modes, so this is purely a threshold change.SEMGREP_FAIL_SEVERITY=WARNING(template) or passfail_on_severity: WARNING(framework workflow input).AgDR-0063-semgrep-severity-policy.Testing
python3 yaml.safe_loadon both workflow files → parse clean.SEMGREP_FAIL_SEVERITYtoERRORand the existingFAIL_SEVbranch logic handles ERROR-only correctly (lines ~163 / ~127).markdownlintclean on the new AgDR.The 4 pre-existing
r/bashWARNING findings remain as ordinary, non-release-blocking debt and can be cleaned up in a follow-up.Closes #511
Glossary
SEMGREP_FAIL_SEVERITY