diff --git a/docs/ARTIFACT_TYPES.md b/docs/ARTIFACT_TYPES.md new file mode 100644 index 0000000..b1ffeee --- /dev/null +++ b/docs/ARTIFACT_TYPES.md @@ -0,0 +1,882 @@ +# Artifact Type Classification + +> Research and design. Nothing in the "Proposed" sections is implemented. + +ATCR classifies every pushed manifest into one of three artifact types +(`container-image`, `helm-chart`, `unknown`) using the config blob's media type +alone. Anything that is not a Helm chart and not a recognised image config +becomes `unknown`, and because no template has an `unknown` branch, the UI +renders it as a container image. This document inventories what the code does +today, what the OCI 1.1 spec actually requires, and proposes a classification +model, a UI contract for non-image artifacts, and a shared definition of +scan eligibility. + +Sections marked **Current** describe behaviour verified in the tree at commit +`a63b839`. Sections marked **Proposed** are design, not description. + +## Contents + +- [Problem statement](#problem-statement) +- [Current state](#current-state) +- [What the OCI spec says](#what-the-oci-spec-says) +- [What this deployment actually sees](#what-this-deployment-actually-sees) +- [Proposed classification model](#proposed-classification-model) +- [Proposed UI contract](#proposed-ui-contract) +- [Proposed scan eligibility](#proposed-scan-eligibility) +- [Migration and compatibility](#migration-and-compatibility) +- [Staged implementation plan](#staged-implementation-plan) +- [Open questions](#open-questions) + +## Problem statement + +Push an OCI artifact whose config media type is +`application/vnd.example.thing.config.v1+json` (test repo `oddart`). The +classifier returns `unknown`, which is correct. The UI then renders it as a +container image, which is not. + +Concretely, the digest page and the repository page for that artifact show: + +- A `docker pull` command with a client switcher (docker / podman / nerdctl / + buildah / crane), because `pull-command-switcher.html` only special-cases + `helm-chart` and falls through to the image branch for everything else. +- Layers / Vulnerabilities / SBOM tabs, because `repo-tag-section.html` only + swaps those for a single Chart tab when the type is `helm-chart`. +- A layers table captioned "Image layer history" with per-row Dockerfile + commands, and "no command recorded" in italics for every row, because the + hold has no OCI image config to supply `history` entries. +- The promise "Scans run automatically shortly after a push. Check back in a + few minutes, or push a new tag to trigger a scan." + (`pkg/appview/templates/partials/vulns-section.html:26` and + `pkg/appview/templates/partials/vuln-details.html:5`). For an artifact with + no scannable layers this is never going to come true, so the page tells the + user to wait for something that will not happen. + +A second, quieter symptom: an **untagged** `unknown` manifest is invisible on +the repository page. `GetTopLevelManifests` filters +`AND m.artifact_type != 'unknown'` (`pkg/appview/db/queries.go:1351`), so the +artifact only appears at all if a tag points at it. The two paths disagree: +tagged unknowns are rendered as images, untagged unknowns are dropped. + +The root cause is that OCI 1.1's manifest-level `artifactType` field, which +exists precisely to answer "what is this thing", is never read, never stored, +and never reaches the UI. + +## Current state + +Everything in this section was read in the tree, and each claim carries the +file and line it came from. + +### Classification is one switch on `config.mediaType` + +`pkg/appview/db/queries.go:50-71`: + +```go +const ( + ArtifactTypeContainerImage = "container-image" + ArtifactTypeHelmChart = "helm-chart" + ArtifactTypeUnknown = "unknown" +) + +func GetArtifactType(configMediaType string) string { + switch { + case strings.Contains(configMediaType, "helm.config"): + return ArtifactTypeHelmChart + case strings.Contains(configMediaType, "oci.image.config") || + strings.Contains(configMediaType, "docker.container.image"): + return ArtifactTypeContainerImage + case configMediaType == "": + // Manifest lists don't have a config - treat as container-image + return ArtifactTypeContainerImage + default: + return ArtifactTypeUnknown + } +} +``` + +Three properties worth naming: + +1. The function takes only the config media type. It cannot see + `artifactType`, the layer media types, the manifest media type, or whether + a `subject` is present. +2. Matching is substring, not equality. `helm.config` matches + `application/vnd.cncf.helm.config.v1+json`, and also anything else that + happens to contain that substring. +3. The empty-config case returns `container-image`. That is meant for manifest + lists, but the OCI 1.1 empty descriptor + (`application/vnd.oci.empty.v1+json`) is not the empty string, so an + empty-config artifact falls to `default` and becomes `unknown`. + +### `artifactType` is not persisted anywhere + +This is the headline fact. + +- `ManifestRecord` (`pkg/atproto/lexicon.go:78-128`) has no `ArtifactType` + field. It carries `MediaType`, `Config`, `Layers`, `Manifests`, + `Annotations`, `Subject`, `ManifestBlob`. +- `NewManifestRecord` (`pkg/atproto/lexicon.go:186-266`) unmarshals the pushed + OCI manifest into an anonymous struct with fields `schemaVersion`, + `mediaType`, `config`, `layers`, `manifests`, `subject`, `annotations`. The + manifest's `artifactType` is simply not in the struct, so it is discarded at + parse time. +- `lexicons/io/atcr/manifest.json` has no `artifactType` property. +- `pkg/appview/db/schema.sql:37` stores the derived string, not the OCI field: + `artifact_type TEXT NOT NULL DEFAULT 'container-image'`. + +The raw manifest bytes do survive: `ManifestStore.Put` +(`pkg/appview/storage/manifest_store.go:190-222`) uploads the full payload as +an ATProto blob and sets `manifestRecord.ManifestBlob`. So the original +`artifactType` is recoverable by fetching that blob and re-parsing, but nothing +does, and no index exists over it. + +There is also no OCI referrers endpoint. `github.com/distribution/distribution/v3 +v3.1.1` contains no referrers code at all (`RouteName*` in +`registry/api/v2/routes.go` is base / manifest / tags / blob / blob-upload / +blob-upload-chunk / catalog, and a case-insensitive grep for `referrer` across +the module returns nothing), and ATCR registers no `/v2/.../referrers/` route +of its own. `docs/REMOVING_DISTRIBUTION.md:164` describes the referrers +endpoint as something a hand-rolled OCI implementation would need to provide, +not as something ATCR provides. What does exist is `subject` **persistence** +plus an attestation UI, described next. + +### `subject` is persisted, but only on the jetstream path + +- `manifests.subject_digest TEXT` (`pkg/appview/db/schema.sql:38`), indexed at + line 47. +- Populated from the record's `subject.digest` in + `pkg/appview/jetstream/processor.go:409-411` and + `pkg/appview/jetstream/backfill_batch.go:74-76`. +- Used to hide referring manifests from listings + (`queries.go:1350` and `queries.go:1788`, both `AND m.subject_digest IS NULL`) + and to cascade deletes (`queries.go:1119`, `queries.go:1122`). + +Separately, buildx attestations that arrive as **children of an index** are +detected by annotation, not by `subject`: `isAttestation` is true when a +manifest reference carries `vnd.docker.reference.type == "attestation-manifest"` +(`pkg/appview/jetstream/processor.go:459-464`, +`pkg/appview/db/models.go:54`). That flag drives the "Attested" badge and the +attestation detail modal (`pkg/appview/handlers/attestation_details.go`). + +**A gap worth flagging:** the appview never sends `subject` to the hold. +`notifyHoldAboutManifest` (`pkg/appview/storage/manifest_store.go:610-681`) +builds `manifestData` with `mediaType`, `config`, `layers` and `manifests` and +nothing else, and `lexicons/io/atcr/hold/notifyManifest.json`'s `#manifestInfo` +defines only those four properties. But the hold's handler gates scan-on-push +on `req.Manifest.Subject == nil` (`pkg/hold/oci/xrpc.go:408`), reading a field +that is never populated over the wire. The "skip attestations" half of that +condition is therefore dead: every referring artifact is enqueued for scanning +on push. Verified by reading both sides; not observed in production logs. + +### How the derived type reaches the UI + +The appview's `manifests` table is fed from the firehose, not from the push +path. Both feeders call the same classifier: + +| Site | Line | Behaviour | +|---|---|---| +| `pkg/appview/jetstream/processor.go` | 384-387 | `artifactType := "container-image"`, overridden by `db.GetArtifactType(...)` only when the record is not an index and `Config != nil` | +| `pkg/appview/jetstream/backfill_batch.go` | 54-57 | identical logic in the batch backfill path | + +Note that "is this an index" is `len(mr.Manifests) > 0`, not the media type. A +non-index manifest with a nil config also lands on `container-image` without +consulting the classifier at all. + +Read paths that surface the column: + +| Query | Line | Notes | +|---|---|---| +| `GetLatestTag` | 960 | `COALESCE(m.artifact_type, 'container-image')` | +| `GetTagsWithPlatforms` | 986 | selects `m.artifact_type` per tag; no filtering | +| `GetTopLevelManifests` | 1331 | **excludes** `artifact_type = 'unknown'` (line 1351) | +| `GetManifestDetail` | 1457 | selects `m.artifact_type`; no filtering | +| repo card queries | 151, 2412, 2496, 2587 | all `COALESCE(m.artifact_type, 'container-image')` | + +Handlers: + +- `pkg/appview/handlers/repository.go:142` seeds `artifactType := "container-image"` + and overwrites it from the selected tag at line 149. +- `pkg/appview/handlers/repository.go:576` and `:599` copy the type into each + `db.ManifestEntry`. +- `pkg/appview/handlers/digest.go:128` branches to the Helm path on + `manifest.ArtifactType == db.ArtifactTypeHelmChart`, and everything else + falls into the image branch at line 146 (layers from the DB, image config + from the hold, vuln + SBOM fetches). +- `pkg/appview/handlers/digest_content.go:58` makes the same two-way split for + the HTMX fragments. + +### Template inventory + +Every consumer is a two-way branch on the literal string `helm-chart`, with the +image rendering in the `else`. There is no `unknown` arm anywhere. + +| Template | Line | What the `else` gives an unknown artifact | +|---|---|---| +| `components/pull-command-switcher.html` | 11 | `docker pull` plus the OCI client dropdown | +| `components/repo-card.html` | 49 | `docker pull` style image ref in the card | +| `components/repo-card.html` | 75 | Helm glyph vs no glyph in the card footer | +| `pages/digest.html` | 45 | Helm badge when metadata is missing; no badge otherwise | +| `pages/digest.html` | 123 | `helm-digest-content` vs `digest-content` (layers + vulns + SBOM) | +| `partials/repo-tag-section.html` | 19 | Helm badge vs nothing | +| `partials/repo-tag-section.html` | 118 | single Chart tab vs Layers / Vulnerabilities / SBOM tabs | +| `partials/repo-tag-section.html` | 242 | Chart panel vs the three image panels | +| `partials/repo-tags.html` | 7 | Helm badge vs Multi-arch badge | +| `partials/repo-tags.html` | 51 | `helm pull` vs `docker pull` per entry | + +Copy that is wrong for a non-image artifact, quoted from the templates: + +- `partials/layers-section.html:18`: caption "Image layer history". +- `partials/layers-section.html:34`: "no command recorded". +- `partials/vulns-section.html:26` and `partials/vuln-details.html:5`: "Scans + run automatically shortly after a push. Check back in a few minutes, or push + a new tag to trigger a scan." + +`themes/seamark` overrides only `templates/components/nav-brand.html` and +`templates/components/hero.html`, so unlike most UI work, artifact-type +branches do **not** need to be duplicated into the theme. That was checked, not +assumed. + +### The hold has a third, independent copy of the rule + +`pkg/hold/oci/xrpc.go:366-369`: + +```go +artifactType := "container-image" +if strings.Contains(req.Manifest.Config.MediaType, "helm.config") { + artifactType = "helm-chart" +} +``` + +This value is only used to word the Bluesky post ("pushed Helm chart X" versus +"pushed X", `pkg/hold/pds/manifest_post.go:36` and `:60`). It has no `unknown` +case at all, so an `oddart` push announces itself as a plain image. + +### The scanner has a fourth copy, keyed differently + +- `scanner/internal/scan/worker.go:149-153` keys on **config** media type: + `application/vnd.cncf.helm.config.v1+json`, `application/vnd.in-toto+json`, + `application/vnd.dsse.envelope.v1+json`. +- `scanner/internal/scan/worker.go:172-184` adds a **layer** shape test + (`hasScannableLayer`: any layer whose media type is empty or contains `tar`). +- `pkg/hold/pds/scan.go:17-21` keys on **layer** media-type substrings + (`helm.chart.content`, `in-toto`, `dsse.envelope`) for the backfill, with a + comment saying it is kept in sync with the scanner's config map by hand. + +So the same question ("is this scannable?") is answered in four places from +three different inputs, and the appview's `artifact_type` is not one of them. + +### Relationship to finding 31 + +Finding 31 was a nine-day scanning outage (2026-08-25 to 2026-09-03) fixed in +`dfd604b`. The proximate cause was dispatch bookkeeping in the hold +(`hasActiveJobs` counting a permanently pending row), and that is fixed. But +the row that wedged the queue was an in-toto attestation manifest, and +`dfd604b`'s commit message states the reason it got that far: + +> the job that wedged this queue was an in-toto attestation whose config +> mediaType is an ordinary image config, so the existing config-type check +> missed it + +That is the same blind spot as `oddart`, seen from the scanner's side. A buildx +attestation manifest carries `application/vnd.oci.image.config.v1+json` as its +config and a single `application/vnd.in-toto+json` layer. `GetArtifactType` +calls it `container-image`; the scanner's config-type map called it scannable. +The manifest-level `artifactType` (or the layer media type, or the presence of +`subject`) would have identified it correctly in both places. `dfd604b` added +the layer-shape test to the scanner, which closes the scanner's exposure, but +the appview still classifies such a manifest as a container image and still +promises it a scan. + +## What the OCI spec says + +Quotes below are verbatim from `opencontainers/image-spec` and +`opencontainers/distribution-spec` `main` as of 2026-09-02. + +### `artifactType` on the manifest + +`image-spec/manifest.md`, Image Manifest Property Descriptions: + +> **`artifactType`** *string* +> +> This OPTIONAL property contains the type of an artifact when the manifest is +> used for an artifact. +> This MUST be set when `config.mediaType` is set to the empty value. +> If defined, the value MUST comply with RFC 6838, including the naming +> requirements in its section 4.2, and MAY be registered with IANA. +> Implementations storing or copying image manifests MUST NOT error on +> encountering an `artifactType` that is unknown to the implementation. + +That last sentence is the spec telling a registry exactly how to behave here: +store and serve an unrecognised type, do not reject it. It says nothing about +displaying it, which is where the design decision lives. + +The fallback to `config.mediaType` is guidance, not a MUST, at the end of the +same file: + +> _Implementers note:_ artifacts have historically been created without an +> `artifactType` field, and tooling to work with artifacts should fallback to +> the `config.mediaType` value. + +The one place it is normative is the referrers API, `distribution-spec/spec.md` +(Listing Referrers): + +> The descriptors MUST include an `artifactType` field that is set to the value +> of the `artifactType` in the image manifest or index, if present. +> If the `artifactType` is empty or missing in the image manifest, the value of +> `artifactType` MUST be set to the config descriptor `mediaType` value. +> If the `artifactType` is empty or missing in an index, the `artifactType` +> MUST be omitted. + +So the precedence order is settled by the spec: manifest `artifactType` first, +`config.mediaType` second, and for an index there is no config fallback at all. + +### Guidelines for Artifact Usage + +`image-spec/manifest.md`: + +> Content other than OCI container images MAY be packaged using the image +> manifest. +> When this is done, the `config.mediaType` value MUST be set to a value +> specific to the artifact type or the empty value. +> If the `config.mediaType` is set to the empty value, the `artifactType` MUST +> be defined. +> If the artifact does not need layers, a single layer SHOULD be included with +> a non-zero size. + +and the three shapes an artifact may take: + +1. No files or blobs: set `artifactType`, set `config` and a single `layers` + element to the empty descriptor. +2. Blobs but no JSON metadata: set `artifactType`, put the artifact in + `layers`, set `config` to the empty descriptor. +3. A config blob: set `artifactType`, put metadata in `config`, put the + artifact in `layers`. + +Shape 3 is what Helm does, which is why the current config-only classifier +works for Helm and fails for everything modern. + +### The empty descriptor + +`image-spec/manifest.md`, Guidance for an Empty Descriptor: + +> The media type `application/vnd.oci.empty.v1+json` (`MediaTypeEmptyJSON`) has +> been specified for a descriptor that has no content for the implementation. +> The blob payload is the most minimal content that is still a valid JSON +> object: `{}` (`size` of 2). +> The blob digest of `{}` is +> `sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a`. + +Fixed digest, fixed size 2, optional `data: "e30="`. An artifact using the +empty config today reaches `GetArtifactType` with +`"application/vnd.oci.empty.v1+json"`, matches no case, and returns `unknown`. +That is the right answer from the wrong evidence: the manifest almost certainly +carried an `artifactType` that would have said what it is, and the spec +requires it to in exactly this case. + +### `subject` and referrers + +`image-spec/manifest.md` and `image-index.md`, identical wording: + +> **`subject`** *descriptor* +> +> This OPTIONAL property specifies a descriptor of another manifest. +> This value defines a weak association to a separate Merkle Directed Acyclic +> Graph (DAG) structure, and is used by the `referrers` API to include this +> manifest in the list of responses for the subject digest. + +`distribution-spec/spec.md`: + +> The registry SHOULD support filtering on `artifactType`. +> To fetch the list of referrers with a filter, perform a `GET` request to a +> path in the following format: +> `/v2//referrers/?artifactType=` +> If filtering is requested and applied, the response MUST include a header +> `OCI-Filters-Applied: artifactType` + +and on push: + +> When processing a request for an image manifest with the `subject` field, a +> registry implementation that supports the referrers API MUST respond with the +> response header `OCI-Subject: ` + +ATCR implements none of these three (no referrers route, no `artifactType` +filter, no `OCI-Subject` header). It does store `subject_digest`, which is the +data a referrers endpoint would need. Building that endpoint is out of scope +here, but note the dependency runs the other way than one might expect: a +correct referrers response needs the manifest's `artifactType` persisted, which +is the very thing this document proposes storing. Persisting `artifactType` +therefore unblocks referrers later; referrers is not a prerequisite for +classification. + +### Index-level `artifactType` + +`image-spec/image-index.md` gives the index the same OPTIONAL `artifactType` +property, minus the "MUST be set when config.mediaType is empty" clause, +because an index has no config. So an index can itself be an artifact, and the +config fallback is unavailable for it: for an index, `artifactType` is the only +signal there is. + +### Media type strings + +| Purpose | String | +|---|---| +| OCI image config | `application/vnd.oci.image.config.v1+json` | +| OCI image manifest | `application/vnd.oci.image.manifest.v1+json` | +| OCI image index | `application/vnd.oci.image.index.v1+json` | +| OCI empty descriptor | `application/vnd.oci.empty.v1+json` | +| Docker schema2 manifest | `application/vnd.docker.distribution.manifest.v2+json` | +| Docker manifest list | `application/vnd.docker.distribution.manifest.list.v2+json` | +| Docker container image config | `application/vnd.docker.container.image.v1+json` | + +## What this deployment actually sees + +Evidence from the tree rather than speculation. Media type strings that appear +in non-test Go code across `pkg/`, `scanner/` and `cmd/`: + +``` +6 application/vnd.oci.image.manifest.v1+json +3 application/vnd.cncf.helm.config.v1+json +3 application/vnd.ipld.car (ATProto CAR, not OCI) +2 application/vnd.oci.image.layer.v1.tar+gzip +2 application/vnd.oci.image.index.v1+json +1 application/vnd.oci.image.config.v1+json +1 application/vnd.in-toto+json +1 application/vnd.dsse.envelope.v1+json +1 application/vnd.docker.distribution.manifest.v2+json +1 application/vnd.docker.distribution.manifest.list.v2+json +1 application/vnd.cncf.oras.artifact.manifest.v1+json +1 application/vnd.cncf.helm.chart.content.v1.tar+gzip +1 application/vnd.atcr.vulnerabilities+json (ATCR-internal scan blob) +``` + +The OAuth scope list (`pkg/auth/oauth/client.go:155-170`) is the clearest +statement of intent in the codebase, because it enumerates what the appview +asks permission to write to a user's PDS: + +```go +// Image manifest types (single-arch) +"blob:application/vnd.oci.image.manifest.v1+json", +"blob:application/vnd.docker.distribution.manifest.v2+json", +// Manifest list/index types (multi-arch) +"blob:application/vnd.oci.image.index.v1+json", +"blob:application/vnd.docker.distribution.manifest.list.v2+json", +// OCI artifact manifests (for cosign signatures, SBOMs, attestations) +"blob:application/vnd.cncf.oras.artifact.manifest.v1+json", +// Helm chart support +"blob:application/vnd.cncf.helm.config.v1+json", +"blob:application/vnd.cncf.helm.chart.content.v1.tar+gzip", +``` + +So cosign signatures, SBOMs and attestations are already an expected workload, +by the codebase's own comment, with no classification support behind it. + +`application/vnd.cncf.oras.artifact.manifest.v1+json` is the withdrawn ORAS +artifact-manifest type from the OCI 1.1 release candidates. It is scoped for +upload but appears in no classifier, so if anything still pushes it the result +is `unknown`. + +The concrete artifact families to design for: + +| Family | Manifest shape | What identifies it | +|---|---|---| +| Container image | image config + tar layers | `config.mediaType` (works today) | +| Multi-arch image | index with `manifests[]` | index media type (works today) | +| Helm chart | `helm.config.v1+json` + `helm.chart.content` layer | `config.mediaType` (works today) | +| buildx attestation | ordinary image config + `in-toto` layer, referenced from an index with `vnd.docker.reference.type=attestation-manifest` | layer media type, or the index annotation; `config.mediaType` lies | +| Referrer attestation | `subject` set, `in-toto` or DSSE payload | `subject` presence plus `artifactType` | +| cosign signature | `simplesigning` payload, or DSSE under the referrers scheme | `artifactType` or layer media type | +| SBOM artifact | operator-chosen `artifactType` via `oras push --artifact-type` | `artifactType` only | +| ORAS generic artifact | empty config + arbitrary layers | `artifactType` only | + +Four of eight are unidentifiable from `config.mediaType`, and two of those four +are actively misidentified rather than merely unrecognised. + +## Proposed classification model + +### Type set + +Keep the stored value a short slug, and keep it in the `manifests.artifact_type` +column. Proposed set: + +| Slug | Meaning | Status | +|---|---|---| +| `container-image` | Single-arch OCI or Docker image | exists | +| `image-index` | Multi-arch index or manifest list | **new**, currently folded into `container-image` | +| `helm-chart` | Helm chart | exists | +| `attestation` | in-toto / DSSE / SLSA provenance, whether via `subject` or a buildx index child | **new** | +| `signature` | cosign and similar signature artifacts | **new** | +| `sbom` | SBOM pushed as a first-class artifact | **new** | +| `artifact` | A well-formed OCI artifact whose type ATCR does not recognise | **new**, replaces `unknown` for this case | +| `unknown` | Classification genuinely failed (missing or unparseable evidence) | narrowed | + +The split between `artifact` and `unknown` matters for the UI. `artifact` means +"we know what this is called, we just do not have a specialised view", and the +page can show the type string with confidence. `unknown` means "we could not +determine anything", and the page should say only what it can prove. If the +distinction proves to have no UI consequence during implementation, collapse +them and keep `unknown`; do not carry two slugs that render identically. + +Whether `image-index` is worth splitting out is a real question, since +`ManifestWithMetadata.IsManifestList` already carries that information derived +from the media type. It is listed here because the classifier currently answers +"container-image" for an index, which is a small lie that a caller reading only +`artifact_type` cannot detect. Cheap to add, easy to drop. + +### Precedence + +Follow the spec, in this order: + +1. **Manifest-level `artifactType`**, if present and non-empty. Map known + values to a slug; map anything else to `artifact`, retaining the raw string. +2. **`config.mediaType`**, if `artifactType` was absent. This is the + `Implementers note` fallback and the referrers-API MUST. Keep the existing + Helm and image-config matches here. +3. **Structural signals**, only when 1 and 2 are inconclusive: + - index media type or a non-empty `manifests[]` implies `image-index`; + - `subject` present, plus in-toto or DSSE layers, implies `attestation`; + - the empty config descriptor with no `artifactType` is a malformed + artifact under the spec ("MUST be defined"), so `unknown` is the honest + answer. +4. Otherwise `unknown`. + +One extra rule the spec does not give but the buildx case demands: an ordinary +image config with **no tar-shaped layer** is not an image. That is the same +predicate `hasScannableLayer` already implements in +`scanner/internal/scan/worker.go:172-184`. Classifying such a manifest as +`attestation` when its layers are in-toto or DSSE, and `artifact` otherwise, +would have caught the finding 31 manifest at index time. + +### Retain the raw string + +Store the manifest's `artifactType` verbatim alongside the slug, so that: + +- the UI can show "application/vnd.example.thing.config.v1+json" rather than + a shrug; +- a future referrers endpoint can satisfy "The descriptors MUST include an + `artifactType` field" without re-fetching every manifest blob; +- adding a new recognised type later is a reclassification pass over a column, + not a re-walk of every user's PDS. + +This means both a lexicon addition (so the value is carried in the ATProto +record and thus reaches the firehose) and a column. See +[Migration and compatibility](#migration-and-compatibility). + +### One classifier, one input struct + +Today the rule is duplicated in `pkg/appview/db/queries.go`, +`pkg/hold/oci/xrpc.go`, `scanner/internal/scan/worker.go` and +`pkg/hold/pds/scan.go`. Proposed: a single function in `pkg/atproto` (the +package all four already import) taking a small descriptor struct +(`artifactType`, manifest media type, config media type, layer media types, +whether `subject` is set) and returning both a slug and a scannability verdict. +The hold's Bluesky wording and the scanner's skip check then read the same +answer as the UI. + +## Proposed UI contract + +An `artifact` or `unknown` page should be **honest and short** rather than a +degraded image page. + +Show: + +- The type. The raw `artifactType` string in a code span if we have it, + otherwise the config media type, otherwise "Unrecognised artifact". +- Digest, size, push time, tags, and the hold it lives on. All of this is real + and already available. +- The blob list as *blobs*: index, digest, size, media type. Same table + component as layers, different caption, no Dockerfile-command column. +- A copyable reference (`registry/handle/repo@sha256:...` or `:tag`) as plain + text, so the artifact is still addressable. +- If `subject` is set, a link to the manifest it refers to. This is the one + piece of context that makes an attestation page useful, and + `subject_digest` is already stored. + +Do NOT show: + +- **A `docker pull` command or the OCI client switcher.** `docker pull` on a + non-image artifact fails, and offering podman / nerdctl / buildah / crane + variants of a failing command is worse than offering none. If a per-type + command is known (`helm pull` for charts, `cosign download` for signatures), + show that. Otherwise show the reference and no verb. +- **Layers / Vulnerabilities / SBOM tabs.** Follow the pattern + `repo-tag-section.html:118` already establishes for Helm: replace the three + image tabs with a single type-appropriate tab. +- **"Image layer history" or "no command recorded".** There is no image config + and there never will be, so a table of empty commands is noise, not data. +- **"Scans run automatically shortly after a push. Check back in a few + minutes."** This is the worst of the four, because it is an assurance rather + than a mislabel. For a non-scannable artifact the correct copy already exists + and is used for Helm: "Vulnerability scanning isn't applied to this artifact + type." (see `docs/SBOM_SCANNING.md`, "Unscannable artifact types", and the + `not-applicable` reason in `digest_content.go:164`). + +The templates already have the right shape for this. `digest_content.go` +computes `VulnReason` / `SbomReason` with a `not-applicable` value, and the +Helm path proves a per-type digest view is viable. What is missing is a third +branch and a generic partial, not new machinery. + +The badge treatment should follow Helm's: a small type badge next to the tag or +digest wherever `repo-tags.html:7` and `repo-tag-section.html:19` place the +Helm badge, so a mixed repository reads correctly at a glance. + +## Proposed scan eligibility + +Scan eligibility is the same question as classification, asked by a different +consumer, and it should have one answer. + +Proposed rule, from the same classifier: an artifact is scannable if it is +`container-image` and it has at least one tar-shaped layer. Everything else is +not scannable, with a reason string. Concretely: + +| Type | Scannable | Reason surfaced | +|---|---|---| +| `container-image` with tar layers | yes | | +| `container-image` with no tar layer | no | "no scannable layers" (this is the finding 31 shape) | +| `image-index` | no | children are scanned individually | +| `helm-chart` | no | "scanning isn't applied to this artifact type" | +| `attestation`, `signature`, `sbom` | no | same | +| `artifact`, `unknown` | no | same | + +Where the decision should live: computed once at classification time and +carried, not recomputed per consumer. The hold needs it at +`pkg/hold/oci/xrpc.go:407` to decide whether to enqueue; the scanner needs it +at `worker.go:193` as a defence in depth for jobs enqueued by older holds; the +appview needs it to pick UI copy. A shared function in `pkg/atproto` satisfies +all three without the appview having to call the hold. + +Two concrete defects this would fix, both verified by reading the code: + +1. The hold's `req.Manifest.Subject == nil` guard at `xrpc.go:408` never fires, + because the appview does not send `subject` and the `notifyManifest` lexicon + does not define it. Every referrer artifact is enqueued today. Adding + `subject` to `#manifestInfo` and to the payload builder at + `manifest_store.go:635` is a small, independently shippable change. +2. The hold enqueues before knowing anything about layer shape beyond what it + marshals into the job, so the "is this scannable" judgement happens after a + round trip to a scanner. Deciding at enqueue time means the wedging class of + job never enters the queue. `dfd604b` made a stuck job survivable; this + would stop creating it. + +Note this does not weaken `dfd604b`'s dispatch fixes, which remain necessary: +a job can still become undispatchable for reasons unrelated to artifact type. + +## Migration and compatibility + +### Rows that would reclassify + +Today's population of `manifests.artifact_type`, by construction: + +- `container-image`: every image, every index, every manifest with a nil + config, and **every buildx attestation manifest** (ordinary image config). +- `helm-chart`: anything whose config media type contains `helm.config`. +- `unknown`: everything else. + +Under the proposed model: + +- Indexes move `container-image` to `image-index` (cosmetic, if that slug is + adopted). +- buildx attestation manifests move `container-image` to `attestation`. These + are mostly hidden from listings already, since they are index children + filtered by the `manifest_list_children` CTE and by `is_attestation`, so the + visible blast radius is small. +- Referrer manifests with `subject` set move to `attestation` or `signature`. + These are already excluded from listings by `subject_digest IS NULL` + (`queries.go:1350`, `queries.go:1788`), so again the visible change is small. +- Most current `unknown` rows become `artifact` once the manifest's + `artifactType` is available, and stay `unknown` until then. + +The important asymmetry: reclassification **reveals** rows rather than hiding +them, because `GetTopLevelManifests` currently drops `unknown`. Anything that +moves out of `unknown` starts appearing on repository pages. That is the +intended outcome, but it is a visible change on existing repositories and +should ship behind its own stage. + +### Migration 0020 already used `unknown` as a proxy + +`pkg/appview/db/migrations/0020_add_subject_digest.yaml` backfilled +`subject_digest = 'backfill'` for rows where +`artifact_type = 'unknown' AND media_type NOT LIKE '%index%' ...` joined to +`manifest_references` with `is_attestation = 1`. So some existing rows carry a +literal `'backfill'` string in `subject_digest` rather than a digest, and those +rows are exactly the ones a new classifier would want to look at. Any +reclassification pass must treat `subject_digest = 'backfill'` as "subject +present, digest unknown" and not try to parse it. + +### Is a schema change needed? + +**Yes, if the raw `artifactType` string is stored**, which this document +recommends. Per `CLAUDE.md` and `pkg/appview/db/migrations/README.md`, that +means both halves: + +1. Add the column to `pkg/appview/db/schema.sql` (fresh installs). +2. Add `pkg/appview/db/migrations/0035_add_oci_artifact_type.yaml` with + `ALTER TABLE manifests ADD COLUMN oci_artifact_type TEXT;` (existing + databases). `TestSchemaMatchesMigrations` fails the build if either half is + missing. + +**No, if only the slug set changes.** `artifact_type` is already +`TEXT NOT NULL DEFAULT 'container-image'` with an index at `schema.sql:46`, so +new slug values need no DDL. A data-only migration that rewrites existing rows +is optional and, per the staging below, probably not worth writing: the +jetstream reprocessing path already upserts `artifact_type` on every record it +sees (`queries.go:853`, `batch.go:85`), and a backfill run reclassifies +everything without SQL. + +The recommended split: one migration for the new column, and no data migration. +Let the backfill do the reclassifying, because it has the record in hand and +SQL does not. + +### Already-stored manifests + +Manifests pushed before the lexicon gains `artifactType` will not have it in +their ATProto record. Two recovery routes: + +1. `ManifestBlob` holds the original manifest bytes, so a backfill can fetch + the blob from the user's PDS and re-parse. Correct, but it is one PDS blob + fetch per manifest across every user, which is a large and slow operation + that must run as a background job. +2. Do nothing, and let structural signals (layer media types, `subject`, + `manifests[]`) classify old rows. Cheaper, no network, and good enough for + the families that matter: buildx attestations and referrers are both + identifiable structurally. + +Route 2 first. Route 1 only if a real gap shows up. + +### Compatibility constraints + +- **Do not reject unrecognised types on push.** The spec is explicit: + "Implementations storing or copying image manifests MUST NOT error on + encountering an `artifactType` that is unknown to the implementation." + Classification affects display and scan dispatch, never admission. +- **Old holds and old appviews must interoperate.** The `notifyManifest` + payload gains optional fields only; a hold that ignores them behaves as it + does today, and an appview that omits them leaves the hold on its existing + defaults. +- **The lexicon addition is additive.** `artifactType` as an optional string + property on `io.atcr.manifest` does not invalidate existing records. + +## Staged implementation plan + +Each stage is independently shippable and independently revertable. + +### Stage 1: stop the false promise + +Add an `unknown` arm to the scan-related copy so a non-scannable artifact never +reads "Check back in a few minutes". Purely a template and handler change, +using the `not-applicable` reason `digest_content.go` already computes. + +*Risk: very low.* No schema, no classifier, no lexicon. Worst case is that an +artifact that would eventually have been scanned shows the not-applicable copy, +which is recoverable by pushing again. + +### Stage 2: persist the truth + +Add `artifactType` to `pkg/atproto/lexicon.go`'s `ManifestRecord` and to +`lexicons/io/atcr/manifest.json`, parse it in `NewManifestRecord`, add the +`oci_artifact_type` column plus migration 0035, and store it from +`processor.go` and `backfill_batch.go`. Do not change any classification +behaviour yet. + +*Risk: low.* Additive lexicon field, additive column, no read path changes. +Run `make lex-lint`. The one thing to watch is that new manifests immediately +start carrying a field old appviews will ignore, which is fine. + +### Stage 3: one classifier + +Move the rule into `pkg/atproto`, implement the spec precedence, and have +`queries.go`, `hold/oci/xrpc.go`, `scanner/internal/scan/worker.go` and +`pkg/hold/pds/scan.go` call it. Keep the emitted slug set unchanged +(`container-image` / `helm-chart` / `unknown`) so nothing reclassifies yet. + +*Risk: medium.* Four call sites with subtly different current behaviour +(substring versus exact match, config-only versus layer-aware) collapse into +one. Needs table-driven tests per family before the switch, and the buildx +attestation case is the one most likely to shift silently. + +### Stage 4: new slugs, UI first + +Introduce `attestation`, `signature`, `sbom`, `artifact` and the generic +artifact page and badges, but keep writing the old slugs to the database. Drive +the new views from a computed value so the pages can be exercised on real data +without a reclassification. + +*Risk: medium.* Template surface is wide (ten branch sites), and each needs an +explicit third arm rather than relying on `else`. + +### Stage 5: reclassify + +Start writing the new slugs, and run a backfill so existing rows follow. +Relax the `artifact_type != 'unknown'` filter at `queries.go:1351` so +newly-classified artifacts become visible. This is the stage users notice. + +*Risk: high, and it is a visibility change.* Repositories gain rows that were +previously hidden. Sequence it after Stage 4 so the pages those rows land on +already render correctly, and consider a per-repository or per-hold rollout. + +### Stage 6: scan eligibility at the source + +Send `subject` in `notifyManifest` (lexicon plus +`manifest_store.go:635`), move the enqueue decision at `xrpc.go:407` onto the +shared classifier, and keep the scanner's `skipReason` as defence in depth for +jobs from older holds. + +*Risk: medium.* Touches the push path, which is the one path that must not +break. The scanner-side check stays, so a mistake here degrades to the current +behaviour rather than to a wedged queue. + +Stages 1 and 2 are worth doing regardless of whether the rest proceeds: Stage 1 +removes a false statement from the UI, and Stage 2 captures data that is +currently being thrown away on every push and cannot be recovered cheaply +later. + +## Open questions + +1. **Do `artifact` and `unknown` earn separate slugs?** They differ only in + whether a type string is displayable. If the generic page reads the raw + string directly, the slug distinction may be redundant. Not resolved here. + +2. **Should `image-index` be a slug at all?** `IsManifestList` already exists + and is derived from the media type. Splitting the slug is more honest but + adds a value every consumer must handle. Leaning yes, weakly. + +3. **How do signatures get identified in practice?** cosign's tag-based scheme + (`sha256-.sig`) puts the signature in a normal image manifest with a + `simplesigning` layer, while the OCI 1.1 referrers scheme uses `subject` + plus DSSE. The layer media type distinguishes them, but no cosign artifact + was found in this tree to check against. This needs a real sample before the + `signature` slug is implemented. + +4. **Is `application/vnd.cncf.oras.artifact.manifest.v1+json` still in use?** + It is scoped for upload at `pkg/auth/oauth/client.go:165` but appears in no + classifier and no handler. It was withdrawn before OCI 1.1 final. Whether + any live client still pushes it is unknown, and it could not be determined + from the repository. + +5. **How many production rows would reclassify?** No appview database was + available in this working tree, so the blast radius of Stage 5 is described + structurally rather than counted. A `SELECT artifact_type, COUNT(*)` against + the production database, plus a count of rows where + `subject_digest IS NOT NULL`, would turn the risk assessment above into a + number and should be run before Stage 5 is scheduled. + +6. **Should the hold reclassify its own stats and Bluesky copy?** The hold's + `artifactType` (`xrpc.go:367`) exists only to word a post. If it moves onto + the shared classifier it starts producing new strings for post copy, which + is a user-visible change to the Bluesky feed with no rollback. Possibly + worth leaving alone. + +7. **Does the referrers endpoint belong in this work?** `subject_digest` plus a + persisted `artifactType` would make `/v2//referrers/` + straightforward, and the spec's `artifactType` filter would be nearly free. + But that is an OCI API surface question, tied to + `docs/REMOVING_DISTRIBUTION.md`, and it is deliberately not proposed here. + +## References + +- [OCI Image Spec, Image Manifest](https://github.com/opencontainers/image-spec/blob/main/manifest.md) +- [OCI Image Spec, Descriptor](https://github.com/opencontainers/image-spec/blob/main/descriptor.md) +- [OCI Image Spec, Image Index](https://github.com/opencontainers/image-spec/blob/main/image-index.md) +- [OCI Image Spec, Media Types](https://github.com/opencontainers/image-spec/blob/main/media-types.md) +- [OCI Distribution Spec](https://github.com/opencontainers/distribution-spec/blob/main/spec.md) +- [SBOM Scanning](./SBOM_SCANNING.md), especially "Unscannable artifact types" +- [AppView](./appview.md) +- [Removing distribution/distribution](./REMOVING_DISTRIBUTION.md), "Referrers (OCI v1.1)" +- `pkg/appview/db/migrations/README.md` for the schema-plus-migration rule