fix: make network isolation mode explicit instead of inferred#2469
Merged
Conversation
eleftherias
reviewed
Jul 15, 2026
eleftherias
left a comment
eleftherias
left a comment
Member
There was a problem hiding this comment.
The network mapping logic looks correct. I left one comment about ipv4 addresses.
Studio always sent `network_isolation: false`, silently overriding the backend's new default of isolating new workloads. A prior attempt (#2332) derived `insecure_allow_all` from whether the allow-list arrays were empty, but reviewers flagged that as ambiguous — it couldn't distinguish "isolate with no restrictions" from "isolate and block everything". Replaces the boolean toggle with an explicit `networkAccess` mode (none / host / proxy) and an explicit `allowedDestinations` (anywhere / selected) field that drives `insecure_allow_all` directly, consolidates the duplicated tab-content components into one, and wires up the previously-unused `permission_profile.network.mode: "host"` backend field. Closes #2326 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Wires the new backend `allow_docker_gateway` field (not yet released upstream; openapi.json patched against the unreleased backend PR) through to the "Allow host machine access" checkbox in both the local-server and registry-server payload builders. Also fixes three gaps found in review: - Registry install now reads the entry's actual declared `insecure_allow_all`/`network.mode` instead of inferring destinations from allow-list array length, so entries like `fetch` (declaring `insecure_allow_all: true`) and entries recommending host networking are honored, and an entry that explicitly declares an empty allow-list defaults to restricted rather than silently allowing everything. - The edit round-trip derived `allowedDestinations` from a strict `=== false` check, which misread an omitted (rather than explicit `false`) `insecure_allow_all` as unrestricted, risking silently widening a restricted server's egress on save. - Allowed-host validation now accepts IPv4 addresses, matching what the UI tooltip already advertised. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The "Network access" and "Allowed destinations" titles sat flush against their radio groups (James Tenniswood's review feedback). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The backend's Squid rule builder only ever emits a dstdomain ACL for allow_host entries (pkg/container/docker/squid.go), with no branch for IP-shaped values, so accepting IPv4 addresses client-side promised something the backend can't reliably enforce. Reverts to domain-only host validation and removes the "or IP addresses" tooltip claim. Also addresses review feedback on the Network access tab layout (James Tenniswood): the Allowed hosts/ports sections no longer show an empty add-a-host/add-a-port list by default, which read as if both were required. DynamicArrayField gains an opt-in collapseWhenEmpty mode that keeps the section title visible but only shows the add button until the user adds an entry, revealing the input row at that point. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
v0.37.0 ships allow_docker_gateway on the create/update workload request types for real, so the hand-patched openapi.json used to develop against the unreleased backend PR is no longer needed — regenerated via `pnpm run generate-client` against the tagged release. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces the ambiguous/inferred networkIsolation boolean with an explicit networking model (networkAccess: none|host|proxy plus allowedDestinations: anywhere|selected) so Studio can represent “no outbound allowed” vs “allow all” unambiguously, while also wiring up backend host networking mode and adding UI/tests for the new flow.
Changes:
- Introduces explicit network access modes and destination scoping; updates request shaping to drive
permission_profile+insecure_allow_alldirectly. - Replaces the old Network Isolation tab UI with a shared
NetworkAccessTabContent(radio groups + optional allow-list + host access checkbox). - Updates OpenAPI + generated types (including
allow_docker_gateway) and expands unit test coverage across local/registry flows.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/constants.ts | Bumps default ToolHive version to v0.37.0. |
| renderer/src/features/registry-servers/lib/orchestrate-run-registry-server.tsx | Builds registry create payloads using networkAccess/allowedDestinations and sets permission profile accordingly. |
| renderer/src/features/registry-servers/lib/tests/orchestrate-run-registry-server.test.tsx | Updates/extends tests for new network access fields and allow_docker_gateway. |
| renderer/src/features/registry-servers/components/form-run-from-registry/index.tsx | Updates registry run dialog defaults and tab content to the new Network access UI. |
| renderer/src/features/registry-servers/components/dynamic-array-field.tsx | Adds collapseWhenEmpty option for compact array field rendering. |
| renderer/src/features/registry-servers/components/tests/form-run-from-registry.test.tsx | Updates tests for renamed tab + new networking controls/defaults. |
| renderer/src/features/network-isolation/components/network-isolation-tab-content.tsx | Removes the old registry-oriented Network Isolation tab component. |
| renderer/src/features/network-isolation/components/network-access-tab-content.tsx | Adds the shared network access tab component used by local + registry forms. |
| renderer/src/features/network-isolation/components/tests/network-access-tab-content.test.tsx | Adds dedicated tests for NetworkAccessTabContent rendering and interactions. |
| renderer/src/features/mcp-servers/lib/orchestrate-run-local-server.tsx | Updates local create/update request shaping and form conversions to new network access model. |
| renderer/src/features/mcp-servers/lib/tests/orchestrate-run-local-server.test.tsx | Updates/extends tests for local create/update payloads and form conversion behavior. |
| renderer/src/features/mcp-servers/lib/tests/form-schema-local-mcp.test.ts | Updates form schema tests for new network access fields. |
| renderer/src/features/mcp-servers/hooks/tests/use-update-server.test.tsx | Updates hook tests to use new form fields. |
| renderer/src/features/mcp-servers/components/network-isolation-tab-content.tsx | Removes the old local-oriented Network Isolation tab component. |
| renderer/src/features/mcp-servers/components/local-mcp/dialog-form-local-mcp.tsx | Switches local MCP dialog to the new tab and updates defaults/field mapping. |
| renderer/src/features/mcp-servers/components/local-mcp/tests/dialog-form-local-mcp.test.tsx | Updates dialog tests for new networking UI and payload shape. |
| renderer/src/features/mcp-servers/components/tests/network-isolation-tab-content.test.tsx | Removes tests targeting the deleted Network Isolation component. |
| renderer/src/common/lib/form-schema-mcp.ts | Adds new network access enums and updates network validation gating logic. |
| common/api/openapi.json | Updates API schema (including allow_docker_gateway and runtime_env). |
| common/api/generated/types.gen.ts | Regenerates TS types reflecting updated OpenAPI schema. |
eleftherias
approved these changes
Jul 15, 2026
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.
recording-2026-07-15_12-11-43.mp4
Summary
Aligns Studio with the ToolHive backend's hardened default (network isolation on for new workloads) and resolves the design flaw flagged in review on #2332.
networkIsolationtoggle with an explicitnetworkAccess: 'none' | 'host' | 'proxy'mode, plus an explicitallowedDestinations: 'anywhere' | 'selected'field that now directly drivesinsecure_allow_all— instead of inferring it from whether the allow-list arrays happened to be empty (the ambiguity Copilot and @kantord flagged on fix: default network isolation to enabled for new workloads #2332: a registry entry that intentionally declares zero outbound permissions could previously get silently treated as "allow all").permission_profile.network.mode: "host", giving the new "Host networking (Advanced)" option a real effect.NetworkIsolationTabContentcomponents (local + registry forms) into one shared, genericNetworkAccessTabContent.allow_docker_gatewayfield on create/update yet (confirmed againststacklok/toolhivemain and the latest release); a companion backend change exists in a separate worktree to close that gap, to be tracked as a follow-up once merged upstream.Closes #2326
Test plan
pnpm run type-checkcleanpnpm run lintcleanpnpm run test:nonInteractive— 2527/2527 tests passing, including 17 new tests dedicated toNetworkAccessTabContent's rendering/interactionspnpm run knip— no new dead codebetter-sqlite3fails to compile against Electron 43's V8 headers; not caused by this change)Fully or partially written by an AI agent.
🤖 Generated with Claude Code