Lock admission probe admits any 2xx: a bogus credential answered with a 203 sign-in page (or a redirect to one) can read another user's cached lock listing #150

What's wrong

LockListRefresher.ProbeAsync decides admission from the status code alone (LockListRefresher.cs L60-L66):

return response.IsSuccessStatusCode
    ? LockRefreshResult.Succeeded(new LockSnapshot([], timeProvider.GetUtcNow()))
    : LockRefreshResult.Refused(response.StatusCode);

LockListService.ProbeAsync then admits the credential for AdmissionTtl and serves the current snapshot. That snapshot was built by a different caller's credential.

The full walk in RefreshAsync is stricter. If a 2xx body does not parse as a lock page, the walk returns Unusable and the request is relayed. So the probe, which stands in for the walk, applies a weaker standard than the walk does.

The README says "an admission only ever exists because a real upstream call succeeded", and ICredentialAdmission says the same. A sign-in page is not a successful lock listing.

Reproduction (run against c6923c8, .NET 10, ProxyFixture)

1. A valid credential (Basic dXNlcjp0b2tlbg==) calls GET /github/owner/repo.git/info/lfs/locks. That caches a snapshot containing Content/secret.uasset.
2. The stub upstream switches to answering GET .../locks with 203 Non-Authoritative Information and a body that is not a lock page.
3. A request with Authorization: Basic Ym9ndXM6Ym9ndXM= (bogus:bogus) gets:

   200 {"locks":[{"id":"0","path":"Content/secret.uasset","owner":{"name":"someone"}}]}
   

   The bogus credential is also admitted for AdmissionTtl.

There is a second route to the same outcome, which I traced but did not run (the stub cannot redirect). The upstream HttpClient is registered with the default primary handler (GitLfsCacheServiceCollectionExtensions.cs L116). That handler has AllowAutoRedirect = true and drops Authorization on the redirect hop. So if the upstream redirects a bad credential to an HTML login page, ProbeAsync ends up looking at the login page's 200 and admits the credential.

Why it matters

Behind a forge or SSO front end that answers bad credentials with a 203 sign-in page or a redirect to a login page, any non-empty Authorization header gets admitted. Azure DevOps is known to return 203 sign-in pages for unauthenticated API calls. The caller then sees:

• lock paths
• lock ids, which are enough to target a force-unlock attempt
• owner names

All of it comes from snapshots that other users populated. #81 handled the same SSO-page shape for batch calls. This is the lock-admission counterpart.

GitHub, GitLab and Gitea answer bad credentials with 401, so they are unaffected.

Suggested fix

• In ProbeAsync, admit only when the status is exactly 200 and the body parses with LockListParser.TryParsePage, the check RefreshAsync already uses.
  • Treat any other 2xx, or an unparseable body, as Unusable, which relays the request. Do not treat it as success.
  • Keep 401, 403 and 404 as Refused.
• Disable automatic redirects on the upstream client, at least for lock calls (ConfigurePrimaryHttpMessageHandler(() => new SocketsHttpHandler { AllowAutoRedirect = false })). A redirect then surfaces as a 3xx, which is relayed or refused, instead of the login page's 200.
• Update the "body is irrelevant" comment at L50-L51 to describe the new check.

Acceptance criteria

• A probe answered with 203, or with a 200 that is not a lock page, does not admit the credential and serves no snapshot.
• A test covers the 203 case above, and a test covers a 302 to a 200 HTML page.
• Existing admission tests still pass.