From 25e2aa022827d31d84298f106784f4e33ee7100d Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Wed, 2 Sep 2026 21:43:09 -0500 Subject: [PATCH] docs: design note for artifact type classification Finding 32: pushing an artifact with an unrecognised config media type classifies as "unknown", and every template branches two ways on "helm-chart" with the container-image page in the else. So an in-toto attestation is served a docker pull command, Layers/Vulnerabilities/SBOM tabs, "Image layer history", and a promise that scans run shortly after push, which for that artifact will never be true. This is a design note rather than a fix, since the change is larger than the symptom. The inventory is the part worth keeping. The classification rule exists in four places, keyed off three different inputs (appview config media type, hold config media type with no unknown case, scanner config map plus layer shape, hold layer substrings), and the appview's artifact_type feeds none of the scan decisions. Manifest-level artifactType is discarded at parse time on every push: it is absent from the record struct, the constructor and the lexicon, surviving only inside the unindexed manifest blob. Two corrections to the framing this started from, both verified rather than assumed. The repo does not have referrers support: the pinned distribution version has no referrers code and ATCR registers no such route, so what exists is subject_digest persistence plus an attestation badge. And GetTopLevelManifests filters artifact_type != 'unknown', so an untagged unknown artifact is invisible while a tagged one renders as an image, which the finding did not mention. Proposes a type set, spec precedence (manifest artifactType, then config media type, then structural signals), a UI contract stating what such a page must not show, and a six stage plan. Only stage 2 needs a migration, for the raw string column; new slug values need no DDL and no data migration, since jetstream upserts artifact_type on every record it sees. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PDqoCE1j3njokkZ9b1C5n9 --- docs/ARTIFACT_TYPES.md | 882 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 882 insertions(+) create mode 100644 docs/ARTIFACT_TYPES.md 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