From d16a73a79521bad6194d69f7822718d62f77fb0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Sun, 31 May 2026 05:15:13 +0200 Subject: [PATCH 1/5] feat(profile): aws cloud accounts list + source_profile A multi-account aws cloud source carries a source_profile (the operator's SSO base) and an accounts list (one read-only role per account). Validation requires source_profile and at least one account with non-empty, source-unique account ids and role_arns when accounts is set; the single-assumed_identity profile form stays valid and is mutually exclusive with accounts. Towards #44 Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/profile/profile.go | 30 +++++++++-- internal/profile/profile_test.go | 91 ++++++++++++++++++++++++++++++++ internal/profile/validate.go | 48 ++++++++++++++++- 3 files changed, 162 insertions(+), 7 deletions(-) diff --git a/internal/profile/profile.go b/internal/profile/profile.go index e5ea953..e948abe 100644 --- a/internal/profile/profile.go +++ b/internal/profile/profile.go @@ -158,15 +158,27 @@ type ExtraMCP struct { // AssumedIdentity is the canonical pinned identity shown in the connections // panel — a service-account email for gcp, a role ARN for aws. The two clouds // realize it through different env: gcp impersonates AssumedIdentity directly, -// while aws selects an assume-role profile (Profile) for credentials and checks -// AssumedIdentity (the role ARN) for strict validity. Profile is therefore -// aws-only; gcp ignores it. +// while aws selects an assume-role profile for credentials and checks +// AssumedIdentity (the role ARN) for strict validity. +// +// AWS has two shapes. The single-account form sets Profile (the operator's +// pre-existing AWS_PROFILE selector). The multi-account form sets SourceProfile +// (the operator's SSO base) plus Accounts (one read-only role per account); +// triagent generates a per-account assume-role profile layering each role over +// SourceProfile, and the agent selects among them via set_active_target. GCP +// spans its projects with one impersonated identity, so it ignores all three. type CloudSource struct { Alias string `yaml:"alias"` Provider string `yaml:"provider"` // "gcp" | "aws" AssumedIdentity string `yaml:"assumed_identity"` - Profile string `yaml:"profile,omitempty"` // aws AWS_PROFILE selector; ignored by gcp - Scope cloud.ScopeAllowlist `yaml:"scope,omitempty"` + Profile string `yaml:"profile,omitempty"` // aws single-account AWS_PROFILE selector; ignored by gcp + // SourceProfile is the operator's SSO base profile the generated multi-account + // assume-role profiles layer their role_arn over. Required when Accounts is set. + SourceProfile string `yaml:"source_profile,omitempty"` + // Accounts is the deployment-pinned multi-account set; each entry becomes a + // generated assume-role profile the agent may make active. aws-only. + Accounts []CloudAccount `yaml:"accounts,omitempty"` + Scope cloud.ScopeAllowlist `yaml:"scope,omitempty"` // CommandAllowlistPath points the cloud MCP at a run_cli allowlist override // file; empty uses the provider's embedded default. A relative path resolves // against the profile.yaml's directory at load time (absolutized so the MCP @@ -174,6 +186,14 @@ type CloudSource struct { CommandAllowlistPath string `yaml:"command_allowlist_path,omitempty"` } +// CloudAccount is one aws account in a multi-account cloud source: the account +// id the agent selects by, and the read-only role_arn triagent assumes into it +// from the source's SourceProfile. +type CloudAccount struct { + AccountID string `yaml:"account_id"` + RoleARN string `yaml:"role_arn"` +} + type InvestigationInput struct { ID string `yaml:"id"` Label string `yaml:"label"` diff --git a/internal/profile/profile_test.go b/internal/profile/profile_test.go index c32d99e..2ebef0e 100644 --- a/internal/profile/profile_test.go +++ b/internal/profile/profile_test.go @@ -307,6 +307,97 @@ func TestValidateCloudAWSMissingProfile(t *testing.T) { assert.Contains(t, err.Error(), "profile") } +const awsAccountsYAML = ` +name: example +description: test profile +auth: + kind: kubeconfig +playbooks: + entrypoint: a + closing: b +cloud: + - alias: prod-aws + provider: aws + assumed_identity: arn:aws:iam::111111111111:role/triage-readonly + source_profile: sso-admin + accounts: + - {account_id: "111111111111", role_arn: "arn:aws:iam::111111111111:role/triage-readonly"} + - {account_id: "222222222222", role_arn: "arn:aws:iam::222222222222:role/triage-readonly"} +` + +func TestCloudSourceAWSAccounts(t *testing.T) { + p, err := profile.Parse(strings.NewReader(awsAccountsYAML)) + require.NoError(t, err) + require.NoError(t, p.Validate()) + require.Len(t, p.Cloud[0].Accounts, 2) + assert.Equal(t, "sso-admin", p.Cloud[0].SourceProfile) + assert.Equal(t, "111111111111", p.Cloud[0].Accounts[0].AccountID) + assert.Equal(t, "arn:aws:iam::222222222222:role/triage-readonly", p.Cloud[0].Accounts[1].RoleARN) +} + +// awsAccountsBase is a valid multi-account aws source the negative-case tests +// each break in exactly one way. +func awsAccountsBase() profile.CloudSource { + return profile.CloudSource{ + Alias: "prod-aws", + Provider: "aws", + AssumedIdentity: "arn:aws:iam::111111111111:role/triage-readonly", + SourceProfile: "sso-admin", + Accounts: []profile.CloudAccount{ + {AccountID: "111111111111", RoleARN: "arn:aws:iam::111111111111:role/triage-readonly"}, + {AccountID: "222222222222", RoleARN: "arn:aws:iam::222222222222:role/triage-readonly"}, + }, + } +} + +func TestValidateCloudAWSAccountsOK(t *testing.T) { + p := validCloudBase() + p.Cloud = []profile.CloudSource{awsAccountsBase()} + assert.NoError(t, p.Validate(), "an aws source with source_profile + unique accounts must validate clean") +} + +func TestValidateCloudAWSAccountsRequireSourceProfile(t *testing.T) { + p := validCloudBase() + src := awsAccountsBase() + src.SourceProfile = "" + p.Cloud = []profile.CloudSource{src} + err := p.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "source_profile") +} + +func TestValidateCloudAWSAccountsDuplicateID(t *testing.T) { + p := validCloudBase() + src := awsAccountsBase() + src.Accounts[1].AccountID = src.Accounts[0].AccountID + p.Cloud = []profile.CloudSource{src} + err := p.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "account_id") + assert.Contains(t, err.Error(), "duplicate") +} + +func TestValidateCloudAWSAccountsDuplicateRoleARN(t *testing.T) { + p := validCloudBase() + src := awsAccountsBase() + src.Accounts[1].RoleARN = src.Accounts[0].RoleARN + p.Cloud = []profile.CloudSource{src} + err := p.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "role_arn") + assert.Contains(t, err.Error(), "duplicate") +} + +func TestValidateCloudAWSAccountsEmptyFields(t *testing.T) { + p := validCloudBase() + src := awsAccountsBase() + src.Accounts[0].AccountID = "" + p.Cloud = []profile.CloudSource{src} + err := p.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "account_id") +} + func TestDefaultProfilePromptsPopulated(t *testing.T) { p, err := profile.LoadEmbedded("default") if err != nil { diff --git a/internal/profile/validate.go b/internal/profile/validate.go index d3103b9..1936e33 100644 --- a/internal/profile/validate.go +++ b/internal/profile/validate.go @@ -84,8 +84,8 @@ func (p *Profile) Validate() error { if c.AssumedIdentity == "" { errs = append(errs, fmt.Sprintf("cloud[%d].assumed_identity: required", i)) } - if c.Provider == "aws" && c.Profile == "" { - errs = append(errs, fmt.Sprintf("cloud[%d].profile: required when provider=aws", i)) + if c.Provider == "aws" { + errs = append(errs, validateAWSCredentials(i, c)...) } } @@ -94,3 +94,47 @@ func (p *Profile) Validate() error { } return errors.New("profile " + p.Name + " invalid:\n - " + strings.Join(errs, "\n - ")) } + +// validateAWSCredentials checks the two valid aws credential shapes. The +// multi-account form (accounts set) requires source_profile, at least one +// account, and non-empty account ids and role_arns that are each unique across +// the source. The single-account form (no accounts) requires the operator's +// pre-existing profile selector. The two are mutually exclusive: an accounts +// list pins its own profile per account, so a top-level profile alongside it is +// a misconfiguration. +func validateAWSCredentials(i int, c CloudSource) []string { + if len(c.Accounts) == 0 { + if c.Profile == "" { + return []string{fmt.Sprintf("cloud[%d].profile: required when provider=aws (or set accounts + source_profile)", i)} + } + return nil + } + + var errs []string + if c.Profile != "" { + errs = append(errs, fmt.Sprintf("cloud[%d].profile: must be empty when accounts is set (each account pins its own generated profile)", i)) + } + if c.SourceProfile == "" { + errs = append(errs, fmt.Sprintf("cloud[%d].source_profile: required when accounts is set", i)) + } + seenIDs := map[string]bool{} + seenARNs := map[string]bool{} + for j, a := range c.Accounts { + switch { + case a.AccountID == "": + errs = append(errs, fmt.Sprintf("cloud[%d].accounts[%d].account_id: required", i, j)) + case seenIDs[a.AccountID]: + errs = append(errs, fmt.Sprintf("cloud[%d].accounts[%d].account_id: duplicate %q", i, j, a.AccountID)) + } + seenIDs[a.AccountID] = true + + switch { + case a.RoleARN == "": + errs = append(errs, fmt.Sprintf("cloud[%d].accounts[%d].role_arn: required", i, j)) + case seenARNs[a.RoleARN]: + errs = append(errs, fmt.Sprintf("cloud[%d].accounts[%d].role_arn: duplicate %q", i, j, a.RoleARN)) + } + seenARNs[a.RoleARN] = true + } + return errs +} From c479b48d7afacdccebe04c00c1d3913a048a9c5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Sun, 31 May 2026 05:17:31 +0200 Subject: [PATCH 2/5] feat(cloud/aws): configured accounts, generated profiles, active-target env aws.New takes the source alias, source_profile, and account set. ConfiguredTargets surfaces the accounts as the agent's selectable targets; ActiveTargetEnv pins AWS_PROFILE to each account's generated profile name. profiles.go writes a delimited, idempotent managed block per alias into ~/.aws/config (or $AWS_CONFIG_FILE), one assume-role profile per account layering its role_arn over the operator's source_profile. The write is tmp-file-then-rename and replaces only the alias's own block, so operator-authored profiles and other aliases survive. New generates the block at construction, so the profiles exist before any probe or run_cli on both the serve subprocess and launcher-side paths. Towards #44 Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/mcp/cloud/providers/aws/profiles.go | 117 +++++++++++++++++++ pkg/mcp/cloud/providers/aws/profiles_test.go | 92 +++++++++++++++ pkg/mcp/cloud/providers/aws/provider.go | 88 +++++++++++--- pkg/mcp/cloud/providers/aws/provider_test.go | 45 +++++++ 4 files changed, 328 insertions(+), 14 deletions(-) create mode 100644 pkg/mcp/cloud/providers/aws/profiles.go create mode 100644 pkg/mcp/cloud/providers/aws/profiles_test.go diff --git a/pkg/mcp/cloud/providers/aws/profiles.go b/pkg/mcp/cloud/providers/aws/profiles.go new file mode 100644 index 0000000..5d0aca0 --- /dev/null +++ b/pkg/mcp/cloud/providers/aws/profiles.go @@ -0,0 +1,117 @@ +package aws + +import ( + "fmt" + "os" + "path/filepath" + "strings" +) + +// profileName is the AWS_PROFILE for one configured account: a deterministic +// triagent-cloud-- so the server's ActiveTargetEnv and the +// generated ~/.aws/config block name the same profile. Exported for reuse by the +// launcher-side env builders, which must pin the same profile the provider +// generated. +func ProfileName(alias, accountID string) string { + return profileName(alias, accountID) +} + +func profileName(alias, accountID string) string { + return "triagent-cloud-" + alias + "-" + accountID +} + +// blockMarkers returns the BEGIN/END comment lines delimiting one alias's +// managed section in ~/.aws/config, so a rewrite replaces exactly that block and +// never touches operator-authored profiles or another alias's block. +func blockMarkers(alias string) (begin, end string) { + return "# BEGIN triagent-cloud-" + alias, "# END triagent-cloud-" + alias +} + +// writeManagedProfiles writes (or replaces) the managed assume-role profiles for +// one cloud source's accounts into configPath, atomically and idempotently. Each +// account gets a [profile triagent-cloud--] section layering +// its role_arn over sourceProfile (the operator's SSO base); triagent holds no +// credential — the aws CLI performs the assume-role from sourceProfile at run +// time. The section is bounded by # BEGIN/# END triagent-cloud- markers, +// so a rewrite replaces only that alias's block and leaves operator-authored +// profiles and other aliases' blocks untouched. Writing is tmp-file-then-rename +// so a crash never leaves a half-written config. +func writeManagedProfiles(configPath, alias, sourceProfile string, accounts []Account) error { + begin, end := blockMarkers(alias) + + var block strings.Builder + block.WriteString(begin) + block.WriteString("\n") + for _, a := range accounts { + fmt.Fprintf(&block, "[profile %s]\n", profileName(alias, a.ID)) + fmt.Fprintf(&block, "role_arn = %s\n", a.RoleARN) + fmt.Fprintf(&block, "source_profile = %s\n", sourceProfile) + } + block.WriteString(end) + block.WriteString("\n") + + existing, err := os.ReadFile(configPath) + if err != nil && !os.IsNotExist(err) { + return fmt.Errorf("aws: read config %s: %w", configPath, err) + } + merged := replaceBlock(string(existing), begin, end, block.String()) + return atomicWrite(configPath, []byte(merged)) +} + +// replaceBlock splices block in place of any existing begin..end region in +// content, appending it (after a separating blank line) when no prior block is +// present. Lines outside the region are preserved verbatim, so operator-authored +// profiles survive. +func replaceBlock(content, begin, end, block string) string { + bIdx := strings.Index(content, begin) + if bIdx < 0 { + if content == "" { + return block + } + if !strings.HasSuffix(content, "\n") { + content += "\n" + } + return content + "\n" + block + } + eIdx := strings.Index(content[bIdx:], end) + if eIdx < 0 { + // Truncated prior block (no END): replace from BEGIN to end of file. + return content[:bIdx] + block + } + tailStart := bIdx + eIdx + len(end) + tail := content[tailStart:] + tail = strings.TrimPrefix(tail, "\n") + return content[:bIdx] + block + tail +} + +// atomicWrite writes body to dst via a sibling tmp file then renames it into +// place, so a reader never observes a partially written ~/.aws/config. +func atomicWrite(dst string, body []byte) error { + if err := os.MkdirAll(filepath.Dir(dst), 0o700); err != nil { + return fmt.Errorf("aws: create config dir for %s: %w", dst, err) + } + tmp := dst + ".tmp" + if err := os.WriteFile(tmp, body, 0o600); err != nil { + return fmt.Errorf("aws: write config tmp %s: %w", tmp, err) + } + if err := os.Rename(tmp, dst); err != nil { + return fmt.Errorf("aws: rename config %s: %w", dst, err) + } + return nil +} + +// awsConfigPath resolves the file writeManagedProfiles writes to: AWS_CONFIG_FILE +// when set (so a deployment or test can redirect it), else $HOME/.aws/config — +// the location the aws CLI reads profiles from. The empty return (no HOME, no +// override) signals the caller to skip generation rather than write to a +// surprising path. +func awsConfigPath() string { + if override := os.Getenv("AWS_CONFIG_FILE"); override != "" { + return override + } + home, err := os.UserHomeDir() + if err != nil || home == "" { + return "" + } + return filepath.Join(home, ".aws", "config") +} diff --git a/pkg/mcp/cloud/providers/aws/profiles_test.go b/pkg/mcp/cloud/providers/aws/profiles_test.go new file mode 100644 index 0000000..e8c0b53 --- /dev/null +++ b/pkg/mcp/cloud/providers/aws/profiles_test.go @@ -0,0 +1,92 @@ +package aws + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestProfileName(t *testing.T) { + assert.Equal(t, "triagent-cloud-prod-aws-111111111111", profileName("prod-aws", "111111111111")) +} + +func TestWriteManagedProfilesBlock(t *testing.T) { + dir := t.TempDir() + cfg := filepath.Join(dir, "config") + accs := []Account{ + {ID: "111111111111", RoleARN: "arn:aws:iam::111111111111:role/triage-readonly"}, + {ID: "222222222222", RoleARN: "arn:aws:iam::222222222222:role/triage-readonly"}, + } + require.NoError(t, writeManagedProfiles(cfg, "prod-aws", "sso-admin", accs)) + + b, err := os.ReadFile(cfg) + require.NoError(t, err) + got := string(b) + assert.Contains(t, got, "# BEGIN triagent-cloud-prod-aws") + assert.Contains(t, got, "# END triagent-cloud-prod-aws") + assert.Contains(t, got, "[profile triagent-cloud-prod-aws-111111111111]") + assert.Contains(t, got, "[profile triagent-cloud-prod-aws-222222222222]") + assert.Contains(t, got, "role_arn = arn:aws:iam::111111111111:role/triage-readonly") + assert.Contains(t, got, "source_profile = sso-admin") +} + +// TestWriteManagedProfilesIdempotent proves a second write for the same alias +// replaces the prior block rather than appending a duplicate. +func TestWriteManagedProfilesIdempotent(t *testing.T) { + dir := t.TempDir() + cfg := filepath.Join(dir, "config") + accs := []Account{{ID: "111111111111", RoleARN: "arn:aws:iam::111111111111:role/r"}} + require.NoError(t, writeManagedProfiles(cfg, "prod-aws", "sso-admin", accs)) + require.NoError(t, writeManagedProfiles(cfg, "prod-aws", "sso-admin", accs)) + + b, err := os.ReadFile(cfg) + require.NoError(t, err) + got := string(b) + assert.Equal(t, 1, strings.Count(got, "[profile triagent-cloud-prod-aws-111111111111]")) + assert.Equal(t, 1, strings.Count(got, "# BEGIN triagent-cloud-prod-aws")) +} + +// TestWriteManagedProfilesPreservesForeignContent proves the managed block is +// delimited: pre-existing operator profiles outside it survive a rewrite. +func TestWriteManagedProfilesPreservesForeignContent(t *testing.T) { + dir := t.TempDir() + cfg := filepath.Join(dir, "config") + foreign := "[profile sso-admin]\nsso_start_url = https://example.awsapps.com/start\n\n" + require.NoError(t, os.WriteFile(cfg, []byte(foreign), 0o600)) + + accs := []Account{{ID: "111111111111", RoleARN: "arn:aws:iam::111111111111:role/r"}} + require.NoError(t, writeManagedProfiles(cfg, "prod-aws", "sso-admin", accs)) + require.NoError(t, writeManagedProfiles(cfg, "prod-aws", "sso-admin", accs)) + + b, err := os.ReadFile(cfg) + require.NoError(t, err) + got := string(b) + assert.Contains(t, got, "[profile sso-admin]") + assert.Contains(t, got, "sso_start_url = https://example.awsapps.com/start") + assert.Equal(t, 1, strings.Count(got, "[profile sso-admin]"), "foreign content must not be duplicated") +} + +// TestWriteManagedProfilesTwoAliases proves two managed blocks for different +// aliases coexist: rewriting one leaves the other intact. +func TestWriteManagedProfilesTwoAliases(t *testing.T) { + dir := t.TempDir() + cfg := filepath.Join(dir, "config") + require.NoError(t, writeManagedProfiles(cfg, "prod-aws", "sso-prod", + []Account{{ID: "111111111111", RoleARN: "arn:aws:iam::111111111111:role/r"}})) + require.NoError(t, writeManagedProfiles(cfg, "staging-aws", "sso-staging", + []Account{{ID: "222222222222", RoleARN: "arn:aws:iam::222222222222:role/r"}})) + require.NoError(t, writeManagedProfiles(cfg, "prod-aws", "sso-prod", + []Account{{ID: "111111111111", RoleARN: "arn:aws:iam::111111111111:role/r"}})) + + b, err := os.ReadFile(cfg) + require.NoError(t, err) + got := string(b) + assert.Contains(t, got, "# BEGIN triagent-cloud-prod-aws") + assert.Contains(t, got, "# BEGIN triagent-cloud-staging-aws") + assert.Equal(t, 1, strings.Count(got, "# BEGIN triagent-cloud-prod-aws")) + assert.Equal(t, 1, strings.Count(got, "# BEGIN triagent-cloud-staging-aws")) +} diff --git a/pkg/mcp/cloud/providers/aws/provider.go b/pkg/mcp/cloud/providers/aws/provider.go index d4527b7..1d2ab91 100644 --- a/pkg/mcp/cloud/providers/aws/provider.go +++ b/pkg/mcp/cloud/providers/aws/provider.go @@ -44,11 +44,35 @@ var _ cloud.Provider = (*Provider)(nil) // ScopeAllowlist.Regions. If a future deployment needs sub-account argv scoping, // it belongs in the shared validateArgv, not in this provider. +// Account is one configured aws account the agent may make active: the account +// id it selects by, and the read-only role_arn triagent generates an assume-role +// profile for, layered over the source's SSO base. +type Account struct { + ID string + RoleARN string +} + +// Options carries the multi-account config the launcher threads through from the +// profile's cloud source: the source alias (the generated profiles' namespace), +// the operator's SSO source_profile, and the account set. The zero value is the +// single-account legacy form — no generated profiles, the selectable set comes +// from inventory. +type Options struct { + Alias string + SourceProfile string + Accounts []Account +} + // Provider is the AWS realization of cloud.Provider. binary is resolved once at // construction (overridable in tests); allowlist is the parsed embedded default. +// alias and accounts carry the multi-account config: ConfiguredTargets surfaces +// the accounts as the selectable set, and ActiveTargetEnv names each account's +// generated profile. type Provider struct { binary string allowlist *cloud.CommandAllowlist + alias string + accounts []Account } // New constructs the AWS provider, resolving aws to an absolute path once via @@ -56,7 +80,13 @@ type Provider struct { // PATH with relative entries makes LookPath return a relative path (flagged with // exec.ErrDot); the path is made absolute so a later subprocess env/PATH change // cannot reinterpret it against a different working directory. -func New() (*Provider, error) { +// +// When opts carries accounts, New generates the per-account assume-role profiles +// into ~/.aws/config (or $AWS_CONFIG_FILE) before returning, so the profiles +// exist for both the serve subprocess and any launcher-side probe that runs the +// CLI under AWS_PROFILE. Generation is idempotent: repeated construction (serve +// and launcher both build the provider) replaces the alias's managed block. +func New(opts ...Options) (*Provider, error) { bin, err := exec.LookPath("aws") if err != nil && !errors.Is(err, exec.ErrDot) { return nil, fmt.Errorf("aws: resolve aws binary: %w", err) @@ -65,17 +95,31 @@ func New() (*Provider, error) { if err != nil { return nil, fmt.Errorf("aws: resolve aws binary to absolute path: %w", err) } - return newWithBinary(abs) + return newWithBinary(abs, opts...) } // newWithBinary builds the provider against an already-resolved binary path. It -// is the seam tests inject a fixed path through, bypassing exec.LookPath. -func newWithBinary(binary string) (*Provider, error) { +// is the seam tests inject a fixed path through, bypassing exec.LookPath. At most +// one Options is honored; the zero value is the single-account legacy form. +func newWithBinary(binary string, opts ...Options) (*Provider, error) { var list cloud.CommandAllowlist if err := json.Unmarshal(defaultCommandsJSON, &list); err != nil { return nil, fmt.Errorf("aws: parse default allowlist: %w", err) } - return &Provider{binary: binary, allowlist: &list}, nil + var o Options + if len(opts) > 0 { + o = opts[0] + } + if len(o.Accounts) > 0 { + cfg := awsConfigPath() + if cfg == "" { + return nil, fmt.Errorf("aws: cannot resolve ~/.aws/config (no HOME and no AWS_CONFIG_FILE) to generate account profiles") + } + if err := writeManagedProfiles(cfg, o.Alias, o.SourceProfile, o.Accounts); err != nil { + return nil, fmt.Errorf("aws: generate account profiles: %w", err) + } + } + return &Provider{binary: binary, allowlist: &list, alias: o.Alias, accounts: o.Accounts}, nil } // Name reports the provider identifier. @@ -119,17 +163,33 @@ func (p *Provider) DenyFloorAdditions() cloud.DenyFloor { } } -// ConfiguredTargets is the deployment-configured account set. The single-account -// deployment carries no accounts list, so the selectable set comes from the -// server's inventory; the multi-account accounts list arrives with the AWS -// accounts config. -func (p *Provider) ConfiguredTargets() []cloud.Target { return nil } +// ConfiguredTargets is the deployment-configured account set surfaced as the +// agent's selectable targets. A configured account's id is both the Target ID +// (what set_active_target receives) and its Name. The single-account deployment +// carries no accounts list and returns nil, so the selectable set comes from the +// server's inventory instead. +func (p *Provider) ConfiguredTargets() []cloud.Target { + if len(p.accounts) == 0 { + return nil + } + out := make([]cloud.Target, 0, len(p.accounts)) + for _, a := range p.accounts { + out = append(out, cloud.Target{ID: a.ID, Name: a.ID}) + } + return out +} -// ActiveTargetEnv pins the aws CLI to the active account via AWS_PROFILE, the -// generated assume-role profile for that account. The value is a profile name, -// not a credential: the CLI performs the assume-role from the operator's base. +// ActiveTargetEnv pins the aws CLI to the active account via AWS_PROFILE. For a +// configured account it names the generated assume-role profile +// (triagent-cloud--); the single-account legacy form passes +// the id through as the profile name directly. Either way the value is a profile +// name, not a credential: the CLI performs the assume-role from the operator's +// base. func (p *Provider) ActiveTargetEnv(id string) []string { - return []string{EnvProfile + "=" + id} + if len(p.accounts) == 0 { + return []string{EnvProfile + "=" + id} + } + return []string{EnvProfile + "=" + profileName(p.alias, id)} } // EnvPassthrough lists the env var NAMES the aws subprocess needs forwarded: diff --git a/pkg/mcp/cloud/providers/aws/provider_test.go b/pkg/mcp/cloud/providers/aws/provider_test.go index b3efd36..4ed2336 100644 --- a/pkg/mcp/cloud/providers/aws/provider_test.go +++ b/pkg/mcp/cloud/providers/aws/provider_test.go @@ -206,3 +206,48 @@ func keyOf(argv []string) string { } var errAccessDenied = errors.New("access denied (AccessDeniedException) when calling the ListAccounts operation") + +func TestConfiguredTargetsEmptyForSingleAccount(t *testing.T) { + p, err := newWithBinary("/usr/bin/aws") + require.NoError(t, err) + assert.Nil(t, p.ConfiguredTargets()) +} + +func TestConfiguredTargetsFromAccounts(t *testing.T) { + p := providerWithAccounts(t, "prod-aws", []Account{ + {ID: "111111111111", RoleARN: "arn:aws:iam::111111111111:role/r"}, + {ID: "222222222222", RoleARN: "arn:aws:iam::222222222222:role/r"}, + }) + targets := p.ConfiguredTargets() + require.Len(t, targets, 2) + assert.Equal(t, "111111111111", targets[0].ID) + assert.Equal(t, "111111111111", targets[0].Name) + assert.Equal(t, "222222222222", targets[1].ID) +} + +func TestActiveTargetEnvUsesGeneratedProfileName(t *testing.T) { + p := providerWithAccounts(t, "prod-aws", []Account{ + {ID: "111111111111", RoleARN: "arn:aws:iam::111111111111:role/r"}, + }) + assert.Equal(t, []string{"AWS_PROFILE=triagent-cloud-prod-aws-111111111111"}, p.ActiveTargetEnv("111111111111")) +} + +// TestActiveTargetEnvSingleAccountPassthrough proves the legacy single-account +// provider (no alias, no accounts) treats the active id as the profile name +// directly, reproducing today's AWS_PROFILE= behavior. +func TestActiveTargetEnvSingleAccountPassthrough(t *testing.T) { + p, err := newWithBinary("/usr/bin/aws") + require.NoError(t, err) + assert.Equal(t, []string{"AWS_PROFILE=ro"}, p.ActiveTargetEnv("ro")) +} + +// providerWithAccounts builds an aws provider with a generated-profile config +// pointed at a temp AWS config file so construction's writeManagedProfiles call +// does not touch the developer's ~/.aws/config. +func providerWithAccounts(t *testing.T, alias string, accs []Account) *Provider { + t.Helper() + t.Setenv("AWS_CONFIG_FILE", filepath.Join(t.TempDir(), "config")) + p, err := newWithBinary("/usr/bin/aws", Options{Alias: alias, SourceProfile: "sso-admin", Accounts: accs}) + require.NoError(t, err) + return p +} From 51948299483a150859ca31b79c5f7adf16247f7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Sun, 31 May 2026 05:18:23 +0200 Subject: [PATCH 3/5] fix(cloud/aws): inventory reflects the configured accounts, not the whole org MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the source carries a configured accounts list, Inventory returns exactly those accounts as the reachable set and shells nothing — each account is its own read-only role, so an org-wide list-accounts would advertise accounts run_cli cannot enter. The single-account form keeps the organizations list-accounts + caller-account fallback. Towards #44 Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/mcp/cloud/providers/aws/inventory.go | 18 +++++++++++++++++- pkg/mcp/cloud/providers/aws/inventory_test.go | 19 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/pkg/mcp/cloud/providers/aws/inventory.go b/pkg/mcp/cloud/providers/aws/inventory.go index 2b59d6f..f5d5a9c 100644 --- a/pkg/mcp/cloud/providers/aws/inventory.go +++ b/pkg/mcp/cloud/providers/aws/inventory.go @@ -21,7 +21,15 @@ type organizationsAccount struct { Status string `json:"Status"` } -// Inventory projects the AWS accounts the pinned identity can read. The primary +// Inventory projects the AWS accounts the pinned identity can read. +// +// When the source is configured with an accounts list, the reachable set is +// exactly those accounts: each is its own read-only role, so the configured set +// already describes what run_cli can reach. Inventory returns them directly and +// shells nothing — an org-wide list-accounts would over-advertise accounts the +// role cannot enter. +// +// Without a configured accounts list (the single-account form), the primary // source is `aws organizations list-accounts`; only when that fails with an // Organizations-unavailable condition (AccessDenied or the account not being a // member of an organization) does it fall back to the single account the caller @@ -30,6 +38,14 @@ type organizationsAccount struct { // behind the single-account fallback. Both commands are allowlisted so the // projection works under the validated run core. func (p *Provider) Inventory(ctx context.Context, run cloud.RunFunc) (cloud.Inventory, error) { + if len(p.accounts) > 0 { + scopes := make([]cloud.Scope, 0, len(p.accounts)) + for _, a := range p.accounts { + scopes = append(scopes, cloud.Scope{ID: a.ID, Name: a.ID}) + } + return cloud.Inventory{Scopes: scopes}, nil + } + res, err := run(ctx, []string{"organizations", "list-accounts", "--output", "json"}) if err != nil { return cloud.Inventory{}, fmt.Errorf("aws organizations list-accounts: %w", err) diff --git a/pkg/mcp/cloud/providers/aws/inventory_test.go b/pkg/mcp/cloud/providers/aws/inventory_test.go index 63053b0..1225c91 100644 --- a/pkg/mcp/cloud/providers/aws/inventory_test.go +++ b/pkg/mcp/cloud/providers/aws/inventory_test.go @@ -17,6 +17,25 @@ const listAccountsOutput = `{ ] }` +// TestInventoryUsesConfiguredAccounts proves a provider built with a configured +// accounts list reports exactly those accounts as the reachable set, without +// calling organizations list-accounts — the run func must never be invoked. +func TestInventoryUsesConfiguredAccounts(t *testing.T) { + p := providerWithAccounts(t, "prod-aws", []Account{ + {ID: "111111111111", RoleARN: "arn:aws:iam::111111111111:role/r"}, + {ID: "222222222222", RoleARN: "arn:aws:iam::222222222222:role/r"}, + }) + failRun := func(_ context.Context, argv []string) (cloud.CLIResult, error) { + t.Fatalf("Inventory must not shell the CLI when accounts are configured; got %v", argv) + return cloud.CLIResult{}, nil + } + inv, err := p.Inventory(context.Background(), failRun) + require.NoError(t, err) + require.Len(t, inv.Scopes, 2) + assert.Equal(t, cloud.Scope{ID: "111111111111", Name: "111111111111"}, inv.Scopes[0]) + assert.Equal(t, cloud.Scope{ID: "222222222222", Name: "222222222222"}, inv.Scopes[1]) +} + func TestInventoryProjectsActiveAccounts(t *testing.T) { f := &fakeRun{results: map[string]cloud.CLIResult{ "organizations list-accounts": {Stdout: listAccountsOutput}, From f413004f89f2c780dce041b43c17af293570c55c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Sun, 31 May 2026 05:26:02 +0200 Subject: [PATCH 4/5] feat(cloud): wire aws accounts + source_profile through serve and mcpconfig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds cloud.EnvAWSAccounts (JSON), cloud.EnvAWSSourceProfile, and cloud.EnvAWSAlias. cloudSourceEnv emits them for a multi-account aws source (and no static AWS_PROFILE, since the server pins it per-exec from the active target); the single-account form is unchanged. runCloud decodes them and builds the provider through the factory. The factory (providers.New) and ProbeSource gain an Options/Source path carrying the alias, source_profile, and accounts, so the launcher-side probe builds the aws provider with its profile map — generating the same ~/.aws/config block the serve subprocess does, before any whoami. The launcher probe targets the default (first) account's generated profile; per-account validity is out of scope for v1. Interface changes beyond the plan: providers.New gained a variadic Options arg and providers.Source gained Alias/SourceProfile/Accounts, both required so the launcher-side provider has the profile map the plan called out as under-specified; cloud.EnvAWSAlias was added so serve and the launcher namespace generated profiles identically; profile.CloudAccount gained snake_case json tags to fix the env wire shape. Towards #44 Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/triagent-mcp/serve.go | 39 ++++++++++++++++++-- cmd/triagent-mcp/serve_cloud_test.go | 30 ++++++++++++++++ internal/preflight/mcpconfig.go | 23 +++++++++--- internal/preflight/mcpconfig_test.go | 43 ++++++++++++++++++++++ internal/preflight/preflight.go | 8 +++++ internal/profile/profile.go | 4 +-- pkg/mcp/cloud/env.go | 13 +++++++ pkg/mcp/cloud/providers/probe.go | 45 +++++++++++++++++++----- pkg/mcp/cloud/providers/probe_test.go | 23 ++++++++++++ pkg/mcp/cloud/providers/registry.go | 31 +++++++++++++--- pkg/mcp/cloud/providers/registry_test.go | 29 +++++++++++++++ 11 files changed, 265 insertions(+), 23 deletions(-) diff --git a/cmd/triagent-mcp/serve.go b/cmd/triagent-mcp/serve.go index ced36e8..7f4c1c8 100644 --- a/cmd/triagent-mcp/serve.go +++ b/cmd/triagent-mcp/serve.go @@ -13,6 +13,7 @@ import ( "github.com/sourcehawk/triagent/pkg/mcp/agentoperator" "github.com/sourcehawk/triagent/pkg/mcp/cloud" "github.com/sourcehawk/triagent/pkg/mcp/cloud/providers" + "github.com/sourcehawk/triagent/pkg/mcp/cloud/providers/aws" "github.com/sourcehawk/triagent/pkg/mcp/git" "github.com/sourcehawk/triagent/pkg/mcp/incidentio" "github.com/sourcehawk/triagent/pkg/mcp/k8s" @@ -441,7 +442,10 @@ func runProm(ctx context.Context, f serveFlags) error { // concrete backend; New plugs it in behind cloud.Provider. The launcher passes // the allowlist override path, target scope, and pinned identity through the // subprocess env (cloud.EnvAllowlistPath, cloud.EnvScope, -// cloud.EnvExpectedIdentity), never argv. +// cloud.EnvExpectedIdentity), never argv. A multi-account aws source additionally +// carries its accounts, source_profile, and alias (cloud.EnvAWSAccounts, +// _SOURCE_PROFILE, _ALIAS); New generates the per-account assume-role profiles +// and surfaces the accounts as the agent's selectable targets. 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) @@ -450,7 +454,15 @@ func runCloud(ctx context.Context, f serveFlags) error { if err != nil { return fmt.Errorf("build cloud mcp server: %w", err) } - provider, err := providers.New(f.cloudProvider) + accounts, err := parseAWSAccounts(os.Getenv(cloud.EnvAWSAccounts)) + if err != nil { + return fmt.Errorf("build cloud mcp server: %w", err) + } + provider, err := providers.New(f.cloudProvider, providers.Options{ + AWSAlias: os.Getenv(cloud.EnvAWSAlias), + AWSSourceProfile: os.Getenv(cloud.EnvAWSSourceProfile), + AWSAccounts: accounts, + }) if err != nil { return err } @@ -483,6 +495,29 @@ func parseCloudScope(raw string) (cloud.ScopeAllowlist, error) { return scope, nil } +// parseAWSAccounts decodes the JSON-encoded aws multi-account set the launcher +// froze into the aws provider's account list. An empty value yields nil, the +// single-account / single-identity form. A malformed value is an error that +// aborts startup: failing closed, since a misconfigured accounts list must never +// silently drop accounts the agent should be able to select. +func parseAWSAccounts(raw string) ([]aws.Account, error) { + if raw == "" { + return nil, nil + } + var wire []struct { + AccountID string `json:"account_id"` + RoleARN string `json:"role_arn"` + } + if err := json.Unmarshal([]byte(raw), &wire); err != nil { + return nil, fmt.Errorf("malformed cloud aws accounts in $%s: %w", cloud.EnvAWSAccounts, err) + } + accounts := make([]aws.Account, 0, len(wire)) + for _, w := range wire { + accounts = append(accounts, aws.Account{ID: w.AccountID, RoleARN: w.RoleARN}) + } + return accounts, nil +} + 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 index 6358782..bccde5e 100644 --- a/cmd/triagent-mcp/serve_cloud_test.go +++ b/cmd/triagent-mcp/serve_cloud_test.go @@ -65,3 +65,33 @@ func TestRunCloud_MalformedScopeAborts(t *testing.T) { require.Error(t, err, "a malformed scope must abort cloud-server startup") assert.Contains(t, err.Error(), "scope", "the error should name the scope") } + +func TestParseAWSAccounts_EmptyYieldsNil(t *testing.T) { + t.Parallel() + accs, err := parseAWSAccounts("") + require.NoError(t, err) + assert.Nil(t, accs) +} + +func TestParseAWSAccounts_DecodesJSON(t *testing.T) { + t.Parallel() + accs, err := parseAWSAccounts(`[{"account_id":"111111111111","role_arn":"arn:aws:iam::111111111111:role/r"},{"account_id":"222222222222","role_arn":"arn:aws:iam::222222222222:role/r"}]`) + require.NoError(t, err) + require.Len(t, accs, 2) + assert.Equal(t, "111111111111", accs[0].ID) + assert.Equal(t, "arn:aws:iam::222222222222:role/r", accs[1].RoleARN) +} + +func TestParseAWSAccounts_MalformedFailsClosed(t *testing.T) { + t.Parallel() + _, err := parseAWSAccounts(`[{"account_id":`) + require.Error(t, err, "a malformed accounts list must fail closed, not silently drop accounts") +} + +func TestRunCloud_MalformedAWSAccountsAborts(t *testing.T) { + t.Setenv("TRIAGENT_CLOUD_PROVIDER", "aws") + t.Setenv("TRIAGENT_CLOUD_AWS_ACCOUNTS", `[{"account_id":`) + err := runCloud(context.Background(), serveFlags{kind: "cloud", cloudProvider: "aws"}) + require.Error(t, err, "a malformed accounts list must abort cloud-server startup") + assert.Contains(t, err.Error(), "accounts", "the error should name the accounts") +} diff --git a/internal/preflight/mcpconfig.go b/internal/preflight/mcpconfig.go index d2686b2..daeeb67 100644 --- a/internal/preflight/mcpconfig.go +++ b/internal/preflight/mcpconfig.go @@ -188,10 +188,13 @@ func kubeEnv(in mcpConfigInputs) map[string]string { // The pinned identity is uniform: TRIAGENT_CLOUD_EXPECTED_IDENTITY carries it // for both clouds, and the probe validates the resolved identity against it. The // credential env differs by mechanism: GCP impersonates the assumed identity -// directly (CLOUDSDK_AUTH_IMPERSONATE_SERVICE_ACCOUNT), AWS selects an -// assume-role profile (AWS_PROFILE) whose role_arn is the deployment's read-only -// role. The env-name constants come from the provider packages, never raw -// literals. +// directly (CLOUDSDK_AUTH_IMPERSONATE_SERVICE_ACCOUNT); single-account AWS selects +// the operator's assume-role profile (AWS_PROFILE) whose role_arn is the +// deployment's read-only role. A multi-account AWS source instead carries its +// accounts and source_profile (TRIAGENT_CLOUD_AWS_ACCOUNTS, _SOURCE_PROFILE): the +// subprocess generates a profile per account and pins AWS_PROFILE per run_cli +// from the active target, so no static profile selector belongs in this env. The +// env-name constants come from the provider packages, never raw literals. func cloudSourceEnv(src profile.CloudSource) (map[string]string, error) { env := map[string]string{ cloud.EnvProvider: src.Provider, @@ -210,7 +213,17 @@ func cloudSourceEnv(src profile.CloudSource) (map[string]string, error) { case "gcp": env[gcp.EnvImpersonate] = src.AssumedIdentity case "aws": - env[aws.EnvProfile] = src.Profile + if len(src.Accounts) > 0 { + accountsRaw, err := json.Marshal(src.Accounts) + if err != nil { + return nil, fmt.Errorf("cloud source %q: encode accounts: %w", src.Alias, err) + } + env[cloud.EnvAWSAccounts] = string(accountsRaw) + env[cloud.EnvAWSSourceProfile] = src.SourceProfile + env[cloud.EnvAWSAlias] = src.Alias + } else { + env[aws.EnvProfile] = src.Profile + } } return env, nil } diff --git a/internal/preflight/mcpconfig_test.go b/internal/preflight/mcpconfig_test.go index b734551..ea89ee9 100644 --- a/internal/preflight/mcpconfig_test.go +++ b/internal/preflight/mcpconfig_test.go @@ -466,4 +466,47 @@ func TestWriteMCPConfig_AWSCloudSource_RegistersServerWithProfileAndExpectedRole assert.Equal(t, "triage-ro", env[aws.EnvProfile]) // gcp impersonation env must not leak onto an aws source. assert.NotContains(t, env, gcp.EnvImpersonate) + // The single-account form carries no accounts/source_profile env. + assert.NotContains(t, env, cloud.EnvAWSAccounts) + assert.NotContains(t, env, cloud.EnvAWSSourceProfile) +} + +func TestCloudSourceEnv_AWSAccounts_EmitsAccountsAndSourceProfile(t *testing.T) { + t.Parallel() + env, err := cloudSourceEnv(profile.CloudSource{ + Alias: "prod-aws", + Provider: "aws", + AssumedIdentity: "arn:aws:iam::111111111111:role/triage-ro", + SourceProfile: "sso-admin", + Accounts: []profile.CloudAccount{ + {AccountID: "111111111111", RoleARN: "arn:aws:iam::111111111111:role/triage-ro"}, + {AccountID: "222222222222", RoleARN: "arn:aws:iam::222222222222:role/triage-ro"}, + }, + }) + require.NoError(t, err) + + assert.Equal(t, "sso-admin", env[cloud.EnvAWSSourceProfile]) + require.NotEmpty(t, env[cloud.EnvAWSAccounts], "accounts must be emitted as JSON") + + var decoded []profile.CloudAccount + require.NoError(t, json.Unmarshal([]byte(env[cloud.EnvAWSAccounts]), &decoded)) + require.Len(t, decoded, 2) + assert.Equal(t, "111111111111", decoded[0].AccountID) + assert.Equal(t, "arn:aws:iam::222222222222:role/triage-ro", decoded[1].RoleARN) + + // The multi-account form pins AWS_PROFILE per-exec from the active target, so + // the subprocess credential env carries no static profile selector. + assert.NotContains(t, env, aws.EnvProfile) +} + +func TestCloudSourceEnv_GCP_CarriesNoAWSAccountsEnv(t *testing.T) { + t.Parallel() + env, err := cloudSourceEnv(profile.CloudSource{ + Alias: "prod-gcp", + Provider: "gcp", + AssumedIdentity: "ro@proj.iam.gserviceaccount.com", + }) + require.NoError(t, err) + assert.NotContains(t, env, cloud.EnvAWSAccounts) + assert.NotContains(t, env, cloud.EnvAWSSourceProfile) } diff --git a/internal/preflight/preflight.go b/internal/preflight/preflight.go index b984783..40f98ea 100644 --- a/internal/preflight/preflight.go +++ b/internal/preflight/preflight.go @@ -20,6 +20,7 @@ import ( "github.com/sourcehawk/triagent/pkg/auth" "github.com/sourcehawk/triagent/pkg/mcp/cloud" "github.com/sourcehawk/triagent/pkg/mcp/cloud/providers" + "github.com/sourcehawk/triagent/pkg/mcp/cloud/providers/aws" ) // Options describes a single preflight invocation. @@ -260,10 +261,17 @@ func probeCloudSources(ctx context.Context, sources []profile.CloudSource, probe // provider and shells its whoami CLI. A construction error degrades to an // invalid status, never a session-fatal error. func defaultCloudProbe(ctx context.Context, src profile.CloudSource) cloud.IdentityStatus { + accounts := make([]aws.Account, 0, len(src.Accounts)) + for _, a := range src.Accounts { + accounts = append(accounts, aws.Account{ID: a.AccountID, RoleARN: a.RoleARN}) + } return providers.ProbeSource(ctx, providers.Source{ Provider: src.Provider, AssumedIdentity: src.AssumedIdentity, Profile: src.Profile, + Alias: src.Alias, + SourceProfile: src.SourceProfile, + Accounts: accounts, }) } diff --git a/internal/profile/profile.go b/internal/profile/profile.go index e948abe..78c3553 100644 --- a/internal/profile/profile.go +++ b/internal/profile/profile.go @@ -190,8 +190,8 @@ type CloudSource struct { // id the agent selects by, and the read-only role_arn triagent assumes into it // from the source's SourceProfile. type CloudAccount struct { - AccountID string `yaml:"account_id"` - RoleARN string `yaml:"role_arn"` + AccountID string `yaml:"account_id" json:"account_id"` + RoleARN string `yaml:"role_arn" json:"role_arn"` } type InvestigationInput struct { diff --git a/pkg/mcp/cloud/env.go b/pkg/mcp/cloud/env.go index d74b715..505716f 100644 --- a/pkg/mcp/cloud/env.go +++ b/pkg/mcp/cloud/env.go @@ -20,4 +20,17 @@ const ( // threads it into the identity probe; the provider validates the resolved // identity against it. EnvExpectedIdentity = "TRIAGENT_CLOUD_EXPECTED_IDENTITY" + // EnvAWSAccounts carries the aws multi-account set as a JSON array of + // {account_id, role_arn} objects. The serve subprocess decodes it and builds + // the aws provider's configured targets and generated assume-role profiles. + // Empty for gcp and for the single-account aws form. + EnvAWSAccounts = "TRIAGENT_CLOUD_AWS_ACCOUNTS" + // EnvAWSSourceProfile carries the operator's SSO base profile the generated + // multi-account assume-role profiles layer their role over. aws-only. + EnvAWSSourceProfile = "TRIAGENT_CLOUD_AWS_SOURCE_PROFILE" + // EnvAWSAlias carries the cloud source's alias, the namespace for the + // generated assume-role profile names (triagent-cloud--). + // The launcher-side probe and the serve subprocess both build the provider + // with it, so they name and generate the same profiles. aws-only. + EnvAWSAlias = "TRIAGENT_CLOUD_AWS_ALIAS" ) diff --git a/pkg/mcp/cloud/providers/probe.go b/pkg/mcp/cloud/providers/probe.go index 2273a52..4e874cf 100644 --- a/pkg/mcp/cloud/providers/probe.go +++ b/pkg/mcp/cloud/providers/probe.go @@ -26,14 +26,24 @@ var probeTimeout = 15 * time.Second // per-source credential vars are overlaid on top. var baseEnvPassthrough = []string{"PATH", "HOME"} -// Source is a neutral description of one cloud connection to probe: the -// provider name, the pinned identity, and (aws only) the assume-role profile. -// It carries exactly what ProbeSource needs without coupling this package to -// the launcher's profile type. +// Source is a neutral description of one cloud connection to probe: the provider +// name, the pinned identity, and the aws credential config. It carries exactly +// what ProbeSource needs without coupling this package to the launcher's profile +// type. +// +// AWS has two forms. The single-account form sets Profile (the operator's +// AWS_PROFILE selector). The multi-account form sets Alias, SourceProfile, and +// Accounts; ProbeSource generates the per-account profiles and probes the default +// (first) account's generated profile — per-account validity is out of scope for +// v1, so the panel reflects the source's default-target validity. gcp ignores +// all four. type Source struct { Provider string AssumedIdentity string - Profile string // aws AWS_PROFILE selector; ignored by gcp + Profile string // aws single-account AWS_PROFILE selector; ignored by gcp + Alias string // aws multi-account: the generated profiles' namespace + SourceProfile string // aws multi-account: the operator's SSO base profile + Accounts []aws.Account } // ProbeSource constructs the source's provider and runs the read-only identity @@ -43,7 +53,11 @@ type Source struct { // binary) returns an invalid status with the error as the hint, exactly like a // failed probe. func ProbeSource(ctx context.Context, src Source) cloud.IdentityStatus { - p, err := New(src.Provider) + p, err := New(src.Provider, Options{ + AWSAlias: src.Alias, + AWSSourceProfile: src.SourceProfile, + AWSAccounts: src.Accounts, + }) if err != nil { return cloud.IdentityStatus{ Provider: src.Provider, @@ -108,15 +122,28 @@ func sourceEnvFor(p passthroughLister, src Source) []string { // credentialEnv is the per-provider credential the CLI authenticates with for // the source: gcp impersonates the assumed identity directly; aws selects the -// assume-role profile. The env-name constants come from the provider packages, -// never raw literals. +// assume-role profile. For a multi-account aws source the profile is the default +// (first) account's generated profile name — the same name aws.New wrote into +// ~/.aws/config — so the launcher-side probe reflects the source's default +// target. The env-name constants come from the provider packages, never raw +// literals. func credentialEnv(src Source) map[string]string { switch src.Provider { case "gcp": return map[string]string{gcp.EnvImpersonate: src.AssumedIdentity} case "aws": - return map[string]string{aws.EnvProfile: src.Profile} + return map[string]string{aws.EnvProfile: awsProbeProfile(src)} default: return nil } } + +// awsProbeProfile is the AWS_PROFILE the launcher-side probe authenticates with: +// the default (first) account's generated profile for a multi-account source, +// else the operator's single-account profile selector. +func awsProbeProfile(src Source) string { + if len(src.Accounts) > 0 { + return aws.ProfileName(src.Alias, src.Accounts[0].ID) + } + return src.Profile +} diff --git a/pkg/mcp/cloud/providers/probe_test.go b/pkg/mcp/cloud/providers/probe_test.go index 3b4a316..36ab890 100644 --- a/pkg/mcp/cloud/providers/probe_test.go +++ b/pkg/mcp/cloud/providers/probe_test.go @@ -101,6 +101,29 @@ func TestProbeSourceConstructionFailureKeepsPinnedIdentity(t *testing.T) { assert.NotEmpty(t, st.Hint) } +// TestCredentialEnvAWSMultiAccountTargetsDefaultProfile proves the launcher-side +// probe for a multi-account aws source pins AWS_PROFILE to the default (first) +// account's generated profile name, not the operator's raw profile. Per-account +// validity is out of scope for v1; the panel shows the default target's validity. +func TestCredentialEnvAWSMultiAccountTargetsDefaultProfile(t *testing.T) { + env := credentialEnv(Source{ + Provider: "aws", + Alias: "prod-aws", + SourceProfile: "sso-admin", + Accounts: []aws.Account{ + {ID: "111111111111", RoleARN: "arn:aws:iam::111111111111:role/r"}, + {ID: "222222222222", RoleARN: "arn:aws:iam::222222222222:role/r"}, + }, + }) + assert.Equal(t, "triagent-cloud-prod-aws-111111111111", env[aws.EnvProfile], + "the multi-account probe must target the default account's generated profile") +} + +func TestCredentialEnvAWSSingleAccountUsesProfile(t *testing.T) { + env := credentialEnv(Source{Provider: "aws", Profile: "triage-ro"}) + assert.Equal(t, "triage-ro", env[aws.EnvProfile]) +} + // fakePassthroughProvider exposes a fixed EnvPassthrough so sourceEnv's // carry-and-overlay behaviour can be asserted without a real cloud CLI. type fakePassthroughProvider struct{ passthrough []string } diff --git a/pkg/mcp/cloud/providers/registry.go b/pkg/mcp/cloud/providers/registry.go index 9c5feee..027f0c0 100644 --- a/pkg/mcp/cloud/providers/registry.go +++ b/pkg/mcp/cloud/providers/registry.go @@ -15,17 +15,38 @@ import ( "github.com/sourcehawk/triagent/pkg/mcp/cloud/providers/gcp" ) -// New constructs the cloud.Provider for the named provider ("gcp" | "aws"). The -// concrete New() resolves the provider's CLI binary to an absolute path; a +// Options carries the multi-account config the aws provider needs from the +// profile's cloud source: the source alias (the generated profiles' namespace), +// the operator's SSO source_profile, and the account set. gcp ignores it. The +// zero value is the single-account / single-identity form, so callers that do +// not configure accounts call New(name) unchanged. +type Options struct { + AWSAlias string + AWSSourceProfile string + AWSAccounts []aws.Account +} + +// New constructs the cloud.Provider for the named provider ("gcp" | "aws"), +// threading the aws multi-account config through when present. The concrete +// New() resolves the provider's CLI binary to an absolute path and, for an aws +// source with accounts, generates the per-account assume-role profiles; a // missing binary surfaces as a construction error, which the launcher degrades // to an unavailable cloud source rather than a fatal failure. An unknown name is -// named in the error. -func New(name string) (cloud.Provider, error) { +// named in the error. At most one Options is honored. +func New(name string, opts ...Options) (cloud.Provider, error) { + var o Options + if len(opts) > 0 { + o = opts[0] + } switch name { case "gcp": return gcp.New() case "aws": - return aws.New() + return aws.New(aws.Options{ + Alias: o.AWSAlias, + SourceProfile: o.AWSSourceProfile, + Accounts: o.AWSAccounts, + }) default: return nil, fmt.Errorf("unknown cloud provider %q (want gcp or aws)", name) } diff --git a/pkg/mcp/cloud/providers/registry_test.go b/pkg/mcp/cloud/providers/registry_test.go index 36371ca..9b041a2 100644 --- a/pkg/mcp/cloud/providers/registry_test.go +++ b/pkg/mcp/cloud/providers/registry_test.go @@ -1,8 +1,10 @@ package providers import ( + "path/filepath" "testing" + "github.com/sourcehawk/triagent/pkg/mcp/cloud/providers/aws" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -43,3 +45,30 @@ func TestNew_UnknownProviderErrors(t *testing.T) { assert.Nil(t, p) assert.Contains(t, err.Error(), "azure") } + +// TestNewAWSWithAccounts proves the factory threads the aws multi-account config +// through to the provider: ConfiguredTargets surfaces the accounts and the +// active-target env names the generated profile. Construction generates profiles +// into a temp config so it does not touch the developer's ~/.aws/config; a +// missing aws binary in CI degrades to a construction error, which the test +// tolerates the same way TestNew_KnownProviders does. +func TestNewAWSWithAccounts(t *testing.T) { + t.Setenv("AWS_CONFIG_FILE", filepath.Join(t.TempDir(), "config")) + p, err := New("aws", Options{ + AWSAlias: "prod-aws", + AWSSourceProfile: "sso-admin", + AWSAccounts: []aws.Account{ + {ID: "111111111111", RoleARN: "arn:aws:iam::111111111111:role/r"}, + {ID: "222222222222", RoleARN: "arn:aws:iam::222222222222:role/r"}, + }, + }) + if err != nil { + assert.Nil(t, p, "a construction error must not also return a provider") + return + } + require.NotNil(t, p) + targets := p.ConfiguredTargets() + require.Len(t, targets, 2) + assert.Equal(t, "111111111111", targets[0].ID) + assert.Equal(t, []string{"AWS_PROFILE=triagent-cloud-prod-aws-111111111111"}, p.ActiveTargetEnv("111111111111")) +} From eac0047f2afe61a886f82148f6d473fbdbe16f7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Sun, 31 May 2026 05:28:24 +0200 Subject: [PATCH 5/5] docs(cloud): document multi-account/project active-target selection Adds the set_active_target tool, the AWS accounts + source_profile multi-account config (with a generated-profiles explanation and example), the GCP-one-identity- many-projects vs AWS-one-account-per-role model, and the run_cli-requires-an-active- target rule. Reconciles the pinned-identity, scope-by-omission, and cloud-block sections with the new bounded-selection behavior. Towards #44 Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/content/cloud-providers.md | 54 ++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 7 deletions(-) diff --git a/docs/content/cloud-providers.md b/docs/content/cloud-providers.md index 64f52ce..bd33d1c 100644 --- a/docs/content/cloud-providers.md +++ b/docs/content/cloud-providers.md @@ -12,7 +12,7 @@ The MCP is read-only by construction, not by convention. The agent supplies argu ## The pinned identity -The cloud identity is a deployment-chosen, read-only principal pinned in the profile. The agent can read which identity is active (it has a `session_status` whoami tool) but has no tool to choose, change, or authenticate one. The deployment grants that identity read-only IAM, and that grant is the outermost floor: even a misconfigured-too-broad command allowlist cannot read secrets or exfiltrate, because the identity itself lacks the permission. +The cloud identity is a deployment-chosen, read-only principal pinned in the profile. The agent can read which identity is active (it has a `session_status` whoami tool) and, when the deployment configures more than one target, switch among that pinned set with `set_active_target` (see [Active target](#active-target-moving-across-projects-and-accounts)) — but it has no tool to name an arbitrary identity, escalate one, or authenticate one. The deployment grants each pinned identity read-only IAM, and that grant is the outermost floor: even a misconfigured-too-broad command allowlist cannot read secrets or exfiltrate, because the identity itself lacks the permission. The operator authenticates as themselves through their own normal cloud tooling. The harness then pins impersonation (GCP) or assume-role (AWS) of the configured read-only identity through environment it controls, never through anything the agent can supply. Triagent stores no cloud credential. Re-authentication is the operator's own corporate flow, outside Triagent. @@ -117,6 +117,39 @@ Scope the trust `Principal` to the specific operator users or SSO role rather th The whoami probe resolves the active caller with `aws sts get-caller-identity`. It reports valid when the caller is an assumed-role ARN whose underlying role matches the pinned `assumed_identity`. A plain user or root ARN means the assume-role pin did not take effect and base credentials leaked through, so the source degrades. +### Spanning several AWS accounts + +An IAM role lives in exactly one account, so the single-profile setup above reaches exactly one account. When an investigation crosses accounts, configure the source's `accounts` list instead of `profile`: one entry per account, each a read-only `role_arn` plus the account id the agent selects by. The source also names a `source_profile`, the operator's own SSO base the generated profiles assume from. + +```yaml +cloud: + - alias: prod-aws + provider: aws + assumed_identity: arn:aws:iam::111111111111:role/triage-readonly + source_profile: sso-admin # the operator's SSO base profile + accounts: + - {account_id: "111111111111", role_arn: "arn:aws:iam::111111111111:role/triage-readonly"} + - {account_id: "222222222222", role_arn: "arn:aws:iam::222222222222:role/triage-readonly"} + - {account_id: "333333333333", role_arn: "arn:aws:iam::333333333333:role/triage-readonly"} +``` + +You do not pre-create an `~/.aws/config` profile per account. Triagent generates one read-only assume-role profile per `accounts` entry at session start, into a managed block in your `~/.aws/config` (or `$AWS_CONFIG_FILE`) delimited by `# BEGIN triagent-cloud-` / `# END triagent-cloud-` markers. The block is rewritten idempotently and never touches profiles you authored yourself or another alias's block. Each generated profile layers its account's `role_arn` over `source_profile`, exactly as the single-account profile does by hand — triagent still stores no credential; the AWS CLI performs the assume-role from your SSO base. + +Give each account's role the same read-only permission and trust policies as the single-account role above. `assumed_identity` is the role ARN the agent's default account validates against; the connections panel shows that default account's validity (per-account validity is not surfaced in the panel). + +`accounts` and `profile` are mutually exclusive: a single-account source sets `profile`, a multi-account source sets `accounts` + `source_profile`. + +## Active target: moving across projects and accounts + +A source can span more than one target — several projects under one GCP identity, or several accounts under an AWS `accounts` list. The agent chooses which one subsequent `run_cli` commands run against with the `set_active_target` tool, naming a target id from `list_inventory` (a project id for GCP, an account id for AWS). The agent can select only among the deployment-configured targets; a target outside that set is rejected, and `session_status` reports the active target alongside the pinned identity. + +The two clouds reach their target set by different mechanisms, which is why AWS needs the `accounts` list and GCP does not: + +- **GCP — one identity, many projects.** A single impersonated read-only service account can be granted viewer on every in-scope project, so one identity already spans them. Switching target changes only `CLOUDSDK_CORE_PROJECT`; the identity is unchanged, and `session_status` reports the same service account throughout. The selectable set is the source's `scope.projects` (or, when that axis is empty, the projects `list_inventory` surfaces). +- **AWS — one account per role.** A role lives in one account, so each account is its own read-only role. The selectable set is the source's `accounts` list, and switching target sets `AWS_PROFILE` to that account's generated profile — a different identity per account, so `session_status` re-probes on switch. + +When a source has exactly one target, it is active from session start and the agent need not choose. When it has several and the agent has not yet chosen, `run_cli` returns an actionable error naming `set_active_target` rather than running against an unintended default. This is also why omitting a target flag is safe under multiple targets: the active target is an in-scope pin, never the CLI's ambient default. + ## The `cloud:` profile block Cloud sources live under a top-level `cloud:` list in the profile. Each entry is one provider connection the launcher wires as a `triagent-cloud-` MCP. @@ -146,13 +179,18 @@ cloud: # For aws, the role ARN the assumed-role caller must resolve to. Validity # checks the resolved caller against this exact ARN. assumed_identity: arn:aws:iam::123456789012:role/triage-readonly - # aws-only: the AWS_PROFILE the harness selects for credentials. Its - # role_arn is the read-only role, with the operator's base as - # source_profile. gcp ignores this field. + # aws single-account: the AWS_PROFILE the harness selects for credentials. + # Its role_arn is the read-only role, with the operator's base as + # source_profile. Mutually exclusive with accounts; gcp ignores it. profile: triage-readonly + # aws multi-account: set source_profile + accounts instead of profile to span + # several accounts the agent selects among via set_active_target. + # source_profile: sso-admin + # accounts: + # - {account_id: "111111111111", role_arn: "arn:aws:iam::111111111111:role/triage-readonly"} scope: regions: [eu-west-1] # enforced on run_cli argv. - accounts: ["123456789012"] # informational; account reach is bounded by the pinned role. + accounts: ["123456789012"] # informational scope note; distinct from the source-level accounts list. ``` The fields: @@ -160,7 +198,9 @@ The fields: - `alias` — stable name for the source; the MCP is aliased `triagent-cloud-` and the connections panel keys off it. - `provider` — `gcp` or `aws`. Selects the concrete provider behind the shared MCP. - `assumed_identity` — the canonical pinned identity shown in the connections panel: a service-account email for GCP, a role ARN for AWS. GCP impersonates it directly. AWS checks it as the expected role ARN for strict validity. -- `profile` — AWS only. The `AWS_PROFILE` selector for the assume-role profile that produces credentials. GCP ignores it. +- `profile` — AWS single-account only. The `AWS_PROFILE` selector for the assume-role profile that produces credentials. Mutually exclusive with `accounts`; GCP ignores it. +- `source_profile` — AWS multi-account only. The operator's SSO base profile the generated per-account assume-role profiles layer their role over. Required when `accounts` is set. +- `accounts` — AWS multi-account only. The deployment-pinned account set the agent selects among via `set_active_target`; each entry is `{account_id, role_arn}`. See [Spanning several AWS accounts](#spanning-several-aws-accounts). This is the source-level selectable set, distinct from the informational `scope.accounts` note. - `scope` — the target allowlist (see below). - `command_allowlist_path` — an optional `run_cli` allowlist override (see below). Empty uses the provider's embedded default. @@ -177,7 +217,7 @@ scope: An empty (or omitted) `projects` or `regions` axis is unconstrained on that axis. A non-empty one is a closed set: a `--project`, `--region`, or `--zone` value outside it fails validation before the command runs. -Scope constrains the value of an explicit flag; it does not force one to be present. If the agent omits `--project`, the CLI falls back to its own default target (the impersonated identity's default project, `CLOUDSDK_CORE_PROJECT`, or for AWS the configured `AWS_REGION`), which scope does not police. Hard project confinement therefore comes from the pinned identity's IAM, not from scope: grant the read-only roles only on the in-scope projects, as the setup above does, so an out-of-scope project is unreachable whatever the argv. Region has no equivalent IAM boundary, so treat region scope as a guardrail against explicit pivots rather than a hard limit. +Scope constrains the value of an explicit flag; it does not force one to be present. When a target is active, an omitted `--project` runs against that active target — an in-scope pin (`CLOUDSDK_CORE_PROJECT` for GCP, the active account's profile for AWS), not the CLI's ambient default — so a target-omitting command stays in-scope. Region still has no active-target equivalent: an omitted `--region` falls back to the configured `AWS_REGION` / gcloud default, which scope does not police. Hard project confinement therefore comes from the pinned identity's IAM, not from scope: grant the read-only roles only on the in-scope projects, as the setup above does, so an out-of-scope project is unreachable whatever the argv. Treat region scope as a guardrail against explicit pivots rather than a hard limit. `accounts` is informational and reserved: it documents which AWS accounts the source is expected to reach, but `run_cli` does not validate account ids on argv. What actually bounds account reach is the pinned assume-role profile, whose role can only see the accounts its trust policy and permissions allow. Treat `accounts` as a note to operators, not an enforced allowlist.