Skip to content

Commit b014d43

Browse files
committed
fix(filesystem): clean up temp files on fallback failures
1 parent c22fad2 commit b014d43

2 files changed

Lines changed: 86 additions & 11 deletions

File tree

src/filesystem/__tests__/lib.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,48 @@ describe('Lib Functions', () => {
363363
{ force: true }
364364
);
365365
});
366+
367+
it('cleans up temp file and re-throws when fs.cp fails after EPERM', async () => {
368+
const eexistError = new Error('EEXIST') as NodeJS.ErrnoException;
369+
eexistError.code = 'EEXIST';
370+
const epermError = new Error('EPERM') as NodeJS.ErrnoException;
371+
epermError.code = 'EPERM';
372+
const enospcError = new Error('ENOSPC') as NodeJS.ErrnoException;
373+
enospcError.code = 'ENOSPC';
374+
375+
mockFs.writeFile
376+
.mockRejectedValueOnce(eexistError)
377+
.mockResolvedValueOnce(undefined);
378+
mockFs.rename.mockRejectedValueOnce(epermError);
379+
mockFs.cp.mockRejectedValueOnce(enospcError);
380+
mockFs.unlink.mockResolvedValue(undefined);
381+
382+
await expect(writeFileContent('/test/file.txt', 'new content'))
383+
.rejects.toThrow('ENOSPC');
384+
385+
expect(mockFs.unlink).toHaveBeenCalledWith(
386+
expect.stringMatching(/\/test\/file\.txt\.[a-f0-9]+\.tmp$/)
387+
);
388+
});
389+
390+
it('cleans up temp file and re-throws when temp write fails', async () => {
391+
const eexistError = new Error('EEXIST') as NodeJS.ErrnoException;
392+
eexistError.code = 'EEXIST';
393+
const enospcError = new Error('ENOSPC') as NodeJS.ErrnoException;
394+
enospcError.code = 'ENOSPC';
395+
396+
mockFs.writeFile
397+
.mockRejectedValueOnce(eexistError)
398+
.mockRejectedValueOnce(enospcError);
399+
mockFs.unlink.mockResolvedValue(undefined);
400+
401+
await expect(writeFileContent('/test/file.txt', 'new content'))
402+
.rejects.toThrow('ENOSPC');
403+
404+
expect(mockFs.unlink).toHaveBeenCalledWith(
405+
expect.stringMatching(/\/test\/file\.txt\.[a-f0-9]+\.tmp$/)
406+
);
407+
});
366408
});
367409

368410
});
@@ -663,6 +705,24 @@ describe('Lib Functions', () => {
663705
);
664706
});
665707

708+
it('cleans up temp file and re-throws when temp write fails during file edit', async () => {
709+
const enospcError = new Error('ENOSPC') as NodeJS.ErrnoException;
710+
enospcError.code = 'ENOSPC';
711+
712+
mockFs.readFile.mockResolvedValue('line1\nline2\nline3\n');
713+
mockFs.writeFile.mockRejectedValueOnce(enospcError);
714+
mockFs.unlink.mockResolvedValue(undefined);
715+
716+
const edits = [{ oldText: 'line2', newText: 'modified line2' }];
717+
718+
await expect(applyFileEdits('/test/file.txt', edits, false))
719+
.rejects.toThrow('ENOSPC');
720+
721+
expect(mockFs.unlink).toHaveBeenCalledWith(
722+
expect.stringMatching(/\/test\/file\.txt\.[a-f0-9]+\.tmp$/)
723+
);
724+
});
725+
666726
it('handles CRLF line endings in file content', async () => {
667727
mockFs.readFile.mockResolvedValue('line1\r\nline2\r\nline3\r\n');
668728

src/filesystem/lib.ts

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -159,23 +159,28 @@ export async function validatePath(requestedPath: string): Promise<string> {
159159
* @param tempPath Path to the temporary file that contains the new content.
160160
* @param targetPath Path to the destination file to be replaced.
161161
*/
162+
async function cleanupTempFile(tempPath: string): Promise<void> {
163+
try {
164+
await fs.unlink(tempPath);
165+
} catch {}
166+
}
167+
162168
async function replaceFileFromTemp(tempPath: string, targetPath: string): Promise<void> {
163169
try {
164170
await fs.rename(tempPath, targetPath);
165171
} catch (renameError) {
166172
if ((renameError as NodeJS.ErrnoException).code === 'EPERM') {
167173
// Fallback: copy then best-effort cleanup
168-
await fs.cp(tempPath, targetPath, { force: true });
169174
try {
170-
await fs.unlink(tempPath);
171-
} catch {
172-
// Best-effort cleanup; target was already written successfully
175+
await fs.cp(tempPath, targetPath, { force: true });
176+
} catch (copyError) {
177+
await cleanupTempFile(tempPath);
178+
throw copyError;
173179
}
180+
await cleanupTempFile(tempPath);
174181
} else {
175182
// For non-EPERM errors, clean up the temp file and re-throw
176-
try {
177-
await fs.unlink(tempPath);
178-
} catch {}
183+
await cleanupTempFile(tempPath);
179184
throw renameError;
180185
}
181186
}
@@ -210,8 +215,13 @@ export async function writeFileContent(filePath: string, content: string): Promi
210215
// could be created between validation and write. Rename operations
211216
// replace the target file atomically and don't follow symlinks.
212217
const tempPath = `${filePath}.${randomBytes(16).toString('hex')}.tmp`;
213-
await fs.writeFile(tempPath, content, 'utf-8');
214-
await replaceFileFromTemp(tempPath, filePath);
218+
try {
219+
await fs.writeFile(tempPath, content, 'utf-8');
220+
await replaceFileFromTemp(tempPath, filePath);
221+
} catch (tempWriteError) {
222+
await cleanupTempFile(tempPath);
223+
throw tempWriteError;
224+
}
215225
} else {
216226
throw error;
217227
}
@@ -301,8 +311,13 @@ export async function applyFileEdits(
301311
// could be created between validation and write. Rename operations
302312
// replace the target file atomically and don't follow symlinks.
303313
const tempPath = `${filePath}.${randomBytes(16).toString('hex')}.tmp`;
304-
await fs.writeFile(tempPath, modifiedContent, 'utf-8');
305-
await replaceFileFromTemp(tempPath, filePath);
314+
try {
315+
await fs.writeFile(tempPath, modifiedContent, 'utf-8');
316+
await replaceFileFromTemp(tempPath, filePath);
317+
} catch (writeError) {
318+
await cleanupTempFile(tempPath);
319+
throw writeError;
320+
}
306321
}
307322

308323
return formattedDiff;

0 commit comments

Comments
 (0)