Cached lock listings never send the client's refspec upstream, so a snapshot keyed by ref holds (and authorizes against) the no-ref answer

[matt-edmondson]

What's wrong

The lock snapshot is keyed by (upstream, repositoryPath, ref) (LockSnapshotKey), and LockRouteHandler.ListAsync fills Ref from the client's ?refspec=. The design doc (docs/superpowers/specs/2026-08-19-locks-subsystem-design.md, "The snapshot") gives the reason:

ref is part of the key because the specification states the locking API's ref property is for authentication only, which means two callers presenting different refs may legitimately receive different answers and must not share an entry.

The requests that actually fill that entry never send the ref, though. UpstreamRequests.BuildLockListRequest(upstreamBase, repositoryPath, cursor, limit, authorization) has no refspec parameter, and neither caller passes key.Ref:

• LockListRefresher.RefreshAsync, the page walk that builds the snapshot
• LockListRefresher.ProbeAsync, the one-page credential check used to admit a caller to an existing snapshot

So the entry stored under refspec=refs/heads/feature holds upstream's answer to an unscoped GET .../locks, and the credential check behind it is also unscoped. Keying by ref adds nothing here: every ref gets the same content, fetched separately.

Batched lock/unlock is handled correctly. LockFanOut does forward request.Ref in the body it sends upstream.

Why it matters

• Answers depend on cache state. With Locks:Enabled=false, or whenever the handler falls back to relaying, GET .../locks?refspec=X goes upstream verbatim with the refspec (UpstreamRelay forwards QueryString). With the cache on, the same request gets the unscoped answer. A forge that scopes listing by ref returns different lock sets for the same client request depending on whether a snapshot happened to be warm.
• Authorization isn't checked against the ref the client presented. The design doc treats ref as an authentication input, and ProbeAsync exists so that upstream stays the authority on whether this client may read these locks. Because the probe omits the ref, a forge that would refuse a credential for refs/heads/feature but accept it unscoped will let that caller through, and it will be served the cached listing.

Against a forge that ignores refspec (probably GitHub), the only effect is duplicate snapshots per ref. Against one that honours it, the cache gives wrong answers.

Suggested fix

• Add a string? refspec parameter to BuildLockListRequest and append refspec=<escaped> when it's non-null.
• Pass key.Ref from both RefreshAsync and ProbeAsync.
• Check whether CredentialAdmission's key should include the ref too, so a credential admitted for one ref isn't treated as admitted for another.

Acceptance: an integration test with StubUpstream that records the query string shows that a cached GET /locks?refspec=refs/heads/main produces upstream list and probe requests that carry refspec=refs%2Fheads%2Fmain, and that a request with no refspec produces upstream requests without one.

Related but separate: #46 covers invalidation after create/unlock using the wrong key. This issue is about what the snapshot is filled with.

[matt-edmondson]

Triage

• Category: Bug
• Priority: Medium. The cache keys lock snapshots by ref, but it fills them and checks credentials with unscoped upstream requests. As a result, answers depend on whether the cache is warm, and the ref-scoped authorization check is skipped. The practical impact depends on the forge. GitHub most likely ignores refspec, in which case the only cost is duplicate snapshots. A forge that honours refspec gets wrong listings and a weaker authorization check. It would move to High if a supported deployment targets such a forge.
• Area / suggested owner: The locks subsystem: UpstreamRequests.BuildLockListRequest, LockListRefresher.RefreshAsync/ProbeAsync, and possibly the key for CredentialAdmission.
• Duplicates / related: Not a duplicate. It is closely related to Creating or unlocking a lock does not invalidate the cached lock list when the list was fetched with ?refspec= #46, a lock-list invalidation bug in the same area involving the ?refspec= key. The two would fit well in one PR.
• In progress: No open PR covers this.
• Next step: Add the StubUpstream query-string acceptance test described in the issue, thread refspec through BuildLockListRequest, and decide whether CredentialAdmission should key on the ref.

---

Generated by Claude Code