From 063cd25a93ee9f16b45635fea196dde0d9309ab9 Mon Sep 17 00:00:00 2001 From: Kaoru0822 Date: Mon, 5 Oct 2026 07:28:51 -0700 Subject: [PATCH] fix(javascript): preserve literal CommonJS path punctuation (#576) --- src/application/ElectronBoundaryAnalysis.ts | 3 + .../ElectronBoundaryGraphNative.ts | 1 + .../JavaScriptArtifactPathResolution.ts | 3 +- .../JavaScriptModuleSpecifierPaths.test.ts | 77 ++++++++++ src/domain/electronNativeLoaderPaths.test.ts | 51 +++++++ src/domain/electronStaticAnalysisNative.ts | 28 +++- src/domain/electronStaticAnalysisTypes.ts | 1 + .../javascriptRequireLiteralPaths.test.ts | 133 ++++++++++++++++++ 8 files changed, 290 insertions(+), 7 deletions(-) create mode 100644 src/application/JavaScriptModuleSpecifierPaths.test.ts create mode 100644 src/domain/electronNativeLoaderPaths.test.ts create mode 100644 tests/boundary/filesystem/javascriptRequireLiteralPaths.test.ts diff --git a/src/application/ElectronBoundaryAnalysis.ts b/src/application/ElectronBoundaryAnalysis.ts index 531e3f907..e362d8593 100644 --- a/src/application/ElectronBoundaryAnalysis.ts +++ b/src/application/ElectronBoundaryAnalysis.ts @@ -169,6 +169,7 @@ export const summarizeElectronBoundaries = ( context: "module-specifier", files, kind: "native-addon", + moduleKind: finding.module_kind, }), ).length, }; @@ -201,6 +202,7 @@ interface ResolveToFileInput { readonly context: "module-specifier" | "filesystem-expression"; readonly files: ReadonlyMap; readonly kind?: JavaScriptArtifactFile["kind"]; + readonly moduleKind?: "import" | "require"; } const resolvesToFile = (input: ResolveToFileInput): boolean => { @@ -209,6 +211,7 @@ const resolvesToFile = (input: ResolveToFileInput): boolean => { sourcePath: input.file.path, context: input.context, files: input.files, + ...(input.moduleKind === undefined ? {} : { moduleKind: input.moduleKind }), }).resolved_path; return ( resolved !== null && diff --git a/src/application/ElectronBoundaryGraphNative.ts b/src/application/ElectronBoundaryGraphNative.ts index 6e8101d0f..d12d83927 100644 --- a/src/application/ElectronBoundaryGraphNative.ts +++ b/src/application/ElectronBoundaryGraphNative.ts @@ -42,6 +42,7 @@ const addNativeBinding = (input: NativeBindingInput): void => { value.specifier, file.path, context.filesByPath, + value.module_kind, ); const addon = resolved === null ? undefined : context.fileNodes.get(resolved); const binding = context.accumulator.addNode({ diff --git a/src/application/JavaScriptArtifactPathResolution.ts b/src/application/JavaScriptArtifactPathResolution.ts index 92462c4de..e981df814 100644 --- a/src/application/JavaScriptArtifactPathResolution.ts +++ b/src/application/JavaScriptArtifactPathResolution.ts @@ -113,7 +113,8 @@ const contextualCandidate = ( ): string | ArtifactPathResolution => { const { context } = input; const declared = - context === "module-specifier" || context === "html-reference" + context === "html-reference" || + (context === "module-specifier" && input.moduleKind !== "require") ? stripQueryAndFragment(input.declaredPath) : input.declaredPath; if (context === "html-reference") return htmlCandidate(input); diff --git a/src/application/JavaScriptModuleSpecifierPaths.test.ts b/src/application/JavaScriptModuleSpecifierPaths.test.ts new file mode 100644 index 000000000..cd5e3eedd --- /dev/null +++ b/src/application/JavaScriptModuleSpecifierPaths.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it } from "vitest"; + +import type { JavaScriptArtifactFile } from "./JavaScriptArtifactFiles.js"; +import { resolveArtifactPathByContext } from "./JavaScriptArtifactPathResolution.js"; + +const filesFor = (paths: readonly string[]) => + new Map( + ["app/main.js", ...paths].map((path) => [ + path, + { + path, + container_sha256: "a".repeat(64), + sha256: "b".repeat(64), + bytes: 0, + inventory_artifact_id: `artifact-${path}`, + kind: "javascript", + unpacked: false, + text: { included: true, value: "" }, + }, + ]), + ); + +const cases = ["#", "?"].flatMap((punctuation) => + (["require", "import", undefined] as const).flatMap((moduleKind) => + (["literal", "stripped", "both"] as const).map((layout) => ({ + punctuation, + moduleKind, + layout, + })), + ), +); + +describe("module specifier punctuation", () => { + it.each(cases)( + "resolves $moduleKind with $punctuation and $layout files", + ({ punctuation, moduleKind, layout }) => { + const literal = `app/dep.cjs${punctuation}literal.cjs`; + const stripped = "app/dep.cjs"; + const files = filesFor([ + ...(layout !== "stripped" ? [literal] : []), + ...(layout !== "literal" ? [stripped] : []), + ]); + const expected = moduleKind === "require" ? literal : stripped; + const result = resolveArtifactPathByContext({ + declaredPath: `./dep.cjs${punctuation}literal.cjs`, + sourcePath: "app/main.js", + context: "module-specifier", + files, + ...(moduleKind === undefined ? {} : { moduleKind }), + }); + expect(result).toMatchObject({ + resolution_status: files.has(expected) ? "resolved" : "not-found", + resolved_path: files.has(expected) ? expected : null, + }); + }, + ); + + it.each(["#", "?"])( + "keeps HTML URL suffix handling for %s", + (punctuation) => { + expect( + resolveArtifactPathByContext({ + declaredPath: `./dep.cjs${punctuation}literal.cjs`, + sourcePath: "app/main.js", + context: "html-reference", + files: filesFor([ + "app/dep.cjs", + `app/dep.cjs${punctuation}literal.cjs`, + ]), + }), + ).toMatchObject({ + resolution_status: "resolved", + resolved_path: "app/dep.cjs", + }); + }, + ); +}); diff --git a/src/domain/electronNativeLoaderPaths.test.ts b/src/domain/electronNativeLoaderPaths.test.ts new file mode 100644 index 000000000..c4baa2ef9 --- /dev/null +++ b/src/domain/electronNativeLoaderPaths.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from "vitest"; + +import { analyzeJavaScriptStaticSource } from "./javascriptStaticAnalysis.js"; + +describe("native addon loader provenance", () => { + it("distinguishes CommonJS and ESM re-exports", () => { + const analysis = analyzeJavaScriptStaticSource(` + const required = require("./addon.node#literal.node"); + module.exports = require("./addon.node#literal.node"); + import imported from "./addon.node#literal.node"; + export { default } from "./addon.node#literal.node"; + `); + expect(analysis.electron.native_addon_bindings).toHaveLength(4); + expect(analysis.electron.native_addon_bindings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + binding_kind: "require", + module_kind: "require", + }), + expect.objectContaining({ + binding_kind: "import", + module_kind: "import", + }), + expect.objectContaining({ + binding_kind: "re-export", + module_kind: "require", + }), + expect.objectContaining({ + binding_kind: "re-export", + module_kind: "import", + }), + ]), + ); + }); + + it.each(["#", "?"])( + "classifies require filenames literally for %s", + (punctuation) => { + const analysis = analyzeJavaScriptStaticSource(` + const script = require("./addon.node${punctuation}helper.js"); + import native from "./addon.node${punctuation}helper.js"; + `); + expect(analysis.electron.native_addon_bindings).toEqual([ + expect.objectContaining({ + binding_kind: "import", + module_kind: "import", + }), + ]); + }, + ); +}); diff --git a/src/domain/electronStaticAnalysisNative.ts b/src/domain/electronStaticAnalysisNative.ts index 0176e2522..288624091 100644 --- a/src/domain/electronStaticAnalysisNative.ts +++ b/src/domain/electronStaticAnalysisNative.ts @@ -17,6 +17,7 @@ interface NativeBindingInput { readonly node: t.Node; readonly specifier: string; readonly kind: ElectronNativeAddonBindingFinding["binding_kind"]; + readonly moduleKind: ElectronNativeAddonBindingFinding["module_kind"]; readonly members: readonly string[]; } @@ -36,7 +37,7 @@ const inspectImport = ( node: t.ImportDeclaration, context: JavaScriptFindingContext, ): void => { - if (!isNativeSpecifier(node.source.value)) return; + if (!isNativeSpecifier(node.source.value, "import")) return; const members = node.specifiers.map((specifier) => { if (t.isImportDefaultSpecifier(specifier)) return "default"; if (t.isImportNamespaceSpecifier(specifier)) return "*"; @@ -47,6 +48,7 @@ const inspectImport = ( node, specifier: node.source.value, kind: "import", + moduleKind: "import", members, }); }; @@ -58,7 +60,7 @@ const inspectNamedExport = ( if ( node.source === null || node.source === undefined || - !isNativeSpecifier(node.source.value) + !isNativeSpecifier(node.source.value, "import") ) return; const members = node.specifiers.map((specifier) => { @@ -70,6 +72,7 @@ const inspectNamedExport = ( node, specifier: node.source.value, kind: "re-export", + moduleKind: "import", members, }); }; @@ -78,13 +81,17 @@ const inspectExportAll = ( node: t.ExportAllDeclaration, context: JavaScriptFindingContext, ): void => { - if (node.source === undefined || !isNativeSpecifier(node.source.value)) + if ( + node.source === undefined || + !isNativeSpecifier(node.source.value, "import") + ) return; addBinding({ context, node, specifier: node.source.value, kind: "re-export", + moduleKind: "import", members: ["*"], }); }; @@ -100,6 +107,7 @@ const inspectRequireBinding = ( node, specifier: required.specifier, kind: "require", + moduleKind: "require", members: required.member === null ? bindingMembers(node.id) : [required.member], }); @@ -116,6 +124,7 @@ const inspectReExport = ( node, specifier: required.specifier, kind: "re-export", + moduleKind: "require", members: [required.member ?? exportedMember(node.left) ?? "*"], }); }; @@ -133,6 +142,7 @@ const addBinding = (input: NativeBindingInput): void => { value: { specifier, binding_kind: kind, + module_kind: input.moduleKind, members, module_key: null, location: range(node), @@ -162,7 +172,7 @@ const nativeRequire = ( ) return undefined; const specifier = stringValue(argumentNode(node.arguments[0])); - return specifier !== undefined && isNativeSpecifier(specifier) + return specifier !== undefined && isNativeSpecifier(specifier, "require") ? { specifier, member: null } : undefined; }; @@ -193,7 +203,13 @@ const exportedMember = (node: t.Node): string | undefined => { return undefined; }; -const isNativeSpecifier = (specifier: string): boolean => { - const path = specifier.split("#", 1)[0]?.split("?", 1)[0] ?? ""; +const isNativeSpecifier = ( + specifier: string, + moduleKind: ElectronNativeAddonBindingFinding["module_kind"], +): boolean => { + const path = + moduleKind === "require" + ? specifier + : (specifier.split("#", 1)[0]?.split("?", 1)[0] ?? ""); return path.toLowerCase().endsWith(".node"); }; diff --git a/src/domain/electronStaticAnalysisTypes.ts b/src/domain/electronStaticAnalysisTypes.ts index 15264c3d9..824c1ad8f 100644 --- a/src/domain/electronStaticAnalysisTypes.ts +++ b/src/domain/electronStaticAnalysisTypes.ts @@ -162,6 +162,7 @@ export type ElectronUtilityProcessFinding = ElectronUtilityProcessFindingState & export interface ElectronNativeAddonBindingFinding { readonly specifier: string; readonly binding_kind: "import" | "require" | "re-export"; + readonly module_kind: "import" | "require"; readonly members: readonly string[]; readonly module_key: string | null; readonly location: JavaScriptSourceRange; diff --git a/tests/boundary/filesystem/javascriptRequireLiteralPaths.test.ts b/tests/boundary/filesystem/javascriptRequireLiteralPaths.test.ts new file mode 100644 index 000000000..a6de4fc3e --- /dev/null +++ b/tests/boundary/filesystem/javascriptRequireLiteralPaths.test.ts @@ -0,0 +1,133 @@ +import { writeFile } from "node:fs/promises"; +import { createRequire } from "node:module"; +import { join, relative, sep } from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { analyzeJavaScriptApplication } from "../../../src/application/JavaScriptApplicationService.js"; +import { parseApplicationGraphEvidence } from "../../../src/application/JavaScriptApplicationEvidenceGraph.js"; +import { javascriptApplicationAnalysisResultSchema } from "../../../src/domain/javascriptApplicationAnalysis.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +const analyze = async (root: string) => { + const result = await analyzeJavaScriptApplication({ + input_path: root, + format: "directory", + }); + if (!result.ok) throw result.error; + const authenticated = parseApplicationGraphEvidence(result.value); + return { + graph: authenticated.graph, + result: javascriptApplicationAnalysisResultSchema.parse( + authenticated.evidence.normalized_result, + ), + }; +}; + +describe("literal CommonJS paths in application graphs", () => { + it("agrees with Node for both require projections while preserving ESM suffixes", async () => { + const root = await createTestTempDirectory("rea-literal-require-"); + const specifier = "./dep.cjs#literal.cjs"; + await Promise.all([ + writeFile( + join(root, "main.cjs"), + `const value = require(${JSON.stringify(specifier)}); module.exports = value;`, + ), + writeFile( + join(root, "imports.mjs"), + `import value from ${JSON.stringify(specifier)}; export { value };`, + ), + writeFile(join(root, "dep.cjs"), "module.exports = 'stripped';"), + writeFile( + join(root, "dep.cjs#literal.cjs"), + "module.exports = 'literal';", + ), + ]); + const expected = relative( + root, + createRequire(join(root, "main.cjs")).resolve(specifier), + ) + .split(sep) + .join("/"); + expect(expected).toBe("dep.cjs#literal.cjs"); + const { graph } = await analyze(root); + const properties = graph.edges + .filter((edge) => edge.properties.specifier === specifier) + .map((edge) => edge.properties); + expect(properties).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + module_link_kind: "require", + resolved_path: expected, + }), + expect.objectContaining({ kind: "require", resolved_path: expected }), + expect.objectContaining({ + module_link_kind: "import", + resolved_path: "dep.cjs", + }), + expect.objectContaining({ + kind: "static-import", + resolved_path: "dep.cjs", + }), + ]), + ); + expect(graph.coverage.status).toBe("complete"); + }); + + it.each(["literal", "stripped", "both"] as const)( + "keeps native graph/count projections aligned with %s files", + async (layout) => { + const root = await createTestTempDirectory("rea-literal-native-"); + const specifier = "./addon.node#literal.node"; + await Promise.all([ + writeFile( + join(root, "main.cjs"), + `const addon = require(${JSON.stringify(specifier)}); module.exports = require(${JSON.stringify(specifier)}); throw new Error('must not execute');`, + ), + writeFile( + join(root, "imports.mjs"), + `import addon from ${JSON.stringify(specifier)}; export { default } from ${JSON.stringify(specifier)}; throw new Error('must not execute');`, + ), + ...(layout !== "stripped" + ? [ + writeFile( + join(root, "addon.node#literal.node"), + Buffer.from([0, 1, 2, 3]), + ), + ] + : []), + ...(layout !== "literal" + ? [writeFile(join(root, "addon.node"), Buffer.from([0, 4, 5, 6]))] + : []), + ]); + const { graph, result } = await analyze(root); + const bindings = graph.nodes + .filter(({ kind }) => kind === "native-export") + .flatMap(({ observations }) => observations) + .map((observation) => ({ + source: + observation.evidence.location.available && + observation.evidence.location.value.kind === "source-range" + ? observation.evidence.location.value.source + : null, + binding: observation.properties.binding_kind, + path: observation.properties.resolved_path, + })); + const required = layout !== "stripped" ? "addon.node#literal.node" : null; + const imported = layout !== "literal" ? "addon.node" : null; + expect(bindings).toHaveLength(4); + expect(bindings).toEqual( + expect.arrayContaining([ + { source: "main.cjs", binding: "require", path: required }, + { source: "main.cjs", binding: "re-export", path: required }, + { source: "imports.mjs", binding: "import", path: imported }, + { source: "imports.mjs", binding: "re-export", path: imported }, + ]), + ); + expect(result.summary).toMatchObject({ + native_addon_bindings: 4, + resolved_native_addon_bindings: layout === "both" ? 4 : 2, + }); + }, + ); +});