A backslash in the request path escapes the upstream base-URL prefix and the repository allow-list

[matt-edmondson]

What's wrong

The only containment guard is in LfsRouteParser.TryParse (GitLfsCache/Endpoints/LfsRouteParser.cs:57). It splits the path on / and refuses any segment that is exactly . or ...

System.Uri treats \ as / in http/https URLs before it removes dot segments. UpstreamRequests.Combine builds every upstream URL as new Uri($"{base}/{path}"). So a segment such as myproject\..\..\otherorg passes the parser, because splitting on / produces no . or .. segment. Uri then turns it into a climb above the configured base URL. Kestrel hands both a raw \ and an encoded %5C through to Request.Path.Value as a literal backslash. It does not normalise them the way it normalises /../.

The allow-list does not catch this either. RepositoryAllowList.IsAllowed matches the raw RelayPath string (called from GitLfsCacheHandler.cs:248). myproject/** compiles to ^myproject/.*$, and that pattern matches myproject\..\..\otherorg/....

This breaks the guarantee in the locks spec's "Path containment" section (docs/superpowers/specs/2026-08-19-locks-subsystem-design.md), which calls the base-URL prefix "a tenancy boundary in practice". The comment at LfsRouteParser.cs:50-55 makes the same claim.

Failure scenario (verified in a scratch Kestrel app on .NET 10)

Config: Upstreams:ado:BaseUrl=https://dev.azure.com/myorg, Repositories=["myproject/**"]

POST /ado/myproject%5C..%5C..%5Cotherorg/proj/_git/repo/info/lfs/objects/batch

The parser's logic plus Combine produce:

path=/ado/myproject\..\..\otherorg/proj/_git/repo/info/lfs/objects/batch  dotsegs=False
combined=https://dev.azure.com/otherorg/proj/_git/repo/info/lfs/objects/batch

• A raw \ (curl --path-as-is) behaves the same.
• The lock routes escape the same way.
• %2F is not a problem: Kestrel keeps it encoded and Uri does not collapse it.
• On GitHub (https://github.com with studio/**), studio\..\anyorg/repo.git/info/lfs/... reaches any repository.

Upstream still authorises each call with the client's own credential, so no access is granted. The escape does defeat the resource controls:

• Batch responses for disallowed repositories are rewritten and cached. They consume the byte budget and evict the working set, which the allow-list exists to prevent.
• Lock routes build snapshots for arbitrary repositories. The spec relies on the allow-list to keep snapshot memory bounded.
• The relay route passes everything else through to URLs outside the prefix.

Suggested fix

• In LfsRouteParser.TryParse, refuse any path that contains \. Git and forge repository paths never contain one, so a 404 is correct.
• As defence in depth, after Combine builds the upstream URI, check that its AbsolutePath still starts with the base URL's path. Refuse the request if it does not.
• Mention backslashes in the spec's "Path containment" paragraph.

Acceptance criteria

• UpstreamContainmentTests covers a raw \ and %5C climbing out of a prefixed base URL on the batch, relay and locks routes. Each returns 404 and makes no upstream request.
• An allow-list test shows that studio\..\other/... is not allowed under studio/**.
• The existing dot-segment tests still pass.

[matt-edmondson]

Triage

• Category: Bug (security-relevant)
• Priority: High. Upstream still checks each call against the client's own credential, so no access is granted. But a \ or %5C segment gets past the base-URL tenancy prefix and the repository allow-list. Unapproved repositories can then consume the cache's byte budget and lock-snapshot memory, and relay traffic can reach URLs outside the configured prefix. The prefix is documented as a tenancy boundary.
• Area / suggested assignment: Routing and containment: Endpoints/LfsRouteParser.cs, UpstreamRequests.Combine, RepositoryAllowList. @matt-edmondson
• Duplicates: None found. It is related to the dot-segment containment work behind UpstreamContainmentTests, but not the same issue.
• In progress: No matching open PR. The only open GitLfsCache PRs, Bump Testably.Abstractions.FileSystem.Interface from 10.3.0 to 10.4.0 #53 and Bump Testably.Abstractions.Testing from 7.0.2 to 7.1.0 #54, are Dependabot bumps.

Notes: Do both halves of the fix. Rejecting \ in the parser is the primary fix. The post-Combine check that AbsolutePath still starts with the base path is the defence in depth that stops the next encoding trick. The same Combine path is shared by batch, relay and locks, so the acceptance tests should cover all three routes.

---

Generated by Claude Code