forge/claude_artifacts/security-20260827-180635.md

security — RBAC + token-management + user-management audit

Date: 2026-08-27 · From: engineer-20260827-180000, principal-20260827-175448 Scope: the new RBAC / API-token / user-management surface on the public domain. Read-only audit (no edits). Out of scope (per task): DB-superuser-in-.env (tracked separately), analytics math.

Method / evidence

• Read server/auth.ts, server/tokens.ts, server/db.ts, index.ts, client App.tsx/services.
• Full route inventory via grep (app.get|post|put|delete|use) — 15 API routes, all accounted for below.
• Grepped for the removed ADMIN_KEY/x-admin-key gate → no matches (fully removed). No dangerouslySetInnerHTML in client/src.
• Live DB (role forge_app, 31.133.115.50:5440/forge): current_user=forge_app; SELECT count(*) FROM app_users WHERE role='admin' → 1; the sole account admin is role admin (bootstrap migrated correctly). No destructive SQL run.

Verified correct (no action)

• Route gating matches the spec matrix. viewer: /api/tickets,/api/tickets/:number,/api/stats,/api/config (requireAuth). pm: /api/analytics/overview|sla,/api/insights (requireRole('pm')). lead: GET/POST /api/users,DELETE /api/users/:username. admin: GET/POST /api/tokens,POST /api/tokens/:id/revoke. /api/sync token-only.
• 401 vs 403 correct (auth.ts:46-61): unauth→401, under-privileged→403.
• Role cannot be spoofed — read from the server-side PG session store (req.session.user, auth.ts:14), never from client input.
• Admin-creation guard present (index.ts:249): non-admin creating role==='admin' → 403.
• isRole validation (db.ts:191): arbitrary/invalid role string → 400 invalid_role.
• Self-delete guard present (index.ts:262).
• Tokens: admin-only; raw fg_… returned once (tokens.ts:81-89), only sha256(secret) persisted; listTokens selects id/token_id/label/dates/revoked — never token_hash (tokens.ts:97-106); revoke honored (resolveToken filters revoked=false AND not expired, tokens.ts:62); constant-time timingSafeEqual compare.
• Cookie hygiene: httpOnly, sameSite=lax, secure-in-prod, SESSION_SECRET required in prod (process.exit(1)), login rate-limited (20/15min), bcrypt dummy-hash compare for unknown users (timing-uniform), bcrypt cost 12.
• No secret leak: /api/users→id/username/role/dates only (no password_hash); /api/me→{username,role} only.
• Client gating is defense-in-depth only (App.tsx Navigate redirects); the server is the true gate.

---

Ranked findings

HIGH-1 — A lead can delete admin accounts (no target-rank check)

Where: index.ts:260 (DELETE /api/users/:username gated only at requireRole('lead')) + server/db.ts:227 (deleteUser = unconditional DELETE FROM app_users WHERE username=$1). Exploit: The create path forbids a non-admin from making an admin (index.ts:249), but the delete path has no symmetric rank check — a lead (or a compromised lead session) can delete any user, admins included. A lead can therefore remove every admin even though it can never create one. Impact: Privilege-boundary violation. Combined with "only an admin creates an admin," a lead that deletes all admins permanently strips the org of token-management / admin-create ability until a server reboot re-seeds AUTH_USER. Fix: Enforce actor-outranks-target in the delete path — refuse deleting a user whose role rank ≥ the actor's (a lead may not delete admins/leads), or raise DELETE /api/users/:username to admin-only. Hand to engineer (route + deleteUser).

HIGH-2 — Last-admin lockout (no guard on deleting/removing the final admin)

Where: server/db.ts:227 deleteUser; no count check anywhere. There is no demote endpoint (confirmed — only seedUser ever writes role, db.ts:179), so deletion is the only removal path. Exploit: Live state is a single admin (admin). Deleting it (reachable today by any lead per HIGH-1, or by a second admin) leaves zero admins. /api/tokens* then 403s for everyone; no one can create a new admin. Impact: Availability/integrity loss of the admin tier. Recovery requires shell/redeploy access — seedUser re-inserts AUTH_USER as admin only when its row is absent (db.ts:176-185), and only that one account. Fix: Guard against removing the last remaining admin (SELECT count(*) … role='admin' before delete; refuse if target is the only admin). Same for any future demote. Hand to engineer/dba.

