Expose inbound network policy across CLI, SDKs, serve, and NetworkInfo - #1206
Open
G4614 wants to merge 4 commits into
Open
Expose inbound network policy across CLI, SDKs, serve, and NetworkInfo#1206G4614 wants to merge 4 commits into
G4614 wants to merge 4 commits into
Conversation
NetworkSpec was a single enum modeling guest egress only; whether a
box's exposed services are publicly reachable had no field anywhere.
Reshape it into a struct with two directions:
Before:
BoxOptions.network: NetworkSpec::Enabled{allow_net}|Disabled
<- egress only; no inbound reachability concept
After:
BoxOptions.network: NetworkSpec{
outbound: OutboundNetworkSpec::Enabled{allow_net}|Disabled,
inbound: InboundNetworkSpec::Enabled{allow_net}|Disabled,
}
Inbound: Enabled = publicly reachable (default), Disabled = private.
Its allow_net exists for shape symmetry but is rejected when non-empty
(try_from and sanitize) until a runtime sink enforces it. The legacy
flat wire shape still deserializes (untagged fallback) with a
deprecation warning.
CLI/serve/REST client and the C/Node/Python bindings are adapted to
compile against the new shape without exposing inbound configuration;
those surfaces follow in a separate PR. Go is untouched (C ABI
unchanged).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
📦 BoxLite review — couldn't completepowered by BoxLite |
|
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)
📝 WalkthroughWalkthroughThe PR splits network configuration and metadata into independent outbound and inbound policies. It updates runtime networking, CLI and REST handling, C, Go, Node, and Python SDKs, compatibility behavior, validation, serialization, and tests. ChangesDirectional networking
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
This was referenced Aug 12, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
sdks/python/src/info.rs (1)
70-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument
NetworkDirectionInfo.
NetworkDirectionInfois a new public Python class. Add a comprehensive class docstring that definesmodeandallow_net, including the inbound allowlist limitation.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/python/src/info.rs` around lines 70 - 77, Document the public Python class represented by PyNetworkDirectionInfo with a comprehensive class docstring, describing the mode and allow_net attributes and explicitly noting the inbound allowlist limitation. Keep the existing #[pyclass(name = "NetworkDirectionInfo")] exposure and field getters unchanged.Source: Coding guidelines
src/deps/libkrun-sys/vendor/libkrunfw (1)
1-1: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy liftAdd CI coverage for the libkrunfw source-build path.
No tracked workflow sets
BOXLITE_BUILD_LIBKRUNFW. Add a Linux CI job that setsBOXLITE_BUILD_LIBKRUNFW=1and runs the relevant build.🤖 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/deps/libkrun-sys/vendor/libkrunfw` at line 1, Add a Linux CI job in the existing workflow configuration that sets BOXLITE_BUILD_LIBKRUNFW=1 and executes the relevant libkrunfw source build, ensuring this source-build path is covered by CI.
🤖 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 `@sdks/c/include/boxlite.h`:
- Around line 810-814: Update the AutoStop documentation for
boxlite_options_set_auto_stop_interval in sdks/c/include/boxlite.h lines 810-814
and its corresponding description in sdks/c/src/options.rs lines 169-172,
replacing “remain paused” with “remain idle” while preserving the rest of the
behavior and wording.
In `@sdks/go/options.go`:
- Around line 82-90: Update the InboundNetworkSpec comment to state that
AllowNet is reserved and must remain empty until inbound allowlist enforcement
is implemented; remove the claim that it currently restricts reachable
hosts/IPs, while preserving the existing ModeEnabled and ModeDisabled
descriptions.
In `@sdks/node/lib/simplebox.ts`:
- Around line 149-154: Update the directional policy validation in SimpleBox
construction to reject record-valued outbound or inbound policies that do not
define mode, while preserving the existing object-type checks. Add regression
tests in sdks/node/tests/options.test.ts:220-228 covering empty outbound and
inbound policy objects; both sites require changes.
In `@sdks/node/src/options.rs`:
- Around line 369-381: Update the inbound network documentation and conversion
behavior: in sdks/node/src/options.rs:369-381,
sdks/node/lib/native-contracts.ts:118-122, and
sdks/node/lib/simplebox.ts:117-122, state that non-empty inbound allowNet values
are currently rejected and cannot restrict access; in
sdks/node/src/options.rs:993-1008, extend the conversion test to assert that a
non-empty inbound allowlist returns an error.
In `@sdks/python/src/options.rs`:
- Around line 276-281: Update the inbound network policy documentation near
InboundNetworkSpec to state that non-empty allow_net values are currently
invalid and rejected because inbound allowlist enforcement is unavailable;
remove the claim that allow_net restricts inbound access, while preserving the
existing mode descriptions.
In `@sdks/python/tests/test_network_spec.py`:
- Around line 17-44: Extend NetworkSpec tests in
sdks/python/tests/test_network_spec.py (lines 17-44) with nested
InboundNetworkSpec coverage, including mode="disabled" and assertions for the
resulting inbound policy. Add a conversion test in sdks/python/src/options.rs
(lines 1091-1101) that uses a non-empty inbound allow_net and asserts conversion
fails.
In `@src/boxlite/src/runtime/options.rs`:
- Around line 860-910: Update PortPublishTask’s fresh-publication and
reattach-reconciliation paths to inspect network.inbound and skip backend.expose
when it is InboundNetworkSpec::Disabled, while preserving existing publication
for Enabled. Add an integration test using a requested PortSpec that verifies no
listener is created for an inbound-disabled box.
In `@src/cli/src/cli.rs`:
- Around line 651-655: Update the help text for the inbound field in the CLI
argument definition to explicitly state that --network disabled does not disable
inbound access; callers must pass --inbound disabled to make services private
and unreachable externally. Preserve the existing enabled/disabled mode
descriptions.
In `@src/cli/src/commands/serve/types.rs`:
- Around line 112-120: Update the documentation above InboundNetworkSpec to
state that inbound allow_net must remain empty and non-empty values are rejected
until enforcement is implemented; remove the claim that it restricts publicly
reachable services, while preserving the mode descriptions.
---
Nitpick comments:
In `@sdks/python/src/info.rs`:
- Around line 70-77: Document the public Python class represented by
PyNetworkDirectionInfo with a comprehensive class docstring, describing the mode
and allow_net attributes and explicitly noting the inbound allowlist limitation.
Keep the existing #[pyclass(name = "NetworkDirectionInfo")] exposure and field
getters unchanged.
In `@src/deps/libkrun-sys/vendor/libkrunfw`:
- Line 1: Add a Linux CI job in the existing workflow configuration that sets
BOXLITE_BUILD_LIBKRUNFW=1 and executes the relevant libkrunfw source build,
ensuring this source-build path is covered by CI.
🪄 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: 1ae6d7c9-c9ea-4f74-b9a2-8a1923b23603
📒 Files selected for processing (40)
sdks/c/README.mdsdks/c/include/boxlite.hsdks/c/src/event_queue.rssdks/c/src/info.rssdks/c/src/options.rssdks/go/boxlite_test.gosdks/go/info.gosdks/go/info_cgo_dev_test.gosdks/go/info_cgo_test_support_dev.gosdks/go/network_secrets_integration_test.gosdks/go/options.gosdks/node/README.mdsdks/node/lib/native-contracts.tssdks/node/lib/simplebox.tssdks/node/src/info.rssdks/node/src/options.rssdks/node/tests/network-secrets.integration.test.tssdks/node/tests/options.test.tssdks/node/tests/skillbox.integration.test.tssdks/python/README.mdsdks/python/boxlite/__init__.pysdks/python/src/info.rssdks/python/src/lib.rssdks/python/src/options.rssdks/python/tests/test_network_spec.pysdks/python/tests/test_secret_substitution.pysdks/python/tests/test_tcp_filter.pysrc/boxlite/src/lib.rssrc/boxlite/src/litebox/init/tasks/guest_init.rssrc/boxlite/src/litebox/init/tasks/vmm_attach.rssrc/boxlite/src/litebox/init/tasks/vmm_spawn.rssrc/boxlite/src/rest/types.rssrc/boxlite/src/runtime/options.rssrc/boxlite/src/runtime/types.rssrc/boxlite/tests/network_spec.rssrc/boxlite/tests/security_enforcement.rssrc/cli/src/cli.rssrc/cli/src/commands/serve/mod.rssrc/cli/src/commands/serve/types.rssrc/deps/libkrun-sys/vendor/libkrunfw
Comment on lines
+810
to
814
| // Set how long an idle box may remain paused before the runtime pauses it. | ||
| // | ||
| // The value is expressed in seconds. `0` preserves the runtime/control-plane | ||
| // default. A null options pointer is treated as a no-op. | ||
| void boxlite_options_set_auto_stop_interval(CBoxliteOptions *opts, uint32_t seconds); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the AutoStop state description.
An idle box is not already paused before AutoStop pauses it. Replace “remain paused” with “remain idle” in both API comments.
sdks/c/include/boxlite.h#L810-L814: describe the idle duration before AutoStop pauses the box.sdks/c/src/options.rs#L169-L172: use the same corrected AutoStop description.
📍 Affects 2 files
sdks/c/include/boxlite.h#L810-L814(this comment)sdks/c/src/options.rs#L169-L172
🤖 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 810 - 814, Update the AutoStop
documentation for boxlite_options_set_auto_stop_interval in
sdks/c/include/boxlite.h lines 810-814 and its corresponding description in
sdks/c/src/options.rs lines 169-172, replacing “remain paused” with “remain
idle” while preserving the rest of the behavior and wording.
Comment on lines
+149
to
+154
| if ("outbound" in value && !isRecord(value.outbound)) { | ||
| throw new TypeError("SimpleBoxOptions.network.outbound must be an object."); | ||
| } | ||
| if ("inbound" in value && !isRecord(value.inbound)) { | ||
| throw new TypeError("SimpleBoxOptions.network.inbound must be an object."); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject incomplete directional policies in the constructor.
isRecord({}) passes Lines 149-154, so { network: { outbound: {} } } reaches lazy native creation instead of failing in SimpleBox construction. Require mode for each supplied directional policy.
sdks/node/lib/simplebox.ts#L149-L154: reject record-valuedoutboundorinboundpolicies that omitmode.sdks/node/tests/options.test.ts#L220-L228: add regression cases for emptyoutboundandinboundpolicy objects.
As per coding guidelines, sdks/node/**/*.test.{js,ts,jsx,tsx} must “Write unit tests for all public functions and critical business logic.”
📍 Affects 2 files
sdks/node/lib/simplebox.ts#L149-L154(this comment)sdks/node/tests/options.test.ts#L220-L228
🤖 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/node/lib/simplebox.ts` around lines 149 - 154, Update the directional
policy validation in SimpleBox construction to reject record-valued outbound or
inbound policies that do not define mode, while preserving the existing
object-type checks. Add regression tests in
sdks/node/tests/options.test.ts:220-228 covering empty outbound and inbound
policy objects; both sites require changes.
Source: Coding guidelines
Comment on lines
+369
to
+381
| /// Aligned field-for-field with `JsOutboundNetworkSpec`: `mode="enabled"` | ||
| /// means services the box exposes are publicly reachable (optionally | ||
| /// restricted to `allowNet`); `mode="disabled"` means private. | ||
| #[napi(object)] | ||
| #[derive(Clone, Debug)] | ||
| pub struct JsInboundNetworkSpec { | ||
| /// Inbound mode: "enabled" or "disabled". | ||
| pub mode: String, | ||
|
|
||
| /// Inbound allowlist when mode is "enabled". Empty/omitted means any | ||
| /// caller may reach the box's exposed services. | ||
| #[napi(js_name = "allowNet")] | ||
| pub allow_net: Option<Vec<String>>, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not describe inbound.allowNet as enforceable.
The runtime rejects non-empty inbound allowlists because no layer enforces them. The current public comments state that callers can restrict inbound access with allowNet. This gives users an unsupported security configuration.
sdks/node/src/options.rs#L369-L381: state that non-empty inboundallowNetis currently rejected.sdks/node/lib/native-contracts.ts#L118-L122: document the same restriction in the native TypeScript contract.sdks/node/lib/simplebox.ts#L117-L122: document the same restriction in the high-level SDK contract.sdks/node/src/options.rs#L993-L1008: add a conversion test that asserts a non-empty inbound allowlist returns an error.
As per coding guidelines, sdks/node/**/*.test.{js,ts,jsx,tsx} must “Write unit tests for all public functions and critical business logic.”
📍 Affects 3 files
sdks/node/src/options.rs#L369-L381(this comment)sdks/node/lib/native-contracts.ts#L118-L122sdks/node/lib/simplebox.ts#L117-L122sdks/node/src/options.rs#L993-L1008
🤖 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/node/src/options.rs` around lines 369 - 381, Update the inbound network
documentation and conversion behavior: in sdks/node/src/options.rs:369-381,
sdks/node/lib/native-contracts.ts:118-122, and
sdks/node/lib/simplebox.ts:117-122, state that non-empty inbound allowNet values
are currently rejected and cannot restrict access; in
sdks/node/src/options.rs:993-1008, extend the conversion test to assert that a
non-empty inbound allowlist returns an error.
Source: Coding guidelines
Comment on lines
+276
to
+281
| /// Inbound network policy. | ||
| /// | ||
| /// Aligned field-for-field with `OutboundNetworkSpec`: `mode` accepts | ||
| /// `"enabled"` (services the box exposes are publicly reachable) or | ||
| /// `"disabled"` (private). `allow_net` restricts which hosts/IPs may reach | ||
| /// in when `mode="enabled"`. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Correct the inbound allowlist documentation.
The runtime rejects non-empty inbound allow_net values because no layer enforces them. These lines state that allow_net restricts inbound access. This implies an access-control guarantee that does not exist. State that non-empty inbound allowlists are currently invalid and rejected.
The PR objective states that inbound allowlist enforcement is not available.
🤖 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/python/src/options.rs` around lines 276 - 281, Update the inbound
network policy documentation near InboundNetworkSpec to state that non-empty
allow_net values are currently invalid and rejected because inbound allowlist
enforcement is unavailable; remove the claim that allow_net restricts inbound
access, while preserving the existing mode descriptions.
Comment on lines
+17
to
+44
| """NetworkSpec accepts the nested outbound shape.""" | ||
| spec = boxlite.NetworkSpec( | ||
| outbound=boxlite.OutboundNetworkSpec( | ||
| mode="enabled", | ||
| allow_net=["example.com", "*.openai.com"], | ||
| ) | ||
| ) | ||
|
|
||
| assert spec.outbound.mode == "enabled" | ||
| assert spec.outbound.allow_net == ["example.com", "*.openai.com"] | ||
|
|
||
| def test_legacy_creation(self): | ||
| """NetworkSpec keeps accepting the legacy mode and allow_net keywords.""" | ||
| spec = boxlite.NetworkSpec( | ||
| mode="enabled", | ||
| allow_net=["example.com", "*.openai.com"], | ||
| ) | ||
|
|
||
| assert spec.mode == "enabled" | ||
| assert spec.allow_net == ["example.com", "*.openai.com"] | ||
| assert spec.outbound.mode == "enabled" | ||
| assert spec.outbound.allow_net == ["example.com", "*.openai.com"] | ||
|
|
||
| def test_rejects_mixed_legacy_and_nested_outbound(self): | ||
| """NetworkSpec rejects callers that mix nested and legacy outbound fields.""" | ||
| with pytest.raises(ValueError): | ||
| boxlite.NetworkSpec( | ||
| outbound=boxlite.OutboundNetworkSpec(mode="enabled"), | ||
| mode="disabled", | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Add regression coverage for inbound policy validation.
The new inbound API has no test for the unsupported inbound allowlist path. A regression could accept an inbound allowlist even though runtime enforcement is unavailable.
sdks/python/tests/test_network_spec.py#L17-L44: construct and assert a nestedInboundNetworkSpec, includingmode="disabled".sdks/python/src/options.rs#L1091-L1101: convert an inbound policy with a non-emptyallow_netand assert that conversion fails.
Based on learnings: "Write unit tests for critical functionality and edge cases."
📍 Affects 2 files
sdks/python/tests/test_network_spec.py#L17-L44(this comment)sdks/python/src/options.rs#L1091-L1101
🤖 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/python/tests/test_network_spec.py` around lines 17 - 44, Extend
NetworkSpec tests in sdks/python/tests/test_network_spec.py (lines 17-44) with
nested InboundNetworkSpec coverage, including mode="disabled" and assertions for
the resulting inbound policy. Add a conversion test in
sdks/python/src/options.rs (lines 1091-1101) that uses a non-empty inbound
allow_net and asserts conversion fails.
Source: Learnings
Comment on lines
+860
to
+910
| /// Whether services the box exposes are reachable from outside it. Mirrors | ||
| /// [`OutboundNetworkSpec`]'s shape: `Enabled` = publicly reachable, | ||
| /// `Disabled` = private, unreachable from outside the box. `allow_net` | ||
| /// exists for shape symmetry but must be empty today — a non-empty inbound | ||
| /// allowlist is rejected (`try_from`/`sanitize`) until enforcement exists. | ||
| #[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] | ||
| pub enum InboundNetworkSpec { | ||
| Enabled { | ||
| #[serde(default)] | ||
| allow_net: Vec<String>, | ||
| }, | ||
| Disabled, | ||
| } | ||
|
|
||
| impl Default for OutboundNetworkSpec { | ||
| fn default() -> Self { | ||
| Self::Enabled { | ||
| allow_net: Vec::new(), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl Default for InboundNetworkSpec { | ||
| // Public unless told otherwise, matching the control plane's | ||
| // longstanding preview-URL default. | ||
| fn default() -> Self { | ||
| Self::Enabled { | ||
| allow_net: Vec::new(), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl NetworkSpec { | ||
| pub fn enabled(allow_net: Vec<String>) -> Self { | ||
| Self { | ||
| outbound: OutboundNetworkSpec::Enabled { allow_net }, | ||
| inbound: InboundNetworkSpec::default(), | ||
| } | ||
| } | ||
|
|
||
| pub fn disabled() -> Self { | ||
| Self { | ||
| outbound: OutboundNetworkSpec::Disabled, | ||
| inbound: InboundNetworkSpec::default(), | ||
| } | ||
| } | ||
|
|
||
| pub fn with_inbound(mut self, inbound: InboundNetworkSpec) -> Self { | ||
| self.inbound = inbound; | ||
| self | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/boxlite/src/litebox --items all --type function --match 'Port|Publish|Network'
rg -n -C 4 'network\.inbound|InboundNetworkSpec|PortPublish|published_ports|publish' \
src/boxlite/src/litebox src/boxlite/testsRepository: boxlite-ai/boxlite
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- port publication structure ---'
ast-grep outline src/boxlite/src/litebox/init/tasks/port_publish.rs --items all --type function
echo '--- port publication implementation ---'
sed -n '1,280p' src/boxlite/src/litebox/init/tasks/port_publish.rs
echo '--- task entry points and config flow ---'
rg -n -C 8 'PortPublishTask|PortPublisher::|planned|options\.ports|build_network_backend|NetworkBackendConfig|InboundNetworkSpec|network\.inbound' \
src/boxlite/src/litebox/init/tasks/port_publish.rs \
src/boxlite/src/litebox/init/mod.rs \
src/boxlite/src/litebox/init/tasks/vmm_spawn.rs \
src/boxlite/src/litebox/init/tasks/vmm_attach.rs \
src/boxlite/src/litebox/config.rs \
src/boxlite/src/runtime/options.rsRepository: boxlite-ai/boxlite
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
publication = Path("src/boxlite/src/litebox/init/tasks/port_publish.rs").read_text()
spawn = Path("src/boxlite/src/litebox/init/tasks/vmm_spawn.rs").read_text()
tests = "\n".join(
p.read_text()
for root in ("src/boxlite/tests", "src/boxlite/src")
for p in Path(root).rglob("*.rs")
)
checks = {
"publication_reads_ports": "ctx.config.options.ports.clone()" in publication,
"publication_reads_inbound": "network.inbound" in publication or "options.network.inbound" in publication,
"publication_calls_expose": ".backend\n .expose(" in publication,
"backend_branches_on_outbound": "match &options.network.outbound" in spawn,
"backend_reads_inbound": "options.network.inbound" in spawn,
"disabled_inbound_test_present": "InboundNetworkSpec::Disabled" in tests
and ("port" in tests.lower() or "publish" in tests.lower()),
}
for name, value in checks.items():
print(f"{name}={value}")
assert checks["publication_reads_ports"]
assert not checks["publication_reads_inbound"]
assert checks["publication_calls_expose"]
assert checks["backend_branches_on_outbound"]
assert not checks["backend_reads_inbound"]
assert not checks["disabled_inbound_test_present"]
PYRepository: boxlite-ai/boxlite
Length of output: 433
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- inbound policy references ---'
rg -n -C 6 'InboundNetworkSpec|inbound' src/boxlite/src src/boxlite/tests
echo '--- publication tests involving disabled policies or requested ports ---'
rg -n -C 10 'Disabled|disabled|mapping\(|PortSpec|publish\(|reconcile\(' \
src/boxlite/src/litebox/init/tasks/port_publish.rs \
src/boxlite/testsRepository: boxlite-ai/boxlite
Length of output: 50375
Enforce InboundNetworkSpec::Disabled during port publication
PortPublishTask reads options.ports and calls backend.expose without reading network.inbound. With outbound networking enabled, inbound-disabled boxes can still publish requested ports. Guard fresh publication and reattach reconciliation, then add an integration test with a requested PortSpec that asserts no listener is created.
🤖 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/options.rs` around lines 860 - 910, Update
PortPublishTask’s fresh-publication and reattach-reconciliation paths to inspect
network.inbound and skip backend.expose when it is InboundNetworkSpec::Disabled,
while preserving existing publication for Enabled. Add an integration test using
a requested PortSpec that verifies no listener is created for an
inbound-disabled box.
Comment on lines
112
to
120
| /// Aligned field-for-field with [`OutboundNetworkSpec`]: `mode="enabled"` | ||
| /// means services the box exposes are publicly reachable (optionally | ||
| /// restricted to `allow_net`); `mode="disabled"` means private. | ||
| #[derive(Deserialize)] | ||
| #[serde(deny_unknown_fields)] | ||
| pub(super) struct InboundNetworkSpec { | ||
| pub mode: String, | ||
| #[serde(default)] | ||
| pub allow_net: Vec<String>, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Do not document inbound allowlists as enforced.
The runtime rejects non-empty inbound allow_net values because enforcement is not available. Lines 112-114 state that allow_net restricts public access. Clients can rely on this text and then receive a configuration error. State that inbound allow_net must remain empty until enforcement is implemented.
Proposed documentation correction
-/// restricted to `allow_net`); `mode="disabled"` means private.
+/// `allow_net` must be empty until inbound allowlist enforcement is available.
+/// `mode="disabled"` means private.The PR objective states that non-empty inbound allowlists remain rejected.
🤖 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/types.rs` around lines 112 - 120, Update the
documentation above InboundNetworkSpec to state that inbound allow_net must
remain empty and non-empty values are rejected until enforcement is implemented;
remove the claim that it restricts publicly reachable services, while preserving
the mode descriptions.
G4614
force-pushed
the
network-info-inbound-outbound-split
branch
2 times, most recently
from
August 12, 2026 10:04
236c112 to
ef42749
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cli/src/cli.rs (1)
1451-1462: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the default inbound policy.
This test states that a bare command preserves the default network policy, but it only asserts outbound behavior. Add an assertion that
opts.network.inboundisInboundNetworkSpec::Enabledwith an empty allowlist. This pins the required public inbound default and prevents a compatibility regression.Proposed test update
assert!( matches!(opts.network.outbound, OutboundNetworkSpec::Enabled { ref allow_net } if allow_net.is_empty()) ); + assert!( + matches!(opts.network.inbound, InboundNetworkSpec::Enabled { ref allow_net } if allow_net.is_empty()) + );🤖 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/cli.rs` around lines 1451 - 1462, Extend test_network_flags_default_left_untouched to also assert that opts.network.inbound matches InboundNetworkSpec::Enabled with an empty allowlist, preserving the existing outbound assertion and confirming the default inbound policy for a bare run.
🤖 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.
Nitpick comments:
In `@src/cli/src/cli.rs`:
- Around line 1451-1462: Extend test_network_flags_default_left_untouched to
also assert that opts.network.inbound matches InboundNetworkSpec::Enabled with
an empty allowlist, preserving the existing outbound assertion and confirming
the default inbound policy for a bare run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cf362336-4763-41cb-be9a-27e8ddc8d6f4
📒 Files selected for processing (5)
sdks/go/boxlite_test.gosdks/go/options.gosdks/node/src/options.rssrc/boxlite/src/runtime/options.rssrc/cli/src/cli.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- sdks/go/options.go
- sdks/node/src/options.rs
- sdks/go/boxlite_test.go
- src/boxlite/src/runtime/options.rs
NetworkSpec::enabled/disabled read as whole-spec constructors but only set the outbound direction, leaving inbound at its default — invisible at the call site now that inbound exists. Rename to outbound_enabled/outbound_disabled and document what each leaves untouched. Same problem in the messages: errors saying "network.mode" point nested callers at a field that no longer exists. Say network.outbound.mode where the check is outbound-specific, and make NetworkMode::from_str's message direction-neutral — it parses both directions now, so "invalid network.mode" was wrong for inbound input. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
G4614
force-pushed
the
network-info-inbound-outbound-split
branch
from
August 12, 2026 11:53
ef42749 to
a4938d0
Compare
Turning NetworkSpec into a two-direction struct broke every pre-split Rust caller: brace-variant literals and match arms have no struct equivalent, so there was no migration short of editing each site. Give the name back to the outbound enum instead — same name, same variants, same shape — and call the container NetworkPolicy. Pre-split literals and match arms now compile untouched; assigning one to BoxOptions::network needs only .into(), via a new From<NetworkSpec> for NetworkPolicy. OutboundNetworkSpec survives as a direction-explicit alias for new code, since the bare name reads as if it covered both directions. network_spec.rs::pre_split_network_spec_source_shape_still_compiles pins the contract: it exercises the old literal, match, and assignment forms, so a future reshape that breaks them fails the build. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Builds on the NetworkSpec outbound/inbound split (the core reshape in
this PR's base): every user-facing surface can now set the inbound
policy, and read it back.
- CLI: --inbound MODE (enabled=public default, disabled=private);
--inbound-allow-net is deliberately not exposed until enforcement
exists
- serve wire: nested network.inbound accepted alongside outbound
- Python/Node/C/Go bindings: inbound setters/fields on the input side,
mirroring outbound's shape
- NetworkInfo reshaped to {outbound, inbound, published_ports}, each
direction a NetworkDirectionInfo{mode, allow_net}, propagated through
all four SDK output bindings (C header regenerated by cbindgen)
A non-empty inbound allowlist stays rejected everywhere (try_from,
sanitize, Go buildCOptions) — the field exists for shape symmetry only
until a runtime sink enforces it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
G4614
force-pushed
the
network-info-inbound-outbound-split
branch
from
August 12, 2026 13:03
a4938d0 to
532ca4b
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.
Summary
Stacked on #996 (the core
NetworkSpecoutbound/inbound reshape). This PR is the exposure half: every user-facing surface can now set the inbound policy, and read it back.--inbound MODE(enabled= public, default;disabled= private).--inbound-allow-netis deliberately not exposed until enforcement exists — a flag that can only error would advertise a feature that doesn't work.network.inboundaccepted alongsideoutbound.NetworkInforeshaped to{outbound, inbound, published_ports}, each direction aNetworkDirectionInfo{mode, allow_net}, propagated through all four SDK output bindings (C header regenerated by cbindgen).A non-empty inbound allowlist stays rejected everywhere (
NetworkSpec::try_from,BoxOptions::sanitize, GobuildCOptions) — the field exists for shape symmetry only until a runtime sink enforces it.Before/after
Merge-order constraint
This PR makes the REST client send the nested
{outbound, inbound}wire shape. The cloud API only understands that shape after #1199 — against the current API, a nested payload 400s (the old DTO's top-levelmodeis required). #1199 must merge and deploy before this PR (#996 can land at any point in between; its client still sends the legacy flat shape, which every API version accepts).Stacking note
GitHub can't set a fork branch as base, so this diff shows #996's commits too until #996 merges; review this PR by its head commit (
f696d35e5) or wait for #996 to land, after which the diff collapses to just the exposure change (34 files, +1151/−261).Test plan
cargo check -p boxlite-python --testsclean (cargo test -p boxlite-pythonfails to link libpython on current main in this environment — pre-existing, verified against a clean origin/main worktree)make test:unit:gogreenmake fmt:check:rustclean🤖 Generated with Claude Code
Replaces #1198 (closed when its branch was deleted; GitHub can't re-associate a recreated branch).
Summary by CodeRabbit