From 305c978c29e7615ebc65d1d2c3b492d6af8a5067 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 7 Oct 2026 12:14:26 +0000 Subject: [PATCH 1/2] fix: report symlinks skipped by build and scan walks Directory and file symlinks are not followed, so a link that points at its parent cannot hang vg build or vg scan. Print one stderr notice when a walk skips any, without changing output files or the exit code. Signed-off-by: Cursor Agent Co-authored-by: Crowers --- CHANGELOG.md | 2 + DOCS.md | 17 +++ src/engine/build.ts | 6 + src/engine/discover.ts | 38 ++++- src/reporting/commands/scan.ts | 11 ++ src/util/skipped-symlinks.ts | 142 ++++++++++++++++++ test/symlink-walk.test.ts | 253 +++++++++++++++++++++++++++++++++ 7 files changed, 463 insertions(+), 6 deletions(-) create mode 100644 src/util/skipped-symlinks.ts create mode 100644 test/symlink-walk.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 72af8f05..55de8b40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ backward compatible. ### Fixed +- **`vg build` and `vg scan` do not follow symlinks, and they say when they skip one.** A directory symlink that points at its parent cannot cycle the walk. When a walk skips symlinks, one stderr notice lists the count and the first five root-relative paths (sorted). Output files and the exit code stay the same. Point the root at the link target, or pass `--exclude`, or use a narrower root. + - **`vg sbom export` merges multi-project lockfiles by ecosystem, name, and version.** The same package from two projects stays one component, and `vibgrate:projects` lists every project that contributed it. A sub-project diff --git a/DOCS.md b/DOCS.md index 5b1d5626..ac58261c 100644 --- a/DOCS.md +++ b/DOCS.md @@ -102,6 +102,7 @@ For a quick overview, see the [README](./README.md). This document covers everyt - [vibgrate.config.ts](#vibgrateconfigts) - [Thresholds](#thresholds) - [Scanner Toggles](#scanner-toggles) + - [Symlinks](#symlinks) - [Extended Scanners](#extended-scanners) - [Platform Matrix](#platform-matrix) - [Dependency Risk](#dependency-risk) @@ -1979,6 +1980,8 @@ vg scan [path] [--vulns] [--full] [--iac] [--format text|json|sarif|md] [--out < By default, the scan writes `.vibgrate/scan_result.json`. Use `--no-local-artifacts` or `--max-privacy` to suppress local JSON artifact files. +The scan walk does not follow symlinks. A skipped link prints the same stderr notice as `vg build`, and that notice is not part of the report (`--format json` stays on stdout). See [Symlinks](#symlinks). `--exclude` on this command is the same glob as on `vg build`. + 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). ### Offline scan with a package-version manifest @@ -2553,6 +2556,8 @@ Maps source code into a graph artifact, enabling all downstream queries (`vg sho | `--attestation ` | `.vibgrate/attestation.intoto.jsonl` | Where `--attest` writes, and where `--verify` reads | | `--pub ` | — | Public key PEM that pins the signer for `--verify` | +**Symlinks are not followed.** A directory or file symlink is left out of the map. When the walk skips one, stderr gets a single notice (the count and the first few root-relative paths). Nothing is written into `graph.json` for the link. See [Symlinks](#symlinks). + **Local by default — no git churn.** The first time vg writes into `.vibgrate/` it also creates `.vibgrate/.gitignore`, keeping the graph artifacts (`graph.json`, `graph.html`, `GRAPH_REPORT.md`, `facts.jsonl`, `mcp-navigation.json`) and the cache out of git — so builds, auto-refreshes, and MCP use never leave your branch dirty. Run `vg share` when you want the map committed for your team (it rewrites that ignore file). vg never touches an existing `.vibgrate/.gitignore`, so edit it (or leave it empty) to manage the ignores yourself. Maven `import` edges, and the Gradle files that stay document nodes, are in [Maven and Gradle manifests](#maven-and-gradle-manifests). How a GitHub Actions `uses:` pin is copied into the map: [Action pins in the code map](./docs/ci/github-actions.md#action-pins-in-the-code-map). @@ -4753,6 +4758,18 @@ of observed memory — so identical input still produces an identical `graph.json`. To give the build more room instead of limiting it, raise the Node heap: `NODE_OPTIONS=--max-old-space-size=8192`. +### Symlinks + +`vg build` and `vg scan` do not follow symlinks. Discovery (`vg build`) and the scan walk both read a directory with `readdir` and only enter an entry when it is a real directory. A symlink is not a directory and not a file under that check, so it is not entered and not read. A directory symlink that points at its parent, or at another directory inside the tree, cannot cycle the walk. The link target is indexed only when it is also a real path under the root you named. + +When a walk skips one or more symlinks, vg prints one notice on stderr. The notice is the count plus the first five root-relative paths, sorted. Further links are a remaining count. It does not print absolute paths. It prints nothing when there are no symlinks, and nothing for a link already covered by `.gitignore` or `--exclude`. The notice is not written into `graph.json`, the scan report, or `--format json` on stdout, and it does not change the exit code. + +```text +skipped 2 symlinks (vg does not follow links): alias.ts, nested/up. Point the root at the link target, or pass --exclude, or use a narrower root. +``` + +To index the tree a link points at, point the root at that directory. To leave a link out of the notice, pass `--exclude` (repeatable; also read from the project config) or run with a narrower root. An unsafe root — the filesystem root, an operating-system image, or a walk over the `VG_MAX_FILES` budget — already stops with that same guidance to narrow the path or add `--exclude`. + --- ## Extended Scanners diff --git a/src/engine/build.ts b/src/engine/build.ts index 99006736..ae42e64c 100644 --- a/src/engine/build.ts +++ b/src/engine/build.ts @@ -100,6 +100,11 @@ export interface BuildOptions { /** Resource-safeguard overrides (else VG_MAX_FILE_BYTES / VG_MAX_FILES / * VG_TSC_MAX_FILES / VG_MEMORY_BUDGET_MB env vars, else defaults). */ limits?: Partial; + /** + * Print discovery's skipped-symlink notice. Default true. + * `vg scan` sets this false: the scan walk prints the notice itself. + */ + symlinkNotice?: boolean; } /** Stat + content hash of one corpus file at build time. */ @@ -163,6 +168,7 @@ export async function buildGraph(options: BuildOptions): Promise { exclude, paths: options.paths, maxEntries: limits.maxFiles, + symlinkNotice: options.symlinkNotice, }); timer.end('discover'); diff --git a/src/engine/discover.ts b/src/engine/discover.ts index 2f523416..915a52dd 100644 --- a/src/engine/discover.ts +++ b/src/engine/discover.ts @@ -6,6 +6,7 @@ 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 { assertSafeWalkRoot, createWalkBudget, noteWalkEntry } from '../core-open/utils/root-safety.js'; +import { emitSkippedSymlinkNotice } from '../util/skipped-symlinks.js'; /** * Deterministic file discovery. @@ -166,6 +167,12 @@ export interface DiscoverOptions { * Default: `VG_MAX_FILES`, else 100000. */ maxEntries?: number; + /** + * Print the skipped-symlink notice on stderr. Default true. + * The walk never follows links either way. `vg scan` passes false + * when it builds a map, because the scan walk already printed one. + */ + symlinkNotice?: boolean; } export interface DiscoveredFile { @@ -248,6 +255,15 @@ export function discover(options: DiscoverOptions): DiscoveredFile[] { const found = new Map(); const budget = createWalkBudget(root, options.maxEntries); + // Links the walk refused to follow. Sorted later, when the notice is formatted. + const skippedSymlinks: string[] = []; + + const noteSymlink = (rel: string): void => { + if (!rel || rel.startsWith('..')) return; + // Already ignored or excluded: the operator asked for this path to be skipped. + if (rootIg.ignores(rel) || rootIg.ignores(`${rel}/`)) return; + skippedSymlinks.push(rel); + }; const considerFile = (abs: string): void => { const rel = toPosix(path.relative(root, abs)); @@ -285,16 +301,26 @@ export function discover(options: DiscoverOptions): DiscoveredFile[] { const over = noteWalkEntry(budget); if (over) throw over; considerFile(abs); + } else if (entry.isSymbolicLink()) { + // isDirectory() / isFile() do not follow links, so a symlink is + // neither branch above. Leave it unentered: a link that points + // at its parent would otherwise cycle. The target is indexed + // only when it is also a real path under the root. + noteSymlink(rel); } } }; - for (const scope of scopeAbs) { - const stat = fs.statSync(scope); - if (stat.isDirectory()) { - assertSafeWalkRoot(scope); - walk(scope); - } else if (stat.isFile()) considerFile(scope); + try { + for (const scope of scopeAbs) { + const stat = fs.statSync(scope); + if (stat.isDirectory()) { + assertSafeWalkRoot(scope); + walk(scope); + } else if (stat.isFile()) considerFile(scope); + } + } finally { + if (options.symlinkNotice !== false) emitSkippedSymlinkNotice(skippedSymlinks); } return [...found.values()].sort((a, b) => (a.rel < b.rel ? -1 : a.rel > b.rel ? 1 : 0)); diff --git a/src/reporting/commands/scan.ts b/src/reporting/commands/scan.ts index f23006a0..75c114f3 100644 --- a/src/reporting/commands/scan.ts +++ b/src/reporting/commands/scan.ts @@ -17,6 +17,7 @@ import { parseExcludePatterns, loadConfig, findConfigFile, + isSkippedDirName, } from '../../core-open/index.js'; import { writeScanSummary } from '../scan-summary.js'; import { compareDriftBudget, evaluateConfigDriftBudget } from '../drift-budget-gate.js'; @@ -64,6 +65,7 @@ import { loadPackageVersionManifest, PackageManifestError } from '../package-ver import { runSecurityPacks, type SecurityRunResult } from '../../security/run-packs.js'; import { evaluateSecurityGate, lowestThreshold, parseFailOn } from '../../security/gate.js'; import { securityFindingRow, securityPacksLabel } from '../../core-open/formatters/text.js'; +import { collectScanSkippedSymlinks, emitSkippedSymlinkNotice } from '../../util/skipped-symlinks.js'; /** The security packs `--iac` requests from the Architecture module. */ const IAC_PACKS = ['iac-cis-v1'] as const; @@ -706,6 +708,8 @@ export const scanCommand = new Command('scan') const result = await buildGraph({ root: rootDir, exclude: opts.exclude, + // The scan walk already printed the skipped-symlink notice. + symlinkNotice: false, onParseProgress: (done, total) => report(done, total, 'parsing'), }); builtGraph = result.graph; @@ -760,6 +764,13 @@ export const scanCommand = new Command('scan') // Open base scan. The optional advanced-analysis hook is a no-op in this // open build, so the scan runs entirely on the open base engine. const advanced = await loadAdvancedScanHook(); + // The scan walk (core-open FileCache) does not follow symlinks: it + // recurses only when Dirent.isDirectory() is true. The notice lives + // here, not in that vendored file, so a later vendor sync cannot drop + // it. Excludes match the walk: config first, then CLI, de-duplicated. + const noticeConfig = await loadConfig(rootDir); + const noticeExclude = [...new Set([...(noticeConfig.exclude ?? []), ...(opts.exclude ?? [])])]; + emitSkippedSymlinkNotice(collectScanSkippedSymlinks(rootDir, noticeExclude, isSkippedDirName)); const artifact = await runCoreScan(rootDir, scanOpts, advanced); // The scan just built a code map (its `postScan` step). Start the local diff --git a/src/util/skipped-symlinks.ts b/src/util/skipped-symlinks.ts new file mode 100644 index 00000000..b0a93dc5 --- /dev/null +++ b/src/util/skipped-symlinks.ts @@ -0,0 +1,142 @@ +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import ignore, { type Ignore } from 'ignore'; +import { compileGlobs, gitignoreWithoutBlankLines } from '../core-open/utils/glob.js'; +import { info } from './output.js'; + +/** + * How many root-relative symlink paths a single notice lists. + * The rest are a count only, so a link farm cannot flood stderr. + */ +export const SKIPPED_SYMLINK_NOTICE_CAP = 5; + +const REMEDY = + 'Point the root at the link target, or pass --exclude, or use a narrower root.'; + +interface GitignoreLevel { + dir: string; + ig: Ignore; +} + +function comparePath(a: string, b: string): number { + return a < b ? -1 : a > b ? 1 : 0; +} + +/** Root-relative POSIX path. Absolute paths and `..` escapes are dropped. */ +export function rootRelativePosix(p: string): string | null { + const posix = p.split(path.sep).join('/').split('\\').join('/'); + if (!posix || posix === '.' || posix === '..' || posix.startsWith('../')) return null; + if (posix.startsWith('/') || /^[A-Za-z]:/.test(posix)) return null; + return posix; +} + +/** + * One stderr line for symlinks a walk did not follow. + * Paths are sorted, de-duplicated, and capped. `null` when there is nothing to say. + */ +export function formatSkippedSymlinkNotice(relPaths: readonly string[]): string | null { + const unique: string[] = []; + const seen = new Set(); + for (const raw of relPaths) { + const rel = rootRelativePosix(raw); + if (!rel || seen.has(rel)) continue; + seen.add(rel); + unique.push(rel); + } + unique.sort(comparePath); + if (unique.length === 0) return null; + const shown = unique.slice(0, SKIPPED_SYMLINK_NOTICE_CAP); + const extra = unique.length - shown.length; + const list = extra > 0 ? `${shown.join(', ')} (+${extra} more)` : shown.join(', '); + const noun = unique.length === 1 ? 'symlink' : 'symlinks'; + return `skipped ${unique.length} ${noun} (vg does not follow links): ${list}. ${REMEDY}`; +} + +/** Print {@link formatSkippedSymlinkNotice} on stderr. No-op when there are none. */ +export function emitSkippedSymlinkNotice(relPaths: readonly string[]): void { + const text = formatSkippedSymlinkNotice(relPaths); + if (!text) return; + info(text); +} + +function extendGitignoreLevels(dir: string, levels: GitignoreLevel[]): GitignoreLevel[] { + try { + const txt = fs.readFileSync(path.join(dir, '.gitignore'), 'utf8'); + const rules = gitignoreWithoutBlankLines(txt); + if (!rules) return levels; + return [...levels, { dir, ig: ignore().add(rules) }]; + } catch { + return levels; + } +} + +/** + * Same root-to-leaf `.gitignore` precedence as the scan walk: a deeper + * match wins, including a negation that un-ignores an ancestor rule. + */ +function isGitignored(levels: GitignoreLevel[], absPath: string, isDir: boolean): boolean { + if (levels.length === 0) return false; + let ignored = false; + for (const { dir, ig } of levels) { + const rel = path.relative(dir, absPath); + if (!rel || rel.startsWith('..')) continue; + const relPosix = rel.split(path.sep).join('/'); + const testPath = isDir ? `${relPosix}/` : relPosix; + const result = ig.test(testPath); + if (result.ignored) ignored = true; + else if (result.unignored) ignored = false; + } + return ignored; +} + +/** + * Symlinks the scan walk will not follow. + * + * Mirrors the scan walker's directory pruning (skip-dir names, nested + * `.gitignore`, config/`--exclude` globs) and reads each directory with + * `readdir({ withFileTypes: true })`. A symlink is not a directory and not + * a file under that API, so the real walk never enters it. This census + * does not either: it only records the link path. Lives outside + * `src/core-open` so a vendor sync that replaces that tree cannot drop the + * notice. + */ +export function collectScanSkippedSymlinks( + root: string, + exclude: readonly string[] | undefined, + isSkippedDirName: (name: string) => boolean, +): string[] { + const rootDir = path.resolve(root); + const excluded = compileGlobs([...(exclude ?? [])]); + const found: string[] = []; + + const walk = (dir: string, parentLevels: GitignoreLevel[]): void => { + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return; + } + const levels = extendGitignoreLevels(dir, parentLevels); + for (const entry of entries) { + const abs = path.join(dir, entry.name); + const rel = path.relative(rootDir, abs); + if (!rel || rel.startsWith('..')) continue; + const relPosix = rel.split(path.sep).join('/'); + if (excluded?.(relPosix)) continue; + if (entry.isDirectory()) { + if (isSkippedDirName(entry.name)) continue; + if (isGitignored(levels, abs, true)) continue; + walk(abs, levels); + continue; + } + if (!entry.isSymbolicLink()) continue; + // A link the operator already ignored is not news. Check both shapes: + // gitignore treats `name` and `name/` differently. + if (isGitignored(levels, abs, false) || isGitignored(levels, abs, true)) continue; + found.push(relPosix); + } + }; + + walk(rootDir, []); + return found; +} diff --git a/test/symlink-walk.test.ts b/test/symlink-walk.test.ts new file mode 100644 index 00000000..8369e60b --- /dev/null +++ b/test/symlink-walk.test.ts @@ -0,0 +1,253 @@ +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { FileCache, findFiles, isSkippedDirName, quickTreeCount } from '../src/core-open/utils/fs.js'; +import { discover } from '../src/engine/discover.js'; +import { + collectScanSkippedSymlinks, + emitSkippedSymlinkNotice, + formatSkippedSymlinkNotice, + SKIPPED_SYMLINK_NOTICE_CAP, +} from '../src/util/skipped-symlinks.js'; + +const NOTICE = + 'skipped 2 symlinks (vg does not follow links): alias.ts, nested/up. Point the root at the link target, or pass --exclude, or use a narrower root.'; + +function symlinksSupported(): boolean { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vg-symlink-probe-')); + try { + fs.symlinkSync('nowhere', path.join(dir, 'link'), 'file'); + return true; + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (code === 'EPERM' || code === 'ENOTSUP' || code === 'EACCES') return false; + throw err; + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +const SUPPORTED = symlinksSupported(); + +function norm(p: string): string { + return p.split(path.sep).join('/'); +} + +/** + * Directory symlink `nested/up` -> parent, plus a file symlink `alias.ts`. + * A link inside `node_modules` must stay out of the notice (that directory + * is pruned before its children are read). + */ +function makeCycleFixture(): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'vg-symlink-')); + fs.mkdirSync(path.join(root, 'nested')); + fs.mkdirSync(path.join(root, 'node_modules', 'pkg'), { recursive: true }); + fs.writeFileSync(path.join(root, 'package.json'), JSON.stringify({ name: 'symlink-fixture', version: '1.0.0' })); + fs.writeFileSync(path.join(root, 'keep.ts'), 'export const n = 1;\n'); + fs.writeFileSync(path.join(root, 'nested', 'inside.ts'), 'export const n = 2;\n'); + fs.symlinkSync('..', path.join(root, 'nested', 'up'), 'dir'); + fs.symlinkSync('keep.ts', path.join(root, 'alias.ts'), 'file'); + fs.symlinkSync('keep.ts', path.join(root, 'node_modules', 'pkg', 'link.ts'), 'file'); + return root; +} + +function captureStderr(run: () => void): string { + const chunks: string[] = []; + const spy = vi.spyOn(process.stderr, 'write').mockImplementation((chunk) => { + chunks.push(String(chunk)); + return true; + }); + try { + run(); + } finally { + spy.mockRestore(); + } + return chunks.join(''); +} + +describe('skipped symlink notice', () => { + it('formats one stable line: sorted, capped, root-relative, silent when empty', () => { + expect(formatSkippedSymlinkNotice([])).toBeNull(); + expect(formatSkippedSymlinkNotice(['/tmp/abs', 'C:/abs', '../out'])).toBeNull(); + const once = formatSkippedSymlinkNotice(['nested/up', 'alias.ts', '/var/abs']); + const twice = formatSkippedSymlinkNotice(['alias.ts', 'nested/up']); + expect(once).toBe(NOTICE); + expect(twice).toBe(once); + + const many = Array.from({ length: SKIPPED_SYMLINK_NOTICE_CAP + 2 }, (_, i) => `s${i}.ts`); + const capped = formatSkippedSymlinkNotice([...many].reverse()); + expect(capped).toBe( + 'skipped 7 symlinks (vg does not follow links): s0.ts, s1.ts, s2.ts, s3.ts, s4.ts (+2 more). Point the root at the link target, or pass --exclude, or use a narrower root.', + ); + expect(formatSkippedSymlinkNotice(['only.ts'])).toBe( + 'skipped 1 symlink (vg does not follow links): only.ts. Point the root at the link target, or pass --exclude, or use a narrower root.', + ); + }); + + it('prints nothing when there are no symlinks', () => { + const text = captureStderr(() => emitSkippedSymlinkNotice([])); + expect(text).toBe(''); + }); +}); + +describe.skipIf(!SUPPORTED)('symlink cycles are not followed', () => { + const dirs: string[] = []; + afterEach(() => { + while (dirs.length) fs.rmSync(dirs.pop()!, { recursive: true, force: true }); + }); + + function fixture(): string { + const root = makeCycleFixture(); + dirs.push(root); + return root; + } + + it('vg build discovery finishes, stays deterministic, and does not descend', () => { + const root = fixture(); + const stderr: string[] = []; + const spy = vi.spyOn(process.stderr, 'write').mockImplementation((chunk) => { + stderr.push(String(chunk)); + return true; + }); + let first: string[] = []; + let second: string[] = []; + try { + first = discover({ root }).map((f) => f.rel); + second = discover({ root }).map((f) => f.rel); + } finally { + spy.mockRestore(); + } + expect(first).toEqual(['keep.ts', 'nested/inside.ts']); + expect(second).toEqual(first); + expect(first.some((rel) => rel === 'alias.ts' || rel.includes('nested/up') || rel.includes('node_modules'))).toBe(false); + const noticeLines = stderr.join('').split('\n').filter((line) => line.includes('does not follow links')); + expect(noticeLines).toEqual([NOTICE, NOTICE]); + expect(noticeLines.join('')).not.toContain(root); + }); + + it('does not print a notice when every symlink is excluded or gitignored', () => { + const root = fixture(); + const excluded = captureStderr(() => { + discover({ root, exclude: ['alias.ts', 'nested/up'] }); + }); + expect(excluded).not.toContain('does not follow links'); + + fs.writeFileSync(path.join(root, '.gitignore'), 'alias.ts\nnested/up\n'); + const ignored = captureStderr(() => { + discover({ root }); + }); + expect(ignored).not.toContain('does not follow links'); + }); + + it('can silence the notice without following the link', () => { + const root = fixture(); + const text = captureStderr(() => { + const rels = discover({ root, symlinkNotice: false }).map((f) => f.rel); + expect(rels).toEqual(['keep.ts', 'nested/inside.ts']); + }); + expect(text).not.toContain('does not follow links'); + }); + + it('the scan walk finishes, stays deterministic, and does not descend', async () => { + const root = fixture(); + const listed = (entries: { relPath: string }[]) => + entries.map((e) => norm(e.relPath)).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); + + const first = listed(await new FileCache().walkDir(root)); + const second = listed(await new FileCache().walkDir(root)); + expect(second).toEqual(first); + expect(first).toEqual(['keep.ts', 'nested', 'nested/inside.ts', 'package.json']); + expect(first.some((rel) => rel === 'alias.ts' || rel === 'nested/up' || rel.startsWith('nested/up/'))).toBe(false); + + const found = (await findFiles(root, (name) => name.endsWith('.ts'))) + .map((abs) => norm(path.relative(root, abs))) + .sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); + expect(found).toEqual(['keep.ts', 'nested/inside.ts']); + + const again = (await findFiles(root, (name) => name.endsWith('.ts'))) + .map((abs) => norm(path.relative(root, abs))) + .sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); + expect(again).toEqual(found); + + expect(await quickTreeCount(root)).toEqual({ totalFiles: 3, totalDirs: 1 }); + + const skipped = collectScanSkippedSymlinks(root, undefined, isSkippedDirName) + .slice() + .sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); + const skippedAgain = collectScanSkippedSymlinks(root, undefined, isSkippedDirName) + .slice() + .sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); + expect(skipped).toEqual(['alias.ts', 'nested/up']); + expect(skippedAgain).toEqual(skipped); + expect(formatSkippedSymlinkNotice(skipped)).toBe(NOTICE); + expect(NOTICE).not.toContain(root); + }); + + it('the scan census honours nested gitignore and --exclude', () => { + const root = fixture(); + fs.writeFileSync(path.join(root, 'nested', '.gitignore'), 'up\n'); + expect(collectScanSkippedSymlinks(root, undefined, isSkippedDirName).slice().sort()).toEqual(['alias.ts']); + expect(collectScanSkippedSymlinks(root, ['alias.ts'], isSkippedDirName)).toEqual([]); + }); + + it('vg build and vg scan keep JSON on stdout and exit 0', () => { + const root = fixture(); + const cli = path.resolve('src/cli.ts'); + const generatedAt = '2026-01-01T00:00:00.000Z'; + const run = (args: string[]) => { + const res = spawnSync(process.execPath, ['--import', 'tsx', cli, ...args], { + cwd: process.cwd(), + encoding: 'utf8', + env: { + ...process.env, + NO_COLOR: '1', + FORCE_COLOR: '0', + VIBGRATE_DSN: '', + VIBGRATE_NO_KERNEL: '1', + }, + timeout: 90_000, + }); + return { status: res.status, stdout: res.stdout ?? '', stderr: res.stderr ?? '' }; + }; + + const buildArgs = [ + 'build', '-C', root, + '--offline', '--json', '--quiet', '--no-publish', '--no-warm', '--no-daemon', + '--generated-at', generatedAt, + ]; + const first = run(buildArgs); + expect(first.status, first.stderr).toBe(0); + expect(first.stdout).not.toContain('does not follow links'); + const built = JSON.parse(first.stdout) as { ok: boolean; artifacts: { graphPath: string } }; + expect(built.ok).toBe(true); + const graphPath = built.artifacts.graphPath; + const bytes = fs.readFileSync(graphPath); + const graph = JSON.parse(bytes.toString('utf8')) as { nodes: { file?: string }[] }; + const files = graph.nodes.map((n) => n.file).filter((f): f is string => typeof f === 'string'); + expect(files).toContain('keep.ts'); + expect(files).toContain('nested/inside.ts'); + expect(files.some((f) => f === 'alias.ts' || f.includes('nested/up') || f.includes('node_modules'))).toBe(false); + const buildNotices = first.stderr.split('\n').filter((line) => line.includes('does not follow links')); + expect(buildNotices).toEqual([NOTICE]); + expect(buildNotices[0]).not.toContain(root); + + const second = run(buildArgs); + expect(second.status, second.stderr).toBe(0); + expect(fs.readFileSync(graphPath).equals(bytes)).toBe(true); + + const scan = run([ + 'scan', root, + '--offline', '--format', 'json', '--no-graph', '--no-local-artifacts', '--quiet', '--no-daemon', + ]); + expect(scan.status, scan.stderr).toBe(0); + expect(scan.stdout).not.toContain('does not follow links'); + const report = JSON.parse(scan.stdout) as { projects?: unknown }; + expect(report).toBeTypeOf('object'); + const scanNotices = scan.stderr.split('\n').filter((line) => line.includes('does not follow links')); + expect(scanNotices).toEqual([NOTICE]); + expect(scanNotices[0]).not.toContain(root); + expect(fs.existsSync(path.join(root, '.vibgrate', 'scan_result.json'))).toBe(false); + }, 120_000); +}); From d9873d625c85808496c474f17b9a6ba563c3cba8 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 8 Oct 2026 20:42:10 +0000 Subject: [PATCH 2/2] refactor(walk): align skipped-symlink notice wording Walks still do not follow symlinks. When any are skipped, one stderr notice lists the count and the first few root-relative paths. Signed-off-by: Cursor Agent Co-authored-by: Crowers --- CHANGELOG.md | 11 +- DOCS.md | 30 ++- src/core-open/utils/fs.ts | 29 ++ src/core-open/utils/skipped-symlinks.ts | 65 +++++ src/engine/build.ts | 6 - src/engine/discover.ts | 47 ++-- src/engine/docs-ingest.ts | 2 + src/reporting/commands/scan.ts | 11 - src/util/skipped-symlinks.ts | 142 ---------- test/symlink-walk.test.ts | 336 ++++++++++-------------- 10 files changed, 279 insertions(+), 400 deletions(-) create mode 100644 src/core-open/utils/skipped-symlinks.ts delete mode 100644 src/util/skipped-symlinks.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 55de8b40..a6f681bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,15 @@ backward compatible. ## [Unreleased] +### Changed + +- **`vg build` and `vg scan` do not follow symlinks, and they say so.** A + directory symlink is not entered, so a link to its parent cannot hang the + walk, and a file symlink is not read. When any are skipped, one stderr + notice lists the count and the first few root-relative paths. Nothing is + printed when there are none. Output files, the exit code, and JSON on + stdout stay the same. + ### Added - **`vg build --attest` and `vg build --verify` are documented.** The docs @@ -24,8 +33,6 @@ backward compatible. ### Fixed -- **`vg build` and `vg scan` do not follow symlinks, and they say when they skip one.** A directory symlink that points at its parent cannot cycle the walk. When a walk skips symlinks, one stderr notice lists the count and the first five root-relative paths (sorted). Output files and the exit code stay the same. Point the root at the link target, or pass `--exclude`, or use a narrower root. - - **`vg sbom export` merges multi-project lockfiles by ecosystem, name, and version.** The same package from two projects stays one component, and `vibgrate:projects` lists every project that contributed it. A sub-project diff --git a/DOCS.md b/DOCS.md index 847c7c23..17129a5e 100644 --- a/DOCS.md +++ b/DOCS.md @@ -1990,7 +1990,7 @@ 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. -The scan walk does not follow symlinks. A skipped link prints the same stderr notice as `vg build`, and that notice is not part of the report (`--format json` stays on stdout). See [Symlinks](#symlinks). `--exclude` on this command is the same glob as on `vg build`. +`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). @@ -2566,7 +2566,7 @@ Maps source code into a graph artifact, enabling all downstream queries (`vg sho | `--attestation ` | `.vibgrate/attestation.intoto.jsonl` | Where `--attest` writes, and where `--verify` reads | | `--pub ` | — | Public key PEM that pins the signer for `--verify` | -**Symlinks are not followed.** A directory or file symlink is left out of the map. When the walk skips one, stderr gets a single notice (the count and the first few root-relative paths). Nothing is written into `graph.json` for the link. See [Symlinks](#symlinks). +`vg build` does not follow symlinks while it discovers files. A skipped link is named once on stderr. See [Symlinks](#symlinks). `.gitignore` and `--exclude` still apply. **Local by default — no git churn.** The first time vg writes into `.vibgrate/` it also creates `.vibgrate/.gitignore`, keeping the graph artifacts (`graph.json`, `graph.html`, `GRAPH_REPORT.md`, `facts.jsonl`, `mcp-navigation.json`) and the cache out of git — so builds, auto-refreshes, and MCP use never leave your branch dirty. Run `vg share` when you want the map committed for your team (it rewrites that ignore file). vg never touches an existing `.vibgrate/.gitignore`, so edit it (or leave it empty) to manage the ignores yourself. @@ -4862,16 +4862,30 @@ Node heap: `NODE_OPTIONS=--max-old-space-size=8192`. ### Symlinks -`vg build` and `vg scan` do not follow symlinks. Discovery (`vg build`) and the scan walk both read a directory with `readdir` and only enter an entry when it is a real directory. A symlink is not a directory and not a file under that check, so it is not entered and not read. A directory symlink that points at its parent, or at another directory inside the tree, cannot cycle the walk. The link target is indexed only when it is also a real path under the root you named. - -When a walk skips one or more symlinks, vg prints one notice on stderr. The notice is the count plus the first five root-relative paths, sorted. Further links are a remaining count. It does not print absolute paths. It prints nothing when there are no symlinks, and nothing for a link already covered by `.gitignore` or `--exclude`. The notice is not written into `graph.json`, the scan report, or `--format json` on stdout, and it does not change the exit code. +`vg build` and `vg scan` do not follow symlinks. While walking, a directory +symlink is not entered and a file symlink is not read. That includes a link +that points at its own parent, so a cycle cannot make the walk hang. The link +is left out of the map and out of the scan. Output files do not change, and +the exit code stays the same. + +The directory you pass as the root is opened even when that path itself is a +symlink. Links found underneath it are not followed. To include the files a +link points at, point the root at that target, or pass a narrower directory +that already contains them. To leave a link out without a notice, list it in +`.gitignore` or pass `--exclude`. + +When a walk skips one or more symlinks, vg prints one notice on stderr and +nothing when there are none. The notice is the count, then the first few +root-relative paths in sorted order (at most five; further links are a +`+N more` count). Paths are relative to the root you passed. Absolute paths +are not printed. The line goes to stderr, so `--format json` and +`vg build --json` keep a JSON document on stdout. `--quiet` hides promotional +text only; it does not hide this notice. ```text -skipped 2 symlinks (vg does not follow links): alias.ts, nested/up. Point the root at the link target, or pass --exclude, or use a narrower root. +notice: skipped 3 symlinks (alias.ts, nested/cycle, via). vg does not follow symlinks. Point the root at the link target, or pass --exclude or a narrower root. ``` -To index the tree a link points at, point the root at that directory. To leave a link out of the notice, pass `--exclude` (repeatable; also read from the project config) or run with a narrower root. An unsafe root — the filesystem root, an operating-system image, or a walk over the `VG_MAX_FILES` budget — already stops with that same guidance to narrow the path or add `--exclude`. - --- ## Extended Scanners diff --git a/src/core-open/utils/fs.ts b/src/core-open/utils/fs.ts index 915e88e5..5e19c4e3 100644 --- a/src/core-open/utils/fs.ts +++ b/src/core-open/utils/fs.ts @@ -17,6 +17,7 @@ import { UnsafeRootError, type WalkBudgetState, } from './root-safety.js'; +import { emitSkippedSymlinkNotice, rememberSkippedSymlink } from './skipped-symlinks.js'; const execFileAsync = promisify(execFile); @@ -490,6 +491,10 @@ export class FileCache { if (budgetError) return; budgetError = noteWalkEntry(budgetState); }; + // Symlinks are not followed. Dirent.isDirectory() is false for a link, so a + // link to a parent cannot re-enter this walk. One notice is printed after + // the walk finishes. The prelude count stays quiet so a scan says it once. + const skippedSymlinks: string[] = []; async function walk(dir: string, gitignoreLevels: GitignoreLevel[]) { if (budgetError) return; @@ -542,6 +547,15 @@ export class FileCache { if (isExcluded && isExcluded(relPath)) continue; if (budgetError) break; + if (e.isSymbolicLink()) { + // Ignored as a file or as a directory: the operator already asked to + // skip it, so it does not join the notice. + if (!(isGitignored(levels, absPath, false) || isGitignored(levels, absPath, true))) { + rememberSkippedSymlink(skippedSymlinks, relPath); + } + continue; + } + if (e.isDirectory()) { if (isSkippedDirName(e.name) || extraSkip.has(e.name)) continue; // Respect .gitignore (root + any nested ones seen so far) so @@ -577,6 +591,7 @@ export class FileCache { await walk(rootDir, []); if (budgetError) throw budgetError; + emitSkippedSymlinkNotice(skippedSymlinks); let totalDirs = 0; const rootNameIndex = new Map(); @@ -885,6 +900,10 @@ async function countTreeWithBudget( const relPath = path.relative(rootDir, absPath); if (isExcluded && isExcluded(relPath)) continue; + // Not followed. The indexing walk prints the notice; this count does not, + // so vg scan does not say the same thing twice. + if (e.isSymbolicLink()) continue; + if (e.isDirectory()) { if (isSkippedDirName(e.name) || extraSkip.has(e.name)) continue; if (isGitignored(levels, absPath, true)) continue; @@ -975,6 +994,8 @@ export async function countFilesInDir(dir: string, recursive = true): Promise[] = []; for (const e of entries) { + // Not followed. A link to a parent cannot re-enter this count. + if (e.isSymbolicLink()) continue; if (e.isDirectory()) { if (!recursive) continue; if (isSkippedDirName(e.name) || extraSkip.has(e.name)) continue; @@ -1009,6 +1030,8 @@ export async function bytesInDir(dir: string, recursive = true): Promise } const subs: Promise[] = []; for (const e of entries) { + // Not followed. A link to a parent cannot re-enter this size walk. + if (e.isSymbolicLink()) continue; if (e.isDirectory()) { if (!recursive) continue; if (isSkippedDirName(e.name) || extraSkip.has(e.name)) continue; @@ -1044,6 +1067,7 @@ export async function findFiles( budgetError = noteWalkEntry(budgetState); }; const results: string[] = []; + const skippedSymlinks: string[] = []; const cores = typeof os.availableParallelism === 'function' ? os.availableParallelism() : os.cpus().length || 4; const maxConcurrentReads = Math.max(8, Math.min(64, cores * 4)); const readDirSemaphore = new Semaphore(maxConcurrentReads); @@ -1061,6 +1085,10 @@ export async function findFiles( for (const e of entries) { if (budgetError) break; + if (e.isSymbolicLink()) { + rememberSkippedSymlink(skippedSymlinks, path.relative(rootDir, path.join(dir, e.name))); + continue; + } if (e.isDirectory()) { if (isSkippedDirName(e.name)) continue; note(); @@ -1081,6 +1109,7 @@ export async function findFiles( await walk(rootDir); if (budgetError) throw budgetError; + emitSkippedSymlinkNotice(skippedSymlinks); return results; } diff --git a/src/core-open/utils/skipped-symlinks.ts b/src/core-open/utils/skipped-symlinks.ts new file mode 100644 index 00000000..17e71d26 --- /dev/null +++ b/src/core-open/utils/skipped-symlinks.ts @@ -0,0 +1,65 @@ +// VENDORED from @vibgrate/core-open (packages/vibgrate-core-open) by +// scripts/vendor-core-open.mjs. Do not edit here — change the source package +// and re-run the vendor script. Apache-2.0. +import * as path from 'node:path'; + +/** + * How many root-relative paths one notice lists. Further links are a count. + * The walk sorts before capping, so the same tree always names the same paths. + */ +export const SKIPPED_SYMLINK_NOTICE_CAP = 5; + +function comparePath(a: string, b: string): number { + return a < b ? -1 : a > b ? 1 : 0; +} + +/** + * Root-relative POSIX path, or '' when `absPath` is the root or escapes it. + * Absolute paths are never returned. + */ +export function walkRelativePath(root: string, absPath: string): string { + const rel = path.relative(root, absPath); + if (!rel || rel === '.' || rel.startsWith('..')) return ''; + return rel.split(path.sep).join('/'); +} + +/** Record one skipped symlink. Empty and out-of-root paths are dropped. */ +export function rememberSkippedSymlink(bucket: string[], relPath: string): void { + const rel = relPath.split(path.sep).join('/'); + if (!rel || rel === '.' || rel.startsWith('..') || rel.startsWith('/')) return; + bucket.push(rel); +} + +/** + * One stderr line when a walk skips symlinks. `null` when `relPaths` is empty. + * Paths are de-duplicated, sorted, and capped. Absolute paths are omitted. + */ +export function formatSkippedSymlinkNotice(relPaths: readonly string[]): string | null { + const seen = new Set(); + const unique: string[] = []; + for (const raw of relPaths) { + const rel = raw.split('\\').join('/'); + if (!rel || rel === '.' || rel.startsWith('../') || rel === '..' || rel.startsWith('/')) continue; + if (/^[A-Za-z]:/.test(rel)) continue; + if (seen.has(rel)) continue; + seen.add(rel); + unique.push(rel); + } + unique.sort(comparePath); + if (unique.length === 0) return null; + const shown = unique.slice(0, SKIPPED_SYMLINK_NOTICE_CAP); + const extra = unique.length - shown.length; + const list = extra > 0 ? `${shown.join(', ')}, +${extra} more` : shown.join(', '); + const noun = unique.length === 1 ? 'symlink' : 'symlinks'; + return ( + `notice: skipped ${unique.length} ${noun} (${list}). ` + + 'vg does not follow symlinks. Point the root at the link target, or pass --exclude or a narrower root.' + ); +} + +/** Write {@link formatSkippedSymlinkNotice} to stderr. No-op when there is nothing to say. */ +export function emitSkippedSymlinkNotice(relPaths: readonly string[]): void { + const line = formatSkippedSymlinkNotice(relPaths); + if (!line) return; + process.stderr.write(`${line}\n`); +} diff --git a/src/engine/build.ts b/src/engine/build.ts index 12ea67d7..9252c40f 100644 --- a/src/engine/build.ts +++ b/src/engine/build.ts @@ -102,11 +102,6 @@ export interface BuildOptions { /** Resource-safeguard overrides (else VG_MAX_FILE_BYTES / VG_MAX_FILES / * VG_TSC_MAX_FILES / VG_MEMORY_BUDGET_MB env vars, else defaults). */ limits?: Partial; - /** - * Print discovery's skipped-symlink notice. Default true. - * `vg scan` sets this false: the scan walk prints the notice itself. - */ - symlinkNotice?: boolean; } /** Stat + content hash of one corpus file at build time. */ @@ -175,7 +170,6 @@ export async function buildGraph(options: BuildOptions): Promise { exclude, paths: options.paths, maxEntries: limits.maxFiles, - symlinkNotice: options.symlinkNotice, }); timer.end('discover'); diff --git a/src/engine/discover.ts b/src/engine/discover.ts index 915a52dd..c5113b9a 100644 --- a/src/engine/discover.ts +++ b/src/engine/discover.ts @@ -6,7 +6,7 @@ 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 { assertSafeWalkRoot, createWalkBudget, noteWalkEntry } from '../core-open/utils/root-safety.js'; -import { emitSkippedSymlinkNotice } from '../util/skipped-symlinks.js'; +import { emitSkippedSymlinkNotice } from '../core-open/utils/skipped-symlinks.js'; /** * Deterministic file discovery. @@ -167,12 +167,6 @@ export interface DiscoverOptions { * Default: `VG_MAX_FILES`, else 100000. */ maxEntries?: number; - /** - * Print the skipped-symlink notice on stderr. Default true. - * The walk never follows links either way. `vg scan` passes false - * when it builds a map, because the scan walk already printed one. - */ - symlinkNotice?: boolean; } export interface DiscoveredFile { @@ -255,16 +249,11 @@ export function discover(options: DiscoverOptions): DiscoveredFile[] { const found = new Map(); const budget = createWalkBudget(root, options.maxEntries); - // Links the walk refused to follow. Sorted later, when the notice is formatted. + // 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 noteSymlink = (rel: string): void => { - if (!rel || rel.startsWith('..')) return; - // Already ignored or excluded: the operator asked for this path to be skipped. - if (rootIg.ignores(rel) || rootIg.ignores(`${rel}/`)) return; - skippedSymlinks.push(rel); - }; - const considerFile = (abs: string): void => { const rel = toPosix(path.relative(root, abs)); if (rel.startsWith('..')) return; // outside root @@ -290,6 +279,11 @@ export function discover(options: DiscoverOptions): DiscoveredFile[] { 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); + continue; + } if (entry.isDirectory()) { if (isSkippedDirName(entry.name)) continue; if (rel && rootIg.ignores(`${rel}/`)) continue; @@ -301,28 +295,19 @@ export function discover(options: DiscoverOptions): DiscoveredFile[] { const over = noteWalkEntry(budget); if (over) throw over; considerFile(abs); - } else if (entry.isSymbolicLink()) { - // isDirectory() / isFile() do not follow links, so a symlink is - // neither branch above. Leave it unentered: a link that points - // at its parent would otherwise cycle. The target is indexed - // only when it is also a real path under the root. - noteSymlink(rel); } } }; - try { - for (const scope of scopeAbs) { - const stat = fs.statSync(scope); - if (stat.isDirectory()) { - assertSafeWalkRoot(scope); - walk(scope); - } else if (stat.isFile()) considerFile(scope); - } - } finally { - if (options.symlinkNotice !== false) emitSkippedSymlinkNotice(skippedSymlinks); + for (const scope of scopeAbs) { + const stat = fs.statSync(scope); + if (stat.isDirectory()) { + assertSafeWalkRoot(scope); + walk(scope); + } else if (stat.isFile()) considerFile(scope); } + emitSkippedSymlinkNotice(skippedSymlinks); return [...found.values()].sort((a, b) => (a.rel < b.rel ? -1 : a.rel > b.rel ? 1 : 0)); } diff --git a/src/engine/docs-ingest.ts b/src/engine/docs-ingest.ts index 21c7ff05..64da6fbb 100644 --- a/src/engine/docs-ingest.ts +++ b/src/engine/docs-ingest.ts @@ -408,6 +408,8 @@ export function discoverDocs(options: DiscoverDocsOptions): DiscoveredDoc[] { if (found.size >= maxFiles) return; const abs = path.join(dir, entry.name); const rel = toPosix(path.relative(root, abs)); + // Not followed. discover() prints the one notice for this tree. + if (entry.isSymbolicLink()) continue; if (entry.isDirectory()) { if (isSkippedDirName(entry.name)) continue; // Workflows / .github must be walked even if other tools ignore them diff --git a/src/reporting/commands/scan.ts b/src/reporting/commands/scan.ts index 75c114f3..f23006a0 100644 --- a/src/reporting/commands/scan.ts +++ b/src/reporting/commands/scan.ts @@ -17,7 +17,6 @@ import { parseExcludePatterns, loadConfig, findConfigFile, - isSkippedDirName, } from '../../core-open/index.js'; import { writeScanSummary } from '../scan-summary.js'; import { compareDriftBudget, evaluateConfigDriftBudget } from '../drift-budget-gate.js'; @@ -65,7 +64,6 @@ import { loadPackageVersionManifest, PackageManifestError } from '../package-ver import { runSecurityPacks, type SecurityRunResult } from '../../security/run-packs.js'; import { evaluateSecurityGate, lowestThreshold, parseFailOn } from '../../security/gate.js'; import { securityFindingRow, securityPacksLabel } from '../../core-open/formatters/text.js'; -import { collectScanSkippedSymlinks, emitSkippedSymlinkNotice } from '../../util/skipped-symlinks.js'; /** The security packs `--iac` requests from the Architecture module. */ const IAC_PACKS = ['iac-cis-v1'] as const; @@ -708,8 +706,6 @@ export const scanCommand = new Command('scan') const result = await buildGraph({ root: rootDir, exclude: opts.exclude, - // The scan walk already printed the skipped-symlink notice. - symlinkNotice: false, onParseProgress: (done, total) => report(done, total, 'parsing'), }); builtGraph = result.graph; @@ -764,13 +760,6 @@ export const scanCommand = new Command('scan') // Open base scan. The optional advanced-analysis hook is a no-op in this // open build, so the scan runs entirely on the open base engine. const advanced = await loadAdvancedScanHook(); - // The scan walk (core-open FileCache) does not follow symlinks: it - // recurses only when Dirent.isDirectory() is true. The notice lives - // here, not in that vendored file, so a later vendor sync cannot drop - // it. Excludes match the walk: config first, then CLI, de-duplicated. - const noticeConfig = await loadConfig(rootDir); - const noticeExclude = [...new Set([...(noticeConfig.exclude ?? []), ...(opts.exclude ?? [])])]; - emitSkippedSymlinkNotice(collectScanSkippedSymlinks(rootDir, noticeExclude, isSkippedDirName)); const artifact = await runCoreScan(rootDir, scanOpts, advanced); // The scan just built a code map (its `postScan` step). Start the local diff --git a/src/util/skipped-symlinks.ts b/src/util/skipped-symlinks.ts deleted file mode 100644 index b0a93dc5..00000000 --- a/src/util/skipped-symlinks.ts +++ /dev/null @@ -1,142 +0,0 @@ -import * as fs from 'node:fs'; -import * as path from 'node:path'; -import ignore, { type Ignore } from 'ignore'; -import { compileGlobs, gitignoreWithoutBlankLines } from '../core-open/utils/glob.js'; -import { info } from './output.js'; - -/** - * How many root-relative symlink paths a single notice lists. - * The rest are a count only, so a link farm cannot flood stderr. - */ -export const SKIPPED_SYMLINK_NOTICE_CAP = 5; - -const REMEDY = - 'Point the root at the link target, or pass --exclude, or use a narrower root.'; - -interface GitignoreLevel { - dir: string; - ig: Ignore; -} - -function comparePath(a: string, b: string): number { - return a < b ? -1 : a > b ? 1 : 0; -} - -/** Root-relative POSIX path. Absolute paths and `..` escapes are dropped. */ -export function rootRelativePosix(p: string): string | null { - const posix = p.split(path.sep).join('/').split('\\').join('/'); - if (!posix || posix === '.' || posix === '..' || posix.startsWith('../')) return null; - if (posix.startsWith('/') || /^[A-Za-z]:/.test(posix)) return null; - return posix; -} - -/** - * One stderr line for symlinks a walk did not follow. - * Paths are sorted, de-duplicated, and capped. `null` when there is nothing to say. - */ -export function formatSkippedSymlinkNotice(relPaths: readonly string[]): string | null { - const unique: string[] = []; - const seen = new Set(); - for (const raw of relPaths) { - const rel = rootRelativePosix(raw); - if (!rel || seen.has(rel)) continue; - seen.add(rel); - unique.push(rel); - } - unique.sort(comparePath); - if (unique.length === 0) return null; - const shown = unique.slice(0, SKIPPED_SYMLINK_NOTICE_CAP); - const extra = unique.length - shown.length; - const list = extra > 0 ? `${shown.join(', ')} (+${extra} more)` : shown.join(', '); - const noun = unique.length === 1 ? 'symlink' : 'symlinks'; - return `skipped ${unique.length} ${noun} (vg does not follow links): ${list}. ${REMEDY}`; -} - -/** Print {@link formatSkippedSymlinkNotice} on stderr. No-op when there are none. */ -export function emitSkippedSymlinkNotice(relPaths: readonly string[]): void { - const text = formatSkippedSymlinkNotice(relPaths); - if (!text) return; - info(text); -} - -function extendGitignoreLevels(dir: string, levels: GitignoreLevel[]): GitignoreLevel[] { - try { - const txt = fs.readFileSync(path.join(dir, '.gitignore'), 'utf8'); - const rules = gitignoreWithoutBlankLines(txt); - if (!rules) return levels; - return [...levels, { dir, ig: ignore().add(rules) }]; - } catch { - return levels; - } -} - -/** - * Same root-to-leaf `.gitignore` precedence as the scan walk: a deeper - * match wins, including a negation that un-ignores an ancestor rule. - */ -function isGitignored(levels: GitignoreLevel[], absPath: string, isDir: boolean): boolean { - if (levels.length === 0) return false; - let ignored = false; - for (const { dir, ig } of levels) { - const rel = path.relative(dir, absPath); - if (!rel || rel.startsWith('..')) continue; - const relPosix = rel.split(path.sep).join('/'); - const testPath = isDir ? `${relPosix}/` : relPosix; - const result = ig.test(testPath); - if (result.ignored) ignored = true; - else if (result.unignored) ignored = false; - } - return ignored; -} - -/** - * Symlinks the scan walk will not follow. - * - * Mirrors the scan walker's directory pruning (skip-dir names, nested - * `.gitignore`, config/`--exclude` globs) and reads each directory with - * `readdir({ withFileTypes: true })`. A symlink is not a directory and not - * a file under that API, so the real walk never enters it. This census - * does not either: it only records the link path. Lives outside - * `src/core-open` so a vendor sync that replaces that tree cannot drop the - * notice. - */ -export function collectScanSkippedSymlinks( - root: string, - exclude: readonly string[] | undefined, - isSkippedDirName: (name: string) => boolean, -): string[] { - const rootDir = path.resolve(root); - const excluded = compileGlobs([...(exclude ?? [])]); - const found: string[] = []; - - const walk = (dir: string, parentLevels: GitignoreLevel[]): void => { - let entries: fs.Dirent[]; - try { - entries = fs.readdirSync(dir, { withFileTypes: true }); - } catch { - return; - } - const levels = extendGitignoreLevels(dir, parentLevels); - for (const entry of entries) { - const abs = path.join(dir, entry.name); - const rel = path.relative(rootDir, abs); - if (!rel || rel.startsWith('..')) continue; - const relPosix = rel.split(path.sep).join('/'); - if (excluded?.(relPosix)) continue; - if (entry.isDirectory()) { - if (isSkippedDirName(entry.name)) continue; - if (isGitignored(levels, abs, true)) continue; - walk(abs, levels); - continue; - } - if (!entry.isSymbolicLink()) continue; - // A link the operator already ignored is not news. Check both shapes: - // gitignore treats `name` and `name/` differently. - if (isGitignored(levels, abs, false) || isGitignored(levels, abs, true)) continue; - found.push(relPosix); - } - }; - - walk(rootDir, []); - return found; -} diff --git a/test/symlink-walk.test.ts b/test/symlink-walk.test.ts index 8369e60b..e064feea 100644 --- a/test/symlink-walk.test.ts +++ b/test/symlink-walk.test.ts @@ -1,253 +1,189 @@ -import { spawnSync } from 'node:child_process'; +import { describe, it, expect, afterEach, vi } from 'vitest'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { afterEach, describe, expect, it, vi } from 'vitest'; -import { FileCache, findFiles, isSkippedDirName, quickTreeCount } from '../src/core-open/utils/fs.js'; import { discover } from '../src/engine/discover.js'; -import { - collectScanSkippedSymlinks, - emitSkippedSymlinkNotice, - formatSkippedSymlinkNotice, - SKIPPED_SYMLINK_NOTICE_CAP, -} from '../src/util/skipped-symlinks.js'; +import { FileCache } from '../src/core-open/utils/fs.js'; +import { runCoreScan } from '../src/core-open/index.js'; const NOTICE = - 'skipped 2 symlinks (vg does not follow links): alias.ts, nested/up. Point the root at the link target, or pass --exclude, or use a narrower root.'; + 'notice: skipped 3 symlinks (alias.ts, nested/cycle, via). vg does not follow symlinks. Point the root at the link target, or pass --exclude or a narrower root.'; function symlinksSupported(): boolean { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'vg-symlink-probe-')); try { - fs.symlinkSync('nowhere', path.join(dir, 'link'), 'file'); - return true; - } catch (err) { - const code = (err as NodeJS.ErrnoException).code; - if (code === 'EPERM' || code === 'ENOTSUP' || code === 'EACCES') return false; - throw err; + fs.writeFileSync(path.join(dir, 't'), 'x'); + try { + fs.symlinkSync('t', path.join(dir, 'l')); + return true; + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (code === 'EPERM' || code === 'ENOTSUP' || code === 'EACCES' || code === 'EINVAL') return false; + throw err; + } } finally { fs.rmSync(dir, { recursive: true, force: true }); } } -const SUPPORTED = symlinksSupported(); - -function norm(p: string): string { - return p.split(path.sep).join('/'); -} +const SYMLINKS = symlinksSupported(); /** - * Directory symlink `nested/up` -> parent, plus a file symlink `alias.ts`. - * A link inside `node_modules` must stay out of the notice (that directory - * is pruned before its children are read). + * Real files, a directory symlink to its parent, a directory symlink to a + * sibling, and a file symlink. Following `nested/cycle` would walk the parent + * again and not terminate. */ -function makeCycleFixture(): string { +function symlinkFixture(): string { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'vg-symlink-')); + fs.writeFileSync(path.join(root, 'keep.ts'), 'export const keep = 1;\n'); + fs.mkdirSync(path.join(root, 'hidden')); + fs.writeFileSync(path.join(root, 'hidden', 'secret.ts'), 'export const secret = 1;\n'); fs.mkdirSync(path.join(root, 'nested')); - fs.mkdirSync(path.join(root, 'node_modules', 'pkg'), { recursive: true }); - fs.writeFileSync(path.join(root, 'package.json'), JSON.stringify({ name: 'symlink-fixture', version: '1.0.0' })); - fs.writeFileSync(path.join(root, 'keep.ts'), 'export const n = 1;\n'); - fs.writeFileSync(path.join(root, 'nested', 'inside.ts'), 'export const n = 2;\n'); - fs.symlinkSync('..', path.join(root, 'nested', 'up'), 'dir'); - fs.symlinkSync('keep.ts', path.join(root, 'alias.ts'), 'file'); - fs.symlinkSync('keep.ts', path.join(root, 'node_modules', 'pkg', 'link.ts'), 'file'); + fs.symlinkSync('..', path.join(root, 'nested', 'cycle')); + fs.symlinkSync('hidden', path.join(root, 'via')); + fs.symlinkSync('keep.ts', path.join(root, 'alias.ts')); return root; } -function captureStderr(run: () => void): string { +function captureStderr(run: () => Promise | void): Promise { const chunks: string[] = []; - const spy = vi.spyOn(process.stderr, 'write').mockImplementation((chunk) => { - chunks.push(String(chunk)); + const spy = vi.spyOn(process.stderr, 'write').mockImplementation((chunk: string | Uint8Array) => { + chunks.push(typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString('utf8')); return true; }); - try { - run(); - } finally { - spy.mockRestore(); - } - return chunks.join(''); + return Promise.resolve(run()).finally(() => spy.mockRestore()).then(() => chunks.join('')); } -describe('skipped symlink notice', () => { - it('formats one stable line: sorted, capped, root-relative, silent when empty', () => { - expect(formatSkippedSymlinkNotice([])).toBeNull(); - expect(formatSkippedSymlinkNotice(['/tmp/abs', 'C:/abs', '../out'])).toBeNull(); - const once = formatSkippedSymlinkNotice(['nested/up', 'alias.ts', '/var/abs']); - const twice = formatSkippedSymlinkNotice(['alias.ts', 'nested/up']); - expect(once).toBe(NOTICE); - expect(twice).toBe(once); - - const many = Array.from({ length: SKIPPED_SYMLINK_NOTICE_CAP + 2 }, (_, i) => `s${i}.ts`); - const capped = formatSkippedSymlinkNotice([...many].reverse()); - expect(capped).toBe( - 'skipped 7 symlinks (vg does not follow links): s0.ts, s1.ts, s2.ts, s3.ts, s4.ts (+2 more). Point the root at the link target, or pass --exclude, or use a narrower root.', - ); - expect(formatSkippedSymlinkNotice(['only.ts'])).toBe( - 'skipped 1 symlink (vg does not follow links): only.ts. Point the root at the link target, or pass --exclude, or use a narrower root.', - ); - }); - - it('prints nothing when there are no symlinks', () => { - const text = captureStderr(() => emitSkippedSymlinkNotice([])); - expect(text).toBe(''); - }); -}); - -describe.skipIf(!SUPPORTED)('symlink cycles are not followed', () => { +describe.skipIf(!SYMLINKS)('vg build discovery and vg scan walk skip symlinks', () => { const dirs: string[] = []; afterEach(() => { while (dirs.length) fs.rmSync(dirs.pop()!, { recursive: true, force: true }); }); - function fixture(): string { - const root = makeCycleFixture(); + it('discover finishes, does not descend, and prints one stable stderr notice', async () => { + const root = symlinkFixture(); dirs.push(root); - return root; - } - - it('vg build discovery finishes, stays deterministic, and does not descend', () => { - const root = fixture(); - const stderr: string[] = []; - const spy = vi.spyOn(process.stderr, 'write').mockImplementation((chunk) => { - stderr.push(String(chunk)); + const stdout: string[] = []; + const outSpy = vi.spyOn(process.stdout, 'write').mockImplementation((chunk: string | Uint8Array) => { + stdout.push(typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString('utf8')); return true; }); + let first: string[] = []; let second: string[] = []; - try { + const stderr1 = await captureStderr(() => { first = discover({ root }).map((f) => f.rel); + }); + const stderr2 = await captureStderr(() => { second = discover({ root }).map((f) => f.rel); - } finally { - spy.mockRestore(); - } - expect(first).toEqual(['keep.ts', 'nested/inside.ts']); - expect(second).toEqual(first); - expect(first.some((rel) => rel === 'alias.ts' || rel.includes('nested/up') || rel.includes('node_modules'))).toBe(false); - const noticeLines = stderr.join('').split('\n').filter((line) => line.includes('does not follow links')); - expect(noticeLines).toEqual([NOTICE, NOTICE]); - expect(noticeLines.join('')).not.toContain(root); - }); - - it('does not print a notice when every symlink is excluded or gitignored', () => { - const root = fixture(); - const excluded = captureStderr(() => { - discover({ root, exclude: ['alias.ts', 'nested/up'] }); }); - expect(excluded).not.toContain('does not follow links'); + outSpy.mockRestore(); - fs.writeFileSync(path.join(root, '.gitignore'), 'alias.ts\nnested/up\n'); - const ignored = captureStderr(() => { - discover({ root }); + expect(first).toEqual(['hidden/secret.ts', 'keep.ts']); + expect(second).toEqual(first); + expect(first.some((rel) => rel.startsWith('via/') || rel.includes('cycle/'))).toBe(false); + expect(first).not.toContain('alias.ts'); + expect(stderr1).toBe(`${NOTICE}\n`); + expect(stderr2).toBe(stderr1); + expect(stderr1).not.toContain(root); + expect(stdout.join('')).not.toContain('symlink'); + }, 5_000); + + it('does not mention a symlink the operator already excluded', async () => { + const root = symlinkFixture(); + dirs.push(root); + const stderr = await captureStderr(() => { + const rels = discover({ root, exclude: ['alias.ts'] }).map((f) => f.rel); + expect(rels).toEqual(['hidden/secret.ts', 'keep.ts']); }); - expect(ignored).not.toContain('does not follow links'); + expect(stderr).toBe( + 'notice: skipped 2 symlinks (nested/cycle, via). vg does not follow symlinks. Point the root at the link target, or pass --exclude or a narrower root.\n', + ); + expect(stderr).not.toContain('alias.ts'); }); - it('can silence the notice without following the link', () => { - const root = fixture(); - const text = captureStderr(() => { - const rels = discover({ root, symlinkNotice: false }).map((f) => f.rel); - expect(rels).toEqual(['keep.ts', 'nested/inside.ts']); - }); - expect(text).not.toContain('does not follow links'); - }); + it('the scan walk finishes with the same files and the same notice', async () => { + const root = symlinkFixture(); + dirs.push(root); - it('the scan walk finishes, stays deterministic, and does not descend', async () => { - const root = fixture(); - const listed = (entries: { relPath: string }[]) => - entries.map((e) => norm(e.relPath)).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); + const read = async (): Promise<{ files: string[]; dirs: string[] }> => { + const entries = await new FileCache().walkDir(root); + const files = entries.filter((e) => e.isFile).map((e) => e.relPath.split(path.sep).join('/')).sort(); + const directories = entries.filter((e) => e.isDirectory).map((e) => e.relPath.split(path.sep).join('/')).sort(); + return { files, dirs: directories }; + }; + + let first: { files: string[]; dirs: string[] } | undefined; + let second: { files: string[]; dirs: string[] } | undefined; + const stderr1 = await captureStderr(async () => { + first = await read(); + }); + const stderr2 = await captureStderr(async () => { + second = await read(); + }); - const first = listed(await new FileCache().walkDir(root)); - const second = listed(await new FileCache().walkDir(root)); + expect(first).toEqual({ files: ['hidden/secret.ts', 'keep.ts'], dirs: ['hidden', 'nested'] }); expect(second).toEqual(first); - expect(first).toEqual(['keep.ts', 'nested', 'nested/inside.ts', 'package.json']); - expect(first.some((rel) => rel === 'alias.ts' || rel === 'nested/up' || rel.startsWith('nested/up/'))).toBe(false); - - const found = (await findFiles(root, (name) => name.endsWith('.ts'))) - .map((abs) => norm(path.relative(root, abs))) - .sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); - expect(found).toEqual(['keep.ts', 'nested/inside.ts']); - - const again = (await findFiles(root, (name) => name.endsWith('.ts'))) - .map((abs) => norm(path.relative(root, abs))) - .sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); - expect(again).toEqual(found); - - expect(await quickTreeCount(root)).toEqual({ totalFiles: 3, totalDirs: 1 }); - - const skipped = collectScanSkippedSymlinks(root, undefined, isSkippedDirName) - .slice() - .sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); - const skippedAgain = collectScanSkippedSymlinks(root, undefined, isSkippedDirName) - .slice() - .sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); - expect(skipped).toEqual(['alias.ts', 'nested/up']); - expect(skippedAgain).toEqual(skipped); - expect(formatSkippedSymlinkNotice(skipped)).toBe(NOTICE); - expect(NOTICE).not.toContain(root); - }); + expect(stderr1).toBe(`${NOTICE}\n`); + expect(stderr2).toBe(stderr1); + expect(stderr1).not.toContain(root); + }, 5_000); - it('the scan census honours nested gitignore and --exclude', () => { - const root = fixture(); - fs.writeFileSync(path.join(root, 'nested', '.gitignore'), 'up\n'); - expect(collectScanSkippedSymlinks(root, undefined, isSkippedDirName).slice().sort()).toEqual(['alias.ts']); - expect(collectScanSkippedSymlinks(root, ['alias.ts'], isSkippedDirName)).toEqual([]); + it('prints nothing when the tree has no symlinks', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'vg-nosym-')); + dirs.push(root); + fs.writeFileSync(path.join(root, 'keep.ts'), 'export const keep = 1;\n'); + const stderr = await captureStderr(() => { + expect(discover({ root }).map((f) => f.rel)).toEqual(['keep.ts']); + }); + expect(stderr).toBe(''); }); - it('vg build and vg scan keep JSON on stdout and exit 0', () => { - const root = fixture(); - const cli = path.resolve('src/cli.ts'); - const generatedAt = '2026-01-01T00:00:00.000Z'; - const run = (args: string[]) => { - const res = spawnSync(process.execPath, ['--import', 'tsx', cli, ...args], { - cwd: process.cwd(), - encoding: 'utf8', - env: { - ...process.env, - NO_COLOR: '1', - FORCE_COLOR: '0', - VIBGRATE_DSN: '', - VIBGRATE_NO_KERNEL: '1', - }, - timeout: 90_000, - }); - return { status: res.status, stdout: res.stdout ?? '', stderr: res.stderr ?? '' }; - }; + it('vg scan keeps JSON on stdout and writes no extra files', async () => { + const root = symlinkFixture(); + dirs.push(root); + fs.writeFileSync( + path.join(root, 'package.json'), + JSON.stringify({ name: 'symlink-fixture', version: '1.0.0' }), + ); + const before = fs.readdirSync(root).sort(); + const logs: string[] = []; + const logSpy = vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + logs.push(args.map((a) => String(a)).join(' ')); + }); + const stderrChunks: string[] = []; + const errSpy = vi.spyOn(process.stderr, 'write').mockImplementation((chunk: string | Uint8Array) => { + stderrChunks.push(typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString('utf8')); + return true; + }); - const buildArgs = [ - 'build', '-C', root, - '--offline', '--json', '--quiet', '--no-publish', '--no-warm', '--no-daemon', - '--generated-at', generatedAt, - ]; - const first = run(buildArgs); - expect(first.status, first.stderr).toBe(0); - expect(first.stdout).not.toContain('does not follow links'); - const built = JSON.parse(first.stdout) as { ok: boolean; artifacts: { graphPath: string } }; - expect(built.ok).toBe(true); - const graphPath = built.artifacts.graphPath; - const bytes = fs.readFileSync(graphPath); - const graph = JSON.parse(bytes.toString('utf8')) as { nodes: { file?: string }[] }; - const files = graph.nodes.map((n) => n.file).filter((f): f is string => typeof f === 'string'); - expect(files).toContain('keep.ts'); - expect(files).toContain('nested/inside.ts'); - expect(files.some((f) => f === 'alias.ts' || f.includes('nested/up') || f.includes('node_modules'))).toBe(false); - const buildNotices = first.stderr.split('\n').filter((line) => line.includes('does not follow links')); - expect(buildNotices).toEqual([NOTICE]); - expect(buildNotices[0]).not.toContain(root); - - const second = run(buildArgs); - expect(second.status, second.stderr).toBe(0); - expect(fs.readFileSync(graphPath).equals(bytes)).toBe(true); - - const scan = run([ - 'scan', root, - '--offline', '--format', 'json', '--no-graph', '--no-local-artifacts', '--quiet', '--no-daemon', - ]); - expect(scan.status, scan.stderr).toBe(0); - expect(scan.stdout).not.toContain('does not follow links'); - const report = JSON.parse(scan.stdout) as { projects?: unknown }; - expect(report).toBeTypeOf('object'); - const scanNotices = scan.stderr.split('\n').filter((line) => line.includes('does not follow links')); - expect(scanNotices).toEqual([NOTICE]); - expect(scanNotices[0]).not.toContain(root); - expect(fs.existsSync(path.join(root, '.vibgrate', 'scan_result.json'))).toBe(false); - }, 120_000); + const artifact = await runCoreScan(root, { + format: 'json', + concurrency: 1, + offline: true, + noLocalArtifacts: true, + quiet: true, + }); + logSpy.mockRestore(); + errSpy.mockRestore(); + + const stdout = logs.join('\n'); + const stderr = stderrChunks.join(''); + expect(artifact).toBeTruthy(); + expect(stdout).not.toContain('notice: skipped'); + expect(stdout).not.toContain(root); + const jsonText = logs.find((line) => line.trim().startsWith('{')); + expect(jsonText).toBeTruthy(); + const parsed = JSON.parse(jsonText!) as { schemaVersion?: string }; + expect(parsed.schemaVersion).toBe('1.0'); + expect(JSON.stringify(parsed)).not.toContain('via/secret.ts'); + expect(JSON.stringify(parsed)).not.toContain('nested/cycle'); + expect(JSON.stringify(parsed)).not.toContain('alias.ts'); + const notices = stderr.match(/notice: skipped \d+ symlinks/g) ?? []; + expect(notices).toEqual(['notice: skipped 3 symlinks']); + expect(stderr).toContain('alias.ts, nested/cycle, via'); + expect(stderr).not.toContain(root); + expect(fs.readdirSync(root).sort()).toEqual(before); + expect(fs.existsSync(path.join(root, '.vibgrate'))).toBe(false); + }, 60_000); });