diff --git a/CHANGELOG.md b/CHANGELOG.md index acd8c08..2c4aaa7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,21 @@ Releases are cut by pushing a `v*` tag, which publishes the images to Docker Hub. Entries before v0.4.0 were reconstructed from git history. +## Unreleased + +### Changed + +- **`:proxy` now requires `LK_UPSTREAM` and exits if it is unset.** It previously inherited a default of `http://127.0.0.1:8080/v1`, which is this container's own loopback and can never hold a model server in an image with no inference engine. Forgetting the variable produced a container that started cleanly and then failed every request. If you do want the container's own loopback, for example with `--network host`, set `LK_UPSTREAM=http://127.0.0.1:8080/v1` explicitly. The model-bundled tags are unaffected; their entrypoint passes `--upstream` directly. + +### Added + +- The resolved upstream and where it came from (flag, `LK_UPSTREAM`, or default) are logged at startup. Nothing previously reported the upstream, so a misconfiguration gave no signal at all. +- The `/health` 503 body names the upstream it could not reach. + +### Fixed + +- Credentials embedded in the upstream URL, whether in userinfo or a query string, are stripped everywhere the URL surfaces: the startup log, the `/health` body, and the error a failed upstream call returns to the client. Go redacts a password in a transport error but not a username, so a key placed in the username would previously have reached the caller. + ## v0.4.0 (2026-08-04) ### Added diff --git a/Dockerfile b/Dockerfile index 62fd843..b0a69d1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -8,6 +8,8 @@ FROM alpine:3@sha256:28bd5fe8b56d1bd048e5babf5b10710ebe0bae67db86916198a6eec4349 RUN apk add --no-cache ca-certificates poppler-utils tini COPY --from=proxy-builder /out/localaik /usr/local/bin/localaik ENV PORT=8090 +# No inference engine here, so the loopback default could never work. +ENV LK_REQUIRE_UPSTREAM=1 HEALTHCHECK --interval=5s --timeout=3s --start-period=5s \ CMD wget -q -O - "http://127.0.0.1:${PORT:-8090}/health" >/dev/null 2>&1 || exit 1 EXPOSE 8090 diff --git a/README.md b/README.md index 9b22942..da07b61 100644 --- a/README.md +++ b/README.md @@ -200,10 +200,17 @@ docker run -d -p 127.0.0.1:8090:8090 \ | Env var | Default | Description | | --- | --- | --- | -| `LK_UPSTREAM` | `http://127.0.0.1:8080/v1` | Base URL of your model server | +| `LK_UPSTREAM` | none, required | Base URL of your model server | | `LK_UPSTREAM_AUTH_HEADER` | unset | A full header line sent to your server, for example `Authorization: Bearer abc123` | | `PORT` | `8090` | Port localaik listens on | +`LK_UPSTREAM` is required here and the container exits if it is unset. The +model-bundled tags default it to this container's own `127.0.0.1:8080`, which +cannot work in an image with no inference engine, so `:proxy` refuses to start +rather than reporting 503 forever. If you do want the container's own loopback, +for example with `--network host`, set it explicitly to +`http://127.0.0.1:8080/v1`. + `LK_UPSTREAM_AUTH_HEADER` is sent only to your upstream. Credentials that clients send to localaik are still discarded and never forwarded. It is attached only to requests whose host matches `LK_UPSTREAM`, and while it is set a @@ -211,7 +218,10 @@ redirect from your upstream is returned to the caller rather than followed. `/health` returns 503 until your upstream answers, so any wait loop that polls it for a 200 works unchanged. As with the model-bundled tags, the port opens before -the upstream is reachable, so a TCP liveness check is not enough. +the upstream is reachable, so a TCP liveness check is not enough. The 503 body +names the upstream it could not reach, with any userinfo in the URL redacted. +Since localaik authenticates none of its callers, treat that as one more reason +not to publish the port on a shared network. ### Security diff --git a/cmd/localaik/main.go b/cmd/localaik/main.go index 48fccbd..3f05754 100644 --- a/cmd/localaik/main.go +++ b/cmd/localaik/main.go @@ -11,6 +11,8 @@ import ( "github.com/harshaneel/localaik/internal/server" ) +const defaultUpstream = "http://127.0.0.1:8080/v1" + func resolveFlagDefault(envName, fallback string) string { if value := os.Getenv(envName); value != "" { return value @@ -18,18 +20,55 @@ func resolveFlagDefault(envName, fallback string) string { return fallback } +// resolveUpstream also reports where the value came from, so an image with no +// model server of its own can refuse to start on the default. +func resolveUpstream(flagValue string, flagSet bool, env string) (string, string) { + switch { + case flagSet: + return flagValue, "flag" + case env != "": + return env, "LK_UPSTREAM" + } + return defaultUpstream, "default" +} + +// requireEnv is an internal image marker set only in the proxy Dockerfile stage, +// so any non-empty value arms the check. +func upstreamRequiredButUnset(source, requireEnv string) bool { + return source == "default" && requireEnv != "" +} + +func flagWasSet(name string) bool { + set := false + flag.Visit(func(f *flag.Flag) { + if f.Name == name { + set = true + } + }) + return set +} + func main() { port := flag.String("port", resolveFlagDefault("PORT", "8090"), "port to listen on") - upstream := flag.String("upstream", resolveFlagDefault("LK_UPSTREAM", "http://127.0.0.1:8080/v1"), "upstream OpenAI-compatible base URL") + upstreamFlag := flag.String("upstream", defaultUpstream, "upstream base URL speaking the OpenAI chat completions API") flag.Parse() + upstream, source := resolveUpstream(*upstreamFlag, flagWasSet("upstream"), os.Getenv("LK_UPSTREAM")) + if upstream == "" { + log.Fatal("localaik: the upstream is set to an empty value; give a base URL that speaks the OpenAI chat completions API, for example http://llama.internal:8080/v1") + } + if upstreamRequiredButUnset(source, os.Getenv("LK_REQUIRE_UPSTREAM")) { + log.Fatal("localaik: LK_UPSTREAM is not set. This image has no model server of its own, so it needs the base URL of one that speaks the OpenAI chat completions API, for example http://llama.internal:8080/v1") + } + log.Printf("localaik: upstream %s (%s)", server.RedactUpstream(upstream), source) + authHeader := os.Getenv("LK_UPSTREAM_AUTH_HEADER") if authHeader != "" && !server.ValidUpstreamAuthHeader(authHeader) { log.Printf("localaik: LK_UPSTREAM_AUTH_HEADER is set but is not a valid \"Name: value\" header line; no credential will be sent upstream") } handler, err := server.New(server.Config{ - UpstreamBaseURL: *upstream, + UpstreamBaseURL: upstream, UpstreamAuthHeader: authHeader, HTTPClient: &http.Client{}, PDFRenderer: pdf.NewExecRenderer("pdftoppm"), diff --git a/cmd/localaik/main_test.go b/cmd/localaik/main_test.go index 6e37a1f..a6cc6c0 100644 --- a/cmd/localaik/main_test.go +++ b/cmd/localaik/main_test.go @@ -28,6 +28,65 @@ func TestResolveFlagDefaultUnsetFallsBack(t *testing.T) { } } +func TestResolveUpstream(t *testing.T) { + tests := []struct { + name string + flagValue string + flagSet bool + env string + wantURL string + wantSource string + }{ + {"flag beats env", "http://flag:1/v1", true, "http://env:2/v1", "http://flag:1/v1", "flag"}, + {"env beats default", defaultUpstream, false, "http://env:2/v1", "http://env:2/v1", "LK_UPSTREAM"}, + {"default when neither", defaultUpstream, false, "", defaultUpstream, "default"}, + {"empty env is not a value", defaultUpstream, false, "", defaultUpstream, "default"}, + // Passing the default explicitly is how an operator opts into this + // container's own loopback, so it must not read as "default". + {"flag matching the default still counts as explicit", defaultUpstream, true, "", defaultUpstream, "flag"}, + // An explicit empty flag must surface as empty so main rejects it, + // rather than falling through to the loopback default. + {"explicit empty flag stays empty", "", true, "http://env:2/v1", "", "flag"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + gotURL, gotSource := resolveUpstream(tc.flagValue, tc.flagSet, tc.env) + if gotURL != tc.wantURL { + t.Errorf("url = %q, want %q", gotURL, tc.wantURL) + } + if gotSource != tc.wantSource { + t.Errorf("source = %q, want %q", gotSource, tc.wantSource) + } + }) + } +} + +func TestUpstreamRequiredButUnset(t *testing.T) { + tests := []struct { + name string + source string + requireEnv string + want bool + }{ + {"proxy image with nothing configured", "default", "1", true}, + {"proxy image with LK_UPSTREAM set", "LK_UPSTREAM", "1", false}, + {"proxy image with an explicit flag", "flag", "1", false}, + // The bundled images pass --upstream, so they are safe even if someone + // sets the marker by hand. + {"bundled image on the default", "default", "", false}, + {"bundled image with a flag", "flag", "", false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := upstreamRequiredButUnset(tc.source, tc.requireEnv); got != tc.want { + t.Errorf("upstreamRequiredButUnset(%q, %q) = %v, want %v", tc.source, tc.requireEnv, got, tc.want) + } + }) + } +} + // The startup warning must be driven by the same predicate the transport uses; // server.ValidUpstreamAuthHeader owns the table of cases. func TestStartupWarningUsesTheServerPredicate(t *testing.T) { diff --git a/internal/server/anthropic.go b/internal/server/anthropic.go index 09e7359..c8a6e13 100644 --- a/internal/server/anthropic.go +++ b/internal/server/anthropic.go @@ -47,7 +47,7 @@ func (s *Server) handleAnthropicMessages(w http.ResponseWriter, r *http.Request) resp, err := s.client.Do(upstreamReq) if err != nil { - anthropic.WriteError(w, http.StatusBadGateway, fmt.Sprintf("failed to reach upstream: %v", err)) + anthropic.WriteError(w, http.StatusBadGateway, fmt.Sprintf("failed to reach upstream %s", s.upstreamDisplay)) return } defer resp.Body.Close() diff --git a/internal/server/gemini.go b/internal/server/gemini.go index 4de9923..c0fc8a1 100644 --- a/internal/server/gemini.go +++ b/internal/server/gemini.go @@ -46,7 +46,7 @@ func (s *Server) handleGeminiGenerateContent(w http.ResponseWriter, r *http.Requ resp, err := s.client.Do(upstreamReq) if err != nil { - gemini.WriteError(w, http.StatusBadGateway, fmt.Sprintf("failed to reach upstream: %v", err)) + gemini.WriteError(w, http.StatusBadGateway, fmt.Sprintf("failed to reach upstream %s", s.upstreamDisplay)) return } defer resp.Body.Close() diff --git a/internal/server/gemini_meta.go b/internal/server/gemini_meta.go index 5003d7c..71c25f7 100644 --- a/internal/server/gemini_meta.go +++ b/internal/server/gemini_meta.go @@ -15,7 +15,7 @@ func (s *Server) handleGeminiModelsList(w http.ResponseWriter, r *http.Request) var upstream openaip.ModelList status, body, err := s.fetchUpstreamJSON(r, s.upstreamModelsURL, &upstream) if err != nil { - gemini.WriteError(w, http.StatusBadGateway, fmt.Sprintf("failed to reach upstream: %v", err)) + gemini.WriteError(w, http.StatusBadGateway, fmt.Sprintf("failed to reach upstream %s", s.upstreamDisplay)) return } if status >= http.StatusBadRequest { @@ -35,7 +35,7 @@ func (s *Server) handleGeminiModelGet(w http.ResponseWriter, r *http.Request) { var upstream openaip.Model status, body, err := s.fetchUpstreamJSON(r, s.upstreamModelsURL+"/"+modelName, &upstream) if err != nil { - gemini.WriteError(w, http.StatusBadGateway, fmt.Sprintf("failed to reach upstream: %v", err)) + gemini.WriteError(w, http.StatusBadGateway, fmt.Sprintf("failed to reach upstream %s", s.upstreamDisplay)) return } if status >= http.StatusBadRequest { diff --git a/internal/server/openai.go b/internal/server/openai.go index ed386ca..5b8bca5 100644 --- a/internal/server/openai.go +++ b/internal/server/openai.go @@ -20,7 +20,7 @@ func (s *Server) handleOpenAIPassthrough(w http.ResponseWriter, r *http.Request, resp, err := s.client.Do(req) if err != nil { - openaip.WriteError(w, http.StatusBadGateway, fmt.Sprintf("failed to reach upstream: %v", err), "server_error") + openaip.WriteError(w, http.StatusBadGateway, fmt.Sprintf("failed to reach upstream %s", s.upstreamDisplay), "server_error") return } defer resp.Body.Close() @@ -65,8 +65,8 @@ func (w flushWriter) Write(p []byte) (int, error) { // fetchUpstreamJSON issues a GET to upstreamURL and decodes the JSON response // into dst. On non-2xx status it returns (status, body, nil) so the caller can // translate the upstream error. On decode failure it returns (status, body, err) -// — body is the raw upstream payload but is malformed JSON, so callers should -// surface the error rather than the body. No request headers are forwarded; +// where body is the raw upstream payload but is malformed JSON, so callers +// should surface the error rather than the body. No request headers are forwarded; // this is intentional so that client-side auth (Authorization, X-Goog-Api-Key) // does not leak to upstream. func (s *Server) fetchUpstreamJSON(r *http.Request, upstreamURL string, dst any) (int, []byte, error) { diff --git a/internal/server/redact_test.go b/internal/server/redact_test.go new file mode 100644 index 0000000..2ea1562 --- /dev/null +++ b/internal/server/redact_test.go @@ -0,0 +1,132 @@ +package server + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/harshaneel/localaik/internal/pdf" +) + +func TestRedactUpstream(t *testing.T) { + tests := []struct { + name string + raw string + want string + }{ + {"no userinfo is returned unchanged", "http://llama.internal:8080/v1", "http://llama.internal:8080/v1"}, + {"password is removed", "http://user:secret@llama.internal:8080/v1", "http://llama.internal:8080/v1"}, + {"username alone is removed", "http://user@llama.internal:8080/v1", "http://llama.internal:8080/v1"}, + {"password-only userinfo is removed", "http://:secret@llama.internal:8080/v1", "http://llama.internal:8080/v1"}, + {"query string is dropped", "http://llama.internal:8080/v1?api_key=secret", "http://llama.internal:8080/v1"}, + {"fragment is dropped", "http://llama.internal:8080/v1#token=secret", "http://llama.internal:8080/v1"}, + {"opaque credential form does not echo back", "http:user:secret@llama.internal/v1", "invalid"}, + {"unparseable input does not echo back", "http://[::1", "invalid"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := RedactUpstream(tc.raw); got != tc.want { + t.Errorf("RedactUpstream(%q) = %q, want %q", tc.raw, got, tc.want) + } + }) + } +} + +// An unreachable upstream is the case an operator has to debug, so /health names +// it rather than only reporting that something is wrong. +func TestHealthReportsTheUpstreamWhenUnreachable(t *testing.T) { + srv, err := New(Config{ + UpstreamBaseURL: "http://127.0.0.1:9/v1", + HTTPClient: &http.Client{}, + PDFRenderer: pdf.RendererFunc(nil), + }) + if err != nil { + t.Fatalf("New returned error: %v", err) + } + + rec := httptest.NewRecorder() + srv.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/health", nil)) + + if rec.Code != http.StatusServiceUnavailable { + t.Fatalf("status = %d, want 503", rec.Code) + } + + var body map[string]string + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decode body: %v", err) + } + if body["status"] != "unhealthy" { + t.Errorf("status = %q, want unhealthy", body["status"]) + } + if body["upstream"] != "http://127.0.0.1:9/v1" { + t.Errorf("upstream = %q, want the configured URL", body["upstream"]) + } +} + +func TestHealthDoesNotLeakUpstreamCredentials(t *testing.T) { + srv, err := New(Config{ + UpstreamBaseURL: "http://user:supersecret@127.0.0.1:9/v1", + HTTPClient: &http.Client{}, + PDFRenderer: pdf.RendererFunc(nil), + }) + if err != nil { + t.Fatalf("New returned error: %v", err) + } + + rec := httptest.NewRecorder() + srv.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/health", nil)) + + if strings.Contains(rec.Body.String(), "supersecret") { + t.Fatalf("/health leaked the upstream password: %s", rec.Body.String()) + } +} + +// Go's http.Client redacts the password in a transport error but not a +// username, so a key-as-username upstream would otherwise reach the caller when +// a request fails. Every proxied route must scrub the whole upstream from the +// error it returns. +func TestUpstreamErrorResponsesDoNotLeakCredentials(t *testing.T) { + routes := []struct { + name string + method string + path string + body string + }{ + {"openai", http.MethodPost, "/v1/chat/completions", `{"model":"m","messages":[]}`}, + {"openai_models_list", http.MethodGet, "/v1/models", ""}, + {"openai_model_get", http.MethodGet, "/v1/models/m", ""}, + {"gemini", http.MethodPost, "/v1beta/models/m:generateContent", `{"contents":[{"role":"user","parts":[{"text":"hi"}]}]}`}, + {"gemini_models_list", http.MethodGet, "/v1beta/models", ""}, + {"gemini_model_get", http.MethodGet, "/v1beta/models/m", ""}, + {"anthropic", http.MethodPost, "/v1/messages", `{"model":"m","max_tokens":1,"messages":[{"role":"user","content":"hi"}]}`}, + {"gemini_count_tokens", http.MethodPost, "/v1beta/models/m:countTokens", `{"contents":[{"role":"user","parts":[{"text":"hi"}]}]}`}, + } + + srv, err := New(Config{ + UpstreamBaseURL: "http://leakedkey@127.0.0.1:9/v1", + HTTPClient: &http.Client{}, + PDFRenderer: pdf.RendererFunc(nil), + }) + if err != nil { + t.Fatalf("New returned error: %v", err) + } + + for _, tc := range routes { + t.Run(tc.name, func(t *testing.T) { + rec := httptest.NewRecorder() + req := httptest.NewRequest(tc.method, tc.path, strings.NewReader(tc.body)) + req.Header.Set("Content-Type", "application/json") + srv.ServeHTTP(rec, req) + + if rec.Code != http.StatusBadGateway { + t.Fatalf("status = %d, want 502", rec.Code) + } + if strings.Contains(rec.Body.String(), "leakedkey") { + t.Fatalf("error response leaked the upstream username: %s", rec.Body.String()) + } + }) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 47fd268..8c9211b 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -3,7 +3,6 @@ package server import ( "encoding/json" "errors" - "fmt" "net/http" "net/url" "path" @@ -30,6 +29,7 @@ type Server struct { upstreamModelsURL string upstreamTokenizeURL string upstreamHealthURL string + upstreamDisplay string } func New(cfg Config) (*Server, error) { @@ -37,9 +37,10 @@ func New(cfg Config) (*Server, error) { cfg.UpstreamBaseURL = "http://127.0.0.1:8080/v1" } + // The raw URL can carry credentials, so its parse error is not wrapped. parsed, err := url.Parse(cfg.UpstreamBaseURL) if err != nil { - return nil, fmt.Errorf("parse upstream URL: %w", err) + return nil, errors.New("upstream URL is not parseable") } if parsed.Scheme == "" || parsed.Host == "" { return nil, errors.New("upstream URL must include scheme and host") @@ -69,6 +70,7 @@ func New(cfg Config) (*Server, error) { return &Server{ client: client, pdfRenderer: renderer, + upstreamDisplay: RedactUpstream(cfg.UpstreamBaseURL), upstreamChatURL: resolveURLPath(parsed, "chat/completions"), upstreamCompletions: resolveURLPath(parsed, "completions"), upstreamModelsURL: resolveURLPath(parsed, "models"), @@ -117,13 +119,13 @@ func (s *Server) handleHealth(w http.ResponseWriter, r *http.Request) { resp, err := s.client.Do(req) if err != nil { - writeJSON(w, http.StatusServiceUnavailable, map[string]string{"status": "unhealthy"}) + writeJSON(w, http.StatusServiceUnavailable, map[string]string{"status": "unhealthy", "upstream": s.upstreamDisplay}) return } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - writeJSON(w, http.StatusServiceUnavailable, map[string]string{"status": "unhealthy"}) + writeJSON(w, http.StatusServiceUnavailable, map[string]string{"status": "unhealthy", "upstream": s.upstreamDisplay}) return } @@ -143,6 +145,20 @@ func (s *Server) handleNotFound(w http.ResponseWriter, r *http.Request) { } } +// RedactUpstream reduces an upstream URL to scheme, host and path, so a value +// carrying credentials in userinfo or a query string can be logged and served +// on /health without leaking them. An unparseable or hostless URL, such as the +// opaque "http:user:pass@host" form, collapses to a constant rather than +// echoing back. +func RedactUpstream(raw string) string { + parsed, err := url.Parse(raw) + if err != nil || parsed.Host == "" { + return "invalid" + } + safe := url.URL{Scheme: parsed.Scheme, Host: parsed.Host, Path: parsed.Path} + return safe.String() +} + func resolveURLPath(base *url.URL, extra string) string { clone := *base basePath := strings.TrimSuffix(clone.Path, "/") diff --git a/internal/server/tokens.go b/internal/server/tokens.go index f77378a..381b164 100644 --- a/internal/server/tokens.go +++ b/internal/server/tokens.go @@ -36,7 +36,7 @@ func (s *Server) countUpstreamTokens(r *http.Request, text string) (count int, u resp, doErr := s.client.Do(req) if doErr != nil { - return 0, 0, nil, &upstreamError{http.StatusBadGateway, fmt.Sprintf("failed to reach upstream: %v", doErr)} + return 0, 0, nil, &upstreamError{http.StatusBadGateway, fmt.Sprintf("failed to reach upstream %s", s.upstreamDisplay)} } defer resp.Body.Close()