Skip to content

fix(drop-sudo): deny the runner user via socket ACLs instead of stripping group/other mode bits - #166

Open
pardhaponugoti wants to merge 1 commit into
openai:mainfrom
pardhaponugoti:fix-drop-sudo-socket-acl
Open

fix(drop-sudo): deny the runner user via socket ACLs instead of stripping group/other mode bits#166
pardhaponugoti wants to merge 1 commit into
openai:mainfrom
pardhaponugoti:fix-drop-sudo-socket-acl

Conversation

@pardhaponugoti

Copy link
Copy Markdown

Fixes #160

Problem

drop-sudo's Linux root phase restricts every root-owned service socket under /run that the runner user can write to, via chmod(fd, mode & 0o700). Stripping the group/other bits doesn't just block the runner user — it locks out the system peers those sockets exist to serve: D-Bus clients such as systemd-resolved, journald stdout connections, docker group members.

The observable consequence on a live host (details and journal excerpts in #160): systemd-resolved, a non-root service, can no longer connect to the system bus, crash-loops with Failed to connect to system bus: Permission denied into the systemd start limit, and name resolution dies machine-wide. On GitHub-hosted runners this eventually starves the runner agent itself — jobs whose codex step runs past ~45 minutes die with "The hosted runner lost communication with the server" (observed at 52–65 minutes, on standard and larger runners alike, 12/12 in our data).

Fix

Deny only the runner user, with a named-user ACL entry:

setfacl -m u:<runner>:--- <socket>

The socket's mode — and therefore every system peer's access — stays untouched, while the dropped user loses access exactly as before. The named-user entry also closes a case the mode check couldn't see: a socket whose group/other bits are already 0 but which grants the runner access through an existing ACL entry (previously the function returned early as "already restricted" and the non-root verification phase then failed hard).

If ACL tooling is unavailable (getfacl probe fails — missing binaries or a filesystem without ACL support), the previous chmod behavior is kept as a fallback so the security posture never regresses; it logs the collateral it's about to cause.

The TOCTOU guards are unchanged: the socket is opened O_PATH|O_NOFOLLOW, stat-checked against the discovered device/inode, and the ACL is applied through /proc/self/fd/N, then read back and verified.

Why the tests didn't need to change

The suite's contract is access-based, not mode-based: the fake codex binary records accessSync(W_OK) per socket and the assertions require the dropped user to have lost access (dockerAccessible, serviceAccessible, worldAccessible, namedUserAclAccessible all false), and verifyPrivilegedSocketsRestricted re-checks writability as the dropped user in the non-root phase. A named-user deny ACL satisfies that contract; the full sudo-gated suite passes on ubuntu-latest unchanged.

Scope

One behavioral change, no interface changes, dist/ rebuilt with pnpm run build. Happy to split the fallback or adjust logging tone if maintainers prefer.

…ping group/other mode bits

restrictRootServiceSocket previously chmod-ed every root-owned service
socket the runner user could write to `mode & 0o700`. On a live host that
removes access for the system peers those sockets exist to serve — D-Bus
clients such as systemd-resolved, journald stdout connections, docker group
members. systemd-resolved (a non-root service) then fails to (re)start with
"Failed to connect to system bus: Permission denied", crash-loops into the
systemd start limit, and name resolution dies machine-wide. On GitHub-hosted
runners this eventually starves the runner agent itself: jobs whose codex
step exceeds ~45 minutes are declared "the hosted runner lost communication
with the server" (observed 52-65 min across both standard and larger
runners).

Deny only the runner user instead, with a named-user ACL entry
(setfacl -m u:<user>:---), leaving the socket mode — and therefore every
system peer's access — untouched. Named-user entries also close the case a
mode-only check missed: a socket whose group/other bits are already 0 but
which grants the runner access through an existing ACL. When ACL tooling is
unavailable the previous chmod behavior is kept as a fallback, so the
security posture never regresses.

The existing test contract is access-based (the fake codex binary records
accessSync(W_OK) per socket) and passes unchanged; verification in the
non-root phase continues to assert the dropped user cannot write any
discovered socket.

Fixes openai#160
@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@pardhaponugoti

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant