Conversation
Extracting as root, two things were fully controlled by the archive's own content: mknod created a device node from whatever major/minor an entry named, and Lchown gave a file to whatever uid/gid an entry's Info-ZIP Unix extra field carried, mode (including setuid/setgid) included. Both only do anything for a caller with the privilege to do them, so an archive extracted as root could alias a path in the destination to a real device the machine already has, or hand a file away to any owner it chose. Neither had a way for the caller to say no. Add WithExtractorDeviceNodes and WithExtractorPreserveOwner, both off by default. With device nodes off, a block or character device entry is left unwritten rather than handed to mknod; named pipes and sockets are unaffected, since making either needs no privilege and aliases no device the kernel already has. With ownership off, Lchown is never attempted at all, so an entry's mode is still applied but its uid/gid never take effect. Both propagate into the extractor a solid archive's contents are unpacked through, the same way every other option does. A few tests used the ownership pass as the one way to make updateFileMetadata fail on demand (an ordinary user is refused a change of owner) or as a proxy for "metadata landed on the right path" -- these now opt in explicitly with WithExtractorPreserveOwner. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
unxed
deleted the
lunobot/agent-lb3-zip56/lunobot-3/6-extractor-root-privileged-opt-in
branch
September 27, 2026 04:50
This was referenced Sep 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On Linux (and other Unix platforms), when extracting as root, two
things were fully controlled by the archive's own content:
Devmajor/Devminor) was created throughmknodwith the major/minor the archive named. An archive couldplace a node inside the extraction directory naming a real system
disk's device number.
Lchownunconditionally, and a non-regular entry's mode (includingthe setuid/setgid bits) was applied through
lchmod. An archivewith
uid=0controlled both the owner and the mode of what itcreated.
Both only take effect when the caller runs with the corresponding
privilege (
CAP_MKNOD/ the right tochownto an arbitrary owner),so neither had any effect for an ordinary user -- the problem shows up
only when extracting as root, which is unusual for a file manager but
not impossible.
WithExtractorDeviceNodes(off by default): with it off, ablock or character device entry is left unwritten rather than handed
to
mknod. Named pipes and sockets are unaffected -- making eitherneeds no privilege an ordinary caller lacks, and neither aliases a
device the kernel already has.
WithExtractorPreserveOwner(off by default): with it off,Lchownis never attempted for any entry, so an archive's uid/gidnever take effect regardless of the extra field it carries.
are unpacked with, the same way every other extractor option
already does.
A few existing tests used the ownership pass as the one reliable way
to make
updateFileMetadatafail on demand, or as a proxy for"metadata landed on the right path" -- those now opt in explicitly
via
WithExtractorPreserveOwner(true).Test plan
go build ./... && go vet ./... && golangci-lint run ./...go test ./...(full suite) andgo test -race ./...on theaffected tests
TestExtractor_DeviceNodesOptIn: off by default nothing iscreated for a device entry; opted in, mknod is actually attempted
(and, run as an ordinary user, refused, same as always).
TestExtractor_PreserveOwnerOptIn: off by default the chownerror handler never runs at all; opted in, it does.
TestExtractSolid_DirectoryMetadataFailure,TestExtractSolid_FileMetadataFailure,TestExtractorCovStreamEntryMetadataFailure, andTestPUA_Zip_NestedUndecodableNameMetadatato opt in explicitly,since they rely on the ownership pass running.
Touch #6
Lunobot-3
🤖 Generated with Claude Code