diff --git a/src/live-worktree.ts b/src/live-worktree.ts index c7a688d4..7a142320 100644 --- a/src/live-worktree.ts +++ b/src/live-worktree.ts @@ -31,7 +31,7 @@ * @module live-worktree */ -import { promises as fs } from 'node:fs'; +import { constants, promises as fs } from 'node:fs'; import { basename, dirname, join } from 'node:path'; import { probePathKind } from './utils/bounded-path-probe.js'; @@ -67,10 +67,11 @@ async function classifyDotGit(dir: string): Promise<'worktree' | 'stop' | 'none' if (kind !== 'file') return 'stop'; let handle: fs.FileHandle | undefined; try { - // lstat, so a `.git` that is a symlink or a FIFO is refused before anything opens it: - // opening a FIFO waits for a writer and would hold a threadpool worker for good. - if (!(await fs.lstat(dotGit)).isFile()) return 'stop'; - handle = await fs.open(dotGit, 'r'); + // The probe follows symlinks and the file can change after it, so the open itself refuses: + // O_NOFOLLOW a symlink, and O_NONBLOCK a FIFO, which would otherwise wait for a writer and + // hold a threadpool worker for good. Whatever it did open is then judged on the handle. + handle = await fs.open(dotGit, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK); + if (!(await handle.stat()).isFile()) return 'stop'; const { buffer, bytesRead } = await handle.read(Buffer.alloc(MAX_DOT_GIT_FILE_BYTES), 0, MAX_DOT_GIT_FILE_BYTES, 0); return isLinkedWorktreeGitFile(buffer.toString('utf8', 0, bytesRead)) ? 'worktree' : 'stop'; } catch { diff --git a/test/live-worktree.test.ts b/test/live-worktree.test.ts index e3cfe071..ab29c206 100644 --- a/test/live-worktree.test.ts +++ b/test/live-worktree.test.ts @@ -7,10 +7,10 @@ * file. Runs against a real filesystem, and one case against a worktree real git made. */ import { execFileSync } from 'node:child_process'; -import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { constants, mkdirSync, mkdtempSync, promises as fsPromises, rmSync, symlinkSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { findLiveWorktree, isLinkedWorktreeGitFile } from '../src/live-worktree.js'; describe('isLinkedWorktreeGitFile', () => { @@ -100,6 +100,28 @@ describe('findLiveWorktree', () => { expect(await findLiveWorktree(repo, piped)).toBeUndefined(); }); + it('refuses a FIFO swapped in after the probe, without waiting for a writer', async () => { + const swapped = join(repo, 'swapped'); + mkdirSync(swapped); + execFileSync('mkfifo', [join(swapped, 'fifo')]); + const flagsSeen: unknown[] = []; + const realOpen = fsPromises.open.bind(fsPromises); + const spy = vi.spyOn(fsPromises, 'open').mockImplementationOnce((_path, flags) => { + flagsSeen.push(flags); + // The test's own open never blocks, so a change that loses the flag fails on the assert below. + return realOpen(join(swapped, 'fifo'), constants.O_RDONLY | constants.O_NONBLOCK); + }); + try { + // The probe and the open disagree here, which is what a swap between the two looks like. + writeFileSync(join(swapped, '.git'), 'gitdir: /home/me/repo/.git/worktrees/swapped\n'); + expect(await findLiveWorktree(repo, swapped)).toBeUndefined(); + } finally { + spy.mockRestore(); + } + expect(Number(flagsSeen[0]) & constants.O_NONBLOCK).toBeTruthy(); + expect(Number(flagsSeen[0]) & constants.O_NOFOLLOW).toBeTruthy(); + }); + it('answers nothing without an absolute reading', async () => { expect(await findLiveWorktree(repo, undefined)).toBeUndefined(); expect(await findLiveWorktree(repo, '')).toBeUndefined();