feat(sandbox): alias/runtime sandbox defaults and persistent network allowlist #2888
There are no files selected for viewing
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,13 +18,87 @@ import ( | |
| "github.com/spf13/pflag" | ||
|
|
||
| "github.com/docker/docker-agent/pkg/config" | ||
| latestcfg "github.com/docker/docker-agent/pkg/config/latest" | ||
| "github.com/docker/docker-agent/pkg/environment" | ||
| "github.com/docker/docker-agent/pkg/paths" | ||
| "github.com/docker/docker-agent/pkg/sandbox" | ||
| "github.com/docker/docker-agent/pkg/sandbox/kit" | ||
| "github.com/docker/docker-agent/pkg/skills" | ||
| "github.com/docker/docker-agent/pkg/userconfig" | ||
| ) | ||
|
|
||
| // peekAgentSandbox returns true when the agent referenced by | ||
| // agentRef declares runtime.sandbox: true in its config. It is | ||
| // best-effort: any failure to resolve, load, or parse the config | ||
| // returns false so the caller falls through to the normal path, | ||
| // which will surface a proper error from the eventual load. | ||
| func peekAgentSandbox(ctx context.Context, agentRef string) bool { | ||
| cfg := loadAgentConfig(ctx, agentRef) | ||
| return cfg != nil && cfg.Runtime != nil && cfg.Runtime.Sandbox | ||
| } | ||
|
|
||
| // resolveSandboxDefault decides whether the sandbox path should be | ||
| // taken when the user did not pass --sandbox on the CLI. The first | ||
| // source that declares sandbox: true wins; in priority order: | ||
| // | ||
| // 1. an alias entry (`docker agent alias add ... --sandbox`); | ||
| // 2. the agent's own `runtime.sandbox: true`. | ||
| // | ||
| // Callers must only invoke this when the CLI flag was not set; an | ||
| // explicit --sandbox=<bool> always wins and bypasses this logic. | ||
| func resolveSandboxDefault(ctx context.Context, args []string, current bool) bool { | ||
| if current || len(args) == 0 { | ||
|
Contributor
There was a problem hiding this comment. [Suggestion #3] The function only reads |
||
| return current | ||
| } | ||
| if alias := config.ResolveAlias(args[0]); alias != nil && alias.Sandbox { | ||
| return true | ||
| } | ||
| return peekAgentSandbox(ctx, args[0]) | ||
| } | ||
|
|
||
| // agentNetworkAllowlist returns the hostnames the agent declared in | ||
| // runtime.network_allowlist. Same best-effort contract as | ||
| // peekAgentSandbox: any failure to load the config returns nil so | ||
| // the run continues with the inferred host set only. | ||
| func agentNetworkAllowlist(ctx context.Context, agentRef string) []string { | ||
| cfg := loadAgentConfig(ctx, agentRef) | ||
| if cfg == nil || cfg.Runtime == nil { | ||
|
Contributor
There was a problem hiding this comment. [Blocking #1] Second |
||
| return nil | ||
| } | ||
| return cfg.Runtime.NetworkAllowlist | ||
| } | ||
|
|
||
| // loadAgentConfig is the shared best-effort loader behind | ||
| // peekAgentSandbox / agentNetworkAllowlist. Returns nil on any | ||
| // resolve or load failure. | ||
| func loadAgentConfig(ctx context.Context, agentRef string) *latestcfg.Config { | ||
| if agentRef == "" { | ||
| return nil | ||
| } | ||
| source, err := config.Resolve(agentRef, nil) | ||
| if err != nil { | ||
| return nil | ||
| } | ||
| cfg, err := config.Load(ctx, source) | ||
| if err != nil { | ||
| return nil | ||
| } | ||
| return cfg | ||
| } | ||
|
|
||
| // userSandboxAllowlist returns the persistent host list the user has | ||
| // taught docker-agent to open via `docker agent sandbox allow`. | ||
| // Best-effort: a missing or unreadable user config returns nil so | ||
| // the sandbox falls back to the inferred set only. | ||
| func userSandboxAllowlist(ctx context.Context) []string { | ||
| cfg, err := userconfig.Load() | ||
| if err != nil { | ||
| slog.DebugContext(ctx, "Failed to load user config; skipping persistent sandbox allowlist", "error", err) | ||
| return nil | ||
| } | ||
| return cfg.SandboxAllowlist | ||
| } | ||
|
|
||
| // runInSandbox delegates the current command to a Docker sandbox. | ||
| // It ensures a sandbox exists (creating or recreating as needed), then | ||
| // executes docker agent inside it via the sandbox exec command. | ||
|
|
@@ -85,8 +159,13 @@ func runInSandbox(ctx context.Context, cmd *cobra.Command, args []string, runCon | |
| } | ||
| } | ||
|
|
||
| agentHosts := agentNetworkAllowlist(ctx, agentRef) | ||
| userHosts := userSandboxAllowlist(ctx) | ||
|
Contributor
There was a problem hiding this comment. [Blocking #1 — call site] |
||
|
|
||
| printModelsGateway(cmd.OutOrStdout(), runConfig.ModelsGateway) | ||
| printToolInstallAllowance(cmd.OutOrStdout(), kitResult) | ||
| printAgentNetworkAllowlist(cmd.OutOrStdout(), agentHosts) | ||
| printUserSandboxAllowlist(cmd.OutOrStdout(), userHosts) | ||
|
|
||
| name, err := backend.Ensure(ctx, wd, extras, template, configDir) | ||
| if err != nil { | ||
|
|
@@ -106,7 +185,7 @@ func runInSandbox(ctx context.Context, cmd *cobra.Command, args []string, runCon | |
| if kitResult != nil { | ||
| toolHosts = kitResult.ToolInstallHosts | ||
| } | ||
| allowSandboxHosts(ctx, backend, name, runConfig.ModelsGateway, toolHosts, agentHosts, userHosts) | ||
|
|
||
| // Resolve env vars the agent needs and forward them into the sandbox. | ||
| // Docker Desktop proxies well-known API keys automatically; this handles | ||
|
|
@@ -182,11 +261,12 @@ func dockerAgentArgs(cmd *cobra.Command, args []string, configDir string) []stri | |
|
|
||
| // allowSandboxHosts adds per-sandbox allow-network rules for every | ||
| // host the in-sandbox runtime is known to need: the configured | ||
| // models gateway (when set), the package hosts the auto-installer | ||
| // reaches for (when the kit build identified at least one | ||
| // auto-installable toolset), and any extra hosts the agent author | ||
| // declared in runtime.network_allowlist. The default sandbox proxy | ||
| // denies all of them; without this, the inner agent's first request | ||
| // returns a misleading "403 Blocked by network policy". | ||
| // | ||
| // Holes are punched only when the corresponding feature is in play: | ||
| // - the gateway host is added only when gatewayURL is non-empty; | ||
|
|
@@ -196,15 +276,21 @@ func dockerAgentArgs(cmd *cobra.Command, args []string, configDir string) []stri | |
| // module proxy + toolchain bootstrap for go_install packages, | ||
| // GitHub release hosts for github_release packages). When a | ||
| // lookup failed, the kit folds in [toolinstall.FallbackHosts] | ||
| // so the run can still succeed; | ||
| // - the agent-declared hosts come straight from the YAML and are | ||
| // unioned with the inferred set so authors can add hosts the | ||
| // resolver doesn't know about (custom MCP endpoints, third-party | ||
| // APIs, ...). | ||
| // | ||
| // Best-effort: a malformed gateway URL or a backend that doesn't | ||
| // support per-sandbox policies is logged at debug level and the run | ||
| // proceeds. The user will then see a network-policy 403 from the | ||
| // inner and we surface that diagnostic verbatim. | ||
| func allowSandboxHosts(ctx context.Context, backend *sandbox.Backend, name, gatewayURL string, toolInstallHosts, agentHosts, userHosts []string) { | ||
| var hosts []string | ||
| hosts = append(hosts, toolInstallHosts...) | ||
| hosts = append(hosts, agentHosts...) | ||
| hosts = append(hosts, userHosts...) | ||
|
|
||
| if gatewayURL != "" { | ||
| if h := gatewayHostPort(gatewayURL); h != "" { | ||
|
|
@@ -405,4 +491,36 @@ func printToolInstallAllowance(w io.Writer, kitResult *kit.Result) { | |
| for _, e := range kitResult.ToolInstallHostsResolutionErr { | ||
| fmt.Fprintf(w, " ! %s (using fallback host set)\n", e.Error()) | ||
| } | ||
| if len(kitResult.ToolInstallHostsResolutionErr) > 0 { | ||
| fmt.Fprintln(w, " hint: persist a missing host with `docker agent sandbox allow <host>`") | ||
| } | ||
| } | ||
|
|
||
| // printAgentNetworkAllowlist prints the host(s) the agent's config | ||
| // asked us to add to the sandbox proxy. Surfacing them next to the | ||
| // kit / gateway lines makes it obvious which holes were punched by | ||
| // the agent author vs auto-discovered, so an unexpected 403 has a | ||
| // short list of suspects. | ||
| func printAgentNetworkAllowlist(w io.Writer, hosts []string) { | ||
| if len(hosts) == 0 { | ||
| return | ||
| } | ||
| fmt.Fprintf(w, "Agent network allowlist: allowlisting %d host(s) declared in runtime.network_allowlist:\n", len(hosts)) | ||
| for _, h := range hosts { | ||
| fmt.Fprintf(w, " - %s\n", h) | ||
| } | ||
| } | ||
|
|
||
| // printUserSandboxAllowlist prints the host(s) the user has added | ||
| // via `docker agent sandbox allow`. Kept on its own line (separate | ||
| // from the agent-declared list) so it's clear which hosts persist | ||
| // across runs vs which travel with the agent config. | ||
| func printUserSandboxAllowlist(w io.Writer, hosts []string) { | ||
| if len(hosts) == 0 { | ||
| return | ||
| } | ||
| fmt.Fprintf(w, "User sandbox allowlist: allowlisting %d host(s) from `docker agent sandbox allow`:\n", len(hosts)) | ||
| for _, h := range hosts { | ||
| fmt.Fprintf(w, " - %s\n", h) | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion #8] Slightly misleading: the comment says "the sandbox path has already been taken" but since we are in
runOrExec(the non-sandbox branch), the sandbox path was not taken — it resolved to false. Suggested wording: "alias.Sandbox is consumed in runRunCommand; since we are here, the sandbox decision resolved to false (explicit --sandbox=false, or neither the alias nor the agent config requested it)."