diff --git a/internal/rules/concerns/shellread/codesearch.go b/internal/rules/concerns/shellread/codesearch.go index cb3255c..d993c11 100644 --- a/internal/rules/concerns/shellread/codesearch.go +++ b/internal/rules/concerns/shellread/codesearch.go @@ -2,6 +2,7 @@ package shellread import ( "os" + "strings" "goodkind.io/agent-gate/internal/rules/concerns/shellparse" "goodkind.io/gksyntax/shelldecomp" @@ -93,12 +94,22 @@ func extractCodeSearchInto(command, cwd, home string, tools map[string]bool, add add(target.Path) } + // The enumerator and recursive-structure layers below split text into shell + // fields, so they must only see shell. An embedded program's source is not + // shell, and reading it as shell fabricates directories out of call syntax + // (a ruby body yields the directory `Dir.glob "/abs/lmd`) or resolves a glob + // base to cwd when a language construct like Dir[...] is mistaken for a + // wildcard. Each embedded region is handled on its own terms further down, + // by its analyzer's reads or by a recursive shell scan of an opaque body, so + // blanking the regions here loses nothing those layers already cover. + shellText := maskEmbeddedRegions(command, decomposition.EmbeddedRegions()) + // Enumerator-driven code search (find/fd/git ls-files feeding a declared // tool over file contents). shelldecomp surfaces find's own operands as // reads, which over- and under-count the enumerated directory, so the // enumerator layer computes the real target and the find/fd/git-ls-files // reads above are skipped by the declared-tools filter. - for _, target := range resolvableTargets(enumeratorCodeSearchTargets(command, cwd, tools)) { + for _, target := range resolvableTargets(enumeratorCodeSearchTargets(shellText, cwd, tools)) { add(target.Path) } @@ -106,7 +117,7 @@ func extractCodeSearchInto(command, cwd, home string, tools map[string]bool, add // without a shallow -maxdepth, git ls-files, a recursive ** glob). shelldecomp // models these as filename enumeration, not a content read, so the directory // they walk is computed here and left for the index-aware validator to judge. - for _, target := range resolvableTargets(recursiveEnumerationTargets(command, cwd)) { + for _, target := range resolvableTargets(recursiveEnumerationTargets(shellText, cwd)) { add(target.Path) } @@ -123,6 +134,76 @@ func parseCommand(command, cwd, home string, resolver shelldecomp.FileResolver) return shelldecomp.ParseWithOptions(command, cwd, shelldecomp.Options{Home: home, FileResolver: resolver}) } +// maskEmbeddedRegions returns command with each foreign-language embedded +// region's source blanked to spaces, so a shell-field scan of the result sees +// only shell. Blanking preserves length, byte offsets, and field boundaries, so +// the surrounding shell splits exactly as it did before. +// +// A region reports a byte span for a heredoc body but reports 0..0 for an +// interpreter -c or -e operand, so the span is used when it is present and +// usable and the region's text is located in the command otherwise. A region +// whose text cannot be located is left alone rather than guessed at. +func maskEmbeddedRegions(command string, regions []shelldecomp.EmbeddedRegion) string { + if len(regions) == 0 { + return command + } + masked := []byte(command) + for _, region := range regions { + if !isForeignLanguageRegion(region.Lang) { + continue + } + start, end, ok := regionSpan(command, region) + if !ok { + continue + } + for index := start; index < end; index++ { + masked[index] = ' ' + } + } + return string(masked) +} + +// isForeignLanguageRegion reports whether a region's body is a language other +// than shell, and therefore must not be read as shell fields. +// +// A shell region stays visible because reading shell as shell is correct and +// because the enumerator layer depends on it: `find DIR | xargs grep` and +// `find DIR -exec grep` both carry the searcher as a nested shell region, and +// blanking it would hide the searcher that makes the enumerated directory a +// content-search target. An opaque region stays visible because it has no +// grammar, so the recursive shell scan of its body is the only reading +// available and the outer scan is a backstop for when the depth budget runs +// out. Every other language is program source with its own analyzer. +func isForeignLanguageRegion(lang shelldecomp.Lang) bool { + switch lang { + case shelldecomp.LangPython, shelldecomp.LangJavaScript, shelldecomp.LangRuby, + shelldecomp.LangPerl, shelldecomp.LangAppleScript, shelldecomp.LangSQL, + shelldecomp.LangAwk, shelldecomp.LangJQ, shelldecomp.LangSed, + shelldecomp.LangRegex: + return true + case shelldecomp.LangShell, shelldecomp.LangOpaque, shelldecomp.LangUnknown: + return false + default: + return false + } +} + +// regionSpan returns the byte range of one embedded region within command, and +// whether a usable range was found. +func regionSpan(command string, region shelldecomp.EmbeddedRegion) (int, int, bool) { + if region.EndByte > region.StartByte && int(region.EndByte) <= len(command) { + return int(region.StartByte), int(region.EndByte), true + } + if region.Text == "" { + return 0, 0, false + } + offset := strings.Index(command, region.Text) + if offset < 0 { + return 0, 0, false + } + return offset, offset + len(region.Text), true +} + // resolvableTargets drops enumerator operands whose path still carries a shell // expansion (a $variable or `command` substitution) that agent-gate cannot // evaluate. The enumerator layer resolves directories with the package's own diff --git a/internal/rules/concerns/shellread/codesearch_embedded.go b/internal/rules/concerns/shellread/codesearch_embedded.go index 176079a..90a75a1 100644 --- a/internal/rules/concerns/shellread/codesearch_embedded.go +++ b/internal/rules/concerns/shellread/codesearch_embedded.go @@ -71,6 +71,21 @@ func extractEmbeddedCodeSearchInto(decomposition *shelldecomp.Decomposition, cwd // language whose analyzer is registered in shelldecomp (python, awk) appears // here; a region whose language is absent folds nothing, so adding a new // analyzer to shelldecomp without a matching entry leaves it inert here. +// +// Two limits decide how much of an interpreter body a rule actually sees, and +// both fail quiet rather than loud. +// +// A rule that does not declare the language's tools folds nothing even when the +// analyzer resolved the reads, so a search_tools list of only grep-family names +// leaves every python body invisible. TestExtractCodeSearchTargetsPythonToolGate +// pins that behavior. +// +// A language with a grammar but no read analyzer parses into a non-nil Parsed +// that yields no read targets, so it folds nothing here no matter what a rule +// declares. As of this writing that covers ruby and javascript, and php parses +// to no region at all. Declaring ruby or node in search_tools therefore buys no +// coverage of a `ruby -e` or `node -e` body; closing that needs read analyzers +// in gksyntax, not config. var regionFoldTools = map[shelldecomp.Lang][]string{ shelldecomp.LangPython: {"python", "python3"}, shelldecomp.LangAwk: {"awk", "gawk"}, diff --git a/internal/rules/concerns/shellread/codesearch_enum.go b/internal/rules/concerns/shellread/codesearch_enum.go index ed46fe7..e9775ee 100644 --- a/internal/rules/concerns/shellread/codesearch_enum.go +++ b/internal/rules/concerns/shellread/codesearch_enum.go @@ -260,29 +260,6 @@ func findIsShallow(fields []string) bool { return false } -// nonPathOperandChars are characters that cannot appear in the directory part -// of a real shell glob operand but do appear when an embedded program's source -// is scanned as if it were shell. A python body quote-stripped to -// glob.glob(/repo/**/*.go, recursive=True) yields the directory -// glob.glob(/repo, which names no directory on disk. The embedded region's own -// analyzer already resolves that program's real read targets, so dropping the -// fabricated operand loses no coverage and stops a validator run against a path -// that cannot exist. -// -// The set is deliberately only the call parentheses. A directory name may -// legitimately contain a space, a comma, an equals sign, or a quote, and -// shellFields preserves those inside a quoted or escaped field, so rejecting -// them would silently drop a real search target and let a read of an indexed -// repository through. A $ or backtick operand is already dropped downstream by -// resolvableTargets, which refuses a path still carrying a shell expansion. -const nonPathOperandChars = "()" - -// isPlausiblePathOperand reports whether dir could be the directory part of a -// shell path operand. -func isPlausiblePathOperand(dir string) bool { - return !strings.ContainsAny(dir, nonPathOperandChars) -} - // recursiveGlobDirs returns the base directory of each recursive ** glob token in // the command: the literal prefix before the first wildcard, taken up to its last // path separator and resolved against cwd. A command reading files matched by a @@ -307,9 +284,6 @@ func recursiveGlobDirs(command, cwd string) []string { dir = "/" } } - if !isPlausiblePathOperand(dir) { - continue - } out = append(out, resolvePath(cwd, dir)) } return out diff --git a/internal/rules/concerns/shellread/codesearch_glob_operand_test.go b/internal/rules/concerns/shellread/codesearch_glob_operand_test.go index 6c3a6f7..f38062b 100644 --- a/internal/rules/concerns/shellread/codesearch_glob_operand_test.go +++ b/internal/rules/concerns/shellread/codesearch_glob_operand_test.go @@ -8,8 +8,8 @@ import ( // TestExtractCodeSearchTargetsPythonGlobRoot covers the glob-driven content // search an agent reaches for when it wants to read a repository without naming // a searcher. The python region's own analyzer resolves the glob root, and the -// shell-level ** scan no longer fabricates a directory out of the program's -// source text, so the enumerated root is the only target. +// shell-level ** scan no longer reads the program's source as shell fields, so +// the enumerated root is the only target. func TestExtractCodeSearchTargetsPythonGlobRoot(t *testing.T) { const cwd = "/repo" @@ -25,31 +25,74 @@ func TestExtractCodeSearchTargetsPythonGlobRoot(t *testing.T) { } } -// TestRecursiveGlobDirsRejectsProgramSourceOperands covers the operand filter -// directly: a real shell glob still resolves its base directory, while an -// operand carrying call syntax from an embedded program's source resolves -// nothing. -func TestRecursiveGlobDirsRejectsProgramSourceOperands(t *testing.T) { +// TestExtractCodeSearchTargetsForeignRegionNotReadAsShell covers the shapes a +// shell-field scan of program source fabricates: a call operand becomes a +// directory that cannot exist, and a language construct opening with a bracket +// is mistaken for a glob wildcard, which resolves the base to cwd and silently +// displaces the directory the program actually reads. Neither may contribute a +// target. +func TestExtractCodeSearchTargetsForeignRegionNotReadAsShell(t *testing.T) { const cwd = "/repo" cases := []struct { name string command string - want []string }{ - {"shell relative glob", "cat src/**/*.go", []string{"/repo/src"}}, - {"shell absolute glob", "cat /abs/pkg/**/*.go", []string{"/abs/pkg"}}, - {"shell glob at root", "cat **/*.go", []string{"/repo"}}, - {"python call operand", `glob.glob("/abs/lmd/**/*.go", recursive=True)`, nil}, - {"python keyword argument operand", `sorted(glob.iglob(root+"/**/*.go"))`, nil}, + {"ruby bracket index is not a glob", `ruby -e 'Dir["/abs/indexed/**/*.rb"].each { |f| puts File.read(f) }'`}, + {"ruby call operand is not a directory", `ruby -e 'Dir.glob "/abs/lmd/**/*.go"'`}, + {"python call operand is not a directory", `python3 -c 'import glob; [open(p).read() for p in glob.glob("/abs/lmd/**/*.go", recursive=True)]'`}, + } - // A directory name may legitimately contain these characters. Rejecting - // them would drop the only layer that names the target for a - // non-searcher command, so an indexed repository would read unblocked. - {"quoted directory with a space", `cat "/abs/My Repo/**/*.go"`, []string{"/abs/My Repo"}}, - {"escaped directory with a space", `cat /abs/My\ Repo/**/*.go`, []string{"/abs/My Repo"}}, - {"directory with a comma", "cat /abs/a,b/**/*.go", []string{"/abs/a,b"}}, - {"directory with an equals sign", "cat /abs/pkg=v1/**/*.go", []string{"/abs/pkg=v1"}}, + tools := []string{"grep", "rg", "python", "python3", "ruby"} + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + for _, target := range targetPaths(ExtractCodeSearchTargets(tc.command, cwd, tools, nil)) { + if target == cwd { + t.Fatalf("%q resolved the working directory as a search target", tc.command) + } + if !isCleanAbsoluteDir(target) { + t.Fatalf("%q produced a fabricated target %q", tc.command, target) + } + } + }) + } +} + +// isCleanAbsoluteDir reports whether a resolved target looks like a path a shell +// operand could name, rather than a fragment of program source. It exists only +// so the test above can state what a fabricated target looks like. +func isCleanAbsoluteDir(target string) bool { + if target == "" || target[0] != '/' { + return false + } + for _, r := range target { + if r == '(' || r == ')' || r == '"' || r == '\'' { + return false + } + } + return true +} + +// TestRecursiveGlobDirsResolvesRealShellOperands covers the operands a real +// shell command writes, including directory names holding characters a +// character-level filter would reject. A duplicated macOS download lands at a +// name with parentheses, and dropping it removes the only layer that names that +// directory for a command whose argv0 is not a declared searcher. +func TestRecursiveGlobDirsResolvesRealShellOperands(t *testing.T) { + const cwd = "/repo" + + cases := []struct { + name string + command string + want []string + }{ + {"relative glob", "cat src/**/*.go", []string{"/repo/src"}}, + {"absolute glob", "cat /abs/pkg/**/*.go", []string{"/abs/pkg"}}, + {"glob at root", "cat **/*.go", []string{"/repo"}}, + {"parenthesized directory", `cat "/abs/My Repo (1)/**/*.go"`, []string{"/abs/My Repo (1)"}}, + {"escaped space directory", `cat /abs/My\ Repo/**/*.go`, []string{"/abs/My Repo"}}, + {"comma directory", "cat /abs/a,b/**/*.go", []string{"/abs/a,b"}}, + {"equals directory", "cat /abs/pkg=v1/**/*.go", []string{"/abs/pkg=v1"}}, } for _, tc := range cases { @@ -61,3 +104,29 @@ func TestRecursiveGlobDirsRejectsProgramSourceOperands(t *testing.T) { }) } } + +// TestMaskEmbeddedRegionsKeepsShellRegions covers the enumerator layer's +// dependency on nested shell staying visible: the searcher in `xargs rg` and in +// `find -exec grep` is carried as a nested shell region, and blanking it would +// hide the searcher that makes the enumerated directory a content-search target. +func TestMaskEmbeddedRegionsKeepsShellRegions(t *testing.T) { + const cwd = "/repo" + + cases := []struct { + name string + command string + want []string + }{ + {"xargs searcher", `fd -e swift | xargs rg toolchain`, []string{"/repo"}}, + {"find exec searcher", `find Sources -name '*.go' -exec grep -l x {} +`, []string{"/repo/Sources"}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := targetPaths(ExtractCodeSearchTargets(tc.command, cwd, enumTestTools, nil)) + if !slices.Equal(got, tc.want) { + t.Fatalf("ExtractCodeSearchTargets(%q) = %v, want %v", tc.command, got, tc.want) + } + }) + } +} diff --git a/internal/rules/exec_expand.go b/internal/rules/exec_expand.go new file mode 100644 index 0000000..16f4d2b --- /dev/null +++ b/internal/rules/exec_expand.go @@ -0,0 +1,121 @@ +package rules + +import ( + "context" + + "goodkind.io/agent-gate/internal/config" + execconcern "goodkind.io/agent-gate/internal/rules/concerns/exec" +) + +// runExpandedCommands runs one validator invocation per expanded target and +// combines their verdicts into the condition's verdict. +// +// Without for_each there is exactly one command and its verdict is the answer. +// With for_each, match_mode decides how the per-target verdicts combine: "any" +// blocks as soon as one target blocks, and "all" blocks only when every target +// blocks. +func (r *ExecRuntime) runExpandedCommands( + ctx context.Context, + ruleName string, + c *config.Condition, + commands [][]string, + stdin []byte, + env []string, +) execconcern.Verdict { + if len(commands) == 0 { + return execconcern.Verdict{Block: false, Message: "", Output: "", Errored: false} + } + + forEach := c.ForEachSelector().Selector != config.FieldSelectorInvalid + matchAll := forEach && c.MatchMode == config.ExecMatchAll + firstBlockMessage := "" + firstErrored := execconcern.Verdict{Block: false, Message: "", Output: "", Errored: false} + sawErrored := false + for _, command := range commands { + verdict := r.runExpandedCommandWithRetry(ctx, ruleName, c, command, stdin, env) + if !forEach { + return verdict + } + if verdict.Errored { + // Under match_mode = "any" one target the validator cannot classify + // must not veto the rest. A single unresolvable target would + // otherwise decide the whole expansion and every remaining target, + // including an indexed one that would block, is never probed. The + // error is remembered and only decides the condition when no target + // blocked. Under match_mode = "all" an errored target still returns + // immediately, because "every target matches" cannot be proven once + // one target is unknown. + if matchAll { + return verdict + } + // A fail-closed rule already blocks on this error, and no later + // target can change that, so probing the rest only buys a better + // message. It costs a validator run per remaining target, each up to + // the background timeout times retry_count, all under a context that + // cannot be cancelled and while this cache key's singleflight entry + // is held, so a validator outage would stall every concurrent event + // sharing the key. The decided verdict is worth more than the message. + if verdict.Block { + return verdict + } + if !sawErrored { + sawErrored = true + firstErrored = verdict + } + continue + } + if verdict.Block && firstBlockMessage == "" { + firstBlockMessage = verdict.Message + } + if !matchAll && verdict.Block { + // The returned verdict is a clean block, so it carries Errored=false + // and the caller records the evaluation as fully classified. That is + // the right verdict, but it hides that an earlier target was never + // classified, so the partial failure is logged here rather than + // disappearing from the record. + if sawErrored { + r.log.WarnContext(ctx, "exec validator blocked with an unclassified target", + "rule", ruleName, "on_error", c.OnError) + } + return verdict + } + if matchAll && !verdict.Block { + return execconcern.Verdict{Block: false, Message: "", Output: "", Errored: false} + } + } + if matchAll { + return execconcern.Verdict{Block: true, Message: firstBlockMessage, Output: "", Errored: false} + } + if sawErrored { + return firstErrored + } + return execconcern.Verdict{Block: false, Message: "", Output: "", Errored: false} +} + +// logExpandedCommandError records why one expanded validator run produced an +// errored verdict, naming the failure mode so a spawn failure, a nonzero exit +// under a JSON predicate, and unparseable predicate output stay distinguishable +// in the log. +func (r *ExecRuntime) logExpandedCommandError( + ctx context.Context, + ruleName string, + c *config.Condition, + command []string, + res execconcern.RunResult, + runErr error, +) { + switch { + case runErr != nil: + r.log.WarnContext(ctx, "exec validator expanded command errored", + "rule", ruleName, "on_error", c.OnError, "command", command, "err", runErr) + case c.BlockOn == config.BlockOnMatch && res.ExitCode != 0: + r.log.WarnContext(ctx, "exec validator expanded command exited nonzero for JSON match", + "rule", ruleName, "on_error", c.OnError, "command", command, "exit_code", res.ExitCode) + case c.BlockOn == config.BlockOnMatch: + r.log.WarnContext(ctx, "exec validator expanded command returned invalid JSON predicate output", + "rule", ruleName, "on_error", c.OnError, "command", command) + default: + r.log.WarnContext(ctx, "exec validator expanded command produced an errored verdict", + "rule", ruleName, "on_error", c.OnError, "command", command, "exit_code", res.ExitCode) + } +} diff --git a/internal/rules/exec_gate.go b/internal/rules/exec_gate.go index 35d6358..b378dcd 100644 --- a/internal/rules/exec_gate.go +++ b/internal/rules/exec_gate.go @@ -608,89 +608,6 @@ func (r *ExecRuntime) runValidator( } } -func (r *ExecRuntime) runExpandedCommands( - ctx context.Context, - ruleName string, - c *config.Condition, - commands [][]string, - stdin []byte, - env []string, -) execconcern.Verdict { - if len(commands) == 0 { - return execconcern.Verdict{Block: false, Message: "", Output: "", Errored: false} - } - - forEach := c.ForEachSelector().Selector != config.FieldSelectorInvalid - matchAll := forEach && c.MatchMode == config.ExecMatchAll - firstBlockMessage := "" - firstErrored := execconcern.Verdict{Block: false, Message: "", Output: "", Errored: false} - sawErrored := false - for _, command := range commands { - verdict := r.runExpandedCommandWithRetry(ctx, ruleName, c, command, stdin, env) - if !forEach { - return verdict - } - if verdict.Errored { - // Under match_mode = "any" one target the validator cannot classify - // must not veto the rest. A single unresolvable target would - // otherwise decide the whole expansion and every remaining target, - // including an indexed one that would block, is never probed. The - // error is remembered and only decides the condition when no target - // blocked. Under match_mode = "all" an errored target still returns - // immediately, because "every target matches" cannot be proven once - // one target is unknown. - if matchAll { - return verdict - } - if !sawErrored { - sawErrored = true - firstErrored = verdict - } - continue - } - if verdict.Block && firstBlockMessage == "" { - firstBlockMessage = verdict.Message - } - if !matchAll && verdict.Block { - return verdict - } - if matchAll && !verdict.Block { - return execconcern.Verdict{Block: false, Message: "", Output: "", Errored: false} - } - } - if matchAll { - return execconcern.Verdict{Block: true, Message: firstBlockMessage, Output: "", Errored: false} - } - if sawErrored { - return firstErrored - } - return execconcern.Verdict{Block: false, Message: "", Output: "", Errored: false} -} - -func (r *ExecRuntime) logExpandedCommandError( - ctx context.Context, - ruleName string, - c *config.Condition, - command []string, - res execconcern.RunResult, - runErr error, -) { - switch { - case runErr != nil: - r.log.WarnContext(ctx, "exec validator expanded command errored", - "rule", ruleName, "on_error", c.OnError, "command", command, "err", runErr) - case c.BlockOn == config.BlockOnMatch && res.ExitCode != 0: - r.log.WarnContext(ctx, "exec validator expanded command exited nonzero for JSON match", - "rule", ruleName, "on_error", c.OnError, "command", command, "exit_code", res.ExitCode) - case c.BlockOn == config.BlockOnMatch: - r.log.WarnContext(ctx, "exec validator expanded command returned invalid JSON predicate output", - "rule", ruleName, "on_error", c.OnError, "command", command) - default: - r.log.WarnContext(ctx, "exec validator expanded command produced an errored verdict", - "rule", ruleName, "on_error", c.OnError, "command", command, "exit_code", res.ExitCode) - } -} - func (r *ExecRuntime) buildInput( ctx context.Context, fields FieldSet, diff --git a/internal/rules/exec_gate_errored_target_test.go b/internal/rules/exec_gate_errored_target_test.go index 238722c..97e71a6 100644 --- a/internal/rules/exec_gate_errored_target_test.go +++ b/internal/rules/exec_gate_errored_target_test.go @@ -26,6 +26,10 @@ func TestExecForEachAnyErroredTargetDoesNotVetoOthers(t *testing.T) { if err != nil { t.Fatalf("EvalSymlinks erroringTarget: %v", err) } + wantMatching, err := filepath.EvalSymlinks(matchingTarget) + if err != nil { + t.Fatalf("EvalSymlinks matchingTarget: %v", err) + } rule := loadExecRule(t, ` [[rules]] @@ -67,8 +71,88 @@ search_tools = ["grep"] if len(violations) == 0 { t.Fatal("an errored target vetoed a matching target under match_mode=any") } - if len(runner.Commands()) != 2 { - t.Fatalf("expected both targets probed, got %d commands", len(runner.Commands())) + // The contract is that the matching target is reached, not that a fixed + // number of validators ran. Asserting a count would pin the order the two + // targets happen to resolve in, and would also forbid a later change that + // probes targets concurrently or stops early once one blocks. + if !probedTarget(runner, wantMatching) { + t.Fatalf("matching target %q was never probed, commands = %v", wantMatching, runner.Commands()) + } +} + +// probedTarget reports whether the runner was asked to validate a given target. +func probedTarget(runner *recordingCommandRunner, target string) bool { + for _, command := range runner.Commands() { + for _, argument := range command { + if argument == target { + return true + } + } + } + return false +} + +// TestExecForEachAnyFailClosedStopsAtFirstErroredTarget covers the bound on +// validator work: under on_error = "closed" an errored target already decides +// the condition, so the remaining targets are not probed. Continuing would cost +// one validator run per remaining target, each up to the background timeout +// times retry_count, under a context that cannot be cancelled and while the +// singleflight entry for this cache key is held. +func TestExecForEachAnyFailClosedStopsAtFirstErroredTarget(t *testing.T) { + erroringTarget := filepath.Join(t.TempDir(), "repo-unknown") + laterTarget := filepath.Join(t.TempDir(), "repo-indexed") + for _, dir := range []string{erroringTarget, laterTarget} { + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatalf("MkdirAll %s: %v", dir, err) + } + } + wantErroring, err := filepath.EvalSymlinks(erroringTarget) + if err != nil { + t.Fatalf("EvalSymlinks erroringTarget: %v", err) + } + + rule := loadExecRule(t, ` +[[rules]] +name = "exec-rule" +events = ["PreToolUse"] +action = "block" +violation_message = "static message" + +[[rules.conditions]] +kind = "regex" +field_paths = ["tool_input.command"] +pattern = "grep" + +[[rules.conditions]] +kind = "exec" +command = ["/bin/check-target", "{{item}}"] +for_each = "cmd_read_targets" +match_mode = "any" +stdout_json_field = "searchable" +stdout_json_equals = true +cache_key = "cmd_read_targets" +cache_ttl_ms = 0 +on_error = "closed" +search_tools = ["grep"] +`) + runner := &recordingCommandRunner{ + run: func(command []string) (execconcern.RunResult, error) { + if command[1] == wantErroring { + return execconcern.RunResult{ExitCode: 0, Stdout: "not json"}, nil + } + return execconcern.RunResult{ExitCode: 0, Stdout: `{"searchable":false}`}, nil + }, + } + + violations := evalRule(runner, rule, map[string]any{ + "cwd": t.TempDir(), + "tool_input": map[string]any{"command": "grep -rn x " + erroringTarget + " " + laterTarget}, + }) + if len(violations) == 0 { + t.Fatal("a fail-closed rule should block once a target errors") + } + if len(runner.Commands()) != 1 { + t.Fatalf("fail-closed should stop at the errored target, got %d commands", len(runner.Commands())) } } @@ -121,7 +205,7 @@ search_tools = ["grep"] if len(violations) == 0 { t.Fatal("all targets errored under on_error=closed should block") } - if len(runner.Commands()) != 2 { - t.Fatalf("expected both targets probed, got %d commands", len(runner.Commands())) + if len(runner.Commands()) == 0 { + t.Fatal("no target was probed at all") } }