Skip to content

Fix off-by-one bounds check in layout layer facade#308

Merged
jabrown85 merged 1 commit into
buildpacks:mainfrom
arpitjain099:fix/layout-facade-layer-index-bounds
Jul 15, 2026
Merged

Fix off-by-one bounds check in layout layer facade#308
jabrown85 merged 1 commit into
buildpacks:mainfrom
arpitjain099:fix/layout-facade-layer-index-bounds

Conversation

@arpitjain099

Copy link
Copy Markdown
Contributor

The two bounds checks in newLayerOrFacadeFrom use > where they should use >=, so when layerIndex equals the slice length the guard doesn't fire and we fall straight through to configFile.RootFS.DiffIDs[layerIndex] / manifestFile.Layers[layerIndex]. That's an index out of range panic instead of the error the guard was there to return.

It's reachable through the layout facade: newImageFacadeFrom ranges over original.Layers() and hands each index to newLayerOrFacadeFrom, which then indexes into the config's diff_ids. Config and manifest get parsed independently and nothing cross-checks the counts, so an OCI layout image whose manifest lists more layer descriptors than the config has diff_ids (or the other way round) loads fine and then panics when it's used as a base or previous image. Only affects the facade path, so data-less/sparse layers.

Changing > to >= gets you the intended "failed to find layer for index N" error. Valid images are unaffected since a valid index never equals the array length.

Added layout/v1_facade_test.go covering both directions of the mismatch plus an in-bounds case. It's an in-package test since newLayerOrFacadeFrom isn't exported. Panics before the change, passes after, and go test ./layout/... is green.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
@arpitjain099
arpitjain099 requested a review from a team as a code owner July 15, 2026 11:38
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 34.78%. Comparing base (c36b3eb) to head (ac7d98e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #308      +/-   ##
==========================================
+ Coverage   34.66%   34.78%   +0.12%     
==========================================
  Files          39       39              
  Lines        3359     3359              
==========================================
+ Hits         1164     1168       +4     
+ Misses       2000     1998       -2     
+ Partials      195      193       -2     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jabrown85 jabrown85 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this!

@jabrown85
jabrown85 merged commit 40ca349 into buildpacks:main Jul 15, 2026
6 checks passed
pull Bot pushed a commit to Spencerx/BuildPackLifecycle that referenced this pull request Jul 15, 2026
Picks up buildpacks/imgutil#308, which fixes an index-out-of-range panic
in the layout layer facade when an OCI layout image's manifest and config
disagree on layer count.
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