Add structured container capability controls - #1047
Conversation
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughLinux capability policies are added as nested advanced options across REST, CLI, C, Go, Node.js, Python, Rust, and guest interfaces. Policies are validated, negotiated, resolved into OCI capabilities, propagated to init and exec processes, versioned in archives, documented, and covered by tests and CI. ChangesLinux capability policy
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
tester seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
1 similar comment
|
tester seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
BREAKING CHANGE: Box capability metadata is no longer exposed by inspection APIs.
Capability support is negotiated by the `linux_capabilities_enabled` config flag, the guest's `linux-capabilities-v2` Ping feature, and runner feature advertisement. The `/strict` routes and the `*_WITH_CAPABILITIES_V2` job types restated that negotiation alongside it, so remove them and keep one contract. Four things those routes did add, and where each goes: - Unknown-field rejection. The cloud capability DTOs now carry it directly, so a misspelled `advanced.capabilites` is refused there rather than accepted and ignored, which would otherwise start the box with the default capability set while the caller believed a policy applied. Not replaced: unknown top-level fields, the runner's strict decoder, and `deny_unknown_fields` on the core `BoxOptions` tree. `boxlite serve` keeps its own on the wire types. - A 404 from an API build predating the field. Given up: these routes never shipped, so nothing depends on the signal. A rolling deploy can still answer from an older task. - Server-side atomic get-or-create with a reuse compatibility check. The REST path returns to the client-side get-then-create on main; the local runtime keeps its own check when adopting a box. - A create schema without the client-controlled `security` preset. The merged schema keeps `security` where main has it and adds `advanced` beside it; `boxlite serve` still refuses `security`. Cover the inspection routes with a test: addressing a versioned variant there 404s, and `get` maps NotFound to `Ok(None)`, so an existing box reads as missing instead of failing loudly. Simplify the rest of the policy surface: drop the no-op v8-to-v9 schema bump, the flat `capAdd`/`cap_add` fields that never shipped, and the cloud rejection of rootfs_path/tty/secrets, which was never about capabilities; collapse the two host capability-name canonicalizers into one, leaving the guest's own untouched; keep `BoxOptions::sanitize` under the name main ships; and move `ContainerConfig.advanced` to proto field 6 rather than 8. Keep the export-side archive v4 stamp: a pre-capability importer accepts up to v3 and would otherwise drop the policy, starting the box with wider privileges than the archive asked for. Import still rejects a too-new manifest; only the check that a policy-bearing manifest must claim v4 is gone, since a forged manifest can claim any version. The runner feature check for an existing box reads through the TTL-cached runner lookup again rather than a dedicated uncached one. Replace the feature-named guest test target with a generic `test:unit:guest` so capability resolution and OCI spec construction stay covered in CI. Move the network FFI ownership docs into `sdks/c/src/network.rs` so cbindgen emits them, and regenerate the API clients. That also unpins `apps/api-client-go/api/openapi.yaml` and drops the hand-written `contracttest/` package that guarded the runner `features` field.
📦 BoxLite review — looks good ·
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/boxlite/src/rest/runtime.rs (1)
108-122: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftREST
get_or_createdoesn't enforce capability-policy compatibility on reuse.
RuntimeImpl::get_or_create(local backend,rt_impl.rs) now rejects reuse when the requestedadvanced.capabilitiesdoesn't match the existing box's stored policy, specifically to prevent "reuse cannot silently weaken or elevate privileges." This REST implementation has no equivalent check: it reuses any box found by name unconditionally, regardless of the capability policy passed inoptions. A caller requesting a plain box could silently get back a box with an elevated (or different) capability policy, or vice versa — exactly the class of bug this PR is closing for the local backend. Note this may be compounded by the capability metadata having been removed from box inspection responses per this PR, making a like-for-like comparison harder to implement here without further API surface.🤖 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 108 - 122, The REST RuntimeImpl::get_or_create reuse path must enforce capability-policy compatibility before returning an existing box. Update the name lookup branch to compare the requested options.advanced.capabilities with the stored box policy, reject mismatches, and only reuse matching boxes; if the REST model lacks the metadata needed for this comparison, expose or retrieve that policy through the appropriate API rather than accepting reuse unconditionally.
🧹 Nitpick comments (5)
sdks/c/include/boxlite.h (1)
413-427: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument accepted capability names in both public setter APIs.
The new API docs explain array validity but not the accepted policy grammar: optional
CAP_prefix, plusALL/CAP_ALL. State this explicitly so C consumers do not need to infer runtime validation rules.
sdks/c/include/boxlite.h#L413-L427: add accepted capability-name syntax to both C API declarations.sdks/c/src/advanced_options.rs#L82-L100: mirror that contract in the exported Rust FFI documentation.As per coding guidelines, “Write comprehensive docstrings for all public functions and classes.”
🤖 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 `@sdks/c/include/boxlite.h` around lines 413 - 427, Document the accepted capability-name grammar for both public setters, boxlite_advanced_options_set_capabilities_add and boxlite_advanced_options_set_capabilities_drop: names may optionally use the CAP_ prefix and must support ALL and CAP_ALL. Update the declarations in sdks/c/include/boxlite.h lines 413-427 and mirror the same contract in the exported Rust FFI documentation in sdks/c/src/advanced_options.rs lines 82-100; retain the existing array-validity rules.Source: Coding guidelines
sdks/c/src/info.rs (1)
105-120: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the ownership contract for
free_box_info_list.Line 105 frees nested C strings and reconstructs the boxed-slice allocation; document that it only accepts lists returned by this SDK and consumes the handle.
As per coding guidelines, “Write comprehensive docstrings for all public functions and classes.”
🤖 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 `@sdks/c/src/info.rs` around lines 105 - 120, Document the public unsafe function free_box_info_list with a comprehensive ownership contract stating that it only accepts CBoxInfoList handles returned by this SDK and consumes the handle, including its nested allocations, so callers must not reuse or free the list afterward.Source: Coding guidelines
src/guest/src/container/capabilities.rs (1)
100-136: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
add=ALLsilently ignores the guest-unknown ceiling gap — worth a doc note.
supported_capabilities()truncates atCAPABILITIES_BY_NUMBER.len()(41). On a kernel withcap_last_cap > 40,add=["ALL"]resolves to the subset this build knows, which is the safe direction but diverges from Docker's "everything the kernel supports". A brief comment insupported_capabilitiesrecording that intentional fail-closed truncation would keep the next table update honest.🤖 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/capabilities.rs` around lines 100 - 136, Add a brief comment to supported_capabilities documenting that its CAPABILITIES_BY_NUMBER.len() limit intentionally truncates capabilities when the kernel supports unknown numbers, preserving fail-closed behavior for add=ALL. Keep the existing resolution logic unchanged.openapi/reference-server/server.py (1)
130-148: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the normalization rules.
Explain the case normalization, optional
CAP_prefix, andALLhandling; these policy choices are non-obvious. As per coding guidelines, “Include docstrings and comments for complex logic, particularly for non-obvious algorithmic choices.”🤖 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 130 - 148, Document the normalization policy in validate_capabilities: explain that capability names are uppercased, an optional CAP_ prefix is removed for validation, and ALL is accepted as a special value. Add a concise docstring or inline comment without changing the existing validation behavior.Source: Coding guidelines
sdks/go/options.go (1)
346-352: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDisambiguate the final lifecycle sentence.
“ If never called” follows
adv.Close(), so it can be read as saying defaults apply when the handle is never closed, contradicting the cleanup requirement. Explicitly say: “If neither setter is called, the box uses the defaults.”As per coding guidelines, comments should be clear about non-obvious behavior and lifecycle expectations.
🤖 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 `@sdks/go/options.go` around lines 346 - 352, Update the documentation comment for WithAdvancedOptions to replace the ambiguous final lifecycle sentence with wording that explicitly states the box uses defaults if neither SetCapabilities nor SetSecurityEnabled is called, while preserving the requirement to call adv.Close() after use.Source: Coding guidelines
🤖 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 `@apps/api/src/box/utils/capability-validation.util.ts`:
- Around line 22-23: Update the capability validation utility around the
normalized name and regex check to validate against the supported Linux
capability allowlist, also accepting ALL. Preserve the existing CAP_ prefix
normalization and format validation, but reject syntactically valid names that
are not in the allowlist before they reach persistence or startup.
In `@apps/libs/runner-api-client/src/docs/CreateBoxDTO.md`:
- Line 8: The generated examples for CreateBoxDTO and RecoverBoxDTO both omit
the documented advanced field. Add advanced, to the example object in
apps/libs/runner-api-client/src/docs/CreateBoxDTO.md lines 8-8 and
apps/libs/runner-api-client/src/docs/RecoverBoxDTO.md lines 8-8.
In `@make/test.mk`:
- Around line 209-224: Update the cargo test fallback in test:unit:guest to run
the capability and spec::tests filters in separate cargo test invocations,
preserving the existing package, binary, and single-thread options. Ensure the
shell exits with failure if either invocation fails, while leaving the
cargo-nextest path unchanged.
In `@openapi/reference-server/server.py`:
- Around line 130-148: Update validate_capabilities in the capability model to
compare each normalized, CAP_-stripped name against the canonical supported
Linux capability set, while preserving the special ALL handling and existing
syntax checks. Reject names absent from that set during request validation, and
add an UNKNOWN_CAPABILITY case to the corresponding test.
In `@sdks/c/README.md`:
- Around line 221-235: Complete cleanup in the capability setup examples: in
sdks/c/README.md lines 221-235, free error when boxlite_advanced_options_new
fails and free runtime on either capability setter failure; in
docs/reference/c/README.md lines 120-135, free both error and runtime on
advanced-options allocation failure and runtime on setter failure; in
docs/reference/c/README.md lines 581-596, free error when
boxlite_advanced_options_new fails.
In `@sdks/go/README.md`:
- Around line 101-109: Update the Go example’s box creation call to use the
declared runtime variable consistently with the earlier initialization,
replacing the undefined runtime reference while preserving the existing Create
arguments.
---
Outside diff comments:
In `@src/boxlite/src/rest/runtime.rs`:
- Around line 108-122: The REST RuntimeImpl::get_or_create reuse path must
enforce capability-policy compatibility before returning an existing box. Update
the name lookup branch to compare the requested options.advanced.capabilities
with the stored box policy, reject mismatches, and only reuse matching boxes; if
the REST model lacks the metadata needed for this comparison, expose or retrieve
that policy through the appropriate API rather than accepting reuse
unconditionally.
---
Nitpick comments:
In `@openapi/reference-server/server.py`:
- Around line 130-148: Document the normalization policy in
validate_capabilities: explain that capability names are uppercased, an optional
CAP_ prefix is removed for validation, and ALL is accepted as a special value.
Add a concise docstring or inline comment without changing the existing
validation behavior.
In `@sdks/c/include/boxlite.h`:
- Around line 413-427: Document the accepted capability-name grammar for both
public setters, boxlite_advanced_options_set_capabilities_add and
boxlite_advanced_options_set_capabilities_drop: names may optionally use the
CAP_ prefix and must support ALL and CAP_ALL. Update the declarations in
sdks/c/include/boxlite.h lines 413-427 and mirror the same contract in the
exported Rust FFI documentation in sdks/c/src/advanced_options.rs lines 82-100;
retain the existing array-validity rules.
In `@sdks/c/src/info.rs`:
- Around line 105-120: Document the public unsafe function free_box_info_list
with a comprehensive ownership contract stating that it only accepts
CBoxInfoList handles returned by this SDK and consumes the handle, including its
nested allocations, so callers must not reuse or free the list afterward.
In `@sdks/go/options.go`:
- Around line 346-352: Update the documentation comment for WithAdvancedOptions
to replace the ambiguous final lifecycle sentence with wording that explicitly
states the box uses defaults if neither SetCapabilities nor SetSecurityEnabled
is called, while preserving the requirement to call adv.Close() after use.
In `@src/guest/src/container/capabilities.rs`:
- Around line 100-136: Add a brief comment to supported_capabilities documenting
that its CAPABILITIES_BY_NUMBER.len() limit intentionally truncates capabilities
when the kernel supports unknown numbers, preserving fail-closed behavior for
add=ALL. Keep the existing resolution logic unchanged.
🪄 Autofix (Beta)
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: 0f69c66c-85aa-4661-854e-28efc7d7362f
📒 Files selected for processing (140)
.github/workflows/test.ymlapps/api-client-go/api/openapi.yamlapps/api-client-go/model_runner_healthcheck.goapps/api/src/box/common/box-advanced-options.tsapps/api/src/box/constants/runner-features.spec.tsapps/api/src/box/constants/runner-features.tsapps/api/src/box/controllers/runner.controller.tsapps/api/src/box/dto/box.dto.spec.tsapps/api/src/box/dto/create-box.dto.tsapps/api/src/box/dto/runner-health.dto.tsapps/api/src/box/entities/box.entity.tsapps/api/src/box/entities/runner.entity.tsapps/api/src/box/managers/box-actions/box-start.action.spec.tsapps/api/src/box/managers/box-actions/box-start.action.tsapps/api/src/box/runner-adapter/runnerAdapter.tsapps/api/src/box/runner-adapter/runnerAdapter.v0.spec.tsapps/api/src/box/runner-adapter/runnerAdapter.v0.tsapps/api/src/box/runner-adapter/runnerAdapter.v2.spec.tsapps/api/src/box/runner-adapter/runnerAdapter.v2.tsapps/api/src/box/services/box.service.spec.tsapps/api/src/box/services/box.service.tsapps/api/src/box/services/runner.service.tsapps/api/src/box/utils/capability-validation.util.tsapps/api/src/boxlite-rest/boxlite-box.controller.tsapps/api/src/boxlite-rest/boxlite-config.controller.spec.tsapps/api/src/boxlite-rest/boxlite-config.controller.tsapps/api/src/boxlite-rest/boxlite-rest-routing.spec.tsapps/api/src/boxlite-rest/dto/create-box.dto.spec.tsapps/api/src/boxlite-rest/dto/create-box.dto.tsapps/api/src/boxlite-rest/mappers/box-to-box.mapper.spec.tsapps/api/src/boxlite-rest/mappers/box-to-box.mapper.tsapps/api/src/migrations/pre-deploy/1785000000000-add-box-capabilities-migration.spec.tsapps/api/src/migrations/pre-deploy/1785000000000-add-box-capabilities-migration.tsapps/libs/api-client/src/docs/RunnerHealthcheck.mdapps/libs/api-client/src/models/runner-healthcheck.tsapps/libs/runner-api-client/src/.openapi-generator/FILESapps/libs/runner-api-client/src/docs/AdvancedBoxOptionsDTO.mdapps/libs/runner-api-client/src/docs/ContainerCapabilitiesDTO.mdapps/libs/runner-api-client/src/docs/CreateBoxDTO.mdapps/libs/runner-api-client/src/docs/RecoverBoxDTO.mdapps/libs/runner-api-client/src/docs/RunnerInfoResponseDTO.mdapps/libs/runner-api-client/src/models/advanced-box-options-dto.tsapps/libs/runner-api-client/src/models/container-capabilities-dto.tsapps/libs/runner-api-client/src/models/create-box-dto.tsapps/libs/runner-api-client/src/models/index.tsapps/libs/runner-api-client/src/models/recover-box-dto.tsapps/libs/runner-api-client/src/models/runner-info-response-dto.tsapps/runner/internal/features.goapps/runner/pkg/api/controllers/box.goapps/runner/pkg/api/controllers/info.goapps/runner/pkg/api/docs/docs.goapps/runner/pkg/api/docs/swagger.jsonapps/runner/pkg/api/docs/swagger.yamlapps/runner/pkg/api/dto/box.goapps/runner/pkg/api/dto/box_capabilities_test.goapps/runner/pkg/api/dto/info.goapps/runner/pkg/boxlite/client.goapps/runner/pkg/boxlite/stubs.goapps/runner/pkg/common/errors.goapps/runner/pkg/common/errors_test.goapps/runner/pkg/runner/v2/executor/box_capabilities_test.goapps/runner/pkg/runner/v2/healthcheck/healthcheck.godocs/architecture/README.mddocs/architecture/container-capabilities.mddocs/reference/README.mddocs/reference/c/README.mddocs/reference/cli/README.mddocs/reference/nodejs/README.mddocs/reference/python/README.mddocs/reference/rust/README.mdmake/test.mkopenapi/box.openapi.yamlopenapi/reference-server/server.pyopenapi/reference-server/tests/test_handle_cache.pysdks/c/README.mdsdks/c/include/boxlite.hsdks/c/src/advanced_options.rssdks/c/src/info.rssdks/c/src/network.rssdks/c/src/options.rssdks/c/src/tests.rssdks/go/README.mdsdks/go/advanced_options.gosdks/go/boxlite_test.gosdks/go/options.gosdks/go/runtime.gosdks/node/README.mdsdks/node/lib/index.tssdks/node/lib/native-contracts.tssdks/node/lib/simplebox.tssdks/node/src/advanced_options.rssdks/node/src/info.rssdks/node/src/lib.rssdks/node/src/options.rssdks/node/src/runtime.rssdks/node/tests/options.test.tssdks/python/README.mdsdks/python/boxlite/__init__.pysdks/python/boxlite/sync_api/_boxlite.pysdks/python/src/advanced_options.rssdks/python/src/info.rssdks/python/src/lib.rssdks/python/src/options.rssdks/python/tests/test_options.pysrc/boxlite/src/lib.rssrc/boxlite/src/litebox/archive.rssrc/boxlite/src/litebox/clone_export.rssrc/boxlite/src/litebox/init/tasks/guest_init.rssrc/boxlite/src/portal/interfaces/container.rssrc/boxlite/src/portal/interfaces/guest.rssrc/boxlite/src/portal/interfaces/mod.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/core.rssrc/boxlite/src/runtime/import.rssrc/boxlite/src/runtime/options.rssrc/boxlite/src/runtime/rt_impl.rssrc/boxlite/tests/security_enforcement.rssrc/cli/README.mdsrc/cli/src/cli.rssrc/cli/src/commands/create.rssrc/cli/src/commands/inspect.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/command.rssrc/guest/src/container/lifecycle.rssrc/guest/src/container/mod.rssrc/guest/src/container/spec.rssrc/guest/src/container/start.rssrc/guest/src/container/zygote.rssrc/guest/src/service/container.rssrc/guest/src/service/guest.rssrc/shared/proto/boxlite/v1/service.protosrc/shared/src/constants.rssrc/shared/src/errors.rs
💤 Files with no reviewable changes (2)
- sdks/node/src/info.rs
- sdks/python/src/info.rs
| const name = normalized.startsWith('CAP_') ? normalized.slice(4) : normalized | ||
| return /^[A-Z][A-Z0-9_]*$/.test(name) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject unknown capability names, not only malformed ones.
CAP_NOT_A_REAL_CAPABILITY satisfies this regex and reaches persistence/startup. Validate the normalized name against the supported Linux capability allowlist (plus ALL) so invalid policies fail at the request boundary as promised.
🤖 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 `@apps/api/src/box/utils/capability-validation.util.ts` around lines 22 - 23,
Update the capability validation utility around the normalized name and regex
check to validate against the supported Linux capability allowlist, also
accepting ALL. Preserve the existing CAP_ prefix normalization and format
validation, but reject syntactically valid names that are not in the allowlist
before they reach persistence or startup.
|
|
||
| Name | Type | Description | Notes | ||
| ------------ | ------------- | ------------- | ------------- | ||
| **advanced** | [**AdvancedBoxOptionsDTO**](AdvancedBoxOptionsDTO.md) | | [optional] [default to undefined] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the new advanced field in both generated examples.
The property tables document the field, but both usage examples omit it, reducing discoverability of capability configuration.
apps/libs/runner-api-client/src/docs/CreateBoxDTO.md#L8-L8: addadvanced,to theCreateBoxDTOexample object.apps/libs/runner-api-client/src/docs/RecoverBoxDTO.md#L8-L8: addadvanced,to theRecoverBoxDTOexample object.
📍 Affects 2 files
apps/libs/runner-api-client/src/docs/CreateBoxDTO.md#L8-L8(this comment)apps/libs/runner-api-client/src/docs/RecoverBoxDTO.md#L8-L8
🤖 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 `@apps/libs/runner-api-client/src/docs/CreateBoxDTO.md` at line 8, The
generated examples for CreateBoxDTO and RecoverBoxDTO both omit the documented
advanced field. Add advanced, to the example object in
apps/libs/runner-api-client/src/docs/CreateBoxDTO.md lines 8-8 and
apps/libs/runner-api-client/src/docs/RecoverBoxDTO.md lines 8-8.
| # Guest crate unit tests. Linux-only (the crate does not build elsewhere) and | ||
| # excluded from test:unit:rust because the zygote suite forks real processes. | ||
| # Runs the pure-logic modules, which is where capability resolution and OCI | ||
| # spec construction live — otherwise nothing exercises them. | ||
| test\:unit\:guest: | ||
| @if [ "$$(uname)" != "Linux" ]; then \ | ||
| echo "⏭️ Guest unit tests require Linux"; \ | ||
| exit 0; \ | ||
| fi; \ | ||
| echo "🧪 Running guest unit tests..."; \ | ||
| if command -v cargo-nextest >/dev/null 2>&1; then \ | ||
| cargo nextest run --no-tests=fail -p boxlite-guest -E 'test(~capabilit) + test(~spec::tests)'; \ | ||
| else \ | ||
| cargo test -p boxlite-guest --bins -- --test-threads=1 capabilit spec::tests; \ | ||
| fi | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package context =="
sed -n '1,280p' make/test.mk | nl -ba | sed -n '190,230p' || true
echo
echo "== tool availability =="
command -v cargo || true
if command -v cargo >/dev/null 2>&1; then
rustc --version || true
cargo --version || true
echo "== cargo test filter parsing/provisional docs =="
cargo test --help 2>&1 | sed -n '/Arguments:/,/Exit status/p' || true
fi
if command -v nextest >/dev/null 2>&1; then
nextest --version || true
fi
echo
echo "== deterministic probe: parse a sample Cargo.lock/rustc hints if present =="
grep -R "name = \"boxlite-guest\"" .cargo Cargo.toml Cargo.lock make 2>/dev/null | head -20 || trueRepository: boxlite-ai/boxlite
Length of output: 148
🏁 Script executed:
#!/bin/bash
echo "== package context =="
sed -n '190,230p' make/test.mk || true
echo
echo "== cargo tool availability =="
if command -v cargo >/dev/null 2>&1; then
cargo --version
echo
cargo test --help 2>&1 | sed -n '1,180p'
else
echo "cargo not available"
fi
echo
echo "== search for boxlite-guest package refs =="
rg -n 'boxlite-guest|test:unit:guest|cargo-nextest|cargo test --bins' Cargo.toml Cargo.lock make 2>/dev/null || trueRepository: boxlite-ai/boxlite
Length of output: 148
🌐 Web query:
Rust libtest test filters only one filter cargo test multiple positional filters behavior
💡 Result:
In Rust's libtest harness, which is used by cargo test, positional arguments are treated as test filters [1][2]. Historically and currently, the test harness only accepts a single filter string as a positional argument [3][4]. If you provide multiple positional arguments (e.g., cargo test filter1 filter2), cargo test will typically interpret the first as the filter and potentially pass others incorrectly or fail to use them as intended, as the underlying libtest implementation for filtering is designed to accept only one filter string [3][5][4]. To effectively run multiple tests that do not share a common substring, you have the following options: 1. Use Substring Matching: If your test names share a common string, you can filter by that common part [4]. 2. Use --skip: While you cannot provide multiple inclusion filters, you can use the --skip flag multiple times to exclude specific tests you do not want to run [1][2][6]. 3. Multiple cargo test calls: If you need to run disjoint sets of tests, you must invoke cargo test multiple times, once for each filter [7]. 4. Future/Alternative Workarounds: There is no native support for multiple positional inclusion filters [3][4]. Some developers use creative test naming conventions to group tests, or rely on external test runners if complex filtering is required [3]. Note that when using cargo test, arguments passed before the -- separator are handled by Cargo, while arguments passed after are passed directly to the test binary (libtest) [8][1][4]. Arguments like --skip are supported by libtest and can be passed multiple times after the separator [1][2].
Citations:
- 1: https://doc.rust-lang.org/stable/rustc/tests/index.html
- 2: https://doc.rust-lang.org/nightly/rustc/tests/index.html
- 3:
cargo testwith arguments can execute only one test rust-lang/rust#30422 - 4: https://doc.rust-lang.org/stable/book/ch11-02-running-tests.html
- 5: https://github.com/rust-lang/libtest/blob/10a09c8beca1dac05b7c9c1b24e7640f9332abcd/lib.rs
- 6: https://doc.rust-lang.org/rustc/tests/index.html
- 7: Bring multiple test filtering to test/bench rust-lang/cargo#6697
- 8: https://doc.rust-lang.org/stable/cargo/commands/cargo-test.html
Split the fallback test filters into separate cargo test invocations.
cargo test forwards only one positional filter to the test harness; the second token is dropped from filtering, so capabilit and spec::tests are not both selected. Run them separately and carry through the first failure.
🤖 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 `@make/test.mk` around lines 209 - 224, Update the cargo test fallback in
test:unit:guest to run the capability and spec::tests filters in separate cargo
test invocations, preserving the existing package, binary, and single-thread
options. Ensure the shell exits with failure if either invocation fails, while
leaving the cargo-nextest path unchanged.
| @field_validator("add", "drop") | ||
| @classmethod | ||
| def validate_capabilities(cls, capabilities: list[str]) -> list[str]: | ||
| for capability in capabilities: | ||
| if not capability.isascii(): | ||
| raise ValueError("capability names must contain only ASCII characters") | ||
| normalized = capability.upper() | ||
| if normalized == "ALL": | ||
| continue | ||
| name = normalized.removeprefix("CAP_") | ||
| if ( | ||
| not name | ||
| or not name[0].isalpha() | ||
| or not all( | ||
| char.isalpha() or char.isdigit() or char == "_" for char in name | ||
| ) | ||
| ): | ||
| raise ValueError(f"malformed Linux capability: {capability}") | ||
| return capabilities |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject unknown capability names, not only malformed ones.
FOO_BAR passes this validator even though it is not a Linux capability. Validate the normalized name against the canonical supported-capability set so this API fails at request validation as promised; add an UNKNOWN_CAPABILITY case to the corresponding test.
🤖 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 130 - 148, Update
validate_capabilities in the capability model to compare each normalized,
CAP_-stripped name against the canonical supported Linux capability set, while
preserving the special ALL handling and existing syntax checks. Reject names
absent from that set during request validation, and add an UNKNOWN_CAPABILITY
case to the corresponding test.
| CAdvancedBoxOptions* advanced = NULL; | ||
| if (boxlite_advanced_options_new(&advanced, &error) != Ok) { | ||
| boxlite_options_free(opts); | ||
| boxlite_runtime_free(runtime); | ||
| return 1; | ||
| } | ||
| const char* cap_add[] = {"NET_ADMIN"}; | ||
| const char* cap_drop[] = {"NET_RAW"}; | ||
| if (boxlite_advanced_options_set_capabilities_add(advanced, cap_add, 1) != Ok || | ||
| boxlite_advanced_options_set_capabilities_drop(advanced, cap_drop, 1) != Ok) { | ||
| fprintf(stderr, "Invalid Linux capability list\n"); | ||
| boxlite_advanced_options_free(advanced); | ||
| boxlite_options_free(opts); | ||
| return 1; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Complete cleanup in the new capability-setup error paths.
The examples return after capability setup failures without releasing all resources acquired on those paths.
sdks/c/README.md#L221-L235: freeerrorafterboxlite_advanced_options_newfails, and freeruntimebefore returning after either capability setter fails.docs/reference/c/README.md#L120-L135: freeerrorandruntimeon advanced-options allocation failure; freeruntimeon capability-setter failure.docs/reference/c/README.md#L581-L596: freeerrorbefore returning whenboxlite_advanced_options_newfails.
📍 Affects 2 files
sdks/c/README.md#L221-L235(this comment)docs/reference/c/README.md#L120-L135docs/reference/c/README.md#L581-L596
🤖 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 `@sdks/c/README.md` around lines 221 - 235, Complete cleanup in the capability
setup examples: in sdks/c/README.md lines 221-235, free error when
boxlite_advanced_options_new fails and free runtime on either capability setter
failure; in docs/reference/c/README.md lines 120-135, free both error and
runtime on advanced-options allocation failure and runtime on setter failure; in
docs/reference/c/README.md lines 581-596, free error when
boxlite_advanced_options_new fails.
| ```go | ||
| advanced, err := boxlite.NewAdvancedBoxOptions() | ||
| if err != nil { log.Fatal(err) } | ||
| defer advanced.Close() | ||
| if err := advanced.SetCapabilities(boxlite.ContainerCapabilities{ | ||
| Add: []string{"NET_ADMIN"}, | ||
| Drop: []string{"NET_RAW"}, | ||
| }); err != nil { log.Fatal(err) } | ||
| box, err := runtime.Create(ctx, "alpine:latest", boxlite.WithAdvancedOptions(advanced)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the declared runtime variable.
Line 109 calls runtime.Create, but the example creates rt; this snippet will not compile.
Proposed fix
- box, err := runtime.Create(ctx, "alpine:latest", boxlite.WithAdvancedOptions(advanced))
+ box, err := rt.Create(ctx, "alpine:latest", boxlite.WithAdvancedOptions(advanced))📝 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.
| ```go | |
| advanced, err := boxlite.NewAdvancedBoxOptions() | |
| if err != nil { log.Fatal(err) } | |
| defer advanced.Close() | |
| if err := advanced.SetCapabilities(boxlite.ContainerCapabilities{ | |
| Add: []string{"NET_ADMIN"}, | |
| Drop: []string{"NET_RAW"}, | |
| }); err != nil { log.Fatal(err) } | |
| box, err := runtime.Create(ctx, "alpine:latest", boxlite.WithAdvancedOptions(advanced)) |
🤖 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 `@sdks/go/README.md` around lines 101 - 109, Update the Go example’s box
creation call to use the declared runtime variable consistently with the earlier
initialization, replacing the undefined runtime reference while preserving the
existing Create arguments.
Custom kernels (#1041, #1051) and the capability policy both extend `AdvancedBoxOptions`, the CLI flag set, and option validation, so the two features are combined rather than either replacing the other. Capability name validation moves to `sanitize_common`, which main split out of `sanitize`: a capability list is request data, not a filesystem source, so it must also be checked on the persisted path. The warm-pool capability test now stubs `organizationUsageService`, which the org-quota work (#1028) made a required collaborator of `BoxService::create`.
| BoxliteErrorCode::InvalidArgument | ||
| ); | ||
| boxlite_options_set_advanced(opts, advanced); | ||
| (*opts) |
| BoxliteErrorCode::InvalidArgument | ||
| ); | ||
| boxlite_options_set_advanced(opts, advanced); | ||
| (*opts) |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The guest already reports its build version on Ping, so a second advertisement channel adds a field without adding information. Drop `PingResponse.features` and compare the reported version instead. The floor is 0.9.8. v0.9.7 is released and its guest predates the `advanced.capabilities` field, so it would decode the field as unknown proto and drop it — accepting that guest is the fail-open this gate exists to prevent. A guest rootfs is cached per version and reused, so such a guest can outlive its release and meet a much newer host. Versions that do not parse are rejected rather than assumed current; a pre-release suffix compares as its numeric core, since a `0.9.8-rc1` guest is built from the tree that carries the field. Because the workspace is still 0.9.7, a guest built from this tree cannot satisfy its own floor, so custom capabilities do not work in tree until the release bump. Drop the end-to-end test that asserted them. Unit tests still cover resolution, the TTY-exec OCI process and the API boundaries, but nothing now exercises the init and non-TTY exec paths that test owned. Restore it with the bump.
Drop every `apps/` change and ship the capability policy in the core runtime, guest, SDKs and CLI only. The control-plane side — the box `advanced` column and DTOs, the runner feature advertisement, the scheduling filter and the start/recover gates — lands separately once the core contract is settled. `boxlite serve` remains the server half of the REST contract and still advertises `linux_capabilities_enabled`, so a remote SDK negotiates against it as before. The hosted API does not advertise the flag, so a BoxLite client refuses to send it a policy. That gate lives on the client: the hosted API does not reject unknown properties, so a caller that bypasses the negotiation would have `advanced` dropped.
ContainerAdvancedOptions.capabilities predates this feature: PR boxlite-ai#1047 introduced it as a direct ContainerCapabilities on field 1, with a guest version floor of 0.9.8, long since deployed. Wrapping it in a new ProcessOptions submessage (also field 1) kept the field number but changed what it decodes to — any already-running guest at or above 0.9.8 (which is every guest that predates this session's nesting work, i.e. most of the current fleet) decodes the new bytes against its old field-1 layout and gets garbage, surfacing as "unknown Linux capability ''" on the next restart of an existing, non-privileged box. A version-gate bump doesn't fix this: it can only refuse old guests, not un-break the ones already running that don't get rebuilt until their box is recreated. The actual fix is to stop reusing field 1's old meaning: capabilities goes back to being flat on the wire, matching what every 0.9.8+ guest already expects. linux/mount keep their nested shape — they are new in this feature with no deployed guest depending on a flat layout, so nesting them under their own submessage costs nothing. The host-side ResolvedContainerSecurityConfig/ContainerAdvancedConfig structs are flattened to match, dropping the now-pointless ResolvedProcessSecurity wrapper (it existed only to mirror this wire shape, which no longer nests capabilities). Test plan: - [x] Added an assertion on `advanced.capabilities` (not previously checked) to the wire-shape test in portal/interfaces/container.rs, alongside the existing linux/mount checks - [x] `make clippy` (workspace + guest cross-compile) — clean - [x] `make fmt:check:rust` — clean - [x] `make test:unit:rust` — 1027 + 49 passed Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ContainerAdvancedOptions.capabilities predates this feature: PR boxlite-ai#1047 introduced it as a direct ContainerCapabilities on field 1, with a guest version floor of 0.9.8, long since deployed. Wrapping it in a new ProcessOptions submessage (also field 1) kept the field number but changed what it decodes to -- any already-running guest at or above 0.9.8 (which is every guest that predates this session's nesting work, i.e. most of the current fleet) decodes the new bytes against its old field-1 layout and gets garbage, surfacing as "unknown Linux capability ''" on the next restart of an existing, non-privileged box. A version-gate bump doesn't fix this: it can only refuse old guests, not un-break the ones already running that don't get rebuilt until their box is recreated. The actual fix is to stop reusing field 1's old meaning: capabilities goes back to being flat on the wire, matching what every 0.9.8+ guest already expects. linux/mount keep their nested shape -- they are new in this feature with no deployed guest depending on a flat layout, so nesting them under their own submessage costs nothing. The host-side ResolvedContainerSecurityConfig/ContainerAdvancedConfig structs are flattened to match, dropping the now-pointless ResolvedProcessSecurity wrapper (it existed only to mirror this wire shape, which no longer nests capabilities). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PyAdvancedBoxOptions.capabilities was a plain, non-Option
PyContainerCapabilities defaulting to {add:[],drop:[]}, and the
TryFrom<PyBoxOptions> conversion called set_capabilities
unconditionally whenever advanced was Some - so a Python caller who
set only security= or health_check= (never mentioning capabilities)
ended up with capabilities() == Some(default) instead of None. That
collapse trips archive_version_for_options's version bump and, over
REST, require_linux_capabilities_enabled's feature gate, for a caller
who never touched capabilities at all.
capabilities predates no published release (confirmed: the last
publish tag v0.9.7 predates boxlite-ai#1047, which introduced the whole
capabilities feature including this Python field), so it's free to
change shape. Make it Option<PyContainerCapabilities>, mirroring the
Node SDK, and only call set_capabilities when the caller actually set
it.
What changed
advanced.capabilities.addandadvanced.capabilities.dropoptions across the core runtime, REST/cloud APIs, runner, persistence, inspection, and Python, Node.js, Go, and C SDKs.--cap-addand--cap-dropflags as convenience syntax.Why
Users need controlled Linux capability customization without expanding the top-level box interface. Nesting the policy under advanced options keeps common creation APIs focused while leaving security-sensitive settings explicit and typed.
Impact and compatibility
The default capability set remains Docker-compatible. Capability names accept optional
CAP_prefixes, andALL/CAP_ALLare supported. Unknown or malformed fields are rejected instead of being silently ignored.Mixed-version peers fail closed when custom capability policy is unsupported. Strict APIs require the nested interface; legacy flat prototype fields are rejected. Existing boxes without custom capability policy retain default behavior.
Validation
Caveats
Linux guest runtime enforcement could not execute on the macOS host; the Linux path compiled successfully and its tests run in Linux CI. One unrelated Rust test requiring the absent vendored
libkrunsubmodule was excluded. The existing app-wide formatter baseline still reports 186 unrelated generated TypeScript files.Summary by CodeRabbit
CAP_prefixes, case-insensitive matching, andALLrules.