Repository navigation
[Bubblewrap] Add regression coverage for the allowLocalNetwork presence guard #982
Description
Description of the task
local_network_rejection (src/backends/bubblewrap/common/src/bwrap_command.rs) guards its rejection on allow_local_network_specified so that an omitted network.allowLocalNetwork is never rejected. The field defaults to false, which is also a value a caller may set explicitly, so without the presence flag every 0.8 request that picks defaultPolicy: 'allow' and stays silent on inbound policy would be refused.
That guard has no test coverage. Deleting the three-line guard entirely leaves all 202 bwrap_common tests passing:
if !request.policy.allow_local_network && !request.policy.allow_local_network_specified {
return None;
}
honored_local_network_is_not_rejected_at_0_8 looks like it covers this case but does not: it uses defaultPolicy: 'block', which resolves to a private namespace, where local_network_diagnostic_for_mode returns None on the mode arm whether or not the guard is present. Nothing exercises the omitted + shared-namespace combination the guard exists for.
Verified by probe — with the guard removed, a request with defaultPolicy: 'allow' and no allowLocalNetwork returns:
Bubblewrap: network.allowLocalNetwork=false is not enforced while the sandbox shares the host network namespace (defaultPolicy='allow')
so the guard is load-bearing, just unenforced by the suite.
Expected outcome
A unit test asserting local_network_rejection returns None for a 0.8 request with default_network_policy = Allow and allow_local_network_specified = false, failing if the guard is removed.
Additional context
Found while responding to review feedback on #956 asking whether allow_local_network_specified is a redundant field. It is not, but the suite currently cannot demonstrate that — the field could be deleted and CI would stay green.
Same presence-tracking pattern as network_specified and ui_specified (IsolationSession uses it for the same "defaults are indistinguishable from an explicit lockdown" reason), so the test also pins a convention used across backends.
Activity
github-actions commentedon Aug 21, 2026
Triage summary
Area-Test-Executor— this task is specifically about adding regression test coverage for the Bubblewrap executor'slocal_network_rejectionpresence guard.Maintainers: Comment
/investigateto check whether this issue or bug is valid against the most current code. Copilot will also produce a report with the changes that will be needed. For a small, unambiguous documentation or test fix, it may also create one draft PR.SohamDas2021 commentedon Aug 21, 2026
Superseded by the fix itself, in e1ef863 on #956.
The guard this issue proposed pinning was a fail-open, so pinning it would have been wrong.
falseis the schema default and a deny, so an omittedallowLocalNetworkis still a request for inbound denial; exempting callers who did not write the field meant a baredefaultPolicy: "allow"ran with a deny the shared namespace never enforces. Backends do not get to choose default-allow when they cannot honor a restriction, and IsolationSession already takes the strict reading of this same field (validate_provision_network_policyrequires it by value and refuses an absent policy as "a restriction the backend cannot honor").So the guard was removed rather than tested, and
allow_local_network_specifiedwent with it -- the guard was its only production consumer, which leavesmodels.rsandnetwork_parser.rswith no net diff in that PR.The coverage gap this issue was really about is closed:
an_omitted_local_network_is_rejected_like_an_explicit_deny_at_0_8-- the default-deny stanceacknowledged_local_network_exposure_is_accepted_at_0_8-- theallowLocalNetwork: trueescape hatchbubblewrap_network_localnet_omitted_rejected.json/bubblewrap_network_localnet_acknowledged.json-- the same two end to end, the second asserting the sandbox really is in the host netns so "reject the deny" cannot be satisfied by refusing every shared-namespace requestReintroducing the guard now fails three tests; previously deleting it outright left all 202 passing.