morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #3658] [CLOSED] fix(artifacts): recognize leading JSON whitespace in ASAR headers #5866

New issue

Closed

opened 2026-10-31 01:10:03 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:10:03 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3658
Author: @Potatoyz908
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: codex/fix-asar-header-whitespace

📝 Commits (1)

3a696da fix(artifacts): recognize leading JSON whitespace in ASAR headers

📊 Changes

9 files changed (+271 additions, -16 deletions)

View changed files

📝 src/artifacts/ArtifactHash.ts (+10 -1)
➕ src/artifacts/AsarSignature.test.ts (+50 -0)
➕ src/artifacts/AsarSignature.ts (+47 -0)
📝 src/artifacts/inventory/ArtifactGraphConstruction.ts (+6 -13)
📝 src/artifacts/inventory/classify.ts (+9 -1)
📝 src/artifacts/inventory/scanReader.ts (+6 -1)
➕ tests/acceptance/asarHeaderWhitespace.test.ts (+50 -0)
➕ tests/boundary/filesystem/asarHeaderWhitespace.test.ts (+62 -0)
➕ tests/fixtures/asarWhitespace.ts (+31 -0)

📄 Description

Summary

Fixes morluto/rea#3574.

Valid ASAR archives with JSON whitespace before the header object were classified as ordinary files, losing their member inventory and preventing JavaScript application analysis. These archives now retain their members and content digests through inventory, the CLI, and stdio MCP.

Problem and expected behavior

