Merge pull request #179 from shanelord01/audit/remediation-clean
Automated Security & Quality Audit via Claude Code
This commit is contained in:
@@ -23,4 +23,6 @@ if (!JWT_SECRET) {
|
||||
}
|
||||
}
|
||||
|
||||
export { JWT_SECRET };
|
||||
const JWT_SECRET_IS_GENERATED = !process.env.JWT_SECRET;
|
||||
|
||||
export { JWT_SECRET, JWT_SECRET_IS_GENERATED };
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import 'dotenv/config';
|
||||
import './config';
|
||||
import { JWT_SECRET_IS_GENERATED } from './config';
|
||||
import express, { Request, Response, NextFunction } from 'express';
|
||||
import { enforceGlobalMfaPolicy } from './middleware/mfaPolicy';
|
||||
import cors from 'cors';
|
||||
@@ -57,13 +57,21 @@ app.use(helmet({
|
||||
contentSecurityPolicy: {
|
||||
directives: {
|
||||
defaultSrc: ["'self'"],
|
||||
scriptSrc: ["'self'", "'unsafe-inline'"],
|
||||
scriptSrc: ["'self'"],
|
||||
styleSrc: ["'self'", "'unsafe-inline'", "https://fonts.googleapis.com", "https://unpkg.com"],
|
||||
imgSrc: ["'self'", "data:", "blob:", "https:", "http:"],
|
||||
connectSrc: ["'self'", "ws:", "wss:", "https:", "http:"],
|
||||
imgSrc: ["'self'", "data:", "blob:", "https:"],
|
||||
connectSrc: [
|
||||
"'self'", "ws:", "wss:",
|
||||
"https://nominatim.openstreetmap.org", "https://overpass-api.de",
|
||||
"https://places.googleapis.com", "https://api.openweathermap.org",
|
||||
"https://en.wikipedia.org", "https://commons.wikimedia.org",
|
||||
"https://*.basemaps.cartocdn.com", "https://*.tile.openstreetmap.org",
|
||||
"https://unpkg.com", "https://open-meteo.com", "https://api.open-meteo.com",
|
||||
"https://geocoding-api.open-meteo.com",
|
||||
],
|
||||
fontSrc: ["'self'", "https://fonts.gstatic.com", "data:"],
|
||||
objectSrc: ["'self'"],
|
||||
frameSrc: ["'self'"],
|
||||
objectSrc: ["'none'"],
|
||||
frameSrc: ["'none'"],
|
||||
frameAncestors: ["'self'"],
|
||||
upgradeInsecureRequests: shouldForceHttps ? [] : null
|
||||
}
|
||||
@@ -224,9 +232,13 @@ if (process.env.NODE_ENV === 'production') {
|
||||
});
|
||||
}
|
||||
|
||||
// Global error handler
|
||||
// Global error handler — do not leak stack traces in production
|
||||
app.use((err: Error, req: Request, res: Response, next: NextFunction) => {
|
||||
console.error('Unhandled error:', err);
|
||||
if (process.env.NODE_ENV !== 'production') {
|
||||
console.error('Unhandled error:', err);
|
||||
} else {
|
||||
console.error('Unhandled error:', err.message);
|
||||
}
|
||||
res.status(500).json({ error: 'Internal server error' });
|
||||
});
|
||||
|
||||
@@ -237,6 +249,9 @@ const server = app.listen(PORT, () => {
|
||||
console.log(`TREK API running on port ${PORT}`);
|
||||
console.log(`Environment: ${process.env.NODE_ENV || 'development'}`);
|
||||
console.log(`Debug logs: ${DEBUG ? 'ENABLED' : 'disabled'}`);
|
||||
if (JWT_SECRET_IS_GENERATED) {
|
||||
console.warn('[SECURITY WARNING] JWT_SECRET was auto-generated. Sessions will not persist across restarts. Set JWT_SECRET env var for production use.');
|
||||
}
|
||||
if (process.env.DEMO_MODE === 'true') console.log('Demo mode: ENABLED');
|
||||
if (process.env.DEMO_MODE === 'true' && process.env.NODE_ENV === 'production') {
|
||||
console.warn('[SECURITY WARNING] DEMO_MODE is enabled in production! Demo credentials are publicly exposed.');
|
||||
|
||||
@@ -90,7 +90,7 @@ function verifyToken(authHeader: string | undefined): User | null {
|
||||
|
||||
// Short-lived JWT
|
||||
try {
|
||||
const decoded = jwt.verify(token, JWT_SECRET) as { id: number };
|
||||
const decoded = jwt.verify(token, JWT_SECRET, { algorithms: ['HS256'] }) as { id: number };
|
||||
const user = db.prepare(
|
||||
'SELECT id, username, email, role FROM users WHERE id = ?'
|
||||
).get(decoded.id) as User | undefined;
|
||||
|
||||
@@ -14,7 +14,7 @@ const authenticate = (req: Request, res: Response, next: NextFunction): void =>
|
||||
}
|
||||
|
||||
try {
|
||||
const decoded = jwt.verify(token, JWT_SECRET) as { id: number };
|
||||
const decoded = jwt.verify(token, JWT_SECRET, { algorithms: ['HS256'] }) as { id: number };
|
||||
const user = db.prepare(
|
||||
'SELECT id, username, email, role FROM users WHERE id = ?'
|
||||
).get(decoded.id) as User | undefined;
|
||||
@@ -39,7 +39,7 @@ const optionalAuth = (req: Request, res: Response, next: NextFunction): void =>
|
||||
}
|
||||
|
||||
try {
|
||||
const decoded = jwt.verify(token, JWT_SECRET) as { id: number };
|
||||
const decoded = jwt.verify(token, JWT_SECRET, { algorithms: ['HS256'] }) as { id: number };
|
||||
const user = db.prepare(
|
||||
'SELECT id, username, email, role FROM users WHERE id = ?'
|
||||
).get(decoded.id) as User | undefined;
|
||||
|
||||
@@ -51,7 +51,7 @@ export function enforceGlobalMfaPolicy(req: Request, res: Response, next: NextFu
|
||||
|
||||
let userId: number;
|
||||
try {
|
||||
const decoded = jwt.verify(token, JWT_SECRET) as { id: number };
|
||||
const decoded = jwt.verify(token, JWT_SECRET, { algorithms: ['HS256'] }) as { id: number };
|
||||
userId = decoded.id;
|
||||
} catch {
|
||||
next();
|
||||
|
||||
@@ -164,7 +164,7 @@ function generateToken(user: { id: number | bigint }) {
|
||||
return jwt.sign(
|
||||
{ id: user.id },
|
||||
JWT_SECRET,
|
||||
{ expiresIn: '24h' }
|
||||
{ expiresIn: '24h', algorithm: 'HS256' }
|
||||
);
|
||||
}
|
||||
|
||||
@@ -321,7 +321,7 @@ router.post('/login', authLimiter, (req: Request, res: Response) => {
|
||||
const mfa_token = jwt.sign(
|
||||
{ id: Number(user.id), purpose: 'mfa_login' },
|
||||
JWT_SECRET,
|
||||
{ expiresIn: '5m' }
|
||||
{ expiresIn: '5m', algorithm: 'HS256' }
|
||||
);
|
||||
return res.json({ mfa_required: true, mfa_token });
|
||||
}
|
||||
@@ -741,7 +741,7 @@ router.post('/mfa/verify-login', authLimiter, (req: Request, res: Response) => {
|
||||
return res.status(400).json({ error: 'Verification token and code are required' });
|
||||
}
|
||||
try {
|
||||
const decoded = jwt.verify(mfa_token, JWT_SECRET) as { id: number; purpose?: string };
|
||||
const decoded = jwt.verify(mfa_token, JWT_SECRET, { algorithms: ['HS256'] }) as { id: number; purpose?: string };
|
||||
if (decoded.purpose !== 'mfa_login') {
|
||||
return res.status(401).json({ error: 'Invalid verification token' });
|
||||
}
|
||||
|
||||
@@ -282,7 +282,9 @@ router.post('/:id/link', authenticate, (req: Request, res: Response) => {
|
||||
db.prepare('INSERT OR IGNORE INTO file_links (file_id, reservation_id, assignment_id, place_id) VALUES (?, ?, ?, ?)').run(
|
||||
id, reservation_id || null, assignment_id || null, place_id || null
|
||||
);
|
||||
} catch {}
|
||||
} catch (err) {
|
||||
console.error('[Files] Error creating file link:', err instanceof Error ? err.message : err);
|
||||
}
|
||||
|
||||
const links = db.prepare('SELECT * FROM file_links WHERE file_id = ?').all(id);
|
||||
res.json({ success: true, links });
|
||||
|
||||
@@ -6,6 +6,29 @@ import { AuthRequest } from '../types';
|
||||
|
||||
const router = express.Router();
|
||||
|
||||
/** Validate that an asset ID is a safe UUID-like string (no path traversal). */
|
||||
function isValidAssetId(id: string): boolean {
|
||||
return /^[a-zA-Z0-9_-]+$/.test(id) && id.length <= 100;
|
||||
}
|
||||
|
||||
/** Validate that an Immich URL is a safe HTTP(S) URL (no internal/metadata IPs). */
|
||||
function isValidImmichUrl(raw: string): boolean {
|
||||
try {
|
||||
const url = new URL(raw);
|
||||
if (url.protocol !== 'http:' && url.protocol !== 'https:') return false;
|
||||
const hostname = url.hostname.toLowerCase();
|
||||
// Block metadata endpoints and localhost
|
||||
if (hostname === 'localhost' || hostname === '127.0.0.1' || hostname === '::1') return false;
|
||||
if (hostname === '169.254.169.254' || hostname === 'metadata.google.internal') return false;
|
||||
// Block link-local and loopback ranges
|
||||
if (hostname.startsWith('10.') || hostname.startsWith('172.') || hostname.startsWith('192.168.')) return false;
|
||||
if (hostname.endsWith('.internal') || hostname.endsWith('.local')) return false;
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// ── Immich Connection Settings ──────────────────────────────────────────────
|
||||
|
||||
router.get('/settings', authenticate, (req: Request, res: Response) => {
|
||||
@@ -20,6 +43,9 @@ router.get('/settings', authenticate, (req: Request, res: Response) => {
|
||||
router.put('/settings', authenticate, (req: Request, res: Response) => {
|
||||
const authReq = req as AuthRequest;
|
||||
const { immich_url, immich_api_key } = req.body;
|
||||
if (immich_url && !isValidImmichUrl(immich_url.trim())) {
|
||||
return res.status(400).json({ error: 'Invalid Immich URL. Must be a valid HTTP(S) URL.' });
|
||||
}
|
||||
db.prepare('UPDATE users SET immich_url = ?, immich_api_key = ? WHERE id = ?').run(
|
||||
immich_url?.trim() || null,
|
||||
immich_api_key?.trim() || null,
|
||||
@@ -189,10 +215,10 @@ router.put('/trips/:tripId/photos/:assetId/sharing', authenticate, (req: Request
|
||||
router.get('/assets/:assetId/info', authenticate, async (req: Request, res: Response) => {
|
||||
const authReq = req as AuthRequest;
|
||||
const { assetId } = req.params;
|
||||
const { userId } = req.query;
|
||||
if (!isValidAssetId(assetId)) return res.status(400).json({ error: 'Invalid asset ID' });
|
||||
|
||||
const targetUserId = userId ? Number(userId) : authReq.user.id;
|
||||
const user = db.prepare('SELECT immich_url, immich_api_key FROM users WHERE id = ?').get(targetUserId) as any;
|
||||
// Only allow accessing own Immich credentials — prevent leaking other users' API keys
|
||||
const user = db.prepare('SELECT immich_url, immich_api_key FROM users WHERE id = ?').get(authReq.user.id) as any;
|
||||
if (!user?.immich_url || !user?.immich_api_key) return res.status(404).json({ error: 'Not found' });
|
||||
|
||||
try {
|
||||
@@ -240,10 +266,10 @@ function authFromQuery(req: Request, res: Response, next: Function) {
|
||||
router.get('/assets/:assetId/thumbnail', authFromQuery, async (req: Request, res: Response) => {
|
||||
const authReq = req as AuthRequest;
|
||||
const { assetId } = req.params;
|
||||
const { userId } = req.query;
|
||||
if (!isValidAssetId(assetId)) return res.status(400).send('Invalid asset ID');
|
||||
|
||||
const targetUserId = userId ? Number(userId) : authReq.user.id;
|
||||
const user = db.prepare('SELECT immich_url, immich_api_key FROM users WHERE id = ?').get(targetUserId) as any;
|
||||
// Only allow accessing own Immich credentials — prevent leaking other users' API keys
|
||||
const user = db.prepare('SELECT immich_url, immich_api_key FROM users WHERE id = ?').get(authReq.user.id) as any;
|
||||
if (!user?.immich_url || !user?.immich_api_key) return res.status(404).send('Not found');
|
||||
|
||||
try {
|
||||
@@ -264,10 +290,10 @@ router.get('/assets/:assetId/thumbnail', authFromQuery, async (req: Request, res
|
||||
router.get('/assets/:assetId/original', authFromQuery, async (req: Request, res: Response) => {
|
||||
const authReq = req as AuthRequest;
|
||||
const { assetId } = req.params;
|
||||
const { userId } = req.query;
|
||||
if (!isValidAssetId(assetId)) return res.status(400).send('Invalid asset ID');
|
||||
|
||||
const targetUserId = userId ? Number(userId) : authReq.user.id;
|
||||
const user = db.prepare('SELECT immich_url, immich_api_key FROM users WHERE id = ?').get(targetUserId) as any;
|
||||
// Only allow accessing own Immich credentials — prevent leaking other users' API keys
|
||||
const user = db.prepare('SELECT immich_url, immich_api_key FROM users WHERE id = ?').get(authReq.user.id) as any;
|
||||
if (!user?.immich_url || !user?.immich_api_key) return res.status(404).send('Not found');
|
||||
|
||||
try {
|
||||
|
||||
@@ -428,7 +428,8 @@ router.get('/place-photo/:placeId', authenticate, async (req: Request, res: Resp
|
||||
const attribution = photo.authorAttributions?.[0]?.displayName || null;
|
||||
|
||||
const mediaRes = await fetch(
|
||||
`https://places.googleapis.com/v1/${photoName}/media?maxHeightPx=600&key=${apiKey}&skipHttpRedirect=true`
|
||||
`https://places.googleapis.com/v1/${photoName}/media?maxHeightPx=600&skipHttpRedirect=true`,
|
||||
{ headers: { 'X-Goog-Api-Key': apiKey } }
|
||||
);
|
||||
const mediaData = await mediaRes.json() as { photoUri?: string };
|
||||
const photoUrl = mediaData.photoUri;
|
||||
|
||||
@@ -80,11 +80,11 @@ async function discover(issuer: string) {
|
||||
return doc;
|
||||
}
|
||||
|
||||
function generateToken(user: { id: number; username: string; email: string; role: string }) {
|
||||
function generateToken(user: { id: number }) {
|
||||
return jwt.sign(
|
||||
{ id: user.id, username: user.username, email: user.email, role: user.role },
|
||||
{ id: user.id },
|
||||
JWT_SECRET,
|
||||
{ expiresIn: '24h' }
|
||||
{ expiresIn: '24h', algorithm: 'HS256' }
|
||||
);
|
||||
}
|
||||
|
||||
@@ -121,9 +121,15 @@ router.get('/login', async (req: Request, res: Response) => {
|
||||
try {
|
||||
const doc = await discover(config.issuer);
|
||||
const state = crypto.randomBytes(32).toString('hex');
|
||||
const proto = (req.headers['x-forwarded-proto'] as string) || req.protocol;
|
||||
const host = (req.headers['x-forwarded-host'] as string) || req.headers.host;
|
||||
const redirectUri = `${proto}://${host}/api/auth/oidc/callback`;
|
||||
const appUrl = process.env.APP_URL || (db.prepare("SELECT value FROM app_settings WHERE key = 'app_url'").get() as { value: string } | undefined)?.value;
|
||||
let redirectUri: string;
|
||||
if (appUrl) {
|
||||
redirectUri = `${appUrl.replace(/\/+$/, '')}/api/auth/oidc/callback`;
|
||||
} else {
|
||||
const proto = (req.headers['x-forwarded-proto'] as string) || req.protocol;
|
||||
const host = (req.headers['x-forwarded-host'] as string) || req.headers.host;
|
||||
redirectUri = `${proto}://${host}/api/auth/oidc/callback`;
|
||||
}
|
||||
const inviteToken = req.query.invite as string | undefined;
|
||||
|
||||
pendingStates.set(state, { createdAt: Date.now(), redirectUri, inviteToken });
|
||||
@@ -185,7 +191,7 @@ router.get('/callback', async (req: Request, res: Response) => {
|
||||
|
||||
const tokenData = await tokenRes.json() as OidcTokenResponse;
|
||||
if (!tokenRes.ok || !tokenData.access_token) {
|
||||
console.error('[OIDC] Token exchange failed:', tokenData);
|
||||
console.error('[OIDC] Token exchange failed: status', tokenRes.status);
|
||||
return res.redirect(frontendUrl('/login?oidc_error=token_failed'));
|
||||
}
|
||||
|
||||
|
||||
@@ -24,9 +24,28 @@ let nextSocketId = 1;
|
||||
|
||||
let wss: WebSocketServer | null = null;
|
||||
|
||||
// Per-connection message rate limiting
|
||||
const WS_MSG_LIMIT = 30; // max messages
|
||||
const WS_MSG_WINDOW = 10_000; // per 10 seconds
|
||||
const socketMsgCounts = new WeakMap<NomadWebSocket, { count: number; windowStart: number }>();
|
||||
|
||||
/** Attaches a WebSocket server with JWT auth, room-based trip channels, and heartbeat keep-alive. */
|
||||
function setupWebSocket(server: http.Server): void {
|
||||
wss = new WebSocketServer({ server, path: '/ws' });
|
||||
const allowedOrigins = process.env.ALLOWED_ORIGINS
|
||||
? process.env.ALLOWED_ORIGINS.split(',').map(o => o.trim())
|
||||
: null;
|
||||
|
||||
wss = new WebSocketServer({
|
||||
server,
|
||||
path: '/ws',
|
||||
maxPayload: 64 * 1024, // 64 KB max message size
|
||||
verifyClient: allowedOrigins
|
||||
? ({ origin }, cb) => {
|
||||
if (!origin || allowedOrigins.includes(origin)) cb(true);
|
||||
else cb(false, 403, 'Origin not allowed');
|
||||
}
|
||||
: undefined,
|
||||
});
|
||||
|
||||
const HEARTBEAT_INTERVAL = 30000; // 30 seconds
|
||||
const heartbeat = setInterval(() => {
|
||||
@@ -53,7 +72,7 @@ function setupWebSocket(server: http.Server): void {
|
||||
|
||||
let user: User | undefined;
|
||||
try {
|
||||
const decoded = jwt.verify(token, JWT_SECRET) as { id: number };
|
||||
const decoded = jwt.verify(token, JWT_SECRET, { algorithms: ['HS256'] }) as { id: number };
|
||||
user = db.prepare(
|
||||
'SELECT id, username, email, role, mfa_enabled FROM users WHERE id = ?'
|
||||
).get(decoded.id) as User | undefined;
|
||||
@@ -81,14 +100,33 @@ function setupWebSocket(server: http.Server): void {
|
||||
|
||||
nws.on('pong', () => { nws.isAlive = true; });
|
||||
|
||||
socketMsgCounts.set(nws, { count: 0, windowStart: Date.now() });
|
||||
|
||||
nws.on('message', (data) => {
|
||||
// Rate limiting
|
||||
const rate = socketMsgCounts.get(nws)!;
|
||||
const now = Date.now();
|
||||
if (now - rate.windowStart > WS_MSG_WINDOW) {
|
||||
rate.count = 1;
|
||||
rate.windowStart = now;
|
||||
} else {
|
||||
rate.count++;
|
||||
if (rate.count > WS_MSG_LIMIT) {
|
||||
nws.send(JSON.stringify({ type: 'error', message: 'Rate limit exceeded' }));
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
let msg: { type: string; tripId?: number | string };
|
||||
try {
|
||||
msg = JSON.parse(data.toString());
|
||||
} catch {
|
||||
return;
|
||||
return; // Malformed JSON, ignore
|
||||
}
|
||||
|
||||
// Basic validation
|
||||
if (!msg || typeof msg !== 'object' || typeof msg.type !== 'string') return;
|
||||
|
||||
if (msg.type === 'join' && msg.tripId) {
|
||||
const tripId = Number(msg.tripId);
|
||||
// Verify the user has access to this trip
|
||||
|
||||
Reference in New Issue
Block a user