Skip to content

[linux sandbox] Isolate IPC namespace in bubblewrap#20240

Open
evawong-oai wants to merge 1 commit intomainfrom
codex/bugb16097_unshare_ipc
Open

[linux sandbox] Isolate IPC namespace in bubblewrap#20240
evawong-oai wants to merge 1 commit intomainfrom
codex/bugb16097_unshare_ipc

Conversation

@evawong-oai
Copy link
Copy Markdown
Contributor

@evawong-oai evawong-oai commented Apr 29, 2026

Why

  • The Linux bubblewrap sandbox already isolates user and PID namespaces, and can isolate the network namespace, but it still shared the host IPC namespace.
  • That left same user System V IPC objects reachable from sandboxed commands, giving sandboxed code a host communication path outside filesystem and network controls.
  • Adding bubblewrap IPC isolation closes that enforcement gap without changing permission profile shape or app server protocol.
  • This is intentionally just the bubblewrap enforcement flag, so it should not get in the way of the current PermissionProfile migration for Linux enforcement or any later memory isolation design.

Summary

  • Add --unshare-ipc to the Linux bubblewrap argv when bubblewrap is active.
  • Update bubblewrap argv fixture expectations.
  • Add a Linux regression that seeds a host System V shared memory segment and verifies the sandboxed command cannot attach to it.
  • Keep the existing /dev/shm workflow validated.

Validation

  • Full validation automation on evawong-pr-validation: 13 passed, 0 failed, 1 skipped.
  • Latest PR head validation log: /home/dev-user/codex-bugb16097-pr-validation/logs/20260430T222251Z-pr20240-codex-comments-main.log.
  • Local just fmt.
  • Local git diff --check.
  • The remaining skipped smoke is UNIX socket creation under the active :workspace permissions profile. This is separate from System V IPC namespace isolation.

Tickets

@evawong-oai
Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown
Contributor

Codex Review: Didn't find any major issues. Chef's kiss.

ℹ️ 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".

Bojun-Vvibe added a commit to Bojun-Vvibe/oss-contributions that referenced this pull request Apr 29, 2026
…+20240

- anomalyco/opencode#24984 fix(core): reconnect editor context for session directory (merge-after-nits)
- openai/codex#20243 Add codex-core public API listing (merge-as-is)
- openai/codex#20240 [linux sandbox] Isolate IPC namespace in bubblewrap (merge-as-is)
@evawong-oai evawong-oai force-pushed the codex/bugb16097_unshare_ipc branch 2 times, most recently from 9163360 to eec6cc3 Compare April 29, 2026 19:10
@evawong-oai
Copy link
Copy Markdown
Contributor Author

@codex review

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

let requires_bubblewrap = !use_legacy_landlock
&& (!file_system_sandbox_policy.has_full_disk_write_access() || allow_network_for_proxy);

P1 Badge Reject WSL1 when IPC isolation requires bubblewrap

Update the WSL1 gate to consider memory IPC isolation. select_initial can now pick LinuxSeccomp for managed full-access profiles, and linux_run_main will invoke bwrap when memory_profile.should_unshare_ipc() is true. But this check still treats unrestricted/no-proxy as “no bwrap required”, so WSL1 paths pass validation and then fail later at runtime with unsupported bwrap behavior.


if use_legacy_landlock
&& file_system_sandbox_policy
.needs_direct_runtime_enforcement(network_sandbox_policy, sandbox_policy_cwd)

P1 Badge Reject legacy landlock when IPC isolation is required

Block --use-legacy-landlock when memory isolation is required. This guard only checks direct-runtime filesystem enforcement, so managed unrestricted profiles (now defaulting to isolate_ipc=true) are allowed in legacy mode, which skips bubblewrap and never adds --unshare-ipc, leaving host IPC shared contrary to the new default.


} => !file_system.to_sandbox_policy().has_full_disk_write_access() || !network.is_enabled(),

P2 Badge Account for memory isolation in unified-exec Windows gating

Include memory-based sandbox requirements in this predicate. After this change, managed unrestricted+network-enabled profiles still require platform sandbox (for IPC isolation), but this check treats them as sandbox-free and keeps UnifiedExec enabled on Windows. That bypasses the intended fallback path for sandbox-required managed sessions and can route commands through an incompatible backend.

ℹ️ 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".

@evawong-oai evawong-oai force-pushed the codex/bugb16097_unshare_ipc branch 8 times, most recently from 612c06f to cfe2e46 Compare April 30, 2026 21:58
@evawong-oai
Copy link
Copy Markdown
Contributor Author

Addressing the Codex review above.

  1. The review was written against commit eec6cc3, when this PR still carried the broader memory permissions design.
  2. The current head cfe2e46 no longer changes PermissionProfile, app server protocol, WSL1 gating, legacy landlock gating, or UnifiedExec gating.
  3. The current scope is only the Linux bubblewrap IPC namespace flag plus Linux sandbox tests.
  4. Because the review findings all depended on the removed memory permissions design, there is no code change to carry forward for those findings in this PR.

@evawong-oai
Copy link
Copy Markdown
Contributor Author

@codex review

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cfe2e46620

ℹ️ 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".

Comment thread codex-rs/linux-sandbox/src/bwrap.rs
Comment thread codex-rs/linux-sandbox/tests/suite/landlock.rs
Comment thread codex-rs/linux-sandbox/tests/suite/landlock.rs
@evawong-oai evawong-oai force-pushed the codex/bugb16097_unshare_ipc branch from cfe2e46 to c3ff56b Compare April 30, 2026 22:22
@evawong-oai evawong-oai marked this pull request as ready for review April 30, 2026 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant