Skip to content

chore(server): always mount runtime volume on egress sidecar#1072

Open
Pangjiping wants to merge 4 commits into
mainfrom
fix/egress-always-mount-runtime-volume
Open

chore(server): always mount runtime volume on egress sidecar#1072
Pangjiping wants to merge 4 commits into
mainfrom
fix/egress-always-mount-runtime-volume

Conversation

@Pangjiping

Copy link
Copy Markdown
Collaborator

Summary

  • Always mount /opt/opensandbox shared volume on the egress sidecar, not just when credential_proxy_enabled=True
  • Fixes manual MITM scenario: user sets OPENSANDBOX_EGRESS_MITMPROXY_TRANSPARENT=true via env without credential proxy → egress exports CA cert to shared volume → bootstrap.sh can read it
  • Previously the volume wasn't mounted without credential proxy, so CA cert was written to a different filesystem and sandbox timed out waiting for it
  • Added test_always_mounts_runtime_volume test

Addresses review feedback from #1069.

Test plan

  • test_always_mounts_runtime_volume — verifies volumeMount present without credential proxy
  • Existing test_contains_transparent_mitm_env_when_credential_proxy_enabled — still passes
  • All 21 egress helper tests pass

🤖 Generated with Claude Code

The shared /opt/opensandbox volume was only mounted on the egress
sidecar when credential_proxy_enabled was true. When users enable
transparent MITM manually via OPENSANDBOX_EGRESS_MITMPROXY_TRANSPARENT
env var without credential proxy, the egress process exports its CA
cert to /opt/opensandbox/mitmproxy-ca-cert.pem but the sandbox
container's bootstrap.sh waits for that same path on a different
filesystem, causing a timeout and HTTPS certificate validation failure.

Mount the runtime volume unconditionally whenever the egress sidecar
exists — the volume is already present in the pod spec from the execd
init container, so this is a no-op for non-MITM scenarios.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Pangjiping Pangjiping changed the title fix(server): always mount runtime volume on egress sidecar chore(server): always mount runtime volume on egress sidecar Jun 15, 2026
Mirror the Kubernetes change: create the shared runtime volume whenever
an egress sidecar exists, not only when credential proxy is enabled.
This ensures manual MITM via OPENSANDBOX_EGRESS_MITMPROXY_TRANSPARENT
env var works in Docker mode — the CA cert can reach the sandbox
container through the shared volume.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

if runtime_volume_name:
volume_binds.append(
f"{runtime_volume_name}:{OPENSANDBOX_RUNTIME_MOUNT_PATH}:rw"

P2 Badge Avoid duplicate mounts at the runtime path

When a Docker sandbox uses networkPolicy and also supplies a request volume mounted at /opt/opensandbox, _build_volume_binds() has already added that user mount before this unconditional runtime bind is appended. That produces two Docker binds with the same destination, so network-policy sandboxes without credentialProxy that previously worked can now fail at container creation with a duplicate mount target; the runtime path should be reserved/validated or de-duplicated before appending this bind.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/opensandbox_server/services/docker/docker_service.py
Comment thread server/opensandbox_server/services/docker/docker_service.py
…time bind

- _expire_sandbox() now reads SANDBOX_MANAGED_VOLUMES_LABEL and calls
  _cleanup_managed_volumes(), matching delete_sandbox() behavior.
  Previously runtime volumes leaked when sandboxes expired via timer.
- Deduplicate runtime volume bind: skip appending the runtime mount
  if a user-supplied volume already targets /opt/opensandbox, avoiding
  Docker's duplicate mount target rejection.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Pangjiping

Copy link
Copy Markdown
Collaborator Author

Re: "Avoid duplicate mounts at the runtime path" — Fixed in 221e99a. Before appending the runtime volume bind, we now check if any existing bind already targets /opt/opensandbox. If a user-supplied volume is already mounted there, the runtime bind is skipped.

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

Copy link
Copy Markdown

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: 221e99afe6

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/opensandbox_server/services/docker/docker_service.py
Substring check `:/opt/opensandbox` false-positives on paths like
`/opt/opensandbox-cache`. Match `:/opt/opensandbox:` (with options
suffix) or bind ending with `:/opt/opensandbox` (no options) instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants