Fix zip unix permissions

[alinpahontu2912]

Fixes #94465

[Copilot]

Pull request overview

This PR changes Unix extraction behavior in System.IO.Compression.ZipFile so that Unix permission bits from ExternalAttributes are only applied when the ZIP entry’s “version made by” platform indicates Unix, preventing Windows-made entries from having their Windows attributes misinterpreted as Unix file modes. It also adds a regression test to validate the intended behavior.

Changes:

• Gate UnixCreateMode application on VersionMadeBy platform being Unix (3) during extraction on non-Windows OSes.
• Add a Unix regression test that verifies Windows-made entries do not have Unix permission bits applied.
• Add a small test helper to patch the entry’s “version made by” platform in the central directory record.

Reviewed changes

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

File	Description
src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs	Adds a regression test + helper to validate permissions aren’t applied for Windows-made entries.
src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchiveEntry.Extract.cs	Restricts Unix permission restoration to entries marked as Unix-made via VersionMadeBy.

[github-actions]

Copilot Code Review

Holistic Assessment

Motivation: The PR addresses a real bug — when extracting zip entries created on Windows (or with a non-Unix platform ID), the upper 16 bits of ExternalAttributes may contain values that are incorrectly interpreted as Unix permission bits, causing extracted files to have wrong permissions.

Approach: The fix correctly uses the VersionMadeBy platform byte (as defined in APPNOTE.TXT §4.4.2.2) to only apply Unix permissions when the entry was actually created on Unix. This is the right approach and consistent with how the codebase already uses ZipVersionMadeByPlatform.

Summary: ✅ LGTM. The fix is correct, well-scoped, and the test properly validates the behavior. One minor suggestion below regarding naming, and one observation about platform coverage, but neither is blocking.

---

Detailed Findings

Detailed Findings

✅ Correctness — Platform check is sound

The fix extracts the platform byte from source.VersionMadeBy >> 8 and compares against 3 (Unix). This is consistent with:

• ZipVersionMadeByPlatform enum in ZipVersion.cs (Unix = 3, Windows = 0)
• How ZipArchiveEntry writes the platform: cdStaticHeader[FieldLocations.VersionMadeByCompatibility] = (byte)CurrentZipPlatform
• The ZIP specification section 4.4.2.2

The local const byte UnixMadeByPlatform = 3 is necessary since ZipVersionMadeByPlatform is internal to System.IO.Compression and not accessible from System.IO.Compression.ZipFile.

✅ Existing test compatibility

Existing tests still pass because:

• UnixExtractSetsFilePermissionsFromExternalAttributes creates archives via .NET on Unix, which writes CurrentZipPlatform = Unix (3).
• UnixExtractFilePermissionsCompat uses pre-made archives (OSX_RWXRW_R__.zip, Linux_RW_RW_R__.zip) which, by convention, macOS and Linux tools write with platform byte 3.

✅ Test quality — Good regression test

The test creates a zip with explicit permission bits, patches the binary to simulate a Windows-originated entry (platform 0), then verifies that extraction uses default system permissions rather than the embedded bits. The binary patching via SetFirstEntryVersionMadeByPlatform correctly targets offset +5 in the central directory header.

💡 Naming — windowsPlatform parameter name is slightly confusing

In SetFirstEntryVersionMadeByPlatform(archivePath, windowsPlatform: 0), the parameter name windowsPlatform with value 0 reads oddly — it could be interpreted as "should this be a Windows platform?" (boolean) rather than "the platform byte value." A name like platformId would be clearer:

private static void SetFirstEntryVersionMadeByPlatform(string archivePath, byte platformId)

This is a minor suggestion and not blocking.

💡 Observation — Strict equality to Unix platform (3)

The check versionMadeByPlatform == UnixMadeByPlatform means only entries with platform byte exactly 3 get Unix permissions applied. The ZIP spec defines other platform IDs (e.g., 19 for OS X/Darwin) that could theoretically carry Unix permissions. In practice, macOS tools write platform 3 (since macOS is Unix), so this is not a real-world issue. The existing OSX_RWXRW_R__.zip compatibility test confirms this behavior continues working. This is just an observation for the author's awareness — no change needed.

Note

This review was generated by GitHub Copilot.

Note

🔒 Integrity filter blocked 1 item

The following item was blocked because it doesn't meet the GitHub integrity level.

• #94465 issue_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by Code Review for issue #130304 · ● 45.6M · ◷

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.