Skip to content

Commit 73582b1

Browse files
committed
fix: next 5 security vulnerabilities (patch, glob, subagent task, transcribe output, session_search get)
1 parent 67e3da7 commit 73582b1

7 files changed

Lines changed: 224 additions & 5 deletions

AGENTS.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ System prompt is loaded by priority: `--system` flag > `~/.odek/IDENTITY.md` > c
9191
### Security Architecture
9292
Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECURITY.md](docs/SECURITY.md).
9393

94-
- **Untrusted-content wrapper** (`cmd/odek/untrusted.go`) — every tool whose output sources from outside the trust boundary (`browser`, `read_file`, `shell`, `search_files`, `multi_grep`, `transcribe`, `head_tail`, `diff`, `tr`, `sort`, `json_query`, `batch_patch`, any MCP tool) wraps results in `<untrusted_content_<nonce> source="...">…</untrusted_content_<nonce>>`. Per-call nonce defeats wrapper-escape via literal close tag.
94+
- **Untrusted-content wrapper** (`cmd/odek/untrusted.go`) — every tool whose output sources from outside the trust boundary (`browser`, `read_file`, `shell`, `search_files`, `multi_grep`, `transcribe`, `head_tail`, `diff`, `tr`, `sort`, `json_query`, `batch_patch`, `glob`, `session_search`, any MCP tool) wraps results in `<untrusted_content_<nonce> source="...">…</untrusted_content_<nonce>>`. Per-call nonce defeats wrapper-escape via literal close tag.
9595
- **Audit log** (`cmd/odek/audit.go` + `internal/session/audit.go`) — every `wrapUntrusted` call records source + content-hash + turn into `<sessions>/audit/<id>.json`. After each turn a divergence heuristic flags `suspicious_divergence=true` when the agent ingested untrusted content AND its tool calls referenced resources the user did not mention. Inspect with `odek audit <session-id>` / `odek audit --list`.
9696
- **Memory taint** (`internal/memory/provenance.go`) — `EpisodeProvenance` tracks Untrusted/Sources/UserApproved. Tainted episodes are stored but `Search()` filters them out, so a one-shot injection cannot persist via the episode pipeline. User must explicitly promote.
9797
- **Skill provenance gate** (`internal/skills/loader.go` + `cache.go`) — `Skill.Provenance{Untrusted, Sources, NeedsReview}`. NeedsReview skills pin to Lazy regardless of `auto_load`. `odek skill promote <name>` clears the flag after user review.
@@ -106,8 +106,12 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
106106
- **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.
107107
- **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.
108108
- **Browser request timeout** (`cmd/odek/browser_tool.go`) — the browser HTTP client enforces a 30-second request timeout so a slow/malicious server cannot hang the agent turn.
109-
- **Transcribe input guard** (`cmd/odek/transcribe_tool.go`) — rejects audio files larger than 10 MiB and writes ffmpeg output to a temp file so it cannot clobber an existing `.wav` next to the source path.
109+
- **Transcribe input/output guard** (`cmd/odek/transcribe_tool.go`) — rejects audio files larger than 10 MiB, caps whisper stdout at 10 MiB, and writes ffmpeg output to a temp file so it cannot clobber an existing `.wav` next to the source path.
110110
- **Tree width cap** (`cmd/odek/perf_tools.go`) — the `tree` tool limits each directory listing to 1,000 entries to avoid OOM from directories with millions of files.
111+
- **patch tool hardening** (`cmd/odek/file_tool.go`) — `patch` rejects files larger than 10 MiB and preserves the original file mode instead of resetting it to 0644.
112+
- **glob tool hardening** (`cmd/odek/file_tool.go`) — `glob` caps results at 1,000 matches and wraps returned paths as untrusted content.
113+
- **Sub-agent task-file cap** (`cmd/odek/subagent.go`) — `odek subagent --task <file>` rejects task files larger than 10 MiB before loading them into memory.
114+
- **session_search get hardening** (`cmd/odek/session_search_tool.go`) — the `get` action returns at most the 100 most recent messages and wraps each message content as untrusted.
111115
- **Serve sandbox default-on**`odek serve` enables `--sandbox` automatically unless `--no-sandbox` is passed.
112116
- **Secret redaction** (`internal/redact/redact.go`) — 20+ patterns: OpenAI, Anthropic, GitHub PAT, AWS, PEM, JWT, Vault, Google OAuth, SendGrid, Discord, DB URLs, etc.
113117

cmd/odek/file_tool.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ const maxSearchLimit = 500
3333
// search_files / multi_grep content query.
3434
const maxSearchResultBytes = maxReadBytes
3535

