feat(cli,rest): expose privileged mode via --privileged and self-hosted REST - #1205
Open
G4614 wants to merge 38 commits into
Open
feat(cli,rest): expose privileged mode via --privileged and self-hosted REST#1205G4614 wants to merge 38 commits into
G4614 wants to merge 38 commits into
Conversation
📦 BoxLite review — couldn't completepowered by BoxLite |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughPrivileged container support spans API schemas, REST and CLI entrypoints, runtime validation, server capability checks, host-resolved security settings, protobuf transport, guest version validation, and OCI specification generation. ChangesPrivileged mode
Estimated code review effort: 4 (Complex) | ~60 minutes Mergeability Score: ⚪ Minimal · up to The change is merge-ready after normal checks and review; no actionable merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant CLI
participant Runtime
participant RESTServer
participant Portal
participant Guest
CLI->>Runtime: request privileged box
Runtime->>Runtime: validate and normalize capabilities
Runtime->>RESTServer: verify privileged_enabled
Runtime->>Portal: send resolved security configuration
Portal->>Guest: initialize container
Guest->>Guest: build OCI specification
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/cli/src/commands/serve/mod.rs (1)
757-786: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winNormalize privileged options before validation.
BoxOptions::sanitize()only validates the value. It does not callnormalize_privileged(). Therefore, a request with{"advanced":{"privileged":true}}leavescapabilitiesempty, and the added test fails at Lines 1360-1361.Make
optionsmutable. If privileged mode is enabled, calloptions.advanced.set_privileged(true)beforeoptions.sanitize()?. This expands an empty or canonical policy without hiding conflicting overrides.Proposed fix
- let options = BoxOptions { + let mut options = BoxOptions { // ... }; + if options.advanced.privileged { + options.advanced.set_privileged(true); + } options.sanitize()?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/src/commands/serve/mod.rs` around lines 757 - 786, Make the BoxOptions value mutable, and before calling sanitize(), check whether options.advanced.privileged is enabled and invoke options.advanced.set_privileged(true). Keep explicit capability overrides intact while ensuring privileged requests with empty or canonical capabilities are normalized before validation.src/boxlite/src/rest/runtime.rs (1)
150-161: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftReject incompatible existing boxes before reuse.
RestRuntime::get_or_createcan return a non-privileged box for a privileged request becauseBoxResponsedoes not expose the existing policy. Expose an immutable policy indicator and reject incompatible reuse. Add a regression test for this case.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/boxlite/src/rest/runtime.rs` around lines 150 - 161, Update RestRuntime::get_or_create to compare the requested privilege policy with the existing box before returning it, rejecting incompatible reuse and creating a new box instead or propagating the established validation error. Expose an immutable policy indicator on BoxResponse for this comparison, and add a regression test covering a privileged request encountering an existing non-privileged box.src/boxlite/src/runtime/rt_impl.rs (2)
547-577: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReuse compatibility check does not fully guard against granting unconfined mounts beyond what was requested.
check_options_compatibilityonly rejects reuse when the request needsprivilegedand the existing box lacks it. It does not reject the reverse case where the request setscap_add=["ALL"]withprivileged=falseand the existing box is privileged. In that case,capabilities.check_compatibilitysees an exact match (both canonicalize toadd=["ALL"], drop=[]), the privileged check passes becauserequested.advanced.privilegedisfalse, and the caller silently receives a box with clearedmasked_paths/readonly_pathsand a writable/sysmount that it never asked for.Compare
requested.advanced.privilegedagainstactual_advanced.privilegedfor exact equality instead of the current one-directional check, so a non-privileged request can never adopt a privileged box regardless of how its capability policy happens to canonicalize.🛡️ Proposed fix for exact privileged-flag comparison
- if requested.advanced.privileged && !actual_advanced.privileged { + if requested.advanced.privileged != actual_advanced.privileged { return Err(BoxliteError::Unsupported(format!( "box '{box_name}' was created without privileged support and cannot satisfy a required privileged request; use a different name or recreate the box" ))); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/boxlite/src/runtime/rt_impl.rs` around lines 547 - 577, Update check_options_compatibility to require exact equality between requested.advanced.privileged and actual_advanced.privileged, replacing the current one-directional privileged-only rejection. Preserve the existing nested virtualization and capability compatibility checks, and return the same unsupported error behavior for either privileged-flag mismatch.
1220-1230: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winValidate clone options before
provision_box.
src/boxlite/src/litebox/clone_export.rs:128passesself.config.options.clone()without validation.provision_boxthen leaves a conflicting capability policy unchanged and persists the invalidBoxConfig. Validate the copied options before normalization, or at theprovision_boxboundary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/boxlite/src/runtime/rt_impl.rs` around lines 1220 - 1230, Validate the copied BoxOptions at the provision_box boundary before calling options.advanced.normalize_privileged(), ensuring conflicting capability policies are rejected or corrected before the resulting BoxConfig is persisted. This must also cover callers such as the clone flow that pass self.config.options.clone().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openapi/reference-server/server.py`:
- Around line 166-170: Update the privileged capability validation around
self.capabilities.add to canonicalize its single capability entry before
comparing the canonical shape, accepting case-insensitive “all” and the
“CAP_ALL” form. Preserve the existing rejection for non-canonical combinations,
and add reference-server tests covering both all and CAP_ALL inputs.
In `@src/boxlite/src/runtime/advanced_options.rs`:
- Around line 753-768: Update set_privileged so capability clearing occurs only
when transitioning from privileged to non-privileged: capture the previous
self.privileged state before assigning enabled, then require that prior state
and is_privileged_capability_shape() before resetting capabilities. Preserve
normalization when enabling and leave independently configured capabilities
unchanged when already non-privileged.
In `@src/guest/src/container/spec.rs`:
- Line 175: Update build_standard_mounts and the privileged mount-policy logic
to keep the guest cgroup2 hierarchy read-only: add a cgroup namespace or
equivalent isolated hierarchy before permitting writes, or exclude
/sys/fs/cgroup from the recursive /sys bind and preserve recursive read-only
behavior. Revise the related tests around cgroup namespace setup and the policy
assertions so they reject the unsafe no-rro configuration.
---
Outside diff comments:
In `@src/boxlite/src/rest/runtime.rs`:
- Around line 150-161: Update RestRuntime::get_or_create to compare the
requested privilege policy with the existing box before returning it, rejecting
incompatible reuse and creating a new box instead or propagating the established
validation error. Expose an immutable policy indicator on BoxResponse for this
comparison, and add a regression test covering a privileged request encountering
an existing non-privileged box.
In `@src/boxlite/src/runtime/rt_impl.rs`:
- Around line 547-577: Update check_options_compatibility to require exact
equality between requested.advanced.privileged and actual_advanced.privileged,
replacing the current one-directional privileged-only rejection. Preserve the
existing nested virtualization and capability compatibility checks, and return
the same unsupported error behavior for either privileged-flag mismatch.
- Around line 1220-1230: Validate the copied BoxOptions at the provision_box
boundary before calling options.advanced.normalize_privileged(), ensuring
conflicting capability policies are rejected or corrected before the resulting
BoxConfig is persisted. This must also cover callers such as the clone flow that
pass self.config.options.clone().
In `@src/cli/src/commands/serve/mod.rs`:
- Around line 757-786: Make the BoxOptions value mutable, and before calling
sanitize(), check whether options.advanced.privileged is enabled and invoke
options.advanced.set_privileged(true). Keep explicit capability overrides intact
while ensuring privileged requests with empty or canonical capabilities are
normalized before validation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e51ba822-e4c5-4331-b434-14d7f1a2a4bd
📒 Files selected for processing (27)
docs/reference/README.mdopenapi/box.openapi.yamlopenapi/reference-server/server.pyopenapi/reference-server/tests/test_handle_cache.pysrc/boxlite/src/experimental.rssrc/boxlite/src/litebox/init/tasks/guest_init.rssrc/boxlite/src/portal/interfaces/container.rssrc/boxlite/src/rest/client.rssrc/boxlite/src/rest/runtime.rssrc/boxlite/src/rest/types.rssrc/boxlite/src/runtime/advanced_options.rssrc/boxlite/src/runtime/import.rssrc/boxlite/src/runtime/options.rssrc/boxlite/src/runtime/rt_impl.rssrc/cli/src/cli.rssrc/cli/src/commands/create.rssrc/cli/src/commands/run.rssrc/cli/src/commands/serve/handlers/config.rssrc/cli/src/commands/serve/mod.rssrc/cli/src/commands/serve/types.rssrc/guest/src/container/capabilities.rssrc/guest/src/container/lifecycle.rssrc/guest/src/container/mod.rssrc/guest/src/container/spec.rssrc/guest/src/container/start.rssrc/guest/src/service/container.rssrc/shared/proto/boxlite/v1/service.proto
Comment on lines
+166
to
+170
| canonical = self.capabilities.drop == [] and self.capabilities.add == ["ALL"] | ||
| if (self.capabilities.add or self.capabilities.drop) and not canonical: | ||
| raise ValueError("privileged mode cannot be combined with cap_add or cap_drop") | ||
|
|
||
| self.capabilities.add = ["ALL"] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Canonicalize the privileged capability shape.
Line 166 requires the exact spelling "ALL". A request with add=["all"] is valid under the documented case-insensitive capability rules, but this validator rejects it when privileged=true. The Rust runtime canonicalizes capability names before it checks this shape.
Canonicalize the single entry before comparison. Cover all and CAP_ALL in this reference-server test suite.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openapi/reference-server/server.py` around lines 166 - 170, Update the
privileged capability validation around self.capabilities.add to canonicalize
its single capability entry before comparing the canonical shape, accepting
case-insensitive “all” and the “CAP_ALL” form. Preserve the existing rejection
for non-canonical combinations, and add reference-server tests covering both all
and CAP_ALL inputs.
Comment on lines
+753
to
+768
| /// Toggle privileged mode, keeping the capability policy consistent in | ||
| /// both directions. | ||
| /// | ||
| /// Enabling expands to the canonical shape; disabling withdraws it again, | ||
| /// so a handle that is toggled off does not leave a non-privileged box | ||
| /// holding `ALL`. An explicit policy the caller set themselves is left | ||
| /// alone — only the shape this method produced is taken back. | ||
| pub fn set_privileged(&mut self, enabled: bool) { | ||
| self.privileged = enabled; | ||
|
|
||
| if enabled { | ||
| self.normalize_privileged(); | ||
| } else if self.capabilities.is_privileged_capability_shape() { | ||
| self.capabilities = ContainerCapabilities::default(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix set_privileged(false) to only clear capabilities when transitioning away from privileged mode.
set_privileged(false) clears capabilities whenever self.capabilities.is_privileged_capability_shape() is true, regardless of whether self.privileged was already false. If a caller calls set_privileged(false) while privileged is already false and capabilities were independently set to add=["ALL"], this silently discards that explicit request. Track whether privileged mode was actually enabled before clearing.
🐛 Proposed fix to guard the clear on an actual privileged→non-privileged transition
pub fn set_privileged(&mut self, enabled: bool) {
+ let was_privileged = self.privileged;
self.privileged = enabled;
if enabled {
self.normalize_privileged();
- } else if self.capabilities.is_privileged_capability_shape() {
+ } else if was_privileged && self.capabilities.is_privileged_capability_shape() {
self.capabilities = ContainerCapabilities::default();
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// Toggle privileged mode, keeping the capability policy consistent in | |
| /// both directions. | |
| /// | |
| /// Enabling expands to the canonical shape; disabling withdraws it again, | |
| /// so a handle that is toggled off does not leave a non-privileged box | |
| /// holding `ALL`. An explicit policy the caller set themselves is left | |
| /// alone — only the shape this method produced is taken back. | |
| pub fn set_privileged(&mut self, enabled: bool) { | |
| self.privileged = enabled; | |
| if enabled { | |
| self.normalize_privileged(); | |
| } else if self.capabilities.is_privileged_capability_shape() { | |
| self.capabilities = ContainerCapabilities::default(); | |
| } | |
| } | |
| /// Toggle privileged mode, keeping the capability policy consistent in | |
| /// both directions. | |
| /// | |
| /// Enabling expands to the canonical shape; disabling withdraws it again, | |
| /// so a handle that is toggled off does not leave a non-privileged box | |
| /// holding `ALL`. An explicit policy the caller set themselves is left | |
| /// alone — only the shape this method produced is taken back. | |
| pub fn set_privileged(&mut self, enabled: bool) { | |
| let was_privileged = self.privileged; | |
| self.privileged = enabled; | |
| if enabled { | |
| self.normalize_privileged(); | |
| } else if was_privileged && self.capabilities.is_privileged_capability_shape() { | |
| self.capabilities = ContainerCapabilities::default(); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/boxlite/src/runtime/advanced_options.rs` around lines 753 - 768, Update
set_privileged so capability clearing occurs only when transitioning from
privileged to non-privileged: capture the previous self.privileged state before
assigning enabled, then require that prior state and
is_privileged_capability_shape() before resetting capabilities. Preserve
normalization when enabling and leave independently configured capabilities
unchanged when already non-privileged.
| ); | ||
| let namespaces = build_default_namespaces()?; | ||
| let mut mounts = build_standard_mounts(bundle_path)?; | ||
| let mut mounts = build_standard_mounts(bundle_path, security_policy.sys_mount_options.clone())?; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Keep the guest cgroup2 hierarchy read-only.
Line 390 omits a cgroup namespace. Lines 616-628 allow privileged mode to remove rro from the recursive /sys bind. This exposes the guest-wide /sys/fs/cgroup hierarchy as writable to the container. The container can then modify cgroups used by sibling containers and the guest agent.
Keep the cgroup2 submount recursively read-only, exclude it from the /sys bind, or create an equivalent isolated hierarchy before allowing write access. Update the test because it currently asserts the unsafe no-rro policy.
Based on learnings: “do not add a writable /sys/fs/cgroup cgroup2 mount unless a cgroup namespace or equivalent isolated hierarchy is created first.”
Also applies to: 390-392, 616-628, 1329-1355
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/guest/src/container/spec.rs` at line 175, Update build_standard_mounts
and the privileged mount-policy logic to keep the guest cgroup2 hierarchy
read-only: add a cgroup namespace or equivalent isolated hierarchy before
permitting writes, or exclude /sys/fs/cgroup from the recursive /sys bind and
preserve recursive read-only behavior. Revise the related tests around cgroup
namespace setup and the policy assertions so they reject the unsafe no-rro
configuration.
Source: Learnings
G4614
force-pushed
the
feat/dind-cli-rest-privileged
branch
8 times, most recently
from
August 13, 2026 07:42
bfc8774 to
6351463
Compare
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
apps/api, apps/runner, apps/libs (generated clients), and sdks/** now ship in boxlite-ai#1156 instead. This PR is left with the guest + core runtime + CLI + REST contract — the actual privileged/ DinD feature — so review stays scoped to the security-relevant parts of the change instead of being split across 84 files of mixed risk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ResolvedSecurityPolicy::from_resolved no longer validates a canonical privileged shape, so a test asserting that mismatched atomic options and capabilities still resolve is testing a trivial pass-through, not guest behavior. resolved_policy_consumes_atomic_security_options and all_capabilities_without_privileged_keep_proc_sys_readonly already cover the same constructor with both all-true and all-false inputs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cgroup_namespace and allow_all_devices are unnecessary for DinD: the
guest never enforced a restrictive device-cgroup default, and dockerd
tolerates running without a private cgroup namespace view. Only
unconfined_paths (masked/readonly path relief, for network sysctls)
and writable_sysfs (/sys stays writable, for dockerd's own cgroup
writes) are load-bearing.
ResolvedContainerSecurityConfig / ContainerAdvancedConfig /
ResolvedSecurityPolicy drop from 4 fields to 2. service.proto
renumbers ContainerAdvancedOptions to match (internal wire message,
no external consumer pinned to the old field numbers).
Before
resolve_container_security (Core · advanced_options.rs:789)
-> ResolvedContainerSecurityConfig{cgroup_namespace, writable_sysfs, allow_all_devices, unconfined_paths}
-> create_oci_spec (Guest · spec.rs:153)
|- build_default_namespaces(cgroup_namespace) — adds a cgroup namespace
|- build_linux_spec(..., allow_all_devices, ...) — builds an allow-all device-cgroup rule
`- build_standard_mounts(..., writable_sysfs)
After
resolve_container_security (Core · advanced_options.rs:789)
-> ResolvedContainerSecurityConfig{unconfined_paths, writable_sysfs}
-> create_oci_spec (Guest · spec.rs:153)
|- build_default_namespaces() — fixed list, no cgroup namespace
|- build_linux_spec(..., unconfined_paths) — no device-cgroup rule
`- build_standard_mounts(..., writable_sysfs)
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
architecture/README.md, container-capabilities.md, cli/README.md, and rust/README.md described privileged mode with a dead link and with the cgroup namespace / allow-all device rule this PR's own last commit removed. Reverting to main here rather than patching them in place: the accurate design record already lives outside the repo, and this PR's diff should stay code, not docs that immediately went stale. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
create_inner's own normalize_privileged() call always ran on data
sanitize_local_options had already normalized moments earlier in the
same call: RuntimeImpl is never re-exported outside the crate, and
core.rs's only construction site wraps it in LocalRuntime immediately,
so every reachable caller goes through sanitize_local_options first.
Replaced the redundant call with a debug_assert documenting that
invariant.
get_or_create_rejects_privileged_upgrade called RuntimeImpl directly
with a hand-built AdvancedBoxOptions{privileged: true, ..} literal,
bypassing that pipeline the way no real caller does; updated it to use
set_privileged, which is what actually produces the normalized shape.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ies verbatim
Host resolve_container_security() now sends literal masked_paths,
readonly_paths, and /sys mount options instead of two booleans; the guest
assigns them as-is with no reinterpretation. Matches how Docker, Podman, and
Kata Containers hand the enforcing side a finished OCI shape rather than a
flag to re-derive. capabilities stays as add/drop deltas — the guest is the
only side that knows its own kernel's capability ceiling.
Before: host sends {capabilities, unconfined_paths: bool, writable_sysfs: bool}
→ guest capabilities.rs::from_resolved(bool, bool)
→ spec.rs::build_linux_spec(.., unconfined_paths: bool) branches on it
→ spec.rs::build_standard_mounts(.., writable_sysfs: bool) branches on it
After: host resolve_container_security() resolves literal values
→ ContainerAdvancedOptions{masked_paths, readonly_paths, sys_mount_options}
→ guest capabilities.rs::from_resolved(Vec<String>, Vec<String>, Vec<String>)
→ spec.rs::build_linux_spec(.., masked_paths, readonly_paths) assigns verbatim
→ spec.rs::build_standard_mounts(.., sys_mount_options) assigns verbatim
Moved the privileged-vs-hardened test coverage to where the decision is now
made (advanced_options.rs); guest-side tests now assert pass-through fidelity.
…ollow-up PR Keeps boxlite-ai#646 to the mechanism itself — proto, host resolve, guest apply — same split rationale boxlite-ai#1156 already used to separate control-plane/runner/SDK consumers from boxlite-ai#646. The Rust API (AdvancedBoxOptions.privileged, set_privileged) stays; only the CLI --privileged flag and the self-hosted REST DTO/OpenAPI exposure move out, since neither adds mechanism, just a caller. Before: CLI/REST call AdvancedBoxOptions.privileged directly, bundled with the guest/core mechanism in one PR. After: CLI/REST removed here, follow-up PR re-adds them on top of this mechanism-only branch (rust API entry point unchanged, so the follow-up is pure plumbing with nothing left to test beyond wiring).
advanced_options.rs hand-copied oci-spec's default masked/readonly path lists rather than calling the crate's own public functions for them, even though the host crate already depends on oci-spec (runtime feature, on by default). Two independent copies of the same 10+5-entry list, one of which could silently drift on an oci-spec bump. Before: default_masked_paths() -> literal 10-entry Vec<String> default_readonly_paths() -> literal 5-entry Vec<String> After: default_masked_paths() -> oci_spec::runtime::get_default_maskedpaths() default_readonly_paths() -> oci_spec::runtime::get_default_readonly_paths()
Nothing in the DinD workflow reads a masked path: an isolated test (guest holding every capability, masked paths left untouched) showed runc, an overlayfs mount, a bridge+veth pair, iptables -t nat, and a network namespace all work, and the OCI masked-path list only ever protects host-wide /proc and /sys interfaces dockerd never touches (/proc/kcore, /proc/keys, /sys/firmware, ...). Clearing it rode in as a side effect of matching `docker run --privileged` wholesale, alongside the cgroup namespace and allow-all device rule this PR already dropped for the same "tested and found unnecessary" reason — masked_paths just never got the same follow-up scrutiny. Before: host resolves masked_paths (empty when privileged, oci-spec's default otherwise) and sends it over the wire; guest assigns it verbatim. After: masked_paths drops out of ContainerAdvancedOptions entirely; the guest never calls .masked_paths(..) and keeps oci-spec's own default, unconditionally, exactly as it did before `privileged` existed.
An old host that predates readonly_paths/sys_mount_options doesn't know to send them, so they arrive as empty — indistinguishable on the wire from a new host's deliberate privileged request. sys_mount_options has no legitimate empty value (every real host resolves at least 4 base flags, privileged or not), so an empty list can only mean the sender doesn't know about these fields at all. Silently proceeding doesn't even degrade gracefully: a real boot test confirmed youki requires bind/rbind in a mount's options to treat it as a bind mount, so an empty sys_mount_options makes the /sys bind fail with an unrelated "failed to prepare rootfs" error — this makes the real cause diagnosable instead. Before: ResolvedSecurityPolicy::from_resolved(caps, [], []) -> build_standard_mounts(.., []) -> youki: not a bind mount, tries fstype "none" -> "failed to prepare rootfs" (opaque) After: ResolvedSecurityPolicy::from_resolved rejects sys_mount_options.is_empty() up front -> "advanced.sys_mount_options is empty; the host predates resolved security fields ... recreate this box with a matching boxlite version" (actionable)
capabilities.rs's own module doc scoped it to "Linux capabilities for
container processes", but ResolvedSecurityPolicy bundles three fields —
capabilities is only one of them, readonly_paths/sys_mount_options are
mount paths and options, unrelated to capability resolution. Two of its
three consumers (start.rs, lifecycle.rs) only ever wanted the bundle, not
CapabilitySet itself, and vice versa for command.rs/zygote.rs — the file
was doing double duty for two different call patterns.
Before: capabilities.rs { CAPABILITIES_BY_NUMBER, CapabilitySet,
ResolvedSecurityPolicy (capabilities + readonly_paths + sys_mount_options) }
After: capabilities.rs { CAPABILITIES_BY_NUMBER, CapabilitySet } — back to
matching its own doc comment
security_policy.rs (new) { ResolvedSecurityPolicy, using
capabilities::CapabilitySet as one of its three fields }
Pure move: no logic changes. CapabilitySet's test-only len()/contains()
went from private to pub(crate) so security_policy.rs's tests (moved
along with the struct) can still reach them from a sibling module.
Keeps the word this module exists to signal: readonly_paths/sys_mount_options here are already-resolved literal values, not rules waiting to be applied — dropping "resolved" left "security_policy" reading like the latter, and close enough to the host's unrelated SecurityOptions (shim sandboxing) to invite confusion across the two crates. Pure rename: git mv + 4 import sites, no logic changes.
Container held capabilities/readonly_paths/sys_mount_options behind one bundled ResolvedSecurityPolicy struct. Unbundled: Container now stores only capabilities: CapabilitySet (the one field cmd() re-reads for every exec); readonly_paths/sys_mount_options flow as plain fn params from the RPC boundary through create_oci_bundle/create_oci_spec and are never stored on Container — clippy's dead_code check confirmed they were write-only there once unbundled, which the old struct's own field accesses elsewhere had been masking. resolved_security.rs keeps only the version-compat guard (validate_sys_mount_options); capability resolution moves to the call site via the newly re-exported CapabilitySet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The file now contains exactly one thing: a version-compat guard on the sys_mount_options field. Every sibling module in this dir (capabilities.rs, spec.rs, kill.rs, ...) is named after its subject, not a verb or a cross-cutting concern -- resolved_security borrowed a 'security' framing left over from when this file also bundled capabilities and readonly_paths, which is gone now. Pure rename: git mv + mod.rs's two references + a test name in spec.rs that repeated the old module name, no logic changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ed REST Adds the CLI --privileged flag and the self-hosted REST/OpenAPI surface for the AdvancedBoxOptions.privileged mechanism feat/dind-privileged-plumbing (boxlite-ai#646) adds. Pure consumer: no new mechanism, just wiring a caller onto the Rust API's set_privileged/AdvancedBoxOptions.privileged that already exists on this branch. Before: --privileged / the REST body field don't exist; only the Rust API (AdvancedBoxOptions.privileged, set_privileged) can request privileged mode. After: cli.rs/commands::create::run parse --privileged and call set_privileged(true) -> AdvancedBoxOptions.privileged rest/types.rs deserializes the same field from the REST request body openapi/box.openapi.yaml documents both the request field and the capabilities.privileged_enabled response flag
G4614
force-pushed
the
feat/dind-cli-rest-privileged
branch
from
August 13, 2026 08:02
6351463 to
3532c88
Compare
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
1 participant
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.
Adds the CLI
--privilegedflag and self-hosted REST/OpenAPI exposure for theAdvancedBoxOptions.privilegedmechanism#646adds — split out of#646the same way#1156split out the control-plane/runner/SDK surfaces, so#646stays to the guest/core mechanism itself.Depends on
#646: needsAdvancedBoxOptions.privileged/set_privilegedto exist, so the diff below overlaps with#646's own content until#646merges — this branch is built on top offeat/dind-privileged-plumbing, not a clean fork ofmain. Merge#646first; this PR's diff will then automatically shrink to just this PR's own 13 files.For the isolated diff right now (13 files, +239/-247): G4614/boxlite@feat/dind-privileged-plumbing...feat/dind-cli-rest-privileged
Test plan:
make clippy— clean, including cross-compiled guest cratemake test:unit:rust— 1006 + 49 passedmake fmt:check:rust— cleanSummary by CodeRabbit
New Features
--privilegedoption for creating and running containers.Bug Fixes