fix: change Dockerfiles to not run as root#21
Conversation
Add non-root user configuration to api and web Dockerfiles to address SonarCloud security hotspot docker:S6471. The python and node base images run as root by default, which is a security risk. - api/Dockerfile: Create appuser (uid 1000) and switch to it - web/Dockerfile: Use built-in node user and switch to it
WalkthroughBoth Docker containers are hardened to run as non-root users (appuser for API, node for web) for improved security. The API container additionally integrates a startup sequence that performs database migrations before launching the application, with dynamic PORT environment variable support. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
api/Dockerfile (1)
34-34: Consider running migrations separately from container startup.Running
prisma migrate deployin the startup command can cause operational issues:
- Race conditions: Multiple replicas starting simultaneously may attempt concurrent migrations, causing database locks or conflicts
- Cascading failures: Migration failures prevent the entire service from starting, even if the application code is healthy
- Deployment coupling: Database schema changes are tightly coupled to deployment, making rollbacks complex
For production environments, consider:
- Running migrations as a separate pre-deployment step (e.g., Railway's release phase, init containers, or CI/CD pipeline)
- Using database migration locks/advisory locks to prevent concurrent execution
- Separating migration responsibility from application runtime
For development or single-instance deployments, the current approach may be acceptable.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
api/Dockerfileweb/Dockerfile
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-02T20:46:00.330Z
Learnt from: CR
Repo: platzhersh/open-cis PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T20:46:00.330Z
Learning: Copy .env.example to .env locally and configure environment variables: DATABASE_URL for Prisma app database connection, EHRBASE_URL for EHRBase REST API endpoint, CORS_ORIGINS for allowed origins JSON array, and VITE_API_URL for frontend API base URL
Applied to files:
api/Dockerfile
🔇 Additional comments (2)
api/Dockerfile (1)
22-27: LGTM! Non-root user configuration is correct.The implementation properly:
- Creates a dedicated user/group with explicit UID/GID 1000
- Transfers ownership of /app after all build operations complete
- Switches to non-root context before runtime
web/Dockerfile (1)
23-27: LGTM! Non-root user implementation looks correct.The security improvement correctly:
- Transfers ownership of /app to the built-in node user after all build operations
- Switches to non-root execution context
However, verify that the globally-installed
servecommand (installed at line 21 as root) is executable by the node user at runtime. While global npm packages are typically installed to world-executable locations, confirm this works as expected.#!/bin/bash # Verify serve is accessible to the node user after switching docker build -t test-web -f web/Dockerfile . docker run --rm test-web sh -c "which serve && serve --version"
Add non-root user configuration to api and web Dockerfiles to address SonarCloud security hotspot docker:S6471. The python and node base images run as root by default, which is a security risk.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.