36+
// maxGlobMatches caps the number of paths returned by the glob tool to prevent
37+
// unbounded JSON responses from broad patterns.
38+
const maxGlobMatches = 1000
39+
3640
type readFileTool struct {
3741
dangerousConfig danger.DangerousConfig
3842
}
@@ -656,6 +660,16 @@ func (t *patchTool) Call(argsJSON string) (string, error) {
656660
}
657661
defer f.Close()
658662

663+
// Reject files that would exhaust memory during the read/edit/write cycle.
664+
info, err := f.Stat()
665+
if err != nil {
666+
return jsonError(fmt.Sprintf("cannot stat %q: %v", args.Path, err))
667+
}
668+
if info.Size() > maxFileReadBytes {
669+
return jsonError(fmt.Sprintf("file too large (%d bytes, max %d)", info.Size(), maxFileReadBytes))
670+
}
671+
origMode := info.Mode().Perm()
672+
659673
// Read content through the opened fd (not re-opening the path)
660674
var sb strings.Builder
661675
_, err = io.Copy(&sb, f)
@@ -699,7 +713,7 @@ func (t *patchTool) Call(argsJSON string) (string, error) {
699713
os.Remove(tmpPath)
700714
return jsonError(fmt.Sprintf("cannot write %q: %v", args.Path, err))
701715
}
702-
if err := tmpFile.Chmod(0644); err != nil {
716+
if err := tmpFile.Chmod(origMode); err != nil {
703717
tmpFile.Close()
704718
os.Remove(tmpPath)
705719
return jsonError(fmt.Sprintf("cannot set permissions %q: %v", args.Path, err))
@@ -1155,6 +1169,9 @@ func (t *globTool) Call(argsJSON string) (result string, err error) {
11551169
if args.Limit <= 0 {
11561170
args.Limit = maxMatches
11571171
}
1172+
if args.Limit > maxGlobMatches {
1173+
args.Limit = maxGlobMatches
1174+
}
11581175

11591176
// Security: classify search root path
11601177
risk := danger.ClassifyPath(args.Path)
@@ -1278,6 +1295,10 @@ func (t *globTool) Call(argsJSON string) (result string, err error) {
12781295
return fi.ModTime().After(fj.ModTime())
12791296
})
12801297

1298+
for i := range matches {
1299+
matches[i].Path = wrapUntrusted("glob:"+args.Path, matches[i].Path)
1300+
}
1301+
12811302
return jsonResult(globResult{Matches: matches})
12821303
}
12831304

cmd/odek/next_security_vulnerabilities_test.go

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ import (
1111
"testing"
1212
"time"
1313

14+
"github.com/BackendStack21/odek/internal/config"
1415
"github.com/BackendStack21/odek/internal/danger"
16+
"github.com/BackendStack21/odek/internal/llm"
17+
"github.com/BackendStack21/odek/internal/session"
1518
)
1619

1720
// ── 1. Browser history must be capped to avoid memory DoS ────────────────
@@ -475,3 +478,173 @@ func TestTree_CapsDirectoryWidth(t *testing.T) {
475478
t.Fatalf("tree did not cap directory width: got %d children", len(r.Tree.Children))
476479
}
477480
}
481+
482+
483+
// ── 11. patch must reject huge files and preserve original permissions ───
484+
485+
func TestPatch_RejectsHugeFile(t *testing.T) {
486+
dir := t.TempDir()
487+
path := filepath.Join(dir, "huge.txt")
488+
os.WriteFile(path, []byte(strings.Repeat("x", 15*1024*1024)), 0644)
489+
490+
tool := &patchTool{}
491+
result := callJSON(t, tool, fmt.Sprintf(`{"path":%q,"old_string":"xxx","new_string":"yyy"}`, path))
492+
var r struct {
493+
Success bool `json:"success"`
494+
Error string `json:"error,omitempty"`
495+
}
496+
mustUnmarshal(t, result, &r)
497+
if r.Success {
498+
t.Fatal("patch should reject a 15 MiB file")
499+
}
500+
if !strings.Contains(r.Error, "too large") {
501+
t.Fatalf("patch should reject huge file with a size error, got: %q", r.Error)
502+
}
503+
}
504+
505+
func TestPatch_PreservesFileMode(t *testing.T) {
506+
dir := t.TempDir()
507+
path := filepath.Join(dir, "script.sh")
508+
os.WriteFile(path, []byte("#!/bin/sh\necho hello\n"), 0755)
509+
510+
tool := &patchTool{}
511+
result := callJSON(t, tool, fmt.Sprintf(`{"path":%q,"old_string":"hello","new_string":"world"}`, path))
512+
var r struct {
513+
Success bool `json:"success"`
514+
}
515+
mustUnmarshal(t, result, &r)
516+
if !r.Success {
517+
t.Fatal("patch failed")
518+
}
519+
520+
info, err := os.Stat(path)
521+
if err != nil {
522+
t.Fatal(err)
523+
}
524+
if info.Mode().Perm() != 0755 {
525+
t.Fatalf("patch changed mode from 0755 to %04o", info.Mode().Perm())
526+
}
527+
}
528+
529+
// ── 12. glob must cap match count and wrap paths as untrusted ────────────
530+
531+
func TestGlob_CapsMatchCount(t *testing.T) {
532+
dir := t.TempDir()
533+
for i := 0; i < 1500; i++ {
534+
os.WriteFile(filepath.Join(dir, fmt.Sprintf("file%d.txt", i)), []byte("x"), 0644)
535+
}
536+
537+
tool := &globTool{dangerousConfig: danger.DangerousConfig{}}
538+
result := callJSON(t, tool, fmt.Sprintf(`{"pattern":"*","path":%q,"limit":10000}`, dir))
539+
var r struct {
540+
Matches []struct {
541+
Path string `json:"path"`
542+
} `json:"matches"`
543+
}
544+
mustUnmarshal(t, result, &r)
545+
if len(r.Matches) > 1000 {
546+
t.Fatalf("glob did not cap match count: got %d", len(r.Matches))
547+
}
548+
if len(r.Matches) == 0 {
549+
t.Fatal("expected at least one match")
550+
}
551+
if !strings.HasPrefix(r.Matches[0].Path, "<untrusted_content_") {
552+
t.Fatalf("glob path should be wrapped in untrusted_content, got: %q", r.Matches[0].Path)
553+
}
554+
}
555+
556+
// ── 13. subagent must reject a huge task file ────────────────────────────
557+
558+
func TestSubagent_RejectsHugeTaskFile(t *testing.T) {
559+
dir := t.TempDir()
560+
path := filepath.Join(dir, "task.json")
561+
os.WriteFile(path, []byte(`{"goal":"`+strings.Repeat("x", 15*1024*1024)+`"}`), 0600)
562+
563+
err := subagentCmd([]string{"--task", path})
564+
if err == nil {
565+
t.Fatal("subagent should reject a huge task file")
566+
}
567+
if !strings.Contains(err.Error(), "too large") {
568+
t.Fatalf("subagent should reject huge task file with a size error, got: %v", err)
569+
}
570+
}
571+
572+
// ── 14. transcribe must cap whisper stdout ───────────────────────────────
573+
574+
func TestTranscribe_CapsWhisperOutput(t *testing.T) {
575+
dir := t.TempDir()
576+
fakeBinary := filepath.Join(dir, "whisper")
577+
fakeModel := filepath.Join(dir, "model.bin")
578+
// Fake whisper: streams valid-ish opening JSON then floods stdout.
579+
script := `#!/bin/sh
580+
head -c 20000000 /dev/zero | tr '\0' 'x'
581+
exit 0
582+
`
583+
os.WriteFile(fakeBinary, []byte(script), 0755)
584+
os.WriteFile(fakeModel, []byte("fake model"), 0644)
585+
586+
audioPath := filepath.Join(dir, "audio.wav")
587+
os.WriteFile(audioPath, []byte("fake wav"), 0644)
588+
589+
tool := newTranscribeTool(danger.DangerousConfig{}, config.TranscriptionConfig{
590+
BinaryPath: fakeBinary,
591+
Model: fakeModel,
592+
})
593+
result := callJSON(t, tool, fmt.Sprintf(`{"path":%q}`, audioPath))
594+
var r struct {
595+
Error string `json:"error,omitempty"`
596+
}
597+
mustUnmarshal(t, result, &r)
598+
if !strings.Contains(r.Error, "too large") {
599+
t.Fatalf("transcribe should cap whisper output, got: %q", r.Error)
600+
}
601+
}
602+
603+
// ── 15. session_search get must cap/wrap returned messages ───────────────
604+
605+
func TestSessionSearchGet_CapsAndWrapsMessages(t *testing.T) {
606+
dir := t.TempDir()
607+
t.Setenv("HOME", dir)
608+
609+
store, err := session.NewStore()
610+
if err != nil {
611+
t.Fatalf("NewStore: %v", err)
612+
}
613+
614+
sess := &session.Session{
615+
ID: "test-session",
616+
Task: "test",
617+
Model: "test-model",
618+
CreatedAt: time.Now(),
619+
UpdatedAt: time.Now(),
620+
}
621+
for i := 0; i < 150; i++ {
622+
sess.Messages = append(sess.Messages, llm.Message{Role: "assistant", Content: fmt.Sprintf("msg %d", i)})
623+
}
624+
if err := store.Save(sess); err != nil {
625+
t.Fatalf("Save: %v", err)
626+
}
627+
628+
tool := &sessionSearchTool{store: store}
629+
result := callJSON(t, tool, `{"action":"get","query":"test-session"}`)
630+
t.Logf("session get result: %s", result)
631+
var r struct {
632+
Error string `json:"error,omitempty"`
633+
SessionMessages []struct {
634+
Content string `json:"content"`
635+
} `json:"session_messages"`
636+
}
637+
mustUnmarshal(t, result, &r)
638+
if r.Error != "" {
639+
t.Fatalf("session get error: %s", r.Error)
640+
}
641+
if len(r.SessionMessages) > 100 {
642+
t.Fatalf("session_search get did not cap messages: got %d", len(r.SessionMessages))
643+
}
644+
if len(r.SessionMessages) == 0 {
645+
t.Fatal("expected at least one message")
646+
}
647+
if !strings.HasPrefix(r.SessionMessages[0].Content, "<untrusted_content_") {
648+
t.Fatalf("session message should be wrapped in untrusted_content, got: %q", r.SessionMessages[0].Content)
649+
}
650+
}

