Skip to content

Workspace grep resolves git through PATH while file discovery refuses to #395

Description

@beardthelion

Workspace file discovery deliberately refuses to resolve git from PATH. trustedGitExecutable in src/core/workspace/workspace_files.zig stats a fixed list of absolute paths and returns null rather than fall back, and every discovery entry point routes through it.

Grep does the opposite. src/core/workspace/grep_search.zig passes a bare "git" as argv0 at three sites, each via std.process.run with .cwd set to the search root:

  • gitIgnoresRoot (git --no-optional-locks check-ignore -q -- .)
  • gitGrepTrackedMatches (git --no-optional-locks grep -n -I -F -z ...)
  • gitGrepTrackedCounts (git --no-optional-locks grep --count ...)

The file contains no reference to trustedGitExecutable. grep_files reaches this code, so it is the live tool path.

What happens

With a git placed earlier on PATH, one grep_files call runs it twice, once for check-ignore and once for git grep. The same workspace searched through the discovery path does not run it, which is what makes this an inconsistency between the two rather than a property of the tree.

A git sitting in the workspace root is not executed. Resolution is PATH-only, so the file has to land in a PATH directory rather than merely in the workspace. On an ordinary dev machine several PATH entries are user-writable.

There is a second effect worth noting. Under a git that exits 1 with empty output, which is git's normal "no matches" signal, grep_files reported zero matches for a workspace that really contained two. A wrong-but-confident empty result is harder to notice than a crash, and nothing surfaces an error.

Reproducing

In a temp git repo containing a file with a match, put an executable git first on PATH that appends its argv to a log and exits 1, then call grep_files for that pattern. The log records both invocations and the tool returns no matches.

Suggested direction

Give grep the executable the discovery layer already selects, and skip the git backend when none is available, which is the shape discoverWithStop already uses for its null case.

Two related things this does not cover, both pre-existing: src/core/github/git_context.zig:37 and src/builtins/skills.zig:253 still build argv from a bare "git", and trustedGitExecutable stats with follow_symlinks = true and checks only that the candidate is a file, so an allowlisted path that is user-writable or a symlink out of the allowlist is accepted.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    type: securityFixes or hardens a security boundary

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions