granted icon indicating copy to clipboard operation
granted copied to clipboard

--exec whitespace handling and limitations

Open sosheskaz opened this issue 2 years ago • 8 comments

--exec has some pretty heavy limitations.

Looking at the code, it seems that it parses the string as an entire arg, then splits using as the delimiter and runs it as a command.

This creates a few limitations:

  • Commands with whitespace cannot be escaped trivially.
  • Multiple commands cannot be run trivially.
  • Commands cannot use input from each other trivially.

Common workarounds, like bash -c ..., are hamstrung by the whitespace limitation.

There are a few workarounds, like writing the output to a script in a tmpfile and running that, but it is pretty limited.


Solutioning:

  1. Ordinarily, I'd like to make exec a subcommand and delineate its argv via --, ex: assume exec -- my command to run 'now supporting quotes'. This would then propagate to the exec invocation, propagating the relevant argv directly from our own argv instead of attempting to parse it. However, it seems like assume isn't really cut out for subcommands. I'm not terribly familiar with the implementation, so if there is a way, that is what I would suggest.
  2. A separate assume-exec entrypoint which implements the described capability.
  3. Instead of passing the given text to exec, pass it to a shell instead, which can do quote parsing (I don't think this is a great option).
  4. Implement quote parsing (I don't think this is a great option).

Interested in particular if maintainers have an opinion on what approach would be best.

sosheskaz avatar Aug 16 '22 16:08 sosheskaz

Also: --exec does not propagate environment to the resulting command, which creates issues for any binaries which depend on environment to function, such as asdf.

sosheskaz avatar Aug 16 '22 17:08 sosheskaz

Thanks for opening this issue @sosheskaz. Agreed there is a long way to go in improving the exec functionality.

Can I ask about your use case for --exec? Are you using the Granted CLI in shell scripts?

In terms of solutions I agree that option 1 (assume exec -- my command) is the best. This is similar to how other CLIs including aws-vault work so I think this will be most familiar to folks. This is potentially a breaking change, as if anyone happens to have an AWS profile named exec the CLI will no longer work as they expect. I'll circulate this issue with the community in our community Slack for feedback.

In terms of the specific technical implementation, making assume exec -- my command work should definitely be possible. I expect that we'll need to add a manual check to see whether the first argument is exec rather than a regular profile name, and if so, run the exec command rather than assuming a role.

chrnorm avatar Aug 17 '22 10:08 chrnorm

@sosheskaz I've opened a discussion on this for feedback: #219

chrnorm avatar Aug 17 '22 11:08 chrnorm

What I use exec for now:

  • The single most important usage of exec that I have is wrapping aws eks get-token in kubeconfigs.
  • I live in a many-account environment, and sometimes run one-off commands against some given credentials, or want to set up an alias which automatically runs the command with a given profile. Ex: Pulling data from one account, and using it in a call to another account.

Where the limitations are affecting me:

  • I have a custom program which loads a config file and performs some queries, some of them against S3. I want to set up my terminal environment such that it always uses appropriate credentials. The "nicest" way to do this would be aliasing it to assume --exec. However, because --exec does not propagate environment appropriately, $HOME is not set, and so it fails. I believe that this may be a separate issue.
  • A previous solution I used, Rapture, implements this functionality very directly via its wrap subcommand. Insofar as I am using granted as a replacement, I have lost functionality, which brings assorted challenges.
    • The other solution I've used is "raw" AWS Profiles. "Raw" AWS Profiles also enable this, via AWS_PROFILE=... my-command. So this is also missing functionality compared to the AWS-native solution.
    • I realize that's a bit obtuse — I am still largely exploring granted and what is needed to make it fully work in my environment, but to put it another way — missing functionality which is implemented by other solutions hampers my adoption, because I need to craft relatively elaborate workarounds.
  • I have a personal script which transforms kubeconfigs to use assumego as described in the recipes. However, what is basically has to do is: grab the old "aws" command -> ' '.join(args) -> pass that as an arg to exec. This means that I have to remove any whitespace arg separation, and impose my own on it, before passing it to assumego. This feels like lossfully compressing an image down, then trying to "zoom and enhance" later. I haven't encountered any issues thus far, but this feels very fragile; it all assumes that no arg I encounter will ever contain whitespace.
  • Within an organization, I may send someone snippets to run on their machine, either to instruct someone how to do something, or for troubleshooting purposes. Passing this via an exec-like flag would make that quicker and easier, especially with standard practices built on top of granted.
  • I sometimes want to use data from one account as an input to another account - i.e. assume prof2 exec aws ec2 accept-peering-connection --peering-connection-id $(assume prof1 exec aws ec2 describe-peering-connections ...). This is a fake example.
  • I would like the ability to construct shell scripts that swap between accounts. While this could be done with normal assume calls, or other workarounds, those are more error-prone and harder to read.

Basically, I think the problem is that I've gotten used to having this feature, and now I depend on it.

sosheskaz avatar Aug 17 '22 14:08 sosheskaz

In terms of the implementation — I am likely wrong about this, but the only positional arg that granted cares about is the first one, correct? If so, could we require that the first positional argument always be a profile, and if there is a second, then it is interpreted as a subcommand?

i.e. assume my-profile [flags] exec cmd argv. This should be backwards-compatible.

That said, if we're talking about changing to using subcommands, then one may argue that -c should be a subcommand instead of a flag, which could have some deeper implications... Honestly, I think that would be a good change, but it would also be painful for backwards-compatibility.

sosheskaz avatar Aug 17 '22 14:08 sosheskaz

@sosheskaz thank you for sharing your context here around how you're using exec - it's extremely helpful.

Good point on the positional arguments. In hindsight my example command assume exec without a specific profile specified doesn't really make sense - a user should always be specifying the profile they want to run exec in.

For example, this wouldn't make much sense

assume my-profile
assume exec -- aws s3 ls # which profile does `assume exec` use here? It should probably be 'my-profile' but the `assume exec` is then redundant (see below)

Because you could just run

assume my-profile
aws s3 ls

to achieve the same effect.

As you've mentioned this won't be a breaking change. As I've flagged in #219 if we proceed with assume my-profile exec I would like to deprecate and remove --exec in future, but for now we can introduce the subcommand and support both.

Interesting point around -c as a subcommand. I do like that assume my-profile c is one less keystroke, avoiding the -.

chrnorm avatar Aug 19 '22 10:08 chrnorm

I've raised #222 to track the environment variable issue too.

chrnorm avatar Aug 19 '22 10:08 chrnorm

Re -c as a subcommand, we currently have some customised flag parsing which means you can use the flags with assume in any order before or after the positional profile argument.

e.g assume -c myprofile or assume my-profile -c

JoshuaWilkes avatar Aug 21 '22 23:08 JoshuaWilkes

Hi, is it expected that something like FORCE_NO_ALIAS=true assume myrole --exec "env | grep 'aws' -i" doesn't quite work?

The error I got was env: |: No such file or directory

alqh avatar May 04 '23 21:05 alqh

With v0.15.0, Users can leverage the --exec flag with a double dash (--) prefix to execute more complex commands that involve quotations. Use the following syntax: assume <profile_name> --exec -- cmd arg... to run your command. We appreciate your feedback 😄 https://github.com/common-fate/granted/pull/470

shwethaumashanker avatar Sep 14 '23 13:09 shwethaumashanker