Avoid rooting cryptography through ZipArchive on browser by Copilot · Pull Request #130688 · dotnet/runtime

Copilot · 2026-07-14T14:01:07Z

Fixes #122093
Using ZipArchive on browser-wasm unnecessarily retained System.Security.Cryptography, increasing trimmed application size even though WinZip AES is unsupported there.

Changes

Exclude WinZip AES implementation files and the cryptography project reference from browser builds.
Provide browser-specific WinZip AES stubs that continue throwing PlatformNotSupportedException.
Keep ZipCrypto supported by removing its cryptography dependency:
- Generate header randomness with Guid.NewGuid().
- Clear pooled password buffers with Array.Clear.
Add coverage ensuring browser ZipCrypto remains functional and the wasm dependency closure excludes System.Security.Cryptography.

Co-authored-by: alinpahontu2912 <56953855+alinpahontu2912@users.noreply.github.com>

azure-pipelines · 2026-07-14T14:01:47Z

Azure Pipelines: Successfully started running 3 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run.

dotnet-policy-service · 2026-07-15T08:28:42Z

Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara
See info in area-owners.md if you want to be subscribed.

Co-authored-by: alinpahontu2912 <56953855+alinpahontu2912@users.noreply.github.com>

azure-pipelines · 2026-07-15T12:14:06Z

Azure Pipelines: Successfully started running 3 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run.

Copilot

Pull request overview

This PR adjusts System.IO.Compression to avoid pulling in System.Security.Cryptography when using ZipArchive on browser-wasm, by conditionally excluding WinZip AES implementation (unsupported on browser) and removing the cryptography dependency from the ZipCrypto path. It also adds test coverage to help ensure the browser build output no longer shows cryptography-related native interop artifacts.

Changes:

Add browser-only WinZip AES stubs (throwing PlatformNotSupportedException) and conditionally exclude the WinZip AES implementation + cryptography project reference for browser builds.
Remove ZipCrypto’s dependency on System.Security.Cryptography (switch header “randomness” and pooled-buffer clearing implementation).
Add tests validating browser ZipCrypto decryption and asserting the browser wasm build output doesn’t include cryptography-related pinvoke entries.

Reviewed changes

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

Show a summary per file

File	Description
src/mono/wasm/Wasm.Build.Tests/NativeBuildTests.cs	Adds an assertion that the generated pinvoke table doesn’t contain cryptography-related entries for the ZipArchive interop scenario.
src/libraries/System.IO.Compression/tests/ZipArchive/zip_ReadTests.cs	Adds a Browser-specific theory to validate ZipCrypto decryption still works on Browser.
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipCryptoStream.cs	Removes `System.Security.Cryptography` usage by changing buffer clearing and header byte generation.
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs	Broadens encryption-stream locals to `Stream` (supports browser stubbing of WinZip AES type).
src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.Async.cs	Broadens encryption-stream locals to `Stream` for the same reason in async paths.
src/libraries/System.IO.Compression/src/System/IO/Compression/WinZipAes.PlatformNotSupported.cs	Introduces browser-only stubs for WinZip AES APIs that throw `PlatformNotSupportedException`.
src/libraries/System.IO.Compression/src/System.IO.Compression.csproj	Conditions WinZip AES sources + `System.Security.Cryptography` reference away from browser builds; includes browser stub file.
src/libraries/System.IO.Compression/src/Resources/Strings.resx	Minor resource formatting adjustment near the WinZip AES browser PNSE string.

+namespace System.IO.Compression
+{
+    internal readonly struct WinZipAesKeyMaterial;
+
+    internal static class WinZipAesStream
+    {


rzikm

See comment about RNG, I believe we need security/cryptography SMEs to chime in

rzikm · 2026-07-16T12:55:37Z

  <data name="WinZipEncryptionNotSupportedOnBrowser" xml:space="preserve">
    <value>WinZip AES encryption is not supported on the browser platform.</value>
  </data>
+


This whitespace change should be reverted

rzikm · 2026-07-16T13:02:59Z

+            Span<byte> randomBytes = stackalloc byte[16];
+            Guid.NewGuid().TryWriteBytes(randomBytes);
+            randomBytes.Slice(0, 10).CopyTo(header);


Guid does guarantee uniqueness, not randomness, so I don't think this is good.

Since ZipCrypto is broken anyway, maybe using Random.Shared.NextBytes is good enough here to avoid rooting System.Security.Cryptography dll. Maybe keep using RandomNumberGenerator on non-browser platforms?

cc @bartonjs, @GrabYourPitchforks for opinions.

github-actions · 2026-07-19T11:18:53Z

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "84240f3d59928f3f2cff0da1cfe312333d70cf78",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "17bbb207a46d1b2ae3741baa62cb88a98e2e9847",
  "last_reviewed_commit": "84240f3d59928f3f2cff0da1cfe312333d70cf78",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "17bbb207a46d1b2ae3741baa62cb88a98e2e9847",
  "last_recorded_worker_run_id": "29684901530",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "84240f3d59928f3f2cff0da1cfe312333d70cf78",
      "review_id": 4730670134
    }
  ]
}

