From f961912597c5d63bf15785c7cf363e29e015ad2d Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 10 Oct 2026 12:04:22 +0000 Subject: [PATCH 1/2] fix: fail scan on a truncated lockfile vg build already stopped when a lockfile was truncated. vg scan did not: a nested package-lock.json, yarn.lock, or pnpm-lock.yaml was treated as absent, or the code-map step was skipped, and the command exited 0. Check in-scope lockfiles before drift scoring and before parse workers start, including vg scan --no-graph. The error names the file and the format and does not include file contents. When several lockfiles are invalid, the path that sorts first is the one reported. Closes #310 Signed-off-by: Cursor Agent Co-authored-by: vibgrate-team --- CHANGELOG.md | 4 + DOCS.md | 2 +- src/core-open/run-core-scan.ts | 6 +- src/engine/discover.ts | 141 ++++++++++++++---- src/engine/lockfile.test.ts | 49 +++++- src/engine/truncated-lockfile.cli.test.ts | 88 ++++++++++- src/reporting/commands/scan.ts | 10 ++ .../truncated-lockfile/package-lock.json | 1 + .../truncated-lockfile/pnpm-lock.yaml | 4 + .../__fixtures__/truncated-lockfile/yarn.lock | 3 + vibgrate.config.json | 6 +- 11 files changed, 278 insertions(+), 36 deletions(-) create mode 100644 test/__fixtures__/truncated-lockfile/package-lock.json create mode 100644 test/__fixtures__/truncated-lockfile/pnpm-lock.yaml create mode 100644 test/__fixtures__/truncated-lockfile/yarn.lock diff --git a/CHANGELOG.md b/CHANGELOG.md index 887e092f..8189952e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ backward compatible. ## [Unreleased] +### Fixed + +- **A truncated lockfile fails `vg scan` and `vg build`.** A `package-lock.json`, `yarn.lock`, `pnpm-lock.yaml`, or peer lockfile cut off mid-file exits non-zero. The error names that file and the format, and tells you to regenerate it with your package manager. It does not include the file's contents. `vg scan` checks before scoring drift or starting parse workers, including `--no-graph` and a lockfile nested in a subdirectory. A nested lockfile used to be reported as "no lockfile", or the code map was skipped, and the command exited 0. When several lockfiles are invalid, the error names the path that sorts first. + ### Changed - **Machine-readable output drops absolute home-directory paths (#282).** diff --git a/DOCS.md b/DOCS.md index b6b753ce..9c670045 100644 --- a/DOCS.md +++ b/DOCS.md @@ -2152,7 +2152,7 @@ pnpm-lock.yaml: unknown optional lockfile field "futureOptional"; continuing wit The path is relative to the directory you ran the command from, when the lockfile is under that directory. Otherwise the warning uses the file name only. The warning names the field. It does not print the field's value, an environment value, or an authorization header. A field name that is not a plain identifier is reported as `(name omitted)`. -A lockfile that is truncated, empty, or missing the structure its format requires still fails the command. The error names the file and the format, tells you to regenerate the file with your package manager, and the process exits non-zero. The error does not include the file's contents. +A lockfile that is truncated, empty, or missing the structure its format requires still fails the command. The error names the file and the format, tells you to regenerate the file with your package manager, and the process exits non-zero. The error does not include the file's contents. `vg scan` performs this check before it scores drift or starts parse workers, including with `--no-graph`, and including a lockfile in a subdirectory. `vg build` checks during discovery, before the parse pool starts. When more than one lockfile is invalid, the error names the path that sorts first. `vg scan` and `vg sbom export` follow this for npm, pnpm, yarn, poetry, uv, pdm, Pipfile, Cargo, Composer, NuGet, Swift package pins, and pub. A Gradle lock line, a Gemfile spec, and a `go.sum` line do not have optional field names. A line that does not match that format is invalid, and the command fails. diff --git a/src/core-open/run-core-scan.ts b/src/core-open/run-core-scan.ts index a6d6ebc5..4b29df1e 100644 --- a/src/core-open/run-core-scan.ts +++ b/src/core-open/run-core-scan.ts @@ -37,6 +37,7 @@ import { formatSarif } from './formatters/sarif.js'; import { formatMarkdown } from './formatters/markdown.js'; import { loadConfig, appendExcludePatterns } from './config.js'; import { pathExists, readJsonFile, writeJsonFile, writeTextFile, ensureDir, FileCache, quickTreeCount } from './utils/fs.js'; +import { LockfileParseError } from './utils/lockfile-parse.js'; import { portableValue } from './utils/portable-path.js'; import { assertSafeWalkRoot } from './utils/root-safety.js'; import { detectVcs } from './utils/vcs.js'; @@ -764,7 +765,10 @@ export async function runCoreScan( { projects: allProjects, solutions, extended }, ); progress.completeStep('map', detail || 'done'); - } catch { + } catch (err) { + // A truncated lockfile is not a map failure. Swallowing it printed + // "skipped (map build failed)" and let the scan exit 0. + if (err instanceof LockfileParseError) throw err; progress.completeStep('map', 'skipped (map build failed)'); } } diff --git a/src/engine/discover.ts b/src/engine/discover.ts index c5113b9a..d070482d 100644 --- a/src/engine/discover.ts +++ b/src/engine/discover.ts @@ -4,7 +4,7 @@ import ignore, { type Ignore } from 'ignore'; import { langForExtension, langById, type LanguageDef } from './languages.js'; import { requireDataConfig } from '../core-open/config.js'; import { dropBlankPatterns, gitignoreWithoutBlankLines } from '../core-open/utils/glob.js'; -import { assertLockfileFile, lockfileKind } from '../core-open/utils/lockfile-parse.js'; +import { assertLockfileFile, lockfileKind, LockfileParseError } from '../core-open/utils/lockfile-parse.js'; import { assertSafeWalkRoot, createWalkBudget, noteWalkEntry } from '../core-open/utils/root-safety.js'; import { emitSkippedSymlinkNotice } from '../core-open/utils/skipped-symlinks.js'; @@ -226,49 +226,86 @@ export function loadRootIgnore(root: string, exclude: string[]): Ignore { return ig; } -export function discover(options: DiscoverOptions): DiscoveredFile[] { - const root = path.resolve(options.root); - const onlyLangs = (options.only ?? []).filter(Boolean); - const allowLang = (lang: LanguageDef): boolean => - onlyLangs.length === 0 || onlyLangs.includes(lang.id); +interface ProjectWalk { + root: string; + exclude?: string[]; + paths?: string[]; + maxEntries?: number; + /** Source files to keep. Omit when the caller only validates lockfiles. */ + found?: Map; + allowLang?: (lang: LanguageDef) => boolean; + /** + * Symlinks this walk skipped. Omit to stay quiet — `vg scan` validates + * lockfiles before the file walk that prints the one notice. + */ + skippedSymlinks?: string[]; +} - // Validate `--only` ids early so a typo is a usage error, not silent emptiness. - for (const id of onlyLangs) { - if (!langById(id)) { - throw new UsageError(`unknown language "${id}" for --only`); - } - } +/** + * The invalid lockfile whose path sorts first. Directory entries are sorted + * before the walk, and this pick does not depend on which of two bad files + * `readdir` returned first. + */ +function firstLockfileError(errors: readonly LockfileParseError[]): LockfileParseError | undefined { + if (errors.length === 0) return undefined; + return [...errors].sort((a, b) => (a.filePath < b.filePath ? -1 : a.filePath > b.filePath ? 1 : 0))[0]; +} +/** + * Walk a project the way {@link discover} does. Lockfiles are syntax-checked + * here, on this thread, before any source parse worker exists. A truncated + * file is recorded and the walk continues so the reported path is stable. + */ +function walkProject(options: ProjectWalk): LockfileParseError[] { + const root = path.resolve(options.root); const rootIg = loadRootIgnore(root, options.exclude ?? []); + const allowLang = options.allowLang ?? ((): boolean => true); + const found = options.found; + const skippedSymlinks = options.skippedSymlinks; + const lockfileErrors: LockfileParseError[] = []; + const budget = createWalkBudget(root, options.maxEntries); - // Scope roots: explicit paths, or the whole repo. const scopeAbs = (options.paths && options.paths.length ? options.paths.map((p) => path.resolve(root, p)) : [root] ).filter((p) => fs.existsSync(p)); - const found = new Map(); - const budget = createWalkBudget(root, options.maxEntries); - // Symlinks are not followed. Dirent.isDirectory() / isFile() are false for a - // link, so a directory link to its parent cannot re-enter this walk. The - // notice lists the ones this walk skipped; ignored links stay quiet. - const skippedSymlinks: string[] = []; - const considerFile = (abs: string): void => { const rel = toPosix(path.relative(root, abs)); if (rel.startsWith('..')) return; // outside root if (rel === '' || rootIg.ignores(rel)) return; if (SKIP_FILES.has(path.basename(abs).toLowerCase())) { - // Lockfiles are not source, but a truncated one must fail the build - // here — before the parse pool starts — rather than being skipped. - if (lockfileKind(path.basename(abs))) assertLockfileFile(abs); + // Lockfiles are not source. A truncated one must fail the command + // rather than be skipped into an empty dependency graph. + if (lockfileKind(path.basename(abs))) { + try { + assertLockfileFile(abs); + } catch (err) { + if (err instanceof LockfileParseError) { + lockfileErrors.push(err); + return; + } + throw err; + } + } return; } + if (!found) return; const lang = langForExtension(path.extname(abs)); if (!lang || !allowLang(lang)) return; found.set(rel, { rel, abs, lang }); }; + const note = (): void => { + const over = noteWalkEntry(budget); + if (!over) return; + // A lockfile we already know is bad is the error to show. The budget + // still stops a walk that has not seen one. + const lockfileError = firstLockfileError(lockfileErrors); + if (lockfileError) throw lockfileError; + throw over; + }; + const walk = (dir: string): void => { let entries: fs.Dirent[]; try { @@ -276,24 +313,24 @@ export function discover(options: DiscoverOptions): DiscoveredFile[] { } catch { return; // unreadable dir — skip rather than crash the build } + // Sorted so two invalid lockfiles always surface the same path. + entries.sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0)); for (const entry of entries) { const abs = path.join(dir, entry.name); const rel = toPosix(path.relative(root, abs)); if (entry.isSymbolicLink()) { if (rel && (rootIg.ignores(rel) || rootIg.ignores(`${rel}/`))) continue; - if (rel) skippedSymlinks.push(rel); + if (rel && skippedSymlinks) skippedSymlinks.push(rel); continue; } if (entry.isDirectory()) { if (isSkippedDirName(entry.name)) continue; if (rel && rootIg.ignores(`${rel}/`)) continue; - const over = noteWalkEntry(budget); - if (over) throw over; + note(); walk(abs); } else if (entry.isFile()) { if (rel && rootIg.ignores(rel)) continue; - const over = noteWalkEntry(budget); - if (over) throw over; + note(); considerFile(abs); } } @@ -307,10 +344,58 @@ export function discover(options: DiscoverOptions): DiscoveredFile[] { } else if (stat.isFile()) considerFile(scope); } + return lockfileErrors; +} + +export function discover(options: DiscoverOptions): DiscoveredFile[] { + const onlyLangs = (options.only ?? []).filter(Boolean); + const allowLang = (lang: LanguageDef): boolean => + onlyLangs.length === 0 || onlyLangs.includes(lang.id); + + // Validate `--only` ids early so a typo is a usage error, not silent emptiness. + for (const id of onlyLangs) { + if (!langById(id)) { + throw new UsageError(`unknown language "${id}" for --only`); + } + } + + const found = new Map(); + // Symlinks are not followed. Dirent.isDirectory() / isFile() are false for a + // link, so a directory link to its parent cannot re-enter this walk. The + // notice lists the ones this walk skipped; ignored links stay quiet. + const skippedSymlinks: string[] = []; + const lockfileError = firstLockfileError(walkProject({ + root: options.root, + exclude: options.exclude, + paths: options.paths, + maxEntries: options.maxEntries, + found, + allowLang, + skippedSymlinks, + })); + // Before the symlink notice, and before the caller starts parse workers. + if (lockfileError) throw lockfileError; emitSkippedSymlinkNotice(skippedSymlinks); return [...found.values()].sort((a, b) => (a.rel < b.rel ? -1 : a.rel > b.rel ? 1 : 0)); } +/** + * Fail when any in-scope lockfile is truncated or invalid. Same skip and + * ignore rules as {@link discover}. Does not print the symlink notice and + * does not start parse workers. `vg scan` calls this before drift scoring. + */ +export function assertDiscoveredLockfiles( + options: Pick, +): void { + const lockfileError = firstLockfileError(walkProject({ + root: options.root, + exclude: options.exclude, + paths: options.paths, + maxEntries: options.maxEntries, + })); + if (lockfileError) throw lockfileError; +} + /** A usage error that maps to exit code 5 at the CLI boundary. */ export class UsageError extends Error { readonly isUsageError = true; diff --git a/src/engine/lockfile.test.ts b/src/engine/lockfile.test.ts index 04282545..82bf1bad 100644 --- a/src/engine/lockfile.test.ts +++ b/src/engine/lockfile.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { discover } from './discover.js'; +import { assertDiscoveredLockfiles, discover } from './discover.js'; import { LockfileParseError } from '../core-open/utils/lockfile-parse.js'; import { lockfileVersion, fullDependencyTree, fullDependencyGraph } from './lockfile.js'; @@ -381,3 +381,50 @@ describe('fullDependencyGraph', () => { expect(fullDependencyGraph(root)?.ecosystem).toBe('pypi'); }); }); + +const TRUNCATED_JSON = '{"name":"fixture","lockfileVersion":3,"packages":{"node_modules/left-pad":{"version":"1.0.0"'; + +describe('truncated lockfiles in the project walk', () => { + it('names the lexicographically first invalid lockfile', () => { + fs.mkdirSync(path.join(root, 'b')); + fs.mkdirSync(path.join(root, 'a')); + fs.writeFileSync(path.join(root, 'b', 'package-lock.json'), TRUNCATED_JSON); + fs.writeFileSync( + path.join(root, 'a', 'pnpm-lock.yaml'), + "lockfileVersion: '9.0'\npackages:\n left-pad@1.0.0:\n resolution: {integrity: sha512-abc", + ); + const expected = path.join(root, 'a', 'pnpm-lock.yaml'); + expect(() => discover({ root })).toThrow(LockfileParseError); + expect(() => assertDiscoveredLockfiles({ root })).toThrow(expected); + try { + assertDiscoveredLockfiles({ root }); + } catch (err) { + const message = (err as Error).message; + expect(message).toContain('truncated or invalid YAML'); + expect(message).toContain('package manager'); + expect(message).not.toContain('left-pad'); + expect(message).not.toContain('sha512-abc'); + expect(message).not.toContain(path.join(root, 'b', 'package-lock.json')); + } + }); + + it('does not fail on a truncated lockfile under node_modules or a gitignore rule', () => { + fs.mkdirSync(path.join(root, 'node_modules', 'left-pad'), { recursive: true }); + fs.writeFileSync(path.join(root, 'node_modules', 'left-pad', 'package-lock.json'), TRUNCATED_JSON); + fs.mkdirSync(path.join(root, 'skip-lock')); + fs.writeFileSync(path.join(root, 'skip-lock', 'yarn.lock'), 'chalk@^5.0.0:\n'); + write('.gitignore', 'skip-lock/\n'); + write('app.ts', 'export const n = 1;\n'); + expect(() => assertDiscoveredLockfiles({ root })).not.toThrow(); + expect(discover({ root }).map((f) => f.rel)).toEqual(['app.ts']); + }); + + it('still fails a nested lockfile an exclude does not cover', () => { + fs.mkdirSync(path.join(root, 'packages', 'web'), { recursive: true }); + fs.writeFileSync(path.join(root, 'packages', 'web', 'package-lock.json'), TRUNCATED_JSON); + write('app.ts', 'export const n = 1;\n'); + expect(() => assertDiscoveredLockfiles({ root, exclude: ['other/**'] })).toThrow( + path.join(root, 'packages', 'web', 'package-lock.json'), + ); + }); +}); diff --git a/src/engine/truncated-lockfile.cli.test.ts b/src/engine/truncated-lockfile.cli.test.ts index 2011f15e..b3486bb5 100644 --- a/src/engine/truncated-lockfile.cli.test.ts +++ b/src/engine/truncated-lockfile.cli.test.ts @@ -1,4 +1,4 @@ -import { spawnSync } from 'node:child_process'; +import { execFileSync, spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -11,6 +11,7 @@ import { describe, it, expect, afterEach } from 'vitest'; */ const pkgRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); const cli = path.join(pkgRoot, 'src/cli.ts'); +const fixtureDir = path.join(pkgRoot, 'test/__fixtures__/truncated-lockfile'); const secret = 'npm_DDDDDDDDDDDDDDDDDDDD'; function fixture(): string { @@ -23,12 +24,29 @@ function fixture(): string { return dir; } -function run(dir: string, command: 'scan' | 'build'): { status: number | null; signal: NodeJS.Signals | null; stderr: string; error?: Error } { +function copyFixture(dir: string, name: string, destName = name): void { + fs.copyFileSync(path.join(fixtureDir, name), path.join(dir, destName)); +} + +/** Parent exited. An orphan parse worker would still mention the fixture path. */ +function assertNoOrphan(dir: string): void { + let listing = ''; + try { + listing = execFileSync('ps', ['-eo', 'args'], { encoding: 'utf8' }); + } catch { + return; + } + expect(listing).not.toContain(dir); +} + +function run(dir: string, command: 'scan' | 'build', graph = false): { status: number | null; signal: NodeJS.Signals | null; stderr: string; error?: Error } { // `-C` makes the fixture the project root. A path argument alone is a scope // inside the process cwd, so a lockfile outside that cwd would be skipped. + const scanArgs = [cli, 'scan', dir, '--offline', '--no-daemon', '--quiet']; + if (!graph) scanArgs.push('--no-graph'); const args = command === 'scan' - ? [cli, 'scan', dir, '--offline', '--no-graph', '--no-daemon', '--quiet'] + ? scanArgs : [cli, 'build', '-C', dir, '--no-warm', '--no-publish', '--offline', '--no-daemon']; const res = spawnSync(process.execPath, ['--import', 'tsx', ...args], { cwd: pkgRoot, @@ -62,6 +80,68 @@ describe('truncated lockfile CLI', () => { expect(res.stderr).toContain('package manager'); expect(res.stderr).not.toContain(secret); expect(res.stderr).not.toContain('left-pad'); + assertNoOrphan(dir); + }, 60_000); + + it('vg scan fails a nested package-lock.json before reporting an empty graph', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vg-trunc-lock-')); + dirs.push(dir); + fs.mkdirSync(path.join(dir, 'packages', 'web'), { recursive: true }); + copyFixture(path.join(dir, 'packages', 'web'), 'package-lock.json'); + fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"root","version":"1.0.0"}\n'); + fs.writeFileSync(path.join(dir, 'app.ts'), 'export const n = 1;\n'); + const res = run(dir, 'scan'); + expect(res.error).toBeUndefined(); + expect(res.signal).toBeNull(); + expect(res.status).toBe(1); + expect(res.stderr).toContain(path.join(dir, 'packages', 'web', 'package-lock.json')); + expect(res.stderr).toContain('truncated or invalid JSON'); + expect(res.stderr).toContain('package manager'); + expect(res.stderr).not.toContain('left-pad'); + expect(res.stderr).not.toContain('map build failed'); + expect(fs.existsSync(path.join(dir, '.vibgrate'))).toBe(false); + assertNoOrphan(dir); + }, 60_000); + + it('vg scan with a code map does not skip a nested truncated lockfile', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vg-trunc-lock-')); + dirs.push(dir); + fs.mkdirSync(path.join(dir, 'packages', 'web'), { recursive: true }); + copyFixture(path.join(dir, 'packages', 'web'), 'package-lock.json'); + fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"root","version":"1.0.0"}\n'); + fs.writeFileSync(path.join(dir, 'app.ts'), 'export const n = 1;\n'); + const res = run(dir, 'scan', true); + expect(res.error).toBeUndefined(); + expect(res.signal).toBeNull(); + expect(res.status).toBe(1); + expect(res.stderr).toContain('truncated or invalid JSON'); + expect(res.stderr).toContain('package manager'); + expect(res.stderr).not.toContain('left-pad'); + expect(res.stderr).not.toContain('map build failed'); + expect(fs.existsSync(path.join(dir, '.vibgrate', 'graph.json'))).toBe(false); + assertNoOrphan(dir); + }, 60_000); + + it('vg scan fails a truncated yarn.lock and pnpm-lock.yaml from the fixture', () => { + for (const [name, kind] of [ + ['yarn.lock', 'yarn.lock'], + ['pnpm-lock.yaml', 'YAML'], + ] as const) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vg-trunc-lock-')); + dirs.push(dir); + copyFixture(dir, name); + fs.writeFileSync(path.join(dir, 'app.ts'), 'export const n = 1;\n'); + const res = run(dir, 'scan'); + expect(res.error, name).toBeUndefined(); + expect(res.signal, name).toBeNull(); + expect(res.status, name).toBe(1); + expect(res.stderr, name).toContain(name); + expect(res.stderr, name).toContain(`truncated or invalid ${kind}`); + expect(res.stderr, name).toContain('package manager'); + expect(res.stderr, name).not.toContain('left-pad'); + expect(res.stderr, name).not.toContain('sha512-abc'); + assertNoOrphan(dir); + } }, 60_000); it('vg build exits non-zero before parse workers stay up', () => { @@ -74,5 +154,7 @@ describe('truncated lockfile CLI', () => { expect(res.stderr).toContain('package-lock.json'); expect(res.stderr).toContain('truncated or invalid JSON'); expect(res.stderr).not.toContain(secret); + expect(fs.existsSync(path.join(dir, '.vibgrate', 'graph.json'))).toBe(false); + assertNoOrphan(dir); }, 60_000); }); diff --git a/src/reporting/commands/scan.ts b/src/reporting/commands/scan.ts index f23006a0..bde26095 100644 --- a/src/reporting/commands/scan.ts +++ b/src/reporting/commands/scan.ts @@ -52,6 +52,7 @@ import { emitIngestIdLine, emitDriftScoreLine } from '../utils/ingest-id-output. import { formatUploadHttpFailure, uploadScanArtifact } from '../utils/upload.js'; import { redactForDisplay } from '../../core-open/utils/redact.js'; import { buildGraph } from '../../engine/build.js'; +import { assertDiscoveredLockfiles, mergeExcludes } from '../../engine/discover.js'; import { writeArtifacts, resolveGraphPath } from '../../engine/artifacts.js'; import { readHaileSidecar } from '../../engine/haile/sidecar.js'; import { isUsableHaileSymbol } from '../../engine/haile/format.js'; @@ -477,6 +478,15 @@ export const scanCommand = new Command('scan') throw err; } + // Before preflight, project scanners, and the code-map parse pool. A + // truncated lockfile — including one nested in a subdirectory, and + // including `vg scan --no-graph` — must not become a drift report that + // looks empty, or a map failure that the command then skips. + assertDiscoveredLockfiles({ + root: rootDir, + exclude: mergeExcludes(rootDir, opts.exclude), + }); + // Fail closed before any scan work. A missing, unreadable, or unusable // --package-manifest must stop the command with a stable, actionable // error — not a stack trace, and not a scan that proceeds without it. diff --git a/test/__fixtures__/truncated-lockfile/package-lock.json b/test/__fixtures__/truncated-lockfile/package-lock.json new file mode 100644 index 00000000..b038d358 --- /dev/null +++ b/test/__fixtures__/truncated-lockfile/package-lock.json @@ -0,0 +1 @@ +{"name":"fixture","lockfileVersion":3,"packages":{"node_modules/left-pad":{"version":"1.0.0" diff --git a/test/__fixtures__/truncated-lockfile/pnpm-lock.yaml b/test/__fixtures__/truncated-lockfile/pnpm-lock.yaml new file mode 100644 index 00000000..5e41334a --- /dev/null +++ b/test/__fixtures__/truncated-lockfile/pnpm-lock.yaml @@ -0,0 +1,4 @@ +lockfileVersion: '9.0' +packages: + left-pad@1.0.0: + resolution: {integrity: sha512-abc diff --git a/test/__fixtures__/truncated-lockfile/yarn.lock b/test/__fixtures__/truncated-lockfile/yarn.lock new file mode 100644 index 00000000..7bbf7c3b --- /dev/null +++ b/test/__fixtures__/truncated-lockfile/yarn.lock @@ -0,0 +1,3 @@ +left-pad@^1.0.0: + version "1.0.0" +chalk@^5.0.0: diff --git a/vibgrate.config.json b/vibgrate.config.json index bb5a11ca..70d61abd 100644 --- a/vibgrate.config.json +++ b/vibgrate.config.json @@ -3,11 +3,13 @@ "$excludeNotes": { "packages/test-solutions/**": "Vendored real-world sample repos (Next.js SaaS, Turborepo, Clean Architecture, …) used as scan test targets and as the recorded CLI-simulator scenarios — their dependencies are intentionally frozen in time and are never installed or executed.", "examples/**": "Documentation fixtures (Go pseudo-version matching, GitHub Actions workflows). Versions are chosen to illustrate the scanner, including a deliberately end-of-life Go toolchain, and are not dependencies of this package. Tests read the files directly.", - "example/**": "Singular form of examples/, excluded for the same reason." + "example/**": "Singular form of examples/, excluded for the same reason.", + "test/__fixtures__/truncated-lockfile/**": "Deliberately truncated lockfiles used to assert vg scan and vg build fail. They are not dependencies of this package." }, "exclude": [ "packages/test-solutions/**", "examples/**", - "example/**" + "example/**", + "test/__fixtures__/truncated-lockfile/**" ] } From 15761cd38a94ec38a01f0dd985112f1ee1914262 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 10 Oct 2026 13:54:31 +0000 Subject: [PATCH 2/2] fix: stop scan and build on invalid lockfile syntax (#310) A truncated or syntactically invalid lockfile fails before a dependency graph is built and before source parse workers start. The error names each bad file in path order, omits the file contents, and exits non-zero. An empty go.sum or go.work.sum remains valid. Signed-off-by: Cursor Agent Co-authored-by: vibgrate-team --- CHANGELOG.md | 14 +- DOCS.md | 6 +- src/cli.ts | 3 +- src/commands/build.ts | 2 + src/core-open/run-core-scan.ts | 6 +- src/core-open/utils/lockfile-parse.ts | 4 +- src/engine/build.ts | 4 + src/engine/discover.ts | 141 ++------ src/engine/lockfile-guard.test.ts | 235 ++++++++++++++ src/engine/lockfile-guard.ts | 307 ++++++++++++++++++ src/engine/lockfile.test.ts | 49 +-- src/engine/truncated-lockfile.cli.test.ts | 92 +----- src/reporting/commands/scan.ts | 18 +- .../truncated-lockfile/package-lock.json | 1 - .../truncated-lockfile/pnpm-lock.yaml | 4 - .../__fixtures__/truncated-lockfile/yarn.lock | 3 - vibgrate.config.json | 6 +- 17 files changed, 614 insertions(+), 281 deletions(-) create mode 100644 src/engine/lockfile-guard.test.ts create mode 100644 src/engine/lockfile-guard.ts delete mode 100644 test/__fixtures__/truncated-lockfile/package-lock.json delete mode 100644 test/__fixtures__/truncated-lockfile/pnpm-lock.yaml delete mode 100644 test/__fixtures__/truncated-lockfile/yarn.lock diff --git a/CHANGELOG.md b/CHANGELOG.md index 8189952e..e98428b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,10 +12,6 @@ backward compatible. ## [Unreleased] -### Fixed - -- **A truncated lockfile fails `vg scan` and `vg build`.** A `package-lock.json`, `yarn.lock`, `pnpm-lock.yaml`, or peer lockfile cut off mid-file exits non-zero. The error names that file and the format, and tells you to regenerate it with your package manager. It does not include the file's contents. `vg scan` checks before scoring drift or starting parse workers, including `--no-graph` and a lockfile nested in a subdirectory. A nested lockfile used to be reported as "no lockfile", or the code map was skipped, and the command exited 0. When several lockfiles are invalid, the error names the path that sorts first. - ### Changed - **Machine-readable output drops absolute home-directory paths (#282).** @@ -46,6 +42,16 @@ backward compatible. ### Fixed +- **A truncated or syntactically invalid lockfile stops `vg scan` and `vg build` (#310).** + The check runs before a dependency graph is built and before source files are + parsed, so parse workers are not started and a partial map is not written. + The error names each bad file, in path order, including lockfiles the walk + otherwise skips (for example `bun.lock`, `mix.lock`, and `Cargo.lock`). It + tells you to regenerate the file with your package manager and does not + include the file's contents. The process exits non-zero. A valid lockfile is + scanned and built as before. An empty `go.sum` or `go.work.sum` is valid: + the module has no dependencies. + - **SBOM component order no longer follows scan or filesystem order.** `vg sbom export` and CycloneDX/SPDX graph export sort a component by its Package URL when one is written, otherwise by package name, then by version. diff --git a/DOCS.md b/DOCS.md index 9c670045..0cd4b08c 100644 --- a/DOCS.md +++ b/DOCS.md @@ -2136,6 +2136,8 @@ Switches (flags that take no value, such as `--vulns`, `--offline` or `--no-grap By default, the scan writes `.vibgrate/scan_result.json`. Use `--no-local-artifacts` or `--max-privacy` to suppress local JSON artifact files. +A truncated or syntactically invalid lockfile stops the scan before a dependency graph is built. The error names the file (for example `package-lock.json`, `pnpm-lock.yaml`, or `yarn.lock`) and tells you to regenerate it with your package manager. The message does not include the lockfile, and the process exits non-zero. A valid lockfile is scanned as before. + `vg scan` does not follow symlinks while it indexes the tree. A skipped link is named once on stderr. See [Symlinks](#symlinks). For offline drift scoring, pass `--package-manifest ` with a downloaded manifest bundle such as `https://github.com/vibgrate/manifests/latest-packages.zip`. The manifest shape, the fail-closed errors, and what offline mode skips are in [Offline scan with a package-version manifest](#offline-scan-with-a-package-version-manifest). @@ -2152,7 +2154,7 @@ pnpm-lock.yaml: unknown optional lockfile field "futureOptional"; continuing wit The path is relative to the directory you ran the command from, when the lockfile is under that directory. Otherwise the warning uses the file name only. The warning names the field. It does not print the field's value, an environment value, or an authorization header. A field name that is not a plain identifier is reported as `(name omitted)`. -A lockfile that is truncated, empty, or missing the structure its format requires still fails the command. The error names the file and the format, tells you to regenerate the file with your package manager, and the process exits non-zero. The error does not include the file's contents. `vg scan` performs this check before it scores drift or starts parse workers, including with `--no-graph`, and including a lockfile in a subdirectory. `vg build` checks during discovery, before the parse pool starts. When more than one lockfile is invalid, the error names the path that sorts first. +A lockfile that is truncated or not valid syntax still fails the command, including the other lockfiles the walk skips (for example `bun.lock`, `mix.lock`, `Cargo.lock`). The error names each bad file, in path order, tells you to regenerate it with your package manager, and the process exits non-zero. The error does not include the file's contents. An empty `go.sum` or `go.work.sum` is valid: the module has no dependencies. `vg scan` and `vg sbom export` follow this for npm, pnpm, yarn, poetry, uv, pdm, Pipfile, Cargo, Composer, NuGet, Swift package pins, and pub. A Gradle lock line, a Gemfile spec, and a `go.sum` line do not have optional field names. A line that does not match that format is invalid, and the command fails. @@ -2708,6 +2710,8 @@ vg build [paths...] Maps source code into a graph artifact, enabling all downstream queries (`vg show`, `vg ask`, `vg impact`, etc.). +A truncated or syntactically invalid lockfile stops the build before source files are parsed, so parse workers are not left running and a partial map is not written. The error names the file and tells you to regenerate it with your package manager. A valid lockfile builds as before. + | Flag | Default | Description | |------|---------|-------------| | `[paths...]` | `.` | Folders or files to map | diff --git a/src/cli.ts b/src/cli.ts index 23a45c54..98bc520f 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -44,6 +44,7 @@ import { registerHcs } from './commands/hcs.js'; import { registerReview } from './commands/review.js'; import { ConfigFileError } from './core-open/config.js'; import { LockfileParseError } from './core-open/utils/lockfile-parse.js'; +import { LockfileSyntaxError } from './engine/lockfile-guard.js'; import { UnsafeRootError } from './core-open/utils/root-safety.js'; import { CliError, ExitCode, usageError } from './util/exit.js'; import { c, info, disableColor, exitAfterFlush } from './util/output.js'; @@ -438,7 +439,7 @@ function handleError(err: unknown): never { /* stdout closed */ } }; - if (err instanceof CliError || err instanceof LockfileParseError || err instanceof ConfigFileError || err instanceof UnsafeRootError) { + if (err instanceof CliError || err instanceof LockfileParseError || err instanceof LockfileSyntaxError || err instanceof ConfigFileError || err instanceof UnsafeRootError) { const code = err instanceof CliError ? err.code : ExitCode.ERROR; emitHostError(err.message); info(c.red(`error: ${err.message}`)); diff --git a/src/commands/build.ts b/src/commands/build.ts index 981cfa95..7e025de9 100644 --- a/src/commands/build.ts +++ b/src/commands/build.ts @@ -21,6 +21,7 @@ import { serializeGraph } from '../engine/serialize.js'; import { renderReport } from '../engine/report.js'; import { renderHtml } from '../engine/html.js'; import { UsageError, mergeExcludes } from '../engine/discover.js'; +import { LockfileSyntaxError } from '../engine/lockfile-guard.js'; import { ResourceLimitError } from '../engine/limits.js'; import { UnsafeRootError } from '../core-open/utils/root-safety.js'; import { CliError, ExitCode, usageError } from '../util/exit.js'; @@ -152,6 +153,7 @@ export async function runBuild( if (err instanceof UsageError) throw usageError(err.message); if (err instanceof ResourceLimitError) throw new CliError(err.message, ExitCode.ERROR); if (err instanceof UnsafeRootError) throw new CliError(err.message, ExitCode.ERROR); + if (err instanceof LockfileSyntaxError) throw new CliError(err.message, ExitCode.ERROR); throw err; } bar?.done(); diff --git a/src/core-open/run-core-scan.ts b/src/core-open/run-core-scan.ts index 4b29df1e..a6d6ebc5 100644 --- a/src/core-open/run-core-scan.ts +++ b/src/core-open/run-core-scan.ts @@ -37,7 +37,6 @@ import { formatSarif } from './formatters/sarif.js'; import { formatMarkdown } from './formatters/markdown.js'; import { loadConfig, appendExcludePatterns } from './config.js'; import { pathExists, readJsonFile, writeJsonFile, writeTextFile, ensureDir, FileCache, quickTreeCount } from './utils/fs.js'; -import { LockfileParseError } from './utils/lockfile-parse.js'; import { portableValue } from './utils/portable-path.js'; import { assertSafeWalkRoot } from './utils/root-safety.js'; import { detectVcs } from './utils/vcs.js'; @@ -765,10 +764,7 @@ export async function runCoreScan( { projects: allProjects, solutions, extended }, ); progress.completeStep('map', detail || 'done'); - } catch (err) { - // A truncated lockfile is not a map failure. Swallowing it printed - // "skipped (map build failed)" and let the scan exit 0. - if (err instanceof LockfileParseError) throw err; + } catch { progress.completeStep('map', 'skipped (map build failed)'); } } diff --git a/src/core-open/utils/lockfile-parse.ts b/src/core-open/utils/lockfile-parse.ts index 35a91846..42f7b925 100644 --- a/src/core-open/utils/lockfile-parse.ts +++ b/src/core-open/utils/lockfile-parse.ts @@ -42,6 +42,7 @@ const KIND_BY_BASENAME: Readonly> = { 'cargo.lock': 'TOML', 'yarn.lock': 'yarn.lock', 'go.sum': 'go.sum', + 'go.work.sum': 'go.sum', 'gradle.lockfile': 'gradle.lockfile', 'gemfile.lock': 'Gemfile.lock', }; @@ -191,7 +192,8 @@ function assertYarn(filePath: string, text: string): void { } function assertGoSum(filePath: string, text: string): void { - if (!text.trim()) throw new LockfileParseError(filePath, 'go.sum'); + // An empty sum is a module with no dependencies, not a truncated file. + if (!text.trim()) return; const lineRe = /^\S+\s+v\S+\s+h1:\S+$/; for (const raw of text.split('\n')) { const line = raw.trim(); diff --git a/src/engine/build.ts b/src/engine/build.ts index 9252c40f..3eb767c9 100644 --- a/src/engine/build.ts +++ b/src/engine/build.ts @@ -1,6 +1,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { discover, mergeExcludes, type DiscoveredFile } from './discover.js'; +import { assertLockfilesValid } from './lockfile-guard.js'; import { parseFiles } from './pool.js'; import { resolve } from './resolve.js'; import { inheritDuties } from './duties-inherit.js'; @@ -159,6 +160,9 @@ export async function buildGraph(options: BuildOptions): Promise { const timer = new StageTimer(); timer.start('total'); const root = path.resolve(options.root); + // Before discovery and source parse workers. A bad lockfile must not start + // a worker pool or leave a partial graph behind. + assertLockfilesValid(root); const exclude = mergeExcludes(root, options.exclude); // Resolved before discovery so the walk itself stops at the corpus cap // instead of reading a filesystem root or an enormous unpack first. diff --git a/src/engine/discover.ts b/src/engine/discover.ts index d070482d..c5113b9a 100644 --- a/src/engine/discover.ts +++ b/src/engine/discover.ts @@ -4,7 +4,7 @@ import ignore, { type Ignore } from 'ignore'; import { langForExtension, langById, type LanguageDef } from './languages.js'; import { requireDataConfig } from '../core-open/config.js'; import { dropBlankPatterns, gitignoreWithoutBlankLines } from '../core-open/utils/glob.js'; -import { assertLockfileFile, lockfileKind, LockfileParseError } from '../core-open/utils/lockfile-parse.js'; +import { assertLockfileFile, lockfileKind } from '../core-open/utils/lockfile-parse.js'; import { assertSafeWalkRoot, createWalkBudget, noteWalkEntry } from '../core-open/utils/root-safety.js'; import { emitSkippedSymlinkNotice } from '../core-open/utils/skipped-symlinks.js'; @@ -226,86 +226,49 @@ export function loadRootIgnore(root: string, exclude: string[]): Ignore { return ig; } -interface ProjectWalk { - root: string; - exclude?: string[]; - paths?: string[]; - maxEntries?: number; - /** Source files to keep. Omit when the caller only validates lockfiles. */ - found?: Map; - allowLang?: (lang: LanguageDef) => boolean; - /** - * Symlinks this walk skipped. Omit to stay quiet — `vg scan` validates - * lockfiles before the file walk that prints the one notice. - */ - skippedSymlinks?: string[]; -} +export function discover(options: DiscoverOptions): DiscoveredFile[] { + const root = path.resolve(options.root); + const onlyLangs = (options.only ?? []).filter(Boolean); + const allowLang = (lang: LanguageDef): boolean => + onlyLangs.length === 0 || onlyLangs.includes(lang.id); -/** - * The invalid lockfile whose path sorts first. Directory entries are sorted - * before the walk, and this pick does not depend on which of two bad files - * `readdir` returned first. - */ -function firstLockfileError(errors: readonly LockfileParseError[]): LockfileParseError | undefined { - if (errors.length === 0) return undefined; - return [...errors].sort((a, b) => (a.filePath < b.filePath ? -1 : a.filePath > b.filePath ? 1 : 0))[0]; -} + // Validate `--only` ids early so a typo is a usage error, not silent emptiness. + for (const id of onlyLangs) { + if (!langById(id)) { + throw new UsageError(`unknown language "${id}" for --only`); + } + } -/** - * Walk a project the way {@link discover} does. Lockfiles are syntax-checked - * here, on this thread, before any source parse worker exists. A truncated - * file is recorded and the walk continues so the reported path is stable. - */ -function walkProject(options: ProjectWalk): LockfileParseError[] { - const root = path.resolve(options.root); const rootIg = loadRootIgnore(root, options.exclude ?? []); - const allowLang = options.allowLang ?? ((): boolean => true); - const found = options.found; - const skippedSymlinks = options.skippedSymlinks; - const lockfileErrors: LockfileParseError[] = []; - const budget = createWalkBudget(root, options.maxEntries); + // Scope roots: explicit paths, or the whole repo. const scopeAbs = (options.paths && options.paths.length ? options.paths.map((p) => path.resolve(root, p)) : [root] ).filter((p) => fs.existsSync(p)); + const found = new Map(); + const budget = createWalkBudget(root, options.maxEntries); + // Symlinks are not followed. Dirent.isDirectory() / isFile() are false for a + // link, so a directory link to its parent cannot re-enter this walk. The + // notice lists the ones this walk skipped; ignored links stay quiet. + const skippedSymlinks: string[] = []; + const considerFile = (abs: string): void => { const rel = toPosix(path.relative(root, abs)); if (rel.startsWith('..')) return; // outside root if (rel === '' || rootIg.ignores(rel)) return; if (SKIP_FILES.has(path.basename(abs).toLowerCase())) { - // Lockfiles are not source. A truncated one must fail the command - // rather than be skipped into an empty dependency graph. - if (lockfileKind(path.basename(abs))) { - try { - assertLockfileFile(abs); - } catch (err) { - if (err instanceof LockfileParseError) { - lockfileErrors.push(err); - return; - } - throw err; - } - } + // Lockfiles are not source, but a truncated one must fail the build + // here — before the parse pool starts — rather than being skipped. + if (lockfileKind(path.basename(abs))) assertLockfileFile(abs); return; } - if (!found) return; const lang = langForExtension(path.extname(abs)); if (!lang || !allowLang(lang)) return; found.set(rel, { rel, abs, lang }); }; - const note = (): void => { - const over = noteWalkEntry(budget); - if (!over) return; - // A lockfile we already know is bad is the error to show. The budget - // still stops a walk that has not seen one. - const lockfileError = firstLockfileError(lockfileErrors); - if (lockfileError) throw lockfileError; - throw over; - }; - const walk = (dir: string): void => { let entries: fs.Dirent[]; try { @@ -313,24 +276,24 @@ function walkProject(options: ProjectWalk): LockfileParseError[] { } catch { return; // unreadable dir — skip rather than crash the build } - // Sorted so two invalid lockfiles always surface the same path. - entries.sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0)); for (const entry of entries) { const abs = path.join(dir, entry.name); const rel = toPosix(path.relative(root, abs)); if (entry.isSymbolicLink()) { if (rel && (rootIg.ignores(rel) || rootIg.ignores(`${rel}/`))) continue; - if (rel && skippedSymlinks) skippedSymlinks.push(rel); + if (rel) skippedSymlinks.push(rel); continue; } if (entry.isDirectory()) { if (isSkippedDirName(entry.name)) continue; if (rel && rootIg.ignores(`${rel}/`)) continue; - note(); + const over = noteWalkEntry(budget); + if (over) throw over; walk(abs); } else if (entry.isFile()) { if (rel && rootIg.ignores(rel)) continue; - note(); + const over = noteWalkEntry(budget); + if (over) throw over; considerFile(abs); } } @@ -344,58 +307,10 @@ function walkProject(options: ProjectWalk): LockfileParseError[] { } else if (stat.isFile()) considerFile(scope); } - return lockfileErrors; -} - -export function discover(options: DiscoverOptions): DiscoveredFile[] { - const onlyLangs = (options.only ?? []).filter(Boolean); - const allowLang = (lang: LanguageDef): boolean => - onlyLangs.length === 0 || onlyLangs.includes(lang.id); - - // Validate `--only` ids early so a typo is a usage error, not silent emptiness. - for (const id of onlyLangs) { - if (!langById(id)) { - throw new UsageError(`unknown language "${id}" for --only`); - } - } - - const found = new Map(); - // Symlinks are not followed. Dirent.isDirectory() / isFile() are false for a - // link, so a directory link to its parent cannot re-enter this walk. The - // notice lists the ones this walk skipped; ignored links stay quiet. - const skippedSymlinks: string[] = []; - const lockfileError = firstLockfileError(walkProject({ - root: options.root, - exclude: options.exclude, - paths: options.paths, - maxEntries: options.maxEntries, - found, - allowLang, - skippedSymlinks, - })); - // Before the symlink notice, and before the caller starts parse workers. - if (lockfileError) throw lockfileError; emitSkippedSymlinkNotice(skippedSymlinks); return [...found.values()].sort((a, b) => (a.rel < b.rel ? -1 : a.rel > b.rel ? 1 : 0)); } -/** - * Fail when any in-scope lockfile is truncated or invalid. Same skip and - * ignore rules as {@link discover}. Does not print the symlink notice and - * does not start parse workers. `vg scan` calls this before drift scoring. - */ -export function assertDiscoveredLockfiles( - options: Pick, -): void { - const lockfileError = firstLockfileError(walkProject({ - root: options.root, - exclude: options.exclude, - paths: options.paths, - maxEntries: options.maxEntries, - })); - if (lockfileError) throw lockfileError; -} - /** A usage error that maps to exit code 5 at the CLI boundary. */ export class UsageError extends Error { readonly isUsageError = true; diff --git a/src/engine/lockfile-guard.test.ts b/src/engine/lockfile-guard.test.ts new file mode 100644 index 00000000..610c64a3 --- /dev/null +++ b/src/engine/lockfile-guard.test.ts @@ -0,0 +1,235 @@ +import { execFileSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { buildGraph } from './build.js'; +import { serializeGraph } from './serialize.js'; +import { assertLockfilesValid, LockfileSyntaxError } from './lockfile-guard.js'; + +const parseState = vi.hoisted(() => ({ n: 0 })); + +vi.mock('./pool.js', async () => { + const actual = await vi.importActual('./pool.js'); + return { + ...actual, + async parseFiles(...args: Parameters): Promise>> { + parseState.n++; + return actual.parseFiles(...args); + }, + }; +}); + +const REPO = fileURLToPath(new URL('../..', import.meta.url)); +const CLI = fileURLToPath(new URL('../cli.ts', import.meta.url)); +const PIN = '2020-01-01T00:00:00.000Z'; +const SENTINEL = 'SENTINEL_LOCKFILE_BODY_9f3a2c'; + +const dirs: string[] = []; +afterEach(() => { + while (dirs.length) fs.rmSync(dirs.pop()!, { recursive: true, force: true }); +}); + +function project(files: Record): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vg-lockguard-')); + dirs.push(dir); + for (const [rel, content] of Object.entries(files)) { + const abs = path.join(dir, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content); + } + return dir; +} + +const VALID_PACKAGE_LOCK = JSON.stringify({ + name: 'ok', + lockfileVersion: 3, + requires: true, + packages: { '': { name: 'ok', version: '1.0.0' } }, +}); + +const TRUNCATED_PACKAGE_LOCK = `{"name":"${SENTINEL}","lockfileVersion":3,"packages":{`; + +function runCli(args: string[]): { code: number; stdout: string; stderr: string } { + try { + const stdout = execFileSync(process.execPath, ['--import', 'tsx', CLI, ...args], { + cwd: REPO, + encoding: 'utf8', + env: { + ...process.env, + NO_COLOR: '1', + FORCE_COLOR: '0', + VIBGRATE_NO_KERNEL: '1', + VIBGRATE_DSN: '', + }, + timeout: 45_000, + stdio: ['ignore', 'pipe', 'pipe'], + }); + return { code: 0, stdout, stderr: '' }; + } catch (err) { + const e = err as { status?: number | null; stdout?: string | Buffer; stderr?: string | Buffer }; + const text = (value: string | Buffer | undefined): string => + typeof value === 'string' ? value : value ? value.toString('utf8') : ''; + return { code: e.status ?? 1, stdout: text(e.stdout), stderr: text(e.stderr) }; + } +} + +describe('assertLockfilesValid', () => { + it('accepts this package\'s own lockfiles', () => { + expect(() => assertLockfilesValid(REPO)).not.toThrow(); + }); + + it('names a truncated package-lock.json and omits its contents', () => { + const root = project({ 'package-lock.json': TRUNCATED_PACKAGE_LOCK }); + expect(() => assertLockfilesValid(root)).toThrow(LockfileSyntaxError); + try { + assertLockfilesValid(root); + } catch (err) { + expect(err).toBeInstanceOf(LockfileSyntaxError); + const message = (err as Error).message; + expect(message).toBe( + 'invalid lockfile package-lock.json: truncated or not valid syntax. Regenerate it with your package manager.', + ); + expect(message).not.toContain(SENTINEL); + expect(message).not.toContain('lockfileVersion'); + } + }); + + it('rejects a truncated pnpm-lock.yaml, yarn.lock, and Cargo.lock', () => { + const pnpm = project({ + 'pnpm-lock.yaml': `lockfileVersion: '9.0'\npackages:\n ${SENTINEL}@1.0.0:\n resolution: {integrity: sha512-dead\n`, + }); + expect(() => assertLockfilesValid(pnpm)).toThrow(/pnpm-lock\.yaml/); + + const yarn = project({ + 'yarn.lock': `# yarn lockfile v1\n${SENTINEL}@^1.0.0:\n version "1.0.0\n`, + }); + expect(() => assertLockfilesValid(yarn)).toThrow(/yarn\.lock/); + + const cargo = project({ + 'Cargo.lock': `[[package]]\nname = "${SENTINEL}"\nversion = "1.0.0\n`, + }); + expect(() => assertLockfilesValid(cargo)).toThrow(/Cargo\.lock/); + }); + + it('rejects garbage that is not a lockfile document', () => { + const root = project({ 'pnpm-lock.yaml': ` not yaml at all ${SENTINEL}` }); + let message = ''; + try { + assertLockfilesValid(root); + } catch (err) { + message = (err as Error).message; + } + expect(message).toContain('pnpm-lock.yaml'); + expect(message).not.toContain(SENTINEL); + }); + + it('lists every bad lockfile in sorted path order', () => { + const root = project({ + 'b/pnpm-lock.yaml': ' not yaml at all', + 'a/package-lock.json': TRUNCATED_PACKAGE_LOCK, + }); + expect(() => assertLockfilesValid(root)).toThrow( + 'invalid lockfile a/package-lock.json, b/pnpm-lock.yaml: truncated or not valid syntax. Regenerate it with your package manager.', + ); + }); + + it('accepts valid lockfiles and an empty go.sum', () => { + const root = project({ + 'package-lock.json': VALID_PACKAGE_LOCK, + 'pnpm-lock.yaml': "lockfileVersion: '9.0'\n", + 'yarn.lock': '# yarn lockfile v1\nleft-pad@^1.3.0:\n version "1.3.0"\n', + 'Cargo.lock': 'version = 3\n\n[[package]]\nname = "memchr"\nversion = "2.7.4"\n', + 'go.sum': '', + 'gradle.lockfile': '# generated\nempty=\n', + 'Gemfile.lock': 'GEM\n remote: https://rubygems.org/\n specs:\n rake (13.0.6)\n\nPLATFORMS\n ruby\n\nDEPENDENCIES\n rake\n\nBUNDLED WITH\n 2.4.0\n', + 'bun.lockb': Buffer.from([0, 1, 2, 3]), + }); + expect(() => assertLockfilesValid(root)).not.toThrow(); + }); + + it('ignores a truncated lockfile that gitignore excludes', () => { + const root = project({ + '.gitignore': 'scratch/\n', + 'scratch/package-lock.json': TRUNCATED_PACKAGE_LOCK, + 'package-lock.json': VALID_PACKAGE_LOCK, + }); + expect(() => assertLockfilesValid(root)).not.toThrow(); + }); +}); + +describe('vg scan / vg build', () => { + it('stops buildGraph before source parse workers run', async () => { + const root = project({ + 'package-lock.json': TRUNCATED_PACKAGE_LOCK, + 'src/a.ts': 'export const a = 1;\n', + }); + const before = parseState.n; + await expect(buildGraph({ root, generatedAt: PIN, inline: true, noCache: true, noIndex: true })).rejects.toBeInstanceOf( + LockfileSyntaxError, + ); + expect(parseState.n).toBe(before); + expect(fs.existsSync(path.join(root, '.vibgrate', 'graph.json'))).toBe(false); + }); + + it('builds the same graph when a valid lockfile is present', async () => { + const files = { + 'package.json': '{"name":"ok","version":"1.0.0"}\n', + 'src/a.ts': 'export const a = 1;\n', + }; + const plain = project(files); + const locked = project({ ...files, 'package-lock.json': VALID_PACKAGE_LOCK }); + const opts = { generatedAt: PIN, inline: true, noCache: true, noIndex: true } as const; + const before = parseState.n; + const a = serializeGraph((await buildGraph({ root: plain, ...opts })).graph); + const b = serializeGraph((await buildGraph({ root: locked, ...opts })).graph); + expect(parseState.n).toBeGreaterThan(before); + expect(b).toBe(a); + }); + + it('exits non-zero from vg scan and vg build without echoing the lockfile', () => { + const root = project({ + 'package.json': '{"name":"broken","version":"1.0.0"}\n', + 'package-lock.json': TRUNCATED_PACKAGE_LOCK, + 'src/a.ts': 'export const a = 1;\n', + }); + const expected = + 'invalid lockfile package-lock.json: truncated or not valid syntax. Regenerate it with your package manager.'; + + const scan = runCli(['--offline', 'scan', root, '--offline', '--quiet', '--no-daemon']); + expect(scan.code).not.toBe(0); + expect(scan.stderr).toContain(expected); + expect(`${scan.stdout}\n${scan.stderr}`).not.toContain(SENTINEL); + expect(fs.existsSync(path.join(root, '.vibgrate', 'scan_result.json'))).toBe(false); + + const build = runCli([ + '--offline', + 'build', + '-C', + root, + '--offline', + '--quiet', + '--no-daemon', + '--no-warm', + '--no-html', + '--no-report', + ]); + expect(build.code).not.toBe(0); + expect(build.stderr).toContain(expected); + expect(`${build.stdout}\n${build.stderr}`).not.toContain(SENTINEL); + expect(fs.existsSync(path.join(root, '.vibgrate', 'graph.json'))).toBe(false); + }); + + it('still scans a project with a valid lockfile', () => { + const root = project({ + 'package.json': '{"name":"ok","version":"1.0.0"}\n', + 'package-lock.json': VALID_PACKAGE_LOCK, + 'index.js': 'module.exports = 1;\n', + }); + const scan = runCli(['--offline', 'scan', root, '--offline', '--no-graph', '--quiet', '--no-daemon']); + expect(scan.code).toBe(0); + expect(`${scan.stdout}\n${scan.stderr}`).not.toContain('truncated or not valid syntax'); + expect(fs.existsSync(path.join(root, '.vibgrate', 'scan_result.json'))).toBe(true); + }); +}); diff --git a/src/engine/lockfile-guard.ts b/src/engine/lockfile-guard.ts new file mode 100644 index 00000000..02f9efe0 --- /dev/null +++ b/src/engine/lockfile-guard.ts @@ -0,0 +1,307 @@ +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import ignore from 'ignore'; +import { parse as parseToml } from 'smol-toml'; +import { parse as parseYaml } from 'yaml'; +import { assertSafeWalkRoot } from '../core-open/utils/root-safety.js'; +import { isSkippedDirName, SKIP_FILES } from './discover.js'; + +/** + * Stop `vg scan` / `vg build` when a lockfile is truncated or not valid + * syntax. Readers that swallow a parse error keep going and can emit a + * partial dependency graph (or look successful with nothing resolved). This + * check runs before source parse workers start, so a bad file never leaves + * workers running and never reaches graph construction. + * + * The message names the lockfile and says to regenerate it. Parser errors are + * discarded: they quote the source, and a lockfile must not be echoed back. + */ + +const MESSAGE_SUFFIX = 'truncated or not valid syntax. Regenerate it with your package manager.'; + +/** Generated JavaScript bundles, not lock documents. */ +const SKIP_VALIDATION = new Set(['.pnp.cjs', '.pnp.loader.mjs']); + +const JSON_LOCKS = new Set([ + 'package-lock.json', + 'npm-shrinkwrap.json', + 'composer.lock', + 'packages.lock.json', + 'package.resolved', + 'pipfile.lock', + 'bun.lock', + 'deno.lock', + '.pnp.data.json', + 'renv.lock', + 'flake.lock', + 'conan.lock', + 'module.bazel.lock', +]); + +const YAML_LOCKS = new Set([ + 'pnpm-lock.yaml', + 'pubspec.lock', + 'conda-lock.yml', + 'chart.lock', + 'stack.yaml.lock', + 'glide.lock', +]); + +const TOML_LOCKS = new Set([ + 'poetry.lock', + 'uv.lock', + 'pdm.lock', + 'cargo.lock', + 'pylock.toml', + 'gopkg.lock', + 'manifest.toml', +]); + +const BINARY_LOCKS = new Set(['bun.lockb']); + +export class LockfileSyntaxError extends Error { + readonly paths: readonly string[]; + constructor(paths: readonly string[]) { + super(`invalid lockfile ${paths.join(', ')}: ${MESSAGE_SUFFIX}`); + this.name = 'LockfileSyntaxError'; + this.paths = paths; + } +} + +/** Throw {@link LockfileSyntaxError} when any lockfile under `root` is unusable. */ +export function assertLockfilesValid(root: string): void { + const abs = path.resolve(root); + assertSafeWalkRoot(abs); + const bad = findLockfiles(abs).filter((rel) => !lockfileFileValid(abs, rel)); + if (bad.length) throw new LockfileSyntaxError(bad); +} + +function findLockfiles(root: string): string[] { + const ig = ignore(); + const gitignorePath = path.join(root, '.gitignore'); + if (fs.existsSync(gitignorePath)) { + try { + ig.add(fs.readFileSync(gitignorePath, 'utf8')); + } catch { + // Unreadable ignore file: still walk, and skip the usual dependency dirs. + } + } + + const found: string[] = []; + const walk = (dir: string): void => { + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return; + } + const sorted = [...entries].sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0)); + for (const entry of sorted) { + if (entry.name === '.' || entry.name === '..') continue; + const abs = path.join(dir, entry.name); + const rel = toPosix(path.relative(root, abs)); + if (!rel || rel.startsWith('..')) continue; + + let isDir = entry.isDirectory(); + let isFile = entry.isFile(); + if (entry.isSymbolicLink()) { + try { + const st = fs.statSync(abs); + isDir = st.isDirectory(); + isFile = st.isFile(); + } catch { + continue; + } + // Directory links can cycle. A linked lockfile is still read. + if (isDir) continue; + } + if (isDir) { + if (isSkippedDirName(entry.name)) continue; + if (ig.ignores(`${rel}/`)) continue; + walk(abs); + continue; + } + if (!isFile) continue; + if (ig.ignores(rel)) continue; + const base = entry.name.toLowerCase(); + if (!SKIP_FILES.has(base) || SKIP_VALIDATION.has(base)) continue; + found.push(rel); + } + }; + + walk(root); + found.sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); + return found; +} + +function lockfileFileValid(root: string, rel: string): boolean { + const abs = path.join(root, rel); + const base = path.posix.basename(rel).toLowerCase(); + let buf: Buffer; + try { + buf = fs.readFileSync(abs); + } catch { + // Unreadable: nothing was parsed, so there is no partial graph to stop. + return true; + } + if (BINARY_LOCKS.has(base)) return buf.length > 0; + return !lockfileTextInvalid(base, stripBom(buf.toString('utf8'))); +} + +function lockfileTextInvalid(basename: string, text: string): boolean { + if (JSON_LOCKS.has(basename)) return jsonLockInvalid(text); + if (YAML_LOCKS.has(basename)) return yamlLockInvalid(text); + if (TOML_LOCKS.has(basename)) return tomlLockInvalid(text); + if (basename === 'yarn.lock') return yarnInvalid(text); + if (basename === 'go.sum' || basename === 'go.work.sum') return goSumInvalid(text); + if (basename === 'gradle.lockfile') return gradleInvalid(text); + if (basename === 'gemfile.lock') return gemfileInvalid(text); + return structuralInvalid(text); +} + +function jsonLockInvalid(text: string): boolean { + if (!text.trim()) return true; + try { + const doc: unknown = JSON.parse(text); + return !isRecord(doc); + } catch { + return true; + } +} + +function yamlLockInvalid(text: string): boolean { + if (!text.trim()) return true; + try { + return !isRecord(parseYaml(text)); + } catch { + return true; + } +} + +function tomlLockInvalid(text: string): boolean { + if (!text.trim()) return true; + try { + return !isRecord(parseToml(text)); + } catch { + return true; + } +} + +/** Classic and Berry yarn.lock. A block that never reaches `version` is cut off. */ +function yarnInvalid(text: string): boolean { + if (!text.trim()) return true; + if (unbalancedDoubleQuotes(text)) return true; + let open = false; + let sawVersion = false; + for (const raw of text.split('\n')) { + const line = raw.replace(/\r$/, ''); + if (!line.trim() || /^\s*#/.test(line)) continue; + if (!/^\s/.test(line)) { + if (open && !sawVersion) return true; + if (!line.trimEnd().endsWith(':')) return true; + open = true; + sawVersion = false; + continue; + } + if (!open) return true; + if (/^\s+version:?\s+\S/.test(line)) sawVersion = true; + } + return open && !sawVersion; +} + +/** `go.sum` / `go.work.sum`. Empty is a module with no dependencies. */ +function goSumInvalid(text: string): boolean { + if (!text.trim()) return false; + for (const raw of text.split('\n')) { + const line = raw.replace(/\r$/, '').trim(); + if (!line) continue; + if (!/^\S+\s+\S+\s+h1:\S+$/.test(line)) return true; + } + return false; +} + +function gradleInvalid(text: string): boolean { + if (!text.trim()) return true; + let sawEntry = false; + for (const raw of text.split('\n')) { + const line = raw.replace(/\r$/, '').trim(); + if (!line || line.startsWith('#')) continue; + if (line.startsWith('empty=')) { + sawEntry = true; + continue; + } + if (!/^[^:=\s#][^:=\s]*:[^:=\s]+:[^=\s]+=\S+/.test(line)) return true; + sawEntry = true; + } + return !sawEntry; +} + +function gemfileInvalid(text: string): boolean { + if (!text.trim()) return true; + if (unbalancedDelimiters(text)) return true; + for (const raw of text.split('\n')) { + const line = raw.replace(/\r$/, ''); + if (/^ {4}\S/.test(line) && !/^ {4}[A-Za-z0-9._-]+ \([^()]+\)$/.test(line.trimEnd())) return true; + } + return false; +} + +function structuralInvalid(text: string): boolean { + if (!text.trim()) return true; + return unbalancedDelimiters(text); +} + +function unbalancedDoubleQuotes(text: string): boolean { + let open = false; + for (let i = 0; i < text.length; i++) { + const ch = text[i]; + if (ch === '\\') { + i++; + continue; + } + if (ch === '"') open = !open; + } + return open; +} + +function unbalancedDelimiters(text: string): boolean { + let brace = 0; + let bracket = 0; + let paren = 0; + let quote = false; + for (let i = 0; i < text.length; i++) { + const ch = text[i]; + if (quote) { + if (ch === '\\') { + i++; + continue; + } + if (ch === '"') quote = false; + continue; + } + if (ch === '"') { + quote = true; + continue; + } + if (ch === '{') brace++; + else if (ch === '}') brace--; + else if (ch === '[') bracket++; + else if (ch === ']') bracket--; + else if (ch === '(') paren++; + else if (ch === ')') paren--; + if (brace < 0 || bracket < 0 || paren < 0) return true; + } + return quote || brace !== 0 || bracket !== 0 || paren !== 0; +} + +function isRecord(doc: unknown): doc is Record { + return !!doc && typeof doc === 'object' && !Array.isArray(doc); +} + +function stripBom(text: string): string { + return text.charCodeAt(0) === 0xfeff ? text.slice(1) : text; +} + +function toPosix(p: string): string { + return p.split(path.sep).join('/'); +} diff --git a/src/engine/lockfile.test.ts b/src/engine/lockfile.test.ts index 82bf1bad..04282545 100644 --- a/src/engine/lockfile.test.ts +++ b/src/engine/lockfile.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { assertDiscoveredLockfiles, discover } from './discover.js'; +import { discover } from './discover.js'; import { LockfileParseError } from '../core-open/utils/lockfile-parse.js'; import { lockfileVersion, fullDependencyTree, fullDependencyGraph } from './lockfile.js'; @@ -381,50 +381,3 @@ describe('fullDependencyGraph', () => { expect(fullDependencyGraph(root)?.ecosystem).toBe('pypi'); }); }); - -const TRUNCATED_JSON = '{"name":"fixture","lockfileVersion":3,"packages":{"node_modules/left-pad":{"version":"1.0.0"'; - -describe('truncated lockfiles in the project walk', () => { - it('names the lexicographically first invalid lockfile', () => { - fs.mkdirSync(path.join(root, 'b')); - fs.mkdirSync(path.join(root, 'a')); - fs.writeFileSync(path.join(root, 'b', 'package-lock.json'), TRUNCATED_JSON); - fs.writeFileSync( - path.join(root, 'a', 'pnpm-lock.yaml'), - "lockfileVersion: '9.0'\npackages:\n left-pad@1.0.0:\n resolution: {integrity: sha512-abc", - ); - const expected = path.join(root, 'a', 'pnpm-lock.yaml'); - expect(() => discover({ root })).toThrow(LockfileParseError); - expect(() => assertDiscoveredLockfiles({ root })).toThrow(expected); - try { - assertDiscoveredLockfiles({ root }); - } catch (err) { - const message = (err as Error).message; - expect(message).toContain('truncated or invalid YAML'); - expect(message).toContain('package manager'); - expect(message).not.toContain('left-pad'); - expect(message).not.toContain('sha512-abc'); - expect(message).not.toContain(path.join(root, 'b', 'package-lock.json')); - } - }); - - it('does not fail on a truncated lockfile under node_modules or a gitignore rule', () => { - fs.mkdirSync(path.join(root, 'node_modules', 'left-pad'), { recursive: true }); - fs.writeFileSync(path.join(root, 'node_modules', 'left-pad', 'package-lock.json'), TRUNCATED_JSON); - fs.mkdirSync(path.join(root, 'skip-lock')); - fs.writeFileSync(path.join(root, 'skip-lock', 'yarn.lock'), 'chalk@^5.0.0:\n'); - write('.gitignore', 'skip-lock/\n'); - write('app.ts', 'export const n = 1;\n'); - expect(() => assertDiscoveredLockfiles({ root })).not.toThrow(); - expect(discover({ root }).map((f) => f.rel)).toEqual(['app.ts']); - }); - - it('still fails a nested lockfile an exclude does not cover', () => { - fs.mkdirSync(path.join(root, 'packages', 'web'), { recursive: true }); - fs.writeFileSync(path.join(root, 'packages', 'web', 'package-lock.json'), TRUNCATED_JSON); - write('app.ts', 'export const n = 1;\n'); - expect(() => assertDiscoveredLockfiles({ root, exclude: ['other/**'] })).toThrow( - path.join(root, 'packages', 'web', 'package-lock.json'), - ); - }); -}); diff --git a/src/engine/truncated-lockfile.cli.test.ts b/src/engine/truncated-lockfile.cli.test.ts index b3486bb5..1e4e3cc4 100644 --- a/src/engine/truncated-lockfile.cli.test.ts +++ b/src/engine/truncated-lockfile.cli.test.ts @@ -1,4 +1,4 @@ -import { execFileSync, spawnSync } from 'node:child_process'; +import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -11,7 +11,6 @@ import { describe, it, expect, afterEach } from 'vitest'; */ const pkgRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); const cli = path.join(pkgRoot, 'src/cli.ts'); -const fixtureDir = path.join(pkgRoot, 'test/__fixtures__/truncated-lockfile'); const secret = 'npm_DDDDDDDDDDDDDDDDDDDD'; function fixture(): string { @@ -24,29 +23,12 @@ function fixture(): string { return dir; } -function copyFixture(dir: string, name: string, destName = name): void { - fs.copyFileSync(path.join(fixtureDir, name), path.join(dir, destName)); -} - -/** Parent exited. An orphan parse worker would still mention the fixture path. */ -function assertNoOrphan(dir: string): void { - let listing = ''; - try { - listing = execFileSync('ps', ['-eo', 'args'], { encoding: 'utf8' }); - } catch { - return; - } - expect(listing).not.toContain(dir); -} - -function run(dir: string, command: 'scan' | 'build', graph = false): { status: number | null; signal: NodeJS.Signals | null; stderr: string; error?: Error } { +function run(dir: string, command: 'scan' | 'build'): { status: number | null; signal: NodeJS.Signals | null; stderr: string; error?: Error } { // `-C` makes the fixture the project root. A path argument alone is a scope // inside the process cwd, so a lockfile outside that cwd would be skipped. - const scanArgs = [cli, 'scan', dir, '--offline', '--no-daemon', '--quiet']; - if (!graph) scanArgs.push('--no-graph'); const args = command === 'scan' - ? scanArgs + ? [cli, 'scan', dir, '--offline', '--no-graph', '--no-daemon', '--quiet'] : [cli, 'build', '-C', dir, '--no-warm', '--no-publish', '--offline', '--no-daemon']; const res = spawnSync(process.execPath, ['--import', 'tsx', ...args], { cwd: pkgRoot, @@ -76,72 +58,10 @@ describe('truncated lockfile CLI', () => { expect(res.signal).toBeNull(); expect(res.status).toBe(1); expect(res.stderr).toContain('package-lock.json'); - expect(res.stderr).toContain('truncated or invalid JSON'); + expect(res.stderr).toContain('truncated or not valid syntax'); expect(res.stderr).toContain('package manager'); expect(res.stderr).not.toContain(secret); expect(res.stderr).not.toContain('left-pad'); - assertNoOrphan(dir); - }, 60_000); - - it('vg scan fails a nested package-lock.json before reporting an empty graph', () => { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vg-trunc-lock-')); - dirs.push(dir); - fs.mkdirSync(path.join(dir, 'packages', 'web'), { recursive: true }); - copyFixture(path.join(dir, 'packages', 'web'), 'package-lock.json'); - fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"root","version":"1.0.0"}\n'); - fs.writeFileSync(path.join(dir, 'app.ts'), 'export const n = 1;\n'); - const res = run(dir, 'scan'); - expect(res.error).toBeUndefined(); - expect(res.signal).toBeNull(); - expect(res.status).toBe(1); - expect(res.stderr).toContain(path.join(dir, 'packages', 'web', 'package-lock.json')); - expect(res.stderr).toContain('truncated or invalid JSON'); - expect(res.stderr).toContain('package manager'); - expect(res.stderr).not.toContain('left-pad'); - expect(res.stderr).not.toContain('map build failed'); - expect(fs.existsSync(path.join(dir, '.vibgrate'))).toBe(false); - assertNoOrphan(dir); - }, 60_000); - - it('vg scan with a code map does not skip a nested truncated lockfile', () => { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vg-trunc-lock-')); - dirs.push(dir); - fs.mkdirSync(path.join(dir, 'packages', 'web'), { recursive: true }); - copyFixture(path.join(dir, 'packages', 'web'), 'package-lock.json'); - fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"root","version":"1.0.0"}\n'); - fs.writeFileSync(path.join(dir, 'app.ts'), 'export const n = 1;\n'); - const res = run(dir, 'scan', true); - expect(res.error).toBeUndefined(); - expect(res.signal).toBeNull(); - expect(res.status).toBe(1); - expect(res.stderr).toContain('truncated or invalid JSON'); - expect(res.stderr).toContain('package manager'); - expect(res.stderr).not.toContain('left-pad'); - expect(res.stderr).not.toContain('map build failed'); - expect(fs.existsSync(path.join(dir, '.vibgrate', 'graph.json'))).toBe(false); - assertNoOrphan(dir); - }, 60_000); - - it('vg scan fails a truncated yarn.lock and pnpm-lock.yaml from the fixture', () => { - for (const [name, kind] of [ - ['yarn.lock', 'yarn.lock'], - ['pnpm-lock.yaml', 'YAML'], - ] as const) { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vg-trunc-lock-')); - dirs.push(dir); - copyFixture(dir, name); - fs.writeFileSync(path.join(dir, 'app.ts'), 'export const n = 1;\n'); - const res = run(dir, 'scan'); - expect(res.error, name).toBeUndefined(); - expect(res.signal, name).toBeNull(); - expect(res.status, name).toBe(1); - expect(res.stderr, name).toContain(name); - expect(res.stderr, name).toContain(`truncated or invalid ${kind}`); - expect(res.stderr, name).toContain('package manager'); - expect(res.stderr, name).not.toContain('left-pad'); - expect(res.stderr, name).not.toContain('sha512-abc'); - assertNoOrphan(dir); - } }, 60_000); it('vg build exits non-zero before parse workers stay up', () => { @@ -152,9 +72,7 @@ describe('truncated lockfile CLI', () => { expect(res.signal).toBeNull(); expect(res.status).toBe(1); expect(res.stderr).toContain('package-lock.json'); - expect(res.stderr).toContain('truncated or invalid JSON'); + expect(res.stderr).toContain('truncated or not valid syntax'); expect(res.stderr).not.toContain(secret); - expect(fs.existsSync(path.join(dir, '.vibgrate', 'graph.json'))).toBe(false); - assertNoOrphan(dir); }, 60_000); }); diff --git a/src/reporting/commands/scan.ts b/src/reporting/commands/scan.ts index bde26095..445514c2 100644 --- a/src/reporting/commands/scan.ts +++ b/src/reporting/commands/scan.ts @@ -52,7 +52,7 @@ import { emitIngestIdLine, emitDriftScoreLine } from '../utils/ingest-id-output. import { formatUploadHttpFailure, uploadScanArtifact } from '../utils/upload.js'; import { redactForDisplay } from '../../core-open/utils/redact.js'; import { buildGraph } from '../../engine/build.js'; -import { assertDiscoveredLockfiles, mergeExcludes } from '../../engine/discover.js'; +import { assertLockfilesValid, LockfileSyntaxError } from '../../engine/lockfile-guard.js'; import { writeArtifacts, resolveGraphPath } from '../../engine/artifacts.js'; import { readHaileSidecar } from '../../engine/haile/sidecar.js'; import { isUsableHaileSymbol } from '../../engine/haile/format.js'; @@ -478,14 +478,14 @@ export const scanCommand = new Command('scan') throw err; } - // Before preflight, project scanners, and the code-map parse pool. A - // truncated lockfile — including one nested in a subdirectory, and - // including `vg scan --no-graph` — must not become a drift report that - // looks empty, or a map failure that the command then skips. - assertDiscoveredLockfiles({ - root: rootDir, - exclude: mergeExcludes(rootDir, opts.exclude), - }); + // Before project scanners and the code-map parse workers. A truncated + // lockfile must not become a partial dependency graph or a successful exit. + try { + assertLockfilesValid(rootDir); + } catch (err) { + if (err instanceof LockfileSyntaxError) throw new CliError(err.message, ExitCode.ERROR); + throw err; + } // Fail closed before any scan work. A missing, unreadable, or unusable // --package-manifest must stop the command with a stable, actionable diff --git a/test/__fixtures__/truncated-lockfile/package-lock.json b/test/__fixtures__/truncated-lockfile/package-lock.json deleted file mode 100644 index b038d358..00000000 --- a/test/__fixtures__/truncated-lockfile/package-lock.json +++ /dev/null @@ -1 +0,0 @@ -{"name":"fixture","lockfileVersion":3,"packages":{"node_modules/left-pad":{"version":"1.0.0" diff --git a/test/__fixtures__/truncated-lockfile/pnpm-lock.yaml b/test/__fixtures__/truncated-lockfile/pnpm-lock.yaml deleted file mode 100644 index 5e41334a..00000000 --- a/test/__fixtures__/truncated-lockfile/pnpm-lock.yaml +++ /dev/null @@ -1,4 +0,0 @@ -lockfileVersion: '9.0' -packages: - left-pad@1.0.0: - resolution: {integrity: sha512-abc diff --git a/test/__fixtures__/truncated-lockfile/yarn.lock b/test/__fixtures__/truncated-lockfile/yarn.lock deleted file mode 100644 index 7bbf7c3b..00000000 --- a/test/__fixtures__/truncated-lockfile/yarn.lock +++ /dev/null @@ -1,3 +0,0 @@ -left-pad@^1.0.0: - version "1.0.0" -chalk@^5.0.0: diff --git a/vibgrate.config.json b/vibgrate.config.json index 70d61abd..bb5a11ca 100644 --- a/vibgrate.config.json +++ b/vibgrate.config.json @@ -3,13 +3,11 @@ "$excludeNotes": { "packages/test-solutions/**": "Vendored real-world sample repos (Next.js SaaS, Turborepo, Clean Architecture, …) used as scan test targets and as the recorded CLI-simulator scenarios — their dependencies are intentionally frozen in time and are never installed or executed.", "examples/**": "Documentation fixtures (Go pseudo-version matching, GitHub Actions workflows). Versions are chosen to illustrate the scanner, including a deliberately end-of-life Go toolchain, and are not dependencies of this package. Tests read the files directly.", - "example/**": "Singular form of examples/, excluded for the same reason.", - "test/__fixtures__/truncated-lockfile/**": "Deliberately truncated lockfiles used to assert vg scan and vg build fail. They are not dependencies of this package." + "example/**": "Singular form of examples/, excluded for the same reason." }, "exclude": [ "packages/test-solutions/**", "examples/**", - "example/**", - "test/__fixtures__/truncated-lockfile/**" + "example/**" ] }