morluto/rea

Fork 0

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

[PR #2724] [MERGED] fix(artifacts): bound expanded ASAR listing before allocation #5345

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:05:53 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2724
Author: @chAng-L19
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/asar-expanded-listing-budget

📝 Commits (4)

fed47d2 fix(artifacts): bound expanded ASAR listing before allocation
455908e Merge commit 'c02fb0ea80' into fix/asar-expanded-listing-budget
48d390a Merge commit 'e05d3399f1' into fix/asar-expanded-listing-budget
06b67d5 chore(ci): merge upstream baseline fixes

📊 Changes

6 files changed (+348 additions, -78 deletions)

View changed files

➖ src/artifacts/AsarArtifactReader.test.ts (+0 -25)
📝 src/artifacts/AsarArtifactReader.ts (+21 -53)
➕ src/artifacts/AsarListing.ts (+110 -0)
➕ tests/boundary/filesystem/asarListingAdmission.test.ts (+130 -0)
➕ tests/boundary/mcp/asarListingAdmissionMcp.test.ts (+60 -0)
➕ tests/fixtures/asarHeader.ts (+27 -0)

📄 Description

An ASAR with a 144,264-byte header-only container produces 4,064 entries and 66,345,386 bytes of expanded paths before inventory admission. The serialized-header limit does not cover this representation growth. Deep headers also exhaust the upstream recursive listing stack, and host path normalization can silently discard empty or dot-named members while the inventory reports complete.

This change admits the listing at its producing boundary: traverse the decoded header with an explicit stack and own-property iterators, charge expanded UTF-16 paths plus record overhead against a 64 MiB representation budget before concatenation/retention, and reject unsafe raw components before host normalization. Parse each node once and reuse its metadata for streamed reads; remove recursive listPackage/per-member statFile, duplicated path representations and their superseded helper-only tests. Portable ordering, prototype-named members, links, packed/unpacked reads, integrity checks and cleanup remain covered. Traversal yields for cancellation. The representation budget is not a process RSS ceiling; exceptionally large listings now fail with the existing resource-limit diagnostic.

Existing PR comparison

Checked 1,701 PR titles/bodies and refreshed the newest PRs through #2723 before submission; inspected relevant patches. #2314 adds serialized-header admission tests and does not bound expanded paths. #2690 shares already-produced inventory; #2144 limits decompressed content bytes. Neither covers zero-byte members amplifying path text. Existing #962/#951 streaming/lifecycle and #1867/#581 ordering/path behavior are retained. No equivalent open expanded-listing fix was found.

Validation

Windows x64, Node 24.18.0 / npm 11.16.0, base c02fb0ea8055f98def2b12ad557aed0faefd3639:

npm run test:focused -- tests/boundary/filesystem/asarListingAdmission.test.ts tests/boundary/mcp/asarListingAdmissionMcp.test.ts: 12 passed. Covers real ASAR amplification, 3,500-level packed reads, raw component rejection, Unicode ordering, prototype names, links, cancellation, unchanged input bytes, and SDK MCP failure followed by a successful normal request in the same session.
npm run check: passed.
npm run verify:test-discovery: all 1,044 test files discovered exactly once.
Existing containment/freshness/cleanup, ZIP integrity, archive admission and MCP extraction tests passed. The initial six-file ASAR run had 45 passed / 2 failed / 12 skipped; both failures enumerate Linux /proc/self/fd on Windows. These are recorded, not claimed green.
Broader boundary audit: 143 passed / 8 failed / 6 skipped. Failures are three Windows symlink-creation EPERM cases and five tests assuming POSIX npx registration instead of the Windows cmd.exe registration. These unrelated tests are unchanged.
Official npm production-dependency audit: zero known advisories.

Standalone Node-process resource probe, same input bytes (SHA-256 5b234cb3c2a334d76cba2ff31a1b485e45eb4fce7d75e0cbb5a4e75571e354c4), reader snapshot/enumeration/close measured after imports, OS cache not cleared:

	Before	After
Elapsed	9,951 ms	20 ms
CPU user + system	9,531 ms	16 ms
Peak process RSS	376,788 KiB	58,472 KiB
Published entries	4,064	0 (`limit`)
Published path bytes	66,345,386	0

These are observations from one pair of fresh processes, not guaranteed timings. No LLM token or warm-session costs were measured. POSIX literal-backslash behavior has a conditional regression but was not executed on this Windows host. Full macOS/Linux suites and real GUI providers were not rerun. A reused native artifact passes its manifest/hash check but predates unrelated upstream native changes; current native-source verification is left to its CI lane.

Reproduce the resource/format regressions with the first command above; the checked-in header fixture generates the archive without a binary fixture or recursive fixture serializer.

Both final admission tests and npm run check were rerun after merging the latest base above. Remote head: 455908e03edf2670738904a85fe40d1170d0ef52.

CI failure follow-up (2026-10-13)

The failing shard in both #2491 and #2724 was the same upstream assertion in src/domain/javascript/javascriptSemanticAnalysis.structure.test.ts:74: a shadowing function parameter was expected to be local, although its producing provenance rule retains caller-dependent parameters as unknown.

Reproduced locally before synchronization: 1 failed / 25 passed across the structure and provenance files. Upstream has already fixed the assertion in e05d3399f19f25fdf04285a02b293418256b26dc; this branch now merges that commit and its preceding main history without adding a duplicate fix or altering the production provenance rule.

Verification on the new head 48d390adb53bd86e5cb6a533b648d419004a16e1:

Failing structure/provenance files plus this PR's regressions: 38 passed.
npm run check:ci: passed (including duplication/TODO gates).
PR diff remains confined to the original six files relative to the synchronized main.
New remote CI is being checked separately; the earlier failed runs are retained as historical evidence.

The Codex review summary's tool failure is separate from the assertion failure. It is a review-service status, not a compiler/test finding; the retrieved Greptile review reported no actionable new defect.

_{🔄 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/2724 **Author:** [@chAng-L19](https://github.com/chAng-L19) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/asar-expanded-listing-budget` --- ### 📝 Commits (4) - [`fed47d2`](https://github.com/morluto/rea/commit/fed47d2c56d2c4a3dcd8f2e66ccccdd5768e388f) fix(artifacts): bound expanded ASAR listing before allocation - [`455908e`](https://github.com/morluto/rea/commit/455908e03edf2670738904a85fe40d1170d0ef52) Merge commit 'c02fb0ea8055f98def2b12ad557aed0faefd3639' into fix/asar-expanded-listing-budget - [`48d390a`](https://github.com/morluto/rea/commit/48d390adb53bd86e5cb6a533b648d419004a16e1) Merge commit 'e05d3399f19f25fdf04285a02b293418256b26dc' into fix/asar-expanded-listing-budget - [`06b67d5`](https://github.com/morluto/rea/commit/06b67d5f2d4820ba10fcedbda3b86ced6f678c8a) chore(ci): merge upstream baseline fixes ### 📊 Changes **6 files changed** (+348 additions, -78 deletions) <details> <summary>View changed files</summary> ➖ `src/artifacts/AsarArtifactReader.test.ts` (+0 -25) 📝 `src/artifacts/AsarArtifactReader.ts` (+21 -53) ➕ `src/artifacts/AsarListing.ts` (+110 -0) ➕ `tests/boundary/filesystem/asarListingAdmission.test.ts` (+130 -0) ➕ `tests/boundary/mcp/asarListingAdmissionMcp.test.ts` (+60 -0) ➕ `tests/fixtures/asarHeader.ts` (+27 -0) </details> ### 📄 Description An ASAR with a 144,264-byte header-only container produces 4,064 entries and 66,345,386 bytes of expanded paths before inventory admission. The serialized-header limit does not cover this representation growth. Deep headers also exhaust the upstream recursive listing stack, and host path normalization can silently discard empty or dot-named members while the inventory reports complete. This change admits the listing at its producing boundary: traverse the decoded header with an explicit stack and own-property iterators, charge expanded UTF-16 paths plus record overhead against a 64 MiB representation budget before concatenation/retention, and reject unsafe raw components before host normalization. Parse each node once and reuse its metadata for streamed reads; remove recursive `listPackage`/per-member `statFile`, duplicated path representations and their superseded helper-only tests. Portable ordering, prototype-named members, links, packed/unpacked reads, integrity checks and cleanup remain covered. Traversal yields for cancellation. The representation budget is not a process RSS ceiling; exceptionally large listings now fail with the existing resource-limit diagnostic. ### Existing PR comparison Checked 1,701 PR titles/bodies and refreshed the newest PRs through #2723 before submission; inspected relevant patches. #2314 adds serialized-header admission tests and does not bound expanded paths. #2690 shares already-produced inventory; #2144 limits decompressed content bytes. Neither covers zero-byte members amplifying path text. Existing #962/#951 streaming/lifecycle and #1867/#581 ordering/path behavior are retained. No equivalent open expanded-listing fix was found. ### Validation Windows x64, Node 24.18.0 / npm 11.16.0, base `c02fb0ea8055f98def2b12ad557aed0faefd3639`: - `npm run test:focused -- tests/boundary/filesystem/asarListingAdmission.test.ts tests/boundary/mcp/asarListingAdmissionMcp.test.ts`: 12 passed. Covers real ASAR amplification, 3,500-level packed reads, raw component rejection, Unicode ordering, prototype names, links, cancellation, unchanged input bytes, and SDK MCP failure followed by a successful normal request in the same session. - `npm run check`: passed. - `npm run verify:test-discovery`: all 1,044 test files discovered exactly once. - Existing containment/freshness/cleanup, ZIP integrity, archive admission and MCP extraction tests passed. The initial six-file ASAR run had 45 passed / 2 failed / 12 skipped; both failures enumerate Linux `/proc/self/fd` on Windows. These are recorded, not claimed green. - Broader boundary audit: 143 passed / 8 failed / 6 skipped. Failures are three Windows symlink-creation `EPERM` cases and five tests assuming POSIX `npx` registration instead of the Windows `cmd.exe` registration. These unrelated tests are unchanged. - Official npm production-dependency audit: zero known advisories. Standalone Node-process resource probe, same input bytes (SHA-256 `5b234cb3c2a334d76cba2ff31a1b485e45eb4fce7d75e0cbb5a4e75571e354c4`), reader snapshot/enumeration/close measured after imports, OS cache not cleared: | | Before | After | |---|---:|---:| | Elapsed | 9,951 ms | 20 ms | | CPU user + system | 9,531 ms | 16 ms | | Peak process RSS | 376,788 KiB | 58,472 KiB | | Published entries | 4,064 | 0 (`limit`) | | Published path bytes | 66,345,386 | 0 | These are observations from one pair of fresh processes, not guaranteed timings. No LLM token or warm-session costs were measured. POSIX literal-backslash behavior has a conditional regression but was not executed on this Windows host. Full macOS/Linux suites and real GUI providers were not rerun. A reused native artifact passes its manifest/hash check but predates unrelated upstream native changes; current native-source verification is left to its CI lane. Reproduce the resource/format regressions with the first command above; the checked-in header fixture generates the archive without a binary fixture or recursive fixture serializer. Both final admission tests and `npm run check` were rerun after merging the latest base above. Remote head: `455908e03edf2670738904a85fe40d1170d0ef52`. ### CI failure follow-up (2026-10-13) The failing shard in both #2491 and #2724 was the same upstream assertion in `src/domain/javascript/javascriptSemanticAnalysis.structure.test.ts:74`: a shadowing function parameter was expected to be `local`, although its producing provenance rule retains caller-dependent parameters as `unknown`. Reproduced locally before synchronization: 1 failed / 25 passed across the structure and provenance files. Upstream has already fixed the assertion in `e05d3399f19f25fdf04285a02b293418256b26dc`; this branch now merges that commit and its preceding main history without adding a duplicate fix or altering the production provenance rule. Verification on the new head `48d390adb53bd86e5cb6a533b648d419004a16e1`: - Failing structure/provenance files plus this PR's regressions: 38 passed. - `npm run check:ci`: passed (including duplication/TODO gates). - PR diff remains confined to the original six files relative to the synchronized main. - New remote CI is being checked separately; the earlier failed runs are retained as historical evidence. The Codex review summary's tool failure is separate from the assertion failure. It is a review-service status, not a compiler/test finding; the retrieved Greptile review reported no actionable new defect. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:05:53 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:45:13 +00:00

[PR #5813] [MERGED] feat(gpu): inspect static CUDA and HIP code objects #7079

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#5345

No description provided.

Rows
Columns