diff --git a/authbridge/authlib/listener/extauthz/server.go b/authbridge/authlib/listener/extauthz/server.go index a8baf543e..f8429e813 100644 --- a/authbridge/authlib/listener/extauthz/server.go +++ b/authbridge/authlib/listener/extauthz/server.go @@ -16,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" ) @@ -44,7 +45,7 @@ func (s *Server) Check(ctx context.Context, req *authv3.CheckRequest) (*authv3.C if host == "" { host = headers["host"] } - path := httpReq.GetPath() + path := httpx.PathOnly(httpReq.GetPath()) scheme := httpReq.GetScheme() // Inbound validation via pipeline @@ -121,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 new file mode 100644 index 000000000..3db831f6e --- /dev/null +++ b/authbridge/authlib/listener/extauthz/server_path_test.go @@ -0,0 +1,86 @@ +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 (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) { + 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: tc.target, + }, + }, + }, + } + if _, err := srv.Check(context.Background(), req); err != nil { + t.Fatalf("Check: %v", err) + } + + 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 ed08d2c24..c81aad9cc 100644 --- a/authbridge/authlib/listener/extproc/server.go +++ b/authbridge/authlib/listener/extproc/server.go @@ -161,7 +161,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: httpx.PathOnly(getHeader(headers, ":path")), Headers: headerMapToHTTP(headers), Body: body, Shared: s.Shared, @@ -186,7 +186,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: httpx.PathOnly(getHeader(headers, ":path")), Headers: headerMapToHTTP(headers), Body: body, Shared: s.Shared, @@ -471,7 +471,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: httpx.PathOnly(getHeader(headers, ":path")), Headers: headerMapToHTTP(headers), Body: body, Shared: s.Shared, @@ -513,7 +513,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: httpx.PathOnly(getHeader(headers, ":path")), Headers: headerMapToHTTP(headers), Body: body, Shared: s.Shared, 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..bcb8e046f --- /dev/null +++ b/authbridge/authlib/listener/extproc/server_path_test.go @@ -0,0 +1,100 @@ +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. 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) { + 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) { + // 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 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 78182dd45..0d6f7c00f 100644 --- a/authbridge/authlib/pipeline/context.go +++ b/authbridge/authlib/pipeline/context.go @@ -104,9 +104,20 @@ 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 (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 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