orlandos-nl / Citadel

SSH Client & Server in Swift
MIT License
181 stars 32 forks source link

Add `inShell` parameter to for SSH command execution #36

Closed Marcocanc closed 1 year ago

Marcocanc commented 1 year ago

This PR adds the option to execute commands in a shell, by launching a shell channel request and sending the command into the channel once the request succeeded.

Joannis commented 1 year ago

Also nice to see a PR of yours! We're using Cilicon ourselves, so thank you for that :)

Marcocanc commented 1 year ago

Thanks for the review, this was just a super rough PoC with a bunch of copy-paste. Will clean it up tomorrow!

We're using Cilicon ourselves

Great to hear! Citadel is making its way into Cilicon in an exciting 2.0 update soon 😊

Marcocanc commented 1 year ago

Looking good! Let's see if we can clean up these final bits. My main remaining question is if we should allow people to choose a shell (e.g. /bin/sh, /bin/bash, /bin/zsh)

I don't think SSH allows picking a shell. If I understood it correctly it will always uses the default shell of the logged in user. In the spec it says:

This message will request that the user's default shell (typically defined in /etc/passwd in UNIX systems) be started at the other end.

Joannis commented 1 year ago

@Marcocanc since this executes a command as the shell launches, I believe we can do something like /bin/sh -c, but I don't know if it's any added value

Marcocanc commented 1 year ago

@Marcocanc since this executes a command as the shell launches, I believe we can do something like /bin/sh -c, but I don't know if it's any added value

Yeah I don't see much value in that tbh. After all users can just wrap their command that way themselves.

Joannis commented 1 year ago

Fair! Then I'm pretty happy with this

Marcocanc commented 1 year ago

@Joannis Cilicon 2.0 (which includes Citadel) is now released 😊 https://github.com/traderepublic/Cilicon

Joannis commented 1 year ago

Cool! Thanks for the heads up @Marcocanc