Skip to content

exporter: sanitize platform IDs in path components - #7022

Open
crazy-max wants to merge 3 commits into
moby:masterfrom
crazy-max:platform-id-path-sanitize-followup
Open

exporter: sanitize platform IDs in path components#7022
crazy-max wants to merge 3 commits into
moby:masterfrom
crazy-max:platform-id-path-sanitize-followup

Conversation

@crazy-max

@crazy-max crazy-max commented Aug 6, 2026

Copy link
Copy Markdown
Member

needs tonistiigi/fsutil#275

This restores the platform ID path sanitization from #6910 after it was reverted in #6935.

The first commit cherry-picks original sanitizer onto the current tree, with the integration test moved into the split client exporter test file. The second commit fixes the regression test by checking that the payload exists under the sanitized platform directory instead of requiring one exact Windows rootfs path.

@crazy-max crazy-max added this to the v0.33.0 milestone Aug 6, 2026
@crazy-max
crazy-max requested a review from tonistiigi August 6, 2026 12:30
@crazy-max
crazy-max marked this pull request as ready for review August 6, 2026 12:54
const platformDir = ".._buildkit-outside"
payloadPath := ""
for name, item := range m {
if item.Header.Typeflag != tar.TypeReg {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These skips at least need comments describing what cases they are handling and why we have files in the tar that we can't strictly verify.

@tonistiigi tonistiigi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Doesn't seem to be passing

=== FAIL: client TestIntegration/slice=1-4/TestExportTarPlatformIDSanitized/worker=containerd (1.00s)
    client_export_local_test.go:541: 

Replace Windows path separators and drive separators when platform IDs are
used as local and tar exporter path components. Add a regression test for tar
exporter output generated from frontend-controlled platform metadata.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@crazy-max

Copy link
Copy Markdown
Member Author

Doesn't seem to be passing

=== FAIL: client TestIntegration/slice=1-4/TestExportTarPlatformIDSanitized/worker=containerd (1.00s)
    client_export_local_test.go:541: 

Seems to be an fsutil issue actually. The tar exporter passes a SubDirFS to fsutil.WriteTar, and WriteTar walks /. On Windows, SubDirFS.Walk was splitting with filepath.Separator, so / was treated as a literal first component and none of the platform directories were visited. That matches the empty tar map from the failure.

I moved the fix to fsutil so root walks work consistently there: tonistiigi/fsutil#275

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
@crazy-max
crazy-max force-pushed the platform-id-path-sanitize-followup branch from bae2e3f to 7c575a2 Compare August 11, 2026 09:08
@github-actions github-actions Bot added the area/dependencies Pull requests that update a dependency file label Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/client area/dependencies Pull requests that update a dependency file area/exporter area/testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants