Added bitcoind container 'user' parameter

[rstmsn]

Added 'user' parameter to bitcoind docker-compose service, to ensure container is launched as 'umbrel' user. This in turn ensures all files created by the container have the expected user:group (umbrel:umbrel).

This update resolves permission related errors encountered by third party apps (e.g. Electrs), when attempting to access bitcoin-knots .cookie file.

[nmfretz]

Thanks for this @rstmsn! We need to make sure that we don't accidentally break existing user's installations. For example, just adding this change will work for new app installs, but existing users would then have bitcoind running with user 1000 permissions and potentially not be able to access files previously created/overwritten with root permissions. We can do this with a lifecycle hook that chowns the bitcoind data directory. I can add this to the existing pre-start hook.

Before I do this though, @Retropex is the issue with knots bitcoind running as root an accidental change from a recent update? Like a change to the Docker image build that resulted in the default user being root?

[rstmsn]

Thanks @nmfretz. @Retropex may provide more clarity, however my understanding is that the 'bitcoin-knots' app used the 'bitcoin' app as a starting template, which was then customised to run the 'bitcoin-knots' client, as opposed to the 'bitcoin-core' client.
The 'user' parameter for the 'bitcoind' container, in the 'bitcoin-knots' 'docker-compose.yml' file was unintentionally omitted, possibly as far back as the birth of the 'bitcoin-knots' app.

As many other apps in the Umbrel eco system typically run as user '1000' (umbrel), so too should the 'bitcoin-knots' app/containers, to ensure these third party apps have sufficient priviledge to access/read files created by the 'bitcoin-knots' services.

In terms of supporting historic installs, presumably something along the lines of 'chown -R umbrel:umbrel ~/umbrel/app-data/bitcoin-knots' would take care of this, if that could be included in the pre-start hook?

Many thanks for your guidance,
R.

[nmfretz]

Thanks for the details @rstmsn! I just want to clarify the image build characteristics with @Retropex before we make the change.

The 'user' parameter for the 'bitcoind' container, in the 'bitcoin-knots' 'docker-compose.yml' file was unintentionally omitted, possibly as far back as the birth of the 'bitcoin-knots' app.

The 'user' parameter was very likely not included in the original submission because the knots bitcoind image hardcoded user 1000:1000 (similar to what the old lncm/bitciond image did before we migrated to our own Docker file https://github.com/getumbrel/docker-bitcoind). I suspect that there has been a recent change in the knots bitcoind Docker image build because we extensively tested the Bitcoin Knots app compatibility with Electrs (and other apps) when we released the umbrelOS 1.3 with swappable apps: https://x.com/umbrel/status/1861030142321254785

@Retropex is this maybe a new debian-based image build based on https://github.com/getumbrel/docker-bitcoind? I just want to make sure before we push a fix in case other changes are needed.

[Retropex]

Hello guys, Bitcoin Knots effectively seems to run in root, I have not hardcoded any users in the Dockerfile. I have effectively used your template with Bitcoin Core.

[nmfretz]

Hello guys, Bitcoin Knots effectively seems to run in root, I have not hardcoded any users in the Dockerfile. I have effectively used your template with Bitcoin Core.

Excellent, thanks for the confirmation @Retropex! What happened then is when the switch was made to using the debian-based template, user 1000:1000 was no longer defined in the Dockerfile like in the old lncm template.

I will create a life-cycle hook to make sure we reset permissions for impacted users.

[github-actions]

⚠️   Linting finished with 1 warning   ⚠️

Thank you for your submission! This is an automated linter that checks for common issues in pull requests to the Umbrel App Store.

Please review the linting results below and make any necessary changes to your submission.

Linting Results

Severity	File	Description
ℹ️	bitcoin-knots/docker-compose.yml	Mounted file/directory "/bitcoin-knots/torrc" doesn't exist: The volume "${APP_DATA_DIR}/torrc:/etc/tor/torrc:ro" tries to mount the file/directory "/bitcoin-knots/torrc", but it is not present. This can lead to permission errors!
ℹ️	bitcoin-knots/docker-compose.yml	External port mapping "${APP_BITCOIN_KNOTS_P2P_PORT}:${APP_BITCOIN_KNOTS_INTERNAL_P2P_PORT}": Port mappings may be unnecessary for the app to function correctly. Docker's internal DNS resolves container names to IP addresses within the same network. External access to the web interface is handled by the app_proxy container. Port mappings are only needed if external access is required to a port not proxied by the app_proxy, or if an app needs to expose multiple ports for its functionality (e.g., DHCP, DNS, P2P, etc.).
ℹ️	bitcoin-knots/docker-compose.yml	External port mapping "${APP_BITCOIN_KNOTS_RPC_PORT}:${APP_BITCOIN_KNOTS_INTERNAL_RPC_PORT}": Port mappings may be unnecessary for the app to function correctly. Docker's internal DNS resolves container names to IP addresses within the same network. External access to the web interface is handled by the app_proxy container. Port mappings are only needed if external access is required to a port not proxied by the app_proxy, or if an app needs to expose multiple ports for its functionality (e.g., DHCP, DNS, P2P, etc.).
ℹ️	bitcoin-knots/docker-compose.yml	Potentially using unsafe user in service "server": The default container user "root" can lead to security vulnerabilities. If you are using the root user, please try to specify a different user (e.g. "1000:1000") in the compose file or try to set the UID/PUID and GID/PGID environment variables to 1000.
ℹ️	bitcoin-knots/docker-compose.yml	Using unsafe user "root" in service "i2pd_daemon": The user "root" can lead to security vulnerabilities. If possible please use a non-root user instead.
⚠️	bitcoin-knots/docker-compose.yml	Invalid restart policy: The restart policy of the container "bitcoind" should be set to "on-failure".

Legend

Symbol	Description
❌	Error: This must be resolved before this PR can be merged.
⚠️	Warning: This is highly encouraged to be resolved, but is not strictly mandatory.
ℹ️	Info: This is just for your information.

[nmfretz]

I have added logic to the existing pre-start hook that recursively chown's the bitcoind data directory. We write out a flag file so that we don't chown unnecessarily on every app start (i.e, for fresh installs where it is unnecessary, and after we have completed the chown action for the first time).

Tested on both a fresh install and an app update impacted by bitcoind running as root. Electrs connects successfully.

Thanks again @rstmsn! Really appreciate it.

[Retropex]

Big thanks to both of you!