Skip to content

fix: handle missing DiffIDs in tarball Manifest() to prevent panic#2263

Open
Yanhu007 wants to merge 1 commit intogoogle:mainfrom
Yanhu007:fix/tarball-layers-diffids-panic
Open

fix: handle missing DiffIDs in tarball Manifest() to prevent panic#2263
Yanhu007 wants to merge 1 commit intogoogle:mainfrom
Yanhu007:fix/tarball-layers-diffids-panic

Conversation

@Yanhu007
Copy link
Copy Markdown

Fixes #2192

Problem

(*compressedImage).Manifest() accesses cfg.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), DiffIDs is empty, causing:

panic: runtime error: index out of range [0] with length 0

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.

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
@google-cla
Copy link
Copy Markdown

google-cla Bot commented Apr 13, 2026

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-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.86%. Comparing base (8b3c303) to head (e860ebb).
⚠️ Report is 90 commits behind head on main.

Files with missing lines Patch % Lines
pkg/v1/tarball/image.go 0.00% 13 Missing and 1 partial ⚠️

❗ There is a different number of reports uploaded between BASE (8b3c303) and HEAD (e860ebb). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (8b3c303) HEAD (e860ebb)
2 1
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Subserial
Copy link
Copy Markdown
Contributor

Hey @Yanhu007, you'll need to sign the CLA before we can accept this PR.

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.

ggcr: panic on getting image layers with config application/vnd.buildkit.cacheconfig.v0 from tar

3 participants