Add sandbox network allowlists for Go and Python#1648
Merged
Conversation
|
Important UI Tests need review – Review now🟡 UI Tests: 5 visual and accessibility changes must be accepted as baselines |
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
EItanya
force-pushed
the
eitan/sandbox-network-allowlist
branch
from
c7851a6 to
15f6f0d
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces sandbox network allowlisting across the Go and Python runtimes while keeping sandboxed execution deny-by-default, and wires the allowlist into the generated runtime configuration consumed by srt.
Changes:
- Add
spec.sandbox.network.allowedDomainsto the Agent/SandboxAgent APIs and CRDs, and translate it into runtime config. - Generate and mount
srt-settings.jsonplusKAGENT_SRT_SETTINGS_PATHfor sandboxed execution paths; update Go and Python sandbox executors to require and pass--settings. - Add/extend unit + e2e tests for deny-by-default and allowlist-enabled scenarios (skills + execute_code).
Reviewed changes
Copilot reviewed 65 out of 65 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| python/packages/kagent-skills/src/kagent/tests/unittests/test_skill_execution.py | Updates skill execution tests to provide mounted srt settings and validate settings args usage. |
| python/packages/kagent-skills/src/kagent/skills/shell.py | Requires KAGENT_SRT_SETTINGS_PATH and passes --settings to srt for sandboxed command execution. |
| python/packages/kagent-adk/tests/unittests/test_sandbox_code_executor.py | Adds unit test ensuring execute_code runs in the session working directory and passes srt settings args. |
| python/packages/kagent-adk/tests/unittests/test_context_config.py | Adds AgentConfig parsing test for network allowlist. |
| python/packages/kagent-adk/src/kagent/adk/types.py | Introduces NetworkConfig and adds it to AgentConfig. |
| python/packages/kagent-adk/src/kagent/adk/tools/skills_plugin.py | Formatting-only change to function signature. |
| python/packages/kagent-adk/src/kagent/adk/tools/bash_tool.py | Formatting-only change to multiline execute_command invocation. |
| python/packages/kagent-adk/src/kagent/adk/sandbox_code_executer.py | Ensures sandboxed code execution uses srt settings args and session working directory. |
| python/packages/kagent-adk/src/kagent/adk/cli.py | Routes skills wiring through a new helper (currently config-aware entrypoint). |
| helm/kagent-crds/templates/kagent.dev_sandboxagents.yaml | Updates Helm-rendered SandboxAgent CRD schema to include spec.sandbox.network.allowedDomains. |
| helm/kagent-crds/templates/kagent.dev_agents.yaml | Updates Helm-rendered Agent CRD schema to include spec.sandbox.network.allowedDomains. |
| go/core/test/e2e/mocks/run_code_network.json | Adds mock LLM script/output for sandbox execute_code network deny/allow scenarios. |
| go/core/test/e2e/mocks/invoke_skill_network.json | Adds mock LLM tool-call/output for skills bash network deny/allow scenarios. |
| go/core/test/e2e/invoke_api_test.go | Adds e2e tests for deny-by-default and allowlist-enabled network access (skills + execute_code). |
| go/core/pkg/env/kagent.go | Registers KAGENT_SRT_SETTINGS_PATH env var with default /config/srt-settings.json. |
| go/core/internal/controller/translator/agent/testdata/outputs/tls-with-system-cas-disabled.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/tls-with-disabled-verify.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/tls-with-custom-ca.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/ollama_agent.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/bedrock_agent.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/basic_agent.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/anthropic_agent.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_system_message_from_secret.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_system_message_from_configmap.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_streaming.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_skills.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_security_context.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_scheduling_attributes.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_require_approval.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_proxy.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_proxy_service.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_proxy_mcpserver.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_proxy_mcpserver_custom_timeout.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_proxy_external_remotemcp.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_prompt_template.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_passthrough.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_nested_agent.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_memory.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_mcp_service.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_http_toolserver.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_git_skills.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_embedding_provider.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_default_sa.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_custom_sa.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_cross_namespace_tools.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_context_config.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_code.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/testdata/outputs/agent_with_allowed_headers.json | Updates golden manifests to include mounted srt-settings.json and env var/config-hash changes. |
| go/core/internal/controller/translator/agent/manifest_builder.go | Generates srt-settings.json, mounts it, sets KAGENT_SRT_SETTINGS_PATH, and includes settings in config hashing. |
| go/core/internal/controller/translator/agent/manifest_builder_test.go | Adds unit tests for default-deny srt settings JSON and needsSRTSettings behavior. |
| go/core/internal/controller/translator/agent/compiler.go | Plumbs spec.sandbox into manifest inputs and translates network allowlist into AgentConfig.Network. |
| go/core/internal/controller/translator/agent/adk_api_translator_test.go | Adds translator test coverage for network allowlist translation. |
| go/api/v1alpha2/zz_generated.deepcopy.go | Adds deepcopy support for new Sandbox/Network config types. |
| go/api/v1alpha2/agent_types.go | Adds spec.sandbox.network.allowedDomains to the v1alpha2 Agent API. |
| go/api/config/crd/bases/kagent.dev_sandboxagents.yaml | Updates generated CRD base schema for SandboxAgent to include sandbox network config. |
| go/api/config/crd/bases/kagent.dev_agents.yaml | Updates generated CRD base schema for Agent to include sandbox network config. |
| go/api/adk/types.go | Adds AgentConfig.Network.allowed_domains to the config schema flowing to runtimes. |
| go/api/adk/types_test.go | Adds unmarshalling test coverage for network.allowed_domains. |
| go/adk/pkg/tools/skills.go | Adds Go ADK skills + filesystem tool implementations (skills/read_file/write_file/edit_file/bash). |
| go/adk/pkg/tools/skills_test.go | Adds unit tests for path resolution constraints and expected tool registration. |
| go/adk/pkg/skills/shell.go | Executes commands via srt --settings ... sh -c ... and requires mounted srt settings path. |
| go/adk/pkg/skills/shell_test.go | Adds test scaffolding for fake srt and asserts missing settings env var errors. |
| go/adk/pkg/config/config_usage.go | Documents/config-logs the new network config field usage. |
| go/adk/pkg/agent/agent.go | Wires skills tools into the Go agent runtime when KAGENT_SKILLS_FOLDER is set. |
| go/adk/pkg/agent/agent_test.go | Adds unit test verifying skills tools are wired based on env var. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+163
to
169
| if cfg != nil || srtSettingsJSON != "" { | ||
| secretData := modelConfigSecretHashBytes | ||
| if secretData == nil { | ||
| secretData = []byte{} | ||
| } | ||
| configHash = computeConfigHash(bCfg, bCard, secretData) | ||
| cfgJSON = string(bCfg) | ||
| agentCard = string(bCard) | ||
| configHash = computeConfigHash([]byte(cfgJSON), []byte(agentCard), append(secretData, []byte(srtSettingsJSON)...)) | ||
| volumes = []corev1.Volume{{ |
There was a problem hiding this comment.
computeConfigHash is called with append(secretData, []byte(srtSettingsJSON)... ). If secretData aliases modelConfigSecretHashBytes and has spare capacity, append can mutate the underlying array, corrupting the original secret hash bytes and making subsequent hashing/logic nondeterministic. Build a new slice for hashing (e.g., clone/copy secretData first, then append) to avoid mutating the input slice.
Contributor
Author
There was a problem hiding this comment.
Fixed in 91a9217. The hash input now uses a fresh buffer before appending the SRT settings payload, so we no longer risk mutating the original secret hash bytes via append aliasing.
Comment on lines
+10
to
+20
| func TestResolveReadPath_AllowsSymlinkedSkillsDirectory(t *testing.T) { | ||
| skillsDir := t.TempDir() | ||
| skillFile := filepath.Join(skillsDir, "script.py") | ||
| if err := os.WriteFile(skillFile, []byte("print('ok')\n"), 0644); err != nil { | ||
| t.Fatalf("failed to write skill file: %v", err) | ||
| } | ||
|
|
||
| resolved, err := resolveReadPath("session-read", skillsDir, "skills/script.py") | ||
| if err != nil { | ||
| t.Fatalf("resolveReadPath() error = %v", err) | ||
| } |
There was a problem hiding this comment.
This test uses a constant session ID ("session-read") which causes GetSessionPath to create a fixed directory under os.TempDir()/kagent/. That directory isn’t cleaned up here and can collide across repeated/parallel test runs. Use a per-test unique session ID and/or isolate os.TempDir() (e.g., set TMPDIR to t.TempDir()) and remove the created session directory in a cleanup.
Contributor
Author
There was a problem hiding this comment.
Fixed in 91a9217. The test now isolates TMPDIR with t.TempDir() and uses a per-test session ID instead of a shared constant path under os.TempDir().
Comment on lines
+26
to
+34
| func TestResolveWritePath_BlocksSkillsSymlink(t *testing.T) { | ||
| skillsDir := t.TempDir() | ||
| _, err := resolveWritePath("session-write", skillsDir, "skills/new-file.txt") | ||
| if err == nil { | ||
| t.Fatal("expected write through skills symlink to be rejected") | ||
| } | ||
| if !strings.Contains(err.Error(), "outside the writable session directory") { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } |
There was a problem hiding this comment.
This test also uses a constant session ID ("session-write"), which creates/updates a shared os.TempDir()/kagent/session-write directory without cleanup. Leftover state can make this test flaky when run repeatedly or in parallel. Prefer a per-test unique session ID and cleanup (or set TMPDIR to an isolated temp dir for the duration of the test).
Contributor
Author
There was a problem hiding this comment.
Fixed in 91a9217. This test now also isolates TMPDIR with t.TempDir() and uses a per-test session ID, so it no longer shares or leaves behind a stable session directory.
Comment on lines
+48
to
+69
| bashDescription = `Execute bash commands in the skills environment with sandbox protection. | ||
|
|
||
| Working Directory & Structure: | ||
| - Commands run in a temporary session directory: /tmp/kagent/{session_id}/ | ||
| - /skills -> All skills are available here (read-only). | ||
| - Your current working directory and /skills are added to PYTHONPATH. | ||
|
|
||
| Python Imports (CRITICAL): | ||
| - To import from a skill, use the name of the skill. | ||
| Example: from skills_name.module import function | ||
| - If the skills name contains a dash '-', you need to use importlib to import it. | ||
| Example: | ||
| import importlib | ||
| skill_module = importlib.import_module('skill-name.module') | ||
|
|
||
| For file operations: | ||
| - Use read_file, write_file, and edit_file for interacting with the filesystem. | ||
|
|
||
| Timeouts: | ||
| - python scripts: 60s | ||
| - other commands: 30s` | ||
| ) |
There was a problem hiding this comment.
bashDescription states commands are executed with bash, but the current Go sandbox execution path (skills.ExecuteCommand) runs srt sh -c ... (POSIX sh), not bash. If users rely on bash-specific syntax, this can cause confusing runtime failures. Either switch the executor to invoke bash (if guaranteed present) or update the tool description (and/or tool naming) to reflect that commands run under sh.
Contributor
Author
There was a problem hiding this comment.
Fixed in 91a9217. The Go sandbox executor now invokes bash -c under srt, so the implementation matches the tool description instead of advertising bash while actually running sh.
EItanya
force-pushed
the
eitan/sandbox-network-allowlist
branch
from
bc8b52e to
91a9217
Compare
peterj
previously approved these changes
Apr 10, 2026
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
EItanya
force-pushed
the
eitan/sandbox-network-allowlist
branch
from
91a9217 to
d83e1ea
Compare
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
Testing