Skip to content

Commit 9a1eaa6

Browse files
committed
fix: next 5 security vulnerabilities (browser history, search caps, file-size limits, serve CSRF, untrusted wrappers)
1 parent 1bc64ba commit 9a1eaa6

10 files changed

Lines changed: 498 additions & 34 deletions

AGENTS.md

Lines changed: 5 additions & 1 deletion
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`, 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`, 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.
@@ -100,6 +100,10 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
100100
- **Approver friction** (`internal/danger/approver.go`, `cmd/odek/wsapprover.go`) — both TTYApprover and WSApprover engage friction mode after 3 approvals of the same class in 60s: require typing literal `approve`, 1.5s pause. Trust-class shortcut disabled for `destructive` + `blocked` regardless.
101101
- **Danger classifier bypass resistance** (`internal/danger/classifier.go`) — `normalize()` pre-processes: expand `$IFS` / `${IFS}`, extract `$(...)` / `` `...` `` substitutions, strip `command` / `exec` / `builtin` wrappers, collapse unquoted backslashes, basename absolute paths. Regression suite in `classifier_bypass_test.go`.
102102
- **WS Origin allowlist** (`cmd/odek/serve.go::checkLocalOrigin`) — rejects non-localhost upgrades. Closes CSRF-on-localhost.
103+
- **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.
104+
- **Browser history cap** (`cmd/odek/browser_tool.go`) — navigation history is capped at 50 snapshots to prevent memory DoS from repeated `browser_navigate` calls.
105+
- **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.
106+
- **Perf-tool file-size cap** (`cmd/odek/perf_tools.go`) — `diff`, `base64`, `tr`, `sort`, and `json_query` reject files larger than 10 MiB to avoid loading multi-gigabyte files into memory.
103107
- **Serve sandbox default-on**`odek serve` enables `--sandbox` automatically unless `--no-sandbox` is passed.
104108
- **Secret redaction** (`internal/redact/redact.go`) — 20+ patterns: OpenAI, Anthropic, GitHub PAT, AWS, PEM, JWT, Vault, Google OAuth, SendGrid, Discord, DB URLs, etc.
105109

cmd/odek/browser_tool.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ type browserSnapshot struct {
4444
Elements []clickableRef `json:"elements,omitempty"`
4545
}
4646

47+
// maxBrowserHistory caps the number of snapshots retained in browser state to
48+
// prevent memory DoS from repeated navigate actions.
49+
const maxBrowserHistory = 50
50+
4751
// browserState holds the shared state for one browser session.
4852
type browserState struct {
4953
mu sync.Mutex
@@ -226,10 +230,15 @@ func (t *browserTool) doNavigate(rawURL string) (string, error) {
226230
html := string(body)
227231
snap := parseHTML(html, rawURL, resp.StatusCode)
228232

229-
// Store in state
233+
// Store in state. Keep a persistent copy of the snapshot for current; the
234+
// local variable's address would otherwise escape to the heap implicitly.
230235
t.state.mu.Lock()
231236
t.state.history = append(t.state.history, snap)
232-
t.state.current = &snap
237+
if len(t.state.history) > maxBrowserHistory {
238+
t.state.history = t.state.history[len(t.state.history)-maxBrowserHistory:]
239+
}
240+
snapCopy := snap
241+
t.state.current = &snapCopy
233242
t.state.nextRef = len(snap.Elements) + 1
234243
t.state.mu.Unlock()
235244

cmd/odek/file_tool.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@ const maxLines = 2000
2525
// memory exhaustion from huge files.
2626
const maxReadBytes = 1 << 20 // 1 MiB
2727

28+
// maxSearchLimit caps the number of matches returned by search_files to
29+
// prevent unbounded result JSON from exhausting memory.
30+
const maxSearchLimit = 500
31+
32+
// maxSearchResultBytes caps the total returned content bytes for a single
33+
// search_files / multi_grep content query.
34+
const maxSearchResultBytes = maxReadBytes
35+
2836
type readFileTool struct {
2937
dangerousConfig danger.DangerousConfig
3038
}
@@ -371,6 +379,9 @@ func (t *searchFilesTool) Call(argsJSON string) (string, error) {
371379
if args.Limit <= 0 {
372380
args.Limit = maxMatches
373381
}
382+
if args.Limit > maxSearchLimit {
383+
args.Limit = maxSearchLimit
384+
}
374385

375386
// Security: check search path
376387
risk := danger.ClassifyPath(args.Path)
@@ -398,6 +409,7 @@ func (t *searchFilesTool) searchContent(args searchFilesArgs) (string, error) {
398409

399410
var matches []searchMatch
400411
limit := args.Limit
412+
resultBytes := 0
401413

402414
err = filepath.Walk(args.Path, func(path string, info os.FileInfo, err error) error {
403415
if err != nil {
@@ -455,10 +467,16 @@ func (t *searchFilesTool) searchContent(args searchFilesArgs) (string, error) {
455467
lineNum++
456468
line := scanner.Text()
457469
if re.MatchString(line) {
470+
trimmed := strings.TrimSpace(line)
471+
if resultBytes+len(trimmed) > maxSearchResultBytes {
472+
limit = len(matches)
473+
break
474+
}
475+
resultBytes += len(trimmed)
458476
matches = append(matches, searchMatch{
459477
Path: path,
460478
Line: lineNum,
461-
Content: wrapUntrusted(fmt.Sprintf("%s:%d", path, lineNum), strings.TrimSpace(line)),
479+
Content: wrapUntrusted(fmt.Sprintf("%s:%d", path, lineNum), trimmed),
462480
})
463481
if len(matches) >= limit {
464482
break

0 commit comments

Comments
 (0)