owenthereal / upterm

Instant Terminal Sharing
https://upterm.dev
Apache License 2.0
861 stars 56 forks source link

Revert "Shrink uptermd Docker image by 5 MB, 20% (#225)" #228

Closed owenthereal closed 9 months ago

owenthereal commented 9 months ago

This reverts commit bb763d6cdd9e5b1da00ffe794c71b3dfe0422828.

@bcspragu I have to revert https://github.com/owenthereal/upterm/pull/225 because the production community server shares the same image and needs sh to compute fly.io node address at boot time: https://github.com/owenthereal/upterm/blob/2e48f7341cf51a291bb626ddfc70ae56da0b49d0/fly.toml#L11-L12. I think we might have to stick with the Alpine image or if you have a suggestion that a smaller image that has a shell in it.

bcspragu commented 9 months ago

Ah, apologies for breaking things! As for adding a shell to SCRATCH, I think that defeats most of the purpose (and some of the size benefits) of it. Some alternatives, from what I think are best to worst:

Not having a shell is also a useful security invariant for some modalities of compromise, gives an attacker less of a toolkit for introspecting things.

owenthereal commented 9 months ago

I think the FLY_* shouldn't be built into uptermd because it's designed to be cloud agnostic. Making a separate Docker image for Fly is the way to go if we want to have a minimum image for self-hosting while keeping a shell for the fly.io deployment.

bcspragu commented 9 months ago

One more idea because I think even Fly users would benefit from the lack of shell: A small, auxiliary flycfg binary that does the "env lookup to uptermd flag" conversion. That way, Fly still has a separate, Fly-specific image, uptermd has no Fly-specific flags, and both images benefit from lack of shell for size + security

owenthereal commented 6 months ago

A small, auxiliary flycfg binary that does the "env lookup to uptermd flag" conversion. That way, Fly still has a separate, Fly-specific image, uptermd has no Fly-specific flags, and both images benefit from lack of shell for size + security

Thanks for the feedback. I created a binary uptermd-fly for this purpose and use Docker's build target for Fly's deployment: https://github.com/owenthereal/upterm/pull/261