From 0d0bf0e67c483466f7723306c7bf4c0706b2c1b3 Mon Sep 17 00:00:00 2001 From: Mathis Chenuet <9201969+artemisart@users.noreply.github.com> Date: Thu, 6 Aug 2026 22:51:59 +0200 Subject: [PATCH 1/2] Harden read_file against symlink escapes --- meat/meat_test.go | 24 ++++++++++++++++++++++++ meat/tools.go | 41 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/meat/meat_test.go b/meat/meat_test.go index d971aa4..039b2b9 100644 --- a/meat/meat_test.go +++ b/meat/meat_test.go @@ -362,6 +362,30 @@ func TestToolboxGrepAndPathConfinement(t *testing.T) { } } +func TestToolboxReadFileRejectsSymlinkOutsideRepo(t *testing.T) { + repo := gitRepo(t, map[string]string{"a.go": "package a\n"}) + outside := filepath.Join(t.TempDir(), "secret.txt") + if err := os.WriteFile(outside, []byte("outside secret\n"), 0o600); err != nil { + t.Fatal(err) + } + link := filepath.Join(repo, "link.txt") + if err := os.Symlink(outside, link); err != nil { + t.Fatal(err) + } + + input, err := json.Marshal(readFileInput{Path: "link.txt"}) + if err != nil { + t.Fatal(err) + } + out, isErr := (&toolbox{root: repo}).readFile(input) + if !isErr { + t.Fatalf("want external symlink rejected, got %q", out) + } + if strings.Contains(out, "outside secret") { + t.Fatalf("rejected read leaked target contents: %q", out) + } +} + // TestAbridge_RejectsOversizeDiff: a diff over the total cap must be refused // up front with actionable advice, never sent to the model — chunking makes // large diffs feasible, not unbounded. diff --git a/meat/tools.go b/meat/tools.go index fb5bbaf..99ab985 100644 --- a/meat/tools.go +++ b/meat/tools.go @@ -118,8 +118,10 @@ func (tb *toolbox) run(ctx context.Context, name string, input json.RawMessage) } } -// resolveInRoot joins rel to the repo root and verifies the result stays inside -// it, defeating path traversal via .. or absolute paths. +// resolveInRoot joins rel to the repo root and verifies both the lexical path +// and its symlink-resolved target stay inside it. The second check is important: +// a repository can contain a symlink whose target is outside the repository, +// and os.ReadFile follows that link. func (tb *toolbox) resolveInRoot(rel string) (string, error) { clean := filepath.Clean(rel) if filepath.IsAbs(clean) { @@ -133,10 +135,41 @@ func (tb *toolbox) resolveInRoot(rel string) (string, error) { if err != nil { return "", err } - if absAbs != rootAbs && !strings.HasPrefix(absAbs, rootAbs+string(os.PathSeparator)) { + if !pathWithinRoot(rootAbs, absAbs) { return "", fmt.Errorf("path escapes the repo root") } - return absAbs, nil + rootReal, err := filepath.EvalSymlinks(rootAbs) + if err != nil { + return "", fmt.Errorf("resolve repo root: %w", err) + } + rootReal, err = filepath.Abs(rootReal) + if err != nil { + return "", err + } + targetReal, err := filepath.EvalSymlinks(absAbs) + if err != nil { + return "", fmt.Errorf("resolve path: %w", err) + } + targetReal, err = filepath.Abs(targetReal) + if err != nil { + return "", err + } + if !pathWithinRoot(rootReal, targetReal) { + return "", fmt.Errorf("path resolves outside the repo root") + } + // Return the canonical target so the subsequent read does not follow the + // validated symlink path a second time. + return targetReal, nil +} + +// pathWithinRoot reports whether path is root itself or a descendant of root. +// filepath.Rel avoids false positives such as /repo-other matching /repo. +func pathWithinRoot(root, path string) bool { + rel, err := filepath.Rel(root, path) + if err != nil || filepath.IsAbs(rel) { + return false + } + return rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator)) } type readFileInput struct { From baee971140d8b2893767b03b8b5c5c042de6432f Mon Sep 17 00:00:00 2001 From: Mathis Chenuet <9201969+artemisart@users.noreply.github.com> Date: Thu, 6 Aug 2026 23:27:30 +0200 Subject: [PATCH 2/2] Use root-confined file opens --- meat/tools.go | 66 +++++++++++++-------------------------------------- 1 file changed, 16 insertions(+), 50 deletions(-) diff --git a/meat/tools.go b/meat/tools.go index 99ab985..361b4d4 100644 --- a/meat/tools.go +++ b/meat/tools.go @@ -118,58 +118,18 @@ func (tb *toolbox) run(ctx context.Context, name string, input json.RawMessage) } } -// resolveInRoot joins rel to the repo root and verifies both the lexical path -// and its symlink-resolved target stay inside it. The second check is important: -// a repository can contain a symlink whose target is outside the repository, -// and os.ReadFile follows that link. -func (tb *toolbox) resolveInRoot(rel string) (string, error) { +// cleanRepoPath validates and cleans a path supplied relative to the repo root. +// File operations must still be performed with os.OpenInRoot: lexical +// validation alone cannot account for symlinks. +func cleanRepoPath(rel string) (string, error) { clean := filepath.Clean(rel) if filepath.IsAbs(clean) { return "", fmt.Errorf("path must be relative to the repo root") } - rootAbs, err := filepath.Abs(tb.root) - if err != nil { - return "", err - } - absAbs, err := filepath.Abs(filepath.Join(tb.root, clean)) - if err != nil { - return "", err - } - if !pathWithinRoot(rootAbs, absAbs) { + if clean == ".." || strings.HasPrefix(clean, ".."+string(os.PathSeparator)) { return "", fmt.Errorf("path escapes the repo root") } - rootReal, err := filepath.EvalSymlinks(rootAbs) - if err != nil { - return "", fmt.Errorf("resolve repo root: %w", err) - } - rootReal, err = filepath.Abs(rootReal) - if err != nil { - return "", err - } - targetReal, err := filepath.EvalSymlinks(absAbs) - if err != nil { - return "", fmt.Errorf("resolve path: %w", err) - } - targetReal, err = filepath.Abs(targetReal) - if err != nil { - return "", err - } - if !pathWithinRoot(rootReal, targetReal) { - return "", fmt.Errorf("path resolves outside the repo root") - } - // Return the canonical target so the subsequent read does not follow the - // validated symlink path a second time. - return targetReal, nil -} - -// pathWithinRoot reports whether path is root itself or a descendant of root. -// filepath.Rel avoids false positives such as /repo-other matching /repo. -func pathWithinRoot(root, path string) bool { - rel, err := filepath.Rel(root, path) - if err != nil || filepath.IsAbs(rel) { - return false - } - return rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator)) + return clean, nil } type readFileInput struct { @@ -183,11 +143,16 @@ func (tb *toolbox) readFile(raw json.RawMessage) (string, bool) { if err := json.Unmarshal(raw, &in); err != nil { return fmt.Sprintf("invalid input: %v", err), true } - abs, err := tb.resolveInRoot(in.Path) + path, err := cleanRepoPath(in.Path) if err != nil { return err.Error(), true } - data, err := os.ReadFile(abs) + file, err := os.OpenInRoot(tb.root, path) + if err != nil { + return fmt.Sprintf("read %s: %v", in.Path, err), true + } + defer file.Close() + data, err := io.ReadAll(file) if err != nil { return fmt.Sprintf("read %s: %v", in.Path, err), true } @@ -213,10 +178,11 @@ func (tb *toolbox) grep(ctx context.Context, raw json.RawMessage) (string, bool) } args := []string{"grep", "-n", "-I", "--no-color", "-e", in.Pattern} if in.Path != "" { - if _, err := tb.resolveInRoot(in.Path); err != nil { + path, err := cleanRepoPath(in.Path) + if err != nil { return err.Error(), true } - args = append(args, "--", in.Path) + args = append(args, "--", path) } cmd := exec.CommandContext(ctx, "git", args...) cmd.Dir = tb.root