Skip to content

Commit b6ae4b0

Browse files
committed
security: memory package hardening (v0.27.1)
Fix #1 (🔴): Path traversal — wire session.ValidateSessionID into episodes.go Write/Read, memory.go OnSessionEnd, tool.go handleView. Prevents arbitrary file read/write via crafted session IDs. Fix #2 (🔴): TOCTOU in FactStore — add sync.Mutex to Add/Replace/Remove, atomic temp+rename for writeEntries. Prevents lost writes and data corruption from concurrent sessions sharing the same memory dir. Fix #3 (🔴): addToIndex TOCTOU — add sync.Mutex to EpisodeStore index operations, temp file cleanup on rename failure. Prevents lost episode index entries (orphaned .md files). Fix #4 (🟠): Thread safety — add sync.RWMutex for promptDirty/promptCache, consolidate into markPromptDirty() helper. Eliminates data race. Fix #5 (🟡): Consolidate bypass ScanContent — scan LLM output before persisting to fact files, invalidate prompt cache, re-fit merge detector. Also: episode dir 0755→0700 (world-readable→owner-only). All 16 packages pass with -race. Zero new warnings.
1 parent fad99dd commit b6ae4b0

4 files changed

Lines changed: 105 additions & 16 deletions

File tree

internal/memory/episodes.go

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ import (
99
"path/filepath"
1010
"sort"
1111
"strings"
12+
"sync"
1213
"time"
1314

1415
"github.com/BackendStack21/go-vector/pkg/vector"
16+
"github.com/BackendStack21/kode/internal/session"
1517
)
1618

