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/6] 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/6] 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/6] 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 bebb30d41289fd555815c23b332e4862143f1ad4 Mon Sep 17 00:00:00 2001 From: Codex Date: Tue, 8 Sep 2026 11:43:32 -0500 Subject: [PATCH 4/6] fix(rule): narrow compound enforcement to static commands Remove expanded interpreter input analysis, preserve detection-only script handling, and cover pipeline grouping and native hook regressions. Built with Codex. --- cmd/numbat/hook_enforce_test.go | 37 ++ docs/enforcement.md | 55 ++- internal/rule/engine.go | 11 +- .../rule/multicommand_enforcement_test.go | 213 ++--------- internal/rule/sequence_test.go | 3 - internal/rule/shell.go | 361 +++--------------- internal/rule/shell_relations.go | 58 +-- internal/rule/shell_types.go | 2 +- internal/sequence/sequence.go | 14 +- rules/builtin_test.go | 5 - 10 files changed, 163 insertions(+), 596 deletions(-) diff --git a/cmd/numbat/hook_enforce_test.go b/cmd/numbat/hook_enforce_test.go index 5304474..761cdce 100644 --- a/cmd/numbat/hook_enforce_test.go +++ b/cmd/numbat/hook_enforce_test.go @@ -472,6 +472,43 @@ func TestEnforceOnMediumEnforceMatchDenies(t *testing.T) { } } +func TestEnforceCompoundCommandsPreservesInterpreterBoundary(t *testing.T) { + dir := writeEnforceRuleFile(t, criticalEnforceRule) + for _, test := range []struct { + name, command string + deny bool + }{ + {"compound", "true; cat .env", true}, + {"safe sibling of interpreter", "sh -c 'true'; cat .env", true}, + {"unsafe pipeline", `cat .env | "$sink"; true`, false}, + {"heredoc script", "sh <<'EOF'\ncat .env\nEOF", false}, + {"invalid zsh option", "zsh -odefinitely_invalid <<'EOF'\ncat .env\nEOF", false}, + {"missing bash option value", "bash --rcfile <<'EOF'\ncat .env\nEOF", false}, + {"invalid option after startup file", "bash --rcfile /dev/fd/3 -i -Z 3<<'EOF'\ncat .env\nEOF", false}, + {"consumed inherited input", "{ cat >/dev/null; sh; } <<'EOF'\ncat .env\nEOF", false}, + } { + t.Run(test.name, func(t *testing.T) { + command, err := json.Marshal(test.command) + if err != nil { + t.Fatal(err) + } + payload := fmt.Sprintf(`{"session_id":"s1","cwd":"/proj","tool_name":"Bash","tool_input":{"command":%s}}`, command) + out, errb, code := runCLIStdin(payload, + enforceHookArgs(t, "hook", "pre-tool", "--agent", "claude", "--enforce", "--no-builtin-rules", "--rules-dir", dir)...) + if code != 0 || errb != "" { + t.Fatalf("hook exit=%d stderr=%q", code, errb) + } + if test.deny { + if decodeDecision(t, out) != "deny" { + t.Fatalf("hook output=%q, want deny", out) + } + } else { + assertAllow(t, out, code) + } + }) + } +} + func TestEnforceUsesCustomDenyMessage(t *testing.T) { dir := writeEnforceRuleFile(t, customMessageEnforceRule) out, _, code := runCLIStdin(catEnvPayload, diff --git a/docs/enforcement.md b/docs/enforcement.md index 1594196..46378b1 100644 --- a/docs/enforcement.md +++ b/docs/enforcement.md @@ -76,34 +76,33 @@ 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. 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. +For POSIX input, a candidate is one parsed command or the direct members of one +`|` or `|&` pipeline. Each candidate must meet the existing checks for static +arguments, assignments, redirect targets, wrappers, and previews. An unsafe +pipeline also excludes its nested substitutions. Malformed top-level input and +truncated command lists stay detection-only. + +Sequencing, groups, subshells, background commands, negation, and substitutions +do not disable an otherwise eligible candidate. Both sides of `&&` and `||` +count as requested intent, even when one side cannot execute. Function calls +remain detection-only. Eligible commands in an invoked function body are +separate candidates. + +Detection evaluates the complete command list. For an `enforce: true` rule that +uses `shell_commands`, enforcement evaluates the same expression against each +eligible candidate. An input that passes the existing whole-input safety checks +uses its complete list for both decisions. Other event fields retain their full +values. + +A candidate match produces a finding and can deny the complete tool +input, even when full-list detection fails. Detection errors remain diagnostics. +A candidate error suppresses enforcement only when no candidate returns true. +Rules without `shell_commands` keep their existing behavior. + +Scripts parsed from `eval` or child interpreter input, including heredocs, remain +detection-only. Compound PowerShell and `cmd.exe` input also remain +detection-only. An independent eligible POSIX command can still enforce beside +an interpreter invocation. numbat recognizes explicit `-WhatIf` and a statically visible `$WhatIfPreference = $true` for known cmdlets. It does not infer ambient diff --git a/internal/rule/engine.go b/internal/rule/engine.go index 0655908..99fc5f9 100644 --- a/internal/rule/engine.go +++ b/internal/rule/engine.go @@ -81,11 +81,13 @@ 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 keeps a detection or candidate match separate from permission to enforce it. type StepEvaluation struct { Match bool EnforcementMatch bool } +// EvalStep can return a clean candidate match alongside a full-list diagnostic. 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) @@ -109,15 +111,6 @@ func (s *SequenceRule) EvalStep(i int, activations SequenceActivations) (StepEva }, errors.Join(errs...) } -// StepUsesShellCommands reports whether step i depends on the derived command -// projection. -func (s *SequenceRule) StepUsesShellCommands(i int) bool { - if i < 0 || i >= len(s.steps) { - return false - } - return s.steps[i].usesShellCommands -} - // newEnv builds the CEL environment shared by every rule. `event` uses emitted // JSON field names; shellCommandsVariable is an activation-only helper. func newEnv() (*cel.Env, error) { diff --git a/internal/rule/multicommand_enforcement_test.go b/internal/rule/multicommand_enforcement_test.go index 4aeea24..9e3ca6a 100644 --- a/internal/rule/multicommand_enforcement_test.go +++ b/internal/rule/multicommand_enforcement_test.go @@ -37,53 +37,6 @@ func TestMultiCommandEnforcementRegression(t *testing.T) { {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)"`}, @@ -142,7 +95,8 @@ func TestMultiCommandEnforcementCandidateEvaluation(t *testing.T) { 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: "complete pipeline candidate", expr: `shell_commands.size() == 2 && shell_commands.exists(command, command.name == "cat") && shell_commands.exists(command, command.name == "grep")`, command: `true; cat .env | grep x`, wantEnforce: true}, + {name: "list all", expr: `event.event_type == "command.exec" && shell_commands.all(command, command.name == "cat")`, command: `cat one; true`, 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}, @@ -192,146 +146,6 @@ func TestMultiCommandEnforcementPreservesPipelineSafety(t *testing.T) { } } -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 TestMultiCommandEnforcementDoesNotProjectInvalidInterpreterInvocation(t *testing.T) { - eng := compoundRuleEngine(t, `shell_commands.exists(command, - command.name == "cat" && command.argv.exists(arg, arg == ".env"))`) - 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", - "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{ - 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) - } - } -} - -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"))`) @@ -385,3 +199,26 @@ func TestMultiCommandEnforcementDoesNotTreatFunctionCallsAsExecutables(t *testin } } } + +func TestMultiCommandEnforcementKeepsInterpreterScriptsDetectionOnly(t *testing.T) { + eng := compoundRuleEngine(t, `shell_commands.exists(command, + command.name == "cat" && command.argv.exists(arg, arg == ".env"))`) + for _, test := range []struct { + name, tool, command string + }{ + {"heredoc", "bash", "sh <<'EOF'\ncat .env\nEOF"}, + {"wrapped heredoc", "bash", "env sh <<'EOF'\ncat .env\nEOF"}, + {"invalid attached option", "bash", "zsh -odefinitely_invalid <<'EOF'\ncat .env\nEOF"}, + {"missing startup file", "bash", "bash --rcfile <<'EOF'\ncat .env\nEOF"}, + {"inline script", "bash", `sh -c 'cat .env'`}, + {"powershell inline script", "PowerShell", `Write-Output ready; sh -c 'cat .env'`}, + {"cmd inline script", "cmd", `echo ready & sh -c "cat .env"`}, + } { + t.Run(test.name, func(t *testing.T) { + matches, err := eng.Eval(model.Event{EventType: model.EventCommandExec, ToolName: test.tool, Command: test.command}) + if err != nil || len(matches) != 1 || matches[0].EnforcementMatch { + t.Fatalf("Eval(%q) = (%+v, %v), want one detection-only match", test.command, matches, err) + } + }) + } +} diff --git a/internal/rule/sequence_test.go b/internal/rule/sequence_test.go index c0013e7..44e5d4e 100644 --- a/internal/rule/sequence_test.go +++ b/internal/rule/sequence_test.go @@ -135,9 +135,6 @@ func TestSequenceStepBounds(t *testing.T) { if err == nil || got.Match || got.EnforcementMatch { t.Fatalf("EvalStep(%d) = %v, %v; want false and error", step, got, err) } - if s.StepUsesShellCommands(step) { - t.Fatalf("StepUsesShellCommands(%d) = true, want false", step) - } } } diff --git a/internal/rule/shell.go b/internal/rule/shell.go index 76d23bd..e0a48cc 100644 --- a/internal/rule/shell.go +++ b/internal/rule/shell.go @@ -3,9 +3,7 @@ package rule import ( "errors" "fmt" - "path" "runtime" - "strconv" "strings" "github.com/google/cel-go/common/types" @@ -104,6 +102,7 @@ func shellCommandCandidateList(adapter types.Adapter, candidates [][]ShellComman return types.NewRefValList(adapter, values) } +// SequenceActivations shares one event's analysis across steps without exposing its safety flags. type SequenceActivations struct { prepared sequenceActivations } @@ -229,10 +228,10 @@ 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) -} - -func (a *shellAnalyzer) parseDialectUnderRedirects(source string, dialect commandDialect, depth int, wrappers []ShellWrapper, parent int64, inheritedRedirects []*syntax.Redirect) { + // Scripts recovered from interpreter input are detection-only in every dialect. + if depth > 0 { + defer a.markCommandsUnsafe(len(a.commands)) + } if a.halt { return } @@ -278,11 +277,11 @@ func (a *shellAnalyzer) parseDialectUnderRedirects(source string, dialect comman if !posixEnforcementShapeSafe(file) { a.enforcementUnsafe = true } - a.walk(source, file, depth, make(map[string]*syntax.Stmt), make(map[string]bool), wrappers, parent, inheritedRedirects) + a.walk(source, file, depth, make(map[string]*syntax.Stmt), make(map[string]bool), wrappers, 0) } -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) +func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functions map[string]*syntax.Stmt, activeFunctions map[string]bool, wrappers []ShellWrapper, parent int64) { + relations := a.buildPOSIXRelations(root) for statement, id := range relations.statements { if relations.parents[statement] == 0 { relations.parents[statement] = parent @@ -356,16 +355,12 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio a.unsafeStatements[ctx.statementID] = true } a.markPipelineUnsafe(ctx) - if command.Executable == "" { - return true - } + 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 || invocation.inputEnforcementUnsafe if add && !a.add(command) { return false } @@ -410,45 +405,36 @@ 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 || invocation.inputEnforcementUnsafe + innerCommand.enforcementUnsafe = !wrapperSafe 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 { innerWrappers := append(cloneWrappers(commandWrappers), wrapper) - a.parseDialectUnderRedirects(script, dialect, depth+1, innerWrappers, ctx.statementID, redirects) + a.parseDialect(script, dialect, depth+1, innerWrappers) a.markCommandsUnsafe(statementStart) } - if scripts := interpreterHeredocs(invocation.inputFDs, redirects); len(scripts) > 0 { + if script, ok := interpreterHeredoc(args, node.Redirs); ok { 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 || invocation.inputEnforcementUnsafe { - a.markCommandsUnsafe(innerStart) - } - } } } } if allowShellBuiltins { if script, ok := evalScript(source, args); ok { - a.parseDialectUnderRedirects(script, dialectPOSIX, depth+1, commandWrappers, ctx.statementID, redirects) + a.parseDialect(script, dialectPOSIX, depth+1, commandWrappers) a.markCommandsUnsafe(statementStart) } } @@ -456,7 +442,7 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio 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, ctx.statementID, redirects) + a.walk(source, body, depth+1, functions, activeFunctions, commandWrappers, ctx.statementID) delete(activeFunctions, name) } } @@ -836,10 +822,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 == "--" || len(flag) < 2 || flag[0] != '-' && flag[0] != '+' { + if flag == "--" || flag == "-" || !strings.HasPrefix(flag, "-") { return "", dialectAuto, ShellWrapper{}, false, nil } - if flag == "-o" || flag == "+o" || flag == "-O" || flag == "+O" || flag == "--rcfile" || flag == "--init-file" { + if flag == "-o" || flag == "-O" || flag == "--rcfile" || flag == "--init-file" { i++ continue } @@ -902,10 +888,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 == "--" || len(flag.Value) < 2 || flag.Value[0] != '-' && flag.Value[0] != '+' { + if flag.Value == "--" || flag.Value == "-" || !strings.HasPrefix(flag.Value, "-") { return "", dialectAuto, 0, false, nil } - if flag.Value == "-o" || flag.Value == "+o" || flag.Value == "-O" || flag.Value == "+O" || flag.Value == "--rcfile" || flag.Value == "--init-file" { + if flag.Value == "-o" || flag.Value == "-O" || flag.Value == "--rcfile" || flag.Value == "--init-file" { i++ continue } @@ -1028,310 +1014,67 @@ func joinProjectedScript(args []ShellArgument) (string, bool) { return strings.Join(values, " "), true } -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 - } +func interpreterHeredoc(args []*syntax.Word, redirects []*syntax.Redirect) (string, bool) { + if !shellReadsStdin(args) { + return "", false } - return scripts -} - -func heredocForFD(redirects []*syntax.Redirect, fd int64) (string, *syntax.Redirect, bool) { for i := len(redirects) - 1; i >= 0; i-- { redirect := redirects[i] - 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 - } + fd := defaultRedirectFD(redirect.Op) + if redirect.N != nil { + if redirect.N.Value != "0" { continue } - 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 + fd = 0 } - if redirectFD != fd { + if fd != 0 { continue } - if (redirect.Op == syntax.Hdoc || redirect.Op == syntax.DashHdoc) && redirect.Hdoc != nil { - script, ok := staticWord(redirect.Hdoc) - return script, redirect, ok - } - if redirect.Op == syntax.WordHdoc { - script, ok := staticWord(redirect.Word) - return script, redirect, ok + if (redirect.Op == syntax.Hdoc || redirect.Op == syntax.DashHdoc || redirect.Op == syntax.WordHdoc) && + redirect.Hdoc != nil { + return staticWord(redirect.Hdoc) } - return "", nil, false + return "", false } - return "", nil, false -} - -type shellInvocation struct { - inputFDs []int64 - inputEnforcementUnsafe bool - noExec bool + return "", false } -func inspectShellInvocation(args []*syntax.Word) shellInvocation { +func shellReadsStdin(args []*syntax.Word) bool { name, ok := commandName(args) - 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 - inputEnforcementUnsafe bool - bashShortOption bool - shOptionLetters bool - ) + if !ok || !isShellInterpreter(commandProgram(name)) { + return false + } + stdin := false for i := 1; i < len(args); i++ { - 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 len(flag) < 2 || flag[0] != '-' && flag[0] != '+' { - if !stdin { - inputFD, inputFound = shellInputPathFD(flag) - } - break - } - 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 + flag, ok := staticWord(args[i]) + if !ok { + return false } - if flag == "-o" || flag == "+o" { - i++ - 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 == "--" { + return stdin || i+1 == len(args) } - if flag == "-O" || flag == "+O" { + if flag == "-o" || flag == "-O" || flag == "--rcfile" || flag == "--init-file" { 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" { - startupDisabled = true - startupFound = false - continue + if isShellCommandFlag(flag) { + return false } - 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 - } - } + if flag == "-" { + stdin = true continue } - if strings.HasPrefix(flag, "--") { - 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] == '-' + if !strings.HasPrefix(flag, "-") { + return stdin } - if strings.ContainsRune(flag[1:], 'n') { - noExec = flag[0] == '-' - } - if program == "bash" && strings.ContainsRune(flag[1:], 'D') { - terminalNoExec = true - } - if flag[0] == '-' && strings.ContainsRune(flag[1:], 'c') { - inputFound = false - break - } - if strings.ContainsRune(flag[1:], 's') { - stdin = flag[0] == '-' - } - } - result := shellInvocation{ - inputEnforcementUnsafe: inputEnforcementUnsafe, - 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 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 + if len(flag) > 1 && flag[0] == '-' && !strings.HasPrefix(flag, "--") && + strings.ContainsRune(flag[1:], 's') { + stdin = true } } return true } -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_relations.go b/internal/rule/shell_relations.go index 9981bf2..2133d87 100644 --- a/internal/rule/shell_relations.go +++ b/internal/rule/shell_relations.go @@ -14,19 +14,24 @@ const ( ) type posixRelations struct { - statements map[*syntax.Stmt]int64 - pipelines map[*syntax.Stmt]int64 - parents map[*syntax.Stmt]int64 - inheritedRedirects map[*syntax.Stmt][]*syntax.Redirect + statements map[*syntax.Stmt]int64 + pipelines map[*syntax.Stmt]int64 + parents map[*syntax.Stmt]int64 } -func (a *shellAnalyzer) buildPOSIXRelations(root syntax.Node, inheritedRedirects []*syntax.Redirect) posixRelations { +func (a *shellAnalyzer) buildPOSIXRelations(root syntax.Node) posixRelations { relations := posixRelations{ - 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), + statements: make(map[*syntax.Stmt]int64), + pipelines: make(map[*syntax.Stmt]int64), + parents: make(map[*syntax.Stmt]int64), } + 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 { @@ -58,10 +63,7 @@ func (a *shellAnalyzer) buildPOSIXRelations(root syntax.Node, inheritedRedirects 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 @@ -69,38 +71,6 @@ func (a *shellAnalyzer) buildPOSIXRelations(root syntax.Node, inheritedRedirects 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 63c250a..6d5a31b 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 command, false, err + return ShellCommand{}, false, err } command.Redirects = append(command.Redirects, projected) } diff --git a/internal/sequence/sequence.go b/internal/sequence/sequence.go index 90ad285..676b5de 100644 --- a/internal/sequence/sequence.go +++ b/internal/sequence/sequence.go @@ -8,10 +8,8 @@ // // - High precision. Agent, sensor source, session id, and project path all // partition the window; artifact events additionally partition on their -// exact source path. A step predicate that errors evaluates as false, -// and a wall-clock window whose endpoint timestamps are missing or -// disordered never matches: every ambiguity resolves to a documented false -// negative, never a fabricated chain. +// exact source path. A step requires a clean detection or candidate match. +// Missing or disordered endpoint timestamps prevent a wall-clock match. // - Determinism. Matching is a pure function of the observed event order // and the events' own timestamps. The tracker never reads the wall clock, // so identical input yields identical matches on every run. @@ -195,11 +193,9 @@ func NewTracker(rules []*rule.SequenceRule, cfg Config) *Tracker { } // Observe feeds one event through every sequence rule and returns its detection -// and enforcement matches in rule load order. Step-evaluation errors are joined -// into the returned error alongside any matches (mirroring Engine.Eval); an -// erroring step counts as false, so an error can suppress a chain but never -// invent one. Observe is nil-safe — a nil Tracker observes nothing — so a -// caller whose rule set has no sequences skips the nil check. +// and enforcement matches in rule load order. Step-evaluation errors can +// accompany a clean candidate match, as in Engine.Eval. A nil Tracker observes +// nothing, so callers with no sequence rules can skip the nil check. func (t *Tracker) Observe(ev model.Event) (Observation, error) { if t == nil { return Observation{}, nil diff --git a/rules/builtin_test.go b/rules/builtin_test.go index 508ee91..a7693d3 100644 --- a/rules/builtin_test.go +++ b/rules/builtin_test.go @@ -85,11 +85,6 @@ func TestBuiltinCheckedEngineMetadataParity(t *testing.T) { checkedSequence.MaxMatches() != sourceSequence.MaxMatches() { t.Fatalf("checked sequence %d metadata differs from source", i) } - for step := range checkedSequence.StepCount() { - if checkedSequence.StepUsesShellCommands(step) != sourceSequence.StepUsesShellCommands(step) { - t.Fatalf("checked sequence %d step %d shell projection differs from source", i, step) - } - } } } From efe7b16e3aef605adcc00e4753f4a3db48000ef6 Mon Sep 17 00:00:00 2001 From: Adel Ka Date: Tue, 15 Sep 2026 22:13:54 +1000 Subject: [PATCH 5/6] fix(rule): preserve compound enforcement semantics --- cmd/numbat/hook_enforce_test.go | 3 + docs/enforcement.md | 28 ++++----- docs/rules.md | 11 ++-- internal/rule/engine.go | 11 ++-- internal/rule/engine_test.go | 4 +- .../rule/multicommand_enforcement_test.go | 46 ++++++++++---- internal/rule/shell.go | 4 ++ internal/sequence/sequence.go | 10 +-- internal/sequence/sequence_test.go | 61 +++++++++++-------- 9 files changed, 110 insertions(+), 68 deletions(-) diff --git a/cmd/numbat/hook_enforce_test.go b/cmd/numbat/hook_enforce_test.go index 761cdce..3a502fa 100644 --- a/cmd/numbat/hook_enforce_test.go +++ b/cmd/numbat/hook_enforce_test.go @@ -486,6 +486,9 @@ func TestEnforceCompoundCommandsPreservesInterpreterBoundary(t *testing.T) { {"missing bash option value", "bash --rcfile <<'EOF'\ncat .env\nEOF", false}, {"invalid option after startup file", "bash --rcfile /dev/fd/3 -i -Z 3<<'EOF'\ncat .env\nEOF", false}, {"consumed inherited input", "{ cat >/dev/null; sh; } <<'EOF'\ncat .env\nEOF", false}, + {"function body", `f(){ cat .env; }; f`, false}, + {"unset function body", `f(){ cat .env; }; unset -f f; f`, false}, + {"conditional function body", `if true; then f(){ :; }; else f(){ cat .env; }; fi; f`, false}, } { t.Run(test.name, func(t *testing.T) { command, err := json.Marshal(test.command) diff --git a/docs/enforcement.md b/docs/enforcement.md index 46378b1..2e8abf3 100644 --- a/docs/enforcement.md +++ b/docs/enforcement.md @@ -83,21 +83,19 @@ pipeline also excludes its nested substitutions. Malformed top-level input and truncated command lists stay detection-only. Sequencing, groups, subshells, background commands, negation, and substitutions -do not disable an otherwise eligible candidate. Both sides of `&&` and `||` -count as requested intent, even when one side cannot execute. Function calls -remain detection-only. Eligible commands in an invoked function body are -separate candidates. - -Detection evaluates the complete command list. For an `enforce: true` rule that -uses `shell_commands`, enforcement evaluates the same expression against each -eligible candidate. An input that passes the existing whole-input safety checks -uses its complete list for both decisions. Other event fields retain their full -values. - -A candidate match produces a finding and can deny the complete tool -input, even when full-list detection fails. Detection errors remain diagnostics. -A candidate error suppresses enforcement only when no candidate returns true. -Rules without `shell_commands` keep their existing behavior. +do not disable an otherwise eligible candidate. numbat does not predict POSIX +control flow: static commands in conditional branches and loop bodies count as +requested intent, as do both sides of `&&` and `||`. Function calls and commands +recovered from same-script function bodies remain detection-only because shell +state can replace or remove the definition before the call. + +Detection always evaluates the complete command list. For an `enforce: true` +rule that uses `shell_commands`, that evaluation must first match cleanly. When +the complete input is not enforcement-safe, numbat evaluates the same expression +against each eligible candidate to confirm that at least one candidate also +matches. Candidate evaluation can suppress a deny; it cannot create a finding +or change the rule's detection result. Other event fields retain their full +values. Rules without `shell_commands` keep their existing behavior. Scripts parsed from `eval` or child interpreter input, including heredocs, remain detection-only. Compound PowerShell and `cmd.exe` input also remain diff --git a/docs/rules.md b/docs/rules.md index b318bcb..73cf95f 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -389,11 +389,12 @@ visible `$WhatIfPreference = $true` for known cmdlet names and exact module-qualified forms. Ambient preference and command-resolution state are not inferred. -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. +For shell-derived blocking, `numbat` first evaluates the rule against the +complete command list. An already-matching rule must then match an eligible +parser-derived candidate. This safety check cannot create a detection. 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). ## Enforcement rules diff --git a/internal/rule/engine.go b/internal/rule/engine.go index 99fc5f9..48d8d87 100644 --- a/internal/rule/engine.go +++ b/internal/rule/engine.go @@ -81,13 +81,14 @@ 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 keeps a detection or candidate match separate from permission to enforce it. +// StepEvaluation keeps a detection match separate from permission to enforce it. type StepEvaluation struct { Match bool EnforcementMatch bool } -// EvalStep can return a clean candidate match alongside a full-list diagnostic. +// EvalStep evaluates detection against the complete event projection. Candidate +// evaluation can only narrow enforcement eligibility for a clean match. 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) @@ -106,7 +107,7 @@ func (s *SequenceRule) EvalStep(i int, activations SequenceActivations) (StepEva errs = append(errs, fmt.Errorf("rule %q step %d: candidate evaluation failed", s.rule.ID, i+1)) } return StepEvaluation{ - Match: evaluation.detectionMatch || evaluation.enforcementMatch, + Match: evaluation.detectionMatch, EnforcementMatch: evaluation.enforcementMatch, }, errors.Join(errs...) } @@ -731,7 +732,7 @@ func (e *Engine) Eval(ev model.Event) ([]Match, error) { if evaluation.candidateErr != nil { errs = append(errs, fmt.Errorf("rule %q: candidate evaluation failed", c.rule.ID)) } - if evaluation.detectionMatch || evaluation.enforcementMatch { + if evaluation.detectionMatch { matches = append(matches, Match{ Rule: cloneRule(c.rule), Event: ev, @@ -757,7 +758,7 @@ func evaluateExpression(expr compiledExpression, activations sequenceActivations enforcementMatch: detectionMatch && enforceEligible, detectionErr: detectionErr, } - if !enforceEligible || !expr.usesShellCommands || activations.shellEnforcementSafe { + if !detectionMatch || !enforceEligible || !expr.usesShellCommands || activations.shellEnforcementSafe { return evaluation } candidate, _, candidateErr := expr.candidateProgram.Eval(activations.detection) diff --git a/internal/rule/engine_test.go b/internal/rule/engine_test.go index cd39e03..e5062b9 100644 --- a/internal/rule/engine_test.go +++ b/internal/rule/engine_test.go @@ -444,8 +444,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 enforceable request match", staticBody, err) + if err != nil || len(staticBody) != 1 || staticBody[0].EnforcementMatch { + t.Fatalf("static function body = (%+v, %v), want detection-only match", staticBody, err) } } diff --git a/internal/rule/multicommand_enforcement_test.go b/internal/rule/multicommand_enforcement_test.go index 9e3ca6a..c1c0654 100644 --- a/internal/rule/multicommand_enforcement_test.go +++ b/internal/rule/multicommand_enforcement_test.go @@ -32,6 +32,8 @@ func TestMultiCommandEnforcementRegression(t *testing.T) { {name: "semicolon", command: `echo hi; cat .env`}, {name: "and", command: `false && cat .env`}, {name: "or", command: `true || cat .env`}, + {name: "conditional branch", command: `if false; then cat .env; fi`}, + {name: "loop body", command: `while false; do cat .env; done`}, {name: "subshell", command: `(cat .env)`}, {name: "group", command: `{ cat .env; }`}, {name: "background", command: `cat .env &`}, @@ -91,18 +93,16 @@ func TestMultiCommandEnforcementUsesPOSIXParserForExecCommand(t *testing.T) { func TestMultiCommandEnforcementCandidateEvaluation(t *testing.T) { tests := []struct { - name, expr, command string - wantErr, wantEnforce bool + name, expr, command string + wantErr, wantMatch, 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: "complete pipeline candidate", expr: `shell_commands.size() == 2 && shell_commands.exists(command, command.name == "cat") && shell_commands.exists(command, command.name == "grep")`, command: `true; cat .env | grep x`, wantEnforce: true}, - {name: "list all", expr: `event.event_type == "command.exec" && shell_commands.all(command, command.name == "cat")`, command: `cat one; true`, 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}, + {name: "candidate confirms complete-list match", expr: `shell_commands.exists(command, command.name == "cat")`, command: `cat .env; true`, wantMatch: true, wantEnforce: true}, + {name: "candidate cannot create size match", expr: `shell_commands.size() == 1 && shell_commands[0].name == "cat"`, command: `cat .env; true`}, + {name: "candidate cannot create pipeline-size match", expr: `shell_commands.size() == 2 && shell_commands.exists(command, command.name == "cat") && shell_commands.exists(command, command.name == "grep")`, command: `true; cat .env | grep x`}, + {name: "candidate cannot create all match", expr: `event.event_type == "command.exec" && shell_commands.all(command, command.name == "cat")`, command: `cat one; true`}, + {name: "full-list error prevents candidate evaluation", expr: `shell_commands.filter(command, command.name == "cat").size() == 1 && shell_commands[0].argv[1] == ".env"`, command: `true; cat .env`, 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`, wantMatch: 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`, wantMatch: true, wantEnforce: true}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -111,7 +111,11 @@ func TestMultiCommandEnforcementCandidateEvaluation(t *testing.T) { if (err != nil) != test.wantErr { t.Fatalf("Eval error = %v, want error %t", err, test.wantErr) } - enforced := len(matches) == 1 && matches[0].EnforcementMatch + matched := len(matches) == 1 + if matched != test.wantMatch { + t.Fatalf("Eval returned %+v, want match %t", matches, test.wantMatch) + } + enforced := matched && matches[0].EnforcementMatch if enforced != test.wantEnforce { t.Fatalf("Eval returned %+v, want enforcement %t", matches, test.wantEnforce) } @@ -200,6 +204,24 @@ func TestMultiCommandEnforcementDoesNotTreatFunctionCallsAsExecutables(t *testin } } +func TestMultiCommandEnforcementKeepsFunctionBodiesDetectionOnly(t *testing.T) { + eng := compoundRuleEngine(t, `shell_commands.exists(command, + command.name == "cat" && command.argv.exists(arg, arg == ".env"))`) + for _, command := range []string{ + `f(){ cat .env; }; f`, + `f(){ cat .env; }; unset -f f; f`, + `if true; then f(){ :; }; else f(){ cat .env; }; fi; f`, + } { + 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-body match", command, matches) + } + } +} + func TestMultiCommandEnforcementKeepsInterpreterScriptsDetectionOnly(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 e0a48cc..f289294 100644 --- a/internal/rule/shell.go +++ b/internal/rule/shell.go @@ -441,9 +441,13 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio if command.FunctionCall { if name, ok := commandName(call.Args); ok { if body := functions[name]; body != nil && depth < maxCommandExpansionDepth && !activeFunctions[name] { + start := len(a.commands) activeFunctions[name] = true a.walk(source, body, depth+1, functions, activeFunctions, commandWrappers, ctx.statementID) delete(activeFunctions, name) + // Shell state may replace or unset the definition before this + // call. Keep the recovered body available to detection only. + a.markCommandsUnsafe(start) } } } diff --git a/internal/sequence/sequence.go b/internal/sequence/sequence.go index 676b5de..bf7ff7d 100644 --- a/internal/sequence/sequence.go +++ b/internal/sequence/sequence.go @@ -8,8 +8,8 @@ // // - High precision. Agent, sensor source, session id, and project path all // partition the window; artifact events additionally partition on their -// exact source path. A step requires a clean detection or candidate match. -// Missing or disordered endpoint timestamps prevent a wall-clock match. +// exact source path. A step predicate that errors evaluates as false, and +// missing or disordered endpoint timestamps prevent a wall-clock match. // - Determinism. Matching is a pure function of the observed event order // and the events' own timestamps. The tracker never reads the wall clock, // so identical input yields identical matches on every run. @@ -193,9 +193,9 @@ func NewTracker(rules []*rule.SequenceRule, cfg Config) *Tracker { } // Observe feeds one event through every sequence rule and returns its detection -// and enforcement matches in rule load order. Step-evaluation errors can -// accompany a clean candidate match, as in Engine.Eval. A nil Tracker observes -// nothing, so callers with no sequence rules can skip the nil check. +// and enforcement matches in rule load order. Candidate evaluation can narrow +// enforcement eligibility but cannot create a detection step. A nil Tracker +// observes nothing, so callers with no sequence rules can skip the nil check. func (t *Tracker) Observe(ev model.Event) (Observation, error) { if t == nil { return Observation{}, nil diff --git a/internal/sequence/sequence_test.go b/internal/sequence/sequence_test.go index 78223f1..2d8a041 100644 --- a/internal/sequence/sequence_test.go +++ b/internal/sequence/sequence_test.go @@ -135,31 +135,44 @@ 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()) +func TestSequenceFinalStepCompoundCandidate(t *testing.T) { + for _, test := range []struct { + name, expr string + wantMatch bool + }{ + {name: "confirms complete-list match", expr: `shell_commands.exists(command, command.name == "cat")`, wantMatch: true}, + {name: "cannot create detection", expr: `shell_commands.size() == 1 && shell_commands[0].name == "cat"`}, + } { + t.Run(test.name, func(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 = test.expr + }) + 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) + 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(err) + } + want := 0 + if test.wantMatch { + want = 1 + } + if len(observation.Findings) != want || len(observation.EnforcementRules) != want { + t.Fatalf("compound observation = %+v, want match %t", observation, test.wantMatch) + } + }) } } From 5be521d363ea14a3510f6811ee2a663be46bdb16 Mon Sep 17 00:00:00 2001 From: Adel Ka Date: Wed, 16 Sep 2026 08:57:54 +1000 Subject: [PATCH 6/6] fix(rule): harden compound enforcement --- docs/enforcement.md | 26 ++--- internal/rule/checked.go | 2 +- internal/rule/engine.go | 100 +++++++++++------- internal/rule/engine_test.go | 40 +++++++ .../rule/multicommand_enforcement_test.go | 8 +- internal/rule/shell.go | 42 +++++--- internal/sequence/sequence_test.go | 27 +++++ 7 files changed, 175 insertions(+), 70 deletions(-) diff --git a/docs/enforcement.md b/docs/enforcement.md index 2e8abf3..2106b50 100644 --- a/docs/enforcement.md +++ b/docs/enforcement.md @@ -76,18 +76,20 @@ 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. -For POSIX input, a candidate is one parsed command or the direct members of one -`|` or `|&` pipeline. Each candidate must meet the existing checks for static -arguments, assignments, redirect targets, wrappers, and previews. An unsafe -pipeline also excludes its nested substitutions. Malformed top-level input and -truncated command lists stay detection-only. - -Sequencing, groups, subshells, background commands, negation, and substitutions -do not disable an otherwise eligible candidate. numbat does not predict POSIX -control flow: static commands in conditional branches and loop bodies count as -requested intent, as do both sides of `&&` and `||`. Function calls and commands -recovered from same-script function bodies remain detection-only because shell -state can replace or remove the definition before the call. +For POSIX input, a candidate contains the projections for one shell statement, +or for all direct members of one `|` or `|&` pipeline. Each candidate must meet +the existing checks for static arguments, assignments, redirect targets, +wrappers, and previews. An unsafe pipeline also excludes its nested +substitutions. Malformed top-level input and truncated command lists stay +detection-only. + +Statement lists, groups, subshells, background commands, negation, and +substitutions do not disable an otherwise eligible candidate. numbat does not +predict POSIX control flow: static commands in conditional branches and loop +bodies count as requested intent, as do both sides of `&&` and `||`. Function +calls and commands recovered from same-script function bodies remain +detection-only because shell state can replace or remove the definition before +the call. Detection always evaluates the complete command list. For an `enforce: true` rule that uses `shell_commands`, that evaluation must first match cleanly. When diff --git a/internal/rule/checked.go b/internal/rule/checked.go index 3814d8d..dbf494e 100644 --- a/internal/rule/checked.go +++ b/internal/rule/checked.go @@ -64,7 +64,7 @@ func BuildCheckedExpressions(sources []Source) (CheckedExpressions, error) { if err != nil { return nil, fmt.Errorf("rule %q: %w", r.ID, err) } - if _, err := programExpr(env, ast); err != nil { + if _, err := programExpr(env, ast, false); err != nil { return nil, fmt.Errorf("rule %q: %w", r.ID, err) } pb, err := cel.AstToCheckedExpr(ast) diff --git a/internal/rule/engine.go b/internal/rule/engine.go index 48d8d87..cfeddd7 100644 --- a/internal/rule/engine.go +++ b/internal/rule/engine.go @@ -28,10 +28,11 @@ import ( // compiled SequenceRules into a window tracker (internal/sequence) that owns // the partitioned state. type Engine struct { - env *cel.Env - rules []compiledRule - usesShellCommands bool - usesContent bool + env *cel.Env + rules []compiledRule + usesShellCommands bool + usesShellCandidates bool + usesContent bool } // compiledRule is one compiled rule of either shape: program is set for a @@ -56,14 +57,15 @@ const contentRuleCostLimit uint64 = 10_000_000 // distills the validated spec (window, cap) next to the compiled step // programs so a tracker never re-parses YAML fields on the hot path. type SequenceRule struct { - rule Rule - steps []compiledExpression - within time.Duration // 0 = no wall-clock window - withinEvents int // 0 = no event-count window - maxMatches int // resolved, >= 1 - usesShellCommands bool - usesContent bool - adapter types.Adapter + rule Rule + steps []compiledExpression + within time.Duration // 0 = no wall-clock window + withinEvents int // 0 = no event-count window + maxMatches int // resolved, >= 1 + usesShellCommands bool + usesShellCandidates bool + usesContent bool + adapter types.Adapter } // Rule returns the source rule (id, severity, tags, ...). @@ -152,12 +154,13 @@ func newEngine(sources []Source, checked CheckedExpressions) (*Engine, error) { } var ( - compiled []compiledRule - seen = map[string]string{} - expressions = map[string]compiledExpression{} - errs []error - usesShellCommands bool - usesContent bool + compiled []compiledRule + seen = map[string]string{} + expressions = map[string]compiledExpression{} + errs []error + usesShellCommands bool + usesShellCandidates bool + usesContent bool ) for _, source := range sources { for i := range source.Rules { @@ -187,6 +190,9 @@ func newEngine(sources []Source, checked CheckedExpressions) (*Engine, error) { if c.program.usesShellCommands { usesShellCommands = true } + if c.seq == nil && c.rule.IsEnforceEligible() && c.program.usesShellCommands { + usesShellCandidates = true + } if c.program.usesContent || c.seq != nil && c.seq.usesContent { usesContent = true } @@ -195,7 +201,13 @@ func newEngine(sources []Source, checked CheckedExpressions) (*Engine, error) { if len(errs) > 0 { return nil, errors.Join(errs...) } - return &Engine{env: env, rules: compiled, usesShellCommands: usesShellCommands, usesContent: usesContent}, nil + return &Engine{ + env: env, + rules: compiled, + usesShellCommands: usesShellCommands, + usesShellCandidates: usesShellCandidates, + usesContent: usesContent, + }, nil } // sourceLabel renders a stable, unambiguous source label for diagnostics. The @@ -300,8 +312,9 @@ func isRuleIDAlnum(c byte) bool { // type, known event fields). A sequence rule also distills its validated // window into the SequenceRule a tracker consumes. func compileRule(env *cel.Env, expressions map[string]compiledExpression, checked CheckedExpressions, r Rule) (compiledRule, error) { + needCandidate := r.IsEnforceEligible() if r.Sequence == nil { - prg, err := compileCachedExpr(env, expressions, checked, r.Expr) + prg, err := compileCachedExpr(env, expressions, checked, r.Expr, needCandidate) if err != nil { return compiledRule{}, err } @@ -311,7 +324,7 @@ func compileRule(env *cel.Env, expressions map[string]compiledExpression, checke usesShellCommands := false usesContent := false for i, st := range r.Sequence.Steps { - prg, err := compileCachedExpr(env, expressions, checked, st.Expr) + prg, err := compileCachedExpr(env, expressions, checked, st.Expr, needCandidate) if err != nil { return compiledRule{}, fmt.Errorf("step %d: %w", i+1, err) } @@ -325,22 +338,25 @@ func compileRule(env *cel.Env, expressions map[string]compiledExpression, checke return compiledRule{}, err } return compiledRule{rule: r, seq: &SequenceRule{ - rule: r, - steps: steps, - within: within, - withinEvents: r.Sequence.WithinEvents, - maxMatches: r.Sequence.resolvedMaxMatches(), - usesShellCommands: usesShellCommands, - usesContent: usesContent, - adapter: env.CELTypeAdapter(), + rule: r, + steps: steps, + within: within, + withinEvents: r.Sequence.WithinEvents, + maxMatches: r.Sequence.resolvedMaxMatches(), + usesShellCommands: usesShellCommands, + usesShellCandidates: needCandidate && usesShellCommands, + usesContent: usesContent, + adapter: env.CELTypeAdapter(), }}, nil } -func compileCachedExpr(env *cel.Env, expressions map[string]compiledExpression, checked CheckedExpressions, expr string) (compiledExpression, error) { +func compileCachedExpr(env *cel.Env, expressions map[string]compiledExpression, checked CheckedExpressions, expr string, needCandidate bool) (compiledExpression, error) { if compiled, ok := expressions[expr]; ok { - return compiled, nil + if !needCandidate || !compiled.usesShellCommands || compiled.candidateProgram != nil { + return compiled, nil + } } - compiled, err := compileExpr(env, checked, expr) + compiled, err := compileExpr(env, checked, expr, needCandidate) if err == nil { expressions[expr] = compiled } @@ -349,10 +365,10 @@ func compileCachedExpr(env *cel.Env, expressions map[string]compiledExpression, // compileExpr loads a matching checked expression when available, otherwise it // parses and type-checks the source. Runtime program construction is shared. -func compileExpr(env *cel.Env, checked CheckedExpressions, expr string) (compiledExpression, error) { +func compileExpr(env *cel.Env, checked CheckedExpressions, expr string, needCandidate bool) (compiledExpression, error) { if ast, ok := checkedExpressionAST(expr, checked); ok { if err := validateRuleAST(ast); err == nil { - if compiled, err := programExpr(env, ast); err == nil { + if compiled, err := programExpr(env, ast, needCandidate); err == nil { return compiled, nil } } @@ -361,7 +377,7 @@ func compileExpr(env *cel.Env, checked CheckedExpressions, expr string) (compile if err != nil { return compiledExpression{}, err } - return programExpr(env, ast) + return programExpr(env, ast, needCandidate) } func checkExpr(env *cel.Env, expr string) (*cel.Ast, error) { @@ -379,13 +395,16 @@ func validateRuleAST(ast *cel.Ast) error { if !ast.OutputType().IsExactType(cel.BoolType) { return fmt.Errorf("expr must be boolean, got %s", ast.OutputType()) } + if astReferencesGlobal(ast, shellCommandCandidatesVariable) { + return fmt.Errorf("expr uses reserved identifier %q", shellCommandCandidatesVariable) + } if err := checkEventFields(ast); err != nil { return err } return nil } -func programExpr(env *cel.Env, ast *cel.Ast) (compiledExpression, error) { +func programExpr(env *cel.Env, ast *cel.Ast, needCandidate bool) (compiledExpression, error) { usesShellCommands := astReferencesGlobal(ast, shellCommandsVariable) usesContent := astReferencesEventField(ast, "content") || astReferencesEventField(ast, "content_bytes") || @@ -399,7 +418,7 @@ func programExpr(env *cel.Env, ast *cel.Ast) (compiledExpression, error) { return compiledExpression{}, fmt.Errorf("program expr: %w", err) } var candidateProgram cel.Program - if usesShellCommands { + if needCandidate && usesShellCommands { candidateAST, err := shellCandidateAST(env, ast) if err != nil { return compiledExpression{}, err @@ -713,7 +732,7 @@ func (e *Engine) RuleIDs() []string { // the others; it is returned alongside any matches so callers can surface it // as a diagnostic without losing detections. func (e *Engine) Eval(ev model.Event) ([]Match, error) { - activations := prepareActivations(e.env.CELTypeAdapter(), ev, e.usesShellCommands) + activations := prepareActivations(e.env.CELTypeAdapter(), ev, e.usesShellCommands, e.usesShellCandidates) var ( matches []Match errs = []error{activations.err} @@ -761,6 +780,11 @@ func evaluateExpression(expr compiledExpression, activations sequenceActivations if !detectionMatch || !enforceEligible || !expr.usesShellCommands || activations.shellEnforcementSafe { return evaluation } + if expr.candidateProgram == nil { + evaluation.enforcementMatch = false + evaluation.candidateErr = errors.New("candidate enforcement program unavailable") + return evaluation + } candidate, _, candidateErr := expr.candidateProgram.Eval(activations.detection) evaluation.enforcementMatch = candidateErr == nil && asBool(candidate) evaluation.candidateErr = candidateErr diff --git a/internal/rule/engine_test.go b/internal/rule/engine_test.go index e5062b9..5a80487 100644 --- a/internal/rule/engine_test.go +++ b/internal/rule/engine_test.go @@ -194,6 +194,33 @@ func TestShellCommandsReferenceDetectionHonorsComprehensionScope(t *testing.T) { } } +func TestShellCandidateProgramsCompileOnlyForEnforcedRules(t *testing.T) { + const expr = `shell_commands.exists(command, command.name == "cat")` + monitor := mustEngine(t, Rule{ + ID: "t.monitor", Severity: model.SeverityLow, Expr: expr, + }) + if monitor.usesShellCandidates || monitor.rules[0].program.candidateProgram != nil { + t.Fatal("monitor-only rule compiled enforcement candidates") + } + + enforced := mustEngine(t, + Rule{ID: "t.monitor", Severity: model.SeverityLow, Expr: expr}, + Rule{ID: "t.enforced", Severity: model.SeverityLow, Enforce: boolPtr(true), Expr: expr}, + ) + if !enforced.usesShellCandidates || enforced.rules[1].program.candidateProgram == nil { + t.Fatal("enforced rule did not compile enforcement candidates") + } + + disabled := false + disabledEnforcement := mustEngine(t, + Rule{ID: "t.disabled", Severity: model.SeverityLow, Enabled: &disabled, Enforce: boolPtr(true), Expr: expr}, + Rule{ID: "t.monitor", Severity: model.SeverityLow, Expr: expr}, + ) + if disabledEnforcement.usesShellCandidates { + t.Fatal("disabled enforcement rule enabled candidate analysis") + } +} + func TestShadowedShellCommandsDoesNotAnalyzeCommand(t *testing.T) { eng := mustEngine(t, Rule{ ID: "shadow", @@ -843,6 +870,19 @@ func TestNewEngineRejectsEventAliases(t *testing.T) { } } +func TestNewEngineRejectsInternalCandidateVariable(t *testing.T) { + _, err := NewEngine([]Source{{Name: "test", Rules: []Rule{{ + ID: "t.internal_candidate", + Title: "internal candidate", + Version: "1", + Severity: model.SeverityLow, + Expr: `__numbat_shell_command_candidates.size() > 0`, + }}}}) + if err == nil || !strings.Contains(err.Error(), "reserved identifier") { + t.Fatalf("internal candidate variable error = %v, want reserved identifier", err) + } +} + func TestEvalErrorDoesNotLeakEventCommand(t *testing.T) { eng := mustEngine(t, Rule{ ID: "t.runtime_error", diff --git a/internal/rule/multicommand_enforcement_test.go b/internal/rule/multicommand_enforcement_test.go index c1c0654..a0d22fa 100644 --- a/internal/rule/multicommand_enforcement_test.go +++ b/internal/rule/multicommand_enforcement_test.go @@ -42,10 +42,6 @@ func TestMultiCommandEnforcementRegression(t *testing.T) { {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}, @@ -142,6 +138,10 @@ func TestMultiCommandEnforcementPreservesPipelineSafety(t *testing.T) { `f(){ cat .env; }; f | echo "$value"`, `f(){ cat .env; }; f |& echo "$value"`, `declare X=1 >"$(cat .env)" {fd}>out | true`, + `{ cat .env; } > "$target"`, + `(cat .env) > "$target"`, + `{ cat .env; } {fd}>out`, + `(cat .env) {fd}>out`, } { matches, _ := eng.Eval(model.Event{EventType: model.EventCommandExec, ToolName: "bash", Command: command}) if len(matches) != 1 || matches[0].EnforcementMatch { diff --git a/internal/rule/shell.go b/internal/rule/shell.go index f289294..96574fe 100644 --- a/internal/rule/shell.go +++ b/internal/rule/shell.go @@ -41,7 +41,7 @@ type sequenceActivations struct { // prepareActivations adds the rule-only shell command projection when a // compiled expression references it. Detection sees every statically proven // command. The projection is never emitted. -func prepareActivations(adapter types.Adapter, ev model.Event, needShellCommands bool) sequenceActivations { +func prepareActivations(adapter types.Adapter, ev model.Event, needShellCommands, needShellCandidates bool) sequenceActivations { detection := ev.CELActivation() detection["event"] = adapter.NativeToValue(detection["event"]) if !needShellCommands { @@ -51,9 +51,11 @@ func prepareActivations(adapter types.Adapter, ev model.Event, needShellCommands shellEnforcementSafe: true, } } - analysis := analyzeEventShellCommandsDetailed(ev) + analysis := analyzeEventShellCommandsDetailed(ev, needShellCandidates) detection[shellCommandsVariable] = shellCommandList(adapter, analysis.commands) - detection[shellCommandCandidatesVariable] = shellCommandCandidateList(adapter, analysis.enforcementCandidates) + if needShellCandidates { + detection[shellCommandCandidatesVariable] = shellCommandCandidateList(adapter, analysis.enforcementCandidates) + } return sequenceActivations{ detection: detection, shellUsable: analysis.usable, @@ -114,13 +116,13 @@ func PrepareSequenceActivations(ev model.Event, rules []*SequenceRule) (Sequence if len(rules) > 0 { adapter = rules[0].adapter } + needShellCommands := false + needShellCandidates := false for _, r := range rules { - if r.usesShellCommands { - prepared := prepareActivations(adapter, ev, true) - return SequenceActivations{prepared: prepared}, prepared.err - } + needShellCommands = needShellCommands || r.usesShellCommands + needShellCandidates = needShellCandidates || r.usesShellCandidates } - prepared := prepareActivations(adapter, ev, false) + prepared := prepareActivations(adapter, ev, needShellCommands, needShellCandidates) return SequenceActivations{prepared: prepared}, prepared.err } @@ -165,20 +167,24 @@ func analyzeShellCommands(source string) ([]ShellCommand, bool, error) { } func analyzeEventShellCommands(ev model.Event) ([]ShellCommand, bool, error) { - analysis := analyzeEventShellCommandsDetailed(ev) + analysis := analyzeEventShellCommandsDetailed(ev, false) return analysis.commands, analysis.usable, analysis.err } -func analyzeEventShellCommandsDetailed(ev model.Event) shellAnalysis { - return analyzeShellCommandsDetailed(ev.Command, commandDialectHint(ev)) +func analyzeEventShellCommandsDetailed(ev model.Event, needCandidates bool) shellAnalysis { + return analyzeShellCommandsDetailedWithCandidates(ev.Command, commandDialectHint(ev), needCandidates) } func analyzeShellCommandsAs(source string, dialect commandDialect) ([]ShellCommand, bool, error) { - analysis := analyzeShellCommandsDetailed(source, dialect) + analysis := analyzeShellCommandsDetailedWithCandidates(source, dialect, false) return analysis.commands, analysis.usable, analysis.err } func analyzeShellCommandsDetailed(source string, dialect commandDialect) shellAnalysis { + return analyzeShellCommandsDetailedWithCandidates(source, dialect, true) +} + +func analyzeShellCommandsDetailedWithCandidates(source string, dialect commandDialect, needCandidates bool) shellAnalysis { if strings.TrimSpace(source) == "" { return shellAnalysis{usable: true, enforcementSafe: true} } @@ -199,7 +205,7 @@ func analyzeShellCommandsDetailed(source string, dialect commandDialect) shellAn } } var candidates [][]ShellCommand - if !a.halt { + if needCandidates && !a.halt { candidates = posixEnforcementCandidates(a.commands, a.unsafePipelines, a.unsafeStatements, a.statementParents) } return shellAnalysis{ @@ -330,12 +336,18 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio 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) + if node.Cmd != nil { + a.markStatementsUnsafe(node.Cmd, ctx.statementIDs) } a.markPipelineUnsafe(ctx) return true } + if !commandSafeForEnforcement(redirectCommand) { + if node.Cmd != nil { + a.markStatementsUnsafe(node.Cmd, ctx.statementIDs) + } + a.markPipelineUnsafe(ctx) + } if node.Cmd == nil && add { return a.add(redirectCommand) } diff --git a/internal/sequence/sequence_test.go b/internal/sequence/sequence_test.go index 2d8a041..a750c3d 100644 --- a/internal/sequence/sequence_test.go +++ b/internal/sequence/sequence_test.go @@ -176,6 +176,33 @@ func TestSequenceFinalStepCompoundCandidate(t *testing.T) { } } +func TestSequenceEarlierStepCompoundCandidate(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.exists(command, command.name == "finish")` + }) + 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 = "true; prep" + }) + if observation, err := tr.Observe(prep); err != nil || len(observation.Findings) != 0 { + t.Fatalf("prep observation = %+v, %v", observation, err) + } + finish := ev("e2", "2026-06-01T10:01:00Z", model.EventCommandExec, func(e *model.Event) { + e.Command = "finish" + }) + observation, err := tr.Observe(finish) + if err != nil { + t.Fatal(err) + } + if len(observation.Findings) != 1 || len(observation.EnforcementRules) != 1 { + t.Fatalf("sequence observation = %+v, want enforceable match", observation) + } +} + func TestSequenceShellAnalysisErrorIsReported(t *testing.T) { r := secretThenEgress(func(spec *rule.SequenceSpec) { spec.Steps[0].Expr = `shell_commands.size() == 0`