From ec2bc5569802966818b33fa8f6568a49ed541299 Mon Sep 17 00:00:00 2001 From: Shuby Mishra Date: Tue, 16 Sep 2025 22:01:13 +0000 Subject: [PATCH 1/2] first pass --- README.md | 8 +++- cmd/main.go | 43 ++++++++++++++++-- cmd/options_test.go | 63 ++++++++++++++++++++++++++ k8s-bench/eval.go | 2 +- pkg/agent/approval_policy.go | 29 ++++++++++++ pkg/agent/conversation.go | 82 ++++++++++++++++++++++++---------- pkg/agent/conversation_test.go | 78 ++++++++++++++++++++++++++++++++ 7 files changed, 277 insertions(+), 28 deletions(-) create mode 100644 cmd/options_test.go create mode 100644 pkg/agent/approval_policy.go diff --git a/README.md b/README.md index 96822ab4..9fcd553b 100644 --- a/README.md +++ b/README.md @@ -253,7 +253,7 @@ skipVerifySSL: false # Skip SSL verification for LLM API calls # Tool and permission settings toolConfigPaths: ["~/.config/kubectl-ai/tools.yaml"] # Custom tools configuration paths -skipPermissions: false # Skip confirmation for resource-modifying commands +approvalPolicy: auto-approve-read # Approval policy: auto-approve-read, paranoid, or yolo enableToolUseShim: false # Enable tool use shim for certain models # MCP configuration @@ -283,6 +283,12 @@ tracePath: "/tmp/kubectl-ai-trace.txt" # Path to trace file +The `approvalPolicy` setting (or the `--approval-policy` flag) controls how `kubectl-ai` requests permission before executing generated commands: + +* `auto-approve-read` (default) skips prompts for commands the agent identifies as read-only. +* `paranoid` always asks for explicit approval before running any command. +* `yolo` never asks for approval—use with caution. + All these settings can be configured through either: 1. Command line flags (e.g., `--model=gemini-2.5-pro`) diff --git a/cmd/main.go b/cmd/main.go index 4a2ac1af..2276b46d 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -81,8 +81,9 @@ func BuildRootCommand(opt *Options) (*cobra.Command, error) { type Options struct { ProviderID string `json:"llmProvider,omitempty"` ModelID string `json:"model,omitempty"` - // SkipPermissions is a flag to skip asking for confirmation before executing kubectl commands - // that modifies resources in the cluster. + // ApprovalPolicy controls how permission prompts are handled before executing kubectl commands. + ApprovalPolicy agent.ApprovalPolicy `json:"approvalPolicy,omitempty"` + // SkipPermissions is deprecated. Use ApprovalPolicy instead. SkipPermissions bool `json:"skipPermissions,omitempty"` // EnableToolUseShim is a flag to enable tool use shim. // TODO(droot): figure out a better way to discover if the model supports tool use @@ -142,6 +143,7 @@ func (o *Options) InitDefaults() { o.ProviderID = "gemini" o.ModelID = "gemini-2.5-pro" // by default, confirm before executing kubectl commands that modify resources in the cluster. + o.ApprovalPolicy = agent.ApprovalPolicyAutoApproveRead o.SkipPermissions = false o.MCPServer = false o.MCPClient = false @@ -187,6 +189,26 @@ func (o *Options) LoadConfiguration(b []byte) error { return nil } +func (o *Options) ResolveApprovalPolicy() error { + if o.ApprovalPolicy == "" { + o.ApprovalPolicy = agent.ApprovalPolicyAutoApproveRead + } + + if o.SkipPermissions { + if o.ApprovalPolicy == agent.ApprovalPolicyAutoApproveRead || o.ApprovalPolicy == "" { + o.ApprovalPolicy = agent.ApprovalPolicyYolo + } else if o.ApprovalPolicy != agent.ApprovalPolicyYolo { + fmt.Fprintln(os.Stderr, "warning: --skip-permissions is deprecated and conflicts with --approval-policy; ignoring --skip-permissions") + } + } + + if !o.ApprovalPolicy.IsValid() { + return fmt.Errorf("invalid approval policy %q", o.ApprovalPolicy) + } + + return nil +} + func (o *Options) LoadConfigurationFile() error { configPaths := defaultConfigPaths for _, configPath := range configPaths { @@ -272,6 +294,10 @@ func run(ctx context.Context) error { return fmt.Errorf("failed to load config file: %w", err) } + if err := opt.ResolveApprovalPolicy(); err != nil { + return err + } + rootCmd, err := BuildRootCommand(&opt) if err != nil { return err @@ -302,7 +328,14 @@ func (opt *Options) bindCLIFlags(f *pflag.FlagSet) error { f.StringVar(&opt.ProviderID, "llm-provider", opt.ProviderID, "language model provider") f.StringVar(&opt.ModelID, "model", opt.ModelID, "language model e.g. gemini-2.0-flash-thinking-exp-01-21, gemini-2.0-flash") + f.StringVar((*string)(&opt.ApprovalPolicy), "approval-policy", string(opt.ApprovalPolicy), "approval policy for executing kubectl commands. Supported values: auto-approve-read, paranoid, yolo") f.BoolVar(&opt.SkipPermissions, "skip-permissions", opt.SkipPermissions, "(dangerous) skip asking for confirmation before executing kubectl commands that modify resources") + if err := f.MarkHidden("skip-permissions"); err != nil { + return err + } + if err := f.MarkDeprecated("skip-permissions", "use --approval-policy=yolo instead"); err != nil { + return err + } f.BoolVar(&opt.MCPServer, "mcp-server", opt.MCPServer, "run in MCP server mode") f.BoolVar(&opt.ExternalTools, "external-tools", opt.ExternalTools, "in MCP server mode, discover and expose external MCP tools") f.StringArrayVar(&opt.ToolConfigPaths, "custom-tools-config", opt.ToolConfigPaths, "path to custom tools config file or directory") @@ -328,6 +361,10 @@ func (opt *Options) bindCLIFlags(f *pflag.FlagSet) error { func RunRootCommand(ctx context.Context, opt Options, args []string) error { var err error // Declare err once for the whole function + if err = opt.ResolveApprovalPolicy(); err != nil { + return err + } + // Validate flag combinations if opt.ExternalTools && !opt.MCPServer { return fmt.Errorf("--external-tools can only be used with --mcp-server") @@ -461,7 +498,7 @@ func RunRootCommand(ctx context.Context, opt Options, args []string) error { Tools: tools.Default(), Recorder: recorder, RemoveWorkDir: opt.RemoveWorkDir, - SkipPermissions: opt.SkipPermissions, + ApprovalPolicy: opt.ApprovalPolicy, EnableToolUseShim: opt.EnableToolUseShim, MCPClientEnabled: opt.MCPClient, RunOnce: opt.Quiet, diff --git a/cmd/options_test.go b/cmd/options_test.go new file mode 100644 index 00000000..b0a0ab97 --- /dev/null +++ b/cmd/options_test.go @@ -0,0 +1,63 @@ +package main + +import ( + "testing" + + "github.com/GoogleCloudPlatform/kubectl-ai/pkg/agent" +) + +func TestResolveApprovalPolicy(t *testing.T) { + tests := []struct { + name string + opt Options + want agent.ApprovalPolicy + wantErr bool + }{ + { + name: "default to auto approve read", + }, + { + name: "skip permissions sets yolo", + opt: Options{SkipPermissions: true}, + want: agent.ApprovalPolicyYolo, + }, + { + name: "explicit paranoid preserved", + opt: Options{ApprovalPolicy: agent.ApprovalPolicyParanoid}, + want: agent.ApprovalPolicyParanoid, + }, + { + name: "invalid policy", + opt: Options{ApprovalPolicy: agent.ApprovalPolicy("invalid")}, + wantErr: true, + }, + { + name: "skip permissions overridden by explicit", + opt: Options{SkipPermissions: true, ApprovalPolicy: agent.ApprovalPolicyParanoid}, + want: agent.ApprovalPolicyParanoid, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + opt := tt.opt + err := opt.ResolveApprovalPolicy() + if tt.wantErr { + if err == nil { + t.Fatalf("expected error but got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := tt.want + if want == "" { + want = agent.ApprovalPolicyAutoApproveRead + } + if opt.ApprovalPolicy != want { + t.Fatalf("expected approval policy %q, got %q", want, opt.ApprovalPolicy) + } + }) + } +} diff --git a/k8s-bench/eval.go b/k8s-bench/eval.go index 0501248f..9447d3a0 100644 --- a/k8s-bench/eval.go +++ b/k8s-bench/eval.go @@ -426,7 +426,7 @@ func (x *TaskExecution) runAgent(ctx context.Context) (string, error) { fmt.Sprintf("--quiet=%t", x.llmConfig.Quiet), "--model", x.llmConfig.ModelID, "--trace-path", tracePath, - "--skip-permissions", + "--approval-policy=yolo", } stdinReader, stdinWriter := io.Pipe() diff --git a/pkg/agent/approval_policy.go b/pkg/agent/approval_policy.go new file mode 100644 index 00000000..f34caf27 --- /dev/null +++ b/pkg/agent/approval_policy.go @@ -0,0 +1,29 @@ +package agent + +// ApprovalPolicy controls how the agent seeks permission before executing tool +// calls suggested by the model. +type ApprovalPolicy string + +const ( + // ApprovalPolicyAutoApproveRead requests approval only when the tool + // call is expected to modify cluster resources or when that is + // unknown. + ApprovalPolicyAutoApproveRead ApprovalPolicy = "auto-approve-read" + + // ApprovalPolicyParanoid always asks for approval before executing any + // tool call, regardless of whether it is read-only. + ApprovalPolicyParanoid ApprovalPolicy = "paranoid" + + // ApprovalPolicyYolo disables approval checks entirely. + ApprovalPolicyYolo ApprovalPolicy = "yolo" +) + +// IsValid reports whether the policy is one of the supported values. +func (p ApprovalPolicy) IsValid() bool { + switch p { + case ApprovalPolicyAutoApproveRead, ApprovalPolicyParanoid, ApprovalPolicyYolo: + return true + default: + return false + } +} diff --git a/pkg/agent/conversation.go b/pkg/agent/conversation.go index 28a78b4e..2073258e 100644 --- a/pkg/agent/conversation.go +++ b/pkg/agent/conversation.go @@ -85,7 +85,7 @@ type Agent struct { // Kubeconfig is the path to the kubeconfig file. Kubeconfig string - SkipPermissions bool + ApprovalPolicy ApprovalPolicy Tools tools.Tools @@ -607,7 +607,7 @@ func (c *Agent) Run(ctx context.Context, initialQuery string) error { continue // Skip execution for interactive commands } - if !c.SkipPermissions && modifiesResourceToolCallIndex >= 0 { + if c.shouldRequestApproval(modifiesResourceToolCallIndex) { // In RunOnce mode, exit with error if permission is required if c.RunOnce { var commandDescriptions []string @@ -615,7 +615,7 @@ func (c *Agent) Run(ctx context.Context, initialQuery string) error { commandDescriptions = append(commandDescriptions, call.ParsedToolCall.Description()) } errorMessage := "RunOnce mode cannot handle permission requests. The following commands require approval:\n* " + strings.Join(commandDescriptions, "\n* ") - errorMessage += "\nUse --skip-permissions flag to bypass permission checks in RunOnce mode." + errorMessage += "\nUse --approval-policy=yolo to bypass permission checks in RunOnce mode." log.Error(nil, "RunOnce mode cannot handle permission requests", "commands", commandDescriptions) c.setAgentState(api.AgentStateExited) @@ -630,13 +630,15 @@ func (c *Agent) Run(ctx context.Context, initialQuery string) error { confirmationPrompt := "The following commands require your approval to run:\n* " + strings.Join(commandDescriptions, "\n* ") confirmationPrompt += "\n\nDo you want to proceed ?" + options := []api.UserChoiceOption{{Value: "yes", Label: "Yes"}} + if c.ApprovalPolicy != ApprovalPolicyParanoid { + options = append(options, api.UserChoiceOption{Value: "yes_and_dont_ask_me_again", Label: "Yes, and don't ask me again"}) + } + options = append(options, api.UserChoiceOption{Value: "no", Label: "No"}) + choiceRequest := &api.UserChoiceRequest{ - Prompt: confirmationPrompt, - Options: []api.UserChoiceOption{ - {Value: "yes", Label: "Yes"}, - {Value: "yes_and_dont_ask_me_again", Label: "Yes, and don't ask me again"}, - {Value: "no", Label: "No"}, - }, + Prompt: confirmationPrompt, + Options: options, } c.setAgentState(api.AgentStateWaitingForInput) c.addMessage(api.MessageSourceAgent, api.MessageTypeUserChoiceRequest, choiceRequest) @@ -944,20 +946,29 @@ func (c *Agent) analyzeToolCalls(ctx context.Context, toolCalls []gollm.Function return toolCallAnalysis, nil } +func (c *Agent) shouldRequestApproval(modifiesResourceToolCallIndex int) bool { + switch c.ApprovalPolicy { + case ApprovalPolicyYolo: + return false + case ApprovalPolicyParanoid: + return len(c.pendingFunctionCalls) > 0 + case ApprovalPolicyAutoApproveRead: + fallthrough + default: + return modifiesResourceToolCallIndex >= 0 + } +} + func (c *Agent) handleChoice(ctx context.Context, choice *api.UserChoiceResponse) (dispatchToolCalls bool) { log := klog.FromContext(ctx) // if user input is a choice and use has declined the operation, // we need to abort all pending function calls. // update the currChatContent with the choice and keep the agent loop running. - // Normalize the input - switch choice.Choice { - case 1: - dispatchToolCalls = true - case 2: - c.SkipPermissions = true - dispatchToolCalls = true - case 3: + decline := func() { + if len(c.pendingFunctionCalls) == 0 { + return + } c.currChatContent = append(c.currChatContent, gollm.FunctionCallResult{ ID: c.pendingFunctionCalls[0].FunctionCall.ID, Name: c.pendingFunctionCalls[0].FunctionCall.Name, @@ -970,13 +981,38 @@ func (c *Agent) handleChoice(ctx context.Context, choice *api.UserChoiceResponse c.pendingFunctionCalls = []ToolCallAnalysis{} dispatchToolCalls = false c.addMessage(api.MessageSourceAgent, api.MessageTypeError, "Operation was skipped. User declined to run this operation.") + } + + switch c.ApprovalPolicy { + case ApprovalPolicyParanoid: + switch choice.Choice { + case 1: + dispatchToolCalls = true + case 2: + decline() + default: + err := fmt.Errorf("invalid confirmation choice: %q", choice.Choice) + log.Error(err, "Invalid choice received from AskForConfirmation") + c.pendingFunctionCalls = []ToolCallAnalysis{} + dispatchToolCalls = false + c.addMessage(api.MessageSourceAgent, api.MessageTypeError, "Invalid choice received. Cancelling operation.") + } default: - // This case should technically not be reachable due to AskForConfirmation loop - err := fmt.Errorf("invalid confirmation choice: %q", choice.Choice) - log.Error(err, "Invalid choice received from AskForConfirmation") - c.pendingFunctionCalls = []ToolCallAnalysis{} - dispatchToolCalls = false - c.addMessage(api.MessageSourceAgent, api.MessageTypeError, "Invalid choice received. Cancelling operation.") + switch choice.Choice { + case 1: + dispatchToolCalls = true + case 2: + c.ApprovalPolicy = ApprovalPolicyYolo + dispatchToolCalls = true + case 3: + decline() + default: + err := fmt.Errorf("invalid confirmation choice: %q", choice.Choice) + log.Error(err, "Invalid choice received from AskForConfirmation") + c.pendingFunctionCalls = []ToolCallAnalysis{} + dispatchToolCalls = false + c.addMessage(api.MessageSourceAgent, api.MessageTypeError, "Invalid choice received. Cancelling operation.") + } } return dispatchToolCalls } diff --git a/pkg/agent/conversation_test.go b/pkg/agent/conversation_test.go index 3a409828..85c1f4c5 100644 --- a/pkg/agent/conversation_test.go +++ b/pkg/agent/conversation_test.go @@ -255,3 +255,81 @@ func TestHandleMetaQuery(t *testing.T) { }) } } + +func TestAgentShouldRequestApproval(t *testing.T) { + a := &Agent{ApprovalPolicy: ApprovalPolicyAutoApproveRead} + + // No pending calls means no approval needed regardless of policy. + if a.shouldRequestApproval(-1) { + t.Fatalf("expected no approval when there are no pending calls") + } + + a.pendingFunctionCalls = []ToolCallAnalysis{{}} + if a.shouldRequestApproval(-1) { + t.Fatalf("expected auto-approve-read to skip readonly commands") + } + if !a.shouldRequestApproval(0) { + t.Fatalf("expected auto-approve-read to require approval for modifying commands") + } + + a.ApprovalPolicy = ApprovalPolicyParanoid + if !a.shouldRequestApproval(-1) { + t.Fatalf("expected paranoid policy to require approval even for readonly commands") + } + + a.ApprovalPolicy = ApprovalPolicyYolo + if a.shouldRequestApproval(0) { + t.Fatalf("expected yolo policy to bypass approvals") + } +} + +func TestHandleChoiceUpdatesApprovalPolicy(t *testing.T) { + a := &Agent{ + ApprovalPolicy: ApprovalPolicyAutoApproveRead, + pendingFunctionCalls: []ToolCallAnalysis{{ + FunctionCall: gollm.FunctionCall{ID: "1", Name: "tool"}, + }}, + Output: make(chan any, 1), + session: &api.Session{ChatMessageStore: sessions.NewInMemoryChatStore()}, + } + + dispatched := a.handleChoice(context.Background(), &api.UserChoiceResponse{Choice: 2}) + if !dispatched { + t.Fatalf("expected tool calls to be dispatched when user opts-in permanently") + } + if a.ApprovalPolicy != ApprovalPolicyYolo { + t.Fatalf("expected approval policy to switch to yolo, got %q", a.ApprovalPolicy) + } +} + +func TestHandleChoiceParanoidDecline(t *testing.T) { + a := &Agent{ + ApprovalPolicy: ApprovalPolicyParanoid, + pendingFunctionCalls: []ToolCallAnalysis{{ + FunctionCall: gollm.FunctionCall{ID: "1", Name: "tool"}, + }}, + Output: make(chan any, 1), + session: &api.Session{ChatMessageStore: sessions.NewInMemoryChatStore()}, + } + + dispatched := a.handleChoice(context.Background(), &api.UserChoiceResponse{Choice: 2}) + if dispatched { + t.Fatalf("expected paranoid decline to cancel tool dispatch") + } + if a.ApprovalPolicy != ApprovalPolicyParanoid { + t.Fatalf("paranoid decline should not alter the approval policy") + } + + select { + case msg := <-a.Output: + m, ok := msg.(*api.Message) + if !ok { + t.Fatalf("expected *api.Message, got %T", msg) + } + if m.Type != api.MessageTypeError { + t.Fatalf("expected error message after decline, got %v", m.Type) + } + default: + t.Fatalf("expected an error message to be emitted after decline") + } +} From 25804fc5f593dac52764299d43dd7774a9abb34e Mon Sep 17 00:00:00 2001 From: Shuby Mishra Date: Tue, 16 Sep 2025 22:03:06 +0000 Subject: [PATCH 2/2] fix format --- cmd/options_test.go | 14 ++++++++++++++ pkg/agent/approval_policy.go | 28 ++++++++++++++++------------ 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/cmd/options_test.go b/cmd/options_test.go index b0a0ab97..86b5d2df 100644 --- a/cmd/options_test.go +++ b/cmd/options_test.go @@ -1,3 +1,17 @@ +// Copyright 2025 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package main import ( diff --git a/pkg/agent/approval_policy.go b/pkg/agent/approval_policy.go index f34caf27..bc7aac8e 100644 --- a/pkg/agent/approval_policy.go +++ b/pkg/agent/approval_policy.go @@ -1,21 +1,25 @@ +// Copyright 2025 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package agent -// ApprovalPolicy controls how the agent seeks permission before executing tool -// calls suggested by the model. type ApprovalPolicy string const ( - // ApprovalPolicyAutoApproveRead requests approval only when the tool - // call is expected to modify cluster resources or when that is - // unknown. ApprovalPolicyAutoApproveRead ApprovalPolicy = "auto-approve-read" - - // ApprovalPolicyParanoid always asks for approval before executing any - // tool call, regardless of whether it is read-only. - ApprovalPolicyParanoid ApprovalPolicy = "paranoid" - - // ApprovalPolicyYolo disables approval checks entirely. - ApprovalPolicyYolo ApprovalPolicy = "yolo" + ApprovalPolicyParanoid ApprovalPolicy = "paranoid" + ApprovalPolicyYolo ApprovalPolicy = "yolo" ) // IsValid reports whether the policy is one of the supported values.