feat(engine): M8 cleanup — Wave B skill enforcement

- Add tool.PathSensitiveTool interface (ExtractPaths); implement on all 6 fs tools
- Add engine.TurnOptions.AllowedPaths: restricts tool filesystem access per skill invocation
- Bash is denied outright when AllowedPaths is active (unparseable command args)
- fs tools with empty path (cwd default) resolved via os.Getwd() and validated
- Add engine.TurnOptions.AllowedTools + AllowedPaths wiring in pipe mode (main.go) and TUI skill dispatch (tui/app.go)
- Remove TODO(M8.3) from skill.Frontmatter — enforcement is now complete
This commit is contained in:
2026-05-07 15:29:33 +02:00
parent fc465e5f29
commit 995b08dc0f
15 changed files with 462 additions and 4 deletions
+6 -1
View File
@@ -649,6 +649,7 @@ func main() {
// Resolve skill invocations in pipe mode (/skillname args).
submitInput := input
var submitOpts engine.TurnOptions
if strings.HasPrefix(input, "/") {
parts := strings.Fields(input)
name := strings.TrimPrefix(parts[0], "/")
@@ -666,10 +667,14 @@ func main() {
os.Exit(1)
}
submitInput = rendered
submitOpts = engine.TurnOptions{
AllowedTools: sk.Frontmatter.AllowedTools,
AllowedPaths: sk.Frontmatter.Paths,
}
}
}
_, err = eng.Submit(ctx, submitInput, cb)
_, err = eng.SubmitWithOptions(ctx, submitInput, submitOpts, cb)
fmt.Println()
if err != nil {
+1
View File
@@ -54,6 +54,7 @@ type Turn struct {
type TurnOptions struct {
ToolChoice provider.ToolChoiceMode // "" = use provider default
AllowedTools []string // if non-nil, only these tools are sent (matched by name)
AllowedPaths []string // if non-nil, tool filesystem access is restricted to these paths
}
// Engine orchestrates the conversation.
+5
View File
@@ -505,6 +505,11 @@ func (e *Engine) executeSingleTool(ctx context.Context, call message.ToolCall, t
}
}
// Path restriction: deny bash and validate fs tool paths against AllowedPaths.
if denied, blocked := checkPathRestriction(call, t, args, e.turnOpts.AllowedPaths); blocked {
return denied
}
e.logger.Debug("executing tool", "name", call.Name, "id", call.ID)
result, err := t.Execute(ctx, args)
+86
View File
@@ -0,0 +1,86 @@
package engine
import (
"encoding/json"
"fmt"
"os"
"path/filepath"
"strings"
"somegit.dev/Owlibou/gnoma/internal/message"
"somegit.dev/Owlibou/gnoma/internal/tool"
)
// isUnderAllowedPaths reports whether target is equal to or a descendant of any
// path in allowed. Both sides are cleaned before comparison. Returns false when
// allowed is empty.
//
// The trailing-separator check prevents "/tmp" from matching "/tmpx/foo".
func isUnderAllowedPaths(target string, allowed []string) bool {
target = filepath.Clean(target)
sep := string(filepath.Separator)
for _, a := range allowed {
a = filepath.Clean(a)
if target == a || strings.HasPrefix(target, a+sep) {
return true
}
}
return false
}
// checkPathRestriction enforces AllowedPaths on a single tool call.
//
// Rules (in order):
// 1. If allowed is empty, everything is permitted (fast-path).
// 2. "bash" is always denied when path restrictions are active.
// 3. Tools implementing tool.PathSensitiveTool have their extracted paths
// checked against allowed. An empty extracted path is resolved to cwd.
// 4. Tools that do not implement PathSensitiveTool are permitted (they don't
// declare filesystem access).
//
// Returns (denied result, true) when blocked, or (zero, false) when allowed.
func checkPathRestriction(call message.ToolCall, t tool.Tool, args json.RawMessage, allowed []string) (message.ToolResult, bool) {
if len(allowed) == 0 {
return message.ToolResult{}, false
}
if call.Name == "bash" {
return message.ToolResult{
ToolCallID: call.ID,
Content: "bash is not permitted when skill path restrictions are active",
IsError: true,
}, true
}
pt, ok := t.(tool.PathSensitiveTool)
if !ok {
return message.ToolResult{}, false
}
for _, p := range pt.ExtractPaths(args) {
var resolved string
if p == "" {
cwd, err := os.Getwd()
if err != nil {
return message.ToolResult{
ToolCallID: call.ID,
Content: fmt.Sprintf("path access denied: cannot determine current directory: %v", err),
IsError: true,
}, true
}
resolved = cwd
} else {
resolved = p
}
if !isUnderAllowedPaths(resolved, allowed) {
return message.ToolResult{
ToolCallID: call.ID,
Content: fmt.Sprintf("path access denied: %q is not in allowed paths %v", resolved, allowed),
IsError: true,
}, true
}
}
return message.ToolResult{}, false
}
+243
View File
@@ -0,0 +1,243 @@
package engine
import (
"context"
"encoding/json"
"testing"
"somegit.dev/Owlibou/gnoma/internal/message"
"somegit.dev/Owlibou/gnoma/internal/provider"
"somegit.dev/Owlibou/gnoma/internal/stream"
"somegit.dev/Owlibou/gnoma/internal/tool"
)
func TestIsUnderAllowedPaths(t *testing.T) {
tests := []struct {
name string
target string
allowed []string
want bool
}{
{"exact match", "/tmp/foo", []string{"/tmp/foo"}, true},
{"under allowed dir", "/tmp/foo/bar.go", []string{"/tmp"}, true},
{"not under allowed", "/etc/passwd", []string{"/tmp"}, false},
{"prevents prefix bypass", "/tmpx/foo", []string{"/tmp"}, false},
{"matches second path", "/home/user/file", []string{"/tmp", "/home/user"}, true},
{"empty allowed slice", "/tmp/foo", []string{}, false},
{"nested dir", "/project/src/main.go", []string{"/project"}, true},
{"sibling dir denied", "/project-evil/foo", []string{"/project"}, false},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := isUnderAllowedPaths(tc.target, tc.allowed)
if got != tc.want {
t.Errorf("isUnderAllowedPaths(%q, %v) = %v, want %v",
tc.target, tc.allowed, got, tc.want)
}
})
}
}
// mockPathSensitiveTool is a mock that implements both Tool and PathSensitiveTool.
type mockPathSensitiveTool struct {
mockTool
extractedPaths []string
}
func (m *mockPathSensitiveTool) ExtractPaths(_ json.RawMessage) []string {
return m.extractedPaths
}
func TestSubmitWithOptions_AllowedPaths_DeniesBash(t *testing.T) {
called := false
reg := tool.NewRegistry()
reg.Register(&mockTool{
name: "bash",
execFn: func(_ context.Context, _ json.RawMessage) (tool.Result, error) {
called = true
return tool.Result{Output: "should not reach here"}, nil
},
})
mp := &mockProvider{
name: "test",
streams: []stream.Stream{
newEventStream(message.StopToolUse, "",
stream.Event{Type: stream.EventToolCallStart, ToolCallID: "tc1", ToolCallName: "bash"},
stream.Event{Type: stream.EventToolCallDone, ToolCallID: "tc1", Args: json.RawMessage(`{"command":"cat /etc/passwd"}`)},
),
newEventStream(message.StopEndTurn, "",
stream.Event{Type: stream.EventTextDelta, Text: "blocked"},
),
},
}
e, _ := New(Config{Provider: mp, Tools: reg})
_, err := e.SubmitWithOptions(context.Background(), "run bash",
TurnOptions{AllowedPaths: []string{"/tmp"}}, nil)
if err != nil {
t.Fatalf("SubmitWithOptions: %v", err)
}
if called {
t.Error("bash tool should not be executed when AllowedPaths is set")
}
}
func TestSubmitWithOptions_AllowedPaths_DeniesOutsidePath(t *testing.T) {
called := false
reg := tool.NewRegistry()
reg.Register(&mockPathSensitiveTool{
mockTool: mockTool{
name: "fs.read",
execFn: func(_ context.Context, _ json.RawMessage) (tool.Result, error) {
called = true
return tool.Result{Output: "secret content"}, nil
},
},
extractedPaths: []string{"/etc/passwd"},
})
mp := &mockProvider{
name: "test",
streams: []stream.Stream{
newEventStream(message.StopToolUse, "",
stream.Event{Type: stream.EventToolCallStart, ToolCallID: "tc1", ToolCallName: "fs.read"},
stream.Event{Type: stream.EventToolCallDone, ToolCallID: "tc1", Args: json.RawMessage(`{"path":"/etc/passwd"}`)},
),
newEventStream(message.StopEndTurn, "",
stream.Event{Type: stream.EventTextDelta, Text: "blocked"},
),
},
}
e, _ := New(Config{Provider: mp, Tools: reg})
_, err := e.SubmitWithOptions(context.Background(), "read file",
TurnOptions{AllowedPaths: []string{"/tmp"}}, nil)
if err != nil {
t.Fatalf("SubmitWithOptions: %v", err)
}
if called {
t.Error("fs.read should not be executed when path is outside AllowedPaths")
}
}
func TestSubmitWithOptions_AllowedPaths_AllowsInsidePath(t *testing.T) {
called := false
reg := tool.NewRegistry()
reg.Register(&mockPathSensitiveTool{
mockTool: mockTool{
name: "fs.read",
execFn: func(_ context.Context, _ json.RawMessage) (tool.Result, error) {
called = true
return tool.Result{Output: "ok"}, nil
},
},
extractedPaths: []string{"/tmp/allowed/file.txt"},
})
mp := &mockProvider{
name: "test",
streams: []stream.Stream{
newEventStream(message.StopToolUse, "",
stream.Event{Type: stream.EventToolCallStart, ToolCallID: "tc1", ToolCallName: "fs.read"},
stream.Event{Type: stream.EventToolCallDone, ToolCallID: "tc1", Args: json.RawMessage(`{"path":"/tmp/allowed/file.txt"}`)},
),
newEventStream(message.StopEndTurn, "",
stream.Event{Type: stream.EventTextDelta, Text: "done"},
),
},
}
e, _ := New(Config{Provider: mp, Tools: reg})
_, err := e.SubmitWithOptions(context.Background(), "read file",
TurnOptions{AllowedPaths: []string{"/tmp/allowed"}}, nil)
if err != nil {
t.Fatalf("SubmitWithOptions: %v", err)
}
if !called {
t.Error("fs.read should be executed when path is inside AllowedPaths")
}
}
func TestSubmitWithOptions_NilAllowedPaths_NoRestriction(t *testing.T) {
called := false
reg := tool.NewRegistry()
reg.Register(&mockPathSensitiveTool{
mockTool: mockTool{
name: "fs.read",
execFn: func(_ context.Context, _ json.RawMessage) (tool.Result, error) {
called = true
return tool.Result{Output: "ok"}, nil
},
},
extractedPaths: []string{"/etc/passwd"},
})
mp := &mockProvider{
name: "test",
streams: []stream.Stream{
newEventStream(message.StopToolUse, "",
stream.Event{Type: stream.EventToolCallStart, ToolCallID: "tc1", ToolCallName: "fs.read"},
stream.Event{Type: stream.EventToolCallDone, ToolCallID: "tc1", Args: json.RawMessage(`{"path":"/etc/passwd"}`)},
),
newEventStream(message.StopEndTurn, "",
stream.Event{Type: stream.EventTextDelta, Text: "done"},
),
},
}
e, _ := New(Config{Provider: mp, Tools: reg})
// No AllowedPaths → no restriction
_, err := e.SubmitWithOptions(context.Background(), "read file", TurnOptions{}, nil)
if err != nil {
t.Fatalf("SubmitWithOptions: %v", err)
}
if !called {
t.Error("fs.read should be executed when AllowedPaths is not set")
}
}
func TestSubmitWithOptions_AllowedPaths_NonPathSensitiveToolAllowed(t *testing.T) {
// A tool that doesn't implement PathSensitiveTool should be permitted even
// when AllowedPaths is set — it doesn't access the filesystem directly.
called := false
reg := tool.NewRegistry()
reg.Register(&mockTool{ // plain mockTool, not PathSensitiveTool
name: "sysinfo",
execFn: func(_ context.Context, _ json.RawMessage) (tool.Result, error) {
called = true
return tool.Result{Output: "linux"}, nil
},
})
mp := &mockProvider{
name: "test",
streams: []stream.Stream{
newEventStream(message.StopToolUse, "",
stream.Event{Type: stream.EventToolCallStart, ToolCallID: "tc1", ToolCallName: "sysinfo"},
stream.Event{Type: stream.EventToolCallDone, ToolCallID: "tc1", Args: json.RawMessage(`{}`)},
),
newEventStream(message.StopEndTurn, "",
stream.Event{Type: stream.EventTextDelta, Text: "done"},
),
},
}
// Register arm with tool support
from := provider.Capabilities{ToolUse: true}
_ = from
e, _ := New(Config{Provider: mp, Tools: reg})
_, err := e.SubmitWithOptions(context.Background(), "get info",
TurnOptions{AllowedPaths: []string{"/tmp"}}, nil)
if err != nil {
t.Fatalf("SubmitWithOptions: %v", err)
}
if !called {
t.Error("non-path-sensitive tool should be permitted regardless of AllowedPaths")
}
}
+2 -2
View File
@@ -18,8 +18,8 @@ type Frontmatter struct {
Name string `yaml:"name"`
Description string `yaml:"description"`
WhenToUse string `yaml:"whenToUse"`
AllowedTools []string `yaml:"allowedTools"` // TODO(M8.3): enforce tool restrictions
Paths []string `yaml:"paths"` // TODO(M8.3): enforce path restrictions
AllowedTools []string `yaml:"allowedTools"`
Paths []string `yaml:"paths"`
}
// Skill is a parsed skill definition ready for invocation.
+8
View File
@@ -45,6 +45,14 @@ func (t *EditTool) Parameters() json.RawMessage { return editParams }
func (t *EditTool) IsReadOnly() bool { return false }
func (t *EditTool) IsDestructive() bool { return false }
func (t *EditTool) ExtractPaths(args json.RawMessage) []string {
var a editArgs
if err := json.Unmarshal(args, &a); err != nil {
return nil
}
return []string{a.Path}
}
type editArgs struct {
Path string `json:"path"`
OldString string `json:"old_string"`
+58
View File
@@ -624,6 +624,64 @@ func TestFormatSize(t *testing.T) {
}
}
// --- ExtractPaths ---
func TestExtractPaths_Read(t *testing.T) {
r := NewReadTool()
paths := r.ExtractPaths(mustJSON(t, readArgs{Path: "/foo/bar.txt"}))
if len(paths) != 1 || paths[0] != "/foo/bar.txt" {
t.Errorf("ExtractPaths = %v, want [/foo/bar.txt]", paths)
}
}
func TestExtractPaths_Write(t *testing.T) {
w := NewWriteTool()
paths := w.ExtractPaths(mustJSON(t, writeArgs{Path: "/foo/out.txt", Content: "x"}))
if len(paths) != 1 || paths[0] != "/foo/out.txt" {
t.Errorf("ExtractPaths = %v, want [/foo/out.txt]", paths)
}
}
func TestExtractPaths_Edit(t *testing.T) {
e := NewEditTool()
paths := e.ExtractPaths(mustJSON(t, editArgs{Path: "/foo/file.go", OldString: "a", NewString: "b"}))
if len(paths) != 1 || paths[0] != "/foo/file.go" {
t.Errorf("ExtractPaths = %v, want [/foo/file.go]", paths)
}
}
func TestExtractPaths_Glob_ExplicitPath(t *testing.T) {
g := NewGlobTool()
paths := g.ExtractPaths(mustJSON(t, globArgs{Pattern: "*.go", Path: "/project/src"}))
if len(paths) != 1 || paths[0] != "/project/src" {
t.Errorf("ExtractPaths = %v, want [/project/src]", paths)
}
}
func TestExtractPaths_Glob_EmptyPathIsCwd(t *testing.T) {
g := NewGlobTool()
paths := g.ExtractPaths(mustJSON(t, globArgs{Pattern: "*.go"}))
if len(paths) != 1 || paths[0] != "" {
t.Errorf("ExtractPaths = %v, want [\"\"] (empty = cwd)", paths)
}
}
func TestExtractPaths_Grep(t *testing.T) {
g := NewGrepTool()
paths := g.ExtractPaths(mustJSON(t, grepArgs{Pattern: "func", Path: "/project"}))
if len(paths) != 1 || paths[0] != "/project" {
t.Errorf("ExtractPaths = %v, want [/project]", paths)
}
}
func TestExtractPaths_LS(t *testing.T) {
l := NewLSTool()
paths := l.ExtractPaths(mustJSON(t, lsArgs{Path: "/project/src"}))
if len(paths) != 1 || paths[0] != "/project/src" {
t.Errorf("ExtractPaths = %v, want [/project/src]", paths)
}
}
// --- Helpers ---
func writeTestFile(t *testing.T, content string) string {
+8
View File
@@ -40,6 +40,14 @@ func (t *GlobTool) Parameters() json.RawMessage { return globParams }
func (t *GlobTool) IsReadOnly() bool { return true }
func (t *GlobTool) IsDestructive() bool { return false }
func (t *GlobTool) ExtractPaths(args json.RawMessage) []string {
var a globArgs
if err := json.Unmarshal(args, &a); err != nil {
return nil
}
return []string{a.Path} // empty string = caller resolves to cwd
}
type globArgs struct {
Pattern string `json:"pattern"`
Path string `json:"path,omitempty"`
+8
View File
@@ -51,6 +51,14 @@ func (t *GrepTool) Parameters() json.RawMessage { return grepParams }
func (t *GrepTool) IsReadOnly() bool { return true }
func (t *GrepTool) IsDestructive() bool { return false }
func (t *GrepTool) ExtractPaths(args json.RawMessage) []string {
var a grepArgs
if err := json.Unmarshal(args, &a); err != nil {
return nil
}
return []string{a.Path} // empty string = caller resolves to cwd
}
type grepArgs struct {
Pattern string `json:"pattern"`
Path string `json:"path,omitempty"`
+8
View File
@@ -34,6 +34,14 @@ func (t *LSTool) Parameters() json.RawMessage { return lsParams }
func (t *LSTool) IsReadOnly() bool { return true }
func (t *LSTool) IsDestructive() bool { return false }
func (t *LSTool) ExtractPaths(args json.RawMessage) []string {
var a lsArgs
if err := json.Unmarshal(args, &a); err != nil {
return nil
}
return []string{a.Path} // empty string = caller resolves to cwd
}
type lsArgs struct {
Path string `json:"path,omitempty"`
}
+8
View File
@@ -58,6 +58,14 @@ func (t *ReadTool) Parameters() json.RawMessage { return readParams }
func (t *ReadTool) IsReadOnly() bool { return true }
func (t *ReadTool) IsDestructive() bool { return false }
func (t *ReadTool) ExtractPaths(args json.RawMessage) []string {
var a readArgs
if err := json.Unmarshal(args, &a); err != nil {
return nil
}
return []string{a.Path}
}
type readArgs struct {
Path string `json:"path"`
Offset int `json:"offset,omitempty"`
+8
View File
@@ -52,6 +52,14 @@ func (t *WriteTool) Parameters() json.RawMessage { return writeParams }
func (t *WriteTool) IsReadOnly() bool { return false }
func (t *WriteTool) IsDestructive() bool { return false }
func (t *WriteTool) ExtractPaths(args json.RawMessage) []string {
var a writeArgs
if err := json.Unmarshal(args, &a); err != nil {
return nil
}
return []string{a.Path}
}
type writeArgs struct {
Path string `json:"path"`
Content string `json:"content"`
+8
View File
@@ -27,3 +27,11 @@ type Tool interface {
type DeferrableTool interface {
ShouldDefer() bool
}
// PathSensitiveTool is an optional interface for tools that access the filesystem.
// Engines enforcing skill path restrictions call ExtractPaths to validate each
// invocation before execution. An empty string in the returned slice means the
// tool will default to the current working directory.
type PathSensitiveTool interface {
ExtractPaths(args json.RawMessage) []string
}
+5 -1
View File
@@ -1164,7 +1164,11 @@ func (m Model) handleCommand(cmd string) (tea.Model, tea.Cmd) {
m.streamBuf.Reset()
m.thinkingBuf.Reset()
m.streamFilterClose = ""
if err := m.session.Send(rendered); err != nil {
skillOpts := engine.TurnOptions{
AllowedTools: sk.Frontmatter.AllowedTools,
AllowedPaths: sk.Frontmatter.Paths,
}
if err := m.session.SendWithOptions(rendered, skillOpts); err != nil {
m.messages = append(m.messages, chatMessage{role: "error", content: err.Error()})
m.streaming = false
return m, nil