diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e8905a7..83e4dcb5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,13 @@ All notable changes to GoClaw are documented here. For full documentation, see [ now prefer `gpt-5.5`, with reasoning metadata and context-window defaults updated for provider-first model selection. +### Fixed + +- **SecureCLI runtime npm binaries** — binary discovery and credentialed exec now + resolve tools installed under the GoClaw runtime directories, including + `{runtimeDir}/npm-global/bin`, and support single-binary npm package aliases + such as `openrouter-cli` exposing `orc`. + ### Breaking Changes - **Context pruning now opt-in.** Previously tool-result trimming ran by default diff --git a/internal/http/secure_cli.go b/internal/http/secure_cli.go index 0eac1b7f..5aa05a5c 100644 --- a/internal/http/secure_cli.go +++ b/internal/http/secure_cli.go @@ -15,6 +15,7 @@ import ( "github.com/nextlevelbuilder/goclaw/internal/bus" "github.com/nextlevelbuilder/goclaw/internal/i18n" "github.com/nextlevelbuilder/goclaw/internal/permissions" + "github.com/nextlevelbuilder/goclaw/internal/skills" "github.com/nextlevelbuilder/goclaw/internal/store" "github.com/nextlevelbuilder/goclaw/internal/tools" "github.com/nextlevelbuilder/goclaw/pkg/protocol" @@ -151,17 +152,17 @@ func (h *SecureCLIHandler) handleList(w http.ResponseWriter, r *http.Request) { // secureCLICreateRequest supports both preset-based and custom creation. type secureCLICreateRequest struct { - Preset string `json:"preset,omitempty"` // auto-fill from preset - BinaryName string `json:"binary_name"` - BinaryPath *string `json:"binary_path,omitempty"` - Description string `json:"description"` - Env map[string]string `json:"env"` // plaintext env vars (encrypted by store) - DenyArgs json.RawMessage `json:"deny_args,omitempty"` - DenyVerbose json.RawMessage `json:"deny_verbose,omitempty"` - TimeoutSeconds int `json:"timeout_seconds,omitempty"` - Tips string `json:"tips,omitempty"` - IsGlobal *bool `json:"is_global,omitempty"` - Enabled bool `json:"enabled"` + Preset string `json:"preset,omitempty"` // auto-fill from preset + BinaryName string `json:"binary_name"` + BinaryPath *string `json:"binary_path,omitempty"` + Description string `json:"description"` + Env map[string]string `json:"env"` // plaintext env vars (encrypted by store) + DenyArgs json.RawMessage `json:"deny_args,omitempty"` + DenyVerbose json.RawMessage `json:"deny_verbose,omitempty"` + TimeoutSeconds int `json:"timeout_seconds,omitempty"` + Tips string `json:"tips,omitempty"` + IsGlobal *bool `json:"is_global,omitempty"` + Enabled bool `json:"enabled"` } func (h *SecureCLIHandler) handleCreate(w http.ResponseWriter, r *http.Request) { @@ -371,6 +372,10 @@ func (h *SecureCLIHandler) handleCheckBinary(w http.ResponseWriter, r *http.Requ } absPath, err := exec.LookPath(req.BinaryName) if err != nil { + if runtimePath, ok := skills.FindRuntimeExecutable(req.BinaryName); ok { + writeJSON(w, http.StatusOK, map[string]any{"found": true, "path": runtimePath}) + return + } writeJSON(w, http.StatusOK, map[string]any{"found": false, "error": fmt.Sprintf("binary %q not found in PATH", req.BinaryName)}) return } diff --git a/internal/http/secure_cli_check_binary_test.go b/internal/http/secure_cli_check_binary_test.go new file mode 100644 index 00000000..ac700b54 --- /dev/null +++ b/internal/http/secure_cli_check_binary_test.go @@ -0,0 +1,98 @@ +package http + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestSecureCLICheckBinaryFindsRuntimeNpmBinary(t *testing.T) { + runtimeDir := t.TempDir() + t.Setenv("RUNTIME_DIR", runtimeDir) + t.Setenv("NPM_CONFIG_PREFIX", "") + t.Setenv("PATH", "/usr/bin") + + binDir := filepath.Join(runtimeDir, "npm-global", "bin") + if err := os.MkdirAll(binDir, 0o755); err != nil { + t.Fatal(err) + } + wantPath := filepath.Join(binDir, "openrouter") + if err := os.WriteFile(wantPath, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + + req := httptest.NewRequest(http.MethodPost, "/v1/cli-credentials/check-binary", strings.NewReader(`{"binary_name":"openrouter"}`)) + rec := httptest.NewRecorder() + + NewSecureCLIHandler(nil, nil).handleCheckBinary(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, body = %s", rec.Code, rec.Body.String()) + } + var got struct { + Found bool `json:"found"` + Path string `json:"path"` + Error string `json:"error"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatal(err) + } + if !got.Found { + t.Fatalf("found = false, error = %q", got.Error) + } + if got.Path != wantPath { + t.Fatalf("path = %q, want %q", got.Path, wantPath) + } +} + +func TestSecureCLICheckBinaryFindsNpmPackageCliAlias(t *testing.T) { + runtimeDir := t.TempDir() + t.Setenv("RUNTIME_DIR", runtimeDir) + t.Setenv("NPM_CONFIG_PREFIX", "") + t.Setenv("PATH", "/usr/bin") + + pkgDir := filepath.Join(runtimeDir, "npm-global", "lib", "node_modules", "openrouter-cli") + if err := os.MkdirAll(pkgDir, 0o755); err != nil { + t.Fatal(err) + } + manifest := []byte(`{"name":"openrouter-cli","bin":{"orc":"dist/index.js"}}`) + if err := os.WriteFile(filepath.Join(pkgDir, "package.json"), manifest, 0o644); err != nil { + t.Fatal(err) + } + + binDir := filepath.Join(runtimeDir, "npm-global", "bin") + if err := os.MkdirAll(binDir, 0o755); err != nil { + t.Fatal(err) + } + wantPath := filepath.Join(binDir, "orc") + if err := os.WriteFile(wantPath, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + + req := httptest.NewRequest(http.MethodPost, "/v1/cli-credentials/check-binary", strings.NewReader(`{"binary_name":"openrouter"}`)) + rec := httptest.NewRecorder() + + NewSecureCLIHandler(nil, nil).handleCheckBinary(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, body = %s", rec.Code, rec.Body.String()) + } + var got struct { + Found bool `json:"found"` + Path string `json:"path"` + Error string `json:"error"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatal(err) + } + if !got.Found { + t.Fatalf("found = false, error = %q", got.Error) + } + if got.Path != wantPath { + t.Fatalf("path = %q, want %q", got.Path, wantPath) + } +} diff --git a/internal/skills/npm_executable_aliases.go b/internal/skills/npm_executable_aliases.go new file mode 100644 index 00000000..b9dbcf83 --- /dev/null +++ b/internal/skills/npm_executable_aliases.go @@ -0,0 +1,113 @@ +package skills + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" +) + +type npmPackageBinManifest struct { + Name string `json:"name"` + Bin json.RawMessage `json:"bin"` +} + +func findNpmPackageExecutableAlias(name string) (string, bool) { + packageDirs, err := npmGlobalPackageDirs() + if err != nil { + return "", false + } + for _, packageDir := range packageDirs { + raw, err := os.ReadFile(filepath.Join(packageDir, "package.json")) + if err != nil { + continue + } + var manifest npmPackageBinManifest + if err := json.Unmarshal(raw, &manifest); err != nil { + continue + } + if !npmPackageNameMatchesExecutableAlias(manifest.Name, name) { + continue + } + binName, ok := singleNpmBinName(manifest) + if !ok { + continue + } + path := filepath.Join(npmGlobalBinDir(), binName) + if IsExecutableFile(path) { + return path, true + } + } + return "", false +} + +func npmGlobalPackageDirs() ([]string, error) { + root := filepath.Join(npmGlobalPrefix(), "lib", "node_modules") + entries, err := os.ReadDir(root) + if err != nil { + return nil, err + } + dirs := make([]string, 0, len(entries)) + for _, entry := range entries { + if !entry.IsDir() { + continue + } + path := filepath.Join(root, entry.Name()) + if strings.HasPrefix(entry.Name(), "@") { + scoped, err := os.ReadDir(path) + if err != nil { + continue + } + for _, scopedEntry := range scoped { + if scopedEntry.IsDir() { + dirs = append(dirs, filepath.Join(path, scopedEntry.Name())) + } + } + continue + } + dirs = append(dirs, path) + } + return dirs, nil +} + +func npmPackageNameMatchesExecutableAlias(packageName, executableName string) bool { + base := npmPackageBaseName(packageName) + if base == "" { + return false + } + if base == executableName { + return true + } + return strings.TrimSuffix(base, "-cli") == executableName +} + +func npmPackageBaseName(packageName string) string { + packageName = strings.TrimSpace(packageName) + if packageName == "" { + return "" + } + if slash := strings.LastIndexByte(packageName, '/'); slash >= 0 { + return packageName[slash+1:] + } + return packageName +} + +func singleNpmBinName(manifest npmPackageBinManifest) (string, bool) { + if len(manifest.Bin) == 0 || string(manifest.Bin) == "null" { + return "", false + } + var binPath string + if err := json.Unmarshal(manifest.Bin, &binPath); err == nil { + return npmPackageBaseName(manifest.Name), strings.TrimSpace(binPath) != "" + } + var bins map[string]string + if err := json.Unmarshal(manifest.Bin, &bins); err != nil || len(bins) != 1 { + return "", false + } + for name, path := range bins { + if strings.TrimSpace(name) != "" && strings.TrimSpace(path) != "" { + return name, true + } + } + return "", false +} diff --git a/internal/skills/package_runtime_paths.go b/internal/skills/package_runtime_paths.go index 7ecd91bb..47f51474 100644 --- a/internal/skills/package_runtime_paths.go +++ b/internal/skills/package_runtime_paths.go @@ -34,6 +34,41 @@ func npmGlobalBinDir() string { return filepath.Join(npmGlobalPrefix(), "bin") } +func RuntimeExecutableDirs() []string { + return uniqueNonEmptyPaths( + filepath.Join(packageRuntimeDir(), "bin"), + npmGlobalBinDir(), + filepath.Join(packageRuntimeDir(), "pip", "bin"), + ) +} + +func FindRuntimeExecutable(name string) (string, bool) { + if strings.ContainsAny(name, `/\`) { + return "", false + } + for _, dir := range RuntimeExecutableDirs() { + path := filepath.Join(dir, name) + if IsExecutableFile(path) { + return path, true + } + } + if path, ok := findNpmPackageExecutableAlias(name); ok { + return path, true + } + return "", false +} + +func IsExecutableFile(path string) bool { + info, err := os.Stat(path) + if err != nil || info.IsDir() { + return false + } + if runtime.GOOS == "windows" { + return true + } + return info.Mode().Perm()&0111 != 0 +} + func npmGlobalNodePath() string { return filepath.Join(npmGlobalPrefix(), "lib", "node_modules") } @@ -92,3 +127,17 @@ func prependPathValue(current, dir string) string { } return dir + string(os.PathListSeparator) + current } + +func uniqueNonEmptyPaths(paths ...string) []string { + seen := make(map[string]bool, len(paths)) + out := make([]string, 0, len(paths)) + for _, path := range paths { + path = strings.TrimSpace(path) + if path == "" || seen[path] { + continue + } + seen[path] = true + out = append(out, path) + } + return out +} diff --git a/internal/skills/package_runtime_paths_test.go b/internal/skills/package_runtime_paths_test.go index 45ea7342..774eef52 100644 --- a/internal/skills/package_runtime_paths_test.go +++ b/internal/skills/package_runtime_paths_test.go @@ -44,6 +44,67 @@ func TestEnsureNpmGlobalEnvPrependsProcessPath(t *testing.T) { } } +func TestFindRuntimeExecutableFindsNpmGlobalBinary(t *testing.T) { + runtimeDir := t.TempDir() + t.Setenv("RUNTIME_DIR", runtimeDir) + t.Setenv("NPM_CONFIG_PREFIX", "") + + binDir := npmGlobalBinDir() + if err := os.MkdirAll(binDir, 0o755); err != nil { + t.Fatal(err) + } + wantPath := filepath.Join(binDir, "openrouter") + if err := os.WriteFile(wantPath, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + + got, ok := FindRuntimeExecutable("openrouter") + if !ok { + t.Fatalf("FindRuntimeExecutable did not find npm global binary") + } + if got != wantPath { + t.Fatalf("FindRuntimeExecutable path = %q, want %q", got, wantPath) + } +} + +func TestFindRuntimeExecutableFindsNpmPackageCliAlias(t *testing.T) { + runtimeDir := t.TempDir() + t.Setenv("RUNTIME_DIR", runtimeDir) + t.Setenv("NPM_CONFIG_PREFIX", "") + + pkgDir := filepath.Join(runtimeDir, "npm-global", "lib", "node_modules", "openrouter-cli") + if err := os.MkdirAll(pkgDir, 0o755); err != nil { + t.Fatal(err) + } + manifest := []byte(`{"name":"openrouter-cli","bin":{"orc":"dist/index.js"}}`) + if err := os.WriteFile(filepath.Join(pkgDir, "package.json"), manifest, 0o644); err != nil { + t.Fatal(err) + } + + binDir := npmGlobalBinDir() + if err := os.MkdirAll(binDir, 0o755); err != nil { + t.Fatal(err) + } + wantPath := filepath.Join(binDir, "orc") + if err := os.WriteFile(wantPath, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + + got, ok := FindRuntimeExecutable("openrouter") + if !ok { + t.Fatalf("FindRuntimeExecutable did not find npm package CLI alias") + } + if got != wantPath { + t.Fatalf("FindRuntimeExecutable alias path = %q, want %q", got, wantPath) + } +} + +func TestFindRuntimeExecutableRejectsPathLikeNames(t *testing.T) { + if got, ok := FindRuntimeExecutable("../openrouter"); ok || got != "" { + t.Fatalf("FindRuntimeExecutable path-like name = %q, %v; want empty false", got, ok) + } +} + func envContainsExact(env []string, want string) bool { for _, item := range env { if item == want { diff --git a/internal/tools/credentialed_exec.go b/internal/tools/credentialed_exec.go index 523e7832..b811ca22 100644 --- a/internal/tools/credentialed_exec.go +++ b/internal/tools/credentialed_exec.go @@ -21,6 +21,7 @@ import ( shellwords "github.com/mattn/go-shellwords" "github.com/nextlevelbuilder/goclaw/internal/sandbox" + "github.com/nextlevelbuilder/goclaw/internal/skills" "github.com/nextlevelbuilder/goclaw/internal/store" ) @@ -263,17 +264,33 @@ func detectShellOperators(command string) []string { } // resolveAndMatchBinary resolves a binary name to an absolute path and -// optionally verifies it matches the stored config path. This prevents -// binary spoofing (e.g. ./gh in workspace instead of /usr/bin/gh). +// verifies any stored path matches either the command binary or a known runtime +// package alias (for example openrouter-cli -> orc). func resolveAndMatchBinary(binaryName string, configPath *string) (string, error) { + if configPath != nil && strings.TrimSpace(*configPath) != "" { + expectedPath := strings.TrimSpace(*configPath) + if !filepath.IsAbs(expectedPath) { + return "", fmt.Errorf("configured binary path must be absolute: %q", expectedPath) + } + if !skills.IsExecutableFile(expectedPath) { + return "", fmt.Errorf("configured binary path %q is not executable", expectedPath) + } + if normalizeBinaryName(expectedPath) == normalizeBinaryName(binaryName) { + return expectedPath, nil + } + if runtimePath, ok := skills.FindRuntimeExecutable(binaryName); ok && runtimePath == expectedPath { + return expectedPath, nil + } + return "", fmt.Errorf("binary path mismatch: command uses %q but config expects %q", binaryName, expectedPath) + } + absPath, err := exec.LookPath(binaryName) if err != nil { + if runtimePath, ok := skills.FindRuntimeExecutable(binaryName); ok { + return runtimePath, nil + } return "", fmt.Errorf("binary %q not found in PATH: %w", binaryName, err) } - // If config specifies an absolute path, verify it matches - if configPath != nil && *configPath != "" && absPath != *configPath { - return "", fmt.Errorf("binary path mismatch: resolved %q but config expects %q", absPath, *configPath) - } return absPath, nil } diff --git a/internal/tools/credentialed_exec_test.go b/internal/tools/credentialed_exec_test.go index 209f27af..484d3dbe 100644 --- a/internal/tools/credentialed_exec_test.go +++ b/internal/tools/credentialed_exec_test.go @@ -2,6 +2,8 @@ package tools import ( "encoding/json" + "os" + "path/filepath" "testing" ) @@ -205,3 +207,90 @@ func TestMatchesBinaryDenyJoinedArgs(t *testing.T) { }) } } + +func TestResolveAndMatchBinaryUsesConfiguredExecutablePath(t *testing.T) { + t.Setenv("PATH", "/usr/bin") + binDir := t.TempDir() + binaryPath := filepath.Join(binDir, "openrouter") + if err := os.WriteFile(binaryPath, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + + got, err := resolveAndMatchBinary("openrouter", &binaryPath) + if err != nil { + t.Fatalf("resolveAndMatchBinary returned error: %v", err) + } + if got != binaryPath { + t.Fatalf("path = %q, want %q", got, binaryPath) + } +} + +func TestResolveAndMatchBinaryAllowsConfiguredAliasPath(t *testing.T) { + runtimeDir := t.TempDir() + t.Setenv("RUNTIME_DIR", runtimeDir) + t.Setenv("NPM_CONFIG_PREFIX", "") + t.Setenv("PATH", "/usr/bin") + + pkgDir := filepath.Join(runtimeDir, "npm-global", "lib", "node_modules", "openrouter-cli") + if err := os.MkdirAll(pkgDir, 0o755); err != nil { + t.Fatal(err) + } + manifest := []byte(`{"name":"openrouter-cli","bin":{"orc":"dist/index.js"}}`) + if err := os.WriteFile(filepath.Join(pkgDir, "package.json"), manifest, 0o644); err != nil { + t.Fatal(err) + } + + binDir := filepath.Join(runtimeDir, "npm-global", "bin") + if err := os.MkdirAll(binDir, 0o755); err != nil { + t.Fatal(err) + } + binaryPath := filepath.Join(binDir, "orc") + if err := os.WriteFile(binaryPath, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + + got, err := resolveAndMatchBinary("openrouter", &binaryPath) + if err != nil { + t.Fatalf("resolveAndMatchBinary returned error: %v", err) + } + if got != binaryPath { + t.Fatalf("path = %q, want %q", got, binaryPath) + } +} + +func TestResolveAndMatchBinaryRejectsArbitraryConfiguredPath(t *testing.T) { + t.Setenv("PATH", "/usr/bin") + binDir := t.TempDir() + binaryPath := filepath.Join(binDir, "sh") + if err := os.WriteFile(binaryPath, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + + if _, err := resolveAndMatchBinary("openrouter", &binaryPath); err == nil { + t.Fatalf("resolveAndMatchBinary accepted arbitrary mismatched path") + } +} + +func TestResolveAndMatchBinaryFallsBackToRuntimeExecutableDirs(t *testing.T) { + runtimeDir := t.TempDir() + t.Setenv("RUNTIME_DIR", runtimeDir) + t.Setenv("NPM_CONFIG_PREFIX", "") + t.Setenv("PATH", "/usr/bin") + + binDir := filepath.Join(runtimeDir, "npm-global", "bin") + if err := os.MkdirAll(binDir, 0o755); err != nil { + t.Fatal(err) + } + binaryPath := filepath.Join(binDir, "openrouter") + if err := os.WriteFile(binaryPath, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + + got, err := resolveAndMatchBinary("openrouter", nil) + if err != nil { + t.Fatalf("resolveAndMatchBinary returned error: %v", err) + } + if got != binaryPath { + t.Fatalf("path = %q, want %q", got, binaryPath) + } +}