Skip to content

Commit 8db7647

Browse files
authored
fix: reuse one TTYApprover per process in CLI mode (M-13) (#70)
shell and parallel_shell created a fresh TTYApprover for every prompt, so the friction counter and trust cache were reset each time and the post-3-approvals friction mode never engaged. Changes: - cmd/odek/shell.go: promptUser now stores the created TTYApprover in t.approver and reuses it on subsequent prompts. - cmd/odek/perf_tools.go: same for parallelShellTool.promptCommand. - Added TestShellTool_PromptUser_ReusesTTYApprover. - Updated docs/SECURITY.md and AGENTS.md.
1 parent 4aa471b commit 8db7647

5 files changed

Lines changed: 50 additions & 6 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
112112
- **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 <name> --force` clears the flag after explicit user review.
113113
- **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.
114114
- **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/<pid>/environ`.
115-
- **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.
115+
- **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.
116116
- **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`.
117117
- **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.<token>` 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.
118118
- **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.

cmd/odek/perf_tools.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -444,19 +444,22 @@ func (t *parallelShellTool) Call(argsJSON string) (result string, err error) {
444444
// parallel_shell cannot bypass interactive approval when no explicit approver
445445
// is injected.
446446
func (t *parallelShellTool) promptCommand(cls danger.RiskClass, cmd, description string) error {
447+
// Reuse a single TTYApprover per tool instance so the friction counter
448+
// and trust cache survive across multiple prompts.
449+
t.trustedMu.Lock()
447450
approver := t.approver
448451
if approver == nil {
449452
ttyApprover := danger.NewTTYApprover(&t.dangerousConfig)
450-
t.trustedMu.Lock()
451453
if t.trustedClasses != nil {
452454
ttyApprover.SetTrustedClasses(t.trustedClasses)
453455
}
454-
t.trustedMu.Unlock()
455456
if t.ttyPath != "" {
456457
ttyApprover.TTYPath = t.ttyPath
457458
}
459+
t.approver = ttyApprover
458460
approver = ttyApprover
459461
}
462+
t.trustedMu.Unlock()
460463

461464
err := approver.PromptCommand(cls, cmd, description)
462465
if err == nil {

cmd/odek/shell.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,20 +251,22 @@ func (t *shellTool) checkApproval(cmd, description string) error {
251251
func (t *shellTool) promptUser(cmd, description string) error {
252252
cls := danger.Classify(cmd)
253253

254-
// Get or create the approver
254+
// Get or create the approver. Reuse a single TTYApprover per tool instance
255+
// so the friction counter and trust cache survive across multiple prompts.
256+
t.trustedMu.Lock()
255257
approver := t.approver
256258
if approver == nil {
257259
ttyApprover := danger.NewTTYApprover(&t.dangerousConfig)
258-
t.trustedMu.Lock()
259260
if t.trustedClasses != nil {
260261
ttyApprover.SetTrustedClasses(t.trustedClasses)
261262
}
262-
t.trustedMu.Unlock()
263263
if t.ttyPath != "" {
264264
ttyApprover.TTYPath = t.ttyPath
265265
}
266+
t.approver = ttyApprover
266267
approver = ttyApprover
267268
}
269+
t.trustedMu.Unlock()
268270

269271
err := approver.PromptCommand(cls, cmd, description)
270272
if err == nil {

cmd/odek/shell_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,12 @@ import (
55
"encoding/json"
66
"os"
77
"os/exec"
8+
"path/filepath"
89
"strings"
910
"testing"
1011
"time"
12+
13+
"github.com/BackendStack21/odek/internal/danger"
1114
)
1215

1316
func TestShellTool_Name(t *testing.T) {
@@ -388,3 +391,38 @@ func stringSlicesEqual(a, b []string) bool {
388391
}
389392
return true
390393
}
394+
395+
func TestShellTool_PromptUser_ReusesTTYApprover(t *testing.T) {
396+
dir := t.TempDir()
397+
ttyPath := filepath.Join(dir, "tty")
398+
// Two approval lines; friction threshold is 3, so both non-friction
399+
// prompts accept the single-letter "a" shortcut.
400+
if err := os.WriteFile(ttyPath, []byte("a\na\n"), 0644); err != nil {
401+
t.Fatalf("write tty input: %v", err)
402+
}
403+
404+
st := &shellTool{
405+
dangerousConfig: danger.DangerousConfig{
406+
Classes: map[danger.RiskClass]danger.Action{
407+
danger.NetworkEgress: danger.Prompt,
408+
},
409+
},
410+
ttyPath: ttyPath,
411+
}
412+
413+
cmd := "curl http://example.com"
414+
if err := st.promptUser(cmd, "test"); err != nil {
415+
t.Fatalf("first prompt failed: %v", err)
416+
}
417+
first := st.approver
418+
if first == nil {
419+
t.Fatal("first prompt did not create an approver")
420+
}
421+
422+
if err := st.promptUser(cmd, "test"); err != nil {
423+
t.Fatalf("second prompt failed: %v", err)
424+
}
425+
if st.approver != first {
426+
t.Error("promptUser created a new TTYApprover instead of reusing the existing one")
427+
}
428+
}

docs/SECURITY.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ Both:
139139

140140
- 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.
141141
- 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.
142+
- 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.
142143
- Default to `non_interactive: deny` for sub-agents and any context without a TTY (configurable).
143144

144145
### 5. Memory taint tracking

0 commit comments

Comments
 (0)