diff --git a/SECURITY.md b/SECURITY.md index 957f9da4..8cb4c0ee 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -118,9 +118,11 @@ submit inference or observe behavior allowed by that endpoint. Do not bind local APIs or inference engines to untrusted interfaces, forward PAIR ports through a router, or place an unauthenticated public reverse proxy in -front of them. Review browser access and CORS behavior before allowing web -content to reach a proxy. Prompts, messages, chunks, and response bodies should -not be written to logs. +front of them. Browser clients are unsupported, and the inference proxies do not +opt into cross-origin browser access. This is defense in depth, not +authentication: native processes and direct engine access remain governed by +operating-system and engine security. Prompts, messages, chunks, and response +bodies should not be written to logs. ### Supervised Workers Share the User's Authority diff --git a/docs/architecture.mdx b/docs/architecture.mdx index f7e7ffed..9295e96f 100644 --- a/docs/architecture.mdx +++ b/docs/architecture.mdx @@ -513,6 +513,8 @@ The practical consequence is the one in [Troubleshooting](troubleshooting.mdx#requests-work-but-pair-shows-no-jobs). Start the Ollama desktop application and it takes `11434` for itself, so PAIR cannot, and requests reach that local Ollama without ever being routed. +PAIR cannot impose security policies on listeners it does not own, so make +sure things like CORS are properly set on engines. ### Port Map @@ -685,8 +687,9 @@ The important boundaries are: 2. **Electron or terminal interface to broker.** Stdio has one parent peer. Optional socket or named-pipe mode relies on operating-system endpoint permissions. JSON-RPC has no independent per-message token. -3. **Loopback HTTP.** Local clients can submit sensitive inference content. - Listener addresses, browser access, CORS, and host account security matter. +3. **Loopback HTTP.** Local native clients can submit sensitive inference + content. Browser clients are unsupported. Engine configurations of listener + addresses, browser access, CORS, and host account security also matter. 4. **LAN discovery and metadata.** The network can reveal service presence and selected host information. Some enrichment endpoints use plain HTTP. 5. **Pairing bootstrap.** A six-digit PIN bootstraps certificate trust. It is a diff --git a/services/lmstudio-proxy/README.md b/services/lmstudio-proxy/README.md index 71a8b70d..79dbdf5a 100644 --- a/services/lmstudio-proxy/README.md +++ b/services/lmstudio-proxy/README.md @@ -7,7 +7,7 @@ SPDX-License-Identifier: Apache-2.0 A discovery-aware HTTP reverse proxy for LM Studio nodes on the local network. It runs no mDNS browse of its own: its routing targets come from the broker's discovery relay (it sends `discovery:subscribe {services:[lm]}` and replaces its routing overlay from each pushed `discovery:nodes` snapshot) plus user-added manual nodes. It forwards HTTP requests to the selected node, aggregates the model-list route across candidate nodes, and exposes a bidirectional JSON-RPC 2.0 control channel over stdio (or an IPC socket). -> **Clone of `ollama-proxy`.** This proxy is a deliberate clone of [`ollama-proxy`](../ollama-proxy/README.md) so the two share identical routing, failover, CORS, and node-selection behavior — the CORS policy is literally the same code, `nvpair-shared/cors`, and is documented [there](../ollama-proxy/README.md#http-reverse-proxy). The differences are engine-specific: it subscribes to the discovery relay for `lm` nodes, forwards the OpenAI-compatible inference routes (`/v1/chat/completions`, `/v1/completions`, `/v1/embeddings`), tags workloads `lmstudio`, and persists its port to its own file. It has no `--alias-address`, so its self-forward guard covers only its own listener. +> **Clone of `ollama-proxy`.** This proxy is a deliberate clone of [`ollama-proxy`](../ollama-proxy/README.md) so the two share identical routing, failover, browser-origin rejection, and node-selection behavior — the browser boundary is enforced by the same `nvpair-shared/cors` code and is documented [there](../ollama-proxy/README.md#http-reverse-proxy). The differences are engine-specific: it subscribes to the discovery relay for `lm` nodes, forwards the OpenAI-compatible inference routes (`/v1/chat/completions`, `/v1/completions`, `/v1/embeddings`), tags workloads `lmstudio`, and persists its port to its own file. It has no `--alias-address`, so its self-forward guard covers only its own listener. ## Build diff --git a/services/lmstudio-proxy/e2e_test.go b/services/lmstudio-proxy/e2e_test.go index 5412ab7c..c56895ab 100644 --- a/services/lmstudio-proxy/e2e_test.go +++ b/services/lmstudio-proxy/e2e_test.go @@ -144,9 +144,9 @@ func e2eSplitHostPort(t *testing.T, serverURL string) (string, int) { // drives it the way the broker/UI does: register a busy (503) and a healthy // (200) upstream as manual nodes over JSON-RPC stdio, then send a genuine // OpenAI inference POST to the proxy's real HTTP port. It asserts the request -// fails over from the busy node to the healthy one, the original body is -// replayed, and CORS headers are present — the whole shipped path (binary + -// stdio control plane + HTTP forwarding + failover) end-to-end, no mocks. +// fails over from the busy node to the healthy one and the original body is +// replayed — the whole shipped path (binary + stdio control plane + HTTP +// forwarding + failover) end-to-end, no mocks. func TestE2EFailoverOverRealBinary(t *testing.T) { var gotBody string busy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -205,8 +205,8 @@ func TestE2EFailoverOverRealBinary(t *testing.T) { if resp.StatusCode != http.StatusOK { t.Fatalf("status = %d, want 200 (should fail over from the 503 node)", resp.StatusCode) } - if got := resp.Header.Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want *", got) + if got := resp.Header.Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("Access-Control-Allow-Origin = %q, want absent", got) } if gotBody != `{"model":"m"}` { t.Errorf("healthy upstream got body %q, want the original request body", gotBody) diff --git a/services/lmstudio-proxy/failover_test.go b/services/lmstudio-proxy/failover_test.go index 02e5361b..ecc9c4d9 100644 --- a/services/lmstudio-proxy/failover_test.go +++ b/services/lmstudio-proxy/failover_test.go @@ -53,115 +53,63 @@ func nodeForModel(t *testing.T, id, serverURL, model string) Node { return node } -// TestHandlePlain_OptionsPreflight: a CORS preflight is answered locally with -// 204 + permissive headers and never forwarded. -func TestHandlePlain_OptionsPreflight(t *testing.T) { - p := testProxy(NewDiscovery(), 11434) - rec := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodOptions, "/v1/chat/completions", nil) - req.RemoteAddr = "127.0.0.1:40000" - req.Header.Set("Access-Control-Request-Headers", "X-Custom-Token") - p.handlePlain(rec, req) - - if rec.Code != http.StatusNoContent { - t.Fatalf("status = %d, want 204", rec.Code) - } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want *", got) - } - if rec.Header().Get("Access-Control-Allow-Methods") == "" { - t.Errorf("missing Access-Control-Allow-Methods") - } - if got := rec.Header().Get("Access-Control-Expose-Headers"); got != "*" { - t.Errorf("Access-Control-Expose-Headers = %q, want *", got) - } - // The browser's requested headers are echoed so an arbitrary header clears preflight. - if got := rec.Header().Get("Access-Control-Allow-Headers"); got != "X-Custom-Token" { - t.Errorf("Access-Control-Allow-Headers = %q, want echoed X-Custom-Token", got) - } -} - -// TestHandlePlain_EngineCredentialedPreflightPreserved: when an engine opts an -// exact origin into credentialed CORS, its preflight policy reaches the browser -// instead of being replaced by the proxy's uncredentialed wildcard fallback. -func TestHandlePlain_EngineCredentialedPreflightPreserved(t *testing.T) { - preflightSeen := make(chan struct{}, 1) +// TestHandlePlainRejectsBrowserRequestBeforeRouting proves that browser-marked +// traffic is denied at ingress and never reaches an engine. +func TestHandlePlainRejectsBrowserRequestBeforeRouting(t *testing.T) { + engineHits := 0 engine := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodOptions { - t.Errorf("engine method = %s, want OPTIONS", r.Method) - } - preflightSeen <- struct{}{} - w.Header().Set("Access-Control-Allow-Origin", "https://app.example") - w.Header().Set("Access-Control-Allow-Credentials", "true") - w.Header().Set("Access-Control-Allow-Methods", "POST") - w.Header().Set("Access-Control-Allow-Headers", "Content-Type") - w.WriteHeader(http.StatusNoContent) + engineHits++ + w.WriteHeader(http.StatusOK) })) defer engine.Close() disc := NewDiscovery() - disc.AddManual(nodeFor(t, "engine", engine.URL)) + disc.AddManual(nodeForModel(t, "engine", engine.URL, "llama")) p := testProxy(disc, 11434) - req := httptest.NewRequest(http.MethodOptions, "/v1/chat/completions", nil) + req := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", strings.NewReader(`{"model":"llama"}`)) req.RemoteAddr = "127.0.0.1:40000" - req.Header.Set("Origin", "https://app.example") - req.Header.Set("Access-Control-Request-Method", http.MethodPost) - req.Header.Set("Access-Control-Request-Headers", "Content-Type") + req.Header.Set("Origin", "https://attacker.example") + req.Header.Set("Sec-Fetch-Site", "cross-site") rec := httptest.NewRecorder() p.handlePlain(rec, req) - select { - case <-preflightSeen: - default: - t.Fatal("engine did not receive the credentialed preflight") - } - if rec.Code != http.StatusNoContent { - t.Errorf("status = %d, want %d", rec.Code, http.StatusNoContent) + if rec.Code != http.StatusForbidden { + t.Errorf("status = %d, want %d", rec.Code, http.StatusForbidden) } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "https://app.example" { - t.Errorf("Access-Control-Allow-Origin = %q, want the engine's exact origin", got) + if engineHits != 0 { + t.Errorf("engine hits = %d, want 0", engineHits) } - if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "true" { - t.Errorf("Access-Control-Allow-Credentials = %q, want the engine's true", got) + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("Access-Control-Allow-Origin = %q, want absent", got) } } -// TestHandleHTTP_EngineCORSPolicyPreserved: an engine that declares its own -// origin policy keeps it. Replacing it with the proxy's wildcard would widen -// what the user configured, and would break a credentialed response outright. -func TestHandleHTTP_EngineCORSPolicyPreserved(t *testing.T) { - engine := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("Access-Control-Allow-Origin", "https://app.example") - w.Header().Set("Access-Control-Allow-Credentials", "true") - w.WriteHeader(http.StatusOK) - io.WriteString(w, `{"done":true}`) - })) - defer engine.Close() - - disc := NewDiscovery() - disc.AddManual(nodeForModel(t, "engine", engine.URL, "llama")) - p := testProxy(disc, 11434) - +func TestHandlePlainRejectsPreflight(t *testing.T) { + p := testProxy(NewDiscovery(), 11434) + req := httptest.NewRequest(http.MethodOptions, "/v1/chat/completions", nil) + req.RemoteAddr = "127.0.0.1:40000" + req.Header.Set("Origin", "https://attacker.example") + req.Header.Set("Access-Control-Request-Method", http.MethodPost) rec := httptest.NewRecorder() - p.handleHTTP(rec, httptest.NewRequest(http.MethodPost, "/v1/chat/completions", strings.NewReader(`{"model":"llama"}`))) - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "https://app.example" { - t.Errorf("Access-Control-Allow-Origin = %q, want the engine's own origin", got) + p.handlePlain(rec, req) + + if rec.Code != http.StatusForbidden { + t.Errorf("status = %d, want %d", rec.Code, http.StatusForbidden) } - if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "true" { - t.Errorf("Access-Control-Allow-Credentials = %q, want the engine's true", got) + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("Access-Control-Allow-Origin = %q, want absent", got) } } -// TestHandleHTTP_EngineCredentialsWithoutOriginDropped: an engine (or an -// intermediary in front of it) that sends Allow-Credentials but no origin has -// declared no policy to keep, so the proxy supplies its own. The wildcard it -// writes is invalid next to Allow-Credentials: true, and a browser rejects that -// pair, so the inherited header must not survive the forward. -func TestHandleHTTP_EngineCredentialsWithoutOriginDropped(t *testing.T) { +// TestHandleHTTPStripsEngineAllowOrigin proves an engine cannot widen PAIR's +// browser-origin boundary with its own Access-Control-Allow-Origin value. +func TestHandleHTTPStripsEngineAllowOrigin(t *testing.T) { engine := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Access-Control-Allow-Origin", "https://app.example") w.Header().Set("Access-Control-Allow-Credentials", "true") + w.Header().Set("Access-Control-Expose-Headers", "*") w.WriteHeader(http.StatusOK) io.WriteString(w, `{"done":true}`) })) @@ -174,16 +122,22 @@ func TestHandleHTTP_EngineCredentialsWithoutOriginDropped(t *testing.T) { rec := httptest.NewRecorder() p.handleHTTP(rec, httptest.NewRequest(http.MethodPost, "/v1/chat/completions", strings.NewReader(`{"model":"llama"}`))) - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want the proxy's wildcard", got) + if rec.Code != http.StatusOK || rec.Body.String() != `{"done":true}` { + t.Errorf("native response = status %d body %q, want forwarded success", rec.Code, rec.Body.String()) + } + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("Access-Control-Allow-Origin = %q, want absent", got) } - if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "" { - t.Errorf("Access-Control-Allow-Credentials = %q, want cleared alongside the wildcard origin", got) + if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "true" { + t.Errorf("Access-Control-Allow-Credentials = %q, want preserved", got) + } + if got := rec.Header().Get("Access-Control-Expose-Headers"); got != "*" { + t.Errorf("Access-Control-Expose-Headers = %q, want preserved", got) } } // TestHandleHTTP_HappyPathSingleNode: the common case — one healthy node -// answers directly, body forwarded, CORS present on the success response. +// answers directly and its body is forwarded to the native client. func TestHandleHTTP_HappyPathSingleNode(t *testing.T) { var gotBody string good := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -207,9 +161,6 @@ func TestHandleHTTP_HappyPathSingleNode(t *testing.T) { if gotBody != `{"model":"llama"}` { t.Errorf("node got body %q, want the original request body", gotBody) } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want * on success", got) - } } // TestHandleHTTP_NoRetryOn400: a client error (400) is returned as-is and not @@ -244,9 +195,7 @@ func TestHandleHTTP_NoRetryOn400(t *testing.T) { } } -// TestHandleHTTP_RejectionHasCORS: even the no-node rejection carries CORS so a -// browser sees the real 502 instead of an opaque CORS error. -func TestHandleHTTP_RejectionHasCORS(t *testing.T) { +func TestHandleHTTPRejectionHasNoCORS(t *testing.T) { p := testProxy(NewDiscovery(), 11434) rec := httptest.NewRecorder() p.handleHTTP(rec, httptest.NewRequest(http.MethodPost, "/v1/chat/completions", strings.NewReader(`{"model":"x"}`))) @@ -254,8 +203,8 @@ func TestHandleHTTP_RejectionHasCORS(t *testing.T) { if rec.Code != http.StatusBadGateway { t.Fatalf("status = %d, want 502", rec.Code) } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want * on rejection", got) + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("Access-Control-Allow-Origin = %q, want absent", got) } } @@ -292,13 +241,10 @@ func TestHandleHTTP_FailoverOn503(t *testing.T) { if gotBody != `{"model":"llama"}` { t.Errorf("failover node got body %q, want the original request body", gotBody) } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want * on proxied success", got) - } } // TestHandleHTTP_AllNodesDownReturnsError: when every candidate fails at the -// transport, the client gets one clean 502 (not a hang), still with CORS. +// transport, the client gets one clean 502 rather than hanging. func TestHandleHTTP_AllNodesDownReturnsError(t *testing.T) { // Two servers we immediately close so dials fail. a := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) @@ -319,9 +265,6 @@ func TestHandleHTTP_AllNodesDownReturnsError(t *testing.T) { if rec.Code != http.StatusBadGateway { t.Fatalf("status = %d, want 502 when all nodes are down", rec.Code) } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want * on exhausted error", got) - } } // TestHandleHTTP_404FailoverInferenceOnly: a 404 (model-not-found) on an @@ -445,9 +388,6 @@ func TestHandleHTTP_AggregatesModelList(t *testing.T) { if got.Data[1].OwnedBy != "first" { t.Errorf("duplicate metadata = %q, want deterministic first candidate", got.Data[1].OwnedBy) } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want *", got) - } } func TestHandleHTTP_ModelListEmptyAndUnavailable(t *testing.T) { diff --git a/services/lmstudio-proxy/ingress.go b/services/lmstudio-proxy/ingress.go index 2b70e697..b6b0c1dd 100644 --- a/services/lmstudio-proxy/ingress.go +++ b/services/lmstudio-proxy/ingress.go @@ -61,14 +61,10 @@ func (p *Proxy) localBackendTarget() (*url.URL, bool) { // what closes the former open-relay exposure (the listener still binds all // interfaces for the TLS personality, but plaintext is loopback-only). func (p *Proxy) handlePlain(w http.ResponseWriter, r *http.Request) { + if cors.RejectBrowserRequest(w, r) { + return + } if !isLoopbackRemote(r.RemoteAddr) { - // Answer a non-loopback preflight ahead of the gate. It grants no access - // on its own; the request that follows still receives the real 403. A - // loopback preflight continues into handleHTTP so an available engine's - // exact origin and credentials policy can be preserved. - if cors.WritePreflight(w, r) { - return - } slog.Warn("rejected non-loopback plaintext request; cluster peers must use mTLS", "remote", r.RemoteAddr, "method", r.Method, "path", r.URL.Path) writeIngressError(w, http.StatusForbidden, "loopback-only", @@ -149,11 +145,8 @@ func isLoopbackRemote(remoteAddr string) bool { } // writeIngressError writes a small structured JSON error. It never echoes the -// request body or any generated output. CORS headers are included because these -// are the proxy's own rejections: without them a browser client cannot read the -// status or reason, and every one of them looks like a generic CORS failure. +// request body or any generated output. func writeIngressError(w http.ResponseWriter, status int, code, msg string) { - cors.Apply(w.Header()) w.Header().Set("Content-Type", "application/json") w.Header().Set("X-Content-Type-Options", "nosniff") w.WriteHeader(status) diff --git a/services/lmstudio-proxy/ingress_test.go b/services/lmstudio-proxy/ingress_test.go index 2f1e1869..f79761bf 100644 --- a/services/lmstudio-proxy/ingress_test.go +++ b/services/lmstudio-proxy/ingress_test.go @@ -46,29 +46,25 @@ func TestHandlePlainRejectsNonLoopback(t *testing.T) { if rec.Code != http.StatusForbidden { t.Fatalf("non-loopback plaintext status = %d, want %d", rec.Code, http.StatusForbidden) } - // The refusal carries CORS so a browser client reads this 403 and its reason - // instead of an opaque "CORS error" that hides why the call failed. - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want * on the refusal", got) + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("Access-Control-Allow-Origin = %q, want absent", got) } } -// TestHandlePlainAnswersPreflightBeforeLoopbackGate: the preflight is answered -// even for a caller the gate will refuse. It authorizes nothing — the request -// that follows is still rejected — but without it the browser never sends that -// request and reports the refusal as a generic CORS failure. -func TestHandlePlainAnswersPreflightBeforeLoopbackGate(t *testing.T) { +func TestHandlePlainRejectsPreflightBeforeLoopbackGate(t *testing.T) { p := testProxy(NewDiscovery(), 1235) req := httptest.NewRequest(http.MethodOptions, "/v1/chat/completions", nil) req.RemoteAddr = "192.0.2.50:40000" + req.Header.Set("Origin", "https://attacker.example") + req.Header.Set("Access-Control-Request-Method", http.MethodPost) rec := httptest.NewRecorder() p.handlePlain(rec, req) - if rec.Code != http.StatusNoContent { - t.Fatalf("preflight status = %d, want %d", rec.Code, http.StatusNoContent) + if rec.Code != http.StatusForbidden { + t.Fatalf("preflight status = %d, want %d", rec.Code, http.StatusForbidden) } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want *", got) + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("Access-Control-Allow-Origin = %q, want absent", got) } } diff --git a/services/lmstudio-proxy/proxy.go b/services/lmstudio-proxy/proxy.go index 6e619e77..1b49d4be 100644 --- a/services/lmstudio-proxy/proxy.go +++ b/services/lmstudio-proxy/proxy.go @@ -815,7 +815,6 @@ type modelListResult struct { // deterministic while an unavailable peer cannot hide healthy inventories. func (p *Proxy) serveModelList(w http.ResponseWriter, r *http.Request, candidates []candidate) (int, error) { writeJSON := func(status int, body []byte) { - cors.Apply(w.Header()) w.Header().Set("Content-Type", "application/json") w.Header().Set("X-Content-Type-Options", "nosniff") w.WriteHeader(status) @@ -973,12 +972,6 @@ func (p *Proxy) handleHTTP(w http.ResponseWriter, r *http.Request) { return } if len(candidates) == 0 { - // With no engine to consult, retain the local permissive preflight used - // for engines that do not publish a CORS policy. - if cors.WritePreflight(w, r) { - return - } - cors.Apply(w.Header()) rejectionBody := `{"error":"no active node selected or available"}` rejectionError := "no active node" if isInf && model != "" { @@ -1148,10 +1141,7 @@ func (p *Proxy) handleHTTP(w http.ResponseWriter, r *http.Request) { retry = true return retrySignal{} } - // Prefer an engine-declared preflight policy so an exact origin plus - // Allow-Credentials can pass a credentialed browser fetch. Engines - // that publish no policy retain the proxy's permissive 204 fallback. - cors.CompletePreflightFallback(resp) + cors.StripAllowOrigin(resp.Header) // Committing to this candidate — body stream is about to begin. ttfbMs = time.Since(start).Milliseconds() servedNodeID = cand.id @@ -1165,15 +1155,6 @@ func (p *Proxy) handleHTTP(w http.ResponseWriter, r *http.Request) { // came from the node. Same goroutine as the body copy, so no // synchronization is needed. sc.upstreamAlive = func() { p.reportActivity(cand.id) } - // The engine may enforce its own origin policy. Honor it: - // overwriting a declared Access-Control-Allow-Origin would - // silently widen the user's policy, and a wildcard is invalid - // alongside Allow-Credentials, so it would break a credentialed - // response outright. An engine that omits the header has - // expressed nothing to preserve, so the proxy supplies its own. - if resp.Header.Get("Access-Control-Allow-Origin") == "" { - cors.Apply(resp.Header) - } if !started { started = true p.codec.Notify("proxy/request-started", RequestStartedEvent{ @@ -1226,10 +1207,6 @@ func (p *Proxy) handleHTTP(w http.ResponseWriter, r *http.Request) { // Last candidate failed at the transport: terminal, surface it. servedNodeID = cand.id servedTarget = cand.url.Host - if cors.WritePreflight(ew, r) { - proxyErr = "" - return - } proxyErr = err.Error() slog.Warn("proxy upstream error, candidates exhausted", "id", reqID, "node_id", cand.id, "target", cand.url.Host, @@ -1241,7 +1218,6 @@ func (p *Proxy) handleHTTP(w http.ResponseWriter, r *http.Request) { if mErr != nil { body = []byte(`{"error":"upstream error"}`) } - cors.Apply(ew.Header()) ew.Header().Set("Content-Type", "application/json") ew.Header().Set("X-Content-Type-Options", "nosniff") ew.WriteHeader(http.StatusBadGateway) diff --git a/services/ollama-proxy/README.md b/services/ollama-proxy/README.md index 35f959b5..b862f9d8 100644 --- a/services/ollama-proxy/README.md +++ b/services/ollama-proxy/README.md @@ -39,11 +39,7 @@ The proxy listens on `--port` (default 11435) and forwards incoming HTTP request **Persisted port.** A port chosen at runtime via the `set-port` request (see below) is saved as `proxy-port.json` in the per-user data dir (`%LocalAppData%\Nvidia Corporation\Personal AI Router` on Windows, `~/.config/Nvidia Corporation/Personal AI Router` on Linux) and **restored on startup**, taking precedence over `--port`/the default — so the proxy comes back up where it was last put. `--ignore-persisted-port` deliberately bypasses that restoration for a broker-coordinated start. Delete the file (or `set-port` back to the default) to revert. -**Browser clients (CORS).** The proxy is usable from a web front end. When an engine is available, the proxy forwards an `OPTIONS` preflight so an engine-declared exact origin and credentials policy reaches the browser unchanged. If no engine is available, the engine returns no CORS policy, or a non-loopback caller must be refused before routing, the proxy answers with its own permissive `204` fallback. It also labels every response it generates — including rejections such as the `502` when no node is available and the `403` refusing a non-loopback plaintext caller — with `Access-Control-Allow-Origin: *`. That matters as much as the success path: a response without those headers reaches the browser as a generic "CORS error" with the real status and reason stripped out, so the caller cannot tell what went wrong. A locally answered preflight grants no access, since the request that follows still faces the same gate. The policy is shared with [`lmstudio-proxy`](../lmstudio-proxy/README.md) through `nvpair-shared/cors`, so both proxies answer identically. - -A response *forwarded from an engine* is different: if the engine set its own `Access-Control-Allow-Origin` (Ollama with `OLLAMA_ORIGINS`), that header is passed through untouched rather than replaced with the wildcard, so a deliberately narrow engine policy is never silently widened and a credentialed response is not broken. An engine that sends no CORS header has expressed no policy to keep, so the proxy supplies its own — and drops any `Access-Control-Allow-Credentials` that arrived without an origin, because a browser rejects that header alongside a wildcard origin and would discard the response the fallback exists to make readable. - -One limit is outside the proxy's control: current Chromium-based browsers gate a request from a public origin to a local or loopback address behind the user's [Local Network Access](https://chromestatus.com/feature/5152728072060928) permission, which replaced the old server-side opt-in header. No header the proxy sends can grant that. A hosted page needs the permission plus a `fetch(url, { targetAddressSpace: 'loopback' })` annotation; a page served from the local machine is unaffected. +**Browser clients are not supported.** The plaintext ingress rejects requests carrying browser-controlled `Origin`, `Sec-Fetch-*`, or `Access-Control-Request-*` headers before routing, and it returns no CORS grant. Responses forwarded from an engine have `Access-Control-Allow-Origin` removed, so engine configuration such as Ollama's `OLLAMA_ORIGINS` cannot widen PAIR's boundary. Originless native clients continue to use the loopback endpoint normally. The policy is shared with [`lmstudio-proxy`](../lmstudio-proxy/README.md) through `nvpair-shared/cors`, so both proxies enforce the same boundary. It applies only to the proxy listener: a direct request to an engine's private port remains subject to that engine's own CORS policy. Node selection: - **Eligibility**: Before routing model-bearing inference, the proxy keeps only nodes whose current Ollama inventory advertises the requested model. Ollama's implicit `:latest` tag is normalized. An empty or non-matching inventory is excluded until a later discovery update; if no advertised owner is routable, the proxy returns a local `502`. diff --git a/services/ollama-proxy/failover_test.go b/services/ollama-proxy/failover_test.go index 48784714..dec6bb0b 100644 --- a/services/ollama-proxy/failover_test.go +++ b/services/ollama-proxy/failover_test.go @@ -53,115 +53,63 @@ func nodeForModel(t *testing.T, id, serverURL, model string) Node { return node } -// TestHandlePlain_OptionsPreflight: a CORS preflight is answered locally with -// 204 + permissive headers and never forwarded. -func TestHandlePlain_OptionsPreflight(t *testing.T) { - p := testProxy(NewDiscovery(), 11434) - rec := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodOptions, "/api/chat", nil) - req.RemoteAddr = "127.0.0.1:40000" - req.Header.Set("Access-Control-Request-Headers", "X-Custom-Token") - p.handlePlain(rec, req) - - if rec.Code != http.StatusNoContent { - t.Fatalf("status = %d, want 204", rec.Code) - } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want *", got) - } - if rec.Header().Get("Access-Control-Allow-Methods") == "" { - t.Errorf("missing Access-Control-Allow-Methods") - } - if got := rec.Header().Get("Access-Control-Expose-Headers"); got != "*" { - t.Errorf("Access-Control-Expose-Headers = %q, want *", got) - } - // The browser's requested headers are echoed so an arbitrary header clears preflight. - if got := rec.Header().Get("Access-Control-Allow-Headers"); got != "X-Custom-Token" { - t.Errorf("Access-Control-Allow-Headers = %q, want echoed X-Custom-Token", got) - } -} - -// TestHandlePlain_EngineCredentialedPreflightPreserved: when an engine opts an -// exact origin into credentialed CORS, its preflight policy reaches the browser -// instead of being replaced by the proxy's uncredentialed wildcard fallback. -func TestHandlePlain_EngineCredentialedPreflightPreserved(t *testing.T) { - preflightSeen := make(chan struct{}, 1) +// TestHandlePlainRejectsBrowserRequestBeforeRouting proves that browser-marked +// traffic is denied at ingress and never reaches an engine. +func TestHandlePlainRejectsBrowserRequestBeforeRouting(t *testing.T) { + engineHits := 0 engine := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodOptions { - t.Errorf("engine method = %s, want OPTIONS", r.Method) - } - preflightSeen <- struct{}{} - w.Header().Set("Access-Control-Allow-Origin", "https://app.example") - w.Header().Set("Access-Control-Allow-Credentials", "true") - w.Header().Set("Access-Control-Allow-Methods", "POST") - w.Header().Set("Access-Control-Allow-Headers", "Content-Type") - w.WriteHeader(http.StatusNoContent) + engineHits++ + w.WriteHeader(http.StatusOK) })) defer engine.Close() disc := NewDiscovery() - disc.AddManual(nodeFor(t, "engine", engine.URL)) + disc.AddManual(nodeForModel(t, "engine", engine.URL, "llama")) p := testProxy(disc, 11434) - req := httptest.NewRequest(http.MethodOptions, "/api/chat", nil) + req := httptest.NewRequest(http.MethodPost, "/api/chat", strings.NewReader(`{"model":"llama"}`)) req.RemoteAddr = "127.0.0.1:40000" - req.Header.Set("Origin", "https://app.example") - req.Header.Set("Access-Control-Request-Method", http.MethodPost) - req.Header.Set("Access-Control-Request-Headers", "Content-Type") + req.Header.Set("Origin", "https://attacker.example") + req.Header.Set("Sec-Fetch-Site", "cross-site") rec := httptest.NewRecorder() p.handlePlain(rec, req) - select { - case <-preflightSeen: - default: - t.Fatal("engine did not receive the credentialed preflight") - } - if rec.Code != http.StatusNoContent { - t.Errorf("status = %d, want %d", rec.Code, http.StatusNoContent) + if rec.Code != http.StatusForbidden { + t.Errorf("status = %d, want %d", rec.Code, http.StatusForbidden) } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "https://app.example" { - t.Errorf("Access-Control-Allow-Origin = %q, want the engine's exact origin", got) + if engineHits != 0 { + t.Errorf("engine hits = %d, want 0", engineHits) } - if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "true" { - t.Errorf("Access-Control-Allow-Credentials = %q, want the engine's true", got) + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("Access-Control-Allow-Origin = %q, want absent", got) } } -// TestHandleHTTP_EngineCORSPolicyPreserved: an engine that declares its own -// origin policy keeps it. Replacing it with the proxy's wildcard would widen -// what the user configured, and would break a credentialed response outright. -func TestHandleHTTP_EngineCORSPolicyPreserved(t *testing.T) { - engine := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.Header().Set("Access-Control-Allow-Origin", "https://app.example") - w.Header().Set("Access-Control-Allow-Credentials", "true") - w.WriteHeader(http.StatusOK) - io.WriteString(w, `{"done":true}`) - })) - defer engine.Close() - - disc := NewDiscovery() - disc.AddManual(nodeForModel(t, "engine", engine.URL, "llama")) - p := testProxy(disc, 11434) - +func TestHandlePlainRejectsPreflight(t *testing.T) { + p := testProxy(NewDiscovery(), 11434) + req := httptest.NewRequest(http.MethodOptions, "/api/chat", nil) + req.RemoteAddr = "127.0.0.1:40000" + req.Header.Set("Origin", "https://attacker.example") + req.Header.Set("Access-Control-Request-Method", http.MethodPost) rec := httptest.NewRecorder() - p.handleHTTP(rec, httptest.NewRequest(http.MethodPost, "/api/chat", strings.NewReader(`{"model":"llama"}`))) - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "https://app.example" { - t.Errorf("Access-Control-Allow-Origin = %q, want the engine's own origin", got) + p.handlePlain(rec, req) + + if rec.Code != http.StatusForbidden { + t.Errorf("status = %d, want %d", rec.Code, http.StatusForbidden) } - if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "true" { - t.Errorf("Access-Control-Allow-Credentials = %q, want the engine's true", got) + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("Access-Control-Allow-Origin = %q, want absent", got) } } -// TestHandleHTTP_EngineCredentialsWithoutOriginDropped: an engine (or an -// intermediary in front of it) that sends Allow-Credentials but no origin has -// declared no policy to keep, so the proxy supplies its own. The wildcard it -// writes is invalid next to Allow-Credentials: true, and a browser rejects that -// pair, so the inherited header must not survive the forward. -func TestHandleHTTP_EngineCredentialsWithoutOriginDropped(t *testing.T) { +// TestHandleHTTPStripsEngineAllowOrigin proves an engine cannot widen PAIR's +// browser-origin boundary with its own Access-Control-Allow-Origin value. +func TestHandleHTTPStripsEngineAllowOrigin(t *testing.T) { engine := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Access-Control-Allow-Origin", "https://app.example") w.Header().Set("Access-Control-Allow-Credentials", "true") + w.Header().Set("Access-Control-Expose-Headers", "*") w.WriteHeader(http.StatusOK) io.WriteString(w, `{"done":true}`) })) @@ -174,16 +122,22 @@ func TestHandleHTTP_EngineCredentialsWithoutOriginDropped(t *testing.T) { rec := httptest.NewRecorder() p.handleHTTP(rec, httptest.NewRequest(http.MethodPost, "/api/chat", strings.NewReader(`{"model":"llama"}`))) - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want the proxy's wildcard", got) + if rec.Code != http.StatusOK || rec.Body.String() != `{"done":true}` { + t.Errorf("native response = status %d body %q, want forwarded success", rec.Code, rec.Body.String()) + } + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("Access-Control-Allow-Origin = %q, want absent", got) } - if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "" { - t.Errorf("Access-Control-Allow-Credentials = %q, want cleared alongside the wildcard origin", got) + if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "true" { + t.Errorf("Access-Control-Allow-Credentials = %q, want preserved", got) + } + if got := rec.Header().Get("Access-Control-Expose-Headers"); got != "*" { + t.Errorf("Access-Control-Expose-Headers = %q, want preserved", got) } } // TestHandleHTTP_HappyPathSingleNode: the common case — one healthy node -// answers directly, body forwarded, CORS present on the success response. +// answers directly and its body is forwarded to the native client. func TestHandleHTTP_HappyPathSingleNode(t *testing.T) { var gotBody string good := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -207,9 +161,6 @@ func TestHandleHTTP_HappyPathSingleNode(t *testing.T) { if gotBody != `{"model":"llama"}` { t.Errorf("node got body %q, want the original request body", gotBody) } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want * on success", got) - } } // TestHandleHTTP_NoRetryOn400: a client error (400) is returned as-is and not @@ -244,9 +195,7 @@ func TestHandleHTTP_NoRetryOn400(t *testing.T) { } } -// TestHandleHTTP_RejectionHasCORS: even the no-node rejection carries CORS so a -// browser sees the real 502 instead of an opaque CORS error. -func TestHandleHTTP_RejectionHasCORS(t *testing.T) { +func TestHandleHTTPRejectionHasNoCORS(t *testing.T) { p := testProxy(NewDiscovery(), 11434) rec := httptest.NewRecorder() p.handleHTTP(rec, httptest.NewRequest(http.MethodPost, "/api/chat", strings.NewReader(`{"model":"x"}`))) @@ -254,8 +203,8 @@ func TestHandleHTTP_RejectionHasCORS(t *testing.T) { if rec.Code != http.StatusBadGateway { t.Fatalf("status = %d, want 502", rec.Code) } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want * on rejection", got) + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("Access-Control-Allow-Origin = %q, want absent", got) } } @@ -292,13 +241,10 @@ func TestHandleHTTP_FailoverOn503(t *testing.T) { if gotBody != `{"model":"llama"}` { t.Errorf("failover node got body %q, want the original request body", gotBody) } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want * on proxied success", got) - } } // TestHandleHTTP_AllNodesDownReturnsError: when every candidate fails at the -// transport, the client gets one clean 502 (not a hang), still with CORS. +// transport, the client gets one clean 502 rather than hanging. func TestHandleHTTP_AllNodesDownReturnsError(t *testing.T) { // Two servers we immediately close so dials fail. a := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) @@ -319,9 +265,6 @@ func TestHandleHTTP_AllNodesDownReturnsError(t *testing.T) { if rec.Code != http.StatusBadGateway { t.Fatalf("status = %d, want 502 when all nodes are down", rec.Code) } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want * on exhausted error", got) - } } // TestHandleHTTP_404FailoverInferenceOnly: a 404 (model-not-found) on an @@ -443,9 +386,6 @@ func TestHandleHTTP_AggregatesModelList(t *testing.T) { if got.Models[1].Digest != "first" { t.Errorf("duplicate metadata = %q, want deterministic first candidate", got.Models[1].Digest) } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want *", got) - } if !events.has(`"method":"proxy/request-started"`) || !events.has(`"method":"proxy/request"`) || !events.has(`"target":"cluster"`) { t.Errorf("aggregate telemetry missing paired cluster events: %s", events.b) } diff --git a/services/ollama-proxy/ingress.go b/services/ollama-proxy/ingress.go index 984b7b20..f4cc0ae0 100644 --- a/services/ollama-proxy/ingress.go +++ b/services/ollama-proxy/ingress.go @@ -61,14 +61,10 @@ func (p *Proxy) localBackendTarget() (*url.URL, bool) { // what closes the former open-relay exposure (the listener still binds all // interfaces for the TLS personality, but plaintext is loopback-only). func (p *Proxy) handlePlain(w http.ResponseWriter, r *http.Request) { + if cors.RejectBrowserRequest(w, r) { + return + } if !isLoopbackRemote(r.RemoteAddr) { - // Answer a non-loopback preflight ahead of the gate. It grants no access - // on its own; the request that follows still receives the real 403. A - // loopback preflight continues into handleHTTP so an available engine's - // exact origin and credentials policy can be preserved. - if cors.WritePreflight(w, r) { - return - } slog.Warn("rejected non-loopback plaintext request; cluster peers must use mTLS", "remote", r.RemoteAddr, "method", r.Method, "path", r.URL.Path) writeIngressError(w, http.StatusForbidden, "loopback-only", @@ -149,11 +145,8 @@ func isLoopbackRemote(remoteAddr string) bool { } // writeIngressError writes a small structured JSON error. It never echoes the -// request body or any generated output. CORS headers are included because these -// are the proxy's own rejections: without them a browser client cannot read the -// status or reason, and every one of them looks like a generic CORS failure. +// request body or any generated output. func writeIngressError(w http.ResponseWriter, status int, code, msg string) { - cors.Apply(w.Header()) w.Header().Set("Content-Type", "application/json") w.Header().Set("X-Content-Type-Options", "nosniff") w.WriteHeader(status) diff --git a/services/ollama-proxy/ingress_test.go b/services/ollama-proxy/ingress_test.go index 34934a1a..e1ec56a1 100644 --- a/services/ollama-proxy/ingress_test.go +++ b/services/ollama-proxy/ingress_test.go @@ -51,29 +51,25 @@ func TestHandlePlainRejectsNonLoopback(t *testing.T) { if rec.Code != http.StatusForbidden { t.Fatalf("non-loopback plaintext status = %d, want %d", rec.Code, http.StatusForbidden) } - // The refusal carries CORS so a browser client reads this 403 and its reason - // instead of an opaque "CORS error" that hides why the call failed. - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want * on the refusal", got) + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("Access-Control-Allow-Origin = %q, want absent", got) } } -// TestHandlePlainAnswersPreflightBeforeLoopbackGate: the preflight is answered -// even for a caller the gate will refuse. It authorizes nothing — the request -// that follows is still rejected — but without it the browser never sends that -// request and reports the refusal as a generic CORS failure. -func TestHandlePlainAnswersPreflightBeforeLoopbackGate(t *testing.T) { +func TestHandlePlainRejectsPreflightBeforeLoopbackGate(t *testing.T) { p := testProxy(NewDiscovery(), 11435) req := httptest.NewRequest(http.MethodOptions, "/api/generate", nil) req.RemoteAddr = "192.0.2.50:40000" + req.Header.Set("Origin", "https://attacker.example") + req.Header.Set("Access-Control-Request-Method", http.MethodPost) rec := httptest.NewRecorder() p.handlePlain(rec, req) - if rec.Code != http.StatusNoContent { - t.Fatalf("preflight status = %d, want %d", rec.Code, http.StatusNoContent) + if rec.Code != http.StatusForbidden { + t.Fatalf("preflight status = %d, want %d", rec.Code, http.StatusForbidden) } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want *", got) + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("Access-Control-Allow-Origin = %q, want absent", got) } } diff --git a/services/ollama-proxy/proxy.go b/services/ollama-proxy/proxy.go index ad4ac7a2..45af2a80 100644 --- a/services/ollama-proxy/proxy.go +++ b/services/ollama-proxy/proxy.go @@ -981,7 +981,6 @@ func ollamaModelKey(model string) string { func (p *Proxy) serveModelList(w http.ResponseWriter, r *http.Request, candidates []candidate) (int, error) { openAI := r.URL.Path == "/v1/models" writeJSON := func(status int, body []byte) { - cors.Apply(w.Header()) w.Header().Set("Content-Type", "application/json") w.Header().Set("X-Content-Type-Options", "nosniff") w.WriteHeader(status) @@ -1168,12 +1167,6 @@ func (p *Proxy) handleHTTP(w http.ResponseWriter, r *http.Request) { return } if len(candidates) == 0 { - // With no engine to consult, retain the local permissive preflight used - // for engines that do not publish a CORS policy. - if cors.WritePreflight(w, r) { - return - } - cors.Apply(w.Header()) rejectionBody := `{"error":"no active node selected or available"}` rejectionError := "no active node" if isInf && model != "" { @@ -1343,10 +1336,7 @@ func (p *Proxy) handleHTTP(w http.ResponseWriter, r *http.Request) { retry = true return retrySignal{} } - // Prefer an engine-declared preflight policy so an exact origin plus - // Allow-Credentials can pass a credentialed browser fetch. Engines - // that publish no policy retain the proxy's permissive 204 fallback. - cors.CompletePreflightFallback(resp) + cors.StripAllowOrigin(resp.Header) // Committing to this candidate — body stream is about to begin. ttfbMs = time.Since(start).Milliseconds() servedNodeID = cand.id @@ -1360,16 +1350,6 @@ func (p *Proxy) handleHTTP(w http.ResponseWriter, r *http.Request) { // came from the node. Same goroutine as the body copy, so no // synchronization is needed. sc.upstreamAlive = func() { p.reportActivity(cand.id) } - // The engine may enforce its own origin policy (Ollama's - // OLLAMA_ORIGINS). Honor it: overwriting a declared - // Access-Control-Allow-Origin would silently widen the user's - // policy, and a wildcard is invalid alongside - // Allow-Credentials, so it would break a credentialed response - // outright. An engine that omits the header has expressed - // nothing to preserve, so the proxy supplies its own. - if resp.Header.Get("Access-Control-Allow-Origin") == "" { - cors.Apply(resp.Header) - } if !started { started = true p.codec.Notify("proxy/request-started", RequestStartedEvent{ @@ -1422,10 +1402,6 @@ func (p *Proxy) handleHTTP(w http.ResponseWriter, r *http.Request) { // Last candidate failed at the transport: terminal, surface it. servedNodeID = cand.id servedTarget = cand.url.Host - if cors.WritePreflight(ew, r) { - proxyErr = "" - return - } proxyErr = err.Error() slog.Warn("proxy upstream error, candidates exhausted", "id", reqID, "node_id", cand.id, "target", cand.url.Host, @@ -1437,7 +1413,6 @@ func (p *Proxy) handleHTTP(w http.ResponseWriter, r *http.Request) { if mErr != nil { body = []byte(`{"error":"upstream error"}`) } - cors.Apply(ew.Header()) ew.Header().Set("Content-Type", "application/json") ew.Header().Set("X-Content-Type-Options", "nosniff") ew.WriteHeader(http.StatusBadGateway) diff --git a/services/shared/cors/cors.go b/services/shared/cors/cors.go index 67f15168..3c2150ba 100644 --- a/services/shared/cors/cors.go +++ b/services/shared/cors/cors.go @@ -1,91 +1,58 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -// Package cors is the single source of truth for the CORS policy NVPAIR's -// inference proxies present to browser clients. Both proxies front a -// local-network inference engine for the same kinds of caller (a local web UI, -// an Electron renderer whose origin differs from the proxy's), so they must -// answer a preflight and label a response identically; keeping one -// implementation is what stops the two from drifting apart. -// -// The policy applies to responses a proxy authors itself. A response forwarded -// from an engine that declared its own Access-Control-Allow-Origin keeps that -// engine's policy — including on a preflight, where preserving an exact origin -// and Access-Control-Allow-Credentials is required for credentialed browser -// requests. An engine without a CORS policy gets the proxy's permissive fallback. +// Package cors enforces the inference proxies' browser-origin boundary. +// PAIR does not expose a browser API: supported local clients are native +// processes, including Electron's main process. Browser-marked requests are +// rejected before routing, and an Access-Control-Allow-Origin supplied by an +// engine is removed before a response crosses the proxy boundary. package cors -import "net/http" +import ( + "net/http" + "strings" +) -// Apply writes the permissive CORS policy so browser-based clients can read -// proxy responses — and, crucially, error bodies. Without an -// Access-Control-Allow-Origin a browser surfaces every failure as an opaque -// "CORS error", hiding the real status the proxy returned. The proxies front a -// local-network inference engine, not a credentialed API, so a wildcard origin -// is appropriate and they never reflect credentials. -func Apply(h http.Header) { - h.Set("Access-Control-Allow-Origin", "*") - h.Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS") - h.Set("Access-Control-Allow-Headers", "Content-Type, Authorization") - // A browser rejects a wildcard origin paired with Allow-Credentials: true, - // so setting the former while inheriting the latter would leave the - // response unreadable — the failure this policy exists to prevent. Callers - // reach here only when no engine policy is being preserved, so an - // Allow-Credentials from an upstream that sent no origin of its own - // describes a policy this response no longer carries. Drop it. - h.Del("Access-Control-Allow-Credentials") - // Uncredentialed wildcard responses may expose every header, so a browser - // client can read engine metadata outside the CORS-safelisted set. - h.Set("Access-Control-Expose-Headers", "*") - h.Set("Access-Control-Max-Age", "86400") -} +const browserRequestError = `{"error":"browser-originated requests are not supported","code":"browser-origin"}` -func applyPreflight(h http.Header, r *http.Request) { - Apply(h) - // Echo the browser's requested headers so an arbitrary client header - // (e.g. a custom auth header) clears preflight instead of being - // rejected by our static default. - if reqHdrs := r.Header.Get("Access-Control-Request-Headers"); reqHdrs != "" { - h.Set("Access-Control-Allow-Headers", reqHdrs) - } -} - -// WritePreflight answers a browser's OPTIONS preflight locally with 204 plus -// the permissive fallback policy, and reports whether it handled the request. -// The proxies use this when no engine can be consulted (or before rejecting a -// non-loopback plaintext caller). When an engine is available, its preflight is -// forwarded instead so an exact origin and credentials policy can survive. -func WritePreflight(w http.ResponseWriter, r *http.Request) bool { - if r.Method != http.MethodOptions { +// RejectBrowserRequest rejects requests carrying browser-controlled CORS or +// Fetch Metadata headers. It returns true after writing the response. +func RejectBrowserRequest(w http.ResponseWriter, r *http.Request) bool { + // TODO: If PAIR adds a supported browser client, replace blanket rejection + // here with an exact, user-configured origin allowlist (scheme, host, and + // port), and return that origin from the response path below. Localhost + // origins should be allowed only when the user deliberately enables them. + if !isBrowserRequest(r.Header) { return false } - applyPreflight(w.Header(), r) - w.WriteHeader(http.StatusNoContent) + + StripAllowOrigin(w.Header()) + w.Header().Set("Content-Type", "application/json") + w.Header().Set("X-Content-Type-Options", "nosniff") + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(browserRequestError)) return true } -// CompletePreflightFallback replaces an upstream OPTIONS response that has no -// CORS policy with the proxy's local 204 fallback. Engines that do declare an -// Access-Control-Allow-Origin are left completely untouched; in particular, -// an exact origin plus Access-Control-Allow-Credentials must reach the browser -// unchanged for a credentialed preflight to pass. -func CompletePreflightFallback(resp *http.Response) bool { - if resp == nil || resp.Request == nil || resp.Request.Method != http.MethodOptions || - resp.Header.Get("Access-Control-Allow-Origin") != "" { - return false - } - if resp.Body != nil { - _ = resp.Body.Close() +// isBrowserRequest identifies headers added by web browsers. If we see one, the +// request came from a browser or a client choosing to look like one; the policy +// rejects both. +// See: +// - https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Origin +// - https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Sec-Fetch-Site +func isBrowserRequest(h http.Header) bool { + for name := range h { + canonical := http.CanonicalHeaderKey(name) + if canonical == "Origin" || strings.HasPrefix(canonical, "Sec-Fetch-") || + strings.HasPrefix(canonical, "Access-Control-Request-") { + return true + } } - applyPreflight(resp.Header, resp.Request) - resp.StatusCode = http.StatusNoContent - resp.Status = "204 No Content" - resp.Body = http.NoBody - resp.ContentLength = 0 - resp.TransferEncoding = nil - resp.Header.Del("Content-Length") - resp.Header.Del("Content-Type") - resp.Header.Del("Content-Encoding") - resp.Header.Del("Transfer-Encoding") - return true + return false +} + +// StripAllowOrigin prevents an engine from widening PAIR's browser-origin +// boundary while preserving every other part of the engine response. +func StripAllowOrigin(h http.Header) { + h.Del("Access-Control-Allow-Origin") } diff --git a/services/shared/cors/cors_test.go b/services/shared/cors/cors_test.go index c561aba9..1d810b93 100644 --- a/services/shared/cors/cors_test.go +++ b/services/shared/cors/cors_test.go @@ -4,155 +4,76 @@ package cors import ( - "io" "net/http" "net/http/httptest" "strings" "testing" ) -func TestApplySetsPolicy(t *testing.T) { - h := http.Header{} - Apply(h) - for header, want := range map[string]string{ - "Access-Control-Allow-Origin": "*", - "Access-Control-Expose-Headers": "*", - } { - if got := h.Get(header); got != want { - t.Errorf("%s = %q, want %q", header, got, want) - } - } - if h.Get("Access-Control-Allow-Methods") == "" { - t.Error("missing Access-Control-Allow-Methods") - } - // A wildcard origin is only valid for uncredentialed responses, so the - // policy must never claim to allow credentials. - if got := h.Get("Access-Control-Allow-Credentials"); got != "" { - t.Errorf("Access-Control-Allow-Credentials = %q, want unset alongside a wildcard origin", got) - } -} +func TestRejectBrowserRequest_Allow(t *testing.T) { + test := func(name, header, value string) { + t.Run(name, func(t *testing.T) { + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/api/chat", nil) + req.Header[header] = []string{value} -// TestApplyClearsInheritedCredentials: Apply runs on forwarded responses too, -// where the header map is whatever the upstream sent. An upstream that emits -// Allow-Credentials without an origin of its own would otherwise leave the -// invalid wildcard + credentials pair, which a browser fails closed. -func TestApplyClearsInheritedCredentials(t *testing.T) { - h := http.Header{"Access-Control-Allow-Credentials": []string{"true"}} - Apply(h) - - if got := h.Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want *", got) - } - if got := h.Get("Access-Control-Allow-Credentials"); got != "" { - t.Errorf("Access-Control-Allow-Credentials = %q, want cleared alongside the wildcard origin", got) + if RejectBrowserRequest(rec, req) { + t.Fatal("RejectBrowserRequest = true, want request allowed") + } + if rec.Code != http.StatusOK || rec.Body.Len() != 0 { + t.Errorf("response was modified: status=%d body=%q", rec.Code, rec.Body.String()) + } + }) } -} -func TestWritePreflightEchoesRequestedHeaders(t *testing.T) { - rec := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodOptions, "/v1/chat/completions", nil) - req.Header.Set("Access-Control-Request-Headers", "X-Custom-Token") - - if !WritePreflight(rec, req) { - t.Fatal("WritePreflight(OPTIONS) = false, want the preflight handled") - } - if rec.Code != http.StatusNoContent { - t.Errorf("status = %d, want %d", rec.Code, http.StatusNoContent) - } - if got := rec.Header().Get("Access-Control-Allow-Headers"); got != "X-Custom-Token" { - t.Errorf("Access-Control-Allow-Headers = %q, want echoed X-Custom-Token", got) - } + test("authorization", "Authorization", "Bearer token") + test("content type", "Content-Type", "application/json") + test("user agent", "User-Agent", "native-client") } -// TestWritePreflightIgnoresOtherMethods: a real request must fall through to the -// caller's own handling untouched, with no response written. -func TestWritePreflightIgnoresOtherMethods(t *testing.T) { - rec := httptest.NewRecorder() - if WritePreflight(rec, httptest.NewRequest(http.MethodPost, "/api/chat", nil)) { - t.Fatal("WritePreflight(POST) = true, want the request left to the caller") - } - if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { - t.Errorf("Access-Control-Allow-Origin = %q, want no headers written", got) - } -} +func TestRejectBrowserRequest_Reject(t *testing.T) { + test := func(name, header, value string) { + t.Run(name, func(t *testing.T) { + rec := httptest.NewRecorder() + rec.Header().Set("Access-Control-Allow-Origin", "*") + req := httptest.NewRequest(http.MethodPost, "/api/chat", nil) + req.Header[header] = []string{value} -func TestCompletePreflightFallbackPreservesEnginePolicy(t *testing.T) { - req := httptest.NewRequest(http.MethodOptions, "/api/chat", nil) - resp := &http.Response{ - StatusCode: http.StatusNoContent, - Status: "204 No Content", - Header: http.Header{ - "Access-Control-Allow-Origin": []string{"https://app.example"}, - "Access-Control-Allow-Credentials": []string{"true"}, - }, - Body: http.NoBody, - Request: req, + if !RejectBrowserRequest(rec, req) { + t.Fatal("RejectBrowserRequest = false, want request rejected") + } + if rec.Code != http.StatusForbidden { + t.Errorf("status = %d, want %d", rec.Code, http.StatusForbidden) + } + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("Access-Control-Allow-Origin = %q, want absent", got) + } + if body := rec.Body.String(); !strings.Contains(body, `"code":"browser-origin"`) { + t.Errorf("body = %q, want browser-origin code", body) + } + }) } - if CompletePreflightFallback(resp) { - t.Fatal("CompletePreflightFallback = true, want the engine policy preserved") - } - if got := resp.Header.Get("Access-Control-Allow-Origin"); got != "https://app.example" { - t.Errorf("Access-Control-Allow-Origin = %q, want the engine's exact origin", got) - } - if got := resp.Header.Get("Access-Control-Allow-Credentials"); got != "true" { - t.Errorf("Access-Control-Allow-Credentials = %q, want the engine's true", got) - } + test("origin", "Origin", "https://attacker.example") + test("null origin", "Origin", "null") + test("empty origin", "Origin", "") + test("fetch metadata", "Sec-Fetch-Site", "cross-site") + test("preflight method", "Access-Control-Request-Method", http.MethodPost) + test("preflight headers", "Access-Control-Request-Headers", "Authorization") } -func TestCompletePreflightFallbackReplacesMissingEnginePolicy(t *testing.T) { - req := httptest.NewRequest(http.MethodOptions, "/api/chat", nil) - req.Header.Set("Access-Control-Request-Headers", "X-Custom-Token") - resp := &http.Response{ - StatusCode: http.StatusNotFound, - Status: "404 Not Found", - Header: http.Header{"Content-Type": []string{"text/plain"}}, - Body: io.NopCloser(strings.NewReader("not found")), - ContentLength: 9, - Request: req, +func TestStripAllowOrigin(t *testing.T) { + h := http.Header{ + "Access-Control-Allow-Origin": []string{"https://app.example"}, + "X-Engine-Metadata": []string{"preserved"}, } - if !CompletePreflightFallback(resp) { - t.Fatal("CompletePreflightFallback = false, want the local fallback applied") - } - if resp.StatusCode != http.StatusNoContent { - t.Errorf("status = %d, want %d", resp.StatusCode, http.StatusNoContent) - } - if got := resp.Header.Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want *", got) - } - if got := resp.Header.Get("Access-Control-Allow-Headers"); got != "X-Custom-Token" { - t.Errorf("Access-Control-Allow-Headers = %q, want echoed X-Custom-Token", got) - } - if resp.Body != http.NoBody || resp.ContentLength != 0 { - t.Errorf("fallback body = %#v with length %d, want http.NoBody with length 0", resp.Body, resp.ContentLength) - } - if got := resp.Header.Get("Content-Type"); got != "" { - t.Errorf("Content-Type = %q, want removed from the empty 204", got) - } -} + StripAllowOrigin(h) -// TestCompletePreflightFallbackDropsCredentialsWithoutOrigin: an upstream -// preflight carrying Allow-Credentials but no origin has no policy to preserve, -// so the fallback applies — and must not leave the credentials header behind to -// invalidate the wildcard origin it just wrote. -func TestCompletePreflightFallbackDropsCredentialsWithoutOrigin(t *testing.T) { - req := httptest.NewRequest(http.MethodOptions, "/api/chat", nil) - resp := &http.Response{ - StatusCode: http.StatusNoContent, - Status: "204 No Content", - Header: http.Header{"Access-Control-Allow-Credentials": []string{"true"}}, - Body: http.NoBody, - Request: req, - } - - if !CompletePreflightFallback(resp) { - t.Fatal("CompletePreflightFallback = false, want the local fallback applied") - } - if got := resp.Header.Get("Access-Control-Allow-Origin"); got != "*" { - t.Errorf("Access-Control-Allow-Origin = %q, want *", got) + if got := h.Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("Access-Control-Allow-Origin = %q, want absent", got) } - if got := resp.Header.Get("Access-Control-Allow-Credentials"); got != "" { - t.Errorf("Access-Control-Allow-Credentials = %q, want cleared alongside the wildcard origin", got) + if got := h.Get("X-Engine-Metadata"); got != "preserved" { + t.Errorf("X-Engine-Metadata = %q, want preserved", got) } } diff --git a/services/versions.json b/services/versions.json index 29d8c230..eca45782 100644 --- a/services/versions.json +++ b/services/versions.json @@ -3,8 +3,8 @@ "product": "0.91.7", "installer": "0.91.7", "components": { - "ollama-proxy": "0.26.2", - "lmstudio-proxy": "0.16.2", + "ollama-proxy": "0.26.3", + "lmstudio-proxy": "0.16.3", "nvpair-node-info": "0.13.3", "nvpair-node-scanner": "0.20.3", "nvpair-manual-nodes": "0.11.1",