Skip to content

fix(gateway): drop root before starting the server - #1853

Open
Sanilas-ai wants to merge 1 commit into
DouglasNeuroInformatics:mainfrom
Sanilas-ai:fix/1792-gateway-drop-root
Open

Sanilas-ai wants to merge 1 commit into
DouglasNeuroInformatics:mainfrom
Sanilas-ai:fix/1792-gateway-drop-root

Conversation

@Sanilas-ai

Copy link
Copy Markdown
Contributor

Closes #1792

Problem

The gateway is the one service published straight to the internet, and its image runs run.sh and node ./dist/main.js as 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/dist it serves). The api image already uses USER node.

Change

As recommended in the triage note, the image still starts as root, and apps/gateway/scripts/run.sh drops privileges before the server starts:

  • As root, it creates the database directory, chowns anything in it not owned by node (this covers the root-owned ./sqlite/gateway.db the old entrypoint copied on every existing deployment), then re-runs itself with setpriv --reuid=node --regid=node --init-groups --no-new-privs. The template copy and node ./dist/main.js then run as node, so a fresh database is owned by node. --reset-env is left out, since it would drop GATEWAY_API_KEY and GATEWAY_DATABASE_URL.
  • Started as any other user (docker run --user, a Kubernetes runAsUser), 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.
  • The database directory is the only path the server writes. /app/dist stays root-owned. setpriv is part of util-linux, which is Essential in Debian, so this adds no dependency.
  • apps/gateway/AGENTS.md records that the image starts as root on purpose and drops privileges in run.sh, and how to check the server's uid (grep ^Uid /proc/1/status, since docker compose exec gateway id still prints root).
  • .agents/docs/architecture/testing-strategy.md records 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 Dockerfile or compose change is needed.

Verification

No unit or Playwright test runs the production image, so I tested run.sh itself. I used a script in a Linux container (dash as /bin/sh, util-linux setpriv, a node user, uid 1001). It ran the real run.sh with only the template path redirected to a temp dir, plus a stub dist/main.js that writes to the database and prints its uid and GATEWAY_API_KEY. 16/16 checks passed:

  1. Fresh start as root with no 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 by node, and dist/ stays root-owned.
  2. Upgrade path, with a root-owned sqlite/, gateway.db and journal: the existing data is kept (not replaced by the template) and handed to node, and the server writes it as node.
  3. Started as node with a writable directory: the server starts.
  4. Started as node with a root-owned directory: exit 1 with the message, and the server never starts.
  5. Started as node with a writable directory but a root-owned gateway.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/status should print 1000, both on a fresh start and with an existing root-owned ./sqlite/gateway.db, and a remote assignment should still complete end to end.

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
@Sanilas-ai
Sanilas-ai requested a review from joshunrau as a code owner November 2, 2026 19:22

@joshunrau joshunrau left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for picking this up. The script follows the #1792 recommendation closely and reads correctly. One thing before it can merge:

  1. 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/status prints 1000 on a fresh start with no ./sqlite.
    • It also prints 1000 when a root-owned ./sqlite/gateway.db from the current image is already there, and the existing data is kept.
    • docker run --user 1000:1000 starts 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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gateway container runs its internet-facing server as root

2 participants