fix: avoid nil platform panic in DiscoverPlatformsFromReference#1646
fix: avoid nil platform panic in DiscoverPlatformsFromReference#1646false200 wants to merge 1 commit into
Conversation
A manifest index entry can have no platform field (for example an attestation entry). The skip branch logged m.Platform.OS and m.Platform.Architecture, and since those arguments are evaluated before the log call, a nil platform caused a panic instead of being skipped. Add a regression test that pushes an index with a platformless entry to an in-memory registry and verifies discovery skips it. Signed-off-by: false200 <214800619+false200@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a nil-pointer dereference panic in DiscoverPlatformsFromReference. When a manifest index contains a child descriptor with no platform field (e.g., an attestation or provenance entry), the skip branch's log.Debugf call read m.Platform.OS and m.Platform.Architecture. Since Go evaluates these arguments before the call and logrus evaluates them regardless of log level, a nil m.Platform caused a panic instead of being safely skipped. The fix changes the debug log to reference the manifest index (i) instead of dereferencing the nil platform, and adds a regression test.
Changes:
- Replaced the panicking debug log in the unknown/missing-platform skip branch with a message that logs the manifest index rather than the nil platform fields.
- Added a regression test that pushes an index with a platformless entry to an in-memory registry and asserts discovery skips it while returning the valid platform.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
pkg/buildkit/buildkit.go |
Fixes the debug log so it no longer dereferences a nil m.Platform, allowing platformless index entries to be skipped safely. |
pkg/buildkit/buildkit_test.go |
Adds a regression test (with in-memory registry helpers) verifying no panic occurs and only the valid platform is discovered. |
I verified the fix is correct (the loop index i is in scope and no longer touches the nil pointer), that the equivalent iteration in pkg/patch/platform.go already nil-guards, that the new test imports resolve to the existing github.com/google/go-containerregistry v0.21.4 dependency, and that the test's localhost host rewrite ensures go-containerregistry uses plain HTTP against the httptest registry via the remote fallback path.
A manifest index entry can have no platform field, for example an attestation or provenance entry. The skip branch logged m.Platform.OS and m.Platform.Architecture, and because those arguments are evaluated before the log call runs, a nil platform caused a panic instead of being skipped.
This changes the debug log so it no longer reads fields from a nil platform. Entries with a missing or unknown platform are now skipped safely and discovery continues with the remaining valid platforms.
Add a regression test that pushes an index with a platformless entry to an in memory registry and verifies discovery skips that entry and still returns the valid platform.
Closes #1645