Skip to content

fix(rules): narrow sql-concat, debug-enabled, hardcoded-role patterns - #492

Merged
ajianaz merged 1 commit into
developfrom
fix/security-scanner-false-positives-485-486-490
Aug 5, 2026
Merged

fix(rules): narrow sql-concat, debug-enabled, hardcoded-role patterns#492
ajianaz merged 1 commit into
developfrom
fix/security-scanner-false-positives-485-486-490

Conversation

@ajianaz

@ajianaz ajianaz commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

What

Three security scanner rules produced false positives or false negatives due to overly broad or overly narrow regex patterns. This PR narrows and adds post-match filters for each, following the same 3-layer defense approach established in #484.

Why

#485injection/sql-concat flagged comments, prose, and non-SQL concatenation. The old regex (?i)(?:SELECT|INSERT|UPDATE|DELETE)\s+.*\+ matched any SQL keyword followed by + anywhere — including // SELECT a + b and let total = a + b; // UPDATE: not SQL.

#486config/debug-enabled flagged CLI flags, argument parser definitions, and Dockerfile RUN commands. The old regex included --debug which is a CLI convention, not a config assignment.

#490auth/hardcoded-role missed the most common real-world pattern: quoted role comparisons. The old regex only matched bareword role == admin (rare outside Ruby/PHP), missing role == "admin" (JS, TS, Python, Go), user.role === "admin" (property access + strict equality), and "superuser".

Closes #485, closes #486, closes #490.

How

injection/sql-concat (security_scanner.rs):

  • Narrowed regex to require SQL keyword inside a string literal before matching +
  • Added format!("SELECT ...") pattern for Rust format macros
  • Added is_false_positive_sql_concat() post-match filter: suppresses comment lines (//, #, --, /*, *) and docstrings

config/debug-enabled (security_scanner.rs):

  • Removed --debug from regex — too many false positives in dev tooling
  • Kept DEBUG = True, debug: true, added debug = true
  • Added is_false_positive_debug() post-match filter: suppresses comment lines, argument parser definitions (add_argument, .option(, parser.), and env var references

auth/hardcoded-role (security_scanner.rs):

  • Rewrote regex to accept optional quotes (["']?), strict equality ([=]{2,3}), property access ((?:\w+\.)*role), and superuser
  • Now covers: role == "admin", user.role === 'super', if role === "superuser", etc.

Testing

New test coverage:

  • 5 sql-concat tests (real injection, format! macro, 3 false positive cases)
  • 5 debug-enabled tests (real assignment, YAML config, CLI flag, argument parser, comment)
  • 8 hardcoded-role tests (unquoted, double/single quotes, strict equality, property access, super/root/superuser)

Related Issues

Closes #485, closes #486, closes #490
Part of epic #487

Checklist

…#485, #486, #490)

sql-concat: require SQL keyword inside string literal + post-match
filter for comments/docstrings. Regex now needs a quote char before
the + to qualify as SQL injection.

debug-enabled: remove bare --debug from regex (too many false positives
on CLI flags, Dockerfiles, argument parsers). Keep DEBUG = True and
debug: true assignments. Add post-match filter for comment lines,
argument parser definitions, and env var references.

hardcoded-role: add quoted patterns ("admin", 'admin'), strict
equality (===), property access (user.role), and superuser to the
regex. Now covers JS, TS, Python, Go patterns.

Tests: 854 pass (18 new), 0 clippy warnings, fmt clean.
Closes #485, closes #486, closes #490.
@ajianaz ajianaz added bug Something isn't working scope:core Core review engine labels Aug 4, 2026
@ajianaz
ajianaz merged commit 12f575b into develop Aug 5, 2026
13 checks passed
@ajianaz
ajianaz deleted the fix/security-scanner-false-positives-485-486-490 branch August 5, 2026 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working scope:core Core review engine

Projects

None yet

1 participant