fix(security): run Docker container as non-root user#14602
fix(security): run Docker container as non-root user#14602JasonOA888 wants to merge 2 commits intotoeverything:canaryfrom
Conversation
- Create affine user with configurable UID/GID (default 1000:1000) - Set proper ownership of /app directory - Switch to non-root user before CMD - Prevents privilege escalation attacks from container breach Security: Fixes potential privilege escalation vulnerability where container running as root could allow attacker to breach host. Fixes toeverything#14584
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds non-root execution to the Node Docker image by introducing Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/deployment/node/Dockerfile:
- Around line 30-35: The Dockerfile introduces a non-root user (ARG
APP_UID/APP_GID, useradd affine) but only chowns /app, causing a mismatch with
the app's homedir() (/home/affine) and mounted volumes in compose; update the
deployment by (1) changing mount targets in .docker/selfhost/compose.yml from
/root/.affine/* to /home/affine/.affine/* so volumes match the affine user's
homedir(), (2) modify the Dockerfile to create the expected runtime directories
(e.g., /home/affine/.affine/storage and /home/affine/.affine/config) and chown
them to "${APP_UID}:${APP_GID}" after useradd, and (3) add a short note in
docs/README about the breaking change for existing deployments so operators can
update their mounts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3f76578d-a5d9-446f-9639-1799ca468afd
📒 Files selected for processing (1)
.github/deployment/node/Dockerfile
| # Create non-root user for security | ||
| ARG APP_UID=1000 | ||
| ARG APP_GID=1000 | ||
| RUN groupadd -g "${APP_GID}" affine && \ | ||
| useradd -m -u "${APP_UID}" -g "${APP_GID}" -s /usr/sbin/nologin affine && \ | ||
| chown -R "${APP_UID}:${APP_GID}" /app |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how the application determines its data/config paths
# Check if paths are configurable via environment variables
echo "=== Checking for hardcoded /root paths in application code ==="
rg -n '/root/.affine' --type js --type ts -g '!node_modules' || echo "No hardcoded /root/.affine paths found in JS/TS"
echo ""
echo "=== Checking for AFFINE_DATA_DIR or similar env var usage ==="
rg -n 'AFFINE.*DIR|DATA.*PATH|CONFIG.*PATH|STORAGE.*PATH' --type js --type ts -g '!node_modules' | head -30
echo ""
echo "=== Checking selfhost compose.yml for volume mounts ==="
cat .docker/selfhost/compose.yml 2>/dev/null | grep -A2 'volumes:'Repository: toeverything/AFFiNE
Length of output: 1107
🏁 Script executed:
#!/bin/bash
# Check how GitHub Actions builds the Docker image
echo "=== Searching for GitHub Actions workflows that build Docker image ==="
find .github/workflows -name '*.yml' -o -name '*.yaml' | xargs grep -l "docker.*build\|Dockerfile" 2>/dev/null | head -5
echo ""
echo "=== Checking if APP_UID/APP_GID are passed as build args in CI ==="
find .github/workflows -name '*.yml' -o -name '*.yaml' | xargs grep -A5 -B5 "APP_UID\|APP_GID" 2>/dev/null
echo ""
echo "=== Searching for all compose files that might mount /root paths ==="
find . -name "compose*.yml" -o -name "docker-compose*.yml" | xargs grep -l "/root" 2>/dev/null
echo ""
echo "=== Full check of selfhost compose.yml ==="
cat .docker/selfhost/compose.yml 2>/dev/null || echo "File not found"Repository: toeverything/AFFiNE
Length of output: 2748
🏁 Script executed:
#!/bin/bash
# Check the GitHub Actions build configuration
echo "=== Checking build-images.yml ==="
cat .github/workflows/build-images.yml | head -100
echo ""
echo "=== Checking migration script to see if it would fail ==="
cat scripts/self-host-predeploy.js 2>/dev/null | head -50
echo ""
echo "=== Check Dockerfile USER directive around lines 42-43 ==="
sed -n '38,50p' .github/deployment/node/DockerfileRepository: toeverything/AFFiNE
Length of output: 3530
🏁 Script executed:
#!/bin/bash
# Verify the migration script
echo "=== Looking for self-host-predeploy.js ==="
find . -name "self-host-predeploy.js" -type f 2>/dev/null
echo ""
echo "=== Check prelude.ts to confirm path handling ==="
sed -n '1,30p' packages/backend/server/src/prelude.ts
echo ""
echo "=== Verify homedir() usage in env.ts ==="
sed -n '55,65p' packages/backend/server/src/env.tsRepository: toeverything/AFFiNE
Length of output: 1387
Breaking change: Volume mount paths incompatible with non-root user.
The selfhost compose.yml mounts volumes to /root/.affine/storage and /root/.affine/config. The application uses homedir() which resolves to /home/affine when running as the non-root user affine. This creates a mismatch: the app will attempt to read/write from /home/affine/.affine/* but the volumes are mounted to /root/.affine/*, resulting in permission errors.
Additionally, the Dockerfile only chowns /app but doesn't create or chown the data directories that the application needs to write to.
Consider:
- Updating
.docker/selfhost/compose.ymlto mount volumes to/home/affine/.affine/storageand/home/affine/.affine/config - Creating and chowning data directories in the Dockerfile for the non-root user
- Documenting this breaking change for existing deployments
🛠️ Suggested fix to create writable data directories
# Create non-root user for security
ARG APP_UID=1000
ARG APP_GID=1000
RUN groupadd -g "${APP_GID}" affine && \
useradd -m -u "${APP_UID}" -g "${APP_GID}" -s /usr/sbin/nologin affine && \
- chown -R "${APP_UID}:${APP_GID}" /app
+ chown -R "${APP_UID}:${APP_GID}" /app && \
+ mkdir -p /home/affine/.affine/storage /home/affine/.affine/config && \
+ chown -R "${APP_UID}:${APP_GID}" /home/affine/.affine📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Create non-root user for security | |
| ARG APP_UID=1000 | |
| ARG APP_GID=1000 | |
| RUN groupadd -g "${APP_GID}" affine && \ | |
| useradd -m -u "${APP_UID}" -g "${APP_GID}" -s /usr/sbin/nologin affine && \ | |
| chown -R "${APP_UID}:${APP_GID}" /app | |
| # Create non-root user for security | |
| ARG APP_UID=1000 | |
| ARG APP_GID=1000 | |
| RUN groupadd -g "${APP_GID}" affine && \ | |
| useradd -m -u "${APP_UID}" -g "${APP_GID}" -s /usr/sbin/nologin affine && \ | |
| chown -R "${APP_UID}:${APP_GID}" /app && \ | |
| mkdir -p /home/affine/.affine/storage /home/affine/.affine/config && \ | |
| chown -R "${APP_UID}:${APP_GID}" /home/affine/.affine |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/deployment/node/Dockerfile around lines 30 - 35, The Dockerfile
introduces a non-root user (ARG APP_UID/APP_GID, useradd affine) but only chowns
/app, causing a mismatch with the app's homedir() (/home/affine) and mounted
volumes in compose; update the deployment by (1) changing mount targets in
.docker/selfhost/compose.yml from /root/.affine/* to /home/affine/.affine/* so
volumes match the affine user's homedir(), (2) modify the Dockerfile to create
the expected runtime directories (e.g., /home/affine/.affine/storage and
/home/affine/.affine/config) and chown them to "${APP_UID}:${APP_GID}" after
useradd, and (3) add a short note in docs/README about the breaking change for
existing deployments so operators can update their mounts.
- Changed /root/.affine/* to /home/affine/.affine/* in volume mounts - Ensures volumes match the affine user's homedir() Breaking change: Existing deployments need to update volume mounts
Fixes #14584
Add non-root user to Docker container for security.
Summary by CodeRabbit