diff --git a/CHANGELOG.md b/CHANGELOG.md index 941ec1600..6076e3fb9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,6 +76,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `codegraph upgrade` now upgrades a global npm install when run from your home folder, which holds npm's global folder on Windows and with tools like nvm: before, it ran `npm install` without `-g` there, which left the global copy on the old version and put a second copy in your home folder. A copy installed in a project is now upgraded in that project wherever you run the command, with a warning if npm left it on the old version, and a copy installed with pnpm, yarn or bun gets the command that upgrades it instead of npm. Thanks @timlincool for the report and @maxmilian for the first fix. (#2477) - `codegraph status` no longer says the index is up to date while indexed files are missing their symbols: it now names files the parser couldn't read and files stored without their symbols (which `codegraph sync` repairs), `status --json` counts both, and `codegraph files --json` lists each file's recorded errors. Thanks @lipchey for the report. (#2336) - Running `codegraph status` while `codegraph index` rebuilds a project no longer makes the rebuild fail with "database is locked": status now only reads the index, and it says when a rebuild is running in another process, in its output and as `rebuildInProgress` in `status --json`. Thanks @sealad886 for the report, and @danusha2345 and @KissFeng for the fixes. (#2913) +- `codegraph init` and `codegraph index` no longer report a file you deleted as one that "could not be read" while git still lists it, which it does until the deletion is staged: such a file is now skipped, and so is a symlink that points at nothing. A file that is there but can't be read is still reported. Thanks @xianzhi-xzw for the report and the first fix. (#2904) - In CFML, a `` or `` block the parser can't read no longer marks its whole file as contributing nothing to the graph: `codegraph init` no longer warns about the file, and `codegraph status` no longer lists it among files whose symbols are missing, since its component, functions and calls are all indexed. The file's recorded errors now name the block and its line instead. Thanks @HarryMuc for the report. (#2654) - In Erlang, a comment inside an argument or parameter list, like the one in `add_to_includes( % give access to .hrl in app's src/` before its two arguments, no longer counts as an argument. The call now links to the function of its real arity, and a function whose parameter list holds a comment keeps its own arity. Before, the comment raised the arity by one, so the call linked to nothing. Re-index Erlang projects after upgrading. Thanks @Dshuishui. (#1619) - Indexing large Python projects is much faster again and needs less memory: since 1.6.2, resolving Python references re-read source files over and over, so a project the size of CPython took several times as long to index. The graph it builds is unchanged. Thanks @bompus for the report. (#2332) diff --git a/__tests__/index-gone-tracked-files.test.ts b/__tests__/index-gone-tracked-files.test.ts new file mode 100644 index 000000000..1c302a732 --- /dev/null +++ b/__tests__/index-gone-tracked-files.test.ts @@ -0,0 +1,132 @@ +/** + * A full index skips a file the scan lists but the disk no longer has (#2904). + * + * `git ls-files` keeps listing a tracked file deleted from the working tree + * until the deletion is staged, so the scan hands the reader a path with + * nothing behind it. That is not a file that "could not be read": the run + * skips it, still accounts for it (an unaccounted file reads as a PARTIAL + * index), and reports only listed files that are there but cannot be read. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as cp from 'child_process'; +import * as fs from 'fs'; +import * as fsp from 'fs/promises'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import type { IndexResult } from '../src/extraction'; + +// A pass-through `stat`, so one test can make a present file unreadable on +// every platform (chmod cannot do that on Windows). +vi.mock('fs/promises', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, stat: vi.fn(actual.stat) }; +}); + +describe('full index of files git lists but the disk lacks (#2904)', () => { + let root: string; + let cg: CodeGraph | undefined; + const git = (...args: string[]) => + cp.execFileSync('git', args, { cwd: root, encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'] }); + const write = (rel: string, name = path.basename(rel, '.py')) => { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), `def ${name}():\n return 1\n`); + }; + const commit = () => { git('add', '-A'); git('commit', '-qm', 'fixture'); }; + const readErrors = (r: IndexResult) => r.errors.filter((e) => e.code === 'read_error').map((e) => e.filePath); + const metadata = (key: string): string | null => (cg as any).queries.getMetadata(key); + const indexedPaths = (): string[] => (cg as any).queries.getAllFilePaths().sort(); + + beforeEach(() => { + root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'cg-gone-tracked-'))); + git('init', '-q'); + git('config', 'user.email', 'test@example.invalid'); + git('config', 'user.name', 'Test'); + git('config', 'commit.gpgsign', 'false'); + fs.writeFileSync(path.join(root, '.gitignore'), '.codegraph/\n'); + }); + + afterEach(() => { + cg?.close(); + cg = undefined; + fs.rmSync(root, { recursive: true, force: true }); + }); + + /** A clean run over exactly `indexed`: nothing errored, unread or unaccounted. */ + const expectCleanRun = (result: IndexResult, indexed: string[]) => { + expect(readErrors(result)).toEqual([]); + expect(result.filesErrored).toBe(0); + expect(result.filesIndexed).toBe(indexed.length); + expect(result.errors.filter((e) => e.code === 'index_partial')).toEqual([]); + expect(metadata('index_state')).toBe('complete'); + expect(indexedPaths()).toEqual(indexed); + }; + + it.each(['committed', 'staged'] as const)('skips a %s file deleted with the deletion unstaged', async (state) => { + write('keep.py'); + write('gone.py'); + if (state === 'committed') commit(); else git('add', '-A'); + fs.unlinkSync(path.join(root, 'gone.py')); + expect(git('ls-files')).toContain('gone.py'); // the premise: git still lists it + + cg = CodeGraph.initSync(root); + expectCleanRun(await cg.indexAll(), ['keep.py']); + // A clean run records the commit it indexed, so `status` keeps its git + // fast path instead of falling back to a full scan. + if (state === 'committed') expect(metadata('indexed_at_commit')).toBe(git('rev-parse', 'HEAD').trim()); + expect(cg.getChangedFiles()).toEqual({ added: [], modified: [], removed: [] }); + }); + + it('skips a tracked file whose folder is now a file', async () => { + write('keep.py'); + write('previous/gone.py'); + commit(); + fs.rmSync(path.join(root, 'previous'), { recursive: true }); + fs.writeFileSync(path.join(root, 'previous'), 'a file where the folder was\n'); + expect(git('ls-files')).toContain('previous/gone.py'); + + cg = CodeGraph.initSync(root); + expectCleanRun(await cg.indexAll(), ['keep.py']); + }); + + it('skips a symlink that points at nothing, as the directory walk does', async (ctx) => { + write('keep.py'); + // File symlinks need Developer Mode or elevation on Windows. + try { fs.symlinkSync('missing-target.py', path.join(root, 'broken.py')); } catch { ctx.skip(); } + commit(); + + cg = CodeGraph.initSync(root); + expectCleanRun(await cg.indexAll(), ['keep.py']); + }); + + it('still reports a listed file that is there but cannot be read', async () => { + write('keep.py'); + write('locked.py'); + write('gone.py'); + commit(); + fs.unlinkSync(path.join(root, 'gone.py')); + const stat = vi.mocked(fsp.stat); + const realStat = stat.getMockImplementation()!; + stat.mockImplementation(((p: fs.PathLike, ...rest: unknown[]) => { + if (String(p).endsWith('locked.py')) { + return Promise.reject(Object.assign(new Error(`EACCES: permission denied, stat '${String(p)}'`), { code: 'EACCES' })); + } + return (realStat as (...a: unknown[]) => Promise)(p, ...rest); + }) as typeof fsp.stat); + try { + cg = CodeGraph.initSync(root); + const result = await cg.indexAll(); + expect(readErrors(result)).toEqual(['locked.py']); + expect(result.filesErrored).toBe(1); + expect(result.filesIndexed).toBe(1); + + // The single-file path draws the same line. + const files = await cg.indexFiles(['gone.py', 'locked.py']); + expect(readErrors(files)).toEqual(['locked.py']); + expect(files.filesErrored).toBe(1); + expect(files.filesSkipped).toBe(1); + } finally { + stat.mockImplementation(realStat); + } + }); +}); diff --git a/__tests__/security.test.ts b/__tests__/security.test.ts index 6c128b4c3..1048c7613 100644 --- a/__tests__/security.test.ts +++ b/__tests__/security.test.ts @@ -312,6 +312,19 @@ describe('Symlink escape prevention (#527)', () => { } }); + // POSIX reports ENOTDIR, not ENOENT, for a path under a FILE (a folder git + // still lists that is now a file, #2904). It is a missing leaf like any other: + // allowed inside the root, still refused below a link out of it. + it('judges a path under an existing file like any missing leaf', () => { + expect(validatePathWithinRoot(root, 'src/in.ts/child.ts')).toBe(path.join(root, 'src', 'in.ts', 'child.ts')); + expect(validatePathWithinRoot(root, 'src/in.ts/child.ts', { allowSymlinkEscape: true })).not.toBeNull(); + }); + + it('rejects a path under an in-repo symlink to an out-of-root FILE', () => { + if (!link(path.join(root, 'escape'), path.join(outside, 'pkg', 'secret.txt'))) return; + expect(validatePathWithinRoot(root, 'escape/child.txt')).toBeNull(); + }); + it('rejects every path under a root that does not exist', () => { expect(validatePathWithinRoot(path.join(root, 'gone'), 'src/in.ts')).toBeNull(); }); diff --git a/src/extraction/index.ts b/src/extraction/index.ts index f437cfd09..c93d26b00 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -28,7 +28,7 @@ import { detectLanguage, isSourceFile, isLanguageSupported, isFileLevelOnlyLangu import { loadExtensionOverrides, loadIncludeIgnoredPatterns, loadExcludePatterns, loadIncludePatterns, PROJECT_CONFIG_FILENAME } from '../project-config'; import { isCodeGraphDataDir } from '../directory'; import { logDebug, logWarn } from '../errors'; -import { validatePathWithinRoot, normalizePath } from '../utils'; +import { validatePathWithinRoot, normalizePath, isMissingPathError } from '../utils'; import ignore, { Ignore } from 'ignore'; import { detectFrameworks, getFrameworkResolver } from '../resolution/frameworks'; import { declaredDependencies } from '../resolution/frameworks/package-deps'; @@ -2579,6 +2579,14 @@ export class ExtractionOrchestrator { const content = bytes.toString('utf-8'); return { filePath: fp, content, stats, error: null as Error | null }; } catch (err) { + // Nothing at the path is not a file that could not be read (#2904): + // git keeps listing a tracked file deleted from the working tree + // until the deletion is staged, a symlink can point at nothing (the + // directory walk skips one quietly), and a file can go between the + // scan and this read. Skip it. EACCES, EIO and the like still report. + if (isMissingPathError(err)) { + return { filePath: fp, content: null as string | null, stats: null as fs.Stats | null, error: null as Error | null, gone: true }; + } return { filePath: fp, content: null as string | null, stats: null as fs.Stats | null, error: err as Error }; } }) @@ -2586,7 +2594,7 @@ export class ExtractionOrchestrator { // Dispatch each readable file into the bounded parse window; the window // stores results on the main thread as they arrive. - for (const { filePath, content, stats, error, skipped } of fileContents) { + for (const { filePath, content, stats, error, skipped, gone } of fileContents) { if (signal?.aborted) { aborted = true; break; } if (skipped) { @@ -2596,6 +2604,15 @@ export class ExtractionOrchestrator { continue; } + if (gone) { + // Listed by the scan but not on disk: skipped, stored as nothing. It + // still counts toward the tallies, or the run would read as PARTIAL. + processed++; + filesSkipped++; + onProgress?.({ phase: 'parsing', current: processed, total }); + continue; + } + if (error || content === null || stats === null) { processed++; filesErrored++; @@ -2921,6 +2938,10 @@ export class ExtractionOrchestrator { } content = read.bytes === null ? oversizeStamp(stats.size) : read.bytes.toString('utf8'); } catch (error) { + // Nothing at the path: nothing to index, and not a read failure (#2904). + if (isMissingPathError(error)) { + return { nodes: [], edges: [], unresolvedReferences: [], errors: [], durationMs: 0 }; + } return { nodes: [], edges: [], diff --git a/src/utils.ts b/src/utils.ts index d817003c8..0a7bca020 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -124,6 +124,16 @@ export function lexicalPathWithinRoot(projectRoot: string, filePath: string): st return isWithinDir(resolved, path.resolve(projectRoot)) ? resolved : null; } +/** + * Whether a filesystem error means nothing is at the path: ENOENT, or ENOTDIR, + * which POSIX reports instead when a folder on the way is a file (Windows + * reports ENOENT for both). + */ +export function isMissingPathError(err: unknown): boolean { + const code = (err as NodeJS.ErrnoException | null)?.code; + return code === 'ENOENT' || code === 'ENOTDIR'; +} + /** * Validate that a file path stays within the project root, resolving symlinks. * @@ -188,7 +198,8 @@ export function validatePathWithinRoot( // nearest existing ancestor before allowing a write path: otherwise // `project/.codegraph/ui -> /outside` plus a missing `trails/` directory // passes the lexical check and mkdir follows the symlink out of the root. - if ((err as NodeJS.ErrnoException).code !== 'ENOENT') return null; + // A path under a file (ENOTDIR on POSIX) is a missing leaf too. + if (!isMissingPathError(err)) return null; } let ancestor = path.dirname(resolved); @@ -200,7 +211,7 @@ export function validatePathWithinRoot( } return null; } catch (err) { - if ((err as NodeJS.ErrnoException).code !== 'ENOENT') return null; + if (!isMissingPathError(err)) return null; const parent = path.dirname(ancestor); if (parent === ancestor) return null; ancestor = parent;