[PR #2700] [MERGED] fix(javascript): preserve ESM path intent and artifact confinement #5325

Closed
opened 2026-10-31 01:05:44 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2700
Author: @rohitg00
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: fix/esm-package-directory-intent


📝 Commits (10+)

  • 0f83944 fix(javascript): preserve legacy ESM directory intent
  • 9bd18e7 Merge upstream main for current ESM resolver verification
  • 49c0023 fix(javascript): normalize legacy ESM URL directory requests
  • e0c41f1 chore: merge upstream validation updates
  • 81e2078 fix(javascript): retain invalid package URL evidence
  • 60c667b chore: merge upstream semantic regression correction
  • 443b404 test(apple): bound cold archive fixture compilation
  • 79ba2a8 chore: merge upstream CI and provenance test fixes
  • e439438 test(release): scope preflight schema to selected jobs
  • 97ec5a5 test(ci): include cold-cache Apple fixture timeout correction

📊 Changes

3 files changed (+89 additions, -4 deletions)

View changed files

📝 src/application/javascript/JavaScriptArtifactPathResolution.ts (+15 -4)
📝 tests/acceptance/applications/packageSpecifierSuffixCli.test.ts (+60 -0)
📝 tests/acceptance/applications/packageUrlUnicodeCli.test.ts (+14 -0)

📄 Description

Summary

Preserve legacy ESM path intent, artifact confinement, and package-map provenance during offline module resolution.

Problem and expected behavior

A request such as legacy/feature.js/. must retain directory intent. A request such as legacy/../../../rea-artifact/outside.js must be rejected before URL normalization erases its escape from the artifact root. An unavailable imports or exports target must retain the selected raw target and map kind.

Unpaired UTF-16 surrogates in ASAR metadata paths must not abort analysis, collide with replacement-character siblings, or mask package-map outcomes that do not require a URL. No dedicated issue was identified for these corrections; the remaining cases were raised in review.

Change and scope

  • Resolve legacy ESM subpaths with URL suffix and directory semantics. Admit preprocessed pathname text before encoded dot segments can disappear.
  • Check relative traversal depth before constructing the synthetic file URL. Preserve permitted parent traversal and significant empty URL segments while rejecting root escape and reentry.
  • Construct package URLs only for relative target selection. Request-local opaque identities preserve structural containment when metadata cannot be represented as a URL. Lookup remains unavailable while an opaque segment remains; traversal beyond it can resolve a representable same-container target.
  • Preserve imports/exports metadata path, map kind, and selected raw target on unavailable outcomes.

Existing upstream integrations and the bounded Apple fixture compiler timeout are retained.

Contract and boundary impact

  • Semantic owner: package URL admission and finalization in JavaScriptArtifactPathResolution, before graph relationships are projected.
  • Contribution scope: correctness within existing offline analysis. A new maintainer authority decision is not applicable.
  • CLI/MCP: no schema or tool-catalog change. Corrected statuses, paths, and limitations flow through both surfaces.
  • Compatibility: CommonJS, package-main probing, and exact package-map keys retain their existing owners.
  • Evidence: raw requests, source identities, selected targets, and unavailable outcomes remain explicit.
  • Authority/cleanup: no new target execution or resource ownership.
  • Generated metadata, dependencies, installation, or migrations: none.

Expected effects:

  • Read existing files or captured data.

Evidence and regression coverage

Existing CLI/stdio MCP fixtures cover loader behavior, suffixes, directory forms, encoded-dot admission, package-map selection, and real ASAR Unicode identities. Added regression source covers synthetic-root escape/reentry, allowed parent traversal, empty segments, query/fragment controls, and imports/exports provenance on unavailable targets.

Source inspection establishes the affected branches and their corrected handling. The new regression cases have not been executed.

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity and source provenance remain preserved.
  • Unsupported and unavailable outcomes remain visible.

Validation performed

Merged current main (cf0a38ad9). The doctorHopperDiscovery CI failure came from the older base and passes after the merge. Executed on Node 24.19.0 (macOS arm64): npm run test:focused -- tests/boundary/cli/doctorHopperDiscovery.test.ts tests/acceptance/applications/packageSpecifierSuffixCli.test.ts tests/acceptance/applications/packageUrlUnicodeCli.test.ts, 7/7 passed through CLI and stdio MCP; pre-push typecheck and lint passed. Independent run by CassieFox23 on Linux, Node 24.18.0 and 26.11.1: the PR test files pass, and 4 of them fail on main. Not run: Windows, Node 22, the full suite, real app ASARs.

Compatibility, safety, and release

No public contract or dependency change. Artifact-root confinement is checked before lossy URL normalization; symlink following or target execution is not introduced. Runtime and cross-platform behavior of the latest changes remain unverified.

Author readiness

Human owner: @rohitg00

  • 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

  • Concrete problem, contribution scope, and semantic owner identified.
  • Regression sources and relevant consumers updated.
  • Final source diff reviewed for unrelated changes and unsupported claims.
  • Latest changes verified by required automated checks.

Maintainer blocker verification

The remaining resolver changes stay within the existing offline artifact inventory: no new symlink following, outside-filesystem lookup, URL fetch, provider, execution or mutation is added. The static-scope routine-fix exception therefore applies. On the exact current head, Node 24.18.0/npm 11.16.0, packageSpecifierSuffixCli.test.ts and packageUrlUnicodeCli.test.ts passed 6/6 through CLI and stdio MCP, and check:fast passed. Coverage includes artifact-root escape/re-entry, Unicode/unrepresentable paths and explicit unavailable outcomes. No target execution was performed.


🔄 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/2700 **Author:** [@rohitg00](https://github.com/rohitg00) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/esm-package-directory-intent` --- ### 📝 Commits (10+) - [`0f83944`](https://github.com/morluto/rea/commit/0f83944c7cdb7122bf8056217732519b603a74a5) fix(javascript): preserve legacy ESM directory intent - [`9bd18e7`](https://github.com/morluto/rea/commit/9bd18e7fb6322aa7ecbb8649d92e039f5a3f1ed5) Merge upstream main for current ESM resolver verification - [`49c0023`](https://github.com/morluto/rea/commit/49c0023885d9756e7b06b1a9edc88bdeead232f8) fix(javascript): normalize legacy ESM URL directory requests - [`e0c41f1`](https://github.com/morluto/rea/commit/e0c41f10f2aa588d360caaabbbb80d766e95a2f8) chore: merge upstream validation updates - [`81e2078`](https://github.com/morluto/rea/commit/81e2078ced336be4b8a945b0ff6e7d3416e6cb53) fix(javascript): retain invalid package URL evidence - [`60c667b`](https://github.com/morluto/rea/commit/60c667b882f591459f03af0146256314655a46f4) chore: merge upstream semantic regression correction - [`443b404`](https://github.com/morluto/rea/commit/443b404caed1391f5348e4b5a21cc586b49bbb8c) test(apple): bound cold archive fixture compilation - [`79ba2a8`](https://github.com/morluto/rea/commit/79ba2a89d7d0a414e4f41b6bbd721e4d2a1cdec5) chore: merge upstream CI and provenance test fixes - [`e439438`](https://github.com/morluto/rea/commit/e4394384a97c5d7cd0d560b546786965a9c283b5) test(release): scope preflight schema to selected jobs - [`97ec5a5`](https://github.com/morluto/rea/commit/97ec5a57f191018a9e581cea1fe747da6ba39d86) test(ci): include cold-cache Apple fixture timeout correction ### 📊 Changes **3 files changed** (+89 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `src/application/javascript/JavaScriptArtifactPathResolution.ts` (+15 -4) 📝 `tests/acceptance/applications/packageSpecifierSuffixCli.test.ts` (+60 -0) 📝 `tests/acceptance/applications/packageUrlUnicodeCli.test.ts` (+14 -0) </details> ### 📄 Description ## Summary Preserve legacy ESM path intent, artifact confinement, and package-map provenance during offline module resolution. ## Problem and expected behavior A request such as `legacy/feature.js/.` must retain directory intent. A request such as `legacy/../../../rea-artifact/outside.js` must be rejected before URL normalization erases its escape from the artifact root. An unavailable imports or exports target must retain the selected raw target and map kind. Unpaired UTF-16 surrogates in ASAR metadata paths must not abort analysis, collide with replacement-character siblings, or mask package-map outcomes that do not require a URL. No dedicated issue was identified for these corrections; the remaining cases were raised in review. ## Change and scope - Resolve legacy ESM subpaths with URL suffix and directory semantics. Admit preprocessed pathname text before encoded dot segments can disappear. - Check relative traversal depth before constructing the synthetic file URL. Preserve permitted parent traversal and significant empty URL segments while rejecting root escape and reentry. - Construct package URLs only for relative target selection. Request-local opaque identities preserve structural containment when metadata cannot be represented as a URL. Lookup remains unavailable while an opaque segment remains; traversal beyond it can resolve a representable same-container target. - Preserve imports/exports metadata path, map kind, and selected raw target on unavailable outcomes. Existing upstream integrations and the bounded Apple fixture compiler timeout are retained. ## Contract and boundary impact - Semantic owner: package URL admission and finalization in JavaScriptArtifactPathResolution, before graph relationships are projected. - Contribution scope: correctness within existing offline analysis. A new maintainer authority decision is not applicable. - CLI/MCP: no schema or tool-catalog change. Corrected statuses, paths, and limitations flow through both surfaces. - Compatibility: CommonJS, package-main probing, and exact package-map keys retain their existing owners. - Evidence: raw requests, source identities, selected targets, and unavailable outcomes remain explicit. - Authority/cleanup: no new target execution or resource ownership. - Generated metadata, dependencies, installation, or migrations: none. Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage Existing CLI/stdio MCP fixtures cover loader behavior, suffixes, directory forms, encoded-dot admission, package-map selection, and real ASAR Unicode identities. Added regression source covers synthetic-root escape/reentry, allowed parent traversal, empty segments, query/fragment controls, and imports/exports provenance on unavailable targets. Source inspection establishes the affected branches and their corrected handling. The new regression cases have not been executed. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity and source provenance remain preserved. - [x] Unsupported and unavailable outcomes remain visible. ## Validation performed Merged current main (cf0a38ad9). The doctorHopperDiscovery CI failure came from the older base and passes after the merge. Executed on Node 24.19.0 (macOS arm64): `npm run test:focused -- tests/boundary/cli/doctorHopperDiscovery.test.ts tests/acceptance/applications/packageSpecifierSuffixCli.test.ts tests/acceptance/applications/packageUrlUnicodeCli.test.ts`, 7/7 passed through CLI and stdio MCP; pre-push typecheck and lint passed. Independent run by CassieFox23 on Linux, Node 24.18.0 and 26.11.1: the PR test files pass, and 4 of them fail on main. Not run: Windows, Node 22, the full suite, real app ASARs. ## Compatibility, safety, and release No public contract or dependency change. Artifact-root confinement is checked before lossy URL normalization; symlink following or target execution is not introduced. Runtime and cross-platform behavior of the latest changes remain unverified. ## Author readiness Human owner: @rohitg00 - [x] 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. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] Concrete problem, contribution scope, and semantic owner identified. - [x] Regression sources and relevant consumers updated. - [x] Final source diff reviewed for unrelated changes and unsupported claims. - [ ] Latest changes verified by required automated checks. <!-- maintainer-blocker-review --> ### Maintainer blocker verification The remaining resolver changes stay within the existing offline artifact inventory: no new symlink following, outside-filesystem lookup, URL fetch, provider, execution or mutation is added. The static-scope routine-fix exception therefore applies. On the exact current head, Node 24.18.0/npm 11.16.0, packageSpecifierSuffixCli.test.ts and packageUrlUnicodeCli.test.ts passed 6/6 through CLI and stdio MCP, and check:fast passed. Coverage includes artifact-root escape/re-entry, Unicode/unrepresentable paths and explicit unavailable outcomes. No target execution was performed. --- <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:44 +00:00
Sign in to join this conversation.
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#5325
No description provided.