mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-06 08:58:05 +00:00
fix(stack-files): atomic write via tmp+rename with optional exclusive mode (#1205)
* fix(stack-files): atomic write via tmp+rename with optional exclusive mode writeStackFile and writeStackFileBuffer previously called fs.writeFile directly, which truncates the target then streams the new bytes. A crash, disk-full event, or process kill between the truncate and the write left the target with partial content and no easy way to detect the half-write at read time. A private writeStackFileAtomic helper stages every write into a sibling .sencho-tmp-<suffix> file in the same directory, fsyncs, then promotes via fs.rename. A crash now leaves either the original target intact or a leftover .sencho-tmp file (cleaned up on the next failure path); a torn target file is no longer reachable through this path. The helper accepts an optional `exclusive: true` flag that swaps the final promote step from rename to link+unlink. link is atomic against EEXIST so a caller that needs "create only if not present" gets a race-free FILE_EXISTS error instead of a clobber. The upload route's overwrite-confirm flow (PR #1204) will wire this through in a follow-up so the existence check becomes authoritative. Behaviour for current callers (writeStackFile, writeStackFileBuffer, BlueprintService deploy) is unchanged: the non-exclusive default matches the prior fs.writeFile semantics from the caller's perspective. * fix(stack-files): tighten atomic write entropy + concurrent / failure tests Tmp suffix now uses crypto.randomBytes(6) so the per-process collision window is a true 48-bit space (Math.random().toString(36).slice(2,6) could drop leading zeros and narrow entropy unpredictably). Adds a short comment on the Windows link path noting NTFS / same-FS POSIX is required, both guaranteed by tmp+target being siblings. Two new tests close coverage gaps the first round missed: - a write step that throws (writeFile rejected) leaves no tmp leak and no partial target; - concurrent non-exclusive writers settle with at least one success and the final file is exactly one of the inputs (POSIX silently overwrites; Windows EPERMs the loser, both consistent).
This commit is contained in:
Anso
@@ -237,6 +237,118 @@ describe('FileSystemService stack methods', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ── atomic write semantics ──────────────────────────────────────────────
|
||||
|
||||
describe('atomic write semantics', () => {
|
||||
it('does not leak .sencho-tmp-* files after a successful write', async () => {
|
||||
const service = FileSystemService.getInstance();
|
||||
await service.writeStackFile(STACK, 'clean.txt', 'final');
|
||||
const dirEntries = await fs.readdir(stackDir);
|
||||
const leftovers = dirEntries.filter(name => name.startsWith('clean.txt.sencho-tmp-'));
|
||||
expect(leftovers).toEqual([]);
|
||||
});
|
||||
|
||||
it('preserves the original target when the rename step throws', async () => {
|
||||
const target = path.join(stackDir, 'crash.txt');
|
||||
await fs.writeFile(target, 'ORIGINAL');
|
||||
|
||||
const fsModule = await import('fs');
|
||||
const renameSpy = vi
|
||||
.spyOn(fsModule.promises, 'rename')
|
||||
.mockRejectedValueOnce(Object.assign(new Error('disk yanked'), { code: 'EIO' }));
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
await expect(service.writeStackFile(STACK, 'crash.txt', 'NEW')).rejects.toThrow(/disk yanked/);
|
||||
|
||||
// Target keeps its original content.
|
||||
const content = await fs.readFile(target, 'utf-8');
|
||||
expect(content).toBe('ORIGINAL');
|
||||
|
||||
// Tmp file is cleaned up.
|
||||
const dirEntries = await fs.readdir(stackDir);
|
||||
const leftovers = dirEntries.filter(name => name.startsWith('crash.txt.sencho-tmp-'));
|
||||
expect(leftovers).toEqual([]);
|
||||
|
||||
renameSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('exclusive write to a fresh target succeeds', async () => {
|
||||
const service = FileSystemService.getInstance();
|
||||
await service.writeStackFile(STACK, 'first.txt', 'hello', { exclusive: true });
|
||||
const content = await fs.readFile(path.join(stackDir, 'first.txt'), 'utf-8');
|
||||
expect(content).toBe('hello');
|
||||
});
|
||||
|
||||
it('exclusive write to an existing target throws FILE_EXISTS and preserves the original', async () => {
|
||||
const target = path.join(stackDir, 'taken.txt');
|
||||
await fs.writeFile(target, 'KEEP');
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
let caught: unknown = null;
|
||||
try {
|
||||
await service.writeStackFile(STACK, 'taken.txt', 'OVERWRITE', { exclusive: true });
|
||||
} catch (err) {
|
||||
caught = err;
|
||||
}
|
||||
expect((caught as { code?: string })?.code).toBe('FILE_EXISTS');
|
||||
|
||||
const content = await fs.readFile(target, 'utf-8');
|
||||
expect(content).toBe('KEEP');
|
||||
|
||||
// Tmp file cleaned up after the link failure.
|
||||
const dirEntries = await fs.readdir(stackDir);
|
||||
const leftovers = dirEntries.filter(name => name.startsWith('taken.txt.sencho-tmp-'));
|
||||
expect(leftovers).toEqual([]);
|
||||
});
|
||||
|
||||
it('cleans up the tmp file when the write step throws', async () => {
|
||||
const fsModule = await import('fs');
|
||||
const originalOpen = fsModule.promises.open;
|
||||
const openSpy = vi
|
||||
.spyOn(fsModule.promises, 'open')
|
||||
.mockImplementationOnce(async (...args) => {
|
||||
const fh = await originalOpen.apply(fsModule.promises, args as Parameters<typeof originalOpen>);
|
||||
// Patch writeFile to throw, the close still runs via the inner finally.
|
||||
(fh as unknown as { writeFile: () => Promise<void> }).writeFile = () =>
|
||||
Promise.reject(Object.assign(new Error('write blew up'), { code: 'EIO' }));
|
||||
return fh;
|
||||
});
|
||||
|
||||
const service = FileSystemService.getInstance();
|
||||
await expect(service.writeStackFile(STACK, 'wfail.txt', 'NEW')).rejects.toThrow(/write blew up/);
|
||||
|
||||
// Target was never created and the tmp was cleaned.
|
||||
const dirEntries = await fs.readdir(stackDir);
|
||||
expect(dirEntries).not.toContain('wfail.txt');
|
||||
const leftovers = dirEntries.filter(name => name.startsWith('wfail.txt.sencho-tmp-'));
|
||||
expect(leftovers).toEqual([]);
|
||||
|
||||
openSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('concurrent non-exclusive writers to the same path leave one winning content and no tmp leaks', async () => {
|
||||
const service = FileSystemService.getInstance();
|
||||
const inputs = ['A', 'B', 'C', 'D', 'E'].map(l => l.repeat(32));
|
||||
// POSIX rename is atomic and silently overwrites: every writer succeeds and
|
||||
// the last-to-rename wins. Windows rename throws EPERM if the destination
|
||||
// is open or being renamed by another process. Both are acceptable: the
|
||||
// contract is "the final file is always exactly one of the inputs, never
|
||||
// torn, and no tmp files leak". Settle individually and require at least
|
||||
// one writer to have succeeded.
|
||||
const results = await Promise.allSettled(
|
||||
inputs.map(content => service.writeStackFile(STACK, 'race.txt', content)),
|
||||
);
|
||||
expect(results.some(r => r.status === 'fulfilled')).toBe(true);
|
||||
|
||||
const finalContent = await fs.readFile(path.join(stackDir, 'race.txt'), 'utf-8');
|
||||
expect(inputs).toContain(finalContent);
|
||||
|
||||
const dirEntries = await fs.readdir(stackDir);
|
||||
const leftovers = dirEntries.filter(name => name.startsWith('race.txt.sencho-tmp-'));
|
||||
expect(leftovers).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
// ── deleteStackPath ─────────────────────────────────────────────────────
|
||||
|
||||
describe('deleteStackPath', () => {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import path from 'path';
|
||||
import os from 'os';
|
||||
import crypto from 'crypto';
|
||||
import { promises as fsPromises, createReadStream } from 'fs';
|
||||
import type { Readable } from 'stream';
|
||||
import { NodeRegistry } from './NodeRegistry';
|
||||
@@ -699,16 +700,80 @@ export class FileSystemService {
|
||||
};
|
||||
}
|
||||
|
||||
async writeStackFile(stackName: string, relPath: string, content: string): Promise<void> {
|
||||
const safePath = await this.resolveSafeStackPath(stackName, relPath);
|
||||
/**
|
||||
* Atomic write: stages the content in a sibling .tmp file in the same
|
||||
* directory, fsyncs, then promotes it to the final path. A crash between
|
||||
* the open and the rename leaves either the original target intact or a
|
||||
* leftover .tmp file (cleaned up on next failure path), never a truncated
|
||||
* target.
|
||||
*
|
||||
* `exclusive: true` uses link+unlink instead of rename so the create is
|
||||
* race-free atomic: link fails with EEXIST if the target already exists,
|
||||
* giving the caller a definitive "did not exist when we wrote it" signal.
|
||||
* The non-exclusive default uses rename, which is atomic against partial
|
||||
* reads but clobbers any existing target.
|
||||
*/
|
||||
private async writeStackFileAtomic(
|
||||
safePath: string,
|
||||
data: string | Buffer,
|
||||
opts: { exclusive?: boolean } = {},
|
||||
): Promise<void> {
|
||||
await fsPromises.mkdir(path.dirname(safePath), { recursive: true });
|
||||
await fsPromises.writeFile(safePath, content, 'utf-8');
|
||||
// crypto.randomBytes gives a guaranteed-length high-entropy suffix; Math.random
|
||||
// can drop leading zeros which narrows entropy unpredictably.
|
||||
const suffix = `${process.pid}-${Date.now()}-${crypto.randomBytes(6).toString('hex')}`;
|
||||
const tmpPath = `${safePath}.sencho-tmp-${suffix}`;
|
||||
let stagedTmp = false;
|
||||
try {
|
||||
const fh = await fsPromises.open(tmpPath, 'wx');
|
||||
stagedTmp = true;
|
||||
try {
|
||||
await fh.writeFile(data);
|
||||
await fh.sync();
|
||||
} finally {
|
||||
await fh.close();
|
||||
}
|
||||
if (opts.exclusive) {
|
||||
// link() is atomic against EEXIST. Tmp and target are guaranteed to live
|
||||
// in the same directory (same filesystem); link works on NTFS and any
|
||||
// POSIX FS without elevated privileges.
|
||||
try {
|
||||
await fsPromises.link(tmpPath, safePath);
|
||||
} catch (err: unknown) {
|
||||
if ((err as NodeJS.ErrnoException).code === 'EEXIST') {
|
||||
throw Object.assign(new Error('File already exists'), { code: 'FILE_EXISTS' as const });
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
} else {
|
||||
await fsPromises.rename(tmpPath, safePath);
|
||||
stagedTmp = false;
|
||||
}
|
||||
} finally {
|
||||
if (stagedTmp) {
|
||||
await fsPromises.unlink(tmpPath).catch(() => {});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async writeStackFileBuffer(stackName: string, relPath: string, buffer: Buffer): Promise<void> {
|
||||
async writeStackFile(
|
||||
stackName: string,
|
||||
relPath: string,
|
||||
content: string,
|
||||
opts?: { exclusive?: boolean },
|
||||
): Promise<void> {
|
||||
const safePath = await this.resolveSafeStackPath(stackName, relPath);
|
||||
await fsPromises.mkdir(path.dirname(safePath), { recursive: true });
|
||||
await fsPromises.writeFile(safePath, buffer);
|
||||
await this.writeStackFileAtomic(safePath, content, opts);
|
||||
}
|
||||
|
||||
async writeStackFileBuffer(
|
||||
stackName: string,
|
||||
relPath: string,
|
||||
buffer: Buffer,
|
||||
opts?: { exclusive?: boolean },
|
||||
): Promise<void> {
|
||||
const safePath = await this.resolveSafeStackPath(stackName, relPath);
|
||||
await this.writeStackFileAtomic(safePath, buffer, opts);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user