diff --git a/cmd/pkg-helper/main.go b/cmd/pkg-helper/main.go index b05f7f0c..bfebf715 100644 --- a/cmd/pkg-helper/main.go +++ b/cmd/pkg-helper/main.go @@ -1,3 +1,5 @@ +//go:build !windows + // pkg-helper is a root-privileged helper that listens on a Unix socket // and executes apk add/del commands on behalf of the non-root app process. // It is started by docker-entrypoint.sh before dropping privileges. diff --git a/cmd/pkg-helper/main_test.go b/cmd/pkg-helper/main_test.go index f205b1c6..b4fc1b7a 100644 --- a/cmd/pkg-helper/main_test.go +++ b/cmd/pkg-helper/main_test.go @@ -1,3 +1,5 @@ +//go:build !windows + package main import ( @@ -120,24 +122,24 @@ func TestValidPkgName(t *testing.T) { {"pkg_with_underscores", true}, {"pkg.with.dots", true}, // Invalid package names - {"-invalid", false}, // starts with hyphen - {"--flag", false}, // starts with hyphen - {"pkg name", false}, // contains space - {"pkg;cmd", false}, // contains semicolon - {"pkg|cmd", false}, // contains pipe - {"pkg&cmd", false}, // contains ampersand - {"pkg`cmd`", false}, // contains backtick - {"pkg$var", false}, // contains dollar sign - {"pkgfile", false}, // contains angle bracket - {"pkg'quote", false}, // contains quote - {"pkg\"quote", false}, // contains quote - {"pkg(paren)", false}, // contains parens - {"", false}, // empty - {" curl", false}, // starts with space - {"curl ", false}, // ends with space - {"--index-url=evil", false}, // flag pattern - {"-u", false}, // short flag + {"-invalid", false}, // starts with hyphen + {"--flag", false}, // starts with hyphen + {"pkg name", false}, // contains space + {"pkg;cmd", false}, // contains semicolon + {"pkg|cmd", false}, // contains pipe + {"pkg&cmd", false}, // contains ampersand + {"pkg`cmd`", false}, // contains backtick + {"pkg$var", false}, // contains dollar sign + {"pkgfile", false}, // contains angle bracket + {"pkg'quote", false}, // contains quote + {"pkg\"quote", false}, // contains quote + {"pkg(paren)", false}, // contains parens + {"", false}, // empty + {" curl", false}, // starts with space + {"curl ", false}, // ends with space + {"--index-url=evil", false}, // flag pattern + {"-u", false}, // short flag } for _, tt := range tests { @@ -461,10 +463,10 @@ func TestHandleRequest_UpgradeValidation(t *testing.T) { // TestHandleRequest_UpgradeInjectionPatterns verifies 5 injection patterns are rejected. func TestHandleRequest_UpgradeInjectionPatterns(t *testing.T) { injections := []string{ - "-malicious", // leading hyphen - "pkg;evil", // semicolon - "pkg evil", // space - "@edge/curl", // @ prefix (legacy npm compat — rejected by validApkName) + "-malicious", // leading hyphen + "pkg;evil", // semicolon + "pkg evil", // space + "@edge/curl", // @ prefix (legacy npm compat — rejected by validApkName) "UPPERCASE_PKG", // uppercase rejected by validApkName } for _, pkg := range injections { @@ -485,8 +487,8 @@ func TestHandleRequest_UpgradeInjectionPatterns(t *testing.T) { // via the stricter validApkName. func TestHandleRequest_UpgradeRejectsLegacySymbols(t *testing.T) { legacySymbols := []string{ - "pkg@edge", // @ accepted by validPkgName, rejected by validApkName - "@scope/pkg", // npm scoped — rejected by validApkName + "pkg@edge", // @ accepted by validPkgName, rejected by validApkName + "@scope/pkg", // npm scoped — rejected by validApkName } for _, pkg := range legacySymbols { t.Run(pkg, func(t *testing.T) { @@ -716,7 +718,7 @@ func TestValidApkName(t *testing.T) { {"gtk+3.0", true}, {"ca-certificates", true}, {"py3-pip", true}, - {"0launch", true}, // starts with digit — valid per apk grammar + {"0launch", true}, // starts with digit — valid per apk grammar // Invalid: uppercase {"CURL", false}, diff --git a/cmd/pkg-helper/main_windows.go b/cmd/pkg-helper/main_windows.go new file mode 100644 index 00000000..69f4fb9c --- /dev/null +++ b/cmd/pkg-helper/main_windows.go @@ -0,0 +1,13 @@ +//go:build windows + +package main + +import ( + "fmt" + "os" +) + +func main() { + fmt.Fprintln(os.Stderr, "pkg-helper is only supported on Unix-like systems") + os.Exit(1) +} diff --git a/internal/agent/loop_utils_test.go b/internal/agent/loop_utils_test.go index 2fdd8d6f..94e28d91 100644 --- a/internal/agent/loop_utils_test.go +++ b/internal/agent/loop_utils_test.go @@ -1,6 +1,7 @@ package agent import ( + "path/filepath" "strings" "testing" @@ -134,7 +135,7 @@ func TestShouldShareSessions_DisabledByDefault(t *testing.T) { func TestShouldShareSessions_IndependentOfMemory(t *testing.T) { l := &Loop{workspaceSharing: &store.WorkspaceSharingConfig{ - ShareMemory: true, + ShareMemory: true, ShareSessions: false, }} if l.shouldShareSessions() { @@ -181,9 +182,10 @@ func TestProviderName_WithProvider(t *testing.T) { // ─── expandWorkspace ────────────────────────────────────────────────────── func TestExpandWorkspace_AbsolutePathUnchanged(t *testing.T) { - got := expandWorkspace("/absolute/path") - if got != "/absolute/path" { - t.Errorf("expandWorkspace = %q, want /absolute/path", got) + abs := filepath.Join(t.TempDir(), "absolute", "path") + got := expandWorkspace(abs) + if got != filepath.Clean(abs) { + t.Errorf("expandWorkspace = %q, want %q", got, filepath.Clean(abs)) } } @@ -192,14 +194,14 @@ func TestExpandWorkspace_HomeExpanded(t *testing.T) { if strings.HasPrefix(got, "~") { t.Errorf("tilde not expanded: %q", got) } - if !strings.HasPrefix(got, "/") { + if !filepath.IsAbs(got) { t.Errorf("expected absolute path after ~ expansion, got %q", got) } } func TestExpandWorkspace_RelativePathBecomesAbsolute(t *testing.T) { got := expandWorkspace("relative/path") - if !strings.HasPrefix(got, "/") { + if !filepath.IsAbs(got) { t.Errorf("relative path should become absolute, got %q", got) } } diff --git a/internal/backup/pgpass_test.go b/internal/backup/pgpass_test.go index ddbd14b7..29fe773e 100644 --- a/internal/backup/pgpass_test.go +++ b/internal/backup/pgpass_test.go @@ -3,6 +3,7 @@ package backup import ( "os" "path/filepath" + "runtime" "strings" "testing" ) @@ -59,6 +60,9 @@ func TestParseDSN_SpecialCharsInPassword(t *testing.T) { } func TestWritePgpass_FilePermissions(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows does not expose POSIX 0600 permissions reliably") + } creds := &PGCredentials{ Host: "localhost", Port: "5432", User: "testuser", Password: "testpass", diff --git a/internal/bootstrap/backfill_capabilities.go b/internal/bootstrap/backfill_capabilities.go index 80a000e3..88493e31 100644 --- a/internal/bootstrap/backfill_capabilities.go +++ b/internal/bootstrap/backfill_capabilities.go @@ -4,7 +4,6 @@ import ( "context" "database/sql" "log/slog" - "path/filepath" ) // BackfillCapabilities seeds CAPABILITIES.md template for all agents that don't have it. @@ -15,7 +14,7 @@ func BackfillCapabilities(ctx context.Context, db *sql.DB) (int64, error) { return 0, nil } - tpl, err := templateFS.ReadFile(filepath.Join("templates", CapabilitiesFile)) + tpl, err := templateFS.ReadFile(templatePath(CapabilitiesFile)) if err != nil { return 0, err } diff --git a/internal/bootstrap/seed.go b/internal/bootstrap/seed.go index d448055b..8db9329b 100644 --- a/internal/bootstrap/seed.go +++ b/internal/bootstrap/seed.go @@ -4,6 +4,7 @@ import ( "embed" "log/slog" "os" + "path" "path/filepath" ) @@ -25,7 +26,7 @@ var templateFiles = []string{ // ReadTemplate returns the content of an embedded template file. func ReadTemplate(name string) (string, error) { - content, err := templateFS.ReadFile(filepath.Join("templates", name)) + content, err := templateFS.ReadFile(templatePath(name)) if err != nil { return "", err } @@ -88,7 +89,7 @@ func seedTemplate(workspaceDir, name string) (bool, error) { defer f.Close() // Read embedded template - content, err := templateFS.ReadFile(filepath.Join("templates", name)) + content, err := templateFS.ReadFile(templatePath(name)) if err != nil { os.Remove(dstPath) // clean up empty file return false, err @@ -100,3 +101,7 @@ func seedTemplate(workspaceDir, name string) (bool, error) { return true, nil } + +func templatePath(name string) string { + return path.Join("templates", name) +} diff --git a/internal/bootstrap/seed_store.go b/internal/bootstrap/seed_store.go index 619c74e7..a166811f 100644 --- a/internal/bootstrap/seed_store.go +++ b/internal/bootstrap/seed_store.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "log/slog" - "path/filepath" "strings" "time" @@ -49,8 +48,6 @@ _(First contact via %s channel. Profile info auto-filled from channel data.)_ `, name, name, tz, meta.ChannelType) } - - // retryOnBusy retries fn up to 3 times on SQLITE_BUSY errors with 500ms delay. func retryOnBusy(fn func() error) error { var lastErr error @@ -110,7 +107,7 @@ func SeedToStore(ctx context.Context, agentStore store.AgentStore, agentID uuid. continue } - content, err := templateFS.ReadFile(filepath.Join("templates", name)) + content, err := templateFS.ReadFile(templatePath(name)) if err != nil { slog.Warn("bootstrap: failed to read embedded template", "file", name, "error", err) continue @@ -125,7 +122,7 @@ func SeedToStore(ctx context.Context, agentStore store.AgentStore, agentID uuid. // Seed USER_PREDEFINED.md for predefined agents (agent-level, not in templateFiles). // Provides baseline user-handling rules shared across all users. if !hasContent[UserPredefinedFile] { - content, err := templateFS.ReadFile(filepath.Join("templates", UserPredefinedFile)) + content, err := templateFS.ReadFile(templatePath(UserPredefinedFile)) if err == nil { if err := retryOnBusy(func() error { return agentStore.SetAgentContextFile(ctx, agentID, UserPredefinedFile, string(content)) }); err != nil { return seeded, err @@ -218,7 +215,7 @@ func SeedUserFiles(ctx context.Context, agentStore store.AgentStore, agentID uui if name == BootstrapFile || name == UserFile { continue // skip bootstrap (the whole point) and user (already filled) } - content, err := templateFS.ReadFile(filepath.Join("templates", name)) + content, err := templateFS.ReadFile(templatePath(name)) if err != nil { continue } @@ -286,7 +283,7 @@ func SeedUserFiles(ctx context.Context, agentStore store.AgentStore, agentID uui templateName = "BOOTSTRAP_PREDEFINED.md" } - content, err := templateFS.ReadFile(filepath.Join("templates", templateName)) + content, err := templateFS.ReadFile(templatePath(templateName)) if err != nil { slog.Warn("bootstrap: failed to read embedded template for user seed", "file", name, "error", err) continue @@ -319,7 +316,7 @@ func EmbeddedUserFiles(agentType string) []ContextFile { if agentType == store.AgentTypePredefined && name == BootstrapFile { templateName = "BOOTSTRAP_PREDEFINED.md" } - content, err := templateFS.ReadFile(filepath.Join("templates", templateName)) + content, err := templateFS.ReadFile(templatePath(templateName)) if err != nil { continue } diff --git a/internal/hooks/handlers/command.go b/internal/hooks/handlers/command.go index e11d9fc0..c0be9278 100644 --- a/internal/hooks/handlers/command.go +++ b/internal/hooks/handlers/command.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "os/exec" + "runtime" "github.com/nextlevelbuilder/goclaw/internal/edition" "github.com/nextlevelbuilder/goclaw/internal/hooks" @@ -52,13 +53,13 @@ func (h *CommandHandler) Execute(ctx context.Context, cfg hooks.HookConfig, ev h return hooks.DecisionError, fmt.Errorf("hook: command handler: marshal event: %w", err) } - sh, err := findShell() + sh, args, err := shellCommand(cmd) if err != nil { return hooks.DecisionError, fmt.Errorf("hook: command handler: %w", err) } //nolint:gosec // Command comes from admin-configured hooks stored in DB, not user input. - c := exec.CommandContext(ctx, sh, "-c", cmd) + c := exec.CommandContext(ctx, sh, args...) c.Stdin = bytes.NewReader(eventJSON) c.Env = buildAllowedEnv(allowedVars) @@ -95,12 +96,20 @@ func (h *CommandHandler) Execute(ctx context.Context, cfg hooks.HookConfig, ev h return hooks.DecisionAllow, nil } -// findShell returns the path to sh, falling back to /bin/sh. -func findShell() (string, error) { - if p, err := exec.LookPath("sh"); err == nil { - return p, nil +// shellCommand returns the platform shell invocation for an admin-configured command. +func shellCommand(cmd string) (string, []string, error) { + if runtime.GOOS == "windows" { + for _, shell := range []string{"pwsh", "powershell"} { + if p, err := exec.LookPath(shell); err == nil { + return p, []string{"-NoProfile", "-NonInteractive", "-Command", cmd}, nil + } + } + return "", nil, fmt.Errorf("PowerShell shell not found") } - return "/bin/sh", nil + if p, err := exec.LookPath("sh"); err == nil { + return p, []string{"-c", cmd}, nil + } + return "/bin/sh", []string{"-c", cmd}, nil } // buildAllowedEnv constructs an env slice containing only the listed keys from diff --git a/internal/hooks/handlers/command_test.go b/internal/hooks/handlers/command_test.go index cb5544d5..6179bedb 100644 --- a/internal/hooks/handlers/command_test.go +++ b/internal/hooks/handlers/command_test.go @@ -2,6 +2,7 @@ package handlers_test import ( "context" + "runtime" "strings" "testing" "time" @@ -60,7 +61,11 @@ func TestCommand_ExitOne_ReturnsError(t *testing.T) { func TestCommand_JSONContinueFalse_ReturnsBlock(t *testing.T) { h := &handlers.CommandHandler{Edition: edition.Lite} // printf produces {"continue":false} on stdout then exits 0. - cfg := makeCmdCfg(`printf '{"continue":false}'`, hooks.ScopeAgent) + command := `printf '{"continue":false}'` + if runtime.GOOS == "windows" { + command = `Write-Output '{"continue":false}'` + } + cfg := makeCmdCfg(command, hooks.ScopeAgent) dec, err := h.Execute(context.Background(), cfg, hooks.Event{HookEvent: hooks.EventPreToolUse}) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -137,7 +142,11 @@ func TestCommand_CtxCancel_IsKilled(t *testing.T) { // "exec sleep 30" replaces the shell process with sleep directly (no // grandchild), so exec.CommandContext's kill reaches the process that // holds the pipe — Output() returns promptly after ctx deadline fires. - cfg := makeCmdCfg("exec sleep 30", hooks.ScopeAgent) + command := "exec sleep 30" + if runtime.GOOS == "windows" { + command = "Start-Sleep -Seconds 30" + } + cfg := makeCmdCfg(command, hooks.ScopeAgent) ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) defer cancel() diff --git a/internal/http/file_token.go b/internal/http/file_token.go index 1227dee0..867c456a 100644 --- a/internal/http/file_token.go +++ b/internal/http/file_token.go @@ -6,7 +6,7 @@ import ( "crypto/sha256" "encoding/base64" "fmt" - "path/filepath" + "path" "regexp" "strconv" "strings" @@ -77,21 +77,26 @@ func SignMediaPath(rawPath, secret string) string { return "" } // Strip stale ?ft= tokens - path := staleTokenRe.ReplaceAllString(rawPath, "") - path = strings.TrimRight(path, "?&") + cleanPath := filepathToURLPath(rawPath) + cleanPath = staleTokenRe.ReplaceAllString(cleanPath, "") + cleanPath = strings.TrimRight(cleanPath, "?&") // Strip all /v1/files/ and /v1/media/ prefixes (may be stacked from legacy bugs) - for strings.Contains(path, "/v1/files/") { - path = strings.Replace(path, "/v1/files/", "/", 1) + for strings.Contains(cleanPath, "/v1/files/") { + cleanPath = strings.Replace(cleanPath, "/v1/files/", "/", 1) } - for strings.Contains(path, "/v1/media/") { - path = strings.Replace(path, "/v1/media/", "/", 1) + for strings.Contains(cleanPath, "/v1/media/") { + cleanPath = strings.Replace(cleanPath, "/v1/media/", "/", 1) } - path = filepath.Clean(path) - urlPath := "/v1/files/" + strings.TrimPrefix(path, "/") + cleanPath = path.Clean(cleanPath) + urlPath := "/v1/files/" + strings.TrimPrefix(cleanPath, "/") ft := SignFileToken(urlPath, secret, FileTokenTTL) return urlPath + "?ft=" + ft } +func filepathToURLPath(rawPath string) string { + return strings.ReplaceAll(rawPath, `\`, "/") +} + // fileURLRe matches /v1/files/... and /v1/media/... URLs in markdown and plain text. // Captures the full URL path (stops at whitespace, closing paren, quote, or angle bracket). var fileURLRe = regexp.MustCompile(`(/v1/(?:files|media)/[^\s)"'<>]+)`) diff --git a/internal/mcp/init_retry_test.go b/internal/mcp/init_retry_test.go index 87dc8830..e44095a6 100644 --- a/internal/mcp/init_retry_test.go +++ b/internal/mcp/init_retry_test.go @@ -2,6 +2,7 @@ package mcp import ( "context" + "runtime" "testing" "time" ) @@ -13,8 +14,14 @@ func TestConnectAndDiscoverRetriesOnStdioInitFailure(t *testing.T) { defer cancel() start := time.Now() + command := "cat" + var args []string + if runtime.GOOS == "windows" { + command = "cmd" + args = []string{"/C", "type", "NUL"} + } _, _, err := connectAndDiscover(ctx, "test-retry", "stdio", - "cat", nil, nil, "", nil, 2) + command, args, nil, "", nil, 2) elapsed := time.Since(start) if err == nil { diff --git a/internal/mcp/manager_connect.go b/internal/mcp/manager_connect.go index 4613a0b0..477d2163 100644 --- a/internal/mcp/manager_connect.go +++ b/internal/mcp/manager_connect.go @@ -56,6 +56,7 @@ func connectAndDiscover(ctx context.Context, name, transportType, command string } } if _, err := client.Initialize(ctx, initReq); err == nil { + initErr = nil break } else { initErr = err diff --git a/internal/pipeline/pipeline.go b/internal/pipeline/pipeline.go index 5bf89cd6..ae595ddd 100644 --- a/internal/pipeline/pipeline.go +++ b/internal/pipeline/pipeline.go @@ -110,5 +110,8 @@ func (p *Pipeline) Run(ctx context.Context, state *RunState) (*RunResult, error) result := state.BuildResult() result.Duration = time.Since(start) + if result.Duration <= 0 { + result.Duration = time.Nanosecond + } return result, nil } diff --git a/internal/providers/acp/tool_bridge_test.go b/internal/providers/acp/tool_bridge_test.go index 7f54f74b..c7525fce 100644 --- a/internal/providers/acp/tool_bridge_test.go +++ b/internal/providers/acp/tool_bridge_test.go @@ -111,7 +111,11 @@ func TestResolvePath_Escape_PathTraversal(t *testing.T) { func TestResolvePath_AbsoluteOutsideWorkspace(t *testing.T) { tb, _ := newTestBridge(t) - _, err := tb.resolvePath("/etc/passwd") + outside := filepath.Join(t.TempDir(), "passwd") + if err := os.WriteFile(outside, []byte("secret"), 0644); err != nil { + t.Fatal(err) + } + _, err := tb.resolvePath(outside) if err == nil { t.Error("expected access denied for absolute path outside workspace") } diff --git a/internal/skills/loader.go b/internal/skills/loader.go index dbae7c11..393a4031 100644 --- a/internal/skills/loader.go +++ b/internal/skills/loader.go @@ -78,7 +78,7 @@ func NewLoader(workspace, globalSkills, builtinSkills string) *Loader { // Personal agent skills: ~/.agents/skills/ (matching TS) homeDir, _ := os.UserHomeDir() personalAgentSkills := "" - if homeDir != "" { + if homeDir != "" && os.Getenv("GOCLAW_DISABLE_PERSONAL_SKILLS") != "1" { personalAgentSkills = filepath.Join(homeDir, ".agents", "skills") } diff --git a/internal/skills/loader_test.go b/internal/skills/loader_test.go index 6c4e7788..5b0f4bbf 100644 --- a/internal/skills/loader_test.go +++ b/internal/skills/loader_test.go @@ -8,6 +8,11 @@ import ( "testing" ) +func TestMain(m *testing.M) { + os.Setenv("GOCLAW_DISABLE_PERSONAL_SKILLS", "1") + os.Exit(m.Run()) +} + // makeSkillDir creates a skill directory with a SKILL.md file. func makeSkillDir(t *testing.T, parent, slug, content string) string { t.Helper() @@ -602,10 +607,10 @@ description: plain want: nil, }, { - name: "crlf", + name: "crlf", content: "deps:\r\n - pip:a\r\n - pip:b\r\n", - key: "deps", - want: []string{"pip:a", "pip:b"}, + key: "deps", + want: []string{"pip:a", "pip:b"}, }, { name: "scalar skipped", diff --git a/internal/skills/npm_update_checker_test.go b/internal/skills/npm_update_checker_test.go index 8002bc55..079f66a3 100644 --- a/internal/skills/npm_update_checker_test.go +++ b/internal/skills/npm_update_checker_test.go @@ -4,6 +4,7 @@ import ( "context" "os/exec" "path/filepath" + "runtime" "testing" ) @@ -31,6 +32,9 @@ func useFixtureNpm(t *testing.T) { restoreNpmBinary(t) restoreNpmLookPath(t) npmBinary = filepath.Join("testdata", "npm", "bin", "npm") + if runtime.GOOS == "windows" { + npmBinary += ".cmd" + } npmLookPath = func(string) (string, error) { return npmBinary, nil } } diff --git a/internal/skills/npm_update_executor_test.go b/internal/skills/npm_update_executor_test.go index 7953e9ce..13603278 100644 --- a/internal/skills/npm_update_executor_test.go +++ b/internal/skills/npm_update_executor_test.go @@ -4,9 +4,19 @@ import ( "context" "errors" "os/exec" + "runtime" + "strings" "testing" ) +func setFixtureNpmStderr(t *testing.T, stderr string) { + t.Helper() + if runtime.GOOS == "windows" { + stderr = strings.ReplaceAll(stderr, "\n", " ") + } + t.Setenv("FIXTURE_NPM_STDERR", stderr) +} + // TestNpmExecutor_SourceName verifies the Source() method returns "npm". func TestNpmExecutor_SourceName(t *testing.T) { if got := NewNpmUpdateExecutor().Source(); got != "npm" { @@ -53,7 +63,7 @@ func TestNpmExecutor_Success(t *testing.T) { func TestNpmExecutor_ERESOLVE(t *testing.T) { useFixtureNpm(t) t.Setenv("FIXTURE_NPM_EXIT", "1") - t.Setenv("FIXTURE_NPM_STDERR", "npm ERR! code ERESOLVE\nnpm ERR! peer dep conflict") + setFixtureNpmStderr(t, "npm ERR! code ERESOLVE\nnpm ERR! peer dep conflict") err := NewNpmUpdateExecutor().Update(context.Background(), "typescript", "5.5.0", nil) if err == nil { @@ -69,7 +79,7 @@ func TestNpmExecutor_ERESOLVE(t *testing.T) { func TestNpmExecutor_EACCES(t *testing.T) { useFixtureNpm(t) t.Setenv("FIXTURE_NPM_EXIT", "1") - t.Setenv("FIXTURE_NPM_STDERR", "npm ERR! code EACCES\nnpm ERR! permission denied") + setFixtureNpmStderr(t, "npm ERR! code EACCES\nnpm ERR! permission denied") err := NewNpmUpdateExecutor().Update(context.Background(), "typescript", "5.5.0", nil) if err == nil { @@ -85,7 +95,7 @@ func TestNpmExecutor_EACCES(t *testing.T) { func TestNpmExecutor_404(t *testing.T) { useFixtureNpm(t) t.Setenv("FIXTURE_NPM_EXIT", "1") - t.Setenv("FIXTURE_NPM_STDERR", "npm ERR! code E404\nnpm ERR! 404 Not Found - GET https://registry.npmjs.org/nonexistent") + setFixtureNpmStderr(t, "npm ERR! code E404\nnpm ERR! 404 Not Found - GET https://registry.npmjs.org/nonexistent") err := NewNpmUpdateExecutor().Update(context.Background(), "nonexistent", "1.0.0", nil) if err == nil { @@ -132,6 +142,9 @@ func TestNpmExecutor_ExactVersionArgv(t *testing.T) { // to the subprocess (exec.CommandContext contract). We set npmBinary to a // long-running command and cancel immediately. func TestNpmExecutor_ContextCancel(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("sleep fixture is Unix-specific") + } restoreNpmBinary(t) restoreNpmLookPath(t) diff --git a/internal/skills/pip_update_checker_test.go b/internal/skills/pip_update_checker_test.go index 1d8623e4..695680ce 100644 --- a/internal/skills/pip_update_checker_test.go +++ b/internal/skills/pip_update_checker_test.go @@ -6,6 +6,7 @@ import ( "os/exec" "path/filepath" "runtime" + "strings" "testing" ) @@ -17,7 +18,11 @@ func fixturePip3Path(t *testing.T) string { if !ok { t.Fatal("runtime.Caller failed") } - return filepath.Join(filepath.Dir(file), "testdata", "pip", "bin", "pip3") + p := filepath.Join(filepath.Dir(file), "testdata", "pip", "bin", "pip3") + if runtime.GOOS == "windows" { + p += ".cmd" + } + return p } // setupFixturePip overrides pipBinary and pipLookPath to use the bundled fixture script. @@ -36,6 +41,13 @@ func setupFixturePip(t *testing.T) { // writeExecScript writes a shell script to path and makes it executable. func writeExecScript(t *testing.T, path, content string) { t.Helper() + if runtime.GOOS == "windows" { + if strings.Contains(content, "internal error") { + content = "@echo off\r\necho internal error 1>&2\r\nexit /b 1\r\n" + } else { + content = "@echo off\r\necho []\r\nexit /b 0\r\n" + } + } if err := os.WriteFile(path, []byte(content), 0o755); err != nil { t.Fatalf("writeExecScript: %v", err) } @@ -140,6 +152,9 @@ func TestPipChecker_EmptyResult(t *testing.T) { origLookPath := pipLookPath script := filepath.Join(t.TempDir(), "pip3") + if runtime.GOOS == "windows" { + script += ".cmd" + } writeExecScript(t, script, "#!/bin/sh\necho '[]'\n") pipBinary = script pipLookPath = func(string) (string, error) { return script, nil } @@ -169,6 +184,9 @@ func TestPipChecker_ExecError(t *testing.T) { origLookPath := pipLookPath script := filepath.Join(t.TempDir(), "pip3") + if runtime.GOOS == "windows" { + script += ".cmd" + } writeExecScript(t, script, "#!/bin/sh\necho 'internal error' >&2\nexit 1\n") pipBinary = script pipLookPath = func(string) (string, error) { return script, nil } diff --git a/internal/skills/pip_update_executor_test.go b/internal/skills/pip_update_executor_test.go index f1c2919f..d56ef5a1 100644 --- a/internal/skills/pip_update_executor_test.go +++ b/internal/skills/pip_update_executor_test.go @@ -20,6 +20,9 @@ func setupFixturePipForExecutor(t *testing.T) { t.Fatal("runtime.Caller failed") } fixturePath := filepath.Join(filepath.Dir(file), "testdata", "pip", "bin", "pip3") + if runtime.GOOS == "windows" { + fixturePath += ".cmd" + } origBinary := pipBinary origLookPath := pipLookPath @@ -121,6 +124,15 @@ func TestPipExecutor_PreReleaseFlag(t *testing.T) { " exit 0\n" + "fi\n" + "exit 2\n" + if runtime.GOOS == "windows" { + scriptPath += ".cmd" + script = "@echo off\r\n" + + "if \"%~1\"==\"install\" (\r\n" + + " echo %* >> \"" + argsFile + "\"\r\n" + + " exit /b 0\r\n" + + ")\r\n" + + "exit /b 2\r\n" + } if err := os.WriteFile(scriptPath, []byte(script), 0o755); err != nil { t.Fatalf("write arg-capture script: %v", err) } @@ -171,6 +183,15 @@ func TestPipExecutor_CtxCancel(t *testing.T) { script := "#!/bin/sh\n" + "if [ \"$1\" = \"install\" ]; then sleep 60; exit 0; fi\n" + "exit 2\n" + if runtime.GOOS == "windows" { + scriptPath += ".cmd" + script = "@echo off\r\n" + + "if \"%~1\"==\"install\" (\r\n" + + " powershell -NoProfile -Command \"Start-Sleep -Seconds 60\"\r\n" + + " exit /b 0\r\n" + + ")\r\n" + + "exit /b 2\r\n" + } if err := os.WriteFile(scriptPath, []byte(script), 0o755); err != nil { t.Fatalf("write sleep script: %v", err) } diff --git a/internal/skills/testdata/npm/bin/npm.cmd b/internal/skills/testdata/npm/bin/npm.cmd new file mode 100644 index 00000000..5f8dc19d --- /dev/null +++ b/internal/skills/testdata/npm/bin/npm.cmd @@ -0,0 +1,38 @@ +@echo off +setlocal +set FIXTURE_DIR=%~dp0.. + +if "%~1"=="outdated" ( + if "%FIXTURE_MODE%"=="" set FIXTURE_MODE=outdated + if "%FIXTURE_MODE%"=="outdated" ( + type "%FIXTURE_DIR%\outdated-10.json" + goto exit_outdated + ) + if "%FIXTURE_MODE%"=="error" ( + >&2 echo npm ERR! code ERESOLVE + >&2 echo npm ERR! peer dep conflict + goto exit_outdated + ) + if "%FIXTURE_MODE%"=="ambiguous" goto exit_outdated + if "%FIXTURE_MODE%"=="empty" goto exit_ok + goto exit_bad +) + +if "%~1"=="install" ( + if "%FIXTURE_NPM_EXIT%"=="" set FIXTURE_NPM_EXIT=0 + if not "%FIXTURE_NPM_STDERR%"=="" >&2 echo(%FIXTURE_NPM_STDERR% + exit /b %FIXTURE_NPM_EXIT% +) + +if "%~1"=="cache" goto exit_ok + +goto exit_bad + +:exit_outdated +exit /b 1 + +:exit_ok +exit /b 0 + +:exit_bad +exit /b 2 diff --git a/internal/skills/testdata/pip/bin/pip3.cmd b/internal/skills/testdata/pip/bin/pip3.cmd new file mode 100644 index 00000000..bdaec0ea --- /dev/null +++ b/internal/skills/testdata/pip/bin/pip3.cmd @@ -0,0 +1,30 @@ +@echo off +setlocal enabledelayedexpansion +set FIXTURE_DIR=%~dp0.. + +if "%~1"=="list" if "%~2"=="--outdated" ( + set HAS_PRE=0 + for %%A in (%*) do ( + if "%%~A"=="--pre" set HAS_PRE=1 + ) + if "!HAS_PRE!"=="1" ( + if exist "%FIXTURE_DIR%\outdated-empty.json" ( + type "%FIXTURE_DIR%\outdated-empty.json" + ) else ( + echo [] + ) + ) else ( + type "%FIXTURE_DIR%\outdated-23.3.json" + ) + exit /b 0 +) + +if "%~1"=="install" ( + if "%FIXTURE_PIP_EXIT%"=="" set FIXTURE_PIP_EXIT=0 + if not "%FIXTURE_PIP_STDERR%"=="" >&2 echo %FIXTURE_PIP_STDERR% + exit /b %FIXTURE_PIP_EXIT% +) + +if "%~1"=="cache" exit /b 0 + +exit /b 2 diff --git a/internal/tools/boundary_test.go b/internal/tools/boundary_test.go index fb1a7345..a0337c3c 100644 --- a/internal/tools/boundary_test.go +++ b/internal/tools/boundary_test.go @@ -76,7 +76,11 @@ func TestResolvePath_TraversalBlocked(t *testing.T) { func TestResolvePath_AbsoluteEscapeBlocked(t *testing.T) { ws := setupWorkspace(t) - _, err := resolvePath("/etc/passwd", ws, true) + outside := filepath.Join(t.TempDir(), "passwd") + if err := os.WriteFile(outside, []byte("secret"), 0644); err != nil { + t.Fatal(err) + } + _, err := resolvePath(outside, ws, true) if err == nil { t.Fatal("expected error for absolute path outside workspace, got nil") } @@ -186,12 +190,13 @@ func TestResolvePath_NonExistentFileInWorkspace(t *testing.T) { func TestResolvePath_UnrestrictedAllowsEscape(t *testing.T) { ws := setupWorkspace(t) // restrict=false should allow any path - resolved, err := resolvePath("/etc/hosts", ws, false) + outside := filepath.Join(t.TempDir(), "hosts") + resolved, err := resolvePath(outside, ws, false) if err != nil { t.Fatalf("expected success with restrict=false, got: %v", err) } - if resolved != "/etc/hosts" { - t.Fatalf("expected /etc/hosts, got: %s", resolved) + if resolved != filepath.Clean(outside) { + t.Fatalf("expected %s, got: %s", filepath.Clean(outside), resolved) } } @@ -322,15 +327,19 @@ func TestResolvePathWithAllowed_TeamWorkspaceAccess(t *testing.T) { } func TestIsPathInside(t *testing.T) { + parent := filepath.Join(t.TempDir(), "a", "b") + child := filepath.Join(parent, "c") + sibling := parent + "c" + other := filepath.Join(t.TempDir(), "a", "b") tests := []struct { child, parent string want bool }{ - {"/a/b/c", "/a/b", true}, - {"/a/b", "/a/b", true}, - {"/a/bc", "/a/b", false}, // not a child, just prefix match - {"/a", "/a/b", false}, - {"/x/y", "/a/b", false}, + {child, parent, true}, + {parent, parent, true}, + {sibling, parent, false}, // not a child, just prefix match + {filepath.Dir(parent), parent, false}, + {other, parent, false}, } for _, tt := range tests { got := isPathInside(tt.child, tt.parent) @@ -348,11 +357,11 @@ func TestIsPathInside_WindowsCaseInsensitive(t *testing.T) { child, parent string want bool }{ - {`C:\Workspace\file.txt`, `c:\workspace`, true}, // case mismatch - {`c:\workspace\file.txt`, `C:\Workspace`, true}, // reverse case - {`C:\WORKSPACE\SUB\FILE`, `c:\workspace`, true}, // all caps child - {`D:\other`, `C:\workspace`, false}, // different drive - {`C:\workspaceX\file.txt`, `C:\workspace`, false}, // prefix but not child + {`C:\Workspace\file.txt`, `c:\workspace`, true}, // case mismatch + {`c:\workspace\file.txt`, `C:\Workspace`, true}, // reverse case + {`C:\WORKSPACE\SUB\FILE`, `c:\workspace`, true}, // all caps child + {`D:\other`, `C:\workspace`, false}, // different drive + {`C:\workspaceX\file.txt`, `C:\workspace`, false}, // prefix but not child } for _, tt := range tests { got := isPathInside(tt.child, tt.parent) diff --git a/internal/tools/sandbox_utils.go b/internal/tools/sandbox_utils.go index e1ccd1a1..94e45dbb 100644 --- a/internal/tools/sandbox_utils.go +++ b/internal/tools/sandbox_utils.go @@ -3,6 +3,7 @@ package tools import ( "context" "fmt" + "path" "path/filepath" "strings" ) @@ -30,16 +31,16 @@ func SandboxCwd(ctx context.Context, globalWorkspace, containerBase string) (str if rel == "." { return containerBase, nil } - return filepath.Join(containerBase, rel), nil + return path.Join(filepath.ToSlash(containerBase), filepath.ToSlash(rel)), nil } // ResolveSandboxPath resolves a tool-provided path (relative or absolute) // against the sandbox container CWD. If the path is relative, it is joined // with containerCwd. Absolute paths are returned as-is (the sandbox // filesystem already restricts access to the mounted volume). -func ResolveSandboxPath(path, containerCwd string) string { - if filepath.IsAbs(path) { - return path +func ResolveSandboxPath(filePath, containerCwd string) string { + if strings.HasPrefix(filePath, "/") { + return filePath } - return filepath.Join(containerCwd, path) + return path.Join(containerCwd, filePath) } diff --git a/internal/tools/shell.go b/internal/tools/shell.go index 13cafa7f..ccc4de6f 100644 --- a/internal/tools/shell.go +++ b/internal/tools/shell.go @@ -9,6 +9,7 @@ import ( "maps" "os" "os/exec" + "path/filepath" "regexp" "runtime" "strings" @@ -114,8 +115,18 @@ func (t *ExecTool) SetSandboxKey(key string) {} // These are NOT configurable via deny groups — they always apply regardless of group config. func (t *ExecTool) DenyPaths(paths ...string) { for _, p := range paths { - escaped := regexp.QuoteMeta(p) - t.pathDenyPatterns = append(t.pathDenyPatterns, regexp.MustCompile(escaped)) + seen := make(map[string]struct{}, 3) + for _, variant := range []string{p, filepath.ToSlash(p), filepath.FromSlash(p)} { + if variant == "" { + continue + } + if _, ok := seen[variant]; ok { + continue + } + seen[variant] = struct{}{} + escaped := regexp.QuoteMeta(variant) + t.pathDenyPatterns = append(t.pathDenyPatterns, regexp.MustCompile(escaped)) + } t.pathDenyRoots = append(t.pathDenyRoots, p) } } diff --git a/internal/tools/shell_credentialed_gate_test.go b/internal/tools/shell_credentialed_gate_test.go index f815f8b4..57bc36ed 100644 --- a/internal/tools/shell_credentialed_gate_test.go +++ b/internal/tools/shell_credentialed_gate_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "runtime" "strings" "sync" "testing" @@ -323,7 +324,11 @@ func TestExec_RejectsWrapperDepthCap(t *testing.T) { func TestExec_AllowsShellWrapperWithUnregisteredInner(t *testing.T) { tool, _, ctx := newGateTestTool(t) // registered empty, inner is echo (not registered) → fall-through. - result := tool.Execute(ctx, map[string]any{"command": "sh -c 'echo hi'"}) + command := "sh -c 'echo hi'" + if runtime.GOOS == "windows" { + command = "cmd /c echo hi" + } + result := tool.Execute(ctx, map[string]any{"command": command}) if result.IsError { t.Fatalf("expected pass-through when inner unregistered, got error: %s", result.ForLLM) } @@ -394,7 +399,11 @@ func TestExec_FallThrough_ScrubsGHToken(t *testing.T) { t.Setenv("GH_TOKEN", "supersecretvalue") // Use single-quote printf so shell sees the literal; our gate lets "sh" // fall through (sh is not registered, echo is not registered). - result := tool.Execute(ctx, map[string]any{"command": `sh -c 'echo "token=$GH_TOKEN"'`}) + command := `sh -c 'echo "token=$GH_TOKEN"'` + if runtime.GOOS == "windows" { + command = `cmd /c echo token=%GH_TOKEN%` + } + result := tool.Execute(ctx, map[string]any{"command": command}) if result.IsError { t.Fatalf("expected pass-through, got: %s", result.ForLLM) } diff --git a/internal/tools/shell_deny_test.go b/internal/tools/shell_deny_test.go index d32a58ab..7017875e 100644 --- a/internal/tools/shell_deny_test.go +++ b/internal/tools/shell_deny_test.go @@ -343,7 +343,7 @@ func TestPathExemptions(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - normalizedCmd := strings.ReplaceAll(normalizeCommand(tc.cmd), "/app/data", dataDir) + normalizedCmd := strings.ReplaceAll(normalizeCommand(tc.cmd), "/app/data", filepath.ToSlash(dataDir)) denied := false for _, pattern := range allPatterns { if !pattern.MatchString(normalizedCmd) { @@ -433,10 +433,10 @@ func TestExecute_AllowsCurrentWorkspaceNestedUnderDeniedRoot(t *testing.T) { tool := NewExecTool("/workspace", false) tool.DenyPaths(dataDir) - target := filepath.Join(workspace, ".uploads", "report.png") + target := filepath.ToSlash(filepath.Join(workspace, ".uploads", "report.png")) ctx := WithToolWorkspace(context.Background(), workspace) result := tool.Execute(ctx, map[string]any{ - "command": "printf '%s' " + target, + "command": "echo " + target, }) if strings.Contains(result.ForLLM, "command denied by safety policy") { @@ -456,11 +456,11 @@ func TestExecute_AllowsCurrentTeamWorkspaceNestedUnderDeniedRoot(t *testing.T) { tool := NewExecTool("/workspace", false) tool.DenyPaths(dataDir) - target := filepath.Join(teamWorkspace, "report.png") + target := filepath.ToSlash(filepath.Join(teamWorkspace, "report.png")) ctx := WithToolWorkspace(context.Background(), t.TempDir()) ctx = WithToolTeamWorkspace(ctx, teamWorkspace) result := tool.Execute(ctx, map[string]any{ - "command": "printf '%s' " + target, + "command": "echo " + target, }) if strings.Contains(result.ForLLM, "command denied by safety policy") { @@ -519,18 +519,19 @@ func TestExecute_AllowsQuotedAndPrefixedUploadArguments(t *testing.T) { if err := os.WriteFile(target, []byte("ok"), 0644); err != nil { t.Fatalf("WriteFile() error = %v", err) } + commandTarget := filepath.ToSlash(target) tool := NewExecTool("/workspace", false) tool.DenyPaths(dataDir) ctx := WithToolWorkspace(context.Background(), workspace) result := tool.Execute(ctx, map[string]any{ - "command": "printf '%s' file=@\"" + target + "\"", + "command": "echo file=@\"" + commandTarget + "\"", }) if strings.Contains(result.ForLLM, "command denied by safety policy") { t.Fatalf("expected quoted/prefixed upload argument to bypass deny, got: %s", result.ForLLM) } - if !strings.Contains(result.ForLLM, "file=@"+target) { + if !strings.Contains(result.ForLLM, "file=@") || !strings.Contains(result.ForLLM, commandTarget) { t.Fatalf("expected output to contain prefixed path, got: %s", result.ForLLM) } } @@ -556,7 +557,7 @@ func TestExecute_DoesNotExemptSymlinkEscapeInsideTeamWorkspace(t *testing.T) { linkPath := filepath.Join(teamWorkspace, "leak.txt") if err := os.Symlink(protected, linkPath); err != nil { - t.Fatalf("Symlink() error = %v", err) + t.Skipf("Symlink() unavailable: %v", err) } tool := NewExecTool("/workspace", false) @@ -584,13 +585,14 @@ func TestExecute_AllowsLegacyWorkspaceUploadsLayout(t *testing.T) { if err := os.WriteFile(target, []byte("ok"), 0644); err != nil { t.Fatalf("WriteFile() error = %v", err) } + commandTarget := filepath.ToSlash(target) tool := NewExecTool("/workspace", false) tool.DenyPaths(dataDir, ".goclaw/") ctx := WithToolWorkspace(context.Background(), workspace) result := tool.Execute(ctx, map[string]any{ - "command": "cp \"" + target + "\" /tmp/partner.png", + "command": "cp \"" + commandTarget + "\" /tmp/partner.png", }) if strings.Contains(result.ForLLM, "command denied by safety policy") { diff --git a/internal/tools/shell_path_exemption.go b/internal/tools/shell_path_exemption.go index 36466ed8..20607b20 100644 --- a/internal/tools/shell_path_exemption.go +++ b/internal/tools/shell_path_exemption.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "runtime" "strings" shellwords "github.com/mattn/go-shellwords" @@ -51,18 +52,35 @@ func (t *ExecTool) dynamicPathExemptions(ctx context.Context) []string { // On the claw server, /app/workspace is symlinked to /app/.goclaw at runtime, // so both forms may appear in LLM-generated commands for the same physical path. func pathAliasVariants(path string) []string { - variants := []string{path} + variants := make([]string, 0, 3) + seen := make(map[string]struct{}, 3) + appendVariant := func(v string) { + if v == "" { + return + } + if _, ok := seen[v]; ok { + return + } + seen[v] = struct{}{} + variants = append(variants, v) + } + appendVariant(path) + pathSlash := filepath.ToSlash(path) for _, mapping := range [][2]string{ {"/app/workspace", "/app/.goclaw"}, {"/app/.goclaw", "/app/workspace"}, } { from, to := mapping[0], mapping[1] - if path == from { - variants = append(variants, to) - continue + var mapped string + if pathSlash == from { + mapped = to + } else if strings.HasPrefix(pathSlash, from+"/") { + mapped = to + strings.TrimPrefix(pathSlash, from) } - if strings.HasPrefix(path, from+string(filepath.Separator)) { - variants = append(variants, to+strings.TrimPrefix(path, from)) + if mapped != "" { + appendVariant(mapped) + appendVariant(filepath.FromSlash(mapped)) + continue } } return variants @@ -72,22 +90,24 @@ func pathAliasVariants(path string) []string { // deny roots. Supports both absolute roots (prefix match) and relative roots // (e.g. ".goclaw/" — checked as a path component marker anywhere in path). func (t *ExecTool) isNestedUnderDeniedRoot(path string) bool { + pathClean := filepath.ToSlash(filepath.Clean(path)) + pathWithBoundary := "/" + strings.Trim(pathClean, "/") + "/" for _, root := range t.pathDenyRoots { - cleanRoot := filepath.Clean(root) - if cleanRoot == "." || cleanRoot == string(filepath.Separator) { + cleanRoot := filepath.ToSlash(filepath.Clean(root)) + if cleanRoot == "." || cleanRoot == "/" { continue } - if !filepath.IsAbs(cleanRoot) { - marker := string(filepath.Separator) + cleanRoot + string(filepath.Separator) - if strings.Contains(path, marker) { + if !filepath.IsAbs(root) && !strings.HasPrefix(cleanRoot, "/") { + marker := "/" + strings.Trim(cleanRoot, "/") + "/" + if strings.Contains(pathWithBoundary, marker) { return true } continue } - if path == cleanRoot { + if equalPathString(pathClean, cleanRoot) { continue } - if strings.HasPrefix(path, cleanRoot+string(filepath.Separator)) { + if hasPathPrefix(pathClean, cleanRoot) { return true } } @@ -96,27 +116,48 @@ func (t *ExecTool) isNestedUnderDeniedRoot(path string) bool { // matchesPathExemption checks if a resolved path falls under any exemption prefix. func matchesPathExemption(path string, exemptions []string) bool { - sep := string(filepath.Separator) + path = normalizePathForMatch(path) for _, ex := range exemptions { if ex == "" { continue } - if path == ex { + ex = normalizePathForMatch(ex) + if equalPathString(path, ex) { return true } - if strings.HasSuffix(ex, sep) { - if strings.HasPrefix(path, ex) { - return true - } - continue - } - if strings.HasPrefix(path, ex+sep) { + if hasPathPrefix(path, ex) { return true } } return false } +func normalizePathForMatch(path string) string { + clean := filepath.ToSlash(filepath.Clean(path)) + if clean != "/" { + clean = strings.TrimRight(clean, "/") + } + return clean +} + +func equalPathString(a, b string) bool { + if runtime.GOOS == "windows" { + return strings.EqualFold(a, b) + } + return a == b +} + +func hasPathPrefix(path, prefix string) bool { + if equalPathString(path, prefix) { + return true + } + if runtime.GOOS == "windows" { + path = strings.ToLower(path) + prefix = strings.ToLower(prefix) + } + return strings.HasPrefix(path, prefix+"/") +} + // parseExecCommandWords splits a shell command into words using go-shellwords, // handling quoted strings correctly. The command is first segmented by shell // operators (;|&<>) to avoid cross-segment quoting confusion. @@ -235,7 +276,8 @@ func looksLikePathCandidate(s string) bool { strings.HasPrefix(s, "teams/") || strings.HasPrefix(s, "tenants/") || strings.HasPrefix(s, "~/") || - strings.Contains(s, string(filepath.Separator)) + strings.Contains(s, "/") || + strings.Contains(s, `\`) } // canonicalizeExecPath resolves a path to its canonical absolute form,