github-actions

Holistic Review

Motivation: Using ZipArchive on browser-wasm unnecessarily pulled in the entire System.Security.Cryptography assembly, inflating trimmed app size even though WinZip AES encryption is unsupported on that platform. The PR aims to sever that dependency for browser while keeping ZipCrypto functional.

Approach: Two complementary tactics: (1) exclude the WinZip AES source files and the System.Security.Cryptography project reference from browser builds via TargetPlatformIdentifier conditions, substituting WinZipAes.PlatformNotSupported.cs stubs that throw PlatformNotSupportedException; and (2) remove the remaining crypto uses in the always-compiled ZipCryptoStream by replacing CryptographicOperations.ZeroMemory with Array.Clear and RandomNumberGenerator.Fill with Guid.NewGuid(). The call sites in ZipArchiveEntry(.Async).cs are widened from concrete stream types to Stream so they compile against the stubs. Coverage adds a browser ZipCrypto decrypt test plus a Wasm build assertion that System_Security_Cryptography no longer appears in the pinvoke table.

Summary: The change is small, well-scoped, and the platform-conditional stubbing pattern is idiomatic for this repo. The Array.Clear substitution is behaviorally equivalent for zeroing a pooled buffer. The one substantive concern is that RandomNumberGenerator.Fill is replaced by Guid.NewGuid() for the ZipCrypto encryption-header randomness on all platforms, not just browser — this trades a CSPRNG for a source whose cryptographic quality is not guaranteed by contract. Given ZipCrypto's already-weak legacy design this is probably acceptable, but it is a security-relevant change worth an explicit, intentional confirmation (see inline comment). Otherwise this looks good.

Detailed Findings

ZipCrypto header randomness source weakened (inline, ZipCryptoStream.cs line 153): Guid.NewGuid() is not a documented cryptographically-secure RNG and is now used for the header's 10 random bytes on every target. Please confirm the trade-off is intentional, or restrict the fallback to browser while retaining RandomNumberGenerator.Fill elsewhere. Full detail in the inline comment.
Non-blocking (browser stub WinZipAes.PlatformNotSupported.cs): The empty readonly struct WinZipAesKeyMaterial; and stub methods are only referenced through code paths that first throw PlatformNotSupportedException, so the unused _derivedAesKeyMaterial.Value access is never reached on browser. This is fine, but relies on WinZip AES entries always being rejected earlier on browser; the added browser test only exercises ZipCrypto, so consider confirming a browser attempt to read/write a WinZip-AES entry surfaces the PlatformNotSupportedException cleanly rather than a different failure.
Minor (Strings.resx): The diff only adds a blank line before UnsupportedEncryptionMethod; no functional impact.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 65.7 AIC · ⌖ 10.1 AIC · ⊞ 10K

github-actions · 2026-07-19T11:33:51Z

            // bytes 0..9 random
-            RandomNumberGenerator.Fill(header.Slice(0, 10));
+            Span<byte> randomBytes = stackalloc byte[16];
+            Guid.NewGuid().TryWriteBytes(randomBytes);


This replaces a cryptographically-secure RandomNumberGenerator.Fill with Guid.NewGuid() to source the 10 random bytes of the ZipCrypto encryption header. Guid.NewGuid() produces a version-4 (random) GUID, but the .NET documentation explicitly does not guarantee it is generated by a cryptographically-secure RNG, and only 122 of its 128 bits are random (6 bits are fixed version/variant markers). Here you copy randomBytes.Slice(0, 10), i.e. the first 10 bytes, which for a v4 GUID includes the 4 version bits at offset 6 — so those 10 bytes carry slightly less than 80 bits of entropy and their quality is implementation-defined.

For the legacy ZipCrypto scheme (already weak by design) this is likely acceptable in practice, and the goal of dropping the System.Security.Cryptography dependency on browser is reasonable. However, please confirm this trade-off is intentional and documented: the header randomness is what protects against known-plaintext recovery of the key stream, so weakening its source is a security-relevant change. If a stronger source without the crypto dependency is desired, consider seeding from a per-header call that mixes multiple Guid.NewGuid() values, or gating this fallback to the browser target only while keeping RandomNumberGenerator.Fill elsewhere.

Copilot AI and others added 2 commits July 14, 2026 13:30

Exclude ZIP encryption crypto from browser builds

8cf8ad5

Co-authored-by: alinpahontu2912 <56953855+alinpahontu2912@users.noreply.github.com>

Preserve browser ZipCrypto support

c315ffb

Co-authored-by: alinpahontu2912 <56953855+alinpahontu2912@users.noreply.github.com>