The signature required { at byte 16, although the upstream ASAR decoder accepts JSON whitespace before the object. A leading space, tab, CR, or LF must not change the supported archive's identity.

Change and scope

An incremental signature observer recognizes only RFC 8259 whitespace within the declared pickle string extent. The existing hashing pass supplies the observation to root and nested classification, including when whitespace extends beyond the 8192-byte retained prefix. It retains only the 16 fixed bytes and scalar state, adds no file reads, and stops at the first non-whitespace byte or the declared string boundary. Full decode, header admission limits, member integrity checks, and cleanup remain with their existing owners.

Contract and boundary impact

Semantic owner / earliest stage: shared artifact signature observation during hashing and inventory classification.
Contribution scope: ordinary static archive-format bug fix; a separate maintainer scope decision is not applicable.
CLI/MCP schemas, generated metadata, dependencies, packaging, and installation: unchanged.
Compatibility: accepts additional valid serialization of an already supported ASAR object header.
Evidence: member occurrences and hashes are preserved; invalid JSON and corrupted member bodies still fail.
Target effects: reads the selected archive; does not execute fixture JavaScript.

Expected effects:

Read existing files or captured data.

Evidence and regression coverage

Official @electron/asar producer fixtures are reserialized without changing member bytes; its header, listing, and extraction APIs confirm equivalent content.
Compact, space, LF, tab, CR, and 12 KB mixed-whitespace headers are tested as roots and nested archives. Member SHA-256 values and unchanged carrier bytes are asserted.
Signature tests cover byte-at-a-time chunks, the retained-prefix bound, invalid whitespace, non-object values, contradictory extents, and padding/member bytes outside the declared JSON string.
Existing header-admission and containment tests preserve limits, malformed-header handling, and integrity behavior.
On base a732c859d, the new filesystem regression file failed six cases, while its compact-header control passed. Whitespace variants produced root_format: file; the corrected implementation passes.
Compiled CLI inventory now reports root_format: asar with ., main.js, and package.json. JavaScript analysis succeeds through the compiled CLI and real stdio MCP with both auto and asar for space, LF, and long-whitespace variants.

Validation performed

Executed on Linux x64 with Node 24.18.0, npm 11.16.0, and locked dependencies:

npm run test:focused -- src/artifacts/AsarSignature.test.ts tests/boundary/filesystem/asarHeaderWhitespace.test.ts tests/boundary/filesystem/asarHeaderAdmission.test.ts tests/boundary/filesystem/asarInventoryContainment.test.ts src/artifacts/inventory/ArtifactGraphConstruction.test.ts — passed.
npm run test:focused -- tests/acceptance/asarHeaderWhitespace.test.ts — all three CLI/stdio MCP cases passed.
npm run check — passed type checking, lint, formatting, unused-code checks, metadata, module boundaries, and architecture guards.
npm run check:fast — passed.
npm run verify:test-discovery — all 1289 test files discovered exactly once.
git diff --check — passed.

CLI/MCP and static subprocess checks required execution outside the local tool sandbox. No installed-package, Windows/macOS, Electron GUI, native-engine, new cancellation, process-family-quiescence, or comparative performance/token measurements are claimed. No breaking changes or migrations.

Author readiness

Human owner: @Potatoyz908

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem and has one focused outcome.
Related issue and executed validation are recorded above.
Human ownership and initial review readiness are declared.
Required CI checks pass on the current PR revision.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/3658 **Author:** [@Potatoyz908](https://github.com/Potatoyz908) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `codex/fix-asar-header-whitespace` --- ### 📝 Commits (1) - [`3a696da`](https://github.com/morluto/rea/commit/3a696da08f120dbb0334a94c80128cf132a4240a) fix(artifacts): recognize leading JSON whitespace in ASAR headers ### 📊 Changes **9 files changed** (+271 additions, -16 deletions) <details> <summary>View changed files</summary> 📝 `src/artifacts/ArtifactHash.ts` (+10 -1) ➕ `src/artifacts/AsarSignature.test.ts` (+50 -0) ➕ `src/artifacts/AsarSignature.ts` (+47 -0) 📝 `src/artifacts/inventory/ArtifactGraphConstruction.ts` (+6 -13) 📝 `src/artifacts/inventory/classify.ts` (+9 -1) 📝 `src/artifacts/inventory/scanReader.ts` (+6 -1) ➕ `tests/acceptance/asarHeaderWhitespace.test.ts` (+50 -0) ➕ `tests/boundary/filesystem/asarHeaderWhitespace.test.ts` (+62 -0) ➕ `tests/fixtures/asarWhitespace.ts` (+31 -0) </details> ### 📄 Description ## Summary Fixes morluto/rea#3574. Valid ASAR archives with JSON whitespace before the header object were classified as ordinary files, losing their member inventory and preventing JavaScript application analysis. These archives now retain their members and content digests through inventory, the CLI, and stdio MCP. ## Problem and expected behavior The signature required `{` at byte 16, although the upstream ASAR decoder accepts JSON whitespace before the object. A leading space, tab, CR, or LF must not change the supported archive's identity. ## Change and scope An incremental signature observer recognizes only RFC 8259 whitespace within the declared pickle string extent. The existing hashing pass supplies the observation to root and nested classification, including when whitespace extends beyond the 8192-byte retained prefix. It retains only the 16 fixed bytes and scalar state, adds no file reads, and stops at the first non-whitespace byte or the declared string boundary. Full decode, header admission limits, member integrity checks, and cleanup remain with their existing owners. ## Contract and boundary impact - Semantic owner / earliest stage: shared artifact signature observation during hashing and inventory classification. - Contribution scope: ordinary static archive-format bug fix; a separate maintainer scope decision is not applicable. - CLI/MCP schemas, generated metadata, dependencies, packaging, and installation: unchanged. - Compatibility: accepts additional valid serialization of an already supported ASAR object header. - Evidence: member occurrences and hashes are preserved; invalid JSON and corrupted member bodies still fail. - Target effects: reads the selected archive; does not execute fixture JavaScript. Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage - Official `@electron/asar` producer fixtures are reserialized without changing member bytes; its header, listing, and extraction APIs confirm equivalent content. - Compact, space, LF, tab, CR, and 12 KB mixed-whitespace headers are tested as roots and nested archives. Member SHA-256 values and unchanged carrier bytes are asserted. - Signature tests cover byte-at-a-time chunks, the retained-prefix bound, invalid whitespace, non-object values, contradictory extents, and padding/member bytes outside the declared JSON string. - Existing header-admission and containment tests preserve limits, malformed-header handling, and integrity behavior. - On base `a732c859d`, the new filesystem regression file failed six cases, while its compact-header control passed. Whitespace variants produced `root_format: file`; the corrected implementation passes. - Compiled CLI inventory now reports `root_format: asar` with `.`, `main.js`, and `package.json`. JavaScript analysis succeeds through the compiled CLI and real stdio MCP with both `auto` and `asar` for space, LF, and long-whitespace variants. ## Validation performed Executed on Linux x64 with Node 24.18.0, npm 11.16.0, and locked dependencies: - `npm run test:focused -- src/artifacts/AsarSignature.test.ts tests/boundary/filesystem/asarHeaderWhitespace.test.ts tests/boundary/filesystem/asarHeaderAdmission.test.ts tests/boundary/filesystem/asarInventoryContainment.test.ts src/artifacts/inventory/ArtifactGraphConstruction.test.ts` — passed. - `npm run test:focused -- tests/acceptance/asarHeaderWhitespace.test.ts` — all three CLI/stdio MCP cases passed. - `npm run check` — passed type checking, lint, formatting, unused-code checks, metadata, module boundaries, and architecture guards. - `npm run check:fast` — passed. - `npm run verify:test-discovery` — all 1289 test files discovered exactly once. - `git diff --check` — passed. CLI/MCP and static subprocess checks required execution outside the local tool sandbox. No installed-package, Windows/macOS, Electron GUI, native-engine, new cancellation, process-family-quiescence, or comparative performance/token measurements are claimed. No breaking changes or migrations. ## Author readiness Human owner: @Potatoyz908 - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem and has one focused outcome. - [x] Related issue and executed validation are recorded above. - [ ] Human ownership and initial review readiness are declared. - [ ] Required CI checks pass on the current PR revision. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:10:03 +00:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#5866

No description provided.

Rows
Columns