Repository navigation
fix(gateway): drop root before starting the server - #1853
Open
Sanilas-ai wants to merge 1 commit into
Open
Sanilas-ai wants to merge 1 commit into
Sanilas-ai wants to merge 1 commit into
Conversation
The gateway image runs its internet-facing server as root. run.sh now fixes the ownership of the database directory while still root, then re-executes itself as node through setpriv. Started as any other user, it exits with a message when the database is not writable instead of failing on the first write. Closes DouglasNeuroInformatics#1792
joshunrau
requested changes
Nov 4, 2026
joshunrau
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for picking this up. The script follows the #1792 recommendation closely and reads correctly. One thing before it can merge:
-
The rule this PR adds says to verify an image-only change against the built image and list the commands and their results in the PR. The PR body says the image wasn't built. Please build the gateway image and post the commands and their output for these checks:
docker compose exec gateway grep ^Uid /proc/1/statusprints1000on a fresh start with no./sqlite.- It also prints
1000when a root-owned./sqlite/gateway.dbfrom the current image is already there, and the existing data is kept. docker run --user 1000:1000starts with a writable mount, and exits with your error message when the mount is root-owned.- A remote assignment still completes end to end.
If you can't build the image on your machine, just say so here, and a maintainer will run these checks before merging.
Reviewed at commit 1b2107f.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1792
Problem
The gateway is the one service published straight to the internet, and its image runs
run.shandnode ./dist/main.jsas root. Any code-execution bug in the gateway or its dependencies would run as root in the container, able to write anywhere in the image (including the/app/distit serves). The api image already usesUSER node.Change
As recommended in the triage note, the image still starts as root, and
apps/gateway/scripts/run.shdrops privileges before the server starts:chowns anything in it not owned bynode(this covers the root-owned./sqlite/gateway.dbthe old entrypoint copied on every existing deployment), then re-runs itself withsetpriv --reuid=node --regid=node --init-groups --no-new-privs. The template copy andnode ./dist/main.jsthen run asnode, so a fresh database is owned bynode.--reset-envis left out, since it would dropGATEWAY_API_KEYandGATEWAY_DATABASE_URL.docker run --user, a KubernetesrunAsUser), it can't fix ownership. So if the directory or an existing database file isn't writable, it exits with a message naming the path and the uid, instead of booting and failing on the first write./app/diststays root-owned.setprivis part ofutil-linux, which is Essential in Debian, so this adds no dependency.apps/gateway/AGENTS.mdrecords that the image starts as root on purpose and drops privileges inrun.sh, and how to check the server's uid (grep ^Uid /proc/1/status, sincedocker compose exec gateway idstill prints root)..agents/docs/architecture/testing-strategy.mdrecords one exception to the unit + e2e rule for image-only changes (Dockerfile, a container entrypoint,docker-compose.yaml), which no test in the repo runs, as the triage note suggested.No
Dockerfileor compose change is needed.Verification
No unit or Playwright test runs the production image, so I tested
run.shitself. I used a script in a Linux container (dash as/bin/sh, util-linuxsetpriv, anodeuser, uid 1001). It ran the realrun.shwith only the template path redirected to a temp dir, plus a stubdist/main.jsthat writes to the database and prints its uid andGATEWAY_API_KEY. 16/16 checks passed:sqlite/: the server runs as uid 1001, the env survives the re-exec, the template is copied and writable, the directory and file are owned bynode, anddist/stays root-owned.sqlite/,gateway.dband journal: the existing data is kept (not replaced by the template) and handed tonode, and the server writes it asnode.nodewith a writable directory: the server starts.nodewith a root-owned directory: exit 1 with the message, and the server never starts.nodewith a writable directory but a root-ownedgateway.db: exit 1.I did not build the full gateway image on my machine. The checks to run on the built image are the ones from the triage note:
docker compose exec gateway grep ^Uid /proc/1/statusshould print1000, both on a fresh start and with an existing root-owned./sqlite/gateway.db, and a remote assignment should still complete end to end.