From abc31e8ace84d6db386fa920ae3de8e552190218 Mon Sep 17 00:00:00 2001 From: GENTILHOMME Thomas Date: Wed, 18 Mar 2026 23:07:48 +0100 Subject: [PATCH] fix(tarball): properly manage relativeFile across EFA and DependencyCollectableSet --- .changeset/chubby-apes-battle.md | 5 + workspaces/rc/package.json | 2 +- workspaces/scanner/package.json | 2 +- workspaces/tarball/package.json | 2 +- .../class/DependencyCollectableSet.class.ts | 59 +++++++--- .../src/class/SourceCodeScanner.class.ts | 5 + .../test/DependencyCollectableSet.spec.ts | 111 +++++++++++------- .../tarball/test/SourceCodeScanner.spec.ts | 2 +- workspaces/tarball/test/tsconfig.json | 12 ++ workspaces/tree-walker/package.json | 2 +- 10 files changed, 140 insertions(+), 62 deletions(-) create mode 100644 .changeset/chubby-apes-battle.md create mode 100644 workspaces/tarball/test/tsconfig.json diff --git a/.changeset/chubby-apes-battle.md b/.changeset/chubby-apes-battle.md new file mode 100644 index 00000000..3bf0c02c --- /dev/null +++ b/.changeset/chubby-apes-battle.md @@ -0,0 +1,5 @@ +--- +"@nodesecure/tarball": patch +--- + +Properly implement metadata.relativePath across EFA and DependencyCollectableSet diff --git a/workspaces/rc/package.json b/workspaces/rc/package.json index 339a3027..879e20c2 100644 --- a/workspaces/rc/package.json +++ b/workspaces/rc/package.json @@ -45,7 +45,7 @@ "ajv": "8.18.0" }, "dependencies": { - "@nodesecure/js-x-ray": "14.1.0", + "@nodesecure/js-x-ray": "14.2.0", "@nodesecure/npm-types": "^1.2.0", "@nodesecure/vulnera": "3.0.0", "@openally/config": "^1.0.1", diff --git a/workspaces/scanner/package.json b/workspaces/scanner/package.json index cd0202df..dec1ac2f 100644 --- a/workspaces/scanner/package.json +++ b/workspaces/scanner/package.json @@ -68,7 +68,7 @@ "@nodesecure/contact": "^3.0.0", "@nodesecure/flags": "^3.0.3", "@nodesecure/i18n": "^4.1.0", - "@nodesecure/js-x-ray": "14.1.0", + "@nodesecure/js-x-ray": "14.2.0", "@nodesecure/mama": "^2.1.1", "@nodesecure/npm-registry-sdk": "^4.4.0", "@nodesecure/npm-types": "^1.3.0", diff --git a/workspaces/tarball/package.json b/workspaces/tarball/package.json index a4e4e0fc..11bbfc1b 100644 --- a/workspaces/tarball/package.json +++ b/workspaces/tarball/package.json @@ -47,7 +47,7 @@ "dependencies": { "@nodesecure/conformance": "^1.2.1", "@nodesecure/fs-walk": "^2.0.0", - "@nodesecure/js-x-ray": "14.1.0", + "@nodesecure/js-x-ray": "14.2.0", "@nodesecure/mama": "^2.1.1", "@nodesecure/npm-types": "^1.2.0", "@nodesecure/utils": "^2.3.0", diff --git a/workspaces/tarball/src/class/DependencyCollectableSet.class.ts b/workspaces/tarball/src/class/DependencyCollectableSet.class.ts index e6575840..a655f5fb 100644 --- a/workspaces/tarball/src/class/DependencyCollectableSet.class.ts +++ b/workspaces/tarball/src/class/DependencyCollectableSet.class.ts @@ -102,9 +102,11 @@ const kExternalThirdPartyDeps = new Set([ ]); const kRelativeImportPath = new Set([".", "..", "./", "../"]); -type Metadata = Dependency & { relativeFile: string; }; +export type DependencyCollectableSetMetadata = Dependency & { + relativeFile: string; +}; -export class DependencyCollectableSet implements CollectableSet { +export class DependencyCollectableSet implements CollectableSet { type = "dependency"; dependencies: Record< string, @@ -121,7 +123,9 @@ export class DependencyCollectableSet implements CollectableSet { #mama: Pick; #hasExternalCapacity: boolean = false; - constructor(mama: Pick) { + constructor( + mama: Pick + ) { this.#mama = mama; } @@ -151,8 +155,15 @@ export class DependencyCollectableSet implements CollectableSet { }; } - add(value: string, { metadata }: CollectableInfos) { - const relativeFile = metadata?.relativeFile!; + add( + value: string, + { metadata }: CollectableInfos + ) { + if (!metadata) { + return; + } + + const relativeFile = metadata.relativeFile; if (!(relativeFile in this.dependencies)) { this.dependencies[relativeFile] = Object.create(null); } @@ -165,7 +176,10 @@ export class DependencyCollectableSet implements CollectableSet { if (metadata?.inTry) { this.#dependenciesInTryBlock.add(value); } - const filtered = this.#filerDependencyByKind(value, relativeFile); + const filtered = this.#filerDependencyByKind( + value, + path.dirname(relativeFile) + ); if (filtered.file) { this.#files.add(filtered.file); @@ -178,7 +192,7 @@ export class DependencyCollectableSet implements CollectableSet { #filerDependencyByKind( dependency: string, - relativeFileLocation: string = "" + relativeFileLocation: string ) { const firstChar = dependency.charAt(0); @@ -202,13 +216,21 @@ export class DependencyCollectableSet implements CollectableSet { return { package: dependency }; } - relativeFileLocation: string; - #analyzeDependency(sourceDependency: string, inTry: boolean) { + #analyzeDependency( + sourceDependency: string, + inTry: boolean + ) { if (this.#values.has(sourceDependency)) { return; } - const { dependencies, devDependencies, nodejsImports = {} } = this.#mama; + + const { + dependencies, + devDependencies, + nodejsImports = {} + } = this.#mama; + let thirdPartyAliasedDependency: string | undefined; // See: https://nodejs.org/api/packages.html#subpath-imports if (this.#isAliasFileModule(sourceDependency) && sourceDependency in nodejsImports) { @@ -233,7 +255,10 @@ export class DependencyCollectableSet implements CollectableSet { this.#thirdPartyDependencies.add(name); } - if (thirdPartyDependency && this.#isMissingDependency(thirdPartyDependency, thirdPartyAliasedDependency)) { + if ( + thirdPartyDependency && + this.#isMissingDependency(thirdPartyDependency, thirdPartyAliasedDependency) + ) { this.#missingDependencies.add(thirdPartyDependency); } @@ -254,7 +279,10 @@ export class DependencyCollectableSet implements CollectableSet { } } - #extractDependencyName(sourceDependency: string, dependencies: string[]) { + #extractDependencyName( + sourceDependency: string, + dependencies: string[] + ) { for (const dependency of dependencies) { if (dependency === sourceDependency) { return sourceDependency; @@ -268,7 +296,10 @@ export class DependencyCollectableSet implements CollectableSet { return parseNpmSpec(sourceDependency)?.name ?? sourceDependency; } - #isMissingDependency(thirdPartyDependency: string, thirdPartyAliasedDependency: string | undefined) { + #isMissingDependency( + thirdPartyDependency: string, + thirdPartyAliasedDependency: string | undefined + ) { const { dependencies, nodejsImports = {} } = this.#mama; return !dependencies.includes(thirdPartyDependency) && @@ -306,7 +337,7 @@ export class DependencyCollectableSet implements CollectableSet { alias: string, nodeImports: Record ): [string, string] { - const importEntry = nodeImports[alias]!; + const importEntry = nodeImports[alias]; return typeof importEntry === "string" ? [alias, importEntry] : diff --git a/workspaces/tarball/src/class/SourceCodeScanner.class.ts b/workspaces/tarball/src/class/SourceCodeScanner.class.ts index aa87510a..781c2002 100644 --- a/workspaces/tarball/src/class/SourceCodeScanner.class.ts +++ b/workspaces/tarball/src/class/SourceCodeScanner.class.ts @@ -133,6 +133,11 @@ export class SourceCodeScanner< for await (const fileReport of efa.analyse(absoluteEntryFiles, { metadata: { spec: this.manifest.spec + }, + fileMetadata: (absoluteFile) => { + const relativeFile = path.relative(location, absoluteFile); + + return { relativeFile }; } })) { report.push(fileReport); diff --git a/workspaces/tarball/test/DependencyCollectableSet.spec.ts b/workspaces/tarball/test/DependencyCollectableSet.spec.ts index 901130e5..14a0e58f 100644 --- a/workspaces/tarball/test/DependencyCollectableSet.spec.ts +++ b/workspaces/tarball/test/DependencyCollectableSet.spec.ts @@ -3,14 +3,20 @@ import path from "node:path"; import { test, describe } from "node:test"; import assert from "node:assert"; +// Import Third-party Dependencies +import type { ManifestManager } from "@nodesecure/mama"; + // Import Internal Dependencies import { DependencyCollectableSet } from "../src/class/DependencyCollectableSet.class.ts"; +type Manifest = Pick; + describe("DependencyCollectableSet", () => { test("should have no dependencies initialy", () => { - const mama = { + const mama: Manifest = { dependencies: [], - devDependencies: [] + devDependencies: [], + nodejsImports: {} }; const dependencyCollectableSet = new DependencyCollectableSet(mama); @@ -18,9 +24,10 @@ describe("DependencyCollectableSet", () => { }); test("should group dependencies by file", () => { - const mama = { + const mama: Manifest = { dependencies: [], - devDependencies: [] + devDependencies: [], + nodejsImports: {} }; const dependencyCollectableSet = new DependencyCollectableSet(mama); @@ -71,9 +78,10 @@ describe("DependencyCollectableSet", () => { }); test("should detect Node.js dependencies and also flag hasExternalCapacity", () => { - const mama = { + const mama: Manifest = { dependencies: [], - devDependencies: [] + devDependencies: [], + nodejsImports: {} }; const dependencyCollectableSet = new DependencyCollectableSet(mama); @@ -121,9 +129,10 @@ describe("DependencyCollectableSet", () => { ]; for (const externalThridPartyDep of externalThridPartyDeps) { - const mama = { + const mama: Manifest = { dependencies: [externalThridPartyDep], - devDependencies: [] + devDependencies: [], + nodejsImports: {} }; const dependencyCollectableSet = new DependencyCollectableSet(mama); @@ -153,9 +162,10 @@ describe("DependencyCollectableSet", () => { test(`should detect no unused or missing dependencies and avoid confusion for package name with dots`, () => { - const mama = { + const mama: Manifest = { dependencies: ["lodash.isequal"], - devDependencies: [] + devDependencies: [], + nodejsImports: {} }; const dependencyCollectableSet = new DependencyCollectableSet(mama); @@ -182,9 +192,10 @@ describe("DependencyCollectableSet", () => { }); test("should detect prefixed (namespaced 'node:') core dependencies", () => { - const mama = { + const mama: Manifest = { dependencies: ["node:foo"], - devDependencies: [] + devDependencies: [], + nodejsImports: {} }; const dependencyCollectableSet = new DependencyCollectableSet(mama); @@ -227,9 +238,10 @@ describe("DependencyCollectableSet", () => { }); test("should be capable of detecting unused dependency 'koa'", () => { - const mama = { + const mama: Manifest = { dependencies: ["koa", "kleur"], - devDependencies: ["mocha"] + devDependencies: ["mocha"], + nodejsImports: {} }; const dependencyCollectableSet = new DependencyCollectableSet(mama); @@ -264,9 +276,10 @@ describe("DependencyCollectableSet", () => { }); test("should not detect unused dependencies on deep import", () => { - const mama = { + const mama: Manifest = { dependencies: ["koa", "kleur"], - devDependencies: ["mocha"] + devDependencies: ["mocha"], + nodejsImports: {} }; const dependencyCollectableSet = new DependencyCollectableSet(mama); @@ -309,9 +322,10 @@ describe("DependencyCollectableSet", () => { }); test("should be capable of detecting missing dependency 'kleur'", () => { - const mama = { + const mama: Manifest = { dependencies: ["mocha"], - devDependencies: [] + devDependencies: [], + nodejsImports: {} }; const dependencyCollectableSet = new DependencyCollectableSet(mama); @@ -346,9 +360,10 @@ describe("DependencyCollectableSet", () => { }); test("should ignore '@types' for third-party dependencies", () => { - const mama = { + const mama: Manifest = { dependencies: ["@types/npm"], - devDependencies: ["kleur"] + devDependencies: ["kleur"], + nodejsImports: {} }; const dependencyCollectableSet = new DependencyCollectableSet(mama); @@ -375,9 +390,10 @@ describe("DependencyCollectableSet", () => { }); test("should ignore file dependencies and try dependencies", () => { - const mama = { + const mama: Manifest = { dependencies: [], - devDependencies: ["kleur"] + devDependencies: ["kleur"], + nodejsImports: {} }; const dependencyCollectableSet = new DependencyCollectableSet(mama); @@ -492,9 +508,10 @@ describe("DependencyCollectableSet", () => { }); test("get all the dependencies name", () => { - const mama = { + const mama: Manifest = { dependencies: [], - devDependencies: [] + devDependencies: [], + nodejsImports: {} }; const dependencyCollectableSet = new DependencyCollectableSet(mama); @@ -518,9 +535,10 @@ describe("DependencyCollectableSet", () => { }); test("should be able to match all relative import path", () => { - const mama = { + const mama: Manifest = { dependencies: [], - devDependencies: [] + devDependencies: [], + nodejsImports: {} }; const dependencyCollectableSet1 = new DependencyCollectableSet(mama); const dependencyCollectableSet2 = new DependencyCollectableSet(mama); @@ -568,48 +586,55 @@ describe("DependencyCollectableSet", () => { }); test("should be able to match a file and join with the relative path", () => { - const mama = { + const mama: Manifest = { dependencies: [], - devDependencies: [] + devDependencies: [], + nodejsImports: {} }; const dependencyCollectableSet = new DependencyCollectableSet(mama); dependencyCollectableSet.add("./foobar.js", { - file: process.cwd(), location: [[0, 0], [0, 0]], metadata: { + file: process.cwd(), + location: [[0, 0], [0, 0]], + metadata: { unsafe: false, inTry: false, - relativeFile: process.cwd() + relativeFile: "src/entry.js" } }); assert.deepEqual([...dependencyCollectableSet.extract().files], [ - path.join(process.cwd(), "foobar.js") + path.join("src", "foobar.js") ]); }); test("should be able to automatically append the '.js' extension", () => { - const mama = { + const mama: Manifest = { dependencies: [], - devDependencies: [] + devDependencies: [], + nodejsImports: {} }; const dependencyCollectableSet = new DependencyCollectableSet(mama); dependencyCollectableSet.add("./foobar", { - file: process.cwd(), location: [[0, 0], [0, 0]], metadata: { + file: process.cwd(), + location: [[0, 0], [0, 0]], + metadata: { unsafe: false, inTry: false, - relativeFile: process.cwd() + relativeFile: "src/entry.js" } }); assert.deepEqual([...dependencyCollectableSet.extract().files], [ - path.join(process.cwd(), "foobar.js") + path.join("src", "foobar.js") ]); }); test("should detect all required dependencies (node, files, third-party)", () => { const thirdpartyDependencies = ["mocha", "yolo"]; - const mama = { + const mama: Manifest = { dependencies: thirdpartyDependencies, - devDependencies: [] + devDependencies: [], + nodejsImports: {} }; const dependencyCollectableSet = new DependencyCollectableSet(mama); @@ -617,7 +642,7 @@ describe("DependencyCollectableSet", () => { file: "one", location: [[0, 0], [0, 0]], metadata: { unsafe: false, inTry: false, - relativeFile: "one" + relativeFile: "one/index.js" } }); @@ -625,7 +650,7 @@ describe("DependencyCollectableSet", () => { file: "one", location: [[0, 0], [0, 0]], metadata: { unsafe: false, inTry: false, - relativeFile: "one" + relativeFile: "one/index.js" } }); @@ -633,7 +658,7 @@ describe("DependencyCollectableSet", () => { file: "one", location: [[0, 0], [0, 0]], metadata: { unsafe: false, inTry: false, - relativeFile: "one" + relativeFile: "one/index.js" } }); @@ -641,7 +666,7 @@ describe("DependencyCollectableSet", () => { file: "one", location: [[0, 0], [0, 0]], metadata: { unsafe: false, inTry: false, - relativeFile: "one" + relativeFile: "one/index.js" } }); @@ -649,7 +674,7 @@ describe("DependencyCollectableSet", () => { file: "one", location: [[0, 0], [0, 0]], metadata: { unsafe: false, inTry: false, - relativeFile: "one" + relativeFile: "one/index.js" } }); diff --git a/workspaces/tarball/test/SourceCodeScanner.spec.ts b/workspaces/tarball/test/SourceCodeScanner.spec.ts index 23a5daa6..87ccc3d4 100644 --- a/workspaces/tarball/test/SourceCodeScanner.spec.ts +++ b/workspaces/tarball/test/SourceCodeScanner.spec.ts @@ -189,7 +189,7 @@ describe("SourceCodeScanner", () => { if (firstReport.ok) { const { files, dependencies } = depsSet.extract(); assert.ok(dependencies.nodeJs.includes("node:http")); - assert.ok(files.has("bar.ts")); + assert.ok(files.has("src\\bar.ts")); } else { assert.fail("First report should be ok"); diff --git a/workspaces/tarball/test/tsconfig.json b/workspaces/tarball/test/tsconfig.json new file mode 100644 index 00000000..c095016c --- /dev/null +++ b/workspaces/tarball/test/tsconfig.json @@ -0,0 +1,12 @@ +{ + "extends": "../../../tsconfig.base.json", + "compilerOptions": { + "rootDir": "..", + "noEmit": true, + "composite": false + }, + "include": [ + "../src", + "." + ] +} diff --git a/workspaces/tree-walker/package.json b/workspaces/tree-walker/package.json index 0f735929..3d53d402 100644 --- a/workspaces/tree-walker/package.json +++ b/workspaces/tree-walker/package.json @@ -37,7 +37,7 @@ }, "homepage": "https://github.com/NodeSecure/tree/master/workspaces/tree-walker#readme", "dependencies": { - "@nodesecure/js-x-ray": "14.1.0", + "@nodesecure/js-x-ray": "14.2.0", "@nodesecure/npm-registry-sdk": "^4.0.0", "@nodesecure/npm-types": "^1.1.0", "@npmcli/arborist": "9.4.0",