feat: add Docker named volume support with subPath for PVC backend #233
Conversation
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.
## Changes
### Server Core
- `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)
### Tests
- `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`
### E2E Tests (Java, JavaScript, Python)
- 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`)
### Documentation
- `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
## Design Notes
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>
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".
| resolved_path = os.path.normpath( | ||
| os.path.join(mountpoint, volume.sub_path) | ||
| ) | ||
| # Defense in depth: ensure resolved path stays within the mountpoint | ||
| if not resolved_path.startswith(mountpoint): |
There was a problem hiding this comment.
This check is only lexical (normpath + startswith) and does not resolve symlinks inside the named volume. In a multi-tenant flow where an attacker can write into the volume first, creating a symlink like datasets -> / and then requesting subPath: datasets/etc will pass validation here, and the later bind mount will target host paths outside the volume root (potential host file exposure/modification depending on readOnly). Resolve and validate the canonical path (realpath) against the canonical mountpoint (with path-boundary checks) before allowing subPath mounts.
Useful? React with 👍 / 👎.
| vol_info = self.docker_client.api.inspect_volume( | ||
| volume.pvc.claim_name | ||
| ) |
There was a problem hiding this comment.
This second inspect_volume call is not wrapped in the same DockerNotFound/DockerException handling used in _validate_pvc_volume. If the volume disappears or the daemon errors after validation (a realistic race in shared Docker environments), sandbox creation fails with an uncaught exception and returns a generic 500 instead of the structured volume error contract. Catch and translate these errors here (or reuse inspection data captured during validation).
Useful? React with 👍 / 👎.
| ### 1. Start OpenSandbox Server | ||
|
|
||
| ```shell | ||
| git clone git@github.com:alibaba/OpenSandbox.git |
There was a problem hiding this comment.
start server with opensandbox-server CLI, you could refer to how the other examples are written.
| sh -c "echo 'hello-from-named-volume' > /data/marker.txt" | ||
| ``` | ||
|
|
||
| > **Note**: Unlike `host` volumes, `pvc` volumes do not require any `[storage]` configuration on the server side. |
There was a problem hiding this comment.
this sentence seems a bit hard to understand. Maybe this example doesn’t need to be conflated with unrelated configurations?
| @@ -0,0 +1,259 @@ | |||
| # Docker PVC（命名卷）挂载示例 | |||
There was a problem hiding this comment.
Maybe we don’t need to provide bilingual versions for every README.
| }, | ||
| ) | ||
|
|
||
| resolved_path = os.path.normpath( |
There was a problem hiding this comment.
Is this incompatible with servers running on Windows? You could wait until #234 is merged, then fixed this.
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