Skip to content

Commit 09e4372

Browse files
committed
fix: next 5 exploitable vulnerabilities (resource prompt injection, session_search wrapping, resource size, subagent summary, patch expansion)
1 parent 73582b1 commit 09e4372

10 files changed

Lines changed: 213 additions & 19 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`, `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.
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`, `@-resources`, `--ctx` files, 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.
@@ -111,7 +111,11 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
111111
- **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.
112112
- **glob tool hardening** (`cmd/odek/file_tool.go`) — `glob` caps results at 1,000 matches and wraps returned paths as untrusted content.
113113
- **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.
114+
- **session_search hardening** (`cmd/odek/session_search_tool.go`) — the `get` action returns at most the 100 most recent messages and wraps each message content, task, and buffer entry as untrusted; `list`/`search`/`find` also wrap session tasks.
115+
- **@-resource / --ctx prompt wrapping** (`cmd/odek/refs.go`, `cmd/odek/serve.go`) — content resolved from `@file` references and `--ctx` files is wrapped as untrusted before being inserted into the prompt.
116+
- **Resource resolver size cap** (`internal/resource/resource.go`) — `@-resource` file loads are capped at 1 MiB to prevent OOM from `@hugefile` references.
117+
- **Sub-agent summary cap** (`cmd/odek/subagent_tool.go`) — each sub-agent result included in the `delegate_tasks` summary is truncated to 100 KiB to prevent memory DoS.
118+
- **patch / batch_patch output expansion cap** (`cmd/odek/file_tool.go`, `cmd/odek/perf_tools.go`) — the post-replacement result is capped at 10 MiB so `ReplaceAll` cannot explode memory.
115119
- **Serve sandbox default-on**`odek serve` enables `--sandbox` automatically unless `--no-sandbox` is passed.
116120
- **Secret redaction** (`internal/redact/redact.go`) — 20+ patterns: OpenAI, Anthropic, GitHub PAT, AWS, PEM, JWT, Vault, Google OAuth, SendGrid, Discord, DB URLs, etc.
117121

cmd/odek/file_tool.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -689,6 +689,9 @@ func (t *patchTool) Call(argsJSON string) (string, error) {
689689
} else {
690690
modified = strings.Replace(original, args.OldString, args.NewString, 1)
691691
}
692+
if len(modified) > maxFileReadBytes {
693+
return jsonError(fmt.Sprintf("patch result too large (%d bytes, max %d)", len(modified), maxFileReadBytes))
694+
}
692695

693696
// Generate a simple diff
694697
diff := fmt.Sprintf("--- a/%s\n+++ b/%s\n@@ -1 +1 @@\n-%s\n+%s\n",

cmd/odek/next_security_vulnerabilities_test.go

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/BackendStack21/odek/internal/config"
1515
"github.com/BackendStack21/odek/internal/danger"
1616
"github.com/BackendStack21/odek/internal/llm"
17+
"github.com/BackendStack21/odek/internal/resource"
1718
"github.com/BackendStack21/odek/internal/session"
1819
)
1920

@@ -648,3 +649,155 @@ func TestSessionSearchGet_CapsAndWrapsMessages(t *testing.T) {
648649
t.Fatalf("session message should be wrapped in untrusted_content, got: %q", r.SessionMessages[0].Content)
649650
}
650651
}
652+
653+
654+
// ── 16. enrichTask must wrap @-resource / --ctx content ──────────────────
655+
656+
func TestEnrichTask_WrapsCtxContent(t *testing.T) {
657+
dir := t.TempDir()
658+
os.WriteFile(filepath.Join(dir, "note.txt"), []byte("hello world"), 0644)
659+
660+
enriched, err := enrichTask("check @note.txt", nil, dir)
661+
if err != nil {
662+
t.Fatalf("enrichTask error: %v", err)
663+
}
664+
if !strings.Contains(enriched, "<untrusted_content_") {
665+
t.Fatalf("enriched prompt should wrap file content in untrusted_content, got: %s", enriched)
666+
}
667+
}
668+
669+
func TestEnrichTask_WrapsCtxFiles(t *testing.T) {
670+
dir := t.TempDir()
671+
os.WriteFile(filepath.Join(dir, "data.txt"), []byte("sensitive data"), 0644)
672+
673+
enriched, err := enrichTask("analyze", []string{"data.txt"}, dir)
674+
if err != nil {
675+
t.Fatalf("enrichTask error: %v", err)
676+
}
677+
if !strings.Contains(enriched, "<untrusted_content_") {
678+
t.Fatalf("--ctx content should be wrapped in untrusted_content, got: %s", enriched)
679+
}
680+
}
681+
682+
// ── 17. session_search list/search/find must wrap Task/Buffer ────────────
683+
684+
func TestSessionSearch_ListWrapsTask(t *testing.T) {
685+
dir := t.TempDir()
686+
t.Setenv("HOME", dir)
687+
store, err := session.NewStore()
688+
if err != nil {
689+
t.Fatalf("NewStore: %v", err)
690+
}
691+
sess := &session.Session{
692+
ID: "list-test",
693+
Task: "user task about go-vector",
694+
Model: "test",
695+
CreatedAt: time.Now(),
696+
UpdatedAt: time.Now(),
697+
}
698+
if err := store.Save(sess); err != nil {
699+
t.Fatalf("Save: %v", err)
700+
}
701+
702+
tool := newSessionSearchTool(store)
703+
result := callJSON(t, tool, `{"action":"list"}`)
704+
var r struct {
705+
Sessions []struct {
706+
Task string `json:"task"`
707+
} `json:"sessions"`
708+
}
709+
mustUnmarshal(t, result, &r)
710+
if len(r.Sessions) == 0 || !strings.HasPrefix(r.Sessions[0].Task, "<untrusted_content_") {
711+
t.Fatalf("session list should wrap task in untrusted_content, got: %s", result)
712+
}
713+
}
714+
715+
// ── 18. Resource resolver must reject huge files ─────────────────────────
716+
717+
func TestResourceResolver_RejectsHugeFile(t *testing.T) {
718+
dir := t.TempDir()
719+
path := filepath.Join(dir, "huge.txt")
720+
os.WriteFile(path, make([]byte, 15*1024*1024), 0644)
721+
722+
res := resource.NewFileResolver(dir)
723+
_, err := res.Load(context.Background(), "huge.txt")
724+
if err == nil {
725+
t.Fatal("resource resolver should reject a 15 MiB file")
726+
}
727+
if !strings.Contains(err.Error(), "too large") {
728+
t.Fatalf("expected size error, got: %v", err)
729+
}
730+
}
731+
732+
// ── 19. delegate_tasks must cap summary size ─────────────────────────────
733+
734+
func TestDelegateTasks_CapsSummarySize(t *testing.T) {
735+
if os.Getenv("ODEK_E2E") == "" {
736+
t.Skip("sub-agent spawning test; set ODEK_E2E=true to run")
737+
}
738+
fakeOdek := filepath.Join(t.TempDir(), "fake-odek")
739+
// Print a valid JSON result whose summary is ~6 MB.
740+
script := `#!/bin/sh
741+
printf '{"status":"success","summary":"%s","files_changed":[],"iterations":1,"tokens_used":10}\n' "$(head -c 6000000 /dev/zero | tr '\0' 'x')"
742+
`
743+
os.WriteFile(fakeOdek, []byte(script), 0755)
744+
745+
tool := &delegateTasksTool{
746+
odekPath: fakeOdek,
747+
maxConcurrency: 1,
748+
timeout: 30 * time.Second,
749+
}
750+
tool.SetContext(context.Background())
751+
result, err := tool.Call(`{"tasks":[{"goal":"a"},{"goal":"b"}],"description":"summary cap test"}`)
752+
if err != nil {
753+
t.Fatalf("Call() error: %v", err)
754+
}
755+
if len(result) > 1024*1024+500 {
756+
t.Fatalf("delegate_tasks summary returned %d bytes, expected cap near 1 MiB", len(result))
757+
}
758+
}
759+
760+
// ── 20. patch / batch_patch must cap ReplaceAll expansion ────────────────
761+
762+
func TestPatch_RejectsOutputExpansion(t *testing.T) {
763+
dir := t.TempDir()
764+
path := filepath.Join(dir, "data.txt")
765+
// 2,000 'a' chars. Replacing each with 10,000 'x' => ~20M chars.
766+
os.WriteFile(path, []byte(strings.Repeat("a", 2000)), 0644)
767+
768+
tool := &patchTool{}
769+
result := callJSON(t, tool, fmt.Sprintf(`{"path":%q,"old_string":"a","new_string":%q,"replace_all":true}`, path, strings.Repeat("x", 10000)))
770+
var r struct {
771+
Success bool `json:"success"`
772+
Error string `json:"error,omitempty"`
773+
}
774+
mustUnmarshal(t, result, &r)
775+
if r.Success {
776+
t.Fatal("patch should reject a ReplaceAll that explodes output size")
777+
}
778+
if !strings.Contains(r.Error, "too large") {
779+
t.Fatalf("expected size error, got: %q", r.Error)
780+
}
781+
}
782+
783+
func TestBatchPatch_RejectsOutputExpansion(t *testing.T) {
784+
dir := t.TempDir()
785+
path := filepath.Join(dir, "data.txt")
786+
os.WriteFile(path, []byte(strings.Repeat("a", 2000)), 0644)
787+
788+
tool := &batchPatchTool{}
789+
result := callJSON(t, tool, fmt.Sprintf(`{"patches":[{"path":%q,"old_string":"a","new_string":%q,"replace_all":true}]}`, path, strings.Repeat("x", 10000)))
790+
var r struct {
791+
Results []struct {
792+
Success bool `json:"success"`
793+
Error string `json:"error,omitempty"`
794+
} `json:"results"`
795+
}
796+
mustUnmarshal(t, result, &r)
797+
if len(r.Results) != 1 || r.Results[0].Success {
798+
t.Fatal("batch_patch should reject a ReplaceAll that explodes output size")
799+
}
800+
if !strings.Contains(r.Results[0].Error, "too large") {
801+
t.Fatalf("expected size error, got: %q", r.Results[0].Error)
802+
}
803+
}

cmd/odek/perf_tools.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,11 @@ func (t *batchPatchTool) Call(argsJSON string) (result string, err error) {
214214
} else {
215215
modified = strings.Replace(original, p.OldString, p.NewString, 1)
216216
}
217+
if len(modified) > maxFileReadBytes {
218+
entry.Error = fmt.Sprintf("patch result too large (%d bytes, max %d)", len(modified), maxFileReadBytes)
219+
results[idx] = entry
220+
continue
221+
}
217222

218223
diff := fmt.Sprintf("--- a/%s\n+++ b/%s\n@@ -1 +1 @@\n-%s\n+%s\n",
219224
p.Path, p.Path, truncateDiff(original, 100), truncateDiff(modified, 100))

cmd/odek/refs.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func enrichTask(task string, ctxFiles []string, cwd string) (string, error) {
3939
// Leave unresolved refs as-is
4040
continue
4141
}
42-
resolved[ref.Raw] = content
42+
resolved[ref.Raw] = wrapUntrusted("resource:"+ref.Raw, content)
4343
}
4444
enriched = resource.ReplaceRefs(task, resolved)
4545
}
@@ -56,7 +56,7 @@ func enrichTask(task string, ctxFiles []string, cwd string) (string, error) {
5656
if err != nil {
5757
return "", fmt.Errorf("ctx file %q: %w", f, err)
5858
}
59-
blocks = append(blocks, fmt.Sprintf("--- %s ---\n%s\n--- end %s ---", f, content, f))
59+
blocks = append(blocks, fmt.Sprintf("--- %s ---\n%s\n--- end %s ---", f, wrapUntrusted("ctx:"+f, content), f))
6060
}
6161
if len(blocks) > 0 {
6262
// Log attached files to stderr

cmd/odek/serve.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -682,7 +682,7 @@ func handlePrompt(
682682
if err != nil {
683683
continue
684684
}
685-
resolvedRefs[ref.Raw] = content
685+
resolvedRefs[ref.Raw] = wrapUntrusted("resource:"+ref.Raw, content)
686686
}
687687
enrichedPrompt := resource.ReplaceRefs(prompt, resolvedRefs)
688688

cmd/odek/session_search_tool.go

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ func (t *sessionSearchTool) handleList(limit int) (string, error) {
142142
Count: 0,
143143
})
144144
}
145-
results := toSummaries(sessions)
145+
results := toSummaries("session_search:list", sessions)
146146
return jsonResult(sessionSearchResult{
147147
Action: "list",
148148
Sessions: results,
@@ -191,7 +191,7 @@ func (t *sessionSearchTool) handleSearch(query string, limit int) (string, error
191191
scoreLabel := fmt.Sprintf("(score: %.3f)", vr.Score)
192192
results = append(results, sessionSummary{
193193
ID: sess.ID,
194-
Task: sess.Task + " " + scoreLabel,
194+
Task: wrapUntrusted("session_search:search", sess.Task+" "+scoreLabel),
195195
Turns: sess.Turns,
196196
CreatedAt: sess.CreatedAt.UTC().Format(time.RFC3339),
197197
UpdatedAt: sess.UpdatedAt.UTC().Format(time.RFC3339),
@@ -248,17 +248,18 @@ func (t *sessionSearchTool) handleSearch(query string, limit int) (string, error
248248

249249
results := make([]sessionSummary, len(matches))
250250
for i, m := range matches {
251+
task := m.session.Task
252+
if m.snippet != "" && m.snippet != m.session.Task {
253+
task = m.session.Task + " — " + m.snippet
254+
}
251255
results[i] = sessionSummary{
252256
ID: m.session.ID,
253-
Task: m.session.Task,
257+
Task: wrapUntrusted("session_search:search", task),
254258
Turns: m.session.Turns,
255259
CreatedAt: m.session.CreatedAt.UTC().Format(time.RFC3339),
256260
UpdatedAt: m.session.UpdatedAt.UTC().Format(time.RFC3339),
257261
Model: m.session.Model,
258262
}
259-
if m.snippet != "" && m.snippet != m.session.Task {
260-
results[i].Task = m.session.Task + " — " + m.snippet
261-
}
262263
}
263264

264265
return jsonResult(sessionSearchResult{
@@ -381,15 +382,19 @@ func (t *sessionSearchTool) handleGet(id string) (string, error) {
381382
sessionMessages[i].Content = wrapUntrusted("session_search:"+sess.ID, sessionMessages[i].Content)
382383
}
383384
msgCount := len(sessionMessages)
385+
wrappedBuffer := make([]string, len(sess.Buffer))
386+
for i, b := range sess.Buffer {
387+
wrappedBuffer[i] = wrapUntrusted("session_search:get:buffer", b)
388+
}
384389
return jsonResult(sessionSearchResult{
385390
Action: "get",
386391
ID: sess.ID,
387-
Task: sess.Task,
392+
Task: wrapUntrusted("session_search:get", sess.Task),
388393
Turns: sess.Turns,
389394
CreatedAt: sess.CreatedAt.UTC().Format(time.RFC3339),
390395
UpdatedAt: sess.UpdatedAt.UTC().Format(time.RFC3339),
391396
Model: sess.Model,
392-
Buffer: sess.Buffer,
397+
Buffer: wrappedBuffer,
393398
Messages: msgCount,
394399
SessionMessages: sessionMessages,
395400
})
@@ -426,7 +431,7 @@ func (t *sessionSearchTool) handleFind(query string, limit int) (string, error)
426431

427432
return jsonResult(sessionSearchResult{
428433
Action: "find",
429-
Sessions: toSummaries(matched),
434+
Sessions: toSummaries("session_search:find", matched),
430435
Count: len(matched),
431436
})
432437
}
@@ -449,12 +454,12 @@ func matchTokens(tokens []string, text string) int {
449454
}
450455

451456
// toSummaries converts session.Session slices to sessionSummary (metadata only).
452-
func toSummaries(sessions []session.Session) []sessionSummary {
457+
func toSummaries(source string, sessions []session.Session) []sessionSummary {
453458
results := make([]sessionSummary, len(sessions))
454459
for i, s := range sessions {
455460
results[i] = sessionSummary{
456461
ID: s.ID,
457-
Task: s.Task,
462+
Task: wrapUntrusted(source, s.Task),
458463
Turns: s.Turns,
459464
CreatedAt: s.CreatedAt.UTC().Format(time.RFC3339),
460465
UpdatedAt: s.UpdatedAt.UTC().Format(time.RFC3339),

cmd/odek/session_search_tool_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ func TestSessionSearch_Get(t *testing.T) {
186186
if r.ID != "20260520-auth-fix" {
187187
t.Errorf("id = %q, want '20260520-auth-fix'", r.ID)
188188
}
189-
if r.Task != "fix O_NOFOLLOW in file_tool.go" {
189+
if unwrapUntrusted(r.Task) != "fix O_NOFOLLOW in file_tool.go" {
190190
t.Errorf("task = %q", r.Task)
191191
}
192192
if r.Turns != 8 {

cmd/odek/subagent_tool.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,12 +157,18 @@ func (t *delegateTasksTool) Call(args string) (string, error) {
157157
sem <- struct{}{}
158158
}
159159

160-
// Build summary for the calling agent
160+
// Build summary for the calling agent. Cap each sub-agent result so the
161+
// summary cannot grow without bound.
161162
var buf strings.Builder
162163
buf.WriteString("📋 Sub-agent results:\n\n")
163164
for i, r := range results {
164165
buf.WriteString(fmt.Sprintf("─── Task %d: %s ───\n", i+1, truncate(input.Tasks[i].Goal, 60)))
165-
buf.WriteString(r)
166+
if len(r) > maxSubagentSummaryResultBytes {
167+
buf.WriteString(r[:maxSubagentSummaryResultBytes])
168+
buf.WriteString("\n... [result truncated]")
169+
} else {
170+
buf.WriteString(r)
171+
}
166172
buf.WriteString("\n\n")
167173
}
168174
return buf.String(), nil
@@ -284,6 +290,11 @@ func (t *delegateTasksTool) runTask(taskIdx int, goal, taskContext, guidance, tr
284290
return `{"error":"no result from sub-agent"}`
285291
}
286292

293+
// maxSubagentSummaryResultBytes caps how much of each sub-agent result is
294+
// included in the parent delegate_tasks summary, preventing memory DoS from
295+
// huge sub-agent outputs.
296+
const maxSubagentSummaryResultBytes = 100 << 10 // 100 KiB
297+
287298
// maxSubagentLine caps a single NDJSON line read from a sub-agent's stdout.
288299
// Streamed tool_call events embed full tool arguments (e.g. a large write_file
289300
// or patch), which routinely exceed bufio.Scanner's default 64KB token cap.

internal/resource/resource.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ import (
2121
"github.com/BackendStack21/odek/internal/session"
2222
)
2323

24+
// maxResourceFileBytes caps how much of a file the @-resource resolver will
25+
// read into memory. It still truncates the returned content to 50 KB, but this
26+
// guard prevents OOM from files larger than 1 MiB.
27+
const maxResourceFileBytes = 1 << 20 // 1 MiB
28+
2429
// Resource is a discovered resource returned by a Resolver.
2530
type Resource struct {
2631
ID string `json:"id"` // Full @ reference (e.g. "@src/main.go")
@@ -268,6 +273,14 @@ func (f *FileResolver) Load(ctx context.Context, id string) (string, error) {
268273
}
269274
defer fd.Close()
270275

276+
info, err := fd.Stat()
277+
if err != nil {
278+
return "", err
279+
}
280+
if info.Size() > maxResourceFileBytes {
281+
return "", fmt.Errorf("resource: file too large (%d bytes, max %d)", info.Size(), maxResourceFileBytes)
282+
}
283+
271284
data, err := io.ReadAll(fd)
272285
if err != nil {
273286
return "", err

0 commit comments

Comments
 (0)