Pular para conteúdo

AUR-503 -- Command injection does not accuse a comment or prose

Outcome

The command-injection branches AUR-486 delivered no longer accuse a textual mention. A exec.Command, Process.Start, Invoke-Expression/iex or eval written inside a comment (//, #, /* */, --) or a string literal produces no finding, and eval := ... / eval = ... (a declaration or assignment) produces none either -- while the real invocation of each branch is still found.

Measured finding (2026-09-15)

AUR-486's reviewer reproduced, in the branches AUR-486 added, the same false-positive class AUR-486's round 2 fixed only in sh -c/SQL. These lines all produced a security/command-injection finding before this card:

// exec.Command("sh","-c","ping "+host)
// Process.Start("cmd.exe","/c ping "+host)
# Invoke-Expression "ping $host"
Write-Host "Invoke-Expression is unsafe"
# iex is dangerous
eval := compute()

The AUR-462 rule applies: a false positive in a merge gate teaches the team to disable the gate.

The fix

A line regexp alone cannot decide whether a token sits inside a comment or a string literal. The naive anchor -- start of line or a ;/|/& separator -- both misses real invocations embedded in expressions and still fires on // foo; exec.Command(...), whose ; is inside the comment. The shipped fix therefore splits the decision in two:

  1. internal/review/rules/security.yml keeps every AUR-486 branch's token and argument shape unanchored (the exact bodies AUR-486 shipped). The Go exec.Command shell branch, both C# Process.Start branches and PowerShell Invoke-Expression/iex match the token anywhere on the added line. This is what keeps a real invocation reachable in every expression context: x := exec.Command(...), if err := exec.Command(...), out, err := exec.Command(...), return exec.Command(...), cmd := exec.Command(...), var p = Process.Start(...), if (x) { Process.Start(...) }, $r = Invoke-Expression $payload, $r = iex $payload. The two branches AUR-486's round 2 already anchored (^\s*(?:ba)?sh\s+-c and the SQL-by-shell branch) are untouched.

  2. internal/review/securitypass.go adds the context filter. The match test is re.FindStringIndex(body) instead of re.MatchString(body), and a security/command-injection match whose first byte is not marked CODE by codeMask is dropped. codeMask is a line-local lexical state machine that marks the bytes of # comments, // comments, -- comments (only after whitespace, so x-- is not one), /* */ spans, and "/'/backtick spans as non-code; both the opening marker byte and the whole span are marked, so a match starting on the marker is dropped too. The filter is scoped to security/command-injection only, so security/hardcoded-secret -- whose true positives are legitimately inside string literals -- is unchanged.

bash eval additionally distinguishes invocation from declaration: the branch requires a whitespace-separated argument that is neither : nor = (\beval\s+[^:=\s]), so eval "ping $1" and eval $cmd still fire while eval := compute() and eval = compute() do not.

Acceptance

  • AC-001: each branch (exec.Command, Process.Start, Invoke-Expression/iex, eval) mentioned inside a comment (//, #, /* */, --) or a string literal produces no finding, including // foo; exec.Command(...).
  • AC-002: the real invocation of each branch is still found (Go exec.Command("sh","-c",...) as a bare statement and in expression context, C# Process.Start concatenation and shell file name, PowerShell Invoke-Expression/iex, sh -c/bash -c, and SQL by shell interpolation).
  • AC-003: eval := ... / eval = ... produce no finding; eval "ping $1" / eval $cmd do.

Selectors: unit tests/unit/AUR-503.go::TestAUR503; integration tests/integration/AUR-503.go::IntegrationAUR503; acceptance tests/acceptance/AUR-503.sh, which also runs the unit and integration bridges under all.

Mutations

  • MUT-001: remove the command-injection context guard in internal/review/securitypass.go (rule.ID == "security/command-injection" && !codeMask(body)[loc[0]] -> ... && false). The comment mentions -- including // foo; exec.Command(...) -- are re-accused; if they are not, the mutation did not take effect and AC-001 would be a false green.
  • MUT-002: over-restrict the Go branch to column zero (\bexec\.Command -> ^exec\.Command). The real, tab-indented statement and every expression-context invocation are lost; if they survive, the restriction did not take effect and AC-002 would be a false green.

Both are exercised by tests/acceptance/AUR-503.sh, each asserting a sibling branch (bash eval, C# Process.Start) is untouched so the mutant is proven specific rather than a blanket disablement.

Non-goals

  • No AST and no semantic analysis: the pass stays a deterministic matcher over the added lines plus a line-local lexical mask.
  • No existing pattern is loosened; the AUR-462/AUR-481 true positives stay unchanged.

Known limitations, accepted deliberately

  • A continued block comment. codeMask is line-local: a /* ... */ opened on an earlier line and not closed does not mask the token on this line if the marker is absent from it. The card's AC-001 names the measured single-line comment and string shapes; a multi-line block comment is not one of them. No AST/lexer can decide this line-locally.
  • A string opened on an earlier line. The same line-local limit applies to an unterminated language-level string spanning lines; a token on the continuation line is treated as code. Declared rather than silently shipped.

These are declared rather than silently shipped, per the card's non-goals.