cmd/odek/session_search_tool.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,9 @@ func (t *sessionSearchTool) handleGet(id string) (string, error) {
362362
})
363363
}
364364

365-
// Build session messages for the LLM to read.
365+
// Build session messages for the LLM to read. Cap how many are returned
366+
// and treat the content as untrusted because it includes prior tool outputs.
367+
const maxSessionGetMessages = 100
366368
var sessionMessages []sessionMessage
367369
for _, m := range sess.Messages {
368370
if m.Role == "user" || m.Role == "assistant" {
@@ -372,6 +374,12 @@ func (t *sessionSearchTool) handleGet(id string) (string, error) {
372374
})
373375
}
374376
}
377+
if len(sessionMessages) > maxSessionGetMessages {
378+
sessionMessages = sessionMessages[len(sessionMessages)-maxSessionGetMessages:]
379+
}
380+
for i := range sessionMessages {
381+
sessionMessages[i].Content = wrapUntrusted("session_search:"+sess.ID, sessionMessages[i].Content)
382+
}
375383
msgCount := len(sessionMessages)
376384
return jsonResult(sessionSearchResult{
377385
Action: "get",

cmd/odek/session_search_tool_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,7 @@ func TestSessionSearch_GetReturnsSessionMessages(t *testing.T) {
712712
if resp.SessionMessages[i].Role != c.role {
713713
t.Errorf("msg[%d] role = %q, want %q", i, resp.SessionMessages[i].Role, c.role)
714714
}
715-
if resp.SessionMessages[i].Content != c.content {
715+
if unwrapUntrusted(resp.SessionMessages[i].Content) != c.content {
716716
t.Errorf("msg[%d] content = %q, want %q", i, resp.SessionMessages[i].Content, c.content)
717717
}
718718
}

cmd/odek/subagent.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,13 @@ func subagentCmd(args []string) error {
224224
var taskTrust string // "trusted" or "untrusted" (from parent agent)
225225
var taskMaxRisk string
226226
if hasTaskFile {
227+
info, err := os.Stat(cfg.taskFile)
228+
if err != nil {
229+
return fmt.Errorf("stat task file: %w", err)
230+
}
231+
if info.Size() > maxFileReadBytes {
232+
return fmt.Errorf("task file too large (%d bytes, max %d)", info.Size(), maxFileReadBytes)
233+
}
227234
data, err := os.ReadFile(cfg.taskFile)
228235
if err != nil {
229236
return fmt.Errorf("read task file: %w", err)

cmd/odek/transcribe_tool.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,14 @@ func (t *transcribeTool) Call(argsJSON string) (result string, err error) {
287287
args2 = append(args2, "--language", lang)
288288
}
289289

290+
const maxWhisperOutputBytes = 10 << 20 // 10 MiB
290291
cmd := exec.CommandContext(t.toolCtx(), binary, args2...)
291292
output, err := cmd.Output()
293+
if err == nil && len(output) > maxWhisperOutputBytes {
294+
return jsonResult(transcribeResult{
295+
Error: fmt.Sprintf("whisper output too large (%d bytes, max %d)", len(output), maxWhisperOutputBytes),
296+
})
297+
}
292298
if err != nil {
293299
if exitErr, ok := err.(*exec.ExitError); ok {
294300
return jsonResult(transcribeResult{

0 commit comments

Comments
 (0)