mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-11 23:40:28 -10:00
* Tools: arg-tolerance, ask_user, secret scrubbing, structured results, hardening First module of the tools->agent->tui chain that leads to the TUI shell (off main). Scope: internal/tools (+ a 1-line specialist vocabulary update). 3-way merged onto main's specialist drift (RunOptions tool-context fields, Safety.AdvertiseInAuto, optionsAwareTool dispatch, PropertySchema, base structured Result) and grafted the runtime-core additions: - Shared arg-tolerance layer (argtolerance.go: alias-aware/type-strict arg resolution + slice coercion) adopted across all tools so weak models' non-canonical arg shapes are accepted; bool/int coercion in args.go. - ask_user tool (interactive clarifying questions) wired into CoreReadOnlyTools; nested PropertySchema (Properties/Required) for its object questions + update_plan. - Secret scrubbing at the registry boundary (scrubResultSecrets via internal/redaction) applied to every tool-execution path, incl. the optionsAwareTool dispatch; structured Result (Display/Redacted/ChangedFiles). - mutation_targets (paths a mutating tool will write, for session checkpointing) + hardened grep (symlink confinement), apply_patch, bash, edit/write. EXCLUDED per the module split: the blueprint task tool (subagents — another dev's area; the merged specialist Task is separate) and the skill tool (ships with the later skills module); plus their subagent-only registry helpers (Without/isolateForChild). ask_user is now a core tool, so internal/specialist knownToolNames adds "ask_user" to keep the TestKnownToolNamesMatchCoreRegistry cross-package invariant. No new deps; build/vet/-race/windows/full-suite green. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * Address #119 review (CodeRabbit): scrub Display, validate apply_patch mutation paths, narrow test skips - registry.go: scrubResultSecrets now redacts Result.Display.Summary too (not just Output), so a caller preferring Display can't bypass the boundary redaction. - mutation_targets.go: apply_patch targets now run through validatePatchPaths(applyRoot, ...) before being returned, so a traversal patch (../x) can't yield an out-of-workspace checkpoint target. Regression case added to TestMutationTargetsRejectsEscapingPaths. - argtolerance_test/write_tools_test: the apply_patch tests no longer Skipf on ANY non-OK result (which masked real diff/apply regressions); they skip ONLY when the git binary is unavailable (gitApplyUnavailable helper checks exec's 'executable file not found') and otherwise Fatalf. build/vet/-race/full-suite + GOOS=windows build green. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * Address #119 re-review (CodeRabbit): best-effort multiSelect + Display.Summary scrub coverage - ask_user: an uncoercible multiSelect no longer fails the whole call — it's a UI hint, so it defaults to false (best-effort, mirroring the lenient options path). Test: TestParseAskUserQuestionsLenientOptions extended. - registry_test: TestRunWithOptionsScrubsSecretsForAllCallers now also leaks a secret via Result.Display.Summary and asserts it is redacted, locking in the boundary scrub for the structured-result path (secretTool gained a display field). build/vet/-race/full-suite + GOOS=windows build green. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * Address #119 review (@Vasanthdev2004): scrub ALL registry return paths + harden intArg [blocker] RunWithOptions now scrubs every return path, not just tool-execution: a named return + a single 'defer scrubResultSecrets(result)' covers the unknown-tool, sandbox-deny, sandbox-approval-required, permission-required and permission-denied early returns too. Previously a sandbox denial that echoed a secret-bearing path/arg returned it raw with Redacted=false. Regression: TestRunWithOptionsScrubsSecretsOnDenialPaths (permission-denied reason carrying a token -> scrubbed + Redacted). [non-blocking] intArg fails closed on NaN/Inf/out-of-range floats (and their string forms) via a floatToInt helper, before any implementation-defined cast. Tests added for NaN/Inf/NaN-string/non-integral. build/vet/-race/full-suite + GOOS=windows build green. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * Address #119 review (CodeRabbit grep TOCTOU + anandh8x dependency blockers) - grep: close the confinement TOCTOU — confineGrepFile now returns the symlink-RESOLVED path and collectGrepMatches reads THAT (not the raw candidate), so a symlink swapped in between check and read can't escape the workspace. - ask_user: removed from CoreReadOnlyTools. It needs the agent loop's interactive intercept (OnAskUser), which is in the (unmerged) agent module; without it the tool only returns the non-interactive fallback. The tool still ships (NewAskUserTool); the agent module registers it in core + wires the intercept together. Reverted the specialist knownToolNames 'ask_user' entry + the ask_user-in-core test accordingly. - update_plan: reverted the plan schema to a flat array (item structure documented in the description). The structured nested-object Items schema depends on the agent's PropertySchema serializer (propertyToRuntimeMap) passing nested Properties/Required through to providers, which is in the agent module; a nested schema here would be silently dropped. PropertySchema.Properties/Required fields are kept (used by ask_user.go's own schema + the later MCP module). build/vet/-race/full-suite + GOOS=windows build green; no new deps. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * Harden #119 from adversarial self-review: sandbox alias-gate, Meta scrub, intArg 2^63 A multi-dimension adversarial review of the tools diff surfaced 3 real issues beyond the reviewer rounds: - [major/security] write_file/edit_file path aliases file_path/filename/filepath bypassed the sandbox workspace+symlink gate: sandbox.requestPaths only inspected path/cwd/file/dir, so an aliased target skipped enforcement (same class as audit H4). Aligned requestPaths with the tools' path-alias lists (internal/sandbox/risk.go). The tools' own resolveWorkspaceTargetPath still confined, but the sandbox LAYER/audit/symlink-check was silently skipped. Regression: TestRegistrySandboxGatesPathAliasKeys. - [minor] scrubResultSecrets skipped Result.Meta, which is forwarded to the transcript and carries model-controlled strings (glob pattern, bash cwd). Now scrubs Meta values too. Covered in TestRunWithOptionsScrubsSecretsForAllCallers. - [minor] floatToInt used f > float64(MaxInt); that constant rounds up to 2^63 so exactly 2^63 slipped to an out-of-range cast. Changed to >=. Test added for 2^63 float+string. build/vet/-race/full-suite + GOOS=windows build green; no new deps. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> --------- Co-authored-by: KRATOS <[email protected]> Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
378 lines
12 KiB
Go
378 lines
12 KiB
Go
package tools
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"os"
|
|
"path/filepath"
|
|
"runtime"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/Gitlawb/zero/internal/sandbox"
|
|
)
|
|
|
|
func TestMain(m *testing.M) {
|
|
if len(os.Args) >= 3 && os.Args[1] == "--zero-bash-helper" {
|
|
runBashToolHelper(os.Args[2])
|
|
return
|
|
}
|
|
|
|
os.Exit(m.Run())
|
|
}
|
|
|
|
func runBashToolHelper(command string) {
|
|
switch command {
|
|
case "success":
|
|
fmt.Println("hello from bash")
|
|
case "stderr":
|
|
fmt.Fprintln(os.Stderr, "warning from bash")
|
|
case "fail":
|
|
fmt.Println("before failure")
|
|
fmt.Fprintln(os.Stderr, "failure details")
|
|
os.Exit(7)
|
|
case "pwd":
|
|
cwd, err := os.Getwd()
|
|
if err != nil {
|
|
fmt.Fprintln(os.Stderr, err)
|
|
os.Exit(1)
|
|
}
|
|
fmt.Println(cwd)
|
|
case "sleep":
|
|
time.Sleep(250 * time.Millisecond)
|
|
fmt.Println("woke up")
|
|
default:
|
|
fmt.Fprintln(os.Stderr, "unknown helper command")
|
|
os.Exit(2)
|
|
}
|
|
}
|
|
|
|
func TestCoreToolsExposeBashTool(t *testing.T) {
|
|
toolset := CoreTools(t.TempDir())
|
|
byName := make(map[string]Tool, len(toolset))
|
|
for _, tool := range toolset {
|
|
byName[tool.Name()] = tool
|
|
}
|
|
|
|
tool, ok := byName["bash"]
|
|
if !ok {
|
|
t.Fatalf("expected core tools to include bash")
|
|
}
|
|
if tool.Safety().SideEffect != SideEffectShell {
|
|
t.Fatalf("bash side effect = %s, want shell", tool.Safety().SideEffect)
|
|
}
|
|
if tool.Safety().Permission != PermissionPrompt {
|
|
t.Fatalf("bash permission = %s, want prompt", tool.Safety().Permission)
|
|
}
|
|
}
|
|
|
|
func TestRegistryBlocksBashWithoutGrant(t *testing.T) {
|
|
registry := NewRegistry()
|
|
registry.Register(NewBashTool(t.TempDir()))
|
|
|
|
result := registry.Run(context.Background(), "bash", map[string]any{
|
|
"command": helperCommand("success"),
|
|
})
|
|
|
|
if result.Status != StatusError {
|
|
t.Fatalf("expected error status, got %s", result.Status)
|
|
}
|
|
if !strings.Contains(result.Output, "Permission required for bash") {
|
|
t.Fatalf("expected permission error, got %q", result.Output)
|
|
}
|
|
}
|
|
|
|
func TestBashToolRunsCommandInWorkspace(t *testing.T) {
|
|
root := t.TempDir()
|
|
|
|
result := NewBashTool(root).Run(context.Background(), map[string]any{
|
|
"command": helperCommand("success"),
|
|
})
|
|
|
|
if result.Status != StatusOK {
|
|
t.Fatalf("expected ok status, got %s: %s", result.Status, result.Output)
|
|
}
|
|
if !strings.Contains(result.Output, "stdout:\nhello from bash") {
|
|
t.Fatalf("expected stdout in output, got %q", result.Output)
|
|
}
|
|
if result.Meta["exit_code"] != "0" {
|
|
t.Fatalf("expected exit_code metadata 0, got %q", result.Meta["exit_code"])
|
|
}
|
|
if result.Meta["cwd"] != "." {
|
|
t.Fatalf("expected cwd metadata ., got %q", result.Meta["cwd"])
|
|
}
|
|
}
|
|
|
|
func TestBashToolUsesRequestedCwd(t *testing.T) {
|
|
root := t.TempDir()
|
|
nested := filepath.Join(root, "nested")
|
|
if err := os.MkdirAll(nested, 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
result := NewBashTool(root).Run(context.Background(), map[string]any{
|
|
"command": helperCommand("pwd"),
|
|
"cwd": "nested",
|
|
})
|
|
|
|
if result.Status != StatusOK {
|
|
t.Fatalf("expected ok status, got %s: %s", result.Status, result.Output)
|
|
}
|
|
normalizedOutput := filepath.ToSlash(strings.TrimSpace(result.Output))
|
|
if !strings.Contains(normalizedOutput, "stdout:\n") || !strings.HasSuffix(normalizedOutput, "/nested") {
|
|
t.Fatalf("expected command to run in nested cwd, got %q", result.Output)
|
|
}
|
|
if result.Meta["cwd"] != "nested" {
|
|
t.Fatalf("expected cwd metadata nested, got %q", result.Meta["cwd"])
|
|
}
|
|
}
|
|
|
|
func TestBashToolRejectsCwdOutsideWorkspace(t *testing.T) {
|
|
outside := t.TempDir()
|
|
|
|
result := NewBashTool(t.TempDir()).Run(context.Background(), map[string]any{
|
|
"command": helperCommand("success"),
|
|
"cwd": outside,
|
|
})
|
|
|
|
if result.Status != StatusError {
|
|
t.Fatalf("expected error status, got %s", result.Status)
|
|
}
|
|
if !strings.Contains(result.Output, "must stay inside the workspace") {
|
|
t.Fatalf("expected workspace error, got %q", result.Output)
|
|
}
|
|
}
|
|
|
|
func TestBashToolReturnsNonzeroExitAsError(t *testing.T) {
|
|
result := NewBashTool(t.TempDir()).Run(context.Background(), map[string]any{
|
|
"command": helperCommand("fail"),
|
|
})
|
|
|
|
if result.Status != StatusError {
|
|
t.Fatalf("expected error status, got %s", result.Status)
|
|
}
|
|
for _, want := range []string{"stdout:\nbefore failure", "stderr:\nfailure details", "exit_code: 7"} {
|
|
if !strings.Contains(result.Output, want) {
|
|
t.Fatalf("expected output to contain %q, got %q", want, result.Output)
|
|
}
|
|
}
|
|
if result.Meta["exit_code"] != "7" {
|
|
t.Fatalf("expected exit_code metadata 7, got %q", result.Meta["exit_code"])
|
|
}
|
|
}
|
|
|
|
func TestBashToolTimesOut(t *testing.T) {
|
|
result := NewBashTool(t.TempDir()).Run(context.Background(), map[string]any{
|
|
"command": helperCommand("sleep"),
|
|
"timeout_ms": 20,
|
|
})
|
|
|
|
if result.Status != StatusError {
|
|
t.Fatalf("expected error status, got %s", result.Status)
|
|
}
|
|
if !strings.Contains(result.Output, "timed out after 20ms") {
|
|
t.Fatalf("expected timeout error, got %q", result.Output)
|
|
}
|
|
if result.Meta["timeout_ms"] != "20" {
|
|
t.Fatalf("expected timeout_ms metadata 20, got %q", result.Meta["timeout_ms"])
|
|
}
|
|
}
|
|
|
|
func TestRegistryRunsBashThroughSandboxEngine(t *testing.T) {
|
|
root := t.TempDir()
|
|
registry := NewRegistry()
|
|
registry.Register(NewBashTool(root))
|
|
engine := sandbox.NewEngine(sandbox.EngineOptions{
|
|
WorkspaceRoot: root,
|
|
Policy: sandbox.DefaultPolicy(),
|
|
Backend: sandbox.Backend{Name: sandbox.BackendPolicyOnly, Message: "policy-only fallback"},
|
|
})
|
|
|
|
result := registry.RunWithOptions(context.Background(), "bash", map[string]any{
|
|
"command": helperCommand("success"),
|
|
}, RunOptions{
|
|
PermissionGranted: true,
|
|
Sandbox: engine,
|
|
PermissionMode: string(sandbox.PermissionUnsafe),
|
|
Autonomy: string(sandbox.AutonomyMedium),
|
|
})
|
|
|
|
if result.Status != StatusOK {
|
|
t.Fatalf("expected ok status, got %s: %s", result.Status, result.Output)
|
|
}
|
|
if !strings.Contains(result.Output, "hello from bash") {
|
|
t.Fatalf("expected helper output, got %q", result.Output)
|
|
}
|
|
if result.Meta["sandbox_backend"] != string(sandbox.BackendPolicyOnly) || result.Meta["sandbox_wrapped"] != "false" {
|
|
t.Fatalf("sandbox metadata = %#v, want policy-only fallback", result.Meta)
|
|
}
|
|
}
|
|
|
|
func TestBashToolReportsDisabledPolicyOnlyRunner(t *testing.T) {
|
|
root := t.TempDir()
|
|
policy := sandbox.DefaultPolicy()
|
|
policy.AllowPolicyOnlyRunner = false
|
|
engine := sandbox.NewEngine(sandbox.EngineOptions{
|
|
WorkspaceRoot: root,
|
|
Policy: policy,
|
|
Backend: sandbox.Backend{Name: sandbox.BackendPolicyOnly, Message: "policy-only fallback"},
|
|
})
|
|
|
|
result := NewBashTool(root).(interface {
|
|
RunWithSandbox(context.Context, map[string]any, *sandbox.Engine) Result
|
|
}).RunWithSandbox(context.Background(), map[string]any{
|
|
"command": helperCommand("success"),
|
|
}, engine)
|
|
|
|
if result.Status != StatusError {
|
|
t.Fatalf("expected error status, got %s", result.Status)
|
|
}
|
|
if !strings.Contains(result.Output, "policy-only sandbox runner is disabled") {
|
|
t.Fatalf("expected disabled fallback error, got %q", result.Output)
|
|
}
|
|
if result.Meta["exit_code"] != "-1" {
|
|
t.Fatalf("exit_code metadata = %q, want -1", result.Meta["exit_code"])
|
|
}
|
|
}
|
|
|
|
func TestBashToolBuildsWrappedSandboxExecCommand(t *testing.T) {
|
|
root := t.TempDir()
|
|
resolvedRoot, err := filepath.EvalSymlinks(root)
|
|
if err != nil {
|
|
t.Fatalf("EvalSymlinks(%q): %v", root, err)
|
|
}
|
|
engine := sandbox.NewEngine(sandbox.EngineOptions{
|
|
WorkspaceRoot: root,
|
|
Policy: sandbox.DefaultPolicy(),
|
|
Backend: sandbox.Backend{
|
|
Name: sandbox.BackendSandboxExec,
|
|
Available: true,
|
|
Executable: "/usr/bin/sandbox-exec",
|
|
},
|
|
})
|
|
|
|
command, plan, err := buildBashCommand(context.Background(), "pwd", root, engine)
|
|
if err != nil {
|
|
t.Fatalf("buildBashCommand: %v", err)
|
|
}
|
|
if command.Path != "/usr/bin/sandbox-exec" || !plan.Wrapped {
|
|
t.Fatalf("command path = %q plan = %#v, want wrapped sandbox-exec", command.Path, plan)
|
|
}
|
|
if len(command.Args) < 5 || command.Args[1] != "-p" || !strings.Contains(command.Args[2], "(deny network*)") {
|
|
t.Fatalf("sandbox-exec args = %#v, want inline profile", command.Args)
|
|
}
|
|
if command.Dir != resolvedRoot || plan.SandboxDir != resolvedRoot {
|
|
t.Fatalf("dirs = command %q plan %q, want root", command.Dir, plan.SandboxDir)
|
|
}
|
|
}
|
|
|
|
func TestBashToolRunsWithHostSandboxBackendWhenAvailable(t *testing.T) {
|
|
if runtime.GOOS == "windows" {
|
|
t.Skip("Windows sandbox adapter is owned by the Windows integration slice")
|
|
}
|
|
backend := sandbox.SelectBackend(sandbox.BackendOptions{})
|
|
if !backend.Available || backend.Name == sandbox.BackendPolicyOnly {
|
|
t.Skipf("host sandbox backend unavailable: %s", backend.Message)
|
|
}
|
|
root := t.TempDir()
|
|
engine := sandbox.NewEngine(sandbox.EngineOptions{
|
|
WorkspaceRoot: root,
|
|
Policy: sandbox.DefaultPolicy(),
|
|
Backend: backend,
|
|
})
|
|
|
|
result := NewBashTool(root).(interface {
|
|
RunWithSandbox(context.Context, map[string]any, *sandbox.Engine) Result
|
|
}).RunWithSandbox(context.Background(), map[string]any{
|
|
"command": "printf sandbox-ok",
|
|
}, engine)
|
|
|
|
if result.Status != StatusOK {
|
|
t.Fatalf("expected host sandbox command to run, got %s: %s; meta=%#v", result.Status, result.Output, result.Meta)
|
|
}
|
|
if !strings.Contains(result.Output, "sandbox-ok") {
|
|
t.Fatalf("expected sandbox command output, got %q", result.Output)
|
|
}
|
|
if result.Meta["sandbox_backend"] != string(backend.Name) || result.Meta["sandbox_wrapped"] != "true" {
|
|
t.Fatalf("sandbox metadata = %#v, want wrapped host backend %s", result.Meta, backend.Name)
|
|
}
|
|
}
|
|
|
|
func TestBashToolBlocksInteractiveCommandBeforeExecution(t *testing.T) {
|
|
root := t.TempDir()
|
|
|
|
result := NewBashTool(root).Run(context.Background(), map[string]any{
|
|
"command": "vim main.go",
|
|
})
|
|
|
|
if result.Status != StatusError {
|
|
t.Fatalf("expected error status for interactive command, got %s: %s", result.Status, result.Output)
|
|
}
|
|
if !strings.Contains(result.Output, "interactive") {
|
|
t.Fatalf("expected interactive guard message, got %q", result.Output)
|
|
}
|
|
if !strings.Contains(result.Output, "edit_file") && !strings.Contains(result.Output, "sed") {
|
|
t.Fatalf("expected actionable suggestion, got %q", result.Output)
|
|
}
|
|
// The command must NOT have run: exit_code metadata should mark a pre-exec block.
|
|
if result.Meta["exit_code"] != "-1" {
|
|
t.Fatalf("expected exit_code -1 (not executed), got %q", result.Meta["exit_code"])
|
|
}
|
|
if result.Meta["safety_block"] != "interactive_command" {
|
|
t.Fatalf("expected safety_block metadata, got %#v", result.Meta)
|
|
}
|
|
if result.Display.Kind != "shell" || !strings.Contains(result.Display.Summary, "Blocked") {
|
|
t.Fatalf("expected blocked display annotation, got %#v", result.Display)
|
|
}
|
|
}
|
|
|
|
func TestBashToolBlocksInteractiveCommandThroughSandbox(t *testing.T) {
|
|
root := t.TempDir()
|
|
engine := sandbox.NewEngine(sandbox.EngineOptions{
|
|
WorkspaceRoot: root,
|
|
Policy: sandbox.DefaultPolicy(),
|
|
Backend: sandbox.Backend{Name: sandbox.BackendPolicyOnly, Message: "policy-only fallback"},
|
|
})
|
|
|
|
result := NewBashTool(root).(interface {
|
|
RunWithSandbox(context.Context, map[string]any, *sandbox.Engine) Result
|
|
}).RunWithSandbox(context.Background(), map[string]any{
|
|
"command": "less /etc/hosts",
|
|
}, engine)
|
|
|
|
if result.Status != StatusError {
|
|
t.Fatalf("expected error status, got %s: %s", result.Status, result.Output)
|
|
}
|
|
if !strings.Contains(result.Output, "interactive") || !strings.Contains(result.Output, "cat") {
|
|
t.Fatalf("expected pager guard message with cat suggestion, got %q", result.Output)
|
|
}
|
|
}
|
|
|
|
func TestBashToolAllowsNonInteractiveCommand(t *testing.T) {
|
|
root := t.TempDir()
|
|
|
|
result := NewBashTool(root).Run(context.Background(), map[string]any{
|
|
"command": helperCommand("success"),
|
|
})
|
|
|
|
if result.Status != StatusOK {
|
|
t.Fatalf("expected ok status for non-interactive command, got %s: %s", result.Status, result.Output)
|
|
}
|
|
if result.Meta["safety_block"] != "" {
|
|
t.Fatalf("did not expect a safety block, got %#v", result.Meta)
|
|
}
|
|
}
|
|
|
|
func helperCommand(name string) string {
|
|
executable := shellQuote(os.Args[0])
|
|
return executable + " --zero-bash-helper " + name
|
|
}
|
|
|
|
func shellQuote(value string) string {
|
|
if runtime.GOOS == "windows" {
|
|
return value
|
|
}
|
|
return "'" + strings.ReplaceAll(value, "'", `'\''`) + "'"
|
|
}
|