Skip to content

Add OCI artifact manifest and Referrers API support #64

Merged
cgwalters merged 4 commits intobootc-dev:mainfrom
cgwalters:oci-artifacts
Mar 23, 2026
Merged

Add OCI artifact manifest and Referrers API support #64
cgwalters merged 4 commits intobootc-dev:mainfrom
cgwalters:oci-artifacts

Conversation

@cgwalters
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces significant new functionality by adding support for OCI artifact manifests and the Referrers API, with new public APIs in src/lib.rs and a comprehensive example in examples/ocidir.rs. While the implementation generally follows OCI specifications, a critical security weakness was identified in the fsck integrity check logic: it fails to verify the SHA-256 digests of configuration blobs for new artifact types, relying only on existence and size checks, which could allow undetected modifications. Additionally, the review suggests improving code robustness by replacing .unwrap() calls with proper error propagation, especially in the library code, and adopting more idiomatic patterns for better readability and safety.

Comment thread src/lib.rs Outdated
return Err(e);
}
};
let mut manifest_desc = desc_builder.build().unwrap();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using .unwrap() in library code can lead to unrecoverable panics for consumers of the library. Since this function returns a Result, you should propagate the error using the ? operator.

Suggested change
let mut manifest_desc = desc_builder.build().unwrap();
let mut manifest_desc = desc_builder.build()?;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops yeah. youki-dev/oci-spec-rs#242 is still relevant...

Comment thread src/lib.rs Outdated
Comment thread src/lib.rs Outdated
Comment thread examples/ocidir.rs Outdated
Comment thread examples/ocidir.rs Outdated
Comment thread examples/ocidir.rs Outdated
Comment thread examples/ocidir.rs Outdated
Comment thread examples/ocidir.rs Outdated
Comment thread examples/ocidir.rs Outdated
henrywang
henrywang previously approved these changes Mar 19, 2026
Copy link
Copy Markdown
Contributor

@henrywang henrywang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review passed and no break change added.

We're working on composefs/composefs-rs#224
and the new idea is we create a composefs OCI artifact.

While working on this I realized we didn't have good enough support for
OCI artifacts here. There's a few things, but especially the
image index can have artifactType for example.

There's also an API to find referrers, mirroring the distribution API.

`fsck` now accepts unknown config media types (verifying the blob
digest) rather than rejecting them, per the OCI image spec requirement
that implementations MUST NOT error on unknown config mediaTypes.

Assisted-by: OpenCode (Claude claude-opus-4-6)
Signed-off-by: Colin Walters <walters@verbum.org>
While we have examples here, it feels really natural to
put together a little CLI that wires us up to oci-distribution
so we have a pure-Rust little demo of fetching from a
registry and storing to ocidir.

Since we have this, add a `selftest` command which can
be used in CI to act as an integration test.

Assisted-by: OpenCode (Claude claude-opus-4-6)
Signed-off-by: Colin Walters <walters@verbum.org>
Previously, fsck_one_manifest only verified SHA-256 digests for config
blobs with unknown media types, while known types (ImageConfig,
EmptyJSON) only got size + JSON deserialization checks. This meant a
targeted same-size modification to a config blob could go undetected
for known types, which is inconsistent with how layer blobs are always
digest-verified.

Now all config blobs get their digest verified first, then known types
additionally get structure validation via deserialization. This also
extracts the duplicate digest verification logic into a shared
verify_blob_digest() helper used by both config and layer verification.

Also fix .unwrap() in write_config to use ? instead, since the function
already returns Result.

Assisted-by: OpenCode (Claude Opus 4)
Signed-off-by: Colin Walters <walters@verbum.org>
Two minor cleanups to verify_blob_digest:

Use ? directly on Hasher::finish() instead of map_err to Error::Other,
since From<ErrorStack> already maps to the more specific
CryptographicError variant.

Also skip config blob verification when the digest is already in the
validated set, matching the existing layer deduplication logic. This
avoids redundant I/O for the common case of artifact manifests sharing
the empty config blob.

Assisted-by: OpenCode (Claude Opus 4)
Signed-off-by: Colin Walters <walters@verbum.org>
@cgwalters cgwalters merged commit 3ba4e57 into bootc-dev:main Mar 23, 2026
4 checks passed
cgwalters added a commit to cgwalters/composefs-rs that referenced this pull request Apr 10, 2026
Upgrade ocidir from 0.6 to 0.7 (which includes OCI artifact manifest
support from bootc-dev/ocidir-rs#64, though that hasn't shipped in a
release yet). This eliminates the duplicate ocidir dependency.

Downgrade oci-client from 0.16 to 0.15 since 0.16 pulls in oci-spec
0.9 while everything else (containers-image-proxy, ocidir, composefs-oci)
uses oci-spec 0.8. With this change, only a single oci-spec version
(0.8.4) appears in the dependency tree, eliminating the need for the
JSON round-tripping bridge in referrers.rs.

Also upgrade cap-std-ext 4 -> 5 to match ocidir 0.7's dependency, and
remove unused cap-std-ext from cstorage (it was declared but never
imported).

Assisted-by: OpenCode (Claude claude-opus-4-6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants