Skip to content
This repository has been archived by the owner on Jun 23, 2023. It is now read-only.

Several cleanups #117

Closed
wants to merge 3 commits into from
Closed

Several cleanups #117

wants to merge 3 commits into from

Conversation

nevets963
Copy link
Contributor

@nevets963 nevets963 commented Jan 20, 2022

Closes #116

I went through the codebase and saw a few issues which I suspect are things that have crepped in over time.

To summarise:

  • Remove version number from /ping endpoint (potential security concern)
  • Fix env pollution when using .exec in bash service, plus tests
  • Change default port from 3005 to 3006 to match what is in docker-compose.yaml within umbrel repo
  • Fix typo in JWT paths to match docker-compose.yaml again

I noticed within docker-compose.yaml (umbrel repo) that the Docker unix socket and binary get mounted in however they are not used at all: https://github.com/getumbrel/umbrel/blob/master/docker-compose.yml#L95
I would suggest we remove that from the yaml because if the manager gets compromised then every single container can get compromised.

@AaronDewes
Copy link
Contributor

services/bash isn't used anyway AFAIK. I already cleaned up a lot of this https://github.com/runcitadel/manager and could completely drop that.

@nevets963 nevets963 closed this Jun 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Version is publicly exposed
2 participants