Skip to content

Commit 19b6986

Browse files
jkyberneeesclaude
andauthored
Security hardening: 30 exploitable vulnerabilities (batches 1–7) (#38)
* fix: next 5 security vulnerabilities (browser history, search caps, file-size limits, serve CSRF, untrusted wrappers) * fix: next 5 security vulnerabilities (shell output, browser timeout, batch_patch, transcribe, tree width) * fix: next 5 security vulnerabilities (patch, glob, subagent task, transcribe output, session_search get) * fix: next 5 exploitable vulnerabilities (resource prompt injection, session_search wrapping, resource size, subagent summary, patch expansion) * fix: cap write_file content, harden file_info, bound WebSocket/session/skill sizes * fix: wrap tree paths, cap head_tail/AGENTS/IDENTITY, harden search_files symlinks * fix: next 5 exploitable vulnerabilities (base64 wrapping, browser title/element wrapping + cap, config size cap, resource resolver symlink Lstat) * fix: clarify head_tail per-file cap + add multi-file cap test vprotocol finding (axis 2.1/2.9): the maxHeadTailTotalBytes comment claimed a "total across all files" guarantee that the code enforces per-file. Correct the comment to state the cap is per-file and that the ~10 MiB aggregate bound comes from the separate 10-file-per-call limit. Add TestHeadTail_CapsOutputSizeMultiFile to lock both the per-file cap and the aggregate bound across the max 10 files. Additive only — no behavior change to security logic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: confine @-resource Search to root (CodeQL path-injection) CodeQL flagged uncontrolled user data (the @-reference query) reaching a path expression in FileResolver.Search. A query like "../../etc/passwd" makes filepath.Join(root, query) clean to a path outside root, so filepath.Glob matched files the workspace must not expose and os.Lstat leaked their metadata (size, etc.) — the same metadata-disclosure class this PR hardens elsewhere. Add a separator-aware withinRoot() confinement primitive and apply it to every Search match before touching the filesystem. Reuse it in Load, replacing the prior HasPrefix(absTarget, absRoot) check that also fixed a boundary bug ("/foo" matching "/foobar"). TestFileResolver_SearchOutsideRoot fails against the pre-fix code (leaks @../secret.txt) and passes with the guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1bc64ba commit 19b6986

30 files changed

Lines changed: 1898 additions & 96 deletions

AGENTS.md

Lines changed: 30 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`, `batch_patch`, `glob`, `file_info`, `tree`, `base64` file mode, `session_search`, `@-resources`, `--ctx` files, any MCP tool) wraps results in `<untrusted_content_<nonce> source="...">…</untrusted_content_<nonce>>`. Browser page title and interactive-element text are wrapped in addition to the main content. 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,35 @@ 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+
- **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.
106+
- **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.
107+
- **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.
108+
- **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.
109+
- **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.
110+
- **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.
111+
- **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.
112+
- **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.
113+
- **glob tool hardening** (`cmd/odek/file_tool.go`) — `glob` caps results at 1,000 matches and wraps returned paths as untrusted content.
114+
- **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.
115+
- **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.
116+
- **@-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.
117+
- **Config file size cap** (`internal/config/loader.go`) — `~/.odek/config.json` and `./odek.json` are rejected if larger than 5 MiB to prevent OOM from a malicious or broken config at startup.
118+
- **Resource resolver size cap** (`internal/resource/resource.go`) — `@-resource` file loads are capped at 1 MiB to prevent OOM from `@hugefile` references.
119+
- **Resource resolver symlink hardening** (`internal/resource/resource.go`) — `FileResolver.Search` uses `os.Lstat` (not `os.Stat`) for search-result metadata, so symlinks cannot leak the size of arbitrary targets outside the workspace.
120+
- **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.
121+
- **Tree path wrapping** (`cmd/odek/perf_tools.go`) — the `tree` tool wraps every filesystem-derived path as untrusted content.
122+
- **head_tail output cap** (`cmd/odek/perf_tools.go`) — `head_tail` truncates returned lines so total content stays within 1 MiB, preventing multi-file/multi-line memory DoS.
123+
- **search_files symlink hardening** (`cmd/odek/file_tool.go`) — the `files` target uses `Lstat` (not `Stat`) and skips symlinks in the glob branch, closing metadata disclosure via symlinked paths.
124+
- **AGENTS.md size cap** (`odek.go`) — project-level `AGENTS.md` is ignored if larger than 256 KiB to prevent OOM/prompt stuffing from a malicious repo.
125+
- **IDENTITY.md size cap** (`cmd/odek/main.go`) — `~/.odek/IDENTITY.md` is ignored if larger than 256 KiB, falling back to the default identity.
126+
- **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.
127+
- **write_file content cap** (`cmd/odek/file_tool.go`) — the `content` argument is capped at 1 MiB to prevent disk exhaustion and memory pressure from a single enormous tool call.
128+
- **file_info confinement + wrapping** (`cmd/odek/file_tool.go`) — `file_info` respects the same `restrictToCWD` path confinement as `write_file`/`patch`, and the returned path is wrapped as untrusted content.
129+
- **WebSocket message-size cap** (`cmd/odek/serve.go`) — `odek serve` sets `MaxPayloadBytes` on every WebSocket connection so a local client cannot OOM the server with a huge frame.
130+
- **Session file size cap** (`internal/session/session.go`) — session files larger than 32 MiB are rejected by `Load()` to prevent OOM from tampered or corrupted transcripts.
131+
- **Skill file size cap** (`internal/skills/loader.go`) — `SKILL.md` files larger than 1 MiB are skipped so a malicious project cannot OOM the process at startup or bloat the system prompt.
103132
- **Serve sandbox default-on**`odek serve` enables `--sandbox` automatically unless `--no-sandbox` is passed.
104133
- **Secret redaction** (`internal/redact/redact.go`) — 20+ patterns: OpenAI, Anthropic, GitHub PAT, AWS, PEM, JWT, Vault, Google OAuth, SendGrid, Discord, DB URLs, etc.
105134

cmd/odek/browser_tool.go

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"regexp"
1010
"strings"
1111
"sync"
12+
"time"
1213

1314
"github.com/BackendStack21/odek/internal/danger"
1415
)
@@ -44,6 +45,15 @@ type browserSnapshot struct {
4445
Elements []clickableRef `json:"elements,omitempty"`
4546
}
4647

48+
// maxBrowserHistory caps the number of snapshots retained in browser state to
49+
// prevent memory DoS from repeated navigate actions.
50+
const maxBrowserHistory = 50
51+
52+
// maxBrowserElements caps the number of interactive elements extracted from a
53+
// page to prevent a hostile page from OOMing the agent with thousands of links
54+
// or buttons.
55+
const maxBrowserElements = 500
56+
4757
// browserState holds the shared state for one browser session.
4858
type browserState struct {
4959
mu sync.Mutex
@@ -62,12 +72,17 @@ type browserTool struct {
6272
trustedClasses map[danger.RiskClass]bool
6373
}
6474

75+
// browserRequestTimeout bounds each browser HTTP request. Tests may lower it to
76+
// verify timeout behavior.
77+
var browserRequestTimeout = 30 * time.Second
78+
6579
func newBrowserTool(dc danger.DangerousConfig) *browserTool {
6680
t := &browserTool{
6781
state: &browserState{nextRef: 1},
6882
dangerousConfig: dc,
6983
}
7084
t.client = &http.Client{
85+
Timeout: browserRequestTimeout,
7186
CheckRedirect: t.checkRedirect,
7287
Transport: ssrfGuardedTransport(),
7388
}
@@ -167,6 +182,7 @@ func (t *browserTool) Call(argsJSON string) (string, error) {
167182
}
168183
if t.client == nil {
169184
t.client = &http.Client{
185+
Timeout: browserRequestTimeout,
170186
CheckRedirect: t.checkRedirect,
171187
Transport: ssrfGuardedTransport(),
172188
}
@@ -226,10 +242,15 @@ func (t *browserTool) doNavigate(rawURL string) (string, error) {
226242
html := string(body)
227243
snap := parseHTML(html, rawURL, resp.StatusCode)
228244

229-
// Store in state
245+
// Store in state. Keep a persistent copy of the snapshot for current; the
246+
// local variable's address would otherwise escape to the heap implicitly.
230247
t.state.mu.Lock()
231248
t.state.history = append(t.state.history, snap)
232-
t.state.current = &snap
249+
if len(t.state.history) > maxBrowserHistory {
250+
t.state.history = t.state.history[len(t.state.history)-maxBrowserHistory:]
251+
}
252+
snapCopy := snap
253+
t.state.current = &snapCopy
233254
t.state.nextRef = len(snap.Elements) + 1
234255
t.state.mu.Unlock()
235256

@@ -363,6 +384,9 @@ func parseHTML(html, pageURL string, status int) browserSnapshot {
363384

364385
// Extract links
365386
for _, m := range reLink.FindAllStringSubmatch(html, -1) {
387+
if len(elements) >= maxBrowserElements {
388+
break
389+
}
366390
href := strings.TrimSpace(m[1])
367391
text := strings.TrimSpace(m[2])
368392
if href == "" || text == "" || href == "#" || strings.HasPrefix(href, "javascript:") {
@@ -388,6 +412,9 @@ func parseHTML(html, pageURL string, status int) browserSnapshot {
388412

389413
// Extract buttons and inputs
390414
for _, m := range reButton.FindAllStringSubmatch(html, -1) {
415+
if len(elements) >= maxBrowserElements {
416+
break
417+
}
391418
text := strings.TrimSpace(m[1])
392419
if text == "" {
393420
text = "button"
@@ -403,6 +430,9 @@ func parseHTML(html, pageURL string, status int) browserSnapshot {
403430
}
404431

405432
for _, m := range reInput.FindAllStringSubmatch(html, -1) {
433+
if len(elements) >= maxBrowserElements {
434+
break
435+
}
406436
tag := m[0]
407437
text := ""
408438
if vm := reInputVal.FindStringSubmatch(tag); len(vm) > 1 {
@@ -426,6 +456,12 @@ func parseHTML(html, pageURL string, status int) browserSnapshot {
426456
snap.Content = strings.Join(contentParts, "\n")
427457
snap.Elements = elements
428458

459+
// Title and element text come from the page — wrap them as untrusted content.
460+
snap.Title = wrapUntrusted(pageURL, snap.Title)
461+
for i := range snap.Elements {
462+
snap.Elements[i].Text = wrapUntrusted(pageURL, snap.Elements[i].Text)
463+
}
464+
429465
return snap
430466
}
431467

cmd/odek/browser_tool_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ func TestBrowser_Navigate(t *testing.T) {
3434
if r.Error != "" {
3535
t.Fatalf("navigate error: %s", r.Error)
3636
}
37-
if r.Title != "Test Page" {
38-
t.Errorf("title = %q, want %q", r.Title, "Test Page")
37+
if unwrapUntrusted(r.Title) != "Test Page" {
38+
t.Errorf("title = %q, want %q", unwrapUntrusted(r.Title), "Test Page")
3939
}
4040
if !strings.Contains(r.Content, "Hello World") {
4141
t.Errorf("content missing 'Hello World': %q", r.Content)

0 commit comments

Comments
 (0)