McpToolset config passes command and args to exec.Command without validation

The McpToolset factory in internal/configurable takes the command and args values out of an agent's YAML config and passes them to exec.Command with no validation of any kind. Loading a config that names /bin/sh with ["-c", "…"] is accepted, and the shell runs with the privileges of the host process as soon as the toolset is used.

The values are read at configurable_utils.go#L209-L216 and reach exec.Command at configurable_utils.go#L238-L243. There is no allowlist, no path resolution and no argument filtering anywhere between the two.

When the process actually starts

Building the toolset does not spawn anything. mcptoolset.New only constructs the transport. The command runs on the first Tools() call, which is what an agent does when it runs. So merely loading a config is not enough — the agent has to be used.

How far it reaches today

internal/configurable is imported only by cmd/internal/adkcli and by packages inside internal/configurable itself. The shipped cmd/adkgo CLI does not reach this path at all, so no released binary exposes it to an end user today.

Within the repository, cmd/internal/adkcli is a package main that recursively walks the working directory for every file named root_agent.yaml (main.go#L61-L74) and loads each one it finds (main.go#L92). It loads the agents but does not run them, so on its own it does not spawn the command. Anything built on the config loader that also runs the agent does.

That containment is the reason this reads as hardening rather than an incident, and it is also the reason it is worth fixing before the config loader gains a public entry point.

Reproduction

Drop this into internal/configurable/ and run go test -count=1 -run TestMcpToolsetSpawnsArbitraryCommand -v ./internal/configurable. It passes on 11521a4a, meaning the command ran.

package configurable

import (
	"context"
	"os"
	"path/filepath"
	"testing"
	"time"

	"google.golang.org/adk/v2/agent"
)

type reproCtx struct{ agent.StrictContextMock }

func TestMcpToolsetSpawnsArbitraryCommand(t *testing.T) {
	marker := filepath.Join(t.TempDir(), "pwned")

	// Exactly the shape internal/configurable decodes out of root_agent.yaml.
	args := map[string]any{
		"stdio_connection_params": map[string]any{
			"server_params": map[string]any{
				"command": "/bin/sh",
				"args":    []any{"-c", "touch " + marker + "; sleep 30"},
			},
		},
		"tool_filter": []any{"anything"},
	}

	_, toolset, err := ResolveToolReference(context.Background(), "McpToolset", args)
	if err != nil {
		t.Fatalf("ResolveToolReference() error = %v, want nil (config was accepted)", err)
	}
	if _, err := os.Stat(marker); err == nil {
		t.Fatal("marker exists before the toolset was used")
	}

	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
	defer cancel()
	// Tools() is what an agent calls when it actually runs.
	_, _ = toolset.Tools(&reproCtx{agent.NewStrictContextMock(ctx)})

	if _, err := os.Stat(marker); err != nil {
		t.Fatalf("marker %q was not created: %v", marker, err)
	}
	t.Logf("PWNED: %q created by a command named only in the config", marker)
}

=== RUN   TestMcpToolsetSpawnsArbitraryCommand
    zz_repro_test.go:48: PWNED: ".../001/pwned" created by a command named only in the config
--- PASS: TestMcpToolsetSpawnsArbitraryCommand (10.01s)

Constraints on any solution

Two facts about the tree, both learned the hard way in #923, which attempted this and is being closed unmerged.

An opt-in switch placed under internal/ cannot be turned on by anyone outside the module. #923 added SetGlobalMCPPolicy in internal/configurable and pointed operators at it in an error message, which for an external consumer produces use of internal package … not allowed from the compiler. Whatever carries the operator's intent has to live somewhere a consumer can reach, or arrive through the config itself.

TestResolveToolReferenceMcpToolsetNonStringArgs currently encodes the pre-fix contract: its all_strings_is_valid case asserts that an McpToolset config with command: "echo" resolves without error. Any change that refuses unapproved commands by default makes that case fail, so it has to be revisited deliberately rather than treated as a merge artifact.

Acceptance criteria

• A config naming a command the operator has not approved is refused, and the refusal is asserted through ResolveToolReference rather than only against an internal helper. Deleting the guard must turn that test red.
• Whatever mechanism grants approval is reachable from outside the module, or the design note says explicitly that the config loader is not supported outside the repository.
• The allowed set can express a command whose arguments are fully pinned, not only a prefix — otherwise approving a launcher such as npx or docker re-opens the hole through its own arguments.
• go test ./internal/configurable/ is green on the merge result with main.