From 1ded7955a1ecd99fa6ee874d8e661c25fec60aa2 Mon Sep 17 00:00:00 2001 From: pengdst Date: Wed, 22 Jul 2026 20:11:33 +0700 Subject: [PATCH 1/6] feat(permission): auto-classifier autonomy, review transparency, and prefix-grant breadth/scope Adds the auto-classifier ("let Zero cook") permission mode with an LLM reviewing low-risk actions, review transparency for prompt decisions, and command-prefix grants offered at multiple breadths and scopes (session/project/global). TUI: - Shift+Tab cycles ask -> auto -> trusted workspace -> let Zero cook; the auto-classifier warning is confirmed once per process, rendered in the live-tail footer so it paints reliably over a resumed transcript. - Permission prompt labels each offered prefix breadth distinctly and omits the empty hotkey bracket for arrow-key-only breadths. ACP: - Maps the project-scoped prefix action and expands each prefix action into one option per offered breadth, encoding and validating the selected breadth on the option round trip. Sandbox/permissions: - Command-prefix ladder excludes the one-token launcher rung and binds a grant to the effective directory so a `cd` outside the workspace cannot borrow it. --- internal/acp/agent.go | 19 +- internal/acp/agent_test.go | 5 +- internal/acp/permission.go | 104 +++++- internal/acp/permission_test.go | 120 +++++- internal/agent/auto_classifier.go | 104 ++++++ internal/agent/auto_classifier_prompt.md | 15 + internal/agent/auto_classifier_test.go | 94 +++++ internal/agent/command_prefix.go | 176 ++++++++- internal/agent/command_prefix_test.go | 142 +++++++ internal/agent/loop.go | 282 +++++++++++--- internal/agent/loop_test.go | 345 +++++++++++++++++- internal/agent/member_auto_test.go | 20 +- .../agent/permission_additional_perms_test.go | 6 +- internal/agent/request_permissions_test.go | 2 +- internal/agent/types.go | 178 +++++++-- internal/cli/exec_test.go | 19 +- internal/cli/exec_tools.go | 12 +- internal/sandbox/command_prefix.go | 65 +++- .../sandbox/command_prefix_wildcard_test.go | 76 ++++ internal/sandbox/engine.go | 29 +- internal/sandbox/grants.go | 32 +- internal/sandbox/grants_test.go | 4 +- internal/specialist/exec.go | 14 +- internal/specialist/exec_test.go | 22 +- internal/swarm/team.go | 16 +- internal/swarm/team_test.go | 8 + internal/tui/command_polish_test.go | 20 +- internal/tui/command_views.go | 128 ++++--- internal/tui/hover.go | 25 +- internal/tui/keybinding_help.go | 2 +- internal/tui/model.go | 148 ++++++-- internal/tui/model_test.go | 198 +++++++++- internal/tui/permission_prompt.go | 64 +++- internal/tui/permission_prompt_test.go | 109 +++++- internal/tui/render_cache.go | 1 + internal/tui/rendering.go | 55 ++- internal/tui/rendering_lime_test.go | 16 +- internal/tui/session.go | 1 + internal/tui/session_test.go | 2 +- internal/tui/transcript.go | 11 +- internal/tui/transcript_selection.go | 32 +- internal/tui/view.go | 67 +++- 42 files changed, 2438 insertions(+), 350 deletions(-) create mode 100644 internal/agent/auto_classifier.go create mode 100644 internal/agent/auto_classifier_prompt.md create mode 100644 internal/agent/auto_classifier_test.go create mode 100644 internal/sandbox/command_prefix_wildcard_test.go diff --git a/internal/acp/agent.go b/internal/acp/agent.go index 16c7110cd..4de74788c 100644 --- a/internal/acp/agent.go +++ b/internal/acp/agent.go @@ -326,7 +326,7 @@ func (a *Agent) requestPermission(ctx context.Context, sessionID string, req age } return agent.PermissionDecision{Action: agent.PermissionDecisionDeny, Reason: "permission request failed: " + err.Error()}, nil } - return decisionFromOutcome(result.Outcome, req.AvailableDecisions), nil + return decisionFromOutcome(result.Outcome, req), nil } func (a *Agent) emitPlan(registry *tools.Registry, note *notifier) { @@ -354,7 +354,7 @@ func (a *Agent) handleSetMode(_ context.Context, params json.RawMessage) (any, e } mode := agent.PermissionMode(p.ModeID) switch mode { - case agent.PermissionModeAuto, agent.PermissionModeAsk: + case agent.PermissionModeAsk, agent.PermissionModeAuto, agent.PermissionModeWorkspaceAuto, agent.PermissionModeAutoClassifier: sess.setMode(mode) (¬ifier{conn: a.conn, sessionID: sess.id}).currentMode(string(mode)) return SetSessionModeResult{}, nil @@ -442,13 +442,20 @@ func (a *Agent) handleCancel(_ context.Context, params json.RawMessage) { // ---- advertising helpers ---- func (a *Agent) modeState(s *acpSession) *SessionModeState { - // Only auto/ask are offered over ACP; Unsafe is gated to the operator (see - // handleSetMode) so a client can't grant itself no-prompt host access. + // Only prompt-respecting modes are offered over ACP; Unsafe is gated to the + // operator (see handleSetMode) so a client can't grant itself no-prompt host + // access. + ask := agent.PermissionModeInfoFor(agent.PermissionModeAsk) + auto := agent.PermissionModeInfoFor(agent.PermissionModeAuto) + workspaceAuto := agent.PermissionModeInfoFor(agent.PermissionModeWorkspaceAuto) + autoClassifier := agent.PermissionModeInfoFor(agent.PermissionModeAutoClassifier) return &SessionModeState{ CurrentModeID: string(s.currentMode()), AvailableModes: []SessionMode{ - {ID: string(agent.PermissionModeAuto), Name: "Auto", Description: "Run safe tools automatically; ask before risky ones."}, - {ID: string(agent.PermissionModeAsk), Name: "Ask", Description: "Ask before every tool that changes state."}, + {ID: string(ask.ID), Name: ask.Label, Description: ask.Description}, + {ID: string(auto.ID), Name: auto.Label, Description: auto.Description}, + {ID: string(workspaceAuto.ID), Name: workspaceAuto.Label, Description: workspaceAuto.Description}, + {ID: string(autoClassifier.ID), Name: autoClassifier.Label, Description: autoClassifier.Description}, }, } } diff --git a/internal/acp/agent_test.go b/internal/acp/agent_test.go index a9e07eccc..2e5d9a084 100644 --- a/internal/acp/agent_test.go +++ b/internal/acp/agent_test.go @@ -380,7 +380,7 @@ func TestACPSetModeUpdatesSession(t *testing.T) { if err := h.client.Call(ctx, MethodSessionNew, NewSessionParams{Cwd: t.TempDir(), McpServers: []McpServer{}}, &newRes); err != nil { t.Fatalf("session/new: %v", err) } - // auto/ask are accepted. + // Prompt-respecting modes are accepted. if err := h.client.Call(ctx, MethodSessionSetMode, SetSessionModeParams{SessionID: newRes.SessionID, ModeID: string(agent.PermissionModeAsk)}, &SetSessionModeResult{}); err != nil { t.Fatalf("set_mode ask: %v", err) } @@ -392,8 +392,7 @@ func TestACPSetModeUpdatesSession(t *testing.T) { } if got := configured.ConfigOptions[1].CurrentValue; got != string(agent.PermissionModeAuto) { t.Fatalf("configured mode = %q", got) - } - // Unsafe must be rejected over ACP — a client can't self-grant no-prompt host access. + } // Unsafe must be rejected over ACP — a client can't self-grant no-prompt host access. if err := h.client.Call(ctx, MethodSessionSetMode, SetSessionModeParams{SessionID: newRes.SessionID, ModeID: string(agent.PermissionModeUnsafe)}, &SetSessionModeResult{}); err == nil { t.Fatal("expected Unsafe mode to be rejected over ACP") } diff --git a/internal/acp/permission.go b/internal/acp/permission.go index cbe24f9bf..a2952dbd6 100644 --- a/internal/acp/permission.go +++ b/internal/acp/permission.go @@ -2,6 +2,7 @@ package acp import ( "encoding/json" + "strings" "github.com/Gitlawb/zero/internal/agent" ) @@ -29,6 +30,21 @@ func buildPermissionOptions(req agent.PermissionRequest) []PermissionOption { if kind == "" { continue // skip actions that have no clean ACP option (e.g. cancel) } + // A command-prefix grant can be offered at several breadths (e.g. `git`, + // `git push`, `git push origin`). Expand each into its own ACP option so the + // client can pick how wide the grant is; the chosen breadth is encoded into + // the option id and validated on the way back. Without this, ACP clients can + // only ever grant the exact default prefix. + if isPrefixAction(action) && len(req.CommandPrefixOptions) > 1 { + for _, prefix := range req.CommandPrefixOptions { + options = append(options, PermissionOption{ + OptionID: encodeOptionID(action, prefix), + Name: name + ": " + strings.Join(prefix, " "), + Kind: kind, + }) + } + continue + } options = append(options, PermissionOption{ OptionID: string(action), Name: name, @@ -38,6 +54,52 @@ func buildPermissionOptions(req agent.PermissionRequest) []PermissionOption { return options } +// isPrefixAction reports whether an action grants a command prefix, which is the +// only decision family that carries a breadth choice (CommandPrefixOptions). +func isPrefixAction(action agent.PermissionDecisionAction) bool { + switch action { + case agent.PermissionDecisionAllowPrefix, + agent.PermissionDecisionAllowPrefixProject, + agent.PermissionDecisionAlwaysAllowPrefix: + return true + default: + return false + } +} + +// encodeOptionID packs the ZERO action plus the chosen command-prefix breadth into +// the ACP option id so the selection round-trips exactly. An action with no +// breadth keeps the bare action string, so non-prefix options stay backward +// compatible on the wire. +func encodeOptionID(action agent.PermissionDecisionAction, prefix []string) string { + if len(prefix) == 0 { + return string(action) + } + payload, err := json.Marshal(optionIDPayload{Action: string(action), Prefix: prefix}) + if err != nil { + return string(action) + } + return string(payload) +} + +// decodeOptionID reverses encodeOptionID. A JSON payload yields the action and its +// breadth; anything else is treated as a bare action string (no breadth). +func decodeOptionID(id string) (agent.PermissionDecisionAction, []string) { + trimmed := strings.TrimSpace(id) + if strings.HasPrefix(trimmed, "{") { + var payload optionIDPayload + if err := json.Unmarshal([]byte(trimmed), &payload); err == nil && payload.Action != "" { + return agent.PermissionDecisionAction(payload.Action), payload.Prefix + } + } + return agent.PermissionDecisionAction(id), nil +} + +type optionIDPayload struct { + Action string `json:"action"` + Prefix []string `json:"prefix"` +} + // optionKindFor maps a ZERO decision action to an ACP PermissionOptionKind and a // human label. Returns an empty kind for actions that ACP expresses through the // outcome rather than an option (cancel). @@ -49,6 +111,8 @@ func optionKindFor(action agent.PermissionDecisionAction) (kind, name string) { return PermAllowAlways, "Allow for this session" case agent.PermissionDecisionAllowPrefix: return PermAllowAlways, "Allow this command for the session" + case agent.PermissionDecisionAllowPrefixProject: + return PermAllowAlways, "Allow this command for this project" case agent.PermissionDecisionAlwaysAllow: return PermAllowAlways, "Always allow" case agent.PermissionDecisionAlwaysAllowPrefix: @@ -66,19 +130,28 @@ func optionKindFor(action agent.PermissionDecisionAction) (kind, name string) { // decision. A cancelled outcome cancels the run; a selected option id is the ZERO // action verbatim (validated against what was offered); anything unrecognized // fails closed to deny. -func decisionFromOutcome(outcome RequestPermissionOutcome, offered []agent.PermissionDecisionAction) agent.PermissionDecision { +func decisionFromOutcome(outcome RequestPermissionOutcome, req agent.PermissionRequest) agent.PermissionDecision { switch outcome.Outcome { case OutcomeCancelled: return agent.PermissionDecision{Action: agent.PermissionDecisionCancel, Reason: "client cancelled"} case OutcomeSelected: - action := agent.PermissionDecisionAction(outcome.OptionID) + action, prefix := decodeOptionID(outcome.OptionID) // Bind to what was actually offered for THIS call: a client must not be able // to return a broader grant (always_allow / allow_for_session) that wasn't // presented. Anything not offered fails closed to deny. - if actionOffered(action, offered) { - return agent.PermissionDecision{Action: action} + if !actionOffered(action, req.AvailableDecisions) { + return agent.PermissionDecision{Action: agent.PermissionDecisionDeny, Reason: "permission option was not offered"} } - return agent.PermissionDecision{Action: agent.PermissionDecisionDeny, Reason: "permission option was not offered"} + decision := agent.PermissionDecision{Action: action} + if len(prefix) > 0 { + // The breadth must be one of the rungs this call offered, so a client can + // only widen the grant to a presented breadth — never an arbitrary prefix. + if !prefixOffered(prefix, req.CommandPrefixOptions) { + return agent.PermissionDecision{Action: agent.PermissionDecisionDeny, Reason: "command prefix was not offered"} + } + decision.CommandPrefix = append([]string(nil), prefix...) + } + return decision default: return agent.PermissionDecision{Action: agent.PermissionDecisionDeny, Reason: "no permission outcome"} } @@ -93,6 +166,27 @@ func actionOffered(action agent.PermissionDecisionAction, offered []agent.Permis return false } +// prefixOffered reports whether prefix exactly matches one of the offered breadth +// rungs, so a selected grant can only be one the request actually presented. +func prefixOffered(prefix []string, offered [][]string) bool { + for _, candidate := range offered { + if len(candidate) != len(prefix) { + continue + } + match := true + for i := range candidate { + if candidate[i] != prefix[i] { + match = false + break + } + } + if match { + return true + } + } + return false +} + // permissionToolCall builds the ToolCall descriptor embedded in a // session/request_permission request from a ZERO permission request. func permissionToolCall(req agent.PermissionRequest) ToolCallUpdate { diff --git a/internal/acp/permission_test.go b/internal/acp/permission_test.go index c70d409e7..5ff8db529 100644 --- a/internal/acp/permission_test.go +++ b/internal/acp/permission_test.go @@ -1,6 +1,7 @@ package acp import ( + "strings" "testing" "github.com/Gitlawb/zero/internal/agent" @@ -13,21 +14,35 @@ func TestBuildPermissionOptions(t *testing.T) { AvailableDecisions: []agent.PermissionDecisionAction{ agent.PermissionDecisionAllow, agent.PermissionDecisionAllowForSession, + agent.PermissionDecisionAllowPrefix, + agent.PermissionDecisionAllowPrefixProject, agent.PermissionDecisionAlwaysAllow, + agent.PermissionDecisionAlwaysAllowPrefix, agent.PermissionDecisionDeny, agent.PermissionDecisionCancel, // must be dropped (expressed as outcome) }, } opts := buildPermissionOptions(req) - if len(opts) != 4 { - t.Fatalf("expected 4 options (cancel dropped), got %d: %+v", len(opts), opts) + // 7 options: allow, allow_for_session, allow_prefix, allow_prefix_for_project, + // always_allow, always_allow_prefix, deny (cancel dropped). No breadth expansion + // because CommandPrefixOptions is empty. + if len(opts) != 7 { + t.Fatalf("expected 7 options (cancel dropped), got %d: %+v", len(opts), opts) } // optionId must carry the ZERO action verbatim for a clean round trip. if opts[0].OptionID != string(agent.PermissionDecisionAllow) || opts[0].Kind != PermAllowOnce { t.Errorf("allow option = %+v", opts[0]) } - if opts[1].Kind != PermAllowAlways || opts[3].Kind != PermRejectOnce { - t.Errorf("kinds = %q, %q", opts[1].Kind, opts[3].Kind) + // The project-scoped prefix grant must be present (regression: it was silently + // dropped because optionKindFor had no case for it). + if opts[3].OptionID != string(agent.PermissionDecisionAllowPrefixProject) || opts[3].Kind != PermAllowAlways { + t.Errorf("project prefix option = %+v", opts[3]) + } + if opts[1].Kind != PermAllowAlways || opts[2].Kind != PermAllowAlways || opts[4].Kind != PermAllowAlways || opts[5].Kind != PermAllowAlways { + t.Errorf("allow kinds = %q, %q, %q, %q", opts[1].Kind, opts[2].Kind, opts[4].Kind, opts[5].Kind) + } + if opts[6].OptionID != string(agent.PermissionDecisionDeny) || opts[6].Kind != PermRejectOnce { + t.Errorf("deny option = %+v", opts[6]) } } @@ -39,30 +54,109 @@ func TestBuildPermissionOptionsDefault(t *testing.T) { } func TestDecisionFromOutcome(t *testing.T) { - offered := []agent.PermissionDecisionAction{ - agent.PermissionDecisionAllow, - agent.PermissionDecisionAlwaysAllow, - agent.PermissionDecisionDeny, + req := agent.PermissionRequest{ + AvailableDecisions: []agent.PermissionDecisionAction{ + agent.PermissionDecisionAllow, + agent.PermissionDecisionAlwaysAllow, + agent.PermissionDecisionDeny, + }, } - if d := decisionFromOutcome(RequestPermissionOutcome{Outcome: OutcomeCancelled}, offered); d.Action != agent.PermissionDecisionCancel { + if d := decisionFromOutcome(RequestPermissionOutcome{Outcome: OutcomeCancelled}, req); d.Action != agent.PermissionDecisionCancel { t.Errorf("cancelled -> %q, want cancel", d.Action) } - if d := decisionFromOutcome(RequestPermissionOutcome{Outcome: OutcomeSelected, OptionID: "allow"}, offered); d.Action != agent.PermissionDecisionAllow { + if d := decisionFromOutcome(RequestPermissionOutcome{Outcome: OutcomeSelected, OptionID: "allow"}, req); d.Action != agent.PermissionDecisionAllow { t.Errorf("selected allow -> %q", d.Action) } - if d := decisionFromOutcome(RequestPermissionOutcome{Outcome: OutcomeSelected, OptionID: "always_allow"}, offered); d.Action != agent.PermissionDecisionAlwaysAllow { + if d := decisionFromOutcome(RequestPermissionOutcome{Outcome: OutcomeSelected, OptionID: "always_allow"}, req); d.Action != agent.PermissionDecisionAlwaysAllow { t.Errorf("selected always_allow -> %q", d.Action) } // Unknown option fails closed to deny. - if d := decisionFromOutcome(RequestPermissionOutcome{Outcome: OutcomeSelected, OptionID: "bogus"}, offered); d.Action != agent.PermissionDecisionDeny { + if d := decisionFromOutcome(RequestPermissionOutcome{Outcome: OutcomeSelected, OptionID: "bogus"}, req); d.Action != agent.PermissionDecisionDeny { t.Errorf("unknown option -> %q, want deny", d.Action) } // Missing/empty outcome fails closed to deny. - if d := decisionFromOutcome(RequestPermissionOutcome{}, offered); d.Action != agent.PermissionDecisionDeny { + if d := decisionFromOutcome(RequestPermissionOutcome{}, req); d.Action != agent.PermissionDecisionDeny { t.Errorf("empty outcome -> %q, want deny", d.Action) } } +func TestBuildPermissionOptionsExpandsPrefixBreadths(t *testing.T) { + req := agent.PermissionRequest{ + ToolName: "bash", + CommandPrefix: []string{"git", "push", "origin"}, + CommandPrefixOptions: [][]string{ + {"git", "push"}, + {"git", "push", "origin"}, + }, + AvailableDecisions: []agent.PermissionDecisionAction{ + agent.PermissionDecisionAllow, + agent.PermissionDecisionAllowPrefix, + agent.PermissionDecisionAllowPrefixProject, + agent.PermissionDecisionAlwaysAllowPrefix, + agent.PermissionDecisionDeny, + }, + } + opts := buildPermissionOptions(req) + // allow + deny (1 each) + 3 prefix actions × 2 breadths = 8. + if len(opts) != 8 { + t.Fatalf("expected 8 options (2 breadths per prefix action), got %d: %+v", len(opts), opts) + } + // Each prefix breadth must round-trip its action AND its exact prefix. + seen := map[string]bool{} + for _, opt := range opts { + action, prefix := decodeOptionID(opt.OptionID) + if !isPrefixAction(action) { + continue + } + if !prefixOffered(prefix, req.CommandPrefixOptions) { + t.Errorf("expanded option %q decoded to a non-offered prefix %v", opt.Name, prefix) + } + seen[string(action)+"|"+strings.Join(prefix, " ")] = true + } + for _, action := range []agent.PermissionDecisionAction{ + agent.PermissionDecisionAllowPrefix, + agent.PermissionDecisionAllowPrefixProject, + agent.PermissionDecisionAlwaysAllowPrefix, + } { + for _, prefix := range req.CommandPrefixOptions { + if !seen[string(action)+"|"+strings.Join(prefix, " ")] { + t.Errorf("missing expanded option for %s / %v", action, prefix) + } + } + } +} + +func TestDecisionFromOutcomePrefixBreadthRoundTrip(t *testing.T) { + req := agent.PermissionRequest{ + CommandPrefix: []string{"git", "push", "origin"}, + CommandPrefixOptions: [][]string{ + {"git", "push"}, + {"git", "push", "origin"}, + }, + AvailableDecisions: []agent.PermissionDecisionAction{ + agent.PermissionDecisionAllowPrefixProject, + agent.PermissionDecisionDeny, + }, + } + + // Selecting the broader `git push` breadth for the project scope must carry + // both the action and the chosen prefix back to ZERO. + id := encodeOptionID(agent.PermissionDecisionAllowPrefixProject, []string{"git", "push"}) + d := decisionFromOutcome(RequestPermissionOutcome{Outcome: OutcomeSelected, OptionID: id}, req) + if d.Action != agent.PermissionDecisionAllowPrefixProject { + t.Fatalf("action = %q, want allow_prefix_for_project", d.Action) + } + if strings.Join(d.CommandPrefix, " ") != "git push" { + t.Fatalf("prefix = %v, want [git push]", d.CommandPrefix) + } + + // A prefix that was never offered fails closed to deny (no silent widening). + tampered := encodeOptionID(agent.PermissionDecisionAllowPrefixProject, []string{"git"}) + if d := decisionFromOutcome(RequestPermissionOutcome{Outcome: OutcomeSelected, OptionID: tampered}, req); d.Action != agent.PermissionDecisionDeny { + t.Fatalf("non-offered prefix -> %q, want deny", d.Action) + } +} + func TestPermissionToolCall(t *testing.T) { tc := permissionToolCall(agent.PermissionRequest{ ToolCallID: "tc9", diff --git a/internal/agent/auto_classifier.go b/internal/agent/auto_classifier.go new file mode 100644 index 000000000..498f5398c --- /dev/null +++ b/internal/agent/auto_classifier.go @@ -0,0 +1,104 @@ +package agent + +import ( + "context" + _ "embed" + "encoding/json" + "errors" + "io" + "strings" + "time" + + "github.com/Gitlawb/zero/internal/zeroruntime" +) + +// autoPermissionClassifierSystemPrompt is the dedicated system prompt for the +// LLM permission classifier used by PermissionModeAutoClassifier. +// +//go:embed auto_classifier_prompt.md +var autoPermissionClassifierSystemPrompt string + +const autoPermissionClassifierTimeout = 10 * time.Second + +func defaultAutoPermissionClassifier(provider Provider) AutoPermissionClassifier { + return func(ctx context.Context, request AutoPermissionClassifierRequest) (AutoPermissionClassifierDecision, error) { + return classifyAutoPermissionWithProvider(ctx, provider, request) + } +} + +func classifyAutoPermissionWithProvider(ctx context.Context, provider Provider, request AutoPermissionClassifierRequest) (AutoPermissionClassifierDecision, error) { + payload, err := json.Marshal(request) + if err != nil { + return AutoPermissionClassifierDecision{}, err + } + classifierCtx, cancel := context.WithTimeout(ctx, autoPermissionClassifierTimeout) + defer cancel() + systemPrompt := strings.TrimSpace(autoPermissionClassifierSystemPrompt) + stream, err := provider.StreamCompletion(classifierCtx, zeroruntime.CompletionRequest{ + Messages: []zeroruntime.Message{ + {Role: zeroruntime.MessageRoleSystem, Content: systemPrompt}, + {Role: zeroruntime.MessageRoleUser, Content: string(payload)}, + }, + Tools: nil, + }) + if err != nil { + return AutoPermissionClassifierDecision{}, err + } + collected := zeroruntime.CollectStream(classifierCtx, stream) + if collected.Error != "" { + return AutoPermissionClassifierDecision{}, errors.New(collected.Error) + } + if collected.FinishReason != "" { + return AutoPermissionClassifierDecision{}, errors.New("auto-classifier response ended early: " + collected.FinishReason) + } + decision, ok := parseAutoPermissionClassifierDecision(collected.Text) + if !ok { + return AutoPermissionClassifierDecision{}, errors.New("invalid auto-classifier response") + } + return decision, nil +} + +func parseAutoPermissionClassifierDecision(output string) (AutoPermissionClassifierDecision, bool) { + output = normalizeAutoPermissionClassifierOutput(output) + if output == "" { + return AutoPermissionClassifierDecision{}, false + } + var raw struct { + Action string `json:"action"` + Reason string `json:"reason"` + } + decoder := json.NewDecoder(strings.NewReader(output)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&raw); err != nil { + return AutoPermissionClassifierDecision{}, false + } + var trailing any + if err := decoder.Decode(&trailing); err != io.EOF { + return AutoPermissionClassifierDecision{}, false + } + decision := AutoPermissionClassifierDecision{Reason: strings.TrimSpace(raw.Reason)} + switch AutoPermissionClassifierAction(strings.TrimSpace(raw.Action)) { + case AutoPermissionClassifierAllow: + decision.Action = AutoPermissionClassifierAllow + case AutoPermissionClassifierPrompt: + decision.Action = AutoPermissionClassifierPrompt + default: + return AutoPermissionClassifierDecision{}, false + } + if decision.Reason == "" { + return AutoPermissionClassifierDecision{}, false + } + return decision, true +} + +func normalizeAutoPermissionClassifierOutput(output string) string { + output = strings.TrimSpace(output) + if !strings.HasPrefix(output, "```") { + return output + } + lines := strings.Split(output, "\n") + if len(lines) < 2 || strings.TrimSpace(lines[len(lines)-1]) != "```" { + return output + } + return strings.TrimSpace(strings.Join(lines[1:len(lines)-1], "\n")) +} diff --git a/internal/agent/auto_classifier_prompt.md b/internal/agent/auto_classifier_prompt.md new file mode 100644 index 000000000..d98414c3e --- /dev/null +++ b/internal/agent/auto_classifier_prompt.md @@ -0,0 +1,15 @@ +You are Zero's auto-classifier permission reviewer. + +You review a single proposed tool call that would otherwise pause to ask the user for permission. Your job is to decide whether it is safe enough to run automatically, without asking. + +If you choose `allow`, the tool call runs immediately with no user confirmation. If you choose `prompt`, the user is asked as usual. When in doubt, choose `prompt` — a needless question is cheap, an unwanted action is not. + +The sandbox has already blocked the highest-risk categories before they reach you: network access, destructive commands, privilege/sandbox escalation, and access outside the workspace all bypass you and always ask. So you are only judging ordinary in-workspace actions. + +Choose `prompt` whenever there is uncertainty, ambiguity, missing context, possible loss of user data, irreversible change, or anything a careful user would want to see first. Choose `allow` only for actions that are clearly routine and low-risk. + +Return strict JSON only, with exactly this shape: + +```json +{"action":"allow"|"prompt","reason":"..."} +``` diff --git a/internal/agent/auto_classifier_test.go b/internal/agent/auto_classifier_test.go new file mode 100644 index 000000000..2d4842411 --- /dev/null +++ b/internal/agent/auto_classifier_test.go @@ -0,0 +1,94 @@ +package agent + +import ( + "context" + "strings" + "testing" + + "github.com/Gitlawb/zero/internal/zeroruntime" +) + +type autoClassifierRecordingProvider struct { + request zeroruntime.CompletionRequest + text string +} + +func (p *autoClassifierRecordingProvider) StreamCompletion(_ context.Context, request zeroruntime.CompletionRequest) (<-chan zeroruntime.StreamEvent, error) { + p.request = request + ch := make(chan zeroruntime.StreamEvent, 2) + ch <- zeroruntime.StreamEvent{Type: zeroruntime.StreamEventText, Content: p.text} + ch <- zeroruntime.StreamEvent{Type: zeroruntime.StreamEventDone} + close(ch) + return ch, nil +} + +func TestAutoPermissionClassifierUsesEmbeddedPrompt(t *testing.T) { + provider := &autoClassifierRecordingProvider{text: `{"action":"allow","reason":"low-risk workspace read"}`} + + decision, err := classifyAutoPermissionWithProvider(context.Background(), provider, AutoPermissionClassifierRequest{ + ToolName: "read_file", + PermissionMode: PermissionModeAutoClassifier, + }) + if err != nil { + t.Fatalf("classifyAutoPermissionWithProvider returned error: %v", err) + } + if decision.Action != AutoPermissionClassifierAllow { + t.Fatalf("decision action = %q, want %q", decision.Action, AutoPermissionClassifierAllow) + } + if len(provider.request.Messages) != 2 { + t.Fatalf("provider got %d messages, want 2", len(provider.request.Messages)) + } + system := provider.request.Messages[0] + if system.Role != zeroruntime.MessageRoleSystem { + t.Fatalf("first message role = %q, want system", system.Role) + } + if !strings.Contains(system.Content, "Zero's auto-classifier permission reviewer") { + t.Fatalf("system prompt missing classifier identity: %q", system.Content) + } + if !strings.Contains(system.Content, "strict JSON only") { + t.Fatalf("system prompt missing response contract: %q", system.Content) + } + if strings.TrimSpace(autoPermissionClassifierSystemPrompt) == "" { + t.Fatal("embedded auto-classifier prompt is empty") + } + if len(provider.request.Tools) != 0 { + t.Fatalf("classifier request exposed %d tools, want none", len(provider.request.Tools)) + } +} + +func TestParseAutoPermissionClassifierDecision(t *testing.T) { + t.Run("valid allow", func(t *testing.T) { + decision, ok := parseAutoPermissionClassifierDecision(`{"action":"allow","reason":"safe"}`) + if !ok || decision.Action != AutoPermissionClassifierAllow || decision.Reason != "safe" { + t.Fatalf("parse = (%#v,%t), want allow/safe", decision, ok) + } + }) + t.Run("valid prompt", func(t *testing.T) { + decision, ok := parseAutoPermissionClassifierDecision(`{"action":"prompt","reason":"unsure"}`) + if !ok || decision.Action != AutoPermissionClassifierPrompt || decision.Reason != "unsure" { + t.Fatalf("parse = (%#v,%t), want prompt/unsure", decision, ok) + } + }) + t.Run("fenced json", func(t *testing.T) { + decision, ok := parseAutoPermissionClassifierDecision("```json\n{\"action\":\"allow\",\"reason\":\"safe\"}\n```") + if !ok || decision.Action != AutoPermissionClassifierAllow || decision.Reason != "safe" { + t.Fatalf("parse = (%#v,%t), want allow/safe", decision, ok) + } + }) + + rejects := map[string]string{ + "empty": ``, + "unknown action": `{"action":"maybe","reason":"x"}`, + "empty reason": `{"action":"allow","reason":""}`, + "trailing content": `{"action":"allow","reason":"x"} trailing`, + "unknown field": `{"action":"allow","reason":"x","extra":true}`, + "not an object": `"allow"`, + } + for name, output := range rejects { + t.Run(name, func(t *testing.T) { + if _, ok := parseAutoPermissionClassifierDecision(output); ok { + t.Fatalf("expected %q to be rejected", output) + } + }) + } +} diff --git a/internal/agent/command_prefix.go b/internal/agent/command_prefix.go index 0e92cdd09..185a02d8c 100644 --- a/internal/agent/command_prefix.go +++ b/internal/agent/command_prefix.go @@ -2,6 +2,7 @@ package agent import ( "fmt" + "path/filepath" "runtime" "strings" @@ -49,6 +50,73 @@ func proposedCommandPrefix(toolName string, args map[string]any) []string { return append([]string(nil), segments[0]...) } +// commandPrefixLadder returns the breadth choices offered for a shell command's +// prefix grant, ordered broadest → most specific. The most specific entry equals +// proposedCommandPrefix (today's default). Broader entries are shorter token +// prefixes of at least two tokens; a lone launcher/command token (e.g. "yarn") +// is never offered because a one-token grant is too broad — it would approve every +// later subcommand of that program (e.g. "yarn add", "yarn publish", arbitrary +// scripts), exactly the package-manager class that must stay non-grantable. When +// the final token carries a namespace separator (e.g. "test:unit") an intra-token +// wildcard level ("test:*") is inserted just before the exact one. Returns nil when +// there is nothing to choose between (zero or one safe level), so callers keep the +// single-prefix behavior. +func commandPrefixLadder(toolName string, args map[string]any) [][]string { + base := proposedCommandPrefix(toolName, args) + if len(base) == 0 { + return nil + } + ladder := make([][]string, 0, len(base)+1) + seen := map[string]bool{} + add := func(candidate []string) { + if len(candidate) == 0 || !sandbox.ValidCommandPrefix(candidate) { + return + } + key := strings.Join(candidate, "\x00") + if seen[key] { + return + } + seen[key] = true + ladder = append(ladder, append([]string(nil), candidate...)) + } + // Start at two tokens: a one-token prefix (base[:1]) is a bare launcher/command + // name, and granting it would approve every subcommand of that program, so it is + // never offered as a reusable breadth. + for length := 2; length < len(base); length++ { + add(base[:length]) + } + if wildcard, ok := intraTokenWildcardPrefix(base); ok { + add(wildcard) + } + add(base) + if len(ladder) <= 1 { + return nil + } + return ladder +} + +// intraTokenWildcardPrefix turns a base prefix whose final token has a namespace +// separator into a trailing-wildcard variant, e.g. ["npm","run","test:unit"] -> +// ["npm","run","test:*"]. It never wildcards a lone launcher token (a single-token +// base). ok is false when the final token has no usable separator. +func intraTokenWildcardPrefix(base []string) ([]string, bool) { + if len(base) < 2 { + return nil, false + } + last := base[len(base)-1] + // Use the LAST separator so a nested name keeps its deepest namespace segment + // (e.g. "test:unit:fast" -> "test:unit:*", not the broader "test:*"). + index := strings.LastIndexAny(last, ":-/.@") + if index <= 0 || index >= len(last)-1 { + // No separator, a leading separator, or a trailing separator — nothing + // meaningful to widen into a namespace wildcard. + return nil, false + } + wildcard := append([]string(nil), base[:len(base)-1]...) + wildcard = append(wildcard, last[:index+1]+"*") + return wildcard, true +} + // otherSegmentsKnownSafe reports whether every segment other than the one at // skip is known-safe on its own. func otherSegmentsKnownSafe(segments [][]string, skip int) bool { @@ -63,6 +131,35 @@ func otherSegmentsKnownSafe(segments [][]string, skip int) bool { return true } +// grantPrefixForDecision resolves which command prefix to grant. It honors the +// approver's breadth choice (decision.CommandPrefix) only when that choice is a +// valid prefix AND was one of the options the request offered; otherwise it falls +// back to the request's default prefix. Because the offered options were derived +// from this exact command, an offered choice already matches it — so a stale or +// overbroad selection can never widen the grant beyond what was presented. +func grantPrefixForDecision(request PermissionRequest, decision PermissionDecision) []string { + fallback := append([]string(nil), request.CommandPrefix...) + if len(decision.CommandPrefix) == 0 { + return fallback + } + if !sandbox.ValidCommandPrefix(decision.CommandPrefix) { + return fallback + } + if !commandPrefixOffered(request.CommandPrefixOptions, decision.CommandPrefix) { + return fallback + } + return append([]string(nil), decision.CommandPrefix...) +} + +func commandPrefixOffered(options [][]string, prefix []string) bool { + for _, option := range options { + if equalStringSlices(option, prefix) { + return true + } + } + return false +} + func matchCommandPrefix(toolName string, args map[string]any, options Options) (sandbox.CommandPrefixGrant, bool, bool) { if !isShellCommandTool(toolName) || options.Sandbox == nil { return sandbox.CommandPrefixGrant{}, false, false @@ -78,6 +175,16 @@ func matchCommandPrefix(toolName string, args map[string]any, options Options) ( if !ok { return sandbox.CommandPrefixGrant{}, false, false } + // A prefix grant that matches lets the command run unsandboxed (see + // shellExecutionArgsForApproval). `cd` is a known-safe segment, so a composite + // like `cd /other && go test` would otherwise honor a grant saved for THIS + // project yet execute in another directory outside it. Bind the grant to the + // effective directory: if a `cd` moves execution outside the workspace root (or + // to a target we cannot prove stays inside it), refuse the match so the command + // falls back to the normal sandboxed prompt instead of an out-of-scope bypass. + if !commandDirStaysWithinProject(segments, options.Sandbox.WorkspaceRoot()) { + return sandbox.CommandPrefixGrant{}, false, false + } var matched sandbox.CommandPrefixGrant matchedAny := false matchedSession := false @@ -108,6 +215,62 @@ func matchCommandPrefix(toolName string, args map[string]any, options Options) ( return sandbox.CommandPrefixGrant{}, false, false } +// commandDirStaysWithinProject reports whether a composite command's `cd` +// segments keep execution inside root. It starts at root and follows each `cd`; +// a target that resolves outside root, or one that cannot be resolved statically +// (no argument, `-`, `~`/home, an environment variable, a glob, or extra args), +// is treated as leaving the project so the caller refuses the unsandboxed grant. +// With no root there is no project to bind to, so any `cd` is rejected. +func commandDirStaysWithinProject(segments [][]string, root string) bool { + root = strings.TrimSpace(root) + effective := root + for _, tokens := range segments { + if len(tokens) == 0 || commandName(tokens[0]) != "cd" { + continue + } + if root == "" { + return false + } + target, ok := resolveCdTarget(tokens[1:], effective) + if !ok || !pathWithinRoot(target, root) { + return false + } + effective = target + } + return true +} + +// resolveCdTarget resolves a `cd` argument list to an absolute directory relative +// to cwd. ok is false for forms whose destination cannot be known statically. +func resolveCdTarget(args []string, cwd string) (string, bool) { + if len(args) != 1 { + return "", false // bare `cd` (home) or too many args + } + arg := args[0] + if arg == "" || arg == "-" || arg == "~" || strings.HasPrefix(arg, "~") { + return "", false // previous dir or home-relative: not statically knowable + } + if strings.ContainsAny(arg, "$*?[") { + return "", false // variable expansion or glob + } + if filepath.IsAbs(arg) { + return filepath.Clean(arg), true + } + return filepath.Clean(filepath.Join(cwd, arg)), true +} + +// pathWithinRoot reports whether target is root or a descendant of it. +func pathWithinRoot(target, root string) bool { + rel, err := filepath.Rel(root, target) + if err != nil { + return false + } + if rel == "." { + return true + } + return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) +} + func shellExecutionArgsForApproval(toolName string, args map[string]any, action PermissionDecisionAction, options Options) map[string]any { if !isShellCommandTool(toolName) || !shellPrefixApprovalBypassesSandbox(action) { return args @@ -127,7 +290,18 @@ func shellExecutionArgsForApproval(toolName string, args map[string]any, action } func shellPrefixApprovalBypassesSandbox(action PermissionDecisionAction) bool { - return action == PermissionDecisionAllowPrefix || action == PermissionDecisionAlwaysAllowPrefix + return isCommandPrefixDecision(action) +} + +// isCommandPrefixDecision reports whether action is one of the command-prefix +// grant tiers (session, project, or global). +func isCommandPrefixDecision(action PermissionDecisionAction) bool { + switch action { + case PermissionDecisionAllowPrefix, PermissionDecisionAllowPrefixProject, PermissionDecisionAlwaysAllowPrefix: + return true + default: + return false + } } func shellCommandRequiresEscalated(args map[string]any) bool { diff --git a/internal/agent/command_prefix_test.go b/internal/agent/command_prefix_test.go index 69508a67c..4bea575a8 100644 --- a/internal/agent/command_prefix_test.go +++ b/internal/agent/command_prefix_test.go @@ -1,6 +1,7 @@ package agent import ( + "path/filepath" "runtime" "testing" @@ -159,6 +160,37 @@ func TestMatchCommandPrefixCoversSegmentedCommandWithSafeTail(t *testing.T) { } } +func TestMatchCommandPrefixRejectsGrantWhenCdEscapesProject(t *testing.T) { + root := t.TempDir() + outside := t.TempDir() + store, err := sandbox.NewGrantStore(sandbox.StoreOptions{FilePath: filepath.Join(t.TempDir(), "grants.json")}) + if err != nil { + t.Fatalf("store: %v", err) + } + engine := sandbox.NewEngine(sandbox.EngineOptions{WorkspaceRoot: root, Store: store}) + if _, err := engine.GrantCommandPrefixForProject(sandbox.CommandPrefixInput{ToolName: "bash", Prefix: []string{"go", "test"}}); err != nil { + t.Fatalf("grant: %v", err) + } + + // `cd` into another project must not let the project-scoped `go test` grant + // authorize an unsandboxed run outside the granted project. + command := "cd " + outside + " && go test ./..." + if grant, ok, _ := matchCommandPrefix("bash", map[string]any{"command": command}, Options{Sandbox: engine}); ok { + t.Fatalf("expected no match for a cd that escapes the project, got %#v", grant) + } + + // A `cd` that stays inside the project still honors the grant. + inside := "cd sub && go test ./..." + if _, ok, _ := matchCommandPrefix("bash", map[string]any{"command": inside}, Options{Sandbox: engine}); !ok { + t.Fatal("expected a within-project cd to still match the grant") + } + + // A non-static cd target (home) cannot be proven in-project, so it is refused. + if _, ok, _ := matchCommandPrefix("bash", map[string]any{"command": "cd && go test ./..."}, Options{Sandbox: engine}); ok { + t.Fatal("expected a bare `cd` (home) to refuse the grant") + } +} + func TestKnownSafeCommandSegmentRejectsMsysProneOnWindows(t *testing.T) { if runtime.GOOS != "windows" { t.Skip("windows-only known-safe MSYS guard") @@ -176,6 +208,32 @@ func TestKnownSafeCommandSegmentRejectsMsysProneOnWindows(t *testing.T) { } } +func TestPersistCommandPrefixGrantScopedOrSessionFallsBackToSession(t *testing.T) { + // A store with no workspace root cannot scope a project grant, so the project + // path must fall back to a session grant instead of recording nothing — a later + // matching command then reuses the approval rather than prompting again. + store, err := sandbox.NewGrantStore(sandbox.StoreOptions{FilePath: filepath.Join(t.TempDir(), "grants.json")}) + if err != nil { + t.Fatalf("new grant store: %v", err) + } + engine := sandbox.NewEngine(sandbox.EngineOptions{Store: store}) + options := Options{Sandbox: engine} + + prefix := persistCommandPrefixGrantScopedOrSession(PermissionDecisionAllowPrefixProject, "bash", []string{"yarn", "test:unit"}, "reason", options) + if !equalStringSlices(prefix, []string{"yarn", "test:unit"}) { + t.Fatalf("fallback prefix = %#v, want [yarn test:unit]", prefix) + } + if _, ok := engine.LookupCommandPrefixForSession("bash", []string{"yarn", "test:unit"}); !ok { + t.Fatal("expected session grant recorded after project scope failed") + } + // Nothing was persisted at project/global scope. + if grants, err := store.ListCommandPrefixes(); err != nil { + t.Fatalf("list command prefixes: %v", err) + } else if len(grants) != 0 { + t.Fatalf("expected no persisted grant, got %#v", grants) + } +} + func TestMatchCommandPrefixRejectsUncoveredSegment(t *testing.T) { engine := sandbox.NewEngine(sandbox.EngineOptions{WorkspaceRoot: t.TempDir()}) engine.GrantCommandPrefixForSession("bash", []string{"ps", "aux"}) @@ -194,3 +252,87 @@ func TestProposedCommandPrefixRejectsRequestedUnsafeLauncherPrefix(t *testing.T) t.Fatalf("unsafe requested launcher prefix should be rejected, got %#v", got) } } + +func TestCommandPrefixLadderOffersBreadthChoices(t *testing.T) { + // test:unit has a namespace separator, so the ladder offers the intra-token + // wildcard alongside the exact prefix. The one-token rung ({"yarn"}) is never + // offered: a bare launcher grant would approve every later yarn subcommand. + got := commandPrefixLadder("bash", map[string]any{"command": "yarn test:unit"}) + want := [][]string{ + {"yarn", "test:*"}, + {"yarn", "test:unit"}, + } + if len(got) != len(want) { + t.Fatalf("ladder = %#v, want %#v", got, want) + } + for index := range want { + if !equalStringSlices(got[index], want[index]) { + t.Fatalf("ladder[%d] = %#v, want %#v", index, got[index], want[index]) + } + } +} + +func TestCommandPrefixLadderExcludesOneTokenRung(t *testing.T) { + // A three-token command still offers the two-token shorter breadth, but never + // the one-token launcher rung ({"docker"}). + got := commandPrefixLadder("bash", map[string]any{"command": "docker compose up"}) + want := [][]string{ + {"docker", "compose"}, + {"docker", "compose", "up"}, + } + if len(got) != len(want) { + t.Fatalf("ladder = %#v, want %#v", got, want) + } + for index := range want { + if !equalStringSlices(got[index], want[index]) { + t.Fatalf("ladder[%d] = %#v, want %#v", index, got[index], want[index]) + } + } +} + +func TestCommandPrefixLadderNilForSingleTokenCommand(t *testing.T) { + // A single-token prefix has no broader/narrower breadth to choose between. + if got := commandPrefixLadder("bash", map[string]any{"command": "go"}); got != nil { + t.Fatalf("expected no ladder for single-token command, got %#v", got) + } +} + +func TestIntraTokenWildcardPrefix(t *testing.T) { + got, ok := intraTokenWildcardPrefix([]string{"yarn", "test:unit"}) + if !ok || !equalStringSlices(got, []string{"yarn", "test:*"}) { + t.Fatalf("wildcard = %#v ok=%v, want [yarn test:*]", got, ok) + } + // A nested name keeps its deepest namespace segment (last separator wins). + if got, ok := intraTokenWildcardPrefix([]string{"yarn", "test:unit:fast"}); !ok || !equalStringSlices(got, []string{"yarn", "test:unit:*"}) { + t.Fatalf("nested wildcard = %#v ok=%v, want [yarn test:unit:*]", got, ok) + } + if _, ok := intraTokenWildcardPrefix([]string{"yarn", "test"}); ok { + t.Fatal("token without a separator must not produce a wildcard") + } + if _, ok := intraTokenWildcardPrefix([]string{"yarn"}); ok { + t.Fatal("a lone launcher token must never be wildcarded") + } +} + +func TestGrantPrefixForDecisionHonorsOfferedChoice(t *testing.T) { + request := PermissionRequest{ + CommandPrefix: []string{"yarn", "test:unit"}, + CommandPrefixOptions: [][]string{{"yarn", "test:*"}, {"yarn", "test:unit"}}, + } + // The intra-token wildcard breadth is honored. + if got := grantPrefixForDecision(request, PermissionDecision{CommandPrefix: []string{"yarn", "test:*"}}); !equalStringSlices(got, []string{"yarn", "test:*"}) { + t.Fatalf("expected wildcard breadth honored, got %#v", got) + } + // A one-token breadth is never offered, so it falls back to the default. + if got := grantPrefixForDecision(request, PermissionDecision{CommandPrefix: []string{"yarn"}}); !equalStringSlices(got, []string{"yarn", "test:unit"}) { + t.Fatalf("expected default prefix on unoffered one-token choice, got %#v", got) + } + // An empty choice falls back to the request default. + if got := grantPrefixForDecision(request, PermissionDecision{}); !equalStringSlices(got, []string{"yarn", "test:unit"}) { + t.Fatalf("expected default prefix on empty choice, got %#v", got) + } + // A choice that was never offered falls back to the default (no widening). + if got := grantPrefixForDecision(request, PermissionDecision{CommandPrefix: []string{"yarn", "install"}}); !equalStringSlices(got, []string{"yarn", "test:unit"}) { + t.Fatalf("expected default prefix on unoffered choice, got %#v", got) + } +} diff --git a/internal/agent/loop.go b/internal/agent/loop.go index 0b1f8dffb..95375ec8f 100644 --- a/internal/agent/loop.go +++ b/internal/agent/loop.go @@ -178,6 +178,9 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options) if permissionMode == "" { permissionMode = PermissionModeAuto } + if permissionMode == PermissionModeAutoClassifier && options.AutoPermissionClassifier == nil { + options.AutoPermissionClassifier = defaultAutoPermissionClassifier(provider) + } runPermissions := &permissionRunState{} options.runPermissions = runPermissions defer runPermissions.cleanup() @@ -1188,12 +1191,25 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal decision := options.Sandbox.Evaluate(ctx, sandboxRequest(call.Name, tool, args, permissionGranted, permissionMode, options)) preflightDecision = &decision } + var classifierAskReason string + if decision, ok := autoClassifierDecision(ctx, call, tool, args, permissionGranted, permissionMode, options, preflightDecision); ok { + if decision.Action == AutoPermissionClassifierAllow { + permissionGranted = true + decisionAction = PermissionDecisionAutoClassifierAllow + decisionReason = "auto-reviewed by LLM classifier: " + strings.TrimSpace(decision.Reason) + } else { + // Classifier chose to ask; remember why so the prompt (and session log) + // can show what made it decline to auto-approve. + classifierAskReason = strings.TrimSpace(decision.Reason) + } + } - if toolFound && options.OnPermissionRequest != nil && shouldRequestPermission(tool, args, permissionGranted, preflightDecision) { + if toolFound && options.OnPermissionRequest != nil && shouldRequestPermission(tool, args, permissionGranted, permissionMode, preflightDecision) { requestEvent, ok := buildPermissionEvent(call, tool, args, permissionGranted, permissionMode, options, preflightDecision) if !ok { requestEvent = fallbackPermissionEvent(call, tool, args, permissionMode, options) } + requestEvent.ClassifierReason = classifierAskReason request := permissionRequestFromEvent(requestEvent, args, options) decision, err := requestPermission(ctx, request, options) if err != nil { @@ -1263,13 +1279,14 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal } } case PermissionDecisionAllowPrefix: - if len(request.CommandPrefix) == 0 { + grantPrefix := grantPrefixForDecision(request, decision) + if len(grantPrefix) == 0 { emitDeniedPermission(options, call, requestEvent, decisionReason) return deniedPermissionResult(call, decisionReason, requestEvent), nil } permissionGranted = true requestEvent.DecisionAction = decision.Action - decisionCommandPrefix = append([]string(nil), request.CommandPrefix...) + decisionCommandPrefix = grantPrefix if options.Sandbox != nil && len(decisionCommandPrefix) > 0 { options.Sandbox.GrantCommandPrefixForSession(call.Name, decisionCommandPrefix) } @@ -1280,18 +1297,17 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal return deniedPermissionResult(call, reason, requestEvent), nil } permissionCleanups = append(permissionCleanups, cleanup) - case PermissionDecisionAlwaysAllowPrefix: - if len(request.CommandPrefix) == 0 { + case PermissionDecisionAllowPrefixProject, PermissionDecisionAlwaysAllowPrefix: + grantPrefix := grantPrefixForDecision(request, decision) + if len(grantPrefix) == 0 { emitDeniedPermission(options, call, requestEvent, decisionReason) return deniedPermissionResult(call, decisionReason, requestEvent), nil } permissionGranted = true requestEvent.DecisionAction = decision.Action - decisionCommandPrefix = append([]string(nil), request.CommandPrefix...) + decisionCommandPrefix = grantPrefix if options.Sandbox != nil && len(decisionCommandPrefix) > 0 { - if grant, err := persistCommandPrefixGrant(call.Name, decisionCommandPrefix, decisionReason, options); err == nil { - decisionCommandPrefix = append([]string(nil), grant.Prefix...) - } + decisionCommandPrefix = persistCommandPrefixGrantScopedOrSession(decision.Action, call.Name, decisionCommandPrefix, decisionReason, options) } cleanup, err := grantNetworkForSandboxPrompt(requestEvent, sandbox.PermissionGrantScopeTurn, options) if err != nil { @@ -1392,6 +1408,9 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal if decisionAction != "" { event.DecisionAction = decisionAction } + if classifierAskReason != "" { + event.ClassifierReason = classifierAskReason + } if len(decisionCommandPrefix) > 0 { event.CommandPrefix = append([]string(nil), decisionCommandPrefix...) } @@ -1474,21 +1493,20 @@ func maybeRetryUnsandboxedAfterSandboxRestriction(ctx context.Context, registry reason = request.Reason } switch decision.Action { - case PermissionDecisionAllow, PermissionDecisionAllowStrict, PermissionDecisionAllowForSession, PermissionDecisionAllowPrefix, PermissionDecisionAlwaysAllowPrefix: + case PermissionDecisionAllow, PermissionDecisionAllowStrict, PermissionDecisionAllowForSession, PermissionDecisionAllowPrefix, PermissionDecisionAllowPrefixProject, PermissionDecisionAlwaysAllowPrefix: prefix := []string(nil) - if decision.Action == PermissionDecisionAllowPrefix || decision.Action == PermissionDecisionAlwaysAllowPrefix { - if len(request.CommandPrefix) == 0 { + if isCommandPrefixDecision(decision.Action) { + prefix = grantPrefixForDecision(request, decision) + if len(prefix) == 0 { emitDeniedPermission(options, call, requestEvent, reason) denied := deniedPermissionResult(call, reason, requestEvent) return result, &denied, true, decision.Action, reason, nil, nil } - prefix = append([]string(nil), request.CommandPrefix...) if options.Sandbox != nil { - if decision.Action == PermissionDecisionAlwaysAllowPrefix { - if grant, err := persistCommandPrefixGrant(call.Name, prefix, reason, options); err == nil { - prefix = append([]string(nil), grant.Prefix...) - } - } else { + switch decision.Action { + case PermissionDecisionAllowPrefixProject, PermissionDecisionAlwaysAllowPrefix: + prefix = persistCommandPrefixGrantScopedOrSession(decision.Action, call.Name, prefix, reason, options) + default: options.Sandbox.GrantCommandPrefixForSession(call.Name, prefix) } } @@ -2084,8 +2102,11 @@ func effectivePermission(tool tools.Tool, args map[string]any) tools.Permission return tool.Safety().Permission } -func shouldRequestPermission(tool tools.Tool, args map[string]any, permissionGranted bool, decision *sandbox.Decision) bool { - if decision != nil && decision.Action == sandbox.ActionPrompt { +func shouldRequestPermission(tool tools.Tool, args map[string]any, permissionGranted bool, permissionMode PermissionMode, decision *sandbox.Decision) bool { + // A prompt decision asks the user unless something already granted this call + // — a matched command prefix or the auto-classifier's LLM approval both set + // permissionGranted, and either one is enough to run without re-asking. + if decision != nil && decision.Action == sandbox.ActionPrompt && !permissionGranted { return true } if tool.Safety().Permission != tools.PermissionPrompt { @@ -2109,6 +2130,82 @@ func shouldRequestPermission(tool tools.Tool, args map[string]any, permissionGra return true } +func autoClassifierDecision(ctx context.Context, call ToolCall, tool tools.Tool, args map[string]any, permissionGranted bool, permissionMode PermissionMode, options Options, decision *sandbox.Decision) (AutoPermissionClassifierDecision, bool) { + if !canAutoClassifierReview(tool, args, permissionGranted, permissionMode, options, decision) { + return AutoPermissionClassifierDecision{}, false + } + request := autoPermissionClassifierRequest(call, tool, args, permissionMode, options, decision) + classifierDecision, err := options.AutoPermissionClassifier(ctx, request) + if err != nil { + return AutoPermissionClassifierDecision{}, false + } + classifierDecision.Reason = strings.TrimSpace(classifierDecision.Reason) + if classifierDecision.Reason == "" { + return AutoPermissionClassifierDecision{}, false + } + // Return both outcomes so the caller can auto-approve an allow or surface the + // reason behind a prompt; ok reports only that the classifier ran and gave a + // usable decision, not that it approved. + return classifierDecision, true +} + +func canAutoClassifierReview(tool tools.Tool, args map[string]any, permissionGranted bool, permissionMode PermissionMode, options Options, decision *sandbox.Decision) bool { + if permissionMode != PermissionModeAutoClassifier || permissionGranted || decision == nil || options.AutoPermissionClassifier == nil { + return false + } + if tool.Safety().Permission != tools.PermissionPrompt { + return false + } + // Auto-classifier trades autonomy for an LLM check: it reviews only actions + // the sandbox would OTHERWISE prompt for, and may auto-approve the ones it + // judges safe. Actions the sandbox already auto-allows run without LLM + // friction (same as workspace-auto); denials are never eligible. + if decision.Action != sandbox.ActionPrompt { + return false + } + // Escalation, network, and destructive actions always ask, no matter how + // confident the LLM is — the classifier can never rescue these. + if shellCommandAdditionalPermissionsRequested(args) || shellCommandRequiresEscalated(args) { + return false + } + if sandboxDecisionRequiresExplicitPermission(decision) || decision.Reason == sandbox.ReasonEscalatedSandboxRequired { + return false + } + // A path block (e.g. out-of-workspace access) always asks — the classifier + // never widens the workspace boundary. + if decision.Block != nil { + return false + } + if sandbox.HasRiskCategory(decision.Risk, "destructive") || sandbox.HasRiskCategory(decision.Risk, "network") { + return false + } + return true +} + +func autoPermissionClassifierRequest(call ToolCall, tool tools.Tool, args map[string]any, permissionMode PermissionMode, options Options, decision *sandbox.Decision) AutoPermissionClassifierRequest { + safety := tool.Safety() + reason := safety.Reason + risk := sandbox.Classify(sandboxRequest(call.Name, tool, args, false, permissionMode, options)) + if decision != nil { + if decision.Reason != "" { + reason = decision.Reason + } + risk = decision.Risk + } + return AutoPermissionClassifierRequest{ + ToolCallID: call.ID, + ToolName: call.Name, + PermissionMode: permissionMode, + SideEffect: string(safety.SideEffect), + Reason: userFacingPermissionReason(call.Name, args, reason, safety.Reason), + Scope: permissionScope(call.Name, args), + Risk: risk, + Args: autoPermissionClassifierArgs(args), + SandboxReason: strings.TrimSpace(decision.Reason), + SandboxRisk: decision.Risk, + } +} + func sandboxDecisionRequiresExplicitPermission(decision *sandbox.Decision) bool { return decision != nil && decision.Action == sandbox.ActionPrompt && decision.Reason == sandbox.ReasonNetworkBlocked } @@ -2125,7 +2222,7 @@ func requestPermission(ctx context.Context, request PermissionRequest, options O func normalizePermissionDecisionAction(action PermissionDecisionAction) PermissionDecisionAction { switch action { - case PermissionDecisionAllow, PermissionDecisionAllowStrict, PermissionDecisionAllowForSession, PermissionDecisionAllowPrefix, PermissionDecisionAlwaysAllowPrefix, PermissionDecisionAlwaysAllow, PermissionDecisionCancel: + case PermissionDecisionAllow, PermissionDecisionAllowStrict, PermissionDecisionAutoClassifierAllow, PermissionDecisionAllowForSession, PermissionDecisionAllowPrefix, PermissionDecisionAllowPrefixProject, PermissionDecisionAlwaysAllowPrefix, PermissionDecisionAlwaysAllow, PermissionDecisionCancel: return action default: return PermissionDecisionDeny @@ -2409,6 +2506,46 @@ func persistCommandPrefixGrant(toolName string, prefix []string, reason string, }) } +// persistCommandPrefixGrantForProject persists a prefix grant scoped to the +// current workspace root, so it only matches inside this project. +func persistCommandPrefixGrantForProject(toolName string, prefix []string, reason string, options Options) (sandbox.CommandPrefixGrant, error) { + if options.Sandbox == nil { + return sandbox.CommandPrefixGrant{}, errors.New("sandbox engine is not configured") + } + return options.Sandbox.GrantCommandPrefixForProject(sandbox.CommandPrefixInput{ + ToolName: toolName, + Prefix: prefix, + Reason: reason, + }) +} + +// persistCommandPrefixGrantScoped persists a prefix grant at the scope the +// decision selected: project-only for AllowPrefixProject, global otherwise. +func persistCommandPrefixGrantScoped(action PermissionDecisionAction, toolName string, prefix []string, reason string, options Options) (sandbox.CommandPrefixGrant, error) { + if action == PermissionDecisionAllowPrefixProject { + return persistCommandPrefixGrantForProject(toolName, prefix, reason, options) + } + return persistCommandPrefixGrant(toolName, prefix, reason, options) +} + +// persistCommandPrefixGrantScopedOrSession persists the prefix at the decision's +// scope and returns the prefix to advertise on the decision. When scoped +// persistence fails — e.g. a project-scoped grant on an engine with no workspace +// root, or a missing grant store — it falls back to a session-scoped grant so a +// later matching command still reuses the approval instead of prompting again +// (the safer session fallback the engine documents). Returns the persisted grant's +// normalized prefix on success, otherwise the input prefix used for the fallback. +func persistCommandPrefixGrantScopedOrSession(action PermissionDecisionAction, toolName string, prefix []string, reason string, options Options) []string { + if options.Sandbox == nil { + return prefix + } + if grant, err := persistCommandPrefixGrantScoped(action, toolName, prefix, reason, options); err == nil { + return append([]string(nil), grant.Prefix...) + } + options.Sandbox.GrantCommandPrefixForSession(toolName, prefix) + return prefix +} + func emitDeniedPermission(options Options, call ToolCall, requestEvent PermissionEvent, reason string) { if options.OnPermission == nil { return @@ -2668,22 +2805,24 @@ func fallbackPermissionEvent(call ToolCall, tool tools.Tool, args map[string]any func permissionRequestFromEvent(event PermissionEvent, args map[string]any, options Options) PermissionRequest { return PermissionRequest{ - ToolCallID: event.ToolCallID, - ToolName: event.ToolName, - Action: event.Action, - Permission: event.Permission, - PermissionMode: event.PermissionMode, - Autonomy: event.Autonomy, - SideEffect: event.SideEffect, - Reason: event.Reason, - Scope: event.Scope, - Risk: event.Risk, - Args: cloneArgs(args), - Block: event.Block, - GrantMatched: event.GrantMatched, - Grant: event.Grant, - CommandPrefix: append([]string(nil), event.CommandPrefix...), - AvailableDecisions: availablePermissionDecisions(event, args, options), + ToolCallID: event.ToolCallID, + ToolName: event.ToolName, + Action: event.Action, + Permission: event.Permission, + PermissionMode: event.PermissionMode, + Autonomy: event.Autonomy, + SideEffect: event.SideEffect, + Reason: event.Reason, + Scope: event.Scope, + Risk: event.Risk, + Args: cloneArgs(args), + Block: event.Block, + GrantMatched: event.GrantMatched, + Grant: event.Grant, + CommandPrefix: append([]string(nil), event.CommandPrefix...), + AvailableDecisions: availablePermissionDecisions(event, args, options), + CommandPrefixOptions: commandPrefixLadder(event.ToolName, args), + ClassifierReason: event.ClassifierReason, } } @@ -2698,7 +2837,9 @@ func availablePermissionDecisions(event PermissionEvent, args map[string]any, op if isShellCommandTool(event.ToolName) && len(event.CommandPrefix) > 0 && !networkSandboxPrompt(event) && !inlineAdditionalPermissions { decisions = append(decisions, PermissionDecisionAllowPrefix) if options.Sandbox.CanPersistGrants() { - decisions = append(decisions, PermissionDecisionAlwaysAllowPrefix) + // Persisted prefix grants come in two scopes: this project only, and + // global (every project). Session (above) is the ephemeral third. + decisions = append(decisions, PermissionDecisionAllowPrefixProject, PermissionDecisionAlwaysAllowPrefix) } } if options.Sandbox.CanPersistGrants() && permissionSupportsPersistentDecision(event.ToolName) && !filesystemSandboxPrompt(event) && !inlineAdditionalPermissions { @@ -2855,6 +2996,59 @@ func cloneArgs(args map[string]any) map[string]any { return copied } +const autoPermissionClassifierStringLimit = 2048 + +func autoPermissionClassifierArgs(args map[string]any) map[string]any { + if len(args) == 0 { + return nil + } + copied := make(map[string]any, len(args)) + for key, value := range args { + copied[key] = autoPermissionClassifierValue(value) + } + return copied +} + +func autoPermissionClassifierValue(value any) any { + switch v := value.(type) { + case string: + return truncateClassifierString(v) + case []any: + out := make([]any, len(v)) + for i, item := range v { + out[i] = autoPermissionClassifierValue(item) + } + return out + case map[string]any: + out := make(map[string]any, len(v)) + for key, item := range v { + out[key] = autoPermissionClassifierValue(item) + } + return out + default: + return v + } +} + +func truncateClassifierString(value string) string { + // Scan runes in place rather than materializing the whole value as a []rune + // (which allocates ~4 bytes per rune for large tool arguments). Record the + // byte offset where the limit-th rune begins, then keep counting to report the + // exact truncated-rune total in the message. + count := 0 + cut := -1 + for offset := range value { + if count == autoPermissionClassifierStringLimit { + cut = offset + } + count++ + } + if count <= autoPermissionClassifierStringLimit { + return value + } + return value[:cut] + fmt.Sprintf("… [truncated %d chars]", count-autoPermissionClassifierStringLimit) +} + func permissionActionFromSandbox(action sandbox.Action) PermissionAction { switch action { case sandbox.ActionAllow: @@ -3121,12 +3315,12 @@ func ToolAdvertised(tool tools.Tool, permissionMode PermissionMode) bool { if permissionMode == PermissionModeAuto { return tool.Safety().Permission == tools.PermissionAllow || tool.Safety().AdvertiseInAuto } - if permissionMode == PermissionModeMemberAuto { - // Like Auto, plus the in-workspace mutators a headless member needs to - // build. The sandbox engine still decides at call time: in-workspace writes - // and sandbox-backed shell auto-allow, while out-of-workspace writes, - // network, and destructive commands prompt → denied headless. So this - // advertises capability without widening sandbox authority. + if permissionMode == PermissionModeWorkspaceAuto || permissionMode == PermissionModeAutoClassifier { + // Like Auto, plus the sandbox-safe in-workspace mutators needed to build. + // The sandbox engine still decides at call time: in-workspace writes and + // sandbox-backed shell auto-allow, while out-of-workspace writes, network, + // and destructive commands prompt → denied headless. So this advertises + // capability without widening sandbox authority. if tool.Safety().Permission == tools.PermissionAllow || tool.Safety().AdvertiseInAuto { return true } diff --git a/internal/agent/loop_test.go b/internal/agent/loop_test.go index 69913a420..8a0cd0888 100644 --- a/internal/agent/loop_test.go +++ b/internal/agent/loop_test.go @@ -1749,6 +1749,324 @@ func TestRunAllowsWorkspaceWriteWithoutPromptWhenSandboxPolicyPermits(t *testing } } +func TestRunAutoClassifierAllowsSandboxReviewedWorkspaceWrite(t *testing.T) { + root := t.TempDir() + registry := tools.NewRegistry() + registry.Register(tools.NewWriteFileTool(root)) + provider := providerCallingWritePathContentThenAnswer("notes.txt", strings.Repeat("x", autoPermissionClassifierStringLimit+12), "write done") + var permissionEvents []PermissionEvent + var classifierRequests []AutoPermissionClassifierRequest + + result, err := Run(context.Background(), "write notes", provider, Options{ + Registry: registry, + PermissionMode: PermissionModeAutoClassifier, + Autonomy: "medium", + // EnforceWorkspace off so the write is an ordinary prompt (not a silent + // auto-allow) — that is the case the classifier is meant to review. + Sandbox: sandbox.NewEngine(sandbox.EngineOptions{ + WorkspaceRoot: root, + Policy: sandbox.Policy{Mode: sandbox.ModeEnforce, Network: sandbox.NetworkDeny}, + }), + AutoPermissionClassifier: func(_ context.Context, request AutoPermissionClassifierRequest) (AutoPermissionClassifierDecision, error) { + classifierRequests = append(classifierRequests, request) + return AutoPermissionClassifierDecision{Action: AutoPermissionClassifierAllow, Reason: "workspace write is low risk"}, nil + }, + OnPermissionRequest: func(context.Context, PermissionRequest) (PermissionDecision, error) { + t.Fatal("auto-classifier should not request permission for LLM-reviewed workspace write") + return PermissionDecision{}, nil + }, + OnPermission: func(event PermissionEvent) { + permissionEvents = append(permissionEvents, event) + }, + }) + + if err != nil { + t.Fatal(err) + } + if result.FinalAnswer != "write done" { + t.Fatalf("expected final answer, got %q", result.FinalAnswer) + } + if len(classifierRequests) != 1 { + t.Fatalf("expected classifier to be invoked once, got %#v", classifierRequests) + } + if classifierRequests[0].ToolName != "write_file" || classifierRequests[0].SandboxReason != "Creates or overwrites files." { + t.Fatalf("unexpected classifier request: %#v", classifierRequests[0]) + } + contentArg, _ := classifierRequests[0].Args["content"].(string) + if len([]rune(contentArg)) <= autoPermissionClassifierStringLimit || !strings.Contains(contentArg, "[truncated 12 chars]") { + suffix := contentArg + if n := len(suffix); n > 32 { + suffix = suffix[n-32:] + } + t.Fatalf("expected classifier request args to be bounded, got content len=%d value suffix=%q", len([]rune(contentArg)), suffix) + } + if len(permissionEvents) != 1 { + t.Fatalf("expected one auto-classifier permission event, got %#v", permissionEvents) + } + event := permissionEvents[0] + if event.Action != PermissionActionAllow || !event.PermissionGranted || event.DecisionAction != PermissionDecisionAutoClassifierAllow { + t.Fatalf("expected auto-classifier allow event, got %#v", event) + } + if event.Reason != "Creates or overwrites files." { + t.Fatalf("expected sandbox reason, got %#v", event) + } + if event.DecisionReason != "auto-reviewed by LLM classifier: workspace write is low risk" { + t.Fatalf("expected LLM classifier decision reason, got %#v", event) + } +} + +func TestRunAutoClassifierPromptFallsBackToPermissionRequest(t *testing.T) { + root := t.TempDir() + registry := tools.NewRegistry() + registry.Register(tools.NewWriteFileTool(root)) + provider := providerCallingWriteFileThenAnswer("write approved") + classifierCalls := 0 + var requests []PermissionRequest + var permissionEvents []PermissionEvent + + result, err := Run(context.Background(), "write notes", provider, Options{ + Registry: registry, + PermissionMode: PermissionModeAutoClassifier, + Sandbox: sandbox.NewEngine(sandbox.EngineOptions{ + WorkspaceRoot: root, + Policy: sandbox.Policy{Mode: sandbox.ModeEnforce, Network: sandbox.NetworkDeny}, + }), + AutoPermissionClassifier: func(context.Context, AutoPermissionClassifierRequest) (AutoPermissionClassifierDecision, error) { + classifierCalls++ + return AutoPermissionClassifierDecision{Action: AutoPermissionClassifierPrompt, Reason: "ask the user"}, nil + }, + OnPermissionRequest: func(_ context.Context, request PermissionRequest) (PermissionDecision, error) { + requests = append(requests, request) + return PermissionDecision{Action: PermissionDecisionAllow, Reason: "approved after classifier prompt"}, nil + }, + OnPermission: func(event PermissionEvent) { + permissionEvents = append(permissionEvents, event) + }, + }) + + if err != nil { + t.Fatal(err) + } + if result.FinalAnswer != "write approved" { + t.Fatalf("expected final answer, got %q", result.FinalAnswer) + } + if classifierCalls != 1 { + t.Fatalf("expected one classifier call, got %d", classifierCalls) + } + if len(requests) != 1 || requests[0].ToolName != "write_file" { + t.Fatalf("expected fallback permission request, got %#v", requests) + } + // The classifier's reason for declining to auto-approve is surfaced on the + // prompt and recorded on the emitted permission event. + if requests[0].ClassifierReason != "ask the user" { + t.Fatalf("expected classifier reason on permission request, got %q", requests[0].ClassifierReason) + } + if len(permissionEvents) != 1 || permissionEvents[0].ClassifierReason != "ask the user" { + t.Fatalf("expected classifier reason on permission event, got %#v", permissionEvents) + } + if content, err := os.ReadFile(filepath.Join(root, "notes.txt")); err != nil || string(content) != "hello" { + t.Fatalf("expected fallback approval to run write, content=%q err=%v", content, err) + } +} + +func TestRunAutoClassifierErrorFallsBackToPermissionRequest(t *testing.T) { + root := t.TempDir() + registry := tools.NewRegistry() + registry.Register(tools.NewWriteFileTool(root)) + provider := providerCallingWriteFileThenAnswer("write approved") + var requests []PermissionRequest + + result, err := Run(context.Background(), "write notes", provider, Options{ + Registry: registry, + PermissionMode: PermissionModeAutoClassifier, + Sandbox: sandbox.NewEngine(sandbox.EngineOptions{ + WorkspaceRoot: root, + Policy: sandbox.Policy{Mode: sandbox.ModeEnforce, Network: sandbox.NetworkDeny}, + }), + AutoPermissionClassifier: func(context.Context, AutoPermissionClassifierRequest) (AutoPermissionClassifierDecision, error) { + return AutoPermissionClassifierDecision{}, errors.New("classifier unavailable") + }, + OnPermissionRequest: func(_ context.Context, request PermissionRequest) (PermissionDecision, error) { + requests = append(requests, request) + return PermissionDecision{Action: PermissionDecisionAllow, Reason: "approved after classifier error"}, nil + }, + }) + + if err != nil { + t.Fatal(err) + } + if result.FinalAnswer != "write approved" { + t.Fatalf("expected final answer, got %q", result.FinalAnswer) + } + if len(requests) != 1 || requests[0].ToolName != "write_file" { + t.Fatalf("expected fallback permission request, got %#v", requests) + } + if content, err := os.ReadFile(filepath.Join(root, "notes.txt")); err != nil || string(content) != "hello" { + t.Fatalf("expected fallback approval to run write, content=%q err=%v", content, err) + } +} + +func TestRunDefaultAutoClassifierInvalidJSONFallsBackToPermissionRequest(t *testing.T) { + root := t.TempDir() + registry := tools.NewRegistry() + registry.Register(tools.NewWriteFileTool(root)) + provider := providerCallingWriteFileThenClassifyThenAnswer("not json", "write approved") + var requests []PermissionRequest + + result, err := Run(context.Background(), "write notes", provider, Options{ + Registry: registry, + PermissionMode: PermissionModeAutoClassifier, + Sandbox: sandbox.NewEngine(sandbox.EngineOptions{ + WorkspaceRoot: root, + Policy: sandbox.Policy{Mode: sandbox.ModeEnforce, Network: sandbox.NetworkDeny}, + }), + OnPermissionRequest: func(_ context.Context, request PermissionRequest) (PermissionDecision, error) { + requests = append(requests, request) + return PermissionDecision{Action: PermissionDecisionAllow, Reason: "approved after invalid classifier output"}, nil + }, + }) + + if err != nil { + t.Fatal(err) + } + if result.FinalAnswer != "write approved" { + t.Fatalf("expected final answer, got %q", result.FinalAnswer) + } + if len(requests) != 1 || requests[0].ToolName != "write_file" { + t.Fatalf("expected fallback permission request, got %#v", requests) + } + if len(provider.requests) < 2 || len(provider.requests[1].Tools) != 0 { + t.Fatalf("classifier provider request should carry no tools, got %#v", provider.requests) + } +} + +func TestRunDefaultAutoClassifierStrictJSONAllowsWorkspaceWrite(t *testing.T) { + root := t.TempDir() + registry := tools.NewRegistry() + registry.Register(tools.NewWriteFileTool(root)) + provider := providerCallingWriteFileThenClassifyThenAnswer(`{"action":"allow","reason":"safe workspace note"}`, "write done") + var permissionEvents []PermissionEvent + + result, err := Run(context.Background(), "write notes", provider, Options{ + Registry: registry, + PermissionMode: PermissionModeAutoClassifier, + Sandbox: sandbox.NewEngine(sandbox.EngineOptions{ + WorkspaceRoot: root, + Policy: sandbox.Policy{Mode: sandbox.ModeEnforce, Network: sandbox.NetworkDeny}, + }), + OnPermissionRequest: func(context.Context, PermissionRequest) (PermissionDecision, error) { + t.Fatal("default classifier allow should skip permission request") + return PermissionDecision{}, nil + }, + OnPermission: func(event PermissionEvent) { + permissionEvents = append(permissionEvents, event) + }, + }) + + if err != nil { + t.Fatal(err) + } + if result.FinalAnswer != "write done" { + t.Fatalf("expected final answer, got %q", result.FinalAnswer) + } + if len(provider.requests) < 2 || len(provider.requests[1].Tools) != 0 { + t.Fatalf("classifier provider request should carry no tools, got %#v", provider.requests) + } + if got := provider.requests[1].Messages[len(provider.requests[1].Messages)-1].Content; !strings.Contains(got, `"content":"hello"`) { + t.Fatalf("classifier request payload missing bounded args: %s", got) + } + if len(permissionEvents) != 1 || permissionEvents[0].DecisionReason != "auto-reviewed by LLM classifier: safe workspace note" { + t.Fatalf("expected LLM auto-reviewed permission event, got %#v", permissionEvents) + } +} + +func TestRunAutoClassifierNotCalledForSandboxPrompt(t *testing.T) { + root := t.TempDir() + registry := tools.NewRegistry() + registry.Register(tools.NewBashTool(root)) + provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{ + { + {Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-1", ToolName: "bash"}, + {Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{"command":"curl https://example.com"}`}, + {Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call-1"}, + {Type: zeroruntime.StreamEventDone}, + }, + { + {Type: zeroruntime.StreamEventText, Content: "done"}, + {Type: zeroruntime.StreamEventDone}, + }, + }} + var requests []PermissionRequest + + _, err := Run(context.Background(), "fetch", provider, Options{ + Registry: registry, + PermissionMode: PermissionModeAutoClassifier, + Sandbox: sandbox.NewEngine(sandbox.EngineOptions{ + WorkspaceRoot: root, + Policy: sandbox.DefaultPolicy(), + }), + AutoPermissionClassifier: func(context.Context, AutoPermissionClassifierRequest) (AutoPermissionClassifierDecision, error) { + t.Fatal("classifier must not be called for sandbox prompt") + return AutoPermissionClassifierDecision{}, nil + }, + OnPermissionRequest: func(_ context.Context, request PermissionRequest) (PermissionDecision, error) { + requests = append(requests, request) + return PermissionDecision{Action: PermissionDecisionDeny, Reason: "network requires prompt"}, nil + }, + }) + + if err != nil { + t.Fatal(err) + } + if len(requests) != 1 || requests[0].ToolName != "bash" || requests[0].Action != PermissionActionPrompt { + t.Fatalf("expected sandbox prompt permission request, got %#v", requests) + } +} + +func TestRunAutoClassifierNotCalledForInlineAdditionalPermissions(t *testing.T) { + root := t.TempDir() + registry := tools.NewRegistry() + registry.Register(tools.NewBashTool(root)) + provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{ + { + {Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-1", ToolName: "bash"}, + {Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{"command":"echo hi","sandbox_permissions":"with_additional_permissions","additional_permissions":{"network":{"enabled":true}}}`}, + {Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call-1"}, + {Type: zeroruntime.StreamEventDone}, + }, + { + {Type: zeroruntime.StreamEventText, Content: "done"}, + {Type: zeroruntime.StreamEventDone}, + }, + }} + var requests []PermissionRequest + + _, err := Run(context.Background(), "write", provider, Options{ + Registry: registry, + PermissionMode: PermissionModeAutoClassifier, + Cwd: root, + Sandbox: sandbox.NewEngine(sandbox.EngineOptions{ + WorkspaceRoot: root, + Policy: sandbox.DefaultPolicy(), + }), + AutoPermissionClassifier: func(context.Context, AutoPermissionClassifierRequest) (AutoPermissionClassifierDecision, error) { + t.Fatal("classifier must not be called for inline additional permissions") + return AutoPermissionClassifierDecision{}, nil + }, + OnPermissionRequest: func(_ context.Context, request PermissionRequest) (PermissionDecision, error) { + requests = append(requests, request) + return PermissionDecision{Action: PermissionDecisionDeny, Reason: "additional permissions require prompt"}, nil + }, + }) + + if err != nil { + t.Fatal(err) + } + if len(requests) != 1 || requests[0].ToolName != "bash" || requests[0].Action != PermissionActionPrompt { + t.Fatalf("expected additional-permission prompt request, got %#v", requests) + } +} + func TestRunDeniesPromptToolWhenPermissionRequestDenied(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() @@ -3019,12 +3337,37 @@ func providerCallingWriteFileThenAnswer(answer string) *mockProvider { return providerCallingWritePathThenAnswer("notes.txt", answer) } +func providerCallingWriteFileThenClassifyThenAnswer(classifierOutput string, answer string) *mockProvider { + return &mockProvider{ + turns: [][]zeroruntime.StreamEvent{ + { + {Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-1", ToolName: "write_file"}, + {Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{"path":"notes.txt","content":"hello"}`}, + {Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call-1"}, + {Type: zeroruntime.StreamEventDone}, + }, + { + {Type: zeroruntime.StreamEventText, Content: classifierOutput}, + {Type: zeroruntime.StreamEventDone}, + }, + { + {Type: zeroruntime.StreamEventText, Content: answer}, + {Type: zeroruntime.StreamEventDone}, + }, + }, + } +} + func providerCallingWritePathThenAnswer(path string, answer string) *mockProvider { + return providerCallingWritePathContentThenAnswer(path, "hello", answer) +} + +func providerCallingWritePathContentThenAnswer(path string, content string, answer string) *mockProvider { return &mockProvider{ turns: [][]zeroruntime.StreamEvent{ { {Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-1", ToolName: "write_file"}, - {Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{"path":` + quoteJSONString(path) + `,"content":"hello"}`}, + {Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{"path":` + quoteJSONString(path) + `,"content":` + quoteJSONString(content) + `}`}, {Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call-1"}, {Type: zeroruntime.StreamEventDone}, }, diff --git a/internal/agent/member_auto_test.go b/internal/agent/member_auto_test.go index 9ce43849d..6e0cc54cb 100644 --- a/internal/agent/member_auto_test.go +++ b/internal/agent/member_auto_test.go @@ -22,10 +22,10 @@ func (t safetyTool) Run(context.Context, map[string]any) tools.Result { return tools.Result{Status: tools.StatusOK} } -// member-auto advertises the in-workspace mutators a headless member needs to -// build (write/edit + shell) that plain Auto hides, but NOT network or denied -// tools — the sandbox still gates the advertised ones at call time. -func TestToolAdvertisedMemberAuto(t *testing.T) { +// workspace-auto advertises the in-workspace mutators needed to build +// (write/edit + shell) that plain Auto hides, but NOT network or denied tools — +// the sandbox still gates the advertised ones at call time. +func TestToolAdvertisedWorkspaceAuto(t *testing.T) { write := safetyTool{name: "write_file", safety: tools.Safety{SideEffect: tools.SideEffectWrite, Permission: tools.PermissionPrompt}} shell := safetyTool{name: "bash", safety: tools.Safety{SideEffect: tools.SideEffectShell, Permission: tools.PermissionPrompt}} read := safetyTool{name: "read_file", safety: tools.Safety{SideEffect: tools.SideEffectRead, Permission: tools.PermissionAllow}} @@ -38,14 +38,14 @@ func TestToolAdvertisedMemberAuto(t *testing.T) { } for _, tool := range []tools.Tool{write, shell, read} { - if !ToolAdvertised(tool, PermissionModeMemberAuto) { - t.Fatalf("member-auto must advertise %q", tool.Name()) + if !ToolAdvertised(tool, PermissionModeWorkspaceAuto) { + t.Fatalf("workspace-auto must advertise %q", tool.Name()) } } - if ToolAdvertised(network, PermissionModeMemberAuto) { - t.Fatal("member-auto must NOT advertise a network prompt tool") + if ToolAdvertised(network, PermissionModeWorkspaceAuto) { + t.Fatal("workspace-auto must NOT advertise a network prompt tool") } - if ToolAdvertised(denied, PermissionModeMemberAuto) { - t.Fatal("member-auto must NOT advertise a denied tool") + if ToolAdvertised(denied, PermissionModeWorkspaceAuto) { + t.Fatal("workspace-auto must NOT advertise a denied tool") } } diff --git a/internal/agent/permission_additional_perms_test.go b/internal/agent/permission_additional_perms_test.go index 44969a74e..0b32d07df 100644 --- a/internal/agent/permission_additional_perms_test.go +++ b/internal/agent/permission_additional_perms_test.go @@ -47,7 +47,7 @@ func TestBuildPermissionEventPromptsForAdditionalPermissions(t *testing.T) { t.Fatalf("Action = %q, want prompt — an additional-permissions elevation must ask the user, not auto-allow", event.Action) } // Sanity: shouldRequestPermission agrees the loop blocks on this. - if !shouldRequestPermission(promptShellTool{}, args, false, decision) { + if !shouldRequestPermission(promptShellTool{}, args, false, PermissionModeAsk, decision) { t.Fatal("shouldRequestPermission must be true for an additional-permissions request") } } @@ -64,7 +64,7 @@ func TestBuildPermissionEventKeepsAllowForOrdinaryAllowedCommand(t *testing.T) { if event.Action == PermissionActionPrompt { t.Fatalf("Action = prompt for an ordinary allowed command; want allow (no spurious prompt)") } - if shouldRequestPermission(promptShellTool{}, args, false, decision) { + if shouldRequestPermission(promptShellTool{}, args, false, PermissionModeAsk, decision) { t.Fatal("shouldRequestPermission must be false for an ordinary sandbox-allowed command") } } @@ -80,7 +80,7 @@ func TestBuildPermissionEventReusesCoveredAdditionalPermissions(t *testing.T) { } decision := &sandbox.Decision{Action: sandbox.ActionAllow, AutoAllowed: true} - if shouldRequestPermission(promptShellTool{}, args, true, decision) { + if shouldRequestPermission(promptShellTool{}, args, true, PermissionModeAsk, decision) { t.Fatal("an already-covered capability must not trigger another prompt") } event, ok := buildPermissionEvent(call, promptShellTool{}, args, true, PermissionModeAsk, Options{}, decision) diff --git a/internal/agent/request_permissions_test.go b/internal/agent/request_permissions_test.go index fb12d6e73..95c062147 100644 --- a/internal/agent/request_permissions_test.go +++ b/internal/agent/request_permissions_test.go @@ -180,7 +180,7 @@ func TestInlineAdditionalPermissionsRequiresPromptUnderAutoAllow(t *testing.T) { }, } decision := &sandbox.Decision{Action: sandbox.ActionAllow, AutoAllowed: true} - if !shouldRequestPermission(tools.NewBashTool(t.TempDir()), args, false, decision) { + if !shouldRequestPermission(tools.NewBashTool(t.TempDir()), args, false, PermissionModeAuto, decision) { t.Fatal("inline additional permissions must request approval even when a sandboxed shell would otherwise auto-allow") } } diff --git a/internal/agent/types.go b/internal/agent/types.go index 0e3391b9e..42f9bfbe9 100644 --- a/internal/agent/types.go +++ b/internal/agent/types.go @@ -21,22 +21,88 @@ type PermissionMode string type PermissionAction string type PermissionDecisionAction string +type PermissionModeInfo struct { + ID PermissionMode + Label string + Summary string + Description string + Unsafe bool +} + const ( - PermissionModeAuto PermissionMode = "auto" - PermissionModeAsk PermissionMode = "ask" - PermissionModeUnsafe PermissionMode = "unsafe" - PermissionModeSpecDraft PermissionMode = "spec-draft" - // PermissionModeMemberAuto is a headless mode for swarm/specialist MEMBERS: it - // advertises the in-workspace mutators a member needs to build (write/edit + - // shell) on top of the Auto set, while the sandbox engine still gates them at - // call time — in-workspace writes and sandbox-backed shell auto-allow, but - // out-of-workspace writes, network, and destructive commands still prompt (and - // a headless member has no approver, so they are denied). It normalizes to Auto - // everywhere except ToolAdvertised, so authority is never widened beyond what an - // interactive auto agent already has inside the sandbox. - PermissionModeMemberAuto PermissionMode = "member-auto" + PermissionModeAuto PermissionMode = "auto" + PermissionModeAsk PermissionMode = "ask" + PermissionModeAutoClassifier PermissionMode = "auto-classifier" + PermissionModeUnsafe PermissionMode = "unsafe" + PermissionModeSpecDraft PermissionMode = "spec-draft" + // PermissionModeWorkspaceAuto advertises sandbox-safe in-workspace mutators + // (write/edit + shell) on top of the Auto set, while the sandbox engine still + // gates them at call time — in-workspace writes and sandbox-backed shell + // auto-allow, but out-of-workspace writes, network, and destructive commands + // still prompt (and a headless member has no approver, so they are denied). It + // normalizes to Auto everywhere except ToolAdvertised, so authority is never + // widened beyond what an interactive auto agent already has inside the sandbox. + PermissionModeWorkspaceAuto PermissionMode = "workspace-auto" + // PermissionModeMemberAuto is a legacy alias for the old internal name. + PermissionModeMemberAuto PermissionMode = PermissionModeWorkspaceAuto ) +func PermissionModeInfoFor(mode PermissionMode) PermissionModeInfo { + switch mode { + case PermissionModeAsk: + return PermissionModeInfo{ + ID: PermissionModeAsk, + Label: "Ask every time", + Summary: "ask every time", + Description: "Ask before every tool that can change state; read-only actions can still run without a prompt.", + } + case PermissionModeUnsafe: + return PermissionModeInfo{ + ID: PermissionModeUnsafe, + Label: "Dangerously skip permissions (YOLO mode)", + Summary: "unsafe YOLO", + Description: "Skip permission prompts for prompt-gated tools. Sandbox and hard safety blocks may still apply.", + Unsafe: true, + } + case PermissionModeSpecDraft: + return PermissionModeInfo{ + ID: PermissionModeSpecDraft, + Label: "Spec draft", + Summary: "spec draft", + Description: "Draft a spec first and stop for review before implementation tools are advertised.", + } + case PermissionModeWorkspaceAuto: + return PermissionModeInfo{ + ID: PermissionModeWorkspaceAuto, + Label: "Auto-approve trusted workspace", + Summary: "trusted workspace", + Description: "Allow sandbox-safe workspace edits and shell; prompt-required escalation is still reviewed or denied when no approver is available.", + } + case PermissionModeAutoClassifier: + return PermissionModeInfo{ + ID: PermissionModeAutoClassifier, + Label: "Let Zero cook (auto reviewed)", + Summary: "let Zero cook (auto reviewed)", + Description: "Auto-run low-risk workspace actions only after an LLM classifier reviews each one; anything the classifier is unsure about, plus high-risk actions, still ask.", + } + default: + return PermissionModeInfo{ + ID: PermissionModeAuto, + Label: "Auto-approve safe actions", + Summary: "auto-approve safe actions", + Description: "Run read-only and sandbox-safe actions automatically; ask before risky commands, network, installs, destructive actions, or access outside the granted workspace.", + } + } +} + +func PermissionModeLabel(mode PermissionMode) string { + return PermissionModeInfoFor(mode).Label +} + +func PermissionModeSummary(mode PermissionMode) string { + return PermissionModeInfoFor(mode).Summary +} + type StopReason string const ( @@ -51,14 +117,16 @@ const ( ) const ( - PermissionDecisionAllow PermissionDecisionAction = "allow" - PermissionDecisionAllowStrict PermissionDecisionAction = "allow_with_strict_auto_review" - PermissionDecisionAllowForSession PermissionDecisionAction = "allow_for_session" - PermissionDecisionAllowPrefix PermissionDecisionAction = "allow_prefix_for_session" - PermissionDecisionAlwaysAllowPrefix PermissionDecisionAction = "always_allow_prefix" - PermissionDecisionDeny PermissionDecisionAction = "deny" - PermissionDecisionAlwaysAllow PermissionDecisionAction = "always_allow" - PermissionDecisionCancel PermissionDecisionAction = "cancel" + PermissionDecisionAllow PermissionDecisionAction = "allow" + PermissionDecisionAllowStrict PermissionDecisionAction = "allow_with_strict_auto_review" + PermissionDecisionAutoClassifierAllow PermissionDecisionAction = "auto_classifier_allow" + PermissionDecisionAllowForSession PermissionDecisionAction = "allow_for_session" + PermissionDecisionAllowPrefix PermissionDecisionAction = "allow_prefix_for_session" + PermissionDecisionAllowPrefixProject PermissionDecisionAction = "always_allow_prefix_for_project" + PermissionDecisionAlwaysAllowPrefix PermissionDecisionAction = "always_allow_prefix" + PermissionDecisionDeny PermissionDecisionAction = "deny" + PermissionDecisionAlwaysAllow PermissionDecisionAction = "always_allow" + PermissionDecisionCancel PermissionDecisionAction = "cancel" ) type ToolResult struct { @@ -173,13 +241,52 @@ type PermissionRequest struct { Grant *sandbox.Grant `json:"grant,omitempty"` CommandPrefix []string `json:"commandPrefix,omitempty"` AvailableDecisions []PermissionDecisionAction `json:"availableDecisions,omitempty"` + // CommandPrefixOptions offers breadth choices for a prefix grant, ordered + // broadest → most specific (the last entry equals CommandPrefix). Empty when + // there is only one safe prefix, so the approver just grants CommandPrefix. + CommandPrefixOptions [][]string `json:"commandPrefixOptions,omitempty"` + // ClassifierReason is the LLM auto-classifier's explanation for why this call + // is being prompted instead of auto-approved (auto-classifier mode only). + ClassifierReason string `json:"classifierReason,omitempty"` } type PermissionDecision struct { Action PermissionDecisionAction `json:"action"` Reason string `json:"reason,omitempty"` + // CommandPrefix, when set on an allow-prefix decision, is the breadth the + // approver chose from PermissionRequest.CommandPrefixOptions. Empty means use + // the request's default CommandPrefix. The loop validates it still matches the + // command before granting, so a stale or overbroad choice cannot widen a grant. + CommandPrefix []string `json:"commandPrefix,omitempty"` +} + +type AutoPermissionClassifierAction string + +const ( + AutoPermissionClassifierAllow AutoPermissionClassifierAction = "allow" + AutoPermissionClassifierPrompt AutoPermissionClassifierAction = "prompt" +) + +type AutoPermissionClassifierRequest struct { + ToolCallID string `json:"toolCallId"` + ToolName string `json:"name"` + PermissionMode PermissionMode `json:"permissionMode"` + SideEffect string `json:"sideEffect"` + Reason string `json:"reason,omitempty"` + Scope string `json:"scope,omitempty"` + Risk sandbox.Risk `json:"risk"` + Args map[string]any `json:"args,omitempty"` + SandboxReason string `json:"sandboxReason,omitempty"` + SandboxRisk sandbox.Risk `json:"sandboxRisk"` } +type AutoPermissionClassifierDecision struct { + Action AutoPermissionClassifierAction `json:"action"` + Reason string `json:"reason,omitempty"` +} + +type AutoPermissionClassifier func(context.Context, AutoPermissionClassifierRequest) (AutoPermissionClassifierDecision, error) + type PermissionEvent struct { ToolCallID string `json:"toolCallId"` ToolName string `json:"name"` @@ -193,11 +300,15 @@ type PermissionEvent struct { Reason string `json:"reason,omitempty"` Scope string `json:"scope,omitempty"` DecisionReason string `json:"decisionReason,omitempty"` - Risk sandbox.Risk `json:"risk"` - Block *sandbox.Block `json:"block,omitempty"` - GrantMatched bool `json:"grantMatched,omitempty"` - Grant *sandbox.Grant `json:"grant,omitempty"` - CommandPrefix []string `json:"commandPrefix,omitempty"` + // ClassifierReason is the LLM auto-classifier's explanation for why it asked + // the user instead of auto-approving (auto-classifier mode only). Empty for + // every other path, including a classifier auto-approval. + ClassifierReason string `json:"classifierReason,omitempty"` + Risk sandbox.Risk `json:"risk"` + Block *sandbox.Block `json:"block,omitempty"` + GrantMatched bool `json:"grantMatched,omitempty"` + Grant *sandbox.Grant `json:"grant,omitempty"` + CommandPrefix []string `json:"commandPrefix,omitempty"` } // AskUserQuestion is one clarifying question the agent wants answered. Options are @@ -319,13 +430,14 @@ type Options struct { // OnToolCallDelta for each argument fragment. A surface can render the // in-progress call (e.g. a file being written) instead of waiting for // OnToolCall, which only fires once the whole call has accumulated. nil no-ops. - OnToolCallStart func(id, name string) - OnToolCallDelta func(id, fragment string) - OnPermissionRequest func(context.Context, PermissionRequest) (PermissionDecision, error) - OnPermission func(PermissionEvent) - OnAskUser func(context.Context, AskUserRequest) (AskUserResponse, error) - OnToolResult func(ToolResult) - OnUsage func(Usage) + OnToolCallStart func(id, name string) + OnToolCallDelta func(id, fragment string) + OnPermissionRequest func(context.Context, PermissionRequest) (PermissionDecision, error) + AutoPermissionClassifier AutoPermissionClassifier + OnPermission func(PermissionEvent) + OnAskUser func(context.Context, AskUserRequest) (AskUserResponse, error) + OnToolResult func(ToolResult) + OnUsage func(Usage) // OnToolProgress, when set, is called with each stream-json event a // specialist child process emits while running. The toolCallID identifies // which Task tool call the progress belongs to. nil is a no-op. diff --git a/internal/cli/exec_test.go b/internal/cli/exec_test.go index a277d553b..badc9069d 100644 --- a/internal/cli/exec_test.go +++ b/internal/cli/exec_test.go @@ -513,7 +513,10 @@ func TestResolveExecPermissionModeMember(t *testing.T) { {"", agent.PermissionModeAuto}, {"low", agent.PermissionModeAuto}, {"medium", agent.PermissionModeAuto}, - {"member", agent.PermissionModeMemberAuto}, // headless members: write + sandboxed shell + {"workspace", agent.PermissionModeWorkspaceAuto}, // workspace writes + sandboxed shell + {"workspace-auto", agent.PermissionModeWorkspaceAuto}, + {"member", agent.PermissionModeWorkspaceAuto}, // legacy headless member alias + {"member-auto", agent.PermissionModeWorkspaceAuto}, {"high", agent.PermissionModeUnsafe}, } for _, c := range cases { @@ -530,16 +533,16 @@ func TestResolveExecPermissionModeMember(t *testing.T) { } } -// A member-auto headless tool list must include the in-workspace mutators that -// plain Auto hides, so a swarm member can actually build. Match the tool ENTRY -// line (" write_file [") — a bare substring would false-match tool descriptions. -func TestExecMemberAutoToolListIncludesMutators(t *testing.T) { +// A workspace-auto tool list must include the in-workspace mutators that plain +// Auto hides. Match the tool ENTRY line (" write_file [") — a bare substring +// would false-match tool descriptions. +func TestExecWorkspaceAutoToolListIncludesMutators(t *testing.T) { registry := newCoreRegistry(t.TempDir()) const writeEntry = "\n write_file [" - member := formatExecToolList(registry, execOptions{}, agent.PermissionModeMemberAuto) - if !strings.Contains(member, writeEntry) { - t.Fatalf("member-auto tool list must include write_file, got %q", member) + workspace := formatExecToolList(registry, execOptions{}, agent.PermissionModeWorkspaceAuto) + if !strings.Contains(workspace, writeEntry) { + t.Fatalf("workspace-auto tool list must include write_file, got %q", workspace) } // Plain Auto must still hide it (unchanged behavior — this is the read-only gate). auto := formatExecToolList(registry, execOptions{}, agent.PermissionModeAuto) diff --git a/internal/cli/exec_tools.go b/internal/cli/exec_tools.go index d019a4250..6f8ac3f64 100644 --- a/internal/cli/exec_tools.go +++ b/internal/cli/exec_tools.go @@ -81,15 +81,15 @@ func resolveExecPermissionMode(options execOptions) (agent.PermissionMode, error switch strings.ToLower(strings.TrimSpace(options.autonomy)) { case "", "low", "medium": mode = agent.PermissionModeAuto - case "member": - // Internal autonomy for headless swarm/specialist members: Auto plus - // advertised in-workspace mutators (see PermissionModeMemberAuto). The - // swarm launcher sets this; it is not part of the public low|medium|high set. - mode = agent.PermissionModeMemberAuto + case "workspace", "workspace-auto", "member", "member-auto": + // Workspace autonomy: Auto plus advertised in-workspace mutators (see + // PermissionModeWorkspaceAuto). The legacy member aliases are kept for + // headless swarm/specialist launchers. + mode = agent.PermissionModeWorkspaceAuto case "high": mode = agent.PermissionModeUnsafe default: - return "", execUsageError{fmt.Sprintf("Invalid autonomy level %q. Expected low, medium, or high.", options.autonomy)} + return "", execUsageError{fmt.Sprintf("Invalid autonomy level %q. Expected low, medium, high, workspace, workspace-auto, member, or member-auto.", options.autonomy)} } if options.skipPermissionsUnsafe { return agent.PermissionModeUnsafe, nil diff --git a/internal/sandbox/command_prefix.go b/internal/sandbox/command_prefix.go index e9f402adb..8c1842b43 100644 --- a/internal/sandbox/command_prefix.go +++ b/internal/sandbox/command_prefix.go @@ -11,17 +11,23 @@ type commandPrefixGrantSet struct { } type CommandPrefixGrant struct { - ToolName string `json:"toolName"` - Prefix []string `json:"prefix"` - ApprovedAt string `json:"approvedAt,omitempty"` - Reason string `json:"reason,omitempty"` - Session bool `json:"session,omitempty"` + ToolName string `json:"toolName"` + Prefix []string `json:"prefix"` + // Project scopes a persisted grant to one workspace root (absolute path). + // Empty means the grant is global — it matches in every project. + Project string `json:"project,omitempty"` + ApprovedAt string `json:"approvedAt,omitempty"` + Reason string `json:"reason,omitempty"` + Session bool `json:"session,omitempty"` } type CommandPrefixInput struct { ToolName string Prefix []string Reason string + // Project, when set, scopes the grant to that workspace root; empty persists + // a global grant. + Project string } func newCommandPrefixGrantSet() *commandPrefixGrantSet { @@ -63,6 +69,17 @@ func hasStringPrefix(values []string, prefix []string) bool { return false } for index := range prefix { + // The last prefix token may carry a trailing "*" wildcard, matching any + // value that starts with the text before it (e.g. "test:*" covers + // "test:unit"). Every earlier token is matched exactly. + if index == len(prefix)-1 { + if stem, ok := trailingWildcardStem(prefix[index]); ok { + if !strings.HasPrefix(values[index], stem) { + return false + } + continue + } + } if values[index] != prefix[index] { return false } @@ -70,6 +87,21 @@ func hasStringPrefix(values []string, prefix []string) bool { return true } +// trailingWildcardStem reports whether token is a trailing-wildcard namespace +// pattern (":*") and returns the non-empty stem before the "*". A +// bare "*", plain token wildcard like "test*", or token without a trailing "*" +// is not a wildcard pattern. +func trailingWildcardStem(token string) (string, bool) { + if !strings.HasSuffix(token, "*") { + return "", false + } + stem := strings.TrimSuffix(token, "*") + if stem == "" || !strings.HasSuffix(stem, ":") { + return "", false + } + return stem, true +} + func NormalizeCommandPrefix(prefix []string) ([]string, bool) { cleaned := make([]string, 0, len(prefix)) for _, part := range prefix { @@ -85,6 +117,18 @@ func NormalizeCommandPrefix(prefix []string) ([]string, bool) { return cleaned, true } +// commandPrefixTokenIsWildcard reports whether the token at index is the +// last-token trailing wildcard that hasStringPrefix honors. A wildcard is only +// valid on the LAST token and never on a lone launcher token (index 0 with no +// exact tokens ahead of it), so it can never widen the command name itself. +func commandPrefixTokenIsWildcard(prefix []string, index int) bool { + if index != len(prefix)-1 || index == 0 { + return false + } + _, ok := trailingWildcardStem(prefix[index]) + return ok +} + func ValidCommandPrefix(prefix []string) bool { _, ok := NormalizeCommandPrefix(prefix) return ok @@ -143,7 +187,16 @@ func unsafeCommandPrefix(prefix []string) bool { if len(prefix) == 0 { return true } - for _, part := range prefix { + for index, part := range prefix { + // A valid last-token wildcard ("stem*") is checked on its stem, so the + // trailing "*" itself is not treated as an unsafe glob character. + if commandPrefixTokenIsWildcard(prefix, index) { + stem, _ := trailingWildcardStem(part) + if unsafeCommandPrefixPart(stem) { + return true + } + continue + } if unsafeCommandPrefixPart(part) { return true } diff --git a/internal/sandbox/command_prefix_wildcard_test.go b/internal/sandbox/command_prefix_wildcard_test.go new file mode 100644 index 000000000..ec0486ffa --- /dev/null +++ b/internal/sandbox/command_prefix_wildcard_test.go @@ -0,0 +1,76 @@ +package sandbox + +import ( + "path/filepath" + "testing" +) + +func TestCommandPrefixProjectScope(t *testing.T) { + store, err := NewGrantStore(StoreOptions{FilePath: filepath.Join(t.TempDir(), "grants.json")}) + if err != nil { + t.Fatalf("new grant store: %v", err) + } + if _, err := store.GrantCommandPrefix(CommandPrefixInput{ToolName: "bash", Prefix: []string{"go", "test"}, Project: "/proj/a"}); err != nil { + t.Fatalf("grant project prefix: %v", err) + } + + if _, ok, _ := store.LookupCommandPrefix("bash", []string{"go", "test", "./..."}, "/proj/a"); !ok { + t.Fatal("project grant should match inside its own project") + } + if _, ok, _ := store.LookupCommandPrefix("bash", []string{"go", "test", "./..."}, "/proj/b"); ok { + t.Fatal("project grant must not match a different project") + } + + // A global grant (empty Project) matches in every project. + if _, err := store.GrantCommandPrefix(CommandPrefixInput{ToolName: "bash", Prefix: []string{"go", "vet"}}); err != nil { + t.Fatalf("grant global prefix: %v", err) + } + if _, ok, _ := store.LookupCommandPrefix("bash", []string{"go", "vet", "./..."}, "/proj/b"); !ok { + t.Fatal("global grant should match any project") + } + + // The same prefix can be held both project-scoped and global without collision. + if _, err := store.GrantCommandPrefix(CommandPrefixInput{ToolName: "bash", Prefix: []string{"go", "test"}}); err != nil { + t.Fatalf("grant global duplicate prefix: %v", err) + } + if _, ok, _ := store.LookupCommandPrefix("bash", []string{"go", "test", "-run", "X"}, "/proj/b"); !ok { + t.Fatal("global copy of a project prefix should still match elsewhere") + } +} + +func TestValidCommandPrefixAllowsTrailingWildcardOnLastToken(t *testing.T) { + // yarn is not a banned launcher, so the wildcard prefix is grantable. + if !ValidCommandPrefix([]string{"yarn", "test:*"}) { + t.Fatal("trailing wildcard on the last token should be valid") + } +} + +func TestValidCommandPrefixRejectsUnsafeWildcards(t *testing.T) { + cases := map[string][]string{ + "lone launcher wildcard": {"go*"}, + "mid-command wildcard": {"yarn", "test:*", "unit"}, + "mid-token glob": {"yarn", "te*st"}, + "plain trailing wildcard": {"yarn", "test*"}, + "bare wildcard": {"yarn", "*"}, + } + for name, prefix := range cases { + if ValidCommandPrefix(prefix) { + t.Fatalf("%s: %#v should be rejected", name, prefix) + } + } +} + +func TestCommandPrefixSessionGrantMatchesWildcard(t *testing.T) { + engine := NewEngine(EngineOptions{Policy: DefaultPolicy()}) + engine.GrantCommandPrefixForSession("bash", []string{"yarn", "test:*"}) + + if _, ok := engine.LookupCommandPrefixForSession("bash", []string{"yarn", "test:unit"}); !ok { + t.Fatal("wildcard grant should match test:unit") + } + if _, ok := engine.LookupCommandPrefixForSession("bash", []string{"yarn", "test:e2e", "--watch"}); !ok { + t.Fatal("wildcard grant should match test:e2e with extra args") + } + if _, ok := engine.LookupCommandPrefixForSession("bash", []string{"yarn", "build"}); ok { + t.Fatal("wildcard grant must not match a non-test: script") + } +} diff --git a/internal/sandbox/engine.go b/internal/sandbox/engine.go index db66d54a9..1561c8118 100644 --- a/internal/sandbox/engine.go +++ b/internal/sandbox/engine.go @@ -109,11 +109,38 @@ func (engine *Engine) GrantCommandPrefix(input CommandPrefixInput) (CommandPrefi return engine.store.GrantCommandPrefix(input) } +// GrantCommandPrefixForProject persists a prefix grant scoped to this engine's +// configured workspace root, so it only matches inside the current project. It +// returns an error when no workspace root is configured. +func (engine *Engine) GrantCommandPrefixForProject(input CommandPrefixInput) (CommandPrefixGrant, error) { + if engine == nil || engine.store == nil { + return CommandPrefixGrant{}, errors.New("sandbox grant store is not configured") + } + // A project-scoped grant must be tied to a real workspace root. With no root + // configured, persisting an empty Project would silently widen the grant to + // global, so refuse instead — the caller keeps the safer session grant. + if strings.TrimSpace(engine.workspaceRoot) == "" { + return CommandPrefixGrant{}, errors.New("no workspace root for a project-scoped command prefix grant") + } + input.Project = engine.workspaceRoot + return engine.store.GrantCommandPrefix(input) +} + +// WorkspaceRoot returns the engine's confinement root, or "" when unset. Callers +// use it to bind a project-scoped grant to the command's effective directory, so a +// grant saved for one project cannot authorize unsandboxed execution in another. +func (engine *Engine) WorkspaceRoot() string { + if engine == nil { + return "" + } + return engine.workspaceRoot +} + func (engine *Engine) LookupCommandPrefix(toolName string, command []string) (CommandPrefixGrant, bool) { if engine == nil || engine.store == nil || len(command) == 0 { return CommandPrefixGrant{}, false } - grant, matched, err := engine.store.LookupCommandPrefix(toolName, command) + grant, matched, err := engine.store.LookupCommandPrefix(toolName, command, engine.workspaceRoot) if err != nil { return CommandPrefixGrant{}, false } diff --git a/internal/sandbox/grants.go b/internal/sandbox/grants.go index ebccc630a..308e98a36 100644 --- a/internal/sandbox/grants.go +++ b/internal/sandbox/grants.go @@ -200,7 +200,9 @@ func (store *GrantStore) GrantCommandPrefix(input CommandPrefixInput) (CommandPr bucket := state.CommandPrefixes[grant.ToolName] replaced := false for i := range bucket { - if sameStringSlice(bucket[i].Prefix, grant.Prefix) { + // A grant is unique per (prefix, project): the same prefix can be held both + // globally and scoped to a specific project without one clobbering the other. + if sameStringSlice(bucket[i].Prefix, grant.Prefix) && bucket[i].Project == grant.Project { bucket[i] = grant replaced = true break @@ -234,13 +236,18 @@ func (store *GrantStore) Lookup(toolName string, reqScope string) (GrantLookup, return lookupGrantBucket(bucket, reqScope), nil } -func (store *GrantStore) LookupCommandPrefix(toolName string, command []string) (CommandPrefixGrant, bool, error) { +// LookupCommandPrefix finds a persisted grant that covers command. project is the +// current workspace root: a grant matches when it is global (empty Project) or its +// Project equals project, so a project-scoped grant never leaks into other +// workspaces. A more specific project grant is preferred over a global one. +func (store *GrantStore) LookupCommandPrefix(toolName string, command []string, project string) (CommandPrefixGrant, bool, error) { if err := ValidateToolName(toolName); err != nil { return CommandPrefixGrant{}, false, err } if _, ok := NormalizeCommandPrefix(command); !ok { return CommandPrefixGrant{}, false, nil } + project = strings.TrimSpace(project) store.mu.Lock() defer store.mu.Unlock() state, err := store.readState() @@ -248,12 +255,28 @@ func (store *GrantStore) LookupCommandPrefix(toolName string, command []string) return CommandPrefixGrant{}, false, err } bucket := state.CommandPrefixes[strings.TrimSpace(toolName)] - for _, grant := range bucket { - if hasStringPrefix(command, grant.Prefix) { + var globalMatch *CommandPrefixGrant + for index := range bucket { + grant := bucket[index] + if !hasStringPrefix(command, grant.Prefix) { + continue + } + if grant.Project == "" { + if globalMatch == nil { + matched := grant + globalMatch = &matched + } + continue + } + if grant.Project == project { grant.Prefix = append([]string(nil), grant.Prefix...) return grant, true, nil } } + if globalMatch != nil { + globalMatch.Prefix = append([]string(nil), globalMatch.Prefix...) + return *globalMatch, true, nil + } return CommandPrefixGrant{}, false, nil } @@ -502,6 +525,7 @@ func createCommandPrefixGrant(input CommandPrefixInput, now func() time.Time) (C return CommandPrefixGrant{ ToolName: toolName, Prefix: prefix, + Project: strings.TrimSpace(input.Project), ApprovedAt: now().UTC().Format(time.RFC3339), Reason: redaction.RedactString(strings.TrimSpace(input.Reason), redaction.Options{}), }, nil diff --git a/internal/sandbox/grants_test.go b/internal/sandbox/grants_test.go index 271ede30c..adec8a47b 100644 --- a/internal/sandbox/grants_test.go +++ b/internal/sandbox/grants_test.go @@ -198,14 +198,14 @@ func TestGrantStorePersistsCommandPrefixes(t *testing.T) { if len(prefixes) != 1 || prefixes[0].ToolName != "bash" || !sameStringSlice(prefixes[0].Prefix, []string{"git", "status"}) || prefixes[0].Reason != "updated" { t.Fatalf("unexpected command prefixes: %#v", prefixes) } - match, matched, err := reopened.LookupCommandPrefix("bash", []string{"git", "status", "--short"}) + match, matched, err := reopened.LookupCommandPrefix("bash", []string{"git", "status", "--short"}, "") if err != nil { t.Fatalf("LookupCommandPrefix returned error: %v", err) } if !matched || !sameStringSlice(match.Prefix, []string{"git", "status"}) { t.Fatalf("lookup = (%#v,%t), want git status match", match, matched) } - if _, matched, err := reopened.LookupCommandPrefix("bash", []string{"git", "diff"}); err != nil || matched { + if _, matched, err := reopened.LookupCommandPrefix("bash", []string{"git", "diff"}, ""); err != nil || matched { t.Fatalf("git diff lookup = matched %t err %v, want no match", matched, err) } text := FormatGrantListWithCommandPrefixes(nil, prefixes) diff --git a/internal/specialist/exec.go b/internal/specialist/exec.go index 6dac1d259..57faca23e 100644 --- a/internal/specialist/exec.go +++ b/internal/specialist/exec.go @@ -68,8 +68,8 @@ type BuildArgsInput struct { // child to unsafe. Authority is therefore never widened beyond the parent. PermissionMode string // MemberAutonomy marks a headless swarm member: when set and the parent is - // non-unsafe, the child runs at "--auto member" (PermissionModeMemberAuto) so - // it can write/edit + run sandboxed shell IN the workspace, instead of the + // non-unsafe, the child runs at "--auto workspace-auto" so it can write/edit + + // run sandboxed shell IN the workspace, instead of the // read-only "--auto low". Off by default, so the Task tool's specialists are // unchanged. The sandbox still confines writes to the workspace root. MemberAutonomy bool @@ -143,14 +143,14 @@ func specialistAutonomy(permissionMode string) string { } // memberAwareAutonomy is specialistAutonomy with one extra rung for headless -// swarm MEMBERS: a non-unsafe member runs at "member" (PermissionModeMemberAuto) -// so it can write/edit + run sandboxed shell in the workspace, rather than the -// read-only "low". An unsafe parent still yields "high" (full unsafe), and a -// non-member (Task specialist) is unchanged. Authority stays sandbox-confined. +// swarm MEMBERS: a non-unsafe member runs at "workspace-auto" so it can +// write/edit + run sandboxed shell in the workspace, rather than the read-only +// "low". An unsafe parent still yields "high" (full unsafe), and a non-member +// (Task specialist) is unchanged. Authority stays sandbox-confined. func memberAwareAutonomy(permissionMode string, member bool) string { autonomy := specialistAutonomy(permissionMode) if member && autonomy == "low" { - return "member" + return "workspace-auto" } return autonomy } diff --git a/internal/specialist/exec_test.go b/internal/specialist/exec_test.go index b0de1b6a1..b8b15ac55 100644 --- a/internal/specialist/exec_test.go +++ b/internal/specialist/exec_test.go @@ -82,8 +82,8 @@ func TestSpecialistAutonomyByPermissionMode(t *testing.T) { } func TestMemberAwareAutonomy(t *testing.T) { - // A non-unsafe member runs at "member" (write/edit + sandboxed shell), a plain - // specialist at "low" (read-only), and an unsafe parent at "high" either way. + // A non-unsafe member runs at "workspace-auto" (write/edit + sandboxed shell), + // a plain specialist at "low" (read-only), and an unsafe parent at "high" either way. cases := []struct { mode string member bool @@ -91,10 +91,10 @@ func TestMemberAwareAutonomy(t *testing.T) { }{ {"auto", false, "low"}, {"ask", false, "low"}, - {"auto", true, "member"}, - {"ask", true, "member"}, - {"", true, "member"}, // member, fail-safe non-unsafe, still write-capable - {"unsafe", true, "high"}, // unsafe parent keeps full autonomy + {"auto", true, "workspace-auto"}, + {"ask", true, "workspace-auto"}, + {"", true, "workspace-auto"}, // member, fail-safe non-unsafe, still write-capable + {"unsafe", true, "high"}, // unsafe parent keeps full autonomy {"unsafe", false, "high"}, } for _, c := range cases { @@ -104,17 +104,17 @@ func TestMemberAwareAutonomy(t *testing.T) { } } -func TestBuildArgsMemberAutonomyEmitsMember(t *testing.T) { +func TestBuildArgsMemberAutonomyEmitsWorkspaceAuto(t *testing.T) { executor := Executor{NewSessionID: func() (string, error) { return "child", nil }} manifest := Manifest{Metadata: Metadata{Name: "subagent"}, SystemPrompt: "x", ResolvedTools: []string{"read_file", "write_file"}} - // A non-unsafe member → --auto member (write-capable), not the read-only low. + // A non-unsafe member → --auto workspace-auto (write-capable), not the read-only low. res, err := executor.BuildArgs(BuildArgsInput{Manifest: manifest, Prompt: "p", PermissionMode: "auto", MemberAutonomy: true}) if err != nil { t.Fatalf("BuildArgs: %v", err) } - if !containsSequence(res.Args, []string{"--auto", "member"}) { - t.Fatalf("non-unsafe member must yield --auto member, got %v", res.Args) + if !containsSequence(res.Args, []string{"--auto", "workspace-auto"}) { + t.Fatalf("non-unsafe member must yield --auto workspace-auto, got %v", res.Args) } // Without the member flag, the same parent stays --auto low (unchanged). @@ -122,7 +122,7 @@ func TestBuildArgsMemberAutonomyEmitsMember(t *testing.T) { if err != nil { t.Fatalf("BuildArgs(plain): %v", err) } - if !containsSequence(plain.Args, []string{"--auto", "low"}) || containsSequence(plain.Args, []string{"--auto", "member"}) { + if !containsSequence(plain.Args, []string{"--auto", "low"}) || containsSequence(plain.Args, []string{"--auto", "workspace-auto"}) { t.Fatalf("a plain specialist must stay --auto low, got %v", plain.Args) } diff --git a/internal/swarm/team.go b/internal/swarm/team.go index c913f987f..a6835d49b 100644 --- a/internal/swarm/team.go +++ b/internal/swarm/team.go @@ -231,10 +231,12 @@ func resolvePermissionMode(pol Policy, def Definition) string { // importing it, to avoid an import cycle). These are the actual values that flow // through tools.RunOptions.PermissionMode — NOT the TUI display names. const ( - permissionModeAsk = "ask" // prompts for every tool (most restrictive) - permissionModeSpecDraft = "spec-draft" // spec-drafting only - permissionModeAuto = "auto" // auto-approve low-risk - permissionModeUnsafe = "unsafe" // approve everything (most permissive) + permissionModeAsk = "ask" // prompts for every tool (most restrictive) + permissionModeSpecDraft = "spec-draft" // spec-drafting only + permissionModeAuto = "auto" // auto-approve low-risk + permissionModeWorkspaceAuto = "workspace-auto" // auto + sandbox-safe workspace mutators + permissionModeAutoClassifier = "auto-classifier" // workspace auto + classifier-reviewed allows + permissionModeUnsafe = "unsafe" // approve everything (most permissive) ) // permissionRank orders permission modes from least to most permissive so the @@ -251,8 +253,12 @@ func permissionRank(mode string) int { return 2 case permissionModeAuto: return 3 - case permissionModeUnsafe: + case permissionModeWorkspaceAuto: return 4 + case permissionModeAutoClassifier: + return 5 + case permissionModeUnsafe: + return 6 default: return 0 } diff --git a/internal/swarm/team_test.go b/internal/swarm/team_test.go index 7348a7127..a7d3da598 100644 --- a/internal/swarm/team_test.go +++ b/internal/swarm/team_test.go @@ -33,6 +33,14 @@ func TestResolvePermissionModeNeverWidens(t *testing.T) { if got := resolvePermissionMode(Policy{PermissionMode: permissionModeUnsafe}, Definition{PermissionMode: permissionModeAuto}); got != permissionModeAuto { t.Fatalf("unsafe parent + auto def = %q, want auto (stricter honored)", got) } + // New canonical modes keep their intended order: auto-classifier can delegate + // to workspace-auto, but workspace-auto cannot widen into auto-classifier. + if got := resolvePermissionMode(Policy{PermissionMode: permissionModeAutoClassifier}, Definition{PermissionMode: permissionModeWorkspaceAuto}); got != permissionModeWorkspaceAuto { + t.Fatalf("auto-classifier parent + workspace-auto def = %q, want workspace-auto", got) + } + if got := resolvePermissionMode(Policy{PermissionMode: permissionModeWorkspaceAuto}, Definition{PermissionMode: permissionModeAutoClassifier}); got != permissionModeWorkspaceAuto { + t.Fatalf("workspace-auto parent + auto-classifier def = %q, want workspace-auto", got) + } } func TestTeamAdmitAndQueue(t *testing.T) { diff --git a/internal/tui/command_polish_test.go b/internal/tui/command_polish_test.go index 2758171fe..eed4ec07b 100644 --- a/internal/tui/command_polish_test.go +++ b/internal/tui/command_polish_test.go @@ -302,10 +302,11 @@ func TestContextAndPermissionsCommandsRenderProductState(t *testing.T) { permissionText := transcriptText(next.transcript) for _, want := range []string{ "Permissions", - "ask permissions", + "ask every time permissions", "1 persistent grant", "State", - "mode ask", + "mode ask", + "label Ask every time", "Grants", "bash [allow]", "[REDACTED]", @@ -332,9 +333,10 @@ func TestPermissionsCommandCardHandlesNilStoreAndEmptyGrants(t *testing.T) { nilStore := model{permissionMode: agent.PermissionModeAuto}.permissionsText() for _, want := range []string{ "Permissions", - "auto permissions", + "auto-approve safe actions permissions", "grants unavailable", - "mode auto", + "mode auto", + "label Auto-approve safe actions", "persistent grants: unavailable", } { assertContains(t, nilStore, want) @@ -348,9 +350,10 @@ func TestPermissionsCommandCardHandlesNilStoreAndEmptyGrants(t *testing.T) { emptyText := model{permissionMode: agent.PermissionModeAsk, sandboxStore: store}.permissionsText() for _, want := range []string{ "Permissions", - "ask permissions", + "ask every time permissions", "no persistent grants", - "mode ask", + "mode ask", + "label Ask every time", "none", } { assertContains(t, emptyText, want) @@ -361,9 +364,10 @@ func TestPermissionsCommandCardHandlesNilStoreAndEmptyGrants(t *testing.T) { errText := model{permissionMode: agent.PermissionModeAsk}.permissionsTextWithStore(errStore) for _, want := range []string{ "Permissions", - "ask permissions", + "ask every time permissions", "grants error", - "mode ask", + "mode ask", + "label Ask every time", "error: storage failure", } { assertContains(t, errText, want) diff --git a/internal/tui/command_views.go b/internal/tui/command_views.go index b7b3f21e4..d73ae5512 100644 --- a/internal/tui/command_views.go +++ b/internal/tui/command_views.go @@ -8,6 +8,7 @@ import ( tea "charm.land/bubbletea/v2" + "github.com/Gitlawb/zero/internal/agent" "github.com/Gitlawb/zero/internal/config" "github.com/Gitlawb/zero/internal/providercatalog" "github.com/Gitlawb/zero/internal/sandbox" @@ -269,42 +270,41 @@ func (m model) permissionsText() string { func (m model) permissionsTextWithStore(store grantLister) string { mode := string(m.permissionMode) + modeInfo := agent.PermissionModeInfoFor(m.permissionMode) + // PermissionModeInfoFor maps any unrecognized value to Auto. For an unknown + // (e.g. rehydrated session) mode, surface the raw string instead of + // mislabeling it "Auto", mirroring modeLabel() in view.go. + if strings.TrimSpace(mode) != "" && modeInfo.ID != m.permissionMode { + modeInfo.Label = mode + modeInfo.Summary = mode + modeInfo.Description = "" + } + modeSummary := modeInfo.Summary + " permissions" + stateSections := permissionStateSections(m.permissionMode, mode, modeInfo) if store == nil { + sections := append([]commandCardSection{}, stateSections...) + sections = append(sections, commandCardSection{ + Title: "Grants", + Lines: []string{"persistent grants: unavailable"}, + }) return renderCommandCardTranscript(commandCard{ - Title: "Permissions", - Summary: []string{mode + " permissions", "grants unavailable"}, - Sections: []commandCardSection{ - { - Title: "State", - Fields: []commandField{ - {Key: "mode", Value: mode}, - }, - }, - { - Title: "Grants", - Lines: []string{"persistent grants: unavailable"}, - }, - }, + Title: "Permissions", + Summary: []string{modeSummary, "grants unavailable"}, + Sections: sections, }) } grants, err := store.List() if err != nil { + sections := append([]commandCardSection{}, stateSections...) + sections = append(sections, commandCardSection{ + Title: "Grants", + Lines: []string{"error: " + err.Error()}, + }) return renderCommandCardTranscript(commandCard{ - Title: "Permissions", - Summary: []string{mode + " permissions", "grants error"}, - Sections: []commandCardSection{ - { - Title: "State", - Fields: []commandField{ - {Key: "mode", Value: mode}, - }, - }, - { - Title: "Grants", - Lines: []string{"error: " + err.Error()}, - }, - }, + Title: "Permissions", + Summary: []string{modeSummary, "grants error"}, + Sections: sections, }) } prefixes := []sandbox.CommandPrefixGrant{} @@ -312,21 +312,15 @@ func (m model) permissionsTextWithStore(store grantLister) string { var prefixErr error prefixes, prefixErr = prefixStore.ListCommandPrefixes() if prefixErr != nil { + sections := append([]commandCardSection{}, stateSections...) + sections = append(sections, commandCardSection{ + Title: "Grants", + Lines: []string{"error: " + prefixErr.Error()}, + }) return renderCommandCardTranscript(commandCard{ - Title: "Permissions", - Summary: []string{mode + " permissions", "grants error"}, - Sections: []commandCardSection{ - { - Title: "State", - Fields: []commandField{ - {Key: "mode", Value: mode}, - }, - }, - { - Title: "Grants", - Lines: []string{"error: " + prefixErr.Error()}, - }, - }, + Title: "Permissions", + Summary: []string{modeSummary, "grants error"}, + Sections: sections, }) } } @@ -347,7 +341,11 @@ func (m model) permissionsTextWithStore(store grantLister) string { grantRows = append(grantRows, commandRow{Text: line}) } for _, grant := range prefixes { - line := fmt.Sprintf("%s `%s` [command-prefix]", grant.ToolName, strings.Join(grant.Prefix, " ")) + scope := "global" + if strings.TrimSpace(grant.Project) != "" { + scope = "project" + } + line := fmt.Sprintf("%s `%s` [command-prefix:%s]", grant.ToolName, strings.Join(grant.Prefix, " "), scope) if grant.ApprovedAt != "" { line += " approved " + grant.ApprovedAt } @@ -358,22 +356,40 @@ func (m model) permissionsTextWithStore(store grantLister) string { } } + sections := append([]commandCardSection{}, stateSections...) + sections = append(sections, commandCardSection{ + Title: "Grants", + Rows: grantRows, + }) return renderCommandCardTranscript(commandCard{ - Title: "Permissions", - Summary: []string{mode + " permissions", formatGrantCount(len(snapshots) + len(prefixes))}, - Sections: []commandCardSection{ - { - Title: "State", - Fields: []commandField{ - {Key: "mode", Value: mode}, - }, - }, - { - Title: "Grants", - Rows: grantRows, + Title: "Permissions", + Summary: []string{modeSummary, formatGrantCount(len(snapshots) + len(prefixes))}, + Sections: sections, + }) +} + +func permissionStateSections(mode agent.PermissionMode, modeID string, modeInfo agent.PermissionModeInfo) []commandCardSection { + sections := []commandCardSection{ + { + Title: "State", + Fields: []commandField{ + {Key: "mode", Value: modeID}, + {Key: "label", Value: modeInfo.Label}, + {Key: "meaning", Value: modeInfo.Description}, }, }, - }) + } + if mode == agent.PermissionModeAutoClassifier { + sections = append(sections, commandCardSection{ + Title: "Auto review", + Lines: []string{ + "engine: LLM classifier (reviews each sandbox-cleared low-risk action)", + "auto-allows: only actions the classifier judges low-risk", + "still asks: anything the classifier is unsure about, plus destructive commands, network, installs, escalated sandbox access, out-of-workspace access", + }, + }) + } + return sections } // grantLister is the subset of sandbox.GrantStore used by permissionsText(). diff --git a/internal/tui/hover.go b/internal/tui/hover.go index 5ec530e1a..06a0fb29a 100644 --- a/internal/tui/hover.go +++ b/internal/tui/hover.go @@ -68,10 +68,10 @@ func (m model) updateHoverTarget(msg tea.MouseMsg) model { } if line, ok := m.transcriptLineAtMouse(msg); ok { // A permission option reuses its OWN existing keyboard-cursor highlight - // (see hoverPermissionOption) rather than the m.hover mechanism, so there's - // nothing further to set here. + // (see hoverPermissionOptionAt) rather than the m.hover mechanism, so + // there's nothing further to set here. if line.permOption { - return m.hoverPermissionOption(line.permChoice).withHover(hoverTarget{}) + return m.hoverPermissionOptionAt(line.permOptionIndex).withHover(hoverTarget{}) } // Only a card or a collapse/expand toggle header is "clickable" here; plain // selectable text (e.g. a user/assistant message) isn't, so hovering over @@ -83,23 +83,20 @@ func (m model) updateHoverTarget(msg tea.MouseMsg) model { return m.withHover(hoverTarget{}) } -// hoverPermissionOption moves the pending permission prompt's cursor to the +// hoverPermissionOptionAt moves the pending permission prompt's cursor to the // hovered option, reusing its EXISTING keyboard-navigation highlight (the same // one Tab/Shift+Tab/arrow keys move) instead of new styling — the popup already -// renders whichever option .cursor points at differently. A no-op when no -// permission prompt is pending or the choice isn't found (defensive; every -// permOption line's choice always comes from permissionOptions(request) in the -// first place). -func (m model) hoverPermissionOption(choice permissionDecision) model { +// renders whichever option .cursor points at differently. Addressing by index +// (not decision action) is required because expanded prefix breadths share a +// decision action. A no-op when no prompt is pending or the index is out of range. +func (m model) hoverPermissionOptionAt(index int) model { if m.pendingPermission == nil { return m } - for index, option := range permissionOptions(m.pendingPermission.request) { - if option.choice == choice { - m.pendingPermission.cursor = index - return m - } + if index < 0 || index >= len(permissionOptions(m.pendingPermission.request)) { + return m } + m.pendingPermission.cursor = index return m } diff --git a/internal/tui/keybinding_help.go b/internal/tui/keybinding_help.go index f1e53ed73..f1cea47ec 100644 --- a/internal/tui/keybinding_help.go +++ b/internal/tui/keybinding_help.go @@ -52,7 +52,7 @@ func (m model) buildKeybindingGroups() []keybindingGroup { title: "Model & run controls", bindings: []keybinding{ {labelOr(m.keyBindings.cycleReasoning, "Ctrl+T"), "cycle reasoning effort (auto \u2192 low \u2192 medium \u2192 high)"}, - {"Shift+Tab", "cycle permission mode (auto \u2194 ask)"}, + {"Shift+Tab", "cycle permission mode (ask \u2192 auto \u2192 trusted workspace \u2192 let Zero cook)"}, {labelOr(m.keyBindings.togglePlan, "Ctrl+P"), "expand / collapse the plan panel (when no menu is open)"}, }, }, diff --git a/internal/tui/model.go b/internal/tui/model.go index 280c8ccfb..53354b769 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -131,8 +131,15 @@ type model struct { agentOptions agent.Options notifier *notify.Notifier permissionMode agent.PermissionMode - selfCorrectTests bool - reasoningEffort modelregistry.ReasoningEffort + autoClassifierConfirmActive bool + // autoClassifierAcknowledged records that the user has confirmed the + // auto-classifier warning once this process. After that, cycling to + // auto-classifier enables it directly without re-prompting (the warning is + // about a mode, not a session, so one acknowledgement covers the process — + // including cycles after a /resume). Resets when zero is closed. + autoClassifierAcknowledged bool + selfCorrectTests bool + reasoningEffort modelregistry.ReasoningEffort // Active execution profile (set by /profile; applies to the NEXT run). // The displaced/applied pairs let a switch or /profile balanced restore // exactly what the profile replaced while leaving later manual overrides @@ -666,14 +673,15 @@ type prWatcherStartedMsg struct { type permissionDecision = agent.PermissionDecisionAction const ( - permissionDecisionAllow permissionDecision = agent.PermissionDecisionAllow - permissionDecisionAllowStrict permissionDecision = agent.PermissionDecisionAllowStrict - permissionDecisionAllowForSession permissionDecision = agent.PermissionDecisionAllowForSession - permissionDecisionAllowPrefix permissionDecision = agent.PermissionDecisionAllowPrefix - permissionDecisionAlwaysAllowPrefix permissionDecision = agent.PermissionDecisionAlwaysAllowPrefix - permissionDecisionDeny permissionDecision = agent.PermissionDecisionDeny - permissionDecisionAlwaysAllow permissionDecision = agent.PermissionDecisionAlwaysAllow - permissionDecisionCancel permissionDecision = agent.PermissionDecisionCancel + permissionDecisionAllow permissionDecision = agent.PermissionDecisionAllow + permissionDecisionAllowStrict permissionDecision = agent.PermissionDecisionAllowStrict + permissionDecisionAllowForSession permissionDecision = agent.PermissionDecisionAllowForSession + permissionDecisionAllowPrefix permissionDecision = agent.PermissionDecisionAllowPrefix + permissionDecisionAllowPrefixProject permissionDecision = agent.PermissionDecisionAllowPrefixProject + permissionDecisionAlwaysAllowPrefix permissionDecision = agent.PermissionDecisionAlwaysAllowPrefix + permissionDecisionDeny permissionDecision = agent.PermissionDecisionDeny + permissionDecisionAlwaysAllow permissionDecision = agent.PermissionDecisionAlwaysAllow + permissionDecisionCancel permissionDecision = agent.PermissionDecisionCancel ) type permissionRequestMsg struct { @@ -1024,7 +1032,7 @@ func (m *model) stopPRWatcher() { func (m model) noBlockingModal() bool { return m.pendingPermission == nil && m.pendingAskUser == nil && m.pendingSpecReview == nil && m.providerWizard == nil && m.mcpAddWizard == nil && m.mcpManager == nil && m.picker == nil && - m.sttKeyPrompt == nil + m.sttKeyPrompt == nil && !m.autoClassifierConfirmActive } func (m model) quit() (tea.Model, tea.Cmd) { @@ -1443,6 +1451,11 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { m.transcriptDetailed = false return m, nil } + if m.autoClassifierConfirmActive { + // Esc cancels the auto-classifier confirmation; mode is unchanged. + m.autoClassifierConfirmActive = false + return m, nil + } // Esc on an ask-user prompt: from the "type my own" free-text it steps // back to the selector for that question; otherwise it cancels the // questionnaire (not the run), delivering whatever answers were collected @@ -1530,6 +1543,11 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { } return m, nil } + if m.autoClassifierConfirmActive { + // Enter confirms enabling auto-classifier mode. + m = m.confirmAutoClassifierMode() + return m, nil + } if m.pendingPermission != nil { // Enter confirms the highlighted option (default: allow once); the // a/y/d hotkeys and a click still resolve directly. @@ -1609,13 +1627,24 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { if m.pendingAskUser != nil { return m.moveAskUserTab(-1), nil } - // shift+tab toggles the permission mode between Auto and Ask (Unsafe - // is intentionally not reachable by a casual keypress — see + // shift+tab toggles permission modes from safest to more autonomous + // (Unsafe is intentionally not reachable by a casual keypress — see // nextPermissionMode), but only when nothing modal is up: a permission // prompt, ask_user questionnaire, or open picker all take precedence // and let the key fall through to their own handlers below. if m.noBlockingModal() { - m.permissionMode = nextPermissionMode(m.permissionMode) + next := nextPermissionMode(m.permissionMode) + // Auto-classifier hands permission decisions to an LLM, so the FIRST + // time it is enabled this process it gets an explicit confirmation modal + // instead of switching on a single keypress (Enter enables it, Esc + // cancels — see the Enter/Esc handlers above). Once acknowledged, later + // cycles enable it directly: the warning is about the mode, not the + // session, so it must not re-prompt after a /resume. + if next == agent.PermissionModeAutoClassifier && !m.autoClassifierAcknowledged { + m.autoClassifierConfirmActive = true + return m, nil + } + m.permissionMode = next return m, nil } case m.keyMatch(m.keyBindings.cycleReasoning, msg, func(tea.KeyMsg) bool { return keyCtrl(msg, 't') }): @@ -1875,6 +1904,17 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) { } return m, nil // picker mode: non-navigation keys do nothing } + if m.autoClassifierConfirmActive { + // Modal over the input: y confirms, n cancels, every other key is + // swallowed. Enter/Esc are already handled in their own cases above. + switch keyText(msg) { + case "y", "Y": + m = m.confirmAutoClassifierMode() + case "n", "N": + m.autoClassifierConfirmActive = false + } + return m, nil + } if m.pendingSpecReview != nil { m.burstCount = 0 return m.handleSpecReviewKey(msg) @@ -2713,6 +2753,9 @@ func (m model) transcriptView() string { mcpOverlay := m.mcpManagerOverlay(width) pickerOverlay := m.pickerOverlay(width) sttKeyOverlay := m.sttKeyPromptOverlay(width) + // The auto-classifier confirmation is intentionally NOT in this scrim-overlay + // set: it renders in the footer (live-tail region) instead, which repaints + // reliably across terminals even over a non-empty (resumed) transcript. viewportOverlay := "" switch { case sttKeyOverlay != "": @@ -2829,6 +2872,18 @@ func (m model) footerView(width int) string { footer.WriteString(m.statusLine(width)) return footer.String() } + // The auto-classifier confirmation renders here in the live-tail region (like + // the ask-user questionnaire) rather than as a floating overlay composited into + // the scrolled transcript viewport. The overlay-over-transcript path renders + // unreliably across terminal multiplexers when the transcript is non-empty (a + // resumed session): the modal could stay armed but unpainted until an unrelated + // re-render. The footer always repaints, so the modal shows immediately. + if m.autoClassifierConfirmActive { + footer.WriteString(m.autoClassifierConfirmOverlay(width)) + footer.WriteString("\n") + footer.WriteString(m.statusLine(width)) + return footer.String() + } // Pinned plan panel: sits directly above the composer so it stays visible // while the transcript scrolls underneath (a streaming turn no longer pushes // the plan off-screen). Budgeted to at most a third of the screen height; a @@ -3987,27 +4042,38 @@ func (m model) handlePermissionKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { key := strings.ToLower(msg.String()) for _, option := range permissionOptions(m.pendingPermission.request) { if option.hotkey == key { - return m.choosePermissionOption(option.choice) + return m.choosePermissionOption(option) } } return m, nil } +// confirmAutoClassifierMode enables auto-classifier mode and clears the pending +// confirmation. Shared by the Enter and y/Y handlers so the state change lives in +// one place. +func (m model) confirmAutoClassifierMode() model { + m.autoClassifierConfirmActive = false + // Remember the acknowledgement for the rest of the process so a later cycle to + // auto-classifier (e.g. after switching modes, or after a /resume) enables it + // without re-showing the warning. + m.autoClassifierAcknowledged = true + m.permissionMode = agent.PermissionModeAutoClassifier + return m +} + func (m model) resolvePermission(decision permissionDecision) (tea.Model, tea.Cmd) { - return m.resolvePermissionWithReason(decision, permissionDecisionReason(decision)) + return m.resolvePermissionOption(permissionOption{choice: decision}) } // resolvePermissionWithReason resolves the pending prompt with an explicit reason -// string. It backs both the fixed-label choices (reason = permissionDecisionReason) -// and the free-text "tell Zero what to do differently" path, where the reason is -// the user's typed instruction and the action is Deny so the agent surfaces it as -// the tool result and keeps going. +// string. It backs the free-text "tell Zero what to do differently" path, where +// the reason is the user's typed instruction and the action is Deny so the agent +// surfaces it as the tool result and keeps going. func (m model) resolvePermissionWithReason(decision permissionDecision, reason string) (tea.Model, tea.Cmd) { pending := m.pendingPermission if pending == nil { return m, nil } - if pending.decide != nil { pending.decide(agent.PermissionDecision{ Action: decision, @@ -4018,6 +4084,42 @@ func (m model) resolvePermissionWithReason(decision permissionDecision, reason s return m, nil } +// resolvePermissionOption resolves the prompt with a chosen option, carrying the +// option's breadth (commandPrefix) when it expands a prefix grant so the loop +// grants exactly the breadth the approver picked. +func (m model) resolvePermissionOption(option permissionOption) (tea.Model, tea.Cmd) { + pending := m.pendingPermission + if pending == nil { + return m, nil + } + if pending.decide != nil { + decision := agent.PermissionDecision{ + Action: option.choice, + Reason: permissionDecisionReason(option.choice), + } + if len(option.commandPrefix) > 0 { + decision.CommandPrefix = append([]string(nil), option.commandPrefix...) + } + pending.decide(decision) + } + m.pendingPermission = nil + return m, nil +} + +// resolvePermissionAt resolves the permission option at the given index (used by +// clicks, which address options positionally so expanded prefix breadths that +// share a decision action stay distinct). +func (m model) resolvePermissionAt(index int) (tea.Model, tea.Cmd) { + if m.pendingPermission == nil { + return m, nil + } + options := permissionOptions(m.pendingPermission.request) + if index < 0 || index >= len(options) { + return m, nil + } + return m.resolvePermissionOption(options[index]) +} + func permissionDecisionReason(decision permissionDecision) string { switch decision { case permissionDecisionAllow: @@ -4028,8 +4130,10 @@ func permissionDecisionReason(decision permissionDecision) string { return "approved for this session in TUI" case permissionDecisionAllowPrefix: return "approved command prefix for this session in TUI" + case permissionDecisionAllowPrefixProject: + return "persistently approved command prefix (project) in TUI" case permissionDecisionAlwaysAllowPrefix: - return "persistently approved command prefix in TUI" + return "persistently approved command prefix (global) in TUI" case permissionDecisionAlwaysAllow: return "persistently approved in TUI" case permissionDecisionCancel: diff --git a/internal/tui/model_test.go b/internal/tui/model_test.go index 2c4dbfb13..a7a56d30b 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -397,8 +397,9 @@ func TestPermissionsCommandListsPersistentSandboxGrants(t *testing.T) { text := transcriptText(next.transcript) for _, want := range []string{ "Permissions", - "ask permissions", - "mode ask", + "ask every time permissions", + "mode ask", + "label Ask every time", "Grants", "bash [allow]", "write_file [deny]", @@ -1600,7 +1601,7 @@ func TestPermissionRequestShowsFocusedPrompt(t *testing.T) { t.Fatalf("expected permission row to preserve scope %q, got %#v", request.Scope, row) } view := plainRender(t, next.View()) - for _, want := range []string{"write_file", "Yes, proceed", "[a]", "these files in this session", "[s]", "don't ask again for this scope", "[y]", "continue without running it", "[d]", "scope: src/main.go", "Creates or overwrites files."} { + for _, want := range []string{"write_file", "Yes, proceed", "[a]", "these files in this session", "[s]", "don't ask again for this scope", "[f]", "continue without running it", "[d]", "scope: src/main.go", "Creates or overwrites files."} { assertContains(t, view, want) } if strings.Contains(view, "risk:") || strings.Contains(view, "risk=") { @@ -1617,7 +1618,7 @@ func TestPermissionPromptChoicesResolveDecision(t *testing.T) { {name: "allow", key: "a", want: permissionDecisionAllow}, {name: "deny", key: "d", want: permissionDecisionDeny}, {name: "session", key: "s", want: permissionDecisionAllowForSession}, - {name: "always", key: "y", want: permissionDecisionAlwaysAllow}, + {name: "always", key: "f", want: permissionDecisionAlwaysAllow}, } for _, tc := range cases { @@ -1852,29 +1853,180 @@ func TestShiftTabCyclesPermissionMode(t *testing.T) { m := newModel(context.Background(), Options{PermissionMode: agent.PermissionModeAuto}) m.width = 96 - // shift+tab toggles Auto<->Ask only; Unsafe is intentionally NOT reachable by - // a casual keypress (it disables permission prompts). + shiftTab := func(m model) model { + updated, cmd := m.Update(testKeyShift(tea.KeyTab)) + if cmd != nil { + t.Fatalf("expected shift+tab to cycle mode synchronously, got command") + } + return updated.(model) + } + + // Auto -> WorkspaceAuto. + m = shiftTab(m) + if m.permissionMode != agent.PermissionModeWorkspaceAuto { + t.Fatalf("expected workspace-auto after first shift+tab, got %q", m.permissionMode) + } + + // WorkspaceAuto -> auto-classifier arms a confirmation modal; the mode does + // not change until the user confirms. + m = shiftTab(m) + if m.permissionMode != agent.PermissionModeWorkspaceAuto { + t.Fatalf("expected mode unchanged while confirmation pending, got %q", m.permissionMode) + } + if !m.autoClassifierConfirmActive { + t.Fatal("expected auto-classifier confirmation to be armed") + } + + // Shift+tab while the modal is up is swallowed (the modal takes precedence). + m = shiftTab(m) + if m.permissionMode != agent.PermissionModeWorkspaceAuto || !m.autoClassifierConfirmActive { + t.Fatalf("expected confirmation still pending, got mode %q active %v", m.permissionMode, m.autoClassifierConfirmActive) + } + + // Enter confirms -> AutoClassifier. + updated, _ := m.Update(testKey(tea.KeyEnter)) + m = updated.(model) + if m.permissionMode != agent.PermissionModeAutoClassifier || m.autoClassifierConfirmActive { + t.Fatalf("expected auto-classifier enabled after Enter, got mode %q active %v", m.permissionMode, m.autoClassifierConfirmActive) + } + + // AutoClassifier -> Ask. + m = shiftTab(m) + if m.permissionMode != agent.PermissionModeAsk { + t.Fatalf("expected ask after cycling past auto-classifier, got %q", m.permissionMode) + } + + // The rendered status label tracks the cycled mode. + label, _ := m.modeLabel() + if label != agent.PermissionModeSummary(agent.PermissionModeAsk) { + t.Fatalf("expected mode label to track cycled mode, got %q", label) + } +} + +func TestAutoClassifierConfirmationCancels(t *testing.T) { + m := newModel(context.Background(), Options{PermissionMode: agent.PermissionModeWorkspaceAuto}) + m.width = 96 + + // Shift+tab from workspace-auto arms the confirmation. + updated, _ := m.Update(testKeyShift(tea.KeyTab)) + m = updated.(model) + if !m.autoClassifierConfirmActive { + t.Fatal("expected auto-classifier confirmation to be armed") + } + + // Esc cancels without changing the mode. + updated, _ = m.Update(testKey(tea.KeyEsc)) + m = updated.(model) + if m.autoClassifierConfirmActive { + t.Fatal("expected confirmation cleared after Esc") + } + if m.permissionMode != agent.PermissionModeWorkspaceAuto { + t.Fatalf("expected mode unchanged after cancel, got %q", m.permissionMode) + } +} + +func TestAutoClassifierConfirmationLetterKeys(t *testing.T) { + cases := []struct { + name string + key string + wantMode agent.PermissionMode + wantActive bool + }{ + {name: "lower confirm", key: "y", wantMode: agent.PermissionModeAutoClassifier}, + {name: "upper confirm", key: "Y", wantMode: agent.PermissionModeAutoClassifier}, + {name: "lower cancel", key: "n", wantMode: agent.PermissionModeWorkspaceAuto}, + {name: "upper cancel", key: "N", wantMode: agent.PermissionModeWorkspaceAuto}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + m := newModel(context.Background(), Options{PermissionMode: agent.PermissionModeWorkspaceAuto}) + m.width = 96 + + updated, _ := m.Update(testKeyShift(tea.KeyTab)) + m = updated.(model) + if !m.autoClassifierConfirmActive { + t.Fatal("expected auto-classifier confirmation to be armed") + } + + updated, _ = m.Update(testKeyText(tc.key)) + m = updated.(model) + if m.permissionMode != tc.wantMode || m.autoClassifierConfirmActive != tc.wantActive { + t.Fatalf("after %q got mode %q active %v, want mode %q active %v", tc.key, m.permissionMode, m.autoClassifierConfirmActive, tc.wantMode, tc.wantActive) + } + }) + } +} + +func TestAutoClassifierConfirmRendersInFooterOverResumedTranscript(t *testing.T) { + // The confirmation must render in the footer (live-tail) region so it paints + // reliably even over a non-empty transcript. It must NOT depend on the scrim + // overlay composited into the scrolled transcript viewport. + m := newModel(context.Background(), Options{PermissionMode: agent.PermissionModeWorkspaceAuto}) + m.width = 100 + m.height = 30 + m.altScreen = true + // Simulate a resumed session: a non-empty transcript. + m.transcript = appendRow(m.transcript, rowSystem, "Resumed Zero session") + for i := 0; i < 10; i++ { + m.transcript = appendRow(m.transcript, rowUser, "old message") + } + m.autoClassifierConfirmActive = true + + footer := plainRender(t, m.footerView(m.width)) + if !strings.Contains(footer, "Enable") { + t.Fatalf("expected the confirmation to render in the footer, got:\n%s", footer) + } + + // And it still shows in the full view over the non-empty transcript. + if !strings.Contains(plainRender(t, m.View()), "Enable") { + t.Fatal("expected the confirmation to appear in the full view over a resumed transcript") + } +} + +func TestAutoClassifierConfirmationOnlyPromptsOncePerProcess(t *testing.T) { + // The warning is about the mode, not the session: once acknowledged, cycling + // back to auto-classifier (including after a /resume) must enable it directly + // without re-showing the modal. + m := newModel(context.Background(), Options{PermissionMode: agent.PermissionModeWorkspaceAuto}) + m.width = 96 + + // First cycle arms the confirmation; Enter acknowledges and enables it. + updated, _ := m.Update(testKeyShift(tea.KeyTab)) + m = updated.(model) + if !m.autoClassifierConfirmActive { + t.Fatal("expected the confirmation to arm on the first cycle") + } + updated, _ = m.Update(testKey(tea.KeyEnter)) + m = updated.(model) + if m.permissionMode != agent.PermissionModeAutoClassifier || !m.autoClassifierAcknowledged { + t.Fatalf("expected auto-classifier enabled and acknowledged, got mode %q ack %v", m.permissionMode, m.autoClassifierAcknowledged) + } + + // Cycle away and all the way back around: AutoClassifier -> Ask -> Auto -> + // WorkspaceAuto -> (shift+tab) AutoClassifier again. for _, want := range []agent.PermissionMode{ agent.PermissionModeAsk, agent.PermissionModeAuto, + agent.PermissionModeWorkspaceAuto, } { - updated, cmd := m.Update(testKeyShift(tea.KeyTab)) + updated, _ = m.Update(testKeyShift(tea.KeyTab)) m = updated.(model) - if cmd != nil { - t.Fatalf("expected shift+tab to cycle mode synchronously, got command") - } if m.permissionMode != want { - t.Fatalf("expected permission mode %q after shift+tab, got %q", want, m.permissionMode) + t.Fatalf("cycling: got %q, want %q", m.permissionMode, want) } - if m.permissionMode == agent.PermissionModeUnsafe { - t.Fatalf("shift+tab must never land on Unsafe") + if m.autoClassifierConfirmActive { + t.Fatalf("no confirmation should arm while cycling through %q", want) } } - // The rendered status label tracks the cycled mode. - label, _ := m.modeLabel() - if label != "auto-approve" { - t.Fatalf("expected mode label to track cycled mode, got %q", label) + // This cycle reaches auto-classifier again — it must enable directly, no modal. + updated, _ = m.Update(testKeyShift(tea.KeyTab)) + m = updated.(model) + if m.autoClassifierConfirmActive { + t.Fatal("second time reaching auto-classifier must not re-prompt") + } + if m.permissionMode != agent.PermissionModeAutoClassifier { + t.Fatalf("expected auto-classifier enabled directly, got %q", m.permissionMode) } } @@ -2635,12 +2787,18 @@ func testSessionStore(t *testing.T) *sessions.Store { } func TestNextPermissionModeFoldsUnsafeToAsk(t *testing.T) { - if got := nextPermissionMode(agent.PermissionModeAuto); got != agent.PermissionModeAsk { - t.Fatalf("Auto -> %s, want Ask", got) - } if got := nextPermissionMode(agent.PermissionModeAsk); got != agent.PermissionModeAuto { t.Fatalf("Ask -> %s, want Auto", got) } + if got := nextPermissionMode(agent.PermissionModeAuto); got != agent.PermissionModeWorkspaceAuto { + t.Fatalf("Auto -> %s, want WorkspaceAuto", got) + } + if got := nextPermissionMode(agent.PermissionModeWorkspaceAuto); got != agent.PermissionModeAutoClassifier { + t.Fatalf("WorkspaceAuto -> %s, want AutoClassifier", got) + } + if got := nextPermissionMode(agent.PermissionModeAutoClassifier); got != agent.PermissionModeAsk { + t.Fatalf("AutoClassifier -> %s, want Ask", got) + } // Unsafe must fold to the STRICTER Ask, never Auto (toggling an Unsafe session // must not make it less strict). if got := nextPermissionMode(agent.PermissionModeUnsafe); got != agent.PermissionModeAsk { diff --git a/internal/tui/permission_prompt.go b/internal/tui/permission_prompt.go index 527c6a6bd..48c77b5c0 100644 --- a/internal/tui/permission_prompt.go +++ b/internal/tui/permission_prompt.go @@ -15,6 +15,10 @@ type permissionOption struct { label string hotkey string choice permissionDecision + // commandPrefix, when set, is the exact breadth this option grants — used + // when a prefix decision is expanded into one option per breadth choice. + // Empty means the loop grants the request's default CommandPrefix. + commandPrefix []string } // permissionOptions returns the ordered choices the popup offers. The backend @@ -43,11 +47,13 @@ func permissionOptions(request agent.PermissionRequest) []permissionOption { case agent.PermissionDecisionAllowForSession: options = append(options, permissionOption{label: "allow for session", hotkey: "s", choice: permissionDecisionAllowForSession}) case agent.PermissionDecisionAllowPrefix: - options = append(options, permissionOption{label: "allow command prefix for session", hotkey: "p", choice: permissionDecisionAllowPrefix}) + options = appendPrefixOptions(options, request, permissionDecisionAllowPrefix, "p", "allow command prefix for session", "prefix (session)") + case agent.PermissionDecisionAllowPrefixProject: + options = appendPrefixOptions(options, request, permissionDecisionAllowPrefixProject, "j", "allow command prefix for this project", "prefix (project)") case agent.PermissionDecisionAlwaysAllowPrefix: - options = append(options, permissionOption{label: "always allow command prefix", hotkey: "y", choice: permissionDecisionAlwaysAllowPrefix}) + options = appendPrefixOptions(options, request, permissionDecisionAlwaysAllowPrefix, "y", "allow command prefix globally", "prefix (global)") case agent.PermissionDecisionAlwaysAllow: - options = append(options, permissionOption{label: "always", hotkey: "y", choice: permissionDecisionAlwaysAllow}) + options = append(options, permissionOption{label: "allow in future", hotkey: "f", choice: permissionDecisionAlwaysAllow}) case agent.PermissionDecisionDeny: options = append(options, permissionOption{label: "deny", hotkey: "d", choice: permissionDecisionDeny}) case agent.PermissionDecisionCancel: @@ -60,6 +66,42 @@ func permissionOptions(request agent.PermissionRequest) []permissionOption { return options } +// appendPrefixOptions adds prefix-grant choices. With a single safe prefix it +// keeps one option carrying the request's default label and hotkey. When the +// request offers several breadths it expands into one option per breadth (e.g. +// "prefix (session): npm run *"), so the approver can pick how wide the grant is; +// the breadth equal to the request's default prefix keeps the hotkey. +func appendPrefixOptions(options []permissionOption, request agent.PermissionRequest, choice permissionDecision, hotkey, singleLabel, verb string) []permissionOption { + ladder := request.CommandPrefixOptions + if len(ladder) <= 1 { + return append(options, permissionOption{label: singleLabel, hotkey: hotkey, choice: choice}) + } + for _, prefix := range ladder { + option := permissionOption{ + label: verb + ": " + strings.Join(prefix, " "), + choice: choice, + commandPrefix: append([]string(nil), prefix...), + } + if equalStringPrefix(prefix, request.CommandPrefix) { + option.hotkey = hotkey + } + options = append(options, option) + } + return options +} + +func equalStringPrefix(left, right []string) bool { + if len(left) != len(right) { + return false + } + for index := range left { + if left[index] != right[index] { + return false + } + } + return true +} + // clampPermissionCursor keeps a cursor index within the option range. func clampPermissionCursor(cursor int, request agent.PermissionRequest) int { n := len(permissionOptions(request)) @@ -102,18 +144,18 @@ func (m model) confirmPermissionCursor() (tea.Model, tea.Cmd) { return m.submitPermissionFeedback() } option := permissionOptions(m.pendingPermission.request)[clampPermissionCursor(m.pendingPermission.cursor, m.pendingPermission.request)] - return m.choosePermissionOption(option.choice) + return m.choosePermissionOption(option) } -// choosePermissionOption applies a chosen decision. The cancel choice (the -// "tell Zero what to do differently" row and its [n] hotkey) opens the inline -// feedback field rather than aborting the run; every other choice resolves -// immediately as before. -func (m model) choosePermissionOption(choice permissionDecision) (tea.Model, tea.Cmd) { +// choosePermissionOption applies a chosen option. The cancel choice (the "tell +// Zero what to do differently" row and its [n] hotkey) opens the inline feedback +// field rather than aborting the run; every other choice resolves immediately, +// carrying the option's command-prefix breadth through resolvePermissionOption. +func (m model) choosePermissionOption(option permissionOption) (tea.Model, tea.Cmd) { if m.pendingPermission == nil { return m, nil } - if choice == permissionDecisionCancel { + if option.choice == permissionDecisionCancel { m.pendingPermission.typing = true // Preserve whatever the user had drafted/queued in the composer so it is // restored when they leave feedback mode (submit or cancel). @@ -121,7 +163,7 @@ func (m model) choosePermissionOption(choice permissionDecision) (tea.Model, tea m.input.SetValue("") return m, nil } - return m.resolvePermission(choice) + return m.resolvePermissionOption(option) } // submitPermissionFeedback ends the feedback field. Non-empty text is sent as a diff --git a/internal/tui/permission_prompt_test.go b/internal/tui/permission_prompt_test.go index dfde83592..863bd2508 100644 --- a/internal/tui/permission_prompt_test.go +++ b/internal/tui/permission_prompt_test.go @@ -143,13 +143,53 @@ func TestPermissionOptionsExposePersistentCommandPrefixApproval(t *testing.T) { } card, _ := renderFocusedPermissionPrompt(request, 2, false, "", 100) got := plainRender(t, card) - for _, want := range []string{"always allow `git status`", "[y]"} { + for _, want := range []string{"allow `git status` globally", "[y]"} { if !strings.Contains(got, want) { t.Fatalf("permission card = %q, missing %q", got, want) } } } +func TestPermissionPromptBreadthRowsAreDistinctAndOmitEmptyHotkey(t *testing.T) { + // A multi-breadth prefix grant expands into one row per breadth; each row must + // label its OWN breadth (not the request's full default on every line) and must + // not render an empty "[]" for the breadths that carry no hotkey. + request := agent.PermissionRequest{ + ToolName: "bash", + SideEffect: string(tools.SideEffectWrite), + CommandPrefix: []string{"git", "push", "origin", "feat/permission-modes-improvements"}, + CommandPrefixOptions: [][]string{ + {"git", "push"}, + {"git", "push", "origin"}, + {"git", "push", "origin", "feat/permission-modes-improvements"}, + }, + AvailableDecisions: []agent.PermissionDecisionAction{ + agent.PermissionDecisionAllow, + agent.PermissionDecisionAllowPrefix, + agent.PermissionDecisionAllowPrefixProject, + agent.PermissionDecisionAlwaysAllowPrefix, + agent.PermissionDecisionDeny, + }, + } + card, _ := renderFocusedPermissionPrompt(request, 0, false, "", 140) + got := plainRender(t, card) + + if strings.Contains(got, "[]") { + t.Fatalf("permission card rendered an empty hotkey bracket:\n%s", got) + } + for _, want := range []string{ + "allow `git push` in this session", + "allow `git push origin` in this session", + "allow `git push origin feat/permission-modes-improvements` in this session", + "allow `git push` in this project", + "allow `git push` globally", + } { + if !strings.Contains(got, want) { + t.Fatalf("permission card missing distinct breadth label %q:\n%s", want, got) + } + } +} + func TestPermissionOptionsCanExposePatchCancelWithoutRecoverableDeny(t *testing.T) { request := agent.PermissionRequest{ ToolName: "apply_patch", @@ -353,7 +393,7 @@ func TestPermissionRenderEmitsHighlightedClickableOffsets(t *testing.T) { } lines := strings.Split(plainRender(t, card), "\n") future := offsets[2] - if future < 0 || future >= len(lines) || !strings.Contains(lines[future], "always") { + if future < 0 || future >= len(lines) || !strings.Contains(lines[future], "future") { t.Fatalf("offset[2] (%d) should point at the future line; lines=%#v", future, lines) } if !strings.Contains(lines[future], "▸") { @@ -370,7 +410,7 @@ func TestPermissionRenderShowsNetworkTargetAndHostScopedAlways(t *testing.T) { } card, _ := renderFocusedPermissionPrompt(request, 1, false, "", 72) got := plainRender(t, card) - for _, want := range []string{"target: example.com", "allow this host for this conversation", "[s]", "allow this host in the future", "[y]"} { + for _, want := range []string{"target: example.com", "allow this host for this conversation", "[s]", "allow this host in the future", "[f]"} { if !strings.Contains(got, want) { t.Fatalf("permission card = %q, missing %q", got, want) } @@ -614,3 +654,66 @@ func TestPermissionFeedbackRendersNoClickableOptionsWhileTyping(t *testing.T) { t.Fatalf("feedback mode must register no clickable option offsets, got %#v", typingOffsets) } } + +func TestPermissionOptionsExpandPrefixBreadths(t *testing.T) { + request := agent.PermissionRequest{ + ToolName: "bash", + Action: agent.PermissionActionPrompt, + AvailableDecisions: []agent.PermissionDecisionAction{ + agent.PermissionDecisionAllow, + agent.PermissionDecisionAllowPrefix, + agent.PermissionDecisionDeny, + }, + CommandPrefix: []string{"npm", "run", "test:unit"}, + CommandPrefixOptions: [][]string{{"npm"}, {"npm", "run"}, {"npm", "run", "test:*"}, {"npm", "run", "test:unit"}}, + } + prefixOptions := 0 + hotkeyed := 0 + for _, option := range permissionOptions(request) { + if option.choice != agent.PermissionDecisionAllowPrefix { + continue + } + prefixOptions++ + if len(option.commandPrefix) == 0 { + t.Fatalf("expanded prefix option missing commandPrefix: %#v", option) + } + if option.hotkey != "" { + hotkeyed++ + if !equalStringPrefix(option.commandPrefix, request.CommandPrefix) { + t.Fatalf("hotkey must sit on the default breadth, got %#v", option.commandPrefix) + } + } + } + if prefixOptions != 4 { + t.Fatalf("expected one option per breadth (4), got %d", prefixOptions) + } + if hotkeyed != 1 { + t.Fatalf("expected exactly one hotkeyed prefix option, got %d", hotkeyed) + } +} + +func TestResolvePermissionSendsChosenBreadth(t *testing.T) { + var got agent.PermissionDecision + request := agent.PermissionRequest{ + ToolName: "bash", + Action: agent.PermissionActionPrompt, + AvailableDecisions: []agent.PermissionDecisionAction{agent.PermissionDecisionAllowPrefix, agent.PermissionDecisionDeny}, + CommandPrefix: []string{"npm", "run", "test:unit"}, + CommandPrefixOptions: [][]string{{"npm"}, {"npm", "run"}, {"npm", "run", "test:unit"}}, + } + m := pendingPermissionModelWithRequest(t, request, func(decision agent.PermissionDecision) { got = decision }) + + index := -1 + for position, option := range permissionOptions(request) { + if option.choice == agent.PermissionDecisionAllowPrefix && len(option.commandPrefix) == 2 { + index = position + } + } + if index < 0 { + t.Fatal("expected an npm run breadth option") + } + m.resolvePermissionAt(index) + if !equalStringPrefix(got.CommandPrefix, []string{"npm", "run"}) { + t.Fatalf("decision prefix = %#v, want [npm run]", got.CommandPrefix) + } +} diff --git a/internal/tui/render_cache.go b/internal/tui/render_cache.go index 0c8937392..232093ed3 100644 --- a/internal/tui/render_cache.go +++ b/internal/tui/render_cache.go @@ -199,6 +199,7 @@ func permissionCacheFingerprint(event *agent.PermissionEvent) string { event.SideEffect, event.Reason, event.Scope, + event.ClassifierReason, string(event.Risk.Level), strconv.FormatBool(event.GrantMatched), strconv.FormatBool(event.Grant != nil), diff --git a/internal/tui/rendering.go b/internal/tui/rendering.go index 4cee924f1..104986709 100644 --- a/internal/tui/rendering.go +++ b/internal/tui/rendering.go @@ -1039,8 +1039,12 @@ func renderPermissionRow(row transcriptRow, width int) string { switch event.Action { case agent.PermissionActionAllow: label := "allowed once" - if event.DecisionAction == agent.PermissionDecisionAlwaysAllowPrefix { - label = "always prefix" + if event.DecisionAction == agent.PermissionDecisionAutoClassifierAllow { + label = "auto-reviewed" + } else if event.DecisionAction == agent.PermissionDecisionAlwaysAllowPrefix { + label = "saved prefix (global)" + } else if event.DecisionAction == agent.PermissionDecisionAllowPrefixProject { + label = "saved prefix (project)" } else if event.DecisionAction == agent.PermissionDecisionAllowPrefix { label = "allowed prefix" } else if event.DecisionAction == agent.PermissionDecisionAllowForSession || @@ -1048,12 +1052,21 @@ func renderPermissionRow(row transcriptRow, width int) string { label = "allowed for session" } else if event.DecisionAction == agent.PermissionDecisionAlwaysAllow || event.Grant != nil || event.GrantMatched { - label = "always" + label = "saved permission" } line := zeroTheme.green.Render(label) + dot + zeroTheme.green.Render(displayName) if scope := strings.TrimSpace(event.Scope); scope != "" { line += dot + zeroTheme.muted.Render(permissionEventScopeLabel(event)+":"+scope) } + if event.DecisionAction == agent.PermissionDecisionAutoClassifierAllow { + reasonText := event.DecisionReason + if strings.TrimSpace(reasonText) == "" { + reasonText = event.Reason + } + if reason := permissionDisplayReason(reasonText); reason != "" { + line += zeroTheme.faint.Render(" — " + truncateRunes(reason, maxInt(16, width-lipgloss.Width(displayName)-24))) + } + } return fitStyledLine(line, width) case agent.PermissionActionDeny: line := zeroTheme.red.Render("denied") + dot + zeroTheme.red.Render(displayName) @@ -1138,6 +1151,11 @@ func renderFocusedPermissionPrompt(request agent.PermissionRequest, cursor int, if scope := strings.TrimSpace(request.Scope); scope != "" { lines = append(lines, fill(zeroTheme.muted).Render(permissionScopeLine(request, scope))) } + // In auto-classifier mode, explain why the LLM declined to auto-approve so the + // user knows what the reviewer flagged rather than seeing a bare prompt. + if note := permissionDisplayReason(request.ClassifierReason); note != "" { + lines = append(lines, fill(zeroTheme.amber).Render("classifier: "+note)) + } lines = append(lines, "") @@ -1162,7 +1180,13 @@ func renderFocusedPermissionPrompt(request agent.PermissionRequest, cursor int, offsets := make([]int, len(options)) for index, option := range options { offsets[index] = 1 + len(lines) - hotkey := fill(zeroTheme.faint).Render(" [" + option.hotkey + "]") + // Expanded prefix breadths other than the default carry no hotkey (they are + // reached with the arrow keys), so skip the bracket entirely rather than + // printing an empty "[]". + hotkey := "" + if option.hotkey != "" { + hotkey = fill(zeroTheme.faint).Render(" [" + option.hotkey + "]") + } optionLabel := permissionOptionLabel(option, request) if index == cursor { // onSel, not badge. zeroTheme.badge is the brand chip (" 0 ", " ASK ", @@ -1217,6 +1241,14 @@ func permissionOptionLabel(option permissionOption, request agent.PermissionRequ return option.label } } + // Prefix decisions expand into one option per breadth, each carrying its own + // commandPrefix; label that breadth, not the request's default, so the rows + // read as distinct grants (`git push` vs `git push origin` vs …) instead of + // repeating the full command on every line. + prefix := option.commandPrefix + if len(prefix) == 0 { + prefix = request.CommandPrefix + } switch option.choice { case permissionDecisionAllow: if request.SideEffect == string(tools.SideEffectNetwork) { @@ -1232,15 +1264,20 @@ func permissionOptionLabel(option permissionOption, request agent.PermissionRequ } return "Yes, and don't ask again for this command in this session" case permissionDecisionAllowPrefix: - if len(request.CommandPrefix) > 0 { - return "Yes, and allow `" + strings.Join(request.CommandPrefix, " ") + "` in this session" + if len(prefix) > 0 { + return "Yes, and allow `" + strings.Join(prefix, " ") + "` in this session" } return "Yes, and allow this command prefix in this session" + case permissionDecisionAllowPrefixProject: + if len(prefix) > 0 { + return "Yes, and allow `" + strings.Join(prefix, " ") + "` in this project" + } + return "Yes, and allow this command prefix in this project" case permissionDecisionAlwaysAllowPrefix: - if len(request.CommandPrefix) > 0 { - return "Yes, and always allow `" + strings.Join(request.CommandPrefix, " ") + "`" + if len(prefix) > 0 { + return "Yes, and allow `" + strings.Join(prefix, " ") + "` globally" } - return "Yes, and always allow this command prefix" + return "Yes, and allow this command prefix globally" case permissionDecisionAlwaysAllow: if request.SideEffect == string(tools.SideEffectNetwork) { return "Yes, and allow this host in the future" diff --git a/internal/tui/rendering_lime_test.go b/internal/tui/rendering_lime_test.go index a363a4df2..8a31df8e7 100644 --- a/internal/tui/rendering_lime_test.go +++ b/internal/tui/rendering_lime_test.go @@ -1661,7 +1661,7 @@ func TestFocusedPermissionCardShowsBadgeAndKeys(t *testing.T) { } card, offsets := renderFocusedPermissionPrompt(request, 0, false, "", 80) got := plainRender(t, card) - for _, want := range []string{"PERMISSION", "edit_file", "writes internal/agent/exec.go", "Yes, proceed", "[a]", "this session", "[s]", "don't ask again", "[y]", "continue without running it", "[d]", "[esc]"} { + for _, want := range []string{"PERMISSION", "edit_file", "writes internal/agent/exec.go", "Yes, proceed", "[a]", "this session", "[s]", "don't ask again", "[f]", "continue without running it", "[d]", "[esc]"} { if !strings.Contains(got, want) { t.Fatalf("permission card = %q, missing %q", got, want) } @@ -1708,6 +1708,16 @@ func TestPermissionPromptCollapsesAfterDecision(t *testing.T) { t.Fatalf("session allow = %q, want allowed for session · bash", got) } + autoReviewed := transcriptRow{kind: rowPermission, id: "call_auto", permission: &agent.PermissionEvent{ + ToolCallID: "call_auto", ToolName: "write_file", Action: agent.PermissionActionAllow, DecisionAction: agent.PermissionDecisionAutoClassifierAllow, Reason: "workspace write is allowed", DecisionReason: "auto-reviewed by LLM classifier: workspace write is low risk", + }} + if rcSession.skip(autoReviewed) { + t.Fatal("auto-reviewed allow rows should remain as audit lines") + } + if got := plainRender(t, m.renderRow(autoReviewed, 96, rcSession)); !strings.Contains(got, "auto-reviewed · write_file") || !strings.Contains(got, "auto-reviewed by LLM classifier") { + t.Fatalf("auto-reviewed allow = %q, want label and decision reason", got) + } + grant := &agent.PermissionEvent{ToolCallID: "call_2", ToolName: "bash", Action: agent.PermissionActionAllow} grant.Grant = &sandbox.Grant{ToolName: "bash"} always := transcriptRow{kind: rowPermission, id: "call_2", permission: grant} @@ -1718,8 +1728,8 @@ func TestPermissionPromptCollapsesAfterDecision(t *testing.T) { if rcTwo.skip(always) { t.Fatal("always allow rows should remain as audit lines") } - if got := plainRender(t, m.renderRow(always, 80, rcTwo)); !strings.Contains(got, "always · bash") { - t.Fatalf("always allow = %q, want always · bash", got) + if got := plainRender(t, m.renderRow(always, 80, rcTwo)); !strings.Contains(got, "saved permission · bash") { + t.Fatalf("saved permission allow = %q, want saved permission · bash", got) } denied := transcriptRow{kind: rowPermission, id: "call_3", permission: &agent.PermissionEvent{ diff --git a/internal/tui/session.go b/internal/tui/session.go index 9f2a6f544..ef79db195 100644 --- a/internal/tui/session.go +++ b/internal/tui/session.go @@ -680,6 +680,7 @@ func permissionEventFromPayload(payload map[string]any) agent.PermissionEvent { Reason: payloadString(payload, "reason"), Scope: payloadString(payload, "scope"), DecisionReason: payloadString(payload, "decisionReason"), + ClassifierReason: payloadString(payload, "classifierReason"), GrantMatched: payloadBool(payload, "grantMatched"), } if risk, ok := payloadMap(payload, "risk"); ok { diff --git a/internal/tui/session_test.go b/internal/tui/session_test.go index 96821794f..d033729b7 100644 --- a/internal/tui/session_test.go +++ b/internal/tui/session_test.go @@ -418,7 +418,7 @@ func TestPermissionPromptAlwaysPersistsGrantAndSkipsLaterPrompt(t *testing.T) { runtimeMessageCh := make(chan tea.Msg, 8) m := newPermissionTestModel(root, provider, registry, store, grantStore, runtimeMessageCh) - next := submitAndDrivePermissionRun(t, m, "write first", "y", runtimeMessageCh, 4) + next := submitAndDrivePermissionRun(t, m, "write first", "f", runtimeMessageCh, 4) // The always-decision persists a grant scoped to exactly the file written. lookup, err := grantStore.Lookup("write_file", filepath.Join(root, "notes.txt")) diff --git a/internal/tui/transcript.go b/internal/tui/transcript.go index e051bfc85..850ae10d7 100644 --- a/internal/tui/transcript.go +++ b/internal/tui/transcript.go @@ -401,6 +401,9 @@ func permissionDetailText(event agent.PermissionEvent) string { if event.Reason != "" { parts = append(parts, permissionDisplayReason(event.Reason)) } + if note := strings.TrimSpace(event.ClassifierReason); note != "" { + parts = append(parts, "classifier: "+note) + } if event.Block != nil { parts = append(parts, permissionBlockDetail(event)) } @@ -413,14 +416,18 @@ func permissionDecisionDetail(decision agent.PermissionDecisionAction) string { return "approved once" case agent.PermissionDecisionAllowStrict: return "approved with review" + case agent.PermissionDecisionAutoClassifierAllow: + return "auto-reviewed approval" case agent.PermissionDecisionAllowForSession: return "approved for this session" case agent.PermissionDecisionAllowPrefix: return "approved command prefix for this session" + case agent.PermissionDecisionAllowPrefixProject: + return "saved command prefix for this project" case agent.PermissionDecisionAlwaysAllowPrefix: - return "always approved command prefix" + return "saved command prefix permission (global)" case agent.PermissionDecisionAlwaysAllow: - return "always approved" + return "saved permission" case agent.PermissionDecisionDeny: return "denied by user" case agent.PermissionDecisionCancel: diff --git a/internal/tui/transcript_selection.go b/internal/tui/transcript_selection.go index f81991545..4a87df2b5 100644 --- a/internal/tui/transcript_selection.go +++ b/internal/tui/transcript_selection.go @@ -70,10 +70,12 @@ type transcriptSelectableLine struct { toggle bool live bool // permOption marks a clickable permission-popup choice; permChoice is the - // decision a left-click on this row resolves. These rows carry no selectable - // text (they are buttons, not content). - permOption bool - permChoice permissionDecision + // decision a left-click on this row resolves and permOptionIndex is the + // option's position (breadths that share a decision action stay distinct by + // index). These rows carry no selectable text (they are buttons, not content). + permOption bool + permChoice permissionDecision + permOptionIndex int // specialistCard marks a clickable specialist card row. specialistID is // the childSessionID to drill into on click or Enter. specialistCard bool @@ -462,10 +464,11 @@ func (m model) buildTranscriptBodyItems(width int, emptyOverlay string, detailed break } selectable = append(selectable, transcriptSelectableLine{ - bodyY: startBodyY + offset, - rowIndex: -1, - permOption: true, - permChoice: options[index].choice, + bodyY: startBodyY + offset, + rowIndex: -1, + permOption: true, + permChoice: options[index].choice, + permOptionIndex: index, }) } return transcriptBodyRenderedItem{lines: viewLines(block), selectable: selectable} @@ -1402,14 +1405,11 @@ func (m model) handleTranscriptSelectionMouse(msg tea.MouseMsg) (model, tea.Cmd, return m, nil, false } if line.permOption && !(m.pendingPermission != nil && m.pendingPermission.typing) { - // A left-click on a permission-popup option resolves it directly. The - // typing guard is defence-in-depth: renderFocusedPermissionPrompt already - // returns nil offsets in feedback mode, so no option row is registered as - // clickable then — but that single early-return is the only thing keeping - // a stray click (Allow included) off the decision path, and it lives in a - // function other PRs also edit. Guarding here makes the safety explicit - // rather than emergent. - next, cmd := m.resolvePermission(line.permChoice) + // A left-click on a permission-popup option resolves it directly. Address + // by index so expanded prefix breadths that share a decision action are + // distinguished. The typing guard defends the feedback field: no option row + // is registered as clickable in feedback mode, and this makes that explicit. + next, cmd := m.resolvePermissionAt(line.permOptionIndex) return next.(model), cmd, true } if line.specialistCard { diff --git a/internal/tui/view.go b/internal/tui/view.go index c3d949805..dbf2a2307 100644 --- a/internal/tui/view.go +++ b/internal/tui/view.go @@ -300,21 +300,26 @@ func providerDisplayNameIsGenericCustom(name string) bool { } } -// nextPermissionMode toggles between the two prompt-respecting modes: -// Auto ⇄ Ask. Unsafe (which disables permission prompts entirely) is -// deliberately NOT reachable by a casual keypress — a single shift+tab landing -// on it would let prompt-required tools run with no decision. Unsafe stays an -// explicit opt-in (the launch/--skip-permissions-unsafe path), not a UI toggle. -// Unsafe is folded back to Ask so the toggle always lands somewhere safe. +// nextPermissionMode cycles the prompt-respecting interactive modes from safest +// to more autonomous: Ask -> Auto -> WorkspaceAuto -> AutoClassifier -> Ask. +// Unsafe (which disables permission prompts entirely) is deliberately NOT +// reachable by a casual keypress — a single shift+tab landing on it would let +// prompt-required tools run with no decision. Unsafe stays an explicit opt-in +// (the launch/--skip-permissions-unsafe path), not a UI toggle. Unsafe is folded +// back to Ask so the toggle always lands somewhere safe. func nextPermissionMode(mode agent.PermissionMode) agent.PermissionMode { switch mode { - case agent.PermissionModeAuto: - return agent.PermissionModeAsk case agent.PermissionModeAsk: return agent.PermissionModeAuto + case agent.PermissionModeAuto: + return agent.PermissionModeWorkspaceAuto + case agent.PermissionModeWorkspaceAuto: + return agent.PermissionModeAutoClassifier + case agent.PermissionModeAutoClassifier: + return agent.PermissionModeAsk default: - // Anything else (incl. an externally-set Unsafe) folds to Ask — the stricter - // landing, so toggling never makes an Unsafe session less strict. + // Anything else (incl. an externally-set Unsafe) folds to Ask — the strictest + // landing, so toggling never silently escalates autonomy. return agent.PermissionModeAsk } } @@ -322,15 +327,19 @@ func nextPermissionMode(mode agent.PermissionMode) agent.PermissionMode { func (m model) modeLabel() (string, lipgloss.Style) { switch m.permissionMode { case agent.PermissionModeAuto: - return "auto-approve", zeroTheme.modeAuto + return agent.PermissionModeSummary(agent.PermissionModeAuto), zeroTheme.modeAuto + case agent.PermissionModeWorkspaceAuto: + return agent.PermissionModeSummary(agent.PermissionModeWorkspaceAuto), zeroTheme.modeAuto + case agent.PermissionModeAutoClassifier: + return agent.PermissionModeSummary(agent.PermissionModeAutoClassifier), zeroTheme.modeAuto case agent.PermissionModeAsk: - return "ask", zeroTheme.modeAsk + return agent.PermissionModeSummary(agent.PermissionModeAsk), zeroTheme.modeAsk case agent.PermissionModeUnsafe: - return "unsafe", zeroTheme.modeUnsafe + return agent.PermissionModeSummary(agent.PermissionModeUnsafe), zeroTheme.modeUnsafe default: mode := strings.TrimSpace(string(m.permissionMode)) if mode == "" { - return "auto-approve", zeroTheme.modeAuto + return agent.PermissionModeSummary(agent.PermissionModeAuto), zeroTheme.modeAuto } return mode, zeroTheme.muted } @@ -732,6 +741,36 @@ func fileSelectableItem(token string) selectableListItem { return selectableListItem{Label: base, Description: dir} } +// autoClassifierConfirmOverlay renders the blocking confirmation shown before +// auto-classifier mode is enabled. Because that mode hands each permission +// decision to an LLM, enabling it needs an explicit acknowledgement of what +// gets auto-run rather than a single shift+tab keypress. +func (m model) autoClassifierConfirmOverlay(width int) string { + if !m.autoClassifierConfirmActive { + return "" + } + overlayWidth := minInt(width, pickerOverlayMaxWidth) + if overlayWidth < pickerOverlayMinWidth { + overlayWidth = width + } + label := agent.PermissionModeLabel(agent.PermissionModeAutoClassifier) + hint := zeroTheme.accent.Render("Enter") + zeroTheme.muted.Render(" / ") + zeroTheme.accent.Render("y") + + zeroTheme.muted.Render(" enable") + zeroTheme.muted.Render(" ") + + zeroTheme.accent.Render("Esc") + zeroTheme.muted.Render(" / ") + zeroTheme.accent.Render("n") + zeroTheme.muted.Render(" cancel") + lines := []string{ + "", + zeroTheme.ink.Bold(true).Render("Enable “" + label + "”?"), + "", + zeroTheme.muted.Render("An LLM reviews each low-risk action and runs it without"), + zeroTheme.muted.Render("asking. Network, destructive, escalated, and out-of-"), + zeroTheme.muted.Render("workspace actions still ask every time."), + "", + hint, + "", + } + return centerRenderedBlock(styledBlockFillTitle(overlayWidth, "Confirm permission mode", lines, zeroTheme.amber, lipgloss.NewStyle()), width) +} + // pickerOverlay renders an open interactive selector as a centered modal: a // bordered panel with a title-and-hints row, rows carrying a provider dot and // right metadata when the catalog exposes them, and the selected row on the From 7f487db4a023ccafdd6e2a9bbce504d61b9b1ff4 Mon Sep 17 00:00:00 2001 From: pengdst Date: Wed, 22 Jul 2026 20:35:23 +0700 Subject: [PATCH 2/6] fix(permission): resolve symlinks before treating a prefix grant as project-confined The cd-containment guard cleaned paths and compared them lexically, so a symlink inside the workspace pointing outside it (e.g. /workspace/link -> /outside) passed the check: `cd link && go test` matched a project prefix grant and was promoted to an escalated, unsandboxed command that then ran outside the approved project. Resolve real paths with EvalSymlinks before the containment check: the root and each cd target are canonicalized, and a target that does not exist or whose real path escapes the root fails closed to the sandboxed prompt. --- internal/agent/command_prefix.go | 35 +++++++++++++++++++++++---- internal/agent/command_prefix_test.go | 14 +++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/internal/agent/command_prefix.go b/internal/agent/command_prefix.go index 185a02d8c..9c1b2e267 100644 --- a/internal/agent/command_prefix.go +++ b/internal/agent/command_prefix.go @@ -223,23 +223,48 @@ func matchCommandPrefix(toolName string, args map[string]any, options Options) ( // With no root there is no project to bind to, so any `cd` is rejected. func commandDirStaysWithinProject(segments [][]string, root string) bool { root = strings.TrimSpace(root) - effective := root + if root == "" { + // No project to bind to: only safe if the command never changes directory. + return !commandChangesDirectory(segments) + } + // Resolve the root's real path once. Containment is checked against real paths + // (EvalSymlinks) so a symlink inside the workspace pointing outside it cannot + // satisfy a lexical prefix check and smuggle an unsandboxed grant out of scope. + realRoot, err := filepath.EvalSymlinks(root) + if err != nil { + return false + } + effective := realRoot for _, tokens := range segments { if len(tokens) == 0 || commandName(tokens[0]) != "cd" { continue } - if root == "" { + target, ok := resolveCdTarget(tokens[1:], effective) + if !ok { return false } - target, ok := resolveCdTarget(tokens[1:], effective) - if !ok || !pathWithinRoot(target, root) { + // The target must exist and, once symlinks are resolved, stay within the + // real workspace root. A non-existent target or a broken/escaping symlink + // cannot be proven in-project, so it fails closed. + realTarget, err := filepath.EvalSymlinks(target) + if err != nil || !pathWithinRoot(realTarget, realRoot) { return false } - effective = target + effective = realTarget } return true } +// commandChangesDirectory reports whether any segment is a `cd`. +func commandChangesDirectory(segments [][]string) bool { + for _, tokens := range segments { + if len(tokens) > 0 && commandName(tokens[0]) == "cd" { + return true + } + } + return false +} + // resolveCdTarget resolves a `cd` argument list to an absolute directory relative // to cwd. ok is false for forms whose destination cannot be known statically. func resolveCdTarget(args []string, cwd string) (string, bool) { diff --git a/internal/agent/command_prefix_test.go b/internal/agent/command_prefix_test.go index 4bea575a8..7816df198 100644 --- a/internal/agent/command_prefix_test.go +++ b/internal/agent/command_prefix_test.go @@ -1,6 +1,7 @@ package agent import ( + "os" "path/filepath" "runtime" "testing" @@ -180,11 +181,24 @@ func TestMatchCommandPrefixRejectsGrantWhenCdEscapesProject(t *testing.T) { } // A `cd` that stays inside the project still honors the grant. + if err := os.MkdirAll(filepath.Join(root, "sub"), 0o755); err != nil { + t.Fatalf("mkdir sub: %v", err) + } inside := "cd sub && go test ./..." if _, ok, _ := matchCommandPrefix("bash", map[string]any{"command": inside}, Options{Sandbox: engine}); !ok { t.Fatal("expected a within-project cd to still match the grant") } + // A symlink inside the workspace that points outside it must not satisfy the + // containment check: the real path is resolved before comparison, so this + // cannot smuggle an unsandboxed grant out of the project. + if err := os.Symlink(outside, filepath.Join(root, "escape")); err != nil { + t.Fatalf("symlink: %v", err) + } + if grant, ok, _ := matchCommandPrefix("bash", map[string]any{"command": "cd escape && go test ./..."}, Options{Sandbox: engine}); ok { + t.Fatalf("expected a symlinked-out cd to refuse the grant, got %#v", grant) + } + // A non-static cd target (home) cannot be proven in-project, so it is refused. if _, ok, _ := matchCommandPrefix("bash", map[string]any{"command": "cd && go test ./..."}, Options{Sandbox: engine}); ok { t.Fatal("expected a bare `cd` (home) to refuse the grant") From d93805ec4027ab6f76de2b7be382e0d186319cb0 Mon Sep 17 00:00:00 2001 From: pengdst Date: Wed, 22 Jul 2026 20:51:22 +0700 Subject: [PATCH 3/6] fix(permission): keep protected-metadata and unsandboxed-shell prompts off the classifier Two auto-classifier boundaries were overridable: - Writes under protected workspace metadata (.git/**, .zero/**, .agents/**) deliberately stay prompts, but the decision carried no marker excluding them from classifier review, so an LLM allow could write .git/hooks/pre-commit or .git/config and defeat the boundary (enabling later Git-triggered execution). The sandbox now flags such writes (Decision.TouchesProtectedMetadata) and the classifier gate refuses them. - When native shell isolation is unavailable the sandbox prompts instead of wrapping a shell command; the classifier could turn that prompt into an allow and run the command unwrapped on the host. Shell commands are now classifier- eligible only when the native sandbox would actually wrap them (Engine.ShellSandboxActive); file tools, confined by path validation, are unaffected. --- internal/agent/loop.go | 15 ++++++ .../agent/permission_additional_perms_test.go | 48 +++++++++++++++++++ internal/sandbox/engine.go | 20 +++++++- internal/sandbox/types.go | 5 ++ 4 files changed, 87 insertions(+), 1 deletion(-) diff --git a/internal/agent/loop.go b/internal/agent/loop.go index 95375ec8f..016fee849 100644 --- a/internal/agent/loop.go +++ b/internal/agent/loop.go @@ -2156,6 +2156,21 @@ func canAutoClassifierReview(tool tools.Tool, args map[string]any, permissionGra if tool.Safety().Permission != tools.PermissionPrompt { return false } + // A shell command may only be reviewed when the native sandbox would actually + // wrap it. With isolation unavailable the command runs unwrapped on the host, + // so an LLM allow there is an unsandboxed-execution bypass — keep it a prompt. + // (File tools like write_file are confined by path validation, not native + // shell isolation, so this gate applies to shell commands only.) + if isShellCommandTool(tool.Name()) && !options.Sandbox.ShellSandboxActive() { + return false + } + // A write under protected workspace metadata (.git/**, .zero/**, .agents/**) + // stays an explicit prompt: an LLM allow must not be able to write + // .git/hooks/pre-commit or .git/config and defeat that boundary (which would + // enable later Git-triggered code execution). + if decision.TouchesProtectedMetadata { + return false + } // Auto-classifier trades autonomy for an LLM check: it reviews only actions // the sandbox would OTHERWISE prompt for, and may auto-approve the ones it // judges safe. Actions the sandbox already auto-allows run without LLM diff --git a/internal/agent/permission_additional_perms_test.go b/internal/agent/permission_additional_perms_test.go index 0b32d07df..ec6f4f017 100644 --- a/internal/agent/permission_additional_perms_test.go +++ b/internal/agent/permission_additional_perms_test.go @@ -24,6 +24,54 @@ func (promptShellTool) Run(context.Context, map[string]any) tools.Result { return tools.Result{Status: tools.StatusOK} } +// promptWriteTool is a non-shell, prompt-gated write tool used to exercise the +// auto-classifier eligibility rules for file writes (not shell commands). +type promptWriteTool struct{} + +func (promptWriteTool) Name() string { return "write_file" } +func (promptWriteTool) Description() string { return "write a file" } +func (promptWriteTool) Parameters() tools.Schema { + return tools.Schema{Type: "object", AdditionalProperties: false} +} +func (promptWriteTool) Safety() tools.Safety { + return tools.Safety{SideEffect: tools.SideEffectWrite, Permission: tools.PermissionPrompt, Reason: "writes a file"} +} +func (promptWriteTool) Run(context.Context, map[string]any) tools.Result { + return tools.Result{Status: tools.StatusOK} +} + +func TestCanAutoClassifierReviewExcludesProtectedMetadataWrite(t *testing.T) { + classifier := func(context.Context, AutoPermissionClassifierRequest) (AutoPermissionClassifierDecision, error) { + return AutoPermissionClassifierDecision{}, nil + } + options := Options{AutoPermissionClassifier: classifier, Sandbox: sandbox.NewEngine(sandbox.EngineOptions{WorkspaceRoot: t.TempDir(), Policy: sandbox.DefaultPolicy()})} + args := map[string]any{"path": ".git/config", "content": "x"} + + ordinary := &sandbox.Decision{Action: sandbox.ActionPrompt} + if !canAutoClassifierReview(promptWriteTool{}, args, false, PermissionModeAutoClassifier, options, ordinary) { + t.Fatal("an ordinary workspace-write prompt should be classifier-eligible") + } + + protected := &sandbox.Decision{Action: sandbox.ActionPrompt, TouchesProtectedMetadata: true} + if canAutoClassifierReview(promptWriteTool{}, args, false, PermissionModeAutoClassifier, options, protected) { + t.Fatal("a protected-metadata write must not be classifier-eligible") + } +} + +func TestCanAutoClassifierReviewRequiresActiveShellSandbox(t *testing.T) { + classifier := func(context.Context, AutoPermissionClassifierRequest) (AutoPermissionClassifierDecision, error) { + return AutoPermissionClassifierDecision{}, nil + } + // A disabled sandbox never wraps a shell command, so an LLM allow would run it + // unwrapped on the host: the classifier must not be eligible in that case. + inactive := sandbox.NewEngine(sandbox.EngineOptions{WorkspaceRoot: t.TempDir(), Policy: sandbox.Policy{Mode: sandbox.ModeDisabled}}) + options := Options{AutoPermissionClassifier: classifier, Sandbox: inactive} + decision := &sandbox.Decision{Action: sandbox.ActionPrompt} + if canAutoClassifierReview(promptShellTool{}, map[string]any{"command": "echo hi"}, false, PermissionModeAutoClassifier, options, decision) { + t.Fatal("a shell command must not be classifier-eligible without an active native sandbox") + } +} + // A command whose sandbox decision is Allow but that explicitly requests // additional sandbox permissions is an ELEVATION: it must be surfaced as a // prompt, never Action=allow. Previously it carried allow while the loop still diff --git a/internal/sandbox/engine.go b/internal/sandbox/engine.go index 1561c8118..691d4f985 100644 --- a/internal/sandbox/engine.go +++ b/internal/sandbox/engine.go @@ -291,6 +291,18 @@ func (engine *Engine) shellSandboxActive(policy Policy) bool { return true } +// ShellSandboxActive reports whether a shell command would actually be wrapped by +// the native sandbox under the engine's effective policy. Callers use it to gate +// autonomy that assumes confinement (e.g. auto-classifier review): when native +// isolation is unavailable, an auto-approved shell command would run unwrapped on +// the host, so that path must stay an explicit prompt instead. +func (engine *Engine) ShellSandboxActive() bool { + if engine == nil { + return false + } + return engine.shellSandboxActive(engine.effectivePolicy(engine.policy)) +} + // Precheck reports the sandbox blocks that would block a tool request BEFORE // it executes, so a caller (e.g. a batch confirmation or a "would this run?" // check) can fail fast and surface the reason instead of discovering it mid-run. @@ -456,7 +468,13 @@ func (engine *Engine) Evaluate(ctx context.Context, request Request) Decision { if request.PermissionGranted || request.PermissionMode == PermissionUnsafe { return Decision{Action: ActionAllow, Risk: risk, Reason: permissionReason(request)} } - return Decision{Action: ActionPrompt, Risk: risk, Reason: permissionReason(request)} + prompt := Decision{Action: ActionPrompt, Risk: risk, Reason: permissionReason(request)} + // Flag a write that targets protected workspace metadata so autonomy layers + // keep prompting for it instead of auto-approving a .git/.zero/.agents change. + if request.SideEffect == SideEffectWrite && requestPathsTouchProtectedMetadata(scope, request.WorkspaceRoot, requestPaths(request)) { + prompt.TouchesProtectedMetadata = true + } + return prompt } func requestRequiresEscalatedSandbox(request Request) bool { diff --git a/internal/sandbox/types.go b/internal/sandbox/types.go index 897254546..59e49a61d 100644 --- a/internal/sandbox/types.go +++ b/internal/sandbox/types.go @@ -218,6 +218,11 @@ type Decision struct { // grant-authorized allow so a prompt tool runs without a separately-recorded // PermissionGranted. AutoAllowed bool `json:"autoAllowed,omitempty"` + // TouchesProtectedMetadata marks a write whose target is under protected + // workspace metadata (.git/**, .zero/**, .agents/**). Such writes deliberately + // stay explicit prompts; callers use this so autonomy layers (e.g. the + // auto-classifier) never auto-approve a change to that control plane. + TouchesProtectedMetadata bool `json:"touchesProtectedMetadata,omitempty"` } type Risk struct { From 287c1d6879ee66e5237d7d225837a4ff8528e04b Mon Sep 17 00:00:00 2001 From: pengdst Date: Wed, 22 Jul 2026 20:51:22 +0700 Subject: [PATCH 4/6] fix(acp): do not expose auto-classifier mode without the required opt-in Enabling auto-classifier lets an LLM auto-approve low-risk actions; the TUI gates it behind an explicit one-time acknowledgement. ACP had no equivalent, so an editor client could select the mode via session/set_mode and silently enable LLM auto-approval. ACP cannot represent that confirmation, so the mode is no longer advertised in the available modes and set_mode rejects it, matching how Unsafe is already withheld from ACP. --- internal/acp/agent.go | 12 +++++++++--- internal/acp/agent_test.go | 5 +++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/internal/acp/agent.go b/internal/acp/agent.go index 4de74788c..db6cf26cd 100644 --- a/internal/acp/agent.go +++ b/internal/acp/agent.go @@ -354,10 +354,16 @@ func (a *Agent) handleSetMode(_ context.Context, params json.RawMessage) (any, e } mode := agent.PermissionMode(p.ModeID) switch mode { - case agent.PermissionModeAsk, agent.PermissionModeAuto, agent.PermissionModeWorkspaceAuto, agent.PermissionModeAutoClassifier: + case agent.PermissionModeAsk, agent.PermissionModeAuto, agent.PermissionModeWorkspaceAuto: sess.setMode(mode) (¬ifier{conn: a.conn, sessionID: sess.id}).currentMode(string(mode)) return SetSessionModeResult{}, nil + case agent.PermissionModeAutoClassifier: + // Auto-classifier hands each low-risk decision to an LLM that may approve it + // with no prompt. The TUI gates enabling it behind an explicit one-time user + // acknowledgement; ACP has no way to represent that confirmation, so an + // editor client must not be able to turn it on over the wire. + return nil, RPCError(codeInvalidParams, "mode requires an in-app confirmation and cannot be enabled over ACP: "+p.ModeID) case agent.PermissionModeUnsafe: // Unsafe = run every tool with no prompt. The TUI gates this behind an // explicit --skip-permissions-unsafe operator flag; an editor client must @@ -448,14 +454,14 @@ func (a *Agent) modeState(s *acpSession) *SessionModeState { ask := agent.PermissionModeInfoFor(agent.PermissionModeAsk) auto := agent.PermissionModeInfoFor(agent.PermissionModeAuto) workspaceAuto := agent.PermissionModeInfoFor(agent.PermissionModeWorkspaceAuto) - autoClassifier := agent.PermissionModeInfoFor(agent.PermissionModeAutoClassifier) + // Auto-classifier is intentionally NOT advertised: enabling it needs an in-app + // confirmation the ACP protocol cannot represent, and handleSetMode rejects it. return &SessionModeState{ CurrentModeID: string(s.currentMode()), AvailableModes: []SessionMode{ {ID: string(ask.ID), Name: ask.Label, Description: ask.Description}, {ID: string(auto.ID), Name: auto.Label, Description: auto.Description}, {ID: string(workspaceAuto.ID), Name: workspaceAuto.Label, Description: workspaceAuto.Description}, - {ID: string(autoClassifier.ID), Name: autoClassifier.Label, Description: autoClassifier.Description}, }, } } diff --git a/internal/acp/agent_test.go b/internal/acp/agent_test.go index 2e5d9a084..0d872f628 100644 --- a/internal/acp/agent_test.go +++ b/internal/acp/agent_test.go @@ -396,6 +396,11 @@ func TestACPSetModeUpdatesSession(t *testing.T) { if err := h.client.Call(ctx, MethodSessionSetMode, SetSessionModeParams{SessionID: newRes.SessionID, ModeID: string(agent.PermissionModeUnsafe)}, &SetSessionModeResult{}); err == nil { t.Fatal("expected Unsafe mode to be rejected over ACP") } + // Auto-classifier requires an in-app confirmation the ACP protocol cannot + // represent, so a client must not be able to enable it over the wire. + if err := h.client.Call(ctx, MethodSessionSetMode, SetSessionModeParams{SessionID: newRes.SessionID, ModeID: string(agent.PermissionModeAutoClassifier)}, &SetSessionModeResult{}); err == nil { + t.Fatal("expected auto-classifier mode to be rejected over ACP") + } // An unknown mode must be rejected. if err := h.client.Call(ctx, MethodSessionSetMode, SetSessionModeParams{SessionID: newRes.SessionID, ModeID: "bogus"}, &SetSessionModeResult{}); err == nil { t.Fatal("expected error for unknown mode") From 70d5e8233d9d297a03f5c68c1fbe6f2d85c1f4ce Mon Sep 17 00:00:00 2001 From: pengdst Date: Wed, 2 Sep 2026 21:30:29 +0700 Subject: [PATCH 5/6] fix(permission): guard workdir aliases and fix merge fallout - commandDirStaysWithinProject now checks effective directory from workdir/cwd/dir/directory (exec_command) and cwd (bash), resolving symlinks and refusing outside-project starts. This covers the Vasanth-found escape where {command:"go test", cwd:outside} bypassed the cd-only guard while being the recommended cwd-over-cd path. - add TestMatchCommandPrefixRejectsGrantWhenWorkdirEscapesProject covering all 4 aliases + bash cwd, inside/outside/symlink/relative cases - fix loop_test.go: NewWriteFileTool -> NewScopedWriteFileTool after main rename - move equalStringSlices to production (command_prefix.go) and remove duplicate from export_test.go so go vet passes for non-test builds Fixes Gitlawb/zero#570 review blocker, re-mergeable after 1b5db176. --- internal/agent/command_prefix.go | 91 +++++++++++++++++++++++---- internal/agent/command_prefix_test.go | 50 +++++++++++++++ internal/agent/export_test.go | 14 +---- internal/agent/loop_test.go | 10 +-- 4 files changed, 137 insertions(+), 28 deletions(-) diff --git a/internal/agent/command_prefix.go b/internal/agent/command_prefix.go index 72668eef3..6839f895d 100644 --- a/internal/agent/command_prefix.go +++ b/internal/agent/command_prefix.go @@ -179,10 +179,12 @@ func matchCommandPrefix(toolName string, args map[string]any, options Options) ( // shellExecutionArgsForApproval). `cd` is a known-safe segment, so a composite // like `cd /other && go test` would otherwise honor a grant saved for THIS // project yet execute in another directory outside it. Bind the grant to the - // effective directory: if a `cd` moves execution outside the workspace root (or - // to a target we cannot prove stays inside it), refuse the match so the command + // effective directory: if the effective starting directory (workdir/cwd/dir/ + // directory) or any `cd` moves execution outside the workspace root (or to a + // target we cannot prove stays inside it), refuse the match so the command // falls back to the normal sandboxed prompt instead of an out-of-scope bypass. - if !commandDirStaysWithinProject(segments, options.Sandbox.WorkspaceRoot()) { + effectiveDir := effectiveCommandDir(args) + if !commandDirStaysWithinProject(segments, options.Sandbox.WorkspaceRoot(), effectiveDir) { return sandbox.CommandPrefixGrant{}, false, false } var matched sandbox.CommandPrefixGrant @@ -215,16 +217,45 @@ func matchCommandPrefix(toolName string, args map[string]any, options Options) ( return sandbox.CommandPrefixGrant{}, false, false } -// commandDirStaysWithinProject reports whether a composite command's `cd` -// segments keep execution inside root. It starts at root and follows each `cd`; -// a target that resolves outside root, or one that cannot be resolved statically -// (no argument, `-`, `~`/home, an environment variable, a glob, or extra args), -// is treated as leaving the project so the caller refuses the unsandboxed grant. -// With no root there is no project to bind to, so any `cd` is rejected. -func commandDirStaysWithinProject(segments [][]string, root string) bool { +// effectiveCommandDir extracts the tool's effective starting directory from args. +// exec_command accepts workdir/cwd/dir/directory (in that priority, see +// internal/tools/exec_command.go), bash accepts cwd. We check all four aliases +// so a grant cannot be smuggled outside the project via the recommended +// workdir argument while the guard only watched ` + "`cd`" + `. +func effectiveCommandDir(args map[string]any) string { + for _, key := range []string{"workdir", "cwd", "dir", "directory"} { + if raw, ok := args[key]; ok { + if s, ok := raw.(string); ok { + s = strings.TrimSpace(s) + if s != "" { + return s + } + } + } + } + return "." +} + +// commandDirStaysWithinProject reports whether a composite command's effective +// directory and ` + "`cd`" + ` segments keep execution inside root. It starts at the +// effective directory (resolved against root) and follows each ` + "`cd`" + `; +// a starting directory or target that resolves outside root, or one that cannot +// be resolved statically (no argument, `-`, ` + "`~`" + `/home, an environment +// variable, a glob, or extra args), is treated as leaving the project so the +// caller refuses the unsandboxed grant. With no root there is no project to bind +// to, so any non-default directory or ` + "`cd`" + ` is rejected. +func commandDirStaysWithinProject(segments [][]string, root, initialDir string) bool { root = strings.TrimSpace(root) + initialDir = strings.TrimSpace(initialDir) + if initialDir == "" { + initialDir = "." + } if root == "" { - // No project to bind to: only safe if the command never changes directory. + // No project to bind to: only safe if the command stays at the default + // directory and never changes it. + if initialDir != "." { + return false + } return !commandChangesDirectory(segments) } // Resolve the root's real path once. Containment is checked against real paths @@ -234,7 +265,31 @@ func commandDirStaysWithinProject(segments [][]string, root string) bool { if err != nil { return false } - effective := realRoot + // Resolve the effective starting directory the same way we resolve a ` + "`cd`" + ` + // target: relative paths join against the workspace root, absolute paths are + // cleaned, and unresolvable forms (home, variables, globs) fail closed. + var effective string + if initialDir == "." { + effective = realRoot + } else { + if initialDir == "-" || initialDir == "~" || strings.HasPrefix(initialDir, "~") { + return false + } + if strings.ContainsAny(initialDir, "$*?[") { + return false + } + var candidate string + if filepath.IsAbs(initialDir) { + candidate = filepath.Clean(initialDir) + } else { + candidate = filepath.Clean(filepath.Join(realRoot, initialDir)) + } + realCandidate, err := filepath.EvalSymlinks(candidate) + if err != nil || !pathWithinRoot(realCandidate, realRoot) { + return false + } + effective = realCandidate + } for _, tokens := range segments { if len(tokens) == 0 || commandName(tokens[0]) != "cd" { continue @@ -700,3 +755,15 @@ func hasStringPrefix(values []string, prefix []string) bool { } return true } + +func equalStringSlices(left []string, right []string) bool { + if len(left) != len(right) { + return false + } + for index := range left { + if left[index] != right[index] { + return false + } + } + return true +} diff --git a/internal/agent/command_prefix_test.go b/internal/agent/command_prefix_test.go index 9512f4b0e..c6a7a7562 100644 --- a/internal/agent/command_prefix_test.go +++ b/internal/agent/command_prefix_test.go @@ -241,6 +241,56 @@ func TestMatchCommandPrefixRejectsGrantWhenCdEscapesProject(t *testing.T) { t.Fatal("expected a bare `cd` (home) to refuse the grant") } } +func TestMatchCommandPrefixRejectsGrantWhenWorkdirEscapesProject(t *testing.T) { + root := t.TempDir() + outside := t.TempDir() + store, err := sandbox.NewGrantStore(sandbox.StoreOptions{FilePath: filepath.Join(t.TempDir(), "grants.json")}) + if err != nil { + t.Fatalf("store: %v", err) + } + engine := sandbox.NewEngine(sandbox.EngineOptions{WorkspaceRoot: root, Store: store}) + if _, err := engine.GrantCommandPrefixForProject(sandbox.CommandPrefixInput{ToolName: "bash", Prefix: []string{"go", "test"}}); err != nil { + t.Fatalf("grant: %v", err) + } + if _, err := engine.GrantCommandPrefixForProject(sandbox.CommandPrefixInput{ToolName: "exec_command", Prefix: []string{"go", "test"}}); err != nil { + t.Fatalf("grant exec: %v", err) + } + // Every workdir alias for exec_command must be guarded: an outside directory + // supplied as workdir/cwd/dir/directory must not honor a project-scoped grant, + // same as the ` + "`cd` outside" guard. This is the recommended ` + "`cwd` over `cd`" + `" path. + for _, key := range []string{"workdir", "cwd", "dir", "directory"} { + args := map[string]any{"command": "go test ./...", key: outside} + if grant, ok, _ := matchCommandPrefix("exec_command", args, Options{Sandbox: engine}); ok { + t.Fatalf("expected no match for exec_command %s outside, got %#v", key, grant) + } + } + // bash's single alias must also be guarded. + if grant, ok, _ := matchCommandPrefix("bash", map[string]any{"command": "go test ./...", "cwd": outside}, Options{Sandbox: engine}); ok { + t.Fatalf("expected no match for bash cwd outside, got %#v", grant) + } + // Inside workdir still honors the grant (relative and absolute forms). + if err := os.MkdirAll(filepath.Join(root, "sub"), 0o755); err != nil { + t.Fatalf("mkdir sub: %v", err) + } + if _, ok, _ := matchCommandPrefix("exec_command", map[string]any{"command": "go test ./...", "workdir": filepath.Join(root, "sub")}, Options{Sandbox: engine}); !ok { + t.Fatal("expected within-project workdir to still match the grant") + } + if _, ok, _ := matchCommandPrefix("exec_command", map[string]any{"command": "go test ./...", "cwd": "sub"}, Options{Sandbox: engine}); !ok { + t.Fatal("expected relative within-project cwd to still match the grant") + } + // workdir escape via symlink inside workspace pointing outside must also be refused. + if err := os.Symlink(outside, filepath.Join(root, "escape")); err != nil { + t.Fatalf("symlink: %v", err) + } + if grant, ok, _ := matchCommandPrefix("exec_command", map[string]any{"command": "go test ./...", "workdir": filepath.Join(root, "escape")}, Options{Sandbox: engine}); ok { + t.Fatalf("expected symlinked-out workdir to refuse grant, got %#v", grant) + } + // Absolute outside with ` + "`cd` outside" already refused; combined case also refused. + if grant, ok, _ := matchCommandPrefix("exec_command", map[string]any{"command": "cd sub && go test ./...", "workdir": outside}, Options{Sandbox: engine}); ok { + t.Fatalf("expected outside workdir + cd to refuse, got %#v", grant) + } +} + func TestKnownSafeCommandSegmentRejectsMsysProneOnWindows(t *testing.T) { if runtime.GOOS != "windows" { diff --git a/internal/agent/export_test.go b/internal/agent/export_test.go index 1a2a153f6..24418b17e 100644 --- a/internal/agent/export_test.go +++ b/internal/agent/export_test.go @@ -6,17 +6,9 @@ import ( "github.com/Gitlawb/zero/internal/zeroruntime" ) -func equalStringSlices(left []string, right []string) bool { - if len(left) != len(right) { - return false - } - for index := range left { - if left[index] != right[index] { - return false - } - } - return true -} +// equalStringSlices is defined in command_prefix.go for production; tests use the +// same implementation via the production symbol. + // parsePreservedState recovers the plan + skills from a prior summary's preserved // block. JSON escaping makes this lossless even when a skill body contains diff --git a/internal/agent/loop_test.go b/internal/agent/loop_test.go index 8b8d331d8..97778f4c9 100644 --- a/internal/agent/loop_test.go +++ b/internal/agent/loop_test.go @@ -1790,7 +1790,7 @@ func TestRunAllowsWorkspaceWriteWithoutPromptWhenSandboxPolicyPermits(t *testing func TestRunAutoClassifierAllowsSandboxReviewedWorkspaceWrite(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := providerCallingWritePathContentThenAnswer("notes.txt", strings.Repeat("x", autoPermissionClassifierStringLimit+12), "write done") var permissionEvents []PermissionEvent var classifierRequests []AutoPermissionClassifierRequest @@ -1856,7 +1856,7 @@ func TestRunAutoClassifierAllowsSandboxReviewedWorkspaceWrite(t *testing.T) { func TestRunAutoClassifierPromptFallsBackToPermissionRequest(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := providerCallingWriteFileThenAnswer("write approved") classifierCalls := 0 var requests []PermissionRequest @@ -1910,7 +1910,7 @@ func TestRunAutoClassifierPromptFallsBackToPermissionRequest(t *testing.T) { func TestRunAutoClassifierErrorFallsBackToPermissionRequest(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := providerCallingWriteFileThenAnswer("write approved") var requests []PermissionRequest @@ -1947,7 +1947,7 @@ func TestRunAutoClassifierErrorFallsBackToPermissionRequest(t *testing.T) { func TestRunDefaultAutoClassifierInvalidJSONFallsBackToPermissionRequest(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := providerCallingWriteFileThenClassifyThenAnswer("not json", "write approved") var requests []PermissionRequest @@ -1981,7 +1981,7 @@ func TestRunDefaultAutoClassifierInvalidJSONFallsBackToPermissionRequest(t *test func TestRunDefaultAutoClassifierStrictJSONAllowsWorkspaceWrite(t *testing.T) { root := t.TempDir() registry := tools.NewRegistry() - registry.Register(tools.NewWriteFileTool(root)) + registry.Register(tools.NewScopedWriteFileTool(root, nil)) provider := providerCallingWriteFileThenClassifyThenAnswer(`{"action":"allow","reason":"safe workspace note"}`, "write done") var permissionEvents []PermissionEvent From 8639965d95470939d6c0d476a3553f412dff5e98 Mon Sep 17 00:00:00 2001 From: pengdst Date: Wed, 2 Sep 2026 21:36:14 +0700 Subject: [PATCH 6/6] fix(tests): use cargo instead of yarn for prefix tests yarn is now a banned launcher in sandbox (unsafeCommandPrefixLauncher), so ValidCommandPrefix rejects yarn prefixes and the wildcard/ ladder tests that used yarn test:unit started failing after the main merge (1b5db176). Switch those tests to cargo which remains allowed. --- internal/agent/command_prefix_test.go | 30 +++++++++---------- .../sandbox/command_prefix_wildcard_test.go | 20 ++++++------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/internal/agent/command_prefix_test.go b/internal/agent/command_prefix_test.go index c6a7a7562..dd13a4c60 100644 --- a/internal/agent/command_prefix_test.go +++ b/internal/agent/command_prefix_test.go @@ -320,11 +320,11 @@ func TestPersistCommandPrefixGrantScopedOrSessionFallsBackToSession(t *testing.T engine := sandbox.NewEngine(sandbox.EngineOptions{Store: store}) options := Options{Sandbox: engine} - prefix := persistCommandPrefixGrantScopedOrSession(PermissionDecisionAllowPrefixProject, "bash", []string{"yarn", "test:unit"}, "reason", options) - if !equalStringSlices(prefix, []string{"yarn", "test:unit"}) { - t.Fatalf("fallback prefix = %#v, want [yarn test:unit]", prefix) + prefix := persistCommandPrefixGrantScopedOrSession(PermissionDecisionAllowPrefixProject, "bash", []string{"cargo", "test:unit"}, "reason", options) + if !equalStringSlices(prefix, []string{"cargo", "test:unit"}) { + t.Fatalf("fallback prefix = %#v, want [cargo test:unit]", prefix) } - if _, ok := engine.LookupCommandPrefixForSession("bash", []string{"yarn", "test:unit"}); !ok { + if _, ok := engine.LookupCommandPrefixForSession("bash", []string{"cargo", "test:unit"}); !ok { t.Fatal("expected session grant recorded after project scope failed") } // Nothing was persisted at project/global scope. @@ -356,12 +356,12 @@ func TestProposedCommandPrefixRejectsRequestedUnsafeLauncherPrefix(t *testing.T) func TestCommandPrefixLadderOffersBreadthChoices(t *testing.T) { // test:unit has a namespace separator, so the ladder offers the intra-token - // wildcard alongside the exact prefix. The one-token rung ({"yarn"}) is never - // offered: a bare launcher grant would approve every later yarn subcommand. - got := commandPrefixLadder("bash", map[string]any{"command": "yarn test:unit"}) + // wildcard alongside the exact prefix. The one-token rung ({"cargo"}) is never + // offered: a bare launcher grant would approve every later cargo subcommand. + got := commandPrefixLadder("bash", map[string]any{"command": "cargo test:unit"}) want := [][]string{ - {"yarn", "test:*"}, - {"yarn", "test:unit"}, + {"cargo", "test:*"}, + {"cargo", "test:unit"}, } if len(got) != len(want) { t.Fatalf("ladder = %#v, want %#v", got, want) @@ -417,23 +417,23 @@ func TestIntraTokenWildcardPrefix(t *testing.T) { func TestGrantPrefixForDecisionHonorsOfferedChoice(t *testing.T) { request := PermissionRequest{ - CommandPrefix: []string{"yarn", "test:unit"}, - CommandPrefixOptions: [][]string{{"yarn", "test:*"}, {"yarn", "test:unit"}}, + CommandPrefix: []string{"cargo", "test:unit"}, + CommandPrefixOptions: [][]string{{"cargo", "test:*"}, {"cargo", "test:unit"}}, } // The intra-token wildcard breadth is honored. - if got := grantPrefixForDecision(request, PermissionDecision{CommandPrefix: []string{"yarn", "test:*"}}); !equalStringSlices(got, []string{"yarn", "test:*"}) { + if got := grantPrefixForDecision(request, PermissionDecision{CommandPrefix: []string{"cargo", "test:*"}}); !equalStringSlices(got, []string{"cargo", "test:*"}) { t.Fatalf("expected wildcard breadth honored, got %#v", got) } // A one-token breadth is never offered, so it falls back to the default. - if got := grantPrefixForDecision(request, PermissionDecision{CommandPrefix: []string{"yarn"}}); !equalStringSlices(got, []string{"yarn", "test:unit"}) { + if got := grantPrefixForDecision(request, PermissionDecision{CommandPrefix: []string{"cargo"}}); !equalStringSlices(got, []string{"cargo", "test:unit"}) { t.Fatalf("expected default prefix on unoffered one-token choice, got %#v", got) } // An empty choice falls back to the request default. - if got := grantPrefixForDecision(request, PermissionDecision{}); !equalStringSlices(got, []string{"yarn", "test:unit"}) { + if got := grantPrefixForDecision(request, PermissionDecision{}); !equalStringSlices(got, []string{"cargo", "test:unit"}) { t.Fatalf("expected default prefix on empty choice, got %#v", got) } // A choice that was never offered falls back to the default (no widening). - if got := grantPrefixForDecision(request, PermissionDecision{CommandPrefix: []string{"yarn", "install"}}); !equalStringSlices(got, []string{"yarn", "test:unit"}) { + if got := grantPrefixForDecision(request, PermissionDecision{CommandPrefix: []string{"cargo", "install"}}); !equalStringSlices(got, []string{"cargo", "test:unit"}) { t.Fatalf("expected default prefix on unoffered choice, got %#v", got) } } diff --git a/internal/sandbox/command_prefix_wildcard_test.go b/internal/sandbox/command_prefix_wildcard_test.go index ec0486ffa..a85507362 100644 --- a/internal/sandbox/command_prefix_wildcard_test.go +++ b/internal/sandbox/command_prefix_wildcard_test.go @@ -39,8 +39,8 @@ func TestCommandPrefixProjectScope(t *testing.T) { } func TestValidCommandPrefixAllowsTrailingWildcardOnLastToken(t *testing.T) { - // yarn is not a banned launcher, so the wildcard prefix is grantable. - if !ValidCommandPrefix([]string{"yarn", "test:*"}) { + // cargo is not a banned launcher, so the wildcard prefix is grantable. + if !ValidCommandPrefix([]string{"cargo", "test:*"}) { t.Fatal("trailing wildcard on the last token should be valid") } } @@ -48,10 +48,10 @@ func TestValidCommandPrefixAllowsTrailingWildcardOnLastToken(t *testing.T) { func TestValidCommandPrefixRejectsUnsafeWildcards(t *testing.T) { cases := map[string][]string{ "lone launcher wildcard": {"go*"}, - "mid-command wildcard": {"yarn", "test:*", "unit"}, - "mid-token glob": {"yarn", "te*st"}, - "plain trailing wildcard": {"yarn", "test*"}, - "bare wildcard": {"yarn", "*"}, + "mid-command wildcard": {"cargo", "test:*", "unit"}, + "mid-token glob": {"cargo", "te*st"}, + "plain trailing wildcard": {"cargo", "test*"}, + "bare wildcard": {"cargo", "*"}, } for name, prefix := range cases { if ValidCommandPrefix(prefix) { @@ -62,15 +62,15 @@ func TestValidCommandPrefixRejectsUnsafeWildcards(t *testing.T) { func TestCommandPrefixSessionGrantMatchesWildcard(t *testing.T) { engine := NewEngine(EngineOptions{Policy: DefaultPolicy()}) - engine.GrantCommandPrefixForSession("bash", []string{"yarn", "test:*"}) + engine.GrantCommandPrefixForSession("bash", []string{"cargo", "test:*"}) - if _, ok := engine.LookupCommandPrefixForSession("bash", []string{"yarn", "test:unit"}); !ok { + if _, ok := engine.LookupCommandPrefixForSession("bash", []string{"cargo", "test:unit"}); !ok { t.Fatal("wildcard grant should match test:unit") } - if _, ok := engine.LookupCommandPrefixForSession("bash", []string{"yarn", "test:e2e", "--watch"}); !ok { + if _, ok := engine.LookupCommandPrefixForSession("bash", []string{"cargo", "test:e2e", "--watch"}); !ok { t.Fatal("wildcard grant should match test:e2e with extra args") } - if _, ok := engine.LookupCommandPrefixForSession("bash", []string{"yarn", "build"}); ok { + if _, ok := engine.LookupCommandPrefixForSession("bash", []string{"cargo", "build"}); ok { t.Fatal("wildcard grant must not match a non-test: script") } }