1719
// maxEpisodeSummaryBytes caps how much summary text we store per episode.
@@ -35,7 +37,10 @@ type RankStrategy func(query string, episodes []EpisodeMeta) ([]EpisodeMeta, err
3537

3638
// EpisodeStore manages on-disk episode summaries (Tier 3 memory).
3739
// Written after sessions with sufficient turns, searchable via SimpleCall.
40+
// Index operations are protected by a mutex to prevent TOCTOU races
41+
// between concurrent sessions sharing the same memory directory.
3842
type EpisodeStore struct {
43+
mu sync.Mutex
3944
dir string
4045
rankFn RankStrategy
4146
}
@@ -54,8 +59,12 @@ func NewEpisodeStore(dir string, rankFn RankStrategy) *EpisodeStore {
5459

5560
// Write stores an episode summary for a session. Creates the episodes
5661
// directory and updates the index.
62+
// sessionID is validated for path traversal before use.
5763
func (e *EpisodeStore) Write(sessionID, summary string, turns int) error {
58-
if err := os.MkdirAll(e.dir, 0755); err != nil {
64+
if err := session.ValidateSessionID(sessionID); err != nil {
65+
return fmt.Errorf("memory: episodes write: %w", err)
66+
}
67+
if err := os.MkdirAll(e.dir, 0700); err != nil {
5968
return fmt.Errorf("memory: episodes mkdir: %w", err)
6069
}
6170

@@ -90,7 +99,11 @@ func (e *EpisodeStore) WriteIfEnough(sessionID, summary string, turns int) error
9099
}
91100

92101
// Read returns the full summary content for a session.
102+
// sessionID is validated for path traversal before use.
93103
func (e *EpisodeStore) Read(sessionID string) (string, error) {
104+
if err := session.ValidateSessionID(sessionID); err != nil {
105+
return "", fmt.Errorf("memory: episodes read: %w", err)
106+
}
94107
path := filepath.Join(e.dir, sessionID+".md")
95108
data, err := os.ReadFile(path)
96109
if err != nil {
@@ -146,7 +159,11 @@ func (e *EpisodeStore) Search(query string, limit int) ([]EpisodeMeta, error) {
146159
// ── Index helpers ─────────────────────────────────────────────────────
147160

148161
// addToIndex appends an entry to the index and writes it.
162+
// Caller must hold e.mu (acquired by Write).
149163
func (e *EpisodeStore) addToIndex(meta EpisodeMeta) error {
164+
e.mu.Lock()
165+
defer e.mu.Unlock()
166+
150167
idx, err := e.ReadIndex()
151168
if err != nil {
152169
// Index error means we start fresh
@@ -156,7 +173,8 @@ func (e *EpisodeStore) addToIndex(meta EpisodeMeta) error {
156173
return e.writeIndex(idx)
157174
}
158175

159-
// writeIndex serializes the index to disk.
176+
// writeIndex serializes the index to disk atomically (temp + rename).
177+
// Caller must hold e.mu.
160178
func (e *EpisodeStore) writeIndex(idx []EpisodeMeta) error {
161179
// Write to temp + rename for atomicity
162180
idxPath := filepath.Join(e.dir, episodeIndexFile)
@@ -169,7 +187,11 @@ func (e *EpisodeStore) writeIndex(idx []EpisodeMeta) error {
169187
if err := os.WriteFile(tmpPath, data, 0600); err != nil {
170188
return err
171189
}
172-
return os.Rename(tmpPath, idxPath)
190+
if err := os.Rename(tmpPath, idxPath); err != nil {
191+
os.Remove(tmpPath) // best-effort cleanup
192+
return err
193+
}
194+
return nil
173195
}
174196

175197
// truncateForIndex shortens the summary for the index entry (first 120 chars).

internal/memory/facts.go

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"os"
2727
"path/filepath"
2828
"strings"
29+
"sync"
2930
)
3031

3132
// File names for fact targets.
@@ -45,10 +46,13 @@ const entrySep = "\n§\n"
4546

4647
// FactStore manages typed fact files (user.md and env.md) with character caps,
4748
// duplicate prevention, and entry-level CRUD via substring matching.
49+
// All write operations are protected by a mutex to prevent TOCTOU races
50+
// between concurrent sessions sharing the same memory directory.
4851
type FactStore struct {
49-
dir string
50-
capUser int
51-
capEnv int
52+
mu sync.Mutex
53+
dir string
54+
capUser int
55+
capEnv int
5256
}
5357

5458
// NewFactStore creates a FactStore rooted at dir. Fact files are stored as
@@ -124,6 +128,9 @@ func (f *FactStore) Add(target, content string) error {
124128
return fmt.Errorf("memory: empty content")
125129
}
126130

131+
f.mu.Lock()
132+
defer f.mu.Unlock()
133+
127134
content = strings.TrimSpace(content)
128135

129136
// Read existing content
@@ -161,7 +168,7 @@ func (f *FactStore) Add(target, content string) error {
161168
newContent = existing + entrySep + content
162169
}
163170

164-
return os.WriteFile(f.path(target), []byte(newContent), 0600)
171+
return f.writeEntries(target, parseEntries(newContent))
165172
}
166173

167174
// Replace finds an entry by substring match and replaces it with new content.
@@ -177,6 +184,9 @@ func (f *FactStore) Replace(target, oldText, content string) error {
177184
return fmt.Errorf("memory: empty old_text")
178185
}
179186

187+
f.mu.Lock()
188+
defer f.mu.Unlock()
189+
180190
content = strings.TrimSpace(content)
181191
oldText = strings.TrimSpace(oldText)
182192

@@ -236,6 +246,9 @@ func (f *FactStore) Remove(target, oldText string) error {
236246
return fmt.Errorf("memory: empty old_text")
237247
}
238248

249+
f.mu.Lock()
250+
defer f.mu.Unlock()
251+
239252
oldText = strings.TrimSpace(oldText)
240253

241254
existing, err := f.Read(target)
@@ -280,10 +293,17 @@ func (f *FactStore) Entries(target string) ([]string, error) {
280293
return parseEntries(existing), nil
281294
}
282295

283-
// writeEntries joins entries and writes to disk.
296+
// writeEntries joins entries and writes to disk atomically (temp + rename).
297+
// Caller must hold f.mu.
284298
func (f *FactStore) writeEntries(target string, entries []string) error {
285299
content := strings.Join(entries, entrySep)
286-
return os.WriteFile(f.path(target), []byte(content), 0600)
300+
path := f.path(target)
301+
tmpPath := path + ".tmp"
302+
303+
if err := os.WriteFile(tmpPath, []byte(content), 0600); err != nil {
304+
return err
305+
}
306+
return os.Rename(tmpPath, path)
287307
}
288308

289309
// parseEntries splits file content into individual entries.

internal/memory/memory.go

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ import (
44
"context"
55
"fmt"
66
"strings"
7+
"sync"
8+
9+
"github.com/BackendStack21/kode/internal/session"
710
)
811

912
// Default memory dir relative to ~/.odek/
@@ -79,6 +82,7 @@ type MemoryManager struct {
7982
// prompt caching avoids rebuilding the system prompt block on every
8083
// iteration when memory hasn't changed. The cache is invalidated
8184
// whenever facts or buffer are modified.
85+
promptMu sync.RWMutex
8286
promptCache string
8387
promptDirty bool
8488
}
@@ -231,7 +235,7 @@ func (m *MemoryManager) AddFact(target, content string) error {
231235
if err := m.facts.Add(target, content); err != nil {
232236
return err
233237
}
234-
m.promptDirty = true
238+
m.markPromptDirty()
235239

236240
// Incrementally update merge detector instead of re-reading + re-embedding all.
237241
// Check dedup: if content already existed in the entries we read at the top,
@@ -270,7 +274,7 @@ func (m *MemoryManager) ReplaceFact(target, oldText, content string) error {
270274
if err := m.facts.Replace(target, oldText, content); err != nil {
271275
return err
272276
}
273-
m.promptDirty = true
277+
m.markPromptDirty()
274278
// Re-fit merge detector
275279
if m.cfg.MergeOnWrite != nil && *m.cfg.MergeOnWrite {
276280
entries, _ := m.facts.Entries(target)
@@ -287,7 +291,7 @@ func (m *MemoryManager) RemoveFact(target, oldText string) error {
287291
if err := m.facts.Remove(target, oldText); err != nil {
288292
return err
289293
}
290-
m.promptDirty = true
294+
m.markPromptDirty()
291295
// Re-fit merge detector
292296
if m.cfg.MergeOnWrite != nil && *m.cfg.MergeOnWrite {
293297
entries, _ := m.facts.Entries(target)
@@ -349,8 +353,28 @@ Entries for %s:
349353
return nil // LLM returned nothing useful
350354
}
351355

356+
// Security: scan LLM output before persisting
357+
for _, entry := range newEntries {
358+
entry = strings.TrimSpace(entry)
359+
if entry == "" {
360+
continue
361+
}
362+
if err := ScanContent(entry); err != nil {
363+
return fmt.Errorf("memory: consolidated entry rejected: %w", err)
364+
}
365+
}
366+
352367
// Write back
353-
return m.facts.writeEntries(target, newEntries)
368+
if err := m.facts.writeEntries(target, newEntries); err != nil {
369+
return err
370+
}
371+
m.markPromptDirty()
372+
// Re-fit merge detector
373+
if m.cfg.MergeOnWrite != nil && *m.cfg.MergeOnWrite {
374+
entries, _ := m.facts.Entries(target)
375+
m.merge.Fit(entries)
376+
}
377+
return nil
354378
}
355379

356380
// ── Buffer Operations ────────────────────────────────────────────────
@@ -362,7 +386,7 @@ func (m *MemoryManager) AppendBuffer(role, message string) {
362386
}
363387
line := FormatBufferLine(role, message)
364388
m.buffer.Append(line)
365-
m.promptDirty = true
389+
m.markPromptDirty()
366390
}
367391

368392
// GetBuffer returns the current buffer lines (for system prompt injection).
@@ -382,20 +406,32 @@ func (m *MemoryManager) RestoreBuffer(lines []string) {
382406
for _, line := range lines {
383407
m.buffer.Append(line)
384408
}
385-
m.promptDirty = true
409+
m.markPromptDirty()
386410
}
387411

388412
// ClearBuffer resets the buffer for a new session.
389413
func (m *MemoryManager) ClearBuffer() {
390414
m.buffer.Clear()
415+
m.markPromptDirty()
416+
}
417+
418+
// markPromptDirty invalidates the cached system prompt so the next
419+
// BuildSystemPrompt() call rebuilds from current facts/buffer state.
420+
func (m *MemoryManager) markPromptDirty() {
421+
m.promptMu.Lock()
391422
m.promptDirty = true
423+
m.promptMu.Unlock()
392424
}
393425

394426
// ── Episode Operations ───────────────────────────────────────────────
395427

396428
// OnSessionEnd is called when a session ends. If turns >= threshold,
397429
// extracts durable facts using the LLM and stores them as an episode.
430+
// sessionID is validated for path traversal before any file I/O.
398431
func (m *MemoryManager) OnSessionEnd(sessionID string, turns int, messages []string) {
432+
if err := session.ValidateSessionID(sessionID); err != nil {
433+
return
434+
}
399435
minTurns := m.cfg.MinTurnsForExtraction
400436
if minTurns <= 0 {
401437
minTurns = defaultMinTurnsForExtraction
@@ -468,9 +504,13 @@ func (m *MemoryManager) BuildSystemPrompt() string {
468504
}
469505

470506
// Return cached prompt if memory hasn't changed since last build.
507+
m.promptMu.RLock()
471508
if !m.promptDirty && m.promptCache != "" {
472-
return m.promptCache
509+
cached := m.promptCache
510+
m.promptMu.RUnlock()
511+
return cached
473512
}
513+
m.promptMu.RUnlock()
474514

475515
userFact, _ := m.facts.Read("user")
476516
envFact, _ := m.facts.Read("env")
@@ -542,8 +582,10 @@ func (m *MemoryManager) BuildSystemPrompt() string {
542582
}
543583

544584
b.WriteString("───────────────────────────────\n")
585+
m.promptMu.Lock()
545586
m.promptCache = b.String()
546587
m.promptDirty = false
588+
m.promptMu.Unlock()
547589
return m.promptCache
548590
}
549591

internal/memory/tool.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"encoding/json"
55
"fmt"
66
"strings"
7+
8+
"github.com/BackendStack21/kode/internal/session"
79
)
810

911
// memoryToolSchema is the JSON schema for the `memory` tool.
@@ -179,6 +181,9 @@ func (t *MemoryTool) handleView(target, query string) (string, error) {
179181
if query == "" {
180182
return errorJSON("query (session_id) is required for view"), nil
181183
}
184+
if err := session.ValidateSessionID(query); err != nil {
185+
return errorJSON("invalid session_id: " + err.Error()), nil
186+
}
182187
content, err := t.manager.episodes.Read(query)
183188
if err != nil {
184189
return errorJSON(err.Error()), nil

0 commit comments

Comments
 (0)