charmbracelet / vhs

Your CLI home video recorder 📼
MIT License
14.98k stars 247 forks source link

ttyd should only listen on localhost #226

Closed rcw5 closed 1 year ago

rcw5 commented 1 year ago

The ttyd that VHS spins up listens on all interfaces. I've had a couple of occasions where ttyd has remained running after I killed VHS, which leaves an unauthenticated remote terminal open to my local network.

Can ttyd be configured with the --interface flag so it only listens on 127.0.0.1?

I'm happy to submit a small PR for this feature (modifying tty.go)

maaslalani commented 1 year ago

I believe this should be fine, since ttyd only needs to communicate with VHS locally. I do want to double check that the SSH server for VHS would still work to submit tapes over SSH but I think that it should be fine.

rcw5 commented 1 year ago

Have submitted a PR. I wasn't able to test vhs serve as I couldn't get it working (not sure if something is missing from the documentation?) but was able to create a gif from a tape just fine

maaslalani commented 1 year ago
image

The default port may be the issue:

vhs serve

Then,

ssh localhost -p 1976 < demo.tape > demo.gif

Although, testing it like this (on localhost) will probably work regardless since it's still on localhost where the ttyd instance is still listening.

rcw5 commented 1 year ago

Got it - the issue happened because I mounted my repo inside the Docker container, so there was a file called /vhs/vhs, which is also where VHS_KEY_PATH points (https://github.com/charmbracelet/vhs/blob/main/Dockerfile#L54). My /vhs/vhs was not a key, hence the error :-).

I was able to test vhs serve in Docker using container networking, will update the PR comments.

taigrr commented 1 year ago

Given the severity of this issue, could I request charm publish a new version hotfix release to incorporate this change please?

bashbunni commented 1 year ago

Will bring it up to the team and see how soon we can do a release 👀

taigrr commented 1 year ago

Hooray! this is fixed with latest release. Now I can install it :)

maaslalani commented 1 year ago

Awesome! Thanks for the patience @taigrr!