From 092ea31f2571808f697eb2910fa187aae8713d90 Mon Sep 17 00:00:00 2001 From: ronheichman <294254458+ronheichman@users.noreply.github.com> Date: Fri, 28 Aug 2026 20:10:35 +0000 Subject: [PATCH 1/4] feat(rule): enforce compound POSIX command candidates --- docs/enforcement.md | 42 +- docs/rules.md | 20 +- internal/rule/checked_test.go | 1 + internal/rule/engine.go | 138 +++++- internal/rule/engine_test.go | 37 +- .../rule/multicommand_enforcement_test.go | 349 +++++++++++++ internal/rule/sequence_test.go | 19 +- internal/rule/shell.go | 460 ++++++++++++++---- internal/rule/shell_enforcement.go | 80 +++ internal/rule/shell_relations.go | 58 ++- internal/rule/shell_types.go | 2 +- internal/sequence/sequence.go | 22 +- internal/sequence/sequence_test.go | 28 ++ internal/sequence/store.go | 2 +- internal/sequence/store_test.go | 2 +- 15 files changed, 1064 insertions(+), 196 deletions(-) create mode 100644 internal/rule/multicommand_enforcement_test.go diff --git a/docs/enforcement.md b/docs/enforcement.md index 680c25e..1594196 100644 --- a/docs/enforcement.md +++ b/docs/enforcement.md @@ -76,20 +76,34 @@ literal input and can include comments, quoted examples, or other text the shell would not execute. Use the parsed `shell_commands` view when a deny depends on executable command semantics. -Detection parses a broad set of shell structures. When a match is derived from -`shell_commands`, blocking uses a smaller static subset: - -- POSIX shells: one simple command or one pipeline of simple commands -- PowerShell and `cmd.exe`: one simple command -- supported transparent launchers, only when the final child command meets the - same rules - -Every projected command must have static arguments, assignments, and redirect -targets. Multiple statements, conditionals, loops, shell background syntax, -substitutions, same-script functions, inline child interpreters, `eval`, -`Invoke-Expression`, PowerShell or CMD pipelines, previews, parser diagnostics, -and truncated projections stay detection-only. This event-wide gate avoids -denying an action based on a command that may not execute. +Detection parses a broad set of shell structures. Blocking keeps the existing +static requirements for executable tokens, arguments, assignments, redirect +targets, wrappers, previews, parser diagnostics, and projection limits. + +For compound POSIX input, `numbat` forms candidates from the existing +`mvdan.cc/sh/v3/syntax` result. One parsed command forms one candidate. Direct +members of one `|` or `|&` pipeline form one candidate together and keep the +existing pipeline safety checks. + +The forms `;`, `&&`, `||`, groups, subshells, background commands, negation, +substitutions, and heredocs do not disable an otherwise eligible candidate. +Both sides of `&&` and `||` are checked because the input requests both +commands, even when one side might not run. Statically resolved shell function +calls remain detection-only; eligible commands in an invoked function body are +considered separately. + +For an `enforce: true` rule that uses `shell_commands`, CEL evaluates the +complete rule against eligible candidates until one returns true. A true result +denies the complete tool input. A candidate error suppresses enforcement only +when no candidate returns true. Detection still evaluates the complete command +list. A full-list detection error remains a diagnostic, but it does not +suppress a clean candidate deny. Rules that do not use `shell_commands` keep +their existing behavior. + +An unsafe command or direct pipeline remains detection-only. A substitution +inside an unsafe direct pipeline cannot become an independent enforcement +candidate. `eval`, inline child interpreters such as `sh -c`, and PowerShell or +`cmd.exe` compound input remain detection-only. numbat recognizes explicit `-WhatIf` and a statically visible `$WhatIfPreference = $true` for known cmdlets. It does not infer ambient diff --git a/docs/rules.md b/docs/rules.md index a3edd18..b318bcb 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -389,16 +389,11 @@ visible `$WhatIfPreference = $true` for known cmdlet names and exact module-qualified forms. Ambient preference and command-resolution state are not inferred. -For a shell-derived match, blocking has a narrower eligibility boundary than -detection: the complete shell program must be one static simple command or one -static POSIX pipeline. Supported transparent launchers are allowed only when -their final child command is also in that subset. Multiple statements, control -flow, same-script functions, inline child interpreters, `eval` or -`Invoke-Expression`, substitutions, runtime-dependent values, PowerShell or CMD -pipelines, previews, parser diagnostics, and truncated projections remain -detection-only. A rule may still use `shell_commands` alongside structured -fields such as `event.file_path`; a matching commandless structured event does -not require a shell projection. See [Enforcement](enforcement.md). +For shell-derived blocking, `numbat` evaluates the rule against eligible +parser-derived candidates. Both sides of `&&` and `||` are checked. A rule can +still use `shell_commands` with fields such as `event.file_path`. A matching +commandless structured event does not need a shell projection. See +[Enforcement](enforcement.md) for candidate eligibility. ## Enforcement rules @@ -410,9 +405,8 @@ Enforcement uses the same CEL expressions as detection; there is no separate rule language or required predicate shape. Raw `event.command` remains available, but it matches literal input and can therefore match text that the shell would not execute. Use `shell_commands` when blocking depends on parsed -command semantics. A shell-derived match can detect broad shell syntax, but a -deny requires the complete shell program to be inside the static subset -described above. +command semantics. A shell-derived deny requires one eligible parser-derived +candidate as described above. All built-ins ship monitor-only. To enforce one, copy its complete YAML into an operator directory, keep the same ID, set `enforce: true`, and bump the rule diff --git a/internal/rule/checked_test.go b/internal/rule/checked_test.go index e732ab1..10ca61a 100644 --- a/internal/rule/checked_test.go +++ b/internal/rule/checked_test.go @@ -36,6 +36,7 @@ func TestCheckedExpressionsPreserveEvaluation(t *testing.T) { } for _, event := range []model.Event{ {EventType: model.EventCommandExec, Command: "echo ready"}, + {EventType: model.EventCommandExec, Command: "true; echo ready"}, {EventType: model.EventCommandExec, Command: "go test ./..."}, {EventType: model.EventFileRead, FilePath: "README.md"}, } { diff --git a/internal/rule/engine.go b/internal/rule/engine.go index a3b08a7..9ab3327 100644 --- a/internal/rule/engine.go +++ b/internal/rule/engine.go @@ -45,6 +45,7 @@ type compiledRule struct { type compiledExpression struct { program cel.Program + candidateProgram cel.Program usesShellCommands bool usesContent bool } @@ -80,19 +81,38 @@ func (s *SequenceRule) WithinEvents() int { return s.withinEvents } // MaxMatches returns the per-(rule, session) finding cap, always >= 1. func (s *SequenceRule) MaxMatches() int { return s.maxMatches } -// EvalStep evaluates one step predicate against a prebuilt activation. An eval -// error reports false: an erroring predicate must never fabricate a link in a -// chain, so the failure surfaces as a diagnostic and, at worst, a documented -// false negative. -func (s *SequenceRule) EvalStep(i int, activation map[string]any) (bool, error) { +// StepEvaluation reports both the detection and enforcement verdict for one +// sequence step. Enforcement may match a safe command candidate even when the +// complete compound command does not match or produces an evaluation error. +type StepEvaluation struct { + Match bool + EnforcementMatch bool +} + +// EvalStep evaluates one step against the prepared event views. Candidate +// selection and fail-closed shell-analysis handling stay inside the rule +// module so sequence tracking cannot accidentally authorize the wrong view. +func (s *SequenceRule) EvalStep(i int, activations SequenceActivations) (StepEvaluation, error) { if i < 0 || i >= len(s.steps) { - return false, fmt.Errorf("rule %q: step index %d out of range", s.rule.ID, i) + return StepEvaluation{}, fmt.Errorf("rule %q: step index %d out of range", s.rule.ID, i) } - out, _, err := s.steps[i].program.Eval(activation) - if err != nil { - return false, fmt.Errorf("rule %q step %d: evaluation failed", s.rule.ID, i+1) + step := s.steps[i] + prepared := activations.prepared + if prepared.err != nil && step.usesShellCommands && !prepared.shellUsable { + return StepEvaluation{}, nil + } + evaluation := evaluateExpression(step, prepared, s.rule.IsEnforceEligible()) + var errs []error + if evaluation.detectionErr != nil { + errs = append(errs, fmt.Errorf("rule %q step %d: evaluation failed", s.rule.ID, i+1)) + } + if evaluation.candidateErr != nil { + errs = append(errs, fmt.Errorf("rule %q step %d: candidate evaluation failed", s.rule.ID, i+1)) } - return asBool(out), nil + return StepEvaluation{ + Match: evaluation.detectionMatch || evaluation.enforcementMatch, + EnforcementMatch: evaluation.enforcementMatch, + }, errors.Join(errs...) } // StepUsesShellCommands reports whether step i depends on the derived command @@ -119,6 +139,7 @@ func newEnv() (*cel.Env, error) { ext.Lists(), cel.Variable("event", cel.MapType(cel.StringType, cel.DynType)), cel.Variable(shellCommandsVariable, cel.ListType(cel.ObjectType("rule.ShellCommand"))), + cel.Variable(shellCommandCandidatesVariable, cel.ListType(cel.ListType(cel.ObjectType("rule.ShellCommand")))), ) } @@ -377,6 +398,7 @@ func validateRuleAST(ast *cel.Ast) error { } func programExpr(env *cel.Env, ast *cel.Ast) (compiledExpression, error) { + usesShellCommands := astReferencesGlobal(ast, shellCommandsVariable) usesContent := astReferencesEventField(ast, "content") || astReferencesEventField(ast, "content_bytes") || astReferencesEventField(ast, "content_truncated") @@ -388,13 +410,67 @@ func programExpr(env *cel.Env, ast *cel.Ast) (compiledExpression, error) { if err != nil { return compiledExpression{}, fmt.Errorf("program expr: %w", err) } + var candidateProgram cel.Program + if usesShellCommands { + candidateAST, err := shellCandidateAST(env, ast) + if err != nil { + return compiledExpression{}, err + } + candidateProgram, err = env.Program(candidateAST, programOptions...) + if err != nil { + return compiledExpression{}, fmt.Errorf("program candidate enforcement expr: %w", err) + } + } return compiledExpression{ program: prg, - usesShellCommands: astReferencesGlobal(ast, shellCommandsVariable), + candidateProgram: candidateProgram, + usesShellCommands: usesShellCommands, usesContent: usesContent, }, nil } +const shellCandidateTemplate = shellCommandCandidatesVariable + ".exists(" + shellCommandsVariable + ", true)" + +// shellCandidateAST wraps an already checked predicate in a CEL exists +// comprehension without rendering and reparsing the authored source. The +// optimizer rechecks the combined tree, resolving shell_commands to the +// comprehension variable while retaining the checked predicate's structure. +func shellCandidateAST(env *cel.Env, predicate *cel.Ast) (*cel.Ast, error) { + template, issues := env.Compile(shellCandidateTemplate) + if issues != nil && issues.Err() != nil { + return nil, fmt.Errorf("compile candidate enforcement template: %w", issues.Err()) + } + optimizer, err := cel.NewStaticOptimizer(shellCandidateOptimizer{predicate: predicate}) + if err != nil { + return nil, fmt.Errorf("build candidate enforcement optimizer: %w", err) + } + optimized, issues := optimizer.Optimize(env, template) + if issues != nil && issues.Err() != nil { + return nil, fmt.Errorf("check candidate enforcement expr: %w", issues.Err()) + } + return optimized, nil +} + +type shellCandidateOptimizer struct { + predicate *cel.Ast +} + +func (o shellCandidateOptimizer) Optimize(ctx *cel.OptimizerContext, wrapper *celast.AST) *celast.AST { + root := wrapper.Expr() + if root.Kind() != celast.ComprehensionKind { + ctx.ReportErrorAtID(root.ID(), "candidate enforcement template did not expand to a comprehension") + return wrapper + } + loopStep := root.AsComprehension().LoopStep() + if loopStep.Kind() != celast.CallKind || len(loopStep.AsCall().Args()) != 2 { + ctx.ReportErrorAtID(loopStep.ID(), "candidate enforcement template has an invalid loop step") + return wrapper + } + predicate := loopStep.AsCall().Args()[1] + ctx.UpdateExpr(predicate, ctx.CopyASTAndMetadata(o.predicate.NativeRep())) + return wrapper +} + func astReferencesEventField(ast *cel.Ast, field string) bool { found := false walkEventFields(ast.NativeRep(), ast.NativeRep().Expr(), false, func(name string, literal bool) { @@ -665,26 +741,48 @@ func (e *Engine) Eval(ev model.Event) ([]Match, error) { if activations.err != nil && c.program.usesShellCommands && !activations.shellUsable { continue } - out, _, err := c.program.program.Eval(activations.detection) - if err != nil { + evaluation := evaluateExpression(c.program, activations, c.rule.IsEnforceEligible()) + if evaluation.detectionErr != nil { errs = append(errs, fmt.Errorf("rule %q: evaluation failed", c.rule.ID)) - continue } - if asBool(out) { - enforcementMatch := c.rule.IsEnforceEligible() - if enforcementMatch && c.program.usesShellCommands && !activations.shellEnforcementSafe { - enforcementMatch = false - } + if evaluation.candidateErr != nil { + errs = append(errs, fmt.Errorf("rule %q: candidate evaluation failed", c.rule.ID)) + } + if evaluation.detectionMatch || evaluation.enforcementMatch { matches = append(matches, Match{ Rule: cloneRule(c.rule), Event: ev, - EnforcementMatch: enforcementMatch, + EnforcementMatch: evaluation.enforcementMatch, }) } } return matches, errors.Join(errs...) } +type expressionEvaluation struct { + detectionMatch bool + enforcementMatch bool + detectionErr error + candidateErr error +} + +func evaluateExpression(expr compiledExpression, activations sequenceActivations, enforceEligible bool) expressionEvaluation { + out, _, detectionErr := expr.program.Eval(activations.detection) + detectionMatch := detectionErr == nil && asBool(out) + evaluation := expressionEvaluation{ + detectionMatch: detectionMatch, + enforcementMatch: detectionMatch && enforceEligible, + detectionErr: detectionErr, + } + if !enforceEligible || !expr.usesShellCommands || activations.shellEnforcementSafe { + return evaluation + } + candidate, _, candidateErr := expr.candidateProgram.Eval(activations.detection) + evaluation.enforcementMatch = candidateErr == nil && asBool(candidate) + evaluation.candidateErr = candidateErr + return evaluation +} + // asBool reports whether a CEL result is a true boolean. Any non-bool result // (which compile-time type checking already forbids) is treated as no match. func asBool(v ref.Val) bool { diff --git a/internal/rule/engine_test.go b/internal/rule/engine_test.go index bfe96cb..8e7b0b1 100644 --- a/internal/rule/engine_test.go +++ b/internal/rule/engine_test.go @@ -105,6 +105,14 @@ func TestEngineContentCostLimit(t *testing.T) { } } +func TestEngineShellRuleAllowsTrailingComment(t *testing.T) { + mustEngine(t, Rule{ + ID: "t.trailing_comment", + Severity: model.SeverityHigh, + Expr: `shell_commands.exists(command, command.name == "cat") // documented intent`, + }) +} + func TestEngineShellCommandsUsesExecutableStatements(t *testing.T) { eng := mustEngine(t, Rule{ ID: "t.scheduler", @@ -444,8 +452,8 @@ func TestEngineDoesNotEnforceRuntimeDependentCommands(t *testing.T) { ToolName: "bash", Command: `run(){ wipefs -a /dev/sda; }; run`, }) - if err != nil || len(staticBody) != 1 || staticBody[0].EnforcementMatch { - t.Fatalf("static function body = (%+v, %v), want detection-only match", staticBody, err) + if err != nil || len(staticBody) != 1 || !staticBody[0].EnforcementMatch { + t.Fatalf("static function body = (%+v, %v), want enforceable request match", staticBody, err) } } @@ -532,7 +540,7 @@ func TestEngineDoesNotInferPowerShellPreviewForNativeCommandsOrAmbientState(t *t } } -func TestEngineDoesNotEnforcePartialShellAnalysis(t *testing.T) { +func TestEngineEnforcesCompleteCandidateWithDynamicSibling(t *testing.T) { enforce := true eng := mustEngine(t, Rule{ ID: "t.remove", @@ -550,8 +558,8 @@ func TestEngineDoesNotEnforcePartialShellAnalysis(t *testing.T) { if err == nil { t.Fatal("Eval succeeded, want dynamic-command diagnostic") } - if len(matches) != 1 || matches[0].EnforcementMatch { - t.Fatalf("partial static match = %+v, want detection-only rm witness", matches) + if len(matches) != 1 || !matches[0].EnforcementMatch { + t.Fatalf("partial static match = %+v, want enforceable rm candidate", matches) } eng = mustEngine(t, Rule{ @@ -600,6 +608,14 @@ func TestEngineLimitsEnforcementToStaticShellSubset(t *testing.T) { {EventType: model.EventCommandExec, ToolName: "bash", Command: `command wipefs -a /dev/sda`}, {EventType: model.EventCommandExec, ToolName: "bash", Command: `exec wipefs -a /dev/sda`}, {EventType: model.EventCommandExec, ToolName: "bash", Command: `nohup wipefs -a /dev/sda`}, + {EventType: model.EventCommandExec, ToolName: "bash", Command: `false && wipefs -a /dev/sda`}, + {EventType: model.EventCommandExec, ToolName: "bash", Command: `true || wipefs -a /dev/sda`}, + {EventType: model.EventCommandExec, ToolName: "bash", Command: `if false; then wipefs -a /dev/sda; fi`}, + {EventType: model.EventCommandExec, ToolName: "bash", Command: `for x in one; do wipefs -a /dev/sda; done`}, + {EventType: model.EventCommandExec, ToolName: "bash", Command: `echo ready; wipefs -a /dev/sda`}, + {EventType: model.EventCommandExec, ToolName: "bash", Command: `! wipefs -a /dev/sda`}, + {EventType: model.EventCommandExec, ToolName: "bash", Command: `wipefs -a /dev/sda &`}, + {EventType: model.EventCommandExec, ToolName: "bash", Command: `echo "$(wipefs -a /dev/sda)"`}, {EventType: model.EventCommandExec, ToolName: "PowerShell", Command: `Stop-Process -Name target -Force`}, {EventType: model.EventCommandExec, ToolName: "PowerShell", Command: `Stop-Process -Name target -Force 2>&1`}, {EventType: model.EventCommandExec, ToolName: "cmd.exe", Command: `del C:\target`}, @@ -613,11 +629,7 @@ func TestEngineLimitsEnforcementToStaticShellSubset(t *testing.T) { } detectionOnly := []model.Event{ - {EventType: model.EventCommandExec, ToolName: "bash", Command: `false && wipefs -a /dev/sda`}, - {EventType: model.EventCommandExec, ToolName: "bash", Command: `true || wipefs -a /dev/sda`}, - {EventType: model.EventCommandExec, ToolName: "bash", Command: `if false; then wipefs -a /dev/sda; fi`}, - {EventType: model.EventCommandExec, ToolName: "bash", Command: `for x in one; do wipefs -a /dev/sda; done`}, - {EventType: model.EventCommandExec, ToolName: "bash", Command: `run(){ wipefs -a /dev/sda; }; run`}, + {EventType: model.EventCommandExec, ToolName: "bash", Command: `wipefs(){ echo safe; }; wipefs -a /dev/sda`}, {EventType: model.EventCommandExec, ToolName: "bash", Command: `eval 'wipefs -a /dev/sda'`}, {EventType: model.EventCommandExec, ToolName: "bash", Command: `command eval 'wipefs -a /dev/sda'`}, {EventType: model.EventCommandExec, ToolName: "bash", Command: `command exec wipefs -a /dev/sda`}, @@ -647,13 +659,8 @@ func TestEngineLimitsEnforcementToStaticShellSubset(t *testing.T) { {EventType: model.EventCommandExec, ToolName: "bash", Command: `env -u PATH wipefs -a /dev/sda`}, {EventType: model.EventCommandExec, ToolName: "bash", Command: `exec -a wipe wipefs -a /dev/sda`}, {EventType: model.EventCommandExec, ToolName: "bash", Command: `pwsh -Command 'Stop-Process -Name target -Force'`}, - {EventType: model.EventCommandExec, ToolName: "bash", Command: `echo "$(wipefs -a /dev/sda)"`}, {EventType: model.EventCommandExec, ToolName: "bash", Command: `wipefs -a /dev/sda --no-*`}, {EventType: model.EventCommandExec, ToolName: "bash", Command: `wipefs -a /dev/sda --{no-act,other}`}, - {EventType: model.EventCommandExec, ToolName: "bash", Command: `echo ready; wipefs -a /dev/sda`}, - {EventType: model.EventCommandExec, ToolName: "bash", Command: `"$next"; wipefs -a /dev/sda`}, - {EventType: model.EventCommandExec, ToolName: "bash", Command: `! wipefs -a /dev/sda`}, - {EventType: model.EventCommandExec, ToolName: "bash", Command: `wipefs -a /dev/sda &`}, {EventType: model.EventCommandExec, ToolName: "PowerShell", Command: `Write-Output ready; Stop-Process -Name target -Force`}, {EventType: model.EventCommandExec, ToolName: "PowerShell", Command: `if ($false) { Stop-Process -Name target -Force }`}, {EventType: model.EventCommandExec, ToolName: "PowerShell", Command: `Write-Output target | Stop-Process -Force`}, diff --git a/internal/rule/multicommand_enforcement_test.go b/internal/rule/multicommand_enforcement_test.go new file mode 100644 index 0000000..1a84db7 --- /dev/null +++ b/internal/rule/multicommand_enforcement_test.go @@ -0,0 +1,349 @@ +package rule + +import ( + "testing" + + "github.com/perplexityai/numbat/internal/model" +) + +func compoundRuleEngine(t *testing.T, expr string) *Engine { + t.Helper() + return mustEngine(t, Rule{ + ID: "t.multicommand", + Title: "multi-command enforcement", + Version: "1", + Severity: model.SeverityHigh, + Enforce: boolPtr(true), + Expr: expr, + }) +} + +func TestMultiCommandEnforcementRegression(t *testing.T) { + eng := compoundRuleEngine(t, `shell_commands.exists(command, + command.name == "cat" && command.argv.exists(arg, arg == ".env"))`) + tests := []struct { + name string + command string + wantErr bool + }{ + {name: "simple", command: `cat .env`}, + {name: "pipeline", command: `cat .env | grep x`}, + {name: "semicolon", command: `echo hi; cat .env`}, + {name: "and", command: `false && cat .env`}, + {name: "or", command: `true || cat .env`}, + {name: "subshell", command: `(cat .env)`}, + {name: "group", command: `{ cat .env; }`}, + {name: "background", command: `cat .env &`}, + {name: "negation", command: `! cat .env`}, + {name: "heredoc", command: "cat .env <<'EOF'\nbody\nEOF"}, + {name: "interpreter heredoc", command: "sh <<'EOF'\ncat .env\nEOF"}, + {name: "plus option interpreter heredoc", command: "bash +n <<'EOF'\ncat .env\nEOF"}, + {name: "disabled short noexec", command: "bash -n +n <<'EOF'\ncat .env\nEOF"}, + {name: "disabled named noexec", command: "bash -o noexec +o noexec <<'EOF'\ncat .env\nEOF"}, + {name: "disabled zsh noexec", command: "zsh --noexec --exec <<'EOF'\ncat .env\nEOF"}, + {name: "grouped interpreter heredoc", command: "{ sh; } <<'EOF'\ncat .env\nEOF"}, + {name: "grouped substitution interpreter heredoc", command: "{ echo \"$(sh)\"; } <<'EOF'\ncat .env\nEOF"}, + {name: "nested grouped substitution interpreter heredoc", command: "{ echo \"$(echo \"$(sh)\")\"; } <<'EOF'\ncat .env\nEOF"}, + {name: "grouped process substitution interpreter heredoc", command: "{ cat < <(sh); } <<'EOF'\ncat .env\nEOF"}, + {name: "grouped descriptor interpreter heredoc", command: "{ echo \"$(sh 0<&3)\"; } 3<<'EOF'\ncat .env\nEOF"}, + {name: "grouped interpreter heredoc with unrelated close", command: "{ sh; } <<'EOF' 3>&-\ncat .env\nEOF"}, + {name: "grouped interpreter heredoc with named output", command: "{ sh; } <<'EOF' {fd}>/dev/null\ncat .env\nEOF", wantErr: true}, + {name: "interpreter heredoc with named output", command: "sh <<'EOF' {fd}>/dev/null\ncat .env\nEOF", wantErr: true}, + {name: "wrapped interpreter heredoc with named output", command: "env sh <<'EOF' {fd}>/dev/null\ncat .env\nEOF", wantErr: true}, + {name: "grouped function interpreter heredoc", command: "f(){ sh; }; { f; } <<'EOF'\ncat .env\nEOF"}, + {name: "grouped interpreter heredoc after assignment", command: "{ x=1; sh; } <<'EOF'\ncat .env\nEOF"}, + {name: "grouped function interpreter heredoc after assignment", command: "f(){ x=1; sh; }; { f; } <<'EOF'\ncat .env\nEOF"}, + {name: "grouped interpreter heredoc after reader", command: "{ read -r ignored; sh; } <<'EOF'\nignored\ncat .env\nEOF"}, + {name: "grouped interpreter heredoc after consumer", command: "{ cat >/dev/null; sh; } <<'EOF'\ncat .env\nEOF"}, + {name: "grouped interpreter heredoc before overridden sibling", command: "{ { { sh 6>&-; sh /dev/null; } 4>/dev/null; } <<'EOF'\ncat .env\nEOF"}, + {name: "output process substitution preserves descriptor input", command: "{ printf x > >(sh /dev/fd/3); wait; } 3<<'EOF'\ncat .env\nEOF"}, + {name: "subshell interpreter heredoc after consumer", command: "(cat >/dev/null; sh) <<'EOF'\ncat .env\nEOF"}, + {name: "if interpreter heredoc", command: "if :; then sh; fi <<'EOF'\ncat .env\nEOF"}, + {name: "while interpreter heredoc", command: "while :; do sh; break; done <<'EOF'\ncat .env\nEOF"}, + {name: "for interpreter heredoc", command: "for x in x; do sh; done <<'EOF'\ncat .env\nEOF"}, + {name: "case interpreter heredoc", command: "case x in x) sh;; esac <<'EOF'\ncat .env\nEOF"}, + {name: "stdin interpreter heredoc", command: "sh /dev/stdin <<'EOF'\ncat .env\nEOF"}, + {name: "explicit stdin interpreter heredoc", command: "sh - <<'EOF'\ncat .env\nEOF"}, + {name: "script after option terminator", command: "sh - /dev/fd/3 3<<'EOF'\ncat .env\nEOF"}, + {name: "zsh script after plus terminator", command: "zsh + /dev/fd/3 3<<'EOF'\ncat .env\nEOF"}, + {name: "stdin interpreter after terminator", command: "sh -- /dev/stdin <<'EOF'\ncat .env\nEOF"}, + {name: "stdin interpreter fd path", command: "sh -- /dev/fd/0 <<'EOF'\ncat .env\nEOF"}, + {name: "zsh named stdin option", command: "zsh --stdin /dev/null <<'EOF'\ncat .env\nEOF"}, + {name: "zsh named stdin shell option", command: "zsh -o SHIN_STDIN /dev/null <<'EOF'\ncat .env\nEOF"}, + {name: "zsh attached stdin shell option", command: "zsh -oSHIN_STDIN /dev/null <<'EOF'\ncat .env\nEOF"}, + {name: "zsh inverse long stdin option", command: "zsh +-no-SHIN_STDIN /dev/null <<'EOF'\ncat .env\nEOF"}, + {name: "zsh sh option letters after b", command: "zsh --sh-option-letters -n -b +n -s <<'EOF'\ncat .env\nEOF"}, + {name: "interpreter here string", command: "sh /dev/stdin <<< 'cat .env'"}, + {name: "heredoc before self duplication", command: "sh /dev/stdin <<'EOF' 0<&0\ncat .env\nEOF"}, + {name: "heredoc before output duplication", command: "sh /dev/stdin <<'EOF' 0>&0\ncat .env\nEOF"}, + {name: "heredoc through descriptor", command: "sh -s 3<<'EOF' 0<&3\ncat .env\nEOF"}, + {name: "heredoc through moved descriptor", command: "sh -s 3<<'EOF' 0<&3-\ncat .env\nEOF"}, + {name: "heredoc from descriptor path", command: "sh /dev/fd/3 3<<'EOF'\ncat .env\nEOF"}, + {name: "heredoc from normalized descriptor path", command: "sh /dev/fd//3 3<<'EOF'\ncat .env\nEOF"}, + {name: "heredoc from rcfile descriptor", command: "bash --noprofile --rcfile /dev/fd/3 -i -c 'exit' 3<<'EOF'\ncat .env\nEOF"}, + {name: "heredoc from enabled rcfile descriptor", command: "bash --rcfile /dev/fd/3 +i -i -c 'exit' 3<<'EOF'\ncat .env\nEOF"}, + {name: "heredoc from second interpreter input", command: "bash --noprofile --rcfile /dev/fd/3 -i 3<<'RC' 0<<'MAIN'\n:\nRC\ncat .env\nexit\nMAIN"}, + {name: "command substitution", command: `echo "$(cat .env)"`}, + {name: "standalone command substitution", command: `$(cat .env)`, wantErr: true}, + {name: "redirect substitution", command: `{ true; } > "$(cat .env)"`}, + {name: "group dynamic redirect", command: `{ cat .env; } > "$target"`}, + {name: "subshell dynamic redirect", command: `(cat .env) > "$target"`}, + {name: "group dynamic descriptor", command: `{ cat .env; } {fd}>out`, wantErr: true}, + {name: "subshell dynamic descriptor", command: `(cat .env) {fd}>out`, wantErr: true}, + {name: "named descriptor redirect substitution", command: `true "$(cat .env)" {fd}>out`, wantErr: true}, + {name: "commandless descriptor redirect substitution", command: `> "$(cat .env)" {fd}>out`, wantErr: true}, + {name: "assignment descriptor redirect substitution", command: `X=1 >"$(cat .env)" {fd}>out`, wantErr: true}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + matches, err := eng.Eval(model.Event{EventType: model.EventCommandExec, ToolName: "bash", Command: test.command}) + if err != nil && !test.wantErr { + t.Fatal(err) + } + if err == nil && test.wantErr { + t.Fatal("Eval returned no dynamic executable diagnostic") + } + if len(matches) != 1 || !matches[0].EnforcementMatch { + t.Fatalf("Eval(%q) did not return one enforceable match", test.command) + } + }) + } +} + +func TestMultiCommandEnforcementUsesPOSIXParserForExecCommand(t *testing.T) { + eng := compoundRuleEngine(t, `shell_commands.exists(command, + command.name == "cat" && command.argv.exists(arg, arg == ".env"))`) + for _, command := range []string{ + "if (true)\nthen\ncat .env\nfi", + "get-process; cat .env", + } { + matches, err := eng.Eval(model.Event{ + SourceAgent: model.AgentCodex, + EventType: model.EventCommandExec, + ToolName: "exec_command", + Command: command, + }) + if err != nil { + t.Fatal(err) + } + if len(matches) != 1 || !matches[0].EnforcementMatch { + t.Fatalf("Eval(%q) returned %+v, want one enforceable POSIX candidate match", command, matches) + } + } +} + +func TestMultiCommandEnforcementCandidateEvaluation(t *testing.T) { + tests := []struct { + name, expr, command string + wantErr, wantEnforce bool + }{ + {name: "complete candidate", expr: `shell_commands.size() == 1 && shell_commands[0].name == "cat" && shell_commands[0].argv.exists(arg, arg == ".env")`, command: `cat .env; true`, wantEnforce: true}, + {name: "list all", expr: `event.event_type == "command.exec" && shell_commands.all(command, command.name == "cat")`, command: `cat one; cat two`, wantEnforce: true}, + {name: "error before match", expr: `shell_commands.size() == 1 && shell_commands[0].argv[1] == "x"`, command: `noop; echo x`, wantEnforce: true}, + {name: "error after match", expr: `shell_commands.size() == 1 && shell_commands[0].argv[1] == "x"`, command: `echo x; noop`, wantEnforce: true}, + {name: "only errors", expr: `shell_commands.size() == 1 && shell_commands[0].argv[1] == "x"`, command: `noop; echo y`, wantErr: true}, + {name: "raw predicate", expr: `event.command.contains("RAW_BLOCK") || shell_commands.exists(command, command.name == "never-match")`, command: `echo RAW_BLOCK "$x"; true`, wantEnforce: true}, + {name: "nested raw predicate", expr: `(event.command.contains("RAW_BLOCK") || shell_commands.exists(command, command.name == "never-match")) == true`, command: `echo RAW_BLOCK "$x"; true`, wantEnforce: true}, + {name: "aggregate error", expr: `shell_commands.filter(command, command.name == "cat").size() == 1 && shell_commands[0].argv[1] == ".env"`, command: `true; cat .env`, wantErr: true, wantEnforce: true}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + eng := compoundRuleEngine(t, test.expr) + matches, err := eng.Eval(model.Event{EventType: model.EventCommandExec, ToolName: "bash", Command: test.command}) + if (err != nil) != test.wantErr { + t.Fatalf("Eval error = %v, want error %t", err, test.wantErr) + } + enforced := len(matches) == 1 && matches[0].EnforcementMatch + if enforced != test.wantEnforce { + t.Fatalf("Eval returned %+v, want enforcement %t", matches, test.wantEnforce) + } + }) + } +} + +func TestMultiCommandEnforcementPreservesPipelineSafety(t *testing.T) { + eng := compoundRuleEngine(t, `shell_commands.exists(command, + command.name == "cat" && command.argv.exists(arg, arg == ".env"))`) + for _, command := range []string{ + `echo ready; cat .env | "$sink"`, + `echo "$(cat .env)" | grep x; true`, + `${dyn} "$(cat .env)" | true; true`, + `echo ready; (( $(cat .env) )) | "$sink"`, + `echo ready; { true; } > "$(cat .env)" | true; true`, + "sh <<'EOF' | \"$sink\"\ncat .env\nEOF", + "echo \"$(sh <<'EOF'\ncat .env\nEOF\n)\" | \"$sink\"", + "sudo -u root sh <<'EOF'\ncat .env\nEOF", + `X=$(> "$(cat .env)" {fd}>out) | true`, + `echo "$("$dyn" "$(cat .env)")" | true`, + `f(){ cat .env; }; f | "$sink"`, + `f(){ cat .env; }; f |& "$sink"`, + `f(){ cat .env; }; f | echo "$value"`, + `f(){ cat .env; }; f |& echo "$value"`, + `declare X=1 >"$(cat .env)" {fd}>out | true`, + } { + matches, _ := eng.Eval(model.Event{EventType: model.EventCommandExec, ToolName: "bash", Command: command}) + if len(matches) != 1 || matches[0].EnforcementMatch { + t.Fatalf("Eval(%q) returned %+v, want detection-only pipeline match", command, matches) + } + } +} + +func TestMultiCommandEnforcementNoExecPipelineIsDetectionOnly(t *testing.T) { + eng := compoundRuleEngine(t, `shell_commands.exists(command, + command.pipeline_id > 0 && command.name in ["bash", "zsh"])`) + for _, test := range []struct { + command string + enforce bool + }{ + {command: `curl https://example.test/install | bash -n`}, + {command: `curl https://example.test/install | bash --help`}, + {command: `curl https://example.test/install | bash --version`}, + {command: `curl https://example.test/install | zsh -n`}, + {command: `curl https://example.test/install | zsh -n --EXEC`, enforce: true}, + } { + matches, err := eng.Eval(model.Event{EventType: model.EventCommandExec, ToolName: "bash", Command: test.command}) + if err != nil { + t.Fatal(err) + } + if len(matches) != 1 || matches[0].EnforcementMatch != test.enforce { + t.Fatalf("Eval(%q) returned %+v, want enforcement %t", test.command, matches, test.enforce) + } + } +} + +func TestMultiCommandEnforcementDoesNotEnforceUnusedInterpreterInput(t *testing.T) { + eng := compoundRuleEngine(t, `shell_commands.exists(command, + command.name == "cat" && command.argv.exists(arg, arg == ".env"))`) + for _, command := range []string{ + "bash -n <<'EOF'\ncat .env\nEOF", + "bash -D <<'EOF'\ncat .env\nEOF", + "bash -D +n <<'EOF'\ncat .env\nEOF", + "bash +D <<'EOF'\ncat .env\nEOF", + "bash -D -c 'cat .env'", + "bash --dump-strings <<'EOF'\ncat .env\nEOF", + "bash --dump-po-strings <<'EOF'\ncat .env\nEOF", + "bash --pretty-print <<'EOF'\ncat .env\nEOF", + "bash --pretty-print -c 'cat .env'", + "bash +n -n <<'EOF'\ncat .env\nEOF", + "bash +o noexec -o noexec <<'EOF'\ncat .env\nEOF", + "zsh --exec --noexec <<'EOF'\ncat .env\nEOF", + "zsh -o SHIN_STDIN +o SHIN_STDIN /dev/null <<'EOF'\ncat .env\nEOF", + "zsh -oSHIN_STDIN +oSHIN_STDIN /dev/null <<'EOF'\ncat .env\nEOF", + "zsh --stdin --no-shinstdin /dev/null <<'EOF'\ncat .env\nEOF", + "zsh +-SHIN_STDIN /dev/null <<'EOF'\ncat .env\nEOF", + "zsh -b - /dev/fd/3 3<<'EOF'\ncat .env\nEOF", + "zsh --help <<'EOF'\ncat .env\nEOF", + "zsh --version <<'EOF'\ncat .env\nEOF", + "{ sh -c sh; } <<'EOF'\ncat .env\nEOF", + "{ eval sh; } <<'EOF'\ncat .env\nEOF", + "bash --rcfile /dev/fd/3 --norc -i -c 'exit' 3<<'EOF'\ncat .env\nEOF", + "bash --rcfile /dev/fd/3 -i +i -c 'exit' 3<<'EOF'\ncat .env\nEOF", + "bash --noprofile --rcfile /dev/fd/3 -c 'true' 3<<'RC'\ncat .env\nRC", + "bash --noprofile --rcfile /dev/fd/3 --rcfile /dev/fd/4 -i -c 'exit' 3<<'FIRST' 4<<'SECOND'\ncat .env\nFIRST\n:\nSECOND", + "{ echo \"$(sh <<'INNER'\n:\nINNER\n)\"; } <<'OUTER'\ncat .env\nOUTER", + "{ sh 0<&3; } 3<<'EOF' 0<&3-\ncat .env\nEOF", + "{ sh 0<&-; } <<'EOF'\ncat .env\nEOF", + "if :; then sh >(sh); wait; } <<'EOF'\ncat .env\nEOF", + "sh -s 3<<'EOF' 0<&+3\ncat .env\nEOF", + } { + matches, _ := eng.Eval(model.Event{EventType: model.EventCommandExec, ToolName: "bash", Command: command}) + for _, match := range matches { + if match.EnforcementMatch { + t.Fatalf("Eval(%q) returned %+v, want no enforcement for unused interpreter input", command, matches) + } + } + } +} + +func TestMultiCommandEnforcementParsesSharedInterpreterInputOnce(t *testing.T) { + eng := compoundRuleEngine(t, `shell_commands.filter(command, + command.name == "cat").size() > 1`) + for _, test := range []struct { + name string + command string + }{ + {name: "same descriptor", command: "bash --rcfile /dev/fd/0 -i <<'EOF'\ncat .env\nEOF"}, + {name: "aliased descriptor", command: "bash --rcfile /dev/fd/3 -i -s 3<<'EOF' 0<&3\ncat .env\nEOF"}, + } { + t.Run(test.name, func(t *testing.T) { + matches, err := eng.Eval(model.Event{EventType: model.EventCommandExec, ToolName: "bash", Command: test.command}) + if err != nil { + t.Fatal(err) + } + if len(matches) != 0 { + t.Fatalf("Eval returned %+v, want one heredoc projection", matches) + } + }) + } +} + +func TestMultiCommandEnforcementDoesNotProjectInvalidBashInvocation(t *testing.T) { + eng := compoundRuleEngine(t, `shell_commands.exists(command, + command.name == "cat" && command.argv.exists(arg, arg == ".env"))`) + matches, err := eng.Eval(model.Event{ + EventType: model.EventCommandExec, + ToolName: "bash", + Command: "bash -i --rcfile /dev/fd/3 -c exit 3<<'EOF'\ncat .env\nEOF", + }) + if err != nil { + t.Fatal(err) + } + if len(matches) != 0 { + t.Fatalf("Eval returned %+v, want no nested projection from an invalid Bash invocation", matches) + } +} + +func TestMultiCommandEnforcementDoesNotUseRecoveredCommand(t *testing.T) { + eng := compoundRuleEngine(t, `shell_commands.exists(command, + command.name == "cat" && command.argv.exists(arg, arg == ".env"))`) + for _, command := range []string{ + `cat .env |`, + `{ cat .env`, + `cat .env; )`, + "cat .env\n)", + } { + matches, err := eng.Eval(model.Event{EventType: model.EventCommandExec, ToolName: "bash", Command: command}) + if err == nil { + t.Fatalf("Eval(%q) returned no malformed syntax error", command) + } + for _, match := range matches { + if match.EnforcementMatch { + t.Fatalf("Eval(%q) returned %+v, want no recovered command enforcement", command, matches) + } + } + } +} + +func TestMultiCommandEnforcementDoesNotDetachUnsafeCommands(t *testing.T) { + eng := compoundRuleEngine(t, `shell_commands.exists(command, + command.argv.exists(arg, arg == "BLOCK"))`) + for _, command := range []string{ + `eval BLOCK | true; true`, + `bash -c true BLOCK; true`, + `declare BLOCK | true; true`, + `BLOCK(){ :; }; false && BLOCK`, + } { + matches, _ := eng.Eval(model.Event{EventType: model.EventCommandExec, ToolName: "bash", Command: command}) + if len(matches) != 1 || matches[0].EnforcementMatch { + t.Fatalf("Eval(%q) returned %+v, want detection-only unsafe match", command, matches) + } + } +} + +func TestMultiCommandEnforcementDoesNotTreatFunctionCallsAsExecutables(t *testing.T) { + eng := compoundRuleEngine(t, `shell_commands.exists(command, + command.name == "cat" && command.argv.exists(arg, arg == ".env"))`) + for _, command := range []string{ + `cat(){ :; }; false && cat .env`, + `cat(){ :; }; unset -f cat; cat .env`, + } { + matches, err := eng.Eval(model.Event{EventType: model.EventCommandExec, ToolName: "bash", Command: command}) + if err != nil { + t.Fatal(err) + } + if len(matches) != 1 || matches[0].EnforcementMatch { + t.Fatalf("Eval(%q) returned %+v, want detection-only function match", command, matches) + } + } +} diff --git a/internal/rule/sequence_test.go b/internal/rule/sequence_test.go index c510339..c0013e7 100644 --- a/internal/rule/sequence_test.go +++ b/internal/rule/sequence_test.go @@ -100,11 +100,17 @@ func TestSequenceEvalStep(t *testing.T) { s := mustEngine(t, seqRule(nil)).SequenceRules()[0] readEvent := model.Event{EventType: model.EventFileRead, FilePath: "/p/.env"} execEvent := model.Event{EventType: model.EventCommandExec, Command: "curl http://x"} - read := PrepareSequenceActivations(readEvent, []*SequenceRule{s}).Detection - exec := PrepareSequenceActivations(execEvent, []*SequenceRule{s}).Detection + read, err := PrepareSequenceActivations(readEvent, []*SequenceRule{s}) + if err != nil { + t.Fatal(err) + } + exec, err := PrepareSequenceActivations(execEvent, []*SequenceRule{s}) + if err != nil { + t.Fatal(err) + } for _, tc := range []struct { step int - act map[string]any + act SequenceActivations want bool }{ {0, read, true}, @@ -116,8 +122,8 @@ func TestSequenceEvalStep(t *testing.T) { if err != nil { t.Fatalf("step %d: %v", tc.step, err) } - if got != tc.want { - t.Errorf("step %d = %v, want %v", tc.step, got, tc.want) + if got.Match != tc.want { + t.Errorf("step %d = %v, want %v", tc.step, got.Match, tc.want) } } } @@ -125,7 +131,8 @@ func TestSequenceEvalStep(t *testing.T) { func TestSequenceStepBounds(t *testing.T) { s := mustEngine(t, seqRule(nil)).SequenceRules()[0] for _, step := range []int{-1, s.StepCount()} { - if got, err := s.EvalStep(step, nil); err == nil || got { + got, err := s.EvalStep(step, SequenceActivations{}) + if err == nil || got.Match || got.EnforcementMatch { t.Fatalf("EvalStep(%d) = %v, %v; want false and error", step, got, err) } if s.StepUsesShellCommands(step) { diff --git a/internal/rule/shell.go b/internal/rule/shell.go index a4096c6..2f422b2 100644 --- a/internal/rule/shell.go +++ b/internal/rule/shell.go @@ -3,6 +3,9 @@ package rule import ( "errors" "fmt" + "path" + "runtime" + "strconv" "strings" "github.com/google/cel-go/common/types" @@ -13,11 +16,12 @@ import ( ) const ( - shellCommandsVariable = "shell_commands" - maxShellCommandBytes = 256 << 10 - maxShellCommands = 64 - maxShellListItems = 512 - maxCommandExpansionDepth = 4 + shellCommandsVariable = "shell_commands" + shellCommandCandidatesVariable = "__numbat_shell_command_candidates" + maxShellCommandBytes = 256 << 10 + maxShellCommands = 64 + maxShellListItems = 512 + maxCommandExpansionDepth = 4 ) type commandDialect uint8 @@ -51,6 +55,7 @@ func prepareActivations(adapter types.Adapter, ev model.Event, needShellCommands } analysis := analyzeEventShellCommandsDetailed(ev) detection[shellCommandsVariable] = shellCommandList(adapter, analysis.commands) + detection[shellCommandCandidatesVariable] = shellCommandCandidateList(adapter, analysis.enforcementCandidates) return sequenceActivations{ detection: detection, shellUsable: analysis.usable, @@ -91,18 +96,24 @@ func shellCommandList(adapter types.Adapter, commands []ShellCommand) ref.Val { return types.NewRefValList(adapter, values) } -// SequenceActivations contains the CEL activation and command-analysis status -// shared by every sequence step for one event. +func shellCommandCandidateList(adapter types.Adapter, candidates [][]ShellCommand) ref.Val { + values := make([]ref.Val, len(candidates)) + for i, commands := range candidates { + values[i] = shellCommandList(adapter, commands) + } + return types.NewRefValList(adapter, values) +} + +// SequenceActivations contains the prepared CEL values shared by every +// sequence step for one event. Its representation stays private so sequence +// callers cannot accidentally select detection or enforcement inputs. type SequenceActivations struct { - Detection map[string]any - ShellUsable bool - ShellEnforcementSafe bool - Err error + prepared sequenceActivations } // PrepareSequenceActivations builds the command view shared by every sequence // step for an event. -func PrepareSequenceActivations(ev model.Event, rules []*SequenceRule) SequenceActivations { +func PrepareSequenceActivations(ev model.Event, rules []*SequenceRule) (SequenceActivations, error) { var adapter types.Adapter = types.DefaultTypeAdapter if len(rules) > 0 { adapter = rules[0].adapter @@ -110,21 +121,11 @@ func PrepareSequenceActivations(ev model.Event, rules []*SequenceRule) SequenceA for _, r := range rules { if r.usesShellCommands { prepared := prepareActivations(adapter, ev, true) - return SequenceActivations{ - Detection: prepared.detection, - ShellUsable: prepared.shellUsable, - ShellEnforcementSafe: prepared.shellEnforcementSafe, - Err: prepared.err, - } + return SequenceActivations{prepared: prepared}, prepared.err } } prepared := prepareActivations(adapter, ev, false) - return SequenceActivations{ - Detection: prepared.detection, - ShellUsable: prepared.shellUsable, - ShellEnforcementSafe: prepared.shellEnforcementSafe, - Err: prepared.err, - } + return SequenceActivations{prepared: prepared}, prepared.err } type fatalShellAnalysisError struct { @@ -149,14 +150,18 @@ type shellAnalyzer struct { enforcementUnsafe bool statementCounter int64 pipelineCounter int64 + unsafePipelines map[int64]bool + unsafeStatements map[int64]bool + statementParents map[int64]int64 halt bool } type shellAnalysis struct { - commands []ShellCommand - usable bool - enforcementSafe bool - err error + commands []ShellCommand + enforcementCandidates [][]ShellCommand + usable bool + enforcementSafe bool + err error } func analyzeShellCommands(source string) ([]ShellCommand, bool, error) { @@ -169,7 +174,7 @@ func analyzeEventShellCommands(ev model.Event) ([]ShellCommand, bool, error) { } func analyzeEventShellCommandsDetailed(ev model.Event) shellAnalysis { - return analyzeShellCommandsDetailed(ev.Command, commandDialectHint(ev.ToolName)) + return analyzeShellCommandsDetailed(ev.Command, commandDialectHint(ev)) } func analyzeShellCommandsAs(source string, dialect commandDialect) ([]ShellCommand, bool, error) { @@ -181,7 +186,11 @@ func analyzeShellCommandsDetailed(source string, dialect commandDialect) shellAn if strings.TrimSpace(source) == "" { return shellAnalysis{usable: true, enforcementSafe: true} } - a := shellAnalyzer{} + a := shellAnalyzer{ + unsafePipelines: make(map[int64]bool), + unsafeStatements: make(map[int64]bool), + statementParents: make(map[int64]int64), + } a.parseDialect(source, dialect, 0, nil) err := errors.Join(a.issues...) enforcementSafe := !a.enforcementUnsafe && len(a.commands) > 0 @@ -193,28 +202,40 @@ func analyzeShellCommandsDetailed(source string, dialect commandDialect) shellAn } } } + var candidates [][]ShellCommand + if !a.halt { + candidates = posixEnforcementCandidates(a.commands, a.unsafePipelines, a.unsafeStatements, a.statementParents) + } return shellAnalysis{ - commands: a.commands, - usable: err == nil || len(a.commands) > 0, - enforcementSafe: enforcementSafe, - err: err, + commands: a.commands, + enforcementCandidates: candidates, + usable: err == nil || len(a.commands) > 0, + enforcementSafe: enforcementSafe, + err: err, } } -func commandDialectHint(toolName string) commandDialect { - switch commandProgram(strings.TrimSpace(toolName)) { +func commandDialectHint(ev model.Event) commandDialect { + switch commandProgram(strings.TrimSpace(ev.ToolName)) { case "bash", "sh", "zsh", "dash", "ksh", "mksh": return dialectPOSIX case "powershell", "pwsh": return dialectPowerShell case "cmd": return dialectCMD - default: - return dialectAuto + case "exec_command": + if ev.SourceAgent == model.AgentCodex && runtime.GOOS != "windows" { + return dialectPOSIX + } } + return dialectAuto } func (a *shellAnalyzer) parseDialect(source string, dialect commandDialect, depth int, wrappers []ShellWrapper) { + a.parseDialectUnderRedirects(source, dialect, depth, wrappers, 0, nil) +} + +func (a *shellAnalyzer) parseDialectUnderRedirects(source string, dialect commandDialect, depth int, wrappers []ShellWrapper, parent int64, inheritedRedirects []*syntax.Redirect) { if a.halt { return } @@ -252,19 +273,34 @@ func (a *shellAnalyzer) parseDialect(source string, dialect commandDialect, dept return } + commandStart := len(a.commands) file, err := parseShell(source) if err != nil { - a.reportFatal(errors.New("shell command analysis: unsupported or malformed syntax")) - return + a.enforcementUnsafe = true + parseErr := errors.New("shell command analysis: unsupported or malformed syntax") + if file == nil || len(file.Stmts) == 0 { + a.reportFatal(parseErr) + return + } + a.report(parseErr) } if !posixEnforcementShapeSafe(file) { a.enforcementUnsafe = true } - a.walk(source, file, depth, make(map[string]*syntax.Stmt), make(map[string]bool), wrappers) + a.walk(source, file, depth, make(map[string]*syntax.Stmt), make(map[string]bool), wrappers, parent, inheritedRedirects) + if err != nil { + a.markCommandsUnsafe(commandStart) + } } -func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functions map[string]*syntax.Stmt, activeFunctions map[string]bool, wrappers []ShellWrapper) { - relations := a.buildPOSIXRelations(root) +func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functions map[string]*syntax.Stmt, activeFunctions map[string]bool, wrappers []ShellWrapper, parent int64, inheritedRedirects []*syntax.Redirect) { + relations := a.buildPOSIXRelations(root, inheritedRedirects) + for statement, id := range relations.statements { + if relations.parents[statement] == 0 { + relations.parents[statement] = parent + } + a.statementParents[id] = relations.parents[statement] + } syntax.Walk(root, func(node syntax.Node) bool { if a.halt { return false @@ -280,7 +316,17 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio return false case *syntax.Stmt: ctx := relations.context(node) + statementStart := len(a.commands) + for _, redirect := range node.Redirs { + if redirect.Hdoc != nil { + a.markPipelineUnsafe(ctx) + } + } if declaration, ok := node.Cmd.(*syntax.DeclClause); ok { + if ctx.pipelineID != 0 { + a.markStatementsUnsafe(node, ctx.statementIDs) + a.markPipelineUnsafe(ctx) + } command, add, err := projectPOSIXDeclaration(source, declaration, node.Redirs, wrappers, ctx) if err != nil { a.report(err) @@ -293,27 +339,45 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio } call, ok := node.Cmd.(*syntax.CallExpr) if !ok { - if node.Cmd == nil && len(node.Redirs) > 0 { - command, add, err := projectPOSIXCommand(source, nil, nil, node.Redirs, wrappers, ctx) + if len(node.Redirs) > 0 { + redirectCommand, add, err := projectPOSIXCommand(source, nil, nil, node.Redirs, wrappers, ctx) if err != nil { a.report(err) + if ctx.pipelineID != 0 { + a.markStatementsUnsafe(node, ctx.statementIDs) + } + a.markPipelineUnsafe(ctx) return true } - if add { - return a.add(command) + if node.Cmd == nil && add { + return a.add(redirectCommand) + } + } + if node.Cmd != nil { + if ctx.pipelineID != 0 { + a.markStatementsUnsafe(node, ctx.statementIDs) } + a.markPipelineUnsafe(ctx) } return true } command, add, err := projectPOSIXCommand(source, call.Args, call.Assigns, node.Redirs, wrappers, ctx) if err != nil { a.report(err) - return true + if ctx.pipelineID != 0 { + a.unsafeStatements[ctx.statementID] = true + } + a.markPipelineUnsafe(ctx) + if command.Executable == "" { + return true + } } if name, ok := commandName(call.Args); ok && functions[name] != nil { command.FunctionCall = true command.Recursive = activeFunctions[name] } + invocation := inspectShellInvocation(call.Args) + command.enforcementUnsafe = invocation.noExec if add && !a.add(command) { return false } @@ -321,6 +385,7 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio args := call.Args commandWrappers := cloneWrappers(wrappers) allowShellBuiltins := !command.FunctionCall + wrapperSafe := true for !command.FunctionCall { wrapperName, _ := commandName(args) inner, enforcementSafe := unwrapCommand(args) @@ -328,16 +393,16 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio break } if !enforcementSafe { - a.enforcementUnsafe = true + wrapperSafe = false } wrapperProgram := commandProgram(wrapperName) if !allowShellBuiltins && (wrapperProgram == "command" || wrapperProgram == "exec") { - a.enforcementUnsafe = true + wrapperSafe = false } // Basename projection aids detection but cannot prove that a // path-qualified program implements the wrapper's semantics. if wrapperName != commandProgram(wrapperName) { - a.enforcementUnsafe = true + wrapperSafe = false } wrapper, err := wrapperProjection(source, args, inner) if err != nil { @@ -348,7 +413,7 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio childName, _ := commandName(inner) switch commandProgram(childName) { case "command", "exec": - a.enforcementUnsafe = true + wrapperSafe = false } } commandWrappers = append(commandWrappers, wrapper) @@ -357,42 +422,53 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio innerCommand, add, err := projectPOSIXCommand(source, args, call.Assigns, node.Redirs, commandWrappers, ctx) if err != nil { a.report(err) + invocation = inspectShellInvocation(args) break } + invocation = inspectShellInvocation(args) + innerCommand.enforcementUnsafe = !wrapperSafe || invocation.noExec if add && !a.add(innerCommand) { return false } } + redirects := append(relations.inheritedRedirects[node], node.Redirs...) if !command.FunctionCall { if script, dialect, wrapper, ok, err := wrapperScript(source, args); err != nil { a.report(err) } else if ok { - a.enforcementUnsafe = true innerWrappers := append(cloneWrappers(commandWrappers), wrapper) - a.parseDialect(script, dialect, depth+1, innerWrappers) + a.parseDialectUnderRedirects(script, dialect, depth+1, innerWrappers, ctx.statementID, redirects) + a.markCommandsUnsafe(statementStart) } - if script, ok := interpreterHeredoc(args, node.Redirs); ok { + if scripts := interpreterHeredocs(invocation.inputFDs, redirects); len(scripts) > 0 { wrapper, err := projectInterpreterWrapper(source, args) if err != nil { a.report(err) } else { innerWrappers := append(cloneWrappers(commandWrappers), wrapper) - a.parseDialect(script, dialectPOSIX, depth+1, innerWrappers) + a.markCommandsUnsafe(statementStart) + for _, script := range scripts { + innerStart := len(a.commands) + a.parseDialectUnderRedirects(script, dialectPOSIX, depth+1, innerWrappers, ctx.statementID, nil) + if ctx.pipelineID != 0 || !wrapperSafe { + a.markCommandsUnsafe(innerStart) + } + } } } } if allowShellBuiltins { if script, ok := evalScript(source, args); ok { - a.enforcementUnsafe = true - a.parseDialect(script, dialectPOSIX, depth+1, commandWrappers) + a.parseDialectUnderRedirects(script, dialectPOSIX, depth+1, commandWrappers, ctx.statementID, redirects) + a.markCommandsUnsafe(statementStart) } } if command.FunctionCall { if name, ok := commandName(call.Args); ok { if body := functions[name]; body != nil && depth < maxCommandExpansionDepth && !activeFunctions[name] { activeFunctions[name] = true - a.walk(source, body, depth+1, functions, activeFunctions, commandWrappers) + a.walk(source, body, depth+1, functions, activeFunctions, commandWrappers, ctx.statementID, redirects) delete(activeFunctions, name) } } @@ -772,10 +848,10 @@ func wrapperScript(source string, args []*syntax.Word) (string, commandDialect, if !ok { return "", dialectAuto, ShellWrapper{}, false, errors.New("shell command analysis: dynamic interpreter option") } - if flag == "--" || flag == "-" || !strings.HasPrefix(flag, "-") { + if flag == "--" || len(flag) < 2 || flag[0] != '-' && flag[0] != '+' { return "", dialectAuto, ShellWrapper{}, false, nil } - if flag == "-o" || flag == "-O" || flag == "--rcfile" || flag == "--init-file" { + if flag == "-o" || flag == "+o" || flag == "-O" || flag == "+O" || flag == "--rcfile" || flag == "--init-file" { i++ continue } @@ -838,10 +914,10 @@ func projectedInterpreterScript(command ShellCommand) (string, commandDialect, i if flag.Expands { return "", dialectAuto, 0, false, errors.New("shell command analysis: dynamic interpreter option") } - if flag.Value == "--" || flag.Value == "-" || !strings.HasPrefix(flag.Value, "-") { + if flag.Value == "--" || len(flag.Value) < 2 || flag.Value[0] != '-' && flag.Value[0] != '+' { return "", dialectAuto, 0, false, nil } - if flag.Value == "-o" || flag.Value == "-O" || flag.Value == "--rcfile" || flag.Value == "--init-file" { + if flag.Value == "-o" || flag.Value == "+o" || flag.Value == "-O" || flag.Value == "+O" || flag.Value == "--rcfile" || flag.Value == "--init-file" { i++ continue } @@ -964,67 +1040,259 @@ func joinProjectedScript(args []ShellArgument) (string, bool) { return strings.Join(values, " "), true } -func interpreterHeredoc(args []*syntax.Word, redirects []*syntax.Redirect) (string, bool) { - if !shellReadsStdin(args) { - return "", false +func interpreterHeredocs(fds []int64, redirects []*syntax.Redirect) []string { + var scripts []string + var seen *syntax.Redirect + for _, fd := range fds { + if script, redirect, ok := heredocForFD(redirects, fd); ok && redirect != seen { + scripts = append(scripts, script) + seen = redirect + } } + return scripts +} + +func heredocForFD(redirects []*syntax.Redirect, fd int64) (string, *syntax.Redirect, bool) { for i := len(redirects) - 1; i >= 0; i-- { redirect := redirects[i] - fd := defaultRedirectFD(redirect.Op) - if redirect.N != nil { - if redirect.N.Value != "0" { + redirectFD, valid := posixRedirectFD(redirect) + if !valid { + continue + } + if redirect.Op == syntax.DplIn || redirect.Op == syntax.DplOut { + target, static := staticWord(redirect.Word) + if !static { + return "", nil, false + } + if target == "-" { + if redirectFD == fd { + return "", nil, false + } continue } - fd = 0 + moved := strings.HasSuffix(target, "-") + sourceFD, valid := parseShellFD(strings.TrimSuffix(target, "-")) + if !valid { + return "", nil, false + } + if moved && sourceFD == fd && redirectFD != fd { + return "", nil, false + } + if redirectFD == fd { + fd = sourceFD + } + continue } - if fd != 0 { + if redirectFD != fd { continue } - if (redirect.Op == syntax.Hdoc || redirect.Op == syntax.DashHdoc || redirect.Op == syntax.WordHdoc) && - redirect.Hdoc != nil { - return staticWord(redirect.Hdoc) + if (redirect.Op == syntax.Hdoc || redirect.Op == syntax.DashHdoc) && redirect.Hdoc != nil { + script, ok := staticWord(redirect.Hdoc) + return script, redirect, ok } - return "", false + if redirect.Op == syntax.WordHdoc { + script, ok := staticWord(redirect.Word) + return script, redirect, ok + } + return "", nil, false } - return "", false + return "", nil, false +} + +type shellInvocation struct { + inputFDs []int64 + noExec bool } -func shellReadsStdin(args []*syntax.Word) bool { +func inspectShellInvocation(args []*syntax.Word) shellInvocation { name, ok := commandName(args) - if !ok || !isShellInterpreter(commandProgram(name)) { - return false - } - stdin := false + if !ok { + return shellInvocation{} + } + program := commandProgram(name) + if !isShellInterpreter(program) { + return shellInvocation{} + } + var ( + noExec, terminalNoExec bool + interactive, stdin bool + startupFD int64 + startupFound, startupDisabled bool + inputFD int64 + inputFound = true + bashShortOption bool + shOptionLetters bool + ) for i := 1; i < len(args); i++ { - flag, ok := staticWord(args[i]) - if !ok { - return false + flag, static := staticWord(args[i]) + if !static { + inputFound = false + break + } + if flag == "--" || flag == "-" || program == "zsh" && (flag == "+" || flag == "+-" || !shOptionLetters && (flag == "-b" || flag == "+b")) { + if !stdin && i+1 < len(args) { + script, static := staticWord(args[i+1]) + inputFD, inputFound = shellInputPathFD(script) + if !static { + inputFound = false + } + } + break } - if flag == "--" { - return stdin || i+1 == len(args) + if len(flag) < 2 || flag[0] != '-' && flag[0] != '+' { + if !stdin { + inputFD, inputFound = shellInputPathFD(flag) + } + break } - if flag == "-o" || flag == "-O" || flag == "--rcfile" || flag == "--init-file" { + if program == "bash" && strings.HasPrefix(flag, "--") && bashShortOption { + terminalNoExec = true + inputFound = false + break + } + if flag == "--help" || flag == "--version" || program == "bash" && (flag == "--dump-strings" || flag == "--dump-po-strings") { + terminalNoExec = true + continue + } + if program == "bash" && flag == "--pretty-print" { + terminalNoExec = true + continue + } + if program == "zsh" && strings.HasPrefix(flag, "+-") && applyZshOption(flag[2:], false, &noExec, &stdin, &shOptionLetters) { + continue + } + longOption := strings.TrimPrefix(flag, "--") + if program == "zsh" { + longOption = normalizedZshOption(longOption) + if applyZshOption(longOption, true, &noExec, &stdin, &shOptionLetters) { + continue + } + } + if longOption == "noexec" { + noExec = true + continue + } + if program == "zsh" && len(flag) > 2 && (flag[:2] == "-o" || flag[:2] == "+o") { + applyZshOption(flag[2:], flag[0] == '-', &noExec, &stdin, &shOptionLetters) + continue + } + if program == "bash" && !strings.HasPrefix(flag, "--") { + bashShortOption = true + } + if flag == "-o" || flag == "+o" { i++ + if i < len(args) { + option, static := staticWord(args[i]) + if static && program == "zsh" { + applyZshOption(option, flag[0] == '-', &noExec, &stdin, &shOptionLetters) + } else if static && option == "noexec" { + noExec = flag[0] == '-' + } + } continue } - if isShellCommandFlag(flag) { - return false + if flag == "-O" || flag == "+O" { + i++ + continue + } + if program == "bash" && flag == "--norc" { + startupDisabled = true + startupFound = false + continue + } + if flag == "--rcfile" || flag == "--init-file" { + i++ + if program == "bash" && !startupDisabled && i < len(args) { + startupPath, static := staticWord(args[i]) + startupFD, startupFound = shellInputPathFD(startupPath) + if !static { + startupFound = false + } + } + continue } - if flag == "-" { - stdin = true + if strings.HasPrefix(flag, "--") { continue } - if !strings.HasPrefix(flag, "-") { - return stdin + if strings.ContainsRune(flag[1:], 'i') { + interactive = flag[0] == '-' + } + if strings.ContainsRune(flag[1:], 'n') { + noExec = flag[0] == '-' + } + if program == "bash" && strings.ContainsRune(flag[1:], 'D') { + terminalNoExec = true } - if len(flag) > 1 && flag[0] == '-' && !strings.HasPrefix(flag, "--") && - strings.ContainsRune(flag[1:], 's') { - stdin = true + if flag[0] == '-' && strings.ContainsRune(flag[1:], 'c') { + inputFound = false + break } + if strings.ContainsRune(flag[1:], 's') { + stdin = flag[0] == '-' + } + } + result := shellInvocation{noExec: noExec || terminalNoExec} + if result.noExec { + return result + } + if program == "bash" && interactive && startupFound { + result.inputFDs = append(result.inputFDs, startupFD) + } + if inputFound && (len(result.inputFDs) == 0 || result.inputFDs[0] != inputFD) { + result.inputFDs = append(result.inputFDs, inputFD) + } + return result +} + +func normalizedZshOption(option string) string { + return strings.ToLower(strings.NewReplacer("-", "", "_", "").Replace(option)) +} + +func applyZshOption(option string, enabled bool, noExec, stdin, shOptionLetters *bool) bool { + switch normalizedZshOption(option) { + case "noexec": + *noExec = enabled + case "exec": + *noExec = !enabled + case "stdin", "shinstdin": + *stdin = enabled + case "nostdin", "noshinstdin": + *stdin = !enabled + case "shoptionletters": + *shOptionLetters = enabled + case "noshoptionletters": + *shOptionLetters = !enabled + default: + return false } return true } +func shellInputPathFD(sourcePath string) (int64, bool) { + sourcePath = path.Clean(sourcePath) + if sourcePath == "/dev/stdin" { + return 0, true + } + for _, prefix := range []string{"/dev/fd/", "/proc/self/fd/"} { + if strings.HasPrefix(sourcePath, prefix) { + return parseShellFD(strings.TrimPrefix(sourcePath, prefix)) + } + } + return 0, false +} + +func parseShellFD(value string) (int64, bool) { + fd, err := strconv.ParseUint(value, 10, 63) + return int64(fd), err == nil +} + +func posixRedirectFD(redirect *syntax.Redirect) (int64, bool) { + if redirect.N == nil { + return defaultRedirectFD(redirect.Op), true + } + return parseShellFD(redirect.N.Value) +} + func evalScript(source string, args []*syntax.Word) (string, bool) { name, ok := commandName(args) if !ok || commandProgram(name) != "eval" || len(args) < 2 { diff --git a/internal/rule/shell_enforcement.go b/internal/rule/shell_enforcement.go index 098de3e..fe0a95e 100644 --- a/internal/rule/shell_enforcement.go +++ b/internal/rule/shell_enforcement.go @@ -6,6 +6,86 @@ import ( "mvdan.cc/sh/v3/syntax" ) +func posixEnforcementCandidates(commands []ShellCommand, unsafePipelines, unsafeStatements map[int64]bool, parents map[int64]int64) [][]ShellCommand { + type candidateKey struct { + pipeline bool + id int64 + } + groups := make(map[candidateKey][]ShellCommand) + unsafeGroups := make(map[candidateKey]bool) + for _, command := range commands { + if command.PipelineID != 0 && !commandSafeForCandidate(command) { + unsafePipelines[command.PipelineID] = true + } + } + for _, command := range commands { + if unsafePipelines[command.PipelineID] { + unsafeStatements[command.StatementID] = true + } + } + order := make([]candidateKey, 0, len(commands)) + for _, command := range commands { + if command.Dialect != dialectPOSIX.String() || command.StatementID == 0 || unsafeStatements[command.StatementID] { + continue + } + unsafeParent := false + for parent := command.ParentStatementID; parent != 0; parent = parents[parent] { + if unsafeStatements[parent] { + unsafeParent = true + break + } + } + if unsafeParent { + continue + } + key := candidateKey{id: command.StatementID} + if command.PipelineID != 0 { + key = candidateKey{pipeline: true, id: command.PipelineID} + } + if _, exists := groups[key]; !exists { + order = append(order, key) + } + if commandSafeForCandidate(command) { + groups[key] = append(groups[key], command) + } else { + unsafeGroups[key] = true + } + } + candidates := make([][]ShellCommand, 0, len(order)) + for _, key := range order { + if !unsafeGroups[key] { + candidates = append(candidates, groups[key]) + } + } + return candidates +} + +func commandSafeForCandidate(command ShellCommand) bool { + return commandSafeForEnforcement(command) +} + +func (a *shellAnalyzer) markCommandsUnsafe(start int) { + for i := start; i < len(a.commands); i++ { + a.commands[i].enforcementUnsafe = true + } +} + +func (a *shellAnalyzer) markStatementsUnsafe(root syntax.Node, statements map[*syntax.Stmt]int64) { + syntax.Walk(root, func(node syntax.Node) bool { + if statement, ok := node.(*syntax.Stmt); ok { + a.unsafeStatements[statements[statement]] = true + } + return true + }) +} + +func (a *shellAnalyzer) markPipelineUnsafe(ctx posixCommandContext) { + if ctx.pipelineID == 0 { + return + } + a.unsafePipelines[ctx.pipelineID] = true +} + func posixEnforcementShapeSafe(file *syntax.File) bool { return file != nil && len(file.Stmts) == 1 && posixStatementEnforcementSafe(file.Stmts[0]) } diff --git a/internal/rule/shell_relations.go b/internal/rule/shell_relations.go index 2133d87..9981bf2 100644 --- a/internal/rule/shell_relations.go +++ b/internal/rule/shell_relations.go @@ -14,24 +14,19 @@ const ( ) type posixRelations struct { - statements map[*syntax.Stmt]int64 - pipelines map[*syntax.Stmt]int64 - parents map[*syntax.Stmt]int64 + statements map[*syntax.Stmt]int64 + pipelines map[*syntax.Stmt]int64 + parents map[*syntax.Stmt]int64 + inheritedRedirects map[*syntax.Stmt][]*syntax.Redirect } -func (a *shellAnalyzer) buildPOSIXRelations(root syntax.Node) posixRelations { +func (a *shellAnalyzer) buildPOSIXRelations(root syntax.Node, inheritedRedirects []*syntax.Redirect) posixRelations { relations := posixRelations{ - statements: make(map[*syntax.Stmt]int64), - pipelines: make(map[*syntax.Stmt]int64), - parents: make(map[*syntax.Stmt]int64), + statements: make(map[*syntax.Stmt]int64), + pipelines: make(map[*syntax.Stmt]int64), + parents: make(map[*syntax.Stmt]int64), + inheritedRedirects: make(map[*syntax.Stmt][]*syntax.Redirect), } - syntax.Walk(root, func(node syntax.Node) bool { - if stmt, ok := node.(*syntax.Stmt); ok { - relations.statements[stmt] = a.nextStatement() - } - return true - }) - syntax.Walk(root, func(node syntax.Node) bool { binary, ok := node.(*syntax.BinaryCmd) if !ok || binary.Op != syntax.Pipe && binary.Op != syntax.PipeAll { @@ -63,7 +58,10 @@ func (a *shellAnalyzer) buildPOSIXRelations(root syntax.Node) posixRelations { return true } if stmt, ok := node.(*syntax.Stmt); ok { + relations.statements[stmt] = a.nextStatement() relations.parents[stmt] = enclosingSubcommandStatement(stack, relations.statements) + redirects := append([]*syntax.Redirect(nil), inheritedRedirects...) + relations.inheritedRedirects[stmt] = append(redirects, enclosingRedirects(stack)...) } stack = append(stack, node) return true @@ -71,6 +69,38 @@ func (a *shellAnalyzer) buildPOSIXRelations(root syntax.Node) posixRelations { return relations } +func enclosingRedirects(stack []syntax.Node) []*syntax.Redirect { + var redirects []*syntax.Redirect + for _, node := range stack { + switch node := node.(type) { + case *syntax.ProcSubst: + if node.Op == syntax.CmdOut { + redirects = redirectsExceptFD(redirects, 0) + } + case *syntax.Stmt: + switch node.Cmd.(type) { + case *syntax.Block, *syntax.Subshell, *syntax.IfClause, *syntax.WhileClause, *syntax.ForClause, *syntax.CaseClause: + redirects = append(redirects, node.Redirs...) + } + } + } + return redirects +} + +func redirectsExceptFD(redirects []*syntax.Redirect, excluded int64) []*syntax.Redirect { + kept := make([]*syntax.Redirect, 0, len(redirects)) + for _, redirect := range redirects { + fd, ok := posixRedirectFD(redirect) + if !ok { + continue + } + if fd != excluded && fd != -1 { + kept = append(kept, redirect) + } + } + return kept +} + func (r posixRelations) context(stmt *syntax.Stmt) posixCommandContext { return posixCommandContext{ statementID: r.statements[stmt], diff --git a/internal/rule/shell_types.go b/internal/rule/shell_types.go index 6d5a31b..63c250a 100644 --- a/internal/rule/shell_types.go +++ b/internal/rule/shell_types.go @@ -133,7 +133,7 @@ func projectPOSIXCommand(source string, args []*syntax.Word, assignments []*synt for _, redirect := range redirects { projected, err := projectPOSIXRedirect(source, redirect, ctx.statementIDs) if err != nil { - return ShellCommand{}, false, err + return command, false, err } command.Redirects = append(command.Redirects, projected) } diff --git a/internal/sequence/sequence.go b/internal/sequence/sequence.go index cae7007..90ad285 100644 --- a/internal/sequence/sequence.go +++ b/internal/sequence/sequence.go @@ -327,28 +327,20 @@ func project(rules []*rule.SequenceRule, ev model.Event, seq uint64) (entry, []e } e.ts, e.tsOK = parseTimestamp(ev.Timestamp) var errs []error - activations := rule.PrepareSequenceActivations(ev, rules) - if activations.Err != nil { - errs = append(errs, activations.Err) + activations, activationErr := rule.PrepareSequenceActivations(ev, rules) + if activationErr != nil { + errs = append(errs, activationErr) } for ri, r := range rules { for si := 0; si < r.StepCount(); si++ { - if activations.Err != nil && r.StepUsesShellCommands(si) && !activations.ShellUsable { - continue - } - ok, evalErr := r.EvalStep(si, activations.Detection) + evaluation, evalErr := r.EvalStep(si, activations) if evalErr != nil { errs = append(errs, evalErr) - continue } - if !ok { - continue - } - e.masks[ri] |= 1 << si - if !r.Rule().IsEnforceEligible() { - continue + if evaluation.Match { + e.masks[ri] |= 1 << si } - if r.StepUsesShellCommands(si) && !activations.ShellEnforcementSafe { + if !evaluation.EnforcementMatch { continue } e.enforcementMasks[ri] |= 1 << si diff --git a/internal/sequence/sequence_test.go b/internal/sequence/sequence_test.go index 80266e4..78223f1 100644 --- a/internal/sequence/sequence_test.go +++ b/internal/sequence/sequence_test.go @@ -135,6 +135,34 @@ func TestSequenceStepUsesShellCommands(t *testing.T) { } } +func TestSequenceFinalStepCanMatchOneCompoundCandidate(t *testing.T) { + enforce := true + r := secretThenEgress(func(spec *rule.SequenceSpec) { + spec.Steps[0].Expr = `shell_commands.exists(command, command.name == "prep")` + spec.Steps[1].Expr = `shell_commands.filter(command, command.name == "cat").size() == 1 && + shell_commands[0].argv[1] == ".env"` + }) + r.Enforce = &enforce + tr := NewTracker(compile(t, r), DefaultConfig()) + + prep := ev("e1", "2026-06-01T10:00:00Z", model.EventCommandExec, func(e *model.Event) { + e.Command = "prep" + }) + if observation, err := tr.Observe(prep); err != nil || len(observation.Findings) != 0 { + t.Fatalf("prep observation = %+v, %v", observation, err) + } + compound := ev("e2", "2026-06-01T10:01:00Z", model.EventCommandExec, func(e *model.Event) { + e.Command = "true; cat .env" + }) + observation, err := tr.Observe(compound) + if err == nil { + t.Fatal("compound observation returned no aggregate evaluation error") + } + if len(observation.Findings) != 1 || len(observation.EnforcementRules) != 1 { + t.Fatalf("compound observation = %+v, want one finding and one enforcement rule", observation) + } +} + func TestSequenceShellAnalysisErrorIsReported(t *testing.T) { r := secretThenEgress(func(spec *rule.SequenceSpec) { spec.Steps[0].Expr = `shell_commands.size() == 0` diff --git a/internal/sequence/store.go b/internal/sequence/store.go index e5b2dae..e995cca 100644 --- a/internal/sequence/store.go +++ b/internal/sequence/store.go @@ -53,7 +53,7 @@ const ( maxStoredSessionBytes = 16 << 20 // Bump sequenceProjectionRevision when projection or enforcement filtering // changes the meaning of persisted verdict masks. - sequenceProjectionRevision = "sequence-projection-v4" + sequenceProjectionRevision = "sequence-projection-v5" ) // NewStore binds a window store for one compiled rule set onto the shared diff --git a/internal/sequence/store_test.go b/internal/sequence/store_test.go index 7370279..47c4e79 100644 --- a/internal/sequence/store_test.go +++ b/internal/sequence/store_test.go @@ -303,7 +303,7 @@ func TestStoreProjectionRevisionInvalidatesEnforcementMasks(t *testing.T) { defer db.Close() st := newStore(t, db, rules, DefaultConfig()) raw, err := json.Marshal(storedSession{ - RulesHash: fingerprintForProjection(rules, "sequence-projection-v3"), + RulesHash: fingerprintForProjection(rules, "sequence-projection-v4"), NextSeq: 1, Entries: []storedEntry{{ Seq: 0, From 76a4bf8e3c67329ff2a144daac395731a301889c Mon Sep 17 00:00:00 2001 From: ronheichman <294254458+ronheichman@users.noreply.github.com> Date: Sat, 29 Aug 2026 00:23:52 +0000 Subject: [PATCH 2/4] fix(rule): reject invalid interpreter options --- internal/rule/engine.go | 10 ----- .../rule/multicommand_enforcement_test.go | 39 ++++++++++------ internal/rule/shell.go | 45 ++++++++++++------- internal/rule/shell_enforcement.go | 8 +--- 4 files changed, 57 insertions(+), 45 deletions(-) diff --git a/internal/rule/engine.go b/internal/rule/engine.go index 9ab3327..0655908 100644 --- a/internal/rule/engine.go +++ b/internal/rule/engine.go @@ -81,17 +81,11 @@ func (s *SequenceRule) WithinEvents() int { return s.withinEvents } // MaxMatches returns the per-(rule, session) finding cap, always >= 1. func (s *SequenceRule) MaxMatches() int { return s.maxMatches } -// StepEvaluation reports both the detection and enforcement verdict for one -// sequence step. Enforcement may match a safe command candidate even when the -// complete compound command does not match or produces an evaluation error. type StepEvaluation struct { Match bool EnforcementMatch bool } -// EvalStep evaluates one step against the prepared event views. Candidate -// selection and fail-closed shell-analysis handling stay inside the rule -// module so sequence tracking cannot accidentally authorize the wrong view. func (s *SequenceRule) EvalStep(i int, activations SequenceActivations) (StepEvaluation, error) { if i < 0 || i >= len(s.steps) { return StepEvaluation{}, fmt.Errorf("rule %q: step index %d out of range", s.rule.ID, i) @@ -431,10 +425,6 @@ func programExpr(env *cel.Env, ast *cel.Ast) (compiledExpression, error) { const shellCandidateTemplate = shellCommandCandidatesVariable + ".exists(" + shellCommandsVariable + ", true)" -// shellCandidateAST wraps an already checked predicate in a CEL exists -// comprehension without rendering and reparsing the authored source. The -// optimizer rechecks the combined tree, resolving shell_commands to the -// comprehension variable while retaining the checked predicate's structure. func shellCandidateAST(env *cel.Env, predicate *cel.Ast) (*cel.Ast, error) { template, issues := env.Compile(shellCandidateTemplate) if issues != nil && issues.Err() != nil { diff --git a/internal/rule/multicommand_enforcement_test.go b/internal/rule/multicommand_enforcement_test.go index 1a84db7..877531c 100644 --- a/internal/rule/multicommand_enforcement_test.go +++ b/internal/rule/multicommand_enforcement_test.go @@ -1,6 +1,7 @@ package rule import ( + "runtime" "testing" "github.com/perplexityai/numbat/internal/model" @@ -113,10 +114,13 @@ func TestMultiCommandEnforcementRegression(t *testing.T) { func TestMultiCommandEnforcementUsesPOSIXParserForExecCommand(t *testing.T) { eng := compoundRuleEngine(t, `shell_commands.exists(command, command.name == "cat" && command.argv.exists(arg, arg == ".env"))`) - for _, command := range []string{ + commands := []string{ "if (true)\nthen\ncat .env\nfi", - "get-process; cat .env", - } { + } + if runtime.GOOS != "windows" { + commands = append(commands, "get-process; cat .env") + } + for _, command := range commands { matches, err := eng.Eval(model.Event{ SourceAgent: model.AgentCodex, EventType: model.EventCommandExec, @@ -278,19 +282,26 @@ func TestMultiCommandEnforcementParsesSharedInterpreterInputOnce(t *testing.T) { } } -func TestMultiCommandEnforcementDoesNotProjectInvalidBashInvocation(t *testing.T) { +func TestMultiCommandEnforcementDoesNotProjectInvalidInterpreterInvocation(t *testing.T) { eng := compoundRuleEngine(t, `shell_commands.exists(command, command.name == "cat" && command.argv.exists(arg, arg == ".env"))`) - matches, err := eng.Eval(model.Event{ - EventType: model.EventCommandExec, - ToolName: "bash", - Command: "bash -i --rcfile /dev/fd/3 -c exit 3<<'EOF'\ncat .env\nEOF", - }) - if err != nil { - t.Fatal(err) - } - if len(matches) != 0 { - t.Fatalf("Eval returned %+v, want no nested projection from an invalid Bash invocation", matches) + for _, command := range []string{ + "bash -i --rcfile /dev/fd/3 -c exit 3<<'EOF'\ncat .env\nEOF", + "bash --invalid-option <<'EOF'\ncat .env\nEOF", + "bash -Z <<'EOF'\ncat .env\nEOF", + "zsh --invalid-option <<'EOF'\ncat .env\nEOF", + } { + matches, err := eng.Eval(model.Event{ + EventType: model.EventCommandExec, + ToolName: "bash", + Command: command, + }) + if err != nil { + t.Fatal(err) + } + if len(matches) != 0 { + t.Fatalf("Eval(%q) returned %+v, want no nested projection from an invalid interpreter invocation", command, matches) + } } } diff --git a/internal/rule/shell.go b/internal/rule/shell.go index 2f422b2..8194b71 100644 --- a/internal/rule/shell.go +++ b/internal/rule/shell.go @@ -104,9 +104,6 @@ func shellCommandCandidateList(adapter types.Adapter, candidates [][]ShellComman return types.NewRefValList(adapter, values) } -// SequenceActivations contains the prepared CEL values shared by every -// sequence step for one event. Its representation stays private so sequence -// callers cannot accidentally select detection or enforcement inputs. type SequenceActivations struct { prepared sequenceActivations } @@ -273,24 +270,15 @@ func (a *shellAnalyzer) parseDialectUnderRedirects(source string, dialect comman return } - commandStart := len(a.commands) file, err := parseShell(source) if err != nil { - a.enforcementUnsafe = true - parseErr := errors.New("shell command analysis: unsupported or malformed syntax") - if file == nil || len(file.Stmts) == 0 { - a.reportFatal(parseErr) - return - } - a.report(parseErr) + a.reportFatal(errors.New("shell command analysis: unsupported or malformed syntax")) + return } if !posixEnforcementShapeSafe(file) { a.enforcementUnsafe = true } a.walk(source, file, depth, make(map[string]*syntax.Stmt), make(map[string]bool), wrappers, parent, inheritedRedirects) - if err != nil { - a.markCommandsUnsafe(commandStart) - } } func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functions map[string]*syntax.Stmt, activeFunctions map[string]bool, wrappers []ShellWrapper, parent int64, inheritedRedirects []*syntax.Redirect) { @@ -1212,7 +1200,18 @@ func inspectShellInvocation(args []*syntax.Word) shellInvocation { continue } if strings.HasPrefix(flag, "--") { - continue + if program == "bash" { + switch flag { + case "--debugger", "--login", "--noediting", "--noprofile", "--posix", "--restricted", "--verbose": + continue + } + } + inputFound = false + break + } + if !validInterpreterOptionLetters(program, flag) { + inputFound = false + break } if strings.ContainsRune(flag[1:], 'i') { interactive = flag[0] == '-' @@ -1244,6 +1243,22 @@ func inspectShellInvocation(args []*syntax.Word) shellInvocation { return result } +func validInterpreterOptionLetters(program, flag string) bool { + allowed := "abefhkmnptuvxCcis" + switch program { + case "bash": + allowed = "abefhkmnptuvxBCEHPTcdilrsD" + case "zsh": + allowed = "bcdfiklmnoprsuvxX" + } + for _, option := range flag[1:] { + if !strings.ContainsRune(allowed, option) { + return false + } + } + return true +} + func normalizedZshOption(option string) string { return strings.ToLower(strings.NewReplacer("-", "", "_", "").Replace(option)) } diff --git a/internal/rule/shell_enforcement.go b/internal/rule/shell_enforcement.go index fe0a95e..1d9d523 100644 --- a/internal/rule/shell_enforcement.go +++ b/internal/rule/shell_enforcement.go @@ -14,7 +14,7 @@ func posixEnforcementCandidates(commands []ShellCommand, unsafePipelines, unsafe groups := make(map[candidateKey][]ShellCommand) unsafeGroups := make(map[candidateKey]bool) for _, command := range commands { - if command.PipelineID != 0 && !commandSafeForCandidate(command) { + if command.PipelineID != 0 && !commandSafeForEnforcement(command) { unsafePipelines[command.PipelineID] = true } } @@ -45,7 +45,7 @@ func posixEnforcementCandidates(commands []ShellCommand, unsafePipelines, unsafe if _, exists := groups[key]; !exists { order = append(order, key) } - if commandSafeForCandidate(command) { + if commandSafeForEnforcement(command) { groups[key] = append(groups[key], command) } else { unsafeGroups[key] = true @@ -60,10 +60,6 @@ func posixEnforcementCandidates(commands []ShellCommand, unsafePipelines, unsafe return candidates } -func commandSafeForCandidate(command ShellCommand) bool { - return commandSafeForEnforcement(command) -} - func (a *shellAnalyzer) markCommandsUnsafe(start int) { for i := start; i < len(a.commands); i++ { a.commands[i].enforcementUnsafe = true From 0cf03d7c5349aa60209b9f945cfa5a84b761f13a Mon Sep 17 00:00:00 2001 From: ronheichman <294254458+ronheichman@users.noreply.github.com> Date: Sat, 29 Aug 2026 00:38:35 +0000 Subject: [PATCH 3/4] fix(rule): keep unvalidated shell options detection-only --- internal/rule/engine_test.go | 8 --- .../rule/multicommand_enforcement_test.go | 27 ++++++++++ internal/rule/shell.go | 50 ++++++++++++++----- 3 files changed, 64 insertions(+), 21 deletions(-) diff --git a/internal/rule/engine_test.go b/internal/rule/engine_test.go index 8e7b0b1..cd39e03 100644 --- a/internal/rule/engine_test.go +++ b/internal/rule/engine_test.go @@ -105,14 +105,6 @@ func TestEngineContentCostLimit(t *testing.T) { } } -func TestEngineShellRuleAllowsTrailingComment(t *testing.T) { - mustEngine(t, Rule{ - ID: "t.trailing_comment", - Severity: model.SeverityHigh, - Expr: `shell_commands.exists(command, command.name == "cat") // documented intent`, - }) -} - func TestEngineShellCommandsUsesExecutableStatements(t *testing.T) { eng := mustEngine(t, Rule{ ID: "t.scheduler", diff --git a/internal/rule/multicommand_enforcement_test.go b/internal/rule/multicommand_enforcement_test.go index 877531c..4aeea24 100644 --- a/internal/rule/multicommand_enforcement_test.go +++ b/internal/rule/multicommand_enforcement_test.go @@ -289,6 +289,8 @@ func TestMultiCommandEnforcementDoesNotProjectInvalidInterpreterInvocation(t *te "bash -i --rcfile /dev/fd/3 -c exit 3<<'EOF'\ncat .env\nEOF", "bash --invalid-option <<'EOF'\ncat .env\nEOF", "bash -Z <<'EOF'\ncat .env\nEOF", + "bash -o <<'EOF'\ncat .env\nEOF", + "bash -O <<'EOF'\ncat .env\nEOF", "zsh --invalid-option <<'EOF'\ncat .env\nEOF", } { matches, err := eng.Eval(model.Event{ @@ -305,6 +307,31 @@ func TestMultiCommandEnforcementDoesNotProjectInvalidInterpreterInvocation(t *te } } +func TestMultiCommandEnforcementDoesNotEnforceUnvalidatedNamedInterpreterOptions(t *testing.T) { + eng := compoundRuleEngine(t, `shell_commands.exists(command, + command.name == "cat" && command.argv.exists(arg, arg == ".env"))`) + for _, command := range []string{ + "bash -o definitely_invalid <<'EOF'\ncat .env\nEOF", + "bash +o definitely_invalid <<'EOF'\ncat .env\nEOF", + "bash -O definitely_invalid <<'EOF'\ncat .env\nEOF", + "bash -o pipefail <<'EOF'\ncat .env\nEOF", + "bash -O extglob <<'EOF'\ncat .env\nEOF", + "env bash -o definitely_invalid <<'EOF'\ncat .env\nEOF", + } { + matches, err := eng.Eval(model.Event{ + EventType: model.EventCommandExec, + ToolName: "bash", + Command: command, + }) + if err != nil { + t.Fatal(err) + } + if len(matches) != 1 || matches[0].EnforcementMatch { + t.Fatalf("Eval(%q) returned %+v, want one detection-only match", command, matches) + } + } +} + func TestMultiCommandEnforcementDoesNotUseRecoveredCommand(t *testing.T) { eng := compoundRuleEngine(t, `shell_commands.exists(command, command.name == "cat" && command.argv.exists(arg, arg == ".env"))`) diff --git a/internal/rule/shell.go b/internal/rule/shell.go index 8194b71..76d23bd 100644 --- a/internal/rule/shell.go +++ b/internal/rule/shell.go @@ -365,7 +365,7 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio command.Recursive = activeFunctions[name] } invocation := inspectShellInvocation(call.Args) - command.enforcementUnsafe = invocation.noExec + command.enforcementUnsafe = invocation.noExec || invocation.inputEnforcementUnsafe if add && !a.add(command) { return false } @@ -414,7 +414,7 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio break } invocation = inspectShellInvocation(args) - innerCommand.enforcementUnsafe = !wrapperSafe || invocation.noExec + innerCommand.enforcementUnsafe = !wrapperSafe || invocation.noExec || invocation.inputEnforcementUnsafe if add && !a.add(innerCommand) { return false } @@ -439,7 +439,7 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio for _, script := range scripts { innerStart := len(a.commands) a.parseDialectUnderRedirects(script, dialectPOSIX, depth+1, innerWrappers, ctx.statementID, nil) - if ctx.pipelineID != 0 || !wrapperSafe { + if ctx.pipelineID != 0 || !wrapperSafe || invocation.inputEnforcementUnsafe { a.markCommandsUnsafe(innerStart) } } @@ -1088,8 +1088,9 @@ func heredocForFD(redirects []*syntax.Redirect, fd int64) (string, *syntax.Redir } type shellInvocation struct { - inputFDs []int64 - noExec bool + inputFDs []int64 + inputEnforcementUnsafe bool + noExec bool } func inspectShellInvocation(args []*syntax.Word) shellInvocation { @@ -1108,6 +1109,7 @@ func inspectShellInvocation(args []*syntax.Word) shellInvocation { startupFound, startupDisabled bool inputFD int64 inputFound = true + inputEnforcementUnsafe bool bashShortOption bool shOptionLetters bool ) @@ -1169,18 +1171,37 @@ func inspectShellInvocation(args []*syntax.Word) shellInvocation { } if flag == "-o" || flag == "+o" { i++ - if i < len(args) { - option, static := staticWord(args[i]) - if static && program == "zsh" { - applyZshOption(option, flag[0] == '-', &noExec, &stdin, &shOptionLetters) - } else if static && option == "noexec" { - noExec = flag[0] == '-' - } + if i >= len(args) { + inputFound = false + break + } + option, static := staticWord(args[i]) + if !static { + inputFound = false + break + } + known := option == "noexec" + if program == "zsh" { + known = applyZshOption(option, flag[0] == '-', &noExec, &stdin, &shOptionLetters) + } else if known { + noExec = flag[0] == '-' + } + if !known { + inputEnforcementUnsafe = true } continue } if flag == "-O" || flag == "+O" { i++ + if i >= len(args) { + inputFound = false + break + } + if _, static := staticWord(args[i]); !static { + inputFound = false + break + } + inputEnforcementUnsafe = true continue } if program == "bash" && flag == "--norc" { @@ -1230,7 +1251,10 @@ func inspectShellInvocation(args []*syntax.Word) shellInvocation { stdin = flag[0] == '-' } } - result := shellInvocation{noExec: noExec || terminalNoExec} + result := shellInvocation{ + inputEnforcementUnsafe: inputEnforcementUnsafe, + noExec: noExec || terminalNoExec, + } if result.noExec { return result } From 1bb0f2363d82dd6fb0dae88777a061410520229e Mon Sep 17 00:00:00 2001 From: Ron Heichman Date: Tue, 8 Sep 2026 04:18:54 -0500 Subject: [PATCH 4/4] feat(rule): resolve a variable executable assigned in the same input A POSIX command whose first word is a variable is projected with the variable's value as its executable when the same input assigned that variable exactly once, by a plain top-level NAME=value statement with a static one-word value, before the use, and the input writes variables no other way. GH=/usr/bin/gh; $GH repo fork x then projects as a static gh command, so a name-based enforce rule denies it. Every other run-time first word keeps the current behavior: dropped with the diagnostic "dynamic command executable". The resolver walks the parsed input twice: it counts plain assignments and loop bindings per variable, records the single top-level static assignment per name, and gives up on the whole input when it sees a write the count cannot follow: arithmetic, coproc, a {name}> redirect, a zsh glob qualifier, a declaration with a non-option or rebinding operand, a write to IFS, PS4, BASH_ENV, ENV, ZDOTDIR, or HOME, printf -v, set -A, set -k, set -o keyword, shopt -o keyword, wait -p, test -v on an array element, the builtins that read or evaluate into variables (also behind command, builtin, noglob, nocorrect), or a command whose first word is neither static nor resolved. Names the shell manages, and values with whitespace, wildcards, ~, =, or $'...' quoting, never resolve. A script passed to an interpreter resolves only when the outer input resolves, the call sets no environment word, its option words, script, and redirects are readable as written, and no option selects keyword mode. The resolved word is substituted as a literal into a shallow copy of the call before projection, so projection, wrapper unwrapping, interpreter detection, and the compound enforcement candidates see one static program. Verified with 33 resolvable and 144 unresolvable forms, 89 source mutations that each fail at least one test, sixteen independent review passes, a differential replay of 9,574 inputs against the previous build, race, shuffle, fuzz, the repository lint set, and a replay of 7,312 real commands with no finding or deny change against the base. Built with Claude Code --- docs/rules.md | 35 ++ internal/rule/shell.go | 26 +- internal/rule/shell_variables.go | 460 ++++++++++++++++++++ internal/rule/variable_executable_test.go | 506 ++++++++++++++++++++++ 4 files changed, 1021 insertions(+), 6 deletions(-) create mode 100644 internal/rule/shell_variables.go create mode 100644 internal/rule/variable_executable_test.go diff --git a/docs/rules.md b/docs/rules.md index b318bcb..65e5ad0 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -375,6 +375,41 @@ here-doc supplied to an interpreter such as `sh` is executable input and is parsed. Supported static wrappers, inline scripts, substitutions, redirects, and shell functions are projected when their meaning can be established. +The one exception to variable expansion is the first word of a simple command +when the same input assigns that variable exactly once, by a plain top-level +`NAME=value` statement with a static one-word value, before the use. That +command is projected with the assigned value as its executable. Resolution +covers the whole input and is withdrawn when the input writes variables in any +way the scan cannot follow: + +- arithmetic: `(( ))`, `$(( ))`, `let`, `for (( ))`, `[[ -eq ]]`, + `[[ -v a[...] ]]`, computed indexes and slices, `${!x}`, `@` operators, and + zsh parameter flags; +- `coproc`, a `{name}>` redirect, a zsh glob qualifier, `nameref` or a + declaration with `-n`, `-i`, `-E`, `-F`, or a non-option operand, and any + write to `IFS`, `PS4`, `SHELLOPTS`, `BASH_ENV`, `ENV`, `ZDOTDIR`, or `HOME`; +- `printf -v`, `print -v`, `set -A`, `set -k`, `set -o keyword`, `shopt -o + keyword`, and `wait -p`, judged by the option words before the first operand; +- `test -v` on an array element, including any test word that can expand to + several words; +- builtins that read or evaluate into variables (`eval`, `source`, `read`, + `unset`, `trap`, `alias`, `enable`, and zsh equivalents), + also behind `command`, `builtin`, `noglob`, `nocorrect`, or `-`; +- a command whose first word is not static and does not resolve, or a command, + option, or builtin word the shell rewrites before lookup (`$'...'`, + `$"..."`, an unquoted backslash, brace, or wildcard). + +A variable in any other position, such as `sudo $GH ...`, is not analyzed. +Names the shell manages (`_`, `PWD`, `RANDOM`, `UID`, `BASH_*`, and similar) +and values with whitespace, wildcards, parentheses, `~`, `=`, or `$'...'` +quoting never resolve. A script passed to an interpreter resolves only when +the outer input resolves, the call sets no environment word, its option words, +script, and redirects are readable as written, and no option selects keyword +mode. Text passed to `eval`, PowerShell, or `cmd.exe` never resolves. zsh +evaluates bare `$name[expr]` subscripts and the integer arguments of `printf +%d`, `return`, `shift`, and similar builtins, and ksh evaluates `test -eq` +operands; neither is modeled. + Known `tool_name` values select POSIX shell, PowerShell, or `cmd.exe` parsing; otherwise numbat infers the dialect from command syntax. Set `tool_name` in a fixture when a Windows command depends on a specific dialect. diff --git a/internal/rule/shell.go b/internal/rule/shell.go index 76d23bd..0e22c3e 100644 --- a/internal/rule/shell.go +++ b/internal/rule/shell.go @@ -151,6 +151,7 @@ type shellAnalyzer struct { unsafeStatements map[int64]bool statementParents map[int64]int64 halt bool + resolved resolvedExecutables } type shellAnalysis struct { @@ -188,7 +189,7 @@ func analyzeShellCommandsDetailed(source string, dialect commandDialect) shellAn unsafeStatements: make(map[int64]bool), statementParents: make(map[int64]int64), } - a.parseDialect(source, dialect, 0, nil) + a.parseDialectUnderRedirects(source, dialect, 0, nil, 0, nil, true) err := errors.Join(a.issues...) enforcementSafe := !a.enforcementUnsafe && len(a.commands) > 0 if enforcementSafe { @@ -229,10 +230,13 @@ func commandDialectHint(ev model.Event) commandDialect { } func (a *shellAnalyzer) parseDialect(source string, dialect commandDialect, depth int, wrappers []ShellWrapper) { - a.parseDialectUnderRedirects(source, dialect, depth, wrappers, 0, nil) + a.parseDialectUnderRedirects(source, dialect, depth, wrappers, 0, nil, false) } -func (a *shellAnalyzer) parseDialectUnderRedirects(source string, dialect commandDialect, depth int, wrappers []ShellWrapper, parent int64, inheritedRedirects []*syntax.Redirect) { +// resolvable is true for the original input and for a nested script whose +// interpreter call passed resolvableScript; text that another shell expanded +// first never resolves. +func (a *shellAnalyzer) parseDialectUnderRedirects(source string, dialect commandDialect, depth int, wrappers []ShellWrapper, parent int64, inheritedRedirects []*syntax.Redirect, resolvable bool) { if a.halt { return } @@ -278,7 +282,14 @@ func (a *shellAnalyzer) parseDialectUnderRedirects(source string, dialect comman if !posixEnforcementShapeSafe(file) { a.enforcementUnsafe = true } + + saved := a.resolved + a.resolved = nil + if resolvable { + a.resolved = resolveTopLevelAssignments(file) + } a.walk(source, file, depth, make(map[string]*syntax.Stmt), make(map[string]bool), wrappers, parent, inheritedRedirects) + a.resolved = saved } func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functions map[string]*syntax.Stmt, activeFunctions map[string]bool, wrappers []ShellWrapper, parent int64, inheritedRedirects []*syntax.Redirect) { @@ -349,6 +360,8 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio } return true } + + call = resolvedCall(call, a.resolved) command, add, err := projectPOSIXCommand(source, call.Args, call.Assigns, node.Redirs, wrappers, ctx) if err != nil { a.report(err) @@ -421,12 +434,13 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio } redirects := append(relations.inheritedRedirects[node], node.Redirs...) + resolvable := a.resolved != nil && resolvableScript(call, args, redirects) if !command.FunctionCall { if script, dialect, wrapper, ok, err := wrapperScript(source, args); err != nil { a.report(err) } else if ok { innerWrappers := append(cloneWrappers(commandWrappers), wrapper) - a.parseDialectUnderRedirects(script, dialect, depth+1, innerWrappers, ctx.statementID, redirects) + a.parseDialectUnderRedirects(script, dialect, depth+1, innerWrappers, ctx.statementID, redirects, resolvable) a.markCommandsUnsafe(statementStart) } if scripts := interpreterHeredocs(invocation.inputFDs, redirects); len(scripts) > 0 { @@ -438,7 +452,7 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio a.markCommandsUnsafe(statementStart) for _, script := range scripts { innerStart := len(a.commands) - a.parseDialectUnderRedirects(script, dialectPOSIX, depth+1, innerWrappers, ctx.statementID, nil) + a.parseDialectUnderRedirects(script, dialectPOSIX, depth+1, innerWrappers, ctx.statementID, nil, resolvable) if ctx.pipelineID != 0 || !wrapperSafe || invocation.inputEnforcementUnsafe { a.markCommandsUnsafe(innerStart) } @@ -448,7 +462,7 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio } if allowShellBuiltins { if script, ok := evalScript(source, args); ok { - a.parseDialectUnderRedirects(script, dialectPOSIX, depth+1, commandWrappers, ctx.statementID, redirects) + a.parseDialectUnderRedirects(script, dialectPOSIX, depth+1, commandWrappers, ctx.statementID, redirects, false) a.markCommandsUnsafe(statementStart) } } diff --git a/internal/rule/shell_variables.go b/internal/rule/shell_variables.go new file mode 100644 index 0000000..776ea74 --- /dev/null +++ b/internal/rule/shell_variables.go @@ -0,0 +1,460 @@ +package rule + +import ( + "strings" + + "mvdan.cc/sh/v3/syntax" +) + +type resolvedExecutable struct { + value string + end syntax.Pos +} + +type resolvedExecutables map[string]resolvedExecutable + +func (r resolvedExecutables) lookup(name string, at syntax.Pos) (string, bool) { + assignment, found := r[name] + if !found || assignment.end.After(at) { + return "", false + } + return assignment.value, true +} + +var ( + variableWritingBuiltins = stringSet( + "eval", "source", ".", "read", "unset", "declare", "local", "typeset", + "export", "readonly", "mapfile", "readarray", "let", "getopts", "trap", + "alias", "enable", "emulate", "integer", "float", "vared", "zparseopts", + "zformat", "zstyle", "zregexparse", "getln", "sysread", "strftime", + "zstat", "pcre_match", "zpty", "zgetattr", "ztie", "sysopen", "zselect", + "zcurses", "private", + ) + builtinWrappers = stringSet("command", "builtin", "noglob", "nocorrect", "-") + shellManagedNames = stringSet( + "_", "argv", "reply", "REPLY", "MATCH", "match", "MBEGIN", "MEND", + "mbegin", "mend", "PWD", "OLDPWD", "DIRSTACK", "RANDOM", "SRANDOM", + "SECONDS", "LINENO", "EPOCHSECONDS", "EPOCHREALTIME", "BASHPID", + "HISTCMD", "SHLVL", "FUNCNAME", "funcstack", "PIPESTATUS", "pipestatus", + "OPTARG", "OPTIND", "MAPFILE", "COPROC", "UID", "EUID", "PPID", "GROUPS", + "SHELLOPTS", "BASHOPTS", "status", "zsh_eval_context", "TTY", "USERNAME", + "EGID", "GID", "ARGC", "signals", "functrace", "funcfiletrace", + "funcsourcetrace", "TTYIDLE", "zsh_scheduled_events", "ERRNO", + ) + shellManagedPrefixes = []string{"BASH_", "ZSH_", "COMP_", "READLINE_"} + // IFS changes how every later word splits, PS4 runs on every traced + // command, and the others configure a child shell before it reads its + // script. + unsafeWrites = stringSet("IFS", "PS4", "SHELLOPTS", "BASH_ENV", "ENV", "ZDOTDIR", "HOME") +) + +// resolveTopLevelAssignments returns nil, not an empty map, when the input +// writes variables in any way the scan cannot follow: a wrong resolution would +// report a command the shell never runs, or hide one it does. +func resolveTopLevelAssignments(file *syntax.File) resolvedExecutables { + counts := make(map[string]int) + syntax.Walk(file, func(node syntax.Node) bool { + switch n := node.(type) { + case *syntax.Assign: + if n.Name != nil { + counts[n.Name.Value]++ + } + case *syntax.WordIter: + if n.Name != nil { + counts[n.Name.Value]++ + } + } + return true + }) + for name := range unsafeWrites { + if counts[name] != 0 { + return nil + } + } + + resolved := make(resolvedExecutables) + for _, stmt := range file.Stmts { + name, value, ok := topLevelStaticAssignment(stmt) + if ok && counts[name] == 1 && !shellManaged(name) { + resolved[name] = resolvedExecutable{value: value, end: stmt.End()} + } + } + if writesOpaquely(file, resolved) { + return nil + } + return resolved +} + +func topLevelStaticAssignment(stmt *syntax.Stmt) (name, value string, ok bool) { + call, isCall := stmt.Cmd.(*syntax.CallExpr) + if !isCall || stmt.Background || stmt.Disown || len(call.Args) != 0 || len(call.Assigns) != 1 { + return "", "", false + } + assign := call.Assigns[0] + if assign.Name == nil || assign.Value == nil || assign.Append || assign.Index != nil { + return "", "", false + } + value, static := staticText(assign.Value) + // `~`, `=`, and `(...)` expand in assignment values (zsh `=cmd`, glob qualifiers). + if !static || value == "" || strings.ContainsAny(value, " \t\n*?[~()=") { + return "", "", false + } + return assign.Name.Value, value, true +} + +func shellManaged(name string) bool { + if shellManagedNames[name] { + return true + } + for _, prefix := range shellManagedPrefixes { + if strings.HasPrefix(name, prefix) { + return true + } + } + return false +} + +// Arithmetic evaluates variable contents recursively, so any arithmetic +// context can assign any variable; zsh glob qualifiers such as (e:code:) run +// code. +func writesOpaquely(file *syntax.File, resolved resolvedExecutables) bool { + opaque := false + syntax.Walk(file, func(node syntax.Node) bool { + if opaque { + return false + } + switch n := node.(type) { + case *syntax.CoprocClause, *syntax.ArithmCmd, *syntax.ArithmExp, *syntax.CStyleLoop, *syntax.LetClause, *syntax.ExtGlob: + opaque = true + case *syntax.DeclClause: + opaque = declarationRebinds(n) + case *syntax.Redirect: + opaque = n.N != nil && strings.HasPrefix(n.N.Value, "{") // {name}>file stores the descriptor in name + case *syntax.CallExpr: + opaque = callWritesVariables(n.Args, resolved) + case *syntax.BinaryTest: + opaque = isArithmeticTest(n.Op) + case *syntax.UnaryTest: + opaque = n.Op == syntax.TsVarSet && namesArrayElement(n.X) + case *syntax.Assign: + opaque = computedIndex(n.Index) + case *syntax.ArrayElem: + opaque = computedIndex(n.Index) + case *syntax.ParamExp: + opaque = paramEvaluates(n) + } + return !opaque + }) + return opaque +} + +func isArithmeticTest(op syntax.BinTestOperator) bool { + switch op { + case syntax.TsEql, syntax.TsNeq, syntax.TsLeq, syntax.TsGeq, syntax.TsLss, syntax.TsGtr: + return true + } + return false +} + +func namesArrayElement(expr syntax.TestExpr) bool { + word, ok := expr.(*syntax.Word) + if !ok { + return false + } + value, static := staticText(word) + return !static || strings.Contains(value, "[") +} + +func paramEvaluates(param *syntax.ParamExp) bool { + if param.Flags != nil || param.Excl || (param.Exp != nil && param.Exp.Op == syntax.OtherParamOps) { + return true + } + if computedIndex(param.Index) { + return true + } + if param.Slice == nil { + return false + } + return !isPlainIndex(param.Slice.Offset) || (param.Slice.Length != nil && !isPlainIndex(param.Slice.Length)) +} + +func computedIndex(index syntax.ArithmExpr) bool { + return index != nil && !isPlainIndex(index) +} + +// The shell does not evaluate @, *, or a literal number as a subscript. +func isPlainIndex(index syntax.ArithmExpr) bool { + word, ok := index.(*syntax.Word) + if !ok { + return false + } + value, static := staticWord(word) + if !static || value == "" { + return false + } + if value == "@" || value == "*" { + return true + } + for _, r := range value { + if r < '0' || r > '9' { + return false + } + } + return true +} + +// A nameref, an attribute among n, i, E, and F, or an operand the Assign +// count does not see, such as `export "GH=echo"`, can change what a later +// assignment does. +func declarationRebinds(decl *syntax.DeclClause) bool { + if decl.Variant != nil && decl.Variant.Value == "nameref" { + return true + } + for _, arg := range decl.Args { + if !arg.Naked || arg.Value == nil { + continue + } + option, ok := staticText(arg.Value) + if !ok || !strings.HasPrefix(option, "-") || strings.ContainsAny(option, "niEF") { + return true + } + } + return false +} + +// A first word that is neither static nor resolved counts as a writer. +func callWritesVariables(args []*syntax.Word, resolved resolvedExecutables) bool { + if len(args) == 0 { + return false + } + name, ok := staticText(args[0]) + if !ok { + if param, _ := variableReference(args[0]); param != nil { + name, ok = resolved.lookup(param.Param.Value, args[0].Pos()) + } + } + if !ok { + return true + } + program := commandProgram(name) + rest := args[1:] + if builtinWrappers[program] { + if inner, unwrapped := commandAfterCommandBuiltin(args); unwrapped { + return callWritesVariables(inner, resolved) + } + // Only `command -v name` and `command -V name` are inert; a wrapper that + // does not unwrap for any other reason may still run a builtin. + if len(rest) == 0 { + return true + } + option, static := staticText(rest[0]) + return !static || (option != "-v" && option != "-V") + } + switch program { + case "printf", "print": + return mayCarryOption(rest, "v") + case "set": + return namesKeyword(rest) || mayCarryOption(rest, "Ak") + case "shopt": + return namesKeyword(rest) + case "wait": + return mayCarryOption(rest, "p") + case "test", "[": + return testWrites(rest) + } + return variableWritingBuiltins[program] +} + +// mayCarryOption reports whether the option words of a call can pass one of +// the letters. +func mayCarryOption(args []*syntax.Word, letters string) bool { + afterOption := false + for _, arg := range args { + value, ok := staticText(arg) + if !ok { + return true + } + isOption := strings.HasPrefix(value, "-") || strings.HasPrefix(value, "+") + if value == "--" || (!afterOption && !isOption) { + return false + } + if isOption && strings.ContainsAny(value, letters) { + return true + } + afterOption = isOption + } + return false +} + +func namesKeyword(words []*syntax.Word) bool { + for _, word := range words { + if value, ok := staticText(word); ok && value == "keyword" { + return true + } + } + return false +} + +// testWrites reports whether a test or [ call can apply -v to an array element, +// whose subscript bash evaluates as arithmetic. A three-word test with a +// readable middle word other than -v is a binary test: in bash and zsh, test +// and [ evaluate nothing for those, and the numeric operators take integer +// literals only, unlike [[. +func testWrites(args []*syntax.Word) bool { + words := make([]*syntax.Word, 0, len(args)) + for _, word := range args { + if wordSplits(word) { + return true + } + if value, ok := staticText(word); ok && (value == "]" || value == "!") { + continue + } + words = append(words, word) + } + if len(words) == 3 { + if value, ok := staticText(words[1]); ok && value != "-v" { + return false + } + } + elements, hasV := 0, false + for _, word := range words { + value, ok := staticText(word) + switch { + case !ok || strings.Contains(value, "["): + elements++ + case value == "-v": + hasV = true + } + } + return elements >= 2 || (elements > 0 && hasV) +} + +// Double quotes do not join $@ or ${name[@]}. +func wordSplits(word *syntax.Word) bool { + for _, part := range word.Parts { + switch p := part.(type) { + case *syntax.Lit: + if strings.ContainsAny(p.Value, "*?{") { + return true + } + case *syntax.SglQuoted, *syntax.DblQuoted: + default: + return true + } + } + list := false + syntax.Walk(word, func(node syntax.Node) bool { + if list { + return false + } + param, ok := node.(*syntax.ParamExp) + if !ok { + return true + } + if param.Param != nil && param.Param.Value == "@" { + list = true + } else if index, isWord := param.Index.(*syntax.Word); isWord { + value, static := staticWord(index) + list = static && value == "@" + } + return !list + }) + return list +} + +// args is the call after wrapper unwrapping, a non-empty suffix of call.Args +// when the call has no assignments. An environment word can configure the +// interpreter before it reads the script, a redirect or script word the outer +// shell rewrites is not the text the inner shell sees (the option scan rejects +// such a script word), and keyword mode (-k, -o keyword) turns assignment +// words after a command into assignments. +func resolvableScript(call *syntax.CallExpr, args []*syntax.Word, redirects []*syntax.Redirect) bool { + if len(call.Assigns) > 0 { + return false + } + for _, word := range call.Args[:len(call.Args)-len(args)] { + if value, ok := staticText(word); !ok || strings.Contains(value, "=") { + return false + } + } + for _, redirect := range redirects { + if redirect.Word == nil { + continue + } + if _, ok := staticText(redirect.Word); !ok { + return false + } + } + return !mayCarryOption(args[1:], "k") && !namesKeyword(args[1:]) +} + +// staticText returns the text of a word the shell looks up as written. Beyond +// posixPartsExpand, an unquoted backslash rewrites the word, and the bare test +// command `[` is the one bracket the shell reads as written. +func staticText(word *syntax.Word) (string, bool) { + value, ok := staticWord(word) + if !ok { + return "", false + } + if value == "[" && len(word.Parts) == 1 { + return value, true + } + if posixPartsExpand(word.Parts, true) { + return "", false + } + for _, part := range word.Parts { + if lit, isLit := part.(*syntax.Lit); isLit && strings.Contains(lit.Value, `\`) { + return "", false + } + } + return value, true +} + +// Indirection and zsh flags need no check here: paramEvaluates makes the +// input opaque. +func variableReference(word *syntax.Word) (*syntax.ParamExp, *syntax.DblQuoted) { + if len(word.Parts) != 1 { + return nil, nil + } + part := word.Parts[0] + quoted, isQuoted := part.(*syntax.DblQuoted) + if isQuoted { + if quoted.Dollar || len(quoted.Parts) != 1 { + return nil, nil + } + part = quoted.Parts[0] + } + param, ok := part.(*syntax.ParamExp) + if !ok || param.Param == nil || param.Length || param.Width || param.IsSet || + param.Index != nil || param.Slice != nil || param.Repl != nil || param.Exp != nil || + param.NestedParam != nil || len(param.Modifiers) != 0 { + return nil, nil + } + return param, quoted +} + +// The literal keeps the span and the double quotes of the reference, so +// positions and quote fields still describe the input as written. +func resolvedCall(call *syntax.CallExpr, resolved resolvedExecutables) *syntax.CallExpr { + if len(call.Args) == 0 { + return call + } + param, quoted := variableReference(call.Args[0]) + if param == nil { + return call + } + executable, ok := resolved.lookup(param.Param.Value, call.Args[0].Pos()) + if !ok { + return call + } + var part syntax.WordPart = &syntax.Lit{ValuePos: param.Pos(), ValueEnd: param.End(), Value: executable} + if quoted != nil { + part = &syntax.DblQuoted{Left: quoted.Left, Right: quoted.Right, Parts: []syntax.WordPart{part}} + } + args := make([]*syntax.Word, len(call.Args)) + copy(args, call.Args) + args[0] = &syntax.Word{Parts: []syntax.WordPart{part}} + out := *call + out.Args = args + return &out +} diff --git a/internal/rule/variable_executable_test.go b/internal/rule/variable_executable_test.go new file mode 100644 index 0000000..e07b310 --- /dev/null +++ b/internal/rule/variable_executable_test.go @@ -0,0 +1,506 @@ +package rule_test + +import ( + "fmt" + "slices" + "strings" + "testing" + + "github.com/perplexityai/numbat/internal/model" + "github.com/perplexityai/numbat/internal/rule" +) + +const ( + forkByNameExpr = `shell_commands.exists(command, + command.name == "gh" && command.argv.size() > 2 && + command.argv[1] == "repo" && command.argv[2] == "fork")` + quotedForkExpr = `shell_commands.exists(command, + command.name == "gh" && command.arguments[0].quote == "double")` + dynamicExecutable = "dynamic command executable" + nestedScript = "bash -c 'GH=/usr/bin/gh; $GH repo fork" + + " example/project'" + resolvedFork = "GH=/usr/bin/gh; $GH repo fork example/project" + quotedResolvedFork = `GH=/usr/bin/gh; "$GH" repo fork example/project` + singleMatch = 1 +) + +// ruleEngine builds an engine holding one enforced rule with expr. +func ruleEngine(t *testing.T, expr string) *rule.Engine { + t.Helper() + + var enforced rule.Rule + + enforced.ID = "t.fork" + enforced.Version = "1" + enforced.Title = "fork" + enforced.Severity = model.SeverityHigh + enforced.Enforce = new(true) + enforced.Expr = expr + + var source rule.Source + + source.Name = "test" + source.Rules = []rule.Rule{enforced} + + engine, err := rule.NewEngine([]rule.Source{source}) + if err != nil { + t.Fatalf("NewEngine: %v", err) + } + + return engine +} + +// commandEvent wraps command as a command.exec event with no tool hint. +func commandEvent(command string) model.Event { + return toolEvent("", command) +} + +// toolEvent wraps command as a command.exec event from the named tool. +func toolEvent(tool, command string) model.Event { + var event model.Event + + event.EventType = model.EventCommandExec + event.ToolName = tool + event.Command = command + + return event +} + +// evalFork evaluates one command against the name-based fork rule and +// returns the matches and the analysis diagnostic. +func evalFork(t *testing.T, command string) ([]rule.Match, error) { + t.Helper() + + matches, err := ruleEngine(t, forkByNameExpr).Eval(commandEvent(command)) + if err != nil { + return matches, fmt.Errorf("eval %q: %w", command, err) + } + + return matches, nil +} + +// resolvedMatch evaluates a command that must analyze without a diagnostic +// and match the fork rule exactly once. +func resolvedMatch(t *testing.T, command string) rule.Match { + t.Helper() + + matches, err := evalFork(t, command) + if err != nil { + t.Fatalf("diagnostic %v, want none", err) + } + + if len(matches) != singleMatch { + t.Fatalf("%q: matches = %+v, want one", command, matches) + } + + return matches[0] +} + +// requireDynamic evaluates a command whose first word must stay a run-time +// executable: the dynamic diagnostic and no match. +func requireDynamic(t *testing.T, command string) { + t.Helper() + requireDynamicFrom(t, "", command) +} + +// requireDynamicFrom evaluates a command from the named tool that must keep +// the dynamic diagnostic and produce no match. +func requireDynamicFrom(t *testing.T, tool, command string) { + t.Helper() + + matches, err := ruleEngine(t, forkByNameExpr).Eval(toolEvent(tool, command)) + if err == nil || !strings.Contains(err.Error(), dynamicExecutable) { + t.Fatalf("%q: diagnostic %v, want %q", command, err, dynamicExecutable) + } + + if len(matches) != 0 { + t.Fatalf("%q: matches = %+v, want none", command, matches) + } +} + +// resolvedForms assign the variable once at the top level before its use. +func resolvedForms() []string { + return []string{ + resolvedFork, + "GH=/usr/bin/gh\n$GH repo fork example/project", + quotedResolvedFork, + "GH=/usr/bin/gh; ${GH} repo fork example/project", + "GH='gh'; $GH repo fork example/project", + "GH=/usr/bin/gh; $GH repo fork example/project; $GH pr view 1", + "GH=gh; echo ${GH:=echo}; $GH repo fork example/project", + "S=sudo; $S gh repo fork example/project", + "GH=/usr/bin/gh;$GH repo fork example/project", + "GH=/usr/bin/gh; f() { $GH repo fork example/project; }; f", + "GH=/usr/bin/gh; ( $GH repo fork example/project )", + "GH=/usr/bin/gh; time $GH repo fork example/project", + "bash <<'EOF'\nGH=/usr/bin/gh; $GH repo fork example/project\nEOF", + } +} + +// inertNeighbors put a statement that reads no variable between the +// assignment and its use. +func inertNeighbors() []string { + return []string{ + "GH=/usr/bin/gh; printf '%s\\n' start; $GH repo fork example/project", + "GH=/usr/bin/gh; command -v gh; $GH repo fork example/project", + "GH=/usr/bin/gh; sh -c 'X=1'; $GH repo fork example/project", + "GH=/usr/bin/gh; export PATH=/x; $GH repo fork example/project", + "GH=/usr/bin/gh; set -euo pipefail; $GH repo fork example/project", + "GH=/usr/bin/gh; printf '%s\\n' \"$x\"; $GH repo fork example/project", + "GH=/usr/bin/gh; set -- \"$@\"; $GH repo fork example/project", + "GH=/usr/bin/gh; [[ -f x ]]; $GH repo fork example/project", + "GH=/usr/bin/gh; echo \"${a[@]}\"; $GH repo fork example/project", + "GH=/usr/bin/gh; printf -- '%s\\n' \"$x\"; $GH repo fork example/p", + "GH=/usr/bin/gh; printf '%s\\n' '--- reviews ---'; $GH repo fork x/y", + "GH=/usr/bin/gh; [ -f \"$x\" ]; $GH repo fork example/project", + "GH=/usr/bin/gh; [ \"$x\" = \"$y\" ]; $GH repo fork example/project", + "GH=/usr/bin/gh; [ \"$x\" ]; $GH repo fork example/project", + "GH=/usr/bin/gh; true & wait; $GH repo fork example/project", + "GH=/usr/bin/gh; [[ -v GH ]]; $GH repo fork example/project", + "GH=/usr/bin/gh; [ -v GH ]; $GH repo fork example/project", + `GH=/usr/bin/gh; [ "$n" -gt 0 ]; $GH repo fork example/project`, + `GH=/usr/bin/gh; [ -n "$x" -a -f y ]; $GH repo fork example/project`, + "GH=/usr/bin/gh; command true; $GH repo fork example/project", + } +} + +// unresolvedAssignments are inputs whose assignment is not one plain +// top-level statement before the use, or whose value is not one static word. +func unresolvedAssignments() []string { + return []string{ + "$GH repo fork example/project", + "GH=gh; GH=echo; $GH repo fork example/project", + "GH=gh; if true; then GH=echo; fi; $GH repo fork example/project", + "cd repo; $GH repo fork example/project; GH=gh", + "GH=gh && $GH repo fork example/project", + `GH="gh --verbose"; $GH repo fork example/project`, + "GH=gh true; $GH repo fork example/project", + "GH=gh & $GH repo fork example/project", + "GH=gh; for GH in echo; do :; done; $GH repo fork example/project", + "GH+=gh; $GH repo fork example/project", + "GH[1]=gh; $GH repo fork example/project", + `GH=""; $GH repo fork example/project`, + "GH=$(which gh); $GH repo fork example/project", + "f() { GH=gh; }; f; $GH repo fork example/project", + "_=echo; true gh; $_ repo fork example/project", + "BASH_REMATCH=echo; [[ gh =~ gh ]]; $BASH_REMATCH repo fork example/p", + "argv=echo; set -- gh; $argv repo fork example/project", + `GH=$'g\x68'; $GH repo fork example/project`, + "UID=gh; $UID repo fork example/project", + `GH=gh; $"$GH" repo fork example/project`, + `GH=$"gh"; $GH repo fork example/project`, + "GH=g\\h; $GH repo fork example/project", + "GH=~/gh; $GH repo fork example/project", + "GH=gh &| $GH repo fork example/project", + "ERRNO=gh; cd /nonexistent; $ERRNO repo fork example/project", + "GH='+(gh)'; $GH repo fork example/project", + "GH==gh; $GH repo fork example/project", + `sudo -u $'r\x6fot' bash -c 'GH=/usr/bin/gh; $GH repo fork example/p'`, + "GH='(gh)'; $GH repo fork example/project", + } +} + +// builtinWrites are inputs that write the variable through a bash builtin, a +// wrapper, or a program the input does not name statically. +func builtinWrites() []string { + return []string{ + "GH=gh; unset GH; $GH repo fork example/project", + "GH=gh; eval x; $GH repo fork example/project", + "GH=gh; printf -v GH echo; $GH repo fork example/project", + "GH=gh; command unset GH; $GH repo fork example/project", + "GH=gh; wait -p GH; $GH repo fork example/project", + "GH=gh; wait -fp GH; $GH repo fork example/project", + "GH=gh; coproc GH { sleep 1; }; $GH repo fork example/project", + "GH=gh; E=eval; $E 'GH=echo'; $GH repo fork example/project", + "GH=gh; $RUN x; $GH repo fork example/project", + "GH=gh; trap 'GH=echo' ERR; false; $GH repo fork example/project", + "GH=gh; command -p unset GH; $GH repo fork example/project", + "GH=gh; U=unset; command $U GH; $GH repo fork example/project", + "GH=echo; F=-v; printf $F GH gh; $GH repo fork example/project", + "GH=echo; X=printf; command $X -v GH gh; $GH repo fork example/p", + "GH=echo; alias u=unset; u GH; $GH repo fork example/project", + "GH=gh; enable -n unset; $GH repo fork example/project", + "GH=gh; builtin unset GH; $GH repo fork example/project", + "GH=echo; set -o keyword; : GH=gh; $GH repo fork example/project", + "GH=echo; set -o posix; set -k; : GH=gh; $GH repo fork example/project", + "GH=gh; shopt -so keyword; : GH=echo; $GH repo fork example/project", + "GH=gh; shopt -s -o keyword; : GH=echo; $GH repo fork example/project", + "GH=gh; U=unset; command -p $U GH; $GH repo fork example/project", + } +} + +// expansionWrites are inputs whose builtin, option, or test word reaches +// the shell only after quote removal, brace expansion, or globbing. +func expansionWrites() []string { + return []string{ + "GH=echo; e\\val 'GH=gh'; $GH repo fork example/project", + "GH=echo; printf \\-v GH gh; $GH repo fork example/project", + `GH=echo; $'e\x76al' 'GH=gh'; $GH repo fork example/project`, + `GH=echo; printf $'\x2dv' GH gh; $GH repo fork example/project`, + `GH=gh; [ $'\x2dv' a[GH=7] ]; $GH repo fork example/project`, + "GH=echo; eva{l,l} 'GH=gh'; $GH repo fork example/project", + "GH=gh; u{nset,nset} GH; $GH repo fork example/project", + "GH=echo; ev?l 'GH=gh'; $GH repo fork example/project", + "GH=echo; printf -[v] GH gh; $GH repo fork example/project", + `GH=gh; [""u]nset GH; $GH repo fork example/project`, + `GH=echo; printf [""-]v GH gh; $GH repo fork example/project`, + } +} + +// interpreterWrites are inputs whose nested script reaches the inner shell +// changed, or runs under an interpreter option that turns argument words +// into assignments. +func interpreterWrites() []string { + return []string{ + "GH=gh; bash -c \"GH=echo; $GH repo fork example/project\"", + `bash -c $'GH=echo; : x\x3bGH=gh; $GH repo fork example/project'`, + `eval 'GH=echo;' $'$GH repo\x20fork example/project'`, + "sh -k <<'EOF'\nGH=echo; : GH=gh; $GH repo fork example/project\nEOF", + `bash <<< $'GH=gh; : x\x3bGH=echo; $GH repo fork example/project'`, + "sudo sh -k <<'EOF'\nGH=gh; : GH=echo; $GH repo fork example/p\nEOF", + "sudo bash -k -c 'GH=echo; : GH=gh; $GH repo fork example/project'", + "sh -o keyword <<'EOF'\nGH=echo; : GH=gh; $GH repo fork example/p\nEOF", + "BASH_ENV=/tmp/e bash -c 'GH=/usr/bin/gh; $GH repo fork example/p'", + "env BASH_ENV=/tmp/e bash -c 'GH=/usr/bin/gh; $GH repo fork example/p'", + "IFS=x; eval 'GH=ghxfoo; $GH repo fork example/project'", + "export BASH_ENV=/tmp/e; bash -c 'GH=echo; $GH repo fork example/project'", + "set -o posix; set -k; eval 'GH=echo; : GH=gh; $GH repo fork example/p'", + "eval 'GH=/usr/bin/gh;' '$GH repo fork example/project'", + "export SHELLOPTS=keyword:posix; bash -c 'GH=echo; : GH=gh; $GH repo fork x/y'", + } +} + +// zshWrites are inputs that write the variable through a zsh builtin, option +// form, or precommand modifier. +func zshWrites() []string { + return []string{ + "GH=gh; set -A GH echo; $GH repo fork example/project", + "GH=gh; print -v GH echo; $GH repo fork example/project", + "GH=echo; print -rv GH gh; $GH repo fork example/project", + "GH=echo; set +A GH gh; $GH repo fork example/project", + "GH=echo; set -o errexit -A GH gh; $GH repo fork example/project", + "GH=echo; print -f '%s' -v GH gh; $GH repo fork example/project", + "GH=echo; noglob print -v GH gh; $GH repo fork example/project", + "GH=echo; nocorrect print -v GH gh; $GH repo fork example/project", + "GH=echo; - print -v GH gh; $GH repo fork example/project", + "GH=echo; emulate sh -c 'GH=gh'; $GH repo fork example/project", + "GH=echo; zformat -f GH %s s:gh; $GH repo fork example/project", + "GH=echo; zstyle -s :x y GH; $GH repo fork example/project", + "GH=gh; typeset -F x; x=GH=7; $GH repo fork example/project", + "GH=gh; x=GH; echo ${(P)x}; $GH repo fork example/project", + "GH=gh; ${GH:s/g/z/} repo fork example/project", + "GH=gh; : *(e:'GH=echo':); $GH repo fork example/project", + } +} + +// arithmeticWrites are inputs that write the variable through an arithmetic +// context or a computed subscript or slice. +func arithmeticWrites() []string { + return []string{ + "GH=/usr/bin/gh; n=$((1 <= 2)); $GH repo fork example/project", + "GH=gh; ((GH = 0)); $GH repo fork example/project", + "GH=gh; n=$((GH++)); $GH repo fork example/project", + "GH=gh; x=GH=7; : $((x)); $GH repo fork example/project", + "GH=gh; x[GH=7]=1; $GH repo fork example/project", + "GH=gh; echo ${a[GH=7]}; $GH repo fork example/project", + "GH=gh; echo ${x:GH=7}; $GH repo fork example/project", + "GH=gh; x=([GH=7]=1); $GH repo fork example/project", + "GH=gh; x=a[GH=7]; echo ${!x}; $GH repo fork example/project", + `GH=gh; p='$((GH=7))'; echo "${p@P}"; $GH repo fork example/project`, + "GH=gh; for ((GH=0; GH<1; GH++)); do :; done; $GH repo fork x/y", + "GH=gh; select GH in a; do :; done; $GH repo fork example/project", + "GH=gh; i=GH=7; echo ${a[i]}; $GH repo fork example/project", + } +} + +// testCommandWrites are inputs that write the variable through a `test`, +// `[`, or `[[` word that names or evaluates an array subscript. +func testCommandWrites() []string { + return []string{ + "GH=gh; [[ GH=7 -eq 7 ]]; $GH repo fork example/project", + "GH=gh; [[ -v a[GH=7] ]]; $GH repo fork example/project", + "GH=gh; i=GH=7; [[ -v a[$i] ]]; $GH repo fork example/project", + "GH=gh; [ -v a[GH=7] ]; $GH repo fork example/project", + "GH=gh; [ ! -v a[GH=7] ]; $GH repo fork example/project", + "GH=gh; O=-v; [ $O a[GH=7] ]; $GH repo fork example/project", + "GH=gh; O=-v; A=a[GH=7]; [ $O $A ]; $GH repo fork example/project", + "GH=gh; command test -v a[GH=7]; $GH repo fork example/project", + "GH=gh; a=x; [ -v 'a[GH=7]' ]; $GH repo fork example/project", + "GH=gh; x='-v a[GH=7]'; [ $x ]; $GH repo fork example/project", + "GH=gh; x='!'; [ $x -v a[GH=7] ]; $GH repo fork example/project", + `GH=gh; x='-v a[GH=7]'; [ ""$x ]; $GH repo fork example/project`, + `GH=gh; x='!'; [ "$x" -v a[GH=7] ]; $GH repo fork example/project`, + `GH=gh; [ \! -v a[GH=7] ]; $GH repo fork example/project`, + `GH=gh; set -- -v 'a[GH=7]'; [ "$@" ]; $GH repo fork example/project`, + "GH=gh; [ {-v,a[GH=7]} ]; $GH repo fork example/project", + "GH=gh; [ * ]; $GH repo fork example/project", + `GH=gh; arr=(-v 'a[GH=7]'); [ "${arr[@]}" ]; $GH repo fork example/p`, + } +} + +// declarationWrites are inputs that write the variable through a +// declaration, a descriptor variable, or a name the shell reads specially. +func declarationWrites() []string { + return []string{ + "GH=gh; declare -n REF=GH; REF=echo; $GH repo fork example/project", + "GH=gh; nameref REF=GH; REF=echo; $GH repo fork example/project", + "GH=gh; declare -i x; x=GH=7; $GH repo fork example/project", + "GH=echo; O=-n; declare $O REF=GH; REF=gh; $GH repo fork example/p", + `GH=gh; export "GH=echo"; $GH repo fork example/project`, + "IFS=h; GH=gh; $GH repo fork example/project", + "GH=gh; PS4='$((GH=7))'; set -x; :; $GH repo fork example/project", + "GH=gh; true {GH}>/dev/null; $GH repo fork example/project", + "GH=gh; declare -F; $GH repo fork example/project", + } +} + +// operatorExpansions use the variable through an expansion that changes or +// extends its value. +func operatorExpansions() []string { + return []string{ + "GH=gh; ${GH:+echo} repo fork example/project", + "GH=gh; ${#GH} repo fork example/project", + "GH=gh; ${!GH} repo fork example/project", + "GH=gh; ${GH:0:1} repo fork example/project", + "GH=gh; ${GH/gh/echo} repo fork example/project", + "GH=gh; ${GH[1]} repo fork example/project", + `GH=gh; "${GH}x" repo fork example/project`, + "GH=gh; ${GH}x repo fork example/project", + "GH=gh; ${(U)GH} repo fork example/project", + "GH=gh; ${+GH} repo fork example/project", + "GH=gh; ${!GH*} repo fork example/project", + } +} + +// forEachCommand runs check on every command as a parallel subtest. +func forEachCommand( + t *testing.T, + commands []string, + check func(*testing.T, string), +) { + t.Helper() + + for _, command := range commands { + t.Run(command, func(t *testing.T) { + t.Parallel() + check(t, command) + }) + } +} + +// requireEnforceable evaluates a command that must resolve to one +// enforceable match. +func requireEnforceable(t *testing.T, command string) { + t.Helper() + + if match := resolvedMatch(t, command); !match.EnforcementMatch { + t.Fatalf("%q: match %+v, want enforceable", command, match) + } +} + +// requireDetectionOnly evaluates a command that must resolve to one match +// that enforcement does not act on. +func requireDetectionOnly(t *testing.T, command string) { + t.Helper() + + if match := resolvedMatch(t, command); match.EnforcementMatch { + t.Fatalf("%q: match %+v, want detection only", command, match) + } +} + +func TestSameInputAssignmentResolvesVariableExecutable(t *testing.T) { + t.Parallel() + + forEachCommand( + t, + slices.Concat(resolvedForms(), inertNeighbors()), + requireEnforceable, + ) +} + +func TestUnresolvableVariableExecutableStaysDynamic(t *testing.T) { + t.Parallel() + + forEachCommand(t, slices.Concat( + unresolvedAssignments(), + builtinWrites(), + expansionWrites(), + interpreterWrites(), + zshWrites(), + arithmeticWrites(), + testCommandWrites(), + declarationWrites(), + operatorExpansions(), + ), requireDynamic) +} + +func TestResolvedInterpreterParsesInlineScript(t *testing.T) { + t.Parallel() + + forEachCommand(t, []string{ + "SH=/bin/sh; $SH -c 'gh repo fork example/project'", + nestedScript, + "sudo bash -c 'GH=/usr/bin/gh; $GH repo fork example/project'", + }, requireDetectionOnly) +} + +func TestVariableOptionWordEndsResolution(t *testing.T) { + t.Parallel() + + forEachCommand(t, []string{ + "GH=/usr/bin/gh; wait $!; $GH repo fork example/project", + "GH=/usr/bin/gh; set \"$@\"; $GH repo fork example/project", + }, requireDynamic) +} + +func TestPowerShellScriptDoesNotResolve(t *testing.T) { + t.Parallel() + requireDynamicFrom(t, "pwsh", nestedScript) + requireDynamicFrom( + t, + "cmd", + `bash -c "GH=/usr/bin/gh; $GH repo fork example/project"`, + ) + requireDynamic( + t, + `pwsh -c 'bash -c '"'"'GH=gh; $GH repo fork example/project'"'"''`, + ) +} + +func TestWrapperOperandStaysUnresolved(t *testing.T) { + t.Parallel() + + command := "GH=/usr/bin/gh; sudo $GH repo fork example/project" + + matches, err := evalFork(t, command) + if err != nil || len(matches) != 0 { + t.Fatalf("%q: matches = %+v, %v, want none", command, matches, err) + } +} + +func TestResolvedQuotedWordKeepsQuote(t *testing.T) { + t.Parallel() + + for command, want := range map[string]int{ + quotedResolvedFork: singleMatch, + `GH=/usr/bin/gh; "$GH"`: singleMatch, + resolvedFork: 0, + } { + t.Run(command, func(t *testing.T) { + t.Parallel() + + engine := ruleEngine(t, quotedForkExpr) + + matches, evalErr := engine.Eval(commandEvent(command)) + if evalErr != nil || len(matches) != want { + t.Fatalf( + "%q: matches = %+v, %v, want %d", + command, + matches, + evalErr, + want, + ) + } + }) + } +}