Add symlink check to file rename util #2576
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds symlink handling to the file rename utility to prevent os.Rename from overriding symlinks. When the destination is a symlink, the code now falls back to copying file contents instead of renaming, preserving the symlink.
Changes:
- Modified
tryRenameFileto detect symlinks and use copy instead of rename - Added comprehensive test coverage for symlink handling
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/yqlib/file_utils.go | Added symlink detection before rename and fallback to copy operation |
| pkg/yqlib/write_in_place_handler_test.go | Added test case verifying symlink preservation during write-in-place operations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if copyError := copyFileContents(from, to); copyError != nil { | ||
| return fmt.Errorf("failed copying from %v to %v: %w", from, to, copyError) | ||
| } | ||
| tryRemoveTempFile(from) |
There was a problem hiding this comment.
The symlink copy path doesn't return after success, which means execution will fall through to the os.Rename call on line 17. Add return nil after tryRemoveTempFile(from) to prevent attempting rename after successful copy.
| tryRemoveTempFile(from) | |
| tryRemoveTempFile(from) | |
| return nil |
| if info, err := os.Lstat(to); err == nil && info.Mode()&os.ModeSymlink != 0 { | ||
| log.Debug("Target file is symlink, skipping rename and attempting to copy contents") | ||
|
|
||
| if copyError := copyFileContents(from, to); copyError != nil { |
There was a problem hiding this comment.
Using copyFileContents will overwrite the symlink with a regular file since os.Create replaces the file. To preserve the symlink, the code should follow the symlink to copy contents to the target file. Use os.Stat to resolve the symlink path, then copy to the resolved destination.
1 participant
Adds a check to see if the destination file in the rename function is a symlink and in that case fall back to the same copy step as when the rename fails. Since os.Rename overrides the symlink.
Fixes #2155