diff --git a/.env.example b/.env.example index fa7567e..7e0112b 100644 --- a/.env.example +++ b/.env.example @@ -8,6 +8,13 @@ SERVER_URL=http://localhost:3001 PORT=3001 NODE_ENV=production +# Comma-separated CORS allowlist. Defaults to CLIENT_URL when unset. +# ALLOWED_ORIGINS=http://localhost:3001,https://gne.example.com + +# Set to true only when the app is served over HTTPS (TLS-terminating proxy). +# Required for the session cookie to carry the Secure attribute. +# COOKIE_SECURE=true + # For local dev (separate Vite dev server + Express) # CLIENT_URL=http://localhost:5173 # SERVER_URL=http://localhost:3001 diff --git a/package.json b/package.json index 76ea330..7bbcb57 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,8 @@ "postcss": ">=8.5.18", "@babel/core": ">=7.29.6", "nanoid": "3.3.16", - "tsx": "4.23.4" + "tsx": "4.23.4", + "qs": ">=6.16.0" } } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 81405dd..f0d7d97 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -9,6 +9,7 @@ overrides: '@babel/core': '>=7.29.6' nanoid: 3.3.16 tsx: 4.23.4 + qs: '>=6.16.0' importers: @@ -532,6 +533,7 @@ packages: engines: {node: ^22.20 || ^24.12 || >=25} cpu: [x64] os: [linux] + libc: [glibc] '@octokit/auth-token@5.1.2': resolution: {integrity: sha512-JcQDsBdg49Yky2w2ld20IHAlwr8d/d8N6NiOXbtuoPCqzbsiJgF633mVUw3x4mo0H5ypataQIX7SFu3yy44Mpw==} @@ -628,66 +630,79 @@ packages: resolution: {integrity: sha512-81wiiX3v7aqy+T+bT61TJ78yJjRquqFFTTbAPt08imfQQzkPIW8t6aJbkTagtCCrXMNc9D66+geqlK7ydLPNqA==} cpu: [arm] os: [linux] + libc: [glibc] '@rollup/rollup-linux-arm-musleabihf@4.62.4': resolution: {integrity: sha512-9kmDIvNZqdoHOBZgNtpTBeLWYO/LVipM3H/j62P8848/l/VPEQL6N3uxU9pvP1oZAsXyC2MEnFP3ovRjo7WYNQ==} cpu: [arm] os: [linux] + libc: [musl] '@rollup/rollup-linux-arm64-gnu@4.62.4': resolution: {integrity: sha512-CcnXHWnXg69g+DX5VWL3FHts3qMRN2uVEHX+BZvGLdd07/gXkn3ePjYtO1LDJvxkGKVHMclKBRa1QUTH+6toYQ==} cpu: [arm64] os: [linux] + libc: [glibc] '@rollup/rollup-linux-arm64-musl@4.62.4': resolution: {integrity: sha512-iFOibiHnTRuhrWLlRsOQFdZJJIa7S8OwkneJr4ocALP16u5yk6lWLINFwhHaEqBFMsKDUZofLkGos7+CPzGB3g==} cpu: [arm64] os: [linux] + libc: [musl] '@rollup/rollup-linux-loong64-gnu@4.62.4': resolution: {integrity: sha512-XnWYMI7euHlb5a871xPja+Gm7DRCFU+FGRrtS2sMq9N8FvqtpagUy6gD4YOemC5MRk9xbh8+jYMEJbigFQwsgA==} cpu: [loong64] os: [linux] + libc: [glibc] '@rollup/rollup-linux-loong64-musl@4.62.4': resolution: {integrity: sha512-qGDAlO0U8xedCcsdRm9oaoQY8DAx/QT7uIxJWhCdx0ceIWX783UC9QSYkdpzAe29wNiVfp24+bZdQmn49o45SQ==} cpu: [loong64] os: [linux] + libc: [musl] '@rollup/rollup-linux-ppc64-gnu@4.62.4': resolution: {integrity: sha512-ru4H6ezD7ysA5EiEK6qkkaEb4modH8CTej6kUy/gQi20u3kB3G7Zn8snXXkeJSCOFKG/rbPPtM/+9Wgas1961w==} cpu: [ppc64] os: [linux] + libc: [glibc] '@rollup/rollup-linux-ppc64-musl@4.62.4': resolution: {integrity: sha512-2W4MO5WQVJnbJaZdvDb9rhBDuFU1nKIepPFpJUBsTh2k1YY2g+ODViaWuyOAjQ5cOP7NvrvLzt3wvHOoiAvc7w==} cpu: [ppc64] os: [linux] + libc: [musl] '@rollup/rollup-linux-riscv64-gnu@4.62.4': resolution: {integrity: sha512-+fxjfuoAmVMCYV5QyjoIpu0cp5DOiOTeqYFk1AVaxGr+/ravWLX89XfQmptsoWcaVy/TGf2hexzbUOrCQIL1CQ==} cpu: [riscv64] os: [linux] + libc: [glibc] '@rollup/rollup-linux-riscv64-musl@4.62.4': resolution: {integrity: sha512-jTn8JfHGL4djjFxPuM06LmNUJDsst2jeVlsd9OmIH6zc5sC9K6rIuO4YajXatLUpBmBKl6b35ro1QZocLi+tcA==} cpu: [riscv64] os: [linux] + libc: [musl] '@rollup/rollup-linux-s390x-gnu@4.62.4': resolution: {integrity: sha512-oCJCJL4pXsoDcP2QZ+JVlPTIRc6266zsIaeJJsWImmF7HO0W8nb6HuSgZlMWxJwaPf8ehbSw8yo0EUw925hKsA==} cpu: [s390x] os: [linux] + libc: [glibc] '@rollup/rollup-linux-x64-gnu@4.62.4': resolution: {integrity: sha512-W69hukhZ3KKNRCaMIEzKvcFye42hh0FE1+YoYaf5+Ikacuftoco6yO/xouz0hc5d5W/s3yBro5jRiuEE/Q5vUw==} cpu: [x64] os: [linux] + libc: [glibc] '@rollup/rollup-linux-x64-musl@4.62.4': resolution: {integrity: sha512-qiXbGG2jkjXhzXpsFZSR2Xpb8DN/UaxYsbb/STbuR/6fpaDgRmmaq1B/LmtF2wQFOFOSsK2jdE0RZ3a0zHn4QA==} cpu: [x64] os: [linux] + libc: [musl] '@rollup/rollup-openbsd-x64@4.62.4': resolution: {integrity: sha512-nWeM//hxv8mIo6jD7Hu4o48DVmV9pbV6gsKaWU+4NFyqHoPKwrkRiZGLKUhOBk8qNmDmpwFtPKg80Bo/Tn4xiQ==} @@ -757,24 +772,28 @@ packages: engines: {node: '>= 20'} cpu: [arm64] os: [linux] + libc: [glibc] '@tailwindcss/oxide-linux-arm64-musl@4.3.3': resolution: {integrity: sha512-Md44bD6veX/PC5iyF8cDVnw4HBIANZepRZZ7a8DQOvkfo5WUBwcp6iAuCUz23u+4SUkhJlD3eL7hNdW8ezd/kA==} engines: {node: '>= 20'} cpu: [arm64] os: [linux] + libc: [musl] '@tailwindcss/oxide-linux-x64-gnu@4.3.3': resolution: {integrity: sha512-tx7us1muwOKAKWao2v/GaafFeQboE6aj88vC6ziN2NCGcRm8gWUhwjzg+YdVB1e4boAtdtma4L43onunI6NS4w==} engines: {node: '>= 20'} cpu: [x64] os: [linux] + libc: [glibc] '@tailwindcss/oxide-linux-x64-musl@4.3.3': resolution: {integrity: sha512-SJxX60smvHgasZoBy11dX6YRjXJFovwWBoedhbQPOBzgFWBHGB+TVPWB9BxzR7TTxU8FQZAI2AyiNCMzFm8Img==} engines: {node: '>= 20'} cpu: [x64] os: [linux] + libc: [musl] '@tailwindcss/oxide-wasm32-wasi@4.3.3': resolution: {integrity: sha512-jx1+rPhY/5Ympkktd656HBWEBLxP7dH06losBLjjf5vgCODXvi9KhtftWcMIwTFIDqBr7cRnQkdLnAG+IOlGvQ==} @@ -1491,24 +1510,28 @@ packages: engines: {node: '>= 12.0.0'} cpu: [arm64] os: [linux] + libc: [glibc] lightningcss-linux-arm64-musl@1.32.0: resolution: {integrity: sha512-UpQkoenr4UJEzgVIYpI80lDFvRmPVg6oqboNHfoH4CQIfNA+HOrZ7Mo7KZP02dC6LjghPQJeBsvXhJod/wnIBg==} engines: {node: '>= 12.0.0'} cpu: [arm64] os: [linux] + libc: [musl] lightningcss-linux-x64-gnu@1.32.0: resolution: {integrity: sha512-V7Qr52IhZmdKPVr+Vtw8o+WLsQJYCTd8loIfpDaMRWGUZfBOYEJeyJIkqGIDMZPwPx24pUMfwSxxI8phr/MbOA==} engines: {node: '>= 12.0.0'} cpu: [x64] os: [linux] + libc: [glibc] lightningcss-linux-x64-musl@1.32.0: resolution: {integrity: sha512-bYcLp+Vb0awsiXg/80uCRezCYHNg1/l3mt0gzHnWV9XP1W5sKa5/TCdGWaR/zBM2PeF/HbsQv/j2URNOiVuxWg==} engines: {node: '>= 12.0.0'} cpu: [x64] os: [linux] + libc: [musl] lightningcss-win32-arm64-msvc@1.32.0: resolution: {integrity: sha512-8SbC8BR40pS6baCM8sbtYDSwEVQd4JlFTOlaD3gWGHfThTcABnNDBda6eTZeqbofalIJhFx0qKzgHJmcPTnGdw==} @@ -1638,8 +1661,8 @@ packages: resolution: {integrity: sha512-llQsMLSUDUPT44jdrU/O37qlnifitDP+ZwrmmZcoSKyLKvtZxpyV0n2/bD/N4tBAAZ/gJEdZU7KMraoK1+XYAg==} engines: {node: '>= 0.10'} - qs@6.15.3: - resolution: {integrity: sha512-O9gl3zCl5h5blw1KGUzQKhA5oUXSl8rwUIM5o0S3nCXMliSvy5Dzx7/DJcI+SwgICv+IneSZwhBh1oSyEHA71A==} + qs@6.16.0: + resolution: {integrity: sha512-h6fhOIaRrID2CbEY2fqs+7t+UXZo+MLAnU5gRIq85uFtdiUPCdsApMlHhXogKVM4HM2DVbIjGNTTYH2OcmP1vA==} engines: {node: '>=0.6'} quadprog@1.6.1: @@ -2660,7 +2683,7 @@ snapshots: http-errors: 2.0.1 iconv-lite: 0.4.24 on-finished: 2.4.1 - qs: 6.15.3 + qs: 6.16.0 raw-body: 2.5.3 type-is: 1.6.18 unpipe: 1.0.0 @@ -3044,7 +3067,7 @@ snapshots: parseurl: 1.3.3 path-to-regexp: 0.1.13 proxy-addr: 2.0.7 - qs: 6.15.3 + qs: 6.16.0 range-parser: 1.2.1 safe-buffer: 5.2.1 send: 0.19.2 @@ -3301,7 +3324,7 @@ snapshots: forwarded: 0.2.0 ipaddr.js: 1.9.1 - qs@6.15.3: + qs@6.16.0: dependencies: es-define-property: 1.0.1 side-channel: 1.1.1 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index d7947f3..9c36538 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -7,3 +7,4 @@ overrides: "@babel/core": ">=7.29.6" nanoid: "3.3.16" tsx: "4.23.4" + qs: ">=6.16.0" diff --git a/server/package-lock.json b/server/package-lock.json index 01a9f21..236019c 100644 --- a/server/package-lock.json +++ b/server/package-lock.json @@ -12,6 +12,7 @@ "@octokit/rest": "^21.0.2", "cors": "^2.8.5", "express": "^4.22.2", + "express-rate-limit": "^8.7.0", "express-session": "^1.18.1", "helmet": "^8.0.0", "node-cache": "^5.1.2" @@ -1108,6 +1109,48 @@ "url": "https://opencollective.com/express" } }, + "node_modules/express-rate-limit": { + "version": "8.7.0", + "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-8.7.0.tgz", + "integrity": "sha512-hOwV7WOxXfjRpAM1DSJWZDXx3GhplwD8IfwuwvogD8i1Qnkgosw/H45s4ZnFAUHDAhPjlY9hLBvJhKmGMyY26g==", + "license": "MIT", + "dependencies": { + "debug": "^4.4.3", + "ip-address": "^10.2.0" + }, + "engines": { + "node": ">= 16" + }, + "funding": { + "url": "https://github.com/sponsors/express-rate-limit" + }, + "peerDependencies": { + "express": ">= 4.11" + } + }, + "node_modules/express-rate-limit/node_modules/debug": { + "version": "4.4.3", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.4.3.tgz", + "integrity": "sha512-RGwwWnwQvkVfavKVt22FGLw+xYSdzARwm0ru6DhTVA3umU5hZc28V3kO4stgYryrTlLpuvgI9GiijltAjNbcqA==", + "license": "MIT", + "dependencies": { + "ms": "^2.1.3" + }, + "engines": { + "node": ">=6.0" + }, + "peerDependenciesMeta": { + "supports-color": { + "optional": true + } + } + }, + "node_modules/express-rate-limit/node_modules/ms": { + "version": "2.1.3", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz", + "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==", + "license": "MIT" + }, "node_modules/express-session": { "version": "1.19.0", "resolved": "https://registry.npmjs.org/express-session/-/express-session-1.19.0.tgz", @@ -1327,6 +1370,15 @@ "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==", "license": "ISC" }, + "node_modules/ip-address": { + "version": "10.7.0", + "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.7.0.tgz", + "integrity": "sha512-BGFsyJd5mpXp3rK6jIdADLNgpJUK1jnjzvYF8lK+VyDab9JAmqN0YOKDdP17HlgKb2+ehPgDc8EtnRLbGCAMhA==", + "license": "MIT", + "engines": { + "node": ">= 12" + } + }, "node_modules/ipaddr.js": { "version": "1.9.1", "resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-1.9.1.tgz", @@ -1503,9 +1555,9 @@ } }, "node_modules/qs": { - "version": "6.15.3", - "resolved": "https://registry.npmjs.org/qs/-/qs-6.15.3.tgz", - "integrity": "sha512-O9gl3zCl5h5blw1KGUzQKhA5oUXSl8rwUIM5o0S3nCXMliSvy5Dzx7/DJcI+SwgICv+IneSZwhBh1oSyEHA71A==", + "version": "6.16.0", + "resolved": "https://registry.npmjs.org/qs/-/qs-6.16.0.tgz", + "integrity": "sha512-h6fhOIaRrID2CbEY2fqs+7t+UXZo+MLAnU5gRIq85uFtdiUPCdsApMlHhXogKVM4HM2DVbIjGNTTYH2OcmP1vA==", "license": "BSD-3-Clause", "dependencies": { "es-define-property": "^1.0.1", diff --git a/server/package.json b/server/package.json index 17c7833..6400560 100644 --- a/server/package.json +++ b/server/package.json @@ -12,6 +12,7 @@ "@octokit/rest": "^21.0.2", "cors": "^2.8.5", "express": "^4.22.2", + "express-rate-limit": "^8.7.0", "express-session": "^1.18.1", "helmet": "^8.0.0", "node-cache": "^5.1.2" @@ -23,5 +24,8 @@ "@types/node": "^22.10.7", "tsx": "^4.23.5", "typescript": "^5.7.3" + }, + "overrides": { + "qs": ">=6.16.0" } } diff --git a/server/src/index.ts b/server/src/index.ts index 677a594..d86aa55 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -2,6 +2,7 @@ import express from 'express'; import helmet from 'helmet'; import cors from 'cors'; import session from 'express-session'; +import rateLimit from 'express-rate-limit'; import path from 'path'; import { authRouter } from './routes/auth.routes.js'; import { repoRouter } from './routes/repo.routes.js'; @@ -12,36 +13,105 @@ import { GitHubService } from './services/github.service.js'; const app = express(); const PORT = parseInt(process.env.PORT ?? '3001', 10); const CLIENT_URL = process.env.CLIENT_URL ?? 'http://localhost:5173'; -const SESSION_SECRET = process.env.SESSION_SECRET ?? 'dev-secret-change-in-production'; const NODE_ENV = process.env.NODE_ENV ?? 'development'; +const IS_PRODUCTION = NODE_ENV === 'production'; + +// Never fall back to a hardcoded session secret in production: a known secret +// lets anyone forge session cookies and impersonate authenticated users. +const SESSION_SECRET = process.env.SESSION_SECRET ?? ''; +if (IS_PRODUCTION && SESSION_SECRET.length < 32) { + throw new Error( + 'SESSION_SECRET must be set to at least 32 characters when NODE_ENV=production' + ); +} +const RESOLVED_SESSION_SECRET = + SESSION_SECRET || 'dev-secret-change-in-production'; + +// Comma-separated allowlist; falls back to CLIENT_URL. +const ALLOWED_ORIGINS = (process.env.ALLOWED_ORIGINS ?? CLIENT_URL) + .split(',') + .map((o) => o.trim()) + .filter(Boolean); + +// Required so express-rate-limit and secure cookies see the real client IP / +// protocol behind a single reverse proxy. Not `true`, which trusts any hop. +app.set('trust proxy', 1); // Security middleware -app.use(helmet({ - contentSecurityPolicy: false, // Disable CSP — default includes upgrade-insecure-requests which forces HTTPS - hsts: false, // Disable HSTS — we don't terminate TLS here -})); +app.use( + helmet({ + // Explicit CSP without upgrade-insecure-requests so HTTP deployments keep + // working, while still blocking injected script/object/frame sources. + contentSecurityPolicy: { + useDefaults: false, + directives: { + defaultSrc: ["'self'"], + baseUri: ["'self'"], + frameAncestors: ["'none'"], + objectSrc: ["'none'"], + scriptSrc: ["'self'"], + styleSrc: ["'self'", "'unsafe-inline'"], + imgSrc: ["'self'", 'data:', 'https://avatars.githubusercontent.com'], + connectSrc: ["'self'", 'https://api.github.com'], + fontSrc: ["'self'", 'data:'], + formAction: ["'self'", 'https://github.com'], + }, + }, + hsts: false, // Disable HSTS — we don't terminate TLS here + referrerPolicy: { policy: 'no-referrer' }, + crossOriginResourcePolicy: { policy: 'same-origin' }, + }) +); // CORS app.use( cors({ - origin: CLIENT_URL, + origin: (origin, callback) => { + // Same-origin / non-browser requests send no Origin header. + if (!origin || ALLOWED_ORIGINS.includes(origin)) { + callback(null, true); + return; + } + callback(new Error('Not allowed by CORS')); + }, credentials: true, + methods: ['GET', 'POST', 'OPTIONS'], }) ); -// Body parsing -app.use(express.json()); -app.use(express.urlencoded({ extended: true })); +// Body parsing — bounded to avoid trivial memory-exhaustion DoS +app.use(express.json({ limit: '100kb' })); +app.use(express.urlencoded({ extended: true, limit: '100kb' })); + +// Rate limiting: protects the upstream GitHub token from being burned by an +// attacker and limits brute-forcing of the OAuth endpoints. +const apiLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, + limit: 600, + standardHeaders: 'draft-7', + legacyHeaders: false, + message: { error: 'Too many requests, please try again later.' }, +}); + +const authLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, + limit: 30, + standardHeaders: 'draft-7', + legacyHeaders: false, + message: { error: 'Too many authentication attempts, please try again later.' }, +}); // Session app.use( session({ - secret: SESSION_SECRET, + secret: RESOLVED_SESSION_SECRET, + name: 'gne.sid', // Avoid advertising the default connect.sid fingerprint resave: false, saveUninitialized: false, cookie: { httpOnly: true, - secure: false, // Set to true only when behind a TLS-terminating reverse proxy + // Opt in via COOKIE_SECURE=true when behind a TLS-terminating proxy. + secure: process.env.COOKIE_SECURE === 'true', maxAge: 7 * 24 * 60 * 60 * 1000, // 7 days sameSite: 'lax', // Must be 'lax' for OAuth redirect flow to work }, @@ -49,12 +119,12 @@ app.use( ); // API routes -app.use('/api/auth', authRouter); -app.use('/api/repos', repoRouter); -app.use('/api/orgs', orgRouter); +app.use('/api/auth', authLimiter, authRouter); +app.use('/api/repos', apiLimiter, repoRouter); +app.use('/api/orgs', apiLimiter, orgRouter); // GET /api/rate-limit — returns GitHub API rate limit (no cache) -app.get('/api/rate-limit', requireAuth, async (req, res): Promise => { +app.get('/api/rate-limit', apiLimiter, requireAuth, async (req, res): Promise => { try { const service = new GitHubService(req.session.accessToken!); const data = await service.getRateLimit(); @@ -71,7 +141,7 @@ app.get('/health', (_req, res) => { }); // Serve static client files in production -if (NODE_ENV === 'production') { +if (IS_PRODUCTION) { const clientDistPath = path.join(__dirname, '../../client/dist'); app.use(express.static(clientDistPath)); diff --git a/server/src/routes/auth.routes.ts b/server/src/routes/auth.routes.ts index 3d6f031..1ad845b 100644 --- a/server/src/routes/auth.routes.ts +++ b/server/src/routes/auth.routes.ts @@ -31,11 +31,23 @@ router.get('/callback', async (req: Request, res: Response): Promise => { return; } - if (state !== req.session.oauthState) { + const expectedState = req.session.oauthState; + // Constant-time comparison; also rejects missing/non-string state outright so + // a request with no prior /github call can never satisfy the CSRF check. + if ( + typeof state !== 'string' || + typeof expectedState !== 'string' || + state.length !== expectedState.length || + !crypto.timingSafeEqual(Buffer.from(state), Buffer.from(expectedState)) + ) { + delete req.session.oauthState; res.redirect(`${clientUrl}?error=invalid_state`); return; } + // Single-use state: prevents replaying a captured callback URL. + delete req.session.oauthState; + try { const tokenResponse = await fetch( 'https://github.com/login/oauth/access_token', @@ -64,10 +76,25 @@ router.get('/callback', async (req: Request, res: Response): Promise => { return; } - req.session.accessToken = tokenData.access_token; - delete req.session.oauthState; - - res.redirect(clientUrl); + // Regenerate the session ID on privilege change to defeat session fixation: + // an attacker-planted pre-auth cookie must not become an authenticated one. + const accessToken = tokenData.access_token; + req.session.regenerate((regenErr) => { + if (regenErr) { + console.error('Session regenerate error:', regenErr); + res.redirect(`${clientUrl}?error=server_error`); + return; + } + req.session.accessToken = accessToken; + req.session.save((saveErr) => { + if (saveErr) { + console.error('Session save error:', saveErr); + res.redirect(`${clientUrl}?error=server_error`); + return; + } + res.redirect(clientUrl); + }); + }); } catch (err) { console.error('OAuth callback error:', err); res.redirect(`${clientUrl}?error=server_error`); @@ -125,6 +152,8 @@ router.post('/logout', (req: Request, res: Response): void => { res.status(500).json({ error: 'Logout failed' }); return; } + // Clear the cookie too, so a stale session id is not replayed. + res.clearCookie('gne.sid'); res.json({ success: true }); }); }); diff --git a/server/src/routes/org.routes.ts b/server/src/routes/org.routes.ts index 4a059cf..deb2c77 100644 --- a/server/src/routes/org.routes.ts +++ b/server/src/routes/org.routes.ts @@ -11,14 +11,33 @@ function getGitHubService(req: Request): GitHubService { return new GitHubService(req.session.accessToken!); } +// Per-user cache namespace: GitHub responses depend on the caller's grants, so +// a shared key can serve private org data to a user who cannot see it. +function scope(req: Request): string { + return cacheService.userScope(req.session.accessToken!); +} + function str(param: string | string[] | undefined): string { if (Array.isArray(param)) return param[0] ?? ''; return param ?? ''; } +// Bounds user-controlled pagination so a request cannot ask for an unbounded +// page size and amplify load against the GitHub API. +function clampInt( + raw: unknown, + fallback: number, + min: number, + max: number +): number { + const parsed = typeof raw === 'string' ? parseInt(raw, 10) : NaN; + if (!Number.isFinite(parsed)) return fallback; + return Math.min(Math.max(parsed, min), max); +} + // GET /api/orgs - list user organizations router.get('/', async (req: Request, res: Response): Promise => { - const cacheKey = cacheService.cacheKey(['orgs', req.session.accessToken!.slice(-8)]); + const cacheKey = cacheService.cacheKey(['orgs', scope(req)]); const cached = cacheService.get(cacheKey); if (cached) { res.json(cached); @@ -39,10 +58,10 @@ router.get('/', async (req: Request, res: Response): Promise => { // GET /api/orgs/:org/repos?page=1&per_page=30 - paginated org repos router.get('/:org/repos', async (req: Request, res: Response): Promise => { const org = str(req.params['org']); - const page = parseInt(typeof req.query.page === 'string' ? req.query.page : '1', 10) || 1; - const perPage = parseInt(typeof req.query.per_page === 'string' ? req.query.per_page : '30', 10) || 30; + const page = clampInt(req.query.page, 1, 1, 1000); + const perPage = clampInt(req.query.per_page, 30, 1, 100); - const cacheKey = cacheService.cacheKey(['org-repos', org, String(page), String(perPage)]); + const cacheKey = cacheService.cacheKey(['org-repos', scope(req), org, String(page), String(perPage)]); const cached = cacheService.get(cacheKey); if (cached) { res.json(cached); diff --git a/server/src/routes/repo.routes.ts b/server/src/routes/repo.routes.ts index cded430..1f94c38 100644 --- a/server/src/routes/repo.routes.ts +++ b/server/src/routes/repo.routes.ts @@ -17,15 +17,34 @@ function str(param: string | string[] | undefined): string { return param ?? ''; } +// Per-user cache namespace: GitHub responses depend on the caller's grants, so +// a shared key can serve one user's private repository data to another. +function scope(req: Request): string { + return cacheService.userScope(req.session.accessToken!); +} + +// Bounds user-controlled pagination so a request cannot ask for an unbounded +// page size and amplify load against the GitHub API. +function clampInt( + raw: unknown, + fallback: number, + min: number, + max: number +): number { + const parsed = typeof raw === 'string' ? parseInt(raw, 10) : NaN; + if (!Number.isFinite(parsed)) return fallback; + return Math.min(Math.max(parsed, min), max); +} + // GET /api/repos - list user repos (supports ?page=1&per_page=30 for paginated ReposPage response) router.get('/', async (req: Request, res: Response): Promise => { const pageParam = typeof req.query.page === 'string' ? req.query.page : undefined; const perPageParam = typeof req.query.per_page === 'string' ? req.query.per_page : undefined; if (pageParam !== undefined || perPageParam !== undefined) { - const page = parseInt(pageParam ?? '1', 10) || 1; - const perPage = parseInt(perPageParam ?? '30', 10) || 30; - const cacheKey = cacheService.cacheKey(['repos-paged', req.session.accessToken!.slice(-8), String(page), String(perPage)]); + const page = clampInt(pageParam, 1, 1, 1000); + const perPage = clampInt(perPageParam, 30, 1, 100); + const cacheKey = cacheService.cacheKey(['repos-paged', scope(req), String(page), String(perPage)]); const cached = cacheService.get(cacheKey); if (cached) { res.json(cached); @@ -45,7 +64,7 @@ router.get('/', async (req: Request, res: Response): Promise => { const cacheKey = cacheService.cacheKey([ 'repos', - req.session.accessToken!.slice(-8), + scope(req), ]); const cached = cacheService.get(cacheKey); if (cached) { @@ -70,7 +89,7 @@ router.get( async (req: Request, res: Response): Promise => { const owner = str(req.params['owner']); const repo = str(req.params['repo']); - const cacheKey = cacheService.cacheKey(['overview', owner, repo]); + const cacheKey = cacheService.cacheKey(['overview', scope(req), owner, repo]); const cached = cacheService.get(cacheKey); if (cached) { res.json(cached); @@ -103,6 +122,7 @@ router.get( const cacheKey = cacheService.cacheKey([ 'commits', + scope(req), owner, repo, branch, @@ -142,7 +162,7 @@ router.get( const owner = str(req.params['owner']); const repo = str(req.params['repo']); const sha = str(req.params['sha']); - const cacheKey = cacheService.cacheKey(['commit', owner, repo, sha]); + const cacheKey = cacheService.cacheKey(['commit', scope(req), owner, repo, sha]); const cached = cacheService.get(cacheKey); if (cached) { res.json(cached); @@ -171,7 +191,7 @@ router.get( async (req: Request, res: Response): Promise => { const owner = str(req.params['owner']); const repo = str(req.params['repo']); - const cacheKey = cacheService.cacheKey(['branches', owner, repo]); + const cacheKey = cacheService.cacheKey(['branches', scope(req), owner, repo]); const cached = cacheService.get(cacheKey); if (cached) { res.json(cached); @@ -196,7 +216,7 @@ router.get( async (req: Request, res: Response): Promise => { const owner = str(req.params['owner']); const repo = str(req.params['repo']); - const cacheKey = cacheService.cacheKey(['tags', owner, repo]); + const cacheKey = cacheService.cacheKey(['tags', scope(req), owner, repo]); const cached = cacheService.get(cacheKey); if (cached) { res.json(cached); @@ -221,10 +241,15 @@ router.get( async (req: Request, res: Response): Promise => { const owner = str(req.params['owner']); const repo = str(req.params['repo']); - const state = - typeof req.query.state === 'string' ? req.query.state : 'OPEN'; - - const cacheKey = cacheService.cacheKey(['pulls', owner, repo, state]); + const requestedState = + typeof req.query.state === 'string' ? req.query.state.toUpperCase() : 'OPEN'; + // Allowlist: this value is interpolated into the upstream GraphQL query. + const ALLOWED_PR_STATES = ['OPEN', 'CLOSED', 'MERGED']; + const state = ALLOWED_PR_STATES.includes(requestedState) + ? requestedState + : 'OPEN'; + + const cacheKey = cacheService.cacheKey(['pulls', scope(req), owner, repo, state]); const cached = cacheService.get(cacheKey); if (cached) { res.json(cached); @@ -254,11 +279,11 @@ router.get( const since = typeof req.query.since === 'string' ? req.query.since : undefined; const until = typeof req.query.until === 'string' ? req.query.until : undefined; const path = typeof req.query.path === 'string' ? req.query.path : undefined; - const maxCommits = typeof req.query.maxCommits === 'string' ? parseInt(req.query.maxCommits, 10) : undefined; - const tzOffset = typeof req.query.tzOffset === 'string' ? parseInt(req.query.tzOffset, 10) : undefined; + const maxCommits = clampInt(req.query.maxCommits, 100, 1, 5000); + const tzOffset = typeof req.query.tzOffset === 'string' ? clampInt(req.query.tzOffset, 0, -840, 840) : undefined; const stream = req.query.stream === '1'; - const cacheKey = cacheService.cacheKey(['code-frequency', owner, repo, path ?? '', since ?? '', until ?? '', String(maxCommits ?? 100), String(tzOffset ?? '')]); + const cacheKey = cacheService.cacheKey(['code-frequency', scope(req), owner, repo, path ?? '', since ?? '', until ?? '', String(maxCommits), String(tzOffset ?? '')]); const cached = cacheService.get(cacheKey); // ── Non-streaming path (backwards compatible) ────────────────────────── diff --git a/server/src/services/cache.service.ts b/server/src/services/cache.service.ts index 6b8d58e..10ef891 100644 --- a/server/src/services/cache.service.ts +++ b/server/src/services/cache.service.ts @@ -1,6 +1,8 @@ import NodeCache from 'node-cache'; +import crypto from 'crypto'; -const cache = new NodeCache({ stdTTL: 300 }); +// useClones:false would hand out shared mutable references; keep cloning on. +const cache = new NodeCache({ stdTTL: 300, maxKeys: 5000 }); export class CacheService { get(key: string): T | undefined { @@ -26,6 +28,20 @@ export class CacheService { cacheKey(parts: string[]): string { return parts.join(':'); } + + /** + * Derives a stable, non-reversible per-user cache namespace from an access + * token. Using a raw token substring as a cache key both leaks token material + * into cache state and collides across users, which can serve one user's + * private repository data to another. + */ + userScope(accessToken: string): string { + return crypto + .createHash('sha256') + .update(accessToken) + .digest('hex') + .slice(0, 32); + } } export const cacheService = new CacheService();