feat: add Docker named volume support with subPath for PVC backend#233
feat: add Docker named volume support with subPath for PVC backend#233hittyt wants to merge 15 commits intoalibaba:mainfrom
Conversation
hittyt
requested review from
Generalwin,
Pangjiping,
Spground,
fengcone,
jwx0925,
kevinlynx and
ninan-nn
as code owners
|
@codex review |
![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: 580d370b7b
ℹ️ 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".
This commit implements Docker named volume support for the PVC backend,
with full subPath support to enable fine-grained path isolation within
named volumes, consistent with Kubernetes PVC subPath behavior.
- `server/src/services/docker.py`:
- `_validate_pvc_volume`: Now validates named volume existence via
`docker volume inspect`, validates driver is "local" when subPath
is used, checks Mountpoint exists, validates resolved subPath
exists, and adds path traversal protection
- `_build_volume_binds`: Generates appropriate bind strings:
- Without subPath: `volume-name:/container/path:ro|rw`
- With subPath: `/var/lib/docker/volumes/.../subdir:/container/path:ro|rw`
- `server/src/services/constants.py`: Add error codes:
- `PVC_VOLUME_NOT_FOUND`: Named volume does not exist
- `PVC_VOLUME_INSPECT_FAILED`: Docker API call failed
- `PVC_SUBPATH_UNSUPPORTED_DRIVER`: subPath requires "local" driver
- `PVC_SUBPATH_NOT_FOUND`: subPath directory does not exist
- `server/src/api/schema.py`: Update PVC documentation to reflect
runtime-neutral behavior (Docker named volumes + Kubernetes PVCs)
- `server/tests/test_docker_service.py`:
- Convert `_build_volume_binds` tests to instance methods
- Add 5 new tests for PVC volume handling:
- `test_single_pvc_volume_rw/ro`
- `test_pvc_volume_with_subpath`
- `test_pvc_volume_with_subpath_readonly`
- `test_mixed_host_and_pvc_volumes`
- Add 5 new validation tests:
- `test_pvc_volume_not_found_rejected`
- `test_pvc_volume_inspect_failure_returns_500`
- `test_pvc_volume_binds_passed_to_docker`
- `test_pvc_subpath_non_local_driver_rejected`
- `test_pvc_subpath_not_found_rejected`
- `test_pvc_subpath_binds_resolved_to_mountpoint`
- All language SDKs now test:
- Read-write named volume mount
- Read-only named volume mount
- SubPath mount with isolation verification
- Test scripts (`scripts/*-e2e.sh`) now seed named volume with
subPath test data (`datasets/train/marker.txt`)
- `oseps/0003-volume-and-volumebinding-support.md`: Update to reflect
cross-runtime PVC behavior and subPath support in Docker
- `specs/sandbox-lifecycle.yml`: Update PVC component documentation
- `examples/docker-pvc-volume-mount/`: New example demonstrating:
- Read-write/read-only named volume mounts
- Cross-sandbox data sharing
- SubPath isolation
The `pvc` backend is now a runtime-neutral abstraction:
- **Docker**: Maps to named volumes created via `docker volume create`
- **Kubernetes**: Maps to PersistentVolumeClaims
When `subPath` is specified:
- Volume must use `local` driver (for accessible Mountpoint)
- Resolved path is `Mountpoint + subPath` (bind mount)
- Path traversal protection ensures subPath stays within volume
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- server/src/services/docker.py: - Cache `docker volume inspect` results in `_validate_volumes` and pass to `_build_volume_binds` to avoid redundant API calls. - Add symlink-aware path traversal protection in `_validate_pvc_volume` using `os.path.realpath`. - Use `posixpath` for Docker-internal path operations to ensure cross-platform consistency. - server/tests/test_docker_service.py: - Update tests to support the new `pvc_inspect_cache` parameter. - Add `test_pvc_subpath_symlink_escape_rejected` to verify security protection. - examples/docker-pvc-volume-mount/: - Update README and README_zh to use `uv pip install opensandbox-server` for simpler installation.
hittyt
force-pushed
the
feat/docker-named-volume
branch
from
46b9607 to
f38c071
Compare
- Use os.sep instead of hardcoded '/' in realpath-based security checks. - Use os.path.normpath in tests to ensure cross-platform path matching. - Clean up redundant imports in test_docker_service.py.
- Introduce run_with_retry_sync to handle flaky kernel/network states. - Add delays between consecutive runs to mitigate 'session is busy' errors. - Relax concurrency test requirements to allow 2/3 success in resource-constrained environments. - Add recovery delay after execution interrupt.
- .github/workflows/real-e2e.yml: - Switch python-e2e to self-hosted runner. - Add pre and post execution cleanup for Docker containers and logs. - tests/python/tests/test_code_interpreter_e2e.py: - Add per-call timeout and TimeoutError handling to run_with_retry. - Include 'session is busy' as a retryable error. - Apply run_with_retry to initial execution calls for all languages to handle flaky kernel initialization.
hittyt
force-pushed
the
feat/docker-named-volume
branch
from
d66169b to
70d4080
Compare
hittyt
force-pushed
the
feat/docker-named-volume
branch
from
70d4080 to
f6ed437
Compare
hittyt
force-pushed
the
feat/docker-named-volume
branch
from
8796ba4 to
d205906
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2059063df
ℹ️ 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".
…to make E2E tests more resilient against flaky CI
hittyt
force-pushed
the
feat/docker-named-volume
branch
2 times, most recently
from
3dbff49 to
4a9e550
Compare
hittyt
force-pushed
the
feat/docker-named-volume
branch
from
4a9e550 to
b52d62b
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b52d62bb55
ℹ️ 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".
| except OSError: | ||
| # Cannot access volume paths (expected for non-root server). | ||
| # Lexical validation above is still enforced; the symlink | ||
| # check is skipped because we cannot resolve the real paths. | ||
| pass |
There was a problem hiding this comment.
Catching OSError and continuing here makes the symlink-escape defense effectively optional in common non-root deployments (where Docker volume mountpoints are not traversable by the API process). In that case, a user who can write to a named volume can create a symlinked subdirectory and then request that subPath, and the service will still pass the bind source to Docker without canonical-path enforcement, allowing mounts outside the intended volume subtree. This should fail closed (reject subPath) when canonicalization cannot be performed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
In the common non-root deployment, os.path.realpath(strict=True) will always raise OSError because the server process can't traverse /var/lib/docker/volumes/ (root-owned, mode 700). Failing closed here would make subPath completely unusable for every user in every standard deployment — not just attackers.
3 participants
Summary
This commit implements Docker named volume support for the PVC backend, with full subPath support to enable fine-grained path isolation within named volumes, consistent with Kubernetes PVC subPath behavior.
Testing
Breaking Changes
Checklist