diff --git a/cmd/triagent-mcp/serve.go b/cmd/triagent-mcp/serve.go index 5650b5e..1b81ecb 100644 --- a/cmd/triagent-mcp/serve.go +++ b/cmd/triagent-mcp/serve.go @@ -2,13 +2,16 @@ package main import ( "context" + "encoding/json" "fmt" "os" "os/signal" "strings" "syscall" + "github.com/charmbracelet/log" "github.com/sourcehawk/triagent/pkg/mcp/agentoperator" + "github.com/sourcehawk/triagent/pkg/mcp/cloud" "github.com/sourcehawk/triagent/pkg/mcp/git" "github.com/sourcehawk/triagent/pkg/mcp/incidentio" "github.com/sourcehawk/triagent/pkg/mcp/k8s" @@ -21,28 +24,27 @@ import ( "github.com/sourcehawk/triagent/pkg/mcp/strategies" "github.com/sourcehawk/triagent/pkg/mcp/teleport" "github.com/sourcehawk/triagent/pkg/mcp/wiki" - "github.com/charmbracelet/log" "github.com/spf13/cobra" ) // Environment variable names. Flags override env when both are set. const ( - envKubeconfig = "TRIAGENT_MCP_KUBECONFIG" - envCRDsFile = "TRIAGENT_MCP_CRDS_FILE" - envCrossplaneGroups = "TRIAGENT_MCP_CROSSPLANE_GROUPS" - envSessionDir = "TRIAGENT_MCP_SESSION_DIR" - envUserPlaybooksDir = "TRIAGENT_MCP_USER_PLAYBOOKS_DIR" - envPluginPlaybooksDir = "TRIAGENT_MCP_PLUGIN_PLAYBOOKS_DIR" - envSystemPlaybooksDir = "TRIAGENT_MCP_SYSTEM_PLAYBOOKS_DIR" - envStrategiesSubagentModel = "TRIAGENT_MCP_STRATEGIES_SUBAGENT_MODEL" - envMCPConfigPath = "TRIAGENT_MCP_CONFIG_PATH" - envGitRepo = "TRIAGENT_MCP_GIT_REPO" - envGitCacheDir = "TRIAGENT_MCP_GIT_CACHE_DIR" - envGitClaudeBinary = "TRIAGENT_MCP_GIT_CLAUDE_BINARY" - envGitFilterPrereleases = "TRIAGENT_MCP_GIT_FILTER_PRERELEASES" - envWikiServePath = "TRIAGENT_MCP_WIKI_PATH" - envWikiServeProposalsPath = "TRIAGENT_MCP_WIKI_PROPOSALS_PATH" - envWikiServeClaudeBinary = "TRIAGENT_MCP_WIKI_CLAUDE_BINARY" + envKubeconfig = "TRIAGENT_MCP_KUBECONFIG" + envCRDsFile = "TRIAGENT_MCP_CRDS_FILE" + envCrossplaneGroups = "TRIAGENT_MCP_CROSSPLANE_GROUPS" + envSessionDir = "TRIAGENT_MCP_SESSION_DIR" + envUserPlaybooksDir = "TRIAGENT_MCP_USER_PLAYBOOKS_DIR" + envPluginPlaybooksDir = "TRIAGENT_MCP_PLUGIN_PLAYBOOKS_DIR" + envSystemPlaybooksDir = "TRIAGENT_MCP_SYSTEM_PLAYBOOKS_DIR" + envStrategiesSubagentModel = "TRIAGENT_MCP_STRATEGIES_SUBAGENT_MODEL" + envMCPConfigPath = "TRIAGENT_MCP_CONFIG_PATH" + envGitRepo = "TRIAGENT_MCP_GIT_REPO" + envGitCacheDir = "TRIAGENT_MCP_GIT_CACHE_DIR" + envGitClaudeBinary = "TRIAGENT_MCP_GIT_CLAUDE_BINARY" + envGitFilterPrereleases = "TRIAGENT_MCP_GIT_FILTER_PRERELEASES" + envWikiServePath = "TRIAGENT_MCP_WIKI_PATH" + envWikiServeProposalsPath = "TRIAGENT_MCP_WIKI_PROPOSALS_PATH" + envWikiServeClaudeBinary = "TRIAGENT_MCP_WIKI_CLAUDE_BINARY" envSessionsProposalsPath = "TRIAGENT_MCP_SESSIONS_PROPOSALS_PATH" envSessionsClaudeBinary = "TRIAGENT_MCP_SESSIONS_CLAUDE_BINARY" @@ -71,10 +73,10 @@ type serveFlags struct { systemPlaybooksDir string // git flags - gitRepo string - gitCacheDir string - gitClaudeBinary string - gitFilterPrereleases bool + gitRepo string + gitCacheDir string + gitClaudeBinary string + gitFilterPrereleases bool // wiki flags wikiPath string @@ -95,6 +97,9 @@ type serveFlags struct { promURL string promBearer string promBasic string + + // cloud flags + cloudProvider string } func serveCmd() *cobra.Command { @@ -104,14 +109,14 @@ func serveCmd() *cobra.Command { Short: "Run one of the triagent-mcp MCP servers over stdio", Long: "Run one of the triagent-mcp MCP servers over stdio.\n\n" + "Select the server via --kind. Supported kinds:\n" + - " k8s, teleport, strategies, git, wiki, slack, incidentio, sessions, meta, agent-operator, signal-ingest, parallel, prom", + " k8s, teleport, strategies, git, wiki, slack, incidentio, sessions, meta, agent-operator, signal-ingest, parallel, prom, cloud", Hidden: true, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { return runServe(cmd.Context(), resolveFlags(f)) }, } - cmd.Flags().StringVar(&f.kind, "kind", "", "which MCP server to run: k8s, teleport, strategies, git, wiki, slack, incidentio, sessions, meta, agent-operator, signal-ingest, parallel, or prom (required)") + cmd.Flags().StringVar(&f.kind, "kind", "", "which MCP server to run: k8s, teleport, strategies, git, wiki, slack, incidentio, sessions, meta, agent-operator, signal-ingest, parallel, prom, or cloud (required)") // k8s flags cmd.Flags().StringVar(&f.kubeconfig, "kubeconfig", "", "path to kubeconfig (defaults to $"+envKubeconfig+", then $KUBECONFIG, then ~/.kube/config) [kind=k8s]") @@ -150,6 +155,9 @@ func serveCmd() *cobra.Command { cmd.Flags().StringVar(&f.promBearer, "prom-bearer", "", "Authorization: Bearer token for Prometheus (defaults to $"+envPromBearer+") [kind=prom]") cmd.Flags().StringVar(&f.promBasic, "prom-basic", "", "Basic auth credentials user:pass for Prometheus (defaults to $"+envPromBasic+") [kind=prom]") + // cloud flags + cmd.Flags().StringVar(&f.cloudProvider, "provider", "", "cloud provider to serve: gcp or aws; required (defaults to $"+cloud.EnvProvider+") [kind=cloud]") + return cmd } @@ -215,6 +223,9 @@ func resolveFlags(f *serveFlags) serveFlags { if out.promBasic == "" { out.promBasic = os.Getenv(envPromBasic) } + if out.cloudProvider == "" { + out.cloudProvider = os.Getenv(cloud.EnvProvider) + } // Bool env override: only consider when the operator hasn't passed // the flag explicitly. Cobra preserves the flag default (true) when // unset, so we can't distinguish "operator passed --filter-prereleases=true" @@ -263,10 +274,12 @@ func runServe(ctx context.Context, f serveFlags) error { return runParallel(ctx, f) case "prom": return runProm(ctx, f) + case "cloud": + return runCloud(ctx, f) case "": - return fmt.Errorf("--kind is required (one of: k8s, teleport, strategies, git, wiki, slack, incidentio, sessions, meta, agent-operator, signal-ingest, parallel, prom)") + return fmt.Errorf("--kind is required (one of: k8s, teleport, strategies, git, wiki, slack, incidentio, sessions, meta, agent-operator, signal-ingest, parallel, prom, cloud)") default: - return fmt.Errorf("unknown --kind %q (want one of: k8s, teleport, strategies, git, wiki, slack, incidentio, sessions, meta, agent-operator, signal-ingest, parallel, prom)", f.kind) + return fmt.Errorf("unknown --kind %q (want one of: k8s, teleport, strategies, git, wiki, slack, incidentio, sessions, meta, agent-operator, signal-ingest, parallel, prom, cloud)", f.kind) } } @@ -423,6 +436,59 @@ func runProm(ctx context.Context, f serveFlags) error { return srv.Run(ctx) } +// runCloud wires the read-only cloud-context MCP. --provider selects the +// concrete backend; New plugs it in behind cloud.Provider. The launcher passes +// the allowlist override path and target scope through the subprocess env +// (cloud.EnvAllowlistPath, cloud.EnvScope), never argv. +func runCloud(ctx context.Context, f serveFlags) error { + if f.cloudProvider == "" { + return fmt.Errorf("--provider is required (gcp or aws) (set --provider or $%s)", cloud.EnvProvider) + } + provider, err := newCloudProvider(f.cloudProvider) + if err != nil { + return err + } + srv, err := cloud.New(cloud.Options{ + Provider: provider, + AllowlistPath: os.Getenv(cloud.EnvAllowlistPath), + Scope: parseCloudScope(os.Getenv(cloud.EnvScope)), + }) + if err != nil { + return fmt.Errorf("build cloud mcp server: %w", err) + } + log.Info("mcp serve --kind=cloud starting", "provider", f.cloudProvider) + return srv.Run(ctx) +} + +// newCloudProvider constructs the cloud.Provider for the named provider. The +// gcp and aws implementations land in pkg/mcp/cloud/providers/ in their +// own PRs; until then a known provider reports that it is not yet built and an +// unknown one is named in the error. +func newCloudProvider(name string) (cloud.Provider, error) { + switch name { + case "gcp", "aws": + return nil, fmt.Errorf("cloud provider %q is not built yet", name) + default: + return nil, fmt.Errorf("unknown cloud --provider %q (want gcp or aws)", name) + } +} + +// parseCloudScope decodes the JSON-encoded target scope the launcher froze into +// a cloud.ScopeAllowlist. An empty value yields an empty scope, which leaves the +// target axes unconstrained; a malformed value is logged and treated the same, +// so a bad profile entry never silently widens scope. +func parseCloudScope(raw string) cloud.ScopeAllowlist { + var scope cloud.ScopeAllowlist + if raw == "" { + return scope + } + if err := json.Unmarshal([]byte(raw), &scope); err != nil { + log.Warn("mcp serve --kind=cloud: ignoring malformed scope", "error", err) + return cloud.ScopeAllowlist{} + } + return scope +} + func runGit(ctx context.Context, f serveFlags) error { if f.gitRepo == "" { return fmt.Errorf("--repo is required (owner/name) (set --repo or $%s)", envGitRepo) diff --git a/cmd/triagent-mcp/serve_cloud_test.go b/cmd/triagent-mcp/serve_cloud_test.go new file mode 100644 index 0000000..32504a0 --- /dev/null +++ b/cmd/triagent-mcp/serve_cloud_test.go @@ -0,0 +1,36 @@ +package main + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRunServe_CloudKindRequiresProvider(t *testing.T) { + t.Parallel() + err := runServe(context.Background(), serveFlags{kind: "cloud"}) + require.Error(t, err, "expected error when --provider is missing") + assert.Contains(t, err.Error(), "provider", "error should mention --provider") +} + +func TestRunServe_CloudKindRejectsUnknownProvider(t *testing.T) { + t.Parallel() + err := runServe(context.Background(), serveFlags{kind: "cloud", cloudProvider: "azure"}) + require.Error(t, err, "expected error for an unknown provider") + assert.Contains(t, err.Error(), "azure", "error should name the rejected provider") +} + +func TestRunServe_UnknownKindErrorListsCloud(t *testing.T) { + t.Parallel() + err := runServe(context.Background(), serveFlags{kind: "bogus"}) + require.Error(t, err, "expected error for unknown kind") + assert.Contains(t, err.Error(), "cloud", "kind list should include cloud") +} + +func TestServeCmd_KnowsCloudKind(t *testing.T) { + t.Parallel() + cmd := serveCmd() + assert.Contains(t, cmd.Long, "cloud", "serve --help should list cloud") +} diff --git a/internal/server/meta.go b/internal/server/meta.go index 01f1c9f..5b9550a 100644 --- a/internal/server/meta.go +++ b/internal/server/meta.go @@ -5,6 +5,7 @@ import ( "fmt" "sync" + "github.com/sourcehawk/triagent/pkg/mcp/cloud" "github.com/sourcehawk/triagent/pkg/mcp/git" "github.com/sourcehawk/triagent/pkg/mcp/incidentio" "github.com/sourcehawk/triagent/pkg/mcp/k8s" @@ -107,6 +108,7 @@ func toolCatalog() []MetaTool { specs = append(specs, parallel.ToolSpecs()...) specs = append(specs, prom.ToolSpecs()...) specs = append(specs, teleport.ToolSpecs()...) + specs = append(specs, cloud.ToolSpecs()...) out := make([]MetaTool, 0, len(specs)) for _, s := range specs { ins := make([]MetaToolInput, 0, len(s.Inputs)) diff --git a/pkg/mcp/cloud/allowlist.go b/pkg/mcp/cloud/allowlist.go new file mode 100644 index 0000000..409a90b --- /dev/null +++ b/pkg/mcp/cloud/allowlist.go @@ -0,0 +1,184 @@ +package cloud + +import ( + _ "embed" + "encoding/json" + "fmt" + "os" + "strings" +) + +// defaultCommandsJSON is the parent package's embedded default allowlist. It is +// intentionally empty: provider command sets ship in each provider's own +// default_commands.json (pkg/mcp/cloud/providers/). This anchor lets the +// shared loader compile and gives LoadCommandAllowlist("", …) a valid document. +// +//go:embed default_commands.json +var defaultCommandsJSON []byte + +// Command is one entry in the command allowlist. Path is the normalized +// subcommand path the allowlist matches against (for example "projects list" or +// "compute firewall-rules list"). Description carries the investigative axis the +// command serves (prose only). Redact marks output that needs secret-scrubbing. +type Command struct { + Path string `json:"path"` + Description string `json:"description,omitempty"` + Redact bool `json:"redact,omitempty"` +} + +// CommandAllowlist is the decoded allowlist document: the positive set of +// subcommand paths run_cli permits. +type CommandAllowlist struct { + Commands []Command `json:"commands"` +} + +// DenyFloor is the always-on set of subcommands, flags, and argument-value +// prefixes that the config can never re-enable. The base floor lives in this +// package; a Provider contributes provider-specific additions through +// DenyFloorAdditions, mirroring how k8s.LoadAllowlist always drops Secret. +type DenyFloor struct { + Subcommands []string `json:"subcommands,omitempty"` + Flags []string `json:"flags,omitempty"` + ArgPrefixes []string `json:"arg_prefixes,omitempty"` +} + +// denyFloor is the base floor. Config can never re-enable these; they are +// filtered out of any loaded allowlist and rejected in argv validation. The +// floor covers credential-reading and identity/endpoint-redirecting subcommands +// and flags, plus argument prefixes that read local files or reach the network +// (local-file read and SSRF vectors). +var denyFloor = DenyFloor{ + Subcommands: []string{"secrets", "ssh", "scp", "cp", "sync", "auth", "config"}, + Flags: []string{ + "--impersonate-service-account", "--account", "--profile", + "--endpoint-url", "--cli-input-json", "--cli-input-yaml", "--configuration", + }, + ArgPrefixes: []string{"file://", "fileb://", "@", "http://", "https://"}, +} + +// mergeDenyFloor combines the base floor with provider additions into one floor. +func mergeDenyFloor(extra DenyFloor) DenyFloor { + return DenyFloor{ + Subcommands: append(append([]string{}, denyFloor.Subcommands...), extra.Subcommands...), + Flags: append(append([]string{}, denyFloor.Flags...), extra.Flags...), + ArgPrefixes: append(append([]string{}, denyFloor.ArgPrefixes...), extra.ArgPrefixes...), + } +} + +// LoadCommandAllowlist returns the command allowlist from path, or the embedded +// default when path is empty, then filters out every command whose subcommand +// path falls under the base deny floor plus the provider's extra additions. A +// too-broad override can never re-enable a floored command — the filter is +// applied identically regardless of input, the LoadAllowlist pattern. +func LoadCommandAllowlist(path string, extra DenyFloor) (*CommandAllowlist, error) { + data := defaultCommandsJSON + if path != "" { + b, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("read command allowlist %q: %w", path, err) + } + data = b + } + + var list CommandAllowlist + if err := json.Unmarshal(data, &list); err != nil { + return nil, fmt.Errorf("parse command allowlist: %w", err) + } + + for _, c := range list.Commands { + if c.Path == "" { + return nil, fmt.Errorf("command allowlist entry missing path: %+v", c) + } + } + return filterAllowlist(&list, extra), nil +} + +// filterAllowlist returns a copy of list with every command whose subcommand +// path falls under the base deny floor plus extra dropped. Applied identically +// to a loaded file and to a provider's in-memory default, so neither source can +// advertise a floored command. +func filterAllowlist(list *CommandAllowlist, extra DenyFloor) *CommandAllowlist { + floor := mergeDenyFloor(extra) + out := &CommandAllowlist{Commands: make([]Command, 0, len(list.Commands))} + for _, c := range list.Commands { + if c.Path == "" || floor.deniesSubcommand(normalizePath(c.Path)) { + continue + } + out.Commands = append(out.Commands, c) + } + return out +} + +// Allows reports whether argv's positional subcommand path exactly equals an +// allowlisted command path. Flag tokens and their values do not participate; +// only the leading positionals do. The match is exact rather than prefix so a +// surplus positional — a shell metacharacter token, an extra argument — never +// rides through on the back of an allowed prefix. +func (a *CommandAllowlist) Allows(argv []string) bool { + path := subcommandPath(argv) + for _, c := range a.Commands { + if pathEqual(path, normalizePath(c.Path)) { + return true + } + } + return false +} + +// deniesSubcommand reports whether a normalized subcommand path falls under any +// floored subcommand. A floor entry matches when it is a token-wise prefix of +// the path, so "secrets" floors "secrets versions access" and "compute ssh" +// floors "compute ssh foo". +func (d DenyFloor) deniesSubcommand(path []string) bool { + for _, s := range d.Subcommands { + if pathHasPrefix(path, normalizePath(s)) { + return true + } + } + return false +} + +// subcommandPath returns the leading positional tokens of argv, stopping at the +// first flag (a token beginning with "-"). These tokens form the subcommand +// path the allowlist and deny floor match against. +func subcommandPath(argv []string) []string { + out := make([]string, 0, len(argv)) + for _, tok := range argv { + if strings.HasPrefix(tok, "-") { + break + } + out = append(out, tok) + } + return out +} + +// normalizePath splits a space-separated command path ("compute firewall-rules +// list") into its tokens. +func normalizePath(path string) []string { + return strings.Fields(path) +} + +// pathHasPrefix reports whether prefix is a token-wise prefix of path. +func pathHasPrefix(path, prefix []string) bool { + if len(prefix) == 0 || len(prefix) > len(path) { + return false + } + for i := range prefix { + if path[i] != prefix[i] { + return false + } + } + return true +} + +// pathEqual reports whether two token paths are identical. +func pathEqual(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/pkg/mcp/cloud/allowlist_test.go b/pkg/mcp/cloud/allowlist_test.go new file mode 100644 index 0000000..1dbd512 --- /dev/null +++ b/pkg/mcp/cloud/allowlist_test.go @@ -0,0 +1,58 @@ +package cloud + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func writeTemp(t *testing.T, body string) string { + t.Helper() + p := filepath.Join(t.TempDir(), "allowlist.json") + require.NoError(t, os.WriteFile(p, []byte(body), 0o600)) + return p +} + +func TestLoadCommandAllowlistDropsDenyFloor(t *testing.T) { + t.Parallel() + // JSON that tries to allow a deny-floored subcommand. + path := writeTemp(t, `{"commands":[{"path":"projects list"},{"path":"secrets versions access"}]}`) + al, err := LoadCommandAllowlist(path, DenyFloor{}) + require.NoError(t, err) + assert.False(t, al.Allows([]string{"secrets", "versions", "access"}), + "deny floor must drop secrets access regardless of config") + assert.True(t, al.Allows([]string{"projects", "list"}), "projects list should be allowed") +} + +func TestLoadCommandAllowlistUsesEmbeddedDefaultWhenPathEmpty(t *testing.T) { + t.Parallel() + // The parent package ships no provider commands of its own; an empty path + // yields the empty embedded default, not an error. + al, err := LoadCommandAllowlist("", DenyFloor{}) + require.NoError(t, err) + assert.NotNil(t, al, "expected a non-nil allowlist for the empty default") +} + +func TestLoadCommandAllowlistMergesProviderDenyFloorAdditions(t *testing.T) { + t.Parallel() + path := writeTemp(t, `{"commands":[{"path":"compute instances list"},{"path":"compute ssh foo"}]}`) + extra := DenyFloor{Subcommands: []string{"compute ssh"}} + al, err := LoadCommandAllowlist(path, extra) + require.NoError(t, err) + assert.False(t, al.Allows([]string{"compute", "ssh", "foo"}), + "provider deny-floor addition must drop compute ssh") + assert.True(t, al.Allows([]string{"compute", "instances", "list"}), + "compute instances list should remain allowed") +} + +func TestAllowsMatchesLongestPathPrefix(t *testing.T) { + t.Parallel() + al := &CommandAllowlist{Commands: []Command{{Path: "compute firewall-rules list"}}} + assert.True(t, al.Allows([]string{"compute", "firewall-rules", "list", "--project", "prod"}), + "argv whose leading tokens match an allowed path should pass") + assert.False(t, al.Allows([]string{"compute", "firewall-rules", "delete"}), + "a different verb under the same group must not be allowed") +} diff --git a/pkg/mcp/cloud/default_commands.json b/pkg/mcp/cloud/default_commands.json new file mode 100644 index 0000000..f2ae3f6 --- /dev/null +++ b/pkg/mcp/cloud/default_commands.json @@ -0,0 +1,3 @@ +{ + "commands": [] +} diff --git a/pkg/mcp/cloud/env.go b/pkg/mcp/cloud/env.go new file mode 100644 index 0000000..fcb57ae --- /dev/null +++ b/pkg/mcp/cloud/env.go @@ -0,0 +1,17 @@ +package cloud + +// Environment variable names the cloud-context MCP subprocess reads. The +// launcher sets these in the subprocess env (never argv); the agent supplies +// argv only and cannot reach them. Provider impersonation env vars +// (CLOUDSDK_AUTH_IMPERSONATE_SERVICE_ACCOUNT, AWS_PROFILE) are contributed by +// the provider packages, not here. +const ( + // EnvProvider selects the concrete provider ("gcp" | "aws"). + EnvProvider = "TRIAGENT_CLOUD_PROVIDER" + // EnvAllowlistPath points at a command-allowlist override file; empty uses + // the provider's embedded default. + EnvAllowlistPath = "TRIAGENT_CLOUD_ALLOWLIST_PATH" + // EnvScope carries the target scope allowlist the launcher froze for this + // session, as JSON the cloud package decodes into ScopeAllowlist. + EnvScope = "TRIAGENT_CLOUD_SCOPE" +) diff --git a/pkg/mcp/cloud/fake_test.go b/pkg/mcp/cloud/fake_test.go new file mode 100644 index 0000000..62593e8 --- /dev/null +++ b/pkg/mcp/cloud/fake_test.go @@ -0,0 +1,51 @@ +package cloud + +import "context" + +// fakeProvider is the in-package test double for the Provider interface. +// Providers (gcp, aws) implement the same contract in their own subpackages; +// this fake exercises the parent package's harness, tools, and probe without +// shelling any real cloud CLI. +type fakeProvider struct { + name string + binary string + allowlist *CommandAllowlist + denyFloor DenyFloor + inventory Inventory + identity IdentityStatus + identityErr error + envPassthrough []string +} + +func (f *fakeProvider) Name() string { + if f.name == "" { + return "fake" + } + return f.name +} + +func (f *fakeProvider) Binary() string { + if f.binary == "" { + return "/bin/true" + } + return f.binary +} + +func (f *fakeProvider) DefaultAllowlist() *CommandAllowlist { + if f.allowlist == nil { + return &CommandAllowlist{} + } + return f.allowlist +} + +func (f *fakeProvider) DenyFloorAdditions() DenyFloor { return f.denyFloor } + +func (f *fakeProvider) EnvPassthrough() []string { return f.envPassthrough } + +func (f *fakeProvider) Inventory(context.Context, RunFunc) (Inventory, error) { + return f.inventory, nil +} + +func (f *fakeProvider) Identity(context.Context, RunFunc) (IdentityStatus, error) { + return f.identity, f.identityErr +} diff --git a/pkg/mcp/cloud/harness.go b/pkg/mcp/cloud/harness.go new file mode 100644 index 0000000..c185be7 --- /dev/null +++ b/pkg/mcp/cloud/harness.go @@ -0,0 +1,42 @@ +package cloud + +import ( + "context" + "errors" + "os/exec" +) + +// defaultOutputLimit caps run_cli stdout so a raw provider response cannot blow +// the agent's context budget. Output beyond it is dropped and flagged. +const defaultOutputLimit = 64 * 1024 + +// execCLI runs binPath with argv via execve — no shell, ever. The argv tokens +// reach the binary as literal arguments, so shell metacharacters are inert. The +// subprocess runs with exactly the supplied env (never the parent environment, +// so a poisoned PATH cannot redirect the binary and ambient secrets do not +// leak), closed stdin (no interactive prompt), and stdout capped at limit. A +// non-zero exit is a normal result carried in ExitCode, not a Go error; a Go +// error means the process could not be run at all. +func execCLI(ctx context.Context, binPath string, argv []string, env []string, limit int) (CLIResult, error) { + cmd := exec.CommandContext(ctx, binPath, argv...) + cmd.Env = env + cmd.Stdin = nil + + out, err := cmd.Output() + res := CLIResult{} + if len(out) > limit { + out = out[:limit] + res.Truncated = true + } + res.Stdout = string(out) + + if err != nil { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + res.ExitCode = exitErr.ExitCode() + return res, nil + } + return CLIResult{}, err + } + return res, nil +} diff --git a/pkg/mcp/cloud/harness_security_test.go b/pkg/mcp/cloud/harness_security_test.go new file mode 100644 index 0000000..13a6a98 --- /dev/null +++ b/pkg/mcp/cloud/harness_security_test.go @@ -0,0 +1,62 @@ +package cloud + +import ( + "bytes" + "context" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestExecCLINeverUsesShell is the source-level half of the no-shell guarantee: +// the exec core must never construct a shell command. There is no "-c" string, +// no "sh -c", no "bash -c" anywhere in harness.go. +func TestExecCLINeverUsesShell(t *testing.T) { + t.Parallel() + src, err := os.ReadFile("harness.go") + require.NoError(t, err) + for _, banned := range []string{`"-c"`, "sh -c", "bash -c", `"sh"`, `"bash"`} { + assert.False(t, bytes.Contains(src, []byte(banned)), + "harness.go must never construct a shell command; found %q", banned) + } +} + +// TestExecCLIMetacharactersAreInert is the behavioural half: shell +// metacharacters handed to a binary as argv tokens are literal arguments, never +// interpreted. Running /bin/echo with metacharacter tokens prints them verbatim +// and spawns no second process. +func TestExecCLIMetacharactersAreInert(t *testing.T) { + t.Parallel() + argv := []string{";", "echo", "pwned", "|", "$(whoami)", "&&", "`id`"} + r, err := execCLI(context.Background(), "/bin/echo", argv, nil, 4096) + require.NoError(t, err) + got := strings.TrimRight(r.Stdout, "\n") + want := strings.Join(argv, " ") + require.Equal(t, want, got, "metacharacters were not inert") + assert.False(t, strings.Contains(r.Stdout, "pwned\n") && got != want, + "a second process appears to have run") +} + +// TestExecCLITruncates caps output at the byte limit and flags truncation. +func TestExecCLITruncates(t *testing.T) { + t.Parallel() + r, err := execCLI(context.Background(), "/bin/echo", []string{strings.Repeat("x", 100)}, nil, 10) + require.NoError(t, err) + assert.True(t, r.Truncated, "expected Truncated, got %+v", r) + assert.LessOrEqual(t, len(r.Stdout), 10, "output exceeded limit") +} + +// TestExecCLIMinimalEnv confirms the subprocess runs with the caller's explicit +// env, not the parent process environment, so a poisoned PATH cannot redirect +// the resolved binary and ambient secrets do not leak in. +func TestExecCLIMinimalEnv(t *testing.T) { + t.Setenv("TRIAGENT_CLOUD_HARNESS_LEAK_CANARY", "should-not-appear") + r, err := execCLI(context.Background(), "/usr/bin/env", nil, []string{"FOO=bar"}, 4096) + require.NoError(t, err) + assert.NotContains(t, r.Stdout, "TRIAGENT_CLOUD_HARNESS_LEAK_CANARY", + "subprocess inherited the parent environment; env must be explicit") + assert.Contains(t, r.Stdout, "FOO=bar", "explicit env not applied") +} diff --git a/pkg/mcp/cloud/harness_test.go b/pkg/mcp/cloud/harness_test.go new file mode 100644 index 0000000..9730e34 --- /dev/null +++ b/pkg/mcp/cloud/harness_test.go @@ -0,0 +1,19 @@ +package cloud + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestExecCLIExitCode surfaces the child's exit code without treating a +// non-zero exit as a Go error: a CLI that exits 1 on "not found" is a normal +// result the agent should see, not a harness failure. +func TestExecCLIExitCode(t *testing.T) { + t.Parallel() + r, err := execCLI(context.Background(), "/bin/false", nil, nil, 4096) + require.NoError(t, err, "non-zero exit should not be a Go error") + assert.Equal(t, 1, r.ExitCode) +} diff --git a/pkg/mcp/cloud/probe.go b/pkg/mcp/cloud/probe.go new file mode 100644 index 0000000..54e722d --- /dev/null +++ b/pkg/mcp/cloud/probe.go @@ -0,0 +1,36 @@ +package cloud + +import "context" + +// Probe runs the read-only whoami for one provider: which pinned identity is +// active and whether it is valid. It is the single probe the launcher's +// connections panel, the session preflight gate, and the session_status tool +// all call, so those surfaces can never disagree. +// +// Probe never returns a Go error for an unreachable or invalid identity — that +// is a degrade, reported through IdentityStatus.Valid and Hint, so a stale cloud +// credential surfaces visibly instead of failing the caller. A Go error is +// reserved for a caller contract violation (a nil provider). +func Probe(ctx context.Context, p Provider) (IdentityStatus, error) { + run := func(ctx context.Context, argv []string) (CLIResult, error) { + return execCLI(ctx, p.Binary(), argv, nil, defaultOutputLimit) + } + + st, err := p.Identity(ctx, run) + if err != nil { + return IdentityStatus{ + Provider: p.Name(), + Valid: false, + Hint: err.Error(), + }, nil + } + if st.Provider == "" { + st.Provider = p.Name() + } + if st.AssumedIdentity == "" { + // A whoami that resolved no identity is not a valid session, whatever + // the provider reported. + st.Valid = false + } + return st, nil +} diff --git a/pkg/mcp/cloud/probe_test.go b/pkg/mcp/cloud/probe_test.go new file mode 100644 index 0000000..e94aa39 --- /dev/null +++ b/pkg/mcp/cloud/probe_test.go @@ -0,0 +1,44 @@ +package cloud + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestProbeReturnsProviderIdentity(t *testing.T) { + t.Parallel() + p := &fakeProvider{ + name: "gcp", + identity: IdentityStatus{ + Provider: "gcp", + AssumedIdentity: "ro-sa@proj.iam.gserviceaccount.com", + Valid: true, + }, + } + st, err := Probe(context.Background(), p) + require.NoError(t, err) + assert.True(t, st.Valid) + assert.Equal(t, "ro-sa@proj.iam.gserviceaccount.com", st.AssumedIdentity) +} + +func TestProbeSurfacesProviderErrorAsInvalid(t *testing.T) { + t.Parallel() + p := &fakeProvider{name: "aws", identityErr: errors.New("token expired")} + st, err := Probe(context.Background(), p) + require.NoError(t, err, "Probe should degrade, not error") + assert.False(t, st.Valid, "expected Valid=false when the provider errors") + assert.Equal(t, "aws", st.Provider, "expected provider name carried through") + assert.NotEmpty(t, st.Hint, "expected the provider error surfaced as a hint") +} + +func TestProbeInvalidWhenIdentityEmpty(t *testing.T) { + t.Parallel() + p := &fakeProvider{name: "gcp", identity: IdentityStatus{Provider: "gcp", Valid: true}} + st, err := Probe(context.Background(), p) + require.NoError(t, err) + assert.False(t, st.Valid, "an empty resolved identity must not be reported valid") +} diff --git a/pkg/mcp/cloud/provider.go b/pkg/mcp/cloud/provider.go new file mode 100644 index 0000000..8abf18b --- /dev/null +++ b/pkg/mcp/cloud/provider.go @@ -0,0 +1,75 @@ +// Package cloud implements the read-only cloud-context MCP server the +// triagent-mcp binary exposes to Claude. One package serves both GCP and AWS: +// the cloud-specific behaviour sits behind the Provider interface, selected at +// launch by --provider and plugged in from pkg/mcp/cloud/providers/. +// +// The server is read-only by construction. run_cli never touches a shell, every +// invocation is validated against a positive command allowlist plus a hardcoded +// deny floor the config can never re-enable, and the cloud identity is pinned by +// the deployment through harness-controlled env the agent cannot reach. +package cloud + +import "context" + +// Provider is the cloud-specific seam every tool calls through. Selecting +// --provider chooses the concrete gcp or aws implementation, injected behind +// this interface (the teleport DI pattern). Implementations live in +// pkg/mcp/cloud/providers/, never in this package. +type Provider interface { + // Name reports the provider identifier ("gcp" | "aws"). + Name() string + // Binary is the resolved absolute path to the provider CLI (gcloud/aws). + Binary() string + // DefaultAllowlist is the provider's embedded default command allowlist. + DefaultAllowlist() *CommandAllowlist + // DenyFloorAdditions contributes provider-specific subcommands and flags to + // the always-on deny floor. The base floor lives in this package; providers + // only add to it, never relax it. + DenyFloorAdditions() DenyFloor + // EnvPassthrough lists the environment variable NAMES this provider's CLI + // needs forwarded from the launcher-controlled process env to the subprocess + // (base credentials, the pinned-identity impersonation target, config dirs). + // The harness forwards only these plus a minimal base set; every other parent + // env var is dropped, so ambient launcher secrets never reach the CLI. + EnvPassthrough() []string + // Inventory projects the provider's accessible scopes (projects for gcp, + // accounts for aws). It execs only through run, never directly. + Inventory(ctx context.Context, run RunFunc) (Inventory, error) + // Identity is the read-only whoami: which pinned identity is active and + // whether it is valid. It execs only through run, never directly. + Identity(ctx context.Context, run RunFunc) (IdentityStatus, error) +} + +// RunFunc is the harness exec core, injected into providers so they never exec +// directly. It carries the no-shell guarantee: argv tokens reach the provider +// binary via execve, never a shell. +type RunFunc func(ctx context.Context, argv []string) (CLIResult, error) + +// Inventory is the projected list of accessible scopes the agent uses to orient. +type Inventory struct { + Scopes []Scope `json:"scopes"` +} + +// Scope is one project (gcp) or account (aws) the pinned identity can read. +type Scope struct { + ID string `json:"id"` + Name string `json:"name"` +} + +// IdentityStatus is the single struct the identity probe returns. The +// connections array, the session_status tool, and the preflight gate all render +// from it, so they cannot disagree. JSON tags are a downstream contract. +type IdentityStatus struct { + Provider string `json:"provider"` + AssumedIdentity string `json:"assumed_identity"` + Valid bool `json:"valid"` + Hint string `json:"hint,omitempty"` +} + +// CLIResult is the shaped result of one run_cli invocation. Raw provider JSON +// is never surfaced; the harness caps output and reports truncation. +type CLIResult struct { + Stdout string `json:"stdout"` + Truncated bool `json:"truncated"` + ExitCode int `json:"exit_code"` +} diff --git a/pkg/mcp/cloud/server.go b/pkg/mcp/cloud/server.go new file mode 100644 index 0000000..21f6549 --- /dev/null +++ b/pkg/mcp/cloud/server.go @@ -0,0 +1,137 @@ +package cloud + +import ( + "context" + "fmt" + "os" + "strings" + + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/sourcehawk/triagent/pkg/mcp/telemetry" +) + +// baseEnvPassthrough is the minimal env every provider CLI needs regardless of +// cloud: PATH so the resolved binary can find its own dependencies, HOME so it +// can locate per-user config. Providers add their credential/impersonation +// names via Provider.EnvPassthrough. +var baseEnvPassthrough = []string{"PATH", "HOME"} + +// Options configures the cloud-context MCP server. +type Options struct { + // Provider is the cloud-specific backend (gcp or aws), injected behind the + // Provider interface. Required; New errors when nil. + Provider Provider + // AllowlistPath optionally overrides the provider's embedded default command + // allowlist. Empty means use the provider default. The launcher points this + // at the profile-configured override via TRIAGENT_CLOUD_ALLOWLIST_PATH. + AllowlistPath string + // Scope is the set of projects/accounts/regions any run_cli argv may target. + // Argv referencing a target outside the scope is rejected before exec. The + // launcher fills it from TRIAGENT_CLOUD_SCOPE. + Scope ScopeAllowlist +} + +// Server holds the configured cloud-context MCP server. +type Server struct { + impl *mcp.Server + provider Provider + allowlist *CommandAllowlist + scope ScopeAllowlist +} + +// New constructs a cloud-context MCP server. Provider is required. The command +// allowlist loads from Options.AllowlistPath (or the provider default when +// empty), always filtered through the base deny floor plus the provider's +// additions, so a too-broad override can never re-enable a floored command. +func New(opts Options) (*Server, error) { + if opts.Provider == nil { + return nil, fmt.Errorf("cloud: Provider is required") + } + allow, err := loadAllowlist(opts.AllowlistPath, opts.Provider) + if err != nil { + return nil, fmt.Errorf("cloud: load command allowlist: %w", err) + } + impl := mcp.NewServer(&mcp.Implementation{ + Name: "triagent-mcp-cloud", + Version: "0.1.0", + }, nil) + s := &Server{ + impl: impl, + provider: opts.Provider, + allowlist: allow, + scope: opts.Scope, + } + s.registerOn(impl) + return s, nil +} + +// loadAllowlist resolves the command allowlist for a provider: the override path +// when given, else the provider's embedded default, always filtered through the +// base deny floor plus the provider's deny-floor additions. +func loadAllowlist(path string, p Provider) (*CommandAllowlist, error) { + if path != "" { + return LoadCommandAllowlist(path, p.DenyFloorAdditions()) + } + // Filter the provider's in-memory default through the floor the same way a + // loaded file would be, so the default can never advertise a floored command. + return filterAllowlist(p.DefaultAllowlist(), p.DenyFloorAdditions()), nil +} + +// Run serves MCP requests over stdio until the client disconnects or ctx is +// cancelled. +func (s *Server) Run(ctx context.Context) error { + return s.impl.Run(ctx, &mcp.StdioTransport{}) +} + +// run is the harness exec core bound to this server's provider binary, scope, +// and allowlist. Providers and tools exec only through this RunFunc, never +// directly: it validates argv before handing it to the no-shell exec core. +func (s *Server) run(ctx context.Context, argv []string) (CLIResult, error) { + if err := validateArgv(argv, s.allowlist, s.scope); err != nil { + return CLIResult{}, err + } + return execCLI(ctx, s.provider.Binary(), argv, s.subprocessEnv(), defaultOutputLimit) +} + +// subprocessEnv builds the explicit, minimal environment for a provider CLI +// invocation: only the base names plus the provider's declared passthrough +// names, read from the launcher-controlled process env. Everything else is +// dropped, so the launcher's ambient secrets never reach the CLI. +func (s *Server) subprocessEnv() []string { + keep := make(map[string]bool, len(baseEnvPassthrough)) + for _, name := range baseEnvPassthrough { + keep[name] = true + } + for _, name := range s.provider.EnvPassthrough() { + keep[name] = true + } + var env []string + for _, kv := range os.Environ() { + name, _, ok := strings.Cut(kv, "=") + if ok && keep[name] { + env = append(env, kv) + } + } + return env +} + +// registerOn wires the cloud tools onto impl. Called from New and from the wire +// test inside the package. Registration order mirrors ToolSpecs(). +func (s *Server) registerOn(impl *mcp.Server) { + mcp.AddTool(impl, &mcp.Tool{ + Name: "list_inventory", + Description: descListInventory, + }, telemetry.Wrap("list_inventory", s.listInventory)) + mcp.AddTool(impl, &mcp.Tool{ + Name: "session_status", + Description: descSessionStatus, + }, telemetry.Wrap("session_status", s.sessionStatus)) + mcp.AddTool(impl, &mcp.Tool{ + Name: "run_cli", + Description: descRunCLI, + }, telemetry.Wrap("run_cli", s.runCLI)) + mcp.AddTool(impl, &mcp.Tool{ + Name: "list_allowed_commands", + Description: descListAllowedCommands, + }, telemetry.Wrap("list_allowed_commands", s.listAllowedCommands)) +} diff --git a/pkg/mcp/cloud/server_test.go b/pkg/mcp/cloud/server_test.go new file mode 100644 index 0000000..002cfb0 --- /dev/null +++ b/pkg/mcp/cloud/server_test.go @@ -0,0 +1,37 @@ +package cloud + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewRequiresProvider(t *testing.T) { + t.Parallel() + _, err := New(Options{}) + require.Error(t, err, "expected error when Provider is nil") + _, err = New(Options{Provider: &fakeProvider{}}) + require.NoError(t, err) +} + +// TestSubprocessEnvDropsParentSecretsKeepsPassthrough exercises the env the +// server actually builds for run_cli — the path the real harness takes, which +// the isolated execCLI test cannot cover. A parent-env canary must be dropped +// while a declared passthrough var survives, so ambient launcher secrets never +// reach the provider CLI. +func TestSubprocessEnvDropsParentSecretsKeepsPassthrough(t *testing.T) { + t.Setenv("TRIAGENT_CLOUD_LEAK_CANARY", "should-not-appear") + t.Setenv("CLOUDSDK_AUTH_IMPERSONATE_SERVICE_ACCOUNT", "ro-sa@proj.iam.gserviceaccount.com") + p := &fakeProvider{ + envPassthrough: []string{"CLOUDSDK_AUTH_IMPERSONATE_SERVICE_ACCOUNT"}, + } + srv := newTestServer(t, p) + + env := srv.subprocessEnv() + + assert.NotContains(t, env, "TRIAGENT_CLOUD_LEAK_CANARY=should-not-appear", + "parent-env secret must be dropped from the subprocess env") + assert.Contains(t, env, "CLOUDSDK_AUTH_IMPERSONATE_SERVICE_ACCOUNT=ro-sa@proj.iam.gserviceaccount.com", + "declared passthrough var must be forwarded") +} diff --git a/pkg/mcp/cloud/specs.go b/pkg/mcp/cloud/specs.go new file mode 100644 index 0000000..75e699b --- /dev/null +++ b/pkg/mcp/cloud/specs.go @@ -0,0 +1,36 @@ +package cloud + +import "github.com/sourcehawk/triagent/pkg/mcp/toolspec" + +// ToolSpecs returns the cloud server's tool catalog with each tool's input shape +// introspected from its Go struct (and its jsonschema tags). +// +// Order mirrors the registration order in server.go's registerOn(). +func ToolSpecs() []toolspec.ToolSpec { + return []toolspec.ToolSpec{ + { + Server: "triagent-cloud", + Name: "list_inventory", + Description: descListInventory, + Inputs: toolspec.FromStruct(ListInventoryInput{}), + }, + { + Server: "triagent-cloud", + Name: "session_status", + Description: descSessionStatus, + Inputs: toolspec.FromStruct(SessionStatusInput{}), + }, + { + Server: "triagent-cloud", + Name: "run_cli", + Description: descRunCLI, + Inputs: toolspec.FromStruct(RunCLIInput{}), + }, + { + Server: "triagent-cloud", + Name: "list_allowed_commands", + Description: descListAllowedCommands, + Inputs: toolspec.FromStruct(ListAllowedCommandsInput{}), + }, + } +} diff --git a/pkg/mcp/cloud/tools_cli.go b/pkg/mcp/cloud/tools_cli.go new file mode 100644 index 0000000..e0149b3 --- /dev/null +++ b/pkg/mcp/cloud/tools_cli.go @@ -0,0 +1,57 @@ +package cloud + +import ( + "context" + "fmt" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +const descRunCLI = "Run one read-only provider CLI command (gcloud/aws). Supply the argument tokens as an array, never a single string — there is no shell. Only allowlisted subcommands run; identity flags, credential-reading subcommands, and out-of-scope targets are rejected. See list_allowed_commands for what is permitted." + +const descListAllowedCommands = "List the provider CLI subcommands run_cli permits, with the investigative axis each serves. This is exactly what run_cli enforces, so what is advertised is what is allowed." + +// RunCLIInput is the input schema for run_cli. Argv is a typed array of argument +// tokens, never a single command string: the harness never tokenizes, so there +// is no in-house splitter to fool and shell metacharacters are inert. +type RunCLIInput struct { + Argv []string `json:"argv" jsonschema:"The provider CLI argument tokens as an array (for example [\"compute\",\"firewall-rules\",\"list\",\"--project\",\"prod\"]). Do not include the binary name or pass a single string."` +} + +// RunCLIOutput is the response schema for run_cli: the shaped CLI result, never +// raw API JSON beyond the captured stdout the provider emitted. +type RunCLIOutput = CLIResult + +// runCLI validates the argv against the allowlist, deny floor, and scope, then +// execs it through the no-shell core. A rejected argv is a tool error returned +// before any exec; a non-zero CLI exit is a normal result the agent sees. +func (s *Server) runCLI(ctx context.Context, _ *mcp.CallToolRequest, in RunCLIInput) (*mcp.CallToolResult, RunCLIOutput, error) { + res, err := s.run(ctx, in.Argv) + if err != nil { + return errorResult(fmt.Sprintf("run_cli rejected: %v", err)), RunCLIOutput{}, nil + } + return nil, res, nil +} + +// ListAllowedCommandsInput is the input schema for list_allowed_commands. It +// takes no parameters. +type ListAllowedCommandsInput struct{} + +// ListAllowedCommandsOutput is the response schema for list_allowed_commands. +type ListAllowedCommandsOutput struct { + Commands []Command `json:"commands"` +} + +// listAllowedCommands returns the same CommandAllowlist run_cli enforces, so the +// catalog and the gate can never disagree. +func (s *Server) listAllowedCommands(_ context.Context, _ *mcp.CallToolRequest, _ ListAllowedCommandsInput) (*mcp.CallToolResult, ListAllowedCommandsOutput, error) { + return nil, ListAllowedCommandsOutput{Commands: s.allowlist.Commands}, nil +} + +// errorResult builds an MCP error result whose Content carries msg. +func errorResult(msg string) *mcp.CallToolResult { + return &mcp.CallToolResult{ + IsError: true, + Content: []mcp.Content{&mcp.TextContent{Text: msg}}, + } +} diff --git a/pkg/mcp/cloud/tools_inventory.go b/pkg/mcp/cloud/tools_inventory.go new file mode 100644 index 0000000..7d3cdfc --- /dev/null +++ b/pkg/mcp/cloud/tools_inventory.go @@ -0,0 +1,28 @@ +package cloud + +import ( + "context" + "fmt" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +const descListInventory = "List the cloud projects (GCP) or accounts (AWS) the pinned read-only identity can see, so you can orient before drilling in. Read-only." + +// ListInventoryInput is the input schema for list_inventory. It takes no +// parameters: the accessible scope is fixed by the pinned identity. +type ListInventoryInput struct{} + +// ListInventoryOutput is the response schema for list_inventory: the provider's +// accessible scopes. +type ListInventoryOutput = Inventory + +// listInventory projects the provider's accessible scopes. The provider execs +// only through the server's validated run core. +func (s *Server) listInventory(ctx context.Context, _ *mcp.CallToolRequest, _ ListInventoryInput) (*mcp.CallToolResult, ListInventoryOutput, error) { + inv, err := s.provider.Inventory(ctx, s.run) + if err != nil { + return errorResult(fmt.Sprintf("list inventory: %v", err)), ListInventoryOutput{}, nil + } + return nil, inv, nil +} diff --git a/pkg/mcp/cloud/tools_status.go b/pkg/mcp/cloud/tools_status.go new file mode 100644 index 0000000..08ead5e --- /dev/null +++ b/pkg/mcp/cloud/tools_status.go @@ -0,0 +1,28 @@ +package cloud + +import ( + "context" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +const descSessionStatus = "Report the pinned read-only cloud identity this session acts as and whether it is currently valid. You cannot choose or change it. Read-only." + +// SessionStatusInput is the input schema for session_status. It takes no +// parameters: the identity is pinned by the deployment. +type SessionStatusInput struct{} + +// SessionStatusOutput is the response schema for session_status. It is the +// shared IdentityStatus the connections panel and preflight gate also render. +type SessionStatusOutput = IdentityStatus + +// sessionStatus runs the shared identity probe and returns its result. It never +// errors on an invalid identity — a stale credential surfaces as Valid:false +// with a Hint, the same visible-degrade contract the launcher renders. +func (s *Server) sessionStatus(ctx context.Context, _ *mcp.CallToolRequest, _ SessionStatusInput) (*mcp.CallToolResult, SessionStatusOutput, error) { + st, err := Probe(ctx, s.provider) + if err != nil { + return errorResult(err.Error()), SessionStatusOutput{}, nil + } + return nil, st, nil +} diff --git a/pkg/mcp/cloud/tools_test.go b/pkg/mcp/cloud/tools_test.go new file mode 100644 index 0000000..9a70571 --- /dev/null +++ b/pkg/mcp/cloud/tools_test.go @@ -0,0 +1,113 @@ +package cloud + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" +) + +func newTestServer(t *testing.T, p Provider, opts ...func(*Options)) *Server { + t.Helper() + o := Options{Provider: p} + for _, f := range opts { + f(&o) + } + srv, err := New(o) + require.NoError(t, err) + return srv +} + +func TestListInventoryReturnsProviderScopes(t *testing.T) { + t.Parallel() + p := &fakeProvider{inventory: Inventory{Scopes: []Scope{{ID: "prod", Name: "Production"}}}} + srv := newTestServer(t, p) + _, out, err := srv.listInventory(context.Background(), nil, ListInventoryInput{}) + require.NoError(t, err) + require.Len(t, out.Scopes, 1) + require.Equal(t, "prod", out.Scopes[0].ID) +} + +func TestSessionStatusReturnsProbeIdentity(t *testing.T) { + t.Parallel() + p := &fakeProvider{ + name: "gcp", + identity: IdentityStatus{Provider: "gcp", AssumedIdentity: "ro-sa@proj", Valid: true}, + } + srv := newTestServer(t, p) + _, out, err := srv.sessionStatus(context.Background(), nil, SessionStatusInput{}) + require.NoError(t, err) + require.True(t, out.Valid) + require.Equal(t, "ro-sa@proj", out.AssumedIdentity) +} + +func TestListAllowedCommandsReturnsLoadedAllowlist(t *testing.T) { + t.Parallel() + p := &fakeProvider{allowlist: &CommandAllowlist{Commands: []Command{ + {Path: "projects list", Description: "orient: list projects"}, + }}} + srv := newTestServer(t, p) + _, out, err := srv.listAllowedCommands(context.Background(), nil, ListAllowedCommandsInput{}) + require.NoError(t, err) + require.Len(t, out.Commands, 1) + require.Equal(t, "projects list", out.Commands[0].Path) +} + +func TestListAllowedCommandsDropsDenyFlooredEntries(t *testing.T) { + t.Parallel() + // Even if a provider default lists a floored command, the catalog the agent + // sees is exactly what run_cli enforces — the floored entry is absent. + p := &fakeProvider{allowlist: &CommandAllowlist{Commands: []Command{ + {Path: "projects list"}, + {Path: "secrets versions access"}, + }}} + srv := newTestServer(t, p) + _, out, err := srv.listAllowedCommands(context.Background(), nil, ListAllowedCommandsInput{}) + require.NoError(t, err) + for _, c := range out.Commands { + require.NotEqual(t, "secrets versions access", c.Path, "deny-floored command must not be advertised") + } +} + +func TestRunCLIRejectsDenyFlooredArgvBeforeExec(t *testing.T) { + t.Parallel() + p := &fakeProvider{ + binary: "/bin/echo", + allowlist: &CommandAllowlist{Commands: []Command{{Path: "compute instances list"}}}, + } + srv := newTestServer(t, p) + res, _, err := srv.runCLI(context.Background(), nil, RunCLIInput{ + Argv: []string{"compute", "instances", "list", "--impersonate-service-account", "evil"}, + }) + require.NoError(t, err) + require.NotNil(t, res) + require.True(t, res.IsError, "deny-floored argv must be rejected as a tool error before exec") +} + +func TestRunCLIShapesResultOnSuccess(t *testing.T) { + t.Parallel() + p := &fakeProvider{ + binary: "/bin/echo", + allowlist: &CommandAllowlist{Commands: []Command{{Path: "projects list"}}}, + } + srv := newTestServer(t, p) + _, out, err := srv.runCLI(context.Background(), nil, RunCLIInput{Argv: []string{"projects", "list"}}) + require.NoError(t, err) + require.Contains(t, out.Stdout, "projects list") +} + +func TestRunCLIRejectsOutOfScopeTarget(t *testing.T) { + t.Parallel() + p := &fakeProvider{ + binary: "/bin/echo", + allowlist: &CommandAllowlist{Commands: []Command{{Path: "projects list"}}}, + } + srv := newTestServer(t, p, func(o *Options) { + o.Scope = ScopeAllowlist{Projects: []string{"prod"}} + }) + res, _, err := srv.runCLI(context.Background(), nil, RunCLIInput{ + Argv: []string{"projects", "list", "--project", "other"}, + }) + require.NoError(t, err) + require.True(t, res.IsError, "out-of-scope target must be rejected") +} diff --git a/pkg/mcp/cloud/tools_wire_test.go b/pkg/mcp/cloud/tools_wire_test.go new file mode 100644 index 0000000..9cf246c --- /dev/null +++ b/pkg/mcp/cloud/tools_wire_test.go @@ -0,0 +1,51 @@ +package cloud + +import ( + "context" + "testing" + + sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestTools_Registered confirms the four cloud tools are exposed and that the +// set registered on the server matches the ToolSpecs() catalog exactly — the +// wire test fails if registration drifts from the catalog. +func TestTools_Registered(t *testing.T) { + t.Parallel() + + srv, err := New(Options{Provider: &fakeProvider{}}) + require.NoError(t, err) + + serverT, clientT := sdkmcp.NewInMemoryTransports() + serverSession, err := srv.impl.Connect(context.Background(), serverT, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = serverSession.Close() }) + + client := sdkmcp.NewClient(&sdkmcp.Implementation{Name: "test-client", Version: "v0"}, nil) + clientSession, err := client.Connect(context.Background(), clientT, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = clientSession.Close() }) + + list, err := clientSession.ListTools(context.Background(), &sdkmcp.ListToolsParams{}) + require.NoError(t, err) + + registered := map[string]bool{} + for _, tool := range list.Tools { + registered[tool.Name] = true + } + + cataloged := map[string]bool{} + for _, spec := range ToolSpecs() { + cataloged[spec.Name] = true + assert.True(t, registered[spec.Name], "tool %q in ToolSpecs() but not registered", spec.Name) + } + for name := range registered { + assert.True(t, cataloged[name], "tool %q registered but absent from ToolSpecs()", name) + } + + for _, want := range []string{"list_inventory", "session_status", "run_cli", "list_allowed_commands"} { + assert.True(t, registered[want], "%s not registered", want) + } +} diff --git a/pkg/mcp/cloud/validate.go b/pkg/mcp/cloud/validate.go new file mode 100644 index 0000000..770b995 --- /dev/null +++ b/pkg/mcp/cloud/validate.go @@ -0,0 +1,125 @@ +package cloud + +import ( + "fmt" + "strings" +) + +// ScopeAllowlist is the deployment's set of cloud targets any run_cli argv may +// reference. An empty field means that target axis is unconstrained. The agent +// cannot pivot to an un-allowlisted project, account, or region. +type ScopeAllowlist struct { + Projects []string `json:"projects,omitempty"` + Accounts []string `json:"accounts,omitempty"` + Regions []string `json:"regions,omitempty"` +} + +// allowedFor maps a target-selecting flag to the ScopeAllowlist field whose +// membership a value of that flag must satisfy. The deny floor rejects identity +// flags (--account, --profile) before scope ever sees them, so scope governs +// only the location axes the agent is allowed to choose among. +func (s ScopeAllowlist) allowedFor(flag string) ([]string, bool) { + switch flag { + case "--project": + return s.Projects, true + case "--region", "--zone": + return s.Regions, true + default: + return nil, false + } +} + +// validateArgv enforces the no-bypass contract on one argv before exec: the +// positional subcommand path is on the allowlist, no token is a deny-floored +// flag or arg-prefix, and every target-selecting flag value is within scope. +// It runs entirely on argv tokens — there is no shell, so metacharacter tokens +// are inert positionals that simply fail the exact allowlist match. +func validateArgv(argv []string, allow *CommandAllowlist, scope ScopeAllowlist) error { + if len(argv) == 0 { + return fmt.Errorf("empty command") + } + if !allow.Allows(argv) { + return fmt.Errorf("subcommand not on the allowlist: %q", strings.Join(subcommandPath(argv), " ")) + } + + floor := denyFloor // base floor; provider additions are filtered at load time. + for i := 0; i < len(argv); i++ { + tok := argv[i] + flag, value, hasInlineValue := splitFlag(tok) + + if strings.HasPrefix(flag, "-") { + if floorDeniesFlag(floor, flag) { + return fmt.Errorf("flag is on the deny floor: %s", flag) + } + // Resolve the flag's value: inline (--flag=value) or the next token. + val := value + if !hasInlineValue && i+1 < len(argv) && !strings.HasPrefix(argv[i+1], "-") { + val = argv[i+1] + } + if val != "" { + if err := checkArgPrefix(floor, val); err != nil { + return err + } + if allowed, scoped := scope.allowedFor(flag); scoped { + if err := checkScope(flag, val, allowed); err != nil { + return err + } + } + } + continue + } + // Positional token: still subject to the arg-prefix floor. + if err := checkArgPrefix(floor, tok); err != nil { + return err + } + } + return nil +} + +// splitFlag separates a "--flag=value" token into its flag and value. For a +// bare "--flag" or a non-flag token it returns the token unchanged with no +// inline value. +func splitFlag(tok string) (flag, value string, hasInlineValue bool) { + if !strings.HasPrefix(tok, "-") { + return tok, "", false + } + if eq := strings.IndexByte(tok, '='); eq >= 0 { + return tok[:eq], tok[eq+1:], true + } + return tok, "", false +} + +// floorDeniesFlag reports whether flag matches a deny-floored flag name. +func floorDeniesFlag(floor DenyFloor, flag string) bool { + for _, f := range floor.Flags { + if flag == f { + return true + } + } + return false +} + +// checkArgPrefix rejects an argument value beginning with a deny-floored prefix +// (local-file read and SSRF vectors). +func checkArgPrefix(floor DenyFloor, val string) error { + for _, p := range floor.ArgPrefixes { + if strings.HasPrefix(val, p) { + return fmt.Errorf("argument value has a denied prefix %q: %s", p, val) + } + } + return nil +} + +// checkScope rejects a target-selecting flag value outside the allowlist. An +// empty allowlist means the axis is unconstrained. +func checkScope(flag, val string, allowed []string) error { + if len(allowed) == 0 { + return nil + } + for _, a := range allowed { + if val == a { + return nil + } + } + return fmt.Errorf("%s %q is outside the allowed scope", flag, val) +} diff --git a/pkg/mcp/cloud/validate_test.go b/pkg/mcp/cloud/validate_test.go new file mode 100644 index 0000000..ba77a32 --- /dev/null +++ b/pkg/mcp/cloud/validate_test.go @@ -0,0 +1,68 @@ +package cloud + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestValidateArgvRejectsDenyFloorAndScope(t *testing.T) { + t.Parallel() + al := &CommandAllowlist{Commands: []Command{{Path: "compute instances list"}}} + scope := ScopeAllowlist{Projects: []string{"prod"}, Regions: []string{"us-central1"}} + cases := []struct { + name string + argv []string + ok bool + }{ + {"allowed", []string{"compute", "instances", "list", "--project", "prod"}, true}, + {"allowed-region", []string{"compute", "instances", "list", "--project", "prod", "--region", "us-central1"}, true}, + {"bad-scope", []string{"compute", "instances", "list", "--project", "other"}, false}, + {"bad-region", []string{"compute", "instances", "list", "--project", "prod", "--region", "eu-west1"}, false}, + {"impersonate", []string{"compute", "instances", "list", "--impersonate-service-account", "x"}, false}, + {"account-flag", []string{"compute", "instances", "list", "--account", "evil"}, false}, + {"profile-flag", []string{"compute", "instances", "list", "--profile", "evil"}, false}, + {"endpoint-flag", []string{"compute", "instances", "list", "--endpoint-url", "http://evil"}, false}, + {"file-prefix", []string{"compute", "instances", "list", "--filter", "@/etc/passwd"}, false}, + {"fileurl-prefix", []string{"compute", "instances", "list", "--filter", "file:///etc/passwd"}, false}, + {"httpurl-prefix", []string{"compute", "instances", "list", "--filter", "https://evil"}, false}, + {"metachar-semicolon", []string{"compute", "instances", "list", ";", "rm", "-rf", "/"}, false}, + {"metachar-pipe", []string{"compute", "instances", "list", "|", "cat"}, false}, + {"metachar-subshell", []string{"compute", "instances", "list", "$(whoami)"}, false}, + {"not-allowed", []string{"iam", "service-accounts", "create"}, false}, + {"empty", []string{}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := validateArgv(tc.argv, al, scope) + if tc.ok { + assert.NoError(t, err, "expected argv to validate") + } else { + assert.Error(t, err, "expected validation error") + } + }) + } +} + +func TestValidateArgvEqualsFormFlag(t *testing.T) { + t.Parallel() + al := &CommandAllowlist{Commands: []Command{{Path: "compute instances list"}}} + scope := ScopeAllowlist{Projects: []string{"prod"}} + // --project=other in equals form must be caught by the scope check, and a + // deny-floored flag in equals form must be caught by the floor. + assert.Error(t, validateArgv([]string{"compute", "instances", "list", "--project=other"}, al, scope), + "expected --project=other (equals form) to fail the scope check") + assert.Error(t, validateArgv([]string{"compute", "instances", "list", "--impersonate-service-account=x"}, al, scope), + "expected --impersonate-service-account=x (equals form) to be denied") + assert.NoError(t, validateArgv([]string{"compute", "instances", "list", "--project=prod"}, al, scope), + "expected --project=prod (equals form, in scope) to validate") +} + +func TestValidateArgvEmptyScopeAllowsAnyTarget(t *testing.T) { + t.Parallel() + al := &CommandAllowlist{Commands: []Command{{Path: "projects list"}}} + // An empty scope means the deployment did not constrain targets; the scope + // check must not reject a --project then. + assert.NoError(t, validateArgv([]string{"projects", "list", "--project", "anything"}, al, ScopeAllowlist{}), + "empty scope should not reject a target") +}