mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 18:39:33 +01:00
fix(files): refuse a .git swapped for a FIFO or symlink at open
classifyDotGit() checked with lstat and then opened by name, so a FIFO swapped in between the two blocked a threadpool worker. The open itself now refuses both, and the handle is judged. Reported on #772. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
9c39b2a9cc
commit
0ec3a37bdd
2 files changed
+30
-7
No files matched your search
@@ -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 {
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in new issue
Block a user