feat(security): custom semgrep rules for silent-success masking (AI004)#311
Open
krokoko wants to merge 3 commits into
Open
feat(security): custom semgrep rules for silent-success masking (AI004)#311krokoko wants to merge 3 commits into
krokoko wants to merge 3 commits into
Conversation
Bare-except and empty-catch are gated, but the more dangerous AI004
variant — swallowing a failure and returning a plausible-but-empty
default ([], {}, null/None, "") — had no detector. Such fallbacks hide
failures from callers and let plausible-but-wrong code through green.
Add custom semgrep rules for TS (catch { return []|null|{}|undefined })
and Python (except: return None|[]|{}|""), with semgrep-test fixtures
covering rethrow/re-raise, finally, multi-except, and return-in-try
cases. Findings anchor on the offending return line and carry
agent-targeted remediation text.
The new `security:sast:masking` task validates the rules, scans the
repo, and writes SARIF to test-reports/ so findings stay agent-routable
(pairs with CA-06). It runs inside `mise run security` but is ADVISORY
(no --error) until the 40-finding baseline in .semgrep/BASELINE.md is
triaged; intentional degraded-mode fallbacks use inline justified
nosemgrep comments (existing repo convention).
Closes #257
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.
Fixes #257
What this does, in plain English
When code hits an error but quietly returns an empty result instead of reporting the failure (for example
catch { return []; }), whoever called it — a person, a test, or an AI agent — sees a "successful" empty answer and has no way to know something actually went wrong. Tests stay green, agents conclude their code works, and real failures go unnoticed. This PR adds an automatic detector for that pattern across the repo's TypeScript and Python code, runs it as part of the existing security checks, and reports each occurrence with a clear explanation of how to fix it. It starts in warn-only mode so the 40 pre-existing occurrences can be reviewed one by one before the check is made strict.Summary
Adds the AI004 detector requested in #257: custom semgrep rules that catch silent-success masking — a
catch/exceptthat swallows a failure and returns a plausible-but-empty default ([],{},null/undefined/None,""), which existing bare-except/empty-catch gates do not detect..semgrep/silent-success-masking.yaml—ts-silent-success-maskingandpy-silent-success-masking, with agent-targeted remediation messages (re-throw / re-raise / encode failure in the result shape; logging alone is not enough). Findings anchor on the offendingreturnline. Coversfinallyvariants, multi-except, and conditional-rethrow fallthrough; returns in thetrybody are not flagged..semgrep/silent-success-masking.{ts,py}—semgrep testfixtures (ruleid:/ok:annotations), run automatically as part of the task.mise run security:sast:masking— validates rules against fixtures, scans the repo, and emits SARIF totest-reports/(gitignored) so findings are agent-routable (pairs with CA-06 / feat(ci): structured CI output — pytest JUnit XML + SARIF from eslint/ruff/semgrep (CA-06) #256). Wired intomise run security.--errorflag, so the task never fails while the baseline is open..semgrep/BASELINE.mddocuments the 40 existing findings (15 Py, 25 TS) and the flip-to-blocking procedure.nosemgrep: <rule-id> -- <reason>on the return line (existing repo convention; precedent inscripts/check-types-sync.ts,agent/src/config.py).Acceptance criteria (from #257)
catch { return []|null|{} }(TS) and equivalent default-substitution-on-failure (Py)nosemgrep)test-reports/semgrep-silent-success-masking.sarif).semgrep/BASELINE.md); advisory first, blocking once cleanTest plan
semgrep test .semgrep/— 2/2 rule unit tests pass (fixtures cover rethrow/re-raise, typed errors, meaningful fallbacks,finally, multi-except, conditional rethrow, return-in-try-body)mise run security:sast:masking— exits 0 (advisory), emits valid SARIF 2.1.0 with both rulessecurity:sastconfig set passes on the new.semgrep/fixtures (no cross-rule noise)mise run build— full monorepo build green (1933 CDK tests, agent quality, cli, docs)