diff --git a/cmd/numbat/hook_enforce_test.go b/cmd/numbat/hook_enforce_test.go index 5304474..3a502fa 100644 --- a/cmd/numbat/hook_enforce_test.go +++ b/cmd/numbat/hook_enforce_test.go @@ -472,6 +472,46 @@ 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}, + {"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) + 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 680c25e..2106b50 100644 --- a/docs/enforcement.md +++ b/docs/enforcement.md @@ -76,20 +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. 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. +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 +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 +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/docs/rules.md b/docs/rules.md index a3edd18..73cf95f 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -389,16 +389,12 @@ 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 +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 require a shell projection. See [Enforcement](enforcement.md). +not need a shell projection. See [Enforcement](enforcement.md). ## Enforcement rules @@ -410,9 +406,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.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/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..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 @@ -45,6 +46,7 @@ type compiledRule struct { type compiledExpression struct { program cel.Program + candidateProgram cel.Program usesShellCommands bool usesContent bool } @@ -55,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, ...). @@ -80,28 +83,35 @@ 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) { - if i < 0 || i >= len(s.steps) { - return false, 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) - } - return asBool(out), nil +// StepEvaluation keeps a detection match separate from permission to enforce it. +type StepEvaluation struct { + Match bool + EnforcementMatch bool } -// StepUsesShellCommands reports whether step i depends on the derived command -// projection. -func (s *SequenceRule) StepUsesShellCommands(i int) bool { +// 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 false + return StepEvaluation{}, fmt.Errorf("rule %q: step index %d out of range", s.rule.ID, i) + } + 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 s.steps[i].usesShellCommands + return StepEvaluation{ + Match: evaluation.detectionMatch, + EnforcementMatch: evaluation.enforcementMatch, + }, errors.Join(errs...) } // newEnv builds the CEL environment shared by every rule. `event` uses emitted @@ -119,6 +129,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")))), ) } @@ -143,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 { @@ -178,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 } @@ -186,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 @@ -291,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 } @@ -302,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) } @@ -316,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 } @@ -340,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 } } @@ -352,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) { @@ -370,13 +395,17 @@ 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") || astReferencesEventField(ast, "content_truncated") @@ -388,13 +417,63 @@ 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 needCandidate && 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)" + +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) { @@ -653,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} @@ -665,26 +744,53 @@ 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 { 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 !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 + 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..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", @@ -532,7 +559,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 +577,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 +627,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 +648,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 +678,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`}, @@ -844,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 new file mode 100644 index 0000000..a0d22fa --- /dev/null +++ b/internal/rule/multicommand_enforcement_test.go @@ -0,0 +1,246 @@ +package rule + +import ( + "runtime" + "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: "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 &`}, + {name: "negation", command: `! cat .env`}, + {name: "heredoc", command: "cat .env <<'EOF'\nbody\nEOF"}, + {name: "command substitution", command: `echo "$(cat .env)"`}, + {name: "standalone command substitution", command: `$(cat .env)`, wantErr: true}, + {name: "redirect substitution", command: `{ true; } > "$(cat .env)"`}, + {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"))`) + commands := []string{ + "if (true)\nthen\ncat .env\nfi", + } + 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, + 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, wantMatch, wantEnforce bool + }{ + {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) { + 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) + } + 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) + } + }) + } +} + +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`, + `{ 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 { + t.Fatalf("Eval(%q) returned %+v, want detection-only pipeline match", command, 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) + } + } +} + +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"))`) + 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 c510339..44e5d4e 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,12 +131,10 @@ 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) { - t.Fatalf("StepUsesShellCommands(%d) = true, want false", step) - } } } diff --git a/internal/rule/shell.go b/internal/rule/shell.go index a4096c6..96574fe 100644 --- a/internal/rule/shell.go +++ b/internal/rule/shell.go @@ -3,6 +3,7 @@ package rule import ( "errors" "fmt" + "runtime" "strings" "github.com/google/cel-go/common/types" @@ -13,11 +14,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 @@ -39,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 { @@ -49,8 +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) + if needShellCandidates { + detection[shellCommandCandidatesVariable] = shellCommandCandidateList(adapter, analysis.enforcementCandidates) + } return sequenceActivations{ detection: detection, shellUsable: analysis.usable, @@ -91,40 +96,34 @@ 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 shares one event's analysis across steps without exposing its safety flags. 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 } + needShellCommands := false + needShellCandidates := false 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, - } - } - } - prepared := prepareActivations(adapter, ev, false) - return SequenceActivations{ - Detection: prepared.detection, - ShellUsable: prepared.shellUsable, - ShellEnforcementSafe: prepared.shellEnforcementSafe, - Err: prepared.err, + needShellCommands = needShellCommands || r.usesShellCommands + needShellCandidates = needShellCandidates || r.usesShellCandidates } + prepared := prepareActivations(adapter, ev, needShellCommands, needShellCandidates) + return SequenceActivations{prepared: prepared}, prepared.err } type fatalShellAnalysisError struct { @@ -149,14 +148,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) { @@ -164,24 +167,32 @@ 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.ToolName)) +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} } - 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 +204,40 @@ func analyzeShellCommandsDetailed(source string, dialect commandDialect) shellAn } } } + var candidates [][]ShellCommand + if needCandidates && !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) { + // Scripts recovered from interpreter input are detection-only in every dialect. + if depth > 0 { + defer a.markCommandsUnsafe(len(a.commands)) + } if a.halt { return } @@ -260,11 +283,17 @@ func (a *shellAnalyzer) parseDialect(source string, dialect commandDialect, dept 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, 0) } -func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functions map[string]*syntax.Stmt, activeFunctions map[string]bool, wrappers []ShellWrapper) { +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 + } + a.statementParents[id] = relations.parents[statement] + } syntax.Walk(root, func(node syntax.Node) bool { if a.halt { return false @@ -280,7 +309,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,21 +332,41 @@ 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 node.Cmd != nil { + a.markStatementsUnsafe(node.Cmd, ctx.statementIDs) + } + a.markPipelineUnsafe(ctx) return true } - if add { - return a.add(command) + 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) } } + 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) + if ctx.pipelineID != 0 { + a.unsafeStatements[ctx.statementID] = true + } + a.markPipelineUnsafe(ctx) return true } if name, ok := commandName(call.Args); ok && functions[name] != nil { @@ -321,6 +380,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 +388,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 +408,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) @@ -359,6 +419,7 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio a.report(err) break } + innerCommand.enforcementUnsafe = !wrapperSafe if add && !a.add(innerCommand) { return false } @@ -368,9 +429,9 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio 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.markCommandsUnsafe(statementStart) } if script, ok := interpreterHeredoc(args, node.Redirs); ok { wrapper, err := projectInterpreterWrapper(source, args) @@ -379,21 +440,26 @@ func (a *shellAnalyzer) walk(source string, root syntax.Node, depth int, functio } else { innerWrappers := append(cloneWrappers(commandWrappers), wrapper) a.parseDialect(script, dialectPOSIX, depth+1, innerWrappers) + a.markCommandsUnsafe(statementStart) } } } if allowShellBuiltins { if script, ok := evalScript(source, args); ok { - a.enforcementUnsafe = true a.parseDialect(script, dialectPOSIX, depth+1, commandWrappers) + a.markCommandsUnsafe(statementStart) } } 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) + 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/rule/shell_enforcement.go b/internal/rule/shell_enforcement.go index 098de3e..1d9d523 100644 --- a/internal/rule/shell_enforcement.go +++ b/internal/rule/shell_enforcement.go @@ -6,6 +6,82 @@ 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 && !commandSafeForEnforcement(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 commandSafeForEnforcement(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 (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/sequence/sequence.go b/internal/sequence/sequence.go index cae7007..bf7ff7d 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 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. @@ -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. 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 @@ -327,28 +323,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..a750c3d 100644 --- a/internal/sequence/sequence_test.go +++ b/internal/sequence/sequence_test.go @@ -135,6 +135,74 @@ func TestSequenceStepUsesShellCommands(t *testing.T) { } } +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(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) + } + }) + } +} + +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` 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, 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) - } - } } }