From 2a00a14c0bad63dae3da905ddf810b44ad0db85d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20Monnom?= Date: Fri, 21 Aug 2026 14:55:54 -0700 Subject: [PATCH] agent/endpoint: fix glob matching and enforce route depth - a path convertor anywhere in a template (non-terminal, or mixed with literal text in a segment) now canonicalizes to a terminal glob that spans slashes, instead of being dropped or narrowed to a single-segment str - the old behavior silently failed to match such routes (a false negative -> misroute). - enforce MaxRouteDepth at ParseManifest (wires up RouteDepth), bounding the replicated filter's prefix count and the matcher's walk. --- pkg/agent/endpoint/manifest.go | 7 ++++ pkg/agent/endpoint/routematch.go | 46 +++++++++++++++++++-------- pkg/agent/endpoint/routematch_test.go | 37 +++++++++++++++++---- 3 files changed, 69 insertions(+), 21 deletions(-) diff --git a/pkg/agent/endpoint/manifest.go b/pkg/agent/endpoint/manifest.go index 6d827d448..6dca6d981 100644 --- a/pkg/agent/endpoint/manifest.go +++ b/pkg/agent/endpoint/manifest.go @@ -59,6 +59,13 @@ func ParseManifest(endpoints []*livekit.AgentHttp_AgentEndpoint) (*Manifest, err if err != nil { return nil, err } + // bound route depth: it caps the replicated filter's prefix count and + // the edge matcher's walk (a request past a route's depth can't match it) + if d, err := RouteDepth(ep.GetPath()); err != nil { + return nil, err + } else if d > MaxRouteDepth { + return nil, fmt.Errorf("endpoint %q exceeds max route depth %d", ep.GetPath(), MaxRouteDepth) + } var methods []string switch ep.GetKind() { case livekit.AgentHttp_AEK_HTTP: diff --git a/pkg/agent/endpoint/routematch.go b/pkg/agent/endpoint/routematch.go index 919324c01..7dac75901 100644 --- a/pkg/agent/endpoint/routematch.go +++ b/pkg/agent/endpoint/routematch.go @@ -73,37 +73,55 @@ func canonicalizeTemplate(path string) ([]string, error) { segs := splitSegments(path) out := make([]string, 0, len(segs)) for _, seg := range segs { - out = append(out, canonicalSegment(seg)) + tok, glob := canonicalSegment(seg) + out = append(out, tok) + if glob { + // a path convertor spans slashes: the glob is terminal, and any + // segments after it are dropped so the walk over-approximates (a + // concrete request matching the pre-glob prefix always matches; the + // worker's router enforces whatever follows). Truncating here can + // only add matches, never drop one. + break + } } return out, nil } -func canonicalSegment(seg string) string { - m := paramRegex.FindStringSubmatchIndex(seg) - if m == nil { - return seg // pure literal +// canonicalSegment maps a template segment to its canonical token; glob is true +// when the segment contains a path convertor (spans slashes). +func canonicalSegment(seg string) (tok string, glob bool) { + matches := paramRegex.FindAllStringSubmatchIndex(seg, -1) + if len(matches) == 0 { + return seg, false // pure literal + } + // any path convertor in the segment makes it a glob, whether it is the whole + // segment ("{rest:path}") or mixed with literal text ("pre{rest:path}") - + // str would be strictly narrower than the glob's `.*` and would drop matches + for _, m := range matches { + if m[4] != -1 && seg[m[4]+1:m[5]] == "path" { + return tokGlob, true + } } // a clean whole-segment param: "{name}" or "{name:conv}" - if m[0] == 0 && m[1] == len(seg) { + if m := matches[0]; len(matches) == 1 && m[0] == 0 && m[1] == len(seg) { conv := "str" if m[4] != -1 { conv = seg[m[4]+1 : m[5]] } switch conv { case "int": - return tokInt + return tokInt, false case "float": - return tokFloat + return tokFloat, false case "uuid": - return tokUUID - case "path": - return tokGlob + return tokUUID, false default: // str and anything ParseTemplate already accepted - return tokStr + return tokStr, false } } - // a param mixed with literal text in one segment: over-approximate to str - return tokStr + // a non-path param mixed with literal text: over-approximate to str (str's + // [^/]+ is wider than any typed single-segment convertor, so no false negative) + return tokStr, false } // candidateTokens returns the canonical tokens a concrete request segment could diff --git a/pkg/agent/endpoint/routematch_test.go b/pkg/agent/endpoint/routematch_test.go index 3aa5ddc8f..d51b8aaa6 100644 --- a/pkg/agent/endpoint/routematch_test.go +++ b/pkg/agent/endpoint/routematch_test.go @@ -38,9 +38,9 @@ func TestRouteMatchStaticAndParams(t *testing.T) { func TestRouteMatchTypedParams(t *testing.T) { x := idx(t, "/orders/{id:int}", "/u/{u:uuid}") - require.True(t, Matches(x, "/orders/42")) // int - require.False(t, Matches(x, "/orders/abc")) // not an int -> edge miss, no relay - require.False(t, Matches(x, "/orders/3.14")) // float is not int + require.True(t, Matches(x, "/orders/42")) // int + require.False(t, Matches(x, "/orders/abc")) // not an int -> edge miss, no relay + require.False(t, Matches(x, "/orders/3.14")) // float is not int uuid := "550e8400-e29b-41d4-a716-446655440000" require.True(t, Matches(x, "/u/"+uuid)) @@ -52,7 +52,7 @@ func TestRouteMatchUUIDvsStr(t *testing.T) { // a uuid value is also a valid str; whichever the route declared wins uuid := "550e8400-e29b-41d4-a716-446655440000" - strOnly := idx(t, "/x/{v}") // str + strOnly := idx(t, "/x/{v}") // str require.True(t, Matches(strOnly, "/x/"+uuid)) // uuid value matches a str route require.True(t, Matches(strOnly, "/x/anything")) @@ -66,11 +66,34 @@ func TestRouteMatchGlob(t *testing.T) { require.True(t, Matches(x, "/files/a")) require.True(t, Matches(x, "/files/a/b/c/d.txt")) // spans segments - require.True(t, Matches(x, "/files")) // path matches the empty remainder + // /files with no trailing slash is a tolerated FALSE POSITIVE (Starlette + // requires the separating slash); the worker returns the real status + require.True(t, Matches(x, "/files")) require.False(t, Matches(x, "/other/a/b")) require.True(t, Matches(x, "/static")) } +// a path convertor that is not the last segment must still match (the glob is +// treated as terminal, over-approximating the suffix) +func TestRouteMatchNonTerminalGlob(t *testing.T) { + x := idx(t, "/files/{rest:path}/edit") + require.True(t, Matches(x, "/files/a/b/edit")) + require.True(t, Matches(x, "/files/a/edit")) + // suffix is over-approximated: a path under /files matches even without + // /edit (the worker's router returns the real 404) + require.True(t, Matches(x, "/files/a/b")) + require.False(t, Matches(x, "/other/a/edit")) +} + +// a path convertor mixed with literal text in a segment spans slashes and must +// not be narrowed to a single-segment str +func TestRouteMatchMixedGlob(t *testing.T) { + x := idx(t, "/files/pre{rest:path}") + require.True(t, Matches(x, "/files/prea")) + require.True(t, Matches(x, "/files/prea/b/c")) // spans segments (would drop under str) + require.False(t, Matches(x, "/other/x")) +} + func TestRouteMatchHeterogeneousShapes(t *testing.T) { // two different workers' route sets, same path shape, different types ints := idx(t, "/item/{id:int}") @@ -85,8 +108,8 @@ func TestRouteMatchHeterogeneousShapes(t *testing.T) { func TestRouteMatchLiteralBeatsWildcardCoexist(t *testing.T) { // a literal segment and a param at the same position coexist x := idx(t, "/users/me", "/users/{id:int}") - require.True(t, Matches(x, "/users/me")) // literal - require.True(t, Matches(x, "/users/42")) // param + require.True(t, Matches(x, "/users/me")) // literal + require.True(t, Matches(x, "/users/42")) // param require.False(t, Matches(x, "/users/abc")) // neither: not "me", not an int }