Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 83 additions & 2 deletions internal/rules/concerns/shellread/codesearch.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package shellread

import (
"os"
"strings"

"goodkind.io/agent-gate/internal/rules/concerns/shellparse"
"goodkind.io/gksyntax/shelldecomp"
Expand Down Expand Up @@ -93,20 +94,30 @@ 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)
}

// Recursive structure discovery with no content searcher (ls -R, a find
// 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)
}

Expand All @@ -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
Expand Down
15 changes: 15 additions & 0 deletions internal/rules/concerns/shellread/codesearch_embedded.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down
26 changes: 0 additions & 26 deletions internal/rules/concerns/shellread/codesearch_enum.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -307,9 +284,6 @@ func recursiveGlobDirs(command, cwd string) []string {
dir = "/"
}
}
if !isPlausiblePathOperand(dir) {
continue
}
out = append(out, resolvePath(cwd, dir))
}
return out
Expand Down
109 changes: 89 additions & 20 deletions internal/rules/concerns/shellread/codesearch_glob_operand_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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 {
Expand All @@ -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)
}
})
}
}
Loading