From 2318113a4b4e48beb608bc618ddebb06b32d5d2b Mon Sep 17 00:00:00 2001 From: Jake Wise Date: Thu, 4 Jun 2026 14:27:27 +0100 Subject: [PATCH] feat(permission): contextual approval with token-aware prefix matching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements Phases 1–3 of the contextual permission redesign: Phase 1 — Permission service (token-aware prefix matching): - Add Contexts []string to CreatePermissionRequest / PermissionRequest - Add PermissionKey.Context for per-token session grants - GrantPersistent records one key per context token (Path omitted from contextual keys; location semantics live in path: tokens) - Request() step 5: all contexts must be satisfied for auto-approval; step 6: legacy path-based key fallback for context-less tools - Replace regex-based contextSatisfied with clean iteration over stored grants using tokenSatisfies helper Phase 2 — context.go: tokenSatisfies helper: - command: satisfies command: iff B == A or B starts with A+space (word boundary: command:go satisfies command:go test, not command:golang; command:py does not satisfy command:python3) - path: satisfies path: iff B == A, or B starts with A+separator, or A is / (root satisfies all absolute paths) Phase 3 — Bash tool produces contexts (bashctx.go): - AnalyzeCommand parses command strings via mvdan.cc/sh AST walker - Emits command: and command: tokens for every SimpleCommand in chains (&&, ||, ;) and pipelines (|) - Emits path: tokens for every path argument, resolved against working dir (abs, relative, .., ~, quoted paths all handled) - Fail-closed: command substitution, backticks, redirects, sh -c, eval, loops, conditionals, grouping, process substitution → single opaque - Wired as Contexts: AnalyzeCommand(params.Command, execWorkingDir) in the permissions.Request call in bash.go Phase 4 — config allowed_commands/allowed_paths to context tokens - Add AllowedCommands []string and AllowedPaths []string to config.Permissions (json: allowed_commands / allowed_paths) allowed_commands: shell command names/subcommands (e.g. "go test", "git diff") — auto-approve requests for matching command: tokens; prefix matching applies ("go" also approves "go test", "go build") allowed_paths: filesystem paths (e.g. "/tmp", "/home/user/projects") — auto-approve requests for matching path: tokens; subpaths are also approved (path:/tmp approves path:/tmp/subdir). Relative paths are resolved against the working directory at startup. - Add MakeCommandToken / MakePathToken to internal/agent/tools/bashctx.go; these are the canonical token constructors for the bash tool domain — both the AST walker in AnalyzeCommand and app.go config translation use them to ensure token format can never diverge - Add allowedContexts []string to permissionService; contextSatisfied checks configured tokens via tokenSatisfies before session grants - Update NewPermissionService to accept allowedContexts; all call sites updated (app.go, testing.go, agent and permission test files) - app.go imports internal/agent/tools and constructs context tokens from AllowedCommands (MakeCommandToken) and AllowedPaths (MakePathToken + filepath resolution for relative paths) - Add TestMakeCommandToken / TestMakePathToken to bashctx_test.go; add TestPermissionService_AllowedContexts to permission_test.go Phase 5 — UI transparency — show only pending context tokens in dialog When the permissions dialog is displayed for a bash tool request, context tokens that are already approved by config or session grants are now omitted. Only the truly new (pending) tokens are shown. A faint note shows how many tokens were already approved when at least one is being omitted. - Add PendingContexts []string to permission.PermissionRequest and proto.PermissionRequest (serialised for client-server mode) - Populate PendingContexts in Request() by reusing the contextSatisfied check that already runs to decide auto-approval - Add renderPendingContexts / parseContextToken helpers to the permissions dialog; update renderBashContent and renderDefaultContent to use them - Update client_workspace.go to map PendingContexts across the wire boundary Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/agent/common_test.go | 2 +- internal/agent/hooked_tool_test.go | 4 +- internal/agent/tools/bash.go | 1 + internal/agent/tools/bashctx.go | 312 ++++++ internal/agent/tools/bashctx_test.go | 475 ++++++++++ internal/app/app.go | 20 +- internal/app/testing.go | 2 +- internal/config/config.go | 4 +- internal/permission/context.go | 45 + internal/permission/context_test.go | 68 ++ internal/permission/permission.go | 156 ++- internal/permission/permission_test.go | 1203 +++++++++++++++++++++++- internal/proto/permission.go | 33 +- internal/ui/dialog/permissions.go | 57 +- internal/workspace/client_workspace.go | 21 +- 15 files changed, 2326 insertions(+), 77 deletions(-) create mode 100644 internal/agent/tools/bashctx.go create mode 100644 internal/agent/tools/bashctx_test.go create mode 100644 internal/permission/context.go create mode 100644 internal/permission/context_test.go diff --git a/internal/agent/common_test.go b/internal/agent/common_test.go index 8eb0f10c4a..5936d6029e 100644 --- a/internal/agent/common_test.go +++ b/internal/agent/common_test.go @@ -75,7 +75,7 @@ func testEnv(t *testing.T) fakeEnv { sessions := session.NewService(q, conn) messages := message.NewService(q) - permissions := permission.NewPermissionService(workingDir, true, []string{}) + permissions := permission.NewPermissionService(workingDir, true, []string{}, nil) history := history.NewService(q, conn) filetrackerService := filetracker.NewService(q) lspClients := csync.NewMap[string, *lsp.Client]() diff --git a/internal/agent/hooked_tool_test.go b/internal/agent/hooked_tool_test.go index 5a7823206f..2e510cf4cc 100644 --- a/internal/agent/hooked_tool_test.go +++ b/internal/agent/hooked_tool_test.go @@ -58,7 +58,7 @@ func TestHookedTool_AllowStampsHookApproval(t *testing.T) { require.True(t, inner.called, "inner tool should have run") // The inner tool's permission service can now treat call-1 as pre-approved. - svc := permission.NewPermissionService(t.TempDir(), false, nil) + svc := permission.NewPermissionService(t.TempDir(), false, nil, nil) granted, err := svc.Request(inner.gotCtx, permission.CreatePermissionRequest{ SessionID: "s1", ToolCallID: "call-1", @@ -85,7 +85,7 @@ func TestHookedTool_SilentDoesNotStampApproval(t *testing.T) { // and must fall through to the normal flow. We verify by checking that // the context does not look pre-approved for this call ID: sending a // request that no subscriber resolves will block until cancelled. - svc := permission.NewPermissionService(t.TempDir(), false, nil) + svc := permission.NewPermissionService(t.TempDir(), false, nil, nil) ctx, cancel := context.WithCancel(inner.gotCtx) cancel() granted, err := svc.Request(ctx, permission.CreatePermissionRequest{ diff --git a/internal/agent/tools/bash.go b/internal/agent/tools/bash.go index 6b91c584ce..8f24003aaf 100644 --- a/internal/agent/tools/bash.go +++ b/internal/agent/tools/bash.go @@ -231,6 +231,7 @@ func NewBashTool(permissions permission.Service, workingDir string, attribution Action: "execute", Description: fmt.Sprintf("Execute command: %s", params.Command), Params: BashPermissionsParams(params), + Contexts: AnalyzeCommand(params.Command, execWorkingDir), }, ) if err != nil { diff --git a/internal/agent/tools/bashctx.go b/internal/agent/tools/bashctx.go new file mode 100644 index 0000000000..174372fae6 --- /dev/null +++ b/internal/agent/tools/bashctx.go @@ -0,0 +1,312 @@ +package tools + +import ( + "os/user" + "path/filepath" + "strings" + + "mvdan.cc/sh/v3/syntax" +) + +// MakeCommandToken constructs a normalised command: context token from a +// command string. cmd may be a bare command name ("go") or include a +// subcommand ("go test"). Surrounding and internal whitespace is normalised +// so callers constructing tokens from config entries and callers constructing +// tokens from parsed AST nodes always agree on the token string. +func MakeCommandToken(cmd string) string { + return "command:" + strings.Join(strings.Fields(cmd), " ") +} + +// MakePathToken constructs a path: context token from an absolute, +// already-cleaned path. +func MakePathToken(path string) string { + return "path:" + filepath.Clean(path) +} + +// AnalyzeCommand parses a shell command string and returns context tokens +// for use with the permission system. It walks the AST to extract command +// names (including subcommands) and path arguments, resolving relative paths +// against the working directory. +// +// Fail-closed semantics: for unsafe constructs (command substitution, +// backticks, redirection, process substitution, eval, sh -c, function +// declarations, grouping, loops, conditionals, chains), the parser emits a +// single opaque token "command!:" instead of individual +// command/path tokens. This ensures the request still carries a context and +// is never silently auto-approved. +// +// Safe constructs: simple commands, and pipelines (|), and command chains +// (&&, ||, ;) produce command:path tokens for each constituent command. +func AnalyzeCommand(cmd, workingDir string) []string { + if cmd == "" || strings.TrimSpace(cmd) == "" { + return []string{MakeCommandToken(strings.TrimSpace(cmd))} + } + parsed, err := syntax.NewParser().Parse(strings.NewReader(cmd), "") + if err != nil { + return opaqueTokens(cmd) + } + + if !hasUnsafeConstructs(parsed) { + return extractTokens(cmd, parsed, workingDir) + } + return opaqueTokens(cmd) +} + +// opaqueTokens returns an opaque context token for the entire command. +// This is used when the command contains unsafe constructs that the parser +// cannot model as individual contexts. +func opaqueTokens(cmd string) []string { + return []string{"command!:" + strings.TrimSpace(cmd)} +} + +// hasUnsafeConstructs reports whether the AST contains constructs that the +// parser cannot safely model as individual command/path contexts. +// +// The spec (§3.4, §3.5) explicitly lists these as "fail-closed": +// - Command substitution: $(…), backticks +// - Process substitution: <(cmd), >(cmd) +// - Redirection: >, <, >>, 2>&1 +// - Shell execution via flags: sh -c, bash -c +// - Eval +// - Function declarations +// - Grouping: (cmd), { cmd; } +// - Loops and conditionals: for, while, if, case +// +// Chains (&&, ||, ;) and pipelines (|) are safe — they produce the same +// effect as separate commands and the spec (§4 §3.4) requires emitting +// command: tokens for each command in a chain. +func hasUnsafeConstructs(n syntax.Node) bool { + hasUnsafe := false + syntax.Walk(n, func(cn syntax.Node) bool { + if hasUnsafe { + return false + } + switch cn.(type) { + case *syntax.CmdSubst: + // $() command substitution (backticks are folded here). + hasUnsafe = true + case *syntax.Redirect: + // >, <, >>, 2>&1 etc. + hasUnsafe = true + case *syntax.Subshell, *syntax.Block: + // (cmd) and { cmd; } — grouping, fail closed. + hasUnsafe = true + case *syntax.FuncDecl: + // Function declarations. + hasUnsafe = true + case *syntax.IfClause, *syntax.WhileClause, *syntax.ForClause, *syntax.CaseClause: + // Conditionals and loops — fail closed. + hasUnsafe = true + case *syntax.CallExpr: + // Detect sh -c / bash -c and eval. + ce := cn.(*syntax.CallExpr) + if len(ce.Args) > 0 { + first := cmdText(ce.Args[0]) + if first == "eval" { + hasUnsafe = true + } else if first == "sh" || first == "bash" || first == "zsh" { + // Check for -c flag. + for _, a := range ce.Args[1:] { + if isSafeWord(a) && cmdText(a) == "-c" { + hasUnsafe = true + break + } + } + } + } + } + // Check for ProcSubst inside Word nodes: <(ls) or >(ls). + if w, ok := cn.(*syntax.Word); ok { + for _, part := range w.Parts { + if _, ok := part.(*syntax.ProcSubst); ok { + hasUnsafe = true + return false + } + } + } + return true + }) + return hasUnsafe +} + +// extractTokens walks the safe AST and extracts command and path tokens. +func extractTokens(cmd string, n syntax.Node, workingDir string) []string { + var commands []string + var paths []string + + syntax.Walk(n, func(cn syntax.Node) bool { + ce, ok := cn.(*syntax.CallExpr) + if !ok { + return true + } + + // Extract command name (first argument). + if len(ce.Args) > 0 && isSafeWord(ce.Args[0]) { + cmdName := cmdText(ce.Args[0]) + hasSub := false + // Check if second word is a subcommand. + if len(ce.Args) > 1 && isSafeWord(ce.Args[1]) { + sub := cmdText(ce.Args[1]) + if sub != "" && isSubcommand(sub) { + commands = append(commands, MakeCommandToken(cmdName+" "+sub)) + hasSub = true + } + } + if cmdName != "" && !hasSub { + commands = append(commands, MakeCommandToken(cmdName)) + } + } + + // Extract path arguments (all arguments after the first). + for _, w := range ce.Args[1:] { + if !isSafeWord(w) { + continue + } + wordStr := cmdText(w) + if looksLikePath(wordStr) { + if p := cleanPath(wordStr, workingDir); p != "" { + paths = append(paths, MakePathToken(p)) + } + } + } + return true + }) + + if len(commands) == 0 { + return opaqueTokens(cmd) + } + + // Deduplicate. + seen := make(map[string]struct{}) + var tokens []string + for _, c := range commands { + if _, ok := seen[c]; !ok { + seen[c] = struct{}{} + tokens = append(tokens, c) + } + } + for _, p := range paths { + if _, ok := seen[p]; !ok { + seen[p] = struct{}{} + tokens = append(tokens, p) + } + } + + if len(tokens) == 0 { + return opaqueTokens(cmd) + } + return tokens +} + +// isSubcommand reports whether w looks like a subcommand argument rather +// than a flag, path, or other option value. We restrict to lowercase alpha +// to avoid treating flags like "czf", build dirs like "build", or numeric +// versions like "1.2.3" as subcommands. Known subcommands like test, diff, +// run, get, show, status all match this pattern. +func isSubcommand(w string) bool { + // Not a flag. + if strings.HasPrefix(w, "-") { + return false + } + // Not a path. + if looksLikePath(w) { + return false + } + // Pure lowercase alpha only. This matches subcommand names like + // "test", "diff", "run", "get", "status" while rejecting + // concatenated flags like "czf", directories like "build", etc. + for _, r := range w { + if r < 'a' || r > 'z' { + return false + } + } + return true +} + +// isSafeWord reports whether w is a plain literal word without any shell +// expansions or special constructs. +func isSafeWord(w *syntax.Word) bool { + if w == nil { + return false + } + for _, part := range w.Parts { + switch part.(type) { + case *syntax.SglQuoted, *syntax.Lit, *syntax.DblQuoted: + // Safe. + default: + return false + } + } + return true +} + +// cmdText extracts the raw text of a word node for command/subcommand +// extraction. +func cmdText(w *syntax.Word) string { + var parts []string + for _, part := range w.Parts { + switch p := part.(type) { + case *syntax.Lit: + parts = append(parts, p.Value) + case *syntax.SglQuoted: + parts = append(parts, p.Value) + case *syntax.DblQuoted: + for _, dp := range p.Parts { + if l, ok := dp.(*syntax.Lit); ok { + parts = append(parts, l.Value) + } + } + } + } + return strings.Join(parts, "") +} + +// looksLikePath reports whether s looks like a filesystem path argument. +func looksLikePath(s string) bool { + if strings.Contains(s, "/") { + return true + } + if s == "." || s == ".." { + return true + } + if strings.HasPrefix(s, "./") || strings.HasPrefix(s, "../") { + return true + } + if strings.HasPrefix(s, "~/") { + return true + } + if strings.HasPrefix(s, "~") { + return true + } + return false +} + +// cleanPath resolves and cleans a path string against the working directory. +func cleanPath(pathStr, workingDir string) string { + // cd - refers to the previous working directory in bash — we can't + // model this, so we skip emitting a path token for it. + if pathStr == "-" { + return "" + } + switch { + case pathStr == ".": + return workingDir + case pathStr == "..": + return filepath.Clean(workingDir + "/..") + case strings.HasPrefix(pathStr, "~") && len(pathStr) > 1: + usr, err := user.Current() + if err != nil { + return pathStr + } + if pathStr[1] == '/' || pathStr[1] == 0 { + return filepath.Join(usr.HomeDir, pathStr[1:]) + } + // ~user format — we can't resolve arbitrary users, use as-is. + return pathStr + default: + if filepath.IsAbs(pathStr) { + return filepath.Clean(pathStr) + } + return filepath.Clean(filepath.Join(workingDir, pathStr)) + } +} diff --git a/internal/agent/tools/bashctx_test.go b/internal/agent/tools/bashctx_test.go new file mode 100644 index 0000000000..9aedb08ce2 --- /dev/null +++ b/internal/agent/tools/bashctx_test.go @@ -0,0 +1,475 @@ +package tools + +import ( + "os/user" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestAnalyzeCommand_SingleCommands(t *testing.T) { + wd := "/repo" + tests := []struct { + name string + cmd string + expect []string + expectOpaq bool // true if we expect an opaque command! token + }{ + { + name: "simple command", + cmd: "ls", + expect: []string{"command:ls"}, + }, + { + name: "command with subcommand", + cmd: "go test", + expect: []string{"command:go test"}, + }, + { + name: "command with subcommand and path arg", + cmd: "go test ./...", + expect: []string{"command:go test", "path:/repo/..."}, + }, + { + name: "git subcommand", + cmd: "git diff", + expect: []string{"command:git diff"}, + }, + { + name: "cd with relative dir (plain word becomes subcommand)", + cmd: "cd build", + expect: []string{"command:cd build"}, + }, + { + name: "cd with absolute dir", + cmd: "cd /tmp", + expect: []string{"command:cd", "path:/tmp"}, + }, + { + name: "cd with dot", + cmd: "cd .", + expect: []string{"command:cd", "path:/repo"}, + }, + { + name: "command with flag and path", + cmd: "head -n 10 /var/log/syslog", + expect: []string{"command:head", "path:/var/log/syslog"}, + }, + { + name: "command with multiple flags and path", + cmd: `grep -rn 'foo' src/file.txt`, + expect: []string{"command:grep", "path:/repo/src/file.txt"}, + }, + { + name: "tar command with flags and path", + cmd: "tar czf archive.tar.gz ./build", + expect: []string{"command:tar czf", "path:/repo/build"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := AnalyzeCommand(tt.cmd, wd) + if tt.expectOpaq { + require.Len(t, got, 1) + require.True(t, len(got[0]) > 9 && got[0][:9] == "command!:", + "expected opaque token, got %q", got[0]) + return + } + require.Equal(t, tt.expect, got) + }) + } +} + +func TestAnalyzeCommand_ChainCommands(t *testing.T) { + wd := "/repo" + + tests := []struct { + name string + cmd string + expect []string + }{ + { + name: "and chain", + cmd: "go test && git diff", + expect: []string{"command:go test", "command:git diff"}, + }, + { + name: "or chain", + cmd: "ls || echo 'not found'", + expect: []string{"command:ls", "command:echo"}, + }, + { + name: "semicolon chain", + cmd: "cd /tmp; pwd; cd -", + expect: []string{"command:cd", "command:pwd", "path:/tmp"}, + }, + { + name: "mixed chains", + cmd: "cd /tmp && pwd; ls /var", + expect: []string{"command:cd", "command:pwd", "command:ls", "path:/tmp", "path:/var"}, + }, + { + name: "chain with paths", + cmd: "cd /tmp && ls ./src", + expect: []string{"command:cd", "command:ls", "path:/tmp", "path:/repo/src"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := AnalyzeCommand(tt.cmd, wd) + require.ElementsMatch(t, tt.expect, got, "command: %s", tt.cmd) + }) + } +} + +func TestAnalyzeCommand_Pipeline(t *testing.T) { + wd := "/repo" + + tests := []struct { + name string + cmd string + expect []string + }{ + { + name: "simple pipe", + cmd: "ls | grep foo", + expect: []string{"command:ls", "command:grep foo"}, + }, + { + name: "double pipe", + cmd: "ls | grep foo | wc -l", + expect: []string{"command:ls", "command:grep foo", "command:wc"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := AnalyzeCommand(tt.cmd, wd) + require.ElementsMatch(t, tt.expect, got, "command: %s", tt.cmd) + }) + } +} + +func TestAnalyzeCommand_FailClosed(t *testing.T) { + wd := "/repo" + + tests := []struct { + name string + cmd string + }{ + { + name: "command substitution", + cmd: "echo $(pwd)", + }, + { + name: "backtick substitution", + cmd: "echo `pwd`", + }, + { + name: "redirect", + cmd: "ls > file.txt", + }, + { + name: "input redirect", + cmd: "grep foo < input.txt", + }, + { + name: "double redirect", + cmd: "cat < input.txt > output.txt", + }, + { + name: "stderr redirect", + cmd: "ls 2>&1", + }, + { + name: "sh -c", + cmd: "sh -c 'echo hello'", + }, + { + name: "bash -c", + cmd: "bash -c 'echo hello'", + }, + { + name: "eval", + cmd: "eval echo hi", + }, + { + name: "function declaration", + cmd: "func() { ls; }", + }, + { + name: "subshell grouping", + cmd: "(ls)", + }, + { + name: "braces grouping", + cmd: "{ ls; }", + }, + { + name: "for loop", + cmd: "for i in 1 2 3; do echo $i; done", + }, + { + name: "if statement", + cmd: "if [ -f foo ]; then echo yes; fi", + }, + { + name: "process substitution", + cmd: "diff <(ls dir1) <(ls dir2)", + }, + { + name: "nested subshell in double-quoted", + cmd: "sh -c 'echo $(pwd)'", + }, + { + name: "appending redirect", + cmd: "echo hello >> logfile", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := AnalyzeCommand(tt.cmd, wd) + require.Len(t, got, 1, "expected exactly one token for: %s", tt.cmd) + require.True(t, len(got[0]) > 9 && got[0][:9] == "command!:", + "expected opaque token, got %q for: %s", got[0], tt.cmd) + }) + } +} + +func TestAnalyzeCommand_PathExtraction(t *testing.T) { + wd := "/repo/project" + + tests := []struct { + name string + cmd string + expect []string + }{ + { + name: "absolute path", + cmd: "cat /etc/hosts", + expect: []string{"command:cat", "path:/etc/hosts"}, + }, + { + name: "relative path", + cmd: "cat src/main.go", + expect: []string{"command:cat", "path:/repo/project/src/main.go"}, + }, + { + name: "relative dot path", + cmd: "cat ./src/main.go", + expect: []string{"command:cat", "path:/repo/project/src/main.go"}, + }, + { + name: "dotdot relative path", + cmd: "cat ../other", + expect: []string{"command:cat", "path:/repo/other"}, + }, + { + name: "tilde path", + cmd: "cat ~/config", + expect: []string{"command:cat", "path:" + mustTildePath("~/config")}, + }, + { + name: "multiple paths", + cmd: "cp src/main.go dist/main.go", + expect: []string{"command:cp", "path:/repo/project/src/main.go", "path:/repo/project/dist/main.go"}, + }, + { + name: "path dedup", + cmd: "diff /tmp/x /tmp/x", + expect: []string{"command:diff", "path:/tmp/x"}, + }, + { + name: "command with path arg", + cmd: "ls /var/log", + expect: []string{"command:ls", "path:/var/log"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := AnalyzeCommand(tt.cmd, wd) + require.ElementsMatch(t, tt.expect, got, "command: %s", tt.cmd) + }) + } +} + +func mustTildePath(s string) string { + usr, err := user.Current() + if err != nil { + return s + } + if len(s) > 1 { + return filepath.Join(usr.HomeDir, s[1:]) + } + return usr.HomeDir +} + +func TestAnalyzeCommand_Deduplication(t *testing.T) { + wd := "/repo" + + tests := []struct { + name string + cmd string + expect []string + }{ + { + name: "duplicate command in chain", + cmd: "ls && ls", + expect: []string{"command:ls"}, + }, + { + name: "duplicate path in same command", + cmd: "diff /tmp/x /tmp/x", + expect: []string{"command:diff", "path:/tmp/x"}, + }, + { + name: "same command repeated in complex chain", + cmd: "go test ./a && go test ./b", + expect: []string{"command:go test", "path:/repo/a", "path:/repo/b"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := AnalyzeCommand(tt.cmd, wd) + // Verify no duplicates + seen := make(map[string]int) + for _, tok := range got { + seen[tok]++ + } + for tok, count := range seen { + require.Equal(t, 1, count, "duplicate token %q in %v", tok, got) + } + require.ElementsMatch(t, tt.expect, got) + }) + } +} + +func TestAnalyzeCommand_EdgeCases(t *testing.T) { + wd := "/repo" + + tests := []struct { + name string + cmd string + expect []string + }{ + { + name: "empty command", + cmd: "", + expect: []string{"command:"}, + }, + { + name: "only whitespace", + cmd: " ", + expect: []string{"command:"}, + }, + { + name: "quoted argument", + cmd: `echo "hello world"`, + expect: []string{"command:echo"}, + }, + { + name: "single quoted arg", + cmd: `echo 'hello world'`, + expect: []string{"command:echo"}, + }, + { + name: "double quoted path", + cmd: `cat "./src/main.go"`, + expect: []string{"command:cat", "path:/repo/src/main.go"}, + }, + { + name: "cd with - (previous dir, no path token)", + cmd: "cd -", + expect: []string{"command:cd"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := AnalyzeCommand(tt.cmd, wd) + require.Equal(t, tt.expect, got) + }) + } +} + +func TestAnalyzeCommand_WorkingDirResolution(t *testing.T) { + tests := []struct { + name string + workingDir string + cmd string + expected []string + }{ + { + name: "relative path resolved", + workingDir: "/a/b/c", + cmd: "cat ./foo.txt", + expected: []string{"command:cat", "path:/a/b/c/foo.txt"}, + }, + { + name: "absolute path unchanged", + workingDir: "/a/b/c", + cmd: "cat /etc/passwd", + expected: []string{"command:cat", "path:/etc/passwd"}, + }, + { + name: "cd with relative target", + workingDir: "/a/b/c", + cmd: "cd dist", + expected: []string{"command:cd dist"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := AnalyzeCommand(tt.cmd, tt.workingDir) + require.ElementsMatch(t, tt.expected, got) + }) + } +} + +func TestMakeCommandToken(t *testing.T) { + t.Parallel() + tests := []struct { + input string + want string + }{ + {"go", "command:go"}, + {"go test", "command:go test"}, + {"git diff", "command:git diff"}, + // Extra whitespace is normalised. + {" go test ", "command:go test"}, + {"kubectl get", "command:kubectl get"}, + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + t.Parallel() + require.Equal(t, tt.want, MakeCommandToken(tt.input)) + }) + } +} + +func TestMakePathToken(t *testing.T) { + t.Parallel() + tests := []struct { + input string + want string + }{ + {"/tmp", "path:/tmp"}, + {"/tmp/subdir", "path:/tmp/subdir"}, + // filepath.Clean is applied. + {"/tmp//subdir", "path:/tmp/subdir"}, + {"/tmp/subdir/..", "path:/tmp"}, + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + t.Parallel() + require.Equal(t, tt.want, MakePathToken(tt.input)) + }) + } +} diff --git a/internal/app/app.go b/internal/app/app.go index d8a3abc63b..0685a355a4 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -10,6 +10,7 @@ import ( "io" "log/slog" "os" + "path/filepath" "strings" "sync" "time" @@ -20,6 +21,7 @@ import ( "charm.land/lipgloss/v2" "github.com/charmbracelet/crush/internal/agent" "github.com/charmbracelet/crush/internal/agent/notify" + "github.com/charmbracelet/crush/internal/agent/tools" "github.com/charmbracelet/crush/internal/agent/tools/mcp" "github.com/charmbracelet/crush/internal/config" "github.com/charmbracelet/crush/internal/db" @@ -96,15 +98,27 @@ func New(ctx context.Context, conn *sql.DB, store *config.ConfigStore, skillsMgr cfg := store.Config() skipPermissionsRequests := store.Overrides().SkipPermissionRequests var allowedTools []string - if cfg.Permissions != nil && cfg.Permissions.AllowedTools != nil { - allowedTools = cfg.Permissions.AllowedTools + var allowedContexts []string + if cfg.Permissions != nil { + if cfg.Permissions.AllowedTools != nil { + allowedTools = cfg.Permissions.AllowedTools + } + for _, cmd := range cfg.Permissions.AllowedCommands { + allowedContexts = append(allowedContexts, tools.MakeCommandToken(cmd)) + } + for _, p := range cfg.Permissions.AllowedPaths { + if !filepath.IsAbs(p) { + p = filepath.Join(store.WorkingDir(), p) + } + allowedContexts = append(allowedContexts, tools.MakePathToken(p)) + } } app := &App{ Sessions: sessions, Messages: messages, History: files, - Permissions: permission.NewPermissionService(store.WorkingDir(), skipPermissionsRequests, allowedTools), + Permissions: permission.NewPermissionService(store.WorkingDir(), skipPermissionsRequests, allowedTools, allowedContexts), FileTracker: filetracker.NewService(q), LSPManager: lsp.NewManager(store), Skills: skillsMgr, diff --git a/internal/app/testing.go b/internal/app/testing.go index 1722e2b154..840d8dcb20 100644 --- a/internal/app/testing.go +++ b/internal/app/testing.go @@ -28,7 +28,7 @@ import ( // tear down the fan-in goroutines and the events broker. func NewForTest(ctx context.Context) *App { app := &App{ - Permissions: permission.NewPermissionService("", false, nil), + Permissions: permission.NewPermissionService("", false, nil, nil), globalCtx: ctx, events: pubsub.NewBroker[tea.Msg](), serviceEventsWG: &sync.WaitGroup{}, diff --git a/internal/config/config.go b/internal/config/config.go index fc3bab3302..10d846edb5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -232,7 +232,9 @@ func (c Completions) Limits() (depth, items int) { } type Permissions struct { - AllowedTools []string `json:"allowed_tools,omitempty" jsonschema:"description=List of tools that don't require permission prompts,example=bash,example=view"` + AllowedTools []string `json:"allowed_tools,omitempty" jsonschema:"description=List of tools that don't require permission prompts,example=bash,example=view"` + AllowedCommands []string `json:"allowed_commands,omitempty" jsonschema:"description=List of shell commands that do not require permission prompts,example=go test,example=git diff"` + AllowedPaths []string `json:"allowed_paths,omitempty" jsonschema:"description=List of filesystem paths that do not require permission prompts; subpaths are also auto-approved,example=/tmp,example=/home/user/projects"` } type TrailerStyle string diff --git a/internal/permission/context.go b/internal/permission/context.go new file mode 100644 index 0000000000..4530756c1c --- /dev/null +++ b/internal/permission/context.go @@ -0,0 +1,45 @@ +package permission + +import ( + "path/filepath" + "strings" +) + +// tokenSatisfies reports whether a stored permission token is broad enough to +// satisfy a request for a candidate token. +// +// Matching rules by token kind: +// +// - command: satisfies command: iff B == A (exact) or B starts with +// A followed by a space. This allows command:go to satisfy command:go +// test but not command:golang, because the word boundary (space) prevents +// partial-word prefix matches. +// +// - path: satisfies path: iff B == A (exact) or B starts with A +// followed by the OS path separator. This allows path:/tmp to satisfy +// path:/tmp/subdir but not path:/tmpfiles. +// +// - All other tokens (including opaque command!: tokens): exact match only. +func tokenSatisfies(stored, candidate string) bool { + if stored == candidate { + return true + } + switch { + case strings.HasPrefix(stored, "command:") && strings.HasPrefix(candidate, "command:"): + s := strings.TrimPrefix(stored, "command:") + c := strings.TrimPrefix(candidate, "command:") + // Word-boundary prefix: stored "go" satisfies "go test" but not "golang". + return strings.HasPrefix(c, s+" ") + case strings.HasPrefix(stored, "path:") && strings.HasPrefix(candidate, "path:"): + s := filepath.Clean(strings.TrimPrefix(stored, "path:")) + c := filepath.Clean(strings.TrimPrefix(candidate, "path:")) + // Root path is a parent of every absolute path. + if s == "/" { + return strings.HasPrefix(c, "/") + } + // Directory-boundary prefix: stored "/tmp" satisfies "/tmp/subdir" but not "/tmpfiles". + return strings.HasPrefix(c, s+string(filepath.Separator)) + default: + return false + } +} diff --git a/internal/permission/context_test.go b/internal/permission/context_test.go new file mode 100644 index 0000000000..d6f5ea2c90 --- /dev/null +++ b/internal/permission/context_test.go @@ -0,0 +1,68 @@ +package permission + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestTokenSatisfies(t *testing.T) { + t.Parallel() + tests := []struct { + stored string + candidate string + want bool + }{ + // Exact matches always satisfy. + {"command:go", "command:go", true}, + {"path:/tmp", "path:/tmp", true}, + {"command!:sh -c echo", "command!:sh -c echo", true}, + + // command: word-boundary prefix — stored is base command, candidate adds subcommand. + {"command:go", "command:go test", true}, + {"command:go", "command:go build", true}, + {"command:kubectl", "command:kubectl get", true}, + {"command:kubectl get", "command:kubectl get pods", true}, + + // command: must not match a different word. + {"command:py", "command:python3", false}, + {"command:go", "command:golang", false}, + {"command:git", "command:gitk", false}, + + // command: more-specific does not satisfy less-specific. + {"command:go test", "command:go", false}, + + // path: root / is parent of all absolute paths. + {"path:/", "path:/etc/hosts", true}, + {"path:/", "path:/tmp/subdir/file.txt", true}, + // path:/ matches path:/ exactly (handled by the == branch above). + {"path:/", "path:/", true}, + + // path: directory-boundary prefix — stored is parent dir, candidate is child. + {"path:/tmp", "path:/tmp/subdir", true}, + {"path:/tmp", "path:/tmp/subdir/deep", true}, + {"path:/home/user", "path:/home/user/projects/repo", true}, + + // path: must not match a sibling with same prefix word. + {"path:/tmp", "path:/tmpfiles", false}, + {"path:/home/user", "path:/home/users", false}, + + // path: more-specific does not satisfy less-specific. + {"path:/tmp/subdir", "path:/tmp", false}, + + // Opaque command!: tokens — exact match only, no prefix extension. + {"command!:sh -c echo", "command!:sh -c echo2", false}, + {"command!:sh -c echo", "command:echo", false}, + + // Mixed kinds never match each other. + {"command:go", "path:/usr/local/go", false}, + {"path:/tmp", "command:rm", false}, + } + + for _, tt := range tests { + t.Run(tt.stored+"→"+tt.candidate, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, tokenSatisfies(tt.stored, tt.candidate)) + }) + } +} diff --git a/internal/permission/permission.go b/internal/permission/permission.go index 25e4994a80..20ef4c3739 100644 --- a/internal/permission/permission.go +++ b/internal/permission/permission.go @@ -36,13 +36,14 @@ func hookApproved(ctx context.Context, toolCallID string) bool { } type CreatePermissionRequest struct { - SessionID string `json:"session_id"` - ToolCallID string `json:"tool_call_id"` - ToolName string `json:"tool_name"` - Description string `json:"description"` - Action string `json:"action"` - Params any `json:"params"` - Path string `json:"path"` + SessionID string `json:"session_id"` + ToolCallID string `json:"tool_call_id"` + ToolName string `json:"tool_name"` + Description string `json:"description"` + Action string `json:"action"` + Params any `json:"params"` + Path string `json:"path"` + Contexts []string `json:"contexts,omitempty"` } type PermissionNotification struct { @@ -52,14 +53,19 @@ type PermissionNotification struct { } type PermissionRequest struct { - ID string `json:"id"` - SessionID string `json:"session_id"` - ToolCallID string `json:"tool_call_id"` - ToolName string `json:"tool_name"` - Description string `json:"description"` - Action string `json:"action"` - Params any `json:"params"` - Path string `json:"path"` + ID string `json:"id"` + SessionID string `json:"session_id"` + ToolCallID string `json:"tool_call_id"` + ToolName string `json:"tool_name"` + Description string `json:"description"` + Action string `json:"action"` + Params any `json:"params"` + Path string `json:"path"` + Contexts []string `json:"contexts,omitempty"` + // PendingContexts is the subset of Contexts that are not yet satisfied by + // config-level allowances or prior session grants. The dialog uses this to + // show only the tokens the user is actually being asked to approve. + PendingContexts []string `json:"pending_contexts,omitempty"` } type Service interface { @@ -90,6 +96,7 @@ type PermissionKey struct { ToolName string Action string Path string + Context string // generic context token; empty for legacy tool grants } type permissionService struct { @@ -103,6 +110,7 @@ type permissionService struct { autoApproveSessionsMu sync.RWMutex skip atomic.Bool allowedTools []string + allowedContexts []string // used to make sure we only process one request at a time requestMu sync.Mutex @@ -161,6 +169,24 @@ func (s *permissionService) GrantPersistent(permission PermissionRequest) bool { // lost to a Deny would still leave an auto-approve entry behind, // silently flipping later denied calls to allowed. return s.resolve(permission, true, false, func() { + // If Contexts is non-empty, record one key per context token. + // The Path field is intentionally omitted from contextual grant + // keys — location semantics are captured by path: tokens, not + // the working directory at approval time. + if len(permission.Contexts) > 0 { + for _, ctx := range permission.Contexts { + s.sessionPermissions.Set(PermissionKey{ + SessionID: permission.SessionID, + ToolName: permission.ToolName, + Action: permission.Action, + Context: ctx, + }, true) + } + return + } + + // Otherwise record the legacy key for backward compatibility + // with tools that don't use contextual approval (edit, write, view, etc.) s.sessionPermissions.Set(PermissionKey{ SessionID: permission.SessionID, ToolName: permission.ToolName, @@ -179,20 +205,18 @@ func (s *permissionService) Deny(permission PermissionRequest) bool { } func (s *permissionService) Request(ctx context.Context, opts CreatePermissionRequest) (bool, error) { + // 1. Skip mode → allow if s.skip.Load() { return true, nil } - // Check if the tool/action combination is in the allowlist + // 2. Tool / tool:action allowlist → allow commandKey := opts.ToolName + ":" + opts.Action if slices.Contains(s.allowedTools, commandKey) || slices.Contains(s.allowedTools, opts.ToolName) { return true, nil } - // A PreToolUse hook that returned decision=allow stamps the context - // with the tool call ID. Treat that as a pre-approval and skip the - // prompt entirely. We still publish a granted notification so the UI - // and audit subscribers see the outcome. + // 3. Hook approval (context-stamped) → allow if hookApproved(ctx, opts.ToolCallID) { s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{ ToolCallID: opts.ToolCallID, @@ -209,6 +233,7 @@ func (s *permissionService) Request(ctx context.Context, opts CreatePermissionRe ToolCallID: opts.ToolCallID, }) + // 4. Auto-approve session → allow s.autoApproveSessionsMu.RLock() autoApprove := s.autoApproveSessions[opts.SessionID] s.autoApproveSessionsMu.RUnlock() @@ -243,19 +268,47 @@ func (s *permissionService) Request(ctx context.Context, opts CreatePermissionRe Description: opts.Description, Action: opts.Action, Params: opts.Params, + Contexts: opts.Contexts, } - if _, ok := s.sessionPermissions.Get(PermissionKey{ - SessionID: permission.SessionID, - ToolName: permission.ToolName, - Action: permission.Action, - Path: permission.Path, - }); ok { - s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{ - ToolCallID: opts.ToolCallID, - Granted: true, - }) - return true, nil + // 5. Contextual approval: if len(Contexts) > 0 and every context is + // satisfied → allow. Matching uses structured token prefix semantics + // (see tokenSatisfies in context.go). + // + // Also compute PendingContexts — the subset that is NOT yet satisfied — + // so the UI can display only what the user is actually being asked to + // approve, omitting tokens already covered by prior grants. + if len(permission.Contexts) > 0 { + allApproved := true + for _, ctx := range permission.Contexts { + if !s.contextSatisfied(permission.SessionID, permission.ToolName, permission.Action, ctx) { + allApproved = false + permission.PendingContexts = append(permission.PendingContexts, ctx) + } + } + if allApproved { + s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{ + ToolCallID: opts.ToolCallID, + Granted: true, + }) + return true, nil + } + } + + // 6. Legacy session approval: if len(Contexts) == 0 → allow + if len(permission.Contexts) == 0 { + if _, ok := s.sessionPermissions.Get(PermissionKey{ + SessionID: permission.SessionID, + ToolName: permission.ToolName, + Action: permission.Action, + Path: permission.Path, + }); ok { + s.notificationBroker.Publish(pubsub.CreatedEvent, PermissionNotification{ + ToolCallID: opts.ToolCallID, + Granted: true, + }) + return true, nil + } } s.activeRequestMu.Lock() @@ -295,7 +348,45 @@ func (s *permissionService) SkipRequests() bool { return s.skip.Load() } -func NewPermissionService(workingDir string, skip bool, allowedTools []string) Service { +// contextSatisfied reports whether the given context token is satisfied by a +// config-level allowed token or by a previously stored session grant. Config +// tokens are checked first (fast path), followed by an exact key lookup, and +// finally a structured prefix scan over all session grants. +func (s *permissionService) contextSatisfied(sessionID, toolName, action, candidate string) bool { + // Config-level allowed contexts (e.g. from allowed_commands): check + // whether any configured token is broad enough to satisfy the candidate. + for _, allowed := range s.allowedContexts { + if tokenSatisfies(allowed, candidate) { + return true + } + } + + // Fast path: exact match on stored contextual grant key. + if _, ok := s.sessionPermissions.Get(PermissionKey{ + SessionID: sessionID, + ToolName: toolName, + Action: action, + Context: candidate, + }); ok { + return true + } + + // Structured prefix match: iterate all contextual grants for this + // session/tool/action and check whether any stored token is broad + // enough to satisfy the candidate (e.g. command:go satisfies + // command:go test; path:/tmp satisfies path:/tmp/subdir). + for key := range s.sessionPermissions.Seq2() { + if key.SessionID != sessionID || key.ToolName != toolName || key.Action != action || key.Context == "" { + continue + } + if tokenSatisfies(key.Context, candidate) { + return true + } + } + return false +} + +func NewPermissionService(workingDir string, skip bool, allowedTools []string, allowedContexts []string) Service { svc := &permissionService{ Broker: pubsub.NewBroker[PermissionRequest](), notificationBroker: pubsub.NewBroker[PermissionNotification](), @@ -303,6 +394,7 @@ func NewPermissionService(workingDir string, skip bool, allowedTools []string) S sessionPermissions: csync.NewMap[PermissionKey, bool](), autoApproveSessions: make(map[string]bool), allowedTools: allowedTools, + allowedContexts: allowedContexts, pendingRequests: csync.NewMap[string, chan bool](), } svc.skip.Store(skip) diff --git a/internal/permission/permission_test.go b/internal/permission/permission_test.go index 9d464f7a0c..8b73a5697b 100644 --- a/internal/permission/permission_test.go +++ b/internal/permission/permission_test.go @@ -1,6 +1,7 @@ package permission import ( + "context" "sync" "sync/atomic" "testing" @@ -57,7 +58,7 @@ func TestPermissionService_AllowedCommands(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - service := NewPermissionService("/tmp", false, tt.allowedTools) + service := NewPermissionService("/tmp", false, tt.allowedTools, nil) // Create a channel to capture the permission request // Since we're testing the allowlist logic, we need to simulate the request @@ -82,7 +83,7 @@ func TestPermissionService_AllowedCommands(t *testing.T) { } func TestSkipRace(t *testing.T) { - svc := NewPermissionService("/tmp", false, nil) + svc := NewPermissionService("/tmp", false, nil, nil) var wg sync.WaitGroup wg.Add(2) go func() { @@ -97,7 +98,7 @@ func TestSkipRace(t *testing.T) { } func TestPermissionService_SkipMode(t *testing.T) { - service := NewPermissionService("/tmp", true, []string{}) + service := NewPermissionService("/tmp", true, []string{}, nil) result, err := service.Request(t.Context(), CreatePermissionRequest{ SessionID: "test-session", @@ -119,7 +120,7 @@ func TestPermissionService_HookApproval(t *testing.T) { t.Run("matching tool call ID short-circuits the prompt", func(t *testing.T) { t.Parallel() - service := NewPermissionService("/tmp", false, nil) + service := NewPermissionService("/tmp", false, nil, nil) ctx := WithHookApproval(t.Context(), "call-42") granted, err := service.Request(ctx, CreatePermissionRequest{ @@ -136,7 +137,7 @@ func TestPermissionService_HookApproval(t *testing.T) { t.Run("approval is scoped to the stamped tool call ID", func(t *testing.T) { t.Parallel() - service := NewPermissionService("/tmp", false, nil) + service := NewPermissionService("/tmp", false, nil, nil) // Stamp for call-42, ask for a different call ID — must not leak. ctx := WithHookApproval(t.Context(), "call-42") @@ -169,7 +170,7 @@ func TestPermissionService_HookApproval(t *testing.T) { t.Run("notifies subscribers that permission was granted", func(t *testing.T) { t.Parallel() - service := NewPermissionService("/tmp", false, nil) + service := NewPermissionService("/tmp", false, nil, nil) notifications := service.SubscribeNotifications(t.Context()) @@ -192,7 +193,7 @@ func TestPermissionService_HookApproval(t *testing.T) { func TestPermissionService_SequentialProperties(t *testing.T) { t.Run("Sequential permission requests with persistent grants", func(t *testing.T) { - service := NewPermissionService("/tmp", false, []string{}) + service := NewPermissionService("/tmp", false, []string{}, nil) req1 := CreatePermissionRequest{ SessionID: "session1", @@ -237,7 +238,7 @@ func TestPermissionService_SequentialProperties(t *testing.T) { assert.True(t, result2, "Second request should be auto-approved") }) t.Run("Sequential requests with temporary grants", func(t *testing.T) { - service := NewPermissionService("/tmp", false, []string{}) + service := NewPermissionService("/tmp", false, []string{}, nil) req := CreatePermissionRequest{ SessionID: "session2", @@ -277,7 +278,7 @@ func TestPermissionService_SequentialProperties(t *testing.T) { assert.False(t, result2, "Second request should be denied") }) t.Run("Concurrent requests with different outcomes", func(t *testing.T) { - service := NewPermissionService("/tmp", false, []string{}) + service := NewPermissionService("/tmp", false, []string{}, nil) events := service.Subscribe(t.Context()) @@ -354,7 +355,7 @@ func TestPermissionService_ResolveIdempotency(t *testing.T) { t.Run("concurrent grants resolve exactly once", func(t *testing.T) { t.Parallel() - service := NewPermissionService("/tmp", false, nil) + service := NewPermissionService("/tmp", false, nil, nil) events := service.Subscribe(t.Context()) notifications := service.SubscribeNotifications(t.Context()) @@ -450,7 +451,7 @@ func TestPermissionService_ResolveIdempotency(t *testing.T) { t.Run("grant after deny is a no-op", func(t *testing.T) { t.Parallel() - service := NewPermissionService("/tmp", false, nil) + service := NewPermissionService("/tmp", false, nil, nil) events := service.Subscribe(t.Context()) notifications := service.SubscribeNotifications(t.Context()) @@ -512,7 +513,7 @@ func TestPermissionService_ResolveIdempotency(t *testing.T) { t.Run("losing GrantPersistent does not record session permission", func(t *testing.T) { t.Parallel() - service := NewPermissionService("/tmp", false, nil) + service := NewPermissionService("/tmp", false, nil, nil) events := service.Subscribe(t.Context()) notifications := service.SubscribeNotifications(t.Context()) @@ -584,7 +585,7 @@ func TestPermissionService_ResolveIdempotency(t *testing.T) { t.Run("grant for unknown id is a safe no-op", func(t *testing.T) { t.Parallel() - service := NewPermissionService("/tmp", false, nil) + service := NewPermissionService("/tmp", false, nil, nil) notifications := service.SubscribeNotifications(t.Context()) @@ -613,3 +614,1179 @@ func TestPermissionService_ResolveIdempotency(t *testing.T) { } }) } + +// ============================================================================= +// Phase 2 Tests: Generic Contextual Permissions +// ============================================================================= + +func TestPermissionService_ContextualAutoApprove(t *testing.T) { + t.Parallel() + + t.Run("context-less request falls back to legacy key (no regression)", func(t *testing.T) { + t.Parallel() + service := NewPermissionService("/tmp", false, nil, nil) + + // First request: no contexts (legacy tool) + events := service.Subscribe(t.Context()) + notifications := service.SubscribeNotifications(t.Context()) + + req := CreatePermissionRequest{ + SessionID: "session-legacy", + ToolCallID: "legacy-call", + ToolName: "edit", + Action: "create", + Description: "Edit a file", + Path: "/tmp/test.txt", + } + + var wg sync.WaitGroup + var granted bool + var err error + wg.Go(func() { + granted, err = service.Request(t.Context(), req) + }) + + var pending PermissionRequest + select { + case ev := <-events: + pending = ev.Payload + case <-time.After(2 * time.Second): + t.Fatal("request was never published") + } + + <-notifications + assert.True(t, service.GrantPersistent(pending)) + + wg.Wait() + require.NoError(t, err) + assert.True(t, granted, "first legacy request should be granted") + + // Second identical request should auto-approve via legacy key + req2 := CreatePermissionRequest{ + SessionID: "session-legacy", + ToolCallID: "legacy-call-2", + ToolName: "edit", + Action: "create", + Description: "Edit same file again", + Path: "/tmp/test.txt", + } + + result2, err2 := service.Request(t.Context(), req2) + require.NoError(t, err2) + assert.True(t, result2, "second legacy request with same key should auto-approve") + }) + + t.Run("all contexts granted → auto-approve", func(t *testing.T) { + t.Parallel() + service := NewPermissionService("/tmp", false, nil, nil) + + // Grant a request with contexts + events := service.Subscribe(t.Context()) + notifications := service.SubscribeNotifications(t.Context()) + + req := CreatePermissionRequest{ + SessionID: "session-cmd", + ToolCallID: "cmd-call", + ToolName: "bash", + Action: "execute", + Description: "Run command chain", + Path: "/tmp", + Contexts: []string{"command:cd", "command:ls", "command:pwd", "path:/tmp"}, + } + + var wg sync.WaitGroup + var granted bool + var reqErr error + wg.Go(func() { + granted, reqErr = service.Request(t.Context(), req) + }) + + var pending PermissionRequest + select { + case ev := <-events: + pending = ev.Payload + case <-time.After(2 * time.Second): + t.Fatal("request was never published") + } + + <-notifications + assert.True(t, service.GrantPersistent(pending)) + + wg.Wait() + require.NoError(t, reqErr) + assert.True(t, granted) + + // Verify each context was recorded as a separate key. + // Contextual grants intentionally omit Path — location semantics + // are captured by path: context tokens instead. + for _, ctx := range req.Contexts { + key := PermissionKey{ + SessionID: pending.SessionID, + ToolName: pending.ToolName, + Action: pending.Action, + Context: ctx, + } + allowed, ok := service.(*permissionService).sessionPermissions.Get(key) + assert.True(t, ok, "context %q should be in sessionPermissions", ctx) + assert.True(t, allowed, "context %q should be approved", ctx) + } + + // Follow-up with same contexts should auto-approve + req2 := CreatePermissionRequest{ + SessionID: "session-cmd", + ToolCallID: "cmd-call-2", + ToolName: "bash", + Action: "execute", + Description: "Same command chain", + Path: "/tmp", + Contexts: []string{"command:cd", "command:ls", "command:pwd", "path:/tmp"}, + } + + result2, err2 := service.Request(t.Context(), req2) + require.NoError(t, err2) + assert.True(t, result2, "follow-up with all contexts should auto-approve") + }) + + t.Run("any context missing → prompt", func(t *testing.T) { + t.Parallel() + service := NewPermissionService("/tmp", false, nil, nil) + + // Grant a request with 3 contexts + events := service.Subscribe(t.Context()) + notifications := service.SubscribeNotifications(t.Context()) + + req := CreatePermissionRequest{ + SessionID: "session-partial", + ToolCallID: "partial-call", + ToolName: "bash", + Action: "execute", + Description: "Partial command chain", + Path: "/tmp", + Contexts: []string{"command:cd", "command:ls", "path:/tmp"}, + } + + var wg sync.WaitGroup + var granted bool + var reqErr error + wg.Go(func() { + granted, reqErr = service.Request(t.Context(), req) + }) + + var pending PermissionRequest + select { + case ev := <-events: + pending = ev.Payload + case <-time.After(2 * time.Second): + t.Fatal("request was never published") + } + + <-notifications + assert.True(t, service.GrantPersistent(pending)) + + wg.Wait() + require.NoError(t, reqErr) + assert.True(t, granted) + + // Follow-up with missing "command:ls" → should NOT auto-approve + req2 := CreatePermissionRequest{ + SessionID: "session-partial", + ToolCallID: "partial-call-2", + ToolName: "bash", + Action: "execute", + Description: "Missing one context", + Path: "/tmp", + Contexts: []string{"command:cd", "command:pwd", "path:/tmp"}, + } + + // Note: "command:pwd" was NOT granted — it was not in the original + events2 := service.Subscribe(t.Context()) + notifications2 := service.SubscribeNotifications(t.Context()) + + var wg2 sync.WaitGroup + var granted2 bool + var reqErr2 error + wg2.Go(func() { + granted2, reqErr2 = service.Request(t.Context(), req2) + }) + + // Should still publish a request (not auto-approve) + select { + case ev := <-events2: + assert.Equal(t, pending.SessionID, ev.Payload.SessionID) + // Drain the request + <-notifications2 + service.Deny(ev.Payload) + case <-time.After(2 * time.Second): + t.Fatal("request with missing context should have been published") + } + + wg2.Wait() + require.NoError(t, reqErr2) + assert.False(t, granted2, "request with missing context should NOT auto-approve") + }) +} + +func TestPermissionService_ContextualRecordAndReuse(t *testing.T) { + t.Parallel() + + t.Run("GrantPersistent records every context", func(t *testing.T) { + t.Parallel() + service := NewPermissionService("/tmp", false, nil, nil) + + events := service.Subscribe(t.Context()) + notifications := service.SubscribeNotifications(t.Context()) + + contexts := []string{"command:go", "command:build", "path:/repo", "path:/repo/cmd"} + + req := CreatePermissionRequest{ + SessionID: "session-record", + ToolCallID: "record-call", + ToolName: "bash", + Action: "execute", + Description: "Go build", + Path: "/repo", + Contexts: contexts, + } + + var wg sync.WaitGroup + var granted bool + var reqErr error + wg.Go(func() { + granted, reqErr = service.Request(t.Context(), req) + }) + + var pending PermissionRequest + select { + case ev := <-events: + pending = ev.Payload + case <-time.After(2 * time.Second): + t.Fatal("request was never published") + } + + <-notifications + assert.True(t, service.GrantPersistent(pending)) + + wg.Wait() + require.NoError(t, reqErr) + assert.True(t, granted) + + // Verify all contexts are individually recorded. + // Contextual grants intentionally omit Path — location semantics + // are captured by path: context tokens instead. + for _, ctx := range contexts { + key := PermissionKey{ + SessionID: pending.SessionID, + ToolName: pending.ToolName, + Action: pending.Action, + Context: ctx, + } + val, ok := service.(*permissionService).sessionPermissions.Get(key) + assert.True(t, ok, "context %q should be recorded", ctx) + assert.True(t, val, "context %q should be approved", ctx) + } + }) + + t.Run("subsequent isolated requests auto-approve", func(t *testing.T) { + t.Parallel() + service := NewPermissionService("/tmp", false, nil, nil) + + // Grant a chain request + events := service.Subscribe(t.Context()) + notifications := service.SubscribeNotifications(t.Context()) + + chainReq := CreatePermissionRequest{ + SessionID: "session-isolated", + ToolCallID: "chain-call", + ToolName: "bash", + Action: "execute", + Description: "Chain", + Path: "/tmp", + Contexts: []string{"command:cd", "command:ls", "path:/tmp"}, + } + + var wg sync.WaitGroup + var granted bool + var reqErr error + wg.Go(func() { + granted, reqErr = service.Request(t.Context(), chainReq) + }) + + var pending PermissionRequest + select { + case ev := <-events: + pending = ev.Payload + case <-time.After(2 * time.Second): + t.Fatal("request was never published") + } + + <-notifications + assert.True(t, service.GrantPersistent(pending)) + + wg.Wait() + require.NoError(t, reqErr) + assert.True(t, granted) + + // Now each individual context should auto-approve + testCases := []CreatePermissionRequest{ + { + SessionID: "session-isolated", + ToolCallID: "isolated-call-1", + ToolName: "bash", + Action: "execute", + Description: "cd only", + Path: "/tmp", + Contexts: []string{"command:cd"}, + }, + { + SessionID: "session-isolated", + ToolCallID: "isolated-call-2", + ToolName: "bash", + Action: "execute", + Description: "ls only", + Path: "/tmp", + Contexts: []string{"command:ls"}, + }, + { + SessionID: "session-isolated", + ToolCallID: "isolated-call-3", + ToolName: "bash", + Action: "execute", + Description: "path only", + Path: "/tmp", + Contexts: []string{"path:/tmp"}, + }, + } + + for i, tc := range testCases { + t.Run(tc.Description, func(t *testing.T) { + result, err := service.Request(t.Context(), tc) + require.NoError(t, err) + assert.True(t, result, "isolated context %d should auto-approve", i+1) + }) + } + }) + + t.Run("recombined requests auto-approve", func(t *testing.T) { + t.Parallel() + service := NewPermissionService("/tmp", false, nil, nil) + + // Grant a chain with 3 contexts + events := service.Subscribe(t.Context()) + notifications := service.SubscribeNotifications(t.Context()) + + chainReq := CreatePermissionRequest{ + SessionID: "session-recombine", + ToolCallID: "chain-recombine", + ToolName: "bash", + Action: "execute", + Description: "Chain", + Path: "/tmp", + Contexts: []string{"command:cd", "command:ls", "command:pwd"}, + } + + var wg sync.WaitGroup + var granted bool + var reqErr error + wg.Go(func() { + granted, reqErr = service.Request(t.Context(), chainReq) + }) + + var pending PermissionRequest + select { + case ev := <-events: + pending = ev.Payload + case <-time.After(2 * time.Second): + t.Fatal("request was never published") + } + + <-notifications + assert.True(t, service.GrantPersistent(pending)) + + wg.Wait() + require.NoError(t, reqErr) + assert.True(t, granted) + + // Now recombine a subset — should auto-approve + recombinedReq := CreatePermissionRequest{ + SessionID: "session-recombine", + ToolCallID: "recombined-call", + ToolName: "bash", + Action: "execute", + Description: "Recombined", + Path: "/tmp", + Contexts: []string{"command:cd", "command:pwd"}, + } + + // Note: "command:ls" is not in this request, so it's fine + result, err := service.Request(t.Context(), recombinedReq) + require.NoError(t, err) + assert.True(t, result, "recombined subset of contexts should auto-approve") + }) +} + +func TestPermissionService_ContextualConservativeApproval(t *testing.T) { + t.Parallel() + + t.Run("different command with same path does not auto-approve", func(t *testing.T) { + t.Parallel() + service := NewPermissionService("/tmp", false, nil, nil) + + events := service.Subscribe(t.Context()) + notifications := service.SubscribeNotifications(t.Context()) + + req := CreatePermissionRequest{ + SessionID: "session-conservative", + ToolCallID: "conservative-call", + ToolName: "bash", + Action: "execute", + Description: "ls /tmp", + Path: "/tmp", + Contexts: []string{"command:ls", "path:/tmp"}, + } + + var wg sync.WaitGroup + var granted bool + var reqErr error + wg.Go(func() { + granted, reqErr = service.Request(t.Context(), req) + }) + + var pending PermissionRequest + select { + case ev := <-events: + pending = ev.Payload + case <-time.After(2 * time.Second): + t.Fatal("request was never published") + } + + <-notifications + assert.True(t, service.GrantPersistent(pending)) + + wg.Wait() + require.NoError(t, reqErr) + assert.True(t, granted) + + // Different command: "command:cd" was NOT granted + req2 := CreatePermissionRequest{ + SessionID: "session-conservative", + ToolCallID: "conservative-call-2", + ToolName: "bash", + Action: "execute", + Description: "cd /tmp (should NOT auto-approve)", + Path: "/tmp", + Contexts: []string{"command:cd", "path:/tmp"}, + } + + events2 := service.Subscribe(t.Context()) + notifications2 := service.SubscribeNotifications(t.Context()) + + var wg2 sync.WaitGroup + var granted2 bool + var reqErr2 error + wg2.Go(func() { + granted2, reqErr2 = service.Request(t.Context(), req2) + }) + + select { + case ev := <-events2: + // Should have published (not auto-approved) + assert.Equal(t, pending.SessionID, ev.Payload.SessionID) + <-notifications2 + service.Deny(ev.Payload) + case <-time.After(2 * time.Second): + t.Fatal("different command should NOT auto-approve") + } + + wg2.Wait() + require.NoError(t, reqErr2) + assert.False(t, granted2, "different command with same path should NOT auto-approve") + }) + + t.Run("same command with different path does not auto-approve", func(t *testing.T) { + t.Parallel() + service := NewPermissionService("/tmp", false, nil, nil) + + events := service.Subscribe(t.Context()) + notifications := service.SubscribeNotifications(t.Context()) + + req := CreatePermissionRequest{ + SessionID: "session-path", + ToolCallID: "path-call", + ToolName: "bash", + Action: "execute", + Description: "ls /tmp", + Path: "/tmp", + Contexts: []string{"command:ls", "path:/tmp"}, + } + + var wg sync.WaitGroup + var granted bool + var reqErr error + wg.Go(func() { + granted, reqErr = service.Request(t.Context(), req) + }) + + var pending PermissionRequest + select { + case ev := <-events: + pending = ev.Payload + case <-time.After(2 * time.Second): + t.Fatal("request was never published") + } + + <-notifications + assert.True(t, service.GrantPersistent(pending)) + + wg.Wait() + require.NoError(t, reqErr) + assert.True(t, granted) + + // Different path: "path:/other" was NOT granted + req2 := CreatePermissionRequest{ + SessionID: "session-path", + ToolCallID: "path-call-2", + ToolName: "bash", + Action: "execute", + Description: "ls /other (should NOT auto-approve)", + Path: "/other", + Contexts: []string{"command:ls", "path:/other"}, + } + + events2 := service.Subscribe(t.Context()) + _ = service.SubscribeNotifications(t.Context()) + + var wg2 sync.WaitGroup + var granted2 bool + var reqErr2 error + wg2.Go(func() { + granted2, reqErr2 = service.Request(t.Context(), req2) + }) + + select { + case ev := <-events2: + // Should have published (not auto-approved) + service.Deny(ev.Payload) + case <-time.After(2 * time.Second): + t.Fatal("same command with different path should NOT auto-approve") + } + + wg2.Wait() + require.NoError(t, reqErr2) + assert.False(t, granted2, "same command with different path should NOT auto-approve") + }) +} + +func TestPermissionService_ContextualRace(t *testing.T) { + t.Parallel() + + t.Run("concurrent context grants resolve exactly once", func(t *testing.T) { + t.Parallel() + service := NewPermissionService("/tmp", false, nil, nil) + + events := service.Subscribe(t.Context()) + notifications := service.SubscribeNotifications(t.Context()) + + req := CreatePermissionRequest{ + SessionID: "session-race-ctx", + ToolCallID: "race-ctx-call", + ToolName: "bash", + Action: "execute", + Description: "Race test", + Path: "/tmp", + Contexts: []string{"command:cd", "command:ls"}, + } + + var wg sync.WaitGroup + var granted bool + var reqErr error + wg.Go(func() { + granted, reqErr = service.Request(t.Context(), req) + }) + + var pending PermissionRequest + select { + case ev := <-events: + pending = ev.Payload + case <-time.After(2 * time.Second): + t.Fatal("request was never published") + } + + // Drain initial neither-granted-nor-denied notification + <-notifications + + // Concurrent GrantPersistent calls — only the first should resolve + var resolveWg sync.WaitGroup + resolveCount := atomic.Int32{} + for i := 0; i < 3; i++ { + resolveWg.Add(1) + go func() { + defer resolveWg.Done() + if service.GrantPersistent(pending) { + resolveCount.Add(1) + } + }() + } + resolveWg.Wait() + + assert.Equal(t, int32(1), resolveCount.Load(), "exactly one grant should resolve") + + wg.Wait() + require.NoError(t, reqErr) + assert.True(t, granted) + + // Only one notification (from first grant) + select { + case ev := <-notifications: + assert.True(t, ev.Payload.Granted, "first notification should be granted") + case <-time.After(2 * time.Second): + t.Fatal("no notification received") + } + + // Second notification should be timeout (no more) + select { + case ev := <-notifications: + t.Fatalf("extra notification: %+v", ev.Payload) + case <-time.After(50 * time.Millisecond): + // good: no extra notification + } + }) + + t.Run("losing GrantPersistent with contexts does not leak", func(t *testing.T) { + t.Parallel() + service := NewPermissionService("/tmp", false, nil, nil) + + events := service.Subscribe(t.Context()) + notifications := service.SubscribeNotifications(t.Context()) + + req := CreatePermissionRequest{ + SessionID: "session-lose-ctx", + ToolCallID: "lose-ctx-call", + ToolName: "bash", + Action: "execute", + Description: "Lose test", + Path: "/tmp", + Contexts: []string{"command:cd", "command:ls"}, + } + + var wg sync.WaitGroup + var granted bool + var reqErr error + wg.Go(func() { + granted, reqErr = service.Request(t.Context(), req) + }) + + var pending PermissionRequest + select { + case ev := <-events: + pending = ev.Payload + case <-time.After(2 * time.Second): + t.Fatal("request was never published") + } + + // Drain initial neither-granted-nor-denied notification + <-notifications + + // Deny wins, then a competing GrantPersistent loses + assert.True(t, service.Deny(pending), "Deny should resolve") + assert.False(t, service.GrantPersistent(pending), + "GrantPersistent after Deny should report already-resolved") + + wg.Wait() + require.NoError(t, reqErr) + assert.False(t, granted) + + // Follow-up request should NOT auto-approve (losing grant leaked nothing) + var wg2 sync.WaitGroup + var granted2 bool + var reqErr2 error + wg2.Go(func() { + granted2, reqErr2 = service.Request(t.Context(), req) + }) + + select { + case ev := <-events: + // Should publish (not auto-approved) + assert.Equal(t, pending.SessionID, ev.Payload.SessionID) + <-notifications + service.Deny(ev.Payload) + case <-time.After(2 * time.Second): + t.Fatal("follow-up should be published; losing GrantPersistent leaked an approval") + } + + wg2.Wait() + require.NoError(t, reqErr2) + assert.False(t, granted2) + }) +} + +func TestPermissionService_ContextEmptyFallback(t *testing.T) { + t.Parallel() + + t.Run("empty contexts uses legacy key", func(t *testing.T) { + t.Parallel() + service := NewPermissionService("/tmp", false, nil, nil) + + events := service.Subscribe(t.Context()) + notifications := service.SubscribeNotifications(t.Context()) + + req := CreatePermissionRequest{ + SessionID: "session-empty-ctx", + ToolCallID: "empty-ctx-call", + ToolName: "view", + Action: "read", + Description: "View file", + Path: "/tmp/file.txt", + Contexts: nil, // explicitly empty + } + + var wg sync.WaitGroup + var granted bool + var reqErr error + wg.Go(func() { + granted, reqErr = service.Request(t.Context(), req) + }) + + var pending PermissionRequest + select { + case ev := <-events: + pending = ev.Payload + case <-time.After(2 * time.Second): + t.Fatal("request was never published") + } + + <-notifications + assert.True(t, service.GrantPersistent(pending)) + + wg.Wait() + require.NoError(t, reqErr) + assert.True(t, granted) + + // Verify legacy key was recorded (Context field should be empty) + legacyKey := PermissionKey{ + SessionID: pending.SessionID, + ToolName: pending.ToolName, + Action: pending.Action, + Path: pending.Path, + // Context is empty — legacy style + } + val, ok := service.(*permissionService).sessionPermissions.Get(legacyKey) + assert.True(t, ok, "legacy key should be recorded") + assert.True(t, val, "legacy key should be approved") + + // Follow-up with same legacy key should auto-approve + req2 := CreatePermissionRequest{ + SessionID: "session-empty-ctx", + ToolCallID: "empty-ctx-call-2", + ToolName: "view", + Action: "read", + Description: "View same file again", + Path: "/tmp/file.txt", + Contexts: nil, + } + + result2, err2 := service.Request(t.Context(), req2) + require.NoError(t, err2) + assert.True(t, result2, "follow-up legacy request should auto-approve") + }) + + t.Run("nil contexts treated same as empty", func(t *testing.T) { + t.Parallel() + service := NewPermissionService("/tmp", false, nil, nil) + + events := service.Subscribe(t.Context()) + + req := CreatePermissionRequest{ + SessionID: "session-nil-ctx", + ToolCallID: "nil-ctx-call", + ToolName: "write", + Action: "create", + Description: "Write file", + Path: "/tmp/write.txt", + Contexts: nil, + } + + var wg sync.WaitGroup + var granted bool + var reqErr error + wg.Go(func() { + granted, reqErr = service.Request(t.Context(), req) + }) + + var pending PermissionRequest + select { + case ev := <-events: + pending = ev.Payload + case <-time.After(2 * time.Second): + t.Fatal("request was never published") + } + + assert.True(t, service.GrantPersistent(pending)) + + wg.Wait() + require.NoError(t, reqErr) + assert.True(t, granted) + + // Follow-up should auto-approve via legacy key + req2 := CreatePermissionRequest{ + SessionID: "session-nil-ctx", + ToolCallID: "nil-ctx-call-2", + ToolName: "write", + Action: "create", + Description: "Write same file", + Path: "/tmp/write.txt", + Contexts: nil, + } + + result2, err2 := service.Request(t.Context(), req2) + require.NoError(t, err2) + assert.True(t, result2) + }) +} + +func TestPermissionService_PathPrefixMatching(t *testing.T) { + t.Parallel() + + t.Run("approved parent path auto-approves child file", func(t *testing.T) { + t.Parallel() + service := NewPermissionService("/tmp", false, nil, nil) + + events := service.Subscribe(t.Context()) + notifications := service.SubscribeNotifications(t.Context()) + + // First request: user approves cat within /tmp. + req := CreatePermissionRequest{ + SessionID: "session-prefix", + ToolCallID: "prefix-call", + ToolName: "bash", + Action: "execute", + Description: "cat /tmp/file.txt", + Path: "/tmp", + Contexts: []string{"command:cat", "path:/tmp"}, + } + + var wg sync.WaitGroup + var granted bool + var reqErr error + wg.Go(func() { + granted, reqErr = service.Request(t.Context(), req) + }) + + var pending PermissionRequest + select { + case ev := <-events: + pending = ev.Payload + case <-time.After(2 * time.Second): + t.Fatal("request was never published") + } + + <-notifications + assert.True(t, service.GrantPersistent(pending)) + + wg.Wait() + require.NoError(t, reqErr) + assert.True(t, granted) + + // Second request: same command to a child path under /tmp should auto-approve. + req2 := CreatePermissionRequest{ + SessionID: "session-prefix", + ToolCallID: "prefix-call-2", + ToolName: "bash", + Action: "execute", + Description: "cat /tmp/subdir/file.txt", + Path: "/tmp", + Contexts: []string{"command:cat", "path:/tmp/subdir/file.txt"}, + } + + result2, err2 := service.Request(t.Context(), req2) + require.NoError(t, err2) + assert.True(t, result2, "child path of approved /tmp should auto-approve") + }) + + t.Run("approved parent path auto-approves nested child path", func(t *testing.T) { + t.Parallel() + service := NewPermissionService("/tmp", false, nil, nil) + + events := service.Subscribe(t.Context()) + notifications := service.SubscribeNotifications(t.Context()) + + // First request: user approves mkdir within /tmp/subpath. + req := CreatePermissionRequest{ + SessionID: "session-nested", + ToolCallID: "nested-call", + ToolName: "bash", + Action: "execute", + Description: "mkdir -p /tmp/subpath", + Path: "/tmp", + Contexts: []string{"command:mkdir", "path:/tmp/subpath"}, + } + + var wg sync.WaitGroup + var granted bool + var reqErr error + wg.Go(func() { + granted, reqErr = service.Request(t.Context(), req) + }) + + var pending PermissionRequest + select { + case ev := <-events: + pending = ev.Payload + case <-time.After(2 * time.Second): + t.Fatal("request was never published") + } + + <-notifications + assert.True(t, service.GrantPersistent(pending)) + + wg.Wait() + require.NoError(t, reqErr) + assert.True(t, granted) + + // Second request: same command to a deeply nested child path should auto-approve. + req2 := CreatePermissionRequest{ + SessionID: "session-nested", + ToolCallID: "nested-call-2", + ToolName: "bash", + Action: "execute", + Description: "mkdir -p /tmp/subpath/deep/dir", + Path: "/tmp", + Contexts: []string{"command:mkdir", "path:/tmp/subpath/deep/dir"}, + } + + result2, err2 := service.Request(t.Context(), req2) + require.NoError(t, err2) + assert.True(t, result2, "deeply nested child path of approved /tmp/subpath should auto-approve") + }) + + t.Run("approved exact path does not auto-approve sibling path", func(t *testing.T) { + t.Parallel() + service := NewPermissionService("/tmp", false, nil, nil) + + events := service.Subscribe(t.Context()) + notifications := service.SubscribeNotifications(t.Context()) + + // First request: user approves exact path /tmp/file.txt only. + req := CreatePermissionRequest{ + SessionID: "session-exact", + ToolCallID: "exact-call", + ToolName: "bash", + Action: "execute", + Description: "cat /tmp/file.txt", + Path: "/tmp", + Contexts: []string{"command:cat", "path:/tmp/file.txt"}, + } + + var wg sync.WaitGroup + var granted bool + var reqErr error + wg.Go(func() { + granted, reqErr = service.Request(t.Context(), req) + }) + + var pending PermissionRequest + select { + case ev := <-events: + pending = ev.Payload + case <-time.After(2 * time.Second): + t.Fatal("request was never published") + } + + <-notifications + assert.True(t, service.GrantPersistent(pending)) + + wg.Wait() + require.NoError(t, reqErr) + assert.True(t, granted) + + // Second request: writing to a sibling path should NOT auto-approve + req2 := CreatePermissionRequest{ + SessionID: "session-exact", + ToolCallID: "exact-call-2", + ToolName: "bash", + Action: "execute", + Description: "cat /tmp/other.txt", + Path: "/tmp", + Contexts: []string{"command:cat", "path:/tmp/other.txt"}, + } + + events2 := service.Subscribe(t.Context()) + notifications2 := service.SubscribeNotifications(t.Context()) + + var wg2 sync.WaitGroup + var granted2 bool + var reqErr2 error + wg2.Go(func() { + granted2, reqErr2 = service.Request(t.Context(), req2) + }) + + select { + case ev := <-events2: + // Should have published (not auto-approved) + assert.Equal(t, pending.SessionID, ev.Payload.SessionID) + <-notifications2 + service.Deny(ev.Payload) + case <-time.After(2 * time.Second): + t.Fatal("sibling path should NOT auto-approve") + } + + wg2.Wait() + require.NoError(t, reqErr2) + assert.False(t, granted2, "sibling path of /tmp/file.txt should NOT auto-approve") + }) + + t.Run("approved / prefix-approves all paths", func(t *testing.T) { + t.Parallel() + service := NewPermissionService("/tmp", false, nil, nil) + + events := service.Subscribe(t.Context()) + notifications := service.SubscribeNotifications(t.Context()) + + // First request: user approves ls with path:/ (root prefix — covers all paths). + req := CreatePermissionRequest{ + SessionID: "session-root", + ToolCallID: "root-call", + ToolName: "bash", + Action: "execute", + Description: "ls /", + Path: "/", + Contexts: []string{"command:ls", "path:/"}, + } + + var wg sync.WaitGroup + var granted bool + var reqErr error + wg.Go(func() { + granted, reqErr = service.Request(t.Context(), req) + }) + + var pending PermissionRequest + select { + case ev := <-events: + pending = ev.Payload + case <-time.After(2 * time.Second): + t.Fatal("request was never published") + } + + <-notifications + assert.True(t, service.GrantPersistent(pending)) + + wg.Wait() + require.NoError(t, reqErr) + assert.True(t, granted) + + // Second request: same command, any path under / should auto-approve. + req2 := CreatePermissionRequest{ + SessionID: "session-root", + ToolCallID: "root-call-2", + ToolName: "bash", + Action: "execute", + Description: "ls /etc", + Path: "/etc", + Contexts: []string{"command:ls", "path:/etc/hosts"}, + } + + result2, err2 := service.Request(t.Context(), req2) + require.NoError(t, err2) + assert.True(t, result2, "any path under / should auto-approve after / is approved with same command") + }) +} + +// TestPermissionService_AllowedContexts verifies that context tokens supplied +// at construction time (from config allowed_commands) auto-approve matching +// requests without a session grant. +func TestPermissionService_AllowedContexts(t *testing.T) { + t.Parallel() + + t.Run("exact configured command auto-approves", func(t *testing.T) { + t.Parallel() + svc := NewPermissionService("/tmp", false, nil, []string{ + "command:go test", + "command:git diff", + }) + + result, err := svc.Request(t.Context(), CreatePermissionRequest{ + SessionID: "s1", + ToolName: "bash", + Action: "execute", + Description: "run tests", + Contexts: []string{"command:go test"}, + }) + require.NoError(t, err) + assert.True(t, result, "command:go test should be auto-approved by config") + }) + + t.Run("configured base command satisfies subcommand token", func(t *testing.T) { + t.Parallel() + svc := NewPermissionService("/tmp", false, nil, []string{ + "command:go", + }) + + result, err := svc.Request(t.Context(), CreatePermissionRequest{ + SessionID: "s1", + ToolName: "bash", + Action: "execute", + Description: "run tests", + Contexts: []string{"command:go test"}, + }) + require.NoError(t, err) + assert.True(t, result, "configured command:go should satisfy command:go test") + }) + + t.Run("unconfigured command still prompts", func(t *testing.T) { + t.Parallel() + svc := NewPermissionService("/tmp", false, nil, []string{ + "command:go test", + }) + + ctx, cancel := context.WithTimeout(t.Context(), 50*time.Millisecond) + defer cancel() + + _, err := svc.Request(ctx, CreatePermissionRequest{ + SessionID: "s1", + ToolName: "bash", + Action: "execute", + Description: "run rm", + Contexts: []string{"command:rm"}, + }) + require.Error(t, err, "unconfigured command:rm should not auto-approve") + }) + + t.Run("all contexts must pass — partial config is insufficient", func(t *testing.T) { + t.Parallel() + // Only command:go is configured; path:/tmp is not. + svc := NewPermissionService("/tmp", false, nil, []string{ + "command:go", + }) + + ctx, cancel := context.WithTimeout(t.Context(), 50*time.Millisecond) + defer cancel() + + _, err := svc.Request(ctx, CreatePermissionRequest{ + SessionID: "s1", + ToolName: "bash", + Action: "execute", + Description: "go test with path", + Contexts: []string{"command:go test", "path:/tmp"}, + }) + require.Error(t, err, "path:/tmp is not configured so request should not auto-approve") + }) + + t.Run("configured base does not match different command word", func(t *testing.T) { + t.Parallel() + svc := NewPermissionService("/tmp", false, nil, []string{ + "command:py", + }) + + ctx, cancel := context.WithTimeout(t.Context(), 50*time.Millisecond) + defer cancel() + + _, err := svc.Request(ctx, CreatePermissionRequest{ + SessionID: "s1", + ToolName: "bash", + Action: "execute", + Description: "python3 script", + Contexts: []string{"command:python3"}, + }) + require.Error(t, err, "command:py must not match command:python3") + }) +} diff --git a/internal/proto/permission.go b/internal/proto/permission.go index 981f1bc3bc..8a422e8f44 100644 --- a/internal/proto/permission.go +++ b/internal/proto/permission.go @@ -6,13 +6,14 @@ import ( // CreatePermissionRequest represents a request to create a permission. type CreatePermissionRequest struct { - SessionID string `json:"session_id"` - ToolCallID string `json:"tool_call_id"` - ToolName string `json:"tool_name"` - Description string `json:"description"` - Action string `json:"action"` - Params any `json:"params"` - Path string `json:"path"` + SessionID string `json:"session_id"` + ToolCallID string `json:"tool_call_id"` + ToolName string `json:"tool_name"` + Description string `json:"description"` + Action string `json:"action"` + Params any `json:"params"` + Path string `json:"path"` + Contexts []string `json:"contexts,omitempty"` } // PermissionNotification represents a notification about a permission change. @@ -24,14 +25,16 @@ type PermissionNotification struct { // PermissionRequest represents a pending permission request. type PermissionRequest struct { - ID string `json:"id"` - SessionID string `json:"session_id"` - ToolCallID string `json:"tool_call_id"` - ToolName string `json:"tool_name"` - Description string `json:"description"` - Action string `json:"action"` - Params any `json:"params"` - Path string `json:"path"` + ID string `json:"id"` + SessionID string `json:"session_id"` + ToolCallID string `json:"tool_call_id"` + ToolName string `json:"tool_name"` + Description string `json:"description"` + Action string `json:"action"` + Params any `json:"params"` + Path string `json:"path"` + Contexts []string `json:"contexts,omitempty"` + PendingContexts []string `json:"pending_contexts,omitempty"` } // UnmarshalJSON implements the json.Unmarshaler interface. This is needed diff --git a/internal/ui/dialog/permissions.go b/internal/ui/dialog/permissions.go index 4dfb9ba665..ef5f19125f 100644 --- a/internal/ui/dialog/permissions.go +++ b/internal/ui/dialog/permissions.go @@ -558,7 +558,12 @@ func (p *Permissions) renderBashContent(width int) string { return "" } - return p.renderContentPanel(params.Command, width) + var parts []string + parts = append(parts, p.renderContentPanel(params.Command, width)) + if ctxBlock := p.renderPendingContexts(width); ctxBlock != "" { + parts = append(parts, ctxBlock) + } + return strings.Join(parts, "\n") } func (p *Permissions) renderEditContent(contentWidth int) string { @@ -722,6 +727,14 @@ func (p *Permissions) renderDefaultContent(width int) string { } } + // Display contexts if available + if pendingCtx := p.renderPendingContexts(width); pendingCtx != "" { + if content != "" { + content += "\n\n" + } + content += pendingCtx + } + if content == "" { return "" } @@ -729,6 +742,48 @@ func (p *Permissions) renderDefaultContent(width int) string { return p.renderContentPanel(strings.TrimSpace(content), width) } +// renderPendingContexts renders only the context tokens that have not yet been +// approved, using styled key-value rows. If some tokens are already covered by +// prior grants, a faint note shows the count. Returns "" when there are no +// contexts at all. +func (p *Permissions) renderPendingContexts(width int) string { + pending := p.permission.PendingContexts + if len(pending) == 0 { + return "" + } + + var lines []string + for _, tok := range pending { + label, value := parseContextToken(tok) + lines = append(lines, p.renderKeyValue(label, value, width)) + } + + alreadyApproved := len(p.permission.Contexts) - len(pending) + if alreadyApproved > 0 { + note := lipgloss.NewStyle().Faint(true).Render( + fmt.Sprintf(" (%d already approved this session)", alreadyApproved), + ) + lines = append(lines, note) + } + + return strings.Join(lines, "\n") +} + +// parseContextToken splits a context token (e.g. "command:go test", +// "path:/tmp", "command!:bash -c ...") into a display label and its value. +func parseContextToken(tok string) (label, value string) { + switch { + case strings.HasPrefix(tok, "command!:"): + return "unsafe", strings.TrimPrefix(tok, "command!:") + case strings.HasPrefix(tok, "command:"): + return "command", strings.TrimPrefix(tok, "command:") + case strings.HasPrefix(tok, "path:"): + return "path", strings.TrimPrefix(tok, "path:") + default: + return "context", tok + } +} + // renderContentPanel renders content in a panel with the full width. func (p *Permissions) renderContentPanel(content string, width int) string { panelStyle := p.com.Styles.Dialog.ContentPanel diff --git a/internal/workspace/client_workspace.go b/internal/workspace/client_workspace.go index 09ff57c612..565c7fd55d 100644 --- a/internal/workspace/client_workspace.go +++ b/internal/workspace/client_workspace.go @@ -278,6 +278,7 @@ func (w *ClientWorkspace) PermissionGrant(perm permission.PermissionRequest) boo Action: perm.Action, Path: perm.Path, Params: perm.Params, + Contexts: perm.Contexts, }, Action: proto.PermissionAllow, }) @@ -295,6 +296,7 @@ func (w *ClientWorkspace) PermissionGrantPersistent(perm permission.PermissionRe Action: perm.Action, Path: perm.Path, Params: perm.Params, + Contexts: perm.Contexts, }, Action: proto.PermissionAllowForSession, }) @@ -312,6 +314,7 @@ func (w *ClientWorkspace) PermissionDeny(perm permission.PermissionRequest) bool Action: perm.Action, Path: perm.Path, Params: perm.Params, + Contexts: perm.Contexts, }, Action: proto.PermissionDeny, }) @@ -669,14 +672,16 @@ func (w *ClientWorkspace) translateEvent(ev any) tea.Msg { return pubsub.Event[permission.PermissionRequest]{ Type: e.Type, Payload: permission.PermissionRequest{ - ID: e.Payload.ID, - SessionID: e.Payload.SessionID, - ToolCallID: e.Payload.ToolCallID, - ToolName: e.Payload.ToolName, - Description: e.Payload.Description, - Action: e.Payload.Action, - Path: e.Payload.Path, - Params: e.Payload.Params, + ID: e.Payload.ID, + SessionID: e.Payload.SessionID, + ToolCallID: e.Payload.ToolCallID, + ToolName: e.Payload.ToolName, + Description: e.Payload.Description, + Action: e.Payload.Action, + Path: e.Payload.Path, + Params: e.Payload.Params, + Contexts: e.Payload.Contexts, + PendingContexts: e.Payload.PendingContexts, }, } case pubsub.Event[proto.PermissionNotification]: