feat(oci): manifest push endpoint (S12 Step 12)

Why

S12 Step 12 implements the OCI manifest push endpoint (PUT /v2/<slug>/container/<repo>/<image>/manifests/<reference>). Without it the dispatcher leaves manifest PUT on the interim 501 and the manifest-push conformance specs fail.

Per the OCI-hosted plan Step 12 and S12 Manifest Push: a 7-step flow validates an untrusted manifest payload (size, type detection, digest, references, subject and annotations) then persists it transactionally (6a payload write to CAS, 6b the database transaction), returning 201. This is the highest-risk step in the OCI surface: a security boundary (untrusted JSON parsing), data integrity (the persist transaction), and a complex state machine in one endpoint.

Review requirement (plan Step 12): two L3 reviewers, at least one with a parser-attack or security lens, distinct from the test- and implementation-authors.

Size. ~850 LOC of production code, over the 500 reviewable-LOC target; the flow, error accumulation, and persistence land together so the contract reviews end-to-end. The rest is the unit, property, fuzz, and integration suite.

What (non-obvious)

  • The flow (push_flow.go) is a pure state machine: it accumulates inputs, outputs, and errors but performs no I/O. Side effects (SHA-256, existence queries, storage writes) live in the handler, which calls the flow for transitions, so the flow is unit-testable without fixtures.
  • The 6b transaction lives in datastore.ContainerManifestPersister (mirrors Step 9's ContainerBlobLinker), keeping raw SQL in the datastore layer per ADR-023; the format package never imports database/sql.
  • Image Index size (own_payload + Σ child tree sizes) is derived from the same batched ChildManifestsExist query that proves existence: one container_manifests query per push regardless of child count (TestImageIndexSizeComputationConstantQueries guards it). Per-descriptor existence is one WHERE digest = ANY(...) per target table, never per-row.
  • The manifest cap (25,000) is race-bounded at N-1 overshoot under N concurrent pushes (closing it would need an image-row lock on the push hot path); the tag cap is race-free via SELECT ... FOR UPDATE on the manifest row.
  • Partial failure: if 6a (CAS write) succeeds and 6b fails, the staged payload is an orphan reclaimed by ADR-011, and the handler returns 500 with oci.manifest.partial_persist=true on the wide event. Cleanup is not retried inline.
  • Conformance stub FK fix (cross-workstream). S06 Step 6 added a validating FK from blob_storage_attachments to blob_storage_blobs. The conformance harness runs the in-memory storage stub over a real database, so the parent blob_storage_blobs row lived only in memory while the format-side attachment inserts (blob upload and manifest push) hit real Postgres, failing the FK and returning 500 on every finalize. That capped main at 19/80 and is not introduced by this MR. The development_stubs wiring now mirrors each committed row into the database through a new datastore.BlobStorageBlobStore (plus a stub WithBlobRowPersister hook), transitional until the real S06 BlobStore writes the row itself at Step 18, where BlobStorageBlobStore.Create is the writer it reuses.

Test plan

  • go test -race ./internal/format/oci/... ./internal/datastore/... ./cmd/artifact-registry/... and the -tags=integration suite pass locally.
  • A rapid state machine drives the 7-step flow; a rapid property pins the tree-size invariant; FuzzManifestParse fuzzes the parser (now wired into the fuzz:oci job).
  • Conformance: conformance:oci rises from 19/80 to 47/80 (job 14882476049), past the 41/80 pre-regression baseline. The manifest-push specs turn green, and the stub FK fix (under "What") restores the blob-upload specs a main-wide regression had cascaded to 500. The 28 remaining failures are all 501 for not-yet-built endpoints (manifest GET/HEAD/DELETE, content discovery), and the job stays allow_failure until Step 18.

Spec coverage

Spec: docs/specs/S12-container-oci-hosted.md

# Item Tests
AC-6 Manifest push (OCI Image Manifest by tag) TestManifestPush_ByTag
AC-7 Manifest push (OCI Image Index) TestManifestPush_ImageIndex, TestImageIndexSizeComputationConstantQueries
AC-8 Manifest push (with subject, subject exists) TestManifestPush_SubjectExists
AC-9 Manifest push (with subject, subject missing) TestManifestPush_SubjectMissing
AC-10 Manifest push (Docker Schema 2) TestManifestPush_DockerSchema2
AC-11 Manifest push (Schema 1 rejection) TestManifestPush_Errors, TestDetectManifestType
AC-12 Manifest push (missing blob) TestManifestPush_Errors, TestManifestPush_MissingConfigAndLayer
AC-13 Manifest push (size limit, 413) TestManifestPush_Errors
AC-14 Manifest push (error accumulation) TestPushFlow_ErrorAccumulation, TestManifestPush_MissingConfigAndLayer, TestManifestPush_MalformedBodyNoPhantomReferenceErrors, TestStateMachinePushFlow
AC-30 Manifest push (by digest only) TestManifestPush_ByDigestOnly
P-4 Push idempotency TestManifestPush_Idempotent, TestPropertyPushIdempotency
E-5 MANIFEST_BLOB_UNKNOWN (400) TestManifestPush_MissingConfigAndLayer, TestManifestPush_Errors
E-6 MANIFEST_INVALID (400) TestManifestPush_Errors, TestDetectManifestType_UnsupportedSchema
E-7 MANIFEST_INVALID (413) TestManifestPush_Errors
E-13 MANIFEST_LIMIT_EXCEEDED (400) TestManifestPush_ManifestLimitExceeded, TestManifestPushManifestLimitExceededIntegration, TestManifestPushConcurrentOvershootBounded
E-14 TAG_LIMIT_EXCEEDED (400) TestManifestPushTagLimitExceeded, TestManifestPushTagLimitExceededIntegration
S-6 Manifest payload 4 MB, JSON-only TestManifestPush_Errors, FuzzManifestParse
S-9 Manifest size limit, reject before parse TestManifestPush_Errors
S-10 Max references per manifest (200) TestManifestPush_ReferenceCountOverCapSkipsStep4, TestPushFlow_ReferenceCountExceededGatesStep4
S-13 Digest verification on writes TestPushFlow_CanEnter_DigestVerifyAlwaysRuns, TestManifestPush_ByDigestOnly
Context for reviewers and agents

The fix(oci): address Step 12 manifest-push validation findings commit applies pre-push /validate-step findings: it enforces schemaVersion == 2 on the header- and mediaType-driven detection paths (the gate ran only on the structural fallback, so a valid Content-Type accepted any schemaVersion), rejects a subject descriptor that omits size (was indistinguishable from size: 0 because the parsed size was a plain int64), and threads the observed count into the MANIFEST_LIMIT_EXCEEDED / TAG_LIMIT_EXCEEDED detail (store.go had dropped it for a bare sentinel). It also adds the plan's named TestImageIndexSizeComputationConstantQueries guard and wires FuzzManifestParse into CI.

Non-goals (deferred, not omissions):

  • Per-repository counter accounting (repositories.size_bytes, last_updated_at) — deferred to S22.
  • GC coordination and manifest-push serialization locks — deferred to S20; the 25,000 manifest cap is intentionally race-bounded at N-1 overshoot.
  • Soft-delete — S12 is hard-delete-only per the plan's Decisions; the container_* tables have no soft_deleted_at.
  • Inline orphan cleanup on a 6a/6b partial failure — the staged payload is reclaimed by ADR-011 reconciliation, surfaced via oci.manifest.partial_persist.

ADR-004 publishes a 250 KB manifest cap; the merged S12 spec sets 4 MB (OCI Distribution SHOULD-level). The code matches the spec; reconciling ADR-004 is a separate handbook-repo change.

Related to #19 (closed)

Edited by Hayley Swimelar

Merge request reports

Loading
Loading