Skip to content

Defer directory attributes without following symlinks - #823

Open
stephenlclarke wants to merge 4 commits into
apple:mainfrom
stephenlclarke:fix/archive-deferred-directory-symlink-safety
Open

Defer directory attributes without following symlinks#823
stephenlclarke wants to merge 4 commits into
apple:mainfrom
stephenlclarke:fix/archive-deferred-directory-symlink-safety

Conversation

@stephenlclarke

@stephenlclarke stephenlclarke commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

During testing of my Container Compose plugin, I found that archives containing read-only directories cannot be extracted by the current main branch because directory attributes are applied before their children are written.

This supersedes and credits @JaewonHur's work in #716. That PR correctly defers directory attributes, but its single openat call passes the full relative path with O_NOFOLLOW. O_NOFOLLOW protects only the final path component, so a directory replaced by an intermediate symlink can redirect the deferred fchown and fchmod operations outside the extraction root.

Reproduction

On stock apple/containerization main at 50f77222964ed3d01dcb53f803ea2d511656a9dc:

  1. Create an archive containing readonly/ with mode 0555 and readonly/file.txt.
  2. Extract it with ArchiveReader.extractContents.
  3. Extraction fails while creating readonly/file.txt because the parent directory was already made read-only.

The regression test archiveDirectoryPreservesReadonlySubdirectory fails on stock main with:

failed to extract archive: failed to create file: readonly/file.txt

On the current #716 head at 77da467c16d8e406b38f3257188a7f1fbe078368:

  1. Extract a directory entry such as parent/child/.
  2. Later in the same archive, replace parent with a symlink to an external directory.
  3. Allow deferred attributes to run.
  4. The external child directory's permissions are changed, demonstrating that metadata escaped the extraction root.

The current #716 head also loses last-entry-wins semantics when the same directory is represented by aliases such as directory/ and ./directory/.

Change

  • Defer directory ownership and permissions until all archive entries have been extracted.
  • Open each existing path component relative to the extraction-root descriptor with O_NOFOLLOW | O_RDONLY | O_DIRECTORY.
  • Refuse symlink and non-directory traversal without replacing the existing entry.
  • Canonicalise successfully extracted directory paths and retain only the final deferred attributes for each directory.
  • Apply deferred attributes deepest-first so restrictive parent permissions do not block child updates.
  • Bind deferred metadata to the extracted directory's device and inode identity so a removed and recreated path cannot inherit stale attributes.
  • Correct the existing read-only-directory regression test, which previously used and expected mode 0777.
  • Add focused regressions for intermediate-symlink traversal, aliased duplicate directory entries, and descriptor-relative directory opening.

Expected result

Read-only directory archives extract successfully and retain their final metadata. Deferred metadata operations remain anchored beneath the extraction root, and the last successful directory entry defines the final attributes.

Testing

  • make fmt
  • make check
  • swift test --filter ArchiveReaderTests: 26 tests passed in 43.917 seconds
  • swift test --filter FileDescriptorOpsTests: 28 tests passed in 18.969 seconds
  • make test: 579 tests in 80 suites passed in 45.230 seconds after build
  • Post-review make check: passed in 3.62 seconds
  • Post-review full make test: passed in 121.14 seconds including the coverage rebuild
  • Stock main comparison: corrected read-only test fails as described
  • Current Fix ArchiveReader to support readonly directory #716 comparison: intermediate-symlink and aliased-directory regressions fail as described

All commits are signed and verified.

JaewonHur and others added 4 commits July 28, 2026 18:35
Apply directory ownership and permissions after streaming extraction so read-only parents do not block their children. Traverse existing directories without following symlinks or recreating paths superseded by later archive entries.

Upstream-PR: apple#716
Upstream-Head: 77da467
Exercise deferred directory attributes through the archive reader and prove a replaced intermediate directory cannot redirect ownership or permission changes outside the extraction root.
Canonicalise successfully extracted directory paths before deferring metadata so archive aliases such as directory and ./directory retain last-entry-wins semantics.
Record the extracted directory identity and verify it before applying deferred metadata so a removed and recreated path cannot inherit attributes from an earlier archive entry.
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