From 1977f4ffc6a4373557ed8a06095002e21a421cf3 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Mon, 2 Nov 2026 18:56:28 -0600 Subject: [PATCH] Restore the fresh recursive lstat in project inspectGit (Vikunja #780) R1 passed the parent's stat of a listed directory into the recursive walk, so a recursive entry no longer took its own lstat. A directory replaced by a symlink after that stat passed the stale directory check and readdirSync followed the link; a directory replaced by a regular file leaked a raw ENOTDIR instead of refusing with project-repository. The walk now takes a root flag. The .git root keeps its raw lstat and its own error; every recursive entry takes a fresh lstat through statEntry, so a vanished entry still maps ENOENT to 'git metadata changed during inspection' and a replaced one refuses with 'expected local .git directory'. The mapper is unchanged: ENOENT only. Two deterministic tests replace a directory under .git/objects inside the lstat call that stats it as a listed entry, after the real stat is taken: once with a symlink to a fixture-only directory outside the repo, once with a regular file. Both refuse with project-repository. --- packages/project/src/git.mjs | 11 +++++--- packages/project/tests/read.test.mjs | 39 ++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/packages/project/src/git.mjs b/packages/project/src/git.mjs index 1808652df..a1dca1faf 100644 --- a/packages/project/src/git.mjs +++ b/packages/project/src/git.mjs @@ -24,17 +24,20 @@ export async function inspectGit(repo) { ? new ProjectError('project-repository', 'git metadata changed during inspection') : e; const statEntry = path => { try { return lstatSync(path); } catch (e) { throw changed(e); } }; const namesIn = directory => { try { return readdirSync(directory); } catch (e) { throw changed(e); } }; - // The root is not a listed name: a missing .git keeps its own error, as before. - const walk = (directory, st = lstatSync(directory)) => { + // Every directory takes a fresh lstat when the walk enters it, so one replaced by a link or a + // file after its listed stat still refuses. The root is not a listed name: a missing .git + // keeps its own error, as before. + const walk = (directory, root = false) => { + const st = root ? lstatSync(directory) : statEntry(directory); if (!st.isDirectory() || st.isSymbolicLink()) throw new ProjectError('project-repository', 'expected local .git directory'); for (const name of namesIn(directory)) { const path = join(directory, name), entry = statEntry(path); - if (entry.isDirectory()) walk(path, entry); + if (entry.isDirectory()) walk(path); else if (!entry.isFile() || entry.nlink !== 1) throw new ProjectError('project-repository', 'git metadata must contain regular private files'); } }; - walk(join(repo, '.git')); + walk(join(repo, '.git'), true); for (const name of ['commondir', 'objects/info/alternates', 'objects/info/http-alternates']) { try { lstatSync(join(repo, '.git', name)); } catch (e) { if (e.code === 'ENOENT') continue; throw e; } diff --git a/packages/project/tests/read.test.mjs b/packages/project/tests/read.test.mjs index 5a0299867..5066aba6a 100644 --- a/packages/project/tests/read.test.mjs +++ b/packages/project/tests/read.test.mjs @@ -134,6 +134,45 @@ test('a git metadata entry that vanishes during inspection refuses as project-re assert.equal(git(repo, 'rev-parse', 'HEAD'), commit); }); +// Row 780 R2: the walk stats a listed directory, then must stat it again when it enters it. The +// test replaces fs.lstatSync for one path and swaps the directory for something else inside the +// call that stats it as a listed entry, after the real stat is taken. A walk that trusts that +// stale stat would read through a link or leak ENOTDIR; the fresh stat refuses both. +async function replaceListedDirectory(t, replace) { + const { context, repo, commit, root } = await fixture(t); + const directory = join(repo, '.git/objects/tmp_dir_replaced'); + const outside = join(root, 'outside'); + mkdirSync(directory); mkdirSync(outside); writeFileSync(join(outside, 'private'), 'x'); + assert.equal((await api.readSpec(context)).commit, commit, 'control: the extra directory alone does not refuse'); + const real = fs.lstatSync; + let armed = true; + fs.lstatSync = function (path, ...rest) { + if (!armed || String(path) !== directory) return real.call(this, path, ...rest); + armed = false; + const stat = real.call(this, path, ...rest); + fs.rmdirSync(directory); replace(directory, outside); + return stat; + }; + syncBuiltinESMExports(); + try { + await assert.rejects(api.readSpec(context), error => { + assert.equal(error.code, 'project-repository', error.message); + assert.match(error.message, /expected local \.git directory/); + return true; + }); + } finally { fs.lstatSync = real; syncBuiltinESMExports(); } + assert.equal(armed, false, 'the listed directory was replaced during inspection'); + assert.equal(git(repo, 'rev-parse', 'HEAD'), commit); +} + +test('a git metadata directory replaced by a symlink after its listed stat refuses, never reads through it', async t => { + await replaceListedDirectory(t, (directory, outside) => symlinkSync(outside, directory)); +}); + +test('a git metadata directory replaced by a file after its listed stat refuses, never raw ENOTDIR', async t => { + await replaceListedDirectory(t, directory => writeFileSync(directory, 'x')); +}); + test('committed UTF-8 BOM, Unicode and trailing newlines are returned without normalization', async t => { const { context, save } = await fixture(t); const value = '\uFEFF# North star 🧭\r\n\r\n';