kvcache-ai/AgentENV

Fork 0

mirror of https://github.com/kvcache-ai/AgentENV.git synced 2026-11-05 01:17:36 +00:00

[GH-ISSUE #156] NewSandbox silently drops allowInternetAccess: sandbox created with open egress when isolation was requested #54

New issue

Closed

opened 2026-10-29 14:18:53 +00:00 by bsakaguchi · 6 comments

bsakaguchi commented

2026-10-29 14:18:53 +00:00

Owner

Originally created by @sunkencity999 on GitHub (Aug 11, 2026).
Original GitHub issue: https://github.com/kvcache-ai/AgentENV/issues/156

Summary

NewSandbox declares allow_internet_access (snake_case) while the rest of the API — including the response model clients read back — uses allowInternetAccess (camelCase). A POST /sandboxes body that sets allowInternetAccess: false therefore has the field silently dropped, and the sandbox is created with the open base policy instead of the requested deny.

This is a fail-open outcome for a containment control, so flagging it as security-relevant rather than a cosmetic naming inconsistency.

The inconsistency

src/api/openapi.yml uses both spellings across four schemas:

Schema	Line	Spelling	Direction
`SandboxNetworkUpdateConfig`	334	`allow_internet_access`	request (update)
`SandboxDetail`	446	`allowInternetAccess`	response
`NewSandbox`	541	`allow_internet_access`	request (create)
`NewColdSandbox`	588	`allowInternetAccess`	request (cold create)

The generated model faithfully reproduces the spec, so src/api/generated/src/models.rs carries #[serde(rename = "allow_internet_access")] on NewSandbox.

The two create paths disagree with each other, and the primary create path disagrees with the response.

Why it fails open rather than erroring

From src/api/impls/sandbox.rs:

fn base_policy_from_allow_internet_access(value: Option<bool>) -> BaseSandboxNetworkPolicy {
    match value {
        Some(true)  => BaseSandboxNetworkPolicy::Allow,
        Some(false) => BaseSandboxNetworkPolicy::Deny,
        None        => BaseSandboxNetworkPolicy::Default,
    }
}

serde ignores the unknown camelCase key, so the field arrives as None → Default, not Deny. Nothing rejects the request and nothing warns.

The readback then makes it hard to notice: allow_internet_access_from_base_policy(Default) returns Nullable::Null, so GET /sandboxes/{id} reports "allowInternetAccess": null — which reads as "never set" rather than "your setting was discarded."

Impact

A client that sends the camelCase spelling gets an internet-enabled sandbox after explicitly asking for an isolated one. That spelling is the natural choice, because it is what SandboxDetail returns, what NewColdSandbox accepts, and what an E2B-compatible client would send given E2B's camelCase conventions.

There is a second-order effect on domain egress: allowOut domain rules are only armed under a Deny base policy, so a create call combining allowInternetAccess: false with domain rules produces a sandbox with no egress restriction at all — neither the deny nor the allowlist.

Reproduction

curl -sS -X POST http://127.0.0.1:8000/sandboxes \
  -H 'Content-Type: application/json' \
  -d '{"templateID":"ubuntu","allowInternetAccess":false}'

curl -sS http://127.0.0.1:8000/sandboxes/<id> | jq .allowInternetAccess
# → null   (expected: false)

From inside that sandbox, arbitrary outbound requests succeed.

Sending allow_internet_access: false instead behaves correctly, which is why the CLI path does not surface this — aenv network and the update model both use the snake_case spelling.

Suggested fix

Two parts, and the second matters more than the first:

Immediate: rename the NewSandbox field to allowInternetAccess to match the spec's response and cold-create schemas, with #[serde(alias = "allow_internet_access")] so existing snake_case clients keep working. Same alias on SandboxNetworkUpdateConfig so both endpoints accept both spellings.
Root cause: settle the spec on one spelling. Right now the two create schemas disagree, so any regeneration reintroduces this.

Worth considering separately: should an unrecognised field in a network-policy request be rejected rather than ignored? A deny_unknown_fields posture on these specific models would turn this class of bug into a 4xx instead of an open sandbox.

Offer

I have this fixed locally against v0.1.1 — the rename plus alias, alias on the update model, and regression tests covering camelCase and snake_case deserialization on both create and update, plus Deny base-policy propagation from a camelCase create body. Happy to open a PR against main if the approach above looks right to you, or to adjust it if you would rather fix the spec first and regenerate.

Verified present on main as of this writing.

Originally created by @sunkencity999 on GitHub (Aug 11, 2026). Original GitHub issue: https://github.com/kvcache-ai/AgentENV/issues/156 ### Summary `NewSandbox` declares `allow_internet_access` (snake_case) while the rest of the API — including the response model clients read back — uses `allowInternetAccess` (camelCase). A `POST /sandboxes` body that sets `allowInternetAccess: false` therefore has the field **silently dropped**, and the sandbox is created with the open base policy instead of the requested deny. This is a fail-open outcome for a containment control, so flagging it as security-relevant rather than a cosmetic naming inconsistency. ### The inconsistency `src/api/openapi.yml` uses both spellings across four schemas: | Schema | Line | Spelling | Direction | |---|---|---|---| | `SandboxNetworkUpdateConfig` | 334 | `allow_internet_access` | request (update) | | `SandboxDetail` | 446 | `allowInternetAccess` | **response** | | `NewSandbox` | 541 | `allow_internet_access` | request (create) | | `NewColdSandbox` | 588 | `allowInternetAccess` | request (cold create) | The generated model faithfully reproduces the spec, so `src/api/generated/src/models.rs` carries `#[serde(rename = "allow_internet_access")]` on `NewSandbox`. The two create paths disagree with each other, and the primary create path disagrees with the response. ### Why it fails open rather than erroring From `src/api/impls/sandbox.rs`: ```rust fn base_policy_from_allow_internet_access(value: Option<bool>) -> BaseSandboxNetworkPolicy { match value { Some(true) => BaseSandboxNetworkPolicy::Allow, Some(false) => BaseSandboxNetworkPolicy::Deny, None => BaseSandboxNetworkPolicy::Default, } } ``` serde ignores the unknown camelCase key, so the field arrives as `None` → `Default`, not `Deny`. Nothing rejects the request and nothing warns. The readback then makes it hard to notice: `allow_internet_access_from_base_policy(Default)` returns `Nullable::Null`, so `GET /sandboxes/{id}` reports `"allowInternetAccess": null` — which reads as "never set" rather than "your setting was discarded." ### Impact A client that sends the camelCase spelling gets an internet-enabled sandbox after explicitly asking for an isolated one. That spelling is the natural choice, because it is what `SandboxDetail` returns, what `NewColdSandbox` accepts, and what an E2B-compatible client would send given E2B's camelCase conventions. There is a second-order effect on domain egress: `allowOut` domain rules are only armed under a `Deny` base policy, so a create call combining `allowInternetAccess: false` with domain rules produces a sandbox with no egress restriction at all — neither the deny nor the allowlist. ### Reproduction ```bash curl -sS -X POST http://127.0.0.1:8000/sandboxes \ -H 'Content-Type: application/json' \ -d '{"templateID":"ubuntu","allowInternetAccess":false}' curl -sS http://127.0.0.1:8000/sandboxes/<id> | jq .allowInternetAccess # → null (expected: false) ``` From inside that sandbox, arbitrary outbound requests succeed. Sending `allow_internet_access: false` instead behaves correctly, which is why the CLI path does not surface this — `aenv network` and the update model both use the snake_case spelling. ### Suggested fix Two parts, and the second matters more than the first: 1. **Immediate:** rename the `NewSandbox` field to `allowInternetAccess` to match the spec's response and cold-create schemas, with `#[serde(alias = "allow_internet_access")]` so existing snake_case clients keep working. Same alias on `SandboxNetworkUpdateConfig` so both endpoints accept both spellings. 2. **Root cause:** settle the spec on one spelling. Right now the two create schemas disagree, so any regeneration reintroduces this. Worth considering separately: should an unrecognised field in a network-policy request be rejected rather than ignored? A `deny_unknown_fields` posture on these specific models would turn this class of bug into a 4xx instead of an open sandbox. ### Offer I have this fixed locally against v0.1.1 — the rename plus alias, alias on the update model, and regression tests covering camelCase and snake_case deserialization on both create and update, plus `Deny` base-policy propagation from a camelCase create body. Happy to open a PR against `main` if the approach above looks right to you, or to adjust it if you would rather fix the spec first and regenerate. Verified present on `main` as of this writing.

bsakaguchi closed this issue

2026-10-29 14:18:53 +00:00

bsakaguchi commented

2026-10-29 14:18:54 +00:00

Author

Owner

@LSX-s-Software commented on GitHub (Aug 12, 2026):

This field name is intentional and strictly aligned with the E2B API contract, so it is not a typo. Renaming it would break compatibility with E2B clients.

That said, we can consider adding an additional alias field in openapi.yml in a follow-up while keeping the current field as the canonical E2B-compatible name.

@LSX-s-Software commented on GitHub (Aug 12, 2026): This field name is intentional and strictly aligned with the [E2B API contract](https://github.com/e2b-dev/infra/blob/e1ce298526050cf6a78ffcdb4de06f30e6961cf4/spec/openapi.yml#L768), so it is not a typo. Renaming it would break compatibility with E2B clients. That said, we can consider adding an additional alias field in `openapi.yml` in a follow-up while keeping the current field as the canonical E2B-compatible name.

bsakaguchi commented

2026-10-29 14:18:54 +00:00

Author

Owner

@sunkencity999 commented on GitHub (Aug 12, 2026):

You're right, and thank you for the correction — I checked the spec you linked and E2B has exactly the same split: NewSandbox takes allow_internet_access, SandboxDetail returns allowInternetAccess. AgentENV is mirroring the contract faithfully, and renaming the create field would break E2B clients. I withdraw the rename proposal; the canonical name should stay as it is.

What I'd like to keep on the table is the behaviour rather than the name, because I don't think the E2B alignment resolves it:

A client that sends the wrong spelling gets an open sandbox, silently. serde drops the unknown key, base_policy_from_allow_internet_access(None) returns Default rather than Deny, and the sandbox comes up with unrestricted egress after isolation was explicitly requested. Nothing rejects the request, nothing logs, and GET /sandboxes/{id} then reports allowInternetAccess: null, which reads as "never set" rather than "your setting was discarded."

The reason clients land on the wrong spelling is the inherited inconsistency itself: allowInternetAccess is what the response hands back and what NewColdSandbox accepts, so it's a natural thing to send on create. That's an E2B-inherited trap, but AgentENV can decline to fail open on it.

So your proposed alias is exactly the right fix, and better than what I suggested. Keeping allow_internet_access canonical for E2B compatibility and adding allowInternetAccess as a serde alias means both spellings work, nothing breaks, and the trap closes. If it's useful I'm happy to rework my patch into that shape — alias only, canonical name untouched, plus the regression tests — or leave it entirely to you if you'd rather do it alongside the openapi.yml change.

One separable question worth considering on its own merits: should an unrecognised field in a network-policy request body be rejected rather than ignored? A deny_unknown_fields posture on NewSandbox / SandboxNetworkUpdateConfig specifically would turn this whole class of mistake into a 4xx instead of a silently-open sandbox, and it stays E2B-compatible because conforming clients only send known fields. It's a behaviour change so it may not be worth it for you — but for a containment control, "reject what I don't understand" seems like the safer default than "ignore it and open the network."

Either way, the alias alone fixes the reported problem. Happy to follow whichever route you prefer.

@sunkencity999 commented on GitHub (Aug 12, 2026): You're right, and thank you for the correction — I checked the spec you linked and E2B has exactly the same split: `NewSandbox` takes `allow_internet_access`, `SandboxDetail` returns `allowInternetAccess`. AgentENV is mirroring the contract faithfully, and renaming the create field would break E2B clients. I withdraw the rename proposal; the canonical name should stay as it is. What I'd like to keep on the table is the behaviour rather than the name, because I don't think the E2B alignment resolves it: **A client that sends the wrong spelling gets an open sandbox, silently.** serde drops the unknown key, `base_policy_from_allow_internet_access(None)` returns `Default` rather than `Deny`, and the sandbox comes up with unrestricted egress after isolation was explicitly requested. Nothing rejects the request, nothing logs, and `GET /sandboxes/{id}` then reports `allowInternetAccess: null`, which reads as "never set" rather than "your setting was discarded." The reason clients land on the wrong spelling is the inherited inconsistency itself: `allowInternetAccess` is what the response hands back and what `NewColdSandbox` accepts, so it's a natural thing to send on create. That's an E2B-inherited trap, but AgentENV can decline to fail open on it. **So your proposed alias is exactly the right fix, and better than what I suggested.** Keeping `allow_internet_access` canonical for E2B compatibility and adding `allowInternetAccess` as a serde alias means both spellings work, nothing breaks, and the trap closes. If it's useful I'm happy to rework my patch into that shape — alias only, canonical name untouched, plus the regression tests — or leave it entirely to you if you'd rather do it alongside the `openapi.yml` change. **One separable question worth considering on its own merits:** should an unrecognised field in a network-policy request body be rejected rather than ignored? A `deny_unknown_fields` posture on `NewSandbox` / `SandboxNetworkUpdateConfig` specifically would turn this whole class of mistake into a 4xx instead of a silently-open sandbox, and it stays E2B-compatible because conforming clients only send known fields. It's a behaviour change so it may not be worth it for you — but for a containment control, "reject what I don't understand" seems like the safer default than "ignore it and open the network." Either way, the alias alone fixes the reported problem. Happy to follow whichever route you prefer.

bsakaguchi commented

2026-10-29 14:18:54 +00:00

Author

Owner

@LSX-s-Software commented on GitHub (Aug 12, 2026):

Thanks for raising this.

We looked into accepting both spellings. Serde supports field aliases, but the generated model is owned by OpenAPI Generator, and the rust-axum generator currently has no way to express or generate such an alias from openapi.yml. Adding it directly to the generated Rust file would be lost the next time we run code generation, so we do not have a maintainable way to support the alias in AgentENV today.

One separable question worth considering on its own merits: should an unrecognised field in a network-policy request body be rejected rather than ignored?

The rust-axum generator also does not generate Serde's deny_unknown_fields, even when a schema specifies additionalProperties: false. We verified this with both our pinned version and the latest stable release.

In summary, adding the attribute directly to generated models, or maintaining a one-off post-processing rule for these fields, would be difficult to keep in sync with code generation. It may be worth proposing alias and deny_unknown_fields support to the upstream rust-axum generator. If upstream supports it, we can revisit enabling it for the relevant request models here.

@LSX-s-Software commented on GitHub (Aug 12, 2026): Thanks for raising this. We looked into accepting both spellings. Serde supports field aliases, but the generated model is owned by [OpenAPI Generator](https://github.com/OpenAPITools/openapi-generator), and the `rust-axum` generator currently has no way to express or generate such an alias from `openapi.yml`. Adding it directly to the generated Rust file would be lost the next time we run code generation, so we do not have a maintainable way to support the alias in AgentENV today. > One separable question worth considering on its own merits: should an unrecognised field in a network-policy request body be rejected rather than ignored? The `rust-axum` generator also does not generate Serde's `deny_unknown_fields`, even when a schema specifies `additionalProperties: false`. We verified this with both our pinned version and the latest stable release. In summary, adding the attribute directly to generated models, or maintaining a one-off post-processing rule for these fields, would be difficult to keep in sync with code generation. It may be worth proposing alias and `deny_unknown_fields` support to the upstream `rust-axum` generator. If upstream supports it, we can revisit enabling it for the relevant request models here.

bsakaguchi commented

2026-10-29 14:18:54 +00:00

Author

Owner

@sunkencity999 commented on GitHub (Aug 12, 2026):

That's a fair call and I don't think you should merge the patch — thanks for taking the time to explain the reasoning rather than just closing it. Drift on regenerated files is a real maintenance cost, and "we verified rust-axum doesn't emit deny_unknown_fields even from additionalProperties: false" is more diligence than the question strictly required.

I won't push on the alias. But there may be a fix for the dangerous part that sits entirely in hand-written code, so codegen never touches it.

The narrower problem

Independent of any spelling question: allowOut rules are silently inert unless the base policy is Deny.

From build_user_egress_commands, a Deny base emits ACCEPT rules for the allowed CIDRs followed by a catch-all -d 0.0.0.0/0 -j REJECT. Under Default the catch-all is absent, so those same ACCEPT rules sit on top of an already-permissive chain and change nothing. denyOut still works; the allowlist does not.

So a caller who sends allowOut without also setting allow_internet_access: false gets a sandbox with unrestricted egress and no indication anything was ignored. has_runtime_egress_rules() returns true, the policy is stored, and GET /sandboxes/{id} reflects the rules back — they just aren't restricting anything.

That combination seems very unlikely to ever be intentional. An allowlist under an open base policy is either a misunderstanding of the semantics or, as in the case that started this thread, the fallout of a field that didn't deserialize.

Suggested fix, no generated files involved

In network_policy_from_create and network_policy_from_update (both in src/api/impls/sandbox.rs, hand-written), reject the combination:

if policy.has_explicit_allow_rules() && policy.base_policy != BaseSandboxNetworkPolicy::Deny {
    anyhow::bail!(
        "allowOut requires allow_internet_access: false — an allowlist has no effect \
         under the default (open) base policy"
    );
}

This survives cargo adev codegen server because it lives in the impl layer, not the generated models. It is also arguably correct on its own merits: today the API accepts a request whose stated intent it cannot honour.

And it happens to catch the original case as a side effect. {"allowInternetAccess": false, "network": {"allowOut": [...]}} currently yields base Default plus inert allow rules and an open sandbox; with this check it becomes a 400 that names the field the caller should have used. Not as clean as the alias, but it converts the silent-open outcome into a loud one without any generated-code maintenance.

Two things I'd want your view on before writing it:

Should it be a hard bail! or a warning? A hard error is a behaviour change and could break an existing caller who is passing redundant allowOut rules today. A logged warning is safer but leaves the failure quiet.
Should denyOut under a Deny base get the same treatment? It's redundant rather than dangerous, so probably not worth the noise — but the asymmetry is worth being deliberate about.

Happy to open a PR if you like the shape of it. Equally happy to leave it — you have better context than I do on which callers exist in the wild.

On the generator route: agreed that alias and deny_unknown_fields support in rust-axum would be the durable answer, and it would benefit every project generating from that template rather than just this one. I don't want to promise work on the OpenAPI Generator side that I'm not certain I can deliver, so I'd rather not claim it — but if someone does pick it up, this thread is a concrete motivating case for it.

Either way, thanks for engaging with the substance.

@sunkencity999 commented on GitHub (Aug 12, 2026): That's a fair call and I don't think you should merge the patch — thanks for taking the time to explain the reasoning rather than just closing it. Drift on regenerated files is a real maintenance cost, and "we verified `rust-axum` doesn't emit `deny_unknown_fields` even from `additionalProperties: false`" is more diligence than the question strictly required. I won't push on the alias. But there may be a fix for the dangerous part that sits entirely in hand-written code, so codegen never touches it. ### The narrower problem Independent of any spelling question: **`allowOut` rules are silently inert unless the base policy is `Deny`.** From `build_user_egress_commands`, a `Deny` base emits ACCEPT rules for the allowed CIDRs followed by a catch-all `-d 0.0.0.0/0 -j REJECT`. Under `Default` the catch-all is absent, so those same ACCEPT rules sit on top of an already-permissive chain and change nothing. `denyOut` still works; the allowlist does not. So a caller who sends `allowOut` without also setting `allow_internet_access: false` gets a sandbox with unrestricted egress and no indication anything was ignored. `has_runtime_egress_rules()` returns true, the policy is stored, and `GET /sandboxes/{id}` reflects the rules back — they just aren't restricting anything. That combination seems very unlikely to ever be intentional. An allowlist under an open base policy is either a misunderstanding of the semantics or, as in the case that started this thread, the fallout of a field that didn't deserialize. ### Suggested fix, no generated files involved In `network_policy_from_create` and `network_policy_from_update` (both in `src/api/impls/sandbox.rs`, hand-written), reject the combination: ```rust if policy.has_explicit_allow_rules() && policy.base_policy != BaseSandboxNetworkPolicy::Deny { anyhow::bail!( "allowOut requires allow_internet_access: false — an allowlist has no effect \ under the default (open) base policy" ); } ``` This survives `cargo adev codegen server` because it lives in the impl layer, not the generated models. It is also arguably correct on its own merits: today the API accepts a request whose stated intent it cannot honour. And it happens to catch the original case as a side effect. `{"allowInternetAccess": false, "network": {"allowOut": [...]}}` currently yields base `Default` plus inert allow rules and an open sandbox; with this check it becomes a 400 that names the field the caller should have used. Not as clean as the alias, but it converts the silent-open outcome into a loud one without any generated-code maintenance. Two things I'd want your view on before writing it: 1. Should it be a hard `bail!` or a warning? A hard error is a behaviour change and could break an existing caller who is passing redundant `allowOut` rules today. A logged warning is safer but leaves the failure quiet. 2. Should `denyOut` under a `Deny` base get the same treatment? It's redundant rather than dangerous, so probably not worth the noise — but the asymmetry is worth being deliberate about. Happy to open a PR if you like the shape of it. Equally happy to leave it — you have better context than I do on which callers exist in the wild. On the generator route: agreed that alias and `deny_unknown_fields` support in `rust-axum` would be the durable answer, and it would benefit every project generating from that template rather than just this one. I don't want to promise work on the OpenAPI Generator side that I'm not certain I can deliver, so I'd rather not claim it — but if someone does pick it up, this thread is a concrete motivating case for it. Either way, thanks for engaging with the substance.

bsakaguchi commented

2026-10-29 14:18:54 +00:00

Author

Owner

@LSX-s-Software commented on GitHub (Aug 12, 2026):

Thanks for digging further into the runtime behavior. We compared this with the current E2B implementation and tests.

allowOut has the same semantics in AgentENV and E2B: it is an exception that takes precedence over matching denyOut rules, rather than an independent allowlist mode. Traffic is allowed by default unless it matches a deny rule. A deny-all rule, either supplied explicitly or introduced by allow_internet_access: false, is what turns allowOut into an allowlist.

E2B explicitly accepts IP/CIDR allowOut entries without deny-all, and its integration tests verify that unrelated destinations remain reachable in that case. allowOut is therefore not always inert under the default base policy: it can still override an overlapping partial denyOut.

Because of that, rejecting every request containing allowOut unless allow_internet_access is false would diverge from the E2B contract and would also reject valid allow-overrides-deny configurations.

We agree that the relationship between allowOut, denyOut, and allow_internet_access is easy to interpret as an allowlist. Improving the
schema description or documentation to make the precedence and default-allow behavior explicit would be reasonable, but we should preserve the existing E2B runtime semantics.

@LSX-s-Software commented on GitHub (Aug 12, 2026): Thanks for digging further into the runtime behavior. We compared this with the current E2B implementation and tests. `allowOut` has the same semantics in AgentENV and E2B: it is an exception that takes precedence over matching `denyOut` rules, rather than an independent allowlist mode. Traffic is allowed by default unless it matches a deny rule. A deny-all rule, either supplied explicitly or introduced by `allow_internet_access: false`, is what turns `allowOut` into an allowlist. E2B explicitly accepts IP/CIDR `allowOut` entries without deny-all, and its integration tests verify that unrelated destinations remain reachable in that case. `allowOut` is therefore not always inert under the default base policy: it can still override an overlapping partial `denyOut`. Because of that, rejecting every request containing `allowOut` unless `allow_internet_access` is false would diverge from the E2B contract and would also reject valid allow-overrides-deny configurations. We agree that the relationship between `allowOut`, `denyOut`, and `allow_internet_access` is easy to interpret as an allowlist. Improving the schema description or documentation to make the precedence and default-allow behavior explicit would be reasonable, but we should preserve the existing E2B runtime semantics.

bsakaguchi commented

2026-10-29 14:18:54 +00:00

Author

Owner

@sunkencity999 commented on GitHub (Aug 12, 2026):

You're right, and my claim was wrong — thank you for checking it against E2B's tests rather than taking my word for it.

I over-generalised. I reasoned about the case where allowOut is the only egress rule, where the ACCEPT entries do sit redundantly on an open chain, and then generalised to "inert unless the base policy is Deny." That skipped the case that matters: since ACCEPTs are appended before REJECTs and iptables is first-match-wins, allowOut overriding an overlapping partial denyOut is both meaningful and clearly intentional. {allowOut: ["10.1.0.0/16"], denyOut: ["10.0.0.0/8"]} under the default base is a sensible policy, and my proposed check would have rejected it. Withdrawn — please don't take that patch.

So the precedence model is: allow-by-default, denyOut carves out exceptions, allowOut carves exceptions back out of those, and a deny-all (explicit or via allow_internet_access: false) is what turns the whole thing into an allowlist. That is coherent; it just isn't what I assumed from the field names.

Your documentation suggestion is the right resolution, and I'd like to take you up on it. It also fits the maintainability constraint from earlier in this thread rather nicely: schema descriptions live in openapi.yml, which is the source of truth you regenerate from, so unlike the serde alias it survives cargo adev codegen server by construction.

If it's useful, I'd propose tightening the descriptions along these lines:

allowOut — currently reads as an allowlist to anyone who hasn't traced the iptables ordering. Something like: "Destinations exempted from denyOut rules. Traffic is allowed by default, so allowOut on its own does not restrict anything — it only overrides overlapping denyOut entries. To use it as an allowlist, combine it with a deny-all denyOut or allow_internet_access: false."
allow_internet_access — worth stating explicitly that false is equivalent to a deny-all denyOut, and that this is what converts allowOut into an allowlist. The current description says the first part; the consequence for allowOut is the part people miss.
denyOut — a note that allowOut takes precedence over matching entries here, so the two are evaluated as exceptions rather than as independent lists.

Happy to open a PR with just that — openapi.yml descriptions plus whatever regenerates from them, no behaviour change, no hand-edits to generated files. Or if you'd rather word it yourselves, this thread has the substance and you should feel free to take it without a PR from me.

Either way I'd suggest closing this issue once the wording lands: the original report was based on a misreading of intended semantics, and the residual is a documentation gap rather than a defect. Appreciate you working through it carefully instead of just closing it — I learned the actual model from your last two replies, which is a better outcome than the patch I came in with.

@sunkencity999 commented on GitHub (Aug 12, 2026): You're right, and my claim was wrong — thank you for checking it against E2B's tests rather than taking my word for it. I over-generalised. I reasoned about the case where `allowOut` is the *only* egress rule, where the ACCEPT entries do sit redundantly on an open chain, and then generalised to "inert unless the base policy is Deny." That skipped the case that matters: since ACCEPTs are appended before REJECTs and iptables is first-match-wins, `allowOut` overriding an overlapping partial `denyOut` is both meaningful and clearly intentional. `{allowOut: ["10.1.0.0/16"], denyOut: ["10.0.0.0/8"]}` under the default base is a sensible policy, and my proposed check would have rejected it. Withdrawn — please don't take that patch. So the precedence model is: allow-by-default, `denyOut` carves out exceptions, `allowOut` carves exceptions back out of those, and a deny-all (explicit or via `allow_internet_access: false`) is what turns the whole thing into an allowlist. That is coherent; it just isn't what I assumed from the field names. **Your documentation suggestion is the right resolution, and I'd like to take you up on it.** It also fits the maintainability constraint from earlier in this thread rather nicely: schema descriptions live in `openapi.yml`, which is the source of truth you regenerate *from*, so unlike the serde alias it survives `cargo adev codegen server` by construction. If it's useful, I'd propose tightening the descriptions along these lines: - **`allowOut`** — currently reads as an allowlist to anyone who hasn't traced the iptables ordering. Something like: *"Destinations exempted from `denyOut` rules. Traffic is allowed by default, so `allowOut` on its own does not restrict anything — it only overrides overlapping `denyOut` entries. To use it as an allowlist, combine it with a deny-all `denyOut` or `allow_internet_access: false`."* - **`allow_internet_access`** — worth stating explicitly that `false` is equivalent to a deny-all `denyOut`, and that this is what converts `allowOut` into an allowlist. The current description says the first part; the consequence for `allowOut` is the part people miss. - **`denyOut`** — a note that `allowOut` takes precedence over matching entries here, so the two are evaluated as exceptions rather than as independent lists. Happy to open a PR with just that — `openapi.yml` descriptions plus whatever regenerates from them, no behaviour change, no hand-edits to generated files. Or if you'd rather word it yourselves, this thread has the substance and you should feel free to take it without a PR from me. Either way I'd suggest closing this issue once the wording lands: the original report was based on a misreading of intended semantics, and the residual is a documentation gap rather than a defect. Appreciate you working through it carefully instead of just closing it — I learned the actual model from your last two replies, which is a better outcome than the patch I came in with.