Skip to content

Commit b46816b

Browse files
authored
fix(search): classify each discovered path in search_files and multi_grep (H-4, H-5) (#52)
* fix(search): classify each discovered path in search_files and multi_grep H-4: search_files and multi_grep previously only classified the root path, then walked and read every matching file. Add checkSearchPath helpers that classify each descended directory and each file the same way the root path is checked; sensitive paths are skipped and reported in a new Skipped field rather than returned silently. H-5: add regression tests for case-insensitive ~/.odek trust-anchor matching (macOS APFS) and shell rc files. - file_tool.go: wire checkSearchPath into searchContent and searchFiles; return Skipped in searchFilesResult. - perf_tools.go: add multiGrepTool.checkSearchPath; guard multi_grep walker; return Skipped in grepPatternResult. - file_tool_test.go / perf_tools_test.go / classifier_test.go: regression tests for skipped sensitive paths and case-insensitive anchors. * docs: document per-path search classification and case-insensitive anchors - AGENTS.md: add Search path classification bullet and note case-insensitive ~/.odek trust-anchor matching. - docs/SECURITY.md: add 3c subsection describing search_files/multi_grep per-path classification and the skipped field. - docs/CHEATSHEET.md: note that search_files/multi_grep report skipped sensitive paths.
1 parent cd1845d commit b46816b

9 files changed

Lines changed: 335 additions & 12 deletions

File tree

AGENTS.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
119119
- **REST API CSRF protection** (`cmd/odek/serve.go::requireLocalOrigin`) — state-changing HTTP endpoints (POST/PUT/PATCH/DELETE) require a localhost origin or no Origin header, and static responses set `X-Frame-Options: DENY` + `Content-Security-Policy: frame-ancestors 'none'` to block clickjacking.
120120
- **Browser history cap** (`cmd/odek/browser_tool.go`) — navigation history is capped at 50 snapshots to prevent memory DoS from repeated `browser_navigate` calls.
121121
- **Browser element cap** (`cmd/odek/browser_tool.go`) — the number of interactive elements extracted per page is capped at 500 so a hostile page cannot OOM the agent with thousands of links or buttons.
122+
- **Search path classification** (`cmd/odek/file_tool.go`, `cmd/odek/perf_tools.go`) — `search_files` and `multi_grep` classify every descended directory and every discovered file the same way the search root is classified. Sensitive paths that would require approval (or are denied) are skipped and reported in the `skipped` field rather than returned silently, closing the gap where a broad search root auto-approved sensitive files.
122123
- **Search result bounds** (`cmd/odek/file_tool.go`, `cmd/odek/perf_tools.go`) — `search_files` and `multi_grep` enforce a max match limit (500) and a total returned-content cap (1 MiB) to avoid unbounded result JSON.
123124
- **Perf-tool file-size cap** (`cmd/odek/perf_tools.go`) — `diff`, `base64`, `tr`, `sort`, `json_query`, and `batch_patch` reject files larger than 10 MiB to avoid loading multi-gigabyte files into memory.
124125
- **Shell output cap** (`cmd/odek/shell.go`, `cmd/odek/perf_tools.go`) — `shell` and `parallel_shell` cap captured stdout/stderr at 1 MiB per stream to prevent memory DoS from commands that dump huge files.
@@ -158,7 +159,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
158159
- **Schedule cross-process lock hard error** (`internal/schedule/store.go`) — `fileLock` now returns an error instead of silently falling back to a no-op releaser when `~/.odek/schedules.lock` cannot be opened or locked. Mutating schedule operations abort rather than risk two concurrent processes loading the same baseline and overwriting each other.
159160
- **Schedule JSON file-size cap** (`internal/schedule/store.go`) — `schedules.json` and `schedule-state.json` are rejected if larger than 10 MiB before being read into memory, preventing a tampered multi-gigabyte file from OOMing the scheduler.
160161
- **Default non-interactive policy is deny** (`internal/danger/classifier.go`, `cmd/odek/main.go`) — headless/CI/piped runs no longer auto-approve dangerous operations. Operators must explicitly set `non_interactive: "allow"` for unattended execution.
161-
- **`~/.odek` trust-anchor protection** (`internal/danger/classifier.go`, `cmd/odek/file_tool.go`) — generic file tools reject writes to `~/.odek/config.json`, `secrets.env`, `IDENTITY.md`, `skills/`, `sessions/`, `audit/`, `plans/`, `schedules.json`, `schedule-state.json`, `mcp_approvals.json`, `mcp_tool_approvals.json`, `restart.json`, `telegram.lock`, and related state files. These paths classify as `system_write` and must be modified through their dedicated subsystems. Shell reads of these trust anchors are also escalated to `system_write`.
162+
- **`~/.odek` trust-anchor protection** (`internal/danger/classifier.go`, `cmd/odek/file_tool.go`) — generic file tools reject writes to `~/.odek/config.json`, `secrets.env`, `IDENTITY.md`, `skills/`, `sessions/`, `audit/`, `plans/`, `schedules.json`, `schedule-state.json`, `mcp_approvals.json`, `mcp_tool_approvals.json`, `restart.json`, `telegram.lock`, and related state files. These paths classify as `system_write` and must be modified through their dedicated subsystems. Matching is case-insensitive so variants such as `CONFIG.JSON` or `SECRETS.ENV` are also blocked on case-insensitive filesystems (e.g. macOS APFS). Shell reads of these trust anchors are also escalated to `system_write`.
162163
- **Nonce'd tool-result delimiter** (`internal/loop/loop.go`) — the static `┌── TOOL RESULT: ... └── END TOOL RESULT: ...` delimiter is now unique per tool call via a random hex nonce embedded in both the opening and closing lines. A tool or MCP server can no longer forge the closing delimiter to break out of the data framing and inject instructions.
163164
- **`parallel_shell` context + process-group kill** (`cmd/odek/perf_tools.go`) — commands now run via `exec.CommandContext` bound to the agent context, in their own process group. Cancellation or timeout kills the whole group (negative PID), so `sh -c 'sleep 3600 &'` cannot leave orphaned children. Per-command timeouts are also capped at 30 minutes.
164165
- **`batch_patch` trusted-class propagation** (`cmd/odek/perf_tools.go`) — `batch_patch` now passes its cached `trustedClasses` to `CheckOperation`, matching `write_file` and `patch`. A trusted `local_write` class is honored across all patches in the batch instead of re-prompting per patch.

cmd/odek/file_tool.go

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,7 @@ type searchMatch struct {
525525

526526
type searchFilesResult struct {
527527
Matches []searchMatch `json:"matches"`
528+
Skipped []string `json:"skipped,omitempty"`
528529
Error string `json:"error,omitempty"`
529530
}
530531

@@ -567,13 +568,28 @@ func (t *searchFilesTool) Call(argsJSON string) (string, error) {
567568
}
568569
}
569570

571+
// checkSearchPath classifies a discovered path the same way the root path was
572+
// classified. If the path is more restrictive (e.g. a file under ~/.odek
573+
// discovered while searching $HOME), it returns skip=true so the walker does
574+
// not silently read sensitive files.
575+
func (t *searchFilesTool) checkSearchPath(path string) (skip bool, reason string) {
576+
risk := danger.ClassifyPath(path)
577+
if err := t.dangerousConfig.CheckOperation(danger.ToolOperation{
578+
Name: "search_files", Resource: path, Risk: risk,
579+
}, nil); err != nil {
580+
return true, err.Error()
581+
}
582+
return false, ""
583+
}
584+
570585
func (t *searchFilesTool) searchContent(args searchFilesArgs) (string, error) {
571586
re, err := regexp.Compile(args.Pattern)
572587
if err != nil {
573588
return jsonError(fmt.Sprintf("invalid regex pattern %q: %v", args.Pattern, err))
574589
}
575590

576591
var matches []searchMatch
592+
var skipped []string
577593
limit := args.Limit
578594
resultBytes := 0
579595

@@ -590,6 +606,13 @@ func (t *searchFilesTool) searchContent(args searchFilesArgs) (string, error) {
590606
if skipDir(info.Name()) {
591607
return filepath.SkipDir
592608
}
609+
// Security: a broad search root may contain a sensitive subtree
610+
// (e.g. ~/.odek under $HOME). Classify the directory before
611+
// descending; if it would require approval/denial, skip it.
612+
if skip, reason := t.checkSearchPath(path); skip {
613+
skipped = append(skipped, path+": "+reason)
614+
return filepath.SkipDir
615+
}
593616
return nil
594617
}
595618
// Skip symlinks — prevents TOCTOU on the path and avoids listing
@@ -598,6 +621,13 @@ func (t *searchFilesTool) searchContent(args searchFilesArgs) (string, error) {
598621
return nil
599622
}
600623

624+
// Security: classify each file before reading. This prevents a broad
625+
// search from silently returning files that read_file would gate.
626+
if skip, reason := t.checkSearchPath(path); skip {
627+
skipped = append(skipped, path+": "+reason)
628+
return nil
629+
}
630+
601631
// Apply file_glob filter
602632
if args.FileGlob != "" {
603633
match, _ := filepath.Match(args.FileGlob, info.Name())
@@ -659,7 +689,7 @@ func (t *searchFilesTool) searchContent(args searchFilesArgs) (string, error) {
659689
return jsonError(fmt.Sprintf("search failed: %v", err))
660690
}
661691

662-
return jsonResult(searchFilesResult{Matches: matches})
692+
return jsonResult(searchFilesResult{Matches: matches, Skipped: skipped})
663693
}
664694

665695
func (t *searchFilesTool) searchFiles(args searchFilesArgs) (string, error) {
@@ -673,7 +703,15 @@ func (t *searchFilesTool) searchFiles(args searchFilesArgs) (string, error) {
673703
}
674704

675705
var matches []searchMatch
706+
var skipped []string
676707
for _, p := range paths {
708+
// Security: each discovered path is classified the same way the root
709+
// path is. Sensitive files (e.g. under ~/.odek, /etc) are skipped
710+
// rather than returned silently.
711+
if skip, reason := t.checkSearchPath(p); skip {
712+
skipped = append(skipped, p+": "+reason)
713+
continue
714+
}
677715
matches = append(matches, searchMatch{Path: wrapUntrusted(t.toolCtx(), "search_files:"+p, p)})
678716
}
679717

@@ -688,7 +726,7 @@ func (t *searchFilesTool) searchFiles(args searchFilesArgs) (string, error) {
688726
return fi.ModTime().After(fj.ModTime())
689727
})
690728

691-
return jsonResult(searchFilesResult{Matches: matches})
729+
return jsonResult(searchFilesResult{Matches: matches, Skipped: skipped})
692730
}
693731

694732
// ── Patch Tool ─────────────────────────────────────────────────────────
@@ -1091,14 +1129,16 @@ func confineToCWD(path string) (string, error) {
10911129
// must not be writable through the generic file tools. Keep in sync with
10921130
// the SystemWrite escalation in danger.ClassifyPath.
10931131
func isProtectedOdekPath(rel string) bool {
1094-
rel = filepath.Clean(rel)
1132+
// Case-folding defends against case-insensitive filesystems (macOS APFS):
1133+
// ~/.odek/CONFIG.JSON and ~/.odek/config.json are the same file.
1134+
rel = strings.ToLower(filepath.Clean(rel))
10951135

10961136
// Exact-file trust anchors. Rewriting any of these can disable safety
10971137
// policy, exfiltrate secrets, or persist attacker control.
10981138
protectedExact := []string{
10991139
"config.json",
11001140
"secrets.env",
1101-
"IDENTITY.md",
1141+
"identity.md",
11021142
"schedules.json",
11031143
"schedule-state.json",
11041144
"schedules.lock",

cmd/odek/file_tool_test.go

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"os"
77
"path/filepath"
88
"strings"
9+
"sync"
910
"testing"
1011

1112
"github.com/BackendStack21/odek/internal/danger"
@@ -1220,6 +1221,179 @@ func TestWriteFile_SecurityDenied(t *testing.T) {
12201221
}
12211222
}
12221223

1224+
// ── Search path classification tests (H-4 / H-5) ────────────────────────
1225+
1226+
// countingApprover is a test double that approves the first N prompt requests
1227+
// and denies the rest. It lets us approve a search root while denying the
1228+
// sensitive files discovered underneath it.
1229+
type countingApprover struct {
1230+
mu sync.Mutex
1231+
calls int
1232+
allowFirstN int
1233+
}
1234+
1235+
func (a *countingApprover) PromptCommand(cls danger.RiskClass, cmd, description string) error {
1236+
return a.prompt()
1237+
}
1238+
1239+
func (a *countingApprover) PromptOperation(op danger.ToolOperation) error {
1240+
return a.prompt()
1241+
}
1242+
1243+
func (a *countingApprover) prompt() error {
1244+
a.mu.Lock()
1245+
defer a.mu.Unlock()
1246+
a.calls++
1247+
if a.calls <= a.allowFirstN {
1248+
return nil
1249+
}
1250+
return fmt.Errorf("denied by test approver")
1251+
}
1252+
1253+
// makeTestHomeDir creates a HOME directory outside /tmp and /var so that
1254+
// ClassifyPath treats paths under it as real home-directory paths rather than
1255+
// temporary/system paths. The directory is created under the current working
1256+
// directory and removed when the test ends.
1257+
func makeTestHomeDir(t *testing.T) string {
1258+
t.Helper()
1259+
cwd, err := os.Getwd()
1260+
if err != nil {
1261+
t.Fatalf("cannot get cwd: %v", err)
1262+
}
1263+
home, err := os.MkdirTemp(cwd, "odek-test-home-*")
1264+
if err != nil {
1265+
t.Fatalf("cannot create test home: %v", err)
1266+
}
1267+
t.Cleanup(func() { os.RemoveAll(home) })
1268+
return home
1269+
}
1270+
1271+
func TestSearchFiles_TargetFiles_SkipsSensitiveOdekPaths(t *testing.T) {
1272+
home := makeTestHomeDir(t)
1273+
t.Setenv("HOME", home)
1274+
1275+
odekDir := filepath.Join(home, ".odek")
1276+
if err := os.MkdirAll(odekDir, 0755); err != nil {
1277+
t.Fatal(err)
1278+
}
1279+
os.WriteFile(filepath.Join(odekDir, "config.json"), []byte("api-key-secret\n"), 0644)
1280+
os.WriteFile(filepath.Join(odekDir, "notes.md"), []byte("hello world\n"), 0644)
1281+
1282+
cfg := danger.DangerousConfig{
1283+
Classes: map[danger.RiskClass]danger.Action{danger.SystemWrite: danger.Prompt},
1284+
Approver: &countingApprover{allowFirstN: 1}, // approve the search root only
1285+
}
1286+
tool := &searchFilesTool{dangerousConfig: cfg}
1287+
1288+
args := fmt.Sprintf(`{"pattern":"*","target":"files","path":%q}`, odekDir)
1289+
result := callJSON(t, tool, args)
1290+
1291+
var r struct {
1292+
Matches []struct {
1293+
Path string `json:"path"`
1294+
} `json:"matches"`
1295+
Skipped []string `json:"skipped"`
1296+
Error string `json:"error,omitempty"`
1297+
}
1298+
mustUnmarshal(t, result, &r)
1299+
if r.Error != "" {
1300+
t.Fatalf("unexpected error: %s", r.Error)
1301+
}
1302+
if len(r.Matches) != 1 {
1303+
t.Fatalf("expected 1 allowed match, got %d", len(r.Matches))
1304+
}
1305+
if !strings.Contains(unwrapUntrusted(r.Matches[0].Path), "notes.md") {
1306+
t.Errorf("expected notes.md match, got %q", r.Matches[0].Path)
1307+
}
1308+
if len(r.Skipped) != 1 {
1309+
t.Fatalf("expected 1 skipped sensitive path, got %d: %v", len(r.Skipped), r.Skipped)
1310+
}
1311+
if !strings.Contains(r.Skipped[0], "config.json") {
1312+
t.Errorf("expected config.json in skipped list, got %q", r.Skipped[0])
1313+
}
1314+
}
1315+
1316+
func TestSearchFiles_TargetFiles_SkipsCaseInsensitiveAnchor(t *testing.T) {
1317+
home := makeTestHomeDir(t)
1318+
t.Setenv("HOME", home)
1319+
1320+
odekDir := filepath.Join(home, ".odek")
1321+
if err := os.MkdirAll(odekDir, 0755); err != nil {
1322+
t.Fatal(err)
1323+
}
1324+
os.WriteFile(filepath.Join(odekDir, "CONFIG.JSON"), []byte("api-key-secret\n"), 0644)
1325+
os.WriteFile(filepath.Join(odekDir, "notes.md"), []byte("hello world\n"), 0644)
1326+
1327+
cfg := danger.DangerousConfig{
1328+
Classes: map[danger.RiskClass]danger.Action{danger.SystemWrite: danger.Prompt},
1329+
Approver: &countingApprover{allowFirstN: 1},
1330+
}
1331+
tool := &searchFilesTool{dangerousConfig: cfg}
1332+
1333+
args := fmt.Sprintf(`{"pattern":"*","target":"files","path":%q}`, odekDir)
1334+
result := callJSON(t, tool, args)
1335+
1336+
var r struct {
1337+
Matches []struct {
1338+
Path string `json:"path"`
1339+
} `json:"matches"`
1340+
Skipped []string `json:"skipped"`
1341+
Error string `json:"error,omitempty"`
1342+
}
1343+
mustUnmarshal(t, result, &r)
1344+
if r.Error != "" {
1345+
t.Fatalf("unexpected error: %s", r.Error)
1346+
}
1347+
if len(r.Matches) != 1 {
1348+
t.Fatalf("expected 1 allowed match, got %d", len(r.Matches))
1349+
}
1350+
if len(r.Skipped) != 1 || !strings.Contains(strings.ToLower(r.Skipped[0]), "config.json") {
1351+
t.Fatalf("expected CONFIG.JSON skipped, got %v", r.Skipped)
1352+
}
1353+
}
1354+
1355+
func TestSearchFiles_CheckSearchPath_DeniesSystemWrite(t *testing.T) {
1356+
home := makeTestHomeDir(t)
1357+
t.Setenv("HOME", home)
1358+
1359+
cfg := danger.DangerousConfig{
1360+
Classes: map[danger.RiskClass]danger.Action{danger.SystemWrite: danger.Deny},
1361+
}
1362+
tool := &searchFilesTool{dangerousConfig: cfg}
1363+
1364+
skip, reason := tool.checkSearchPath(filepath.Join(home, ".odek", "config.json"))
1365+
if !skip {
1366+
t.Fatalf("expected checkSearchPath to skip ~/.odek/config.json")
1367+
}
1368+
if !strings.Contains(reason, "system_write") {
1369+
t.Errorf("expected system_write denial, got %q", reason)
1370+
}
1371+
1372+
skip, _ = tool.checkSearchPath(filepath.Join(home, ".odek", "notes.md"))
1373+
if skip {
1374+
t.Errorf("expected non-anchor ~/.odek path to be allowed")
1375+
}
1376+
}
1377+
1378+
// TestIsProtectedOdekPath_CaseInsensitive verifies the file-tool write guard
1379+
// rejects case variants of trust anchors on case-insensitive filesystems.
1380+
func TestIsProtectedOdekPath_CaseInsensitive(t *testing.T) {
1381+
cases := []string{
1382+
"CONFIG.JSON",
1383+
"Secrets.Env",
1384+
"IDENTITY.MD",
1385+
"SKILLS/evil/SKILL.md",
1386+
"SESSIONS/abc.json",
1387+
"AUDIT/turn.json",
1388+
"PLANS/evil.md",
1389+
}
1390+
for _, c := range cases {
1391+
if !isProtectedOdekPath(c) {
1392+
t.Errorf("isProtectedOdekPath(%q) = false, want true", c)
1393+
}
1394+
}
1395+
}
1396+
12231397
// ── Helpers ────────────────────────────────────────────────────────────
12241398

12251399
func callJSON(t *testing.T, tool interface {

cmd/odek/perf_tools.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,6 +1198,7 @@ type grepPatternResult struct {
11981198
Pattern string `json:"pattern"`
11991199
Matches []grepMatch `json:"matches"`
12001200
Count int `json:"count"`
1201+
Skipped []string `json:"skipped,omitempty"`
12011202
Error string `json:"error,omitempty"`
12021203
}
12031204

@@ -1269,6 +1270,20 @@ func (t *multiGrepTool) Call(argsJSON string) (string, error) {
12691270
return jsonResult(multiGrepResult{Results: results})
12701271
}
12711272

1273+
// checkSearchPath classifies a discovered path the same way the root path was
1274+
// checked in Call. If the path is more restrictive (e.g. a file under ~/.odek
1275+
// discovered while searching $HOME), it returns skip=true so the walker does
1276+
// not silently read sensitive files.
1277+
func (t *multiGrepTool) checkSearchPath(path string) (skip bool, reason string) {
1278+
risk := danger.ClassifyPath(path)
1279+
if err := t.dangerousConfig.CheckOperation(danger.ToolOperation{
1280+
Name: "multi_grep", Resource: path, Risk: risk,
1281+
}, nil); err != nil {
1282+
return true, err.Error()
1283+
}
1284+
return false, ""
1285+
}
1286+
12721287
func (t *multiGrepTool) searchPattern(pattern, root, fileGlob string, limit int) (result grepPatternResult) {
12731288
defer func() {
12741289
if r := recover(); r != nil {
@@ -1283,6 +1298,7 @@ func (t *multiGrepTool) searchPattern(pattern, root, fileGlob string, limit int)
12831298
var matches []grepMatch
12841299
resultBytes := 0
12851300

1301+
var skipped []string
12861302
filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
12871303
if err != nil || info == nil {
12881304
return nil
@@ -1294,12 +1310,22 @@ func (t *multiGrepTool) searchPattern(pattern, root, fileGlob string, limit int)
12941310
if skipDir(info.Name()) {
12951311
return filepath.SkipDir
12961312
}
1313+
// Security: classify each directory before descending.
1314+
if skip, reason := t.checkSearchPath(path); skip {
1315+
skipped = append(skipped, path+": "+reason)
1316+
return filepath.SkipDir
1317+
}
12971318
return nil
12981319
}
12991320
// Skip symlinks — prevents TOCTOU and listing unreadable files.
13001321
if info.Mode()&os.ModeSymlink != 0 {
13011322
return nil
13021323
}
1324+
// Security: classify each file before reading.
1325+
if skip, reason := t.checkSearchPath(path); skip {
1326+
skipped = append(skipped, path+": "+reason)
1327+
return nil
1328+
}
13031329
if fileGlob != "" {
13041330
match, _ := filepath.Match(fileGlob, info.Name())
13051331
if !match {
@@ -1352,6 +1378,7 @@ func (t *multiGrepTool) searchPattern(pattern, root, fileGlob string, limit int)
13521378
Pattern: pattern,
13531379
Matches: matches,
13541380
Count: len(matches),
1381+
Skipped: skipped,
13551382
}
13561383
}
13571384

0 commit comments

Comments
 (0)