Vectrum
Sign In
CodeyPublic
Like0 likesFork0 forks

tools: read-only extra roots in the path jail

Let read_file, glob, and grep reach into additional directories outside
the working root (e.g. a skill's own directory) via a new variadic
extra parameter, while edit_file and write_file stay confined exactly
as before. jail is now jailWith(root, extra, path) underneath, with
jail(root, path) as the nil-extra case; extra roots get the same
symlink resolution as the working root so a skill directory cannot be
used to step sideways into the rest of the filesystem.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Browse Files
Jjonicommitted 1 parent e5248a4commit1c9caee

Showing 4 changed files with 146 additions and 18 deletions.

+4−3internal/tools/glob.goView File
@@ -32,8 +32,9 @@ type GlobArgs struct {
3232 Path string `json:"path,omitempty" jsonschema:"description=Directory to search under; defaults to the working root"`
3333 }
3434
35- // NewGlob builds the glob tool.
36- func NewGlob(root string) Tool {
35+ // NewGlob builds the glob tool. extra names additional directories that may be
36+ // searched but never written, such as a skill's own directory.
37+ func NewGlob(root string, extra ...string) Tool {
3738 return New("glob", strings.TrimSpace(`
3839 Find files by name. Returns paths relative to the working root, most recently
3940 modified first. "*.go" matches Go files at any depth; "internal/**/*_test.go"
@@ -42,7 +43,7 @@ matches within a subtree. Use this instead of guessing where a file lives.`),
4243 if in.Pattern == "" {
4344 return "", fmt.Errorf("pattern is required")
4445 }
45- base, err := jail(root, resolveBase(root, in.Path))
46+ base, err := jailWith(root, extra, resolveBase(root, in.Path))
4647 if err != nil {
4748 return "", err
4849 }
+4−3internal/tools/grep.goView File
@@ -57,8 +57,9 @@ type GrepArgs struct {
5757
5858 // NewGrep builds the grep tool. It shells out to ripgrep when available and
5959 // falls back to a pure-Go walk otherwise; the output format is the same either
60- // way, so the model cannot tell which ran.
61- func NewGrep(root string) Tool {
60+ // way, so the model cannot tell which ran. extra names additional directories
61+ // that may be searched but never written, such as a skill's own directory.
62+ func NewGrep(root string, extra ...string) Tool {
6263 return New("grep", strings.TrimSpace(`
6364 Search file contents by regular expression. Returns "path:line: text" for each
6465 match. Narrow with glob (e.g. "*.go") or path. This is how you find code you
@@ -71,7 +72,7 @@ have not been shown — reach for it before guessing at file names.`),
7172 if err != nil {
7273 return "", err
7374 }
74- base, err := jail(root, resolveBase(root, in.Path))
75+ base, err := jailWith(root, extra, resolveBase(root, in.Path))
7576 if err != nil {
7677 return "", err
7778 }
+87−0internal/tools/jail_test.goView File
@@ -7,6 +7,93 @@ import (
77 "testing"
88 )
99
10+ // A skill lives outside the working root. The read tools reach into it; the
11+ // write tools must not.
12+ func TestExtraRootsAreReadableNotWritable(t *testing.T) {
13+ root := t.TempDir()
14+ skill := t.TempDir()
15+ ref := filepath.Join(skill, "references.md")
16+ if err := os.WriteFile(ref, []byte("conventions\n"), 0o644); err != nil {
17+ t.Fatal(err)
18+ }
19+
20+ read := NewRead(root, nil, skill)
21+ out, err := run(t, read, map[string]any{"path": ref})
22+ if err != nil {
23+ t.Fatalf("read_file refused an extra root: %v", err)
24+ }
25+ if !strings.Contains(out, "conventions") {
26+ t.Errorf("read_file returned %q", out)
27+ }
28+
29+ write := NewWrite(root, nil)
30+ if _, err := run(t, write, map[string]any{
31+ "path": ref, "content": "rewritten",
32+ }); err == nil {
33+ t.Error("write_file was allowed into a skill directory")
34+ }
35+ }
36+
37+ func TestExtraRootsDoNotWidenTheJail(t *testing.T) {
38+ root := t.TempDir()
39+ skill := t.TempDir()
40+ outside := filepath.Join(t.TempDir(), "secret.txt")
41+ if err := os.WriteFile(outside, []byte("secret\n"), 0o644); err != nil {
42+ t.Fatal(err)
43+ }
44+
45+ read := NewRead(root, nil, skill)
46+ if _, err := run(t, read, map[string]any{"path": outside}); err == nil {
47+ t.Error("a path under neither the root nor an extra root was allowed")
48+ }
49+ }
50+
51+ func TestExtraRootSymlinkCannotStepSideways(t *testing.T) {
52+ root := t.TempDir()
53+ skill := t.TempDir()
54+ outside := t.TempDir()
55+ if err := os.WriteFile(filepath.Join(outside, "secret.txt"), []byte("secret\n"), 0o644); err != nil {
56+ t.Fatal(err)
57+ }
58+ link := filepath.Join(skill, "escape")
59+ if err := os.Symlink(outside, link); err != nil {
60+ t.Skipf("symlinks unavailable: %v", err)
61+ }
62+
63+ read := NewRead(root, nil, skill)
64+ if _, err := run(t, read, map[string]any{
65+ "path": filepath.Join(link, "secret.txt"),
66+ }); err == nil {
67+ t.Error("a symlink out of a skill directory was followed")
68+ }
69+ }
70+
71+ func TestGlobAndGrepAcceptExtraRoots(t *testing.T) {
72+ root := t.TempDir()
73+ skill := t.TempDir()
74+ if err := os.WriteFile(filepath.Join(skill, "notes.md"), []byte("needle\n"), 0o644); err != nil {
75+ t.Fatal(err)
76+ }
77+
78+ g := NewGlob(root, skill)
79+ if _, err := run(t, g, map[string]any{
80+ "pattern": "*.md", "path": skill,
81+ }); err != nil {
82+ t.Errorf("glob refused an extra root: %v", err)
83+ }
84+
85+ gr := NewGrep(root, skill)
86+ out, err := run(t, gr, map[string]any{
87+ "pattern": "needle", "path": skill,
88+ })
89+ if err != nil {
90+ t.Errorf("grep refused an extra root: %v", err)
91+ }
92+ if !strings.Contains(out, "needle") {
93+ t.Errorf("grep returned %q", out)
94+ }
95+ }
96+
1097 func TestJail(t *testing.T) {
1198 root := t.TempDir()
1299 writeFile(t, root, "inside/file.go", "package inside\n")
+51−12internal/tools/read.goView File
@@ -38,15 +38,17 @@ type ReadArgs struct {
3838 Limit int `json:"limit,omitempty" jsonschema:"description=Maximum number of lines to return; defaults to 2000"`
3939 }
4040
41- // NewRead builds the read_file tool. root anchors relative paths; track may be nil.
42- func NewRead(root string, track ReadTracker) Tool {
41+ // NewRead builds the read_file tool. root anchors relative paths; track may be
42+ // nil; extra names additional directories that may be read but never written,
43+ // such as a skill's own directory outside the working root.
44+ func NewRead(root string, track ReadTracker, extra ...string) Tool {
4345 return New("read_file", strings.TrimSpace(`
4446 Read a file from disk. Returns the contents with 1-indexed line numbers, in the
4547 form " 12\tcode". Use offset and limit to page through a large file rather
4648 than reading it whole. Line numbers are for your reference only — never include
4749 them in text you write back to the file.`),
4850 func(_ context.Context, in ReadArgs) (string, error) {
49- path, err := resolve(root, in.Path)
51+ path, err := resolveWith(root, extra, in.Path)
5052 if err != nil {
5153 return "", err
5254 }
@@ -112,10 +114,17 @@ func splitLines(s string) []string {
112114
113115 // resolve anchors a relative path to root and refuses anything outside it.
114116 func resolve(root, path string) (string, error) {
117+ return resolveWith(root, nil, path)
118+ }
119+
120+ // resolveWith is resolve with additional permitted roots. It is used by
121+ // read_file, whose extra roots let a skill's own directory be read without
122+ // widening what edit_file or write_file may touch.
123+ func resolveWith(root string, extra []string, path string) (string, error) {
115124 if path == "" {
116125 return "", fmt.Errorf("path is required")
117126 }
118- return jail(root, resolveBase(root, path))
127+ return jailWith(root, extra, resolveBase(root, path))
119128 }
120129
121130 // resolveBase is resolve for the search tools, where an omitted path means the
@@ -139,13 +148,17 @@ func resolveBase(root, path string) string {
139148 // used to step around the check. It is cheap and catches a lot; it is not a
140149 // substitute for a sandbox, since bash can still cd anywhere.
141150 func jail(root, path string) (string, error) {
151+ return jailWith(root, nil, path)
152+ }
153+
154+ // jailWith is jail with additional permitted roots, used by the read-only
155+ // tools so a skill can read the files it ships with. Each extra root gets the
156+ // same symlink resolution as the working root, so an extra root cannot be used
157+ // to step sideways into the rest of the filesystem.
158+ func jailWith(root string, extra []string, path string) (string, error) {
142159 if root == "" {
143160 return path, nil
144161 }
145- realRoot, err := filepath.EvalSymlinks(root)
146- if err != nil {
147- realRoot = filepath.Clean(root)
148- }
149162 // A file being created does not exist yet, so resolve the deepest existing
150163 // ancestor and re-attach the rest.
151164 probe, rest := path, ""
@@ -163,9 +176,35 @@ func jail(root, path string) (string, error) {
163176 probe = parent
164177 }
165178
166- rel, err := filepath.Rel(realRoot, path)
167- if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
168- return "", fmt.Errorf("%s is outside the working root %s; this harness only touches files under the root", path, realRoot)
179+ realRoot := realpath(root)
180+ if contains(realRoot, path) {
181+ return path, nil
182+ }
183+ for _, e := range extra {
184+ if e != "" && contains(realpath(e), path) {
185+ return path, nil
186+ }
187+ }
188+ if len(extra) > 0 {
189+ return "", fmt.Errorf("%s is outside the working root %s and every skill directory; this harness only touches files under the root", path, realRoot)
190+ }
191+ return "", fmt.Errorf("%s is outside the working root %s; this harness only touches files under the root", path, realRoot)
192+ }
193+
194+ // realpath resolves symlinks, falling back to a lexical clean for a path that
195+ // does not exist.
196+ func realpath(p string) string {
197+ if resolved, err := filepath.EvalSymlinks(p); err == nil {
198+ return resolved
199+ }
200+ return filepath.Clean(p)
201+ }
202+
203+ // contains reports whether path is root or sits beneath it.
204+ func contains(root, path string) bool {
205+ rel, err := filepath.Rel(root, path)
206+ if err != nil {
207+ return false
169208 }
170- return path, nil
209+ return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
171210 }