morluto/rea

Fork 0

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

[GH-ISSUE #2960] [Bug] Relative module specifiers with %2e or %2F are rejected even when Node loads the file #1043

New issue

Closed

opened 2026-10-15 18:30:36 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-15 18:30:36 +00:00

Owner

Originally created by @ShivekRanjan on GitHub (Oct 13, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/2960

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Evidence, artifacts, or reconstruction

Observed behavior

analyze-javascript-application marks relative require and import specifiers that contain %2e or %2F as rejected (or leaves them unresolved), even when the file exists in the application and Node loads it.

require("./lit%2eb"): Node loads the literal file lit%2eb.js, because CommonJS does not percent-decode. REA reports it unresolved.
require("./x%2Fy"): Node loads the literal file x%2Fy.js. REA reports it unresolved.
import "./a%2eb.mjs": Node decodes %2e and loads a.b.mjs. REA rejects it.
import "./sub/%2e%2e/x.mjs": Node treats %2e%2e as .. and loads x.mjs, which is inside the application root. REA rejects it.

A plain require("./plain") in the same kind of application resolves normally. On current main the two require cases are reported as rejected rather than unresolved.

Reproduction

Save the script below as encoded-dot-repro.mjs.
Run node encoded-dot-repro.mjs.

It creates a small application in a temporary directory, runs it with Node, then runs npx -y rea-agents@latest analyze-javascript-application <dir> --json and compares the results for each specifier.

encoded-dot-repro.mjs

// Compare how Node and REA resolve module specifiers that contain
// percent-encoded dots and slashes.
//
// Usage:  node encoded-dot-repro.mjs
// Optional: set REA_COMMAND to use a source build instead of the published
// package, e.g. REA_COMMAND="node /path/to/rea/scripts/rea.mjs".

import { execFileSync, execSync } from "node:child_process";
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";

const app = mkdtempSync(join(tmpdir(), "rea-encoded-dot-"));
const write = (name, text) => writeFileSync(join(app, name), text);

mkdirSync(join(app, "sub"));
write("package.json", '{ "name": "repro", "version": "1.0.0", "main": "cjs.js" }\n');
write("lit%2eb.js", 'module.exports = "lit%2eb.js";\n');
write("x%2Fy.js", 'module.exports = "x%2Fy.js";\n');
write("a.b.mjs", 'export default "a.b.mjs";\n');
write("x.mjs", 'export default "x.mjs";\n');
write(
  "cjs.js",
  'console.log(require("./lit%2eb"));\nconsole.log(require("./x%2Fy"));\n',
);
write(
  "esm.mjs",
  'import a from "./a%2eb.mjs";\nimport x from "./sub/%2e%2e/x.mjs";\nconsole.log(a);\nconsole.log(x);\n',
);

const cases = [
  ["require", "./lit%2eb"],
  ["require", "./x%2Fy"],
  ["import", "./a%2eb.mjs"],
  ["import", "./sub/%2e%2e/x.mjs"],
];

const run = (file) =>
  execFileSync(process.execPath, [file], { cwd: app, encoding: "utf8" })
    .trim()
    .split("\n");
const nodeLoaded = [...run("cjs.js"), ...run("esm.mjs")];

const reaCommand = process.env.REA_COMMAND ?? "npx -y rea-agents@latest";
const output = execSync(
  `${reaCommand} analyze-javascript-application "${app}" --json`,
  { encoding: "utf8", maxBuffer: 64 * 1024 * 1024, stdio: ["ignore", "pipe", "ignore"] },
);
const result = JSON.parse(output);
const edges = result.normalized_result.graph.edges;
// Field names vary between releases, so read every import edge for the
// specifier and report the most specific status any of them records.
const reaResult = (specifier) => {
  const found = edges.filter(
    (e) => e.relation === "imports" && e.properties.specifier === specifier,
  );
  if (found.length === 0) return "no import edge";
  const resolved = found.find((e) => e.properties.resolved_path);
  if (resolved)
    return `${resolved.properties.resolution_status ?? "resolved"} ${resolved.properties.resolved_path}`;
  const status = found.find((e) => e.properties.resolution_status);
  return status ? status.properties.resolution_status : "unresolved";
};

console.log(`Node ${process.version}, REA via: ${reaCommand}\n`);
console.log("mode     specifier             Node loads   REA");
cases.forEach(([mode, specifier], index) =>
  console.log(
    `${mode.padEnd(8)} ${specifier.padEnd(21)} ${nodeLoaded[index].padEnd(12)} ${reaResult(specifier)}`,
  ),
);

rmSync(app, { recursive: true, force: true });

Expected behavior

CommonJS: require("./lit%2eb") and require("./x%2Fy") resolve to the literal files lit%2eb.js and x%2Fy.js. docs/javascript-artifact-reconstruction.md states that CommonJS relative paths retain literal filename punctuation, and CommonJS does not percent-decode, so these specifiers contain no encoded dot or separator. If CommonJS should stay strict anyway, the documentation should say so.
ESM: #2700 says some encoded-dot cases intentionally keep REA's stricter admission policy. If that applies to relative imports too, I'd expect the policy to be documented and these cases reported in a way that shows Node itself would accept them (for example unavailable with a limitation naming the policy), rather than rejected, which reads as an invalid specifier. Otherwise, decode first and then apply the existing artifact-root containment check, matching Node.

Impact and scope

Affected: the static module graph from analyze-javascript-application. Files that Node loads are missing from the import relationships and are reported as rejected or unresolved references, so graph coverage is understated.

Not affected, as far as I can tell: specifiers without percent-encoded dot or separator bytes; HTML script references, where rejecting encoded dots is documented; package exports targets, where Node itself rejects encoded dot segments.

No evidence integrity or process isolation impact: every file in the reproduction is inside the application root.

REA version

rea-agents 6.4.0 (npx rea-agents@latest)

Runtime and platform

Node v24.15.0, Windows x64

Target and artifact context

Extracted JavaScript application directory: package.json, CommonJS and ESM files

Interface

CLI

Earliest failure stage

Artifact or evidence registration

Relevant output or evidence

Node v24.15.0, REA via: npx -y rea-agents@latest

mode     specifier             Node loads   REA
require  ./lit%2eb             lit%2eb.js   unresolved
require  ./x%2Fy               x%2Fy.js     unresolved
import   ./a%2eb.mjs           a.b.mjs      rejected
import   ./sub/%2e%2e/x.mjs    x.mjs        rejected

Typed outcome or recovery expected

For require: resolution_status: "resolved" with resolved_path set to the literal file (lit%2eb.js, x%2Fy.js).

For import: either resolved with the decoded file (a.b.mjs, x.mjs), or, if the stricter encoded-dot policy is intentional, unavailable with a limitation naming that policy, rather than rejected.

Analysis or suspected cause

Source-derived, not yet verified with a fix: admitsCanonicalPathSyntax in src/domain/artifactPathSyntax.ts rejects any %2e, %2f or %5c. rejectDeclaration in src/application/javascript/JavaScriptArtifactPathResolution.ts applies it to module specifiers before the resolver distinguishes require from import, so CommonJS never sees the specifier as literal text and ESM never reaches its decoding step.

The existing test "fails closed for unsafe or external module declaration" includes %2e%2e/escape.js, so the ESM behavior may be deliberate; the CommonJS behavior contradicts the documented literal-punctuation rule.

Additional context

Related, but distinct:

#2827 / #2833: the same admission rule rejecting query and fragment bytes in HTML script URLs; that PR leaves module specifiers unchanged.
#2700: says some encoded-dot cases intentionally retain REA's stricter artifact admission policy.

Possible fix for CommonJS: skip the encoded-byte check for require specifiers while still rejecting NUL and backslash, since CommonJS treats % literally. Regression tests would cover both require cases above, plus an encoded .. that escapes the application root, which should stay rejected.

I'm happy to submit a PR once you confirm the expected CommonJS and ESM behavior.

Originally created by @ShivekRanjan on GitHub (Oct 13, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2960 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Evidence, artifacts, or reconstruction ### Observed behavior `analyze-javascript-application` marks relative `require` and `import` specifiers that contain `%2e` or `%2F` as `rejected` (or leaves them unresolved), even when the file exists in the application and Node loads it. - `require("./lit%2eb")`: Node loads the literal file `lit%2eb.js`, because CommonJS does not percent-decode. REA reports it unresolved. - `require("./x%2Fy")`: Node loads the literal file `x%2Fy.js`. REA reports it unresolved. - `import "./a%2eb.mjs"`: Node decodes `%2e` and loads `a.b.mjs`. REA rejects it. - `import "./sub/%2e%2e/x.mjs"`: Node treats `%2e%2e` as `..` and loads `x.mjs`, which is inside the application root. REA rejects it. A plain `require("./plain")` in the same kind of application resolves normally. On current `main` the two `require` cases are reported as `rejected` rather than unresolved. ### Reproduction 1. Save the script below as `encoded-dot-repro.mjs`. 2. Run `node encoded-dot-repro.mjs`. It creates a small application in a temporary directory, runs it with Node, then runs `npx -y rea-agents@latest analyze-javascript-application <dir> --json` and compares the results for each specifier. <details> <summary>encoded-dot-repro.mjs</summary> ```js // Compare how Node and REA resolve module specifiers that contain // percent-encoded dots and slashes. // // Usage: node encoded-dot-repro.mjs // Optional: set REA_COMMAND to use a source build instead of the published // package, e.g. REA_COMMAND="node /path/to/rea/scripts/rea.mjs". import { execFileSync, execSync } from "node:child_process"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; const app = mkdtempSync(join(tmpdir(), "rea-encoded-dot-")); const write = (name, text) => writeFileSync(join(app, name), text); mkdirSync(join(app, "sub")); write("package.json", '{ "name": "repro", "version": "1.0.0", "main": "cjs.js" }\n'); write("lit%2eb.js", 'module.exports = "lit%2eb.js";\n'); write("x%2Fy.js", 'module.exports = "x%2Fy.js";\n'); write("a.b.mjs", 'export default "a.b.mjs";\n'); write("x.mjs", 'export default "x.mjs";\n'); write( "cjs.js", 'console.log(require("./lit%2eb"));\nconsole.log(require("./x%2Fy"));\n', ); write( "esm.mjs", 'import a from "./a%2eb.mjs";\nimport x from "./sub/%2e%2e/x.mjs";\nconsole.log(a);\nconsole.log(x);\n', ); const cases = [ ["require", "./lit%2eb"], ["require", "./x%2Fy"], ["import", "./a%2eb.mjs"], ["import", "./sub/%2e%2e/x.mjs"], ]; const run = (file) => execFileSync(process.execPath, [file], { cwd: app, encoding: "utf8" }) .trim() .split("\n"); const nodeLoaded = [...run("cjs.js"), ...run("esm.mjs")]; const reaCommand = process.env.REA_COMMAND ?? "npx -y rea-agents@latest"; const output = execSync( `${reaCommand} analyze-javascript-application "${app}" --json`, { encoding: "utf8", maxBuffer: 64 * 1024 * 1024, stdio: ["ignore", "pipe", "ignore"] }, ); const result = JSON.parse(output); const edges = result.normalized_result.graph.edges; // Field names vary between releases, so read every import edge for the // specifier and report the most specific status any of them records. const reaResult = (specifier) => { const found = edges.filter( (e) => e.relation === "imports" && e.properties.specifier === specifier, ); if (found.length === 0) return "no import edge"; const resolved = found.find((e) => e.properties.resolved_path); if (resolved) return `${resolved.properties.resolution_status ?? "resolved"} ${resolved.properties.resolved_path}`; const status = found.find((e) => e.properties.resolution_status); return status ? status.properties.resolution_status : "unresolved"; }; console.log(`Node ${process.version}, REA via: ${reaCommand}\n`); console.log("mode specifier Node loads REA"); cases.forEach(([mode, specifier], index) => console.log( `${mode.padEnd(8)} ${specifier.padEnd(21)} ${nodeLoaded[index].padEnd(12)} ${reaResult(specifier)}`, ), ); rmSync(app, { recursive: true, force: true }); ``` </details> ### Expected behavior - CommonJS: `require("./lit%2eb")` and `require("./x%2Fy")` resolve to the literal files `lit%2eb.js` and `x%2Fy.js`. `docs/javascript-artifact-reconstruction.md` states that CommonJS relative paths retain literal filename punctuation, and CommonJS does not percent-decode, so these specifiers contain no encoded dot or separator. If CommonJS should stay strict anyway, the documentation should say so. - ESM: #2700 says some encoded-dot cases intentionally keep REA's stricter admission policy. If that applies to relative imports too, I'd expect the policy to be documented and these cases reported in a way that shows Node itself would accept them (for example `unavailable` with a limitation naming the policy), rather than `rejected`, which reads as an invalid specifier. Otherwise, decode first and then apply the existing artifact-root containment check, matching Node. ### Impact and scope Affected: the static module graph from `analyze-javascript-application`. Files that Node loads are missing from the import relationships and are reported as rejected or unresolved references, so graph coverage is understated. Not affected, as far as I can tell: specifiers without percent-encoded dot or separator bytes; HTML script references, where rejecting encoded dots is documented; package exports targets, where Node itself rejects encoded dot segments. No evidence integrity or process isolation impact: every file in the reproduction is inside the application root. ### REA version rea-agents 6.4.0 (npx rea-agents@latest) ### Runtime and platform Node v24.15.0, Windows x64 ### Target and artifact context Extracted JavaScript application directory: package.json, CommonJS and ESM files ### Interface CLI ### Earliest failure stage Artifact or evidence registration ### Relevant output or evidence ```text Node v24.15.0, REA via: npx -y rea-agents@latest mode specifier Node loads REA require ./lit%2eb lit%2eb.js unresolved require ./x%2Fy x%2Fy.js unresolved import ./a%2eb.mjs a.b.mjs rejected import ./sub/%2e%2e/x.mjs x.mjs rejected ``` ### Typed outcome or recovery expected For `require`: `resolution_status: "resolved"` with `resolved_path` set to the literal file (`lit%2eb.js`, `x%2Fy.js`). For `import`: either `resolved` with the decoded file (`a.b.mjs`, `x.mjs`), or, if the stricter encoded-dot policy is intentional, `unavailable` with a limitation naming that policy, rather than `rejected`. ### Analysis or suspected cause Source-derived, not yet verified with a fix: `admitsCanonicalPathSyntax` in `src/domain/artifactPathSyntax.ts` rejects any `%2e`, `%2f` or `%5c`. `rejectDeclaration` in `src/application/javascript/JavaScriptArtifactPathResolution.ts` applies it to module specifiers before the resolver distinguishes `require` from `import`, so CommonJS never sees the specifier as literal text and ESM never reaches its decoding step. The existing test "fails closed for unsafe or external module declaration" includes `%2e%2e/escape.js`, so the ESM behavior may be deliberate; the CommonJS behavior contradicts the documented literal-punctuation rule. ### Additional context Related, but distinct: - #2827 / #2833: the same admission rule rejecting query and fragment bytes in HTML script URLs; that PR leaves module specifiers unchanged. - #2700: says some encoded-dot cases intentionally retain REA's stricter artifact admission policy. Possible fix for CommonJS: skip the encoded-byte check for `require` specifiers while still rejecting NUL and backslash, since CommonJS treats `%` literally. Regression tests would cover both `require` cases above, plus an encoded `..` that escapes the application root, which should stay rejected. I'm happy to submit a PR once you confirm the expected CommonJS and ESM behavior.

bsakaguchi

2026-10-15 18:30:36 +00:00

closed this issue
added the
triage

area: domain

bug
labels

bsakaguchi commented

2026-10-15 18:30:37 +00:00

Author

Owner

@ShivekRanjan commented on GitHub (Oct 13, 2026):

I'm working on a fix for the CommonJS cases and will open a draft PR shortly. I'll leave the ESM cases until the expected behavior is confirmed.

@ShivekRanjan commented on GitHub (Oct 13, 2026): I'm working on a fix for the CommonJS cases and will open a draft PR shortly. I'll leave the ESM cases until the expected behavior is confirmed.

bsakaguchi commented

2026-10-15 18:30:37 +00:00

Author

Owner

@morluto commented on GitHub (Oct 14, 2026):

Addressed on main in b58adafa4.

Module-kind-aware path resolution: CommonJS percent bytes stay literal; ESM URL decoding, encoded separator rejection, and decoded dot-segment confinement follow Node loading semantics.

Validation: Resolver tests 158 passed; compiled CLI/stdio MCP controls passed. A separate referenceSourceImport failure was corrected; focused test passed 18/18. Global gates pass.

Verification scope: No Windows-specific runtime claim.

@morluto commented on GitHub (Oct 14, 2026): Addressed on main in [b58adafa4](https://github.com/morluto/rea/commit/b58adafa4). Module-kind-aware path resolution: CommonJS percent bytes stay literal; ESM URL decoding, encoded separator rejection, and decoded dot-segment confinement follow Node loading semantics. Validation: Resolver tests 158 passed; compiled CLI/stdio MCP controls passed. A separate referenceSourceImport failure was corrected; focused test passed 18/18. Global gates pass. Verification scope: No Windows-specific runtime claim.

bsakaguchi referenced this issue

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

[PR #1043] [MERGED] docs: simplify the English README and organize guides #2602

bsakaguchi referenced this issue

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

[PR #1121] [CLOSED] docs(zh): restructure README_zh against the post-1043 English baseline #2666