a7e7de6407
`pumpStreamToFile` opened the user-chosen destination with `fs.createWriteStream`, which truncates the target the instant it opens, and its error path then unlinked that same path. When a user picked an existing file in the Save dialog (and confirmed the overwrite) and the gateway dropped mid-stream, the original was gone: truncated first, deleted second, with nothing written in its place. The data-URL compatibility fallback (`saveGatewayFileViaDataUrl`) had the same class of bug via `fs.promises.writeFile`, which truncates before the write completes. Both paths now go through one failure-atomic primitive. Bytes land in a short, randomly named sibling temp file (`.hermes-download-<hex>.part`, same directory so the final step is a same-volume rename), created with `flags: 'wx'`, and are renamed onto the destination only after the whole body has been written and the descriptor released. The destination is never opened before that point, so a failed download leaves whatever was there untouched. - Ownership-gated cleanup: the temp file is unlinked only after the stream's 'open' event proved THIS operation created it. An exclusive create that fails before open (EEXIST collision, EACCES, missing parent) never removes a file that belongs to someone else. - `WriteStream.close(cb)` rather than `end(cb)` before renaming: `end`'s callback fires on 'finish' while the fd may still be open, and Windows refuses to rename a file with an open handle. Falls back to `end` for stream shapes without `close`. - The failure path waits for 'close' (bounded by a 2s grace period) before unlinking, for the same reason: `destroy()` releases the fd asynchronously and an unlink racing the open handle would leak the `.part` file on Windows. - A rename failure (destination locked, permissions) removes the owned temp file and rejects; nothing is left behind. - Fixed-length temp name so a long user-chosen filename cannot push it past the filesystem's name limit. - `fsPumpDeps()` is the single production deps factory (`'wx'` create, `fs.promises.rename`, `fs.promises.unlink`); `writeBufferToFile()` routes the data-URL fallback through the same pump. `PumpDeps` gains `rename` and a `tempPathFor` test seam. Tests. Fakes: temp-then-rename on success, close-before-rename ordering, the regression itself (destination neither opened nor unlinked when the response fails mid-stream), write-error cleanup, close-before-unlink ordering, rename-failure cleanup, pre-open EEXIST leaves the colliding file alone, `writeBufferToFile` success and post-open write failure, the temp-name length bound, and the `main.ts` wiring. Real filesystem (`gateway-file-download.fs.test.ts`, exact production deps in a scratch dir): completed download replaces the destination with no temp left; mid-stream failure leaves the pre-existing destination byte-for-byte with no `.part`; failure into a fresh name leaves nothing; seeded temp path survives a pre-open EEXIST with no rename; rename failure (directory at the destination) cleans the owned temp; data-URL fallback success and missing-directory failure. Adds the contributor email mapping required by the attribution check. Fixes #96597 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015u8q2pHVPZmxpSrgkt94jC
3 lines
71 B
Plaintext
3 lines
71 B
Plaintext
amansk
|
|
# PR #96608 (desktop: failure-atomic gateway downloads, #96597)
|