-
Notifications
You must be signed in to change notification settings - Fork 396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
App Submission: AlbyHub v1.10.1 #1409
Conversation
what is the warning with the container user? The app needs to be able to write to the data directory. for this root is needed, isn't it? |
Have been looking forward to this one @bumi! I will review and test tomorrow.
Ideally, we want containers to run as non-root as part of app sandboxing measures, but it is not a strict requirement right now which is why the github actions linter is showing a severity indicator of "info" here. If the
On app install, umbrelOS will create any host bind mounts that are committed to this directory (in this case |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested and this is working really well @bumi. Fantastic work by the whole alby team 🐝
I've left some comments/suggestions below. After they are addressed we can go live.
albyhub/docker-compose.yml
Outdated
server: | ||
image: ghcr.io/getalby/hub:v1.6.0@sha256:45ec367860a8f42c6f66317dd5889d8e2af8b0df1e0bc59b33e67c08770e4e17 | ||
volumes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this container can run as a non-root user (which should be possible based on this Dockerfile https://github.com/getAlby/hub/blob/master/Dockerfile) then we can change this to:
server: | |
image: ghcr.io/getalby/hub:v1.6.0@sha256:45ec367860a8f42c6f66317dd5889d8e2af8b0df1e0bc59b33e67c08770e4e17 | |
volumes: | |
server: | |
image: ghcr.io/getalby/hub:v1.6.0@sha256:45ec367860a8f42c6f66317dd5889d8e2af8b0df1e0bc59b33e67c08770e4e17 | |
user: "1000:1000" | |
volumes: |
When the app is installed, umbrelOS creates the data directory that's commited to this repo with 1000:1000 ownership/permissions, so a getalby/hub
container running as user 1000 will have write permissions to ${APP_DATA_DIR}/data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My docker and docker compose knowledge is limited. We do not want to change the image now as it is used in various places and I would be worried to break other deployments.
The app only writes to the configured volume. (log files and SQLite DB files)
If we configure this user here what does that exactly mean? can you point me to some docs?
It also binds to ports > 1024 - 8080 for the webapp and the lightning port 9375 (if the embedded LDK node is used and not the default Umbrel LND node) - so no root is needed for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries. The user
key in a docker-compose.yml file allows you to run the container as a certain user WITHOUT having to change the underlying image.
Setting user
in the docker-compose.yml overrides the user used to run the container process. If you don't set a user
in the docker-compose.yml file then Docker uses whatever has been defined in the Dockerfile with the USER
instruction. In the case of your Dockerfile, the lack of USER
instruction means that by default it runs as root.
As long as the container can write to it's log and db files as user 1000:1000 then we should be good. I can give this a test for you.
Docs:
https://docs.docker.com/reference/dockerfile/#user
https://docs.docker.com/reference/compose-file/services/#user
Hey @bumi, just pinging you again in case you missed the above review from last week. |
@bumi - I forgot to ask: with the addition of Alby Hub should we be removing Nostr Wallet Connect from the app store? If so, what I can do is add |
The cookie secret is used to encrypt/sign the cookies of the web app Co-authored-by: Nathan Fretz <[email protected]>
bitcoin ftw! Co-authored-by: Nathan Fretz <[email protected]>
Co-authored-by: Nathan Fretz <[email protected]>
Co-authored-by: Nathan Fretz <[email protected]>
@nmfretz thanks a lot for the feedback and improvements.
I am not sure what's best here. We are not actively working on it anymore and for us Alby Hub is the successor of that old NWC app. We recommend people to update to Alby Hub. So to me |
fyi. we have one patch release coming this week which I would like to have in this release. |
If you'd like to keep NWC available for new users still then perhaps for now we can just add a note at the top of NWC's app description saying that it is recommended to use the successor app Alby Hub instead. Although I imagine that since you're not working on NWC anymore, there will come a time when you'd want the app retired (e.g., it's not receiving security updates, etc).
Great, thanks for letting me know. Ping me when it's ready and I'll give it a final test before we go live! |
Hey @bumi, I see that hub is now on v1.10.1. Would you like to go live with that version? |
when used with LDK it could take a bit to stop and we don't want to risk a kill
@nmfretz yes, v1.10.1 is it! |
Excellent, thanks again for all your work bringing Alby Hub to the Umbrel app store @bumi! I have tested the new image with user 1000:1000 and everything is working well (e.g., app install, sending/receiving, connection to alby go, connection to apps, etc). If at some point it becomes apparent that there is a permissions issue for something (very unlikely) then the fix is very simple in this direction (i.e., going from user 1000 back to root). All we would need to do is remove this line from the compose file: I don't have permissions to push a commit to |
Co-authored-by: Nathan Fretz <[email protected]>
Perfect, thanks @bumi. Going live 🐝 |
Amazing. Thanks for publishing Alby Hub, @nmfretz. Would appreciate any hints. |
I don't think people can update from the community store actually. it's a different app |
AlbyHub is Alby's new self-custodial wallet. It is build around the NWC protocol to allow users connect their Umbrel lightning nodes to the growing number applications supporting NWC for payments. Most known are the various nostr apps to enable zapping.
Alby Hub is also the continuation of the Nostr Wallet Connect application with is already in the store.
Links:
App Submission
App name
Alby Hub
256x256 SVG icon
https://svgur.com/s/19oF
Gallery images
PNG:
JPG:
I have tested my app on: