diff --git a/server/scripts/entrypoint.sh b/server/scripts/entrypoint.sh index 7b8604f8d..1e0594e10 100644 --- a/server/scripts/entrypoint.sh +++ b/server/scripts/entrypoint.sh @@ -31,16 +31,25 @@ if [ "$(id -u)" != "0" ]; then exec node --require tsconfig-paths/register dist/index.js fi -# Hand the mounted data dirs to node, touching only what is not node's yet. A -# blanket `chown -R` rewrote every inode on every start, which on a large or -# network-backed volume (NFS, CephFS, Longhorn) kept the container busy for -# minutes before Node printed its first line (#2918). Best effort: a read-only -# host filesystem or a locked-down bind mount must keep booting. +# Hand the mounted data dirs to node. A blanket `chown -R` on every start +# rewrote, and even a search for strays still read, every inode of the volume, +# which on a large or network-backed one (NFS, CephFS, Longhorn) kept the +# container busy for minutes before Node printed its first line (#2918). So only +# the dir and what sits directly in it are looked at: a volume that is not +# node's yet (a fresh one, or one copied in as root) shows there, and is handed +# over once. A volume that refuses chown altogether (NFS with root_squash) is +# said once per start instead of being walked file by file; such a pod belongs +# under runAsUser 1000, see the Helm page of the wiki. Best effort either way: a +# read-only host filesystem or a locked-down bind mount must keep booting. for dir in /app/data /app/uploads; do [ -d "$dir" ] || continue - if [ -n "$(find "$dir" \( ! -user node -o ! -group node \) -print -quit 2>/dev/null)" ]; then - echo "Correcting ownership of $dir for the node user..." - find "$dir" \( ! -user node -o ! -group node \) -exec chown -h node:node {} + 2>/dev/null || true + if [ -n "$(find "$dir" -maxdepth 1 \( ! -user node -o ! -group node \) -print -quit 2>/dev/null)" ]; then + if chown node:node "$dir" 2>/dev/null; then + echo "Correcting ownership of $dir for the node user..." + chown -R -h node:node "$dir" 2>/dev/null || true + else + echo "WARNING: $dir cannot be handed to the node user (chown refused); run the container as uid 1000 instead." + fi fi done diff --git a/server/tests/unit/docker-entrypoint.test.ts b/server/tests/unit/docker-entrypoint.test.ts index aa51bbd13..bf95cfe55 100644 --- a/server/tests/unit/docker-entrypoint.test.ts +++ b/server/tests/unit/docker-entrypoint.test.ts @@ -59,12 +59,22 @@ describe('entrypoint script', () => { .filter((line) => !line.trimStart().startsWith('#')) .join('\n'); - it('never chowns the volumes recursively and unconditionally', () => { - expect(code).not.toMatch(/chown\s+-R/); - expect(code).toContain('! -user node -o ! -group node'); + it('never walks the volumes on a start where they already belong to node', () => { + // Only the dir and its direct entries are looked at, never the whole tree. + expect(code).toContain('find "$dir" -maxdepth 1 \\( ! -user node -o ! -group node \\) -print -quit'); + expect(code).not.toMatch(/find "\$dir" \\\(/); + // The recursive hand-over sits behind that check and behind a chown of the dir that worked. + const check = code.indexOf('-maxdepth 1'); + const dirChown = code.indexOf('if chown node:node "$dir"'); + expect(dirChown).toBeGreaterThan(check); + expect(code.indexOf('chown -R')).toBeGreaterThan(dirChown); expect(code).toContain('Correcting ownership of $dir for the node user...'); }); + it('says so once instead of walking a volume that refuses chown', () => { + expect(code).toContain('cannot be handed to the node user (chown refused); run the container as uid 1000 instead.'); + }); + // Under runAsUser: 1000 there is no privilege to drop and gosu cannot switch // users, so the non-root branch has to exec Node itself before any chown or gosu. it('starts Node directly, without chown or gosu, when not running as root', () => { diff --git a/wiki/Install-Helm.md b/wiki/Install-Helm.md index df0ce8555..9c139880a 100644 --- a/wiki/Install-Helm.md +++ b/wiki/Install-Helm.md @@ -199,7 +199,7 @@ podSecurityContext: securityContext: {} ``` -With these defaults the container starts as root, gives every file under `/app/data` and `/app/uploads` that does not belong to the `node` user (uid 1000) to `node`, and then drops to `node`. Files that already belong to `node` are left alone, so a large uploads volume does not slow down the start. If ownership has to be corrected, the log says `Correcting ownership of /app/uploads for the node user...` before TREK's own start-up lines. +With these defaults the container starts as root, hands `/app/data` and `/app/uploads` to the `node` user (uid 1000) if they are not `node`'s yet, and then drops to `node`. Only the two directories and what sits directly in them are checked, so a large uploads volume that already belongs to `node` does not slow down the start. If ownership has to be corrected, the log says `Correcting ownership of /app/uploads for the node user...` before TREK's own start-up lines. A volume that refuses `chown` (NFS with `root_squash`, for example) logs a warning instead; run TREK as uid 1000 then, as shown below. To run TREK as `node` from the first process on, for example in a namespace that enforces the `restricted` Pod Security Standard, set the user in the pod security context. The image notices it is not root and starts Node directly, with no ownership step and no privilege drop: