fix(fs): write files atomically to avoid truncated concurrent reads by lazerg · Pull Request #798 · unjs/unstorage

lazerg · 2026-07-30T00:14:40Z

writeFile opened the target with O_TRUNC and then streamed the data in, so a getItem/getItemRaw running against the same key during a write could observe the file while it was still empty or half-written and return a truncated value. Writing to a temp file in the same directory and renaming it over the target makes the swap atomic, so a concurrent read sees either the old value or the new one.

Fixes #797

Summary by CodeRabbit

Bug Fixes
- Improved raw file writing to be atomic: writes go to a uniquely named temp file first, then safely finalize to the destination.
- Prevented spurious temp entries from appearing in listings by skipping temp files during recursive directory scans.
- Preserved existing file permissions on overwrite and tightened error handling to only ignore missing destinations.
Tests
- Strengthened concurrency tests to ensure reads never return truncated or mixed-byte buffers during overlapping writes.
- Added regression coverage that key listings never include temp markers while temp keys are being written.
- Added a platform-gated overwrite permission test (skipped on Windows).

coderabbitai · 2026-07-30T00:14:54Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 74556239-ef7b-4d0b-8d6f-f4a90cf19775

📥 Commits

Reviewing files that changed from the base of the PR and between 6d85e18 and 504014a.

📒 Files selected for processing (2)

src/drivers/utils/node-fs.ts
test/drivers/fs.test.ts

📝 Walkthrough

Walkthrough

Changes

The shared Node filesystem writer now writes through uniquely named temporary files, preserves existing permissions, renames atomically, and hides temporary entries. The fs and fs-lite suites validate complete concurrent reads, key listings, permissions, and stat-error handling.

Atomic filesystem writes

Layer / File(s)	Summary
Atomic write helper `src/drivers/utils/node-fs.ts`	`writeFile` uses PID- and UUID-based temporary paths, preserves existing permissions, renames files atomically, cleans up failures, rethrows non-`ENOENT` stat errors, and filters temporary entries from recursive listings.
Concurrent write validation `test/drivers/fs.test.ts`, `test/drivers/fs-lite.test.ts`	Both drivers verify complete uniform buffers, hide temporary filenames during concurrent writes, preserve permissions when overwriting files, and retain contents when stat fails.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Writer
  participant writeFile
  participant Filesystem
  participant Reader
  Writer->>writeFile: write data
  writeFile->>Filesystem: write temporary file
  writeFile->>Filesystem: rename temporary file to target
  Reader->>Filesystem: read target
  Filesystem-->>Reader: complete old or new value

Suggested reviewers: pi0

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: making fs writes atomic to prevent truncated concurrent reads.
Linked Issues check	✅ Passed	The changes implement atomic temp-file writes for fs and fs-lite, preserve same-directory renames, and hide temp files from recursive key listing.
Out of Scope Changes check	✅ Passed	The extra permission-preservation and stat error handling changes are supporting details for the atomic overwrite fix, not unrelated scope.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/drivers/utils/node-fs.ts`:
- Around line 20-23: Update readdirRecursive() to exclude only temporary files
matching the generated writeFile() pattern `*.<pid>.<UUID>.tmp` during recursive
key enumeration, while preserving user keys that merely end in `.tmp`;
alternatively, add a concurrent setItem/getKeys test that verifies these
in-progress files are omitted.

In `@test/drivers/fs.test.ts`:
- Around line 30-31: Update the concurrency read assertions in
test/drivers/fs.test.ts lines 30-31 and test/drivers/fs-lite.test.ts lines 29-30
to verify each buffer’s complete content is either all `a` or all `b`, rather
than checking length only; preserve the existing read loop and ensure
mixed-content buffers fail.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a8641cb7-2430-40c0-a50b-7c3f2b658e75

📥 Commits

Reviewing files that changed from the base of the PR and between e6be613 and 3227ba7.

📒 Files selected for processing (3)

src/drivers/utils/node-fs.ts
test/drivers/fs-lite.test.ts
test/drivers/fs.test.ts

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/drivers/utils/node-fs.ts (1)
22-25: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Preserve destination permissions when replacing files atomically.

writeFile() creates the temporary path with the default mode (0o666 minus the umask), then rename() replaces the existing destination. An existing restrictive file such as 0600 can end up as roughly 0644 after an overwrite. Capture and restore the destination’s mode before renaming, and cover this with a regression test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/drivers/utils/node-fs.ts` around lines 22 - 25, Update the atomic write
flow using the temporary path and rename operation to capture the existing
destination file mode and apply it to the temporary file before renaming,
preserving restrictive permissions during replacement. Handle destinations that
do not yet exist with the current creation behavior, and add a regression test
verifying an overwrite retains the original mode.
Source: MCP tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/drivers/utils/node-fs.ts`:
- Around line 22-25: Update the atomic write flow using the temporary path and
rename operation to capture the existing destination file mode and apply it to
the temporary file before renaming, preserving restrictive permissions during
replacement. Handle destinations that do not yet exist with the current creation
behavior, and add a regression test verifying an overwrite retains the original
mode.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0b02ad41-0c88-4f5e-9fb5-9f82923f0903

📥 Commits

Reviewing files that changed from the base of the PR and between 3227ba7 and 4eb91cc.

📒 Files selected for processing (3)

src/drivers/utils/node-fs.ts
test/drivers/fs-lite.test.ts
test/drivers/fs.test.ts

lazerg · 2026-07-30T01:29:16Z

Thanks, good catch. Fixed in 6d85e18 by reading the destination file's mode and applying it to the temp file before the rename, so an existing 0600 file keeps its permissions after an overwrite. Added a regression test to both the fs and fs-lite suites (skipped on Windows).

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/drivers/utils/node-fs.ts`:
- Around line 25-31: The destination mode lookup should suppress only an ENOENT
error. Update the fsPromises.stat(path) catch in the destination-mode flow to
rethrow all other filesystem errors, while retaining the existing undefined
result and chmod behavior when the destination is absent.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d9ee1632-d4ff-400f-b28e-b45eff602c5d

📥 Commits

Reviewing files that changed from the base of the PR and between 4eb91cc and 6d85e18.

📒 Files selected for processing (3)

src/drivers/utils/node-fs.ts
test/drivers/fs-lite.test.ts
test/drivers/fs.test.ts

fix(fs): write files atomically to avoid truncated concurrent reads

3227ba7

lazerg requested a review from pi0 as a code owner July 30, 2026 00:14

coderabbitai Bot reviewed Jul 30, 2026

View reviewed changes

Comment thread src/drivers/utils/node-fs.ts

Comment thread test/drivers/fs.test.ts Outdated

fix(fs): exclude in-progress temp files from key listing

4eb91cc

coderabbitai Bot reviewed Jul 30, 2026

View reviewed changes

fix(fs): preserve destination file permissions on atomic overwrite

6d85e18

coderabbitai Bot reviewed Jul 30, 2026

View reviewed changes

Comment thread src/drivers/utils/node-fs.ts

fix(fs): only ignore ENOENT when reading destination mode

504014a

Navigation Menu

Uh oh!

fix(fs): write files atomically to avoid truncated concurrent reads - #798

fix(fs): write files atomically to avoid truncated concurrent reads#798
lazerg wants to merge 4 commits into
unjs:mainfrom
lazerg:fix/fs-atomic-write

lazerg commented Jul 30, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 30, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

lazerg commented Jul 30, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

lazerg commented Jul 30, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

lazerg commented Jul 30, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lazerg commented Jul 30, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 30, 2026 •

edited

Loading