Skip to content

fix: compute stream package integrity from stream content, not from the destination path#447

Open
mmaietta wants to merge 1 commit into
electron:mainfrom
mmaietta:fix/stream-integrity-fast-path
Open

fix: compute stream package integrity from stream content, not from the destination path#447
mmaietta wants to merge 1 commit into
electron:mainfrom
mmaietta:fix/stream-integrity-fast-path

Conversation

@mmaietta

Copy link
Copy Markdown
Contributor

Summary

  • Fixes a bug introduced in 4.1.2 where createPackageFromStreams could embed the wrong per-file integrity hash in the archive header. The insertFile synchronous fast path computes integrity via fs.readFileSync(p), but for the streams API p is the file's destination path inside the archive (relative), so it resolves against the process CWD. If the CWD happens to contain a file at the same relative path (e.g. package.json when building a project from its root), the header records the hash of that unrelated file while the archive stores the correct bytes from the stream.
  • Apps packed this way and built with the EnableEmbeddedAsarIntegrityValidation fuse fail Electron's load-time integrity check and exit immediately at launch. The corruption is silent at pack time and environment-dependent: when no colliding file exists, the readFileSync throws ENOENT and the code silently falls back to (correct) stream hashing.
  • Fix (src/filesystem.ts, src/asar.ts):
    • insertFile accepts a new internal option fromStream; when set, the readFileSync fast path is skipped entirely and integrity is always computed from streamGenerator() — the only authoritative source for stream content.
    • createPackageFromStreams passes { fromStream: true }.
    • Safety guard in the remaining fast path (files API): the buffer returned by fs.readFileSync(p) is only trusted when fileBuffer.length === size (the size recorded in the header); otherwise it falls through to stream hashing. This protects against stat/read races and any future caller passing a p that isn't the real source file.
  • No performance impact: the stream write path (streamFilesystem) never consumed file.cachedBuffer, so the fast path provided no write-batching benefit for streams — it only produced the (potentially wrong) hash. createPackageFromFiles behavior is unchanged.

Regression test

test/api-spec.tsshould compute stream package integrity from stream content, not from a same-named file in the CWD:

  1. Creates a source file with known content and a decoy file with different content located in the CWD at the same relative path as the archive destination (index.js).
  2. Packs via createPackageFromStreams while chdir'd into the decoy directory (emulating building a project from its root).
  3. Asserts the archived bytes are the stream content and the header integrity hash equals sha256(stream content).

Verified the test fails on unpatched code (header hash matched the decoy's sha256) and passes with the fix. Full suite: yarn lint clean, yarn vitest run 188/188 passing.

…he destination path

## Summary

- Fixes a bug introduced in `4.1.2` where `createPackageFromStreams` could embed the **wrong per-file integrity hash** in the archive header. The `insertFile` synchronous fast path computes integrity via `fs.readFileSync(p)`, but for the streams API `p` is the file's *destination path inside the archive* (relative), so it resolves against the process CWD. If the CWD happens to contain a file at the same relative path (e.g. `package.json` when building a project from its root), the header records the hash of that unrelated file while the archive stores the correct bytes from the stream.
- Apps packed this way and built with the `EnableEmbeddedAsarIntegrityValidation` fuse fail Electron's load-time integrity check and exit immediately at launch. The corruption is silent at pack time and environment-dependent: when no colliding file exists, the `readFileSync` throws `ENOENT` and the code silently falls back to (correct) stream hashing.
- Fix ([src/filesystem.ts](src/filesystem.ts), [src/asar.ts](src/asar.ts)):
  - `insertFile` accepts a new internal option `fromStream`; when set, the `readFileSync` fast path is skipped entirely and integrity is always computed from `streamGenerator()` — the only authoritative source for stream content.
  - `createPackageFromStreams` passes `{ fromStream: true }`.
  - Safety guard in the remaining fast path (files API): the buffer returned by `fs.readFileSync(p)` is only trusted when `fileBuffer.length === size` (the size recorded in the header); otherwise it falls through to stream hashing. This protects against stat/read races and any future caller passing a `p` that isn't the real source file.
- No performance impact: the stream write path (`streamFilesystem`) never consumed `file.cachedBuffer`, so the fast path provided no write-batching benefit for streams — it only produced the (potentially wrong) hash. `createPackageFromFiles` behavior is unchanged.

## Regression test

[test/api-spec.ts](test/api-spec.ts) — `should compute stream package integrity from stream content, not from a same-named file in the CWD`:

1. Creates a source file with known content and a **decoy** file with different content located in the CWD at the same relative path as the archive destination (`index.js`).
2. Packs via `createPackageFromStreams` while `chdir`'d into the decoy directory (emulating building a project from its root).
3. Asserts the archived bytes are the stream content **and** the header integrity hash equals `sha256(stream content)`.

Verified the test fails on unpatched code (header hash matched the decoy's sha256) and passes with the fix. Full suite: `yarn lint` clean, `yarn vitest run` 188/188 passing.
@mmaietta

mmaietta commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Note: I didn't see any previous usage of logging, so I'm curious about the approach/policy there, as I think some of the swallowed errors and/or if-statements could use some (console? debug?) logging if desired.

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.

@electron/asar ≥ 4.1.2: createPackageFromStreams computes per-file integrity from the **destination path / CWD** instead of the stream content

1 participant