feat(sandbox-envs): per-environment egress policy UI#1336
Merged
Conversation
Address PR-review findings on the per-environment egress policy feature. - An empty/untouched egress editor now stores egress_policy=None instead of an explicit deny-all, so re-saving a network-enabled env no longer changes its runtime network behavior. To block all egress, use Network off. - Reject credential header names with control chars / CR-LF and values with newlines, guarding against header injection into the sidecar. - Remove references to the nonexistent DAIV_SANDBOX_EGRESS_PROXY_ENABLED setting from the error message, model comment, docs, and tests. - Log egress-proxy-unavailable distinctly (still fail-closed) when a fresh session aborts, instead of the generic seed-failure message. - Trim the CHANGELOG entry to the user-facing feature (Keep a Changelog); correct the "empty hosts = no outbound access" overstatement in docs. - Drop dead code (has_egress property, blankHost label) and extract _empty_egress_state() and the _MASK_SENTINEL constant to remove duplicated literals.
srtab
force-pushed
the
worktree-egress-policy-ui
branch
from
a6ecbfc to
6ff5d95
Compare
…itor
The "Unlisted hosts" toggle bound to an Alpine state property named
"default", a reserved JavaScript keyword. Alpine compiles directive
expressions inside a with(scope){...} block, where a bare reserved word
raises a SyntaxError, so x-model/:class/x-show all failed silently: the
radios never bound (not selectable), the active highlight never applied,
and the amber note misbehaved.
Rename the state property to "defaultPolicy". The JSON wire keys stay
"default" on read and write, so the form-parsing contract is unchanged.
) * feat(sandbox): send egress block (plaintext secrets) at start_session; drop configure_egress * feat(sandbox-envs): drop network_enabled; derive network from egress_policy presence * feat(sandbox-envs): resolve egress from effective env; fail closed to no-network; drop network_enabled * test(sandbox-envs): reconcile row_to_override fail-closed tests to egress=None + add non-DB guard * feat(sandbox): attach egress at start_session; drop separate provisioning + warm re-provision * feat(sandbox-envs): On/Off network toggle; On must permit something; allow-all stores policy * feat(mcp): derive env network_enabled output from egress_policy presence * feat(sandbox-envs): migration drops network_enabled, preserving off intent * test(sandbox-envs): reconcile env tests to egress-derived network; drop network_enabled stragglers - test_forms.py: replace all network_choice="default" (invalid choice) with "off"; rewrite test_form_network_choice_maps_to_network_enabled → maps_to_egress_policy (drop "default" param, assert egress_policy presence instead of network_enabled attr); fix prefill tests to use egress_policy=... instead of network_enabled=True/None; add network_choice="on" to egress tests that needed it; tighten test_on_with_no_rules_and_deny_default_is_rejected to assert only "network_choice" in errors (not __all__) - test_models.py: replace network_enabled=True/False with egress_policy=.../None in summary/short_summary tests - test_services.py: replace SandboxEnvOverride(network_enabled=...) with egress=...; consolidate _override helper into _ov (drop the stale network_enabled-carrying one); rewrite test_merge_falls_back_to_global_egress to use per_run=None (the correct egress-fallback scenario under new semantics); fix test_humanise_global_default and test_get_global_default to use egress_policy instead of network_enabled - test_views.py: replace all network_choice="default" with "off" in POST payloads * test(sandbox-envs): drop duplicate merge fallback test; rename per-run-off test * refactor(sandbox-envs): derive network from egress via is_networked Consolidate the network/egress model around single sources of truth and make network state explicit per environment. - Add SandboxEnvironment.is_networked (true iff egress_policy is set) as the single derivation; use it in summary, the form, the services layer, and the MCP get_environment output. - Extract EgressConfigRequest.to_wire() as the one serializer for the POST /session/ egress block (plaintext secrets + every policy field); the client calls it instead of hand-building the dict, so new policy fields flow through instead of being silently dropped. - Network is explicit per environment and never inherited from the global default; drop network from humanise_global_default and update the docs. - Narrow the egress-unavailable 400 match to the "egress proxy" marker so an unrelated 400 re-raises as-is, and simplify build/seed error logging.
… egress injection (#1340)
DAIV pushes from inside the sandbox, so the run's git platform must be reachable and write-capable for publish to work. Mint the GitHub egress token with `contents: write` (was `read`), and inject the runtime-only git-platform allow-rule even on Network-Off environments when a push token exists: such an env opens into a minimal deny-all carrying only the platform rule, so publishing always works while token-less eval/ benchmark runs stay fully network-isolated. Also self-heal GitLab clones from a stale cached clone token: a clone rejected for auth while using the ephemeral token now drops it and retries once with a freshly minted one (the PAT fallback is not retried). Auth-error detection is factored into a shared `core.utils.is_git_auth_error_text` used by both the clone-retry and the push-failure classifier.
srtab
added a commit
that referenced
this pull request
Jun 30, 2026
* feat(sandbox): add egress proxy wire schemas
* feat(sandbox): client.configure_egress provisions the sidecar policy
* feat(sandbox-envs): store per-env egress policy + encrypted secrets
* feat(sandbox-envs): carry typed egress config through the runtime merge
* feat(sandbox): provision egress policy at session start, fail-closed
* docs(sandbox-envs): document per-env egress policy and deny-all caveat
* fix(sandbox): harden egress fail-closed semantics
Only HTTP 404 (egress proxy disabled — permanent) converts to
SandboxEgressUnavailableError. 409 (ambiguous "session busy" vs "no
proxy", already classified as transient) and 5xx now propagate
unchanged instead of being mislabeled as a permanent "enable the
proxy" diagnosis; they still fail closed via the existing cleanup.
Re-provision egress on warm session reuse so the guarantee holds on
resumed turns too, without force-closing the resumable container on a
possibly transient blip.
When stored egress config is unusable (rotated DAIV_ENCRYPTION_KEY,
hand-edited row), fail closed to an empty deny-all policy rather than
dropping to None — None would let a network-enabled session fall back
to raw network or the sidecar default.
Extract EgressConfigRequest.from_stored() as the single source of
truth for mapping persisted policy/secrets onto the wire schema, and
simplify egress precedence in merge_sandbox_runtime via the generic
pick() helper.
* fix(sandbox): align egress contract with CA-gated proxy model
The earlier egress commits described the sandbox proxy as gated by a
DAIV_SANDBOX_EGRESS_PROXY_ENABLED boolean flag, but daiv-sandbox enables
egress by configuring a shared MITM CA (EGRESS_CA_CERT_FILE +
EGRESS_CA_KEY_FILE) instead. A network-enabled session is built as a
triad and reaches the internet only through the per-session proxy; if no
CA is configured the run is rejected up front rather than dropping to
raw network.
Correct the contract everywhere it was described: the actionable
SandboxEgressUnavailableError message, the provisioning docstrings, the
SandboxEnvironment/services comments, and the test rationale comments.
Clarify that the 404 fail-closed path is effectively unreachable on a
fresh create (rejected at start_session) but still matters for warm
reuse after CA rotation.
Wire the local docker setup to the real mechanism: mount ./data/certs
read-only and set DAIV_SANDBOX_EGRESS_CA_{CERT,KEY}_FILE, replacing the
raw-network DAIV_SANDBOX_EXTRA_HOSTS sibling-resolution approach with
egress-only routing through the proxy.
* feat(sandbox-envs): per-environment egress policy UI (#1336)
* fix(sandbox): resolve git-platform egress credential after clone
Address review findings on the per-environment egress work:
- Resolve the git-platform egress credential AFTER the repo clone in
set_runtime_ctx, so it observes any token the GitLab clone self-heal
re-minted. The proxy overrides Authorization on every platform request,
so a pre-clone credential would pin the sidecar to the stale token the
clone just discarded and break the in-sandbox push.
- Harden the egress wire schema: reject CR/LF and blank host/header on
EgressRule/EgressSecret via a shared _no_crlf helper, closing the
header-injection gap on the from_stored/apply_platform_egress paths
that bypass the form.
- Correct the fail-closed abort message to name the prefixed
DAIV_SANDBOX_EGRESS_CA_CERT_FILE / _KEY_FILE env vars.
- Log unusable egress config via logger.exception so the shape-error
cause (which nothing else logs) is captured, with the env name.
- Document the upgrade behaviour (existing network-enabled envs become
network-isolated, no auto-conversion) and fix two doc inaccuracies
(empty allow-list under Network On is a validation error; saved
credentials show a "keep existing" placeholder, not bullets).
- Add regression/branch tests: clone-before-credential ordering,
_validate_egress non-dict/size-cap branches, is_git_auth_error_text
contract, env_vars decryption-swallow, and the new CR/LF validators.
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
Adds a first-class editor for per-environment network egress policy to the sandbox-environment drawer form, so users/admins can define allowed hosts, injected credentials, and
default/interceptmodes through the UI. This is a pure UI/form layer on top of the already-built egress backend (PR #1331) — no model, migration, schema, services, or middleware changes.What's included
forms.py): anegress_jsonhidden field;clean_egress_json(validates + normalizes, uppercases HTTP methods to match the wire schema);_build_egress(builds the stored(egress_policy, egress_secrets)dicts, synthesizing named secrets); masked initial state (_initial_egress_json— stored credential plaintext never reaches the page); and preserve-unchanged-on-edit (_preserve_unchanged_egress_secrets, raises rather than silently destroying a still-valid secret on decryption failure). Validation is delegated tomodel.full_clean()(EgressConfigRequest.from_stored) — the form never re-implements the stored shape.egress-editor.js): hosts list with inline credentials; mints a stable per-hostsecret_name; serialises to the hidden field._form_body.html): the Network control is promoted to a top-level control; a "Network access" egress<fieldset>is shown only when the effective network state is on (effectiveNetworkOn);resource-control.jsde-networked.docs/features/sandbox-environments.md.Always stores an explicit policy (fail-closed: no allowed hosts ⇒ no outbound access).
Verification
sandbox_envssuite: 168 passed.mkdocs build --strict) passes for tracked docs.Reviewer notes
make tailwind-build, needs the docker stack) couldn't be verified headless. The no-leak view test confirms server-side rendering; please eyeball the drawer in a browser before merge.env_varsleniency; (2) a latent shared-injectlast-write-wins in_build_egressis only reachable for ORM-authored policies (the UI mints a uniquesecret_nameper host, so it cannot create this).