fix(javascript): resolve module URLs and reject invalid exports targets (#1031)

Resolve module URL paths once and reject invalid root package exports targets before normalization. Preserve actionable graph diagnostics and array selection behavior.

Fixes #1040.

Co-authored-by: Rudy Mizrahi Celekli <47457359+rudycelekli@users.noreply.github.com>
Co-authored-by: Ysopwnable <161784452+N0zoM1z0@users.noreply.github.com>

src/application/javascript/JavaScriptArtifactPathResolution.ts

@@ -86,10 +86,11 @@ const resolvePackagePath = (
   input: ResolveArtifactPathInput,
   packageChain: ReadonlySet<string>,
   mode: "full" | "files-and-index" = "full",
+  exportsTargetUrl = false,
 ): ArtifactPathResolution => {
   const rejected = rejectDeclaration(input);
   if (rejected !== null) return rejected;
-  const candidate = contextualCandidate(input);
+  const candidate = contextualCandidate(input, exportsTargetUrl);
   if (typeof candidate !== "string") return candidate;
   const confined = confineCandidate(input, candidate);
   if (typeof confined !== "string") return confined;
@@ -135,11 +136,13 @@ const rejectDeclaration = (
 
 const contextualCandidate = (
   input: ResolveArtifactPathInput,
+  exportsTargetUrl: boolean,
 ): string | ArtifactPathResolution => {
   const { context } = input;
-  const declared =
+  let declared =
     context === "html-reference" ||
     context === "url-reference" ||
+    exportsTargetUrl ||
     (context === "module-specifier" && input.moduleKind !== "require")
       ? stripQueryAndFragment(input.declaredPath)
       : input.declaredPath;
@@ -157,6 +160,24 @@ const contextualCandidate = (
     return unresolvedOutcome(input, "external", [
       "URL schemes and protocol-relative URLs are outside this local artifact path context.",
     ]);
+  // Exports targets use URL paths for both loaders; legacy main fields remain literal.
+  if (
+    exportsTargetUrl ||
+    (context === "module-specifier" && input.moduleKind !== "require")
+  ) {
+    try {
+      declared = decodeURIComponent(declared);
+    } catch (cause: unknown) {
+      void cause;
+      return unresolvedOutcome(input, "rejected", [
+        "The module URL path contains malformed percent encoding.",
+      ]);
+    }
+    if (declared.includes("\0"))
+      return unresolvedOutcome(input, "rejected", [
+        "The decoded module URL path contains NUL.",
+      ]);
+  }
   const relative = declared.startsWith("/") ? declared.slice(1) : declared;
   return declared.startsWith("/")
     ? relative
@@ -328,6 +349,12 @@ const resolveCandidate = (
         `Directory package metadata ${packagePath} is not valid package JSON.`,
       ],
     };
+  if (main.status === "rejected")
+    return {
+      resolvedPath: null,
+      status: "rejected",
+      limitations: [`Package metadata ${packagePath}: ${main.limitation}`],
+    };
   if (main.status === "unmatched")
     return {
       resolvedPath: null,
@@ -364,7 +391,9 @@ const resolveCandidate = (
     if (index !== null) return index;
   }
   // Preserve recursive artifact package lookup only after legacy file/index fallbacks.
-  return candidateOutcome(resolvePackagePath(entryInput, chain));
+  return candidateOutcome(
+    resolvePackagePath(entryInput, chain, "full", main.source === "exports"),
+  );
 };
 
 const candidateOutcome = (
@@ -419,6 +448,7 @@ const packageEntry = (
     }
   | { readonly status: "missing" }
   | { readonly status: "invalid" }
+  | { readonly status: "rejected"; readonly limitation: string }
   | { readonly status: "unmatched"; readonly declared: readonly string[] } => {
   try {
     const value: unknown = JSON.parse(text);
@@ -450,24 +480,53 @@ const packageExport = (
   value: unknown,
   moduleKind: ResolveArtifactPathInput["moduleKind"],
 ): PackageExportOutcome => {
-  if (typeof value === "string") return packagePathValue(value);
-  if (typeof value !== "object" || value === null) return { status: "invalid" };
-  const root = Reflect.get(value, ".") ?? value;
-  if (typeof root === "string") return packagePathValue(root);
-  if (typeof root !== "object" || root === null) return { status: "invalid" };
+  const root =
+    typeof value === "object" && value !== null && Object.hasOwn(value, ".")
+      ? Reflect.get(value, ".")
+      : value;
   const flattened = exportTargets(root, packageExportConditions(moduleKind));
-  if (flattened.kind === "invalid") return { status: "invalid" };
+  if (flattened.kind === "invalid")
+    return { status: "rejected", limitation: flattened.limitation };
   const first =
     flattened.kind === "unmatched" ? undefined : flattened.values[0];
   return first === undefined
-    ? { status: "unmatched", declared: Object.keys(root) }
+    ? {
+        status: "unmatched",
+        declared:
+          typeof root === "object" && root !== null ? Object.keys(root) : [],
+      }
     : { status: "value", value: first };
 };
 
 type ExportTargets =
   | { readonly kind: "targets"; readonly values: readonly string[] }
   | { readonly kind: "unmatched" }
-  | { readonly kind: "invalid" };
+  | { readonly kind: "invalid"; readonly limitation: string };
+
+const invalidExportTarget = (
+  value: unknown,
+  reason: string,
+): ExportTargets => ({
+  kind: "invalid",
+  limitation: `The package exports target ${JSON.stringify(value) ?? String(value)} ${reason}.`,
+});
+
+// Node checks raw target segments before URL normalization. Compare encoded
+// segment names here; full URL decoding follows array target selection.
+const forbiddenExportSegment = (value: string): string | undefined =>
+  value
+    .slice(2)
+    .split(/[\\/]/u)
+    .find((raw) => {
+      const segment = raw.replace(/%[0-9a-f]{2}/giu, (encoded) =>
+        String.fromCharCode(Number.parseInt(encoded.slice(1), 16)),
+      );
+      return (
+        segment === "." ||
+        segment === ".." ||
+        segment.toLowerCase() === "node_modules"
+      );
+    });
 
 /**
  * Resolve a target in Node's declared order. Explicit null blocks an active
@@ -479,26 +538,39 @@ const exportTargets = (
   value: unknown,
   conditions: ReadonlySet<string>,
 ): ExportTargets => {
-  if (typeof value === "string")
-    return value.startsWith("./")
+  if (typeof value === "string") {
+    if (!value.startsWith("./"))
+      return invalidExportTarget(value, 'must start with "./"');
+    const forbidden = forbiddenExportSegment(value);
+    return forbidden === undefined
       ? { kind: "targets", values: [value] }
-      : { kind: "invalid" };
+      : invalidExportTarget(
+          value,
+          `contains forbidden path segment ${JSON.stringify(forbidden)} before URL normalization`,
+        );
+  }
   if (value === null) return { kind: "targets", values: [] };
   if (Array.isArray(value)) {
-    let invalid = false;
+    let invalid: string | null = null;
     for (const entry of value) {
       const nested = exportTargets(entry, conditions);
       if (nested.kind === "invalid") {
-        invalid = true;
+        invalid = nested.limitation;
         continue;
       }
       if (nested.kind === "unmatched") continue;
-      invalid = false;
+      invalid = null;
       if (nested.values.length > 0) return nested;
     }
-    return invalid ? { kind: "invalid" } : { kind: "targets", values: [] };
+    return invalid === null
+      ? { kind: "targets", values: [] }
+      : { kind: "invalid", limitation: invalid };
   }
-  if (typeof value !== "object") return { kind: "invalid" };
+  if (typeof value !== "object")
+    return invalidExportTarget(
+      value,
+      "must be a relative string, object, array, or null",
+    );
   for (const [condition, target] of Object.entries(value)) {
     if (!conditions.has(condition)) continue;
     const nested = exportTargets(target, conditions);
@@ -525,7 +597,7 @@ const packageExportConditions = (
 
 type PackageExportOutcome =
   | { readonly status: "value"; readonly value: string }
-  | { readonly status: "invalid" }
+  | { readonly status: "rejected"; readonly limitation: string }
   | { readonly status: "unmatched"; readonly declared: readonly string[] };
 
 const packagePathValue = (

src/application/javascript/JavaScriptModuleSpecifierPaths.test.ts

@@ -1,7 +1,14 @@
 import { describe, expect, it } from "vitest";
+import { execFile } from "node:child_process";
+import { mkdir, writeFile } from "node:fs/promises";
+import { join } from "node:path";
+import { promisify } from "node:util";
 
+import { createTestTempDirectory } from "../../../tests/fixtures/temporaryDirectory.js";
 import type { JavaScriptArtifactFile } from "../../domain/javascript/javascriptArtifactFiles.js";
 import { resolveArtifactPathByContext } from "./JavaScriptArtifactPathResolution.js";
+import { analyzeJavaScriptApplication } from "./JavaScriptApplicationService.js";
+import { javascriptApplicationAnalysisResultSchema } from "../../domain/javascript/javascriptApplicationAnalysis.js";
 
 const filesFor = (paths: readonly string[]) =>
   new Map<string, JavaScriptArtifactFile>(
@@ -84,3 +91,241 @@ describe("module specifier punctuation", () => {
     },
   );
 });
+
+describe("ESM module URL decoding", () => {
+  it.each([
+    ["./plain.mjs", "plain.mjs"],
+    ["./space%20name.mjs", "space name.mjs"],
+    ["./pr%C3%A9load.mjs", "préload.mjs"],
+    ["./name%3F%23.mjs", "name?#.mjs"],
+    ["./percent%2520.mjs?cache=1#v2", "percent%20.mjs"],
+  ])(
+    "points %s imports to the file loaded by Node",
+    async (specifier, target) => {
+      const root = await createTestTempDirectory("rea-module-url-");
+      const main = join(root, "main.mjs");
+      await writeFile(
+        main,
+        `import value from ${JSON.stringify(specifier)}; console.log(value);`,
+      );
+      await writeFile(
+        join(root, target),
+        `export default ${JSON.stringify(target)};`,
+      );
+      const encoded = specifier.slice(2).split("?", 1)[0]?.split("#", 1)[0];
+      if (encoded !== undefined && encoded !== target)
+        await writeFile(join(root, encoded), 'export default "encoded decoy";');
+      const native = await promisify(execFile)(process.execPath, [main], {
+        timeout: 5_000,
+      });
+      expect(native.stdout.trim()).toBe(target);
+
+      const result = await analyzeJavaScriptApplication({
+        input_path: root,
+        format: "directory",
+      });
+      expect(result.ok).toBe(true);
+      if (!result.ok) throw result.error;
+      const { graph } = javascriptApplicationAnalysisResultSchema.parse(
+        result.value.normalized_result,
+      );
+      const imports = graph.edges.filter(
+        (edge) =>
+          edge.relation === "imports" &&
+          edge.properties.specifier === specifier,
+      );
+      expect(imports.length).toBeGreaterThan(0);
+      for (const edge of imports)
+        expect(edge.properties.resolved_path).toBe(target);
+    },
+  );
+
+  it("preserves CommonJS literal percent names", () => {
+    expect(
+      resolveArtifactPathByContext({
+        declaredPath: "./space%20name.cjs",
+        sourcePath: "app/main.js",
+        context: "module-specifier",
+        moduleKind: "require",
+        files: filesFor(["app/space%20name.cjs", "app/space name.cjs"]),
+      }),
+    ).toMatchObject({
+      resolved_path: "app/space%20name.cjs",
+      resolution_status: "resolved",
+    });
+  });
+
+  it.each(["./invalid%.mjs", "./invalid%C3.mjs", "./nul%00.mjs"])(
+    "rejects malformed or NUL URL %s",
+    (declaredPath) => {
+      expect(
+        resolveArtifactPathByContext({
+          declaredPath,
+          sourcePath: "app/main.js",
+          context: "module-specifier",
+          moduleKind: "import",
+          files: filesFor([`app/${declaredPath.slice(2)}`]),
+        }),
+      ).toMatchObject({ resolved_path: null, resolution_status: "rejected" });
+    },
+  );
+});
+
+const packageEntryUrlCases = [
+  ["exports", "./actual%20file.cjs", "actual file.cjs"],
+  ["exports", "./actual.cjs?variant", "actual.cjs"],
+  ["exports", "./actual.cjs#variant", "actual.cjs"],
+  ["exports", "./actual%23file.cjs", "actual#file.cjs"],
+  ["exports", "./actual%3Ffile.cjs", "actual?file.cjs"],
+  ["exports", "./actual%252Ffile.cjs", "actual%2Ffile.cjs"],
+  ["exports", "./actual.cjs", "actual.cjs"],
+  ["main", "./actual%20file.cjs", "actual%20file.cjs"],
+  ["main", "./actual?file.cjs", "actual?file.cjs"],
+  ["main", "./actual#file.cjs", "actual#file.cjs"],
+] as const;
+
+describe("package exports URL paths", () => {
+  it.each(
+    (["import", "require"] as const).flatMap((moduleKind) =>
+      packageEntryUrlCases.map(([field, declaredPath, target]) => ({
+        moduleKind,
+        field,
+        declaredPath,
+        target,
+      })),
+    ),
+  )(
+    "matches Node $moduleKind for $field $declaredPath",
+    async ({ moduleKind, field, declaredPath, target }) => {
+      const root = await createTestTempDirectory("rea-package-export-url-");
+      const packageRoot = join(root, "node_modules", "fixture");
+      await mkdir(packageRoot, { recursive: true });
+      await writeFile(
+        join(packageRoot, "package.json"),
+        JSON.stringify({
+          name: "fixture",
+          [field]: declaredPath,
+        }),
+      );
+      await writeFile(
+        join(packageRoot, target),
+        `module.exports=${JSON.stringify(target)};`,
+      );
+      const literal = declaredPath.slice(2).split("?", 1)[0]?.split("#", 1)[0];
+      if (literal !== undefined && literal !== target)
+        await writeFile(
+          join(packageRoot, literal),
+          'module.exports="literal decoy";',
+        );
+      const main = join(
+        root,
+        moduleKind === "import" ? "main.mjs" : "main.cjs",
+      );
+      await writeFile(
+        main,
+        moduleKind === "import"
+          ? 'import value from "fixture"; console.log(value);'
+          : 'console.log(require("fixture"));',
+      );
+      const native = await promisify(execFile)(process.execPath, [main], {
+        timeout: 5_000,
+      });
+      expect(native.stdout.trim()).toBe(target);
+      const result = await analyzeJavaScriptApplication({
+        input_path: root,
+        format: "directory",
+      });
+      expect(result.ok).toBe(true);
+      if (!result.ok) throw result.error;
+      const { graph } = javascriptApplicationAnalysisResultSchema.parse(
+        result.value.normalized_result,
+      );
+      const imports = graph.edges.filter(
+        (edge) =>
+          edge.relation === "imports" &&
+          edge.properties.specifier === "fixture",
+      );
+      expect(imports.length).toBeGreaterThan(0);
+      for (const edge of imports)
+        expect(edge.properties.resolved_path).toBe(
+          `node_modules/fixture/${target}`,
+        );
+    },
+  );
+
+  it.each(
+    (["import", "require"] as const).flatMap((moduleKind) =>
+      [
+        "./actual%file.cjs",
+        "./actual%E0%A4file.cjs",
+        "./actual%00file.cjs",
+        "./actual%2Ffile.cjs",
+        "./actual%5Cfile.cjs",
+        "./actual%2Efile.cjs",
+        "./../../../../escape.cjs",
+      ].map((declaredPath) => ({ moduleKind, declaredPath })),
+    ),
+  )(
+    "rejects $moduleKind URL $declaredPath without borrowing a literal decoy",
+    ({ moduleKind, declaredPath }) => {
+      const packagePath = "app/node_modules/fixture/package.json";
+      const files = filesFor([
+        packagePath,
+        `app/node_modules/fixture/${declaredPath.slice(2)}`,
+      ]);
+      const metadata = files.get(packagePath);
+      if (metadata === undefined) throw new Error("Missing fixture metadata");
+      files.set(packagePath, {
+        ...metadata,
+        kind: "package-json",
+        text: {
+          included: true,
+          value: JSON.stringify({ exports: declaredPath }),
+        },
+      });
+      expect(
+        resolveArtifactPathByContext({
+          declaredPath: "fixture",
+          sourcePath: "app/main.js",
+          context: "module-specifier",
+          moduleKind,
+          files,
+        }),
+      ).toMatchObject({ resolved_path: null, resolution_status: "rejected" });
+    },
+  );
+
+  it.each(["import", "require"] as const)(
+    "keeps %s exports inside their inventoried container",
+    (moduleKind) => {
+      const packagePath = "app/node_modules/fixture/package.json";
+      const target = "app/node_modules/fixture/actual file.cjs";
+      const files = filesFor([packagePath, target]);
+      const metadata = files.get(packagePath);
+      const crossContainer = files.get(target);
+      if (metadata === undefined || crossContainer === undefined)
+        throw new Error("Missing fixture files");
+      files.set(packagePath, {
+        ...metadata,
+        kind: "package-json",
+        text: {
+          included: true,
+          value: '{"exports":"./actual%20file.cjs"}',
+        },
+      });
+      files.set(target, {
+        ...crossContainer,
+        container_sha256: "c".repeat(64),
+      });
+      expect(
+        resolveArtifactPathByContext({
+          declaredPath: "fixture",
+          sourcePath: "app/main.js",
+          context: "module-specifier",
+          moduleKind,
+          files,
+        }),
+      ).toMatchObject({ resolved_path: null, resolution_status: "not-found" });
+    },
+  );
+});