fix: preserve symlinks in atomicWrite to prevent config.toml symlink breakage#752
Open
hkk778530 wants to merge 3 commits into
Open
fix: preserve symlinks in atomicWrite to prevent config.toml symlink breakage#752hkk778530 wants to merge 3 commits into
hkk778530 wants to merge 3 commits into
Conversation
When config.toml is a symlink (e.g., pointing to iCloud), atomicWrite's rename() was replacing the symlink itself instead of writing through it. Root cause: fs.rename() replaces the directory entry, not the target. When the target is a symlink, the symlink gets overwritten. Fix: Before rename, resolve symlinks with lstat + realpath. The tmp file is created in the real path's parent directory, and rename targets the real path, preserving the symlink. Fallback behavior unchanged: if lstat or realpath fails (broken symlink, non-existent file), fall back to original path — same behavior as before. Fixes: symlinked config.toml gets replaced with regular file after adding/removing providers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 671d523 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b7bd39320
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address Codex review: following symlinks by default can break path- traversal boundaries in guarded stores (e.g., per-id-json-store). Changes: - atomicWrite: add options.followSymlinks parameter (default false) - writeFileAtomicDurable: add same opt-in parameter - writeConfigFile: pass followSymlinks: true (config.toml is a user-controlled fixed path, not a dynamic store) - Tests: verify default behavior replaces symlink, opt-in preserves it, add circular symlink test This preserves the security boundary for tasks/cron stores while fixing the symlink issue for config.toml. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When config.toml is a symlink (e.g., pointing to iCloud), atomicWrite's rename() was replacing the symlink itself instead of writing through it.
Root cause: fs.rename() replaces the directory entry, not the target. When the target is a symlink, the symlink gets overwritten.
Fix: Before rename, resolve symlinks with lstat + realpath. The tmp file is created in the real path's parent directory, and rename targets the real path, preserving the symlink.
Fallback behavior unchanged: if lstat or realpath fails (broken symlink, non-existent file), fall back to original path — same behavior as before.
Fixes: symlinked config.toml gets replaced with regular file after adding/removing providers.
Related Issue
Resolve #751
Problem
When
config.tomlis a symlink (e.g.,ln -s /iCloud/config.toml config.toml), adding/removing providers in kimi-code replaces the symlink with a regular file, breaking iCloudsync.
Root cause:
atomicWrite()usesfs.rename(tmp, target)which replaces the directory entry. When target is a symlink, the symlink itself gets overwritten instead of writingthrough to the target.
What changed
Made symlink following opt-in via
options.followSymlinksparameter (defaultfalse):Default behavior unchanged: without
followSymlinks, symlinks are replaced (not followed). This addresses the Codex review concern about guarded stores.Added tests:
followSymlinks: true: symlink is preservedNote: Cross-filesystem symlinks (e.g., symlink pointing to a network mount) are not handled —
rename()will fail withEXDEV. This is an edge case that can be addressed ina follow-up if needed.
Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.