Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
6 changes: 6 additions & 0 deletions docs/SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 14 additions & 0 deletions internal/session/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
58 changes: 58 additions & 0 deletions internal/session/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading