Skip to content

[Security] Harden session cookie with HttpOnly, Secure, and SameSite flags #456

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
advikdivekar wants to merge 1 commit into from
Closed

[Security] Harden session cookie with HttpOnly, Secure, and SameSite flags #456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions backend/Dockerfile.prod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ RUN npm install --production
# Copy the rest of the application files
COPY . .

# Set production environment so cookie security flags are applied
ENV NODE_ENV=production

# Expose the port for the application
EXPOSE 5000

Expand Down
6 changes: 6 additions & 0 deletions backend/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ app.use(session({
secret: process.env.SESSION_SECRET,
resave: false,
saveUninitialized: false,
cookie: {
httpOnly: true,
secure: process.env.NODE_ENV === 'production',
sameSite: process.env.NODE_ENV === 'production' ? 'strict' : 'lax',
Comment on lines +25 to +26
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check whether the backend already trusts an upstream proxy
rg -nP --type=js -C2 "app\.set\(\s*['\"]trust proxy['\"]|proxy\s*:\s*true" backend

Repository: GitMetricsLab/github_tracker

Length of output: 54

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== trust proxy (repo-wide) =="
rg -n --type=js "trust proxy" .

echo "== express-session usage (repo-wide) =="
rg -n --type=js "express-session" .

echo "== session cookie options (repo-wide) =="
rg -n --type=js "cookie\s*:\s*\{[^}]*secure" . || true

Repository: GitMetricsLab/github_tracker

Length of output: 102

🏁 Script executed:

#!/bin/bash
set -u

echo "== trust proxy (repo-wide) =="
rg -n --type=js "trust proxy" . || true

echo "== express-session usage (repo-wide) =="
rg -n --type=js "express-session" . || true

echo "== session middleware / cookie secure options (repo-wide) =="
rg -n --type=js -S "cookie\s*:\s*\{|\bcookie\b.*secure:|secure:\s*process\.env\.NODE_ENV\s*===\s*'production'|secure:\s*true" .

echo "== likely app/session config files (backend) =="
ls -la backend

Repository: GitMetricsLab/github_tracker

Length of output: 1146

🏁 Script executed:

#!/bin/bash
set -euo pipefail
nl -ba backend/server.js | sed -n '1,90p'

Repository: GitMetricsLab/github_tracker

Length of output: 115

🏁 Script executed:

#!/bin/bash
set -euo pipefail
awk 'NR>=1 && NR<=90 {print NR ":" $0}' backend/server.js

Repository: GitMetricsLab/github_tracker

Length of output: 1416

Set trust proxy when behind a TLS-terminating reverse proxy/ingress.

backend/server.js configures express-session cookies with secure: process.env.NODE_ENV === 'production', but the Express app never sets app.set('trust proxy', ...). If HTTPS is terminated upstream, Express may not treat requests as secure, and the cookie may not be set in production—leading to broken sessions/auth. Add app.set('trust proxy', <value>) (and ensure the proxy forwards X-Forwarded-Proto) when deployed behind a proxy.

🤖 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 25 - 26, Express is configuring session
cookies with secure: process.env.NODE_ENV === 'production' but never sets
app.set('trust proxy', ...), so when HTTPS is terminated upstream the app won't
see requests as secure and cookies may not be sent; add a call like
app.set('trust proxy', 1) (or app.set('trust proxy', process.env.TRUST_PROXY ||
1) / conditional when process.env.NODE_ENV === 'production') in the server
initialization (before express-session is used) to ensure req.secure is
populated, and confirm the ingress/proxy forwards X-Forwarded-Proto.

maxAge: 24 * 60 * 60 * 1000,
},
}));
app.use(passport.initialize());
app.use(passport.session());
Expand Down
Loading