bootstrap: set UNIX socket permissions to 770#833
bootstrap: set UNIX socket permissions to 770#833ungeskriptet wants to merge 1 commit intotinyauthapp:mainfrom
Conversation
This grants other processes access to the socket, provided they are in the correct group. Signed-off-by: David Wronek <david.wronek@mainlining.org>
dosubot
Bot
added
the
size:XS
📝 WalkthroughWalkthroughWhen the application is configured to run on a UNIX socket, the startup sequence now spawns a goroutine that applies file permissions (0770) to the socket path and logs a fatal error if the operation fails. ChangesUNIX Socket Permission Setup
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/bootstrap/app_bootstrap.go`:
- Around line 218-223: The os.Chmod call for app.config.Server.SocketPath must
be moved out of the fire-and-forget goroutine and executed immediately after the
UNIX socket is created in the unix-socket startup path (i.e., after
router.RunUnix(...) or the call that binds/creates the socket) so it runs
deterministically once the socket exists and before accepting connections;
remove the goroutine, call os.Chmod(app.config.Server.SocketPath, 0770) right
after the successful bind in the same control flow, and keep the current error
handling (use tlog.App.Fatal().Err(err).Msg(...)) if Chmod fails so startup
fails fast on permission issues.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cab04b9c-8310-4324-b98d-35938504224e
📒 Files selected for processing (1)
internal/bootstrap/app_bootstrap.go
| go func() { | ||
| // Ensure processes running as a different user can access the socket. | ||
| if err := os.Chmod(app.config.Server.SocketPath, 0770); err != nil { | ||
| tlog.App.Fatal().Err(err).Msg("Failed to update UNIX socket permissions") | ||
| } | ||
| }() |
There was a problem hiding this comment.
Apply the chmod after the socket is bound, not in a fire-and-forget goroutine.
os.Chmod can run before router.RunUnix(...) creates the socket, which makes startup nondeterministic: the goroutine may hit ENOENT and fatal the process even on a healthy boot. It also leaves a window where the socket still has the old permissions.
Please move the permission update into the unix-socket startup path so it runs once the socket exists and before connections are accepted.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/bootstrap/app_bootstrap.go` around lines 218 - 223, The os.Chmod
call for app.config.Server.SocketPath must be moved out of the fire-and-forget
goroutine and executed immediately after the UNIX socket is created in the
unix-socket startup path (i.e., after router.RunUnix(...) or the call that
binds/creates the socket) so it runs deterministically once the socket exists
and before accepting connections; remove the goroutine, call
os.Chmod(app.config.Server.SocketPath, 0770) right after the successful bind in
the same control flow, and keep the current error handling (use
tlog.App.Fatal().Err(err).Msg(...)) if Chmod fails so startup fails fast on
permission issues.
scottmckendry
left a comment
scottmckendry
left a comment
There was a problem hiding this comment.
Maybe I'm out of the loop, but could you also describe some use cases for this change?
| } | ||
|
|
||
| tlog.App.Info().Msgf("Starting server on unix socket %s", app.config.Server.SocketPath) | ||
| go func() { |
There was a problem hiding this comment.
I think umask would be the more correct approach here, rather than chmod.
2 participants
This grants other processes access to the socket, provided they are in the correct group.
Summary by CodeRabbit