mirror of
https://tangled.org/evan.jarrett.net/at-container-registry
synced 2026-08-29 20:27:16 +00:00
Evan JarrettandClaude Opus 5
5aa13abdc2
a7569a7added credential-less pulls of public images. Three things about it were wrong, all of them in how the appview handled the decision that belongs to the hold. **Scope handling was all-or-nothing.** IsPullOnlyScope required every requested action to already be "pull", but clients routinely ask for more than the operation needs — pull,push is common for a plain read, and some ask for pull,push,delete up front. Those were rejected and challenged, leaving a credential-less client no way to pull even a public image, which is the entire feature. NarrowToPullOnly drops the write actions and issues a token carrying "pull" and nothing else. Granting a subset is what the distribution token spec expects. The allowlist property is preserved: "pull" is the only action that survives, and "*" is deliberately not expanded into it, since a wildcard request is not evidence the caller wants a read. **The appview-side read gate was inert.** checkReadAccess passed p.ctx.DID, the DID of the repository *owner*, not the requester. Any non-empty DID satisfies a private hold's check, and the owner's is never empty, so it asked "may the owner read their own hold", answered yes, and admitted everyone. Worse, CheckReadAccessWithCaptain admitted any authenticated DID to a private hold at all, on an explicitly-MVP assumption that holding a DID was close enough to being a sailor. Every doc says otherwise (docs/hold.md:109 "Crew with blob:read", CLAUDE.md:140, docs/BYOS.md:280) and so does the hold (ValidateBlobReadAccess: owner, or crew carrying blob:read/blob:write). It now takes isCrew and requires owner-or-crew, and callers only pay for the crew lookup when it can change the answer — a public hold or an anonymous caller is decided by the captain record alone. Nothing here loosens access; it brings the local gate into agreement with the authority. **Denials could not reach the client.** distribution's blobHandler.GetBlob maps everything except ErrBlobUnknown to ErrorCodeUnknown, so a 401 raised in the blob store left as a 500 — misreporting an auth failure as a server fault, and giving BearerChallenge no 401 to attach WWW-Authenticate to, so Docker was told "server error" instead of being prompted for credentials. Clients that retry 5xx looped: 4.1s per case in the matrix, now 0.01s. The check moves to Repository(), where an errcode.Error is passed through verbatim by the registry app — the same mechanisma7569a7used for NAME_UNKNOWN. It fails open on a lookup error, since the hold is the authority and a transient failure should not break public pulls. Removes auth.allow_anonymous_pull. It could only ever withhold — captain.Public is what grants — so it was a second flag for a decision the hold already owns, and gating it appview-side was never the intent. Layer bytes 307 straight to S3, so the appview is not even in the path whose cost might have justified an operator-side lever. Tests: TestAuthMatrix only ever ran against a public hold, and its pull cases never fetched a layer — crane.Pull is lazy and img.Digest() needs only the manifest, which ATCR serves from the user's PDS where it is world-readable, so no pull row in the matrix touched blob authorization at all. Pulls now materialize layer bytes, and testharness.WithPrivateHold plus TestAuthMatrixPrivateHold cover public:false + allow_all_crew:true — the production shape, where anyone with an account pulls and pushes and anonymous gets nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>