Skip to content

fix: don't use all permissions for socket#810

Closed
justinrubek wants to merge 3 commits intocloudflare:mainfrom
justinrubek:fix-socket-perms
Closed

fix: don't use all permissions for socket#810
justinrubek wants to merge 3 commits intocloudflare:mainfrom
justinrubek:fix-socket-perms

Conversation

@justinrubek
Copy link
Contributor

The call to fchmodat for the upgrade socket was panicking in my systemd service. I had set RestrictSUIDSGID which doesn't allow for setting stat::Mode::all (which is 0o7777 and sets the special bits).

I believe we should only need the permission bits for the socket and not the execute bits set. This only requires 0o666 (rw-rw-rw-) to function since the execute bits and special bits don't have an effect on connecting or communicating through the socket. In my testing this was sufficient to function and I was able to use RestrictSUIDSGID with the service.

@drcaramelsyrup drcaramelsyrup added the enhancement New feature or request label Feb 13, 2026
johnhurt pushed a commit that referenced this pull request Mar 2, 2026
---
Merge branch 'cloudflare:main' into fix-socket-perms

Includes-commit: 7a748e7
Includes-commit: fdc2027
Replicated-from: #810
johnhurt pushed a commit that referenced this pull request Mar 2, 2026
---
Merge branch 'cloudflare:main' into fix-socket-perms

Includes-commit: 7a748e7
Includes-commit: fdc2027
Replicated-from: #810
@justinrubek
Copy link
Contributor Author

justinrubek commented Mar 18, 2026

Looks like this was pulled in commit b85f724 so this PR isn't needed anymore. It might've been useful to have an explicit update, but I finally noticed. Thanks for taking this fix! I'm happy to not have to use my forked version.

@justinrubek justinrubek deleted the fix-socket-perms branch March 18, 2026 02:12
@duke8253
Copy link
Collaborator

Looks like this was pulled in commit b85f724 so this PR isn't needed anymore. It might've been useful to have an explicit update, but I finally noticed. Thanks for taking this fix! I'm happy to not have to use my forked version.

Sorry, I forgot to put the accepted label on there.

@duke8253 duke8253 added the Accepted This change is accepted by us and merged to our internal repo label Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Accepted This change is accepted by us and merged to our internal repo enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants