feat: add security middleware and rate limiting#323
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR implements centralized production security hardening for the backend Express server. Environment configuration and npm dependencies are updated to introduce Helmet security headers and API rate limiting. CORS is switched from permissive to allowlist-based origin validation, and session cookies are hardened with secure attributes. The server startup flow is improved with environment validation and sensible defaults for database connection and port configuration. ChangesProduction Security Hardening for Backend
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/server.js`:
- Around line 73-80: The code currently falls back to insecure defaults (secret:
'dev-secret' and a localhost MONGO_URI) regardless of environment; update the
startup logic so that sessionSecret (SESSION_SECRET) and MONGO_URI are required
when NODE_ENV !== 'development' (or when not running on localhost), logging an
error and exiting process if missing, while preserving the existing dev-only
defaults only when NODE_ENV === 'development'; specifically, change the session
setup that references sessionSecret (used in app.use(session({...}))) to fail
fast in non-development, and apply the same check to wherever MONGO_URI /
mongoUri is read and passed to your Mongo connection function (e.g.,
mongoose.connect or connectToMongo) so the server aborts start when running in
production without those env vars.
- Around line 20-32: Set Express trust proxy before any middleware that depends
on the client IP or secure cookies: call app.set('trust proxy', 1) (or
appropriate trust value) before creating/using the rate limiter (the limiter/
rateLimit config) and before initializing the session middleware (the session
configuration that contains secure: true). Update the startup order so the call
to app.set('trust proxy', ...) appears above the Helmet line or at least before
the limiter variable and the session setup, ensuring rateLimit sees the real
client IP and the session cookie secure flag works correctly behind a reverse
proxy.
- Around line 79-89: The session middleware is using express-session's default
MemoryStore (app.use(session(...))) which is unsafe for production; replace it
with a production-ready store by configuring the session middleware to use a
store instance (e.g., RedisStore from connect-redis, MongoStore from
connect-mongo, or MemoryStore with pruning) and passing it as the store option
to session({ store, secret: sessionSecret || 'dev-secret', ... }); initialize
the chosen store client (e.g., new RedisClient or MongoClient) before creating
the session middleware, wire it into the existing app.use(session(...)) call,
and ensure sessionSecret and cookie settings remain unchanged while adding
graceful error handling for store connection failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 58b9520d-8d20-4645-bdd5-953ec2e3072c
📒 Files selected for processing (3)
backend/.env.samplebackend/package.jsonbackend/server.js
| app.use(session({ | ||
| secret: process.env.SESSION_SECRET, | ||
| secret: sessionSecret || 'dev-secret', | ||
| resave: false, | ||
| saveUninitialized: false, | ||
| cookie: { | ||
| httpOnly: true, | ||
| secure: process.env.NODE_ENV === 'production', | ||
| sameSite: 'lax', | ||
| maxAge: 1000 * 60 * 60 * 24, // 1 day | ||
| }, | ||
| })); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n backend/server.js | sed -n '70,100p'Repository: GitMetricsLab/github_tracker
Length of output: 976
🏁 Script executed:
rg -i "store.*session|session.*store|MemoryStore" backend/Repository: GitMetricsLab/github_tracker
Length of output: 152
🏁 Script executed:
cat backend/package.json | head -50Repository: GitMetricsLab/github_tracker
Length of output: 743
🏁 Script executed:
rg "session\(" backend/ -A 5 -B 2Repository: GitMetricsLab/github_tracker
Length of output: 646
🏁 Script executed:
rg "require.*session|import.*session" backend/Repository: GitMetricsLab/github_tracker
Length of output: 134
🌐 Web query:
express-session MemoryStore default production warning documentation
💡 Result:
The express-session middleware uses MemoryStore as its default session storage, which is explicitly documented as not designed for production environments [1][2]. The official documentation provides the following warning: "Warning: The default server-side session storage, MemoryStore, is purposely not designed for a production environment. It will leak memory under most conditions, does not scale past a single process, and is meant for debugging and developing" [1][2]. Key reasons for this warning include: - Memory Leaks: The default MemoryStore does not have a robust mechanism to prune expired sessions, leading to memory accumulation over time [3][4][5]. - Lack of Scalability: Because it stores sessions in the memory of a single Node.js process, it cannot be shared across multiple server instances or processes [1][2][6]. - Data Persistence: All session data is lost if the Node.js process restarts [3]. To address this in production, developers are advised to use a compatible, production-ready session store [1][7]. A common community-recommended solution for those who still require an in-memory store but need to avoid memory leaks is the 'memorystore' package, which implements automatic pruning of expired sessions [3][4][5]. For distributed or highly scalable applications, external stores such as Redis or databases (e.g., MongoDB) are standard practice [3][8][6].
Citations:
- 1: https://github.com/expressjs/session
- 2: https://expressjs.com/en/resources/middleware/session.html
- 3: https://stackoverflow.com/questions/10760620/using-memorystore-in-production
- 4: https://stackoverflow.com/questions/44882535/warning-connect-session-memorystore-is-not-designed-for-a-production-environm
- 5: https://github.com/roccomuso/memorystore
- 6: [Question]: Warning concerning connect.session() MemoryStore not designed for prod environment danny-avila/LibreChat#1014
- 7: Warning: connect.session() MemoryStore is not designed for a production environment expressjs/session#556
- 8: https://dev.to/sandeepbansod/rate-limiting-isnt-optional-here-how-to-actually-implement-it-in-nodejs-2can
Avoid the default in-process session store.
Because no store is configured on line 79, this uses express-session's default MemoryStore. The official documentation explicitly warns it is not designed for production, leaks memory, does not scale past a single process, and loses all session data on restart. Once this app runs on multiple instances, auth state will be lost and memory will accumulate over time.
Use a production-ready session store such as Redis, MongoDB (via connect-mongo), or memorystore with automatic expiration pruning.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/server.js` around lines 79 - 89, The session middleware is using
express-session's default MemoryStore (app.use(session(...))) which is unsafe
for production; replace it with a production-ready store by configuring the
session middleware to use a store instance (e.g., RedisStore from connect-redis,
MongoStore from connect-mongo, or MemoryStore with pruning) and passing it as
the store option to session({ store, secret: sessionSecret || 'dev-secret', ...
}); initialize the chosen store client (e.g., new RedisClient or MongoClient)
before creating the session middleware, wire it into the existing
app.use(session(...)) call, and ensure sessionSecret and cookie settings remain
unchanged while adding graceful error handling for store connection failures.
There was a problem hiding this comment.
Thanks for the review and suggestion.
I agree that a production-ready session store such as Redis or connect-mongo would be more suitable for scalable production deployments. However, since this PR is focused on introducing the initial security middleware improvements and production validation checks, I’d prefer to keep session store migration as a separate enhancement to avoid significantly expanding the scope of this contribution.
There was a problem hiding this comment.
@DineshSivalanka, that's a fair call — keeping this PR focused on the core security middleware improvements is a reasonable scoping decision. The MemoryStore concern is real but non-blocking for the initial hardening step.
Would you like me to open a follow-up GitHub issue to track the session store migration (e.g., connect-mongo or Redis) so it doesn't get lost? I can link it back to this PR for context.
🐇 _/_
( ´ω`)
/ >🎫 follow-up issue incoming?
1 participant
Related Issue
Description
Added backend security improvements and middleware enhancements to improve API protection and application security.
Implemented:
How Has This Been Tested?
npm run devScreenshots (if applicable)
N/A
Type of Change
Summary by CodeRabbit
New Features
Improvements
Chores