Conversation
WalkthroughThe pull request switches the Dockerfile base images from Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 |
🔍 Vulnerabilities of
|
| digest | sha256:90f6b12d34cfdc6482579c3e179af7200c0061748fc4031365aa52fd16aec134 |
| vulnerabilities | |
| platform | linux/amd64 |
| size | 173 MB |
| packages | 975 |
📦 Base Image node:24-alpine
Description
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Description
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile (1)
1-37: 🧹 Nitpick | 🔵 TrivialAdd HEALTHCHECK instruction for container orchestration.
Both Trivy and Checkov flag the missing HEALTHCHECK. Without it, orchestrators (Kubernetes, Docker Swarm, ECS) cannot determine if the application inside the container is actually healthy and serving requests.
🩺 Suggested HEALTHCHECK
# Switch to non-root user USER appuser # Set working directory WORKDIR /app +# Health check for container orchestration +HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \ + CMD node -e "require('http').get('http://localhost:3000/health', (r) => process.exit(r.statusCode === 200 ? 0 : 1)).on('error', () => process.exit(1))" + # Expose the port the app runs on EXPOSE 3000This requires a
/healthendpoint in your Express app. Alternatively, usewgetorcurlif available in the Alpine image.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 1 - 37, Add a Docker HEALTHCHECK instruction to the Dockerfile so orchestrators can probe the app; add a HEALTHCHECK that periodically curl/wget http://localhost:3000/health (or your Express /health route) and returns non-zero on failure, placed after EXPOSE 3000 and before CMD ["npm","start"]. Ensure your app exposes a /health handler in the Express server (e.g., in the code that starts under npm start) that returns 200 when healthy; if curl/wget are not present in the base image, either install a tiny probe utility during the build stage or switch the healthcheck command to use a shell-built-in check appropriate for node.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Line 24: Typo in the Dockerfile comment: change the word "wfrom" to "from" in
the comment line that reads "Copy layer wfrom build image" so it becomes "Copy
layer from build image"; locate this in the Dockerfile comment and correct the
spelling.
In `@website/modules/`@apostrophecms/express/index.js:
- Around line 11-19: The request-logging middleware (middleware -> logRequests
-> middleware) uses morgan unconditionally; make it environment-aware by
checking NODE_ENV or an equivalent runtime config and only registering or using
the verbose morgan format in non-production (or when a config flag like
ENABLE_REQUEST_LOGGING is true). Modify the middleware factory to read
process.env.NODE_ENV (or the module's options) and conditionally return the
logRequests entry (or swap to a compact/obfuscated format) so production avoids
the current verbose morgan(':date[iso] :method :url :status :response-time ms -
:remote-addr - :user-agent') while dev/test keep it.
---
Outside diff comments:
In `@Dockerfile`:
- Around line 1-37: Add a Docker HEALTHCHECK instruction to the Dockerfile so
orchestrators can probe the app; add a HEALTHCHECK that periodically curl/wget
http://localhost:3000/health (or your Express /health route) and returns
non-zero on failure, placed after EXPOSE 3000 and before CMD ["npm","start"].
Ensure your app exposes a /health handler in the Express server (e.g., in the
code that starts under npm start) that returns 200 when healthy; if curl/wget
are not present in the base image, either install a tiny probe utility during
the build stage or switch the healthcheck command to use a shell-built-in check
appropriate for node.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 19897063-72dc-4ae7-9a88-73ee937b44c9
⛔ Files ignored due to path filters (1)
website/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
Dockerfilewebsite/modules/@apostrophecms/express/index.jswebsite/package.json
| # Create a non-root user and group | ||
| RUN addgroup -S appgroup && adduser -S -G appgroup appuser | ||
|
|
||
| # Copy layer wfrom build image |
There was a problem hiding this comment.
Typo in comment.
wfrom → from
-# Copy layer wfrom build image
+# Copy layer from build image📝 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.
| # Copy layer wfrom build image | |
| # Copy layer from build image |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile` at line 24, Typo in the Dockerfile comment: change the word
"wfrom" to "from" in the comment line that reads "Copy layer wfrom build image"
so it becomes "Copy layer from build image"; locate this in the Dockerfile
comment and correct the spelling.
| middleware(_self) { | ||
| return { | ||
| logRequests: { | ||
| middleware: morgan( | ||
| ':date[iso] :method :url :status :response-time ms - :remote-addr - :user-agent', | ||
| ), | ||
| }, | ||
| }; | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider making request logging environment-aware.
The morgan middleware is unconditionally enabled for all environments. In production, verbose request logging can:
- Generate significant log volume
- Potentially impact performance on high-traffic endpoints
- Log sensitive URL parameters
Consider gating logging or adjusting the format based on environment:
♻️ Suggested approach
+const { getEnv } = require('../../../utils/env');
+
module.exports = {
options: {
session: {
secret: getEnv('SESSION_SECRET'),
},
},
middleware(_self) {
+ const nodeEnv = getEnv('NODE_ENV') || 'development';
+ const format = nodeEnv === 'production'
+ ? 'combined'
+ : ':date[iso] :method :url :status :response-time ms - :remote-addr - :user-agent';
return {
logRequests: {
- middleware: morgan(
- ':date[iso] :method :url :status :response-time ms - :remote-addr - :user-agent',
- ),
+ middleware: morgan(format),
},
};
},
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/modules/`@apostrophecms/express/index.js around lines 11 - 19, The
request-logging middleware (middleware -> logRequests -> middleware) uses morgan
unconditionally; make it environment-aware by checking NODE_ENV or an equivalent
runtime config and only registering or using the verbose morgan format in
non-production (or when a config flag like ENABLE_REQUEST_LOGGING is true).
Modify the middleware factory to read process.env.NODE_ENV (or the module's
options) and conditionally return the logRequests entry (or swap to a
compact/obfuscated format) so production avoids the current verbose
morgan(':date[iso] :method :url :status :response-time ms - :remote-addr -
:user-agent') while dev/test keep it.
The PR makes two main changes: (1) switches the Docker base image from node:24-slim to node:24-alpine to reduce the container image size, updating user creation commands for Alpine compatibility, and (2) adds request logging functionality using the morgan middleware to the Express configuration with detailed logging including timestamps, HTTP methods, URLs, status codes, and response times.