feat(code-scanner): add the sast subsystem (PRD 0004) - #51
Merged
Conversation
Implements PRD 0004: line-oriented rules for dangerous constructs — dynamic
code execution, shell/SQL interpolation, unsafe HTML, disabled TLS
verification, Math.random for tokens, path traversal, prototype pollution.
The design constraint is honesty about what a regex can know. CodeQL and
Semgrep already run in this repo's CI and do real interprocedural analysis;
this covers what they cannot see — source on a running server that never
passed through the repository. So every finding carries a confidence:
pattern the construct exists. Capped at medium severity, enforced in
severityFor() rather than per-rule so no future rule can opt out
of the cap by accident.
contextual the construct sits alongside apparent untrusted input. The only
level allowed to escalate to high or critical.
Presenting a bare pattern match with the confidence of a proven vulnerability
is the failure mode that discredits a scanner and every accurate finding
standing next to it. There is a test asserting the cap holds for every rule.
Noise control is the rest of the work: literal `exec('ls')` is not a finding
while `exec(\`${x}\`)` is; Math.random is only flagged when assigned to
something security-shaped; comment lines are skipped, without which the
subsystem reports findings about its own rule descriptions; rules describing
ordinary operations (reading a computed path) require context before firing at
all.
Suppressions are inline, per-rule, carry a reason, and are counted and
reported — a quiet scan full of suppressions is not a clean one.
86 tests (20 new).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
vu1nz Security Review0 finding(s) in PR #? No security issues found. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| let text: string; | ||
| try { | ||
| text = await readFile(file, 'utf8'); |
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
2 participants
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.
Implements PRD 0004 — the
sast/subsystem ofcode-scanner.The honest framing
Real static analysis — proving a path from an untrusted source to a dangerous sink — needs a parser, a CFG and interprocedural taint tracking per language. CodeQL and Semgrep are years of work each, and both already run in this repo's CI.
So this is not a SAST engine, and does not claim to be one. It is line-oriented pattern matching, and its value is coverage: source on a running server that never passed through the repository — vendored trees, generated files, a hotfix applied in place.
Confidence is part of the finding
patterncontextualThe cap is enforced in
severityFor()rather than per-rule, so no future rule can opt out of it by accident, and a test asserts it holds for every rule and every finding a real scan produces.Presenting a regex match with the confidence of a proven vulnerability is what discredits a scanner — and, worse, discredits the accurate findings standing next to it.
Rules
Code execution (
eval,new Function,vm), shell/SQL interpolation, unsafe HTML (innerHTML,dangerouslySetInnerHTML), disabled TLS verification, weak hashes on credentials,Math.random()for tokens, path traversal, unsafe YAML load, prototype pollution. Each carries a CWE and a consequence — an operator triages on what happens if it's real, not on a rule name.Noise control is most of the work
exec('ls')with a literal is not a finding;exec(\${x}`)` is.Math.random()is only flagged when assigned to something security-shaped —const jitter = Math.random() * 100is silent.vendor/still executes.Suppressions can't be used to go quiet
// threatcrush-disable-next-line <rule> <reason>is per-rule, requires a reason, and every suppression is counted and reported. A scan reporting 0 findings and 200 suppressions must not read as clean.Verification
86 tests (20 new), build clean.