remove trustedTools and Confirm callback; trust is implicit via allowExecutors
This commit is contained in:
parent
93c0b374d5
commit
1207e26a07
|
|
@ -99,13 +99,6 @@ func BuildAgentEnv(cfg *config.Config, d tools.Dispatcher, cwd string) AgentEnv
|
|||
Verbosity: cfg.Verbosity,
|
||||
}
|
||||
maxSteps = cfg.MaxSteps
|
||||
if len(cfg.TrustedTools) > 0 {
|
||||
if srv, ok := d.GetServer("execute"); ok {
|
||||
if ts, ok := srv.(tools.TrustedToolsSetter); ok {
|
||||
ts.SetTrustedTools(cfg.TrustedTools)
|
||||
}
|
||||
}
|
||||
}
|
||||
if len(cfg.AllowExecutors) > 0 || len(cfg.AllowTools) > 0 {
|
||||
if srv, ok := d.GetServer("execute"); ok {
|
||||
if rs, ok := srv.(tools.ToolRestrictionSetter); ok {
|
||||
|
|
|
|||
|
|
@ -2315,19 +2315,6 @@ func (m *mockDispatcher) Dispatch(ctx context.Context, server, tool string, args
|
|||
return nil, fmt.Errorf("not implemented")
|
||||
}
|
||||
|
||||
type mockTrustedSetter struct {
|
||||
tools.Server
|
||||
trusted []string
|
||||
}
|
||||
|
||||
func (m *mockTrustedSetter) SetTrustedTools(t []string) { m.trusted = t }
|
||||
func (m *mockTrustedSetter) ListTools() ([]tools.ToolInfo, error) {
|
||||
return nil, nil
|
||||
}
|
||||
func (m *mockTrustedSetter) CallTool(ctx context.Context, tool string, args json.RawMessage) (json.RawMessage, error) {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func TestBuildAgentEnv_ListToolsError(t *testing.T) {
|
||||
setupCfgDir(t)
|
||||
d := &mockDispatcher{listErr: fmt.Errorf("boom")}
|
||||
|
|
@ -2361,17 +2348,6 @@ func TestBuildAgentEnv_ToolsDisabled(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestBuildAgentEnv_TrustedTools(t *testing.T) {
|
||||
setupCfgDir(t)
|
||||
setter := &mockTrustedSetter{}
|
||||
d := &mockDispatcher{servers: map[string]tools.Server{"execute": setter}}
|
||||
cfg := &config.Config{TrustedTools: []string{"bash"}}
|
||||
BuildAgentEnv(cfg, d, t.TempDir())
|
||||
if len(setter.trusted) != 1 || setter.trusted[0] != "bash" {
|
||||
t.Errorf("trusted = %v", setter.trusted)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildAgentEnv_PromptOnly(t *testing.T) {
|
||||
setupCfgDir(t)
|
||||
d := tools.NewDispatcher()
|
||||
|
|
|
|||
|
|
@ -64,7 +64,6 @@ type Config struct {
|
|||
Backend string `json:"backend,omitempty"`
|
||||
Model string `json:"model,omitempty"`
|
||||
Tools *bool `json:"tools,omitempty"`
|
||||
TrustedTools []string `json:"trustedTools,omitempty"`
|
||||
AllowExecutors []string `json:"allowExecutors,omitempty"`
|
||||
AllowTools []string `json:"allowTools,omitempty"`
|
||||
MaxTokens int `json:"maxTokens,omitempty"`
|
||||
|
|
|
|||
|
|
@ -93,7 +93,6 @@ func TestPromptInvalid(t *testing.T) {
|
|||
func TestLoadAllFields(t *testing.T) {
|
||||
cfg, err := Load(strings.NewReader(`{
|
||||
"prompt": "be helpful",
|
||||
"trustedTools": ["execute_code"],
|
||||
"maxTokens": 4096,
|
||||
"temperature": 0.7,
|
||||
"frequencyPenalty": 0.5,
|
||||
|
|
@ -105,9 +104,6 @@ func TestLoadAllFields(t *testing.T) {
|
|||
if cfg.Prompt.IsExec || len(cfg.Prompt.Value) != 1 || cfg.Prompt.Value[0] != "be helpful" {
|
||||
t.Errorf("Prompt = %v", cfg.Prompt)
|
||||
}
|
||||
if len(cfg.TrustedTools) != 1 || cfg.TrustedTools[0] != "execute_code" {
|
||||
t.Errorf("TrustedTools = %v", cfg.TrustedTools)
|
||||
}
|
||||
if cfg.MaxTokens != 4096 {
|
||||
t.Errorf("MaxTokens = %d", cfg.MaxTokens)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -248,11 +248,6 @@ func (e *Server) dispatchSteps(ctx context.Context, steps []CodeStep, timeout in
|
|||
}
|
||||
}
|
||||
|
||||
label := buildExecLabel(steps)
|
||||
if !e.allowed("execute_code", label) {
|
||||
return "", fmt.Errorf("execute_code: denied by user")
|
||||
}
|
||||
|
||||
// Single step: run directly.
|
||||
if len(steps) == 1 && len(steps[0].Parallel) == 0 {
|
||||
if steps[0].Elevated {
|
||||
|
|
@ -324,11 +319,6 @@ func (e *Server) dispatchPipe(ctx context.Context, pipe []CodeStep, timeout int,
|
|||
}
|
||||
}
|
||||
|
||||
label := buildExecLabel(pipe)
|
||||
if !e.allowed("execute_code", label) {
|
||||
return "", fmt.Errorf("execute_code: denied by user")
|
||||
}
|
||||
|
||||
var input string
|
||||
for i, stage := range pipe {
|
||||
out, err := e.runStage(ctx, i, stage, timeout, sandboxName, input)
|
||||
|
|
@ -340,31 +330,6 @@ func (e *Server) dispatchPipe(ctx context.Context, pipe []CodeStep, timeout int,
|
|||
return input, nil
|
||||
}
|
||||
|
||||
func buildExecLabel(stages []CodeStep) string {
|
||||
stageLabel := func(s CodeStep) string {
|
||||
prefix := ""
|
||||
if s.Elevated {
|
||||
prefix = "[elevated] "
|
||||
}
|
||||
if len(s.Parallel) > 0 {
|
||||
return fmt.Sprintf("%sparallel(%d)", prefix, len(s.Parallel))
|
||||
}
|
||||
if s.Tool != "" {
|
||||
return fmt.Sprintf("%stool:%s %s", prefix, s.Tool, strings.Join(s.Args, " "))
|
||||
}
|
||||
return prefix + s.Code
|
||||
}
|
||||
if len(stages) == 1 {
|
||||
return fmt.Sprintf("execute_code: %s", stageLabel(stages[0]))
|
||||
}
|
||||
var sb strings.Builder
|
||||
fmt.Fprintf(&sb, "execute_code: %d stages", len(stages))
|
||||
for i, s := range stages {
|
||||
fmt.Fprintf(&sb, "\n [%d]: %s", i, stageLabel(s))
|
||||
}
|
||||
return sb.String()
|
||||
}
|
||||
|
||||
// enforceStrict rejects any step that uses inline code rather than a named tool.
|
||||
func enforceStrict(stages []CodeStep) error {
|
||||
for i, s := range stages {
|
||||
|
|
|
|||
|
|
@ -20,12 +20,10 @@ func TestMain(m *testing.M) {
|
|||
os.Exit(m.Run())
|
||||
}
|
||||
|
||||
// newServer returns a Server with Confirm always returning true and Yolo=true
|
||||
// so tests run without landrun installed in CI.
|
||||
// newServer returns a Server with Yolo=true so tests run without landrun in CI.
|
||||
func newServer(t *testing.T) *Server {
|
||||
t.Helper()
|
||||
s := New(t.TempDir())
|
||||
s.Confirm = func(string) bool { return true }
|
||||
s.Yolo = true // skip landrun in unit tests
|
||||
return s
|
||||
}
|
||||
|
|
@ -193,31 +191,6 @@ func TestValidateCodeSafe(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// ---- allowed ----
|
||||
|
||||
func TestAllowedWithConfirm(t *testing.T) {
|
||||
s := New(t.TempDir())
|
||||
s.Confirm = func(string) bool { return true }
|
||||
if !s.allowed("execute_code", "echo hi") {
|
||||
t.Error("expected allowed=true when Confirm returns true")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAllowedDeniedNoConfirm(t *testing.T) {
|
||||
s := New(t.TempDir())
|
||||
if s.allowed("execute_code", "echo hi") {
|
||||
t.Error("expected allowed=false when Confirm is nil")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAllowedTrustedTool(t *testing.T) {
|
||||
s := New(t.TempDir())
|
||||
s.SetTrustedTools([]string{"execute_code"})
|
||||
if !s.allowed("execute_code", "anything") {
|
||||
t.Error("expected trusted tool to be allowed without Confirm")
|
||||
}
|
||||
}
|
||||
|
||||
// ---- Execute (integration, requires bash) ----
|
||||
|
||||
func TestExecuteSimpleBash(t *testing.T) {
|
||||
|
|
@ -284,14 +257,6 @@ func TestDispatchNoSteps(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestDispatchDenied(t *testing.T) {
|
||||
s := New(t.TempDir()) // no Confirm, not trusted → denied
|
||||
_, err := callCode(t, s, []map[string]any{{"code": "echo hi"}})
|
||||
if err == nil || !strings.Contains(err.Error(), "denied") {
|
||||
t.Errorf("expected denied error, got %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDispatchSingleStep(t *testing.T) {
|
||||
s := newServer(t)
|
||||
out, err := callCode(t, s, []map[string]any{{"code": "echo single"}})
|
||||
|
|
|
|||
|
|
@ -28,14 +28,6 @@ const (
|
|||
|
||||
// Server runs code in a sandboxed environment.
|
||||
type Server struct {
|
||||
// Confirm is an optional function called before executing sensitive operations.
|
||||
// Returns true to allow, false to deny. Trusted tools bypass this check.
|
||||
Confirm func(string) bool
|
||||
|
||||
// trusted is the set of tool names that bypass Confirm.
|
||||
trustedMu sync.RWMutex
|
||||
trusted map[string]bool
|
||||
|
||||
// cwd is the working directory for sandboxed commands. If empty,
|
||||
// the process working directory is used.
|
||||
wdMu sync.RWMutex
|
||||
|
|
@ -330,16 +322,6 @@ func (e *Server) SetCWD(dir string) {
|
|||
e.wdMu.Unlock()
|
||||
}
|
||||
|
||||
// SetTrustedTools marks the given tool names as trusted; they bypass Confirm.
|
||||
func (e *Server) SetTrustedTools(tools []string) {
|
||||
e.trustedMu.Lock()
|
||||
e.trusted = make(map[string]bool, len(tools))
|
||||
for _, t := range tools {
|
||||
e.trusted[t] = true
|
||||
}
|
||||
e.trustedMu.Unlock()
|
||||
}
|
||||
|
||||
// SetAllowExecutors restricts which executors are available.
|
||||
func (e *Server) SetAllowExecutors(names []string) {
|
||||
if len(names) > 0 {
|
||||
|
|
@ -364,22 +346,6 @@ func (e *Server) SetAllowTools(names []string) {
|
|||
}
|
||||
}
|
||||
|
||||
// allowed returns true if the tool call should proceed.
|
||||
// Trusted tools are always allowed. Untrusted tools are passed to Confirm;
|
||||
// if Confirm is nil, untrusted tools are denied.
|
||||
func (e *Server) allowed(tool, detail string) bool {
|
||||
e.trustedMu.RLock()
|
||||
trusted := e.trusted[tool]
|
||||
e.trustedMu.RUnlock()
|
||||
if trusted {
|
||||
return true
|
||||
}
|
||||
if e.Confirm == nil {
|
||||
return false // no confirm fn and not trusted: deny by default
|
||||
}
|
||||
return e.Confirm(detail)
|
||||
}
|
||||
|
||||
// SetLockDir sets the directory used for advisory flock files during parallel
|
||||
// step dispatch. Must be called before the Server handles concurrent requests.
|
||||
func (e *Server) SetLockDir(dir string) { e.lockDir = dir }
|
||||
|
|
|
|||
|
|
@ -101,12 +101,6 @@ type EnvSetter interface {
|
|||
SetEnv(key, value string)
|
||||
}
|
||||
|
||||
// TrustedToolsSetter is implemented by tool servers that support a trusted-tool
|
||||
// list. Tools in the list bypass the Confirm callback.
|
||||
type TrustedToolsSetter interface {
|
||||
SetTrustedTools(tools []string)
|
||||
}
|
||||
|
||||
// LockDirSetter is implemented by tool servers that use advisory flock files
|
||||
// for parallel-step coordination. SetLockDir must be called once at session init.
|
||||
type LockDirSetter interface {
|
||||
|
|
|
|||
Reference in New Issue