From eef01c93950d3f51e085c02336e2d9f4d1a4c2d1 Mon Sep 17 00:00:00 2001 From: Abdulkhalek Muhammad Date: Tue, 7 Apr 2026 14:17:29 +0200 Subject: [PATCH 1/2] docs(security): add auth hardening plans --- ...07-sec-auth-01-session-secret-fail-fast.md | 115 +++++++ ...6-04-07-sec-auth-02-oauth-open-redirect.md | 260 ++++++++++++++ ...-sec-auth-03-oauth-state-cookie-cleanup.md | 140 ++++++++ ...c-auth-04-session-regeneration-on-login.md | 137 ++++++++ ...026-04-07-sec-auth-05-stale-guild-cache.md | 321 ++++++++++++++++++ ...26-04-07-sec-auth-06-csrf-double-submit.md | 243 +++++++++++++ ...2026-04-07-sec-auth-07-csp-nonce-styles.md | 158 +++++++++ ...sec-auth-08-oauth-state-samesite-strict.md | 72 ++++ .../2026-04-07-sec-auth-09-session-ttl-24h.md | 125 +++++++ 9 files changed, 1571 insertions(+) create mode 100644 docs/superpowers/plans/2026-04-07-sec-auth-01-session-secret-fail-fast.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-auth-02-oauth-open-redirect.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-auth-03-oauth-state-cookie-cleanup.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-auth-04-session-regeneration-on-login.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-auth-05-stale-guild-cache.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-auth-06-csrf-double-submit.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-auth-07-csp-nonce-styles.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-auth-08-oauth-state-samesite-strict.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-auth-09-session-ttl-24h.md diff --git a/docs/superpowers/plans/2026-04-07-sec-auth-01-session-secret-fail-fast.md b/docs/superpowers/plans/2026-04-07-sec-auth-01-session-secret-fail-fast.md new file mode 100644 index 0000000..434829e --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-auth-01-session-secret-fail-fast.md @@ -0,0 +1,115 @@ +# Session Secret Per-Process Fallback — Fix Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Severity:** CRITICAL +**Goal:** Hard-fail at config load when `DASHBOARD_SESSION_SECRET` is missing in production; warn (and generate ephemeral) in development. +**Architecture:** Move the production guard from `apps/dashboard/src/server/index.ts` (which fires after `config` is already loaded with a random value) into `loadConfig()` itself, so `config.dashboardSessionSecret` is never silently randomized in production. Multi-process deployments will then refuse to boot rather than diverge in cookie-signing keys. +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +`packages/config/src/index.ts:48-50` reads `DASHBOARD_SESSION_SECRET` from env and falls back to `randomBytes(32).toString("hex")` per process. In multi-instance production deployments each process generates its own secret, so cookies signed by one instance fail validation on another (DoS / forced re-login storms). Even on a single instance, the secret rotates on every restart, invalidating all live sessions. + +## Files +- Modify: `packages/config/src/index.ts:48-50` +- Test: `packages/config/tests/index.test.ts` (new file) + +## Tasks + +### Task 1: Fail-fast in production, warn in dev + +- [ ] **Step 1: Write the failing test** + +Create `packages/config/tests/index.test.ts`: + +```typescript +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; + +describe("loadConfig — DASHBOARD_SESSION_SECRET", () => { + const ORIGINAL_ENV = { ...process.env }; + + beforeEach(() => { + vi.resetModules(); + process.env = { ...ORIGINAL_ENV }; + process.env.DISCORD_TOKEN = "test-token"; + process.env.CLIENT_ID = "test-client-id"; + }); + + afterEach(() => { + process.env = ORIGINAL_ENV; + vi.resetModules(); + }); + + it("throws in production when DASHBOARD_SESSION_SECRET is missing", async () => { + process.env.NODE_ENV = "production"; + delete process.env.DASHBOARD_SESSION_SECRET; + await expect(import("../src/index.js")).rejects.toThrow( + /DASHBOARD_SESSION_SECRET/, + ); + }); + + it("uses provided DASHBOARD_SESSION_SECRET in production", async () => { + process.env.NODE_ENV = "production"; + process.env.DASHBOARD_SESSION_SECRET = "a".repeat(64); + const mod = await import("../src/index.js"); + expect(mod.config.dashboardSessionSecret).toBe("a".repeat(64)); + }); + + it("generates an ephemeral secret in development with a warning", async () => { + process.env.NODE_ENV = "development"; + delete process.env.DASHBOARD_SESSION_SECRET; + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const mod = await import("../src/index.js"); + expect(mod.config.dashboardSessionSecret).toMatch(/^[0-9a-f]{64}$/); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining("DASHBOARD_SESSION_SECRET"), + ); + warn.mockRestore(); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `pnpm --filter @fluxcore/config test` +Expected: FAIL — production case currently does not throw (config silently generates a random secret). + +- [ ] **Step 3: Implement fix** + +Edit `packages/config/src/index.ts` lines 48-50: + +```typescript + const dashboardSessionSecret = process.env.DASHBOARD_SESSION_SECRET; + const isProduction = process.env.NODE_ENV === "production"; + let resolvedSessionSecret: string; + if (dashboardSessionSecret && dashboardSessionSecret.length >= 32) { + resolvedSessionSecret = dashboardSessionSecret; + } else if (isProduction) { + throw new Error( + "DASHBOARD_SESSION_SECRET is required in production and must be at least 32 characters. " + + "Generate one with: node -e \"console.log(require('crypto').randomBytes(32).toString('hex'))\"", + ); + } else { + resolvedSessionSecret = randomBytes(32).toString("hex"); + console.warn( + "[config] DASHBOARD_SESSION_SECRET not set — generated an ephemeral secret for development. " + + "All sessions will be invalidated on restart.", + ); + } +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `pnpm --filter @fluxcore/config test` +Expected: PASS (all 3 cases). + +Also verify the dashboard server still typechecks: `pnpm typecheck`. + +- [ ] **Step 5: Commit** + +```bash +git add packages/config/src/index.ts packages/config/tests/index.test.ts +git commit -m "fix(security): hard-fail in production when DASHBOARD_SESSION_SECRET missing" +``` diff --git a/docs/superpowers/plans/2026-04-07-sec-auth-02-oauth-open-redirect.md b/docs/superpowers/plans/2026-04-07-sec-auth-02-oauth-open-redirect.md new file mode 100644 index 0000000..f46188d --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-auth-02-oauth-open-redirect.md @@ -0,0 +1,260 @@ +# OAuth Open Redirect via x-forwarded-host — Fix Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Severity:** HIGH +**Goal:** Build the OAuth callback URL from a trusted, server-configured `DASHBOARD_PUBLIC_URL` instead of attacker-controllable proxy headers. +**Architecture:** Add `dashboardPublicUrl` to `@fluxcore/config` (required in production, defaulted in dev). Replace the header-derived `origin` in both `/auth/login` and `/auth/callback` with this trusted value, so the registered OAuth callback can never be redirected to an attacker host. +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +`apps/dashboard/src/server/features/auth/routes.ts:21-37` and `:62-66` build `origin` from `request.headers["x-forwarded-proto"]` and `request.headers["x-forwarded-host"]`. These headers are set by any upstream client when Fastify is not configured with `trustProxy`, allowing an attacker to make `/auth/login` issue a Discord authorization URL whose `redirect_uri` points at `https://evil.example/auth/callback`. Once a victim follows the link, Discord will redirect their `code` to the attacker (provided the attacker registered the URL with Discord, or in the case of phishing — to a look-alike host). + +## Files +- Modify: `packages/config/src/index.ts` (add `dashboardPublicUrl`) +- Modify: `apps/dashboard/src/server/features/auth/routes.ts:21-37`, `:62-66` +- Test: `apps/dashboard/tests/server/features/auth/routes.test.ts` (extend or create) + +## Tasks + +### Task 1: Add DASHBOARD_PUBLIC_URL to config + +- [ ] **Step 1: Write the failing test** + +Add to `packages/config/tests/index.test.ts`: + +```typescript +describe("loadConfig — DASHBOARD_PUBLIC_URL", () => { + const ORIGINAL_ENV = { ...process.env }; + + beforeEach(() => { + vi.resetModules(); + process.env = { ...ORIGINAL_ENV }; + process.env.DISCORD_TOKEN = "t"; + process.env.CLIENT_ID = "c"; + process.env.DASHBOARD_SESSION_SECRET = "x".repeat(64); + }); + + afterEach(() => { + process.env = ORIGINAL_ENV; + vi.resetModules(); + }); + + it("throws in production when DASHBOARD_PUBLIC_URL is missing", async () => { + process.env.NODE_ENV = "production"; + delete process.env.DASHBOARD_PUBLIC_URL; + await expect(import("../src/index.js")).rejects.toThrow( + /DASHBOARD_PUBLIC_URL/, + ); + }); + + it("rejects DASHBOARD_PUBLIC_URL without https in production", async () => { + process.env.NODE_ENV = "production"; + process.env.DASHBOARD_PUBLIC_URL = "http://example.com"; + await expect(import("../src/index.js")).rejects.toThrow(/https/); + }); + + it("accepts a valid https URL", async () => { + process.env.NODE_ENV = "production"; + process.env.DASHBOARD_PUBLIC_URL = "https://dash.example.com"; + const mod = await import("../src/index.js"); + expect(mod.config.dashboardPublicUrl).toBe("https://dash.example.com"); + }); + + it("defaults to http://localhost:PORT in development", async () => { + process.env.NODE_ENV = "development"; + process.env.DASHBOARD_PORT = "3000"; + delete process.env.DASHBOARD_PUBLIC_URL; + const mod = await import("../src/index.js"); + expect(mod.config.dashboardPublicUrl).toBe("http://localhost:3000"); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `pnpm --filter @fluxcore/config test` +Expected: FAIL — `dashboardPublicUrl` is not defined on `Config`. + +- [ ] **Step 3: Implement fix** + +In `packages/config/src/index.ts`, add `dashboardPublicUrl: string;` to the `Config` interface and inside `loadConfig()`: + +```typescript + const isProduction = process.env.NODE_ENV === "production"; + const rawPublicUrl = process.env.DASHBOARD_PUBLIC_URL; + let dashboardPublicUrl: string; + if (rawPublicUrl) { + let parsed: URL; + try { + parsed = new URL(rawPublicUrl); + } catch { + throw new Error( + `Invalid DASHBOARD_PUBLIC_URL: "${rawPublicUrl}" is not a valid URL`, + ); + } + if (isProduction && parsed.protocol !== "https:") { + throw new Error( + "DASHBOARD_PUBLIC_URL must use https:// in production", + ); + } + // Strip trailing slash for predictable concatenation + dashboardPublicUrl = rawPublicUrl.replace(/\/$/, ""); + } else if (isProduction) { + throw new Error( + "DASHBOARD_PUBLIC_URL is required in production (e.g. https://dashboard.example.com)", + ); + } else { + dashboardPublicUrl = `http://localhost:${dashboardPort}`; + } +``` + +Add `dashboardPublicUrl` to the returned config object. + +- [ ] **Step 4: Run test to verify it passes** + +Run: `pnpm --filter @fluxcore/config test` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add packages/config/src/index.ts packages/config/tests/index.test.ts +git commit -m "feat(config): add DASHBOARD_PUBLIC_URL trusted origin" +``` + +### Task 2: Use dashboardPublicUrl in auth routes + +- [ ] **Step 1: Write the failing test** + +Create or extend `apps/dashboard/tests/server/features/auth/routes.test.ts`: + +```typescript +import { describe, it, expect, beforeEach, vi } from "vitest"; +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; + +vi.mock("@fluxcore/config", () => ({ + config: { + dashboardClientSecret: "secret", + dashboardSessionSecret: "x".repeat(64), + dashboardPublicUrl: "https://dash.example.com", + clientId: "client-id", + }, +})); + +vi.mock("@fluxcore/utils", () => ({ + logger: { error: vi.fn(), info: vi.fn(), debug: vi.fn(), warn: vi.fn() }, +})); + +vi.mock("../../../../src/server/shared/auth.js", async () => { + const actual = await vi.importActual< + typeof import("../../../../src/server/shared/auth.js") + >("../../../../src/server/shared/auth.js"); + return { + ...actual, + getAuthorizationUrl: vi.fn((callbackUrl: string) => ({ + url: `https://discord.com/oauth2/authorize?redirect_uri=${encodeURIComponent(callbackUrl)}`, + state: "state-123", + })), + }; +}); + +import { registerAuthRoutes } from "../../../../src/server/features/auth/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "x".repeat(64) }); + registerAuthRoutes(app); + return app; +} + +describe("/auth/login redirect_uri", () => { + it("ignores x-forwarded-host and uses DASHBOARD_PUBLIC_URL", async () => { + const app = await buildApp(); + const res = await app.inject({ + method: "GET", + url: "/auth/login", + headers: { + "x-forwarded-proto": "https", + "x-forwarded-host": "evil.example.com", + }, + }); + expect(res.statusCode).toBe(302); + const location = res.headers.location as string; + expect(location).toContain( + encodeURIComponent("https://dash.example.com/auth/callback"), + ); + expect(location).not.toContain("evil.example.com"); + await app.close(); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `pnpm test apps/dashboard/tests/server/features/auth/routes.test.ts` +Expected: FAIL — current code constructs origin from `x-forwarded-host`, so the callback contains `evil.example.com`. + +- [ ] **Step 3: Implement fix** + +Edit `apps/dashboard/src/server/features/auth/routes.ts`: + +```typescript +import type { FastifyInstance } from "fastify"; +import { config } from "@fluxcore/config"; +import { + buildCallbackUrl, + getAuthorizationUrl, + exchangeCode, + fetchUser, + fetchGuilds, +} from "../../shared/auth.js"; +import { createSession, deleteSession, getSession } from "../../shared/session.js"; +import { logger } from "@fluxcore/utils"; + +const isProduction = process.env.NODE_ENV === "production"; + +const authRateLimit = { + config: { + rateLimit: { max: 10, timeWindow: "1 minute" }, + }, +}; + +export function registerAuthRoutes(app: FastifyInstance): void { + app.get("/auth/login", { ...authRateLimit }, async (_request, reply) => { + const callbackUrl = buildCallbackUrl(config.dashboardPublicUrl); + const { url, state } = getAuthorizationUrl(callbackUrl); + reply + .setCookie("oauth_state", state, { + path: "/", + httpOnly: true, + sameSite: "lax", + secure: isProduction, + signed: true, + maxAge: 300, + }) + .redirect(url); + }); +``` + +Replace the equivalent block at lines 62-66 in the callback handler with: + +```typescript + const callbackUrl = buildCallbackUrl(config.dashboardPublicUrl); + const token = await exchangeCode(code, callbackUrl); +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `pnpm test apps/dashboard/tests/server/features/auth/routes.test.ts` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/features/auth/routes.ts apps/dashboard/tests/server/features/auth/routes.test.ts +git commit -m "fix(security): use trusted DASHBOARD_PUBLIC_URL for OAuth callback" +``` diff --git a/docs/superpowers/plans/2026-04-07-sec-auth-03-oauth-state-cookie-cleanup.md b/docs/superpowers/plans/2026-04-07-sec-auth-03-oauth-state-cookie-cleanup.md new file mode 100644 index 0000000..302d906 --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-auth-03-oauth-state-cookie-cleanup.md @@ -0,0 +1,140 @@ +# OAuth State Cookie Replay on Token-Exchange Failure — Fix Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Severity:** MEDIUM +**Goal:** Clear the `oauth_state` cookie immediately after the state is validated so a failed token exchange cannot leave a replayable state. +**Architecture:** Move `reply.clearCookie("oauth_state", ...)` from the success branch to right after the successful state comparison, before the network call to Discord. This guarantees one-time-use semantics regardless of downstream errors. +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +`apps/dashboard/src/server/features/auth/routes.ts:50-98` validates the `oauth_state` cookie against the query `state`, then proceeds to call `exchangeCode`. If `exchangeCode` throws (network error, Discord 5xx, code already redeemed), the catch handler sends a 500 response without clearing `oauth_state`. The cookie remains valid for its 5-minute `maxAge`, so an attacker who tricks a user into re-visiting the callback (or replays a captured request) can re-use the same state value to validate a second authorization code. + +## Files +- Modify: `apps/dashboard/src/server/features/auth/routes.ts:60-98` +- Test: `apps/dashboard/tests/server/features/auth/routes.test.ts` + +## Tasks + +### Task 1: Clear oauth_state cookie immediately after validation + +- [ ] **Step 1: Write the failing test** + +Add to `apps/dashboard/tests/server/features/auth/routes.test.ts`: + +```typescript +import { describe, it, expect, vi } from "vitest"; + +vi.mock("../../../../src/server/shared/auth.js", async () => { + const actual = await vi.importActual< + typeof import("../../../../src/server/shared/auth.js") + >("../../../../src/server/shared/auth.js"); + return { + ...actual, + exchangeCode: vi.fn(async () => { + throw new Error("Discord 500"); + }), + fetchUser: vi.fn(), + fetchGuilds: vi.fn(), + }; +}); + +describe("/auth/callback failure cleanup", () => { + it("clears oauth_state cookie even when token exchange fails", async () => { + const app = await buildApp(); // helper from Task 2 of plan 02 + // First hit /auth/login to get a signed state cookie + const loginRes = await app.inject({ + method: "GET", + url: "/auth/login", + }); + const setCookie = loginRes.headers["set-cookie"] as string | string[]; + const cookieHeader = Array.isArray(setCookie) ? setCookie.join("; ") : setCookie; + const stateMatch = /oauth_state=([^;]+)/.exec(cookieHeader); + expect(stateMatch).toBeTruthy(); + + const res = await app.inject({ + method: "GET", + url: "/auth/callback?code=abc&state=state-123", + cookies: { oauth_state: decodeURIComponent(stateMatch![1]) }, + }); + + expect(res.statusCode).toBe(500); + const cleared = res.headers["set-cookie"]; + const clearedStr = Array.isArray(cleared) ? cleared.join("; ") : (cleared ?? ""); + expect(clearedStr).toMatch(/oauth_state=;/); + await app.close(); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `pnpm test apps/dashboard/tests/server/features/auth/routes.test.ts` +Expected: FAIL — `oauth_state` is not cleared on the error path. + +- [ ] **Step 3: Implement fix** + +Edit `apps/dashboard/src/server/features/auth/routes.ts` lines 56-98: + +```typescript + const unsignedState = request.unsignCookie(stateCookie); + if (!unsignedState.valid || unsignedState.value !== state) { + reply + .clearCookie("oauth_state", { path: "/" }) + .code(403) + .send({ error: "Invalid state parameter" }); + return; + } + + // State is valid — burn it immediately so it cannot be replayed + // regardless of whether the rest of the flow succeeds. + reply.clearCookie("oauth_state", { path: "/" }); + + try { + const callbackUrl = buildCallbackUrl(config.dashboardPublicUrl); + const token = await exchangeCode(code, callbackUrl); + const [user, guilds] = await Promise.all([ + fetchUser(token.access_token), + fetchGuilds(token.access_token), + ]); + + const sessionId = await createSession({ + userId: user.id, + username: user.username, + avatar: user.avatar, + accessToken: token.access_token, + guilds, + }); + + reply + .setCookie("session", sessionId, { + path: "/", + httpOnly: true, + sameSite: "lax", + secure: isProduction, + signed: true, + maxAge: 604800, + }) + .redirect("/"); + } catch (error) { + logger.error( + "OAuth callback failed", + error instanceof Error ? error : new Error(String(error)), + ); + reply.code(500).send({ error: "Authentication failed" }); + } +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `pnpm test apps/dashboard/tests/server/features/auth/routes.test.ts` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/features/auth/routes.ts apps/dashboard/tests/server/features/auth/routes.test.ts +git commit -m "fix(security): clear oauth_state cookie before token exchange" +``` diff --git a/docs/superpowers/plans/2026-04-07-sec-auth-04-session-regeneration-on-login.md b/docs/superpowers/plans/2026-04-07-sec-auth-04-session-regeneration-on-login.md new file mode 100644 index 0000000..fe2ca50 --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-auth-04-session-regeneration-on-login.md @@ -0,0 +1,137 @@ +# Session Fixation: No Session Regeneration on Login — Fix Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Severity:** MEDIUM +**Goal:** Delete all existing `DashboardSession` rows for a userId at login time, so re-login invalidates prior sessions and prevents indefinite parallel-session accumulation. +**Architecture:** Add `deleteSessionsForUser(userId)` to `session.ts` and invoke it inside `createSession` (or just before it in the callback). This both prevents session-fixation-style attacks (where an attacker pre-plants a session ID) and bounds DB growth. +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +`apps/dashboard/src/server/features/auth/routes.ts:73-79` calls `createSession` on every successful OAuth callback, but the existing rows for the same `userId` in `DashboardSession` are never removed. Old sessions remain valid until their TTL expires (7 days). If a user's machine was compromised and an old session cookie was exfiltrated, re-logging in does not revoke it. It also enables session-fixation: a attacker who plants their own valid `session` cookie in a victim's browser (e.g. via subdomain) is not displaced when the victim logs in. + +## Files +- Modify: `apps/dashboard/src/server/shared/session.ts:47-70` +- Test: `apps/dashboard/tests/server/shared/session.test.ts` + +## Tasks + +### Task 1: Delete prior sessions for userId on createSession + +- [ ] **Step 1: Write the failing test** + +Add to `apps/dashboard/tests/server/shared/session.test.ts`: + +```typescript +import { describe, it, expect, beforeEach, vi } from "vitest"; + +const deleteMany = vi.fn().mockResolvedValue({ count: 0 }); +const create = vi.fn().mockResolvedValue({}); + +vi.mock("@fluxcore/database", () => ({ + getPrisma: () => ({ + dashboardSession: { + create, + deleteMany, + findUnique: vi.fn(), + update: vi.fn(), + }, + }), +})); + +vi.mock("../../../src/server/shared/crypto.js", () => ({ + encrypt: (s: string) => s, + decrypt: (s: string) => s, +})); + +vi.mock("@fluxcore/utils", () => ({ + logger: { error: vi.fn(), info: vi.fn(), debug: vi.fn(), warn: vi.fn() }, +})); + +import { createSession } from "../../../src/server/shared/session.js"; + +describe("createSession session regeneration", () => { + beforeEach(() => { + deleteMany.mockClear(); + create.mockClear(); + }); + + it("deletes existing sessions for the user before creating a new one", async () => { + await createSession({ + userId: "user-1", + username: "u", + avatar: null, + accessToken: "tok", + guilds: [], + }); + + expect(deleteMany).toHaveBeenCalledWith({ where: { userId: "user-1" } }); + expect(create).toHaveBeenCalled(); + // delete must be called before create + expect(deleteMany.mock.invocationCallOrder[0]).toBeLessThan( + create.mock.invocationCallOrder[0], + ); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `pnpm test apps/dashboard/tests/server/shared/session.test.ts` +Expected: FAIL — `deleteMany` is never called by current `createSession`. + +- [ ] **Step 3: Implement fix** + +Edit `apps/dashboard/src/server/shared/session.ts` `createSession` (lines 47-70): + +```typescript +export async function createSession( + data: Omit, +): Promise { + const id = randomUUID(); + const now = new Date(); + const expiresAt = new Date(now.getTime() + SESSION_TTL); + + const prisma = getPrisma(); + + // Invalidate any prior sessions for this user (session fixation defense + // and prevents unbounded session row growth on repeated logins). + await prisma.dashboardSession.deleteMany({ where: { userId: data.userId } }); + // Also drop any cached entries for this user + for (const [cacheId, entry] of sessionCache) { + if (entry.session.userId === data.userId) { + sessionCache.delete(cacheId); + } + } + + await prisma.dashboardSession.create({ + data: { + id, + userId: data.userId, + username: data.username, + avatar: data.avatar, + accessToken: encrypt(data.accessToken), + guilds: JSON.stringify(data.guilds), + guildsRefreshedAt: now, + createdAt: now, + expiresAt, + }, + }); + + return id; +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `pnpm test apps/dashboard/tests/server/shared/session.test.ts` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/shared/session.ts apps/dashboard/tests/server/shared/session.test.ts +git commit -m "fix(security): regenerate session on login by deleting prior user sessions" +``` diff --git a/docs/superpowers/plans/2026-04-07-sec-auth-05-stale-guild-cache.md b/docs/superpowers/plans/2026-04-07-sec-auth-05-stale-guild-cache.md new file mode 100644 index 0000000..594696e --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-auth-05-stale-guild-cache.md @@ -0,0 +1,321 @@ +# Stale session.guilds Cache Allows Revoked Admins — Fix Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Severity:** MEDIUM +**Goal:** Re-fetch fresh guild membership/permissions in `requireGuildAdmin` whenever the cached entry is more than 5 minutes old, so revoked Discord admins lose dashboard access within 5 minutes instead of up to 30. +**Architecture:** Lower the effective `GUILD_REFRESH_INTERVAL` for `requireGuildAdmin` by awaiting `forceRefreshSessionGuilds` (or a new `ensureFreshGuilds`) when the guard runs against a stale session, then re-checking permissions against the freshly-fetched list. +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +`apps/dashboard/src/server/shared/middleware.ts:60-89` (`requireGuildAdmin`) checks `session.guilds` for the requested `guildId`. The guild list is cached in `session.guilds` (loaded by `getSession` from DB) and only background-refreshed when older than `GUILD_REFRESH_INTERVAL = 30 * 60 * 1000` ms. A user whose `MANAGE_GUILD` permission was revoked in Discord (or who was kicked from the guild) keeps full dashboard write access for up to 30 minutes — long enough to ban members, change settings, or wipe configuration after their access was supposed to be terminated. + +## Files +- Modify: `apps/dashboard/src/server/shared/session.ts` (export `ensureFreshGuilds`) +- Modify: `apps/dashboard/src/server/shared/middleware.ts:60-89` +- Test: `apps/dashboard/tests/server/shared/middleware.test.ts` + +## Tasks + +### Task 1: Add ensureFreshGuilds with 5-minute threshold + +- [ ] **Step 1: Write the failing test** + +Add to `apps/dashboard/tests/server/shared/session.test.ts`: + +```typescript +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const update = vi.fn().mockResolvedValue({}); +vi.mock("@fluxcore/database", () => ({ + getPrisma: () => ({ + dashboardSession: { + create: vi.fn().mockResolvedValue({}), + deleteMany: vi.fn().mockResolvedValue({ count: 0 }), + findUnique: vi.fn(), + update, + }, + }), +})); + +const fetchGuilds = vi.fn(); +vi.mock("../../../src/server/shared/auth.js", () => ({ fetchGuilds })); + +import { + ensureFreshGuilds, + __setSessionCacheForTest, +} from "../../../src/server/shared/session.js"; + +describe("ensureFreshGuilds", () => { + beforeEach(() => { + update.mockClear(); + fetchGuilds.mockClear(); + }); + + it("re-fetches when cached entry is older than 5 minutes", async () => { + const sixMinAgo = Date.now() - 6 * 60 * 1000; + const session = { + userId: "u", + username: "u", + avatar: null, + accessToken: "tok", + guilds: [], + createdAt: Date.now(), + }; + __setSessionCacheForTest("sid", { + session, + cacheExpiresAt: Date.now() + 30_000, + sessionExpiresAt: Date.now() + 1_000_000, + guildsRefreshedAt: sixMinAgo, + }); + fetchGuilds.mockResolvedValueOnce([ + { id: "g1", name: "g", icon: null, permissions: "32" }, + ]); + + const result = await ensureFreshGuilds("sid"); + expect(fetchGuilds).toHaveBeenCalledOnce(); + expect(result[0]?.id).toBe("g1"); + }); + + it("does not re-fetch when cached entry is fresh", async () => { + __setSessionCacheForTest("sid2", { + session: { + userId: "u", + username: "u", + avatar: null, + accessToken: "tok", + guilds: [{ id: "g0", name: "g", icon: null, permissions: "32" }], + createdAt: Date.now(), + }, + cacheExpiresAt: Date.now() + 30_000, + sessionExpiresAt: Date.now() + 1_000_000, + guildsRefreshedAt: Date.now() - 1_000, + }); + + await ensureFreshGuilds("sid2"); + expect(fetchGuilds).not.toHaveBeenCalled(); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `pnpm test apps/dashboard/tests/server/shared/session.test.ts` +Expected: FAIL — `ensureFreshGuilds` and `__setSessionCacheForTest` do not exist. + +- [ ] **Step 3: Implement fix** + +In `apps/dashboard/src/server/shared/session.ts`, add near the bottom: + +```typescript +const FRESH_GUILD_THRESHOLD = 5 * 60 * 1000; // 5 minutes + +/** + * Ensure session.guilds is no older than FRESH_GUILD_THRESHOLD. + * Used by requireGuildAdmin to fail closed for revoked admins quickly. + */ +export async function ensureFreshGuilds( + id: string, +): Promise { + const cached = sessionCache.get(id); + if (!cached) { + const session = await getSession(id); + if (!session) return null; + const reloaded = sessionCache.get(id); + if (!reloaded) return session.guilds; + if (Date.now() - reloaded.guildsRefreshedAt <= FRESH_GUILD_THRESHOLD) { + return reloaded.session.guilds; + } + return refreshSessionGuilds(id, session.accessToken, reloaded); + } + if (Date.now() - cached.guildsRefreshedAt <= FRESH_GUILD_THRESHOLD) { + return cached.session.guilds; + } + return refreshSessionGuilds(id, cached.session.accessToken, cached); +} + +// Test-only hook +export function __setSessionCacheForTest( + id: string, + entry: CachedSession, +): void { + sessionCache.set(id, entry); +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `pnpm test apps/dashboard/tests/server/shared/session.test.ts` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/shared/session.ts apps/dashboard/tests/server/shared/session.test.ts +git commit -m "feat(session): add ensureFreshGuilds with 5-minute staleness threshold" +``` + +### Task 2: Use ensureFreshGuilds in requireGuildAdmin + +- [ ] **Step 1: Write the failing test** + +Add to `apps/dashboard/tests/server/shared/middleware.test.ts`: + +```typescript +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const ensureFreshGuilds = vi.fn(); +const isBotInGuild = vi.fn().mockResolvedValue(true); +const resolveUserPermissions = vi.fn().mockResolvedValue({}); + +vi.mock("../../../src/server/shared/session.js", async () => { + const actual = await vi.importActual< + typeof import("../../../src/server/shared/session.js") + >("../../../src/server/shared/session.js"); + return { ...actual, ensureFreshGuilds }; +}); +vi.mock("../../../src/server/shared/discordApi.js", () => ({ isBotInGuild })); +vi.mock("../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions, + hasPermission: () => true, +})); + +import { requireGuildAdmin } from "../../../src/server/shared/middleware.js"; + +function makeReply() { + const reply: any = { + code: vi.fn().mockReturnThis(), + send: vi.fn().mockReturnThis(), + }; + return reply; +} + +describe("requireGuildAdmin freshness", () => { + beforeEach(() => { + ensureFreshGuilds.mockReset(); + }); + + it("denies access when fresh guilds no longer include the requested guild", async () => { + ensureFreshGuilds.mockResolvedValueOnce([]); // user no longer admin + const reply = makeReply(); + const request: any = { + params: { guildId: "g1" }, + session: { + userId: "u", + guilds: [{ id: "g1", name: "g", icon: null, permissions: "32" }], + }, + sessionId: "sid", + t: (k: string) => k, + }; + + await requireGuildAdmin(request, reply); + expect(reply.code).toHaveBeenCalledWith(403); + }); + + it("allows access when fresh guilds still grant MANAGE_GUILD", async () => { + ensureFreshGuilds.mockResolvedValueOnce([ + { id: "g1", name: "g", icon: null, permissions: "32" }, + ]); + const reply = makeReply(); + const request: any = { + params: { guildId: "g1" }, + session: { + userId: "u", + guilds: [], + }, + sessionId: "sid", + t: (k: string) => k, + }; + + await requireGuildAdmin(request, reply); + expect(reply.code).not.toHaveBeenCalledWith(403); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `pnpm test apps/dashboard/tests/server/shared/middleware.test.ts` +Expected: FAIL — current code uses stale `session.guilds`. + +- [ ] **Step 3: Implement fix** + +Edit `apps/dashboard/src/server/shared/middleware.ts`: + +```typescript +import type { FastifyRequest, FastifyReply } from "fastify"; +import { + getSession, + touchSession, + ensureFreshGuilds, + type Session, +} from "./session.js"; +import { isBotInGuild } from "./discordApi.js"; +import { + resolveUserPermissions, + hasPermission, + type ResolvedPermissions, +} from "./permissions.js"; + +const MANAGE_GUILD = BigInt(0x20); + +// ... requireAuth unchanged ... + +export async function requireGuildAdmin( + request: FastifyRequest, + reply: FastifyReply, +): Promise { + const { guildId } = request.params as { guildId: string }; + const session = request.session!; + const sessionId = request.sessionId!; + + // Re-validate guild membership against fresh Discord data (max 5 min stale) + let guilds = session.guilds; + try { + const fresh = await ensureFreshGuilds(sessionId); + if (fresh) { + guilds = fresh; + session.guilds = fresh; + } + } catch { + // If Discord is unreachable, fall back to cached guilds — this is the + // existing behavior. We still re-check permissions below. + } + + const userGuild = guilds.find((g) => g.id === guildId); + if (!userGuild || !(BigInt(userGuild.permissions) & MANAGE_GUILD)) { + reply.code(403).send({ + error: request.t("errors:permissions.noGuildPermission"), + errorKey: "errors:permissions.noGuildPermission", + }); + return; + } + + if (!(await isBotInGuild(guildId))) { + reply.code(403).send({ + error: request.t("errors:permissions.botNotInGuild"), + errorKey: "errors:permissions.botNotInGuild", + }); + return; + } + + request.resolvedPermissions = await resolveUserPermissions( + session.userId, + guildId, + ); +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `pnpm test apps/dashboard/tests/server/shared/middleware.test.ts` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/shared/middleware.ts apps/dashboard/tests/server/shared/middleware.test.ts +git commit -m "fix(security): refresh guild membership in requireGuildAdmin" +``` diff --git a/docs/superpowers/plans/2026-04-07-sec-auth-06-csrf-double-submit.md b/docs/superpowers/plans/2026-04-07-sec-auth-06-csrf-double-submit.md new file mode 100644 index 0000000..4cdb90a --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-auth-06-csrf-double-submit.md @@ -0,0 +1,243 @@ +# Add Explicit CSRF Tokens to Mutating Routes — Fix Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Severity:** MEDIUM +**Goal:** Add a synchronizer-token (double-submit cookie) CSRF defense to all state-mutating dashboard routes. +**Architecture:** Add a `requireCsrf` Fastify hook that checks `x-csrf-token` request header against a separately stored, non-HttpOnly `csrf_token` cookie. Also add a `/auth/csrf` GET endpoint that issues a fresh token. Apply the hook to POST/PUT/PATCH/DELETE under `/api/*`. +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +`apps/dashboard/src/server/shared/middleware.ts` and the route registrations in `index.ts` rely solely on `SameSite=lax` cookies and Content-Type checks for CSRF defense. `SameSite=lax` allows top-level POST navigations from browsers in some scenarios (e.g. form submission via `
` triggered by user click on an attacker page), and lacks the explicit synchronizer-token guarantee. There is no CSRF token validation at all on `POST /api/guilds/:guildId/...` routes. + +## Files +- Add: `apps/dashboard/src/server/shared/csrf.ts` +- Modify: `apps/dashboard/src/server/index.ts` (register hook + route) +- Modify: `apps/dashboard/src/server/features/auth/routes.ts` (issue token on login) +- Test: `apps/dashboard/tests/server/shared/csrf.test.ts` + +## Tasks + +### Task 1: Implement CSRF helper + Fastify hook + +- [ ] **Step 1: Write the failing test** + +Create `apps/dashboard/tests/server/shared/csrf.test.ts`: + +```typescript +import { describe, it, expect, beforeEach } from "vitest"; +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import { generateCsrfToken, requireCsrf } from "../../../src/server/shared/csrf.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "x".repeat(64) }); + app.get("/issue", async (_req, reply) => { + const token = generateCsrfToken(); + reply + .setCookie("csrf_token", token, { path: "/", sameSite: "lax" }) + .send({ token }); + }); + app.post("/safe", { preHandler: requireCsrf }, async () => ({ ok: true })); + return app; +} + +describe("CSRF double-submit", () => { + let app: Awaited>; + + beforeEach(async () => { + app = await buildApp(); + }); + + it("rejects POST without csrf cookie", async () => { + const res = await app.inject({ method: "POST", url: "/safe" }); + expect(res.statusCode).toBe(403); + await app.close(); + }); + + it("rejects POST with mismatched token", async () => { + const res = await app.inject({ + method: "POST", + url: "/safe", + cookies: { csrf_token: "abc" }, + headers: { "x-csrf-token": "xyz" }, + }); + expect(res.statusCode).toBe(403); + await app.close(); + }); + + it("accepts POST with matching token", async () => { + const issue = await app.inject({ method: "GET", url: "/issue" }); + const body = issue.json() as { token: string }; + const res = await app.inject({ + method: "POST", + url: "/safe", + cookies: { csrf_token: body.token }, + headers: { "x-csrf-token": body.token }, + }); + expect(res.statusCode).toBe(200); + await app.close(); + }); + + it("rejects when token is short (likely empty)", async () => { + const res = await app.inject({ + method: "POST", + url: "/safe", + cookies: { csrf_token: "" }, + headers: { "x-csrf-token": "" }, + }); + expect(res.statusCode).toBe(403); + await app.close(); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `pnpm test apps/dashboard/tests/server/shared/csrf.test.ts` +Expected: FAIL — `csrf.ts` does not exist. + +- [ ] **Step 3: Implement fix** + +Create `apps/dashboard/src/server/shared/csrf.ts`: + +```typescript +import { randomBytes, timingSafeEqual } from "node:crypto"; +import type { FastifyRequest, FastifyReply } from "fastify"; + +const SAFE_METHODS = new Set(["GET", "HEAD", "OPTIONS"]); +const TOKEN_BYTES = 32; +const MIN_TOKEN_LENGTH = TOKEN_BYTES * 2; // hex + +export function generateCsrfToken(): string { + return randomBytes(TOKEN_BYTES).toString("hex"); +} + +function safeEqual(a: string, b: string): boolean { + if (a.length !== b.length) return false; + try { + return timingSafeEqual(Buffer.from(a, "utf8"), Buffer.from(b, "utf8")); + } catch { + return false; + } +} + +export async function requireCsrf( + request: FastifyRequest, + reply: FastifyReply, +): Promise { + if (SAFE_METHODS.has(request.method)) return; + + const cookieToken = request.cookies?.csrf_token; + const headerToken = request.headers["x-csrf-token"]; + const headerStr = Array.isArray(headerToken) ? headerToken[0] : headerToken; + + if ( + !cookieToken || + !headerStr || + cookieToken.length < MIN_TOKEN_LENGTH || + headerStr.length < MIN_TOKEN_LENGTH || + !safeEqual(cookieToken, headerStr) + ) { + reply.code(403).send({ + error: "CSRF token missing or invalid", + errorKey: "errors:csrf.invalid", + }); + return; + } +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `pnpm test apps/dashboard/tests/server/shared/csrf.test.ts` +Expected: PASS (all 4 cases). + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/shared/csrf.ts apps/dashboard/tests/server/shared/csrf.test.ts +git commit -m "feat(security): add CSRF double-submit token helper" +``` + +### Task 2: Wire CSRF into the dashboard server + +- [ ] **Step 1: Write the failing test** + +Add to `apps/dashboard/tests/server/features/auth/routes.test.ts`: + +```typescript +describe("/auth/csrf", () => { + it("issues a csrf_token cookie and matching body token", async () => { + const app = await buildApp(); + const res = await app.inject({ method: "GET", url: "/auth/csrf" }); + expect(res.statusCode).toBe(200); + const body = res.json() as { token: string }; + expect(body.token).toMatch(/^[0-9a-f]{64}$/); + const setCookie = res.headers["set-cookie"]; + const cookieStr = Array.isArray(setCookie) ? setCookie.join("; ") : (setCookie ?? ""); + expect(cookieStr).toContain(`csrf_token=${body.token}`); + await app.close(); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `pnpm test apps/dashboard/tests/server/features/auth/routes.test.ts` +Expected: FAIL — route does not exist. + +- [ ] **Step 3: Implement fix** + +In `apps/dashboard/src/server/features/auth/routes.ts`, add inside `registerAuthRoutes` and import `generateCsrfToken`: + +```typescript +import { generateCsrfToken } from "../../shared/csrf.js"; + +// ... inside registerAuthRoutes: + app.get("/auth/csrf", async (_request, reply) => { + const token = generateCsrfToken(); + reply + .setCookie("csrf_token", token, { + path: "/", + httpOnly: false, // double-submit: JS must read it + sameSite: "lax", + secure: isProduction, + maxAge: 604800, + }) + .send({ token }); + }); +``` + +In `apps/dashboard/src/server/index.ts`, after registering plugins and before route registration: + +```typescript +import { requireCsrf } from "./shared/csrf.js"; + +// ... after fastifyCookie / fastifyHelmet / fastifyRateLimit: + app.addHook("preHandler", async (request, reply) => { + if ( + request.url.startsWith("/api/") && + !["GET", "HEAD", "OPTIONS"].includes(request.method) + ) { + await requireCsrf(request, reply); + } + }); +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `pnpm test apps/dashboard/tests/server/features/auth/routes.test.ts` +Expected: PASS. Then run the broader dashboard tests to ensure existing routes still pass when given CSRF tokens (or are GETs): +Run: `pnpm --filter @fluxcore/dashboard test` +Expected: any failing route tests must be updated to include `x-csrf-token` headers + matching cookies. + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/shared/csrf.ts apps/dashboard/src/server/features/auth/routes.ts apps/dashboard/src/server/index.ts apps/dashboard/tests/server/features/auth/routes.test.ts apps/dashboard/tests/server/shared/csrf.test.ts +git commit -m "feat(security): enforce CSRF double-submit on /api mutating routes" +``` diff --git a/docs/superpowers/plans/2026-04-07-sec-auth-07-csp-nonce-styles.md b/docs/superpowers/plans/2026-04-07-sec-auth-07-csp-nonce-styles.md new file mode 100644 index 0000000..c277390 --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-auth-07-csp-nonce-styles.md @@ -0,0 +1,158 @@ +# Helmet CSP 'unsafe-inline' for Styles — Fix Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Severity:** MEDIUM +**Goal:** Replace `'unsafe-inline'` in the `style-src` CSP directive with a per-request nonce that matches inline `