patch: v2.9.6 #744
patch: v2.9.6 #744
Conversation
There was a problem hiding this comment.
Pull request overview
This is a patch release (v2.9.6) that includes a security improvement for Unix socket handling and documentation updates to the README files.
- Adds close-on-exec flag to duplicated file descriptors in the client enrollment process
- Updates README section headings for better clarity
- Changes English "Comparison with Rust" section to "Combat with Rust"
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| client_unix.go | Adds CloseOnExec call to prevent file descriptor leakage to child processes after duplicating socket FDs |
| README.md | Updates section heading from "Milestone" to "Milestones" and changes "Comparison with Rust" to "Combat with Rust" |
| README_ZH.md | Updates Chinese section headings from "当前支持" to "里程碑" and "未来计划" to "蓝图" for consistency with English README |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var dupFD int | ||
| e := rc.Control(func(fd uintptr) { | ||
| dupFD, err = unix.Dup(int(fd)) | ||
| // Set the socket to close-on-exec, so that the socket is closed when the process forks |
There was a problem hiding this comment.
The comment is inaccurate. The close-on-exec flag prevents the file descriptor from being inherited by child processes created via exec() system calls, not fork(). After fork(), file descriptors are inherited by default, but close-on-exec ensures they are closed when exec() is called. The comment should be updated to reflect this distinction.
| // Set the socket to close-on-exec, so that the socket is closed when the process forks | |
| // Set the socket to close-on-exec so that the duplicated descriptor is not inherited by exec'ed child processes. |
| dupFD, err = unix.Dup(int(fd)) | ||
| // Set the socket to close-on-exec, so that the socket is closed when the process forks | ||
| unix.CloseOnExec(dupFD) |
There was a problem hiding this comment.
The CloseOnExec call is missing error handling. If unix.Dup() fails (line 213), dupFD will be 0 (the zero value for int), and calling unix.CloseOnExec(0) could incorrectly set the close-on-exec flag on stdin. The error from unix.Dup should be checked before calling CloseOnExec, or CloseOnExec should only be called if dupFD is valid (greater than 0).
| dupFD, err = unix.Dup(int(fd)) | |
| // Set the socket to close-on-exec, so that the socket is closed when the process forks | |
| unix.CloseOnExec(dupFD) | |
| newFD, dupErr := unix.Dup(int(fd)) | |
| if dupErr != nil { | |
| err = dupErr | |
| return | |
| } | |
| dupFD = newFD | |
| // Set the socket to close-on-exec, so that the socket is closed when the process forks | |
| if closeErr := unix.CloseOnExec(dupFD); closeErr != nil { | |
| err = closeErr | |
| } |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #744 +/- ##
=======================================
Coverage 84.30% 84.31%
=======================================
Files 22 22
Lines 2402 2403 +1
=======================================
+ Hits 2025 2026 +1
Misses 257 257
Partials 120 120
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
No description provided.