non-root users for Dockerfiles

[hkiang01]

---

category: Maintenance
authors: hkiang01

Non-root users for Dockerfiles

As a regular Node application, the default user doesn't need to be root. For why this is important, see Why non-root containers are important for security

[hkiang01]

I tested this locally by altering docker-compose.yml to point to each edited Dockerfile. I was able to successfully build and start each corresponding built container to serve localhost:5006

[hkiang01]

Syncing works as of the latest commit

Expand details below for some details related to confirming functionality

Edit: This was not be due to this PR, but a mis-configured security context in my kubernetes cluster

hmmm got an error while syncing

Rejection: SqliteError: attempt to write a readonly database
    at WrappedDatabase.mutate (file:///app/src/db.js:39:21)
    at file:///app/src/sync-simple.js:31:23
    at sqliteTransaction (/app/node_modules/better-sqlite3/lib/methods/transaction.js:65:24)
    at WrappedDatabase.transaction (file:///app/src/db.js:47:35)
    at addMessages (file:///app/src/sync-simple.js:26:6)
    at Module.sync (file:///app/src/sync-simple.js:81:14)
    at file:///app/src/app-sync.js:111:42
    at Layer.handle [as handle_request] (/app/node_modules/express/lib/router/layer.js:95:5)
    at next (/app/node_modules/express/lib/router/route.js:144:13)
    at Route.dispatch (/app/node_modules/express/lib/router/route.js:114:3) {
  code: 'SQLITE_READONLY'

[hkiang01]

I recommend bumping major version as the non-root user wouldn't have access to read/write files owned by root. This can be fixed by running chown -R 1001:1001 /data in the container

[twk3]

@hkiang01 the app code should remain root, so the runtime user can't manipulate the code. Runtime users should just be able to influence the config and data dir.

[twk3]

I recommend bumping major version as the non-root user wouldn't have access to read/write files owned by root. This can be fixed by running chown -R 1001:1001 /data in the container

This is a good point, but I think that's a bit of a blocker for us for now. We also have to consider users already running on pikapods and fly.io

@hkiang01 I think as an iteration, adding the user, but not setting it as the run user for entry. Chowning the data directory contents on entry to the new user, and then running the app as the new user will set us up to switchover in a major release.

[hkiang01]

Requested changes made. I expect a retry of the build jobs will succeed. Please retry the jobs and re-review

[hkiang01]

failing builds are due to #287 which is out of scope of this PR

[hkiang01]

confirmed non-breaking change with manual steps:

1. spin up container with default user (root)
2. navigate to localhost:5006, perform some actions, sync
3. re-create container with user appuser
4. repeat step 2

[twk3]

@hkiang01 this looks like a good first step, and I think we can move forward with this change after a few minor tweaks for users who want to set the user to 1001 when running from their very first run. (Like in your helm chart). But the swap from root to app user does not currently work without manual chowning of the data. (database files will be readonly by the appuser)

A followup (doesn't have to block this PR). Would be to move our current RUN command to a startup script, then earlier in that script check if the running uid is 0, if so, chown all data (at runtime this time) to our non-root user, and then exec our tini command as that user. Else we just exec the tini command like normal.

Bitnami has some examples of using chroot to change the user at runtime if running as root by just chrooting to / again but with userspace. https://github.com/bitnami/containers/blob/8a67664e7a937c6aa603a9b0477561b9af7abb0e/bitnami/postgresql/16/debian-11/prebuildfs/opt/bitnami/scripts/libos.sh#L653

This would allow us to get everyone running as non-root, and in a later major change we could just drop the extra work for the USER line in the dockerfile.

[twk3]

Thanks @hkiang01

This is a good first step towards moving the container to always run non-root.