feat(oci): blob pull GET, HEAD, Range with per-image scoping (S12 Step 10)
Why
Step 10 of the OCI local plan: the blob-pull surface (GET/HEAD /v2/<slug>/container/<repo>/<image>/blobs/<digest>, RFC 9110 Range). Step 9 pushes blobs; without this nothing is pullable.
Per S12 Blob Pull, the read is scoped to the requested image: resolve the full <repository>/<image> name and serve a blob only if container_blobs links that digest to that image. The first cut of the handler resolved the repository only and served from namespace-scoped CAS, which serves any blob in the namespace under any resolvable image (cross-image disclosure) and returns BLOB_UNKNOWN instead of image-tier NAME_UNKNOWN for an unknown image. This MR closes that.
What's notable
- Per-image scoping is the security boundary.
scopeToImageresolves the image (ContainerImageStore.FindByName, new, find-only, reuses the existing triple-SELECT) then gates on thecontainer_blobslink (FindByDigest). Image miss returns 404NAME_UNKNOWN, a digest not linked to the image returns 404BLOB_UNKNOWN, a store error returns 500. The CAS byte read runs only after the link check passes; the Range and streaming path is unchanged. - Range parser is self-contained and bounded.
parseRangehandles satisfiable, open-ended, suffix, end-clamp, and unsatisfiable (416) forms. Streaming peak allocation is independent of blob size. Multi-range and malformed headers fall back to a full 200 representation per the RFC's ignore allowance. - 416 reuses
CodeBlobUploadInvalidon the download path. S12's Blob Pull error table and OCI both define no 416 code, and the container-registry reference emits a bare 416. Flagged for the spec author; not a blocker. - Test-first, two commits. Scoping tests (
29bbbcd) precede the implementation (516818d) per the OCI two-agent flow. The test commit used--no-verifyfor its panic-skeleton, the one documented test-first exception.
Test plan
go test -race ./internal/format/oci/... ./internal/datastore/...(integration tests need Postgres).- OCI conformance runs in the
conformance:ociCI job.
Spec coverage
Spec: S12 Blob Pull (lines 476-514). Plan: Step 10.
Acceptance criteria
| # | Criterion | Tests |
|---|---|---|
| AC-21 | Zero-byte blob round-trip | TestBlobGet_ZeroByte, TestBlobHeadGetParity/zero-byte_blob |
| AC-24 | Blob HEAD (full header set, no body) | TestBlobHead, TestBlobHeadGetParity, TestBlob_PerImageHappyPath (HEAD) |
| AC-25 | Blob GET / Range | TestBlobGet_RangeTable, TestBlobGet_FullContentNoRange, TestBlobGet_RangeKeepsDigestHeader, TestBlobGet_RedirectReturns307, TestBlobGetRangeBoundedAllocation, TestBlob_PerImageHappyPath (GET), FuzzRangeHeader |
Error cases
| # | Code | HTTP | Condition | Tests |
|---|---|---|---|---|
| E-1 | BLOB_UNKNOWN |
404 | Blob not linked to the resolved image | TestBlob_BlobUnknownTable, TestBlob_CrossImageIsolation (GET + HEAD) |
| E-4 | DIGEST_INVALID |
400 | Malformed digest, before scoping | TestBlob_DigestInvalidTable, TestBlob_DigestInvalidBeforeScoping |
| E-10 | NAME_UNKNOWN |
404 | Image does not exist (image-tier resolution) | TestBlob_ImageUnknownTable (GET + HEAD), TestContainerImageStore_FindByName |
| E-18 | INTERNAL |
500 | Unexpected storage/datastore failure | TestBlob_StorageErrorReturns500, TestBlob_ScopingStoreErrorReturns500 |
Security
| # | Concern | Tests |
|---|---|---|
| S-3 | Digest format, lowercase canonical | TestBlob_DigestInvalidTable, TestBlob_DigestInvalidBeforeScoping |
| S-7 | Range request parsing / Content-Range emission (RFC 9110) | TestBlobGet_RangeTable, FuzzRangeHeader |
| (cross-image isolation) | A blob linked to image B is not served under image A | TestBlob_CrossImageIsolation, TestContainerImageStore_FindByName (isolation subtests) |
FindByName argument guards: TestContainerImageStore_FindByName_ArgumentGuards (nil ctx, zero UUID).
Context for LLM reviewers
Design. Per-image scoping mirrors the upload path's seam: BlobHandler takes separate imageFinder (FindByName) and containerBlobFinder (FindByDigest) consumer-side interfaces rather than folding image resolution into the Resolver interface or namespace.Resolution. FindByName reuses the existing findContainerImageByTriple SELECT builder (no third near-duplicate), mirrors FindByID's read shape (s.client.DB(), qrm.ErrNoRows wrapped as ErrNotFound), and keeps the nil-ctx and zero-namespace guards.
Rejected alternatives.
- Folding image resolution into
namespace.Resolution/ theResolverinterface: mixes the namespace package's concerns with container-image lookup. The upload path already keepsImageUpserterseparate fromResolver; this matches it. - Using
UpsertContainerImageon the read path: it lazy-creates on miss, which is wrong for a read (a GET must not create an image row).
Non-goals.
- Per-image authorization (who may read an image) is S08/ADR-020; the bootstrap token grants all scopes today. This MR enforces resource scoping (does the digest belong to this image), which the spec mandates independent of auth.
- The dispatcher-resolves-then-handler-re-resolves double repository resolution (~2 extra round-trips per pull) is pre-existing Step 8 architecture, consistent across the upload and blob handlers; not touched here.
- The 416 error code is a spec-author call (no code defined in S12 or OCI).
- The conformance status flip to
Implementedis Step 18.