crashappsec / chalk

Chalk allows you to follow code from development, through builds and into production.
https://crashoverride.com/
GNU General Public License v3.0
321 stars 11 forks source link

aliasing chalk to docker breaks a bunch of similar docker commands #174

Open indecisivedragon opened 5 months ago

indecisivedragon commented 5 months ago

running chalk with the config:

default_command: "docker"

and then aliasing chalk to docker so that docker is always run via chalk will break multiple docker commands that chalk thinks is a chalk command but doesn't recognize.

For example exec:

liming@system76-pc:~/workspace/chalk$ docker exec -it alpine sh -c "echo a && echo b"
error: This chalk instance has no configured process to exec.
error: At the command line, you can pass --exec-command-name to set the program name (PATH is searched).
error: Add extra directories to search with --exec-search-path.
error: In a config file, set exec.command_name and/or exec.search_path

Or login:

liming@system76-pc:~/workspace/chalk$ docker login
warn:  Code signing not initialized. Run `chalk setup` to fix.
Error: Cannot perform an interactive login from a non TTY device

help kicks the user into the chalk help output instead of the docker help output, which may not be what they're looking for, and version gives the chalk version instead of the docker version

Aliasing is what we recommend our users do when running docker with chalk, so it's likely that someone doing this will run into a whole bunch of unexpected issues when trying to use docker "normally".

viega commented 5 months ago

This isn't a bug; it is why I have long said it's better to name the binary docker than to actually alias it, as we can make sure that if we're called as docker to prioritize docker behavior. That was the way we had assurance that we were standing in for docker, and in those scenarios I believe we always assume the command is for docker (assuming nothing changed here in the past few months).

But given that's now the primary way people want to deploy, this is definitely worth revisiting. The 'try to guess if they used an alias' approach is pretty unsatisfying because it's not possible to get this right in every world.

We can definitely rename exec to wrap or similar; that's the big important one.

For the rest, the easy thing to do is to add a flag we can automatically set up in the 'wrap' component that, when enabled, will always try to pass the command on to docker by default, unless there's an explicit flag. For instance:

For help and version I think, when figure out we are wrapping docker, we can always: 1) Pass through so you get the wrapped output 2) Give a message saying, 'pass --chalk to run Chalk's version while wrapping' or something to that effect.

For login, I would defer to @MyNameIsMeerkat here, could do the same trick, or we could rename the command, which is def what I think we should do w/ 'exec'.

indecisivedragon commented 5 months ago

ran into this because docs recommend aliasing -- will make a ticket to update docs to rename instead of aliasing

miki725 commented 5 months ago

thats what setup chalk github action does:

https://github.com/crashappsec/setup-chalk-action/blob/b09c10e1d7ef2048ca1fa90dab44a7ecc96c9dae/setup.sh#L287-L326

This way even if there is only one docker on PATH, chalk can successfully wrap it and still be able to find original chalkless docker.

viega commented 5 months ago

Yes, but aliasing is easier to explain, so would be good to be able to have an 'assume an alias' mode essentially.

On Mon, Jan 29, 2024 at 1:44 PM Miroslav Shubernetskiy < @.***> wrote:

thats what setup chalk github action does:

https://github.com/crashappsec/setup-chalk-action/blob/b09c10e1d7ef2048ca1fa90dab44a7ecc96c9dae/setup.sh#L287-L326

  • moves original docker to */bin/chalkless/docker so that chalk can find it later

  • copies chalk to temporary file

  • for temporary file loads:

    default_command: "docker" docker_exe: "*/bin/chalkless/docker"

  • moves temporary chalk to bin/docker

This way even if there is only one docker on PATH, chalk can successfully wrap it and still be able to find original chalkless docker.

— Reply to this email directly, view it on GitHub https://github.com/crashappsec/chalk/issues/174#issuecomment-1915346096, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABELGQM7Z2JBQIY24B5XCL3YQ7UXHAVCNFSM6AAAAABCP4S2WGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJVGM2DMMBZGY . You are receiving this because you commented.Message ID: @.***>

MyNameIsMeerkat commented 5 months ago

RE the chalk login command I have no strong preference as to the name of this sub-command at all so happy to rename it to co-login (or any other alternative people prefer more) if that means we don't have to add special guessing/renaming logic to chalk core