Skip to content

Commit 448485d

Browse files
tae2089claude
andcommitted
Block git option injection via base ref and handle non-ASCII paths
The caller-supplied base ref of detect_changes/get_affected_flows sat before "--" with no validation, so values like --output=<path> were parsed as git diff options. Reject refs starting with '-' and add the "--" separator to ChangedFiles. Also run diff with core.quotePath=false: quoted octal output for non-ASCII paths broke hunk-to-file attribution and file matching. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 72bee1e commit 448485d

2 files changed

Lines changed: 61 additions & 2 deletions

File tree

internal/analysis/changes/git.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ func NewExecGitClient() *ExecGitClient {
3737
// @requires repoDir points to a valid git working tree
3838
// @ensures returned file paths are trimmed and exclude blank lines
3939
func (g *ExecGitClient) ChangedFiles(ctx context.Context, repoDir, baseRef string) ([]string, error) {
40-
out, err := runGitLimited(ctx, repoDir, []string{"diff", "--name-only", baseRef})
40+
if err := validateBaseRef(baseRef); err != nil {
41+
return nil, err
42+
}
43+
out, err := runGitLimited(ctx, repoDir, []string{"-c", "core.quotePath=false", "diff", "--name-only", baseRef, "--"})
4144
if err != nil {
4245
return nil, trace.Wrap(err, "git diff --name-only")
4346
}
@@ -66,7 +69,10 @@ func (g *ExecGitClient) ChangedFiles(ctx context.Context, repoDir, baseRef strin
6669
// @requires repoDir points to a valid git working tree
6770
// @ensures each returned hunk has a file path and inclusive start/end lines
6871
func (g *ExecGitClient) DiffHunks(ctx context.Context, repoDir, baseRef string, paths []string) ([]Hunk, error) {
69-
args := []string{"diff", "-U0", baseRef, "--"}
72+
if err := validateBaseRef(baseRef); err != nil {
73+
return nil, err
74+
}
75+
args := []string{"-c", "core.quotePath=false", "diff", "-U0", baseRef, "--"}
7076
args = append(args, paths...)
7177
out, err := runGitLimited(ctx, repoDir, args)
7278
if err != nil {
@@ -98,6 +104,19 @@ func (g *ExecGitClient) DiffHunks(ctx context.Context, repoDir, baseRef string,
98104
return hunks, nil
99105
}
100106

107+
// validateBaseRef rejects base revisions that git would parse as command options.
108+
// @intent block git argument injection through the caller-supplied base ref, which sits
109+
// before the "--" separator and would otherwise be interpreted as a diff flag.
110+
func validateBaseRef(baseRef string) error {
111+
if baseRef == "" {
112+
return fmt.Errorf("base ref must not be empty")
113+
}
114+
if strings.HasPrefix(baseRef, "-") {
115+
return fmt.Errorf("invalid base ref %q: must not start with '-'", baseRef)
116+
}
117+
return nil
118+
}
119+
101120
// runGitLimited runs a git subcommand with the default output size cap.
102121
// @intent share a single bounded git invocation helper across diff operations
103122
func runGitLimited(ctx context.Context, repoDir string, args []string) ([]byte, error) {

internal/analysis/changes/git_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,46 @@ func TestGitClient_ChangedFiles(t *testing.T) {
2929
}
3030
}
3131

32+
func TestGitClient_RejectsOptionLikeBaseRef(t *testing.T) {
33+
dir := initTestRepo(t)
34+
writeFile(t, dir, "hello.go", "package main\n")
35+
gitCommit(t, dir, "initial")
36+
37+
git := NewExecGitClient()
38+
for _, base := range []string{"--output=/tmp/pwned", "-U9999", "--no-index"} {
39+
if _, err := git.ChangedFiles(context.Background(), dir, base); err == nil {
40+
t.Errorf("ChangedFiles accepted option-like base %q", base)
41+
}
42+
if _, err := git.DiffHunks(context.Background(), dir, base, []string{"hello.go"}); err == nil {
43+
t.Errorf("DiffHunks accepted option-like base %q", base)
44+
}
45+
}
46+
}
47+
48+
func TestGitClient_ChangedFiles_NonASCIIPath(t *testing.T) {
49+
dir := initTestRepo(t)
50+
writeFile(t, dir, "한글.go", "package main\n")
51+
gitCommit(t, dir, "initial")
52+
writeFile(t, dir, "한글.go", "package main\n\nfunc Hello() {}\n")
53+
54+
git := NewExecGitClient()
55+
files, err := git.ChangedFiles(context.Background(), dir, "HEAD")
56+
if err != nil {
57+
t.Fatal(err)
58+
}
59+
if len(files) != 1 || files[0] != "한글.go" {
60+
t.Fatalf("expected unquoted 한글.go, got %v", files)
61+
}
62+
63+
hunks, err := git.DiffHunks(context.Background(), dir, "HEAD", []string{"한글.go"})
64+
if err != nil {
65+
t.Fatal(err)
66+
}
67+
if len(hunks) == 0 || hunks[0].FilePath != "한글.go" {
68+
t.Fatalf("expected hunk for unquoted 한글.go, got %+v", hunks)
69+
}
70+
}
71+
3272
func TestGitClient_DiffHunks(t *testing.T) {
3373
dir := initTestRepo(t)
3474
writeFile(t, dir, "hello.go", "package main\n\nfunc Old() {}\n")

0 commit comments

Comments
 (0)