fix: preserve file owner/group on atomic rewrite#82
Conversation
Chown the temp file to the original uid/gid before rename on Unix so rewrites do not silently change ownership when run as root. Fixes dolph#17
![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: 0842d98c39
ℹ️ 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".
| if err := chownTempFromInfo(tempName, info); err != nil { | ||
| return fmt.Errorf("preserve ownership on temp file %v: %w", tempName, err) |
There was a problem hiding this comment.
When chownTempFromInfo fails after the temp file has been created, this returns before the defer os.Remove(tempName) is registered, leaving the random temp file in the target directory. This can happen for a non-root user rewriting a group-writable file owned by another uid/gid, where the write succeeds but os.Chown returns EPERM; move the cleanup defer immediately after os.WriteFile succeeds so all post-create failures are cleaned up.
Useful? React with 👍 / 👎.
| if err := os.WriteFile(tempName, []byte(content), mode); err != nil { | ||
| return fmt.Errorf("create tempfile in %v: %w", f.Dir(), err) | ||
| } | ||
| if err := chownTempFromInfo(tempName, info); err != nil { |
There was a problem hiding this comment.
For setuid/setgid files, Unix kernels clear those special permission bits on chown, so the temp file is created with info.Mode() and then loses those bits here before it is renamed over the original. Rewriting a matching setuid/setgid executable therefore silently strips its mode bits; apply ownership before restoring the original mode, or chmod again after the chown succeeds.
Useful? React with 👍 / 👎.
1 participant
Fixes #17
Made with Cursor