fix: handle missing DiffIDs in tarball Manifest() to prevent panic#2263
fix: handle missing DiffIDs in tarball Manifest() to prevent panic#2263Yanhu007 wants to merge 1 commit intogoogle:mainfrom
Conversation
When loading a tar image with a non-standard config type (e.g. buildkit cacheconfig), RootFS.DiffIDs may be empty. The Manifest() method accesses DiffIDs[i] without bounds checking, causing a panic. Add a bounds check: when the layer index exceeds the DiffIDs length, read the layer directly from the tar to compute its digest and size. Fixes google#2192
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2263 +/- ##
===========================================
- Coverage 71.67% 52.86% -18.82%
===========================================
Files 123 165 +42
Lines 9935 11221 +1286
===========================================
- Hits 7121 5932 -1189
- Misses 2115 4574 +2459
- Partials 699 715 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hey @Yanhu007, you'll need to sign the CLA before we can accept this PR. |
Fixes #2192
Problem
(*compressedImage).Manifest()accessescfg.RootFS.DiffIDs[i]without bounds checking. When loading a tar image whose config is a non-standard type (e.g.application/vnd.buildkit.cacheconfig.v0),DiffIDsis empty, causing:Fix
Add a bounds check before accessing
DiffIDs[i]. When the index exceeds the DiffIDs length (as with buildkit cache configs), fall through to read the layer directly from the tar archive to compute its digest and size, similar to the existing non-foreign-layer path.