diff --git a/internal/api/ai_session_handlers.go b/internal/api/ai_session_handlers.go index e1329cd..46c45dd 100644 --- a/internal/api/ai_session_handlers.go +++ b/internal/api/ai_session_handlers.go @@ -14,6 +14,18 @@ import ( "github.com/whilesmartgo/agents" ) +// redactSessionInput strips known secret values from operator-supplied text +// before it enters the transcript, so seeded file content or pasted output +// never carries a credential to the model provider. +func (s *Server) redactSessionInput(sess *ai.Session, text string) string { + secrets := s.systemSecretValues() + if sess.Deployment != "" { + secrets = s.deploymentSecretValues(sess.Deployment) + } + redacted, _ := ai.NewRedactor(secrets).Redact(text) + return redacted +} + // composeUserMessage merges a short message with optional bulky // context. The model sees both; the operator's transcript shows only // the message. @@ -72,6 +84,8 @@ func (s *Server) advanceSession(c *gin.Context, sess *ai.Session) error { // aiRunner assembles the runner for one session turn. A session that does not // auto-run pauses before any tools run, surfacing them for per-call approval. +// Even with auto-run on, a batch containing a state-changing tool pauses: +// reads run free, writes always ask. func (s *Server) aiRunner(c *gin.Context, sess *ai.Session, engine agents.Engine) agents.Runner { runner := agents.Runner{ Engine: engine, @@ -82,8 +96,16 @@ func (s *Server) aiRunner(c *gin.Context, sess *ai.Session, engine agents.Engine StepLimitMessage: aiStepLimitMessage, }, } - if !sess.AutoRun { - runner.Approve = func(context.Context, []agents.ToolCall) (bool, error) { return false, nil } + runner.Approve = func(_ context.Context, calls []agents.ToolCall) (bool, error) { + if !sess.AutoRun { + return false, nil + } + for _, call := range calls { + if s.toolMutates(call.Name) { + return false, nil + } + } + return true, nil } return runner } @@ -220,7 +242,7 @@ func (s *Server) createAISession(c *gin.Context) { prompt := ai.BuildSessionPrompt(req.Scope, req.Deployment, s.config.AI.DocsURL) sess := ai.NewSession(req.Scope, req.Deployment, req.AutoRun, sessionActorFrom(c), prompt) content, display := composeUserMessage(req.Message, req.Context) - sess.AddUserMessage(content, display, req.Seed) + sess.AddUserMessage(s.redactSessionInput(sess, content), display, req.Seed) if err := s.advanceSession(c, sess); err != nil { c.JSON(http.StatusBadGateway, gin.H{"error": err.Error()}) @@ -303,7 +325,7 @@ func (s *Server) postAISessionMessage(c *gin.Context) { } content, display := composeUserMessage(req.Message, req.Context) - sess.AddUserMessage(content, display, false) + sess.AddUserMessage(s.redactSessionInput(sess, content), display, false) if err := s.advanceSession(c, sess); err != nil { c.JSON(http.StatusBadGateway, gin.H{"error": err.Error()}) return diff --git a/internal/api/ai_session_test.go b/internal/api/ai_session_test.go index 99a9b5c..da9e73f 100644 --- a/internal/api/ai_session_test.go +++ b/internal/api/ai_session_test.go @@ -5,6 +5,8 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "os" + "path/filepath" "strings" "testing" @@ -305,6 +307,81 @@ func TestAISessionPerCallApproval(t *testing.T) { } } +func TestAutoRunStillPausesForMutatingTools(t *testing.T) { + s, tmpDir, ts := setupPlanTestServer(t) + createTestDeployment(t, tmpDir, "myapp", &models.ServiceMetadata{Name: "myapp"}) + + s.aiProvider = &scriptedProvider{responses: []*ai.Response{ + {ToolCalls: []ai.ToolCall{{ID: "c1", Name: "write_deployment_file", + Arguments: `{"path":"conf/app.conf","content":"key = value\n"}`}}, Model: "scripted"}, + {Content: "Written.", Model: "scripted"}, + }} + + resp, parsed := doJSON(t, http.MethodPost, ts.URL+"/api/ai/sessions", map[string]interface{}{ + "scope": "deployment", "deployment": "myapp", "auto_run": true, + "message": "set key to value in conf/app.conf", + }) + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, body %v", resp.StatusCode, parsed) + } + // Auto-run must not silently execute a state-changing tool. + if parsed["status"] != "awaiting_approval" { + t.Fatalf("status = %v, want awaiting_approval", parsed["status"]) + } + if _, err := os.Stat(filepath.Join(tmpDir, "myapp", "conf", "app.conf")); !os.IsNotExist(err) { + t.Fatal("the file must not be written before approval") + } + + id := parsed["id"].(string) + resp, parsed = doJSON(t, http.MethodPost, ts.URL+"/api/ai/sessions/"+id+"/approve", + map[string]interface{}{"approved": map[string]bool{"c1": true}}) + if resp.StatusCode != http.StatusOK { + t.Fatalf("approve status = %d, body %v", resp.StatusCode, parsed) + } + if parsed["status"] != "ready" { + t.Errorf("status after approve = %v", parsed["status"]) + } + data, err := os.ReadFile(filepath.Join(tmpDir, "myapp", "conf", "app.conf")) + if err != nil || string(data) != "key = value\n" { + t.Errorf("approved write not applied: %q err=%v", string(data), err) + } +} + +func TestAISessionRedactsSeededSecrets(t *testing.T) { + s, tmpDir, ts := setupPlanTestServer(t) + createTestDeployment(t, tmpDir, "myapp", &models.ServiceMetadata{Name: "myapp"}) + envContent := "DB_PASSWORD=hunter2secret\nAPP_NAME=myapp\n" + if err := os.WriteFile(filepath.Join(tmpDir, "myapp", ".env.flatrun"), []byte(envContent), 0600); err != nil { + t.Fatal(err) + } + + stub := &scriptedProvider{responses: []*ai.Response{{Content: "Looks fine.", Model: "scripted"}}} + s.aiProvider = stub + + resp, parsed := doJSON(t, http.MethodPost, ts.URL+"/api/ai/sessions", map[string]interface{}{ + "scope": "deployment", + "deployment": "myapp", + "auto_run": true, + "message": "Review this file.", + "context": "```\nDB_PASSWORD=hunter2secret\nhost=db\n```", + }) + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, body %v", resp.StatusCode, parsed) + } + + // The provider must never see the secret value from the seeded context. + var sent strings.Builder + for _, m := range stub.lastRequestMessages() { + sent.WriteString(m.Content) + } + if strings.Contains(sent.String(), "hunter2secret") { + t.Error("a deployment env value in seeded context reached the provider") + } + if !strings.Contains(sent.String(), "[REDACTED]") { + t.Error("expected the seeded secret to be replaced with a redaction marker") + } +} + func TestAISessionDisabledReturns503(t *testing.T) { _, _, ts := setupPlanTestServer(t) resp, parsed := doJSON(t, http.MethodPost, ts.URL+"/api/ai/sessions", diff --git a/internal/api/ai_tools.go b/internal/api/ai_tools.go index 5a6fff2..e47a359 100644 --- a/internal/api/ai_tools.go +++ b/internal/api/ai_tools.go @@ -20,11 +20,14 @@ import ( const maxToolOutputChars = 8000 -// aiTool is one read-only investigation capability the model can call -// to discover facts about this installation instead of guessing. +// aiTool is one capability the model can call: read-only investigation +// by default, or a state change when Mutates is set. type aiTool struct { Spec ai.Tool - Run func(s *Server, c *gin.Context, boundDeployment string, args map[string]interface{}) (string, error) + // Mutates marks a tool that changes state. A batch containing one always + // pauses for operator approval, even in an auto-run session. + Mutates bool + Run func(s *Server, c *gin.Context, boundDeployment string, args map[string]interface{}) (string, error) } // destructiveCommand matches obviously state-changing shell tokens, so @@ -304,6 +307,7 @@ func (s *Server) aiToolRegistry() map[string]aiTool { }, }, "write_deployment_file": { + Mutates: true, Spec: ai.Tool{ Name: "write_deployment_file", Description: "Create or overwrite a text file inside a deployment's directory, for example a config or compose file. Requires write access to the deployment; the path cannot escape the deployment directory.", @@ -338,6 +342,7 @@ func (s *Server) aiToolRegistry() map[string]aiTool { }, }, "run_quick_action": { + Mutates: true, Spec: ai.Tool{ Name: "run_quick_action", Description: "Run one of a deployment's configured quick actions by its id. Requires write access to the deployment.", @@ -370,6 +375,7 @@ func (s *Server) aiToolRegistry() map[string]aiTool { }, }, "control_deployment": { + Mutates: true, Spec: ai.Tool{ Name: "control_deployment", Description: "Start, stop, or restart a whole deployment. Requires write access to the deployment.", @@ -608,6 +614,17 @@ func (s *Server) runAITool(c *gin.Context, boundDeployment string, call ai.ToolC return result } +// toolMutates reports whether a named tool changes state, covering both +// built-in tools and plugin-contributed ones. +func (s *Server) toolMutates(name string) bool { + if plugin, tool, ok := parsePluginToolName(name); ok { + spec, found := s.pluginToolSpec(plugin, tool) + return found && spec.Mutates + } + t, ok := s.aiToolRegistry()[name] + return ok && t.Mutates +} + // toolAllowedDeploymentWrite resolves the target deployment and verifies the actor may write // to it, gating a mutating plugin tool. func (s *Server) toolAllowedDeploymentWrite(c *gin.Context, boundDeployment string, args map[string]interface{}) (string, error) {