diff --git a/AGENTS.md b/AGENTS.md index 37b2ca8..5bdd00b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -112,7 +112,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU - **Skill provenance gate** (`internal/skills/loader.go` + `cache.go` + `tools.go`) — `Skill.Provenance{Untrusted, Sources, NeedsReview}`. NeedsReview skills pin to Lazy regardless of `auto_load`. The auto-save path declines tainted suggestions by default. Agent-created skills via `skill_save` and patched skills via `skill_patch` are forced to `Untrusted` + `NeedsReview`, and `skill_patch` refuses edits that touch the YAML frontmatter, blocking an injected agent from flipping `auto_load` or clearing `needs_review`. `odek skill promote --force` clears the flag after explicit user review. - **Sub-agent damage cap** (`cmd/odek/subagent.go::applySubagentTrust`) — `delegate_tasks` carries `trust_level` + `max_risk`. Untrusted ⇒ NonInteractive=deny, Destructive/CodeExec/Install/SystemWrite/NetworkEgress all forced to Deny. `max_risk` ⇒ everything above cap forced to Deny. - **FD-based API key handoff** (`cmd/odek/subagent_key.go`) — parent writes key to a 0600 tempfile, immediately `unlink()`s, passes the FD via `cmd.ExtraFiles`. Sub-agent reads from `$ODEK_API_KEY_FD` and closes. Key never in `/proc//environ`. -- **Approver friction** (`internal/danger/approver.go`, `cmd/odek/wsapprover.go`) — both TTYApprover and WSApprover engage friction mode after 3 approvals of the same class in 60s: require typing literal `approve`, 1.5s pause. Trust-class shortcut disabled for `destructive` + `blocked` regardless. +- **Approver friction** (`internal/danger/approver.go`, `cmd/odek/wsapprover.go`, `cmd/odek/shell.go`, `cmd/odek/perf_tools.go`) — both TTYApprover and WSApprover engage friction mode after 3 approvals of the same class in 60s: require typing literal `approve`, 1.5s pause. Trust-class shortcut disabled for `destructive` + `blocked` regardless. CLI `shell`/`parallel_shell` reuse a single TTYApprover instance per process so the counter and trust cache persist across prompts. - **Danger classifier bypass resistance** (`internal/danger/classifier.go`) — `normalize()` pre-processes: expand `$IFS` / `${IFS}`, extract `$(...)` / `` `...` `` substitutions, strip `command` / `exec` / `builtin` wrappers, collapse unquoted backslashes, basename absolute paths. `awk`/`gawk`, `sed` (`e` command / `-f`), and editors (`vi`/`vim`/`nvim`/`emacs`/`ed`/`ex`) are classified as `code_execution` when given a script or file operand, closing `awk 'BEGIN{system(...)}'`, `sed 's///e'`, and editor `!` shell escapes. `rm -rf ./` / `rm -rf ./..` are normalised to `.` / `..` for wipe-target matching, and `${VAR:--rf}` default-value flag substitutions are treated as fail-closed. Regression suite in `classifier_bypass_test.go`. - **WebSocket CSRF token** (`cmd/odek/serve.go`) — `odek serve` issues a random 256-bit token at startup and requires it on `/ws` via cookie, `X-Odek-Ws-Token` header, or `odek.` subprotocol. The token is no longer embedded in every `GET /` response; it is only delivered when the request includes the correct `?token=` query parameter (Jupyter-style), and a non-loopback bind prints a loud warning. The localhost origin check remains as defense-in-depth. - **SSRF / DNS-rebinding dial guard** (`cmd/odek/ssrf_guard.go` + `internal/danger/classifier.go`) — `browser`, `http_batch`, and `web_search` resolve hostnames at dial time and refuse internal IPs (loopback, RFC1918, RFC4193, link-local, RFC 6598 CGNAT `100.64.0.0/10`, RFC 2544 benchmark `198.18.0.0/15`, and unspecified), then pin the dial to the validated IP. Operator-configured backends (e.g. `web_search.base_url`) are added to an allowlist so container-internal services such as SearXNG remain reachable. diff --git a/cmd/odek/perf_tools.go b/cmd/odek/perf_tools.go index f7f121b..b34055e 100644 --- a/cmd/odek/perf_tools.go +++ b/cmd/odek/perf_tools.go @@ -444,19 +444,22 @@ func (t *parallelShellTool) Call(argsJSON string) (result string, err error) { // parallel_shell cannot bypass interactive approval when no explicit approver // is injected. func (t *parallelShellTool) promptCommand(cls danger.RiskClass, cmd, description string) error { + // Reuse a single TTYApprover per tool instance so the friction counter + // and trust cache survive across multiple prompts. + t.trustedMu.Lock() approver := t.approver if approver == nil { ttyApprover := danger.NewTTYApprover(&t.dangerousConfig) - t.trustedMu.Lock() if t.trustedClasses != nil { ttyApprover.SetTrustedClasses(t.trustedClasses) } - t.trustedMu.Unlock() if t.ttyPath != "" { ttyApprover.TTYPath = t.ttyPath } + t.approver = ttyApprover approver = ttyApprover } + t.trustedMu.Unlock() err := approver.PromptCommand(cls, cmd, description) if err == nil { diff --git a/cmd/odek/shell.go b/cmd/odek/shell.go index 3258285..930c3f5 100644 --- a/cmd/odek/shell.go +++ b/cmd/odek/shell.go @@ -251,20 +251,22 @@ func (t *shellTool) checkApproval(cmd, description string) error { func (t *shellTool) promptUser(cmd, description string) error { cls := danger.Classify(cmd) - // Get or create the approver + // Get or create the approver. Reuse a single TTYApprover per tool instance + // so the friction counter and trust cache survive across multiple prompts. + t.trustedMu.Lock() approver := t.approver if approver == nil { ttyApprover := danger.NewTTYApprover(&t.dangerousConfig) - t.trustedMu.Lock() if t.trustedClasses != nil { ttyApprover.SetTrustedClasses(t.trustedClasses) } - t.trustedMu.Unlock() if t.ttyPath != "" { ttyApprover.TTYPath = t.ttyPath } + t.approver = ttyApprover approver = ttyApprover } + t.trustedMu.Unlock() err := approver.PromptCommand(cls, cmd, description) if err == nil { diff --git a/cmd/odek/shell_test.go b/cmd/odek/shell_test.go index 34dd162..04e81cb 100644 --- a/cmd/odek/shell_test.go +++ b/cmd/odek/shell_test.go @@ -5,9 +5,12 @@ import ( "encoding/json" "os" "os/exec" + "path/filepath" "strings" "testing" "time" + + "github.com/BackendStack21/odek/internal/danger" ) func TestShellTool_Name(t *testing.T) { @@ -388,3 +391,38 @@ func stringSlicesEqual(a, b []string) bool { } return true } + +func TestShellTool_PromptUser_ReusesTTYApprover(t *testing.T) { + dir := t.TempDir() + ttyPath := filepath.Join(dir, "tty") + // Two approval lines; friction threshold is 3, so both non-friction + // prompts accept the single-letter "a" shortcut. + if err := os.WriteFile(ttyPath, []byte("a\na\n"), 0644); err != nil { + t.Fatalf("write tty input: %v", err) + } + + st := &shellTool{ + dangerousConfig: danger.DangerousConfig{ + Classes: map[danger.RiskClass]danger.Action{ + danger.NetworkEgress: danger.Prompt, + }, + }, + ttyPath: ttyPath, + } + + cmd := "curl http://example.com" + if err := st.promptUser(cmd, "test"); err != nil { + t.Fatalf("first prompt failed: %v", err) + } + first := st.approver + if first == nil { + t.Fatal("first prompt did not create an approver") + } + + if err := st.promptUser(cmd, "test"); err != nil { + t.Fatalf("second prompt failed: %v", err) + } + if st.approver != first { + t.Error("promptUser created a new TTYApprover instead of reusing the existing one") + } +} diff --git a/docs/SECURITY.md b/docs/SECURITY.md index b7ae358..1c0b0e0 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -139,6 +139,7 @@ Both: - 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. +- In CLI mode, the `shell` and `parallel_shell` tools reuse a single `TTYApprover` instance per process, so the friction counter and trust cache persist across prompts. Previously each prompt created a fresh approver, disabling friction entirely. - Default to `non_interactive: deny` for sub-agents and any context without a TTY (configurable). ### 5. Memory taint tracking