Change Docker image to non-root usage

.gitattributes

@@ -1 +1,2 @@
 test/testFixtures/** linguist-vendored
+*.sh text eol=lf

Dockerfile

@@ -25,15 +25,19 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
     fonts-ipafont-gothic fonts-wqy-zenhei fonts-tlwg-loma-otf \
     python3 make g++ \
   && rm -rf /var/lib/apt/lists/* \
-  && mkdir -p /db /conf /fredy
+  && mkdir -p /db /conf /fredy /home/node/.config /home/node/.cache \
+  && chown -R node:node /fredy /db /conf /home/node
 
 WORKDIR /fredy
 
+
 ENV NODE_ENV=production \
     IS_DOCKER=true \
     CLOAKBROWSER_SUPPRESS_FONT_WARNING=1
 
-COPY package.json yarn.lock ./
+COPY --chown=node:node package.json yarn.lock ./
+
+USER node
 
 # Install dependencies and purge build tools (only needed to compile better-sqlite3)
 RUN yarn config set network-timeout 600000 \
@@ -47,39 +51,39 @@ RUN node -e "const D = require('better-sqlite3'); new D(':memory:').close()"
 # Pre-download the CloakBrowser stealth Chromium binary (supports x86_64 and arm64)
 RUN node -e "import('cloakbrowser').then(({ensureBinary}) => ensureBinary())"
 
+USER root
+
 # Purge build tools now that native modules are compiled
 RUN apt-get purge -y python3 make g++ \
   && apt-get autoremove -y \
   && rm -rf /var/lib/apt/lists/*
 
-COPY index.html vite.config.js ./
+COPY --chown=node:node index.html vite.config.js ./
 # Static files Vite copies into the build as they are, such as the onboarding tour's pictures.
-COPY public ./public
-COPY ui ./ui
-COPY lib ./lib
+COPY --chown=node:node public ./public
+COPY --chown=node:node ui ./ui
+COPY --chown=node:node lib ./lib
 
+USER node
 RUN yarn build:frontend
 
-COPY index.js ./
+COPY --chown=node:node index.js ./
 
+
+USER root
 RUN ln -s /db /fredy/db \
   && ln -s /conf /fredy/conf
 
+COPY docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh
+RUN chmod 755 /usr/local/bin/docker-entrypoint.sh
+
 EXPOSE 9998
 VOLUME /db
 VOLUME /conf
 
 HEALTHCHECK --interval=30s --timeout=10s --start-period=30s --retries=3 \
   CMD curl -f http://localhost:9998/ || exit 1
 
-# Run node under tini instead of as pid 1.
-#
-# Chromium spawns helper processes (crashpad handler, gpu, and - because of --no-zygote - one
-# process per renderer). Whenever the browser process dies before them, e.g. when a page crashes
-# it or Puppeteer has to kill it, those helpers are reparented to pid 1. libuv only waits for the
-# pids node itself spawned, so a node running as pid 1 never reaps them and every failed scrape
-# left two more `[chrome] <defunct>` entries behind until the container hit the pid limit.
-# tini reaps whatever it inherits and forwards signals (-g: to the whole process group), so
-# shutdown keeps working as before.
-ENTRYPOINT ["/usr/bin/tini", "-g", "--"]
+
+ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"]
 CMD ["node", "index.js"]

docker-entrypoint.sh

@@ -0,0 +1,42 @@
+#!/bin/sh
+# Fredy startup script
+# Runs node under tini instead of as pid 1, and as the unprivileged node user.
+#
+# Chromium spawns helper processes (crashpad handler, gpu, and - because of --no-zygote - one
+# process per renderer). Whenever the browser process dies before them, e.g. when a page crashes
+# it or Puppeteer has to kill it, those helpers are reparented to pid 1. libuv only waits for the
+# pids node itself spawned, so a node running as pid 1 never reaps them and every failed scrape
+# left two more `[chrome] <defunct>` entries behind until the container hit the pid limit.
+# tini reaps whatever it inherits and forwards signals (-g: to the whole process group), so
+# shutdown keeps working as before.
+#
+# Started as root (the default), it hands the volumes to node and drops privileges. Started with
+# `--user`, it cannot fix anything, so it refuses to start when the volumes are not writable instead
+# of letting SQLite fail later with a less helpful error.
+
+set -e
+
+uid="$(id -u)"
+gid="$(id -g)"
+
+if [ "$uid" = "0" ]; then
+  # Only touches what is not node's already. -h: a symlink in a volume must not hand its target
+  # to node.
+  find /db /conf \( \! -user node -o \! -group node \) -exec chown -h node:node {} + \
+    || echo "WARN: could not fix ownership of /db or /conf" >&2
+  # setpriv keeps root's environment, so HOME would still point at /root. Chromium's profile and
+  # CloakBrowser's binary cache both live under HOME.
+  export HOME=/home/node
+  exec setpriv --reuid=node --regid=node --init-groups /usr/bin/tini -g -- "$@"
+fi
+
+unwritable="$(find /db /conf \! -writable 2>/dev/null | head -n 5)"
+if [ -n "$unwritable" ]; then
+  echo "ERROR: running as uid $uid:$gid, but these paths in /db or /conf are not writable:" >&2
+  echo "$unwritable" | sed 's/^/  /' >&2
+  echo "Fix the ownership on the host, e.g. 'chown -R 1000:1000 <your db dir> <your conf dir>'," >&2
+  echo "or start the container without --user to let it fix the ownership itself." >&2
+  exit 1
+fi
+
+exec /usr/bin/tini -g -- "$@"

test/docker/dockerfile.test.js

@@ -8,24 +8,111 @@ import { readFileSync } from 'fs';
 import path from 'path';
 import { fileURLToPath } from 'url';
 
-const dockerfile = readFileSync(path.join(path.dirname(fileURLToPath(import.meta.url)), '../../Dockerfile'), 'utf-8');
+const projectRoot = path.join(path.dirname(fileURLToPath(import.meta.url)), '../..');
+
+const dockerfile = readFileSync(path.join(projectRoot, 'Dockerfile'), 'utf-8');
+
+const entrypointScript = readFileSync(path.join(projectRoot, 'docker-entrypoint.sh'), 'utf-8');
+
+const parseDockerJsonInstruction = (instruction) => {
+  const instructionValue = dockerfile.match(new RegExp(`^${instruction}\\s+(\\[.+\\])\\s*$`, 'm'))?.[1];
+
+  expect(instructionValue, `${instruction} must be defined`).toBeDefined();
+  return JSON.parse(instructionValue ?? 'null');
+};
+
+const lastDockerUserBefore = (marker) => {
+  const markerIndex = dockerfile.search(marker);
+
+  expect(markerIndex, `${marker} must exist in Dockerfile`).toBeGreaterThanOrEqual(0);
+
+  const users = dockerfile.slice(0, markerIndex).match(/^USER\s+\S+\s*$/gm);
+
+  return users?.at(-1);
+};
 
 /**
- * Without an init as pid 1 nobody reaps the Chromium helper processes that get reparented when a
- * browser dies, and the container slowly fills up with `<defunct>` entries. Node cannot do the
- * reaping itself (libuv only waits for the pids it spawned), so this has to stay in the image.
+ * Without an init as pid 1 nobody reaps the Chromium helper processes that get
+ * reparented when a browser dies, and the container slowly fills up with
+ * `<defunct>` entries.
+ *
+ * Node cannot do the reaping itself (libuv only waits for the pids it spawned),
+ * so tini must remain in the process chain as the container's init/subreaper.
+ *
+ * The Docker ENTRYPOINT is a small wrapper. Started as root, it repairs volume
+ * ownership and hands execution to tini after switching to the unprivileged node
+ * user. Started with `--user`, it checks the volumes are writable and execs
+ * tini directly.
  */
 describe('Dockerfile init process', () => {
   it('installs tini', () => {
     expect(dockerfile).toMatch(/apt-get install[\s\S]*?\btini\b/);
   });
 
-  it('runs the app under tini rather than as pid 1', () => {
-    const entrypoint = dockerfile.match(/^ENTRYPOINT (.+)$/m)?.[1];
-    const cmd = dockerfile.match(/^CMD (.+)$/m)?.[1];
+  it('copies the startup entrypoint into the image and makes it executable', () => {
+    expect(dockerfile).toMatch(/^COPY\s+docker-entrypoint\.sh\s+\/usr\/local\/bin\/docker-entrypoint\.sh\s*$/m);
+
+    expect(dockerfile).toMatch(/^RUN\s+chmod\s+755\s+\/usr\/local\/bin\/docker-entrypoint\.sh\s*$/m);
+  });
+
+  it('keeps the final image startup as root so mounted volumes can be fixed', () => {
+    expect(lastDockerUserBefore(/^ENTRYPOINT\s/m)).toBe('USER root');
+  });
+
+  it('uses the startup script as Docker ENTRYPOINT', () => {
+    expect(parseDockerJsonInstruction('ENTRYPOINT')).toEqual(['/usr/local/bin/docker-entrypoint.sh']);
+
+    expect(parseDockerJsonInstruction('CMD')).toEqual(['node', 'index.js']);
+  });
+
+  it('only repairs volume ownership when started as root', () => {
+    const rootBranch = entrypointScript.indexOf('if [ "$uid" = "0" ]; then');
+    const chown = entrypointScript.indexOf('-exec chown -h node:node {} +');
+    const exec = entrypointScript.indexOf('exec setpriv');
+
+    expect(rootBranch).toBeGreaterThanOrEqual(0);
+    expect(chown).toBeGreaterThan(rootBranch);
+    expect(chown).toBeLessThan(exec);
+  });
+
+  it('only chowns what node does not own yet, without following symlinks', () => {
+    expect(entrypointScript).toMatch(
+      /find \/db \/conf \\\( \\! -user node -o \\! -group node \\\) -exec chown -h node:node/,
+    );
+  });
+
+  it('keeps starting when the ownership cannot be fixed', () => {
+    expect(entrypointScript).toMatch(/-exec chown[^\n]*\\\n\s*\|\| echo "WARN:/);
+  });
+
+  it("points HOME at node before dropping privileges, since setpriv keeps root's environment", () => {
+    const home = entrypointScript.indexOf('export HOME=/home/node');
+    const exec = entrypointScript.indexOf('exec setpriv');
+
+    expect(home).toBeGreaterThanOrEqual(0);
+    expect(home).toBeLessThan(exec);
+  });
+
+  it('drops privileges and runs the application under tini', () => {
+    expect(entrypointScript).toMatch(/\bsetpriv\b[\s\S]*?--reuid=node\b[\s\S]*?--regid=node\b/);
+
+    expect(entrypointScript).toContain('--init-groups');
+
+    expect(entrypointScript).toMatch(/\/usr\/bin\/tini\s+-g\s+--\s+"\$@"/);
+  });
+
+  it('execs the privilege drop / tini process instead of leaving the shell as pid 1', () => {
+    expect(entrypointScript).toMatch(/\bexec\s+setpriv\b[\s\S]*?\/usr\/bin\/tini\s+-g\s+--\s+"\$@"/);
+  });
+
+  it('refuses to start as another user when the volumes are not writable', () => {
+    const check = entrypointScript.indexOf('find /db /conf \\! -writable');
+    const exit = entrypointScript.indexOf('exit 1', check);
+    const exec = entrypointScript.lastIndexOf('exec /usr/bin/tini -g -- "$@"');
 
-    expect(entrypoint).toBeDefined();
-    expect(JSON.parse(entrypoint)).toEqual(['/usr/bin/tini', '-g', '--']);
-    expect(JSON.parse(cmd)).toEqual(['node', 'index.js']);
+    expect(check).toBeGreaterThan(entrypointScript.indexOf('exec setpriv'));
+    expect(exit).toBeGreaterThan(check);
+    expect(exec).toBeGreaterThan(exit);
+    expect(entrypointScript).toMatch(/chown -R 1000:1000/);
   });
 });