search

awslabs / aws-shell

An integrated shell for working with the AWS CLI.
Apache License 2.0
7.15k stars 770 forks source link

Pop explicit credentials when switching profiles #178

Open joguSD opened 6 years ago

joguSD commented 6 years ago

The CLI will prefer explicit AWS credentials in the environment variables thus ignoring the profile set by the shell. If you manually invoke a profile using the shell this will ensure that any explicit credentials are removed from the shell's environment.

Fixes https://github.com/awslabs/aws-shell/issues/166

joguSD commented 6 years ago

@jamesls This will still allow us to pull creds from the env vars if no explicit profile is set. The code in the PR is only executed if -p is passed or .profile <profile> is invoked. I think having implicit credentials override the explicitly set profile is counter intuitive.

I had thought of adding a --profile <profile> to the command executed if a profile is present, but I think it's fragile and could potentially have a lot edge cases. I think the shell should avoid manipulating the command executed as much as possible.

kyleknap commented 6 years ago

@joguSD I also don't think popping the environment variables is the right thing to do either. I actually think we should not be setting the AWS_DEFAULT_PROFILE, which is what the current implementation does as well. In general environment variables are the user's space. We should not be manipulating them silently as it can lead to unexpected results.

The best option in general would be to set the profile on the session. However, we do not have access to that as the command is ran in a subprocess. So for now the best you probably would be able to do is set --profile when the command is ran. But like you said, there will probably be a decent amount of edge cases with it.

joguSD commented 6 years ago

@kyleknap I can see where you're coming from about the environment variables thing, but I'm not sure if it's entirely relevant in the current state of variable support of the shell. Upon entering the aws-shell we copy the parent process environment and have an internal copy that we pass to sub-processes. Once this copy is made it can no longer be manipulated by the user within the shell. We do not support export or source currently. If this was a CLI feature I would totally agree, but I'm not sure if the same logic applies to the shell's internal environment variables. This is a shell feature. You are setting the profile for the shell, and if you were to do that in bash or zsh you would simply export the profile. This is an alias for that behavior in our shell.

I really don't think the behavior of .profile should be to append --profile <profile> to cli commands. This would require us to check every cli command before it's executed for the presence of a profile and only in the case that it's not passed should it be appended. There may be other situations where simply adding cmd + ' --profile {profile}' will not be sufficient. I also don't think the command the user types and the command that is executed should ever differ. This will complicate the command history and the .edit functionality.