feat(cloud): scaffold the read-only cloud-context MCP package and safety harness#48
Merged
sourcehawk merged 9 commits intoMay 30, 2026
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#45) Exact-match the positional subcommand path so a surplus token (a shell metacharacter, an extra argument) cannot ride through on an allowed prefix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mmands (#45) Wire the four tools onto the server, load the command allowlist through the deny floor at construction, and bind run_cli + the providers to a single validated no-shell run core. list_allowed_commands reads the same allowlist run_cli enforces, so advertised equals permitted. Add the TRIAGENT_CLOUD_* env-name constants the launcher injects through the subprocess env. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Parse --provider, decode the frozen scope and allowlist override from the subprocess env, and construct the server behind cloud.Provider. The gcp/aws implementations land in their own PRs; until then a known provider reports it is not yet built and an unknown one is named in the error. Also fold cloud.ToolSpecs() into the launcher tool catalog so the four tools surface in the MCP catalog view alongside every other server. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…iting the parent (#45) s.run passed nil to execCLI, which makes Go's exec set cmd.Env = nil and inherit the entire parent process environment — violating the spec's minimal-env guarantee and harness.go's own no-leak doc comment. The existing TestExecCLIMinimalEnv passed only because it called execCLI directly with an explicit env; the real caller bypassed that. Add Provider.EnvPassthrough() so each provider declares the env var names its CLI needs forwarded, and build the subprocess env from os.Environ() filtered to the base set plus those names via Server.subprocessEnv. A new test exercises the server-built env: a parent-env canary is dropped while a declared passthrough var survives. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Convert the scaffold's tests from bare t.Fatal/t.Fatalf/t.Errorf to testify, the repo standard: require for preconditions a failure must stop at (a non-nil error before a dereference, setup that must succeed), assert for independent checks that should keep running. Assertion intent is preserved exactly; no security assertion is weakened, and the harness_security_test source-scan logic (reading harness.go bytes) stays intact. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
7d2b921 to
43390a1
Compare
sourcehawk
added a commit
that referenced
this pull request
May 30, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Description
Towards #45
Lays the foundation for the read-only cloud-context MCP (
pkg/mcp/cloud/): the provider-agnostic interface, the bypass-resistant command harness, the shared identity probe, the four tools, and theserve.gowiring. This is the scaffold every downstream PR builds on — the GCP provider (#43), the AWS provider (#46), and the launcher integration all consume the contracts that land here. No real cloud provider ships in this PR; afakeProviderdrives the package tests, and--provider=gcp|awsreports "not built yet" until B/C wire the implementations.The security model is the heart of the feature and is implemented by construction here:
run_clinever touches a shell, argv is a typed token array (no in-house splitter to fool), a positive allowlist gates the subcommand path, and a hardcoded deny floor the config can never re-enable covers credential-reading subcommands, identity/endpoint flags, and local-file/SSRF argument prefixes.Changes
cloud.Providerinterface plus projection structs (Inventory,IdentityStatus,CLIResult) and afakeProvidertest double — the contract Implement the GCP provider for the cloud-context MCP #43 and Implement the AWS provider for the cloud-context MCP #46 implement.LoadCommandAllowlist) mirroring the k8sLoadAllowlistpattern: embedded default or profile override, always filtered through a hardcodeddenyFloorplus provider additions. A too-broad override can never re-enable a floored command.--project/region against the scope allowlist.execCLI): directexecve, explicit minimal env, closed stdin, output truncation. Shell metacharacters handed in as argv tokens are inert.cloud.Probe) returningIdentityStatus— the single structsession_status, the connections panel, and preflight all render from. Degrades (Valid:false + Hint), never errors, on a stale credential.list_inventory,session_status,run_cli,list_allowed_commands) withToolSpecs()and a wire test that fails if registration drifts from the catalog.triagent-mcp serve --kind=cloud --provider=<gcp|aws>wiring, theTRIAGENT_CLOUD_PROVIDER/TRIAGENT_CLOUD_ALLOWLIST_PATH/TRIAGENT_CLOUD_SCOPEenv-name constants, andcloud.ToolSpecs()folded into the launcher tool catalog.Challenges
The metacharacter-rejection guarantee and the allowlist match are the same mechanism: making
Allowsmatch the positional subcommand path exactly (rather than as a prefix) is what makes["compute","instances","list",";","rm"]fail — the surplus;andrmtokens make the positional path no longer equal any allowlisted entry. That keeps the no-shell guarantee structural rather than relying on string sanitization. The security tests assert both halves: a source-level scan that no"-c"/sh -cconstruction exists, and a behavioural check that an argv full of metacharacters prints them verbatim and spawns no second process.Related
Testing
make test-gois race-clean and green across all 45 packages;make lintreports 0 issues. Every task was built TDD-first (failing test watched fail for the right reason, then implemented). The load-bearing security tests inharness_security_test.gocover the no-shell guarantee (source scan + behavioural metacharacter-inert + minimal-env + truncation);validate_test.gotables every deny-floor flag, arg-prefix, scope pivot, and metacharacter token;tools_wire_test.goguards registration-vs-catalog drift. Reviewers may want to poke at the deny-floor coverage inallowlist.go/validate.goand confirm theAllowsexact-match reasoning above holds for their threat model.🤖 Generated with Claude Code