From 875a84e301fa622cdbfbb80898fcb3dbca1fe474 Mon Sep 17 00:00:00 2001 From: John Costa Date: Fri, 18 Sep 2026 07:35:05 -0700 Subject: [PATCH] fix(docker): copy the app with node ownership instead of a second chown layer (#14010) The runner-base stage COPY'd the standalone build as root and then ran `RUN chown -R node:node /app`. On overlayfs a chown rewrites every file it touches into the new layer, so the published image carried the ~2 GB standalone tree twice (docker history of diegosouzapw/omniroute:latest: `COPY /app/.build/next/standalone ./` 2.03 GB followed by `RUN chown -R node:node /app` 2.04 GB). Set `--chown=node:node` on the three COPYs that populate /app, drop the recursive chown, and hand /app and /app/data to node non-recursively next to the `mkdir -p /app/data` so the data dir stays writable without a volume. Measured by rebuilding the runner-base COPY/chown sequence against the published /app tree (root-owned source, same base image, linux/arm64): before: 4.38 GB of layers (COPY 2.04 GB + chown -R 2.04 GB), inspect Size 1220846106 after: 2.34 GB of layers (COPY --chown 2.04 GB), inspect Size 655117142 The fixed image runs as uid 1000, /app and /app/data are node-owned and writable, the server boots and healthcheck.mjs exits 0. hadolint output is unchanged. tests/unit/dockerfile-copy-chown-13990.test.ts guards the mechanism. Fixes #13990 Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --- Dockerfile | 16 ++--- .../fixes/14010-docker-single-app-layer.md | 1 + .../unit/dockerfile-copy-chown-13990.test.ts | 63 +++++++++++++++++++ 3 files changed, 72 insertions(+), 8 deletions(-) create mode 100644 changelog.d/fixes/14010-docker-single-app-layer.md create mode 100644 tests/unit/dockerfile-copy-chown-13990.test.ts diff --git a/Dockerfile b/Dockerfile index e42e9bcf22..8b92bf92fe 100644 --- a/Dockerfile +++ b/Dockerfile @@ -226,7 +226,7 @@ ENV NODE_OPTIONS="--max-old-space-size=${OMNIROUTE_MEMORY_MB}" # Data directory inside Docker — must match the volume mount in docker-compose.yml ENV DATA_DIR=/app/data -RUN mkdir -p /app/data +RUN mkdir -p /app/data && chown node:node /app /app/data # #13679: default the PUBLISHED image to requiring an API key. A bare # `docker run -p 20128:20128 … diegosouzapw/omniroute` (README/QUICK-START @@ -248,24 +248,24 @@ ENV REQUIRE_API_KEY=true # The old per-module overrides were therefore pure duplication and were removed # (build-output-isolation cleanup). See scripts/build/assembleStandalone.mjs # (EXTRA_MODULE_ENTRIES) for the single source of truth. -COPY --from=builder /app/.build/next/standalone ./ +COPY --chown=node:node --from=builder /app/.build/next/standalone ./ # better-sqlite3 is the one exception still copied explicitly: assembleStandalone # only syncs its native build/ dir; the JS wrapper (lib/, package.json) is left to # Next.js tracing. bootstrap-env requires SQLite BEFORE the standalone server # starts, so guarantee the complete package independent of trace behaviour. -COPY --from=builder /app/node_modules/better-sqlite3 ./node_modules/better-sqlite3 +COPY --chown=node:node --from=builder /app/node_modules/better-sqlite3 ./node_modules/better-sqlite3 RUN test -f /app/node_modules/better-sqlite3/build/Release/better_sqlite3.node # migrations land at /migrations via assembleStandalone; point the runtime at them. ENV OMNIROUTE_MIGRATIONS_DIR=/app/migrations # Docker healthcheck script — not traced by Next.js standalone output, so copy # it explicitly. The HEALTHCHECK CMD references it as `node healthcheck.mjs`. -COPY --from=builder /app/scripts/dev/healthcheck.mjs ./healthcheck.mjs +COPY --chown=node:node --from=builder /app/scripts/dev/healthcheck.mjs ./healthcheck.mjs -# Hand /app over to the baked-in `node` non-root user (UID/GID 1000) so the -# runtime process never holds root privileges. The chown happens after all -# COPYs so it covers files originally owned by root in the builder stage. -RUN chown -R node:node /app +# Every COPY above hands its files to the baked-in `node` non-root user +# (UID/GID 1000) at copy time. Do NOT add a `RUN chown -R node:node /app` +# afterwards: in the overlay filesystem changing ownership rewrites every file +# into a new layer, which stored the ~2 GB standalone build twice (#13990). EXPOSE 20128 diff --git a/changelog.d/fixes/14010-docker-single-app-layer.md b/changelog.d/fixes/14010-docker-single-app-layer.md new file mode 100644 index 0000000000..56e554668a --- /dev/null +++ b/changelog.d/fixes/14010-docker-single-app-layer.md @@ -0,0 +1 @@ +- **fix(docker):** copy the app into the runtime image with `--chown=node:node` instead of a second `chown -R` layer, so the image no longer stores the ~2 GB standalone build twice ([#13990](https://github.com/diegosouzapw/OmniRoute/issues/13990)) diff --git a/tests/unit/dockerfile-copy-chown-13990.test.ts b/tests/unit/dockerfile-copy-chown-13990.test.ts new file mode 100644 index 0000000000..c537e64997 --- /dev/null +++ b/tests/unit/dockerfile-copy-chown-13990.test.ts @@ -0,0 +1,63 @@ +/** + * #13990 — the published image stored the app twice. The `runner-base` stage + * COPY'd the standalone build as root and then ran `RUN chown -R node:node /app`. + * On an overlay filesystem a chown rewrites every touched file into the new + * layer, so the ~2 GB standalone tree landed in two layers with identical + * content (the registry manifest showed two ~565 MB compressed layers back to + * back). + * + * Fix: every COPY into /app in `runner-base` carries `--chown=node:node`, so the + * files are written with the right owner in their own layer, and the recursive + * chown is gone. `/app` itself (created root-owned by `WORKDIR /app` in the + * `base` stage) and `/app/data` still get a non-recursive chown so the runtime + * user can write there when no volume is mounted. + * + * The real proof is the image size (see the PR's `docker history` output); this + * guards the mechanism so the duplicate layer does not creep back in. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../.."); +const dockerfile = fs.readFileSync(path.join(repoRoot, "Dockerfile"), "utf-8"); +const lines = dockerfile.split("\n"); + +/** Non-comment instruction lines of the `runner-base` stage (FROM to next FROM). */ +function runnerBaseStage(): string[] { + const start = lines.findIndex((l) => /^FROM\s+\S+\s+AS\s+runner-base\b/i.test(l.trim())); + assert.ok(start >= 0, "Dockerfile must declare a `runner-base` stage"); + const after = lines.slice(start + 1).findIndex((l) => /^FROM\s+/i.test(l.trim())); + const end = after === -1 ? lines.length : start + 1 + after; + return lines.slice(start, end).filter((l) => !l.trim().startsWith("#")); +} + +test("#13990 runner-base copies the app with --chown=node:node instead of a second chown layer", () => { + const stage = runnerBaseStage(); + + const copiesFromBuilder = stage.filter((l) => /^COPY\b.*--from=builder\b/.test(l)); + assert.ok( + copiesFromBuilder.length >= 3, + "runner-base must COPY the standalone build, better-sqlite3 and healthcheck.mjs from the builder" + ); + for (const line of copiesFromBuilder) { + assert.match(line, /--chown=node:node\b/, `COPY must set node ownership at copy time: ${line}`); + } + + assert.ok( + !stage.some((l) => /^RUN\b.*chown\s+-R\b/.test(l)), + "runner-base must not run a recursive chown: it rewrites the whole standalone tree into a duplicate layer" + ); +}); + +test("#13990 runner-base still hands /app and /app/data to the node user", () => { + const stage = runnerBaseStage(); + assert.ok( + stage.some((l) => + /^RUN\b.*mkdir -p \/app\/data\s*&&\s*chown node:node \/app \/app\/data\b/.test(l) + ), + "runner-base must create /app/data and chown /app and /app/data (non-recursively) to node" + ); +});