Skip to content

Commit 2013bd7

Browse files
committed
fix: validate session/episode vector index IDs and reject symlinks
Finding #14 — Session vector index rebuild: - Validate each session filename-derived ID with ValidateSessionID before embedding it. - Skip entries whose DirEntry.Type() is a symlink and double-check with os.Lstat. - Add internal/session/vector_index_test.go regression tests. Finding #15 — Episode vector index session_id trust: - Treat index.json as untrusted input. - Call session.ValidateSessionID(m.SessionID) before building the .md path. - Add defense-in-depth checks for .. and path separators. - Log warnings for rejected entries. - Add regression tests in internal/memory/episode_index_test.go. Update docs/SECURITY.md and AGENTS.md for both defenses.
1 parent c8e97af commit 2013bd7

6 files changed

Lines changed: 323 additions & 3 deletions

File tree

AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
113113
- **glob tool hardening** (`cmd/odek/file_tool.go`) — `glob` caps results at 1,000 matches and wraps returned paths as untrusted content.
114114
- **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.
115115
- **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+
- **Session vector index hardening** (`internal/session/vector_index.go`) — `rebuildLocked` validates every session filename with `ValidateSessionID` and skips symlinks via `DirEntry.Type()` and `os.Lstat`, preventing a planted symlink from embedding arbitrary files into the semantic search corpus.
116117
- **@-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.
117118
- **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.
118119
- **Resource resolver size cap** (`internal/resource/resource.go`) — `@-resource` file loads are capped at 1 MiB to prevent OOM from `@hugefile` references.
@@ -141,6 +142,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
141142
- **Telegram outbound media path allowlist** (`internal/telegram/media_path.go` + `internal/telegram/handler.go` + `internal/tool/send_message.go` + `cmd/odek/telegram.go`) — paths supplied to `MEDIA:...` prefixes or `send_message(file=...)` are resolved to an absolute path and verified against an allowlist (cwd, `~/.odek/media/`, system temp dir). `os.Lstat` rejects symlink final components and `filepath.EvalSymlinks` ensures the resolved path does not escape the allowlist, preventing prompt-injection-driven exfiltration of arbitrary files.
142143
- **Session ID entropy + session-scoped auth tokens** (`internal/session/session.go`, `cmd/odek/serve.go`) — session IDs now carry 128 bits of randomness (16 bytes / 32 hex chars); each session stores a 256-bit `AuthToken` required by `GET/DELETE/POST /api/sessions/<id>` and WebSocket session-resume messages via `X-Session-Token` header, `session_token` cookie, or `auth_token` WS field. Per-IP rate limiting (60/min) on session lookups adds a brute-force backstop.
143144
- **Skill/episode untrusted wrapper** (`internal/loop/loop.go` + `odek.go`) — skill context and retrieved session-episode context are passed through the caller-provided untrusted wrapper (the same nonce'd `<untrusted_content_*>` boundary used for tool output) before being injected into the model's system context. This prevents a compromised or tainted skill/episode from being treated as trusted system instructions.
145+
- **Episode index session ID validation** (`internal/memory/episode_index.go` + `internal/session/session.go`) — `readAllSummaries` treats `index.json` as untrusted input and validates every `session_id` with `session.ValidateSessionID` before building the `filepath.Join(dir, sessionID+".md")` path. Invalid / traversal / separator-containing IDs are skipped with a warning, preventing a tampered episode index from pulling arbitrary files (e.g. `~/.odek/config.json`, `IDENTITY.md`) into the embedding space.
144146
- **Secret redaction** (`internal/redact/redact.go`) — 20+ patterns: OpenAI, Anthropic, GitHub PAT, AWS, PEM, JWT, Vault, Google OAuth, SendGrid, Discord, DB URLs, etc.
145147

146148
### Platform Support

docs/SECURITY.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,19 @@ Legacy sessions created before this defense have no `AuthToken`; the first acces
350350

351351
Skill content and retrieved session episodes are externally-sourced data that cross the trust boundary. Before injecting them as `system` messages, the loop passes them through the same nonce'd `<untrusted_content_*>` wrapper used for tool output. The skill manager already gates `NeedsReview`/tainted skills, and the memory manager filters tainted episodes from search, but the wrapper provides defense-in-depth so a compromised skill or episode cannot pose as trusted system instructions.
352352

353+
### 26. Session vector index rebuild hardening
354+
355+
`internal/session/vector_index.go::rebuildLocked` scans the session directory to build the semantic search corpus. Before a file is read it must pass two checks:
356+
357+
1. **Session-ID validation** — the filename is stripped of its `.json` suffix and passed through `ValidateSessionID`. Names that are empty, contain path separators, or contain `..` are skipped.
358+
2. **Symlink rejection** — the `os.DirEntry.Type()` is checked for `ModeSymlink`, and the full path is then `os.Lstat`ed to skip symlinks even on platforms/filesystems where `Type()` does not report the link.
359+
360+
This closes the path where an attacker plants a symlink named like a session file (e.g. `20260518-abc….json`) that points to a sensitive file outside the sessions directory, which would otherwise have its content embedded into the session search corpus.
361+
362+
### 27. Episode index session ID validation
363+
364+
The episode vector index is rebuilt from `index.json` plus one `.md` summary file per entry. Because `index.json` is persisted JSON that can be tampered with on disk, `internal/memory/episode_index.go::readAllSummaries` treats every `session_id` as untrusted input. It calls `session.ValidateSessionID` before constructing the path `filepath.Join(dir, sessionID+".md")` and skips (with a stderr warning) any entry that is empty, contains path separators, contains `..`, or is otherwise malformed. This prevents a tampered entry such as `"../../../.odek/config"` from causing the rebuild to read arbitrary files (e.g. `~/.odek/config.json` or `IDENTITY.md`) and include them in the embedding space.
365+
353366
### YOLO mode
354367

355368
```json
@@ -404,6 +417,7 @@ Defaults: `FrictionThreshold=3`, `FrictionWindow=60s`. To opt out (TTYApprover o
404417
| Memory replays a previously-injected episode forever | Tainted episodes filtered from `Search` |
405418
| User reflex-approves a destructive class after many benign ones | Friction mode requires typed `approve` + 1.5 s pause |
406419
| Successful injection steers agent to attacker URL | `odek audit` flags `suspicious_divergence` on the turn |
420+
| Symlink planted as session file exfiltrates arbitrary file into semantic search | `rebuildLocked` validates IDs and skips symlinks via `Lstat` |
407421

408422
---
409423

internal/memory/episode_index.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@ package memory
22

33
import (
44
"encoding/json"
5+
"fmt"
56
"os"
67
"path/filepath"
78
"strings"
89
"sync"
910
"time"
1011

1112
"github.com/BackendStack21/go-vector/pkg/vector"
13+
"github.com/BackendStack21/odek/internal/session"
1214
)
1315

1416
const (
@@ -323,6 +325,21 @@ func (vi *episodeVectorIndex) readAllSummaries() []idText {
323325
}
324326
out := make([]idText, 0, len(index))
325327
for _, m := range index {
328+
// index.json is untrusted input; validate the session ID before using it
329+
// to construct a filesystem path. Reject traversal, separators, and any
330+
// other malformed ID silently so a tampered index cannot pull arbitrary
331+
// files (e.g. ~/.odek/config.json) into the embedding space.
332+
if err := session.ValidateSessionID(m.SessionID); err != nil {
333+
fmt.Fprintf(os.Stderr, "odek: warning: episode index rejected invalid session_id %q: %v\n", m.SessionID, err)
334+
continue
335+
}
336+
// Defense-in-depth: ValidateSessionID already rejects these, but the
337+
// consequences of a missed check are a path traversal read, so verify
338+
// explicitly before joining.
339+
if strings.Contains(m.SessionID, "..") || strings.ContainsAny(m.SessionID, "/\\") {
340+
fmt.Fprintf(os.Stderr, "odek: warning: episode index rejected unsafe session_id %q\n", m.SessionID)
341+
continue
342+
}
326343
path := filepath.Join(vi.dir, m.SessionID+".md")
327344
b, err := os.ReadFile(path)
328345
if err != nil {

internal/memory/episode_index_test.go

Lines changed: 105 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,14 @@ package memory
22

33
import (
44
"context"
5+
"encoding/json"
56
"fmt"
7+
"os"
68
"path/filepath"
79
"sync"
810
"sync/atomic"
911
"testing"
12+
"time"
1013
)
1114

1215
// resetEpIdxes clears the process-wide singleton map so each test gets a fresh
@@ -402,5 +405,106 @@ func TestSearchEpisodes_OOVFallbackToLLM(t *testing.T) {
402405
// After D-05 fix: OOV → recallByVector returns nil → SearchEpisodes falls back to episodes.Search
403406
// which uses the LLM ranker. We don't assert an exact count because the fallback
404407
// path (episodes.Search) may or may not call LLM depending on whether the index
405-
// is also empty from LLM ranker's perspective, but we confirm no panic.
408+
// is also empty from the LLM ranker's perspective, but we confirm no panic.
409+
}
410+
411+
// ── Episode index untrusted-session-id validation (Finding #15) ───────────────
412+
413+
// TestReadAllSummaries_ValidSessionID confirms that a well-formed session ID is
414+
// accepted and its summary is loaded normally.
415+
func TestReadAllSummaries_ValidSessionID(t *testing.T) {
416+
dir := t.TempDir()
417+
validID := "20260601-valid"
418+
if err := os.WriteFile(filepath.Join(dir, validID+".md"), []byte("valid summary"), 0600); err != nil {
419+
t.Fatalf("write valid episode: %v", err)
420+
}
421+
idx := []EpisodeMeta{{SessionID: validID, CreatedAt: time.Now().UTC()}}
422+
data, err := json.Marshal(idx)
423+
if err != nil {
424+
t.Fatalf("marshal index: %v", err)
425+
}
426+
if err := os.WriteFile(filepath.Join(dir, episodeIndexFile), data, 0600); err != nil {
427+
t.Fatalf("write index: %v", err)
428+
}
429+
430+
vi := &episodeVectorIndex{dir: dir}
431+
out := vi.readAllSummaries()
432+
if len(out) != 1 || out[0].id != validID || out[0].text != "valid summary" {
433+
t.Fatalf("expected 1 valid result for %q, got %v", validID, out)
434+
}
435+
}
436+
437+
// TestReadAllSummaries_TraversalRejected confirms that a tampered session_id
438+
// like "../secret" cannot escape the episodes directory and pull arbitrary
439+
// files into the embedding space.
440+
func TestReadAllSummaries_TraversalRejected(t *testing.T) {
441+
root := t.TempDir()
442+
epDir := filepath.Join(root, "episodes")
443+
if err := os.MkdirAll(epDir, 0700); err != nil {
444+
t.Fatalf("mkdir episodes: %v", err)
445+
}
446+
validID := "20260601-valid"
447+
if err := os.WriteFile(filepath.Join(epDir, validID+".md"), []byte("valid summary"), 0600); err != nil {
448+
t.Fatalf("write valid episode: %v", err)
449+
}
450+
// Place a file one directory above the episodes dir; a traversal would read it.
451+
if err := os.WriteFile(filepath.Join(root, "secret.md"), []byte("stolen"), 0600); err != nil {
452+
t.Fatalf("write secret: %v", err)
453+
}
454+
455+
idx := []EpisodeMeta{
456+
{SessionID: validID, CreatedAt: time.Now().UTC()},
457+
{SessionID: "../secret", CreatedAt: time.Now().UTC()},
458+
}
459+
data, err := json.Marshal(idx)
460+
if err != nil {
461+
t.Fatalf("marshal index: %v", err)
462+
}
463+
if err := os.WriteFile(filepath.Join(epDir, episodeIndexFile), data, 0600); err != nil {
464+
t.Fatalf("write index: %v", err)
465+
}
466+
467+
vi := &episodeVectorIndex{dir: epDir}
468+
out := vi.readAllSummaries()
469+
if len(out) != 1 || out[0].id != validID || out[0].text != "valid summary" {
470+
t.Fatalf("expected only valid episode, got %v", out)
471+
}
472+
}
473+
474+
// TestReadAllSummaries_PathSeparatorRejected confirms that session IDs
475+
// containing forward or backward slashes are rejected.
476+
func TestReadAllSummaries_PathSeparatorRejected(t *testing.T) {
477+
dir := t.TempDir()
478+
validID := "20260601-valid"
479+
if err := os.WriteFile(filepath.Join(dir, validID+".md"), []byte("valid summary"), 0600); err != nil {
480+
t.Fatalf("write valid episode: %v", err)
481+
}
482+
483+
// Create a subdirectory with a file that a separator-containing ID could reach.
484+
subDir := filepath.Join(dir, "sub")
485+
if err := os.MkdirAll(subDir, 0700); err != nil {
486+
t.Fatalf("mkdir sub: %v", err)
487+
}
488+
if err := os.WriteFile(filepath.Join(subDir, "secret.md"), []byte("stolen"), 0600); err != nil {
489+
t.Fatalf("write secret: %v", err)
490+
}
491+
492+
idx := []EpisodeMeta{
493+
{SessionID: validID, CreatedAt: time.Now().UTC()},
494+
{SessionID: "sub/secret", CreatedAt: time.Now().UTC()},
495+
{SessionID: "sub\\secret", CreatedAt: time.Now().UTC()},
496+
}
497+
data, err := json.Marshal(idx)
498+
if err != nil {
499+
t.Fatalf("marshal index: %v", err)
500+
}
501+
if err := os.WriteFile(filepath.Join(dir, episodeIndexFile), data, 0600); err != nil {
502+
t.Fatalf("write index: %v", err)
503+
}
504+
505+
vi := &episodeVectorIndex{dir: dir}
506+
out := vi.readAllSummaries()
507+
if len(out) != 1 || out[0].id != validID {
508+
t.Fatalf("expected only valid episode, got %v", out)
509+
}
406510
}

internal/session/vector_index.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,15 +155,34 @@ func (vi *VectorIndex) rebuildLocked() error {
155155
if e.IsDir() || !isSessionFile(e.Name()) {
156156
continue
157157
}
158-
data, err := os.ReadFile(filepath.Join(vi.dir, e.Name()))
158+
159+
// Only load session files whose base name is a valid session ID and
160+
// that are not symlinks. This prevents a planted symlink named like a
161+
// session file from pointing outside the directory and having its
162+
// content embedded into the search corpus.
163+
id := idFromPath(e.Name())
164+
if err := ValidateSessionID(id); err != nil {
165+
continue
166+
}
167+
if e.Type()&os.ModeSymlink != 0 {
168+
continue
169+
}
170+
171+
path := filepath.Join(vi.dir, e.Name())
172+
info, err := os.Lstat(path)
173+
if err != nil || info.Mode()&os.ModeSymlink != 0 {
174+
continue
175+
}
176+
177+
data, err := os.ReadFile(path)
159178
if err != nil {
160179
continue
161180
}
162181
text := extractConversationText(data)
163182
if text == "" {
164183
continue
165184
}
166-
ids = append(ids, idFromPath(e.Name()))
185+
ids = append(ids, id)
167186
corpus = append(corpus, text)
168187
}
169188

0 commit comments

Comments
 (0)