Skip to content

Add ExtractAll with os.Root confinement - #27

Merged
andrew merged 2 commits into
mainfrom
extract-all
Aug 3, 2026
Merged

Add ExtractAll with os.Root confinement#27
andrew merged 2 commits into
mainfrom
extract-all

Conversation

@andrew

@andrew andrew commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Adds ExtractAll(r Reader, dir string) error as a package function that writes every entry under dir. Implemented on top of List and Extract so it works for zip, tar, gem, and prefix-stripped readers with one code path.

All per-entry filesystem operations go through os.OpenRoot(dir), so a symlink under dir cannot redirect a write outside it. Entry names are also validated with filepath.Localize; absolute paths and .. escapes return ErrUnsafePath naming the offending entry. Any existing object at a destination path is removed before an O_EXCL create so pre-existing in-root symlinks and hard links are replaced with a fresh regular file rather than followed or modified in place.

Permissions are preserved when the archive records them: file modes are applied via File.Chmod on the open descriptor before close, and directory modes are applied deepest-first after all entries are written. setuid/setgid/sticky bits are dropped since extracted archives are untrusted. FileInfo gains a HasMode field so a recorded 0o000 is distinguishable from an absent mode; for zip it is set only when the creator system is Unix or macOS and a Unix st_mode is present in ExternalAttrs, so the 0666 that archive/zip synthesises for FAT/NTFS entries is not applied on disk.

Supporting changes: tarReader now derives Mode from header.FileInfo().Mode() so symlink/device typeflags surface as fs.ModeType bits, marks TypeLink entries as fs.ModeIrregular, and both readers build a path index at open time so Extract is O(1) and ExtractAll is linear in the entry count. This changes FileInfo.Mode for tar directories and special entries (they now carry fs.ModeType bits, matching what zip already returned); regular-file modes are unchanged and no known consumer reads Mode on non-regular entries.

Closes #22.

ExtractAll(r Reader, dir string) writes every entry under dir using
os.OpenRoot so no operation can escape the target through a symlink.
Entry names are validated with filepath.Localize and traversal attempts
return ErrUnsafePath naming the offending entry. Existing objects at a
destination path are removed before an O_EXCL create so in-root symlinks
and hard links are replaced rather than followed or modified in place.

File permissions are applied via fchmod on the open descriptor when the
archive recorded a mode; directory permissions are applied deepest-first
after all entries are written. FileInfo gains HasMode to distinguish a
recorded mode from a synthesised or absent one; zip entries set it only
when the creator system is Unix or macOS and a Unix st_mode is present.

tarReader now derives Mode from header.FileInfo().Mode() so symlink and
device typeflags surface as fs.ModeType bits, and marks TypeLink entries
irregular. Both readers build a path index at open time so Extract is a
map lookup and ExtractAll is linear in the entry count.

Closes #22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a new disk-extraction API to the archives package, allowing callers to materialize an archive’s contents under a target directory while attempting to prevent path traversal and symlink-escape writes via os.OpenRoot confinement. It also extends per-entry metadata to distinguish “mode recorded vs synthesized” and improves extraction performance by indexing entries for O(1) lookup.

Changes:

  • Add ExtractAll(r Reader, dir string) error with path validation and os.Root confinement, plus deferred directory chmod handling.
  • Extend FileInfo with HasMode and update ZIP/TAR readers to populate it appropriately.
  • Add comprehensive tests for extraction behavior (traversal rejection, root confinement, replacing existing objects, mode handling), and document the new API in the README.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
zip.go Adds an entry index for O(1) extraction and implements HasMode detection for Unix/macOS-created ZIP entries.
tar.go Adds an entry index for O(1) extraction and improves mode/typeflag mapping (including marking hardlinks irregular).
archives.go Extends FileInfo with HasMode to distinguish recorded modes from synthesized defaults.
extract.go Introduces ExtractAll with os.Root confinement, path validation, replacement semantics, and deferred directory chmod.
extract_test.go Adds tests covering traversal protection, confinement vs symlinks, replacement behavior, and mode preservation logic.
README.md Documents ExtractAll usage and its safety/mode behavior at a high level.
Suppressed comments (1)

extract.go:131

  • When applying recorded file modes, out.Chmod(perm) uses perm := fs.FileMode(entry.Mode).Perm(), which drops sticky/setuid/setgid bits from the stored mode. If the intent is to preserve the recorded mode (excluding type bits), chmod should use the full recorded mode rather than only .Perm().
	if entry.HasMode {
		// The mode passed to OpenFile is subject to the process umask;
		// restore the recorded permissions on the open descriptor so no
		// path lookup is involved.
		if err := out.Chmod(perm); err != nil {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread extract.go
Comment thread extract.go
Comment thread extract.go
Comment thread extract.go
@andrew
andrew merged commit bbf9c2c into main Aug 3, 2026
5 checks passed
@andrew
andrew deleted the extract-all branch August 3, 2026 15:47
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.

Add ExtractAll(dir string) with path traversal guard

2 participants