diff --git a/pkg/client/llm_gateway_credential_helper.go b/pkg/client/llm_gateway_credential_helper.go index 7254cd11dd..6bfe71cca2 100644 --- a/pkg/client/llm_gateway_credential_helper.go +++ b/pkg/client/llm_gateway_credential_helper.go @@ -61,13 +61,6 @@ type claudeDesktopConfig struct { // key ToolHive does not own, so a user's other saved configurations are left // intact. Writes are crash-safe via AtomicWriteFile. func (cm *ClientManager) configureCredentialHelper(appCfg *clientAppConfig, cfg llmgateway.ApplyConfig) (string, error) { - if runtime.GOOS == "windows" { - // The shim is a POSIX /bin/sh script — consistent with the rest of the - // LLM gateway token-helper feature, which is POSIX-only (see - // tokenHelperShellCommand in pkg/llm). Windows support is a follow-up. - return "", fmt.Errorf("claude-desktop LLM gateway setup is not supported on Windows yet") - } - metaPath := cm.buildLLMSettingsPath(appCfg) dir := filepath.Dir(metaPath) if err := os.MkdirAll(dir, 0o700); err != nil { @@ -227,9 +220,16 @@ func (cm *ClientManager) revertCredentialHelper(appCfg *clientAppConfig, configP }) } +func credentialHelperShimName() string { + if runtime.GOOS == "windows" { + return "claude-desktop-helper.cmd" + } + return "claude-desktop-helper.sh" +} + // credentialHelperShimPath is the fixed location of the generated shim. func (cm *ClientManager) credentialHelperShimPath() string { - return filepath.Join(cm.homeDir, ".toolhive", "llm", "claude-desktop-helper.sh") + return filepath.Join(cm.homeDir, ".toolhive", "llm", credentialHelperShimName()) } // writeCredentialHelperShim generates the no-arg executable that Claude Desktop @@ -238,15 +238,13 @@ func (cm *ClientManager) credentialHelperShimPath() string { // the "llm token" arguments — and to choose them per invocation context. // // tokenHelperPath is the absolute thv path (ApplyConfig.TokenHelperPath). It is -// single-quoted into the exec lines rather than resolved via PATH at call time: -// Claude Desktop is only ever GUI-launched, and GUI apps inherit launchd's -// environment, which does not contain thv's install directory — a bare "thv" -// would never be found. This is what distinguishes Claude Desktop from the -// direct-mode clients, which deliberately keep the bare, PATH-resolved -// tokenHelperShellCommand (see pkg/llm/setup.go). Single-quoting is a total -// transform (see quoteForPOSIXShell), so no character in the path needs -// rejecting and no metacharacter validation is required to keep this 0700 -// script injection-free. +// quoted into the exec lines rather than resolved via PATH at call time: +// Claude Desktop is only ever GUI-launched, and GUI apps inherit a minimal +// environment (launchd on macOS; explorer on Windows), which does not contain +// thv's install directory — a bare "thv" would never be found. This is what +// distinguishes Claude Desktop from the direct-mode clients, which deliberately +// keep the bare, PATH-resolved tokenHelperShellCommand (see pkg/llm/setup.go). +// POSIX uses quoteForPOSIXShell; Windows writes a .cmd via quoteForCmd. // // Claude Desktop sets CLAUDE_HELPER_CONTEXT on each call. Only "interactive" // permits an OIDC browser flow; silent contexts (background / setup-test / @@ -264,6 +262,13 @@ func (cm *ClientManager) writeCredentialHelperShim(tokenHelperPath string) (stri return "", fmt.Errorf( "credential helper shim requires an absolute token-helper path, got %q", tokenHelperPath) } + if runtime.GOOS == "windows" { + return cm.writeWindowsCredentialHelperShim(tokenHelperPath) + } + return cm.writePOSIXCredentialHelperShim(tokenHelperPath) +} + +func (cm *ClientManager) writePOSIXCredentialHelperShim(tokenHelperPath string) (string, error) { shimPath := cm.credentialHelperShimPath() if err := os.MkdirAll(filepath.Dir(shimPath), 0o700); err != nil { return "", fmt.Errorf("creating credential helper directory: %w", err) @@ -282,6 +287,29 @@ func (cm *ClientManager) writeCredentialHelperShim(tokenHelperPath string) (stri return shimPath, nil } +func (cm *ClientManager) writeWindowsCredentialHelperShim(tokenHelperPath string) (string, error) { + shimPath := cm.credentialHelperShimPath() + if err := os.MkdirAll(filepath.Dir(shimPath), 0o700); err != nil { + return "", fmt.Errorf("creating credential helper directory: %w", err) + } + quoted := quoteForCmd(tokenHelperPath) + // goto, not a parenthesized if: a path containing ")" would terminate the + // block. call, not a bare invoke: a .cmd token helper must return here. + script := "@echo off\r\n" + + "REM Generated by `thv llm setup` - Claude Desktop credential helper.\r\n" + + "REM Prints a fresh LLM gateway token. Do not edit; `thv llm teardown` removes it.\r\n" + + "if /I \"%CLAUDE_HELPER_CONTEXT%\"==\"interactive\" goto interactive\r\n" + + "call " + quoted + " llm token --skip-browser\r\n" + + "exit /b %ERRORLEVEL%\r\n" + + ":interactive\r\n" + + "call " + quoted + " llm token\r\n" + + "exit /b %ERRORLEVEL%\r\n" + if err := fileutils.AtomicWriteFile(shimPath, []byte(script), 0o700); err != nil { + return "", fmt.Errorf("writing credential helper shim %s: %w", shimPath, err) + } + return shimPath, nil +} + // quoteForPOSIXShell wraps s in single quotes so a POSIX shell reads every byte // of it literally, escaping any embedded single quote as the four-character // sequence quote-backslash-quote-quote (close the quoted run, emit a @@ -290,12 +318,17 @@ func (cm *ClientManager) writeCredentialHelperShim(tokenHelperPath string) (stri // Single-quoting is a total transform: inside a single-quoted run no character // is special to the shell — not $, backtick, backslash, or even a newline — so // there is no input this fails on and callers need no metacharacter validation. -// The shim it serves is POSIX-only by construction (configureCredentialHelper -// hard-errors on Windows), so no cmd.exe equivalent is needed. func quoteForPOSIXShell(s string) string { return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" } +// quoteForCmd wraps s in double quotes for cmd.exe. "%" is doubled first so +// "%VAR%" inside the path is not expanded; quotes inside the path are doubled +// after that. Used only by the Windows .cmd shim. +func quoteForCmd(s string) string { + return `"` + strings.ReplaceAll(strings.ReplaceAll(s, `%`, `%%`), `"`, `""`) + `"` +} + // managedProfilePresent reports whether an MDM/managed-preferences profile for // the given plist domain is present. A managed profile overrides a client's // local config, so "thv llm setup" warns when one is detected (the local config diff --git a/pkg/client/llm_gateway_credential_helper_test.go b/pkg/client/llm_gateway_credential_helper_test.go index fe8abb720c..c81ac9ac95 100644 --- a/pkg/client/llm_gateway_credential_helper_test.go +++ b/pkg/client/llm_gateway_credential_helper_test.go @@ -50,13 +50,20 @@ func readConfigDoc(t *testing.T, path string) claudeDesktopConfig { return doc } +func claudeDesktopTestHelperPath() string { + if runtime.GOOS == "windows" { + return `C:\opt\toolhive\bin\thv.exe` + } + return "/opt/toolhive/bin/thv" +} + func claudeDesktopApplyCfg() llmgateway.ApplyConfig { return llmgateway.ApplyConfig{ GatewayURL: "https://gw.example.com", AnthropicBaseURL: "https://gw.example.com/anthropic", // The shim consumes TokenHelperPath (the absolute thv path), not the // shell-string TokenHelperCommand that direct-mode clients use. - TokenHelperPath: "/opt/toolhive/bin/thv", + TokenHelperPath: claudeDesktopTestHelperPath(), } } @@ -86,10 +93,17 @@ func TestConfigureCredentialHelper_WritesConfigMetaAndShim(t *testing.T) { assert.Equal(t, shimPath, doc.InferenceCredentialHelper) info, err := os.Stat(shimPath) require.NoError(t, err) - assert.Equal(t, os.FileMode(0o700), info.Mode().Perm()) + if runtime.GOOS != "windows" { + assert.Equal(t, os.FileMode(0o700), info.Mode().Perm()) + } shim, err := os.ReadFile(shimPath) // #nosec G304 -- test-controlled path require.NoError(t, err) - assert.Contains(t, string(shim), `exec '/opt/toolhive/bin/thv' llm token`) + if runtime.GOOS == "windows" { + assert.True(t, strings.HasSuffix(shimPath, "claude-desktop-helper.cmd")) + assert.Contains(t, string(shim), `call "`+claudeDesktopTestHelperPath()+`" llm token`) + } else { + assert.Contains(t, string(shim), `exec '/opt/toolhive/bin/thv' llm token`) + } assert.Contains(t, string(shim), "--skip-browser") // _meta.json selects our config by the config document's id. @@ -252,11 +266,10 @@ func TestRevertCredentialHelper_LeavesForeignAppliedIDIntact(t *testing.T) { // Each case gets its own sentinel file so subtests run in parallel safely. func TestRevertCredentialHelper_RejectsUnsafeConfigPath(t *testing.T) { t.Parallel() - cm, _ := newClaudeDesktopManager(t) + cm, metaPath := newClaudeDesktopManager(t) // configLibrary must exist so revert reaches the guard rather than // early-returning on a missing dir. - require.NoError(t, os.MkdirAll( - filepath.Join(cm.homeDir, "Library", "Application Support", "Claude-3p", "configLibrary"), 0o700)) + require.NoError(t, os.MkdirAll(filepath.Dir(metaPath), 0o700)) cases := []struct { name string @@ -348,6 +361,55 @@ func TestQuoteForPOSIXShell_SurvivesRealShell(t *testing.T) { assert.True(t, strings.HasPrefix(string(out), "/a")) } +func TestQuoteForCmd(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + in string + want string + }{ + {"plain path", `C:\opt\thv.exe`, `"C:\opt\thv.exe"`}, + {"space", `C:\Program Files\thv.exe`, `"C:\Program Files\thv.exe"`}, + {"embedded quote", `C:\say "hi"\thv.exe`, `"C:\say ""hi""\thv.exe"`}, + {"percent", `C:\100%\thv.exe`, `"C:\100%%\thv.exe"`}, + {"empty", "", `""`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.want, quoteForCmd(tc.in)) + }) + } +} + +func TestWriteCredentialHelperShim_ExecutesWithHostilePath_Windows(t *testing.T) { + t.Parallel() + if runtime.GOOS != "windows" { + t.Skip("Windows .cmd shim") + } + + hostileDir := filepath.Join(t.TempDir(), "O'Brien we ird") + require.NoError(t, os.MkdirAll(hostileDir, 0o700)) + fakeThv := filepath.Join(hostileDir, "thv.cmd") + require.NoError(t, os.WriteFile(fakeThv, []byte("@echo off\r\necho ARGS: %*\r\n"), 0o700)) + + cm := &ClientManager{homeDir: t.TempDir()} + shimPath, err := cm.writeCredentialHelperShim(fakeThv) + require.NoError(t, err) + assert.True(t, strings.HasSuffix(shimPath, "claude-desktop-helper.cmd")) + + out, err := exec.Command("cmd.exe", "/c", shimPath).CombinedOutput() // #nosec G204 -- test-controlled path + require.NoError(t, err, "shim failed: %s", out) + assert.Equal(t, "ARGS: llm token --skip-browser", strings.TrimSpace(string(out))) + + cmd := exec.Command("cmd.exe", "/c", shimPath) // #nosec G204 -- test-controlled path + cmd.Env = append(os.Environ(), "CLAUDE_HELPER_CONTEXT=interactive") + out, err = cmd.CombinedOutput() + require.NoError(t, err, "shim failed: %s", out) + assert.Equal(t, "ARGS: llm token", strings.TrimSpace(string(out))) +} + // TestWriteCredentialHelperShim_RequiresAbsolutePath proves the writer fails // closed on anything that is not an absolute path. Defeating PATH resolution is // the whole point of the shim, so a relative path — which Claude Desktop would @@ -381,13 +443,24 @@ func TestWriteCredentialHelperShim_UsesAbsolutePath(t *testing.T) { t.Parallel() cm := &ClientManager{homeDir: t.TempDir()} - shimPath, err := cm.writeCredentialHelperShim("/opt/toolhive/bin/thv") + helperPath := claudeDesktopTestHelperPath() + shimPath, err := cm.writeCredentialHelperShim(helperPath) require.NoError(t, err) shim, err := os.ReadFile(shimPath) // #nosec G304 -- test-controlled path require.NoError(t, err) - assert.Contains(t, string(shim), `exec '/opt/toolhive/bin/thv' llm token`) - assert.Contains(t, string(shim), `exec '/opt/toolhive/bin/thv' llm token --skip-browser`) + if runtime.GOOS == "windows" { + assert.Contains(t, string(shim), `call "`+helperPath+`" llm token`) + assert.Contains(t, string(shim), `call "`+helperPath+`" llm token --skip-browser`) + silent, interactive, found := strings.Cut(string(shim), ":interactive") + require.True(t, found, "Windows shim must have a :interactive label") + assert.Contains(t, silent, "--skip-browser") + assert.NotContains(t, interactive, "--skip-browser") + return + } + + assert.Contains(t, string(shim), `exec '`+helperPath+`' llm token`) + assert.Contains(t, string(shim), `exec '`+helperPath+`' llm token --skip-browser`) // The interactive branch must NOT pass --skip-browser: it is the only // context permitted to open a browser for a full OIDC re-auth. interactive, _, found := strings.Cut(string(shim), "\nfi\n")