[PR #2347] [MERGED] fix(javascript): keep ASAR-named files without an ASAR header opaque #5180

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:04:35 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2347
Author: @huiq777
Created: 10/12/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/javascript-opaque-asar-files

📝 Commits (2)

3f39d8c fix(javascript): keep ASAR-named files without an ASAR header opaque
bfb8ec5 chore(ci): merge current main verification fixes

📊 Changes

2 files changed (+34 additions, -0 deletions)

View changed files

📝 src/artifacts/javascript/JavaScriptArtifactFiles.ts (+6 -0)
📝 tests/boundary/filesystem/javascriptIntegrityPolicy.test.ts (+28 -0)

📄 Description

Summary

analyze_javascript_application fails outright on a directory that contains any file named *.asar whose bytes are not an ASAR. Inventory already treats such a file as ordinary bytes; JavaScript reconstruction expanded it anyway because it only looked at the suffix.

No issue is linked: I found this while comparing how inventory and reconstruction decide what a nested container is, and it is an ordinary fix inside the existing analysis.

Problem and expected behavior

Smallest trigger: a directory with main.js and a 14-byte ordinary.asar containing ordinary bytes.

rea analyze-javascript-application <dir> --artifact-format directory --format json

fails with:

{ "code": "artifact_operation_failed",
  "details": { "operation": "analyze_javascript_application", "reason": "format",
               "detail": "Truncated or invalid ASAR size pickle" } }

Renaming the same bytes to ordinary.bin makes the same command succeed.

The two owners disagree about that file:

Inventory (src/artifacts/inventory/scanReader.ts) expands an ASAR-named filesystem file only when digested.classification.format === "asar". Otherwise it records an ordinary verified occurrence. tests/boundary/filesystem/asarInventoryContainment.test.ts pins this as "records filesystem files without an ASAR header as ordinary bytes".
Reconstruction (src/artifacts/javascript/JavaScriptArtifactFiles.ts) registered a container for every *.asar occurrence and, once its hash was verified, opened it with AsarArtifactReader. Its own comment says "Inventory deliberately does not expand unverified or unavailable nested archives. Keep the same boundary here", but the format check was missing.

Expected: the file stays opaque and the rest of the directory is analyzed. An Electron app that ships an encrypted or otherwise non-standard app.asar beside readable sources is a realistic case.

Change and scope

ExpectedContainer now carries the inventory occurrence's artifact_format.
In visitReader, a verified ASAR-named file whose inventory format is not asar is skipped instead of expanded. Reconstruction uses inventory's verdict rather than re-deciding from the name.
The existing refusals are unchanged: a file missing from inventory is still artifact drift, and mismatched, unavailable and unverified containers behave as before.

Not included: no change to inventory, to the path registry, or to how real nested ASARs are read.

Follow the contribution scope.
Scope decision: not applicable. This narrows what an existing static analysis opens.

Contract and boundary impact

Semantic owner and earliest changed stage: inventory owns the container classification; src/artifacts/javascript/JavaScriptArtifactFiles.ts now consumes it.
Contribution scope fit and linked maintainer decision: static analysis within existing boundaries; not applicable.
CLI and MCP/tool-catalog contract: no schema change. An input that used to fail with artifact_operation_failed now returns its analysis.
Provider, bridge, target-format, or platform compatibility: JavaScript application analysis of directory inputs and unpacked ASAR companions; real nested ASARs are unaffected.
Evidence, artifact, provenance, or reconstruction contract: the opaque file keeps its ordinary inventory occurrence; it contributes no container and no members.
Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated: one case in tests/boundary/filesystem/javascriptIntegrityPolicy.test.ts. It reads the directory through readJavaScriptArtifactFiles (files are ["main.js"], no containers) and through analyzeJavaScriptApplication (nested_asar_containers is 0, no integrity contradictions).
Base reproduction or other evidence (executed): without the fix the new test fails, and the CLI command above returns the Truncated or invalid ASAR size pickle error shown.
User-visible CLI/MCP output (executed): after the fix the same CLI command returns the analysis Evidence with statistics.nested_asar_containers: 0 and an empty integrity_contradictions.
Remaining proof gaps: not exercised with a real Electron application that ships a non-ASAR app.asar; the fixture uses plain bytes.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

macOS 15.6.1 arm64, Node 24.18.0, npm 11.16.0.

npm run test:focused -- tests/boundary/filesystem/javascriptIntegrityPolicy.test.ts — 5 passed (rerun after rebasing onto 4dd44dd)
npm run test:focused over the 40 boundary and acceptance test files that mention nested containers, .asar or JavaScript artifact files — 40 files, 516 tests passed
npm run check — passed
npm run check:fast — passed (rerun after the rebase)

Not run: npm run check:pr and npm run docs:check (no contract or generated metadata changes).

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable.
Package or release metadata impact: none.
Security, privacy, process, or containment review: reconstruction now opens fewer files than before; nothing new is read.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

AI assistance (Claude Code, model Claude Opus 5.5) was used for investigation, implementation and running the tests locally.

🤖 Generated with Claude Code

_{🔄 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/2347 **Author:** [@huiq777](https://github.com/huiq777) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/javascript-opaque-asar-files` --- ### 📝 Commits (2) - [`3f39d8c`](https://github.com/morluto/rea/commit/3f39d8cf4ddd88c538a01e36aadc875250f89e7d) fix(javascript): keep ASAR-named files without an ASAR header opaque - [`bfb8ec5`](https://github.com/morluto/rea/commit/bfb8ec500948129a77841784d578a416e7ce2c7e) chore(ci): merge current main verification fixes ### 📊 Changes **2 files changed** (+34 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/artifacts/javascript/JavaScriptArtifactFiles.ts` (+6 -0) 📝 `tests/boundary/filesystem/javascriptIntegrityPolicy.test.ts` (+28 -0) </details> ### 📄 Description ## Summary `analyze_javascript_application` fails outright on a directory that contains any file named `*.asar` whose bytes are not an ASAR. Inventory already treats such a file as ordinary bytes; JavaScript reconstruction expanded it anyway because it only looked at the suffix. No issue is linked: I found this while comparing how inventory and reconstruction decide what a nested container is, and it is an ordinary fix inside the existing analysis. ## Problem and expected behavior Smallest trigger: a directory with `main.js` and a 14-byte `ordinary.asar` containing `ordinary bytes`. ```text rea analyze-javascript-application <dir> --artifact-format directory --format json ``` fails with: ```json { "code": "artifact_operation_failed", "details": { "operation": "analyze_javascript_application", "reason": "format", "detail": "Truncated or invalid ASAR size pickle" } } ``` Renaming the same bytes to `ordinary.bin` makes the same command succeed. The two owners disagree about that file: - Inventory (`src/artifacts/inventory/scanReader.ts`) expands an ASAR-named filesystem file only when `digested.classification.format === "asar"`. Otherwise it records an ordinary verified occurrence. `tests/boundary/filesystem/asarInventoryContainment.test.ts` pins this as "records filesystem files without an ASAR header as ordinary bytes". - Reconstruction (`src/artifacts/javascript/JavaScriptArtifactFiles.ts`) registered a container for every `*.asar` occurrence and, once its hash was verified, opened it with `AsarArtifactReader`. Its own comment says "Inventory deliberately does not expand unverified or unavailable nested archives. Keep the same boundary here", but the format check was missing. Expected: the file stays opaque and the rest of the directory is analyzed. An Electron app that ships an encrypted or otherwise non-standard `app.asar` beside readable sources is a realistic case. ## Change and scope - `ExpectedContainer` now carries the inventory occurrence's `artifact_format`. - In `visitReader`, a verified ASAR-named file whose inventory format is not `asar` is skipped instead of expanded. Reconstruction uses inventory's verdict rather than re-deciding from the name. - The existing refusals are unchanged: a file missing from inventory is still artifact drift, and mismatched, unavailable and unverified containers behave as before. Not included: no change to inventory, to the path registry, or to how real nested ASARs are read. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Scope decision: not applicable. This narrows what an existing static analysis opens. ## Contract and boundary impact - Semantic owner and earliest changed stage: inventory owns the container classification; `src/artifacts/javascript/JavaScriptArtifactFiles.ts` now consumes it. - Contribution scope fit and linked maintainer decision: static analysis within existing boundaries; not applicable. - CLI and MCP/tool-catalog contract: no schema change. An input that used to fail with `artifact_operation_failed` now returns its analysis. - Provider, bridge, target-format, or platform compatibility: JavaScript application analysis of directory inputs and unpacked ASAR companions; real nested ASARs are unaffected. - Evidence, artifact, provenance, or reconstruction contract: the opaque file keeps its ordinary inventory occurrence; it contributes no container and no members. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: one case in `tests/boundary/filesystem/javascriptIntegrityPolicy.test.ts`. It reads the directory through `readJavaScriptArtifactFiles` (files are `["main.js"]`, no containers) and through `analyzeJavaScriptApplication` (`nested_asar_containers` is 0, no integrity contradictions). - Base reproduction or other evidence (executed): without the fix the new test fails, and the CLI command above returns the `Truncated or invalid ASAR size pickle` error shown. - User-visible CLI/MCP output (executed): after the fix the same CLI command returns the analysis Evidence with `statistics.nested_asar_containers: 0` and an empty `integrity_contradictions`. - Remaining proof gaps: not exercised with a real Electron application that ships a non-ASAR `app.asar`; the fixture uses plain bytes. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed macOS 15.6.1 arm64, Node 24.18.0, npm 11.16.0. - `npm run test:focused -- tests/boundary/filesystem/javascriptIntegrityPolicy.test.ts` — 5 passed (rerun after rebasing onto `4dd44dd`) - `npm run test:focused` over the 40 boundary and acceptance test files that mention nested containers, `.asar` or JavaScript artifact files — 40 files, 516 tests passed - `npm run check` — passed - `npm run check:fast` — passed (rerun after the rebase) Not run: `npm run check:pr` and `npm run docs:check` (no contract or generated metadata changes). ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. - Package or release metadata impact: none. - Security, privacy, process, or containment review: reconstruction now opens fewer files than before; nothing new is read. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. AI assistance (Claude Code, model Claude Opus 5.5) was used for investigation, implementation and running the tests locally. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:04:35 +00:00