mirror of
https://github.com/garrytan/gstack.git
synced 2026-06-17 07:10:12 +02:00
fix(security-classifier): await writer close before unlinking tmp on error
The earlier downloadFile() error-path cleanup hit a race: Node's createWriteStream lazily opens the FD and flushes buffered writes during destroy(), so a naive `fs.unlinkSync(tmp)` immediately after `writer.destroy()` hits ENOENT (file not yet on disk), then the writer's destroy finishes on the next tick and creates the file fresh — leaving the half-written tmp behind exactly as the original fix tried to prevent. The new sequence awaits the writer's 'close' event before unlinking, so the FD is fully torn down and no subsequent flush can re-create the path. Caught by browse/test/security-classifier-download-cleanup.test.ts in the next commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -156,9 +156,15 @@ export async function downloadFile(url: string, dest: string): Promise<void> {
|
||||
});
|
||||
fs.renameSync(tmp, dest);
|
||||
} catch (err) {
|
||||
// Close the writer and drop the half-written tmp so we don't leak an FD
|
||||
// or ship a truncated model file to the renameSync path on retry.
|
||||
try { writer.destroy(); } catch { /* already destroyed */ }
|
||||
// Drop the half-written tmp so we don't ship a truncated model file to
|
||||
// a retry's renameSync. Wait for the writer to close fully before
|
||||
// unlinking: Node's createWriteStream lazily opens the FD and flushes
|
||||
// buffered writes during destroy(), so a naive unlinkSync hits ENOENT
|
||||
// first and the writer re-creates the file on the next tick.
|
||||
await new Promise<void>((resolve) => {
|
||||
writer.once('close', () => resolve());
|
||||
writer.destroy();
|
||||
});
|
||||
try { fs.unlinkSync(tmp); } catch { /* nothing to clean */ }
|
||||
throw err;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user