diff --git a/.gitattributes b/.gitattributes index 60bca2ac..e5df7496 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1 +1,2 @@ test/testFixtures/** linguist-vendored +*.sh text eol=lf \ No newline at end of file diff --git a/Dockerfile b/Dockerfile index a39ea0a3..040f1b4b 100644 --- a/Dockerfile +++ b/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,24 +51,32 @@ 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 @@ -72,14 +84,6 @@ 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] ` 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"] diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh new file mode 100644 index 00000000..02443626 --- /dev/null +++ b/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] ` 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 '," >&2 + echo "or start the container without --user to let it fix the ownership itself." >&2 + exit 1 +fi + +exec /usr/bin/tini -g -- "$@" diff --git a/test/docker/dockerfile.test.js b/test/docker/dockerfile.test.js index 99a5cdf2..4fc3326e 100644 --- a/test/docker/dockerfile.test.js +++ b/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 `` 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 + * `` 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/); }); });