diff --git a/.env.example b/.env.example index 10a2c6e..87a9e35 100644 --- a/.env.example +++ b/.env.example @@ -49,6 +49,9 @@ PROOFDESK_TERMINAL_MODE=restricted PROOFDESK_TERMINAL_RUNTIME=container PROOFDESK_SHARED_STATE_BACKEND=redis PROOFDESK_REDIS_URL=redis://redis:6379/0 +# Comma-separated GitHub logins allowed to read /monitoring/events. +# Unset or empty means no one is an administrator (the endpoint returns 403). +PROOFDESK_ADMIN_LOGINS= PROOFDESK_MONITORING_ENABLED=true PROOFDESK_MONITORING_WEBHOOK_URL= diff --git a/backend/src/middleware/auth.ts b/backend/src/middleware/auth.ts index 3b812d2..90fd3f1 100644 --- a/backend/src/middleware/auth.ts +++ b/backend/src/middleware/auth.ts @@ -36,6 +36,52 @@ export const requireAccessToken = async (req: Request, res: Response, next: Next next(); }; +/** + * Administrative allow-list, read from PROOFDESK_ADMIN_LOGINS as a + * comma-separated list of GitHub logins. + * + * Parsed per request rather than at module load so an operator can change the + * variable without a rebuild, and so tests can set it per case. + */ +const getAdminLogins = (): Set => + new Set( + String(process.env.PROOFDESK_ADMIN_LOGINS || '') + .split(',') + .map((entry) => entry.trim().toLowerCase()) + .filter(Boolean) + ); + +/** + * Restricts a route to operators named in PROOFDESK_ADMIN_LOGINS. + * + * Must be mounted after `requireAccessToken`, which is what populates + * `req.authSession`. + * + * Denies in three cases, all deliberately fail-closed: + * + * - the allow-list is empty or unset. An unconfigured allow-list means "no + * administrators have been designated", never "everyone qualifies"; + * - there is no authenticated session. `requireAccessToken` also accepts a + * raw bearer token, and in that path `req.authSession` is null — a bearer + * token on its own carries no verified identity to check against the list; + * - the session's login is not on the list. + * + * Logins are compared case-insensitively, matching GitHub's own treatment. + */ +export const requireAdmin = (req: Request, res: Response, next: NextFunction): any => { + const admins = getAdminLogins(); + if (admins.size === 0) { + return res.status(403).json({ error: 'Administrative access is not configured' }); + } + + const login = req.authSession?.user?.login; + if (!login || !admins.has(String(login).toLowerCase())) { + return res.status(403).json({ error: 'Administrative access required' }); + } + + next(); +}; + export const checkWorkspaceOwner = (req: Request, res: Response, next: NextFunction): any => { const { sessionId } = req.params; const login = req.authSession?.user?.login; diff --git a/backend/src/routes/system.routes.ts b/backend/src/routes/system.routes.ts index c43c6c8..4d042c5 100644 --- a/backend/src/routes/system.routes.ts +++ b/backend/src/routes/system.routes.ts @@ -1,5 +1,5 @@ import { Router, Request, Response } from 'express'; -import { requireAccessToken } from '../middleware/auth.js'; +import { requireAccessToken, requireAdmin } from '../middleware/auth.js'; import { getMonitoringContextFromRequest, readRecentMonitoringEvents, @@ -76,7 +76,10 @@ export const createSystemRouter = (): Router => { }); }); - router.get('/monitoring/events', requireAccessToken, async (req: Request, res: Response) => { + // System-wide monitoring events carry backend and frontend stack traces, + // internal filesystem paths and request metadata for every user, so this is + // restricted to operators rather than to any authenticated caller. + router.get('/monitoring/events', requireAccessToken, requireAdmin, async (req: Request, res: Response) => { const limitQuery = req.query.limit; const limit = typeof limitQuery === 'string' || typeof limitQuery === 'number' ? Number(limitQuery) : undefined; const events = await readRecentMonitoringEvents({