diff --git a/lexicons/io/atcr/hold/notifyManifest.json b/lexicons/io/atcr/hold/notifyManifest.json index 9d58ee6..2d54d3d 100644 --- a/lexicons/io/atcr/hold/notifyManifest.json +++ b/lexicons/io/atcr/hold/notifyManifest.json @@ -113,6 +113,11 @@ "type": "ref", "ref": "#childManifestInfo" } + }, + "subject": { + "type": "ref", + "ref": "#blobInfo", + "description": "Manifest this artifact refers to (attestations, signatures, SBOMs). Absent for ordinary images." } } }, @@ -125,6 +130,10 @@ }, "size": { "type": "integer" + }, + "mediaType": { + "type": "string", + "maxLength": 256 } } }, diff --git a/pkg/appview/storage/manifest_store.go b/pkg/appview/storage/manifest_store.go index 3507a08..e4ca525 100644 --- a/pkg/appview/storage/manifest_store.go +++ b/pkg/appview/storage/manifest_store.go @@ -678,6 +678,17 @@ func (s *ManifestStore) notifyHoldAboutManifest(ctx context.Context, manifestRec manifestData["manifests"] = manifests } + // Add subject if present. An OCI referrer artifact (attestation, + // signature, SBOM) points at the manifest it describes; the hold uses + // this to tell those apart from real images and skip scanning them. + if manifestRecord.Subject != nil { + manifestData["subject"] = map[string]any{ + "digest": manifestRecord.Subject.Digest, + "size": manifestRecord.Subject.Size, + "mediaType": manifestRecord.Subject.MediaType, + } + } + notifyReq["manifest"] = manifestData } diff --git a/pkg/appview/storage/notify_subject_test.go b/pkg/appview/storage/notify_subject_test.go new file mode 100644 index 0000000..3c6d86b --- /dev/null +++ b/pkg/appview/storage/notify_subject_test.go @@ -0,0 +1,127 @@ +package storage + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + + "atcr.io/pkg/atproto" +) + +// captureNotify stands in for the hold's notifyManifest endpoint and returns the +// decoded request body the AppView sent. +func captureNotify(t *testing.T, record *atproto.ManifestRecord) map[string]any { + t.Helper() + + var captured map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + t.Errorf("read body: %v", err) + } + if err := json.Unmarshal(body, &captured); err != nil { + t.Errorf("unmarshal body: %v", err) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true,"operation":"push","statsUpdated":true}`)) + })) + defer srv.Close() + + store := NewManifestStore(&RegistryContext{ + Repository: "myapp", + DID: "did:plc:alice123", + Handle: "alice.test", + HoldURL: srv.URL, + ServiceToken: "service-token", + }, nil) + + if err := store.notifyHoldAboutManifest(context.Background(), record, "latest", "sha256:deadbeef", "push"); err != nil { + t.Fatalf("notifyHoldAboutManifest: %v", err) + } + if captured == nil { + t.Fatal("hold received no notification") + } + return captured +} + +func manifestInfoFrom(t *testing.T, payload map[string]any) map[string]any { + t.Helper() + info, ok := payload["manifest"].(map[string]any) + if !ok { + t.Fatalf("payload has no manifest object: %#v", payload) + } + return info +} + +// TestNotifyHold_IncludesSubjectForReferrer verifies the notify payload carries +// the subject descriptor when the pushed manifest is a referrer artifact. Without +// it the hold's attestation guard (which tests for a nil subject) can never fire. +func TestNotifyHold_IncludesSubjectForReferrer(t *testing.T) { + // An in-toto attestation as buildx pushes one: an ordinary image config, + // non-tar layers, and a subject pointing at the image it describes. + ociManifest := []byte(`{ + "schemaVersion": 2, + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "config": {"mediaType":"application/vnd.oci.image.config.v1+json","digest":"sha256:cfg","size":233}, + "layers": [ + {"mediaType":"application/vnd.in-toto+json","digest":"sha256:att","size":1024} + ], + "subject": {"mediaType":"application/vnd.oci.image.manifest.v1+json","digest":"sha256:subj","size":528} + }`) + + record, err := atproto.NewManifestRecord("myapp", "sha256:deadbeef", ociManifest) + if err != nil { + t.Fatalf("NewManifestRecord: %v", err) + } + if record.Subject == nil { + t.Fatal("AppView did not parse subject off the pushed manifest") + } + + info := manifestInfoFrom(t, captureNotify(t, record)) + + subject, ok := info["subject"].(map[string]any) + if !ok { + t.Fatalf("notify payload omitted subject: %#v", info) + } + if subject["digest"] != "sha256:subj" { + t.Errorf("subject digest = %v, want sha256:subj", subject["digest"]) + } + if subject["mediaType"] != "application/vnd.oci.image.manifest.v1+json" { + t.Errorf("subject mediaType = %v", subject["mediaType"]) + } + if size, ok := subject["size"].(float64); !ok || int64(size) != 528 { + t.Errorf("subject size = %v, want 528", subject["size"]) + } +} + +// TestNotifyHold_OmitsSubjectForPlainImage verifies an ordinary image still +// sends no subject, so the hold keeps enqueueing real images for scanning. +func TestNotifyHold_OmitsSubjectForPlainImage(t *testing.T) { + ociManifest := []byte(`{ + "schemaVersion": 2, + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "config": {"mediaType":"application/vnd.oci.image.config.v1+json","digest":"sha256:cfg","size":233}, + "layers": [ + {"mediaType":"application/vnd.oci.image.layer.v1.tar+gzip","digest":"sha256:l1","size":4096} + ] + }`) + + record, err := atproto.NewManifestRecord("myapp", "sha256:deadbeef", ociManifest) + if err != nil { + t.Fatalf("NewManifestRecord: %v", err) + } + + info := manifestInfoFrom(t, captureNotify(t, record)) + + if _, present := info["subject"]; present { + t.Errorf("notify payload carried a subject for a plain image: %#v", info["subject"]) + } + // The rest of the payload is unchanged. + layers, ok := info["layers"].([]any) + if !ok || len(layers) != 1 { + t.Errorf("layers = %#v, want one layer", info["layers"]) + } +} diff --git a/pkg/hold/oci/notify_scan_guard_test.go b/pkg/hold/oci/notify_scan_guard_test.go new file mode 100644 index 0000000..b143621 --- /dev/null +++ b/pkg/hold/oci/notify_scan_guard_test.go @@ -0,0 +1,86 @@ +package oci + +import ( + "encoding/json" + "testing" +) + +// TestNotifyManifestInfo_ScanGuard checks the decision at the scan-enqueue guard +// in HandleNotifyManifest, driven off the JSON the AppView actually sends. +func TestNotifyManifestInfo_ScanGuard(t *testing.T) { + tests := []struct { + name string + manifestJSON string + wantScannable bool + }{ + { + name: "plain image is enqueued", + manifestJSON: `{ + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "config": {"digest":"sha256:cfg","size":233,"mediaType":"application/vnd.oci.image.config.v1+json"}, + "layers": [{"digest":"sha256:l1","size":4096,"mediaType":"application/vnd.oci.image.layer.v1.tar+gzip"}] + }`, + wantScannable: true, + }, + { + name: "attestation with subject is skipped", + manifestJSON: `{ + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "config": {"digest":"sha256:cfg","size":233,"mediaType":"application/vnd.oci.image.config.v1+json"}, + "layers": [{"digest":"sha256:att","size":1024,"mediaType":"application/vnd.in-toto+json"}], + "subject": {"digest":"sha256:subj","size":528,"mediaType":"application/vnd.oci.image.manifest.v1+json"} + }`, + wantScannable: false, + }, + { + name: "manifest list is skipped", + manifestJSON: `{ + "mediaType": "application/vnd.oci.image.index.v1+json", + "manifests": [{"digest":"sha256:m1","size":528,"mediaType":"application/vnd.oci.image.manifest.v1+json","platform":{"os":"linux","architecture":"amd64"}}] + }`, + wantScannable: false, + }, + { + // An AppView older than the subject field sends nothing, and the + // hold must behave exactly as it did before: enqueue. + name: "payload from an older AppView still enqueues", + manifestJSON: `{ + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "config": {"digest":"sha256:cfg","size":233}, + "layers": [{"digest":"sha256:l1","size":4096,"mediaType":"application/vnd.oci.image.layer.v1.tar+gzip"}] + }`, + wantScannable: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var info notifyManifestInfo + if err := json.Unmarshal([]byte(tt.manifestJSON), &info); err != nil { + t.Fatalf("unmarshal manifest info: %v", err) + } + if got := info.HasScannableContent(); got != tt.wantScannable { + t.Errorf("HasScannableContent() = %v, want %v", got, tt.wantScannable) + } + }) + } +} + +// TestNotifyManifestInfo_SubjectDecoded confirms the subject descriptor survives +// the wire, not just its presence. +func TestNotifyManifestInfo_SubjectDecoded(t *testing.T) { + var info notifyManifestInfo + body := `{"subject":{"digest":"sha256:subj","size":528,"mediaType":"application/vnd.oci.image.manifest.v1+json"}}` + if err := json.Unmarshal([]byte(body), &info); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if !info.IsReferrer() { + t.Fatal("IsReferrer() = false, want true") + } + if info.Subject.Digest != "sha256:subj" || info.Subject.Size != 528 { + t.Errorf("subject = %+v", info.Subject) + } + if info.Subject.MediaType != "application/vnd.oci.image.manifest.v1+json" { + t.Errorf("subject mediaType = %q", info.Subject.MediaType) + } +} diff --git a/pkg/hold/oci/xrpc.go b/pkg/hold/oci/xrpc.go index f4056fb..b01b36a 100644 --- a/pkg/hold/oci/xrpc.go +++ b/pkg/hold/oci/xrpc.go @@ -186,6 +186,59 @@ func (h *XRPCHandler) HandleAbortUpload(w http.ResponseWriter, r *http.Request) }) } +// notifyManifestInfo mirrors io.atcr.hold.notifyManifest#manifestInfo: the +// slice of the pushed OCI manifest the AppView forwards to the hold. +type notifyManifestInfo struct { + MediaType string `json:"mediaType"` + Config struct { + Digest string `json:"digest"` + Size int64 `json:"size"` + MediaType string `json:"mediaType"` + } `json:"config"` + Layers []struct { + Digest string `json:"digest"` + Size int64 `json:"size"` + MediaType string `json:"mediaType"` + } `json:"layers"` + Manifests []struct { + Digest string `json:"digest"` + Size int64 `json:"size"` + MediaType string `json:"mediaType"` + Platform *struct { + OS string `json:"os"` + Architecture string `json:"architecture"` + } `json:"platform"` + } `json:"manifests"` + // Subject is the descriptor an OCI referrer artifact points at. Only an + // AppView new enough to send it populates this; an older one leaves it nil, + // which reads as "ordinary image" exactly as it did before the field existed. + Subject *struct { + Digest string `json:"digest"` + Size int64 `json:"size"` + MediaType string `json:"mediaType"` + } `json:"subject"` +} + +// IsMultiArch reports whether this is a manifest list / image index, which has +// child manifests instead of layers of its own. +func (m *notifyManifestInfo) IsMultiArch() bool { + return len(m.Manifests) > 0 +} + +// IsReferrer reports whether this manifest is an OCI referrer artifact +// (attestation, signature, SBOM) attached to another manifest via subject, +// rather than an image in its own right. +func (m *notifyManifestInfo) IsReferrer() bool { + return m.Subject != nil +} + +// HasScannableContent reports whether a vulnerability scan of this manifest +// could find anything. Manifest lists carry no layers, and referrer artifacts +// carry metadata rather than a filesystem, so neither is worth enqueueing. +func (m *notifyManifestInfo) HasScannableContent() bool { + return !m.IsMultiArch() && !m.IsReferrer() +} + // HandleNotifyManifest handles manifest notifications from AppView // For pushes: Creates layer records and optionally posts to Bluesky // For pulls: Just increments stats (no layer records or posts) @@ -211,38 +264,12 @@ func (h *XRPCHandler) HandleNotifyManifest(w http.ResponseWriter, r *http.Reques // Parse request var req struct { - Repository string `json:"repository"` - Tag string `json:"tag"` - UserDID string `json:"userDid"` - ManifestDigest string `json:"manifestDigest"` // For building layer record AT-URIs - Operation string `json:"operation"` // "push" or "pull", defaults to "push" for backward compatibility - Manifest struct { - MediaType string `json:"mediaType"` - Config struct { - Digest string `json:"digest"` - Size int64 `json:"size"` - MediaType string `json:"mediaType"` - } `json:"config"` - Layers []struct { - Digest string `json:"digest"` - Size int64 `json:"size"` - MediaType string `json:"mediaType"` - } `json:"layers"` - Manifests []struct { - Digest string `json:"digest"` - Size int64 `json:"size"` - MediaType string `json:"mediaType"` - Platform *struct { - OS string `json:"os"` - Architecture string `json:"architecture"` - } `json:"platform"` - } `json:"manifests"` - Subject *struct { - Digest string `json:"digest"` - Size int64 `json:"size"` - MediaType string `json:"mediaType"` - } `json:"subject"` - } `json:"manifest"` + Repository string `json:"repository"` + Tag string `json:"tag"` + UserDID string `json:"userDid"` + ManifestDigest string `json:"manifestDigest"` // For building layer record AT-URIs + Operation string `json:"operation"` // "push" or "pull", defaults to "push" for backward compatibility + Manifest notifyManifestInfo `json:"manifest"` } if err := render.Decode(r, &req); err != nil { @@ -341,7 +368,7 @@ func (h *XRPCHandler) HandleNotifyManifest(w http.ResponseWriter, r *http.Reques } // Check if this is a multi-arch image (has manifests instead of layers) - isMultiArch := len(req.Manifest.Manifests) > 0 + isMultiArch := req.Manifest.IsMultiArch() // Calculate total size from all layers (for single-arch images) var totalSize int64 @@ -404,8 +431,9 @@ func (h *XRPCHandler) HandleNotifyManifest(w http.ResponseWriter, r *http.Reques } } - // Enqueue scan job if scanner is connected (skip manifest lists and attestations — no scannable content) - if h.scanBroadcaster != nil && !isMultiArch && req.Manifest.Subject == nil { + // Enqueue scan job if scanner is connected (skip manifest lists and + // referrer artifacts such as attestations — no scannable content). + if h.scanBroadcaster != nil && req.Manifest.HasScannableContent() { tier := "deckhand" if stats != nil && stats.Tier != "" { tier = stats.Tier