Repository navigation
feat: offer to run aws sso login when SSO credentials expire - #919
Conversation
|
bugbot run |
e06e5a5 to
5e39d8f
Compare
5e39d8f to
45d5b06
Compare
4e8a268 to
75aa295
Compare
|
|
||
| // nextLine reads up to and including the next newline. It reads a file one | ||
| // byte at a time, so the lines after it stay for the next prompt. | ||
| func nextLine(in io.Reader) (string, error) { |
There was a problem hiding this comment.
Resolves cursor's comment from original PR: #875 (comment)
75aa295 to
c75a222
Compare
| "no AWS credentials for profile %q. Run `aws sso login --profile %s`, then run this command again", | ||
| sso, sso, | ||
| ) | ||
| case useful && named: |
There was a problem hiding this comment.
The credential error can print the AWS secret key.
noCredentialsError puts the SDK cause in the message with %v. If a credential_process helper prints output that is not pure JSON, the SDK copies the raw output into that cause (processcreds/provider.go:190: "parse failed of process output: %s"). The output can contain SecretAccessKey.
To reproduce, use a helper that prints one warning line before valid JSON:
#!/bin/sh
echo "warning: token cache refreshed"
echo '{"Version":1,"AccessKeyId":"AKIAFAKEEXAMPLE","SecretAccessKey":"FAKE-SECRET-DO-NOT-USE"}'
ldcli aws-devops-agent status --profile leak then prints:
unable to get AWS credentials for profile "leak": failed to refresh cached credentials, process provider error: parse failed of process output: warning: token cache refreshed
{"Version":1,"AccessKeyId":"AKIAFAKEEXAMPLE","SecretAccessKey":"FAKE-SECRET-DO-NOT-USE"}
, error: invalid character 'w' looking for beginning of value
main has the same leak, because noCredsErr.Error() joins the cause. Suggestion: do not print the cause, and add a test with this helper. TestCredentialErrorHidesTheSDKCause covers only the IMDS cause.
There was a problem hiding this comment.
Thanks Francisco! Commits cde1655c and 0641cae2 fixes this.
The credential error no longer prints the SDK cause. It names the command that shows the cause:
unable to get AWS credentials for profile "leak". Run `aws sts get-caller-identity --profile leak` to see the cause
TestCredentialErrorHidesTheProcessOutput runs thte helper through the real SDK. It fails without the fix. Three other tests now make sure that the cause is not in the message. The code can still get the cause with errors.Unwrap. The SSO advice did not change.
This reply is from Claude.
| assert.Equal(t, 1, f.calls) | ||
| } | ||
|
|
||
| // JSON runs never ask, like the setup plan. |
There was a problem hiding this comment.
Nit: the comment says "JSON runs never ask, like the setup plan", but the test below asserts that a JSON run in a terminal does ask. Suggest: "A JSON run in a terminal asks. The prompt goes to stderr, so stdout keeps only the JSON."
There was a problem hiding this comment.
Thanks! Done in cde1655c
|
A few things from reading through this PR — overall the design is clean and the test coverage is excellent, especially the real-SDK tests. These are mostly questions / smaller cleanups. 1.
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1a571e3. Configure here.
1a571e3 to
cde1655
Compare
|
@Andrewjeska, thanks! Commits
This reply is from Claude. |
If the SSO token of the profile is expired or not in the cache, setup and status ask to run aws sso login --profile <name>. Other SSO errors, for example a removed role assignment, do not get the question. If you answer yes, the command runs the login and builds the AWS clients again. If the token is still not valid, the error says so and shows the cause. They ask only in a terminal, also with --output json, because the prompt goes to stderr. All prompts read from one stdin through sharedStdin. nextLine reads a file one byte at a time, so a pasted answer stays for the next prompt. clients.go reads the yesFlag constant. The --yes flag comes in a later PR. Until then, yesSet returns false. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
If a credential_process prints output that is not valid JSON, the SDK puts all of the output in the error. The output can contain the secret key, so the credential error now shows a fixed message for this case. The prompts need a terminal on stderr too. A run with stderr sent to a file does not wait for an answer that the user cannot see. canPrompt is a plain function again. A test covers --yes in yesSet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The credential error no longer shows the SDK cause, because a cause can contain the output of a credential_process with the secret key. The error now names the command that shows the cause: aws sts get-caller-identity --profile <name>. The SSO login runs only in a terminal, because a person must finish the sign-in in a browser. In a terminal, --yes runs the login without a question. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
canPrompt now needs a terminal on stderr. The token prompt went to stdout, so a run with stderr sent to a file skipped a prompt that the user could see. The prompt now goes to stderr, like the SSO prompt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
If the SSO token is still not valid after aws sso login, the error now names only the profile. It still wraps the credential error, so errors.Is matches ErrNoCredentials. Two tests that checked only their own fake are gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
d019fca to
c6ee83b
Compare

Requirements
Related issues
None.
Describe the solution you've provided
setupandstatusstopped with an error. You had to runaws sso loginyourself, then run the command again.setupandstatusask to runaws sso login --profile <name>. If you answer yes, the command runs it and builds the AWS clients again. It asks only once.SSOTokenInvalidfinds it. That coverssso_sessionprofiles, whichaws configure ssowrites, and legacysso_start_urlprofiles. A test uses a realsso_sessionconfig. Other errors show their cause. If the token is still not valid after the login, the error says so and shows the cause.--output json, because the prompt goes to stderr and stdout keeps only the JSON. If theawsbinary is not on the PATH, it does not ask.sharedStdin.nextLinereads a file one byte at a time, so the next lines stay for the next prompt.readSecretstill gets the real file, so the token prompt stays hidden.clients.goreads theyesFlagconstant. A later PR adds the--yesflag. Until then,yesSetreturns false.Describe alternatives you've considered
None.
Additional context
Testing:
go test ./...andgolangci-lintpass on this branch.🤖 Generated with Claude Code
Note
Overview
setupandstatusnow go throughloadClients, which detects expired or missing SSO tokens and can runaws sso login --profile <name>after a stderr prompt (or automatically whensetuppasses--yes). Prompts run only when stdin and stderr are terminals, the profile is SSO-backed,SSOTokenInvalidmatches, and theawsCLI is on PATH; client creation is retried once, with a clearer error if the token is still invalid after login.Prompt and output behavior is tightened so multiple interactive steps share one stdin (
sharedStdin/nextLine),canPromptrequires a TTY on stderr as well as stdin, and the LaunchDarkly service-token prompt duringsetupwrites to stderr so JSON output stays clean.Credential error messages no longer embed SDK causes that might leak secrets (e.g.
credential_processoutput); non-SSO failures now point users ataws sts get-caller-identityinstead. README documents the SSO login offer.Reviewed by Cursor Bugbot for commit d019fca. Bugbot is set up for automated code reviews on this repo. Configure here.