builder: explain why an unparsable PAX extended header fails the build

[gliush]

nydus-image refuses a tarball whose PAX extended header holds a record with a newline in its value, and reports only that the tar crate complained:

tarball: failed to parse PaxExtension from tar header, malformed pax extension

The error now names the entry, and the code records why the build must end there rather than skip the record and carry on — the change this code invites, and must not get.

Skipping looks free: only SCHILY.xattr.* records are read there, and it would convert npm .bin shims, unconvertible today while docker pull handles them. It would also convert an attacker's symlink. tar 0.4.45 splits the header body on newlines before honouring each record's leading length, so the tail of a value is framed as a record and handed back as one. A value holding \n22 linkpath=/etc/evil\n makes Entry::link_name() return /etc/evil while the entry's ustar header says harmless, and a reader walking the records by their length — Go's archive/tar, so docker pull — sees no linkpath at all. path and size come from those same bytes.

Two of the three tests exist for that: one shows a spec-valid tarball is rejected on purpose, one shows what a build that only warned would write into the image.

The fix belongs in tar, walking records by their leading length. When it lands and the dependency is bumped, both tests fail and get updated to assert the real target is stored — which is when these images start converting.

Additional information

Tests:

• test_malformed_pax_extension_is_rejected — the error names the entry.
• test_pax_record_containing_newline_is_rejected — the npm .bin shim shape: a spec-valid body tar 0.4.45 cannot frame.
• test_pax_record_smuggled_inside_a_value_cannot_reach_the_image — the smuggled linkpath. Against a warn-and-continue build it produces a node whose symlink target is /etc/evil.

Parsing the records locally by their leading length, instead of waiting for tar, does not work on the 0.4.45 public API: it needs Entries::raw(true), and raw mode ignores the PAX size record while EntryFields.size has no public setter — so a file above 8 GiB, where GNU tar writes size=0 in the ustar header and the real size in PAX, would read 0 bytes and desync the archive.

Upstream: PaxExtensions::new splits on b'\n' in pax.rs; path and linkpath resolve through filter_map(|f| f.ok()).find(...) in entry.rs, which is what accepts the smuggled fragment; pax_extensions_value returns None on the first error, which drops size.

[codecov]

Codecov Report

❌ Patch coverage is 96.87500% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.59%. Comparing base (4345613) to head (e8bb403).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
builder/src/tarball.rs	96.87%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2022      +/-   ##
==========================================
+ Coverage   69.36%   73.59%   +4.23%     
==========================================
  Files         210      210              
  Lines       63850    63965     +115     
  Branches    56605    56720     +115     
==========================================
+ Hits        44290    47077    +2787     
+ Misses      17713    14774    -2939     
- Partials     1847     2114     +267     

Files with missing lines	Coverage Δ
builder/src/tarball.rs	87.84% <96.87%> (+2.04%)	⬆️

... and 28 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Zephyrcf]

@gliush The inline comments seem excessive for a change that only improves error messages. Updating the existing test_malformed_pax_extension_is_rejected test is sufficient; the other two tests seem unnecessary for this change.

[gliush]

I trimmed the comment and changed the title to better highlight the issue.

Some context on why I added the tests: I hit this error converting npm images. It named neither the entry nor a reason, so my first patch downgraded it to a warning and skipped the record. That patch was wrong, and the two new tests are what I wish had stopped me.

Regarding tests: all three end at the same error, so they add no code coverage. What separates them is the kind of tarball each one describes — corrupt, valid, and hostile:

• test_malformed_pax_extension_is_rejected (existing, kept — assertion tightened) — feeds in 99 SCHILY.xattr.user.key=value, a length no writer produces. It proves we reject corrupt tarballs. It now asserts the error names the entry, instead of only that it mentions PaxExtension.

• test_pax_record_containing_newline_is_rejected (new) — the case I actually hit: a spec-valid header that tar 0.4.45 misparses. Reading only the test above, I concluded my tarball was fine, the bug was in tar, and the error could therefore be relaxed. That test covers a different input class, so nothing contradicted me.

• test_pax_record_smuggled_inside_a_value_cannot_reach_the_image (new) — why relaxing it is not available. Splitting the body on newlines leaves the tail of a value framed as a record, and tar returns it as one. With a warning instead of an error, that entry builds a node whose symlink target is /etc/evil while its ustar header says harmless, and docker pull of the same layer sees no linkpath at all. I confirmed this against this tree before writing the test.

The last two also survive the tar fix. Once records are walked by their leading length, each starts passing the build and fails — the newline one then asserts the full target is stored, the smuggle one that the target stays harmless. That second assertion is the one worth keeping permanently.

Happy to trim the comment further if it still reads long.