feat: add egress/ingress Off switches for traffic blocking#2166
Open
levb wants to merge 8 commits intolev-ingress-controlfrom
Open
feat: add egress/ingress Off switches for traffic blocking#2166levb wants to merge 8 commits intolev-ingress-controlfrom
levb wants to merge 8 commits intolev-ingress-controlfrom
Conversation
Add `off` bool to egress/ingress proto configs, OpenAPI spec, and DB types. When set, all traffic is denied immediately without iterating rules. Replaces the legacy `allow_internet_access=false` pattern of injecting 0.0.0.0/0 into denied list. Off is mutually exclusive with allow/deny rules (validated at API layer). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rename validateEgressRules → validateEgress, validateIngressRules → validateIngress. Each now takes an `off` bool and handles the mutual exclusivity check internally, removing duplicated logic from both the create and update handlers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move DerefOrDefault calls inside the validate functions so callers pass raw API pointer fields directly without pre-dereferencing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move the acl.Off check before the domain loop in isEgressAllowed. Previously, if Off=true coexisted with AllowedDomains (possible via direct gRPC), a matching domain would bypass the Off restriction. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add off to egress/ingress event data for observability. Remove ACL.IsOff() — all callers use acl.Off directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Off blocks unconditionally — including allow rules. The legacy allowInternetAccess=false path needs allow rules to take precedence over the deny, so revert to the 0.0.0.0/0 deny pattern for that code path. Off is only for the new explicit egressOff API field. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
dobrac reviewed Mar 19, 2026
Contributor
dobrac left a comment
There was a problem hiding this comment.
Lets not do the Off right now and implement only the "AllowInternetAccess" switch for the egress network setup in the update_network API call.
dobrac requested changes Mar 20, 2026
Contributor Author
| @dobrac I am not sure I see the value of adding this shortcut: it would accomplish nothing different from setting egress to Deny All, which can already be accomplished with the Update API. |
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
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
egressOffandingressOffboolean fields to the sandbox network config (OpenAPI, proto, DB types)allowInternetAccess=falsenow setsegressOff=trueinternally instead of injecting0.0.0.0/0into the deny listPUT /sandboxes/{id}/network)