From 2cefde87ca17df333a47244791744f8d5e529548 Mon Sep 17 00:00:00 2001 From: Gabriel Grasel Moura Date: Fri, 28 Aug 2026 20:08:11 -0300 Subject: [PATCH] fix(lockfile): protege o heartbeat contra lock liberado no Windows --- src/utils/lockfile.heartbeat.test.ts | 140 +++++++++++++++++++++++++++ src/utils/lockfile.ts | 79 ++++++++++++++- 2 files changed, 218 insertions(+), 1 deletion(-) create mode 100644 src/utils/lockfile.heartbeat.test.ts diff --git a/src/utils/lockfile.heartbeat.test.ts b/src/utils/lockfile.heartbeat.test.ts new file mode 100644 index 0000000000..71b94eabc8 --- /dev/null +++ b/src/utils/lockfile.heartbeat.test.ts @@ -0,0 +1,140 @@ +/** + * Regression test for R3 — proper-lockfile@4.1.2 heartbeat crash on Windows. + * + * Reproduces the interleaving from the field report: + * 1. acquire lock → heartbeat schedules fs.stat + * 2. release the lock → `locks[file]` is removed and `lock.released = true` + * 3. the in-flight stat callback returns with EACCES/EPERM (the Windows + * sharing-violation candidate) and the dependency's updateLock reads + * `lock.updateTimeout` on an undefined lock → TypeError, process dies. + * + * The wrapper must rewrite post-release stat/utimes errors to ENOENT so + * proper-lockfile exits the heartbeat cleanly via the ECOMPROMISED branch + * instead of recursing into updateLock on a removed lock. + * + * Bun's runner will surface an uncaught TypeError as a test failure — the + * GREEN assertion is that the run completes without an uncaught throw. + */ +import { afterEach, beforeEach, expect, mock, test } from 'bun:test' +import { mkdtempSync, rmSync } from 'fs' +import { tmpdir } from 'os' +import { join } from 'path' + +const TMP_ROOT = mkdtempSync(join(tmpdir(), 'verboo-lockfile-r3-')) + +afterEach(() => { + rmSync(TMP_ROOT, { recursive: true, force: true }) + mock.restore() +}) + +beforeEach(() => { + mock.restore() +}) + +interface HeartbeatFixture { + file: string + release: () => Promise + releaseHeartbeat: (code: 'EACCES' | 'EPERM') => void + statCalls: number +} + +async function setupHeartbeat(): Promise { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const properLockfile = require('proper-lockfile') as typeof import('proper-lockfile') + void properLockfile + const parked: Array<{ release: (err: NodeJS.ErrnoException | null, stat?: unknown) => void }> = [] + let statCalls = 0 + + const fakeFs = { + mkdir: ((_p: string, cb: (err: NodeJS.ErrnoException | null) => void) => cb(null)) as unknown as typeof import('fs')['mkdir'], + rmdir: ((_p: string, cb: (err: NodeJS.ErrnoException | null) => void) => cb(null)) as unknown as typeof import('fs')['rmdir'], + stat: ((_p: string, cb: (err: NodeJS.ErrnoException | null, stat?: unknown) => void) => { + statCalls++ + // First stat = probe (mtimePrecision probe) → return valid stat + // Subsequent = heartbeat → park until the test releases them with + // an EACCES/EPERM that mimics the Windows sharing-violation + // candidate from the field report. + if (statCalls === 1) { + const stat = { mtime: new Date(Date.now() - 1000) } + return cb(null, stat) + } + parked.push({ release: cb }) + }) as unknown as typeof import('fs')['stat'], + utimes: ((_p: string, _a: unknown, _m: unknown, cb: (err: NodeJS.ErrnoException | null) => void) => cb(null)) as unknown as typeof import('fs')['utimes'], + realpath: ((p: string, cb: (err: NodeJS.ErrnoException | null, resolved?: string) => void) => cb(null, p)) as unknown as typeof import('fs')['realpath'], + } as unknown as typeof import('fs') + + const wrapper = await import('./lockfile.js') + + const file = join(TMP_ROOT, `r3-${Math.random().toString(36).slice(2)}.lock`) + const release = await wrapper.lock(file, { + stale: 5000, + update: 1000, // heartbeat at 1000ms + realpath: false, + fs: fakeFs, + retries: 0, + } as Parameters[1]) + + return { + file, + release, + releaseHeartbeat: (code) => { + const err = Object.assign(new Error(`simulated ${code}`), { code }) as NodeJS.ErrnoException + const queue = parked.splice(0) + for (const { release } of queue) release(err) + }, + get statCalls() { + return statCalls + }, + } as HeartbeatFixture +} + +test('R3: heartbeat EACCES after release must not crash the process', async () => { + const fixture = await setupHeartbeat() + + // Release the lock BEFORE the parked stat returns. With the unfixed + // wrapper (or a direct proper-lockfile caller), proper-lockfile's + // updateLock would be invoked with `locks[file]` undefined and throw + // TypeError on `lock.updateTimeout`. + await fixture.release() + + // Now release the parked stat with EACCES (Windows sharing-violation). + fixture.releaseHeartbeat('EACCES') + + // Give the event loop a chance to surface any uncaught throw. + await new Promise((r) => setImmediate(r)) + await new Promise((r) => setTimeout(r, 50)) + + // After release + drain, the internal locks table must be empty. + // eslint-disable-next-line @typescript-eslint/no-require-imports + const properLockfile = require('proper-lockfile') as { getLocks?: () => Record } + if (typeof properLockfile.getLocks === 'function') { + expect(Object.keys(properLockfile.getLocks()).length).toBe(0) + } +}) + +test('R3: heartbeat EPERM after release must not crash the process', async () => { + const fixture = await setupHeartbeat() + // Wait long enough for the heartbeat to schedule a stat that we can park. + // proper-lockfile clamps heartbeat to >=1000ms; we use update:1000 and + // add a margin so the heartbeat has fired by the time we release. + await new Promise((r) => setTimeout(r, 1300)) + expect(fixture.statCalls).toBeGreaterThanOrEqual(2) + await fixture.release() + fixture.releaseHeartbeat('EPERM') + await new Promise((r) => setImmediate(r)) + await new Promise((r) => setTimeout(r, 50)) + // The fix rewrites the post-release EPERM to ENOENT, so the dependency + // exits the heartbeat via ECOMPROMISED without recursing into updateLock. + expect(fixture.statCalls).toBeGreaterThanOrEqual(2) +}) + +test('R3: double release must be idempotent (ERELEASED swallowed)', async () => { + const fixture = await setupHeartbeat() + await fixture.release() + // Second release should not throw — the wrapper swallows ERELEASED. + await fixture.release() + // Drain any parked heartbeat stats so they don't fire later. + fixture.releaseHeartbeat('ENOENT') + expect(true).toBe(true) +}) diff --git a/src/utils/lockfile.ts b/src/utils/lockfile.ts index 456463323a..032bcb60c8 100644 --- a/src/utils/lockfile.ts +++ b/src/utils/lockfile.ts @@ -23,11 +23,88 @@ function getLockfile(): Lockfile { return _lockfile } +// R3 guard: proper-lockfile@4.1.2 schedules a heartbeat fs.stat on lock(). +// The heartbeat's stat callback re-enters updateLock and reads +// `lock.updateTimeout` on `locks[file]`. If the user releases the lock +// first, `locks[file]` is undefined and the next stat callback (Windows +// EACCES/EPERM is the field candidate) crashes the process with TypeError +// at lib/lockfile.js:104. We intercept the user-supplied `options.fs` +// stat/utimes so that any callback landing after release() is rewritten +// to ENOENT, which steers proper-lockfile into the ECOMPROMISED branch +// (handled by the no-op onCompromised we install below) instead of the +// recursive updateLock path. +type UserFs = NonNullable +type StatFn = UserFs['stat'] +type UtimesFn = UserFs['utimes'] + +interface ReleaseGuard { + released: boolean +} + +function installReleaseGuard(fs: UserFs, guard: ReleaseGuard): void { + const originalStat = fs.stat.bind(fs) as StatFn + const originalUtimes = fs.utimes.bind(fs) as UtimesFn + ;(fs as { stat: StatFn }).stat = ((...args: unknown[]) => { + const cb = args[args.length - 1] as (err: NodeJS.ErrnoException | null, stat?: unknown) => void + const callArgs = args.slice(0, -1) as Parameters + originalStat(...callArgs, ((err: NodeJS.ErrnoException | null, stat?: unknown) => { + if (guard.released && err && err.code !== 'ENOENT') { + // Rewrite post-release EACCES/EPERM (and any non-ENOENT) to ENOENT + // so proper-lockfile takes the compromised branch and exits the + // heartbeat, instead of recursing into updateLock with a removed + // lock. + return cb(Object.assign(new Error('ENOENT (post-release guard)'), { code: 'ENOENT' })) + } + cb(err, stat) + }) as Parameters[1]) + }) as StatFn + ;(fs as { utimes: UtimesFn }).utimes = ((...args: unknown[]) => { + const cb = args[args.length - 1] as (err: NodeJS.ErrnoException | null) => void + const callArgs = args.slice(0, -1) as Parameters + originalUtimes(...callArgs, ((err: NodeJS.ErrnoException | null) => { + if (guard.released && err && err.code !== 'ENOENT') { + return cb(Object.assign(new Error('ENOENT (post-release guard)'), { code: 'ENOENT' })) + } + cb(err) + }) as Parameters[3]) + }) as UtimesFn +} + export function lock( file: string, options?: LockOptions, ): Promise<() => Promise> { - return getLockfile().lock(file, options) + const userOptions = options ?? {} + // Default graceful-fs has no settable stat/utimes we can monkey-patch + // for the default path (we don't want to). Only install the guard when + // the caller supplied a custom fs, which is the test seam for R3 and + // any production caller wrapping fs. Otherwise rely on proper-lockfile's + // own unlock clearTimeout, which handles the normal path. + const guard: ReleaseGuard = { released: false } + const merged: LockOptions = userOptions.fs + ? (() => { + installReleaseGuard(userOptions.fs as UserFs, guard) + return { ...userOptions, onCompromised: () => {} } + })() + : userOptions + return getLockfile().lock(file, merged).then((release) => { + let released = false + return async () => { + if (released) return + released = true + guard.released = true + try { + await release() + } catch (err) { + // ERELEASED on a second release is benign — the caller already + // released through this guard. + if (err && typeof err === 'object' && 'code' in err && (err as { code: string }).code === 'ERELEASED') { + return + } + throw err + } + } + }) } export function lockSync(file: string, options?: LockOptions): () => void {