fix(agents): harden ACP launcher configuration (#1917)

This commit is contained in:
Carlos Florêncio
2026-07-24 16:36:44 +01:00
committed by GitHub
parent 7e4f2bb1e5
commit 2f6be0c777
59 changed files with 2077 additions and 302 deletions
+4 -3
View File
@@ -152,7 +152,7 @@ type CommandOptions struct {
AgentType string // for --agent flag (e.g. "task" for subagent)
// CLIFlagTokens are user-configured CLI flag argv tokens derived from
// AgentProfile.CLIFlags (only Enabled entries, shell-tokenised). Appended
// verbatim to the built command by every agent's BuildCommand.
// verbatim by lifecycle.CommandBuilder after the agent builds its command.
CLIFlagTokens []string
// CommandPrefixTokens are user-configured launcher tokens derived from
// AgentProfile.CommandPrefix (shell-tokenised). Prepended to the built
@@ -184,8 +184,9 @@ type PassthroughOptions struct {
// mcp_servers.…" overrides). Appended to the built command.
MCPArgs []string
// CLIFlagTokens are user-configured CLI flag argv tokens derived from
// AgentProfile.CLIFlags (only Enabled entries, shell-tokenised). Appended
// verbatim to the built passthrough command, mirroring CommandOptions.
// AgentProfile.CLIFlags (only Enabled entries, shell-tokenised). Unlike ACP
// commands, which lifecycle.CommandBuilder appends centrally, passthrough
// agents opt in by appending these tokens in BuildPassthroughCommand.
CLIFlagTokens []string
}
@@ -3,6 +3,7 @@ package client
import (
"context"
"encoding/json"
"io"
"net/http"
"net/http/httptest"
"strings"
@@ -23,6 +24,46 @@ func newTestLogger() *logger.Logger {
return log
}
func TestConfigureAgent_SerializesStructuredArgsAndOmitsAbsentFallback(t *testing.T) {
var bodies [][]byte
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
body, err := io.ReadAll(r.Body)
if err != nil {
t.Fatalf("read configure request: %v", err)
}
bodies = append(bodies, body)
_, _ = w.Write([]byte(`{"success":true}`))
}))
defer server.Close()
client := &Client{baseURL: server.URL, httpClient: server.Client()}
args := []string{"runner", "two words", "", `C:\tools\agent.exe`}
continueArgs := []string{"runner", "continue", "thread id"}
if err := client.ConfigureAgent(context.Background(), "runner two words C:\\tools\\agent.exe", args, nil, "", "runner continue thread id", continueArgs); err != nil {
t.Fatalf("configure structured args: %v", err)
}
if err := client.ConfigureAgent(context.Background(), "legacy --flag", nil, nil, "", "", nil); err != nil {
t.Fatalf("configure legacy fallback: %v", err)
}
var structured map[string]json.RawMessage
if err := json.Unmarshal(bodies[0], &structured); err != nil {
t.Fatalf("decode structured request: %v", err)
}
for _, field := range []string{"agent_args", "continue_args"} {
if _, ok := structured[field]; !ok {
t.Fatalf("structured request omitted %s: %s", field, bodies[0])
}
}
var legacy map[string]json.RawMessage
if err := json.Unmarshal(bodies[1], &legacy); err != nil {
t.Fatalf("decode legacy request: %v", err)
}
if _, ok := legacy["agent_args"]; ok {
t.Fatalf("legacy fallback unexpectedly sent agent_args: %s", bodies[1])
}
}
// wsTestServer creates a test WebSocket server that echoes back response messages.
// The handler receives each request and should return a response message.
type wsTestServer struct {
@@ -258,13 +258,15 @@ func (c *Client) GetStatus(ctx context.Context) (*StatusResponse, error) {
// ConfigureAgent configures the agent command and optional approval policy. Must be called before Start().
// continueCommand is optional — when set, the adapter uses it for one-shot follow-up prompts.
func (c *Client) ConfigureAgent(ctx context.Context, command string, env map[string]string, approvalPolicy, continueCommand string) error {
func (c *Client) ConfigureAgent(ctx context.Context, command string, agentArgs []string, env map[string]string, approvalPolicy, continueCommand string, continueArgs []string) error {
ctx, span := tracing.TraceHTTPRequest(ctx, "POST", "/api/v1/agent/configure", c.executionID)
defer span.End()
payload := struct {
Command string `json:"command"`
AgentArgs *[]string `json:"agent_args,omitempty"`
ContinueCommand string `json:"continue_command,omitempty"`
ContinueArgs *[]string `json:"continue_args,omitempty"`
Env map[string]string `json:"env,omitempty"`
ApprovalPolicy string `json:"approval_policy,omitempty"`
}{
@@ -273,6 +275,12 @@ func (c *Client) ConfigureAgent(ctx context.Context, command string, env map[str
Env: env,
ApprovalPolicy: approvalPolicy,
}
if agentArgs != nil {
payload.AgentArgs = &agentArgs
}
if continueArgs != nil {
payload.ContinueArgs = &continueArgs
}
body, err := json.Marshal(payload)
if err != nil {
@@ -4,7 +4,6 @@ package lifecycle
import (
"os"
"strings"
"github.com/kandev/kandev/internal/agent/agents"
)
@@ -44,20 +43,16 @@ func prependCommandPrefix(cmd agents.Command, prefixTokens []string) agents.Comm
return agents.NewCommand(args...)
}
// BuildCommandString builds a command as a single string (for standalone mode)
func (cb *CommandBuilder) BuildCommandString(ag agents.Agent, opts agents.CommandOptions) string {
cmd := cb.BuildCommand(ag, opts)
return strings.Join(cmd.Args(), " ")
// BuildCommandArgs builds the executable argv for an agent launch.
func (cb *CommandBuilder) BuildCommandArgs(ag agents.Agent, opts agents.CommandOptions) []string {
return cb.BuildCommand(ag, opts).Args()
}
// BuildContinueCommandString builds the continue command with the same model/permission
// flags as the initial command. Used by one-shot agents (Amp) where each follow-up prompt
// needs a separate "threads continue" command. Returns empty string if the agent has no
// ContinueSessionCmd configured.
func (cb *CommandBuilder) BuildContinueCommandString(ag agents.Agent, opts agents.CommandOptions) string {
// BuildContinueCommandArgs builds argv for a one-shot follow-up command.
func (cb *CommandBuilder) BuildContinueCommandArgs(ag agents.Agent, opts agents.CommandOptions) []string {
sessionCfg := ag.Runtime().SessionConfig
if sessionCfg.ContinueSessionCmd.IsEmpty() {
return ""
return nil
}
// Start from the continue command base and apply the same flags as BuildCommand
@@ -68,7 +63,7 @@ func (cb *CommandBuilder) BuildContinueCommandString(ag agents.Agent, opts agent
Build()
cmd = prependCommandPrefix(cmd, opts.CommandPrefixTokens)
return strings.Join(cmd.Args(), " ")
return cmd.Args()
}
// ExpandSessionDir resolves the host-side directory that should be bind-
@@ -3,11 +3,15 @@ package lifecycle
import (
"context"
"fmt"
"net/http"
"net/http/httptest"
"reflect"
"strings"
"sync/atomic"
"testing"
"github.com/stretchr/testify/require"
"github.com/kandev/kandev/internal/agent/agents"
"github.com/kandev/kandev/internal/agent/docker"
"github.com/kandev/kandev/internal/agent/executor"
@@ -164,6 +168,38 @@ func TestDockerStopInstanceStopsContainerOnStaleCleanup(t *testing.T) {
}
}
func TestRollbackLaunchExecutionForceKillsUnregisteredDockerContainer(t *testing.T) {
requests := make(chan string, 1)
dockerDaemon := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodHead && r.URL.Path == "/_ping" {
w.WriteHeader(http.StatusOK)
return
}
requests <- r.Method + " " + r.URL.Path
w.WriteHeader(http.StatusNoContent)
}))
t.Cleanup(dockerDaemon.Close)
dockerExec := NewDockerExecutor(config.DockerConfig{
Host: "tcp://" + strings.TrimPrefix(dockerDaemon.URL, "http://"),
}, "", newTestDockerLogger())
t.Cleanup(func() { require.NoError(t, dockerExec.Close()) })
mgr := newTestManager(t)
mgr.rollbackLaunchExecution(context.Background(), dockerExec, &ExecutorInstance{
InstanceID: "failed-instance",
ContainerID: "failed-container",
}, &AgentExecution{ID: "failed-execution", SessionID: "failed-session"}, "command construction failed")
select {
case got := <-requests:
require.Equal(t, http.MethodPost, strings.Fields(got)[0])
require.True(t, strings.HasSuffix(got, "/containers/failed-container/kill"), got)
default:
t.Fatal("rollback must force Docker cleanup for an unregistered failed instance")
}
}
func TestDockerCleanupContextIgnoresCanceledParentAfterAgentStopFailed(t *testing.T) {
parent, parentCancel := context.WithCancel(context.Background())
parentCancel()
@@ -619,7 +619,7 @@ func (m *Manager) rollbackRacedExecution(ctx context.Context, rt ExecutorBackend
zap.String("execution_id", execution.ID),
zap.String("session_id", execution.SessionID))
if rt != nil && runtimeInstance != nil {
if stopErr := rt.StopInstance(ctx, runtimeInstance, false); stopErr != nil {
if stopErr := rt.StopInstance(ctx, runtimeInstance, true); stopErr != nil {
m.logger.Warn("failed to stop raced runtime instance during rollback",
zap.String("execution_id", execution.ID),
zap.Error(stopErr))
@@ -892,6 +892,7 @@ type createInstanceExecutor struct {
client *agentctl.Client
createCount atomic.Int32
stopCount atomic.Int32
forceStopped atomic.Bool
lastRequest *ExecutorCreateRequest
authToken string
nonce string
@@ -950,6 +951,7 @@ func (e *createInstanceExecutor) CreateInstance(ctx context.Context, req *Execut
func (e *createInstanceExecutor) StopInstance(ctx context.Context, instance *ExecutorInstance, force bool) error {
e.stopCount.Add(1)
e.forceStopped.Store(force)
return nil
}
@@ -625,20 +625,9 @@ func (m *Manager) RestartAgentProcess(ctx context.Context, executionID string) e
return fmt.Errorf("execution %q has no agentctl client", executionID)
}
m.logger.Info("restarting agent process for context reset",
zap.String("execution_id", executionID),
zap.String("task_id", execution.TaskID),
zap.String("session_id", execution.SessionID))
// Capture the active session mode before the restart so it can be re-applied to
// the fresh ACP session (issue #1183). Capture now, before the streams reconnect
// and the restarted agent reports its default mode (which would overwrite the cache).
prevMode := execution.GetModeState()
// Resolve agent config early — needed for both command rebuild and ACP session init
agentConfig, err := m.getAgentConfigForExecution(execution)
preparation, err := m.prepareAgentRestart(ctx, execution)
if err != nil {
return fmt.Errorf("failed to get agent config for restart: %w", err)
return err
}
// 1. Close WebSocket streams (updates + workspace). Use per-stream Close
@@ -657,36 +646,8 @@ func (m *Manager) RestartAgentProcess(ctx context.Context, executionID string) e
// Continue — the process may already be stopped
}
// 3. Rebuild agent command without resume flags and reset execution state.
// Fail closed: if the launcher prefix can't be resolved, abort the restart
// rather than relaunch a sandboxed agent unwrapped.
freshCmd, freshContinueCmd, err := m.buildFreshAgentCommand(ctx, execution, agentConfig)
if err != nil {
return fmt.Errorf("failed to rebuild agent command for restart: %w", err)
}
_ = m.executionStore.WithLock(executionID, func(exec *AgentExecution) {
exec.ACPSessionID = ""
exec.Status = v1.AgentStatusStarting
exec.ErrorMessage = ""
exec.needsResumeContext = false
exec.resumeContextInjected = false
exec.sessionInitialized = false
exec.AgentCommand = freshCmd
exec.ContinueCommand = freshContinueCmd
exec.messageMu.Lock()
exec.messageBuffer.Reset()
exec.thinkingBuffer.Reset()
exec.currentMessageID = ""
exec.currentThinkingID = ""
exec.messageMu.Unlock()
// Drain any stale prompt completion signal
select {
case <-exec.promptDoneCh:
default:
}
})
// 3. Reset execution state after the replacement command has been validated.
m.resetAgentRestartState(executionID, preparation.commands)
// 4. Wait for agentctl to be ready (it should still be running)
if err := execution.agentctl.WaitForReady(ctx, 30*time.Second); err != nil {
@@ -708,18 +669,18 @@ func (m *Manager) RestartAgentProcess(ctx context.Context, executionID string) e
zap.Error(err))
}
mcpServers, err := m.resolveMcpServers(ctx, execution, agentConfig)
mcpServers, err := m.resolveMcpServers(ctx, execution, preparation.agentConfig)
if err != nil {
return fmt.Errorf("failed to resolve MCP config for restart: %w", err)
}
if err := m.initializeACPSessionForRestart(ctx, execution, agentConfig, mcpServers); err != nil {
if err := m.initializeACPSessionForRestart(ctx, execution, preparation.agentConfig, mcpServers); err != nil {
m.updateExecutionError(executionID, "failed to initialize ACP session after restart: "+err.Error())
return fmt.Errorf("failed to initialize ACP session after restart: %w", err)
}
// Restore the user's session permission mode onto the fresh ACP session.
m.reapplySessionModeAfterReset(ctx, execution, execution.ACPSessionID, prevMode)
m.reapplySessionModeAfterReset(ctx, execution, execution.ACPSessionID, preparation.previousMode)
m.logger.Info("agent process restarted with fresh context",
zap.String("execution_id", executionID),
@@ -730,6 +691,59 @@ func (m *Manager) RestartAgentProcess(ctx context.Context, executionID string) e
return nil
}
type agentRestartPreparation struct {
agentConfig agents.Agent
commands agentCommands
previousMode *CachedModeState
}
// prepareAgentRestart builds and validates the replacement before touching the current process.
func (m *Manager) prepareAgentRestart(ctx context.Context, execution *AgentExecution) (agentRestartPreparation, error) {
m.logger.Info("restarting agent process for context reset",
zap.String("execution_id", execution.ID),
zap.String("task_id", execution.TaskID),
zap.String("session_id", execution.SessionID))
agentConfig, err := m.getAgentConfigForExecution(execution)
if err != nil {
return agentRestartPreparation{}, fmt.Errorf("failed to get agent config for restart: %w", err)
}
commands, err := m.buildFreshAgentCommand(ctx, execution, agentConfig)
if err != nil {
return agentRestartPreparation{}, fmt.Errorf("failed to rebuild agent command for restart: %w", err)
}
return agentRestartPreparation{
agentConfig: agentConfig,
commands: commands,
previousMode: execution.GetModeState(),
}, nil
}
func (m *Manager) resetAgentRestartState(executionID string, commands agentCommands) {
_ = m.executionStore.WithLock(executionID, func(exec *AgentExecution) {
exec.ACPSessionID = ""
exec.Status = v1.AgentStatusStarting
exec.ErrorMessage = ""
exec.needsResumeContext = false
exec.resumeContextInjected = false
exec.sessionInitialized = false
exec.AgentCommand = commands.initial
exec.ContinueCommand = commands.continue_
exec.AgentArgs = commands.args
exec.ContinueArgs = commands.continueArgs
exec.messageMu.Lock()
exec.messageBuffer.Reset()
exec.thinkingBuffer.Reset()
exec.currentMessageID = ""
exec.currentThinkingID = ""
exec.messageMu.Unlock()
select {
case <-exec.promptDoneCh:
default:
}
})
}
// initializeACPSessionForRestart connects streams and creates a new ACP session without
// sending an initial prompt. The caller (workflow processOnEnter) handles prompting separately.
func (m *Manager) initializeACPSessionForRestart(
@@ -1495,13 +1509,13 @@ func (m *Manager) stopPassthroughProcess(ctx context.Context, executionID string
}
// buildFreshAgentCommand rebuilds the agent command without resume flags by going through
// the standard BuildCommandString pipeline with an empty SessionID. This works for all
// the standard command-building pipeline with an empty SessionID. This works for all
// agent types because each agent's BuildCommand respects SessionID="" to skip resume flags.
//
// It fails closed: if the profile cannot be resolved, or a configured
// command_prefix cannot be tokenised, it returns an error so a context reset
// never relaunches a sandboxed agent without its wrapper.
func (m *Manager) buildFreshAgentCommand(ctx context.Context, execution *AgentExecution, agentConfig agents.Agent) (initial, continueCmd string, err error) {
// never relaunches a configured agent without its wrapper.
func (m *Manager) buildFreshAgentCommand(ctx context.Context, execution *AgentExecution, agentConfig agents.Agent) (agentCommands, error) {
var profileInfo *AgentProfileInfo
if execution.AgentProfileID != "" && m.profileResolver != nil {
pi, resolveErr := m.profileResolver.ResolveProfile(ctx, execution.AgentProfileID)
@@ -1509,7 +1523,7 @@ func (m *Manager) buildFreshAgentCommand(ctx context.Context, execution *AgentEx
// A profile was expected but could not be resolved. We cannot tell
// whether it carried a sandbox prefix, so refuse to relaunch rather
// than risk running unwrapped.
return "", "", fmt.Errorf("resolve profile %s for restart: %w", execution.AgentProfileID, resolveErr)
return agentCommands{}, fmt.Errorf("resolve profile %s for restart: %w", execution.AgentProfileID, resolveErr)
}
profileInfo = pi
}
@@ -1530,10 +1544,10 @@ func (m *Manager) buildFreshAgentCommand(ctx context.Context, execution *AgentEx
// Preserve the profile's cli_flags and command_prefix across restarts. A
// context reset that dropped the launcher prefix would relaunch a
// sandboxed agent unwrapped — the exact protection the prefix exists for.
// configured agent unwrapped.
cliFlagTokens, commandPrefixTokens, err := m.resolveProfileLaunchTokens(profileInfo)
if err != nil {
return "", "", err
return agentCommands{}, err
}
opts := agents.CommandOptions{
@@ -1548,6 +1562,10 @@ func (m *Manager) buildFreshAgentCommand(ctx context.Context, execution *AgentEx
// an absolute host path.
Runtime: execution.RuntimeName,
}
return m.commandBuilder.BuildCommandString(agentConfig, opts),
m.commandBuilder.BuildContinueCommandString(agentConfig, opts), nil
args := m.commandBuilder.BuildCommandArgs(agentConfig, opts)
continueArgs := m.commandBuilder.BuildContinueCommandArgs(agentConfig, opts)
if err := validateBuiltAgentCommands(args, continueArgs); err != nil {
return agentCommands{}, err
}
return newAgentCommands(args, continueArgs), nil
}
@@ -3,6 +3,7 @@ package lifecycle
import (
"context"
"encoding/json"
"errors"
"fmt"
"net/http"
"net/http/httptest"
@@ -14,6 +15,7 @@ import (
"github.com/gorilla/websocket"
"github.com/stretchr/testify/require"
"github.com/kandev/kandev/internal/agent/agents"
"github.com/kandev/kandev/internal/agent/executor"
agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl"
"github.com/kandev/kandev/internal/agentctl/server/process"
@@ -24,6 +26,15 @@ import (
ws "github.com/kandev/kandev/pkg/websocket"
)
type restartProfileResolver struct {
profile *AgentProfileInfo
err error
}
func (r *restartProfileResolver) ResolveProfile(_ context.Context, _ string) (*AgentProfileInfo, error) {
return r.profile, r.err
}
type restartMockAgentctlServer struct {
server *httptest.Server
@@ -287,6 +298,159 @@ func TestManager_RestartAgentProcess_Success(t *testing.T) {
}
}
func TestManager_RestartAgentProcess_InvalidReplacementLeavesCurrentProcessUntouched(t *testing.T) {
tests := []struct {
name string
resolver ProfileResolver
}{
{
name: "unresolvable profile",
resolver: &restartProfileResolver{err: errors.New("profile unavailable")},
},
{
name: "malformed persisted command prefix",
resolver: &restartProfileResolver{profile: &AgentProfileInfo{
ProfileID: "profile-1",
AgentName: "auggie",
CommandPrefix: `greywall "unterminated`,
}},
},
{
name: "persisted prefix has an empty quoted executable",
resolver: &restartProfileResolver{profile: &AgentProfileInfo{
ProfileID: "profile-1",
AgentName: "auggie",
CommandPrefix: `'' --arg`,
}},
},
{
name: "persisted prefix has a whitespace executable",
resolver: &restartProfileResolver{profile: &AgentProfileInfo{
ProfileID: "profile-1",
AgentName: "auggie",
CommandPrefix: `" " --arg`,
}},
},
{
name: "persisted prefix starts with a flag",
resolver: &restartProfileResolver{profile: &AgentProfileInfo{
ProfileID: "profile-1",
AgentName: "auggie",
CommandPrefix: `--not-a-launcher`,
}},
},
{
name: "persisted prefix starts with a space-prefixed flag",
resolver: &restartProfileResolver{profile: &AgentProfileInfo{
ProfileID: "profile-1",
AgentName: "auggie",
CommandPrefix: `" --not-a-launcher"`,
}},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mgr := newTestManager(t)
mgr.profileResolver = tt.resolver
mock := newRestartMockAgentctlServer(t, false, false)
client := createTestClient(t, mock.server.URL)
t.Cleanup(client.Close)
execution := &AgentExecution{
ID: "exec-invalid-restart",
TaskID: "task-1",
SessionID: "session-1",
AgentProfileID: "profile-1",
ACPSessionID: "old-acp-session",
AgentCommand: "auggie --resume old-acp-session",
ContinueCommand: "auggie continue old-acp-session",
AgentArgs: []string{"auggie", "--resume", "old-acp-session"},
ContinueArgs: []string{"auggie", "continue", "old-acp-session"},
Status: v1.AgentStatusRunning,
ErrorMessage: "existing error",
agentctl: client,
promptDoneCh: make(chan PromptCompletionSignal, 1),
}
require.NoError(t, mgr.executionStore.Add(execution))
err := mgr.RestartAgentProcess(context.Background(), execution.ID)
require.Error(t, err)
require.Empty(t, mock.getHTTPActions(), "invalid replacement must not stop, configure, or start the current process")
current, found := mgr.executionStore.Get(execution.ID)
require.True(t, found)
require.Equal(t, v1.AgentStatusRunning, current.Status)
require.Equal(t, "old-acp-session", current.ACPSessionID)
require.Equal(t, "auggie --resume old-acp-session", current.AgentCommand)
require.Equal(t, []string{"auggie", "--resume", "old-acp-session"}, current.AgentArgs)
require.Equal(t, "existing error", current.ErrorMessage)
})
}
}
func TestManager_RestartAgentProcess_InvalidBuiltReplacementLeavesCurrentProcessUntouched(t *testing.T) {
tests := []struct {
name string
agent *invalidCommandTestAgent
}{
{
name: "initial executable is a flag",
agent: &invalidCommandTestAgent{command: agents.NewCommand("--not-an-executable")},
},
{
name: "continue executable is whitespace",
agent: &invalidCommandTestAgent{
command: agents.NewCommand("agent"),
testAgent: testAgent{runtimeConfig: &agents.RuntimeConfig{SessionConfig: agents.SessionConfig{
ContinueSessionCmd: agents.NewCommand(" "),
}}},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mgr := newTestManager(t)
tt.agent.id = "invalid-restart"
require.NoError(t, mgr.registry.Register(tt.agent))
mgr.profileResolver = &restartProfileResolver{profile: &AgentProfileInfo{
ProfileID: "profile-1",
AgentName: tt.agent.ID(),
}}
mock := newRestartMockAgentctlServer(t, false, false)
client := createTestClient(t, mock.server.URL)
t.Cleanup(client.Close)
execution := &AgentExecution{
ID: "exec-invalid-built-restart",
TaskID: "task-1",
SessionID: "session-1",
AgentProfileID: "profile-1",
ACPSessionID: "old-acp-session",
AgentCommand: "agent --resume old-acp-session",
AgentArgs: []string{"agent", "--resume", "old-acp-session"},
Status: v1.AgentStatusRunning,
ErrorMessage: "existing error",
agentctl: client,
promptDoneCh: make(chan PromptCompletionSignal, 1),
}
require.NoError(t, mgr.executionStore.Add(execution))
err := mgr.RestartAgentProcess(context.Background(), execution.ID)
require.Error(t, err)
require.Empty(t, mock.getHTTPActions())
current, found := mgr.executionStore.Get(execution.ID)
require.True(t, found)
require.Equal(t, v1.AgentStatusRunning, current.Status)
require.Equal(t, "old-acp-session", current.ACPSessionID)
require.Equal(t, []string{"agent", "--resume", "old-acp-session"}, current.AgentArgs)
require.Equal(t, "existing error", current.ErrorMessage)
})
}
}
func TestManager_RestartAgentProcess_StopErrorIsNonFatal(t *testing.T) {
mgr := newTestManager(t)
mock := newRestartMockAgentctlServer(t, true, false)
@@ -204,21 +204,32 @@ func baseBranchMetadataKey(spec RepoLaunchSpec) string {
return repoName + "-" + branchSlug
}
// agentCommands holds the initial and continue command strings for an agent execution.
// agentCommands holds both the display strings and structured argv for an agent execution.
type agentCommands struct {
initial string
continue_ string // continue command for one-shot agents (empty if not applicable)
initial string
continue_ string // continue command for one-shot agents (empty if not applicable)
args []string
continueArgs []string
}
func newAgentCommands(args, continueArgs []string) agentCommands {
return agentCommands{
initial: strings.Join(args, " "),
continue_: strings.Join(continueArgs, " "),
args: args,
continueArgs: continueArgs,
}
}
// resolveProfileLaunchTokens resolves the user-configured cli_flags argv tokens
// and the launcher command_prefix tokens for a profile. Both must be applied on
// every command build — the initial launch AND fresh restarts (context reset) —
// or a sandboxed profile would silently relaunch without its wrapper.
// or a configured profile could silently relaunch without its wrapper.
//
// cli_flags are best-effort: a malformed entry is logged and dropped so a typo
// doesn't block the task. command_prefix is a sandbox boundary, so it fails
// CLOSED: a profile that configured a prefix which cannot be resolved returns an
// error and aborts the launch rather than running the agent unwrapped.
// doesn't block the task. command_prefix is launcher policy, so it fails closed:
// a profile that configured a prefix which cannot be resolved returns an error
// and aborts the launch rather than running the agent unwrapped.
func (m *Manager) resolveProfileLaunchTokens(profileInfo *AgentProfileInfo) (cliFlagTokens, commandPrefixTokens []string, err error) {
if profileInfo == nil {
return nil, nil, nil
@@ -231,12 +242,15 @@ func (m *Manager) resolveProfileLaunchTokens(profileInfo *AgentProfileInfo) (cli
cliFlagTokens = tokens
}
if strings.TrimSpace(profileInfo.CommandPrefix) != "" {
tokens, tokErr := cliflags.Tokenise(profileInfo.CommandPrefix)
if tokErr != nil || len(tokens) == 0 {
if validateErr := cliflags.ValidateCommandPrefix(profileInfo.CommandPrefix); validateErr != nil {
return nil, nil, fmt.Errorf("resolve command_prefix for profile %s: %w",
profileInfo.ProfileID, tokErr)
profileInfo.ProfileID, validateErr)
}
commandPrefixTokens, err = cliflags.Tokenise(profileInfo.CommandPrefix)
if err != nil {
return nil, nil, fmt.Errorf("resolve command_prefix for profile %s: %w",
profileInfo.ProfileID, err)
}
commandPrefixTokens = tokens
}
return cliFlagTokens, commandPrefixTokens, nil
}
@@ -244,7 +258,7 @@ func (m *Manager) resolveProfileLaunchTokens(profileInfo *AgentProfileInfo) (cli
// buildAgentCommand builds the agent command strings for the execution.
// Returns both the initial command and the continue command (for one-shot agents like Amp).
// Returns an error when a configured command_prefix cannot be resolved, so a
// sandboxed profile fails closed instead of launching unwrapped.
// configured profile fails closed instead of launching unwrapped.
func (m *Manager) buildAgentCommand(req *LaunchRequest, profileInfo *AgentProfileInfo, agentConfig agents.Agent, preferNative bool) (agentCommands, error) {
model := ""
autoApprove := false
@@ -281,10 +295,24 @@ func (m *Manager) buildAgentCommand(req *LaunchRequest, profileInfo *AgentProfil
Runtime: models.ExecutorType(req.ExecutorType).Runtime(),
PreferNativeBinary: preferNative,
}
return agentCommands{
initial: m.commandBuilder.BuildCommandString(agentConfig, cmdOpts),
continue_: m.commandBuilder.BuildContinueCommandString(agentConfig, cmdOpts),
}, nil
args := m.commandBuilder.BuildCommandArgs(agentConfig, cmdOpts)
continueArgs := m.commandBuilder.BuildContinueCommandArgs(agentConfig, cmdOpts)
if err := validateBuiltAgentCommands(args, continueArgs); err != nil {
return agentCommands{}, err
}
return newAgentCommands(args, continueArgs), nil
}
func validateBuiltAgentCommands(args, continueArgs []string) error {
if err := cliflags.ValidateCommandArgs(args); err != nil {
return fmt.Errorf("validate agent command: %w", err)
}
if continueArgs != nil {
if err := cliflags.ValidateCommandArgs(continueArgs); err != nil {
return fmt.Errorf("validate continue command: %w", err)
}
}
return nil
}
// launchResolveWorkspacePath resolves the effective workspace path for non-worktree executors.
@@ -925,6 +953,8 @@ func (m *Manager) promoteWorkspaceExecution(ctx context.Context, execution *Agen
}
execution.AgentCommand = cmds.initial
execution.ContinueCommand = cmds.continue_
execution.AgentArgs = cmds.args
execution.ContinueArgs = cmds.continueArgs
if req.ACPSessionID != "" && execution.ACPSessionID == "" {
execution.ACPSessionID = req.ACPSessionID
}
@@ -936,6 +966,8 @@ func (m *Manager) promoteWorkspaceExecution(ctx context.Context, execution *Agen
if err := m.materializeRuntimeProjectMCP(sharedCtx, execution, agentConfig); err != nil {
execution.AgentCommand = ""
execution.ContinueCommand = ""
execution.AgentArgs = nil
execution.ContinueArgs = nil
execution.isResumedSession = false
execution.IsPassthrough = false
return nil, err
@@ -1055,12 +1087,15 @@ func (m *Manager) launchInternal(ctx context.Context, req *LaunchRequest) (*Agen
// instance directly to avoid leaking it, then fail closed.
if rt != nil && execInstance != nil {
cleanupCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
if stopErr := rt.StopInstance(cleanupCtx, execInstance, false); stopErr != nil {
if stopErr := rt.StopInstance(cleanupCtx, execInstance, true); stopErr != nil {
m.logger.Warn("failed to stop runtime instance after command resolution error",
zap.Error(stopErr))
}
cancel()
}
if execInstance != nil && execInstance.Client != nil {
execInstance.Client.Close()
}
return nil, err
}
if profileInfo != nil && len(profileInfo.EnvVars) > 0 {
@@ -1118,6 +1153,8 @@ func (m *Manager) buildExecutionFromInstance(
}
execution.AgentCommand = cmds.initial
execution.ContinueCommand = cmds.continue_
execution.AgentArgs = cmds.args
execution.ContinueArgs = cmds.continueArgs
return execution, nil
}
@@ -1167,7 +1204,7 @@ func (m *Manager) rollbackLaunchExecution(_ context.Context, rt ExecutorBackend,
if rt != nil && execInstance != nil {
cleanupCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
if stopErr := rt.StopInstance(cleanupCtx, execInstance, false); stopErr != nil {
if stopErr := rt.StopInstance(cleanupCtx, execInstance, true); stopErr != nil {
m.logger.Warn("failed to stop runtime instance during launch rollback",
zap.String("execution_id", execution.ID),
zap.Error(stopErr))
@@ -1322,7 +1359,7 @@ func (m *Manager) configureAndStartAgent(ctx context.Context, execution *AgentEx
return "", fmt.Errorf("failed to prepare agent env: %w", err)
}
if err := execution.agentctl.ConfigureAgent(ctx, execution.AgentCommand, env, approvalPolicy, execution.ContinueCommand); err != nil {
if err := execution.agentctl.ConfigureAgent(ctx, execution.AgentCommand, execution.AgentArgs, env, approvalPolicy, execution.ContinueCommand, execution.ContinueArgs); err != nil {
return "", fmt.Errorf("failed to configure agent: %w", err)
}
@@ -11,8 +11,10 @@ import (
"os"
"os/exec"
"path/filepath"
"reflect"
"strconv"
"strings"
"sync/atomic"
"testing"
"time"
@@ -136,6 +138,15 @@ func (a *cliFlagTestAgent) BuildCommand(_ agents.CommandOptions) agents.Command
return agents.Cmd("copilot", "--acp").Build()
}
type invalidCommandTestAgent struct {
testAgent
command agents.Command
}
func (a *invalidCommandTestAgent) BuildCommand(_ agents.CommandOptions) agents.Command {
return a.command
}
func TestBuildAgentCommand_CLIFlagsAppended(t *testing.T) {
mgr := newTestManager(t)
ag := &cliFlagTestAgent{}
@@ -242,6 +253,141 @@ func TestBuildAgentCommand_CommandPrefix(t *testing.T) {
})
}
func TestBuildAgentCommand_RejectsInvalidBuiltArgv(t *testing.T) {
mgr := newTestManager(t)
tests := []struct {
name string
agent *invalidCommandTestAgent
}{
{
name: "initial executable is a flag",
agent: &invalidCommandTestAgent{command: agents.NewCommand("--not-an-executable")},
},
{
name: "continue executable is whitespace",
agent: &invalidCommandTestAgent{
command: agents.NewCommand("agent"),
testAgent: testAgent{runtimeConfig: &agents.RuntimeConfig{SessionConfig: agents.SessionConfig{
ContinueSessionCmd: agents.NewCommand(" "),
}}},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := mgr.buildAgentCommand(&LaunchRequest{}, nil, tt.agent, false)
require.Error(t, err)
})
}
}
func TestManager_LaunchRejectsInvalidBuiltArgvBeforeRegistration(t *testing.T) {
tests := []struct {
name string
agent *invalidCommandTestAgent
}{
{
name: "initial executable is a flag",
agent: &invalidCommandTestAgent{
testAgent: testAgent{id: "invalid-command", enabled: true},
command: agents.NewCommand("--not-an-executable"),
},
},
{
name: "continue executable is whitespace",
agent: &invalidCommandTestAgent{
testAgent: testAgent{
id: "invalid-command",
enabled: true,
runtimeConfig: &agents.RuntimeConfig{SessionConfig: agents.SessionConfig{
ContinueSessionCmd: agents.NewCommand(" "),
}},
},
command: agents.NewCommand("agent"),
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
resolver := &countingProfileResolver{info: &AgentProfileInfo{
ProfileID: "profile-invalid-command",
AgentName: tt.agent.ID(),
}}
mgr, backend := newEnvironmentExecutionTestManagerWithProfileResolver(t, nil, resolver)
require.NoError(t, mgr.registry.Register(tt.agent))
var configureCalls, startCalls atomic.Int32
backend.client = newLaunchRollbackAgentctlClient(t, mgr.logger, &configureCalls, &startCalls)
execution, err := mgr.Launch(context.Background(), &LaunchRequest{
TaskID: "task-invalid-command",
SessionID: "session-invalid-command",
AgentProfileID: "profile-invalid-command",
ExecutorType: string(models.ExecutorTypeLocal),
IsEphemeral: true,
})
require.Nil(t, execution)
require.ErrorContains(t, err, "validate")
require.Equal(t, int32(1), backend.createCount.Load())
require.Equal(t, int32(1), backend.stopCount.Load())
require.True(t, backend.forceStopped.Load())
_, found := mgr.executionStore.GetBySessionID("session-invalid-command")
require.False(t, found)
require.Zero(t, configureCalls.Load())
require.Zero(t, startCalls.Load())
})
}
}
func TestManager_LaunchBuildExecutionFailureRollsBackCreatedInstance(t *testing.T) {
resolver := &countingProfileResolver{info: &AgentProfileInfo{
ProfileID: "profile-1",
AgentName: "auggie",
CommandPrefix: `greywall "unterminated`,
}}
mgr, backend := newEnvironmentExecutionTestManagerWithProfileResolver(t, nil, resolver)
var configureCalls, startCalls atomic.Int32
backend.client = newLaunchRollbackAgentctlClient(t, mgr.logger, &configureCalls, &startCalls)
execution, err := mgr.Launch(context.Background(), &LaunchRequest{
TaskID: "task-launch-rollback",
SessionID: "session-launch-rollback",
AgentProfileID: "profile-1",
ExecutorType: string(models.ExecutorTypeLocal),
IsEphemeral: true,
})
require.Nil(t, execution)
require.Error(t, err)
require.ErrorContains(t, err, "resolve command_prefix")
require.Equal(t, int32(1), backend.createCount.Load(), "instance must be created before command construction fails")
require.Equal(t, int32(1), backend.stopCount.Load(), "created instance must be stopped exactly once")
require.True(t, backend.forceStopped.Load(), "failed instance cleanup must force runtime teardown")
_, found := mgr.executionStore.GetBySessionID("session-launch-rollback")
require.False(t, found, "failed execution must never be registered")
require.Zero(t, configureCalls.Load(), "command construction failure must not configure agentctl")
require.Zero(t, startCalls.Load(), "command construction failure must not start agentctl")
}
func newLaunchRollbackAgentctlClient(t *testing.T, log *logger.Logger, configureCalls, startCalls *atomic.Int32) *agentctl.Client {
t.Helper()
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/health":
w.WriteHeader(http.StatusOK)
case "/api/v1/agent/configure":
configureCalls.Add(1)
_, _ = w.Write([]byte(`{"success":true}`))
case "/api/v1/start":
startCalls.Add(1)
_, _ = w.Write([]byte(`{"success":true}`))
default:
w.WriteHeader(http.StatusNotFound)
}
}))
t.Cleanup(server.Close)
return newTestAgentctlClient(t, server.URL, log)
}
func TestBuildEnvForExecution_ResolvesSecretBackedProfileEnv(t *testing.T) {
store := newInMemorySecretStore()
if err := store.Create(context.Background(), &secrets.SecretWithValue{
@@ -491,6 +637,30 @@ func TestConfigureAndStartAgent_DoesNotSendTaskDescriptionEnv(t *testing.T) {
}
}
func TestConfigureAndStartAgent_SendsStructuredArgv(t *testing.T) {
mgr := newTestManager(t)
var captured []string
client := newConfigureArgsCaptureAgentctlClient(t, newTestLogger(), &captured)
execution := &AgentExecution{
ID: "exec-argv",
TaskID: "task-1",
SessionID: "session-1",
AgentProfileID: "profile-1",
AgentCommand: "runner two words C:\\tools\\agent.exe",
AgentArgs: []string{"runner", "two words", "", `C:\tools\agent.exe`},
WorkspacePath: t.TempDir(),
agentctl: client,
}
if _, err := mgr.configureAndStartAgent(context.Background(), execution, "never"); err != nil {
t.Fatalf("configure and start agent: %v", err)
}
want := []string{"runner", "two words", "", `C:\tools\agent.exe`}
if !reflect.DeepEqual(captured, want) {
t.Fatalf("configured argv = %#v, want %#v", captured, want)
}
}
func TestConfigureAndStartAgent_SpillsLargeWakePayloadEnv(t *testing.T) {
mgr := newTestManager(t)
var configuredEnv map[string]string
@@ -586,7 +756,38 @@ func newConfigureCaptureAgentctlClient(t *testing.T, log *logger.Logger, capture
}))
t.Cleanup(server.Close)
parsed, err := url.Parse(server.URL)
return newTestAgentctlClient(t, server.URL, log)
}
func newConfigureArgsCaptureAgentctlClient(t *testing.T, log *logger.Logger, captured *[]string) *agentctl.Client {
t.Helper()
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/v1/agent/configure":
var req struct {
AgentArgs []string `json:"agent_args"`
}
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
t.Errorf("decode configure request: %v", err)
w.WriteHeader(http.StatusBadRequest)
return
}
*captured = req.AgentArgs
_, _ = w.Write([]byte(`{"success":true}`))
case "/api/v1/start":
_, _ = w.Write([]byte(`{"success":true,"command":"runner"}`))
default:
w.WriteHeader(http.StatusNotFound)
}
}))
t.Cleanup(server.Close)
return newTestAgentctlClient(t, server.URL, log)
}
func newTestAgentctlClient(t *testing.T, serverURL string, log *logger.Logger) *agentctl.Client {
t.Helper()
parsed, err := url.Parse(serverURL)
if err != nil {
t.Fatalf("parse test server URL: %v", err)
}
@@ -1011,6 +1212,11 @@ func TestLaunchResolveWorkspacePath_WorktreeWithoutRepoFallsBackToScratch(t *tes
// path and StartAgentProcess() then failed with "no agent command configured".
func TestLaunch_PromotesWorkspaceOnlyExecution(t *testing.T) {
mgr := newTestManager(t)
mgr.profileResolver = &countingProfileResolver{info: &AgentProfileInfo{
ProfileID: "profile-1",
AgentName: "auggie",
CommandPrefix: `"/opt/wrapper dir/wrapper" --`,
}}
// Inject a workspace-only execution: AgentCommand is intentionally empty,
// matching what createExecution produces when called from
@@ -1035,6 +1241,9 @@ func TestLaunch_PromotesWorkspaceOnlyExecution(t *testing.T) {
require.NoError(t, err)
require.Same(t, existing, got, "Launch must reuse the workspace-only execution, not create a new one")
require.NotEmpty(t, got.AgentCommand, "AgentCommand must be populated by promotion")
require.GreaterOrEqual(t, len(got.AgentArgs), 2, "promotion must populate structured argv")
require.Equal(t, []string{"/opt/wrapper dir/wrapper", "--"}, got.AgentArgs[:2],
"promotion must preserve the prefix argv token containing spaces")
require.Equal(t, "acp-session-abc", got.ACPSessionID, "ACPSessionID must be carried over from the request")
require.True(t, got.isResumedSession, "isResumedSession must be set when PreviousExecutionID is non-empty")
}
@@ -144,6 +144,12 @@ func TestPromoteWorkspaceExecutionResetsCommandWhenProjectMCPFails(t *testing.T)
if execution.ContinueCommand != "" {
t.Fatalf("ContinueCommand = %q, want reset to empty", execution.ContinueCommand)
}
if execution.AgentArgs != nil {
t.Fatalf("AgentArgs = %#v, want nil after failed promotion", execution.AgentArgs)
}
if execution.ContinueArgs != nil {
t.Fatalf("ContinueArgs = %#v, want nil after failed promotion", execution.ContinueArgs)
}
if execution.IsPassthrough {
t.Fatal("IsPassthrough should be reset after failed promotion")
}
@@ -43,6 +43,8 @@ type AgentExecution struct {
ACPSessionID string // ACP session ID to resume, if available
AgentCommand string // Command to start the agent subprocess
ContinueCommand string // Command for follow-up prompts (one-shot agents like Amp)
AgentArgs []string // Structured argv for AgentCommand
ContinueArgs []string // Structured argv for ContinueCommand
RuntimeName agentruntime.Runtime // Name of the runtime used (e.g., "docker", "standalone")
Status v1.AgentStatus
StartedAt time.Time
@@ -35,6 +35,41 @@ func Tokenise(raw string) ([]string, error) {
return st.tokens, nil
}
// ValidateCommandPrefix verifies an optional launcher command prefix. Empty
// input means no launcher; otherwise argv[0] must be a non-blank executable
// name rather than a flag. This is shared by profile save/preview and runtime
// launch paths so legacy persisted profiles cannot bypass launcher policy.
func ValidateCommandPrefix(prefix string) error {
if strings.TrimSpace(prefix) == "" {
return nil
}
tokens, err := Tokenise(prefix)
if err != nil {
return err
}
if err := ValidateCommandArgs(tokens); err != nil {
return err
}
return nil
}
// ValidateCommandArgs rejects argv that cannot safely identify an executable.
// It intentionally permits empty secondary arguments because they can be
// meaningful values passed to an otherwise valid command.
func ValidateCommandArgs(args []string) error {
if len(args) == 0 {
return fmt.Errorf("agent command cannot be empty")
}
executable := strings.TrimSpace(args[0])
if executable == "" {
return fmt.Errorf("agent command executable is invalid")
}
if strings.HasPrefix(executable, "-") {
return fmt.Errorf("first token %q must be a launcher command, not a flag", args[0])
}
return nil
}
// tokeniseState carries the scanner state between step calls.
type tokeniseState struct {
tokens []string
@@ -51,6 +51,58 @@ func TestTokenise(t *testing.T) {
}
}
func TestValidateCommandPrefix(t *testing.T) {
cases := []struct {
name string
prefix string
wantErr bool
}{
{name: "empty means no launcher"},
{name: "whitespace means no launcher", prefix: " \t "},
{name: "launcher command", prefix: "greywall --"},
{name: "unterminated quote", prefix: `greywall "unterminated`, wantErr: true},
{name: "empty quoted executable", prefix: `'' --arg`, wantErr: true},
{name: "whitespace executable", prefix: `" " --arg`, wantErr: true},
{name: "flag executable", prefix: "--not-a-launcher", wantErr: true},
{name: "leading-space flag executable", prefix: `" --not-a-launcher"`, wantErr: true},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
err := ValidateCommandPrefix(tc.prefix)
if tc.wantErr && err == nil {
t.Fatal("expected error")
}
if !tc.wantErr && err != nil {
t.Fatalf("unexpected error: %v", err)
}
})
}
}
func TestValidateCommandArgs(t *testing.T) {
cases := []struct {
name string
args []string
wantErr bool
}{
{name: "valid executable with empty secondary argument", args: []string{"runner", ""}},
{name: "empty argv", wantErr: true},
{name: "whitespace executable", args: []string{" \t "}, wantErr: true},
{name: "flag executable", args: []string{" --not-a-launcher"}, wantErr: true},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
err := ValidateCommandArgs(tc.args)
if tc.wantErr && err == nil {
t.Fatal("expected error")
}
if !tc.wantErr && err != nil {
t.Fatalf("unexpected error: %v", err)
}
})
}
}
func TestResolve(t *testing.T) {
flags := []models.CLIFlag{
{Flag: "--allow-all-tools", Enabled: true},
@@ -174,6 +174,9 @@ type CommandPreviewRequest struct {
// PreviewAgentCommand generates a preview of the CLI command that will be executed
func (c *Controller) PreviewAgentCommand(ctx context.Context, agentName string, req CommandPreviewRequest) (*dto.CommandPreviewResponse, error) {
if err := validateCommandPrefix(req.CommandPrefix); err != nil {
return nil, err
}
agentConfig, ok := c.agentRegistry.Get(agentName)
if !ok {
return nil, fmt.Errorf("agent type %q not found in registry", agentName)
@@ -209,10 +212,9 @@ func (c *Controller) PreviewAgentCommand(ctx context.Context, agentName string,
cmd = cmd.With().Flag(cliFlagTokens...).Build()
}
// Prepend the launcher prefix last, mirroring
// lifecycle.CommandBuilder.BuildCommand. Tolerate malformed input
// silently — the preview is informational. Passthrough never applies
// the prefix (neither does the launch path), so this stays in the ACP
// branch only.
// lifecycle.CommandBuilder.BuildCommand. Prefixes were validated above;
// passthrough never applies one (neither does the launch path), so this
// stays in the ACP branch only.
if prefixTokens, err := cliflags.Tokenise(req.CommandPrefix); err == nil && len(prefixTokens) > 0 {
cmd = agents.NewCommand(append(prefixTokens, cmd.Args()...)...)
}
@@ -2,6 +2,7 @@ package controller
import (
"context"
"errors"
"os"
"path/filepath"
"strings"
@@ -172,6 +173,26 @@ func TestController_PreviewAgentCommand_StandardCommand(t *testing.T) {
}
}
func TestController_PreviewAgentCommand_RejectsInvalidCommandPrefix(t *testing.T) {
controller := newTestController(map[string]agents.Agent{
"test-agent": &testAgent{
id: "test-agent",
name: "test-agent",
enabled: true,
runtime: &agents.RuntimeConfig{Cmd: agents.NewCommand("test-cli")},
},
})
for _, prefix := range []string{`greywall "unterminated`, `'' --arg`, `--not-a-launcher`} {
_, err := controller.PreviewAgentCommand(context.Background(), "test-agent", CommandPreviewRequest{
CommandPrefix: prefix,
})
if !errors.Is(err, ErrInvalidCommandPrefix) {
t.Errorf("prefix %q: expected ErrInvalidCommandPrefix, got %v", prefix, err)
}
}
}
func TestController_PreviewAgentCommand_PassthroughCommand(t *testing.T) {
agentList := map[string]agents.Agent{
"claude-code": &testAgent{
@@ -234,23 +234,9 @@ func validateCLIFlagDTOs(in []dto.CLIFlagDTO) error {
// launch path's cliflags.Tokenise error branch unreachable in practice, so a
// bad prefix surfaces at save time rather than silently dropping at task start.
func validateCommandPrefix(prefix string) error {
if strings.TrimSpace(prefix) == "" {
return nil
}
tokens, err := cliflags.Tokenise(prefix)
if err != nil {
if err := cliflags.ValidateCommandPrefix(prefix); err != nil {
return fmt.Errorf("%w: %v", ErrInvalidCommandPrefix, err)
}
if len(tokens) == 0 || tokens[0] == "" {
return fmt.Errorf("%w: must start with a launcher command", ErrInvalidCommandPrefix)
}
// The first token is the launcher executable. A leading '-' means the user
// typed a flag first (e.g. "--foo greywall"), which would run the flag as
// the program — almost certainly a mistake, and a confusing failure mode
// for a sandbox boundary.
if strings.HasPrefix(tokens[0], "-") {
return fmt.Errorf("%w: first token %q must be a launcher command, not a flag", ErrInvalidCommandPrefix, tokens[0])
}
return nil
}
@@ -137,6 +137,7 @@ func TestValidateCommandPrefix(t *testing.T) {
{name: "unterminated quote rejected", prefix: `greywall "unterminated`, wantErr: true},
{name: "trailing backslash rejected", prefix: `greywall foo\`, wantErr: true},
{name: "only-empty-quotes rejected", prefix: `""`, wantErr: true},
{name: "whitespace executable rejected", prefix: `" " --arg`, wantErr: true},
{name: "leading flag rejected", prefix: `--foo greywall`, wantErr: true},
{name: "leading dash rejected", prefix: `-x sandbox`, wantErr: true},
}
@@ -325,4 +326,35 @@ func TestCreateAgentProfiles_RejectsBadCommandPrefix(t *testing.T) {
if !errors.Is(err, ErrInvalidCommandPrefix) {
t.Fatalf("expected ErrInvalidCommandPrefix, got %v", err)
}
if len(st.created) != 0 {
t.Fatalf("bad nested profile create made %d profile writes", len(st.created))
}
}
func TestCreateAgent_RejectsBadNestedProfileBeforeAnyWrite(t *testing.T) {
t.Setenv("KANDEV_E2E_MOCK", "true")
ctrl := newTestController(map[string]agents.Agent{
"test-agent": &testAgent{
id: "test-agent",
name: "test-agent",
displayName: "Test Agent",
enabled: true,
},
})
st := newFakeStore()
ctrl.repo = st
_, err := ctrl.CreateAgent(context.Background(), CreateAgentRequest{
Name: "test-agent",
Profiles: []CreateAgentProfileRequest{{
Name: "Bad",
CommandPrefix: `--not-a-launcher`,
}},
})
if !errors.Is(err, ErrInvalidCommandPrefix) {
t.Fatalf("expected ErrInvalidCommandPrefix, got %v", err)
}
if len(st.agents) != 0 || len(st.created) != 0 {
t.Fatalf("bad create made writes: agents=%d profiles=%d", len(st.agents), len(st.created))
}
}
@@ -13,6 +13,7 @@ import (
"github.com/kandev/kandev/internal/agent/mcpconfig"
"github.com/kandev/kandev/internal/agent/settings/controller"
"github.com/kandev/kandev/internal/agent/settings/dto"
"github.com/kandev/kandev/internal/common/httpmw"
"github.com/kandev/kandev/internal/common/logger"
ws "github.com/kandev/kandev/pkg/websocket"
"go.uber.org/zap"
@@ -26,25 +27,27 @@ type Handlers struct {
controller *controller.Controller
hub Broadcaster
logger *logger.Logger
interlock gin.HandlerFunc
}
type Broadcaster interface {
Broadcast(msg *ws.Message)
}
func NewHandlers(ctrl *controller.Controller, hub Broadcaster, log *logger.Logger) *Handlers {
func NewHandlers(ctrl *controller.Controller, hub Broadcaster, log *logger.Logger, interlockToken string) *Handlers {
return &Handlers{
controller: ctrl,
hub: hub,
logger: log.WithFields(zap.String("component", "agent-settings-handlers")),
interlock: httpmw.InterimSettingsInterlock(interlockToken),
}
}
func RegisterRoutes(router *gin.Engine, ctrl *controller.Controller, hub Broadcaster, log *logger.Logger) {
func RegisterRoutes(router *gin.Engine, ctrl *controller.Controller, hub Broadcaster, log *logger.Logger, interlockToken string) {
// Wire the install job store with the same broadcaster used by other
// agent-settings notifications so streaming install events reach the UI.
ctrl.SetJobBroadcaster(hub)
handlers := NewHandlers(ctrl, hub, log)
handlers := NewHandlers(ctrl, hub, log, interlockToken)
handlers.registerHTTP(router)
}
@@ -54,22 +57,22 @@ func (h *Handlers) registerHTTP(router *gin.Engine) {
api.GET("/agents/available", h.httpListAvailableAgents)
api.GET("/agents/usage", h.httpAgentSubscriptionUsage)
api.GET("/agents", h.httpListAgents)
api.POST("/agents", h.httpCreateAgent)
api.POST("/agents/tui", h.httpCreateCustomTUIAgent)
api.POST("/agents", h.interlock, h.httpCreateAgent)
api.POST("/agents/tui", h.interlock, h.httpCreateCustomTUIAgent)
api.GET("/agents/:id", h.httpGetAgent)
api.PATCH("/agents/:id", h.httpUpdateAgent)
api.DELETE("/agents/:id", h.httpDeleteAgent)
api.POST("/agents/:id/profiles", h.httpCreateProfile)
api.PATCH("/agents/:id", h.interlock, h.httpUpdateAgent)
api.DELETE("/agents/:id", h.interlock, h.httpDeleteAgent)
api.POST("/agents/:id/profiles", h.interlock, h.httpCreateProfile)
api.GET("/agents/:id/logo", h.httpGetAgentLogo)
api.GET("/agent-models/:agentName", h.httpGetAgentModels)
api.POST("/agent-command-preview/:agentName", h.httpPreviewAgentCommand)
api.POST("/agent-install/:agentName", h.httpInstallAgent)
api.POST("/agent-install/:agentName", h.interlock, h.httpInstallAgent)
api.GET("/agent-install/jobs", h.httpListInstallJobs)
api.GET("/agent-install/jobs/:id", h.httpGetInstallJob)
api.PATCH("/agent-profiles/:id", h.httpUpdateProfile)
api.DELETE("/agent-profiles/:id", h.httpDeleteProfile)
api.PATCH("/agent-profiles/:id", h.interlock, h.httpUpdateProfile)
api.DELETE("/agent-profiles/:id", h.interlock, h.httpDeleteProfile)
api.GET("/agent-profiles/:id/mcp-config", h.httpGetProfileMcpConfig)
api.POST("/agent-profiles/:id/mcp-config", h.httpUpdateProfileMcpConfig)
api.POST("/agent-profiles/:id/mcp-config", h.interlock, h.httpUpdateProfileMcpConfig)
}
func (h *Handlers) httpDiscoverAgents(c *gin.Context) {
@@ -0,0 +1,67 @@
package handlers
import (
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/gin-gonic/gin"
"github.com/kandev/kandev/internal/common/logger"
)
func TestRegisterRoutesRejectsMutationWithoutInterimSettingsInterlock(t *testing.T) {
gin.SetMode(gin.TestMode)
log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json"})
if err != nil {
t.Fatalf("NewLogger: %v", err)
}
router := gin.New()
NewHandlers(nil, nil, log, "test-interlock").registerHTTP(router)
request := httptest.NewRequest(http.MethodPost, "/api/v1/agents", strings.NewReader("{"))
request.Header.Set("Content-Type", "application/json")
response := httptest.NewRecorder()
router.ServeHTTP(response, request)
if response.Code != http.StatusForbidden {
t.Fatalf("status = %d, want %d", response.Code, http.StatusForbidden)
}
}
func TestRegisterRoutesProtectsEveryStateChangingAgentSettingsRoute(t *testing.T) {
gin.SetMode(gin.TestMode)
log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json"})
if err != nil {
t.Fatalf("NewLogger: %v", err)
}
router := gin.New()
NewHandlers(nil, nil, log, "test-interlock").registerHTTP(router)
requests := []struct {
method string
path string
}{
{method: http.MethodPost, path: "/api/v1/agents"},
{method: http.MethodPost, path: "/api/v1/agents/tui"},
{method: http.MethodPatch, path: "/api/v1/agents/agent-1"},
{method: http.MethodDelete, path: "/api/v1/agents/agent-1"},
{method: http.MethodPost, path: "/api/v1/agents/agent-1/profiles"},
{method: http.MethodPost, path: "/api/v1/agent-install/agent-1"},
{method: http.MethodPatch, path: "/api/v1/agent-profiles/profile-1"},
{method: http.MethodDelete, path: "/api/v1/agent-profiles/profile-1"},
{method: http.MethodPost, path: "/api/v1/agent-profiles/profile-1/mcp-config"},
}
for _, requestSpec := range requests {
t.Run(requestSpec.method+" "+requestSpec.path, func(t *testing.T) {
request := httptest.NewRequest(requestSpec.method, requestSpec.path, nil)
response := httptest.NewRecorder()
router.ServeHTTP(response, request)
if response.Code != http.StatusForbidden {
t.Fatalf("status = %d, want %d", response.Code, http.StatusForbidden)
}
})
}
}
@@ -7,6 +7,7 @@ import (
"io"
"net/http"
"net/http/httptest"
"slices"
"strings"
"testing"
"time"
@@ -43,6 +44,60 @@ func newTestServer(t *testing.T) *Server {
return NewServer(cfg, procMgr, nil, nil, log)
}
func TestHandleAgentConfigure_PrefersPresentStructuredArgs(t *testing.T) {
log := newTestLogger()
cfg := &config.InstanceConfig{Port: 0, WorkDir: t.TempDir()}
procMgr := process.NewManager(cfg, log)
server := NewServer(cfg, procMgr, nil, nil, log)
body := strings.NewReader(`{"command":"legacy command","agent_args":["runner","two words","","C:\\tools\\agent.exe"]}`)
req := httptest.NewRequest(http.MethodPost, "/api/v1/agent/configure", body)
req.Header.Set("Content-Type", "application/json")
resp := httptest.NewRecorder()
server.router.ServeHTTP(resp, req)
if resp.Code != http.StatusOK {
t.Fatalf("configure status = %d, want %d: %s", resp.Code, http.StatusOK, resp.Body.String())
}
want := []string{"runner", "two words", "", `C:\tools\agent.exe`}
if got := cfg.AgentArgs; !slices.Equal(got, want) {
t.Fatalf("AgentArgs = %#v, want %#v", got, want)
}
}
func TestHandleAgentConfigure_UsesLegacyOnlyWhenArgsAbsent(t *testing.T) {
log := newTestLogger()
cfg := &config.InstanceConfig{Port: 0, WorkDir: t.TempDir()}
server := NewServer(cfg, process.NewManager(cfg, log), nil, nil, log)
for _, tc := range []struct {
name string
body string
code int
want []string
}{
{name: "legacy absent", body: `{"command":"legacy --flag"}`, code: http.StatusOK, want: []string{"legacy", "--flag"}},
{name: "present empty rejects", body: `{"command":"legacy --flag","agent_args":[]}`, code: http.StatusInternalServerError},
{name: "present flag executable rejects", body: `{"command":"legacy --flag","agent_args":["--flag"]}`, code: http.StatusInternalServerError},
{name: "present null rejects", body: `{"command":"legacy --flag","agent_args":null}`, code: http.StatusInternalServerError},
{name: "present empty continue rejects", body: `{"command":"runner","agent_args":["runner"],"continue_command":"legacy continue","continue_args":[]}`, code: http.StatusInternalServerError},
} {
t.Run(tc.name, func(t *testing.T) {
req := httptest.NewRequest(http.MethodPost, "/api/v1/agent/configure", strings.NewReader(tc.body))
req.Header.Set("Content-Type", "application/json")
resp := httptest.NewRecorder()
server.router.ServeHTTP(resp, req)
if resp.Code != tc.code {
t.Fatalf("configure status = %d, want %d: %s", resp.Code, tc.code, resp.Body.String())
}
if tc.want != nil && !slices.Equal(cfg.AgentArgs, tc.want) {
t.Fatalf("AgentArgs = %#v, want %#v", cfg.AgentArgs, tc.want)
}
})
}
}
// dialTestWS connects a WebSocket client to the test server's /api/v1/agent/stream endpoint.
func dialTestWS(t *testing.T, server *httptest.Server) *websocket.Conn {
t.Helper()
@@ -374,11 +374,23 @@ func (s *Server) handleStart(c *gin.Context) {
// AgentConfigure request/response - configures the agent command before starting
type AgentConfigureRequest struct {
Command string `json:"command"`
AgentArgs optionalArgs `json:"agent_args"`
ContinueCommand string `json:"continue_command,omitempty"` // For one-shot agents (Amp): command for follow-up prompts
ContinueArgs optionalArgs `json:"continue_args"`
Env map[string]string `json:"env,omitempty"`
ApprovalPolicy string `json:"approval_policy,omitempty"` // For Codex: "untrusted", "on-failure", "on-request", "never"
}
type optionalArgs struct {
Present bool
Args []string
}
func (a *optionalArgs) UnmarshalJSON(data []byte) error {
a.Present = true
return json.Unmarshal(data, &a.Args)
}
type AgentConfigureResponse struct {
Success bool `json:"success"`
Message string `json:"message,omitempty"`
@@ -395,7 +407,7 @@ func (s *Server) handleAgentConfigure(c *gin.Context) {
return
}
if req.Command == "" {
if !req.AgentArgs.Present && req.Command == "" {
c.JSON(http.StatusBadRequest, AgentConfigureResponse{
Success: false,
Error: "command is required",
@@ -403,7 +415,7 @@ func (s *Server) handleAgentConfigure(c *gin.Context) {
return
}
if err := s.procMgr.Configure(req.Command, req.Env, req.ApprovalPolicy, req.ContinueCommand); err != nil {
if err := s.procMgr.Configure(req.Command, req.AgentArgs.Args, req.AgentArgs.Present, req.Env, req.ApprovalPolicy, req.ContinueCommand, req.ContinueArgs.Args, req.ContinueArgs.Present); err != nil {
s.logger.Error("failed to configure agent", zap.Error(err), zap.String("command", req.Command))
c.JSON(http.StatusInternalServerError, AgentConfigureResponse{
Success: false,
@@ -14,6 +14,7 @@ import (
"crypto/rand"
"crypto/subtle"
"encoding/hex"
"fmt"
"os"
"strconv"
"strings"
@@ -195,6 +196,9 @@ type InstanceConfig struct {
// continuation (thread ID appended at runtime). Only used by Amp.
ContinueCommand string
// ContinueArgs is the structured argv for ContinueCommand.
ContinueArgs []string
// VscodeCommand is the command to run the VS Code server (e.g., "code-server")
VscodeCommand string
@@ -457,6 +461,18 @@ func ParseCommand(cmd string) []string {
return strings.Fields(cmd)
}
// ValidateCommandArgs rejects argv that cannot safely identify an executable.
func ValidateCommandArgs(args []string) error {
if len(args) == 0 {
return fmt.Errorf("agent command cannot be empty")
}
executable := strings.TrimSpace(args[0])
if executable == "" || strings.HasPrefix(executable, "-") {
return fmt.Errorf("agent command executable is invalid")
}
return nil
}
// CollectAgentEnv collects environment variables to pass to the agent.
// It filters out AGENTCTL_* variables and optionally merges additional env vars.
func CollectAgentEnv(additional map[string]string) []string {
@@ -2,6 +2,27 @@ package config
import "testing"
func TestValidateCommandArgs(t *testing.T) {
for _, tc := range []struct {
name string
args []string
want bool
}{
{name: "valid preserves empty argument", args: []string{"runner", "", "two words"}},
{name: "empty argv", args: nil, want: true},
{name: "empty executable", args: []string{"", "arg"}, want: true},
{name: "whitespace executable", args: []string{" \t", "arg"}, want: true},
{name: "flag executable", args: []string{"--runner", "arg"}, want: true},
} {
t.Run(tc.name, func(t *testing.T) {
err := ValidateCommandArgs(tc.args)
if (err != nil) != tc.want {
t.Fatalf("ValidateCommandArgs(%#v) error = %v, want error=%v", tc.args, err, tc.want)
}
})
}
}
func TestConsumeNonce(t *testing.T) {
t.Run("valid nonce returns token and burns nonce", func(t *testing.T) {
cfg := &Config{
@@ -798,9 +798,9 @@ func (m *Manager) Start(ctx context.Context) error {
m.exitCode.Store(-1)
m.exitErr.Store(errorWrapper{err: nil})
if len(m.cfg.AgentArgs) == 0 {
if err := config.ValidateCommandArgs(m.cfg.AgentArgs); err != nil {
m.status.Store(StatusError)
return fmt.Errorf("no agent command configured")
return err
}
// Build adapter config and create protocol adapter
@@ -941,10 +941,14 @@ func (m *Manager) buildAdapterConfig() error {
// Configure one-shot mode when a continue command is provided.
// One-shot adapters (e.g., Amp) spawn a new subprocess per prompt.
if m.cfg.ContinueCommand != "" {
continueArgs := m.cfg.ContinueArgs
if continueArgs == nil && m.cfg.ContinueCommand != "" {
continueArgs = config.ParseCommand(m.cfg.ContinueCommand)
}
if len(continueArgs) > 0 {
m.adapterCfg.OneShotConfig = &adapter.OneShotConfig{
InitialArgs: m.cfg.AgentArgs,
ContinueArgs: config.ParseCommand(m.cfg.ContinueCommand),
ContinueArgs: continueArgs,
Env: m.cfg.AgentEnv,
WorkDir: m.cfg.WorkDir,
}
@@ -1160,7 +1164,7 @@ func lookupEnvValue(env []string, key string) string {
// Configure sets the agent command and optional environment variables.
// This must be called before Start() if the instance was created without a command.
// continueCommand is optional — when set, the adapter uses it for one-shot follow-up prompts.
func (m *Manager) Configure(command string, env map[string]string, approvalPolicy, continueCommand string) error {
func (m *Manager) Configure(command string, agentArgs []string, agentArgsPresent bool, env map[string]string, approvalPolicy, continueCommand string, continueArgs []string, continueArgsPresent bool) error {
m.startMu.Lock()
defer m.startMu.Unlock()
@@ -1168,14 +1172,22 @@ func (m *Manager) Configure(command string, env map[string]string, approvalPolic
return fmt.Errorf("cannot configure while agent is running")
}
if command == "" {
return fmt.Errorf("agent command cannot be empty")
args := agentArgs
if !agentArgsPresent {
args = config.ParseCommand(command)
}
// Parse the command string and update config
args := config.ParseCommand(command)
if len(args) == 0 {
return fmt.Errorf("failed to parse agent command")
if err := config.ValidateCommandArgs(args); err != nil {
return err
}
if continueArgsPresent {
if err := config.ValidateCommandArgs(continueArgs); err != nil {
return fmt.Errorf("invalid continue command: %w", err)
}
} else if continueCommand != "" {
continueArgs = config.ParseCommand(continueCommand)
if err := config.ValidateCommandArgs(continueArgs); err != nil {
return fmt.Errorf("invalid continue command: %w", err)
}
}
m.cfg.AgentCommand = command
@@ -1187,8 +1199,12 @@ func (m *Manager) Configure(command string, env map[string]string, approvalPolic
}
// Store continue command for one-shot adapters (e.g., Amp)
if continueCommand != "" {
if continueArgsPresent {
m.cfg.ContinueCommand = continueCommand
m.cfg.ContinueArgs = continueArgs
} else if continueCommand != "" {
m.cfg.ContinueCommand = continueCommand
m.cfg.ContinueArgs = continueArgs
}
// Merge additional env vars
+39 -37
View File
@@ -453,42 +453,43 @@ func sessionACPConfigBaseline(session *models.TaskSession) map[string]string {
// routeParams holds all dependencies needed for HTTP and WebSocket route registration.
type routeParams struct {
router *gin.Engine
gateway *gateways.Gateway
taskSvc *taskservice.Service
taskRepo *sqliterepo.Repository
officeRepo *officesqlite.Repository
analyticsRepo analyticsrepository.Repository
orchestratorSvc *orchestrator.Service
lifecycleMgr *lifecycle.Manager
hostUtilityMgr *hostutility.Manager
eventBus bus.EventBus
services *Services
systemSvc *systemsvc.Service
workspaceRestorer taskhandlers.WorkspaceQuarantineRestorer
runtimeFlagsSvc *runtimeflags.Service
agentSettingsController *agentsettingscontroller.Controller
agentSettingsRepo settingsstore.Repository
agentList taskhandlers.AgentLister
agentRegistry *registry.Registry
userCtrl *usercontroller.Controller
notificationCtrl *notificationcontroller.Controller
editorCtrl *editorcontroller.Controller
promptCtrl *promptcontroller.Controller
utilityCtrl *utilitycontroller.Controller
msgCreator *messageCreatorAdapter
secretsSvc *secrets.Service
secretStore secrets.SecretStore
mcpConfigSvc *mcpconfig.Service
addCleanup func(func() error)
repoCloner *repoclone.Cloner
version string
webInternalURL string
devMode bool
httpPort int
features config.FeaturesConfig
voice config.VoiceConfig
log *logger.Logger
router *gin.Engine
gateway *gateways.Gateway
taskSvc *taskservice.Service
taskRepo *sqliterepo.Repository
officeRepo *officesqlite.Repository
analyticsRepo analyticsrepository.Repository
orchestratorSvc *orchestrator.Service
lifecycleMgr *lifecycle.Manager
hostUtilityMgr *hostutility.Manager
eventBus bus.EventBus
services *Services
systemSvc *systemsvc.Service
workspaceRestorer taskhandlers.WorkspaceQuarantineRestorer
runtimeFlagsSvc *runtimeflags.Service
agentSettingsController *agentsettingscontroller.Controller
agentSettingsRepo settingsstore.Repository
agentList taskhandlers.AgentLister
agentRegistry *registry.Registry
userCtrl *usercontroller.Controller
notificationCtrl *notificationcontroller.Controller
editorCtrl *editorcontroller.Controller
promptCtrl *promptcontroller.Controller
utilityCtrl *utilitycontroller.Controller
msgCreator *messageCreatorAdapter
secretsSvc *secrets.Service
secretStore secrets.SecretStore
mcpConfigSvc *mcpconfig.Service
addCleanup func(func() error)
repoCloner *repoclone.Cloner
version string
webInternalURL string
devMode bool
httpPort int
features config.FeaturesConfig
voice config.VoiceConfig
interimSettingsInterlockToken string
log *logger.Logger
}
// registerRoutes sets up all HTTP and WebSocket routes on the given router.
@@ -696,6 +697,7 @@ func bootPayload(ctx context.Context, req *http.Request, p routeParams, route we
)
payload.RouteData = bootRouteData(ctx, req, p, route)
payload.Plugins = bootActivePlugins(p)
payload.InterimSettingsInterlockToken = p.interimSettingsInterlockToken
return payload
}
@@ -915,7 +917,7 @@ func registerSecondaryRoutes(
workflowhandlers.RegisterRoutes(p.router, p.gateway.Dispatcher, workflowCtrl, p.eventBus, p.log)
p.log.Info("Registered Workflow handlers (HTTP + WebSocket)")
agentsettingshandlers.RegisterRoutes(p.router, p.agentSettingsController, p.gateway.Hub, p.log)
agentsettingshandlers.RegisterRoutes(p.router, p.agentSettingsController, p.gateway.Hub, p.log, p.interimSettingsInterlockToken)
p.log.Debug("Registered Agent Settings handlers (HTTP)")
// Login PTY: spawns agent login commands under a PTY on the kandev host
@@ -2,17 +2,34 @@ package backendapp
import (
"context"
"errors"
"fmt"
"net"
"net/http"
"testing"
"time"
"github.com/kandev/kandev/internal/common/config"
"github.com/kandev/kandev/internal/common/logger"
"go.uber.org/goleak"
"go.uber.org/zap"
)
func TestBuildHTTPServerAbortsWhenInterlockTokenGenerationFails(t *testing.T) {
wantErr := errors.New("entropy unavailable")
original := newInterimSettingsInterlockToken
newInterimSettingsInterlockToken = func() (string, error) { return "", wantErr }
t.Cleanup(func() { newInterimSettingsInterlockToken = original })
server, err := buildHTTPServer(&config.Config{}, testLogger(t), nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil)
if !errors.Is(err, wantErr) {
t.Fatalf("buildHTTPServer error = %v, want %v", err, wantErr)
}
if server != nil {
t.Fatal("buildHTTPServer returned a server after token generation failed")
}
}
func testLogger(t *testing.T) *logger.Logger {
t.Helper()
log, err := logger.NewFromZap(zap.NewNop())
+57 -47
View File
@@ -788,14 +788,15 @@ func startGatewayAndServe(
// ============================================
// HTTP SERVER
// ============================================
server := buildHTTPServer(cfg, log, gateway, repos, services, agentSettingsController,
server, err := buildHTTPServer(cfg, log, gateway, repos, services, agentSettingsController,
lifecycleMgr, eventBus, orchestratorSvc, notificationCtrl, msgCreator, agentRegistry, hostUtilityMgr,
addCleanup, repoCloner, systemSvc, storageComposition.workspaceRestorer)
port := cfg.Server.Port
if port == 0 {
port = ports.Backend
if err != nil {
log.Error("Failed to build HTTP server", zap.Error(err))
return false
}
port := resolvedHTTPPort(cfg)
hosts, err := cfg.Server.ResolvedBinds()
if err != nil {
log.Error("Invalid server bind configuration", zap.Error(err))
@@ -1644,6 +1645,15 @@ func buildOfficeDashboardService(
}
// buildHTTPServer creates the HTTP server with all routes registered.
var newInterimSettingsInterlockToken = httpmw.NewInterimSettingsInterlockToken
func resolvedHTTPPort(cfg *config.Config) int {
if cfg.Server.Port != 0 {
return cfg.Server.Port
}
return ports.Backend
}
func buildHTTPServer(
cfg *config.Config,
log *logger.Logger,
@@ -1662,56 +1672,56 @@ func buildHTTPServer(
repoCloner *repoclone.Cloner,
systemSvc *systemsvc.Service,
workspaceRestorer taskhandlers.WorkspaceQuarantineRestorer,
) *http.Server {
) (*http.Server, error) {
gin.SetMode(gin.ReleaseMode)
router := gin.New()
router.Use(httpmw.RequestLogger(log, "kandev"))
router.Use(httpmw.OtelTracing("kandev"))
router.Use(gin.Recovery())
router.Use(corsMiddleware())
port := cfg.Server.Port
if port == 0 {
port = ports.Backend
interimSettingsInterlockToken, err := newInterimSettingsInterlockToken()
if err != nil {
return nil, fmt.Errorf("generate interim settings interlock token: %w", err)
}
registerRoutes(routeParams{
router: router,
gateway: gateway,
taskSvc: services.Task,
taskRepo: repos.Task,
officeRepo: repos.Office,
analyticsRepo: repos.Analytics,
orchestratorSvc: orchestratorSvc,
lifecycleMgr: lifecycleMgr,
hostUtilityMgr: hostUtilityMgr,
eventBus: eventBus,
services: services,
systemSvc: systemSvc,
workspaceRestorer: workspaceRestorer,
runtimeFlagsSvc: services.RuntimeFlags,
agentSettingsController: agentSettingsController,
agentSettingsRepo: repos.AgentSettings,
agentList: agentRegistry,
agentRegistry: agentRegistry,
userCtrl: usercontroller.NewController(services.User),
notificationCtrl: notificationCtrl,
editorCtrl: editorcontroller.NewController(services.Editor),
promptCtrl: promptcontroller.NewController(services.Prompts),
utilityCtrl: utilitycontroller.NewController(services.Utility),
msgCreator: msgCreator,
secretsSvc: secrets.NewService(repos.Secrets, log),
secretStore: repos.Secrets,
mcpConfigSvc: mcpconfig.NewService(repos.AgentSettings),
addCleanup: addCleanup,
repoCloner: repoCloner,
version: Version,
webInternalURL: cfg.Server.WebInternalURL,
devMode: cfg.Debug.DevMode || cfg.Debug.PprofEnabled,
httpPort: port,
features: cfg.Features,
voice: cfg.Voice,
log: log,
router: router,
gateway: gateway,
taskSvc: services.Task,
taskRepo: repos.Task,
officeRepo: repos.Office,
analyticsRepo: repos.Analytics,
orchestratorSvc: orchestratorSvc,
lifecycleMgr: lifecycleMgr,
hostUtilityMgr: hostUtilityMgr,
eventBus: eventBus,
services: services,
systemSvc: systemSvc,
workspaceRestorer: workspaceRestorer,
runtimeFlagsSvc: services.RuntimeFlags,
agentSettingsController: agentSettingsController,
agentSettingsRepo: repos.AgentSettings,
agentList: agentRegistry,
agentRegistry: agentRegistry,
userCtrl: usercontroller.NewController(services.User),
notificationCtrl: notificationCtrl,
editorCtrl: editorcontroller.NewController(services.Editor),
promptCtrl: promptcontroller.NewController(services.Prompts),
utilityCtrl: utilitycontroller.NewController(services.Utility),
msgCreator: msgCreator,
secretsSvc: secrets.NewService(repos.Secrets, log),
secretStore: repos.Secrets,
mcpConfigSvc: mcpconfig.NewService(repos.AgentSettings),
addCleanup: addCleanup,
repoCloner: repoCloner,
version: Version,
webInternalURL: cfg.Server.WebInternalURL,
devMode: cfg.Debug.DevMode || cfg.Debug.PprofEnabled,
httpPort: resolvedHTTPPort(cfg),
features: cfg.Features,
voice: cfg.Voice,
interimSettingsInterlockToken: interimSettingsInterlockToken,
log: log,
})
// Addr is intentionally left unset: bind addresses are resolved from
@@ -1722,7 +1732,7 @@ func buildHTTPServer(
Handler: router,
ReadTimeout: cfg.Server.ReadTimeoutDuration(),
WriteTimeout: cfg.Server.WriteTimeoutDuration(),
}
}, nil
}
// awaitShutdown waits for an OS signal then performs graceful shutdown.
@@ -37,7 +37,7 @@ func corsMiddleware() gin.HandlerFunc {
c.Header("Access-Control-Allow-Origin", "*")
}
c.Header("Access-Control-Allow-Methods", "GET, POST, PATCH, PUT, DELETE, OPTIONS")
c.Header("Access-Control-Allow-Headers", "Origin, Content-Type, Authorization, Upgrade, Connection, Sec-WebSocket-Key, Sec-WebSocket-Version, Sec-WebSocket-Protocol")
c.Header("Access-Control-Allow-Headers", "Origin, Content-Type, Authorization, X-Kandev-Interim-Settings-Interlock, Upgrade, Connection, Sec-WebSocket-Key, Sec-WebSocket-Version, Sec-WebSocket-Protocol")
if c.Request.Method == "OPTIONS" {
c.AbortWithStatus(http.StatusNoContent)
@@ -3,6 +3,7 @@ package backendapp
import (
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/gin-gonic/gin"
@@ -30,6 +31,28 @@ func TestCORSMiddlewareEchoesOriginForCredentialedRequests(t *testing.T) {
}
}
func TestCORSMiddlewareAllowsInterimSettingsInterlockHeader(t *testing.T) {
gin.SetMode(gin.TestMode)
router := gin.New()
router.Use(corsMiddleware())
router.POST("/api/v1/agents", func(c *gin.Context) { c.Status(http.StatusNoContent) })
req := httptest.NewRequest(http.MethodOptions, "/api/v1/agents", nil)
req.Host = "localhost:38429"
req.Header.Set("Origin", "http://localhost:37429")
req.Header.Set("Access-Control-Request-Method", http.MethodPost)
req.Header.Set("Access-Control-Request-Headers", "X-Kandev-Interim-Settings-Interlock")
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
if rec.Code != http.StatusNoContent {
t.Fatalf("status = %d, want %d", rec.Code, http.StatusNoContent)
}
if !strings.Contains(rec.Header().Get("Access-Control-Allow-Headers"), "X-Kandev-Interim-Settings-Interlock") {
t.Fatalf("Access-Control-Allow-Headers = %q, want interim settings interlock header", rec.Header().Get("Access-Control-Allow-Headers"))
}
}
func TestCORSMiddlewareAllowsLoopbackAliasOriginForCredentialedRequests(t *testing.T) {
gin.SetMode(gin.TestMode)
router := gin.New()
@@ -0,0 +1,53 @@
package httpmw
import (
"crypto/rand"
"crypto/subtle"
"encoding/base64"
"net/http"
"strings"
"github.com/gin-gonic/gin"
)
// InterimSettingsInterlockHeader carries the replayable, per-boot SPA
// interlock used to reduce accidental settings mutations. It is not an
// authentication credential.
const InterimSettingsInterlockHeader = "X-Kandev-Interim-Settings-Interlock"
// NewInterimSettingsInterlockToken returns a random token for one backend
// boot. Callers must fail closed when token generation fails.
func NewInterimSettingsInterlockToken() (string, error) {
value := make([]byte, 32)
if _, err := rand.Read(value); err != nil {
return "", err
}
return base64.RawURLEncoding.EncodeToString(value), nil
}
// InterimSettingsInterlock rejects agent/settings mutations without the
// current per-boot token and rejects Office bearer credentials outright. The
// token is intentionally replayable by a client that can read the SPA boot
// payload, so this is an interim CSRF/accidental-mutation interlock only.
func InterimSettingsInterlock(token string) gin.HandlerFunc {
return func(c *gin.Context) {
if strings.TrimSpace(token) == "" || hasBearerAuthorization(c.GetHeader("Authorization")) ||
!constantTimeEqual(c.GetHeader(InterimSettingsInterlockHeader), token) {
c.AbortWithStatusJSON(http.StatusForbidden, gin.H{"error": "interim settings interlock required"})
return
}
c.Next()
}
}
func hasBearerAuthorization(value string) bool {
parts := strings.Fields(value)
return len(parts) > 0 && strings.EqualFold(parts[0], "Bearer")
}
func constantTimeEqual(value, expected string) bool {
if value == "" || expected == "" || len(value) != len(expected) {
return false
}
return subtle.ConstantTimeCompare([]byte(value), []byte(expected)) == 1
}
@@ -0,0 +1,87 @@
package httpmw
import (
"net/http"
"net/http/httptest"
"testing"
"github.com/gin-gonic/gin"
)
func TestInterimSettingsInterlockRejectsMissingWrongBearerAndEmptyServerToken(t *testing.T) {
gin.SetMode(gin.TestMode)
tests := []struct {
name string
serverToken string
requestToken string
authorization string
}{
{name: "missing token", serverToken: "per-boot-token"},
{name: "wrong token", serverToken: "per-boot-token", requestToken: "wrong"},
{name: "bearer credential", serverToken: "per-boot-token", requestToken: "per-boot-token", authorization: "Bearer office-jwt"},
{name: "empty server token"},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
called := false
router := gin.New()
router.POST("/settings", InterimSettingsInterlock(tc.serverToken), func(c *gin.Context) {
called = true
c.Status(http.StatusNoContent)
})
request := httptest.NewRequest(http.MethodPost, "/settings", nil)
request.Header.Set(InterimSettingsInterlockHeader, tc.requestToken)
request.Header.Set("Authorization", tc.authorization)
response := httptest.NewRecorder()
router.ServeHTTP(response, request)
if response.Code != http.StatusForbidden {
t.Fatalf("status = %d, want %d", response.Code, http.StatusForbidden)
}
if called {
t.Fatal("handler was called")
}
})
}
}
func TestInterimSettingsInterlockAllowsMatchingTokenWithoutBearerCredential(t *testing.T) {
gin.SetMode(gin.TestMode)
called := false
router := gin.New()
router.POST("/settings", InterimSettingsInterlock("per-boot-token"), func(c *gin.Context) {
called = true
c.Status(http.StatusNoContent)
})
request := httptest.NewRequest(http.MethodPost, "/settings", nil)
request.Header.Set(InterimSettingsInterlockHeader, "per-boot-token")
response := httptest.NewRecorder()
router.ServeHTTP(response, request)
if response.Code != http.StatusNoContent {
t.Fatalf("status = %d, want %d", response.Code, http.StatusNoContent)
}
if !called {
t.Fatal("handler was not called")
}
}
func TestNewInterimSettingsInterlockTokenIsRandomAndNonEmpty(t *testing.T) {
first, err := NewInterimSettingsInterlockToken()
if err != nil {
t.Fatalf("first token: %v", err)
}
second, err := NewInterimSettingsInterlockToken()
if err != nil {
t.Fatalf("second token: %v", err)
}
if first == "" || second == "" {
t.Fatal("token is empty")
}
if first == second {
t.Fatal("generated tokens match")
}
}
+3
View File
@@ -11,6 +11,9 @@ type BootPayload struct {
InitialState map[string]any `json:"initialState"`
RouteData map[string]any `json:"routeData,omitempty"`
Errors []BootError `json:"errors,omitempty"`
// InterimSettingsInterlockToken is a replayable per-boot SPA CSRF and
// accidental-mutation interlock. It is not an authentication credential.
InterimSettingsInterlockToken string `json:"interimSettingsInterlockToken,omitempty"`
// Plugins lists every active, UI-bundle-declaring plugin, per
// docs/plans/plugins/PLUGIN-API.md ("Loading model"). Populated
// only when the plugins feature flag is on; the flag itself is not
+27 -35
View File
@@ -1,6 +1,8 @@
"use server";
"use client";
import { getBackendConfig } from "@/lib/config";
import { fetchJson } from "@/lib/api/client";
import { readInterimSettingsInterlockToken } from "@/src/boot-payload";
import type {
Agent,
AgentProfile,
@@ -17,6 +19,7 @@ import { normalizeAgentProfile } from "@/lib/api/domains/agent-profile-normalize
type ProfilePermissions = Record<PermissionKey, boolean>;
const { apiBaseUrl } = getBackendConfig();
const interimSettingsInterlockHeader = "X-Kandev-Interim-Settings-Interlock";
function normalizeAgentInPlace(agent: Agent): Agent {
return {
@@ -25,7 +28,7 @@ function normalizeAgentInPlace(agent: Agent): Agent {
};
}
async function fetchJson<T>(url: string, options?: RequestInit): Promise<T> {
function agentSettingsRequest<T>(url: string, init?: RequestInit): Promise<T> {
// Pin the URL origin to the configured backend so a tainted path segment
// (agent ID from a form, profile ID from a route param) cannot redirect
// the request to a different host. Closes the CodeQL SSRF finding.
@@ -34,33 +37,15 @@ async function fetchJson<T>(url: string, options?: RequestInit): Promise<T> {
if (parsed.origin !== allowed.origin) {
throw new Error(`Refusing to fetch outside configured backend origin: ${parsed.origin}`);
}
const response = await fetch(parsed.toString(), {
...options,
cache: "no-store",
headers: {
"Content-Type": "application/json",
...(options?.headers ?? {}),
},
});
if (!response.ok) {
throw new Error(`Request failed: ${response.status} ${response.statusText}`);
}
if (response.status === 204) {
return undefined as T;
}
const text = await response.text();
if (!text) {
return undefined as T;
}
return JSON.parse(text) as T;
return fetchJson<T>(parsed.toString(), { cache: "no-store", init });
}
export async function listAgentDiscoveryAction(): Promise<ListAgentDiscoveryResponse> {
return fetchJson<ListAgentDiscoveryResponse>(`${apiBaseUrl}/api/v1/agents/discovery`);
return agentSettingsRequest<ListAgentDiscoveryResponse>(`${apiBaseUrl}/api/v1/agents/discovery`);
}
export async function listAgentsAction(): Promise<ListAgentsResponse> {
const res = await fetchJson<ListAgentsResponse>(`${apiBaseUrl}/api/v1/agents`);
const res = await agentSettingsRequest<ListAgentsResponse>(`${apiBaseUrl}/api/v1/agents`);
return { ...res, agents: (res.agents ?? []).map(normalizeAgentInPlace) };
}
@@ -79,7 +64,7 @@ export async function createAgentAction(payload: {
} & ProfilePermissions
>;
}): Promise<Agent> {
const res = await fetchJson<Agent>(`${apiBaseUrl}/api/v1/agents`, {
const res = await agentSettingsRequest<Agent>(`${apiBaseUrl}/api/v1/agents`, {
method: "POST",
body: JSON.stringify(payload),
});
@@ -94,7 +79,7 @@ export async function updateAgentAction(
mcp_config_path?: string | null;
},
): Promise<Agent> {
const res = await fetchJson<Agent>(`${apiBaseUrl}/api/v1/agents/${id}`, {
const res = await agentSettingsRequest<Agent>(`${apiBaseUrl}/api/v1/agents/${id}`, {
method: "PATCH",
body: JSON.stringify(payload),
});
@@ -102,7 +87,7 @@ export async function updateAgentAction(
}
export async function deleteAgentAction(id: string) {
await fetchJson<void>(`${apiBaseUrl}/api/v1/agents/${id}`, { method: "DELETE" });
await agentSettingsRequest<void>(`${apiBaseUrl}/api/v1/agents/${id}`, { method: "DELETE" });
}
export async function createAgentProfileAction(
@@ -118,10 +103,13 @@ export async function createAgentProfileAction(
env_vars?: ProfileEnvVar[];
} & ProfilePermissions,
): Promise<AgentProfile> {
const raw = await fetchJson<unknown>(`${apiBaseUrl}/api/v1/agents/${agentId}/profiles`, {
method: "POST",
body: JSON.stringify(payload),
});
const raw = await agentSettingsRequest<unknown>(
`${apiBaseUrl}/api/v1/agents/${agentId}/profiles`,
{
method: "POST",
body: JSON.stringify(payload),
},
);
return normalizeAgentProfile(raw);
}
@@ -140,7 +128,7 @@ export async function updateAgentProfileAction(
env_vars?: ProfileEnvVar[];
},
): Promise<AgentProfile> {
const raw = await fetchJson<unknown>(`${apiBaseUrl}/api/v1/agent-profiles/${id}`, {
const raw = await agentSettingsRequest<unknown>(`${apiBaseUrl}/api/v1/agent-profiles/${id}`, {
method: "PATCH",
body: JSON.stringify(payload),
});
@@ -168,10 +156,14 @@ export async function deleteAgentProfileAction(
force?: boolean,
): Promise<DeleteProfileResult> {
const url = `${apiBaseUrl}/api/v1/agent-profiles/${id}${force ? "?force=true" : ""}`;
const token = readInterimSettingsInterlockToken();
const response = await fetch(url, {
method: "DELETE",
cache: "no-store",
headers: { "Content-Type": "application/json" },
headers: {
"Content-Type": "application/json",
...(token ? { [interimSettingsInterlockHeader]: token } : {}),
},
});
if (!response.ok) {
const body = await response.json().catch(() => ({}));
@@ -197,7 +189,7 @@ export async function deleteAgentProfileAction(
export async function getAgentProfileMcpConfigAction(
profileId: string,
): Promise<AgentProfileMcpConfig> {
return fetchJson<AgentProfileMcpConfig>(
return agentSettingsRequest<AgentProfileMcpConfig>(
`${apiBaseUrl}/api/v1/agent-profiles/${profileId}/mcp-config`,
);
}
@@ -210,7 +202,7 @@ export async function updateAgentProfileMcpConfigAction(
meta?: Record<string, unknown>;
},
): Promise<AgentProfileMcpConfig> {
return fetchJson<AgentProfileMcpConfig>(
return agentSettingsRequest<AgentProfileMcpConfig>(
`${apiBaseUrl}/api/v1/agent-profiles/${profileId}/mcp-config`,
{
method: "POST",
@@ -237,7 +229,7 @@ export async function previewAgentCommandAction(
agentName: string,
payload: CommandPreviewRequest,
): Promise<CommandPreviewResponse> {
return fetchJson<CommandPreviewResponse>(
return agentSettingsRequest<CommandPreviewResponse>(
`${apiBaseUrl}/api/v1/agent-command-preview/${agentName}`,
{
method: "POST",
@@ -1,5 +1,5 @@
import { afterEach, describe, expect, it, vi } from "vitest";
import { cleanup, render, screen, waitFor } from "@testing-library/react";
import { act, cleanup, render, screen, waitFor } from "@testing-library/react";
import { CommandPreviewCard } from "./command-preview-card";
import type { CommandPreviewResponse } from "@/app/actions/agents";
@@ -60,10 +60,14 @@ describe("CommandPreviewCard", () => {
await waitFor(() => expect(screen.getByText("agent --new")).toBeTruthy());
// The stale request finally resolves — it must not clobber the newer preview.
resolveStale(response("agent --stale"));
await new Promise((resolve) => setTimeout(resolve, 50));
await act(async () => {
resolveStale(response("agent --stale"));
await stalePromise;
});
expect(screen.getByText("agent --new")).toBeTruthy();
expect(screen.queryByText("agent --stale")).not.toBeTruthy();
await waitFor(() => {
expect(screen.getByText("agent --new")).toBeTruthy();
expect(screen.queryByText("agent --stale")).not.toBeTruthy();
});
});
});
+31 -8
View File
@@ -9,6 +9,7 @@ import type {
MCPTaskAgentProfileDefault,
} from "../../lib/types/http";
import type { Agent, AgentProfile } from "../../lib/types/http-agents";
import { normalizeAgentProfile } from "../../lib/api/domains/agent-profile-normalize";
import type { TaskCIAutomationOptions, TaskCIAutomationPatch } from "../../lib/types/github";
import type { VoiceModeSettings } from "../../lib/types/http-voice";
import type {
@@ -29,6 +30,7 @@ import type {
SSHTestRequest,
SSHTestResult,
} from "../../lib/types/http-ssh";
import { loadInterimSettingsInterlockToken } from "./interim-settings-interlock";
// --- GitHub Mock Types ---
@@ -227,7 +229,7 @@ export class ApiClient {
async rawRequest(method: string, path: string, body?: unknown): Promise<Response> {
return fetch(`${this.baseUrl}${path}`, {
method,
headers: body ? { "Content-Type": "application/json" } : undefined,
headers: await this.requestHeaders(method, body),
body: body ? JSON.stringify(body) : undefined,
});
}
@@ -235,7 +237,7 @@ export class ApiClient {
private async request<T>(method: string, path: string, body?: unknown): Promise<T> {
const res = await fetch(`${this.baseUrl}${path}`, {
method,
headers: body ? { "Content-Type": "application/json" } : undefined,
headers: await this.requestHeaders(method, body),
body: body ? JSON.stringify(body) : undefined,
});
if (!res.ok) {
@@ -245,6 +247,19 @@ export class ApiClient {
return res.json() as Promise<T>;
}
private async requestHeaders(
method: string,
body?: unknown,
): Promise<Record<string, string> | undefined> {
const headers: Record<string, string> = body ? { "Content-Type": "application/json" } : {};
if (["POST", "PUT", "PATCH", "DELETE"].includes(method.toUpperCase())) {
headers["X-Kandev-Interim-Settings-Interlock"] = await loadInterimSettingsInterlockToken(
this.baseUrl,
);
}
return Object.keys(headers).length > 0 ? headers : undefined;
}
private async activeWorkspaceId(): Promise<string | undefined> {
const { settings } = await this.getUserSettings();
const workspaceId = settings.workspace_id;
@@ -384,7 +399,17 @@ export class ApiClient {
}
async listAgents(): Promise<{ agents: Agent[]; total: number }> {
return this.request("GET", "/api/v1/agents");
const response = await this.request<{ agents: Agent[]; total: number }>(
"GET",
"/api/v1/agents",
);
return {
...response,
agents: response.agents.map((agent) => ({
...agent,
profiles: (agent.profiles ?? []).map(normalizeAgentProfile),
})),
};
}
async deleteAgentProfile(profileId: string, force?: boolean): Promise<void> {
@@ -426,11 +451,8 @@ export class ApiClient {
command_prefix?: string;
env_vars?: Array<{ key: string; value?: string; secret_id?: string }>;
},
): Promise<{
id: string;
cli_flags: Array<{ description: string; flag: string; enabled: boolean }>;
}> {
return this.request("POST", `/api/v1/agents/${agentId}/profiles`, {
): Promise<AgentProfile> {
const response = await this.request<unknown>("POST", `/api/v1/agents/${agentId}/profiles`, {
name,
model: opts.model,
mode: opts.mode,
@@ -440,6 +462,7 @@ export class ApiClient {
command_prefix: opts.command_prefix,
env_vars: opts.env_vars,
});
return normalizeAgentProfile(response);
}
async getAgentProfile(profileId: string): Promise<AgentProfile> {
@@ -0,0 +1,16 @@
const appStatePath = "/api/v1/app-state?path=%2Fsettings%2Fagents";
export async function loadInterimSettingsInterlockToken(baseUrl: string): Promise<string> {
const response = await fetch(`${baseUrl}${appStatePath}`);
if (!response.ok) {
throw new Error(`Unable to load E2E settings interlock (${response.status})`);
}
const payload = (await response.json()) as { interimSettingsInterlockToken?: unknown };
if (
typeof payload.interimSettingsInterlockToken !== "string" ||
!payload.interimSettingsInterlockToken
) {
throw new Error("E2E settings interlock token missing from boot payload");
}
return payload.interimSettingsInterlockToken;
}
@@ -12,6 +12,7 @@ import { test, expect } from "../../fixtures/test-base";
*/
test.describe("agent install streaming", () => {
test("POST returns a job_id, WS streams started+output+finished, GET reflects success", async ({
apiClient,
backend,
}) => {
test.setTimeout(15_000);
@@ -29,9 +30,7 @@ test.describe("agent install streaming", () => {
}
});
const enqueueRes = await fetch(`${backend.baseUrl}/api/v1/agent-install/mock-agent`, {
method: "POST",
});
const enqueueRes = await apiClient.rawRequest("POST", "/api/v1/agent-install/mock-agent");
expect(enqueueRes.status).toBe(202);
const job = (await enqueueRes.json()) as { job_id: string; status: string };
expect(job.job_id).toBeTruthy();
@@ -67,7 +66,10 @@ test.describe("agent install streaming", () => {
expect(snap.status).toBe("succeeded");
});
test("POST while a job is running returns the same job_id (idempotent)", async ({ backend }) => {
test("POST while a job is running returns the same job_id (idempotent)", async ({
apiClient,
backend,
}) => {
test.setTimeout(15_000);
// Note: mock-agent's script is fast (~10ms), so the second POST usually
@@ -75,11 +77,11 @@ test.describe("agent install streaming", () => {
// same job_id (still running) OR different job_id with first already in
// retention. The contract under test is: no duplicate concurrent runs.
const first = (await (
await fetch(`${backend.baseUrl}/api/v1/agent-install/mock-agent`, { method: "POST" })
await apiClient.rawRequest("POST", "/api/v1/agent-install/mock-agent")
).json()) as { job_id: string };
const second = (await (
await fetch(`${backend.baseUrl}/api/v1/agent-install/mock-agent`, { method: "POST" })
await apiClient.rawRequest("POST", "/api/v1/agent-install/mock-agent")
).json()) as { job_id: string };
// If both raced before finish they must share IDs; if first already
@@ -92,13 +94,14 @@ test.describe("agent install streaming", () => {
}
});
test("GET /agent-install/jobs lists recent jobs for rehydration", async ({ backend }) => {
test("GET /agent-install/jobs lists recent jobs for rehydration", async ({
apiClient,
backend,
}) => {
test.setTimeout(15_000);
// Trigger an install so there's at least one job in retention.
const enqueueRes = await fetch(`${backend.baseUrl}/api/v1/agent-install/mock-agent`, {
method: "POST",
});
const enqueueRes = await apiClient.rawRequest("POST", "/api/v1/agent-install/mock-agent");
const job = (await enqueueRes.json()) as { job_id: string };
// Wait for it to finish (mock script is fast).
@@ -128,6 +131,7 @@ test.describe("agent install streaming", () => {
});
test("after install probe completes, agent.available.updated is broadcast", async ({
apiClient,
backend,
}) => {
test.setTimeout(20_000);
@@ -145,7 +149,7 @@ test.describe("agent install streaming", () => {
}
});
await fetch(`${backend.baseUrl}/api/v1/agent-install/mock-agent`, { method: "POST" });
await apiClient.rawRequest("POST", "/api/v1/agent-install/mock-agent");
// The controller fires a `Refresh` after install and then broadcasts
// `agent.available.updated`. The mock-agent's host-utility probe may
@@ -76,10 +76,10 @@ test.describe("Agent profile — CLI flags", () => {
);
// Direct DB-path verification: fetch the profile via the API and assert
// the cli_flags JSON contains our entry, enabled.
// the normalized CLI flags contain our entry, enabled.
const stored = await apiClient.getAgentProfile(profile.id);
const found = stored.cli_flags.find((f) => f.flag === flagText);
expect(found, `cli_flags should include ${flagText}`).toBeDefined();
const found = stored.cliFlags.find((f) => f.flag === flagText);
expect(found, `cliFlags should include ${flagText}`).toBeDefined();
expect(found?.enabled).toBe(true);
} finally {
await apiClient.deleteAgentProfile(profile.id, true);
@@ -123,7 +123,7 @@ test.describe("Agent profile — CLI flags", () => {
);
const stored = await apiClient.getAgentProfile(profile.id);
const found = stored.cli_flags.find((f) => f.flag === "--toggle-me");
const found = stored.cliFlags.find((f) => f.flag === "--toggle-me");
expect(found?.enabled).toBe(false);
} finally {
await apiClient.deleteAgentProfile(profile.id, true);
@@ -166,7 +166,7 @@ test.describe("Agent profile — CLI flags", () => {
await expect(testPage.getByTestId("cli-flag-row-1")).toBeHidden();
const stored = await apiClient.getAgentProfile(profile.id);
expect(stored.cli_flags.map((f) => f.flag)).toEqual(["--keep-me"]);
expect(stored.cliFlags.map((f) => f.flag)).toEqual(["--keep-me"]);
} finally {
await apiClient.deleteAgentProfile(profile.id, true);
}
@@ -76,10 +76,9 @@ test.describe("Mobile agent profile config selector", () => {
timeout: 15_000,
});
// Direct DB-path verification: fetch the profile and assert the
// persisted command_prefix.
// API-path verification confirms the normalized persisted value.
const stored = await apiClient.getAgentProfile(profile.id);
expect((stored as unknown as { command_prefix?: string }).command_prefix).toBe("greywall --");
expect(stored.commandPrefix).toBe("greywall --");
// Clearing a previously-saved prefix must persist an empty value.
await testPage.getByTestId("command-prefix-input").fill("");
@@ -95,9 +94,7 @@ test.describe("Mobile agent profile config selector", () => {
timeout: 15_000,
});
const storedAfterClear = await apiClient.getAgentProfile(profile.id);
expect(
(storedAfterClear as unknown as { command_prefix?: string }).command_prefix ?? "",
).toBe("");
expect(storedAfterClear.commandPrefix).toBeUndefined();
} finally {
await apiClient.deleteAgentProfile(profile.id, true);
}
+1 -1
View File
@@ -111,7 +111,7 @@ test.describe("Task creation", () => {
auth_specs: [
{
id: agent.name,
display_name: profile.agent_display_name,
display_name: profile.agentDisplayName,
methods: [{ method_id: "test-token", type: "env", env_var: "TEST_TOKEN" }],
},
],
@@ -27,7 +27,7 @@ test.describe("Workflow agent profile", () => {
const { agents } = await apiClient.listAgents();
const agentProfile = agents.flatMap((a) => a.profiles ?? [])[0];
expect(agentProfile).toBeDefined();
const profileLabel = `${agentProfile.agent_display_name} \u2022 ${agentProfile.name}`;
const profileLabel = `${agentProfile.agentDisplayName} \u2022 ${agentProfile.name}`;
// Open the select and pick the agent profile
await profileSelect.click();
@@ -74,7 +74,7 @@ test.describe("Workflow agent profile", () => {
const { agents } = await apiClient.listAgents();
const agentProfile = agents.flatMap((a) => a.profiles ?? [])[0];
expect(agentProfile).toBeDefined();
const profileLabel = `${agentProfile.agent_display_name} \u2022 ${agentProfile.name}`;
const profileLabel = `${agentProfile.agentDisplayName} \u2022 ${agentProfile.name}`;
// Select an agent profile for this step
await testPage.addStyleTag({
+65
View File
@@ -0,0 +1,65 @@
import { afterEach, describe, expect, it, vi } from "vitest";
import { fetchJson } from "./client";
const interlockToken = "replayable-per-boot-value";
describe("fetchJson", () => {
afterEach(() => {
vi.unstubAllGlobals();
delete (window as unknown as { __KANDEV_BOOT_PAYLOAD__?: unknown }).__KANDEV_BOOT_PAYLOAD__;
});
it("attaches the replayable interim settings interlock to mutations", async () => {
(window as unknown as { __KANDEV_BOOT_PAYLOAD__?: unknown }).__KANDEV_BOOT_PAYLOAD__ = {
interimSettingsInterlockToken: interlockToken,
};
const fetcher = vi.fn().mockResolvedValue(new Response("{}", { status: 200 }));
vi.stubGlobal("fetch", fetcher);
await fetchJson("/api/v1/agents", {
baseUrl: "http://backend.test",
init: { method: "POST", body: "{}" },
});
expect(fetcher).toHaveBeenCalledWith("http://backend.test/api/v1/agents", expect.any(Object));
expect(
new Headers(fetcher.mock.calls[0][1]?.headers).get("X-Kandev-Interim-Settings-Interlock"),
).toBe(interlockToken);
});
it("replaces a lowercase content type without losing mutation headers", async () => {
(window as unknown as { __KANDEV_BOOT_PAYLOAD__?: unknown }).__KANDEV_BOOT_PAYLOAD__ = {
interimSettingsInterlockToken: interlockToken,
};
const fetcher = vi.fn().mockResolvedValue(new Response("{}", { status: 200 }));
vi.stubGlobal("fetch", fetcher);
await fetchJson("/api/v1/agents", {
baseUrl: "http://backend.test",
init: {
method: "POST",
body: "{}",
headers: { "content-type": "text/plain", "X-Caller-Header": "preserved" },
},
});
const headers = fetcher.mock.calls[0][1]?.headers;
const normalizedHeaders = new Headers(headers);
expect({
isHeaders: headers instanceof Headers,
contentType: normalizedHeaders.get("content-type"),
contentTypeCount: [...normalizedHeaders.entries()].filter(
([name]) => name.toLowerCase() === "content-type",
).length,
callerHeader: normalizedHeaders.get("X-Caller-Header"),
interlock: normalizedHeaders.get("X-Kandev-Interim-Settings-Interlock"),
}).toEqual({
isHeaders: true,
contentType: "application/json",
contentTypeCount: 1,
callerHeader: "preserved",
interlock: interlockToken,
});
});
});
+18 -4
View File
@@ -1,4 +1,7 @@
import { getBackendConfig } from "@/lib/config";
import { readInterimSettingsInterlockToken } from "@/src/boot-payload";
const interimSettingsInterlockHeader = "X-Kandev-Interim-Settings-Interlock";
export type ApiRequestOptions = {
baseUrl?: string;
@@ -47,13 +50,16 @@ async function throwFromResponse(response: Response): Promise<never> {
export async function fetchJson<T>(pathOrUrl: string, options?: ApiRequestOptions): Promise<T> {
const baseUrl = options?.baseUrl ?? getBackendConfig().apiBaseUrl;
const url = resolveUrl(pathOrUrl, baseUrl);
const headers = requestHeaders(options?.init?.headers);
headers.set("Content-Type", "application/json");
if (isMutation(options?.init?.method)) {
const token = readInterimSettingsInterlockToken();
if (token) headers.set(interimSettingsInterlockHeader, token);
}
const response = await fetch(url, {
...options?.init,
cache: options?.cache,
headers: {
"Content-Type": "application/json",
...(options?.init?.headers ?? {}),
},
headers,
});
if (!response.ok) await throwFromResponse(response);
if (response.status === 204) return undefined as T;
@@ -62,6 +68,14 @@ export async function fetchJson<T>(pathOrUrl: string, options?: ApiRequestOption
return JSON.parse(text) as T;
}
function isMutation(method: string | undefined): boolean {
return ["POST", "PUT", "PATCH", "DELETE"].includes(method?.toUpperCase() ?? "GET");
}
function requestHeaders(headers: HeadersInit | undefined): Headers {
return new Headers(headers);
}
/**
* Wraps fetchJson with up to `maxAttempts` retries (default 3) and
* exponential backoff starting at `baseDelayMs` (default 100 ms).
+1 -1
View File
@@ -118,7 +118,7 @@ describe("installPluginFromUrl", () => {
expect(JSON.parse(String(init?.body))).toEqual({
url: "https://example.test/acme-tools-1.0.0.tar.gz",
});
expect((init?.headers as Record<string, string>)?.["Content-Type"]).toBe("application/json");
expect(new Headers(init?.headers).get("Content-Type")).toBe("application/json");
expect(result.plugin.id).toBe(PLUGIN_ID);
expect(result.plugin.status).toBe("active");
expect(result.warning).toBeUndefined();
+27
View File
@@ -0,0 +1,27 @@
import { afterEach, describe, expect, it, vi } from "vitest";
import { loadInterimSettingsInterlockToken } from "../../e2e/helpers/interim-settings-interlock";
describe("E2E interim settings interlock", () => {
afterEach(() => {
vi.unstubAllGlobals();
});
it("loads the current boot token after boot rotation", async () => {
let bootToken = "first-boot-token";
vi.stubGlobal(
"fetch",
vi.fn(async (url: string | URL) => {
if (String(url).endsWith("/api/v1/app-state?path=%2Fsettings%2Fagents")) {
return Response.json({ interimSettingsInterlockToken: bootToken });
}
}),
);
const firstToken = await loadInterimSettingsInterlockToken("http://backend.test");
bootToken = "second-boot-token";
const secondToken = await loadInterimSettingsInterlockToken("http://backend.test");
expect([firstToken, secondToken]).toEqual(["first-boot-token", "second-boot-token"]);
});
});
+2
View File
@@ -27,6 +27,7 @@ describe("readBootPayload", () => {
initialState: {
tasks: { activeTaskId: "task-1" },
},
interimSettingsInterlockToken: "replayable-per-boot-value",
},
} as unknown as Window;
@@ -45,6 +46,7 @@ describe("readBootPayload", () => {
initialState: {
tasks: { activeTaskId: "task-1" },
},
interimSettingsInterlockToken: "replayable-per-boot-value",
});
});
+13
View File
@@ -46,6 +46,8 @@ export type BootPayload = {
initialState?: Partial<AppState>;
routeData?: BootRouteData;
plugins?: ActivePlugin[];
/** Replayable per-boot CSRF/accidental-mutation interlock; not authentication. */
interimSettingsInterlockToken?: string;
};
type BootWindow = Window & {
@@ -68,9 +70,15 @@ export function readBootPayload(win: Window = window): BootPayload {
initialState: isRecord(payload.initialState) ? (payload.initialState as Partial<AppState>) : {},
routeData: isRecord(payload.routeData) ? (payload.routeData as BootRouteData) : undefined,
plugins: Array.isArray(payload.plugins) ? readPlugins(payload.plugins) : undefined,
interimSettingsInterlockToken: readNonEmptyString(payload.interimSettingsInterlockToken),
};
}
export function readInterimSettingsInterlockToken(): string | undefined {
if (typeof window === "undefined") return undefined;
return readBootPayload(window).interimSettingsInterlockToken;
}
function readPlugins(value: unknown[]): ActivePlugin[] {
return value.filter(isRecord).flatMap((entry) => {
const plugin = readPlugin(entry);
@@ -133,6 +141,11 @@ function readString(value: unknown): string | undefined {
return typeof value === "string" ? value : undefined;
}
function readNonEmptyString(value: unknown): string | undefined {
const result = readString(value);
return result ? result : undefined;
}
function isRecord(value: unknown): value is Record<string, unknown> {
return Boolean(value) && typeof value === "object" && !Array.isArray(value);
}
@@ -0,0 +1,157 @@
# ADR-2026-07-24-operator-owned-agent-launcher-settings: Operator-Owned Agent Launcher Settings
**Status:** accepted
**Date:** 2026-07-24
**Area:** backend, frontend, protocol
## Context
Agent execution profiles contain inputs that affect the process before an ACP
agent is confined, including the launcher command prefix and the environment
inherited by that launcher. Office agents receive a runtime JWT and the backend
API URL, but the general agent-profile mutation routes currently require no
operator authentication; omitting the JWT is treated as an administrative UI
request. Kandev also has no application-level operator session or secure
browser bootstrap: the current web UI, HTTP API, and WebSocket are intentionally
unauthenticated, while the desktop health token and agentctl tokens authenticate
different process boundaries.
A per-boot value exposed through the SPA shell or `/api/v1/app-state` can
mitigate browser CSRF, but it is not operator authentication. Any agent or
direct client that can reach the backend can read and replay that value. This
limitation is also documented by the unfinished plugin boot-token work in
[PR #1808](https://github.com/kdlbs/kandev/pull/1808).
## Decision
Launcher-affecting definitions and agent-profile settings are operator
control-plane data. Agent processes and Office runtime JWTs are data-plane
principals and MUST NOT authorize their mutation. The protected resources and
required authority are:
| Resource | Examples | Required authority |
| --- | --- | --- |
| Install-wide agent definitions | custom TUI commands in `agents.tui_config`, agent creation/deletion, registry-backed executable selection | install operator |
| Global execution profiles | `agent_profiles` rows with an empty `workspace_id`, including nested MCP configuration | install operator |
| Workspace execution profiles | `agent_profiles` rows with a non-empty `workspace_id`, including nested MCP configuration | operator authorized for that workspace |
Workspace operators cannot mutate shared global definitions or profiles.
Creating a resource, deleting it, replacing it, or moving it between ownership
scopes requires the authority for both its old and new scope.
Within those resources, the boundary includes at least:
- launcher executable and command-prefix argv;
- CLI arguments and mode/configuration that select the launched program's
behavior;
- profile environment values or secret references inherited by the launcher;
- profile creation, deletion, and any replacement operation that can select
different launcher inputs.
These mutations require a mandatory authenticated operator principal. An
absent credential is unauthenticated, not an implicit administrator. An Office
agent JWT is rejected even when its role is `ceo` or otherwise administrative
inside the Office data plane. Authorization also validates that the operator
can mutate the target workspace/profile when Kandev supports more than one
operator or tenant.
Full-detail reads of the protected resources require the same operator
authority when they include literal environment values, resolved secret
material, MCP server environment, or MCP headers. Agent/data-plane callers may
receive a separate redacted catalog DTO containing identifiers, names,
compatibility/capability metadata, and secret-reference presence, but never the
literal values used by another execution profile.
The operator credential and its bootstrap material MUST NOT be placed in agent
process environments, workspaces, executor metadata, logs, Office runtime
responses, or an unauthenticated SPA/boot payload. A same-origin boot token may
still be used as a CSRF control, but it never satisfies this authentication
requirement. Deployments may satisfy the principal requirement with native
Kandev application authentication or with a trusted external identity proxy,
provided direct backend access is blocked and Kandev verifies a non-forgeable
proxy assertion.
Ambient browser credentials are safe only after agent-controlled and
workspace-controlled web content is isolated from the operator origin. Preview
ports, task applications, plugin content, and other untrusted scripts MUST use
a separate origin that cannot read operator boot/session state or send an
operator credential to the control-plane origin. Cookie path scoping,
iframe sandbox flags combined with `allow-same-origin`, and CSRF headers do not
replace this origin boundary.
Until that operator-authentication surface exists, command prefixes are a
launch customization feature, not a security boundary. A partial profile-route
guard based on optional Office JWT middleware, `Origin`/CORS, client IP,
user-agent headers, or a secret returned by an unauthenticated endpoint MUST
NOT be described or tested as operator authentication.
As an interim risk reduction, state-changing agent/settings routes may require
a random per-boot token exposed to the same-origin SPA and reject requests that
carry an Office bearer token. This interlock reduces accidental agent writes,
straightforward use of the injected runtime JWT, and cross-origin request
forgery. It does not stop an intentional agent or direct client from fetching
the unauthenticated boot payload and replaying the token, and therefore does not
satisfy the operator principal, secret-read, workspace-authorization, or
untrusted-origin requirements above. Tests and public documentation for the
interlock must preserve that distinction.
Execution-critical commands cross lifecycle and agentctl boundaries as
structured argv. The additive configure fields are `agent_args []string` and
`continue_args []string`; the existing `command` and `continue_command` fields
remain display/legacy compatibility strings. During migration, senders
dual-write both representations. Receivers prefer structured argv whenever its
field is present, reject present-but-empty or otherwise invalid argv without
falling back, and parse the legacy string only when the corresponding argv
field is absent. The same precedence applies to initial launch,
continue/one-shot execution, restart/context reset, and recovered executions.
Human-readable command strings are never reparsed when structured argv is
available. Legacy fields may be removed only after all supported lifecycle and
agentctl version combinations send and consume structured argv.
## Consequences
- Correctly hardening agent-profile mutation depends on a broader
application-authentication and browser-session design. That work must cover
native launcher, desktop WebView, ordinary browser, headless service, and
mobile access without putting the credential in agent-visible surfaces. It
must also move untrusted preview/content rendering off the operator origin.
- The post-merge launcher hardening cannot claim a security boundary until
that dependency lands. Structured argv, restart ordering, and rollback tests
remain valid follow-up work, but shipping them alone does not satisfy the
launcher/settings acceptance criteria.
- The interim boot-token interlock is intentionally compatible with later
operator authentication: it remains a CSRF layer or is removed once the
authenticated session supplies a stronger principal.
- Existing settings clients will need an authenticated mutation transport and
explicit handling for unauthenticated/forbidden responses. Read-only
agent-facing discovery requires a dedicated redacted DTO; existing
full-detail profile and MCP responses remain operator-only.
- Profile environment values may continue to be applied to the launcher only
after operator-only mutation is enforced. If agents later gain a supported
way to propose child environment values, those values require a separate
post-confinement channel or an explicit sanitization contract.
- External-proxy deployments must prevent bypassing the proxy by connecting
directly to the backend.
## Alternatives Considered
1. **Apply the current optional `AgentAuthMiddleware`.** Rejected because a
caller can omit its bearer token and is then treated as an administrative
UI request.
2. **Expose a random per-boot token in the SPA shell or boot payload.** Rejected
as authentication because an agent can fetch the same unauthenticated
resource and replay the token. Accepted only as the explicitly limited
interim interlock and future CSRF protection described above.
3. **Protect only `command_prefix`.** Rejected because profile environment
values can influence the wrapper through `LD_PRELOAD`, `PATH`,
wrapper-specific variables, and interpreter resolution before confinement.
4. **Rely on CORS, `Origin`, loopback binding, source IP, or browser headers.**
Rejected because non-browser agent clients can omit or forge those signals,
and local executors may share the host network namespace.
5. **Move launcher policy to another unauthenticated HTTP route or table.**
Rejected because changing storage ownership without authenticating the
mutation principal preserves the same confused-deputy boundary.
6. **Use an ambient operator cookie while serving previews on the backend
origin.** Rejected because agent-controlled same-origin scripts can exercise
the credential and read CSRF/bootstrap material. Authentication and
untrusted-content origin isolation must land together.
+1
View File
@@ -59,6 +59,7 @@ Read individual ADRs for full context. Create new ones via `/record decision` or
| 2026-07-23-opencode-review-evidence-trust | [Trusted OpenCode Review Evidence](2026-07-23-opencode-review-evidence-trust.md) | accepted | workflow, infra | 2026-07-23 |
| 2026-07-23-planner-direct-small-work | [Planner Direct Small Work](2026-07-23-planner-direct-small-work.md) | accepted | workflow | 2026-07-23 |
| 2026-07-23-post-commit-hook-aware-verification | [Post-Commit Hook-Aware Verification](2026-07-23-post-commit-hook-aware-verification.md) | accepted | workflow | 2026-07-23 |
| 2026-07-24-operator-owned-agent-launcher-settings | [Operator-Owned Agent Launcher Settings](2026-07-24-operator-owned-agent-launcher-settings.md) | accepted | backend, frontend, protocol | 2026-07-24 |
| 2026-07-18-turn-configuration-snapshots | [Attribute runtime configuration to turns](2026-07-18-turn-configuration-snapshots.md) | accepted | backend, frontend | 2026-07-18 |
| 2026-07-22-gpg-signed-release-tags | [GPG-signed release tags](2026-07-22-gpg-signed-release-tags.md) | accepted | infra, workflow | 2026-07-22 |
| 2026-07-19-reject-mcp-actions-on-raw-websocket | [Reject MCP Actions on the Raw WebSocket](2026-07-19-reject-mcp-actions-on-raw-websocket.md) | accepted | backend, protocol | 2026-07-19 |
@@ -0,0 +1,140 @@
---
spec: docs/specs/office/agents.md
created: 2026-07-24
status: done
---
# Implementation Plan: Agent Launcher Hardening Follow-ups
## Overview
Deliver an explicitly limited per-boot settings interlock while preserving the
long-term operator boundary in
`docs/decisions/2026-07-24-operator-owned-agent-launcher-settings.md`. In
parallel, remove lossy command-string execution, then make restart and launch
rollback ordering fail safely. Finish with the verified review cleanups,
mobile settings coverage, and public documentation that accurately distinguishes
risk reduction from authentication.
## Backend
### Interim settings interlock
- Add a process-random boot token and constant-time validation middleware under
`apps/backend/internal/common/httpmw/`.
- Generate the token once in `apps/backend/internal/backendapp/main.go`, pass it
through `routeParams`, and expose it only as the existing unauthenticated SPA
runtime payload.
- Apply the interlock to state-changing agent/settings routes in
`apps/backend/internal/agent/settings/handlers/handlers.go`, including agent
creation/update/delete, custom TUI creation, profile creation/update/delete,
MCP configuration mutation, and agent installation.
- Reject requests carrying a bearer credential on those routes so the injected
Office runtime JWT is never accepted as the UI interlock. Missing/wrong boot
tokens fail closed.
### Structured command transport
- Extend lifecycle execution state and the lifecycle-to-agentctl configure
request with `agent_args` and `continue_args`.
- Dual-write structured argv and legacy display strings. Agentctl prefers a
present structured field, rejects present-but-invalid argv, and uses
`strings.Fields` only when the structured field is absent.
- Preserve argv through initial launch, continue/one-shot execution,
restart/context reset, standalone, Docker/remote configuration, and recovery.
- Validate argv through the shared executable-token rules: non-empty argv and a
non-empty, non-flag-like `argv[0]`.
### Restart and rollback safety
- Build and validate replacement initial/continue argv before closing streams,
stopping the existing process, changing execution state, or configuring
agentctl.
- When execution construction fails after an executor instance exists, stop the
instance exactly once and never register/configure/start the failed execution.
## Frontend
### Interlock transport
- Add the boot token to the typed runtime payload in
`apps/web/src/boot-payload.ts`.
- Centralize the mutation header builder in the web API layer and apply it to
every state-changing agent/profile action, preserving existing desktop and
mobile save flows without layout changes.
### Review cleanups
- Replace the fixed timeout in `command-preview-card.test.tsx` with controlled
promises and `waitFor`.
- Normalize E2E `AgentProfile.commandPrefix` typing and remove wire-shape casts
from the mobile settings test.
Mobile design contract: the existing mobile agent-profile route, fields, save
action, scroll owner, and touch targets remain unchanged. Only the request
transport and test typing change, so the existing
`mobile-agent-profile-config-selector.spec.ts` remains the mobile parity proof.
## Tests
- Interlock middleware: missing, wrong, valid, empty-server-token, and bearer
rejection.
- Settings routes: Office bearer denied; omitted token denied; valid UI token
succeeds; guarded operations make zero writes on rejection.
- Structured argv: spaces, quotes, empty non-executable arguments,
Windows/backslash paths, prefix arguments, CLI flag values, initial,
continue, reset, JSON compatibility, and legacy absence fallback.
- Restart: resolver failure and malformed persisted prefix cause no
Stop/Configure/Start call and no state mutation.
- Launch rollback: instance is stopped once, absent from execution store, and
never configured/started after construction failure.
- Preview validation: empty/flag-like prefix executable rejected using the same
semantics as save/launch.
- Profile creation validation: bad prefixes produce zero store/profile writes.
## E2E Tests
- Rebuild backend and Vite production artifacts.
- Run the existing mobile agent-profile settings test through create, set
prefix, save, clear, and save again. The browser must carry the boot token
automatically; direct E2E API setup helpers receive the token through their
controlled test bootstrap where mutations are required.
## Implementation Waves
Wave 1 (parallel):
- [x] [task-01-interim-settings-interlock](task-01-interim-settings-interlock.md)
- [x] [task-02-structured-argv-transport](task-02-structured-argv-transport.md)
Wave 2:
- [x] [task-03-restart-and-launch-rollback](task-03-restart-and-launch-rollback.md)
Wave 3:
- [x] [task-04-review-cleanups-e2e-docs](task-04-review-cleanups-e2e-docs.md)
## Risks
- The boot token is deliberately replayable by an intentional agent and must
never be described as operator authentication.
- Structured-field presence must be distinguishable from an absent JSON field;
a present invalid slice must fail instead of silently falling back.
- Lifecycle task ownership overlaps between tasks 02 and 03, so they run in
separate waves.
- E2E helpers that mutate settings must use the same guarded transport without
weakening production middleware.
## Verification
Targeted commands are owned by each task. After all tasks:
```bash
make fmt
make typecheck test lint
cd apps/web && pnpm e2e:run tests/settings/mobile-agent-profile-config-selector.spec.ts
```
Commit through active hooks, run delegated full `/verify`, then push and open a
PR against `main`.
@@ -0,0 +1,55 @@
---
id: "01-interim-settings-interlock"
title: "Interim settings interlock"
status: done
wave: 1
depends_on: []
plan: "plan.md"
spec: "../../specs/office/agents.md"
---
# Task 01: Interim settings interlock
## Acceptance
- State-changing agent/settings routes reject a missing/wrong boot token and
any bearer credential before handler side effects.
- Legitimate SPA requests attach the per-boot token and existing desktop/mobile
settings saves continue to work.
- Tests and naming describe the control as a replayable interim interlock/CSRF
token, never operator authentication.
## Verification
```bash
cd apps/backend && go test ./internal/common/httpmw ./internal/agent/settings/handlers ./internal/backendapp
cd apps && pnpm --filter @kandev/web test -- --run src/boot-payload.test.ts lib/api
```
## Files likely touched
- `apps/backend/internal/common/httpmw/`
- `apps/backend/internal/backendapp/main.go`
- `apps/backend/internal/backendapp/helpers.go`
- `apps/backend/internal/agent/settings/handlers/handlers.go`
- `apps/backend/internal/agent/settings/handlers/*_test.go`
- `apps/backend/internal/webapp/payload.go`
- `apps/web/src/boot-payload.ts`
- `apps/web/src/boot-payload.test.ts`
- `apps/web/lib/api/client.ts`
- `apps/web/app/actions/agents.ts`
- focused API/action tests
- this task file
## Inputs
- ADR sections “Decision” and “Consequences”.
- Existing CORS middleware and SPA boot-payload hydration.
- Existing agent settings route registration and action clients.
## Output contract
Return a compact handoff capsule with intent/acceptance, base/head SHA, changed
files and entry points, ADR sections, risk tags, exact RED/GREEN verification
commands/results, uncertainties, and this task status set to `done`. Do not edit
`plan.md`.
@@ -0,0 +1,54 @@
---
id: "02-structured-argv-transport"
title: "Structured argv transport"
status: done
wave: 1
depends_on: []
plan: "plan.md"
spec: "../../specs/office/agents.md"
---
# Task 02: Structured argv transport
## Acceptance
- Lifecycle stores and sends structured initial/continue argv end to end;
agentctl executes it without reparsing display strings.
- `agent_args`/`continue_args` take strict precedence when present, while legacy
`command`/`continue_command` remain absence-only compatibility fallbacks.
- Tests cover spaces, quotes, empty arguments, Windows paths/backslashes,
prefix/CLI flags, initial, continue, reset/recovery, serialization, and
legacy clients.
## Verification
```bash
cd apps/backend && go test ./internal/agent/runtime/agentctl ./internal/agent/runtime/lifecycle ./internal/agentctl/server/api ./internal/agentctl/server/config ./internal/agentctl/server/process
```
## Files likely touched
- `apps/backend/internal/agent/runtime/lifecycle/command.go`
- `apps/backend/internal/agent/runtime/lifecycle/types.go`
- `apps/backend/internal/agent/runtime/lifecycle/manager_launch.go`
- `apps/backend/internal/agent/runtime/lifecycle/manager_startup.go`
- `apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go`
- `apps/backend/internal/agent/runtime/agentctl/client.go`
- `apps/backend/internal/agentctl/server/config/config.go`
- `apps/backend/internal/agentctl/server/api/agent.go`
- `apps/backend/internal/agentctl/server/process/manager.go`
- focused tests beside those files
- this task file
## Inputs
- ADR structured-argv paragraph.
- Existing `agents.Command` token slices and lifecycle display-string builders.
- Current agentctl configure JSON and `config.ParseCommand` fallback.
## Output contract
Return a compact handoff capsule with intent/acceptance, base/head SHA, changed
files and entry points, ADR sections, risk tags, exact RED/GREEN verification
commands/results, uncertainties, and this task status set to `done`. Do not edit
`plan.md`.
@@ -0,0 +1,48 @@
---
id: "03-restart-and-launch-rollback"
title: "Restart and launch rollback safety"
status: done
wave: 2
depends_on: ["02-structured-argv-transport"]
plan: "plan.md"
spec: "../../specs/office/agents.md"
---
# Task 03: Restart and launch rollback safety
## Acceptance
- Replacement commands are fully resolved and validated before the existing
process, streams, execution state, or agentctl configuration changes.
- Resolver or persisted-prefix failure produces no Stop/Configure/Start calls
and leaves execution state unchanged.
- Construction failure after instance creation stops the instance exactly once,
never registers the execution, and never configures/starts agentctl.
## Verification
```bash
cd apps/backend && go test -run 'TestManager_(RestartAgentProcess|Launch|BuildExecution)' ./internal/agent/runtime/lifecycle
```
## Files likely touched
- `apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go`
- `apps/backend/internal/agent/runtime/lifecycle/manager_interaction_test.go`
- `apps/backend/internal/agent/runtime/lifecycle/manager_launch.go`
- `apps/backend/internal/agent/runtime/lifecycle/manager_launch_test.go`
- lifecycle deterministic fakes/helpers
- this task file
## Inputs
- Completed task 02 structured execution state.
- `RestartAgentProcess`, `buildFreshAgentCommand`, launch cleanup and execution
store registration ordering.
- Review thread `discussion_r3641362228`.
## Output contract
Return a compact handoff capsule with intent/acceptance, base/head SHA, changed
files and entry points, risk tags, exact RED/GREEN verification commands/results,
uncertainties, and this task status set to `done`. Do not edit `plan.md`.
@@ -0,0 +1,64 @@
---
id: "04-review-cleanups-e2e-docs"
title: "Review cleanups, E2E, and docs"
status: done
wave: 3
depends_on:
- "01-interim-settings-interlock"
- "03-restart-and-launch-rollback"
plan: "plan.md"
spec: "../../specs/office/agents.md"
---
# Task 04: Review cleanups, E2E, and docs
## Acceptance
- Preview/save/launch prefix validation is identical; bad create requests make
zero writes; the stale ownership comment and E2E typing casts are removed.
- Frontend async tests use controlled promises/`act`/`waitFor`, with no fixed
sleep.
- The rebuilt mobile settings E2E passes, and public docs accurately describe
ACP-only scope, quoting/argv preservation, fail-closed runtime behavior, and
the replayable interim interlock versus the future operator boundary.
## Verification
```bash
cd apps/backend && go test ./internal/agent/settings/controller
cd apps && pnpm --filter @kandev/web test -- --run \
'app/settings/agents/[agentId]/profiles/[profileId]/command-preview-card.test.tsx' \
'lib/api/domains/agent-profile-normalize.test.ts'
cd apps/web && pnpm e2e:run tests/settings/mobile-agent-profile-config-selector.spec.ts
node --test scripts/validate-public-docs.test.mjs
node scripts/validate-public-docs.mjs
```
## Files likely touched
- `apps/backend/internal/agent/settings/controller/agent_config.go`
- `apps/backend/internal/agent/settings/controller/controller_test.go`
- `apps/backend/internal/agent/settings/controller/agent_crud.go`
- `apps/backend/internal/agent/settings/controller/profile_crud_test.go`
- `apps/backend/internal/agent/agents/agent.go`
- `apps/web/app/settings/agents/[agentId]/profiles/[profileId]/command-preview-card.test.tsx`
- `apps/web/e2e/helpers/api-client.ts`
- `apps/web/e2e/tests/settings/mobile-agent-profile-config-selector.spec.ts`
- `apps/web/lib/types/agent-profile.ts`
- `docs/public/agents-and-profiles.md`
- `docs/public/security.md`
- `docs/specs/office/agents.md`
- this task file
## Inputs
- Completed tasks 01 and 03.
- Review threads named in the parent task.
- `/mobile-parity`, `/e2e`, and `/docs-maintainer`.
## Output contract
Return a compact handoff capsule with intent/acceptance, base/head SHA, changed
files and entry points, risk tags, exact RED/GREEN/E2E/docs verification
commands/results, uncertainties, and this task status set to `done`. Do not edit
`plan.md`.
+9 -2
View File
@@ -38,7 +38,8 @@ Select an agent, create a profile, then open **Settings > Agents > _Agent_ > _Pr
| Model | Requested through ACP when the agent supports model selection. Leaving it unset uses the agent's default where the form allows that. |
| Mode | Requested with ACP `session/set_mode`. The choices come from the installed agent. |
| Configuration options | Dynamic ACP values requested with `session/set_config_option`. |
| CLI flags | Enabled entries are tokenized and appended to the launch command. |
| CLI flags | Enabled entries are tokenized and appended to the ACP launch command. |
| Command prefix | Optional ACP-only launcher argv prepended to the command, for example `greywall --`. |
| Environment | Literal values or references to Kandev secrets, resolved when the process starts. |
| CLI passthrough | Uses the CLI's native terminal interface instead of a structured ACP conversation. |
| Auto-approve all permissions | Answers automatically: the first `allow_once`/`allow_always` option, otherwise the first option supplied by the agent; no options cancels. It is off by default. |
@@ -60,6 +61,12 @@ The field is not a shell script. Pipes, redirects, variable expansion, and comma
Some older profiles contain compatibility fields such as Auggie's `allow_indexing`; current launch behavior is represented by the active profile settings and flags.
### ACP command prefixes
**Command prefix** is available for ACP launches, not terminal-passthrough launches. Kandev parses the prefix into structured argv rather than running a shell: quote a path containing spaces, for example `"/opt/launchers/safe wrapper" --`. The resulting argv is prefixed to the agent command, so shell features such as pipes, redirects, variable expansion, and command substitution are not evaluated.
The prefix must contain a nonempty first argv element that is not flag-like. Malformed quotes, a trailing escape, an empty launcher, or a prefix beginning with `-` is rejected when you save or preview it. If an older persisted profile contains an invalid prefix, Kandev fails the launch rather than silently running the agent without its configured launcher. Check the command preview after changing a prefix.
## Environment variables and secrets
Create reusable secrets at **Settings > Secrets** (`/settings/general/secrets`), then select a secret reference in a profile environment entry. Secret names are 1–100 characters and values are 1–10,000 characters. Editing a secret with a blank value keeps the saved value.
@@ -89,7 +96,7 @@ Workspace automation selectors do not offer passthrough agent profiles or Local
## Structured ACP and terminal passthrough
ACP sessions can expose typed messages, tool updates, permission requests, models, modes, dynamic configuration, todos, usage, and resume metadata. Each capability depends on the agent's actual ACP implementation.
ACP sessions can expose typed messages, tool updates, permission requests, models, modes, dynamic configuration, todos, usage, and resume metadata. Each capability depends on the agent's actual ACP implementation. ACP-only profile settings, including command prefixes and structured configuration, do not add those capabilities to a terminal-passthrough CLI.
Passthrough preserves the CLI's native PTY interface. It is useful when the native terminal has features that ACP does not expose, but Kandev cannot manufacture structured capabilities that are absent. Custom TUI profiles are locked to passthrough. Profile-specific MCP injection also varies by CLI; verify the command preview and the MCP section before depending on it.
+13
View File
@@ -51,6 +51,19 @@ Use separate profiles for different trust levels. Do not give a routine document
An agent profile combines a CLI, model, mode, flags, environment values, secret references, permissions, and optional MCP servers. Treat it as a reusable authority package.
The configurable ACP command prefix is currently a launch-customization
feature, not an isolation boundary. Kandev does not yet have a separate
authenticated operator session for profile mutations, so an agent that can
reach the main HTTP API may attempt the same profile edits as the UI. Do not
rely on the prefix to confine a hostile agent until the operator-owned settings
boundary in
[ADR-2026-07-24-operator-owned-agent-launcher-settings](../decisions/2026-07-24-operator-owned-agent-launcher-settings.md)
is enforced. The current per-boot token is a deliberately replayable interim
CSRF and accidental-mutation interlock, not authentication: an intentional
agent can fetch and replay it. It does not close this direct-client path, and
an ambient browser login is insufficient while agent-controlled previews can
share the operator origin.
1. Create a profile for one purpose, such as local implementation, read-focused review, or unattended maintenance.
2. Select the least-privileged Git, provider, and cloud credentials that purpose needs.
3. Leave approval or sandbox bypasses disabled unless the executor is disposable and the task is trusted.
+47
View File
@@ -171,6 +171,30 @@ Service-layer permission checks run on every mutating endpoint. Task scope is en
When a CEO calls `POST /office/agents`: must have `can_create_agents`; must specify `role` (defaults applied automatically); may pass `permissions` overrides, but cannot grant permissions it doesn't have itself (no privilege escalation).
### Required operator boundary (not yet implemented)
The no-JWT UI convention must not apply to execution-profile mutations.
Launcher definitions, executable/prefix argv, CLI configuration, and
environment values are operator control-plane settings. Once operator
authentication is implemented, creating, updating, or deleting those resources
requires an authenticated operator principal; an omitted credential and an
Office agent JWT are both rejected. Operator credentials are never included in
Office runtime environment, workspace files, executor metadata, logs, or
unauthenticated boot/runtime APIs. Full-detail profile and MCP reads containing
literal environment values, headers, or resolved secrets are operator-only;
agent-facing discovery uses a redacted catalog shape. Agent/workspace preview
content runs on an origin that cannot exercise ambient operator credentials or
read operator session/bootstrap state. Until these requirements are enforced,
launcher prefixes are customization rather than an isolation boundary.
Decision:
[ADR-2026-07-24-operator-owned-agent-launcher-settings](../../decisions/2026-07-24-operator-owned-agent-launcher-settings.md).
The interim risk-reduction guard requires a per-boot SPA token on
state-changing agent/settings requests and rejects Office bearer tokens. Because
an intentional agent can fetch and replay the unauthenticated boot payload,
this guard is a CSRF and accidental-mutation interlock only; it does not satisfy
the operator-boundary scenarios below.
### Hire flow
When the CEO (or any instance with `can_create_agents`) creates a new agent instance, a hire request is submitted:
@@ -567,6 +591,29 @@ There are no TTLs on agent rows, runtime rows, instructions, skills, run history
- **GIVEN** an agent run scoped to task `KAN-1`, **WHEN** the agent tries to update `KAN-2` without explicit scope, **THEN** the runtime denies the action and no task mutation is attempted.
The following operator-boundary scenarios are acceptance criteria that must
pass before launcher settings are described as an isolation boundary:
- **GIVEN** a running Office agent that can reach the backend, **WHEN** it
submits an execution-profile mutation with its runtime JWT or without a
credential, **THEN** the backend rejects the request and persists no launcher
or environment change.
- **GIVEN** an authenticated operator authorized for the target profile,
**WHEN** the settings UI saves an execution-profile change, **THEN** the
backend persists the change without exposing the operator credential to any
agent runtime surface.
- **GIVEN** an Office agent requests execution-profile discovery, **WHEN** the
backend returns the agent-facing catalog, **THEN** the response contains no
literal profile environment values, MCP environment values, MCP headers, or
resolved secret material.
- **GIVEN** agent-controlled JavaScript is rendered in a task preview, **WHEN**
it attempts to use the operator session, **THEN** browser origin isolation
prevents it from reading operator state or issuing an authenticated
control-plane mutation.
- **GIVEN** an agent run with `create_subtask` capability and mutation scope for a parent task, **WHEN** it creates a subtask under that parent, **THEN** Office creates the task through the runtime action surface and preserves the caller agent identity.
- **GIVEN** an agent run without `create_subtask` capability or without mutation scope for the requested parent, **WHEN** it attempts parented task creation, **THEN** Office rejects the request before reading parent relationships or creating any task.