Skip to content

Add an API to resolve manifest lists#57

Merged
cgwalters merged 1 commit intobootc-dev:mainfrom
cgwalters:add-platform-manifest-resolution
Mar 4, 2026
Merged

Add an API to resolve manifest lists#57
cgwalters merged 1 commit intobootc-dev:mainfrom
cgwalters:add-platform-manifest-resolution

Conversation

@cgwalters
Copy link
Copy Markdown
Collaborator

I wanted this in composefs-rs.

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 a new API to resolve OCI manifest lists, correctly handling platform and tag matching. However, the implementation lacks safeguards against malicious input, making it vulnerable to infinite recursion (stack overflow) via circular index references and potential memory exhaustion (OOM) when formatting error messages for indexes with a very large number of manifests, both of which can be exploited for Denial of Service. Additionally, my review identified a critical compilation issue related to platform matching logic and a high-severity bug in how nested image indices are resolved. Addressing these points will improve the robustness, correctness, and security of the new API.

Comment thread src/lib.rs Outdated
Comment thread src/lib.rs Outdated
Comment thread src/lib.rs Outdated
Comment thread src/lib.rs Outdated
Comment thread src/lib.rs Outdated
@cgwalters cgwalters force-pushed the add-platform-manifest-resolution branch from 0927145 to 8da25ab Compare February 26, 2026 17:12
@cgwalters cgwalters marked this pull request as ready for review February 26, 2026 17:12
@cgwalters cgwalters force-pushed the add-platform-manifest-resolution branch 2 times, most recently from f965e06 to 0baa1cb Compare February 26, 2026 19:21
@cgwalters cgwalters enabled auto-merge (squash) February 26, 2026 19:26
@cgwalters cgwalters force-pushed the add-platform-manifest-resolution branch from 0baa1cb to a29c579 Compare February 27, 2026 00:15
@cgwalters cgwalters requested a review from ckyrouac February 27, 2026 20:05
@cgwalters cgwalters force-pushed the add-platform-manifest-resolution branch from a29c579 to f280162 Compare March 2, 2026 16:30
Add open_image_this_platform() which resolves a manifest from an OCI
directory for the native platform (OS and architecture). This handles
manifest lists by peeling them one level to find platform-specific
manifests.

Returns ResolvedManifest containing:
- The resolved manifest and its digest
- The source image index and digest if resolution went through a
  manifest list

Assisted-by: OpenCode (claude-opus-4-5)
Signed-off-by: Colin Walters <walters@verbum.org>
@cgwalters cgwalters force-pushed the add-platform-manifest-resolution branch from f280162 to e9282e1 Compare March 3, 2026 23:03
@cgwalters
Copy link
Copy Markdown
Collaborator Author

I condensed the tests a bit more

Copy link
Copy Markdown
Contributor

@jmarrero jmarrero left a comment

Choose a reason for hiding this comment

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

lgtm

@cgwalters cgwalters merged commit b851d0f into bootc-dev:main Mar 4, 2026
4 of 5 checks passed
@jlebon
Copy link
Copy Markdown
Contributor

jlebon commented Mar 4, 2026

Hmm, this was merged over red.

error[E0658]: `let` expressions in this position are unstable
   --> src/lib.rs:712:24
    |
712 |                     if let Some(platform) = desc.platform().as_ref()
    |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #53667 <https://github.com/rust-lang/rust/issues/53667> for more information

MSRV is pinned to 1.86 but assuming we're tracking el9 could be bumped to 1.88 which stabilizes this.

@cgwalters
Copy link
Copy Markdown
Collaborator Author

Thanks for looking, that issue is partially in #61

I dropped the MSRV check entirely for now in that; but yeah we may need to re-add it at some point for our crates. If we do it needs to be done in a way that uses rust-version (the modern way).

@henrywang henrywang mentioned this pull request Mar 5, 2026
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.

3 participants