fix(docker): non-root web image and secret-proof dockerignore - #74
Conversation
…ecrets The runner stage operated as root (audit HI-12 remainder): a container escape would land directly on root. Copy the app tree with --chown so the unprivileged node user owns it (keeping runtime cache writes working) and switch to USER node after the root-only apt step. .dockerignore now excludes .env*, *.pem, *.key, *.p12, *.pfx and .husky so local secrets cannot be baked into image layers. The root context is shared by the api/v2 image build, which protects that image too.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request updates .dockerignore to exclude sensitive environment and key files, and modifies the Dockerfile to run the container as the unprivileged node user with appropriate file ownership. Feedback on the changes includes changing the ownership of the /calcom directory itself to prevent permission issues, using the numeric UID 1000 instead of the username node for better compatibility with security scanners, and adding the --no-hsts flag to the wget healthcheck to avoid write failures in read-only filesystems.
| @@ -101,14 +101,21 @@ WORKDIR /calcom | |||
|
|
|||
| RUN apt-get update && apt-get install -y --no-install-recommends netcat-openbsd wget && rm -rf /var/lib/apt/lists/* | |||
There was a problem hiding this comment.
While COPY --chown=node:node correctly sets the ownership of the copied files, the /calcom directory itself was created by WORKDIR /calcom while running as root. This leaves the /calcom directory owned by root:root with 0755 permissions, which prevents the non-root node user from creating any new files or directories directly under /calcom at runtime (e.g., lockfiles, temporary files, or local cache directories).
To prevent potential Permission denied errors, change the ownership of the /calcom directory to node:node during the setup phase.
RUN apt-get update && apt-get install -y --no-install-recommends netcat-openbsd wget && rm -rf /var/lib/apt/lists/* && chown node:node /calcom
|
|
||
| # HI-12: run as the unprivileged node user (uid 1000) shipped with the base | ||
| # image instead of root; apt above already ran as root. | ||
| USER node |
There was a problem hiding this comment.
Using a non-numeric user name like USER node can cause issues with strict container security scanners, Kubernetes admission controllers (such as Kyverno or OPA Gatekeeper), and certain container runtimes that require or prefer numeric UIDs to verify non-root execution without inspecting the image's /etc/passwd file.
Since the node user in the official Node.js base image is guaranteed to have UID 1000, it is a best practice to use the numeric UID instead.
USER 1000
| HEALTHCHECK --interval=30s --timeout=30s --retries=5 \ | ||
| CMD wget --spider http://localhost:3000 || exit 1 |
There was a problem hiding this comment.
By default, wget attempts to write a .wget-hsts file to the user's home directory (/home/node). If the container is run with a read-only root filesystem (a common security best practice), this write operation will fail and may cause the healthcheck to fail or log warnings.
Adding the --no-hsts flag to wget disables HSTS tracking and prevents it from attempting to write to the filesystem, making the healthcheck more robust in read-only environments.
HEALTHCHECK --interval=30s --timeout=30s --retries=5 \
CMD wget --no-hsts --spider http://localhost:3000 || exit 1
What
Closes the HI-12 remainder of the audit:
--chown=node:node(no duplicate copy-up layer) and the container switches toUSER nodeafter the root-only apt step. Runtime cache writes (Next ISR, prisma engines) keep working because node owns the tree..dockerignoreblocks secrets -.env,.env.*,*.pem,*.key,.huskyetc. can no longer be baked into image layers. The repo root context is shared by the api/v2 image build, so that image is protected too.Evidence
git ls-filesconfirms no tracked .pem/.key/.env file is needed by any build step, so ignoring them is safe.Risk
🤖 Generated by ZCode