fix(aws): stop credential_process recursion in AWS CLI child processes (IGA-3789) - #149
fix(aws): stop credential_process recursion in AWS CLI child processes (IGA-3789)#149highb wants to merge 2 commits into
Conversation
…rsion claim IGA-3789 hypothesizes that getRoleCredentials (aws.go:618-634) shells out to `aws sso get-role-credentials` without setting cmd.Env, letting the child process inherit cone's env, re-resolve the same ~/.aws/config profile, and re-trigger that profile's own credential_process=cone entry recursively. Add a gated, tag-excluded test (reprocredrecursion) that calls the real getRoleCredentials/ssoLogin functions against an isolated HOME with a non-recursive credential_process marker in place of cone, plus a positive control proving the harness can detect an invocation at all. Run against real AWS CLI v1 and v2: the marker is never invoked by either exec call site. The hypothesis does not reproduce as stated — see PR body and report for the mechanism (both subcommands use SSO bearer-token auth, not the standard SigV4 credential provider chain, so credential_process is never consulted for them). No behavior change to cone. Test-only. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
cone installs itself as an AWS `credential_process` and then shells out to the AWS CLI for `sso get-role-credentials` and `sso login`. Neither spawn set `cmd.Env`, so the child `aws` inherited `AWS_PROFILE` and `HOME` and could resolve a cone-managed profile — whose `credential_process` re-invoked cone, which shelled out to `aws` again, unbounded. A customer killed the loop at 685 processes. `aws sso get-role-credentials` authenticates with the SSO bearer token passed on the command line, so it needs nothing out of `~/.aws`: its child environment now drops `AWS_PROFILE`/`AWS_DEFAULT_PROFILE` and pins `AWS_CONFIG_FILE` and `AWS_SHARED_CREDENTIALS_FILE` at os.DevNull, leaving no profile — named or `[default]` — that could re-enter cone. `aws sso login --sso-session cone-sso` still needs the `[sso-session]` block out of the real config, so only the profile selectors are dropped there. Reproduced end to end against AWS CLI 2.35.24 (the reported version): with an inherited environment the credential_process re-enters itself until the harness depth cap stops it; with the fixed child environment it is never entered. An earlier version of the gated harness reported this refuted. That was an artifact of the environment it ran in: botocore resolves the web-identity provider before the custom-process provider, so an ambient `AWS_ROLE_ARN`/`AWS_WEB_IDENTITY_TOKEN_FILE` pair satisfied the lookup and credential_process was never consulted. The harness now clears the full set. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
Independent verification on macOS + AWS CLI 2.36.6Re-ran this on the reported platform, against a different AWS CLI version than the one in the PR body, and mutation-tested the regression coverage. Everything the PR listed as unverified is now closed except the live-tenant The fix's own regression risk is disprovenThis was the important one — the PR argued from the call's flags, rather than observing, that a legitimate vend still works with
and the credentials vended by the second run then authenticated against The harness reproduces identically on macOS
Worth noting what the third case actually proves: Also relevant to the earlier false negative: this machine exports no The regression tests actually gate the fixA test asserting a child "cannot recurse" passes trivially if it is wired up wrong, so I mutated the fix out. Note that simply reverting
The second one is the useful result: the Housekeeping
One thing I'd still flag
Still open
|
Fixes the
credential_processrecursion reported in IGA-3789.The bug
cone aws setupwritescredential_process = cone aws credentials "<profile>"into~/.aws/config.cone aws credentialsthen shells out to the AWS CLI (cmd/cone/aws.go) at two sites —sso get-role-credentialsandsso login— and neither setcmd.Env. The childawsinheritedAWS_PROFILEandHOME, resolved a cone-managed profile, and ran that profile'scredential_process, which re-invoked cone, which shelled out toawsagain. Unbounded. The reporter killed it at 685 processes.The recursion needs the child to be able to select a cone-managed profile, which is why it does not happen for everyone:
AWS_PROFILEexported (inherited by the child)[default]aws --profile Xonly, nothing exportedThat last row is why "running the same
aws sso get-role-credentialsby hand works fine" was in the report.The fix
aws sso get-role-credentialsauthenticates with the SSO bearer token passed on the command line — it needs nothing from~/.aws. Its child environment now dropsAWS_PROFILE/AWS_DEFAULT_PROFILEand pinsAWS_CONFIG_FILEandAWS_SHARED_CREDENTIALS_FILEatos.DevNull, so there is no profile left — named or[default]— that could re-enter cone.aws sso login --sso-session cone-ssostill has to read the[sso-session cone-sso]block out of the real config, so only the profile selectors are dropped there. It is not itself a recursion vector, but an inheritedAWS_PROFILEhas no business reaching it either.No new dependency; the AWS SDK migration suggested on the ticket is a larger change and is not attempted here.
Verification
Reproduced and fixed on two platforms, both making real calls to the public AWS SSO OIDC endpoint. Counts are identical on Linux/aarch64 with AWS CLI 2.35.24 (the version on the ticket) and on macOS/arm64 with AWS CLI 2.36.6 (the reported platform):
The live
UnauthorizedExceptionis what proves the call reached AWS rather than failing locally; the test asserts on it, so a run that never got that far cannot pass. ThessoLogincase reachesRegisterClientand fails there on the placeholder start URL — getting that far confirms the[sso-session cone-sso]block is still readable with the profile selectors dropped.The
os.DevNullpinning does not break a legitimate credential vend. Tested against a real AWS Identity Center account and permission set with a valid cached SSO token:aws sso get-role-credentialsrun under exactly the child environment the fix constructs returned the sameaccessKeyId/secretAccessKey/sessionToken/expirationset as a control run under a normal environment, and those credentials then authenticated againststs get-caller-identity.The regression tests were mutation-checked, since a test asserting a child "cannot recurse" passes trivially if it is wired up wrong. Removing both
cmd.Envassignments fails all four assertions; removing only theos.DevNulloverrides and keeping the profile stripping fails exactly the two config-file assertions — so the[default]-profile vector has its own coverage.go test ./...and-raceare green, andgolangci-lintreports the same 12 issues as the pre-PR baseline — this PR adds none.Tests
cmd/cone/aws_child_env_test.go— runs in normal CI. Installs a fakeawsonPATHthat dumps the environment it was handed, then asserts the child ofgetRoleCredentialscarries no profile selectors and has both config paths pinned, thatssoLoginkeeps a readable config, and that an override cannot be shadowed by an inherited value.cmd/cone/aws_recursion_repro_test.go— the end-to-end harness above, behind//go:build reprocredrecursion, excluded from default builds and CI because it needs a real AWS CLI and live network. Run withgo test -tags reprocredrecursion -run TestIGA3789 -v ./cmd/cone/.... Its recursion is bounded by a depth counter in the child environment and a per-call context timeout, andHOMEis redirected to a temp dir.One trap worth knowing if you touch that harness: botocore resolves the web-identity provider ahead of the custom-process provider, so on any host exporting
AWS_ROLE_ARN+AWS_WEB_IDENTITY_TOKEN_FILE(Kubernetes IRSA, many CI runners) credential resolution succeeds beforecredential_processis ever consulted, and the harness records zero invocations no matter how recursive the configuration is.scrubAWSEnvclears the whole provider set for this reason; weakening it turns the test into a silent false negative. The test fails with a message pointing at that scrub if the count comes back zero.No CI path lints the gated file.
golangci-lint run --build-tags reprocredrecursionflags three issues in it (gosecG101 on the shell-script constant, G306 on the0o700script write, oneerrcheck) — all artifacts of it being a test harness, but it will keep drifting unlinted.Known limitation
Pinning
AWS_CONFIG_FILEatos.DevNulldiscards more than profiles:ca_bundle,proxy,endpoint_url,use_fips_endpointand similar settings in~/.aws/configalso stop reaching the childaws. The environment-variable forms (AWS_CA_BUNDLE,HTTPS_PROXY, …) still pass through, sinceawsChildEnvonly strips the two profile keys, so the exposure is config-file-only. A user behind a TLS-inspecting proxy who setsca_bundlein~/.aws/configrather than in the environment would regress. If that turns out to matter, the fix is to write a minimal temp config carrying those settings instead of using/dev/null.Not verified
cone aws setupend to end against a live tenant: no cone-managed profile was installed on either test machine, so the fullsetup→AWS_PROFILE=<cone-profile>→aws sts get-caller-identitypath — the exact customer trigger — has not been walked. The credential-vend half of it is covered by the real-account test above.