Make sandlock run a drop-in for docker run#61
Open
yunwei37 wants to merge 2 commits into
Open
Conversation
`sandlock run` now accepts Docker's command-line shape: the first positional is the image and the rest is the command, so `docker run ... IMAGE CMD` can usually be swapped for `sandlock run ... IMAGE CMD` unchanged — the container runs confined by Landlock + seccomp instead of namespaces, with no root and no daemon. CLI changes (sandlock-cli): - First positional = image (Docker mode). A `--` terminator selects native host-command mode (`sandlock run [flags] -- CMD`), preserving full access to the security flags. `--image` stays as an explicit alternative. - Add Docker flags: -v/--volume (HOST:CONTAINER[:ro|:rw]) -> per-sandbox mount + fs access, -w/--workdir -> cwd, -p/--publish -> net-bind + port-remap, -u/--user -> uid, -e/--env (+ --env-file) -> env, --hostname -> sandbox name, --entrypoint, --cpus -> cpu count, --network none|host, -t/--tty, --rm. --detach/--privileged/ --cap-add/--cap-drop/--pull are accepted for compatibility but ignored. - Repurpose colliding short flags to Docker meanings (-t/-e/-p/-w); the native equivalents remain as long options (--timeout, --exec-shell, --profile, --fs-write). The COW storage dir moves from --workdir to --fs-workdir. Image config (sandlock-core): - Add image::inspect_config to read ENTRYPOINT, CMD, WorkingDir, Env and User from the image and apply them like `docker run` (image defaults sit below CLI overrides). The image rootfs stays read-only (rootless sandboxing has no privileged overlay mount); use -v for writable paths or opt into --fs-isolation. Tests: docker-mode arg-splitting, volume/publish/user parsing, image config precedence, and Docker-gated integration tests (skip when no Docker daemon). README documents the drop-in usage and the flag changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR adds Docker-compatible sandlock run behavior (image-first positional + common Docker flags) while resolving flag collisions with sandlock-native options, and extends image inspection to apply baked-in defaults (env/workdir/user/entrypoint/cmd).
Changes:
- Introduces Docker-style positional parsing (
IMAGE [CMD...]) plus Docker-compatible flags (-v/-w/-p/-u/-e,--env-file,--network,--cpus, etc.) in the CLI. - Adds
ImageConfig+inspect_config()to read additional Docker imageConfigfields and derive default command precedence. - Updates docs/tests and renames/adjusts conflicting flags (
--fs-write,--fs-workdir,--timeout,--profile, etc.).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/sandlock-core/src/sandbox.rs | Adjusts clap-exposed flag names to avoid collisions with Docker-compatible short/long flags. |
| crates/sandlock-core/src/image.rs | Adds ImageConfig, expands Docker metadata inspection, and introduces JSON parsing helpers + tests. |
| crates/sandlock-cli/src/main.rs | Implements Docker-compatible run parsing/flags and maps them onto sandbox policy builder behavior. |
| crates/sandlock-cli/tests/cli_test.rs | Updates flag usage and adds integration tests for Docker-compat CLI behavior (skipping when Docker unavailable). |
| README.md | Updates examples and documents the new Docker-compatible CLI and renamed flags. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+196
to
203
| /// Parse a JSON string literal like `"abc"` (or `null`) into its value. | ||
| fn parse_json_string(s: &str) -> Option<String> { | ||
| let s = s.trim(); | ||
| if s == "null" || s.len() < 2 || !s.starts_with('"') || !s.ends_with('"') { | ||
| return None; | ||
| } | ||
| Some(s[1..s.len() - 1].replace("\\\"", "\"").replace("\\\\", "\\")) | ||
| } |
Comment on lines
162
to
178
| /// Inspect a local Docker image's `Config`, returning the fields sandlock maps | ||
| /// onto its sandbox (entrypoint, cmd, working dir, env, user). | ||
| /// | ||
| /// On any inspection failure this returns a default config so callers can fall | ||
| /// back to running `/bin/sh`. | ||
| pub fn inspect_config(image: &str) -> Result<ImageConfig, SandlockError> { | ||
| // One field per line keeps parsing simple and avoids delimiter clashes with | ||
| // values that may themselves contain `|`. | ||
| let output = Command::new("docker") | ||
| .args([ | ||
| "inspect", "--format", | ||
| "{{json .Config.Entrypoint}}|{{json .Config.Cmd}}", | ||
| "{{json .Config.Entrypoint}}\n{{json .Config.Cmd}}\n\ | ||
| {{json .Config.WorkingDir}}\n{{json .Config.Env}}\n{{json .Config.User}}", | ||
| image, | ||
| ]) | ||
| .output() | ||
| .map_err(|_| SandboxRuntimeError::Child("docker inspect failed".into()))?; |
Comment on lines
+191
to
+205
| /// Did the invocation use the `--` options terminator? When present, the | ||
| /// trailing positionals are a host command (native mode) rather than a Docker | ||
| /// `IMAGE [CMD...]` sequence. | ||
| fn had_options_terminator() -> bool { | ||
| std::env::args().any(|a| a == "--") | ||
| } | ||
|
|
||
| /// Decide whether we are running a Docker image or a host command, and split | ||
| /// the trailing positionals accordingly. | ||
| /// | ||
| /// * `--image IMG` → image mode; positionals are the command. | ||
| /// * `... -- CMD` → native mode; positionals are the host command, no image. | ||
| /// * `IMG [CMD...]` → Docker mode; first positional is the image. | ||
| fn resolve_run_target(args: &RunArgs) -> RunTarget { | ||
| split_run_target(args.image.as_deref(), &args.image_and_cmd, had_options_terminator()) |
Comment on lines
+664
to
+682
| // ── Image: extract rootfs, then layer in the image's baked-in config ────── | ||
| // (env, working dir, user) the way `docker run` does. The image's defaults | ||
| // sit at the lowest precedence; the CLI flags below override them. | ||
| let image_config = if let Some(ref img) = target.image { | ||
| let rootfs = sandlock_core::image::extract(img, None)?; | ||
| builder = builder.chroot(rootfs).fs_read("/"); | ||
| Some(sandlock_core::image::inspect_config(img)?) | ||
| } else { | ||
| None | ||
| }; | ||
| let mut effective_cwd: Option<String> = None; | ||
| let mut effective_uid: Option<u32> = None; | ||
| if let Some(ref cfg) = image_config { | ||
| for kv in &cfg.env { | ||
| if let Some((k, v)) = kv.split_once('=') { builder = builder.env_var(k, v); } | ||
| } | ||
| effective_cwd = cfg.workdir.clone(); | ||
| effective_uid = cfg.user.as_deref().and_then(parse_user); | ||
| } |
Comment on lines
957
to
+967
| if args.status_fd.is_some() { bad.push("--status-fd"); } | ||
| if !pb.fs_denied.is_empty() { bad.push("--fs-deny"); } | ||
| if !args.fs_mount.is_empty() { bad.push("--fs-mount"); } | ||
| // Docker-compatible flags that map onto supervisor-only features. | ||
| if !args.volume.is_empty() { bad.push("--volume"); } | ||
| if !args.publish.is_empty() { bad.push("--publish"); } | ||
| if args.docker_workdir.is_some() { bad.push("--workdir"); } | ||
| if args.user.is_some() { bad.push("--user"); } | ||
| if args.hostname.is_some() { bad.push("--hostname"); } | ||
| if args.network.is_some() { bad.push("--network"); } | ||
| if args.cpus.is_some() { bad.push("--cpus"); } |
Comment on lines
+744
to
+745
| if n <= 0.0 { return Err(anyhow!("--cpus must be positive, got: {}", n)); } | ||
| builder = builder.max_cpu(n.ceil() as u8); |
Comment on lines
+699
to
+700
| } else if let Ok(v) = std::env::var(spec) { | ||
| // Docker: `-e VAR` (no `=value`) forwards the host's value. |
Contributor
|
Thanks for the PR. A quick note: it is hard to be 100% compatible with Docker CLI. Going to this direction is okay. One small request: maybe we should use '-W' for '--fs-write', which is shorter? |
- image.rs: parse `docker inspect` output with serde_json so all JSON escapes are decoded correctly; fall back to a default ImageConfig when `docker` cannot be executed (matching the docstring). - main.rs: detect native (host-command) mode by a *leading* `--` terminator only, so `sandlock run IMAGE -- args` keeps IMAGE as the image instead of misclassifying it as a host command. - main.rs: layer image defaults below the profile (cwd/uid/env) so a profile's settings are no longer clobbered by image defaults; CLI flags still win. - main.rs: reject non-finite/out-of-range --cpus values before casting. - main.rs: warn instead of silently dropping `-e VAR` when VAR is unset in the host environment (both supervisor and no-supervisor paths). - main.rs: report the storage-dir flag as --fs-workdir in the --no-supervisor incompatibility message. - sandbox.rs: give --fs-write the uppercase short flag -W. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes
sandlock runaccept Docker's command-line shape so you can usually swapdocker run … IMAGE CMDforsandlock run … IMAGE CMDunchanged — the workload runs confined by Landlock + seccomp instead of namespaces, with no root and no daemon. This is the "podman-style drop-in" idea: keep the muscle memory, change only the binary name.What changed
CLI shape (
sandlock-cli)--terminator selects the native host-command mode (sandlock run [flags] -- CMD), which keeps full access to the security flags.--imageremains as an explicit alternative.-v/--volume HOST:CONTAINER[:ro|:rw]→ per-sandbox mount + fs read/write-w/--workdir→ working directory (cwd)-p/--publish→ net-bind + port virtualization-u/--user→ uid-e/--env,--env-file→ environment--hostname→ sandbox name,--entrypoint,--cpus→ CPU count,--network none|host,-t/--tty,--rm--detach/--privileged/--cap-add/--cap-drop/--pullare accepted for compatibility but ignored (warn).Image config (
sandlock-core)image::inspect_confignow readsENTRYPOINT,CMD,WorkingDir,Env, andUserand applies them the waydocker rundoes (image defaults sit below CLI overrides).Backward-incompatible flag changes
Because the first positional is now the image, the colliding short flags follow Docker. The native equivalents remain as long options:
-t--timeout-e--exec-shell-p--profile-w--fs-writeThe COW storage directory moved from
--workdirto--fs-workdir(since--workdiris now Docker's working directory).Limitations
The image rootfs is mounted read-only — rootless sandboxing has no privileged overlay mount, and the userspace branchfs needs a custom kernel FS. Use
-vfor writable host paths, or--fs-isolation overlayfs --fs-workdir DIRwhere privileges allow. Documented in the README.Testing
-e/-w, image default PATH,-vread-only) that skip cleanly when no Docker daemon is present.sandlock-cli(unit +cli_test+profile_integration) andsandlock-coreimage tests pass.🤖 Generated with Claude Code