Merge pull request #25 from digitopvn/codex/securecli-openrouter-alias

fix(secure-cli): resolve runtime npm binary aliases

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
 }
 

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)
+	}
+}