Skip to content

Commit 83c1f60

Browse files
committed
feat: add outside roots approval
1 parent 705a241 commit 83c1f60

File tree

10 files changed

+582
-82
lines changed

10 files changed

+582
-82
lines changed

internal/approval/engine.go

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,12 @@ const (
5454
)
5555

5656
type Prompt struct {
57-
Tool string
58-
Summary string
59-
Reason string
60-
Capability Capability
61-
Preview string
57+
Tool string
58+
Summary string
59+
Reason string
60+
Capability Capability
61+
Preview string
62+
OutsideRoots bool // hint to UI: only allow_once and deny are safe choices
6263
}
6364

6465
type ApproverFunc func(ctx context.Context, prompt Prompt) (Choice, error)
@@ -92,8 +93,14 @@ type AuditEntry struct {
9293
Allow bool
9394
}
9495

96+
type FilesystemRoots struct {
97+
ReadRoots []string
98+
WriteRoots []string
99+
}
100+
95101
type Engine struct {
96102
workspace string
103+
fsRoots FilesystemRoots
97104
profile Profile
98105
rules *RuleSet
99106

@@ -114,7 +121,11 @@ func NewEngine(cwd string, profile Profile, rules *RuleSet, onAudit func(AuditEn
114121
return nil, fmt.Errorf("load approvals: %w", err)
115122
}
116123
return &Engine{
117-
workspace: cwd,
124+
workspace: cwd,
125+
fsRoots: FilesystemRoots{
126+
ReadRoots: []string{cwd},
127+
WriteRoots: []string{cwd},
128+
},
118129
profile: profile,
119130
rules: rules,
120131
mode: ModeNormal,
@@ -125,6 +136,12 @@ func NewEngine(cwd string, profile Profile, rules *RuleSet, onAudit func(AuditEn
125136
}, nil
126137
}
127138

139+
func (e *Engine) SetFilesystemRoots(roots FilesystemRoots) {
140+
e.mu.Lock()
141+
defer e.mu.Unlock()
142+
e.fsRoots = normalizeFilesystemRoots(e.workspace, roots)
143+
}
144+
128145
func (e *Engine) SetMode(mode Mode) {
129146
e.mu.Lock()
130147
defer e.mu.Unlock()
@@ -304,6 +321,12 @@ func (e *Engine) decide(info toolInfo, mode Mode, planMode bool) (ruleAction, st
304321
}
305322
}
306323

324+
// Paths outside configured roots always require explicit approval,
325+
// regardless of profile, skill allows, or cached approvals.
326+
if info.outsideRoots {
327+
return ruleAsk, info.reason
328+
}
329+
307330
// Skill allowed-tools: auto-approve tools granted by the active skill.
308331
e.mu.RLock()
309332
skillRules := e.skillAllows
@@ -377,15 +400,22 @@ func (e *Engine) ask(ctx context.Context, info toolInfo, mode Mode, planMode boo
377400
return ChoiceDeny, nil
378401
}
379402
return fn(ctx, Prompt{
380-
Tool: info.tool,
381-
Summary: info.summary,
382-
Reason: info.reason,
383-
Capability: info.capability,
384-
Preview: info.preview,
403+
Tool: info.tool,
404+
Summary: info.summary,
405+
Reason: info.reason,
406+
Capability: info.capability,
407+
Preview: info.preview,
408+
OutsideRoots: info.outsideRoots,
385409
})
386410
}
387411

388412
func (e *Engine) resolveChoice(info toolInfo, mode Mode, planMode bool, choice Choice) *agentcore.ToolApprovalResult {
413+
// Outside-roots approvals must not be cached at session/always granularity,
414+
// otherwise a single "allow session" would open the entire filesystem.
415+
if info.outsideRoots && (choice == ChoiceAllowAlways || choice == ChoiceAllowSession) {
416+
choice = ChoiceAllowOnce
417+
}
418+
389419
switch choice {
390420
case ChoiceAllowAlways:
391421
entry := storedEntry{

internal/approval/engine_test.go

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,197 @@ func TestBalancedReadOutsideWorkspaceIsDenied(t *testing.T) {
107107
}
108108
}
109109

110+
func TestBalancedReadInExtraReadRootIsAllowed(t *testing.T) {
111+
t.Setenv("HOME", t.TempDir())
112+
113+
workspace := t.TempDir()
114+
extraReadRoot := t.TempDir()
115+
engine, err := NewEngine(workspace, ProfileBalanced, nil, nil)
116+
if err != nil {
117+
t.Fatalf("NewEngine: %v", err)
118+
}
119+
engine.SetFilesystemRoots(FilesystemRoots{
120+
ReadRoots: []string{workspace, extraReadRoot},
121+
WriteRoots: []string{workspace},
122+
})
123+
124+
args, _ := json.Marshal(map[string]any{"path": filepath.Join(extraReadRoot, "outside.txt")})
125+
result, err := engine.ApproveTool(context.Background(), agentcore.ToolApprovalRequest{
126+
Call: agentcore.ToolCall{Name: "read", Args: args},
127+
})
128+
if err != nil {
129+
t.Fatalf("ApproveTool: %v", err)
130+
}
131+
if result != nil {
132+
t.Fatalf("read in extra read root should bypass approval, got %#v", result)
133+
}
134+
}
135+
136+
func TestBalancedWriteInReadOnlyRootIsDenied(t *testing.T) {
137+
t.Setenv("HOME", t.TempDir())
138+
139+
workspace := t.TempDir()
140+
extraReadRoot := t.TempDir()
141+
engine, err := NewEngine(workspace, ProfileBalanced, nil, nil)
142+
if err != nil {
143+
t.Fatalf("NewEngine: %v", err)
144+
}
145+
engine.SetFilesystemRoots(FilesystemRoots{
146+
ReadRoots: []string{workspace, extraReadRoot},
147+
WriteRoots: []string{workspace},
148+
})
149+
engine.SetApprover(func(context.Context, Prompt) (Choice, error) {
150+
t.Fatal("approver should not be called for read-only external root")
151+
return ChoiceAllowOnce, nil
152+
})
153+
154+
args, _ := json.Marshal(map[string]any{"path": filepath.Join(extraReadRoot, "blocked.txt")})
155+
result, err := engine.ApproveTool(context.Background(), agentcore.ToolApprovalRequest{
156+
Call: agentcore.ToolCall{Name: "write", Args: args},
157+
})
158+
if err != nil {
159+
t.Fatalf("ApproveTool: %v", err)
160+
}
161+
if result == nil || result.Approved {
162+
t.Fatalf("write in read-only external root should be denied, got %#v", result)
163+
}
164+
}
165+
166+
func TestBalancedWriteInExtraWriteRootUsesApprovalFlow(t *testing.T) {
167+
t.Setenv("HOME", t.TempDir())
168+
169+
workspace := t.TempDir()
170+
extraWriteRoot := t.TempDir()
171+
engine, err := NewEngine(workspace, ProfileBalanced, nil, nil)
172+
if err != nil {
173+
t.Fatalf("NewEngine: %v", err)
174+
}
175+
engine.SetFilesystemRoots(FilesystemRoots{
176+
ReadRoots: []string{workspace, extraWriteRoot},
177+
WriteRoots: []string{workspace, extraWriteRoot},
178+
})
179+
180+
var prompts int
181+
engine.SetApprover(func(context.Context, Prompt) (Choice, error) {
182+
prompts++
183+
return ChoiceAllowOnce, nil
184+
})
185+
186+
args, _ := json.Marshal(map[string]any{"path": filepath.Join(extraWriteRoot, "allowed.txt")})
187+
result, err := engine.ApproveTool(context.Background(), agentcore.ToolApprovalRequest{
188+
Call: agentcore.ToolCall{Name: "write", Args: args},
189+
})
190+
if err != nil {
191+
t.Fatalf("ApproveTool: %v", err)
192+
}
193+
if result == nil || !result.Approved {
194+
t.Fatalf("write in extra write root should be approvable, got %#v", result)
195+
}
196+
if prompts != 1 {
197+
t.Fatalf("expected one approval prompt, got %d", prompts)
198+
}
199+
}
200+
201+
func TestOutsideRootsAllowSessionDegradesToAllowOnce(t *testing.T) {
202+
t.Setenv("HOME", t.TempDir())
203+
204+
workspace := t.TempDir()
205+
outside := filepath.Join(t.TempDir(), "outside.txt")
206+
engine, err := NewEngine(workspace, ProfileBalanced, nil, nil)
207+
if err != nil {
208+
t.Fatalf("NewEngine: %v", err)
209+
}
210+
211+
var prompts int
212+
engine.SetApprover(func(_ context.Context, p Prompt) (Choice, error) {
213+
prompts++
214+
if !p.OutsideRoots {
215+
t.Fatal("expected OutsideRoots=true in prompt")
216+
}
217+
// User tries to allow session — engine should degrade this.
218+
return ChoiceAllowSession, nil
219+
})
220+
221+
// First call: outside roots read, user says "allow session".
222+
args, _ := json.Marshal(map[string]any{"path": outside})
223+
result, err := engine.ApproveTool(context.Background(), agentcore.ToolApprovalRequest{
224+
Call: agentcore.ToolCall{Name: "read", Args: args},
225+
})
226+
if err != nil {
227+
t.Fatalf("ApproveTool: %v", err)
228+
}
229+
if result == nil || !result.Approved {
230+
t.Fatalf("first outside read should be approved (once), got %#v", result)
231+
}
232+
233+
// Second call: same outside path — must prompt again (not cached).
234+
result, err = engine.ApproveTool(context.Background(), agentcore.ToolApprovalRequest{
235+
Call: agentcore.ToolCall{Name: "read", Args: args},
236+
})
237+
if err != nil {
238+
t.Fatalf("ApproveTool second: %v", err)
239+
}
240+
if prompts != 2 {
241+
t.Fatalf("expected 2 approval prompts (no caching), got %d", prompts)
242+
}
243+
}
244+
245+
func TestBalancedBashWorkdirOutsideRootsRequiresApproval(t *testing.T) {
246+
t.Setenv("HOME", t.TempDir())
247+
248+
workspace := t.TempDir()
249+
engine, err := NewEngine(workspace, ProfileBalanced, nil, nil)
250+
if err != nil {
251+
t.Fatalf("NewEngine: %v", err)
252+
}
253+
254+
args, _ := json.Marshal(map[string]any{"command": "ls", "workdir": "/tmp/elsewhere"})
255+
result, err := engine.ApproveTool(context.Background(), agentcore.ToolApprovalRequest{
256+
Call: agentcore.ToolCall{Name: "bash", Args: args},
257+
})
258+
if err != nil {
259+
t.Fatalf("ApproveTool: %v", err)
260+
}
261+
if result == nil || result.Approved {
262+
t.Fatalf("bash with workdir outside roots should be denied, got %#v", result)
263+
}
264+
}
265+
266+
func TestBalancedBashWorkdirInWriteRootIsAllowed(t *testing.T) {
267+
t.Setenv("HOME", t.TempDir())
268+
269+
workspace := t.TempDir()
270+
extraDir := t.TempDir()
271+
engine, err := NewEngine(workspace, ProfileBalanced, nil, nil)
272+
if err != nil {
273+
t.Fatalf("NewEngine: %v", err)
274+
}
275+
engine.SetFilesystemRoots(FilesystemRoots{
276+
ReadRoots: []string{workspace, extraDir},
277+
WriteRoots: []string{workspace, extraDir},
278+
})
279+
280+
var prompts int
281+
engine.SetApprover(func(context.Context, Prompt) (Choice, error) {
282+
prompts++
283+
return ChoiceAllowOnce, nil
284+
})
285+
286+
args, _ := json.Marshal(map[string]any{"command": "ls", "workdir": extraDir})
287+
result, err := engine.ApproveTool(context.Background(), agentcore.ToolApprovalRequest{
288+
Call: agentcore.ToolCall{Name: "bash", Args: args},
289+
})
290+
if err != nil {
291+
t.Fatalf("ApproveTool: %v", err)
292+
}
293+
if result == nil || !result.Approved {
294+
t.Fatalf("bash with workdir in write root should be approvable, got %#v", result)
295+
}
296+
if prompts != 1 {
297+
t.Fatalf("expected one approval prompt, got %d", prompts)
298+
}
299+
}
300+
110301
func TestBalancedWriteViaSymlinkEscapeIsDenied(t *testing.T) {
111302
t.Setenv("HOME", t.TempDir())
112303

0 commit comments

Comments
 (0)