fix(security): require API keys for Gemini CLI routes#26
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens access control around the Gemini CLI-compatible /v1internal:* endpoints by moving enforcement to the server router, removing the handler’s localhost-only restriction so authenticated traffic can traverse reverse proxies.
Changes:
- Add
AuthMiddlewareto the/v1internal:methodGemini CLI route. - Remove the handler-level
RemoteAddrlocalhost gate in the Gemini CLI handler. - Add regression tests for unauthenticated rejection and authenticated non-local access.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| sdk/api/handlers/gemini/gemini-cli_handlers.go | Removes localhost-only access logic; updates handler comment to reflect router-level access control. |
| internal/api/server.go | Wraps /v1internal:method with AuthMiddleware(s.accessManager) to require access-manager auth. |
| internal/api/server_test.go | Adds tests asserting /v1internal:* rejects missing API key and allows authenticated non-local requests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // CLIHandler handles CLI-specific requests for Gemini API operations. | ||
| // It restricts access to localhost only and routes requests to appropriate internal handlers. | ||
| // Access control is enforced at the router level so authenticated clients can | ||
| // use Gemini CLI-compatible routes through the public proxy. | ||
| func (h *GeminiCLIAPIHandler) CLIHandler(c *gin.Context) { | ||
| if !strings.HasPrefix(c.Request.RemoteAddr, "127.0.0.1:") { | ||
| c.JSON(http.StatusForbidden, handlers.ErrorResponse{ | ||
| Error: handlers.ErrorDetail{ | ||
| Message: "CLI reply only allow local access", | ||
| Type: "forbidden", | ||
| }, | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| rawJSON, _ := c.GetRawData() | ||
| requestRawURI := c.Request.URL.Path |
There was a problem hiding this comment.
Now that this handler is reachable through the public proxy, the fallback branch in CLIHandler that proxies to cloudcode-pa.googleapis.com copies all incoming headers to the upstream request. That includes the client’s Authorization header (which in this setup is the proxy API key), causing credential leakage to the upstream Google endpoint. Strip sensitive auth headers (Authorization / X-Api-Key / X-Goog-Api-Key, etc.) before forwarding, or explicitly whitelist safe headers to forward.
| }) | ||
| }) | ||
| s.engine.POST("/v1internal:method", geminiCLIHandlers.CLIHandler) | ||
| s.engine.POST("/v1internal:method", AuthMiddleware(s.accessManager), geminiCLIHandlers.CLIHandler) |
There was a problem hiding this comment.
AuthMiddleware() intentionally allows requests when the access manager has no providers configured (legacy behavior). With the localhost-only gate removed from the Gemini CLI handler, this means /v1internal:* becomes publicly reachable whenever API keys aren’t configured, which is a security regression. Consider using a dedicated middleware for /v1internal:* that fails closed when no access providers are configured (or preserves local-only access as a fallback).
|
|
||
| if rr.Code == http.StatusUnauthorized || rr.Code == http.StatusForbidden { | ||
| t.Fatalf("status = %d, want request to pass auth and localhost gate; body=%s", rr.Code, rr.Body.String()) | ||
| } |
There was a problem hiding this comment.
TestGeminiCLIRouteAllowsAuthenticatedNonLocalRequest only asserts the response is not 401/403, so it would still pass if the route were mis-registered and returned 404 (or if some other unrelated error occurred). Tighten the assertion to prove the request actually reached the intended route/handler (e.g., fail on 404, and/or assert the body is not the auth error).
| } | |
| } | |
| if rr.Code == http.StatusNotFound { | |
| t.Fatalf("status = %d, want request to reach the Gemini CLI route; body=%s", rr.Code, rr.Body.String()) | |
| } | |
| if strings.Contains(rr.Body.String(), "Missing API key") { | |
| t.Fatalf("body = %q, want request not to fail API key auth", rr.Body.String()) | |
| } |
| // CLIHandler handles CLI-specific requests for Gemini API operations. | ||
| // It restricts access to localhost only and routes requests to appropriate internal handlers. | ||
| // Access control is enforced at the router level so authenticated clients can | ||
| // use Gemini CLI-compatible routes through the public proxy. |
There was a problem hiding this comment.
The package-level comment at the top of this file still says the Gemini CLI handlers “restrict access to localhost only”, but this PR moves access control to router-level API key auth. Update/remove that file header comment to avoid misleading documentation.
Summary
Testing