Harden sandbox: drop ptrace, tighten socket egress, enforce cap-drop/no-new-privileges - #160
Harden sandbox: drop ptrace, tighten socket egress, enforce cap-drop/no-new-privileges#160MauricioPerera wants to merge 3 commits into
Conversation
Two fixes from a security audit of this repo: - Remove process_vm_readv/process_vm_writev/ptrace from the allowlist. All sandboxed processes share the same uid, so ptrace let one process inspect/modify another's memory (e.g. read a secret held by a sibling process) inside the sandbox -- inter-process isolation was effectively void despite the seccomp profile's intent. - Replace the 3 socket-domain rules with an explicit allowlist of AF_UNIX(1)/AF_INET(2)/AF_INET6(10). The previous rules (domain<38, domain==39, domain>40) only actually blocked AF_ALG(38) and AF_VSOCK(40) -- AF_NETLINK(16) and AF_PACKET(17) were allowed, along with everything else outside that narrow gap, making the egress control close to a no-op. Verified with a real Docker build + run on a separate host, under the exact production security contract (--cap-drop ALL --security-opt no-new-privileges + this seccomp profile): - DNS resolution (getaddrinfo) and `git clone` over HTTPS still work (AF_INET/AF_INET6 unaffected) -- confirms this doesn't break the scanner's own legitimate network use. - AF_NETLINK and AF_PACKET socket() calls now fail with EPERM inside the container (previously succeeded) -- confirms the fix is real, not cosmetic. Local commit only -- this is an audit clone of openai/codex-security, not a fork with push access.
…-privileges docker/codex-security-seccomp.json unconditionally allows mount/pivot_root/ unshare/setns/clone for Codex's own unprivileged Bubblewrap sandboxing -- those syscalls can't be removed without breaking that feature entirely. Their safety previously depended entirely on a comment in the seccomp profile asserting the caller runs with --cap-drop ALL and --security-opt no-new-privileges (true for compose.yaml, not enforced for a bare `docker run`). Adds a runtime self-check at the top of the entrypoint: read /proc/self/status, refuse to start (exit 3, clear message) unless CapEff is all-zero and NoNewPrivs is 1. Converts an undocumented-until-you-read- the-seccomp-comment assumption into a loud, immediate failure. Verified with a real Docker build + run on a separate host, 4 launch scenarios: - --cap-drop ALL --security-opt no-new-privileges (contract met): starts normally. - No flags, default non-root user (10001 from the Dockerfile): blocked by the NoNewPrivs check. - --security-opt no-new-privileges only, no --cap-drop: starts -- correct, not a false block, since a non-root process already has an empty effective capability set regardless of --cap-drop. - --user root, no flags: blocked by the CapEff check, message shows the actual non-empty capability bitmask (00000000a80425fb). Local commit only -- this is an audit clone of openai/codex-security, not a fork with push access.
![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: d88aec5165
ℹ️ 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".
| exit 3 | ||
| fi | ||
| nnp=$(awk '/^NoNewPrivs:/{print $2}' /proc/self/status 2>/dev/null || true) | ||
| if [ "$nnp" != "1" ]; then |
There was a problem hiding this comment.
When the image is invoked by the bare docker run commands in .github/workflows/container-ci.yml lines 92-94 and 136, NoNewPrivs remains 0, so this new check exits with status 3 before --version, help, or discovery validation can run. The first failure aborts the container CI job, while the discovery test also explicitly expects status 2; update those invocations to supply the required security options or otherwise preserve the smoke-test behavior.
Useful? React with 👍 / 👎.
| "action": "SCMP_ACT_ALLOW", | ||
| "args": [{ "index": 0, "value": 38, "op": "SCMP_CMP_LT" }] | ||
| "comment": "AF_UNIX(1)/AF_INET(2)/AF_INET6(10) only: local IPC + HTTPS for git clone and the OpenAI API. Everything else (notably AF_NETLINK=16, AF_PACKET=17) is denied by defaultAction; the previous 3-rule form here only blocked AF_ALG=38/AF_VSOCK=40 and left NETLINK/PACKET/everything-else allowed.", | ||
| "args": [{ "index": 0, "value": 1, "op": "SCMP_CMP_EQ" }] |
There was a problem hiding this comment.
When Codex starts its Linux Bubblewrap sandbox with network isolation, Bubblewrap needs an AF_NETLINK socket to configure the loopback interface, but this allowlist now rejects that socket before RTM_NEWLINK/RTM_NEWADDR can run. The hardened-sandbox workflow in .github/workflows/container-ci.yml lines 122-126 explicitly recognizes this failure and survives only by manually rerunning with use_legacy_landlock; the normal scan path contains no equivalent retry or feature override, so container scans that execute sandboxed commands can fail rather than run.
Useful? React with 👍 / 👎.
| "comment": "AF_UNIX(1)/AF_INET(2)/AF_INET6(10) only: local IPC + HTTPS for git clone and the OpenAI API. Everything else (notably AF_NETLINK=16, AF_PACKET=17) is denied by defaultAction; the previous 3-rule form here only blocked AF_ALG=38/AF_VSOCK=40 and left NETLINK/PACKET/everything-else allowed.", | ||
| "args": [{ "index": 0, "value": 1, "op": "SCMP_CMP_EQ" }] |
There was a problem hiding this comment.
On the explicitly supported SCMP_ARCH_X86 subarchitecture, a 32-bit executable can create sockets through the multiplexed socketcall syscall, which remains unconditionally allowed at line 326, so these argument filters on the direct socket syscall are bypassed. A bundled or repository-supplied i386 executable can therefore still open AF_NETLINK and other domains that this hardening intends to deny; remove or constrain the legacy path as part of the allowlist.
Useful? React with 👍 / 👎.
…socketcall
Three fixes for the automated review findings on this PR:
1. .github/workflows/container-ci.yml: the bare `docker run` smoke-test
invocations (Verify bundled scanner, Reject interactive repository
discovery) never passed --cap-drop ALL --security-opt no-new-privileges,
so entrypoint.sh's new fail-closed check aborted them with exit 3 before
--version/help/discovery validation could run. Added the flags to match
the production contract these tests are supposed to exercise.
2. docker/codex-security-seccomp.json: the AF_UNIX/AF_INET/AF_INET6-only
socket() allowlist rejected AF_NETLINK, which Bubblewrap needs (a
NETLINK_ROUTE socket) to run RTM_NEWLINK/RTM_NEWADDR when configuring
the loopback interface inside its own isolated network namespace during
codex sandbox startup. The CI workflow already silently tolerated this
by retrying with --enable use_legacy_landlock; added an explicit
AF_NETLINK(16) rule restricted to protocol NETLINK_ROUTE(0) so the
normal Bubblewrap path succeeds without needing that fallback.
3. docker/codex-security-seccomp.json: "socketcall" was unconditionally
allowed in the generic syscall list with no argument filtering. On the
supported SCMP_ARCH_X86 subarchitecture, this legacy multiplexer lets a
32-bit executable open sockets (including AF_NETLINK/AF_PACKET) via
socketcall's own SYS_SOCKET dispatch, bypassing the domain-restricted
socket() rules entirely -- seccomp can't filter socketcall by socket
domain since that lives behind a pointer argument, not a plain integer,
so the safe fix is to stop allowing it unconditionally. socketpair (only
ever AF_UNIX by kernel definition) stays allowed.
Verified with a real Docker build + container runs on a separate host
(not the CI runner, not the openai org):
- Bare `docker run` (no flags): still fails closed with exit 3, unchanged.
- With the added flags: --version, bulk-scan --help, info --json all
exit 0; the discovery-rejection test still returns exit 2 with the
expected message.
- A/B on the sandbox seccomp: the pre-fix profile reproduces the exact
error from the review ("bwrap: loopback: Failed to create NETLINK_ROUTE
socket: Operation not permitted"); the fixed profile runs the same
`codex sandbox /usr/bin/true` command to completion (exit 0) without
the use_legacy_landlock fallback.
- Normal HTTPS egress (AF_INET, via Node's https module) still works
under the fixed profile, confirming the socketcall removal doesn't
regress git-clone/API access on x86_64.
1 participant
Summary
Three sandbox-hardening fixes found during a security audit of this repo,
all verified with real
docker build+docker run(not just readlocally) under the exact
compose.yamlsecurity contract:Remove
ptrace/process_vm_readv/process_vm_writevfromdocker/codex-security-seccomp.json. All sandboxed processes sharethe same uid (10001), so
ptracelet one process inspect/modifyanother's memory inside the sandbox -- inter-process isolation was
effectively void despite the profile's intent.
Replace the 3
socketdomain rules with an explicit allowlist ofAF_UNIX(1)/AF_INET(2)/AF_INET6(10). The previous rules(
domain<38,domain==39,domain>40) only actually blockedAF_ALG(38) andAF_VSOCK(40) --AF_NETLINK(16) andAF_PACKET(17)were allowed, along with everything else outside that narrow gap,
making the egress control close to a no-op.
Fail closed in
docker/entrypoint.shif not launched with--cap-drop ALL --security-opt no-new-privileges. The seccompprofile unconditionally allows
mount/pivot_root/unshare/setns/clonefor Codex's own unprivileged Bubblewrap sandboxing -- thosesyscalls can't be removed without breaking that feature. Their safety
depended entirely on a comment asserting the caller applies the
hardened contract (true for
compose.yaml, not enforced for a baredocker run). Now the entrypoint reads/proc/self/statusandrefuses to start (exit 3, clear message) unless effective capabilities
are empty and
NoNewPrivsis set.Verification (real Docker, not narrated)
Built the image from this branch and ran it under
--cap-drop ALL --security-opt no-new-privileges --security-opt seccomp=...:--help/--version: still work (exit 0).getaddrinfo) andgit cloneover HTTPS: still work-- confirms the socket fix doesn't break the scanner's own legitimate
network use.
AF_NETLINK/AF_PACKETsocket(): now fail withEPERM(previouslysucceeded).
ptrace(PTRACE_TRACEME, ...): fails withEPERM.--cap-drop ALL/--security-opt no-new-privileges: entrypoint refuses to start (exit 3) instead ofsilently running under a weaker contract than the seccomp profile
assumes. Tested both the no-flags case and
--user root(capabilitiescheck fires correctly with the real non-empty bitmask).
Test plan
previously-open socket domains (e.g.
AF_NETLINKfor somediagnostic path I didn't find)