morluto/rea

Fork 0

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

[PR #1031] [MERGED] fix(javascript): resolve module URLs and reject invalid exports targets #2595

New issue

Closed

opened 2026-10-17 21:08:01 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-17 21:08:01 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1031
Author: @rudycelekli
Created: 10/7/2026
Status: ✅ Merged
Merged: 10/8/2026
Merged by: @N0zoM1z0

Base: main ← Head: fix/esm-module-url-decoding

📝 Commits (7)

1b86ae4 fix(javascript): decode ESM module URL paths
cf0ab1a chore: integrate current upstream export comparison correction
baa17cc fix(javascript): decode package exports URL targets
331c85b chore: integrate tested main before package URL validation
7aa89a9 test(javascript): compare package target admission with Node
56b013e fix(javascript): validate exports targets before URL normalization
912a44b test(javascript): distinguish invalid exports from malformed JSON

📊 Changes

5 files changed (+591 additions, -25 deletions)

View changed files

📝 docs/javascript-artifact-reconstruction.md (+9 -0)
📝 src/application/javascript/JavaScriptArtifactPathResolution.ts (+92 -20)
📝 src/application/javascript/JavaScriptModuleSpecifierPaths.test.ts (+245 -0)
➕ tests/boundary/filesystem/javascriptPackageExportTargets.test.ts (+231 -0)
📝 tests/boundary/javascript/packageConditionalExports.test.ts (+14 -5)

📄 Description

Summary

Fixes #1040.

Resolve ESM relative module URLs and selected package exports targets to the decoded artifact filename, then reject invalid exports targets before normalization can point them at another package asset. The original contributor implementation is retained; maintainer follow-ups add raw-target validation, actionable diagnostics, regressions, and integration with tested main.

Problem and expected behavior

exports: "./../outside.cjs" previously produced a resolved relationship to node_modules/outside.cjs, although Node rejects that target with ERR_INVALID_PACKAGE_TARGET. Encoding also confused literal filenames with URL paths: ./actual%20file.cjs under exports must select actual file.cjs, while a legacy main field preserves the literal percent filename.

Change and scope

Decode relative ESM URLs and selected exports targets once, after query/fragment handling. Preserve literal CommonJS relative paths and legacy main fields.
Require exports targets to start with ./; reject raw ., .., and node_modules segments, including encoded forms, before path normalization.
Apply the same target validation to direct, root-subpath, conditional, and array entries. Arrays skip invalid package targets in declared order, retaining final invalid/null behavior. A selected malformed URL or missing file does not trigger selection of another entry.
Return resolution_status: "rejected" with the package metadata path, original target, failed constraint, and importing source location. Malformed package JSON remains a separate unavailable outcome.
Document the supported local inventory behavior; existing canonical-path admission and container confinement remain in effect.

Contract and boundary impact

Semantic owner and earliest changed stage: application artifact path resolution, with producer-specific package metadata validation before normalization.
CLI and MCP/tool-catalog contract: existing tool schemas; corrected shared-workflow module graph identities and explicit rejected-target diagnostics.
Provider, bridge, target-format, or platform compatibility: inventoried JavaScript artifacts and root package exports; provider adapters unchanged.
Evidence, artifact, provenance, or reconstruction contract: module relationships remain inferred; declarations, source digests/ranges, and failed-constraint limitations are retained.
Process execution, authorization, cleanup, or containment impact: analysis does not execute target code. Tests execute owned Node fixture modules and remove their workspaces.
Generated metadata, package, or installation impact: no dependency or catalog change; generated documents checked through their normal generators.

Evidence and regression coverage

Five focused files pass 197 tests, including the contributor's 49 URL-path regressions and 60 new real Node import/require target-selection cases.
The new boundary regression fails 44 cases on the integrated contributor base before the target-admission follow-up; 16 controls pass there.
Actual CLI and stdio MCP return identical normalized results in 28 cases, matching real Node 24.18.0 import/require outcomes. Examples cover encoded names, literal main fields, prohibited parent segments, nested arrays, malformed selected URLs, and missing first targets.
Representative retained diagnostic for specifier: "fixture":

{
  "resolved_path": null,
  "resolution_status": "rejected",
  "limitations": [
    "Package metadata node_modules/fixture/package.json: The package exports target \"./../outside.cjs\" contains forbidden path segment \"..\" before URL normalization."
  ]
}

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

Node 24.18.0/npm 11.16.0, Linux x64, fresh normal locked installation.

npm run build:cached — passed.
Five focused test files — 197 passed.
npm run typecheck, npm run lint, npm run docs:check — passed.
Actual CLI/stdio MCP + real Node fixture verification — 28 cases passed.
npm run verify:package — passed with normal isolated tarball installation, CLI/setup lifecycle checks, and all 134 MCP tools.
Normal Windows tarball installation — passed on Node 22.19.0/npm 10.9.3. Actual CLI/stdio MCP results match in 29 cases, including 22 real Node loader comparisons and 7 HTML regressions; all 134 tools are advertised.
Exact-head hosted CI — all 22 checks passed; generated metadata skipped. Linux x64/arm64, macOS x64/arm64, and Windows package lanes passed.
Normal pre-commit/pre-push hooks — passed without bypassing checks.

Compatibility, safety, and release

No breaking schemas or migration. The graph now rejects relationships based on package targets that Node refuses. This is bounded root exports/URL-path coverage; package subpath maps and other unmodeled loader behavior retain existing limitations. Real Hopper/Ghidra verification was not performed because their adapters are unchanged. Local verification uses isolated owned fixtures, reduced CPU priority, memory monitoring, and storage cleanup.

Review checklist

One focused module-path outcome and a Conventional Commit title.
Related issue linked.
Real producer regressions and failure reasons covered.
Owning guide and generated-document checks updated.
Representative CLI/MCP behavior and evidence described.
Final diff checked for host paths, credentials, unrelated changes, and generated catalog tracking.

Prepared with Codex assistance. Contributor ancestry is preserved; additional commit and squash co-author identities use GitHub noreply addresses.

_{🔄 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/1031 **Author:** [@rudycelekli](https://github.com/rudycelekli) **Created:** 10/7/2026 **Status:** ✅ Merged **Merged:** 10/8/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/esm-module-url-decoding` --- ### 📝 Commits (7) - [`1b86ae4`](https://github.com/morluto/rea/commit/1b86ae46058f0c377b1eb824c0a43acbbf9b2827) fix(javascript): decode ESM module URL paths - [`cf0ab1a`](https://github.com/morluto/rea/commit/cf0ab1ad8847eb9d18396332aa27f3f14b85c0c3) chore: integrate current upstream export comparison correction - [`baa17cc`](https://github.com/morluto/rea/commit/baa17cc523570f5717e7e2668f07a5e7f88f00dc) fix(javascript): decode package exports URL targets - [`331c85b`](https://github.com/morluto/rea/commit/331c85bf3dbd7664b59d41a4b449b9569d7dd932) chore: integrate tested main before package URL validation - [`7aa89a9`](https://github.com/morluto/rea/commit/7aa89a959f2ec0e8fd340d7509c834fa12d2dfde) test(javascript): compare package target admission with Node - [`56b013e`](https://github.com/morluto/rea/commit/56b013ea344175b472764c6961ebab372c280982) fix(javascript): validate exports targets before URL normalization - [`912a44b`](https://github.com/morluto/rea/commit/912a44b65ec41774dff0693decab8c007494bfed) test(javascript): distinguish invalid exports from malformed JSON ### 📊 Changes **5 files changed** (+591 additions, -25 deletions) <details> <summary>View changed files</summary> 📝 `docs/javascript-artifact-reconstruction.md` (+9 -0) 📝 `src/application/javascript/JavaScriptArtifactPathResolution.ts` (+92 -20) 📝 `src/application/javascript/JavaScriptModuleSpecifierPaths.test.ts` (+245 -0) ➕ `tests/boundary/filesystem/javascriptPackageExportTargets.test.ts` (+231 -0) 📝 `tests/boundary/javascript/packageConditionalExports.test.ts` (+14 -5) </details> ### 📄 Description ## Summary Fixes #1040. Resolve ESM relative module URLs and selected package exports targets to the decoded artifact filename, then reject invalid exports targets before normalization can point them at another package asset. The original contributor implementation is retained; maintainer follow-ups add raw-target validation, actionable diagnostics, regressions, and integration with tested main. ## Problem and expected behavior `exports: "./../outside.cjs"` previously produced a resolved relationship to `node_modules/outside.cjs`, although Node rejects that target with `ERR_INVALID_PACKAGE_TARGET`. Encoding also confused literal filenames with URL paths: `./actual%20file.cjs` under exports must select `actual file.cjs`, while a legacy main field preserves the literal percent filename. ## Change and scope - Decode relative ESM URLs and selected exports targets once, after query/fragment handling. Preserve literal CommonJS relative paths and legacy main fields. - Require exports targets to start with `./`; reject raw `.`, `..`, and `node_modules` segments, including encoded forms, before path normalization. - Apply the same target validation to direct, root-subpath, conditional, and array entries. Arrays skip invalid package targets in declared order, retaining final invalid/null behavior. A selected malformed URL or missing file does not trigger selection of another entry. - Return `resolution_status: "rejected"` with the package metadata path, original target, failed constraint, and importing source location. Malformed package JSON remains a separate unavailable outcome. - Document the supported local inventory behavior; existing canonical-path admission and container confinement remain in effect. ## Contract and boundary impact - Semantic owner and earliest changed stage: application artifact path resolution, with producer-specific package metadata validation before normalization. - CLI and MCP/tool-catalog contract: existing tool schemas; corrected shared-workflow module graph identities and explicit rejected-target diagnostics. - Provider, bridge, target-format, or platform compatibility: inventoried JavaScript artifacts and root package exports; provider adapters unchanged. - Evidence, artifact, provenance, or reconstruction contract: module relationships remain inferred; declarations, source digests/ranges, and failed-constraint limitations are retained. - Process execution, authorization, cleanup, or containment impact: analysis does not execute target code. Tests execute owned Node fixture modules and remove their workspaces. - Generated metadata, package, or installation impact: no dependency or catalog change; generated documents checked through their normal generators. ## Evidence and regression coverage - Five focused files pass 197 tests, including the contributor's 49 URL-path regressions and 60 new real Node import/require target-selection cases. - The new boundary regression fails 44 cases on the integrated contributor base before the target-admission follow-up; 16 controls pass there. - Actual CLI and stdio MCP return identical normalized results in 28 cases, matching real Node 24.18.0 import/require outcomes. Examples cover encoded names, literal main fields, prohibited parent segments, nested arrays, malformed selected URLs, and missing first targets. - Representative retained diagnostic for `specifier: "fixture"`: ```json { "resolved_path": null, "resolution_status": "rejected", "limitations": [ "Package metadata node_modules/fixture/package.json: The package exports target \"./../outside.cjs\" contains forbidden path segment \"..\" before URL normalization." ] } ``` - [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 Node 24.18.0/npm 11.16.0, Linux x64, fresh normal locked installation. - `npm run build:cached` — passed. - Five focused test files — 197 passed. - `npm run typecheck`, `npm run lint`, `npm run docs:check` — passed. - Actual CLI/stdio MCP + real Node fixture verification — 28 cases passed. - `npm run verify:package` — passed with normal isolated tarball installation, CLI/setup lifecycle checks, and all 134 MCP tools. - Normal Windows tarball installation — passed on Node 22.19.0/npm 10.9.3. Actual CLI/stdio MCP results match in 29 cases, including 22 real Node loader comparisons and 7 HTML regressions; all 134 tools are advertised. - Exact-head hosted CI — all 22 checks passed; generated metadata skipped. Linux x64/arm64, macOS x64/arm64, and Windows package lanes passed. - Normal pre-commit/pre-push hooks — passed without bypassing checks. ## Compatibility, safety, and release No breaking schemas or migration. The graph now rejects relationships based on package targets that Node refuses. This is bounded root exports/URL-path coverage; package subpath maps and other unmodeled loader behavior retain existing limitations. Real Hopper/Ghidra verification was not performed because their adapters are unchanged. Local verification uses isolated owned fixtures, reduced CPU priority, memory monitoring, and storage cleanup. ## Review checklist - [x] One focused module-path outcome and a Conventional Commit title. - [x] Related issue linked. - [x] Real producer regressions and failure reasons covered. - [x] Owning guide and generated-document checks updated. - [x] Representative CLI/MCP behavior and evidence described. - [x] Final diff checked for host paths, credentials, unrelated changes, and generated catalog tracking. Prepared with Codex assistance. Contributor ancestry is preserved; additional commit and squash co-author identities use GitHub noreply addresses. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-17 21:08:01 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-10-31 01:06:33 +00:00

[PR #2839] [CLOSED] fix(browser): classify malformed screenshot payloads as protocol errors #5429

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

No description provided.

Rows
Columns