Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 28 additions & 11 deletions pkg/client/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,20 @@ const (
// literals by hujson.Patch.
// ValueField names which LLMApplyConfig field to write: "GatewayURL",
// "ProxyBaseURL", "TokenHelperCommand", "PlaceholderAPIKey" (constant "thv-proxy"),
// or "NodeTLSRejectUnauthorized" (writes "0" when TLSSkipVerify is true).
// "NodeTLSRejectUnauthorized" (writes "0" when TLSSkipVerify is true), or
// "ProxyOrigin" (like ProxyBaseURL but with the path stripped — use for tools
// such as Gemini CLI that append their own API path to avoid doubled segments).
// Any other string is written as a literal value; use this to hard-code a
// constant string into a settings key (e.g. forcing an auth type to a fixed value).
Comment thread
yrobla marked this conversation as resolved.
// ClearWhenEmpty: when true and the resolved value is empty, the key is removed
// from the settings file rather than skipped. Use for conditional keys like
// NODE_TLS_REJECT_UNAUTHORIZED that must be cleaned up when the flag is cleared.
type LLMGatewayKeySpec struct {
JSONPointer string // RFC 6901 path
ValueField string // "GatewayURL" | "ProxyBaseURL" | "TokenHelperCommand" | "PlaceholderAPIKey" | "NodeTLSRejectUnauthorized"
ClearWhenEmpty bool // remove the key when the resolved value is empty
JSONPointer string // RFC 6901 path
// ValueField: "GatewayURL" | "ProxyBaseURL" | "ProxyOrigin" |
// "TokenHelperCommand" | "PlaceholderAPIKey" | "NodeTLSRejectUnauthorized"
ValueField string
ClearWhenEmpty bool // remove the key when the resolved value is empty
}

// clientAppConfig represents a configuration path for a supported MCP client.
Expand Down Expand Up @@ -836,17 +842,28 @@ var supportedClientIntegrations = []clientAppConfig{
SupportsSkills: true,
SkillsGlobalPath: []string{".agents", "skills"},
SkillsProjectPath: []string{".agents", "skills"},
// LLM gateway: patches the same settings.json used for MCP
LLMGatewayMode: "direct",
// LLM gateway: patches the same settings.json used for MCP.
// Gemini CLI has no dynamic token-command equivalent, so it uses the
// proxy path. GOOGLE_GEMINI_BASE_URL is only honoured when
// security.auth.selectedType is "gemini-api-key" (fixed in
// gemini-cli v0.40.0, PR #25357); OAuth auth ignores the override.
//
// NODE_TLS_REJECT_UNAUTHORIZED is intentionally omitted: in proxy mode
// the tool connects to the local proxy over plain HTTP, so there is no
// TLS handshake on that leg. Setting the env var would globally disable
// TLS verification for all other HTTPS requests the Gemini CLI process
// makes, which is an unacceptable side-effect.
LLMGatewayMode: "proxy",
LLMBinaryName: "gemini",
LLMSettingsFile: "settings.json",
LLMSettingsRelPath: []string{".gemini"},
LLMGatewayKeys: []LLMGatewayKeySpec{
{JSONPointer: "/auth/tokenCommand", ValueField: "TokenHelperCommand"},
{JSONPointer: "/baseUrl", ValueField: "GatewayURL"},
// NODE_TLS_REJECT_UNAUTHORIZED is only written when --tls-skip-verify is set.
// ClearWhenEmpty ensures it is removed when the flag is later cleared.
{JSONPointer: "/env/NODE_TLS_REJECT_UNAUTHORIZED", ValueField: "NodeTLSRejectUnauthorized", ClearWhenEmpty: true},
// Force API-key auth so GOOGLE_GEMINI_BASE_URL is respected.
// "gemini-api-key" is an unrecognised ValueField, so llmValueForSpec
// writes it as a literal string (see LLMGatewayKeySpec doc comment).
{JSONPointer: "/security/auth/selectedType", ValueField: "gemini-api-key"},
{JSONPointer: "/env/GEMINI_API_KEY", ValueField: "PlaceholderAPIKey"},
{JSONPointer: "/env/GOOGLE_GEMINI_BASE_URL", ValueField: "ProxyOrigin"},
Comment thread
yrobla marked this conversation as resolved.
Comment thread
yrobla marked this conversation as resolved.
},
},
{
Expand Down
7 changes: 7 additions & 0 deletions pkg/client/llm_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,10 @@ func (cm *ClientManager) buildLLMSettingsPath(cfg *clientAppConfig) string {
// llmValueForSpec returns the config value corresponding to the ValueField name.
// For "NodeTLSRejectUnauthorized", returns "0" when TLSSkipVerify is true, or ""
// when false (which triggers removal when ClearWhenEmpty is set on the spec).
// For "ProxyOrigin", returns ProxyBaseURL with the path stripped (origin only),
// for tools like Gemini CLI that append their own API path to avoid doubled segments.
// Any unrecognised ValueField string is returned as-is, allowing a spec entry to
// hard-code a constant value into a settings key (e.g. forcing an auth type).
func llmValueForSpec(valueField string, cfg llmgateway.ApplyConfig) string {
switch valueField {
case "GatewayURL":
Expand All @@ -276,6 +280,9 @@ func llmValueForSpec(valueField string, cfg llmgateway.ApplyConfig) string {
return "0"
}
return ""
case "ProxyOrigin":
// Like ProxyBaseURL but with the path stripped; see llmgateway.ProxyOriginOf.
return llmgateway.ProxyOriginOf(cfg.ProxyBaseURL)
default:
return valueField // treat unknown field names as literal values
}
Expand Down
78 changes: 77 additions & 1 deletion pkg/client/llm_gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,12 @@ func TestRealClientConfigs_ConfigureAndRevert(t *testing.T) {
},
{
// ~/.gemini/settings.json
// NODE_TLS_REJECT_UNAUTHORIZED must NOT appear when TLSSkipVerify is false.
clientType: GeminiCli,
expectedKeys: []string{
"tokenCommand", "baseUrl", "https://gw.example.com",
"selectedType", "gemini-api-key",
"GEMINI_API_KEY", "thv-proxy",
"GOOGLE_GEMINI_BASE_URL", "http://localhost:14000",
},
},
{
Expand Down Expand Up @@ -624,6 +627,79 @@ func TestConfigureLLMGateway_TLSSkipVerify_ClearRemovesKey(t *testing.T) {
assert.NotContains(t, string(data), "NODE_TLS_REJECT_UNAUTHORIZED", "key must be removed when TLSSkipVerify is cleared")
}

// TestRealClientConfigs_GeminiCLI_NeverWritesTLSKey verifies that
// NODE_TLS_REJECT_UNAUTHORIZED is never written for Gemini CLI regardless of
// TLSSkipVerify. In proxy mode the tool connects to localhost over plain HTTP,
// so setting the env var would only globally suppress TLS for other HTTPS
// requests — an unacceptable side-effect. The key spec is intentionally absent.
func TestRealClientConfigs_GeminiCLI_NeverWritesTLSKey(t *testing.T) {
t.Parallel()

home := t.TempDir()
cm := NewTestClientManager(home, nil, supportedClientIntegrations, nil)
require.NoError(t, os.MkdirAll(filepath.Join(home, ".gemini"), 0o700))

for _, skipVerify := range []bool{false, true} {
path, err := cm.ConfigureLLMGateway(GeminiCli, llmgateway.ApplyConfig{
ProxyBaseURL: "http://localhost:14000/v1",
TLSSkipVerify: skipVerify,
})
require.NoError(t, err)

data, err := os.ReadFile(path)
require.NoError(t, err)
assert.NotContains(t, string(data), "NODE_TLS_REJECT_UNAUTHORIZED",
"TLS key must never be written for Gemini CLI (TLSSkipVerify=%v)", skipVerify)
}
}

// ── llmValueForSpec unit tests ────────────────────────────────────────────────

func TestLLMValueForSpec(t *testing.T) {
t.Parallel()

cfg := llmgateway.ApplyConfig{
GatewayURL: "https://gw.example.com",
ProxyBaseURL: "http://localhost:14000/v1",
TokenHelperCommand: `"thv" llm token`,
TLSSkipVerify: false,
}

cases := []struct {
name string
valueField string
cfg llmgateway.ApplyConfig
want string
}{
{"GatewayURL", "GatewayURL", cfg, "https://gw.example.com"},
{"ProxyBaseURL", "ProxyBaseURL", cfg, "http://localhost:14000/v1"},
{"TokenHelperCommand", "TokenHelperCommand", cfg, `"thv" llm token`},
{"PlaceholderAPIKey", "PlaceholderAPIKey", cfg, "thv-proxy"},
// NodeTLSRejectUnauthorized: "0" when set, "" when clear
{"NodeTLSRejectUnauthorized/skip=false", "NodeTLSRejectUnauthorized", cfg, ""},
{"NodeTLSRejectUnauthorized/skip=true", "NodeTLSRejectUnauthorized", llmgateway.ApplyConfig{TLSSkipVerify: true}, "0"},
// ProxyOrigin strips path, query, and fragment from ProxyBaseURL
{"ProxyOrigin/strips_path", "ProxyOrigin", cfg, "http://localhost:14000"},
{"ProxyOrigin/long_path", "ProxyOrigin", llmgateway.ApplyConfig{ProxyBaseURL: "http://localhost:9000/v1beta/openai"}, "http://localhost:9000"},
{"ProxyOrigin/strips_query_and_fragment", "ProxyOrigin", llmgateway.ApplyConfig{ProxyBaseURL: "http://host:8080/path?q=1#frag"}, "http://host:8080"},
// ForceQuery: trailing "?" with no key must not leak into the origin.
{"ProxyOrigin/force_query", "ProxyOrigin", llmgateway.ApplyConfig{ProxyBaseURL: "http://host:8080/path?"}, "http://host:8080"},
// ProxyOrigin falls back to the raw value when URL parsing fails
{"ProxyOrigin/invalid_url_fallback", "ProxyOrigin", llmgateway.ApplyConfig{ProxyBaseURL: "::invalid"}, "::invalid"},
// Unrecognised ValueField is written as a literal constant
{"literal/gemini-api-key", "gemini-api-key", cfg, "gemini-api-key"},
{"literal/some-literal", "some-literal", cfg, "some-literal"},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
got := llmValueForSpec(tc.valueField, tc.cfg)
assert.Equal(t, tc.want, got)
})
Comment thread
yrobla marked this conversation as resolved.
}
}

// countSubstring counts non-overlapping occurrences of substr in s.
func countSubstring(s, substr string) int {
count := 0
Expand Down
21 changes: 21 additions & 0 deletions pkg/llmgateway/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,27 @@
// imported by both pkg/llm and pkg/client to avoid an import cycle.
package llmgateway

import "net/url"

// ProxyOriginOf returns rawURL with its path, query, and fragment stripped so
// only the scheme and host remain (the "origin"). Tools like Gemini CLI that
// append their own API path (e.g. /v1beta/...) need the origin rather than the
// full proxy base URL to avoid doubled path segments. Falls back to rawURL when
// parsing fails.
func ProxyOriginOf(rawURL string) string {
u, err := url.Parse(rawURL)
if err != nil || u == nil {
return rawURL
}
u.Path = ""
u.RawPath = ""
u.RawQuery = ""
u.ForceQuery = false
u.Fragment = ""
u.RawFragment = ""
return u.String()
Comment thread
yrobla marked this conversation as resolved.
}

// ApplyConfig holds the values needed to configure a single tool's LLM
// gateway settings. Using a struct prevents positional-argument mistakes when
// the caller has multiple similar string values in scope.
Expand Down
23 changes: 15 additions & 8 deletions test/e2e/cli_llm_all_clients_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

"github.com/stacklok/toolhive/pkg/auth/tokensource"
"github.com/stacklok/toolhive/pkg/llm"
"github.com/stacklok/toolhive/pkg/llmgateway"
"github.com/stacklok/toolhive/pkg/networking"
"github.com/stacklok/toolhive/test/e2e"
)
Expand Down Expand Up @@ -89,11 +90,17 @@
settingsPath: func(tempDir string) string {
return filepath.Join(tempDir, ".gemini", "settings.json")
},
mode: "direct",
mode: "proxy",
expectedKeys: map[string]func(string, string) string{
"/auth/tokenCommand": func(_, _ string) string { return "llm token" },
"/baseUrl": func(gatewayURL, _ string) string {
return gatewayURL
// Force API-key auth so GOOGLE_GEMINI_BASE_URL is respected.
"/security/auth/selectedType": func(_, _ string) string { return "gemini-api-key" },
"/env/GEMINI_API_KEY": func(_, _ string) string { return clientThvProxy },
// ProxyOrigin strips the entire path (and any query/fragment) from
// the proxy base URL so Gemini CLI can append its own /v1beta/...
// path without doubling segments. Mirror the production logic here
// so the expectation stays correct if the proxy path changes.
"/env/GOOGLE_GEMINI_BASE_URL": func(_, proxyURL string) string {
return llmgateway.ProxyOriginOf(proxyURL)
},
},
},
Expand Down Expand Up @@ -274,7 +281,7 @@
clientTC := clientTC // capture loop variable
It(clientTC.name, func() {
if clientTC.skipOnOS != "" && runtime.GOOS == clientTC.skipOnOS {
Skip(fmt.Sprintf("client %q not supported on %s", clientTC.name, runtime.GOOS))

Check notice on line 284 in test/e2e/cli_llm_all_clients_test.go

View workflow job for this annotation

GitHub Actions / E2E Tests / E2E Tests Core (llm)

It 04/30/26 15:33:48.832
}

issuerURL := fmt.Sprintf("http://localhost:%d", oidcPort)
Expand Down Expand Up @@ -448,11 +455,11 @@
Expect(err).ToNot(HaveOccurred())
var geminiAfter map[string]any
Expect(json.Unmarshal(data, &geminiAfter)).To(Succeed())
tokenCmd, found := jsonPointerGet(geminiAfter, "/auth/tokenCommand")
baseURL, found := jsonPointerGet(geminiAfter, "/env/GOOGLE_GEMINI_BASE_URL")
Expect(found).To(BeTrue(),
"gemini-cli /auth/tokenCommand should still be present after claude-code teardown")
Expect(tokenCmd).To(ContainSubstring("llm token"),
"gemini-cli tokenCommand should still reference 'llm token' after claude-code teardown")
"gemini-cli /env/GOOGLE_GEMINI_BASE_URL should still be present after claude-code teardown")
Expect(baseURL).ToNot(BeEmpty(),
"gemini-cli GOOGLE_GEMINI_BASE_URL should still be set after claude-code teardown")

By("verifying ConfiguredTools has only gemini-cli")
showOut, _ = thvCmd("llm", "config", "show", "--format", "json").ExpectSuccess()
Expand Down Expand Up @@ -709,7 +716,7 @@
clientTC := clientTC
It(fmt.Sprintf("forwards Bearer token to gateway for %s", clientTC.name), func() {
if clientTC.skipOnOS != "" && runtime.GOOS == clientTC.skipOnOS {
Skip(fmt.Sprintf("client %q not supported on %s", clientTC.name, runtime.GOOS))

Check notice on line 719 in test/e2e/cli_llm_all_clients_test.go

View workflow job for this annotation

GitHub Actions / E2E Tests / E2E Tests Core (llm)

It 04/30/26 15:34:03.272
}

// Allocate ports for the mock gateway and the proxy.
Expand Down
Loading