Copilot AI self-assigned this Jul 14, 2026

Copilot AI review requested due to automatic review settings July 14, 2026 14:01

Copilot AI assigned alinpahontu2912 Jul 14, 2026

Copilot AI removed the request for review from Copilot July 14, 2026 14:01

Copilot AI changed the title ~~Avoid rooting WinZip AES cryptography on browser~~ Avoid rooting cryptography through ZipArchive on browser Jul 14, 2026

Copilot created this pull request from a session on behalf of alinpahontu2912 July 14, 2026 14:04 View session

Copilot AI requested a review from alinpahontu2912 July 14, 2026 14:04

Copilot finished work on behalf of alinpahontu2912 July 14, 2026 14:04

github-actions Bot added the area-System.IO.Compression label Jul 14, 2026

Copilot started work on behalf of alinpahontu2912 July 15, 2026 08:16 View session

pavelsavara added arch-wasm WebAssembly architecture os-browser Browser variant of arch-wasm labels Jul 15, 2026

pavelsavara temporarily deployed to copilot-pat-pool July 15, 2026 08:28 — with GitHub Actions Inactive

Check WASM P/Invoke table in ZIP test

119fe70

Co-authored-by: alinpahontu2912 <56953855+alinpahontu2912@users.noreply.github.com>

Copilot AI requested review from Copilot and removed request for Copilot July 15, 2026 08:34

Handle both WASM runtime table names

7ad6074

Co-authored-by: alinpahontu2912 <56953855+alinpahontu2912@users.noreply.github.com>

Copilot AI requested review from Copilot and removed request for Copilot July 15, 2026 08:48

Use CoreCLR PInvoke output name

88e7cf0

Co-authored-by: alinpahontu2912 <56953855+alinpahontu2912@users.noreply.github.com>

Copilot AI requested review from Copilot and removed request for Copilot July 15, 2026 08:53

Copilot finished work on behalf of alinpahontu2912 July 15, 2026 08:59

Copilot AI temporarily deployed to copilot-pat-pool July 15, 2026 09:28 Inactive

build-analysis Bot mentioned this pull request Jul 15, 2026

[wasm] OpenQA.Selenium.WebDriverTimeoutException: timeout: Timed out receiving message from renderer #117486

Open

Merge branch 'main' into copilot/fix-ziparchive-password-support

84240f3

Copilot AI review requested due to automatic review settings July 15, 2026 12:12

alinpahontu2912 temporarily deployed to copilot-pat-pool July 15, 2026 12:13 — with GitHub Actions Inactive

Copilot started reviewing on behalf of alinpahontu2912 July 15, 2026 12:13 View session

alinpahontu2912 marked this pull request as ready for review July 15, 2026 12:13

alinpahontu2912 requested review from ilonatommy and maraf as code owners July 15, 2026 12:13

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Comment thread src/libraries/System.IO.Compression/src/System/IO/Compression/WinZipAes.PlatformNotSupported.cs

Comment on lines +7 to +12

namespace System.IO.Compression

{

internal readonly struct WinZipAesKeyMaterial;

internal static class WinZipAesStream

{

This was referenced Jul 15, 2026

browser-wasm linux Release LibraryTests queues timing out #117974

Open

Android WebSocket failure #121518

Open

alinpahontu2912 requested a review from rzikm July 16, 2026 12:09

rzikm requested changes Jul 16, 2026

View reviewed changes

github-actions Bot reviewed Jul 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Avoid rooting cryptography through ZipArchive on browser#130688

Avoid rooting cryptography through ZipArchive on browser#130688
alinpahontu2912 with Copilot wants to merge 6 commits into
mainfrom
copilot/fix-ziparchive-password-support

Copilot AI commented Jul 14, 2026 •

edited by alinpahontu2912

Loading

Uh oh!

azure-pipelines Bot commented Jul 14, 2026

Uh oh!

dotnet-policy-service Bot commented Jul 15, 2026

Uh oh!

azure-pipelines Bot commented Jul 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

rzikm left a comment

Uh oh!

rzikm Jul 16, 2026

Uh oh!

rzikm Jul 16, 2026

Uh oh!

github-actions Bot commented Jul 19, 2026 •

edited

Loading

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot Jul 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

Copilot AI commented Jul 14, 2026 • edited by alinpahontu2912 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

azure-pipelines Bot commented Jul 14, 2026

Uh oh!

dotnet-policy-service Bot commented Jul 15, 2026

Uh oh!

azure-pipelines Bot commented Jul 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

rzikm left a comment

Choose a reason for hiding this comment

Uh oh!

rzikm Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

rzikm Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Holistic Review

Detailed Findings

Uh oh!

github-actions Bot Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Copilot AI commented Jul 14, 2026 •

edited by alinpahontu2912

Loading

github-actions Bot commented Jul 19, 2026 •

edited

Loading