HIGH-3 — Deleting a user does not invalidate their active session

Where: auth.ts:46-61 (requireAuth/requireRole read identity+role from req.session.user set once at login, index.ts:47); session maxAge 7 days (auth.ts:40); session rows persist in user_sessions. deleteUser (db.ts:227) removes only the app_users row. Exploit: An off-boarded/removed user keeps their cookie working at their cached role for up to 7 days. A deleted lead still passes requireRole('lead') and can keep deleting users (feeding HIGH-1) until session expiry. Role is never re-validated against the DB per request. Impact: On a public domain, account removal is not effective access revocation; no mechanism to force-logout a user. Fix: On delete (and any future role change), purge that user's sessions — e.g. DELETE FROM user_sessions WHERE (sess->'user'->>'username') = $1 — or re-check user existence/role from app_users per request (short-cached). Hand to engineer/dba.

MEDIUM-1 — No session-fixation protection on login

Where: index.ts:43-49 assigns req.session.user = ok on the pre-existing session id; no req.session.regenerate() at the privilege transition. Exploit: Classic session fixation — an attacker who can plant a session cookie (e.g. via a sibling/adjacent context) rides the same id after the victim authenticates and it becomes privileged. Mitigated but not eliminated by httpOnly + sameSite=lax. Fix: Regenerate the session id on successful login before writing user. Hand to engineer.

MEDIUM-2 — Weak password policy on a public login

Where: index.ts:246 — minimum 6 chars, no complexity/denylist. Throttle is per-IP only (loginLimiter 20/15min, index.ts:41). Exploit: Created accounts can carry trivially guessable passwords; per-IP throttling doesn't stop distributed/slow guessing against a public domain. Fix: Raise minimum (~12 chars) and/or add a zxcvbn/denylist check; consider per-account lockout in addition to per-IP. Hand to engineer.

LOW-1 — No HTTP security headers (helmet absent)

Where: app has no helmet/CSP (grep empty); serves SPA + API on a public domain. Impact: No X-Frame-Options/frame-ancestors (clickjacking of the authenticated UI), no HSTS, no X-Content-Type-Options. Adjacent to the auth perimeter, not RBAC-specific. Fix: Add helmet with a same-origin CSP + HSTS-in-prod. Hand to engineer/devops.

LOW-2 — Silent no-op on delete/revoke of a non-existent id

Where: db.ts:227 deleteUser, tokens.ts:108 revokeToken — no affected-row check. DELETE /api/users/:missing and revoke of an unknown token id both return {success:true}. Impact: Cosmetic/idempotency only (no enumeration leak — always success). Consider returning 404 when 0 rows affected so the admin UI reflects reality. Hand to engineer.

LOW-3b (informational) — CSRF on state-changing POSTs rests on sameSite=lax

State-changing routes (/login, /api/users, /api/tokens*, DELETE /api/users/:username) carry no CSRF token; cross-site protection is provided by the sameSite=lax cookie (auth.ts:38). Defensible today. If any of these routes ever move to sameSite=none (e.g. cross-origin embedding), add an explicit CSRF token. No fix required now.

LOW-3 (informational) — Leads self-propagate the user-management tier

Where: index.ts:248-249 blocks only role==='admin' for non-admins; a lead may create unlimited additional leads. This is per the capability spec ("leadership tops out at lead"), so it is by design — but note it compounds HIGH-1/HIGH-3: a lead can mint a co-lead, and either can delete the sole admin. No fix required; revisit if leads should not be able to grant lead.

Next

engineer/dba — fix HIGH-1 (target-rank check on delete), HIGH-2 (last-admin guard), HIGH-3 (session purge on delete). engineer — MEDIUM-1 (session regenerate), MEDIUM-2 (password policy), LOW-1/2. Then re-audit the delete path.