fix(javascript): normalize package URLs before array selection (#1916)

docs/javascript-artifact-reconstruction.md

@@ -482,8 +482,12 @@ and one percent-decoding pass. CommonJS relative paths and legacy package main
 fields retain literal filename punctuation. Exports targets must start with
 `./` and exclude `.`, `..`, and `node_modules` path segments, including their
 encoded forms, before URL normalization. Arrays skip invalid exports targets
-in declared order; URL decoding and file lookup failures for a selected target
-do not select a later entry. Rejected references retain the package metadata
+in declared order, including URLs that escape the package root after normalization.
+Selected package targets use the normalized URL path for lookup; tabs and
+newlines in the URL text are removed by the URL parser. CommonJS package targets
+reject encoded separators anywhere in the selected URL; ESM checks its pathname.
+URL decoding and file lookup failures for a selected target do not select a
+later entry. Rejected references retain the package metadata
 path, original target, failed constraint, and importing source location.
 
 HTML script references resolve to exact inventoried files after applying the

src/application/javascript/JavaScriptArtifactPathResolution.ts

@@ -74,6 +74,21 @@ const EXTENSIONS = [
   ".node",
 ];
 
+// A private URL namespace for logical artifact paths, including root packages.
+// Encode inventory segments so filename punctuation remains literal in the base.
+const PACKAGE_ARTIFACT_ROOT_URL = new URL("file:///rea-artifact/");
+const packageMetadataUrl = (path: string): URL =>
+  new URL(
+    path.split("/").map(encodeURIComponent).join("/"),
+    PACKAGE_ARTIFACT_ROOT_URL,
+  );
+
+interface PackageFileTarget {
+  readonly value: string;
+  /** Normalized URL retained until loader-specific validation and file lookup. */
+  readonly url: URL;
+}
+
 type CandidateResolution =
   | {
       readonly resolvedPath: string;
@@ -313,7 +328,11 @@ const resolvePackageImportAlias = (
     return unresolvedCandidate("external", [
       `Wildcard package import aliases remain unresolved; package metadata ${metadata.path} has no exact imports key ${specifier}.`,
     ]);
-  const selected = selectPackageImportTarget(lookup.target, input.moduleKind);
+  const selected = selectPackageImportTarget(
+    lookup.target,
+    input.moduleKind,
+    metadata.path,
+  );
   if (selected.status === "rejected")
     return unresolvedCandidate("rejected", [
       `Package metadata ${metadata.path}: ${selected.limitation}`,
@@ -334,14 +353,15 @@ const resolvePackageImportAlias = (
         : `External-package import targets remain unresolved; package metadata ${metadata.path} maps ${specifier} to ${JSON.stringify(target.target)}.`,
     ]);
   }
-  return resolveExportTarget(input, metadata.path, selected.value, "imports");
+  return resolveExportTarget(input, metadata.path, selected, "imports");
 };
 
 const selectPackageImportTarget = (
   value: unknown,
   moduleKind: ResolveArtifactPathInput["moduleKind"],
+  packagePath: string,
 ):
-  | { readonly status: "value"; readonly value: string }
+  | ({ readonly status: "value" } & PackageFileTarget)
   | { readonly status: "non-relative"; readonly value: string }
   | { readonly status: "blocked"; readonly cause: PackageTargetBlockCause }
   | { readonly status: "unmatched"; readonly declared: readonly string[] }
@@ -349,6 +369,7 @@ const selectPackageImportTarget = (
   const selected = selectExportTarget(
     value,
     packageExportConditions(moduleKind),
+    packageMetadataUrl(packagePath),
     "imports",
   );
   if (selected.kind === "invalid" || selected.kind === "invalid-config")
@@ -362,7 +383,11 @@ const selectPackageImportTarget = (
     return { status: "blocked", cause: selected.cause };
   if (selected.kind === "non-relative")
     return { status: "non-relative", value: selected.value };
-  return { status: "value", value: selected.value };
+  return {
+    status: "value",
+    value: selected.value,
+    url: selected.url,
+  };
 };
 
 /** Self references use the nearest package scope before dependency lookup. */
@@ -430,7 +455,12 @@ const resolvePackageExports = (
   subpath: string,
   rawExports: string | object,
 ): CandidateResolution => {
-  const exported = selectPackageExport(rawExports, subpath, input.moduleKind);
+  const exported = selectPackageExport(
+    rawExports,
+    subpath,
+    input.moduleKind,
+    packagePath,
+  );
   if (exported.status === "rejected")
     return unresolvedCandidate("rejected", [
       `Package metadata ${packagePath}: ${exported.limitation}`,
@@ -443,25 +473,45 @@ const resolvePackageExports = (
     return unresolvedCandidate("external", [
       `Directory package metadata ${packagePath} blocks subpath ${subpath} with ${exported.cause === "null" ? "an explicit null exports target" : "an empty exports target array"}.`,
     ]);
-  return resolveExportTarget(input, packagePath, exported.value);
+  return resolveExportTarget(input, packagePath, exported);
 };
 
 /** An exports or import target is a URL naming one exact file for both Node loaders. */
 const resolveExportTarget = (
   input: ResolveArtifactPathInput,
   packagePath: string,
-  target: string,
+  target: PackageFileTarget,
   mapKind: "exports" | "imports" = "exports",
 ): CandidateResolution => {
-  const rejected = rejectDeclaration(target);
-  if (rejected !== null) return rejected;
-  const decoded = decodeModuleUrlPath(stripQueryAndFragment(target));
-  if (typeof decoded !== "string") return decoded;
-  const candidate = confineCandidate(relativeCandidate(packagePath, decoded));
-  if (typeof candidate !== "string") return candidate;
+  const withTargetSource = (
+    outcome: CandidateResolution,
+  ): CandidateResolution => ({
+    ...outcome,
+    limitations: outcome.limitations.map(
+      (reason) =>
+        `Package metadata ${packagePath}, ${mapKind} target ${JSON.stringify(target.value)}: ${reason}`,
+    ),
+  });
+  // Node's CommonJS finalizer checks the serialized URL, including suffixes;
+  // its ESM finalizer checks only the pathname. Neither error retries arrays.
+  if (input.moduleKind === "require" && /%(?:2f|5c)/iu.test(target.url.href))
+    return withTargetSource(
+      unresolvedCandidate("rejected", [
+        "The selected package URL contains encoded separator bytes.",
+      ]),
+    );
+  const encodedPath = target.url.pathname.slice(
+    PACKAGE_ARTIFACT_ROOT_URL.pathname.length,
+  );
+  const rejected = rejectDeclaration(encodedPath);
+  if (rejected !== null) return withTargetSource(rejected);
+  const decoded = decodeModuleUrlPath(encodedPath);
+  if (typeof decoded !== "string") return withTargetSource(decoded);
+  const candidate = confineCandidate(decoded);
+  if (typeof candidate !== "string") return withTargetSource(candidate);
   return (
     resolveFileCandidates(input, [candidate]) ??
-    exactPackageTargetNotFound(candidate, mapKind)
+    withTargetSource(exactPackageTargetNotFound(candidate, mapKind))
   );
 };
 
@@ -776,7 +826,7 @@ const indexCandidates = (candidate: string): readonly string[] =>
 type PackageTargetBlockCause = "null" | "empty-array";
 
 type PackageExportOutcome =
-  | { readonly status: "value"; readonly value: string }
+  | ({ readonly status: "value" } & PackageFileTarget)
   | { readonly status: "rejected"; readonly limitation: string }
   | { readonly status: "blocked"; readonly cause: PackageTargetBlockCause }
   | { readonly status: "unmatched"; readonly declared: readonly string[] };
@@ -786,6 +836,7 @@ const selectPackageExport = (
   value: unknown,
   subpath: string,
   moduleKind: ResolveArtifactPathInput["moduleKind"],
+  packagePath: string,
 ): PackageExportOutcome => {
   let target = value;
   let declared: string[] = [];
@@ -812,6 +863,7 @@ const selectPackageExport = (
   const selected = selectExportTarget(
     target,
     packageExportConditions(moduleKind),
+    packageMetadataUrl(packagePath),
   );
   if (selected.kind === "invalid" || selected.kind === "invalid-config")
     return { status: "rejected", limitation: selected.limitation };
@@ -827,11 +879,15 @@ const selectPackageExport = (
       status: "rejected",
       limitation: `The package exports target ${JSON.stringify(selected.value)} must start with "./".`,
     };
-  return { status: "value", value: selected.value };
+  return {
+    status: "value",
+    value: selected.value,
+    url: selected.url,
+  };
 };
 
 type ExportTargetOutcome =
-  | { readonly kind: "value"; readonly value: string }
+  | ({ readonly kind: "value" } & PackageFileTarget)
   | { readonly kind: "non-relative"; readonly value: string }
   | { readonly kind: "blocked"; readonly cause: PackageTargetBlockCause }
   | { readonly kind: "unmatched"; readonly declared: readonly string[] }
@@ -866,6 +922,7 @@ const forbiddenExportSegment = (value: string): string | undefined =>
 
 const selectStringPackageTarget = (
   value: string,
+  packageUrl: URL,
   mapKind: "exports" | "imports",
 ): ExportTargetOutcome => {
   if (mapKind === "imports") {
@@ -880,18 +937,33 @@ const selectStringPackageTarget = (
   } else if (!value.startsWith("./"))
     return invalidExportTarget(value, 'must start with "./"', mapKind);
   const forbidden = forbiddenExportSegment(value);
-  return forbidden === undefined
-    ? { kind: "value", value }
-    : invalidExportTarget(
-        value,
-        `contains forbidden path segment ${JSON.stringify(forbidden)} before URL normalization`,
-        mapKind,
-      );
+  if (forbidden !== undefined)
+    return invalidExportTarget(
+      value,
+      `contains forbidden path segment ${JSON.stringify(forbidden)} before URL normalization`,
+      mapKind,
+    );
+  // Node constructs the URL before deciding whether this array entry is an
+  // invalid package target. Tabs/newlines and dot segments normalize here;
+  // percent decoding and existence checks belong after target selection.
+  const url = new URL(value, packageUrl);
+  if (!url.pathname.startsWith(new URL(".", packageUrl).pathname))
+    return invalidExportTarget(
+      value,
+      "escapes the package root after URL normalization",
+      mapKind,
+    );
+  return {
+    kind: "value",
+    value,
+    url,
+  };
 };
 
 const selectArrayPackageTarget = (
   value: readonly unknown[],
   conditions: ReadonlySet<string>,
+  packageUrl: URL,
   mapKind: "exports" | "imports",
 ): ExportTargetOutcome => {
   if (value.length === 0) return { kind: "blocked", cause: "empty-array" };
@@ -900,7 +972,7 @@ const selectArrayPackageTarget = (
     | undefined;
   const declared = new Set<string>();
   for (const entry of value) {
-    const nested = selectExportTarget(entry, conditions, mapKind);
+    const nested = selectExportTarget(entry, conditions, packageUrl, mapKind);
     // Node's array fallback catches invalid targets, not invalid configuration.
     if (nested.kind === "invalid-config") return nested;
     if (nested.kind === "unmatched") {
@@ -926,13 +998,14 @@ const selectArrayPackageTarget = (
 const selectExportTarget = (
   value: unknown,
   conditions: ReadonlySet<string>,
+  packageUrl: URL,
   mapKind: "exports" | "imports" = "exports",
 ): ExportTargetOutcome => {
   if (typeof value === "string")
-    return selectStringPackageTarget(value, mapKind);
+    return selectStringPackageTarget(value, packageUrl, mapKind);
   if (value === null) return { kind: "blocked", cause: "null" };
   if (Array.isArray(value))
-    return selectArrayPackageTarget(value, conditions, mapKind);
+    return selectArrayPackageTarget(value, conditions, packageUrl, mapKind);
   if (typeof value !== "object")
     return invalidExportTarget(
       value,
@@ -951,7 +1024,7 @@ const selectExportTarget = (
   for (const [condition, target] of Object.entries(value)) {
     declared.add(condition);
     if (!conditions.has(condition)) continue;
-    const nested = selectExportTarget(target, conditions, mapKind);
+    const nested = selectExportTarget(target, conditions, packageUrl, mapKind);
     if (nested.kind !== "unmatched") return nested;
     for (const name of nested.declared) declared.add(name);
   }

tests/acceptance/applications/packageImportAliasesCli.test.ts

@@ -13,6 +13,11 @@ import { cliTest } from "../../support/cli/cliFixture.js";
 
 const literalAliases = ["#/x", "#x%2Fy", "#x%2Ey", "#x\\y", "#x\0y"];
 const fallbackAliases = {
+  "#tab-escape": ["./.\t./outside.cjs", "./config.cjs"],
+  "#newline-escape": ["./.%\n2e/outside.cjs", "./config.cjs"],
+  "#normalized-url": "./con\tfig.cjs",
+  "#trimmed-url": "./config.cjs \t",
+  "#url-suffix": "./config.cjs?selected=one#fragment",
   "#inactive-array": {
     node: [{ browser: "./browser.cjs" }],
     default: "./config.cjs",
@@ -30,29 +35,53 @@ const fallbackAliases = {
 };
 const resolvedAliases = [...literalAliases, ...Object.keys(fallbackAliases)];
 const refusedAliases = {
-  "#empty": { target: [], limitation: "an empty imports target array" },
+  "#empty": {
+    target: [],
+    status: "external",
+    limitation: "an empty imports target array",
+  },
   "#empty-active": {
     target: { node: [] },
+    status: "external",
     limitation: "an empty imports target array",
   },
   "#null-last": {
     target: [[], null],
+    status: "external",
     limitation: "an explicit null imports target",
   },
   "#empty-last": {
     target: [null, []],
+    status: "external",
     limitation: "an empty imports target array",
   },
   "#inactive": {
     target: [{ browser: "./browser.cjs" }],
+    status: "external",
     limitation: "it declares browser.",
   },
   "#nested-inactive": {
+    status: "external",
     target: [
       [{ browser: "./browser.cjs" }, { "rea-inactive": "./inactive.cjs" }],
     ],
     limitation: "it declares browser, rea-inactive.",
   },
+  "#escape": {
+    target: "./.\t./outside.cjs",
+    status: "rejected",
+    limitation: "escapes the package root",
+  },
+  "#encoded-separator": {
+    target: ["./con%2ffig.cjs", "./config.cjs"],
+    status: "rejected",
+    limitation: "separator bytes",
+  },
+  "#missing-url": {
+    target: ["./miss\ting.cjs", "./config.cjs"],
+    status: "not-found",
+    limitation: JSON.stringify("./miss\ting.cjs"),
+  },
 };
 const allAliases = [...resolvedAliases, ...Object.keys(refusedAliases)];
 
@@ -168,7 +197,9 @@ cliTest.for(["CLI", "stdio MCP"] as const)(
     const imports = graph.edges.filter(
       ({ properties }) => properties.module_link_kind === "require",
     );
-    for (const [specifier, { limitation }] of Object.entries(refusedAliases))
+    for (const [specifier, { status, limitation }] of Object.entries(
+      refusedAliases,
+    ))
       for (const [moduleKind, sourcePath, source] of [
         ["require", "main.cjs", mainSource],
         ["import", "main.mjs", moduleSource],
@@ -179,7 +210,7 @@ cliTest.for(["CLI", "stdio MCP"] as const)(
               module_link_kind: moduleKind,
               specifier,
               resolved_path: null,
-              resolution_status: "external",
+              resolution_status: status,
             }),
             evidence: sourceEvidence(sourcePath, source, limitation),
           }),

tests/boundary/javascript/packageConditionalExports.test.ts

@@ -741,6 +741,51 @@ describe("nested and array package export targets", () => {
     },
   );
 });
+describe("package export target URL normalization compared with Node", () => {
+  it.each([
+    [["./.\t./outside.cjs", "./array.cjs"]],
+    [["./.%\n2e/outside.cjs", "./array.cjs"]],
+    [["./.\r./outside.cjs", "./array.cjs"]],
+    ["./arr\tay.cjs"],
+    ["./array.cjs \t"],
+    ["./array.cjs?selected=one#fragment"],
+    ["./.\t./example/array.cjs"],
+  ] as const)(
+    "resolves normalized target %j through both loaders",
+    async (target) => {
+      for (const moduleKind of ["import", "require"] as const) {
+        const { node, outcome } = await compareWithNode(target, moduleKind);
+        expect(node).toBe("array.cjs");
+        expect(outcome).toMatchObject({
+          resolution_status: "resolved",
+          resolved_path: "node_modules/example/array.cjs",
+        });
+      }
+    },
+  );
+
+  it.each([
+    [["./arr%2fay.cjs", "./array.cjs"], "rejected"],
+    [["./miss\ting.cjs", "./array.cjs"], "not-found"],
+  ] as const)(
+    "retains the first selected target failure in %j",
+    async (target, status) => {
+      for (const moduleKind of ["import", "require"] as const) {
+        const { node, node_error, outcome } = await compareWithNode(
+          target,
+          moduleKind,
+        );
+        expect(node).toBeNull();
+        expect(node_error).not.toBeNull();
+        expect(outcome).toMatchObject({
+          resolution_status: status,
+          resolved_path: null,
+        });
+      }
+    },
+  );
+});
+
 describe("unmatched package export conditions", () => {
   it.each([
     [[{ browser: "./default.cjs" }], "browser"],

tests/boundary/javascript/packageImportAliases.test.ts

@@ -455,6 +455,138 @@ describe("package import target arrays compared with Node", () => {
   });
 });
 
+describe("package import target URL normalization compared with Node", () => {
+  it("normalizes targets before package-root validation and array selection", async () => {
+    const root = await createTestTempDirectory("rea-package-target-url-");
+    const directory = "app space#?%λ";
+    const imports = {
+      "#tab-escape": ["./.\t./outside.cjs", "./fallback.cjs"],
+      "#newline-escape": ["./.%\n2e/outside.cjs", "./fallback.cjs"],
+      "#cr-escape": ["./.\r./outside.cjs", "./fallback.cjs"],
+      "#normalized": "./fall\tback.cjs",
+      "#trimmed": "./fallback.cjs \t",
+      "#suffix": "./fallback.cjs?selected=one#fragment",
+      "#return": `./.\t./${encodeURIComponent(directory)}/fallback.cjs`,
+    };
+    const contents = await writeTree(root, {
+      [`${directory}/package.json`]: JSON.stringify({ imports }),
+      [`${directory}/main.cjs`]: "module.exports = 1;",
+      [`${directory}/fallback.cjs`]: "module.exports = 1;",
+    });
+    const inventoried = inventory(contents);
+    for (const specifier of Object.keys(imports)) {
+      const required = await nodeRequireTarget(
+        join(root, directory, "main.cjs"),
+        specifier,
+      );
+      expect(required.error_code, specifier).toBeNull();
+      expect(posixRelative(root, required.target ?? "")).toBe(
+        `${directory}/fallback.cjs`,
+      );
+      const imported = await nodeImportTarget(join(root, directory), specifier);
+      expect(imported.error_code, specifier).toBeNull();
+      expect(posixRelative(root, fileURLToPath(imported.target ?? ""))).toBe(
+        `${directory}/fallback.cjs`,
+      );
+      for (const moduleKind of ["require", "import"] as const)
+        expect(
+          resolveAlias(
+            inventoried,
+            specifier,
+            `${directory}/main.cjs`,
+            moduleKind,
+          ),
+          specifier,
+        ).toMatchObject({
+          declared_path: specifier,
+          resolution_status: "resolved",
+          resolved_path: `${directory}/fallback.cjs`,
+        });
+    }
+  });
+
+  it("keeps URL decoding and missing-file failures after array selection", async () => {
+    const root = await createTestTempDirectory(
+      "rea-package-target-url-refusal-",
+    );
+    const contents = await writeTree(root, {
+      "package.json": JSON.stringify({
+        imports: {
+          "#escape": "./.\t./outside.cjs",
+          "#separator": ["./fall%2fback.cjs", "./fallback.cjs"],
+          "#missing": ["./miss\ting.cjs", "./fallback.cjs"],
+          "#query-separator": ["./first.cjs?selected=%2f", "./fallback.cjs"],
+        },
+      }),
+      "main.cjs": "module.exports = 1;",
+      "fallback.cjs": "module.exports = 1;",
+      "first.cjs": "module.exports = 2;",
+    });
+    const inventoried = inventory(contents);
+    for (const [specifier, code] of [
+      ["#escape", "ERR_INVALID_PACKAGE_TARGET"],
+      ["#separator", "ERR_INVALID_MODULE_SPECIFIER"],
+    ] as const) {
+      expect(
+        (await nodeRequireTarget(join(root, "main.cjs"), specifier)).error_code,
+      ).toBe(code);
+      expect((await nodeImportTarget(root, specifier)).error_code).toBe(code);
+      for (const moduleKind of ["require", "import"] as const)
+        expect(
+          resolveAlias(inventoried, specifier, "main.cjs", moduleKind),
+        ).toMatchObject({ resolution_status: "rejected", resolved_path: null });
+    }
+    expect(
+      (await nodeRequireTarget(join(root, "main.cjs"), "#missing")).error_code,
+    ).toBe("MODULE_NOT_FOUND");
+    const imported = await nodeImportTarget(root, "#missing");
+    expect(posixRelative(root, fileURLToPath(imported.target ?? ""))).toBe(
+      "missing.cjs",
+    );
+    for (const moduleKind of ["require", "import"] as const)
+      expect(
+        resolveAlias(inventoried, "#missing", "main.cjs", moduleKind),
+      ).toMatchObject({
+        resolution_status: "not-found",
+        resolved_path: null,
+        limitations: [expect.stringContaining("missing.cjs")],
+      });
+    expect(
+      resolveAlias(
+        inventoried,
+        "#escape",
+        "main.cjs",
+        "require",
+      ).limitations.join(" "),
+    ).toContain(JSON.stringify("./.\t./outside.cjs"));
+    expect(
+      (await nodeRequireTarget(join(root, "main.cjs"), "#query-separator"))
+        .error_code,
+    ).toBe("ERR_INVALID_MODULE_SPECIFIER");
+    expect(
+      resolveAlias(inventoried, "#query-separator", "main.cjs", "require"),
+    ).toMatchObject({ resolution_status: "rejected", resolved_path: null });
+    const withQuery = await nodeImportTarget(root, "#query-separator");
+    expect(posixRelative(root, fileURLToPath(withQuery.target ?? ""))).toBe(
+      "first.cjs",
+    );
+    expect(
+      resolveAlias(inventoried, "#query-separator", "main.cjs", "import"),
+    ).toMatchObject({
+      resolution_status: "resolved",
+      resolved_path: "first.cjs",
+    });
+    expect(
+      resolveAlias(
+        inventoried,
+        "#missing",
+        "main.cjs",
+        "require",
+      ).limitations.join(" "),
+    ).toContain(JSON.stringify("./miss\ting.cjs"));
+  });
+});
+
 describe("package import aliases compared with Node", () => {
   it("matches Node require and import for an exact root alias", async () => {
     const root = await createTestTempDirectory("rea-import-alias-root-");