Add blocking Semgrep SAST gate - #752
Conversation
CodeQL's default setup has no per-PR pass/fail signal, so a SAST violation could merge without ever being blocked. Add a Semgrep job that fails the build on ERROR-severity findings across security-audit, secrets, OWASP Top 10, and JS/TS/React rulesets, satisfying OSPS-VM-06.02's requirement that SAST violations block merge unless declared non-exploitable via inline nosemgrep suppression.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the vulnerability management documentation to describe the integration of Semgrep as a second SAST layer alongside CodeQL, specifically serving as a pre-merge blocking gate. The review feedback suggests formatting a workflow file reference as a root-absolute link for consistency and correcting a multi-line nosemgrep suppression example, as Semgrep requires the directive to be on the line immediately preceding the flagged code to function correctly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 829c90a12a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Run semgrep without --severity/--error so WARNING/INFO findings still reach the Code Scanning tab; enforce the ERROR-only blocking policy in a dedicated step that reads the JSON output instead, per Codex review feedback that filtering the scan itself dropped lower-severity findings from the SARIF upload the docs claimed stayed visible. - Fix the nosemgrep suppression example: Semgrep only recognizes the directive on the flagged line or the line immediately before it, so the two-line comment in the docs wouldn't actually suppress anything.
- server/credential-crypto.ts: pass authTagLength explicitly to createCipheriv/createDecipheriv (gcm-no-tag-length) instead of relying on Node's default, hardening against tag-truncation attacks. - Dockerfile: suppress the two missing-USER findings with nosemgrep and a justification -- the container intentionally starts as root so entrypoint.sh can chown/usermod to the host's PUID/PGID before dropping privileges itself via su-exec, per docs/VULNERABILITY_MANAGEMENT.md §2.3's suppression policy.
|
3 participants
Description
Follow-up to #750, which added the SCA gate (
npm audit+license-checker, blocking on Critical/High) and adocs/VULNERABILITY_MANAGEMENT.mdpolicy — but explicitly left SAST as triage-only (§2.3): CodeQL's default setup has no per-PR pass/fail signal, so a SAST violation could merge without ever being blocked. That leaves OSPS-VM-06.02 (SAST violations must be blocked unless declared/suppressed as non-exploitable) unmet.This PR adds that missing blocking gate:
.github/workflows/sast.yml: asemgrepjob that runssemgrep scanwith thep/security-audit,p/secrets,p/owasp-top-ten,p/javascript,p/typescript, andp/reactrulesets, restricted toERROR-severity findings (--severity ERROR --error). Any match fails the job and blocks merge. Results are also uploaded to the repo's Code Scanning tab (separate SARIF "tool" from CodeQL, so it doesn't conflict with CodeQL's default setup).docs/VULNERABILITY_MANAGEMENT.md§2 to document Semgrep as the new pre-merge SAST gate (§2.3), including the suppression process for declaring a blocking finding non-exploitable (inline// nosemgrep: <rule-id>with a mandatory justification comment, reviewed like any other diff).Once merged, add
semgrepas a required status check alongside the existingsca-scancheck in themainbranch protection rule (Settings → Branches).Type of change
Checklist
docs/VULNERABILITY_MANAGEMENT.md)yaml.safe_loadsemgrepjob runs green on this PR, then enable it as a required status check🤖 Generated with Claude Code
Generated by Claude Code