diff --git a/AGENTS.md b/AGENTS.md index 287c5b3..f207b96 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -145,6 +145,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU - **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. - **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. - **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. - **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. diff --git a/cmd/odek/wsapprover.go b/cmd/odek/wsapprover.go index bab5302..2a3f591 100644 --- a/cmd/odek/wsapprover.go +++ b/cmd/odek/wsapprover.go @@ -201,12 +201,20 @@ func (a *wsApprover) PromptOperation(op danger.ToolOperation) error { // HandleResponse processes a response from the browser. // Called by the WebSocket read loop when an approval_response arrives. // Returns true if the response matched a pending request. +// +// The send to the response channel is non-blocking: the channel has capacity +// 1, so if a duplicate or late response races with the first one, or if the +// request has already timed out and the reader is gone, the send drops the +// response instead of wedging the WebSocket read goroutine. func (a *wsApprover) HandleResponse(id, action string) bool { a.mu.Lock() resp, ok := a.pending[id] a.mu.Unlock() if ok { - resp <- action + select { + case resp <- action: + default: + } } return ok } diff --git a/cmd/odek/wsapprover_test.go b/cmd/odek/wsapprover_test.go index 04bf1b3..71dd932 100644 --- a/cmd/odek/wsapprover_test.go +++ b/cmd/odek/wsapprover_test.go @@ -76,6 +76,49 @@ func TestWSApprover_HandleResponse_NonMatching(t *testing.T) { } } +// TestWSApprover_HandleResponse_DoesNotBlock verifies that a duplicate or +// late approval response is dropped instead of blocking the WebSocket read +// goroutine. This is a regression test for the L3 relay race: previously the +// second response could wedge on a buffer-1 channel and exhaust the connection +// semaphore. +func TestWSApprover_HandleResponse_DoesNotBlock(t *testing.T) { + a := newWSApprover(func(v any) error { return nil }) + id := "test-id-block" + + a.mu.Lock() + a.pending[id] = make(chan string, 1) + a.mu.Unlock() + + // First response fills the buffer-1 channel. + if !a.HandleResponse(id, "approve") { + t.Fatal("first HandleResponse should match") + } + + // Second response must return promptly even though the channel is full. + done := make(chan struct{}) + go func() { + a.HandleResponse(id, "approve") + close(done) + }() + + select { + case <-done: + // expected + case <-time.After(2 * time.Second): + t.Fatal("second HandleResponse blocked on a full response channel") + } + + // Only one value should have been delivered. + select { + case action := <-a.pending[id]: + if action != "approve" { + t.Errorf("expected 'approve', got %q", action) + } + default: + t.Fatal("expected exactly one buffered response") + } +} + func TestWSApprover_PromptCommand_TrustedClass(t *testing.T) { callCount := 0 a := newWSApprover(func(v any) error { diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 4ff8858..58b234c 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -133,9 +133,7 @@ The `guard` section is operator-controlled: project-level `./odek.json` cannot s When a classification is set to `prompt`, an approver pauses the agent until the user decides. Two implementations: - **TTYApprover** (CLI / REPL) — reads from `/dev/tty`. -- **WSApprover** (Web UI) — sends `approval_request` over WebSocket; the browser shows a modal. - -Both: +- **WSApprover** (Web UI) — sends `approval_request` over WebSocket; the browser shows a modal. Responses are relayed to the pending prompt via a non-blocking send on a capacity-1 channel, so a duplicate, late, or raced response cannot block the WebSocket read goroutine and exhaust the global connection semaphore. - Disable the "Trust class for session" shortcut for `destructive` and `blocked`. A forged or stale UI that sends `"trust"` for those classes is coerced to a single approve. - Engage **friction mode** after 3 approvals of the same class in 60 s: require typing the literal word `approve` (no single-letter / button shortcut) and impose a 1.5 s pause before accepting input. This breaks reflex click-through under sustained LLM-driven approval pressure.