Add project-level security scan rules#744
Conversation
commit: |
|
No React Doctor issues found. 🎉 Reviewed by React Doctor for commit |
51e8816 to
cdfe430
Compare
cdfe430 to
b1fe41d
Compare
b1fe41d to
d1f18fc
Compare
d1f18fc to
d9adf7e
Compare
d9adf7e to
4a1840e
Compare
4a1840e to
b18753e
Compare
b18753e to
a940685
Compare
|
/rde parity |
|
❌ Parity failed — trace |
…rule posture modules One rule file = one rule, like every other bucket: 36 definePostureRule modules under rules/security-posture/ replace the 1433-line scanner monolith and the 364-line no-op stub file. The registry generator's multi-export hack is reverted to main's single-match contract with definePostureRule added to the alternation. checkSecurityPosture is now a thin walk-and-dispatch over registry entries carrying `scan`.
Project-level posture rules now execute through core's environment check machinery — same walker, same dispatch, same diagnostics — selected via the shared shouldEnableRule capability/tag gate instead of a hand-rolled filter. The plugin dispatcher remains only as a parity reference until the next phase removes it.
The scan now joins reduced-motion, pnpm hardening, and expo/RN checks in run-inspect's environment phase: skipped in diff mode, streamed through the per-element pipeline (severity controls, inline disables, surfaces), and gated per rule by shouldEnableRule. services/linter.ts returns to main unchanged; posture rules are excluded from generated oxlint configs and ESLint presets instead of shipping as no-ops.
… surface The scanner file, the /ast subpath export, and the plugin-side copies of core fs utils are gone; the posture rules and their bucket utils are the single home for scan logic, and core owns the walk.
Adds ignoredTags + registry-metadata single-sourcing tests and a package-metadata-secret fixture to the core suite, a runPostureRule harness with colocated regressions tests for the AST and dynamic-severity rules, registry invariants (exactly 36 tagged posture rules, scan field nowhere else), and posture-rule authoring docs. The changeset now describes the environment-check architecture and its intentional behavior changes.
Mutation-verified gaps from review: the ESLint-preset posture filter in rules.ts could be reverted without any test failing (same bug class as the defaultEnabled preset leak), and the runInspect dispatch line could be deleted silently. Adds a registry-derived preset regression and three runInspect integration tests covering full-scan emission, the diff-mode gate, and user severity overrides restamping posture diagnostics.
CodeQL js/polynomial-redos: the unanchored (\d+)\.(\d+) scan over the
package.json version string backtracks quadratically on long digit
runs. Bounded {1,4} quantifiers keep it linear, matching the fix
parse-react-major-minor.ts already carries.
Replaces the hand-rolled digit regex (the CodeQL polynomial-redos surface) with semver, which core already depends on: minVersion gives the range's lower bound (>=3.4 <5 → 3.4) and coerce covers prefixed specs like npm:tailwindcss@^3.4.1.
'Posture' meant nothing to anyone. Project-level rules are now plain 'scan rules': defineScanRule modules in rules/security-scan/, tagged security-scan, typed ScanFinding/FileScan/ScannedFile, executed by core's checkSecurityScan environment check. Same light shape as before — a scan field on Rule and a distinctly-named wrapper — no parallel type system.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c3f0d7c. Configure here.
| const finding: ScanFinding = { | ||
| message: "A browser-reachable debug, log, dump, report, or env artifact is present.", | ||
| line: location.line, | ||
| column: location.column, |
There was a problem hiding this comment.
Public debug scan skips inline disables
Medium Severity
public-debug-artifact findings are emitted with line and column of 0 because location comes from getMatchLocation with no pattern. The diagnostic pipeline only evaluates inline react-doctor-disable comments when line is greater than 0, so suppressions on the affected file never apply to this rule despite the PR’s promise that scan diagnostics honor inline disables.
Reviewed by Cursor Bugbot for commit c3f0d7c. Configure here.
One definition for both rule kinds: an AST rule provides create, a scan rule provides scan, and defineRule injects the inert visitor factory hosts require. defineScanRule is gone and the registry generator's matcher is back to main's defineRule|defineRetiredRule form. The catch-all generic overload is constrained to extends Rule so context-sensitive scan arrows resolve to the scan overload instead of freezing widened literal types.


Summary
Adds project-level security scan rules: 36 first-class rules that detect what per-file linting never sees — browser-artifact secret leaks, Firebase/Supabase authorization mistakes, CI/install trust-boundary issues, clickjacking/SVG risks, agent/MCP tool capability risks, injection patterns, and committed secret files.
Architecture
Scan rules follow the same lifecycle as every other rule in the repo:
oxlint-plugin-react-doctor/src/plugin/rules/security-scan/<rule-id>.tsasdefineRule({ id, title, severity, recommendation, scan })— same definition as AST rules, withscanin place ofcreate. Thescan(file) => findingsfield replaces AST visitors for this kind; per-findingseverity/title/helpoverrides cover the two dynamic rules (public-debug-artifact,active-static-asset).Security, auto-tagsecurity-scan), so tags, severities, titles, and help text are single-sourced. The registry generator keeps main's one-rule-per-file contract.@react-doctor/core'scheckSecurityScan(shaped likecheckPnpmHardening) runs inrunInspect's environment-checks phase: one bounded whole-tree walk, per-rule gating through the realshouldEnableRule(capabilities,ignoredTags,disabledBy), and diagnostics streamed through the per-element pipeline so severity controls, inline disables, and surfaces apply like any other diagnostic.services/linter.tsis untouched vs main.Behavior notes
includePaths === undefinedproxy) — projects configuringignore.filesget the security scan.rules/categoriesseverity overrides, inline disables, and surface filtering apply to scan-rule diagnostics.ignore.tags: ["security-scan"](orrules ignore-tag security-scan) silences the whole family.Verification
packages/core/tests/check-security-scan.test.ts) over 31 fixture projects; a differential harness compared the original monolith implementation vs the final engine over all fixtures: 0 diagnostic differences.runInspectintegration tests (full-scan emission, diff-mode gate, severity restamp), ESLint-preset + oxlint-config exclusion regressions, registry invariants (exactly 36 tagged scan rules;scannowhere else),runScanRuleunit harness with regressions for the AST and dynamic-severity rules.pnpm typecheck,pnpm build, core (713 tests), react-doctor (1737 tests), and plugin suites green (the 6 failing plugin files are pre-existing onmain).Test plan
pnpm --filter @react-doctor/core testpnpm --filter oxlint-plugin-react-doctor testpnpm --filter ./packages/react-doctor testpnpm typecheck && pnpm build && pnpm format:checkNote
Medium Risk
New whole-repo security scanning on full inspect increases false-positive surface and scan cost, but diff mode skips it and rules are opt-out via
security-scantag; changes are additive to the diagnostic pipeline rather than altering auth or runtime behavior.Overview
Adds a project-level security file scan with 36
security-scanrules registered like normal React Doctor rules (defineRule+scaninstead of AST visitors) but executed only by@react-doctor/coreduring inspect’s environment-check phase—not via generated oxlint configs or ESLint presets.checkSecurityScanwalks the repo once with caps (depth, file count, size), classifies paths into priority / artifact / other buckets, runs enabled rules through the sameshouldEnableRulegating as lint, and emits Security diagnostics through the standard pipeline (severity overrides, inline disables, surfaces,ignore.tagsforsecurity-scan).Coverage spans shipped bundles and maps,
.env/credential files, Firebase rules, Supabase SQL, CI/package metadata, and pattern-based risks (SQL/NoSQL injection, webhooks, postMessage, BaaS authz, etc.).--diff/ staged scans skip the scan like other whole-project checks. Docs describe how to author scan rules; Tailwind version parsing in core now uses semver lower bounds for gating.Reviewed by Cursor Bugbot for commit a056719. Bugbot is set up for automated code reviews on this repo. Configure here.