Added handling of —permit-arguments option

[QuentinPerez]

Hi,
I do this PR to handle parameters with the permit-arguments flag.

Example:

$> gotty --permit-arguments vim

You can pass argument like this : http://server.exmple.com:8080/?params=file1&params=file2

[jcoste]

Hello,
I was working on a pull request with the same need. It would be nice to be able to pass multiple parameters instead of one like this : http://server.exmple.com:8080/?Params=options&Params=filename

In the code, you juste have to change :
params := query.Query().Get("Params")
by :
params := query.Query()["Params"]

and :
cmd := exec.Command(app.command[0], argv)
by :
cmd := exec.Command(app.command[0], argv...)

Hope this help.

[QuentinPerez]

Thanks @jcoste

I added your code 😊

[moul]

Fix #33

[moul]

What do you think about renaming Params to params which is more _http_ish ?

[moul]

👍

[aimxhaisse]

👌

[cerisier]

👍

[yudai]

Hi, thank you for the PR and sorry for the delay to reply.

I'm still not sure wether this approach is safe or not. If the given file to VIM is "/proc" or something, the command might leak secret information. To avoid security risks by restricting possible parameters, we may implement regular expression filters or white lists, but that makes gotty really complicated. So I'm feeling like when you need dynamic configuration of the gotty command, writing a custom wrapper script/application might be a better way like https://github.com/Arno0x/TermGate.

Could you present some actual use cases for which you need this feature?

[moul]

This feature mimics the tty.js shellArgs argument which can be a list of predefined arguments or a hook function: https://github.com/chjj/tty.js/blob/master/lib/tty.js#L443-L445

---

Thanks to this dynamic shellArgs, we can host this service to give access to customer ttys with urls like:

• https://tty.scaleway.com/?server_id=0ff1ce0f-f1ce-1ff1-ce0f-f1ce0ff1ce0f&type=serial&auth_token=31337313-3731-1373-1337-313373133731

tty.js executes a custom program we wrote which parses (and check) arguments

---

I don't understand how using TermGate would be more secure, is TermGate blacklisting /proc argument when the base program is vim ? :)

TermGate:

• Cons:
  • removes the easiness of configuration we have in GoTTY
  • adds dependencies on PHP + a web server
  • manual webserver configuration
  • requires sudo
  • sets -w by default
  • forks 1 GoTTY instance per connection
• Pros:
  • it handles arguments (as this PR)

---

In my opinion, giving access to vim/emacs/bash/tmux without argument is already a huge security risk

The GoTTY user needs to take care of the program he gives access to, and if he wants to allow parameters passing, he also needs to think about security issues it involves

A message in the README.md file about why is it risky to give access to the filesystem or shell execution would be great though

[yudai]

@moul TermGate is just an example that shows you can implement arguments in a wrapper. If you want, you can implement some logic to filter arguments in your wrapper.

[jcoste]

Like @moul I find this PR very useful. The Gotty user is responsible of the program he gives acces to. And @QuentinPerez puts a flag to enable this mechanism! It's not enabled by default.

I don't want to develop a wrapper around Gotty like TermGate if this little PR can do the job.

[yudai]

Thank you for the comments. Ok, I'm now thinking this option would be useful for many use cases.
However, I'm feeling like writing a lot of parameter= is a little bit bothering. Is there any better way to provide arguments in the URL?

[QuentinPerez]

Hi @yudai,

I can propose you other ways, what do you think about:

?arg[]=AAA&arg[]=BBB&arg[]=CCC%20CCC
?args=[“AAA”,”BBB”,”CCC%20CCC”]

Or if you have another way, feel free to propose 😄

[yudai]

hi @QuentinPerez, I checked some specs to find a standard way to send arrays in URL arguments, however it looks there is no formal way for it.
I think arg=AAA&arg=BBB&... (without []) is the best way. The name parameter sounds inconsistent with the parameter name permit-arguments.

[QuentinPerez]

I updated the PR 😉

[yudai]

Could you rebase to the master? There are some conflicts.

[QuentinPerez]

Done

[yudai]

Thanks! Merged!