From 7fc00f68b01519939096609522fab3e6e5cee4ba Mon Sep 17 00:00:00 2001 From: "Thomas.G" Date: Mon, 8 Jul 2024 14:58:10 +0200 Subject: [PATCH] fix: use object-hash & expose utils from mama for Scanner (#263) --- package-lock.json | 1 + workspaces/mama/src/ManifestManager.class.ts | 154 +++++++++++++++++ workspaces/mama/src/index.ts | 162 +----------------- workspaces/mama/src/utils/index.ts | 1 + workspaces/mama/src/utils/integrity-hash.ts | 37 ++++ .../test/packageJSONIntegrityHash.spec.ts | 55 ++++++ workspaces/scanner/package.json | 1 + workspaces/scanner/src/npmRegistry.ts | 37 +--- .../fixtures/depWalker/slimio.is-result.json | 2 +- workspaces/scanner/test/npmRegistry.spec.ts | 2 +- workspaces/scanner/tsconfig.json | 3 + 11 files changed, 264 insertions(+), 191 deletions(-) create mode 100644 workspaces/mama/src/ManifestManager.class.ts create mode 100644 workspaces/mama/src/utils/index.ts create mode 100644 workspaces/mama/src/utils/integrity-hash.ts create mode 100644 workspaces/mama/test/packageJSONIntegrityHash.spec.ts diff --git a/package-lock.json b/package-lock.json index 5ad7e55..f07aa99 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9136,6 +9136,7 @@ "@nodesecure/flags": "^2.4.0", "@nodesecure/i18n": "^4.0.1", "@nodesecure/js-x-ray": "^6.3.0", + "@nodesecure/mama": "^1.0.0", "@nodesecure/npm-registry-sdk": "^3.0.0", "@nodesecure/npm-types": "^1.0.0", "@nodesecure/tarball": "^1.0.0", diff --git a/workspaces/mama/src/ManifestManager.class.ts b/workspaces/mama/src/ManifestManager.class.ts new file mode 100644 index 0000000..afcd169 --- /dev/null +++ b/workspaces/mama/src/ManifestManager.class.ts @@ -0,0 +1,154 @@ +// Import Node.js Dependencies +import fs from "node:fs/promises"; +import path from "node:path"; + +// Import Third-party Dependencies +import { parseAuthor } from "@nodesecure/utils"; +import type { + PackumentVersion, PackageJSON, WorkspacesPackageJSON, Contact +} from "@nodesecure/npm-types"; + +// Import Internal Dependencies +import { packageJSONIntegrityHash } from "./utils/index.js"; + +type WithRequired = T & { [P in K]-?: T[P] } + +export type NonOptionalPackageJSONProperties = + "dependencies" | + "devDependencies" | + "scripts" | + "gypfile"; + +// CONSTANTS +const kNativeNpmPackages = new Set([ + "node-gyp", + "node-pre-gyp", + "node-gyp-build", + "node-addon-api" +]); + +/** + * @see https://www.nerdycode.com/prevent-npm-executing-scripts-security/ + */ +export const kUnsafeNPMScripts = new Set([ + "install", + "preinstall", + "postinstall", + "preuninstall", + "postuninstall" +]); + +export type ManifestManagerDefaultProperties = Required< + Pick +>; + +export type ManifestManagerDocument = + PackageJSON | + WorkspacesPackageJSON | + PackumentVersion; + +export class ManifestManager< + MetadataDef extends Record = Record +> { + static Default: Readonly = Object.freeze({ + dependencies: {}, + devDependencies: {}, + scripts: {}, + gypfile: false + }); + + public metadata: MetadataDef = Object.create(null); + public document: WithRequired< + ManifestManagerDocument, + NonOptionalPackageJSONProperties + >; + + public flags = Object.seal({ + hasUnsafeScripts: false, + isNative: false + }); + + constructor( + document: ManifestManagerDocument + ) { + this.document = Object.assign( + { ...ManifestManager.Default }, + structuredClone(document) + ); + + this.flags.isNative = [ + ...this.dependencies, + ...this.devDependencies + ].some((pkg) => kNativeNpmPackages.has(pkg)) || this.document.gypfile; + this.flags.hasUnsafeScripts = Object + .keys(this.document.scripts) + .some((script) => kUnsafeNPMScripts.has(script.toLowerCase())); + } + + get nodejsImports() { + return this.document.imports ?? {}; + } + + get dependencies() { + return Object.keys(this.document.dependencies); + } + + get devDependencies() { + return Object.keys(this.document.devDependencies); + } + + get spec(): `${string}@${string}` { + const hasBothProperties = ["name", "version"] + .every((key) => key in this.document); + if (this.isWorkspace && !hasBothProperties) { + throw new Error("spec is not available for the given workspace"); + } + + return `${this.document.name}@${this.document.version}`; + } + + get author(): Contact | null { + return parseAuthor(this.document.author); + } + + get isWorkspace(): boolean { + return "workspaces" in this.document; + } + + get integrity(): string { + if (this.isWorkspace) { + throw new Error("integrity is not available for workspaces"); + } + + return packageJSONIntegrityHash(this.document); + } + + static async fromPackageJSON( + location: string + ): Promise { + if (typeof location !== "string") { + throw new TypeError("location must be a string primitive"); + } + + const packageLocation = location.endsWith("package.json") ? + location : + path.join(location, "package.json"); + const packageStr = await fs.readFile(packageLocation, "utf-8"); + + try { + const packageJSON = JSON.parse( + packageStr + ) as PackageJSON | WorkspacesPackageJSON; + + return new ManifestManager( + packageJSON + ); + } + catch (cause) { + throw new Error( + `Failed to parse package.json located at: ${packageLocation}`, + { cause } + ); + } + } +} diff --git a/workspaces/mama/src/index.ts b/workspaces/mama/src/index.ts index 5a3ce3d..d6c3c05 100644 --- a/workspaces/mama/src/index.ts +++ b/workspaces/mama/src/index.ts @@ -1,158 +1,4 @@ -// Import Node.js Dependencies -import fs from "node:fs/promises"; -import path from "node:path"; - -// Import Third-party Dependencies -import hash from "object-hash"; -import { parseAuthor } from "@nodesecure/utils"; -import type { - PackumentVersion, PackageJSON, WorkspacesPackageJSON, Contact -} from "@nodesecure/npm-types"; - -type WithRequired = T & { [P in K]-?: T[P] } - -export type NonOptionalPackageJSONProperties = - "dependencies" | - "devDependencies" | - "scripts" | - "gypfile"; - -// CONSTANTS -const kNativeNpmPackages = new Set([ - "node-gyp", - "node-pre-gyp", - "node-gyp-build", - "node-addon-api" -]); - -/** - * @see https://www.nerdycode.com/prevent-npm-executing-scripts-security/ - */ -export const kUnsafeNPMScripts = new Set([ - "install", - "preinstall", - "postinstall", - "preuninstall", - "postuninstall" -]); - -export type ManifestManagerDefaultProperties = Required< - Pick ->; - -export type ManifestManagerDocument = - PackageJSON | - WorkspacesPackageJSON | - PackumentVersion; - -export class ManifestManager< - MetadataDef extends Record = Record -> { - static Default: Readonly = Object.freeze({ - dependencies: {}, - devDependencies: {}, - scripts: {}, - gypfile: false - }); - - public metadata: MetadataDef = Object.create(null); - public document: WithRequired< - ManifestManagerDocument, - NonOptionalPackageJSONProperties - >; - - public flags = Object.seal({ - hasUnsafeScripts: false, - isNative: false - }); - - constructor( - document: ManifestManagerDocument - ) { - this.document = Object.assign( - { ...ManifestManager.Default }, - structuredClone(document) - ); - - this.flags.isNative = [ - ...this.dependencies, - ...this.devDependencies - ].some((pkg) => kNativeNpmPackages.has(pkg)) || this.document.gypfile; - this.flags.hasUnsafeScripts = Object - .keys(this.document.scripts) - .some((script) => kUnsafeNPMScripts.has(script.toLowerCase())); - } - - get nodejsImports() { - return this.document.imports ?? {}; - } - - get dependencies() { - return Object.keys(this.document.dependencies); - } - - get devDependencies() { - return Object.keys(this.document.devDependencies); - } - - get spec(): `${string}@${string}` { - const hasBothProperties = ["name", "version"] - .every((key) => key in this.document); - if (this.isWorkspace && !hasBothProperties) { - throw new Error("spec is not available for the given workspace"); - } - - return `${this.document.name}@${this.document.version}`; - } - - get author(): Contact | null { - return parseAuthor(this.document.author); - } - - get isWorkspace(): boolean { - return "workspaces" in this.document; - } - - get integrity(): string { - if (this.isWorkspace) { - throw new Error("integrity is not available for workspaces"); - } - - return hash({ - name: this.document.name, - version: this.document.version, - dependencies: this.document.dependencies, - license: this.document.license ?? "NONE", - scripts: this.document.scripts - }); - } - - static async fromPackageJSON( - location: string - ): Promise { - if (typeof location !== "string") { - throw new TypeError("location must be a string primitive"); - } - - const packageLocation = location.endsWith("package.json") ? - location : - path.join(location, "package.json"); - const packageStr = await fs.readFile(packageLocation, "utf-8"); - - try { - const packageJSON = JSON.parse( - packageStr - ) as PackageJSON | WorkspacesPackageJSON; - - return new ManifestManager( - packageJSON - ); - } - catch (cause) { - throw new Error( - `Failed to parse package.json located at: ${packageLocation}`, - { cause } - ); - } - } -} +export * from "./ManifestManager.class.js"; +export { + packageJSONIntegrityHash +} from "./utils/index.js"; diff --git a/workspaces/mama/src/utils/index.ts b/workspaces/mama/src/utils/index.ts new file mode 100644 index 0000000..f99c4ab --- /dev/null +++ b/workspaces/mama/src/utils/index.ts @@ -0,0 +1 @@ +export * from "./integrity-hash.js"; diff --git a/workspaces/mama/src/utils/integrity-hash.ts b/workspaces/mama/src/utils/integrity-hash.ts new file mode 100644 index 0000000..ad31cf2 --- /dev/null +++ b/workspaces/mama/src/utils/integrity-hash.ts @@ -0,0 +1,37 @@ +// Import Third-party Dependencies +import hash from "object-hash"; +import type { + PackumentVersion, PackageJSON, WorkspacesPackageJSON +} from "@nodesecure/npm-types"; + +export interface packageJSONIntegrityHashOptions { + /** + * Know whether the document comes from the NPM registry or a local tarball/project + * + * @default false + */ + isFromRemoteRegistry?: boolean; +} + +export function packageJSONIntegrityHash( + document: PackumentVersion | PackageJSON | WorkspacesPackageJSON, + options: packageJSONIntegrityHashOptions = {} +) { + const { isFromRemoteRegistry = false } = options; + const { dependencies = {}, license = "NONE", scripts = {} } = document; + + if (isFromRemoteRegistry) { + // See https://github.com/npm/cli/issues/5234 + if ("install" in dependencies && dependencies.install === "node-gyp rebuild") { + delete dependencies.install; + } + } + + return hash({ + name: document.name, + version: document.version, + dependencies, + license, + scripts + }); +} diff --git a/workspaces/mama/test/packageJSONIntegrityHash.spec.ts b/workspaces/mama/test/packageJSONIntegrityHash.spec.ts new file mode 100644 index 0000000..8def8a4 --- /dev/null +++ b/workspaces/mama/test/packageJSONIntegrityHash.spec.ts @@ -0,0 +1,55 @@ +// Import Node.js Dependencies +import assert from "node:assert"; +import { describe, test } from "node:test"; + +// Import Third-party Dependencies +import type { + PackageJSON, + WorkspacesPackageJSON +} from "@nodesecure/npm-types"; +import hash from "object-hash"; + +// Import Internal Dependencies +import { packageJSONIntegrityHash } from "../src/utils/index.js"; + +// CONSTANTS +const kMinimalPackageJSON = { + name: "foo", + version: "1.5.0" +}; +const kMinimalPackageJSONIntegrity = hash({ + ...kMinimalPackageJSON, + dependencies: {}, + scripts: {}, + license: "NONE" +}); + +describe("packageJSONIntegrityHash", () => { + test("Given isFromRemoteRegistry: true then it should remove install script if it contains 'node-gyp rebuild'", () => { + const integrityHash = packageJSONIntegrityHash({ + ...kMinimalPackageJSON, + dependencies: { + install: "node-gyp rebuild" + } + }, { isFromRemoteRegistry: true }); + + assert.strictEqual( + integrityHash, + kMinimalPackageJSONIntegrity + ); + }); + + test("Given isFromRemoteRegistry: false then the integrity should not match", () => { + const integrityHash = packageJSONIntegrityHash({ + ...kMinimalPackageJSON, + dependencies: { + install: "node-gyp rebuild" + } + }); + + assert.notStrictEqual( + integrityHash, + kMinimalPackageJSONIntegrity + ); + }); +}); diff --git a/workspaces/scanner/package.json b/workspaces/scanner/package.json index cf9ac59..0580140 100644 --- a/workspaces/scanner/package.json +++ b/workspaces/scanner/package.json @@ -53,6 +53,7 @@ "@nodesecure/flags": "^2.4.0", "@nodesecure/i18n": "^4.0.1", "@nodesecure/js-x-ray": "^6.3.0", + "@nodesecure/mama": "^1.0.0", "@nodesecure/npm-registry-sdk": "^3.0.0", "@nodesecure/npm-types": "^1.0.0", "@nodesecure/tarball": "^1.0.0", diff --git a/workspaces/scanner/src/npmRegistry.ts b/workspaces/scanner/src/npmRegistry.ts index 2d5ef65..70d6fd7 100644 --- a/workspaces/scanner/src/npmRegistry.ts +++ b/workspaces/scanner/src/npmRegistry.ts @@ -1,10 +1,7 @@ -// Import Node.js Dependencies -import crypto from "node:crypto"; - // Import Third-party Dependencies import semver from "semver"; import * as npmRegistrySDK from "@nodesecure/npm-registry-sdk"; -import type { PackumentVersion } from "@nodesecure/npm-types"; +import { packageJSONIntegrityHash } from "@nodesecure/mama"; // Import Internal Dependencies import { getLinks } from "./utils/index.js"; @@ -26,7 +23,9 @@ export async function manifestMetadata( version ); - const integrity = getPackumentVersionIntegrity(pkgVersion); + const integrity = packageJSONIntegrityHash(pkgVersion, { + isFromRemoteRegistry: true + }); Object.assign( dependency.versions[version], { @@ -90,8 +89,8 @@ export async function packageMetadata( flags.push("isDeprecated"); } - metadata.integrity[ver.version] = getPackumentVersionIntegrity( - ver + metadata.integrity[ver.version] = packageJSONIntegrityHash( + ver, { isFromRemoteRegistry: true } ); } @@ -137,30 +136,6 @@ export async function packageMetadata( } } -function getPackumentVersionIntegrity( - packumentVersion: PackumentVersion -): string { - const { name, version, dependencies = {}, license = "", scripts = {} } = packumentVersion; - - // See https://github.com/npm/cli/issues/5234 - if ("install" in dependencies && dependencies.install === "node-gyp rebuild") { - delete dependencies.install; - } - - const integrityObj = { - name, - version, - dependencies, - license, - scripts - }; - - return crypto - .createHash("sha256") - .update(JSON.stringify(integrityObj)) - .digest("hex"); -} - async function addNpmAvatar( metadata: Dependency["metadata"] ): Promise { diff --git a/workspaces/scanner/test/fixtures/depWalker/slimio.is-result.json b/workspaces/scanner/test/fixtures/depWalker/slimio.is-result.json index fcfc47e..8fa955c 100644 --- a/workspaces/scanner/test/fixtures/depWalker/slimio.is-result.json +++ b/workspaces/scanner/test/fixtures/depWalker/slimio.is-result.json @@ -118,7 +118,7 @@ } ], "integrity": { - "1.5.1": "c9781c55ab750e58bed9ce2560581ff4087b8c3129462543fa6fee4e717ba2a9" + "1.5.1": "d4466c4f46f5bf6550f569f0938d0a607e0cf6e9" } } } diff --git a/workspaces/scanner/test/npmRegistry.spec.ts b/workspaces/scanner/test/npmRegistry.spec.ts index a6b626d..6f3ca50 100644 --- a/workspaces/scanner/test/npmRegistry.spec.ts +++ b/workspaces/scanner/test/npmRegistry.spec.ts @@ -37,7 +37,7 @@ test("registry.manifestMetadata", async() => { assert.equal(Object.keys(dep.metadata).length, 1); assert.deepEqual(dep.metadata, { integrity: { - "1.5.0": "d9cdfeeddb9e5cadfa4188942b4456e2a9c2f60787e772e59394076711ebb9e1" + "1.5.0": "0df0f03a28f621111c667e3b50db97a24abf5c02" } }); assert.deepEqual(dep.versions["1.5.0"], { diff --git a/workspaces/scanner/tsconfig.json b/workspaces/scanner/tsconfig.json index 5ac8f52..32d1468 100644 --- a/workspaces/scanner/tsconfig.json +++ b/workspaces/scanner/tsconfig.json @@ -12,6 +12,9 @@ { "path": "../conformance" }, + { + "path": "../mama" + }, { "path": "../tarball" },