L9: validate session ID on write and detect embedded ID mismatch (#81)

- internal/session/session.go: saveLocked now validates sess.ID before
  building the destination path; Load now verifies the embedded ID matches
  the filename it was loaded from, blocking planted files with a traversal
  'id' field from redirecting Append/Save.
- internal/session/session_test.go: regression tests for malformed save IDs
  and embedded-ID mismatches on Load and Append.
- docs/SECURITY.md + AGENTS.md: document L9 mitigation.

Author: jkyberneees

AGENTS.md

@@ -149,6 +149,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
 - **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.
 - **WebSocket approval relay non-blocking** (`cmd/odek/wsapprover.go`) — `HandleResponse` uses a non-blocking send on the pending response channel, so duplicate or late approval responses are dropped instead of blocking the WebSocket read goroutine and exhausting the global connection semaphore.
 - **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.
+- **Session ID write-path validation** (`internal/session/session.go`) — `saveLocked` validates `sess.ID` with `ValidateSessionID` before computing the destination path, and `Load` checks that the embedded ID matches the filename it was loaded from. A planted session file with an attacker-controlled `"id"` field can no longer redirect `Append`/`Save` to write outside the session directory.
 - **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.
 - **Serve sandbox default-on** — `odek serve` enables `--sandbox` automatically unless `--no-sandbox` is passed.
 - **Sandbox volume confinement** (`internal/sandbox/sandbox.go`) — extra `--sandbox-volume` host paths must resolve to a location under the working directory, cannot contain `..` or symlink escapes, and cannot match sensitive prefixes such as `/etc`, `/proc`, `/sys`, `/dev`, `/root`, `/home`, `/var`, `/run`, or `/var/run/docker.sock`.

docs/SECURITY.md

@@ -655,6 +655,12 @@ Now:
 - `recordTurnAudit` scans `user` messages for untrusted wrappers as well as `tool` messages.
 - The divergence check receives the original, pre-enrichment user prompt, so injected resources are treated as novel when the agent acts on them.
 
+### 39l. Session store write-path validates the embedded session ID
+
+`internal/session/session.go` built the destination path from `sess.ID` without validating it, while `Load` only validated the filename requested by the caller. A planted session file (for example, dropped by a malicious local process or extracted from an archive) whose JSON contained `"id": "../config"` would cause the next `Append` or `Save` to write outside the session directory, such as overwriting `~/.odek/config.json`.
+
+`saveLocked` now calls `ValidateSessionID` before computing the filesystem path, and `Load` checks that the ID inside the file matches the filename it was loaded from. Any mismatch aborts the operation instead of following the attacker-controlled embedded ID.
+
 ### 40. `/api/resources` result limit cap
 
 The `/api/resources?q=...&limit=N` autocomplete endpoint previously accepted any positive `limit` value. It is now capped to 100 results both in the HTTP handler and in `Registry.Search`. This prevents a prompt-injected or attacker-forged request from forcing an unbounded directory walk and returning a multi-megabyte JSON response.

internal/session/session.go

@@ -311,6 +311,14 @@ func (s *Store) Save(sess *Session) error {
 // read and write gets replaced with a regular file.
 // Also atomically updates the session index with the session's metadata.
 func (s *Store) saveLocked(sess *Session) error {
+	// Reject malformed or traversal-bearing session IDs before the ID is used
+	// to build a filesystem path. A planted session file with an embedded
+	// "id":"../config" must not cause a subsequent Save/Append to overwrite
+	// files outside the session directory.
+	if err := ValidateSessionID(sess.ID); err != nil {
+		return fmt.Errorf("session: refusing unsafe save: %w", err)
+	}
+
 	// Redact secrets from all messages and the task label before writing to
 	// disk. This is defense-in-depth: the loop engine already redacts tool
 	// outputs, but this catches any secrets that slipped through
@@ -369,6 +377,12 @@ func (s *Store) Load(id string) (*Session, error) {
 	if err := json.Unmarshal(data, &sess); err != nil {
 		return nil, fmt.Errorf("session: parse %q: %w", id, err)
 	}
+	// The on-disk ID must match the filename it was loaded from. This prevents
+	// a planted session file from redirecting a later Save/Append to a path
+	// derived from an attacker-controlled embedded ID.
+	if sess.ID != id {
+		return nil, fmt.Errorf("session: load %q: ID mismatch (file contains %q)", id, sess.ID)
+	}
 	return &sess, nil
 }
 

internal/session/session_test.go

@@ -774,6 +774,64 @@ func TestStore_Delete_PathTraversalRejected(t *testing.T) {
 	}
 }
 
+// TestStore_Save_RejectMalformedID verifies that Save refuses to write a
+// session whose embedded ID contains path traversal.
+func TestStore_Save_RejectMalformedID(t *testing.T) {
+	store := newTestStore(t)
+	msgs := []llm.Message{{Role: "user", Content: "test"}}
+	sess, _ := store.Create(msgs, "m", "test")
+
+	sess.ID = "../config"
+	err := store.Save(sess)
+	if err == nil {
+		t.Fatal("Save() with traversal ID should return error")
+	}
+	if !strings.Contains(err.Error(), "invalid ID") {
+		t.Errorf("error should mention 'invalid ID', got: %v", err)
+	}
+}
+
+// TestStore_Load_RejectEmbeddedIDMismatch verifies that Load detects a
+// session file whose embedded ID does not match its filename.
+func TestStore_Load_RejectEmbeddedIDMismatch(t *testing.T) {
+	store := newTestStore(t)
+
+	// Plant a session file whose filename is benign but whose JSON ID is not.
+	plantedID := "20260718-plant001"
+	malicious := `{"id":"../config","created_at":"2026-07-18T00:00:00Z","updated_at":"2026-07-18T00:00:00Z","model":"m","turns":1,"task":"x","messages":[]}`
+	if err := os.WriteFile(store.Path(plantedID), []byte(malicious), 0600); err != nil {
+		t.Fatal(err)
+	}
+
+	_, err := store.Load(plantedID)
+	if err == nil {
+		t.Fatal("Load() of mismatched embedded ID should return error")
+	}
+	if !strings.Contains(err.Error(), "ID mismatch") {
+		t.Errorf("error should mention 'ID mismatch', got: %v", err)
+	}
+}
+
+// TestStore_Append_RejectEmbeddedIDMismatch verifies that Append refuses to
+// rewrite a planted session file whose embedded ID differs from the filename.
+func TestStore_Append_RejectEmbeddedIDMismatch(t *testing.T) {
+	store := newTestStore(t)
+
+	plantedID := "20260718-plant002"
+	malicious := `{"id":"../config","created_at":"2026-07-18T00:00:00Z","updated_at":"2026-07-18T00:00:00Z","model":"m","turns":1,"task":"x","messages":[]}`
+	if err := os.WriteFile(store.Path(plantedID), []byte(malicious), 0600); err != nil {
+		t.Fatal(err)
+	}
+
+	err := store.Append(plantedID, []llm.Message{{Role: "user", Content: "more"}})
+	if err == nil {
+		t.Fatal("Append() to planted mismatched file should return error")
+	}
+	if !strings.Contains(err.Error(), "ID mismatch") {
+		t.Errorf("error should mention 'ID mismatch', got: %v", err)
+	}
+}
+
 // ── Additional edge-case coverage ──────────────────────────────────────
 
 func TestValidateSessionID_NullByte(t *testing.T) {