From 6e1e66baef89adff6ae9978dc3398d2aad4e8599 Mon Sep 17 00:00:00 2001 From: YehoshuaSagron Date: Sun, 6 Sep 2026 10:19:36 +0300 Subject: [PATCH 1/2] Fix: Strip query string from pctx.Path in ext_proc and ext_authz MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pipeline.Context.Path meant different things depending on the listener: the forward and reverse proxies populate it from r.URL.Path (query-free, percent-decoded by net/http's parser), while ext_proc used the raw :path pseudo-header and ext_authz used AttributeContext.HttpRequest.path — both of which carry the full request target, query string included. Any plugin behavior keyed on Path therefore differed by deployment mode. Three consumers had already grown defensive strips (bypass matcher, tool-prune's gate, inference-parser's dialect dispatch), while others were still exposed: context-guru's suffix gate misses /v1/messages?beta=true under Envoy modes, OPA policies exact-matching input.path break only there, and ibac's judge prompt includes query parameters only there. Run the raw request target through url.ParseRequestURI — the same parser net/http runs for the proxy listeners — at pctx construction in both Envoy-fed listeners, so Path is byte-identical across listener modes. The invariant is documented on Context.Path and pinned by new tests in both fixed listeners (red before this change). inference-parser's defensive strip stays as defense in depth for contexts constructed outside a listener; its comment now reflects the guaranteed invariant. Assisted-By: Claude (Anthropic AI) Signed-off-by: YehoshuaSagron --- .../authlib/listener/extauthz/server.go | 23 +++++- .../listener/extauthz/server_path_test.go | 72 +++++++++++++++++ authbridge/authlib/listener/extproc/server.go | 27 ++++++- .../listener/extproc/server_path_test.go | 77 +++++++++++++++++++ authbridge/authlib/pipeline/context.go | 14 +++- .../authlib/plugins/inferenceparser/plugin.go | 18 ++--- 6 files changed, 212 insertions(+), 19 deletions(-) create mode 100644 authbridge/authlib/listener/extauthz/server_path_test.go create mode 100644 authbridge/authlib/listener/extproc/server_path_test.go diff --git a/authbridge/authlib/listener/extauthz/server.go b/authbridge/authlib/listener/extauthz/server.go index a8baf543e..79d888854 100644 --- a/authbridge/authlib/listener/extauthz/server.go +++ b/authbridge/authlib/listener/extauthz/server.go @@ -6,6 +6,8 @@ package extauthz import ( "context" "net/http" + "net/url" + "strings" "time" corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" @@ -44,7 +46,7 @@ func (s *Server) Check(ctx context.Context, req *authv3.CheckRequest) (*authv3.C if host == "" { host = headers["host"] } - path := httpReq.GetPath() + path := pathOnly(httpReq.GetPath()) scheme := httpReq.GetScheme() // Inbound validation via pipeline @@ -113,6 +115,25 @@ func authzOutcome(pctx *pipeline.Context) pipeline.Outcome { return pipeline.Outcome{FinalAction: pipeline.OutcomeAllow} } +// pathOnly extracts the URL path from a request target. +// AttributeContext.HttpRequest.path carries the full target, query string +// included, but pctx.Path must hold only the path — see +// pipeline.Context.Path. It runs the same parser net/http runs for the +// proxy listeners, so pctx.Path is byte-identical across listener modes +// (decoding included). +func pathOnly(target string) string { + u, err := url.ParseRequestURI(target) + if err != nil { + // Unparseable target: fall back to a manual strip so a query + // never leaks into Path. + if i := strings.IndexByte(target, '?'); i >= 0 { + return target[:i] + } + return target + } + return u.Path +} + func mapToHTTPHeader(m map[string]string) http.Header { h := make(http.Header) for k, v := range m { diff --git a/authbridge/authlib/listener/extauthz/server_path_test.go b/authbridge/authlib/listener/extauthz/server_path_test.go new file mode 100644 index 000000000..c313f86be --- /dev/null +++ b/authbridge/authlib/listener/extauthz/server_path_test.go @@ -0,0 +1,72 @@ +package extauthz + +import ( + "context" + "testing" + + authv3 "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3" + + "github.com/rossoctl/cortex/authbridge/authlib/pipeline" + "github.com/rossoctl/cortex/authbridge/authlib/plugins/plugintesting" +) + +// pathCapture records the pctx.Path each pipeline run sees, so tests can +// assert on what the listener actually constructed rather than on side +// effects of a real plugin. +type pathCapture struct { + paths []string +} + +func (p *pathCapture) Name() string { return "path-capture" } +func (p *pathCapture) Capabilities() pipeline.PluginCapabilities { + return pipeline.PluginCapabilities{} +} +func (p *pathCapture) OnRequest(_ context.Context, pctx *pipeline.Context) pipeline.Action { + p.paths = append(p.paths, pctx.Path) + return pipeline.Action{Type: pipeline.Continue} +} +func (p *pathCapture) OnResponse(_ context.Context, _ *pipeline.Context) pipeline.Action { + return pipeline.Action{Type: pipeline.Continue} +} + +// Envoy's AttributeContext.HttpRequest.path is "the request target, as it +// appears in the first line of the HTTP request" — query string included. +// pctx.Path must contain only the URL path, exactly as the proxy listeners +// produce it from r.URL.Path, so plugin behavior keyed on Path cannot +// differ by listener mode. +func TestCheck_PathMatchesProxyListeners(t *testing.T) { + inCap, outCap := &pathCapture{}, &pathCapture{} + inbound, err := plugintesting.BuildPipeline([]pipeline.Plugin{inCap}) + if err != nil { + t.Fatalf("building inbound pipeline: %v", err) + } + outbound, err := plugintesting.BuildPipeline([]pipeline.Plugin{outCap}) + if err != nil { + t.Fatalf("building outbound pipeline: %v", err) + } + srv := &Server{ + InboundPipeline: pipeline.NewHolder(inbound), + OutboundPipeline: pipeline.NewHolder(outbound), + } + + req := &authv3.CheckRequest{ + Attributes: &authv3.AttributeContext{ + Request: &authv3.AttributeContext_Request{ + Http: &authv3.AttributeContext_HttpRequest{ + Headers: map[string]string{":authority": "target-svc"}, + Path: "/api/x?secret=1", + }, + }, + }, + } + if _, err := srv.Check(context.Background(), req); err != nil { + t.Fatalf("Check: %v", err) + } + + if len(inCap.paths) != 1 || inCap.paths[0] != "/api/x" { + t.Errorf("inbound pctx.Path = %q, want [/api/x]", inCap.paths) + } + if len(outCap.paths) != 1 || outCap.paths[0] != "/api/x" { + t.Errorf("outbound pctx.Path = %q, want [/api/x]", outCap.paths) + } +} diff --git a/authbridge/authlib/listener/extproc/server.go b/authbridge/authlib/listener/extproc/server.go index ed08d2c24..62f0f2eb1 100644 --- a/authbridge/authlib/listener/extproc/server.go +++ b/authbridge/authlib/listener/extproc/server.go @@ -10,6 +10,7 @@ import ( "io" "log/slog" "net/http" + "net/url" "slices" "strconv" "strings" @@ -161,7 +162,7 @@ func (s *Server) handleInbound(stream extprocv3.ExternalProcessor_ProcessServer, Direction: pipeline.Inbound, Method: getHeader(headers, ":method"), Scheme: getHeader(headers, ":scheme"), - Path: getHeader(headers, ":path"), + Path: pathOnly(getHeader(headers, ":path")), Headers: headerMapToHTTP(headers), Body: body, Shared: s.Shared, @@ -186,7 +187,7 @@ func (s *Server) handleInboundBody(stream extprocv3.ExternalProcessor_ProcessSer Direction: pipeline.Inbound, Method: getHeader(headers, ":method"), Scheme: getHeader(headers, ":scheme"), - Path: getHeader(headers, ":path"), + Path: pathOnly(getHeader(headers, ":path")), Headers: headerMapToHTTP(headers), Body: body, Shared: s.Shared, @@ -471,7 +472,7 @@ func (s *Server) handleOutbound(stream extprocv3.ExternalProcessor_ProcessServer Method: getHeader(headers, ":method"), Scheme: getHeader(headers, ":scheme"), Host: authorityOf(headers), - Path: getHeader(headers, ":path"), + Path: pathOnly(getHeader(headers, ":path")), Headers: headerMapToHTTP(headers), Body: body, Shared: s.Shared, @@ -513,7 +514,7 @@ func (s *Server) handleOutboundBody(stream extprocv3.ExternalProcessor_ProcessSe Method: getHeader(headers, ":method"), Scheme: getHeader(headers, ":scheme"), Host: authorityOf(headers), - Path: getHeader(headers, ":path"), + Path: pathOnly(getHeader(headers, ":path")), Headers: headerMapToHTTP(headers), Body: body, Shared: s.Shared, @@ -977,6 +978,24 @@ func requestHasBody(headers *corev3.HeaderMap) bool { return te != "" } +// pathOnly extracts the URL path from a request target. The :path +// pseudo-header carries the full target ("/api/x?a=1"), but pctx.Path +// must hold only the path — see pipeline.Context.Path. It runs the same +// parser net/http runs for the proxy listeners, so pctx.Path is +// byte-identical across listener modes (decoding included). +func pathOnly(target string) string { + u, err := url.ParseRequestURI(target) + if err != nil { + // Unparseable target (empty on CONNECT, garbage): fall back to a + // manual strip so a query never leaks into Path. + if i := strings.IndexByte(target, '?'); i >= 0 { + return target[:i] + } + return target + } + return u.Path +} + func getHeader(headers *corev3.HeaderMap, key string) string { if headers == nil { return "" diff --git a/authbridge/authlib/listener/extproc/server_path_test.go b/authbridge/authlib/listener/extproc/server_path_test.go new file mode 100644 index 000000000..360b072b7 --- /dev/null +++ b/authbridge/authlib/listener/extproc/server_path_test.go @@ -0,0 +1,77 @@ +package extproc + +import ( + "context" + "testing" + + extprocv3 "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3" + "github.com/rossoctl/cortex/authbridge/authlib/pipeline" + "github.com/rossoctl/cortex/authbridge/authlib/plugins/plugintesting" +) + +// pathCapture records the pctx.Path each pipeline run sees, so tests can +// assert on what the listener actually constructed rather than on side +// effects of a real plugin. +type pathCapture struct { + paths []string +} + +func (p *pathCapture) Name() string { return "path-capture" } +func (p *pathCapture) Capabilities() pipeline.PluginCapabilities { + return pipeline.PluginCapabilities{} +} +func (p *pathCapture) OnRequest(_ context.Context, pctx *pipeline.Context) pipeline.Action { + p.paths = append(p.paths, pctx.Path) + return pipeline.Action{Type: pipeline.Continue} +} +func (p *pathCapture) OnResponse(_ context.Context, _ *pipeline.Context) pipeline.Action { + return pipeline.Action{Type: pipeline.Continue} +} + +func captureServer(t *testing.T) (*Server, *pathCapture, *pathCapture) { + t.Helper() + inCap, outCap := &pathCapture{}, &pathCapture{} + inbound, err := plugintesting.BuildPipeline([]pipeline.Plugin{inCap}) + if err != nil { + t.Fatalf("building inbound pipeline: %v", err) + } + outbound, err := plugintesting.BuildPipeline([]pipeline.Plugin{outCap}) + if err != nil { + t.Fatalf("building outbound pipeline: %v", err) + } + return &Server{ + InboundPipeline: pipeline.NewHolder(inbound), + OutboundPipeline: pipeline.NewHolder(outbound), + }, inCap, outCap +} + +// The :path pseudo-header carries the full request target, query string +// included. pctx.Path must contain only the URL path, exactly as the +// forward and reverse proxy listeners produce it from r.URL.Path (query +// dropped, percent-decoding applied) — so plugin behavior keyed on Path +// cannot differ by listener mode. +func TestExtProc_PathMatchesProxyListeners(t *testing.T) { + srv, inCap, outCap := captureServer(t) + + stream := &mockStream{ + ctx: context.Background(), + requests: []*extprocv3.ProcessingRequest{ + inboundRequest(makeHeaders( + "x-authbridge-direction", "inbound", + ":path", "/api/x?secret=1", + )), + outboundRequest(makeHeaders( + ":authority", "target-svc", + ":path", "/api/hello%20world?secret=1&b=2", + )), + }, + } + _ = srv.Process(stream) + + if want := []string{"/api/x"}; len(inCap.paths) != 1 || inCap.paths[0] != want[0] { + t.Errorf("inbound pctx.Path = %q, want %q", inCap.paths, want) + } + if want := []string{"/api/hello world"}; len(outCap.paths) != 1 || outCap.paths[0] != want[0] { + t.Errorf("outbound pctx.Path = %q, want %q", outCap.paths, want) + } +} diff --git a/authbridge/authlib/pipeline/context.go b/authbridge/authlib/pipeline/context.go index 78182dd45..c788d5e5c 100644 --- a/authbridge/authlib/pipeline/context.go +++ b/authbridge/authlib/pipeline/context.go @@ -104,9 +104,17 @@ type Context struct { // fixtures, an unrecognized transport, etc.). Plugins that need a // concrete scheme should pick a default explicitly — treating "" // as "assume http" would silently mask missing listener plumbing. - Scheme string - Host string - Path string + Scheme string + Host string + + // Path is the URL path of the request, never including a query + // string: the proxy listeners populate it from r.URL.Path, and + // ext_proc / ext_authz run the raw request target through the same + // URL parser (url.ParseRequestURI), so the value is identical + // across listener modes. Plugins may match, log, or feed Path into + // policy without stripping a query themselves. + Path string + Headers http.Header Body []byte // nil unless at least one plugin declares BodyAccess: true diff --git a/authbridge/authlib/plugins/inferenceparser/plugin.go b/authbridge/authlib/plugins/inferenceparser/plugin.go index 5c80afd31..770610566 100644 --- a/authbridge/authlib/plugins/inferenceparser/plugin.go +++ b/authbridge/authlib/plugins/inferenceparser/plugin.go @@ -33,17 +33,13 @@ func (p *InferenceParser) Capabilities() pipeline.PluginCapabilities { // endpointPath returns pctx.Path with any query string removed. // -// The listeners disagree on what Path holds, and dialect dispatch below is -// exact-match, so this has to be normalised in one place. The HTTP listeners -// set Path from r.URL.Path, which already excludes the query; extproc sets it -// from the HTTP/2 :path pseudo-header, which per RFC 9113 §8.3.1 includes it. -// -// Claude Code posts to /v1/messages?beta=true, so without this the request -// falls to the default arm on the envoy-sidecar path and the parser records no -// inference telemetry at all — and once OnRequest did match, the four -// dialect-selection sites below would send an Anthropic stream to the OpenAI -// parser. Both failure modes are silent, which is why every site normalises -// rather than only the dispatch switch. +// Every listener now guarantees Path is query-free (see pipeline.Context.Path), +// so this is defense in depth for contexts constructed outside a listener +// (tests, future transports). It stays because the failure mode it guards is +// silent: Claude Code posts to /v1/messages?beta=true, and with a query +// attached the exact-match dialect dispatch below falls to the default arm and +// records no inference telemetry at all — or worse, sends an Anthropic stream +// to the OpenAI parser. func endpointPath(pctx *pipeline.Context) string { path, _, _ := strings.Cut(pctx.Path, "?") return path From 29d8be3a55a9337b63fc81099386e33a967a2a83 Mon Sep 17 00:00:00 2001 From: YehoshuaSagron Date: Sun, 6 Sep 2026 12:30:45 +0300 Subject: [PATCH 2/2] =?UTF-8?q?Fix:=20Harden=20pctx.Path=20parity=20?= =?UTF-8?q?=E2=80=94=20shared=20helper,=20hedged=20docs,=20fallback=20test?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review-hardening pass on the previous commit: - Hoist pathOnly to a single exported httpx.PathOnly used by both Envoy-fed listeners. The function now defines a cross-listener invariant documented on pipeline.Context.Path; two private copies could drift silently. - Hedge the Context.Path and PathOnly doc comments: values are identical across listener modes modulo unparseable targets, which net/http rejects with 400 before any pipeline runs while the Envoy-fed listeners keep them query-stripped but otherwise raw. - Table-drive both listener tests and extend them to pin all three behaviors per listener: query strip, percent-decoding, and the unparseable-target fallback (previously uncovered — a regression there would have passed green). - Run each ext_proc test request on its own mock stream, matching the one-request-per-stream production shape instead of relying on incidental cross-request state handling. Assisted-By: Claude (Anthropic AI) Signed-off-by: YehoshuaSagron --- .../authlib/listener/extauthz/server.go | 25 +----- .../listener/extauthz/server_path_test.go | 76 +++++++++++-------- authbridge/authlib/listener/extproc/server.go | 27 +------ .../listener/extproc/server_path_test.go | 65 +++++++++++----- authbridge/authlib/listener/httpx/path.go | 26 +++++++ authbridge/authlib/pipeline/context.go | 9 ++- 6 files changed, 127 insertions(+), 101 deletions(-) create mode 100644 authbridge/authlib/listener/httpx/path.go diff --git a/authbridge/authlib/listener/extauthz/server.go b/authbridge/authlib/listener/extauthz/server.go index 79d888854..f8429e813 100644 --- a/authbridge/authlib/listener/extauthz/server.go +++ b/authbridge/authlib/listener/extauthz/server.go @@ -6,8 +6,6 @@ package extauthz import ( "context" "net/http" - "net/url" - "strings" "time" corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" @@ -18,6 +16,7 @@ import ( rpcstatus "google.golang.org/genproto/googleapis/rpc/status" "github.com/rossoctl/cortex/authbridge/authlib/auth" + "github.com/rossoctl/cortex/authbridge/authlib/listener/httpx" "github.com/rossoctl/cortex/authbridge/authlib/pipeline" ) @@ -46,7 +45,7 @@ func (s *Server) Check(ctx context.Context, req *authv3.CheckRequest) (*authv3.C if host == "" { host = headers["host"] } - path := pathOnly(httpReq.GetPath()) + path := httpx.PathOnly(httpReq.GetPath()) scheme := httpReq.GetScheme() // Inbound validation via pipeline @@ -115,25 +114,6 @@ func authzOutcome(pctx *pipeline.Context) pipeline.Outcome { return pipeline.Outcome{FinalAction: pipeline.OutcomeAllow} } -// pathOnly extracts the URL path from a request target. -// AttributeContext.HttpRequest.path carries the full target, query string -// included, but pctx.Path must hold only the path — see -// pipeline.Context.Path. It runs the same parser net/http runs for the -// proxy listeners, so pctx.Path is byte-identical across listener modes -// (decoding included). -func pathOnly(target string) string { - u, err := url.ParseRequestURI(target) - if err != nil { - // Unparseable target: fall back to a manual strip so a query - // never leaks into Path. - if i := strings.IndexByte(target, '?'); i >= 0 { - return target[:i] - } - return target - } - return u.Path -} - func mapToHTTPHeader(m map[string]string) http.Header { h := make(http.Header) for k, v := range m { @@ -142,7 +122,6 @@ func mapToHTTPHeader(m map[string]string) http.Header { return h } - // deniedFromAction renders a pipeline Reject as an ext_authz CheckResponse // preserving the plugin's status, headers, and body. The flat // {"error":reason} body of the old denied() is gone — plugins can now diff --git a/authbridge/authlib/listener/extauthz/server_path_test.go b/authbridge/authlib/listener/extauthz/server_path_test.go index c313f86be..3db831f6e 100644 --- a/authbridge/authlib/listener/extauthz/server_path_test.go +++ b/authbridge/authlib/listener/extauthz/server_path_test.go @@ -32,41 +32,55 @@ func (p *pathCapture) OnResponse(_ context.Context, _ *pipeline.Context) pipelin // Envoy's AttributeContext.HttpRequest.path is "the request target, as it // appears in the first line of the HTTP request" — query string included. // pctx.Path must contain only the URL path, exactly as the proxy listeners -// produce it from r.URL.Path, so plugin behavior keyed on Path cannot -// differ by listener mode. +// produce it from r.URL.Path (query dropped, percent-decoding applied), so +// plugin behavior keyed on Path cannot differ by listener mode. An +// unparseable target keeps the plain query-strip fallback. func TestCheck_PathMatchesProxyListeners(t *testing.T) { - inCap, outCap := &pathCapture{}, &pathCapture{} - inbound, err := plugintesting.BuildPipeline([]pipeline.Plugin{inCap}) - if err != nil { - t.Fatalf("building inbound pipeline: %v", err) - } - outbound, err := plugintesting.BuildPipeline([]pipeline.Plugin{outCap}) - if err != nil { - t.Fatalf("building outbound pipeline: %v", err) - } - srv := &Server{ - InboundPipeline: pipeline.NewHolder(inbound), - OutboundPipeline: pipeline.NewHolder(outbound), + cases := []struct { + name string + target string + want string + }{ + {"query stripped", "/api/x?secret=1", "/api/x"}, + {"percent-decoded", "/api/hello%20world?secret=1&b=2", "/api/hello world"}, + {"unparseable falls back to query strip", "/a%zz?secret=1", "/a%zz"}, } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + inCap, outCap := &pathCapture{}, &pathCapture{} + inbound, err := plugintesting.BuildPipeline([]pipeline.Plugin{inCap}) + if err != nil { + t.Fatalf("building inbound pipeline: %v", err) + } + outbound, err := plugintesting.BuildPipeline([]pipeline.Plugin{outCap}) + if err != nil { + t.Fatalf("building outbound pipeline: %v", err) + } + srv := &Server{ + InboundPipeline: pipeline.NewHolder(inbound), + OutboundPipeline: pipeline.NewHolder(outbound), + } - req := &authv3.CheckRequest{ - Attributes: &authv3.AttributeContext{ - Request: &authv3.AttributeContext_Request{ - Http: &authv3.AttributeContext_HttpRequest{ - Headers: map[string]string{":authority": "target-svc"}, - Path: "/api/x?secret=1", + req := &authv3.CheckRequest{ + Attributes: &authv3.AttributeContext{ + Request: &authv3.AttributeContext_Request{ + Http: &authv3.AttributeContext_HttpRequest{ + Headers: map[string]string{":authority": "target-svc"}, + Path: tc.target, + }, + }, }, - }, - }, - } - if _, err := srv.Check(context.Background(), req); err != nil { - t.Fatalf("Check: %v", err) - } + } + if _, err := srv.Check(context.Background(), req); err != nil { + t.Fatalf("Check: %v", err) + } - if len(inCap.paths) != 1 || inCap.paths[0] != "/api/x" { - t.Errorf("inbound pctx.Path = %q, want [/api/x]", inCap.paths) - } - if len(outCap.paths) != 1 || outCap.paths[0] != "/api/x" { - t.Errorf("outbound pctx.Path = %q, want [/api/x]", outCap.paths) + if len(inCap.paths) != 1 || inCap.paths[0] != tc.want { + t.Errorf("inbound pctx.Path = %q, want [%q]", inCap.paths, tc.want) + } + if len(outCap.paths) != 1 || outCap.paths[0] != tc.want { + t.Errorf("outbound pctx.Path = %q, want [%q]", outCap.paths, tc.want) + } + }) } } diff --git a/authbridge/authlib/listener/extproc/server.go b/authbridge/authlib/listener/extproc/server.go index 62f0f2eb1..c81aad9cc 100644 --- a/authbridge/authlib/listener/extproc/server.go +++ b/authbridge/authlib/listener/extproc/server.go @@ -10,7 +10,6 @@ import ( "io" "log/slog" "net/http" - "net/url" "slices" "strconv" "strings" @@ -162,7 +161,7 @@ func (s *Server) handleInbound(stream extprocv3.ExternalProcessor_ProcessServer, Direction: pipeline.Inbound, Method: getHeader(headers, ":method"), Scheme: getHeader(headers, ":scheme"), - Path: pathOnly(getHeader(headers, ":path")), + Path: httpx.PathOnly(getHeader(headers, ":path")), Headers: headerMapToHTTP(headers), Body: body, Shared: s.Shared, @@ -187,7 +186,7 @@ func (s *Server) handleInboundBody(stream extprocv3.ExternalProcessor_ProcessSer Direction: pipeline.Inbound, Method: getHeader(headers, ":method"), Scheme: getHeader(headers, ":scheme"), - Path: pathOnly(getHeader(headers, ":path")), + Path: httpx.PathOnly(getHeader(headers, ":path")), Headers: headerMapToHTTP(headers), Body: body, Shared: s.Shared, @@ -472,7 +471,7 @@ func (s *Server) handleOutbound(stream extprocv3.ExternalProcessor_ProcessServer Method: getHeader(headers, ":method"), Scheme: getHeader(headers, ":scheme"), Host: authorityOf(headers), - Path: pathOnly(getHeader(headers, ":path")), + Path: httpx.PathOnly(getHeader(headers, ":path")), Headers: headerMapToHTTP(headers), Body: body, Shared: s.Shared, @@ -514,7 +513,7 @@ func (s *Server) handleOutboundBody(stream extprocv3.ExternalProcessor_ProcessSe Method: getHeader(headers, ":method"), Scheme: getHeader(headers, ":scheme"), Host: authorityOf(headers), - Path: pathOnly(getHeader(headers, ":path")), + Path: httpx.PathOnly(getHeader(headers, ":path")), Headers: headerMapToHTTP(headers), Body: body, Shared: s.Shared, @@ -978,24 +977,6 @@ func requestHasBody(headers *corev3.HeaderMap) bool { return te != "" } -// pathOnly extracts the URL path from a request target. The :path -// pseudo-header carries the full target ("/api/x?a=1"), but pctx.Path -// must hold only the path — see pipeline.Context.Path. It runs the same -// parser net/http runs for the proxy listeners, so pctx.Path is -// byte-identical across listener modes (decoding included). -func pathOnly(target string) string { - u, err := url.ParseRequestURI(target) - if err != nil { - // Unparseable target (empty on CONNECT, garbage): fall back to a - // manual strip so a query never leaks into Path. - if i := strings.IndexByte(target, '?'); i >= 0 { - return target[:i] - } - return target - } - return u.Path -} - func getHeader(headers *corev3.HeaderMap, key string) string { if headers == nil { return "" diff --git a/authbridge/authlib/listener/extproc/server_path_test.go b/authbridge/authlib/listener/extproc/server_path_test.go index 360b072b7..bcb8e046f 100644 --- a/authbridge/authlib/listener/extproc/server_path_test.go +++ b/authbridge/authlib/listener/extproc/server_path_test.go @@ -5,6 +5,7 @@ import ( "testing" extprocv3 "github.com/envoyproxy/go-control-plane/envoy/service/ext_proc/v3" + "github.com/rossoctl/cortex/authbridge/authlib/pipeline" "github.com/rossoctl/cortex/authbridge/authlib/plugins/plugintesting" ) @@ -49,29 +50,51 @@ func captureServer(t *testing.T) (*Server, *pathCapture, *pathCapture) { // included. pctx.Path must contain only the URL path, exactly as the // forward and reverse proxy listeners produce it from r.URL.Path (query // dropped, percent-decoding applied) — so plugin behavior keyed on Path -// cannot differ by listener mode. +// cannot differ by listener mode. An unparseable target (which net/http +// would reject with 400 before any pipeline runs) keeps the plain +// query-strip fallback. func TestExtProc_PathMatchesProxyListeners(t *testing.T) { - srv, inCap, outCap := captureServer(t) - - stream := &mockStream{ - ctx: context.Background(), - requests: []*extprocv3.ProcessingRequest{ - inboundRequest(makeHeaders( - "x-authbridge-direction", "inbound", - ":path", "/api/x?secret=1", - )), - outboundRequest(makeHeaders( - ":authority", "target-svc", - ":path", "/api/hello%20world?secret=1&b=2", - )), - }, + cases := []struct { + name string + target string + want string + }{ + {"query stripped", "/api/x?secret=1", "/api/x"}, + {"percent-decoded", "/api/hello%20world?secret=1&b=2", "/api/hello world"}, + {"unparseable falls back to query strip", "/a%zz?secret=1", "/a%zz"}, } - _ = srv.Process(stream) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + // One mock stream per request — each ext_proc stream carries a + // single request in production. + srv, inCap, outCap := captureServer(t) + inStream := &mockStream{ + ctx: context.Background(), + requests: []*extprocv3.ProcessingRequest{ + inboundRequest(makeHeaders( + "x-authbridge-direction", "inbound", + ":path", tc.target, + )), + }, + } + _ = srv.Process(inStream) + outStream := &mockStream{ + ctx: context.Background(), + requests: []*extprocv3.ProcessingRequest{ + outboundRequest(makeHeaders( + ":authority", "target-svc", + ":path", tc.target, + )), + }, + } + _ = srv.Process(outStream) - if want := []string{"/api/x"}; len(inCap.paths) != 1 || inCap.paths[0] != want[0] { - t.Errorf("inbound pctx.Path = %q, want %q", inCap.paths, want) - } - if want := []string{"/api/hello world"}; len(outCap.paths) != 1 || outCap.paths[0] != want[0] { - t.Errorf("outbound pctx.Path = %q, want %q", outCap.paths, want) + if len(inCap.paths) != 1 || inCap.paths[0] != tc.want { + t.Errorf("inbound pctx.Path = %q, want [%q]", inCap.paths, tc.want) + } + if len(outCap.paths) != 1 || outCap.paths[0] != tc.want { + t.Errorf("outbound pctx.Path = %q, want [%q]", outCap.paths, tc.want) + } + }) } } diff --git a/authbridge/authlib/listener/httpx/path.go b/authbridge/authlib/listener/httpx/path.go new file mode 100644 index 000000000..0dfd79db2 --- /dev/null +++ b/authbridge/authlib/listener/httpx/path.go @@ -0,0 +1,26 @@ +package httpx + +import ( + "net/url" + "strings" +) + +// PathOnly extracts the URL path from a raw request target. The Envoy-fed +// listeners (ext_proc's :path pseudo-header, ext_authz's +// AttributeContext.HttpRequest.path) receive the full request target, query +// string included, but pctx.Path must hold only the path — see +// pipeline.Context.Path. It runs the same parser net/http runs for the +// proxy listeners, so pctx.Path is identical across listener modes +// (percent-decoding included), modulo targets that parser rejects: net/http +// answers those with 400 before any pipeline runs, while the Envoy-fed +// listeners fall back to a plain query strip. +func PathOnly(target string) string { + u, err := url.ParseRequestURI(target) + if err != nil { + if i := strings.IndexByte(target, '?'); i >= 0 { + return target[:i] + } + return target + } + return u.Path +} diff --git a/authbridge/authlib/pipeline/context.go b/authbridge/authlib/pipeline/context.go index c788d5e5c..0d6f7c00f 100644 --- a/authbridge/authlib/pipeline/context.go +++ b/authbridge/authlib/pipeline/context.go @@ -110,9 +110,12 @@ type Context struct { // Path is the URL path of the request, never including a query // string: the proxy listeners populate it from r.URL.Path, and // ext_proc / ext_authz run the raw request target through the same - // URL parser (url.ParseRequestURI), so the value is identical - // across listener modes. Plugins may match, log, or feed Path into - // policy without stripping a query themselves. + // URL parser (httpx.PathOnly → url.ParseRequestURI), so the value + // is identical across listener modes — modulo unparseable targets, + // which net/http rejects with 400 before any pipeline runs and the + // Envoy-fed listeners keep query-stripped but otherwise raw. + // Plugins may match, log, or feed Path into policy without + // stripping a query themselves. Path string Headers http.Header