From 8ee39a355d387cf3a35dbd64e868ead0642e284c Mon Sep 17 00:00:00 2001 From: Mariusz Sabath Date: Wed, 28 Oct 2026 20:51:41 -0400 Subject: [PATCH 1/4] fix: Refuse static-inject credentials over plain HTTP (403 plaintext_credential) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A guest can send an absolute-form GET http://host/... through the forward proxy, and static-inject never read pctx.Scheme — a credential resolved for that host would be injected onto a request that crosses the network in cleartext (#1481, MOCA MI1 §8.5 item 3). The gate runs in the injector, after the resolver succeeds and before any header write, so the refusal names the credential rather than the scheme: plain HTTP to a host without a credential keeps its normal fail-closed treatment, and an empty scheme (listener couldn't tell) is not guessed to be plaintext. The check deliberately lives here rather than in a pre-pipeline gate or as a blanket rule for credential-writing plugins — only the resolver knows whether the host has a credential, and token-exchange injects onto plaintext in-cluster calls by design. The denial is 403 with code plaintext_credential, the MOCA egress contract's identifier verbatim rather than the usual static-inject. form, so the conformance suite Cortex shares with moca-egress matches it without a translation table. It also records a deny invocation so the refusal is attributable in the session timeline. Plugin tests pin the contract case, its negative space (no credential means 401 unresolved-key, not 403), https and unknown-scheme pass-through, and the recorded invocation; a forward-proxy end-to-end test pins that the upstream is never dialed and the client sees the 403 with the code. Closes #1481 Assisted-By: Claude (Anthropic AI) Signed-off-by: Mariusz Sabath --- core/listener/forwardproxy/server_test.go | 64 +++++++++ core/plugins/staticinject/plugin.go | 41 +++++- core/plugins/staticinject/plugin_test.go | 152 ++++++++++++++++++++++ docs/static-inject-plugin.md | 20 +++ 4 files changed, 275 insertions(+), 2 deletions(-) diff --git a/core/listener/forwardproxy/server_test.go b/core/listener/forwardproxy/server_test.go index d6be7a995..01ec1370a 100644 --- a/core/listener/forwardproxy/server_test.go +++ b/core/listener/forwardproxy/server_test.go @@ -17,6 +17,7 @@ import ( "github.com/rossoctl/cortex/core/pipeline" "github.com/rossoctl/cortex/core/plugins/jwtvalidation/validation" "github.com/rossoctl/cortex/core/plugins/plugintesting" + "github.com/rossoctl/cortex/core/plugins/staticinject" "github.com/rossoctl/cortex/core/plugins/tokenexchange/cache" "github.com/rossoctl/cortex/core/plugins/tokenexchange/exchange" "github.com/rossoctl/cortex/core/routing" @@ -434,6 +435,69 @@ func TestForwardProxy_BodyBuffering(t *testing.T) { } } +// TestForwardProxy_PlaintextCredentialRefused pins #1481's conformance case +// end to end: an absolute-form http:// request through the proxy to a host +// that has a credential is refused 403 plaintext_credential, and the upstream +// is never dialed — the secret cannot cross the wire in the clear because +// nothing crosses it at all. +func TestForwardProxy_PlaintextCredentialRefused(t *testing.T) { + var upstreamHits int + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + upstreamHits++ + w.WriteHeader(http.StatusOK) + })) + defer backend.Close() + + // static-inject keyed by host:port — pctx.Host for a proxied request is + // the URL's host:port, so the resolver key must match it exactly. + inject := staticinject.New() + cfg := `{ + "source": "mappings", + "mappings": {"` + mustParseURL(backend.URL).Host + `": "REAL"}, + "key_by": "host", + "placeholder": "PLACEHOLDER" + }` + if err := inject.Configure(json.RawMessage(cfg)); err != nil { + t.Fatalf("Configure() error = %v", err) + } + p, err := pipeline.New([]pipeline.Plugin{inject}) + if err != nil { + t.Fatal(err) + } + + srv := &Server{OutboundPipeline: pipeline.NewHolder(p), Client: http.DefaultClient} + proxy := httptest.NewServer(srv.Handler()) + defer proxy.Close() + + req, _ := http.NewRequest("GET", backend.URL+"/x", nil) + req.Header.Set("Authorization", "Bearer PLACEHOLDER") + + proxyClient := &http.Client{ + Transport: &http.Transport{ + Proxy: http.ProxyURL(mustParseURL(proxy.URL)), + }, + } + resp, err := proxyClient.Do(req) + if err != nil { + t.Fatalf("request failed: %v", err) + } + defer resp.Body.Close() + body, _ := io.ReadAll(resp.Body) + + if resp.StatusCode != http.StatusForbidden { + t.Errorf("status = %d, want 403 (body %q)", resp.StatusCode, body) + } + if !strings.Contains(string(body), "plaintext_credential") { + t.Errorf("body = %q, want the plaintext_credential error code", body) + } + if strings.Contains(string(body), "REAL") { + t.Errorf("body = %q, must not carry the credential", body) + } + if upstreamHits != 0 { + t.Errorf("upstream hits = %d, want 0 (nothing sent)", upstreamHits) + } +} + // postThroughBodyReader sends a size-byte POST through a forward proxy whose // pipeline buffers request bodies, and reports the status the client got and // how many bytes the plugin and the upstream each saw — -1 for a side the diff --git a/core/plugins/staticinject/plugin.go b/core/plugins/staticinject/plugin.go index 866472ca8..f1128ecf6 100644 --- a/core/plugins/staticinject/plugin.go +++ b/core/plugins/staticinject/plugin.go @@ -178,8 +178,9 @@ func (p *StaticInject) Configure(raw json.RawMessage) error { // OnRequest implements the fail-closed injection sequence: missing/mismatched // placeholder, unresolved key, or an unsafe resolved value all deny with 401 -// and leave the Authorization header unchanged. Only a fully successful -// resolution swaps the header. +// and leave the Authorization header unchanged. A resolved credential on a +// plaintext (http) request denies with 403 plaintext_credential — see the +// gate below. Only a fully successful resolution swaps the header. func (p *StaticInject) OnRequest(ctx context.Context, pctx *pipeline.Context) pipeline.Action { if p.resolver == nil { return pipeline.DenyStatus(401, "static-inject.unconfigured", "static-inject plugin not configured") @@ -211,6 +212,42 @@ func (p *StaticInject) OnRequest(ctx context.Context, pctx *pipeline.Context) pi return pipeline.DenyStatus(401, "static-inject.unresolved-key", "no credential available for the resolved key") } + // A credential that is about to be injected must never cross the network + // in cleartext (#1481, MOCA MI1 §8.5: 403 plaintext_credential). pctx.Scheme + // is "http" exactly for a request the listener knows is plaintext — an + // absolute-form http:// URL through the forward proxy, or Envoy's :scheme + // on the ext_proc path; a TLS-bridged request carries "https". Checked + // here, after resolution and before any header write, so the refusal + // names the credential rather than the scheme: plain HTTP to a host + // WITHOUT one keeps its normal allow-list/SSRF treatment, and an empty + // scheme (listener couldn't tell) is left to those same checks rather + // than denied on a guess. + // + // The check lives in the injector, not in a pre-pipeline gate, because + // only the resolver knows whether this host has a credential — a + // listener-level gate would have to duplicate that knowledge or refuse + // plain HTTP to credential-less hosts too, which the contract routes to + // the allow list instead. It is also deliberately not a blanket rule for + // every credential-writing plugin: token-exchange injects short-lived + // tokens onto plaintext in-cluster calls by design. + // + // The code is the MOCA contract's identifier verbatim rather than the + // repo's usual `static-inject.` form, so the egress conformance + // suite Cortex and moca-egress share matches it without a translation + // table — the same reasoning that freezes `x-authbridge-*` wire names. + if strings.EqualFold(pctx.Scheme, "http") { + pctx.Record(pipeline.Invocation{ + Action: pipeline.ActionDeny, + Reason: "plaintext_credential", + Details: map[string]string{ + "scheme": pctx.Scheme, + "host": key, + }, + }) + return pipeline.DenyStatus(403, "plaintext_credential", + "refusing to send a credential over plain HTTP") + } + target := p.cfg.InjectHeader var headerVal string if strings.EqualFold(target, "Authorization") { diff --git a/core/plugins/staticinject/plugin_test.go b/core/plugins/staticinject/plugin_test.go index 39a2426d0..6fb22f302 100644 --- a/core/plugins/staticinject/plugin_test.go +++ b/core/plugins/staticinject/plugin_test.go @@ -302,6 +302,158 @@ func TestDenyOnEmptyResolvedValue(t *testing.T) { }) } +// ============================================================================= +// Plaintext-credential gate (#1481, MOCA MI1 §8.5) +// ============================================================================= + +// newPlaintextGatePlugin builds a configured plugin whose resolver holds a +// credential for api.example.com and whose placeholder matches what the +// conformance-shaped requests present. +func newPlaintextGatePlugin(t *testing.T) *StaticInject { + t.Helper() + p := New() + cfg := `{ + "source": "mappings", + "mappings": {"api.example.com": "REAL"}, + "key_by": "host", + "placeholder": "PLACEHOLDER" + }` + if err := p.Configure(json.RawMessage(cfg)); err != nil { + t.Fatalf("Configure() error = %v", err) + } + return p +} + +// TestDenyPlaintextCredential pins the contract case: a plain-HTTP request to +// a host that has a credential is refused 403 plaintext_credential, nothing +// is injected, and the refusal is recorded as an invocation so the denial is +// visible in the session timeline. +func TestDenyPlaintextCredential(t *testing.T) { + p := newPlaintextGatePlugin(t) + + pctx := &pipeline.Context{ + Direction: pipeline.Outbound, + Scheme: "http", + Host: "api.example.com", + Headers: http.Header{ + "Authorization": []string{"Bearer PLACEHOLDER"}, + }, + } + + action := p.OnRequest(context.Background(), pctx) + + if action.Type != pipeline.Reject { + t.Fatalf("OnRequest() action.Type = %v, want %v (Reject)", action.Type, pipeline.Reject) + } + if action.Violation == nil { + t.Fatalf("OnRequest() Violation = nil, want 403 plaintext_credential") + } + if status, _, _ := action.Violation.Render(); status != 403 { + t.Errorf("Violation status = %d, want 403", status) + } + if action.Violation.Code != "plaintext_credential" { + t.Errorf("Violation code = %q, want %q (MOCA contract identifier, verbatim)", + action.Violation.Code, "plaintext_credential") + } + if got := pctx.Headers.Get("Authorization"); got != "Bearer PLACEHOLDER" { + t.Errorf("Authorization header = %q, want unmodified %q (nothing injected)", got, "Bearer PLACEHOLDER") + } + // The refusal must be attributable in the invocation timeline, the same + // way the conformance suite reads moca-egress's decision event. Record + // buckets by Direction, so the outbound bucket is the one to read. + inv := pctx.Extensions.Invocations + if inv == nil { + t.Fatalf("Extensions.Invocations = nil, want one outbound deny record") + } + if len(inv.Outbound) != 1 { + t.Fatalf("outbound invocations length = %d, want 1 (the deny record)", len(inv.Outbound)) + } + if inv.Outbound[0].Action != pipeline.ActionDeny || inv.Outbound[0].Reason != "plaintext_credential" { + t.Errorf("invocation = {%s %q}, want {deny plaintext_credential}", + inv.Outbound[0].Action, inv.Outbound[0].Reason) + } +} + +// TestPlaintextGateNamesTheCredentialNotTheScheme pins the negative space of +// the contract: plain HTTP to a host WITHOUT a credential is not refused by +// this gate — it keeps its normal (401 unresolved-key) treatment, leaving +// allow-list/SSRF policy to judge the request. +func TestPlaintextGateNamesTheCredentialNotTheScheme(t *testing.T) { + p := newPlaintextGatePlugin(t) + + pctx := &pipeline.Context{ + Scheme: "http", + Host: "no-credential.example.com", + Headers: http.Header{ + "Authorization": []string{"Bearer PLACEHOLDER"}, + }, + } + + action := p.OnRequest(context.Background(), pctx) + + if action.Type != pipeline.Reject { + t.Fatalf("OnRequest() action.Type = %v, want %v (Reject)", action.Type, pipeline.Reject) + } + if action.Violation == nil || action.Violation.Code != "static-inject.unresolved-key" { + t.Errorf("Violation = %+v, want code %q (unresolved key, not plaintext_credential)", + action.Violation, "static-inject.unresolved-key") + } +} + +// TestPlaintextGateAllowsHTTPS pins that a TLS-bridged request to a +// credentialed host injects exactly as before the gate existed. +func TestPlaintextGateAllowsHTTPS(t *testing.T) { + p := newPlaintextGatePlugin(t) + + pctx := &pipeline.Context{ + Scheme: "https", + Host: "api.example.com", + Headers: http.Header{ + "Authorization": []string{"Bearer PLACEHOLDER"}, + }, + } + + action := p.OnRequest(context.Background(), pctx) + + if action.Type != pipeline.Continue { + t.Fatalf("OnRequest() action.Type = %v, want %v (violation: %+v)", + action.Type, pipeline.Continue, action.Violation) + } + if got := pctx.Headers.Get("Authorization"); got != "Bearer REAL" { + t.Errorf("Authorization header = %q, want %q", got, "Bearer REAL") + } +} + +// TestPlaintextGateIgnoresUnknownScheme pins that an empty/unknown scheme is +// NOT treated as plaintext: Context.Scheme's contract says assuming "" means +// http would silently mask missing listener plumbing, so the gate denies only +// on a scheme the listener explicitly declared plaintext. +func TestPlaintextGateIgnoresUnknownScheme(t *testing.T) { + for _, scheme := range []string{"", "tcp"} { + t.Run("scheme="+scheme, func(t *testing.T) { + p := newPlaintextGatePlugin(t) + + pctx := &pipeline.Context{ + Scheme: scheme, + Host: "api.example.com", + Headers: http.Header{ + "Authorization": []string{"Bearer PLACEHOLDER"}, + }, + } + + action := p.OnRequest(context.Background(), pctx) + + if action.Type != pipeline.Continue { + t.Fatalf("OnRequest() action.Type = %v, want %v (violation: %+v)", + action.Type, pipeline.Continue, action.Violation) + } + if got := pctx.Headers.Get("Authorization"); got != "Bearer REAL" { + t.Errorf("Authorization header = %q, want %q", got, "Bearer REAL") + } + }) + } +} + // ============================================================================= // Name / Capabilities / ConfigSchema / Configure sanity checks // ============================================================================= diff --git a/docs/static-inject-plugin.md b/docs/static-inject-plugin.md index 1e9a9b59b..9c6a7c011 100644 --- a/docs/static-inject-plugin.md +++ b/docs/static-inject-plugin.md @@ -50,3 +50,23 @@ completed cleanly: There is no partial-injection path: a denied request never forwards the placeholder, and a successful swap only ever writes the fully resolved real credential. + +### Plaintext refusal (`403 plaintext_credential`) + +A credential that is about to be injected is never sent over plain +HTTP: when the request's scheme is `http` and the resolved host has a +credential, the plugin denies with `403 plaintext_credential` before +any header is written, so nothing is injected and nothing is sent +upstream. The code is the MOCA egress contract's identifier verbatim +(MI1 §8.5) rather than the usual `static-inject.` form, so the +conformance suite Cortex shares with `moca-egress` matches it without +a translation table. + +The refusal names the credential, not the scheme: plain HTTP to a host +*without* a credential is not refused here — it keeps its normal +fail-closed treatment above, and allow-list/SSRF policy judges the +request as it would any other. An empty scheme (the listener couldn't +determine one) is likewise not treated as plaintext. The check runs +after resolution on purpose: only the resolver knows whether this host +has a credential, which is why the gate lives in the injector rather +than in a pre-pipeline check. From 725f5ab8c9596ba9911dd29c0665ccc5fd5e08b3 Mon Sep 17 00:00:00 2001 From: Mariusz Sabath Date: Wed, 28 Oct 2026 21:33:26 -0400 Subject: [PATCH 2/4] test: pin the extproc :scheme wiring for the plaintext gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the #1481 gate: - The extproc leg was verified only by reading the source: the plugin tests cover the gate unit-level and the forwardproxy test pins the proxy-sidecar leg, but nothing would fail if extproc's :scheme read regressed, silently stopping the envoy-sidecar shape of the gate. Two tests now drive Process with Envoy's own message wire — :scheme http to a credentialed host refuses 403 plaintext_credential, and :scheme https still injects via the SetHeaders mutation. - The deny invocation's Details labelled the resolver key "host", which is pctx.Host (host:port) under key_by: host but the configured static key — not a host — under key_by: static. Renamed to "key". - docs: the 403 fires only after the bearer and placeholder checks pass (a plaintext request failing one of those keeps its 401; either way nothing is sent), and a new paragraph names the redirect boundary — the gate judges the arrival scheme, a WritesDestination plugin can Redirect to an http target after the pipeline has run, and the inference-router's credential drop/overwrite is what covers that path. Signed-off-by: Mariusz Sabath Assisted-By: Claude (Anthropic AI) --- core/listener/extproc/plaintextgate_test.go | 124 ++++++++++++++++++++ core/plugins/staticinject/plugin.go | 5 +- docs/static-inject-plugin.md | 22 +++- 3 files changed, 146 insertions(+), 5 deletions(-) create mode 100644 core/listener/extproc/plaintextgate_test.go diff --git a/core/listener/extproc/plaintextgate_test.go b/core/listener/extproc/plaintextgate_test.go new file mode 100644 index 000000000..148745254 --- /dev/null +++ b/core/listener/extproc/plaintextgate_test.go @@ -0,0 +1,124 @@ +package extproc + +import ( + "context" + "encoding/json" + "strings" + "testing" + + extprocv3 "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3" + + "github.com/rossoctl/cortex/core/pipeline" + "github.com/rossoctl/cortex/core/plugins/staticinject" +) + +// plaintextGateServer builds an ext_proc server whose outbound pipeline holds +// a static-inject configured with a credential for api.example.com — the #1481 +// shape, driven here through Envoy's own message wire so what is pinned is the +// LISTENER's plumbing: the :scheme pseudo-header reaching pctx.Scheme, which +// the plugin's plaintext gate reads. +// +// The plugin-level behaviour is pinned in staticinject's own tests and the +// forward proxy's leg end to end; without this file the extproc leg was +// verified only by reading the source, so a dropped :scheme read would stop +// the envoy-sidecar shape of the gate firing while every one of those stayed +// green — exactly the silent regression a listener-wiring test exists to catch. +func plaintextGateServer(t *testing.T) *Server { + t.Helper() + inject := staticinject.New() + cfg := `{ + "source": "mappings", + "mappings": {"api.example.com": "REAL"}, + "key_by": "host", + "placeholder": "PLACEHOLDER" + }` + if err := inject.Configure(json.RawMessage(cfg)); err != nil { + t.Fatalf("static-inject Configure: %v", err) + } + p, err := pipeline.New([]pipeline.Plugin{inject}) + if err != nil { + t.Fatalf("pipeline.New: %v", err) + } + return &Server{OutboundPipeline: pipeline.NewHolder(p)} +} + +// TestExtProc_Outbound_PlaintextCredentialRefusedOnSchemeHeader: a request +// whose :scheme pseudo-header says http, to a host the pipeline holds a +// credential for, is refused 403 plaintext_credential — the same conformance +// case TestForwardProxy_PlaintextCredentialRefused pins on the proxy-sidecar +// leg, asserted here on Envoy's message wire so the two deployment shapes are +// both pinned against the same regression. +func TestExtProc_Outbound_PlaintextCredentialRefusedOnSchemeHeader(t *testing.T) { + srv := plaintextGateServer(t) + + stream := &mockStream{ctx: context.Background(), requests: []*extprocv3.ProcessingRequest{ + { + Request: &extprocv3.ProcessingRequest_RequestHeaders{ + RequestHeaders: &extprocv3.HttpHeaders{Headers: makeHeaders( + "x-authbridge-direction", "outbound", + ":method", "GET", + ":scheme", "http", + ":path", "/x", + ":authority", "api.example.com", + "authorization", "Bearer PLACEHOLDER", + )}, + }, + }, + }} + _ = srv.Process(stream) + + if len(stream.responses) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream.responses)) + } + ir := stream.responses[0].GetImmediateResponse() + if ir == nil { + t.Fatalf("expected ImmediateResponse (deny), got %+v", stream.responses[0]) + } + if ir.Status.Code != 403 { + t.Errorf("status = %d, want 403", ir.Status.Code) + } + if !strings.Contains(string(ir.Body), "plaintext_credential") { + t.Errorf("body = %q, want the plaintext_credential error code", ir.Body) + } +} + +// TestExtProc_Outbound_PlaintextGatePassesSchemeHTTPS: the same request with +// :scheme https injects exactly as before the gate existed — the SetHeaders +// mutation carries the credential — so this leg also pins that the gate does +// not over-fire on the envoy path. +func TestExtProc_Outbound_PlaintextGatePassesSchemeHTTPS(t *testing.T) { + srv := plaintextGateServer(t) + + stream := &mockStream{ctx: context.Background(), requests: []*extprocv3.ProcessingRequest{ + { + Request: &extprocv3.ProcessingRequest_RequestHeaders{ + RequestHeaders: &extprocv3.HttpHeaders{Headers: makeHeaders( + "x-authbridge-direction", "outbound", + ":method", "GET", + ":scheme", "https", + ":path", "/x", + ":authority", "api.example.com", + "authorization", "Bearer PLACEHOLDER", + )}, + }, + }, + }} + _ = srv.Process(stream) + + if len(stream.responses) != 1 { + t.Fatalf("expected 1 response, got %d", len(stream.responses)) + } + resp := stream.responses[0] + if ir := resp.GetImmediateResponse(); ir != nil { + t.Fatalf("https request was refused: status %d body %q", ir.Status.Code, ir.Body) + } + // setHeaderValue is placeholder_test.go's accessor for the allow path's + // header mutation; the injected credential must be on it. + got, ok := setHeaderValue(resp, "authorization") + if !ok { + t.Fatalf("expected SetHeaders mutation for authorization, got %+v", resp) + } + if got != "Bearer REAL" { + t.Errorf("authorization = %q, want %q (injected on https as before the gate)", got, "Bearer REAL") + } +} diff --git a/core/plugins/staticinject/plugin.go b/core/plugins/staticinject/plugin.go index f1128ecf6..e6c0ea8a5 100644 --- a/core/plugins/staticinject/plugin.go +++ b/core/plugins/staticinject/plugin.go @@ -241,7 +241,10 @@ func (p *StaticInject) OnRequest(ctx context.Context, pctx *pipeline.Context) pi Reason: "plaintext_credential", Details: map[string]string{ "scheme": pctx.Scheme, - "host": key, + // The resolver key, not the host: under key_by: host it is + // pctx.Host (host:port), under key_by: static the configured + // key — which is not a host at all. + "key": key, }, }) return pipeline.DenyStatus(403, "plaintext_credential", diff --git a/docs/static-inject-plugin.md b/docs/static-inject-plugin.md index 9c6a7c011..cf2c7f284 100644 --- a/docs/static-inject-plugin.md +++ b/docs/static-inject-plugin.md @@ -54,10 +54,12 @@ resolved real credential. ### Plaintext refusal (`403 plaintext_credential`) A credential that is about to be injected is never sent over plain -HTTP: when the request's scheme is `http` and the resolved host has a -credential, the plugin denies with `403 plaintext_credential` before -any header is written, so nothing is injected and nothing is sent -upstream. The code is the MOCA egress contract's identifier verbatim +HTTP: when the request's scheme is `http` and resolution reached a +credential — the bearer and placeholder checks above passed — the +plugin denies with `403 plaintext_credential` before any header is +written, so nothing is injected and nothing is sent upstream. A +plaintext request that fails an earlier check keeps that check's 401; +either way nothing is sent. The code is the MOCA egress contract's identifier verbatim (MI1 §8.5) rather than the usual `static-inject.` form, so the conformance suite Cortex shares with `moca-egress` matches it without a translation table. @@ -70,3 +72,15 @@ determine one) is likewise not treated as plaintext. The check runs after resolution on purpose: only the resolver knows whether this host has a credential, which is why the gate lives in the injector rather than in a pre-pipeline check. + +One boundary worth naming: the gate judges the scheme the request +*arrived* with. A `WritesDestination` plugin later in the chain can +`Redirect` to an `http://` target (an operator-pinned server), and the +listener applies the redirect after the pipeline has run, headers +already injected. The shipped inference-router clears that path +itself, dropping credential-sounding headers and overwriting +`Authorization` / `X-Api-Key` with the routed server's own key — so +what can survive is a custom `inject_header` whose name avoids every +credential word. If static-inject shares a pipeline with a redirecting +plugin and a plaintext pin, give the injected header a name containing +one of those words so the router's drop covers it. From a11bdf195552da97af184658cdd4520cf92494cb Mon Sep 17 00:00:00 2001 From: Mariusz Sabath Date: Thu, 29 Oct 2026 10:52:27 -0400 Subject: [PATCH 3/4] fix: gate the plaintext refusal on outbound only (#1481) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plaintext gate judged the wrong hop on inbound requests, and that broke a real deployment rather than a hypothetical one. On the reverse proxy `pctx.Scheme` is `requestScheme(r)` (`reverseproxy/server.go:797`) — the scheme the CLIENT used to reach Cortex, `http` for any non-TLS inbound listener. The credential does not travel on that hop. It leaves on the Cortex→backend hop, whose scheme comes from `reverse_proxy_backend` and is invisible from the plugin. So refusing an inbound request because its front was plaintext protects nothing and blocks everything. MOCA's AB1 is exactly that shape: static-inject in the INBOUND pipeline, `reverse_proxy_addr: ":8080"` plaintext, `reverse_proxy_backend: "https://api.anthropic.com"` (moca `deploy/knative/authbridge/ab1-deployment.yaml`, which I fetched and confirmed). I reproduced the breakage end to end before fixing: status 403, zero upstream hits, every LLM call through AB1 refused once it picked up this image. The gate now requires `pctx.Direction == pipeline.Outbound`, which is the case #1481 is about. Covering the inbound hop properly needs the reverse proxy to expose its backend scheme — a larger change this issue does not ask for, and the code and docs now say so rather than leaving it implied. Tests. Three existing gate tests relied on `Direction`'s zero value, which is Inbound — so after this change they would have passed for the wrong reason, on the direction rather than on the scheme. All three now set `Direction: pipeline.Outbound` explicitly, with a comment on the one where it is load-bearing. Two new tests: - `TestPlaintextGateDoesNotFireOnInbound` — the unit case. - `TestStaticInject_InboundPlaintextFrontHTTPSBackend_StillInjects` (`core/listener/reverseproxy`) — the end-to-end case, and the one that matters: the staticinject unit tests build a `pipeline.Context` by hand, so a wrong-hop `Scheme` can only appear through a real listener. This is my reproduction promoted to a regression test. Both fail against the code before the fix. Also from the review: - **Docs now name who this breaks.** MOCA's AB2 leaf smoke (`deploy/knative/leaf-smoke.sh`, the `H2-inject` claim) sends `curl -s http://echo-target/ -H "Authorization: Bearer $ECHO_CRED"` through the forward proxy to a credentialed host — refused by this gate. AB2 is pinned to an image predating it, so nothing breaks today, but the next bump fails that check unless echo-target moves to TLS or the smoke expects the 403. Added as a table of which shapes this refuses, plus the AB2 note. - **The envoy comment no longer overclaims.** `staticinject` is in `{full, fullcpex, lite}` and not `ProfileEnvoy` (`scripts/profile-tags/profiles.go`), so no shipped envoy-sidecar image runs this gate; the comment says "would be, if the envoy profile shipped it" and names the test that pins the wiring. Verified: full core suite, 72 packages, 0 failures; go vet and gofmt clean. Assisted-By: Claude (Anthropic AI) Signed-off-by: Mariusz Sabath --- .../staticinject_plaintext_test.go | 94 +++++++++++++++++++ core/plugins/staticinject/plugin.go | 28 +++++- core/plugins/staticinject/plugin_test.go | 54 +++++++++-- docs/static-inject-plugin.md | 36 +++++++ 4 files changed, 204 insertions(+), 8 deletions(-) create mode 100644 core/listener/reverseproxy/staticinject_plaintext_test.go diff --git a/core/listener/reverseproxy/staticinject_plaintext_test.go b/core/listener/reverseproxy/staticinject_plaintext_test.go new file mode 100644 index 000000000..f9cafbe71 --- /dev/null +++ b/core/listener/reverseproxy/staticinject_plaintext_test.go @@ -0,0 +1,94 @@ +package reverseproxy + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "github.com/rossoctl/cortex/core/pipeline" + "github.com/rossoctl/cortex/core/plugins/staticinject" +) + +// MOCA AB1's shape, end to end: static-inject in the INBOUND pipeline, a +// PLAINTEXT front, and an HTTPS backend. The credential never crosses the +// plaintext hop — it is injected on the way out, to a TLS backend — so the +// request must be forwarded with the real key. +// +// This is a listener-level test on purpose. #1481's plaintext gate reads +// pctx.Scheme, which on the reverse proxy is requestScheme(r): the scheme the +// CLIENT used to reach Cortex, "http" for any non-TLS inbound listener. The +// backend's scheme comes from reverse_proxy_backend and is invisible from the +// plugin. So the gate, before it also checked Direction, refused every request +// through AB1 — 403 with zero upstream hits. +// +// The unit tests in core/plugins/staticinject could not have caught it: they +// build a pipeline.Context by hand, so the wrong-hop Scheme has to come from a +// real listener to show up. AB1 runs +// reverse_proxy_addr ":8080" in front of reverse_proxy_backend +// "https://api.anthropic.com" (moca deploy/knative/authbridge/ab1-deployment.yaml). +func TestStaticInject_InboundPlaintextFrontHTTPSBackend_StillInjects(t *testing.T) { + var hits int64 + var sawKey string + backend := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt64(&hits, 1) + sawKey = r.Header.Get("x-api-key") + w.WriteHeader(http.StatusOK) + })) + defer backend.Close() + + // AB1's static-inject config: key_by static, inject_header x-api-key. + p := staticinject.New() + cfg, err := json.Marshal(map[string]any{ + "source": "mappings", + "mappings": map[string]string{"anthropic": "sk-real-secret"}, + "key_by": "static", + "key": "anthropic", + "inject_header": "x-api-key", + }) + if err != nil { + t.Fatal(err) + } + if err := p.Configure(json.RawMessage(cfg)); err != nil { + t.Fatalf("Configure: %v", err) + } + pl, err := pipeline.New([]pipeline.Plugin{p}) + if err != nil { + t.Fatalf("pipeline.New: %v", err) + } + srv, err := NewServer(pipeline.NewHolder(pl), nil, backend.URL, nil) + if err != nil { + t.Fatalf("NewServer: %v", err) + } + srv.proxy.Transport = backend.Client().Transport // trust the test CA + + // Plaintext front, as reverse_proxy_addr: ":8080". + front := httptest.NewServer(srv.Handler()) + defer front.Close() + + req, err := http.NewRequest(http.MethodPost, front.URL+"/v1/messages", nil) + if err != nil { + t.Fatal(err) + } + // The placeholder bearer the workload presents; static-inject swaps it. + req.Header.Set("Authorization", "Bearer placeholder-token") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("request: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode == http.StatusForbidden { + t.Fatalf("status = 403 — the inbound plaintext front was read as the credential's hop; AB1's every LLM call would be refused") + } + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, want 200", resp.StatusCode) + } + if n := atomic.LoadInt64(&hits); n != 1 { + t.Errorf("backend hits = %d, want 1", n) + } + if sawKey != "sk-real-secret" { + t.Errorf("backend saw x-api-key = %q, want the injected credential", sawKey) + } +} diff --git a/core/plugins/staticinject/plugin.go b/core/plugins/staticinject/plugin.go index e6c0ea8a5..77dbf19f0 100644 --- a/core/plugins/staticinject/plugin.go +++ b/core/plugins/staticinject/plugin.go @@ -216,13 +216,37 @@ func (p *StaticInject) OnRequest(ctx context.Context, pctx *pipeline.Context) pi // in cleartext (#1481, MOCA MI1 §8.5: 403 plaintext_credential). pctx.Scheme // is "http" exactly for a request the listener knows is plaintext — an // absolute-form http:// URL through the forward proxy, or Envoy's :scheme - // on the ext_proc path; a TLS-bridged request carries "https". Checked + // on the ext_proc path (would be, if the envoy profile shipped this plugin + // — it does not, so no released envoy-sidecar image runs this gate; + // extproc/plaintextgate_test.go pins the wiring against that changing); + // a TLS-bridged request carries "https". Checked // here, after resolution and before any header write, so the refusal // names the credential rather than the scheme: plain HTTP to a host // WITHOUT one keeps its normal allow-list/SSRF treatment, and an empty // scheme (listener couldn't tell) is left to those same checks rather // than denied on a guess. // + // OUTBOUND ONLY, and the direction is load-bearing rather than a + // narrowing for caution's sake. On the reverse proxy pctx.Scheme is + // requestScheme(r) (reverseproxy/server.go:797) — the scheme the CLIENT + // used to reach Cortex, which is "http" whenever the inbound listener is + // not TLS. The credential does not travel on that hop; it leaves on the + // Cortex→backend hop, whose scheme comes from reverse_proxy_backend and + // is invisible from here. So judging an inbound request by pctx.Scheme + // asks about the wrong hop entirely. + // + // That is a real deployment, not a hypothetical: MOCA's AB1 runs + // static-inject in the INBOUND pipeline behind a plaintext + // reverse_proxy_addr: ":8080" with reverse_proxy_backend: + // "https://api.anthropic.com" (moca deploy/knative/authbridge/ + // ab1-deployment.yaml). Gating on scheme alone refused every LLM call + // through it — reproduced at 403 with zero upstream hits (#1537 review). + // + // Covering inbound properly would mean teaching the reverse proxy to + // expose its backend scheme, which #1481 does not ask for; until then an + // inbound plaintext hop is the operator's own listener, under their own + // network, rather than the guest-controlled egress this gate is about. + // // The check lives in the injector, not in a pre-pipeline gate, because // only the resolver knows whether this host has a credential — a // listener-level gate would have to duplicate that knowledge or refuse @@ -235,7 +259,7 @@ func (p *StaticInject) OnRequest(ctx context.Context, pctx *pipeline.Context) pi // repo's usual `static-inject.` form, so the egress conformance // suite Cortex and moca-egress share matches it without a translation // table — the same reasoning that freezes `x-authbridge-*` wire names. - if strings.EqualFold(pctx.Scheme, "http") { + if pctx.Direction == pipeline.Outbound && strings.EqualFold(pctx.Scheme, "http") { pctx.Record(pipeline.Invocation{ Action: pipeline.ActionDeny, Reason: "plaintext_credential", diff --git a/core/plugins/staticinject/plugin_test.go b/core/plugins/staticinject/plugin_test.go index 6fb22f302..1e80127e5 100644 --- a/core/plugins/staticinject/plugin_test.go +++ b/core/plugins/staticinject/plugin_test.go @@ -382,8 +382,12 @@ func TestPlaintextGateNamesTheCredentialNotTheScheme(t *testing.T) { p := newPlaintextGatePlugin(t) pctx := &pipeline.Context{ - Scheme: "http", - Host: "no-credential.example.com", + // Outbound explicitly: Direction's zero value is Inbound, where the gate + // deliberately does not fire, so without this the test would pass for the + // wrong reason — on the direction rather than on the missing credential. + Direction: pipeline.Outbound, + Scheme: "http", + Host: "no-credential.example.com", Headers: http.Header{ "Authorization": []string{"Bearer PLACEHOLDER"}, }, @@ -406,8 +410,9 @@ func TestPlaintextGateAllowsHTTPS(t *testing.T) { p := newPlaintextGatePlugin(t) pctx := &pipeline.Context{ - Scheme: "https", - Host: "api.example.com", + Direction: pipeline.Outbound, + Scheme: "https", + Host: "api.example.com", Headers: http.Header{ "Authorization": []string{"Bearer PLACEHOLDER"}, }, @@ -424,6 +429,42 @@ func TestPlaintextGateAllowsHTTPS(t *testing.T) { } } +// INBOUND + http STILL INJECTS, and this is the test that keeps MOCA's AB1 +// working. On the reverse proxy pctx.Scheme is the scheme the CLIENT used to +// reach Cortex (requestScheme, reverseproxy/server.go:797) — "http" whenever +// the inbound listener is not TLS. The credential leaves on the Cortex→backend +// hop instead, whose scheme comes from reverse_proxy_backend and cannot be read +// from here, so an inbound refusal keyed on Scheme judges the wrong hop. +// +// AB1 is exactly that shape: static-inject in the inbound pipeline behind a +// plaintext reverse_proxy_addr ":8080", forwarding to +// reverse_proxy_backend "https://api.anthropic.com". Gating on scheme alone +// refused every LLM call through it — reproduced end to end at 403 with zero +// upstream hits before the direction check (#1537 review). +func TestPlaintextGateDoesNotFireOnInbound(t *testing.T) { + p := newPlaintextGatePlugin(t) + + pctx := &pipeline.Context{ + Direction: pipeline.Inbound, + // The client's hop, not the credential's: the backend may well be https. + Scheme: "http", + Host: "api.example.com", + Headers: http.Header{ + "Authorization": []string{"Bearer PLACEHOLDER"}, + }, + } + + action := p.OnRequest(context.Background(), pctx) + + if action.Type != pipeline.Continue { + t.Fatalf("OnRequest() action.Type = %v, want %v — an inbound plaintext front says nothing about the backend hop the credential takes (violation: %+v)", + action.Type, pipeline.Continue, action.Violation) + } + if got := pctx.Headers.Get("Authorization"); got != "Bearer REAL" { + t.Errorf("Authorization header = %q, want %q — the credential must still be injected", got, "Bearer REAL") + } +} + // TestPlaintextGateIgnoresUnknownScheme pins that an empty/unknown scheme is // NOT treated as plaintext: Context.Scheme's contract says assuming "" means // http would silently mask missing listener plumbing, so the gate denies only @@ -434,8 +475,9 @@ func TestPlaintextGateIgnoresUnknownScheme(t *testing.T) { p := newPlaintextGatePlugin(t) pctx := &pipeline.Context{ - Scheme: scheme, - Host: "api.example.com", + Direction: pipeline.Outbound, + Scheme: scheme, + Host: "api.example.com", Headers: http.Header{ "Authorization": []string{"Bearer PLACEHOLDER"}, }, diff --git a/docs/static-inject-plugin.md b/docs/static-inject-plugin.md index cf2c7f284..8542b2ccf 100644 --- a/docs/static-inject-plugin.md +++ b/docs/static-inject-plugin.md @@ -73,6 +73,42 @@ after resolution on purpose: only the resolver knows whether this host has a credential, which is why the gate lives in the injector rather than in a pre-pipeline check. +**Outbound only.** The gate fires on outbound requests — the forward +proxy's egress, which is what the contract is about. It deliberately +does not fire on an inbound (reverse-proxy) request, because there +`pctx.Scheme` describes the **wrong hop**: it is the scheme the client +used to reach Cortex, `http` for any non-TLS inbound listener, while +the credential leaves on the Cortex→backend hop whose scheme comes +from `reverse_proxy_backend` and is not visible to the plugin. An +inbound static-inject behind a plaintext listener in front of an +`https://` backend is a supported shape — MOCA's AB1 is exactly that — +and refusing it would block every request while protecting nothing. + +Covering the inbound hop properly would mean teaching the reverse +proxy to expose its backend scheme; that is not implemented. Until it +is, an operator running static-inject inbound is responsible for the +front listener's own transport (`mtls:`, or a trusted network), the +same as for any other header the reverse proxy forwards. + +### Deployments this refuses + +The refusal is the intended contract, but it is a behaviour change for +anything that was relying on plaintext egress with a credential: + +| Shape | Effect | +|---|---| +| Forward-proxy egress, `http://` target **with** a credential | **Now 403.** The request must move to `https://`, or the host must stop having a credential. | +| Forward-proxy egress, `http://` target without a credential | Unchanged (401 `unresolved-key`, as before). | +| Inbound static-inject, plaintext front, any backend | Unchanged — the gate does not fire inbound (above). | + +Known downstream: **MOCA's AB2 leaf smoke** (`deploy/knative/leaf-smoke.sh`, +the `H2-inject` allow-path claim) sends +`curl -s http://echo-target/ -H "Authorization: Bearer $ECHO_CRED"` +through the forward proxy to a host that has a credential. That request +is refused by this gate. AB2 is pinned to an image predating it, so +nothing breaks today, but the next image bump fails that check unless +`echo-target` moves to TLS or the smoke is updated to expect the 403. + One boundary worth naming: the gate judges the scheme the request *arrived* with. A `WritesDestination` plugin later in the chain can `Redirect` to an `http://` target (an operator-pinned server), and the From 805529b19705359d62904478cb151fbc6acc91bb Mon Sep 17 00:00:00 2001 From: Mariusz Sabath Date: Thu, 29 Oct 2026 11:20:10 -0400 Subject: [PATCH 4/4] docs: name the backend hop as the inbound operator's job (#1481) Round-2 review follow-ups on #1537. Documentation and comments only; no behaviour change. The inbound prose named the wrong hop. The paragraph above it already explains that the credential does not travel on the client->Cortex hop, so the uncovered inbound risk is not the front listener's transport -- it is a reverse_proxy_backend of "http://", where the real key is injected and then crosses that hop in cleartext. mtls: on the front listener does not change that. Both the doc and the plugin.go comment carried the same slip; both now point at reverse_proxy_backend and say plainly that Cortex does not refuse an http:// backend. Also adds the shape the "Deployments this refuses" table was missing: under mtls: mode: strict the forward proxy dials every upstream through mtlsDialer (TLS-or-fail), so an http:// target is TLS on the wire, yet the gate reads the request scheme and still returns 403. Fail-closed and rare, but worth a row so the 403 is not a mystery. Drops two line-number references (reverseproxy/server.go:797) that would drift with the next edit to that file; the requestScheme function name is enough to find it. Assisted-By: Claude (Anthropic AI) Signed-off-by: Mariusz Sabath --- core/plugins/staticinject/plugin.go | 11 +++++++---- core/plugins/staticinject/plugin_test.go | 2 +- docs/static-inject-plugin.md | 11 ++++++++--- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/core/plugins/staticinject/plugin.go b/core/plugins/staticinject/plugin.go index 77dbf19f0..f5dc71188 100644 --- a/core/plugins/staticinject/plugin.go +++ b/core/plugins/staticinject/plugin.go @@ -228,7 +228,7 @@ func (p *StaticInject) OnRequest(ctx context.Context, pctx *pipeline.Context) pi // // OUTBOUND ONLY, and the direction is load-bearing rather than a // narrowing for caution's sake. On the reverse proxy pctx.Scheme is - // requestScheme(r) (reverseproxy/server.go:797) — the scheme the CLIENT + // requestScheme(r) (reverseproxy/server.go) — the scheme the CLIENT // used to reach Cortex, which is "http" whenever the inbound listener is // not TLS. The credential does not travel on that hop; it leaves on the // Cortex→backend hop, whose scheme comes from reverse_proxy_backend and @@ -243,9 +243,12 @@ func (p *StaticInject) OnRequest(ctx context.Context, pctx *pipeline.Context) pi // through it — reproduced at 403 with zero upstream hits (#1537 review). // // Covering inbound properly would mean teaching the reverse proxy to - // expose its backend scheme, which #1481 does not ask for; until then an - // inbound plaintext hop is the operator's own listener, under their own - // network, rather than the guest-controlled egress this gate is about. + // expose its backend scheme, which #1481 does not ask for; until then the + // uncovered inbound risk is a reverse_proxy_backend of "http://" — the + // real key is injected and crosses THAT hop in cleartext — and pointing + // it at https:// (or a trusted network) is the operator's own call, not + // the guest-controlled egress this gate is about. The front listener's + // transport is not what matters here; the credential never travels on it. // // The check lives in the injector, not in a pre-pipeline gate, because // only the resolver knows whether this host has a credential — a diff --git a/core/plugins/staticinject/plugin_test.go b/core/plugins/staticinject/plugin_test.go index 1e80127e5..36b21d67a 100644 --- a/core/plugins/staticinject/plugin_test.go +++ b/core/plugins/staticinject/plugin_test.go @@ -431,7 +431,7 @@ func TestPlaintextGateAllowsHTTPS(t *testing.T) { // INBOUND + http STILL INJECTS, and this is the test that keeps MOCA's AB1 // working. On the reverse proxy pctx.Scheme is the scheme the CLIENT used to -// reach Cortex (requestScheme, reverseproxy/server.go:797) — "http" whenever +// reach Cortex (requestScheme, reverseproxy/server.go) — "http" whenever // the inbound listener is not TLS. The credential leaves on the Cortex→backend // hop instead, whose scheme comes from reverse_proxy_backend and cannot be read // from here, so an inbound refusal keyed on Scheme judges the wrong hop. diff --git a/docs/static-inject-plugin.md b/docs/static-inject-plugin.md index 8542b2ccf..ce003df1c 100644 --- a/docs/static-inject-plugin.md +++ b/docs/static-inject-plugin.md @@ -86,9 +86,13 @@ and refusing it would block every request while protecting nothing. Covering the inbound hop properly would mean teaching the reverse proxy to expose its backend scheme; that is not implemented. Until it -is, an operator running static-inject inbound is responsible for the -front listener's own transport (`mtls:`, or a trusted network), the -same as for any other header the reverse proxy forwards. +is, an operator running static-inject inbound is responsible for +pointing `reverse_proxy_backend` at an `https://` backend (or one on a +trusted network). That is the hop the credential actually travels on: +an inbound static-inject in front of a plaintext backend still injects +the real key, which then crosses that hop in cleartext. Cortex does +not refuse an `http://` backend, and `mtls:` on the front listener +does not change it. ### Deployments this refuses @@ -99,6 +103,7 @@ anything that was relying on plaintext egress with a credential: |---|---| | Forward-proxy egress, `http://` target **with** a credential | **Now 403.** The request must move to `https://`, or the host must stop having a credential. | | Forward-proxy egress, `http://` target without a credential | Unchanged (401 `unresolved-key`, as before). | +| Forward-proxy egress, `http://` target with a credential, under `mtls: mode: strict` | **Now 403**, even though the hop is TLS on the wire. Strict mode dials every upstream TLS-or-fail (`mtlsDialer`), but the gate reads the request's scheme, not the dialer, so it cannot see that. Fail-closed, and rare (static-inject plus strict mesh mTLS); move the target to `https://` to inject. | | Inbound static-inject, plaintext front, any backend | Unchanged — the gate does not fire inbound (above). | Known downstream: **MOCA's AB2 leaf smoke** (`deploy/knative/leaf-smoke.sh`,