From 82a5b98dbd58345e7d5ba9e10dffae2c3dd16667 Mon Sep 17 00:00:00 2001 From: Mario Tarosso Date: Thu, 4 Jun 2026 09:32:24 +0100 Subject: [PATCH] refactor: simplify lockfile parsers and core logic for maintainability Release 0.2.6 with several refactors and improvements: export isUuid from config and use it in the CLI (improves UUID validation and provisioning flow), introduce isSuccessStatus in client for clearer HTTP status checks, and refactor normalize.compare logic by extracting comparePartTokens and handling shorter segment ordering. Parsers: unify lockfile detection via LOCKFILE_SPECS (simpler probing and clearer error message), adjust npm lockfile name extraction to use forward slashes, add a small unquote() helper and apply it in pnpm and yarn parsers, and remove an unused path import. Public API changes: export NormalizeStats and return stats from scanAndReport. Misc: bumped package version to 0.2.6. --- package.json | 2 +- src/cli.ts | 25 ++++++++++------ src/client.ts | 6 +++- src/config.ts | 2 +- src/normalize.ts | 50 ++++++++++++++++++------------- src/parsers/index.ts | 67 ++++++++++++------------------------------ src/parsers/npm.ts | 5 ++-- src/parsers/pnpm.ts | 19 ++---------- src/parsers/strings.ts | 10 +++++++ src/parsers/yarn.ts | 20 ++----------- 10 files changed, 90 insertions(+), 116 deletions(-) create mode 100644 src/parsers/strings.ts diff --git a/package.json b/package.json index 8e203ce..d06de9e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@patchstack/connect", - "version": "0.2.5", + "version": "0.2.6", "description": "Patchstack connector for JavaScript applications. Scans your lockfile and reports installed packages to Patchstack for vulnerability monitoring.", "keywords": [ "patchstack", diff --git a/src/cli.ts b/src/cli.ts index a339e33..d92eaab 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -1,7 +1,7 @@ import { scanLockfile } from './parsers/index.js'; import { buildWirePayload } from './normalize.js'; import { buildClaimUrl, postManifest } from './client.js'; -import { persistSiteUuid, resolveConfig, writeConfigFile } from './config.js'; +import { isUuid, persistSiteUuid, resolveConfig, writeConfigFile } from './config.js'; import { PatchstackError } from './types.js'; const HELP = `@patchstack/connect — scan your lockfile and report packages to Patchstack. @@ -87,7 +87,7 @@ async function runInit(args: ParsedArgs): Promise { console.error('Usage: patchstack-connect init '); return 1; } - if (!/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(uuid)) { + if (!isUuid(uuid)) { console.error(`Error: "${uuid}" does not look like a valid UUID.`); return 1; } @@ -142,11 +142,18 @@ async function runScan(args: ParsedArgs): Promise { const response = await postManifest(config, payload); - // The server always returns the UUID. If we didn't have one, persist it so - // every subsequent scan targets the same site. - if (provisioning && response.uuid !== undefined && response.uuid.length > 0) { - const target = await persistSiteUuid(process.cwd(), response.uuid); - console.log(`Provisioned site ${response.uuid}. Saved UUID to ${target}.`); + // When provisioning, the server returns a fresh UUID for this site. (null in + // every other case — either we already had one, or the server didn't send one.) + const provisionedUuid = + provisioning && response.uuid !== undefined && response.uuid.length > 0 + ? response.uuid + : null; + + // If we didn't have a UUID, persist the provisioned one so every subsequent + // scan targets the same site. + if (provisionedUuid !== null) { + const target = await persistSiteUuid(process.cwd(), provisionedUuid); + console.log(`Provisioned site ${provisionedUuid}. Saved UUID to ${target}.`); } if (response.stored) { @@ -160,10 +167,10 @@ async function runScan(args: ParsedArgs): Promise { // On the first scan (provisioning), surface the claim URL so the user can // attach this site to their Patchstack account. `npx @patchstack/connect status` // re-displays it any time. - if (provisioning && response.uuid !== undefined && response.uuid.length > 0) { + if (provisionedUuid !== null) { console.log(''); console.log('Claim this site to view vulnerability reports in your Patchstack dashboard:'); - console.log(` ${buildClaimUrl(config.endpoint, response.uuid)}`); + console.log(` ${buildClaimUrl(config.endpoint, provisionedUuid)}`); } return 0; diff --git a/src/client.ts b/src/client.ts index 61a3da8..aecf781 100644 --- a/src/client.ts +++ b/src/client.ts @@ -79,7 +79,7 @@ export async function postManifest( ); } - if (response.status < 200 || response.status >= 300) { + if (!isSuccessStatus(response.status)) { throw new PatchstackError( `Patchstack returned ${response.status}: ${text.slice(0, 200)}`, 'SERVER_ERROR', @@ -93,6 +93,10 @@ export async function postManifest( return body; } +function isSuccessStatus(status: number): boolean { + return status >= 200 && status < 300; +} + function isTimeoutError(cause: unknown): boolean { if (cause instanceof Error) { return cause.name === 'TimeoutError' || cause.name === 'AbortError'; diff --git a/src/config.ts b/src/config.ts index 58ee699..3c3bf11 100644 --- a/src/config.ts +++ b/src/config.ts @@ -126,6 +126,6 @@ function readEnv(): ConfigFile { }; } -function isUuid(value: string): boolean { +export function isUuid(value: string): boolean { return /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(value); } diff --git a/src/normalize.ts b/src/normalize.ts index 31ffb9f..22f349d 100644 --- a/src/normalize.ts +++ b/src/normalize.ts @@ -101,37 +101,47 @@ function compareSegments(a: string[], b: string[]): number { for (let i = 0; i < max; i++) { const aPart = a[i]; const bPart = b[i]; + // A shorter segment list sorts before a longer one (e.g. `1.0` before `1.0.1`). if (aPart === undefined) { return -1; } if (bPart === undefined) { return 1; } - const aNum = /^\d+$/.test(aPart); - const bNum = /^\d+$/.test(bPart); - if (aNum && bNum) { - const diff = Number(aPart) - Number(bPart); - if (diff !== 0) { - return diff < 0 ? -1 : 1; - } - continue; - } - if (aNum) { - return -1; - } - if (bNum) { - return 1; - } - if (aPart < bPart) { - return -1; - } - if (aPart > bPart) { - return 1; + const cmp = comparePartTokens(aPart, bPart); + if (cmp !== 0) { + return cmp; } } return 0; } +/** + * Compares two version tokens. Numeric tokens sort numerically and ahead of + * non-numeric ones; otherwise tokens fall back to lexical comparison. + */ +function comparePartTokens(a: string, b: string): number { + const aNum = /^\d+$/.test(a); + const bNum = /^\d+$/.test(b); + if (aNum && bNum) { + const diff = Number(a) - Number(b); + return diff === 0 ? 0 : diff < 0 ? -1 : 1; + } + if (aNum) { + return -1; + } + if (bNum) { + return 1; + } + if (a < b) { + return -1; + } + if (a > b) { + return 1; + } + return 0; +} + export function findPackageInManifest( manifest: Manifest, name: string, diff --git a/src/parsers/index.ts b/src/parsers/index.ts index 86e621d..528fd2b 100644 --- a/src/parsers/index.ts +++ b/src/parsers/index.ts @@ -26,55 +26,26 @@ interface DetectedLockfile { strategy: DetectionStrategy; } -export async function detectLockfile(cwd: string): Promise { - const npmLock = path.join(cwd, 'package-lock.json'); - if (await exists(npmLock)) { - return { - ecosystem: 'npm', - filePath: npmLock, - filename: 'package-lock.json', - strategy: 'npm-lockfile', - }; - } - - const bunLock = path.join(cwd, 'bun.lock'); - if (await exists(bunLock)) { - return { - ecosystem: 'npm', - filePath: bunLock, - filename: 'bun.lock', - strategy: 'node-modules-walk', - }; - } - - const bunLockB = path.join(cwd, 'bun.lockb'); - if (await exists(bunLockB)) { - return { - ecosystem: 'npm', - filePath: bunLockB, - filename: 'bun.lockb', - strategy: 'node-modules-walk', - }; - } - - const pnpmLock = path.join(cwd, 'pnpm-lock.yaml'); - if (await exists(pnpmLock)) { - return { - ecosystem: 'npm', - filePath: pnpmLock, - filename: 'pnpm-lock.yaml', - strategy: 'pnpm-lockfile', - }; - } +// Probed in order; the first match wins, so more specific lockfiles must come +// before fallbacks. bun has no standalone parser, so it falls back to walking +// node_modules. +const LOCKFILE_SPECS: ReadonlyArray<{ + filename: LockfileFilename; + strategy: DetectionStrategy; +}> = [ + { filename: 'package-lock.json', strategy: 'npm-lockfile' }, + { filename: 'bun.lock', strategy: 'node-modules-walk' }, + { filename: 'bun.lockb', strategy: 'node-modules-walk' }, + { filename: 'pnpm-lock.yaml', strategy: 'pnpm-lockfile' }, + { filename: 'yarn.lock', strategy: 'yarn-lockfile' }, +]; - const yarnLock = path.join(cwd, 'yarn.lock'); - if (await exists(yarnLock)) { - return { - ecosystem: 'npm', - filePath: yarnLock, - filename: 'yarn.lock', - strategy: 'yarn-lockfile', - }; +export async function detectLockfile(cwd: string): Promise { + for (const { filename, strategy } of LOCKFILE_SPECS) { + const filePath = path.join(cwd, filename); + if (await exists(filePath)) { + return { ecosystem: 'npm', filePath, filename, strategy }; + } } throw new PatchstackError( diff --git a/src/parsers/npm.ts b/src/parsers/npm.ts index 89cc90f..50d65ab 100644 --- a/src/parsers/npm.ts +++ b/src/parsers/npm.ts @@ -1,5 +1,4 @@ import { readFile } from 'node:fs/promises'; -import path from 'node:path'; import { PatchstackError, type PackageEntry } from '../types.js'; interface LockfileV2Package { @@ -96,11 +95,11 @@ function extractFromV1( } function extractNameFromPath(pkgPath: string): string | null { - const segments = pkgPath.split('node_modules' + path.sep === pkgPath ? path.sep : '/'); + // npm lockfile keys always use forward slashes, regardless of platform. const parts = pkgPath.split('/'); const nmIndex = parts.lastIndexOf('node_modules'); if (nmIndex === -1 || nmIndex >= parts.length - 1) { - return segments[segments.length - 1] ?? null; + return parts[parts.length - 1] ?? null; } const tail = parts.slice(nmIndex + 1); if (tail.length === 0) { diff --git a/src/parsers/pnpm.ts b/src/parsers/pnpm.ts index 7196a52..a6fa14e 100644 --- a/src/parsers/pnpm.ts +++ b/src/parsers/pnpm.ts @@ -1,5 +1,6 @@ import { readFile } from 'node:fs/promises'; import { PatchstackError, type PackageEntry } from '../types.js'; +import { unquote } from './strings.js'; /** * Parses pnpm-lock.yaml without pulling in a YAML library. We don't need full @@ -62,12 +63,7 @@ export function parsePackageKey(rawKey: string): ParsedKey | null { return null; } - if ( - (k.startsWith("'") && k.endsWith("'")) || - (k.startsWith('"') && k.endsWith('"')) - ) { - k = k.slice(1, -1); - } + k = unquote(k); if (k.startsWith('/')) { k = k.slice(1); @@ -343,15 +339,6 @@ function extractLeafName(trimmed: string): string | null { if (colonIdx < 0) { return null; } - let name = trimmed.slice(0, colonIdx).trim(); - if (name.length === 0) { - return null; - } - if ( - (name.startsWith("'") && name.endsWith("'")) || - (name.startsWith('"') && name.endsWith('"')) - ) { - name = name.slice(1, -1); - } + const name = unquote(trimmed.slice(0, colonIdx).trim()); return name.length > 0 ? name : null; } diff --git a/src/parsers/strings.ts b/src/parsers/strings.ts new file mode 100644 index 0000000..7c37ccb --- /dev/null +++ b/src/parsers/strings.ts @@ -0,0 +1,10 @@ +/** Strips a single matching pair of surrounding single or double quotes. */ +export function unquote(s: string): string { + if ( + (s.startsWith("'") && s.endsWith("'")) || + (s.startsWith('"') && s.endsWith('"')) + ) { + return s.slice(1, -1); + } + return s; +} diff --git a/src/parsers/yarn.ts b/src/parsers/yarn.ts index d2b1393..f0bc390 100644 --- a/src/parsers/yarn.ts +++ b/src/parsers/yarn.ts @@ -1,6 +1,7 @@ import { readFile } from 'node:fs/promises'; import path from 'node:path'; import { PatchstackError, type PackageEntry } from '../types.js'; +import { unquote } from './strings.js'; /** * Parses yarn.lock (yarn classic v1 and yarn berry v2+) without a YAML @@ -173,16 +174,10 @@ export function splitDescriptors(keyLine: string): string[] { * version comes from the `version` field of the block. */ export function extractName(rawSpec: string): string | null { - let s = rawSpec.trim(); + const s = unquote(rawSpec.trim()); if (s.length === 0) { return null; } - if ( - (s.startsWith('"') && s.endsWith('"')) || - (s.startsWith("'") && s.endsWith("'")) - ) { - s = s.slice(1, -1); - } // Position-0 `@` belongs to a scope, so we want the last `@` after it. const atIdx = s.lastIndexOf('@'); if (atIdx <= 0) { @@ -204,16 +199,7 @@ function parseVersionField(content: string): string | null { return null; } let rest = firstChar === ':' ? after.slice(1) : after; - rest = rest.trim(); - if (rest.length === 0) { - return null; - } - if ( - (rest.startsWith('"') && rest.endsWith('"')) || - (rest.startsWith("'") && rest.endsWith("'")) - ) { - rest = rest.slice(1, -1); - } + rest = unquote(rest.trim()); return rest.length > 0 ? rest : null; }