From 2f5769e7f4ed754cdbc5b4d0d96f63ca5bc37978 Mon Sep 17 00:00:00 2001 From: Abdulkhalek Muhammad Date: Tue, 7 Apr 2026 13:55:38 +0200 Subject: [PATCH 01/61] docs(security): add api hardening plans --- ...-sec-api-01-permission-grant-escalation.md | 270 ++++++++++++++++++ ...sec-api-02-audit-log-userid-enumeration.md | 177 ++++++++++++ ...07-sec-api-03-audit-log-date-validation.md | 121 ++++++++ ...ec-api-04-resource-creation-rate-limits.md | 169 +++++++++++ ...-05-welcome-image-magic-byte-validation.md | 254 ++++++++++++++++ .../2026-04-07-sec-api-06-cron-preview-dos.md | 208 ++++++++++++++ ...6-04-07-sec-api-07-custom-command-redos.md | 200 +++++++++++++ ...-04-07-sec-api-08-permission-error-leak.md | 87 ++++++ ...-07-sec-api-09-audit-action-exact-match.md | 122 ++++++++ ...-04-07-sec-api-10-image-preview-nosniff.md | 147 ++++++++++ ...07-sec-api-11-actions-update-error-mask.md | 183 ++++++++++++ 11 files changed, 1938 insertions(+) create mode 100644 docs/superpowers/plans/2026-04-07-sec-api-01-permission-grant-escalation.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-api-02-audit-log-userid-enumeration.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-api-03-audit-log-date-validation.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-api-04-resource-creation-rate-limits.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-api-05-welcome-image-magic-byte-validation.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-api-06-cron-preview-dos.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-api-07-custom-command-redos.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-api-08-permission-error-leak.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-api-09-audit-action-exact-match.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-api-10-image-preview-nosniff.md create mode 100644 docs/superpowers/plans/2026-04-07-sec-api-11-actions-update-error-mask.md diff --git a/docs/superpowers/plans/2026-04-07-sec-api-01-permission-grant-escalation.md b/docs/superpowers/plans/2026-04-07-sec-api-01-permission-grant-escalation.md new file mode 100644 index 0000000..97765ce --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-api-01-permission-grant-escalation.md @@ -0,0 +1,270 @@ +# Permission Grant Escalation Flaw — 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:** Prevent privilege escalation through user permission overrides by enforcing a strict tier check, blocking self-grant, and rejecting any attempt to grant a permission the caller does not strictly out-rank. +**Architecture:** Replace the current "if I can match it I can grant it" check in `PUT /api/guilds/:guildId/user-permissions/:userId` with three explicit gates: (1) reject when `session.userId === target userId`, (2) require non-owner callers to hold strictly more keys than the resulting set (i.e. they must be a strict superset that excludes any wildcard the caller does not literally hold), and (3) explicitly forbid granting `*` or any module-level wildcard unless the caller is the guild owner. +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +`apps/dashboard/src/server/features/permissions/routes.ts:109-135` validates each requested permission against `matchPermission(userPerms, perm)`. Because `matchPermission` treats `dashboard.*` as matching `dashboard.roles.manage`, a non-owner who holds `dashboard.roles.manage` can grant *themselves* `dashboard.roles.manage` (self-grant is not blocked) and, more dangerously, a holder of any wildcard like `actions.*` can grant `actions.rules.execute` to a user who previously had nothing — there is no check that the new permission set is *less* than the caller's. Combined with the missing self-grant block, an attacker who compromises any non-owner admin account can chain grants until they reach `*`. + +## Files +- Modify: `apps/dashboard/src/server/features/permissions/routes.ts:103-167` +- Test: `apps/dashboard/tests/server/features/permissions/userPermissions.test.ts` (new file) + +## Tasks + +### Task 1: Block self-grant and wildcard escalation + +- [ ] **Step 1: Write the failing test** + +Create `apps/dashboard/tests/server/features/permissions/userPermissions.test.ts`: + +```typescript +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { + token: "test-token", + clientId: "test-client-id", + dashboardSessionSecret: "session-secret", + logLevel: "info", + }, +})); + +const MANAGE_GUILD = BigInt(0x20); +const callerSession = { + userId: "caller-1", + username: "caller", + guilds: [{ id: "guild-1", name: "Test", permissions: MANAGE_GUILD.toString() }], +}; + +const mockGetSession = vi.fn().mockResolvedValue(callerSession); +vi.mock("../../../../src/server/shared/session.js", () => ({ + getSession: (...args: unknown[]) => mockGetSession(...args), + touchSession: vi.fn().mockResolvedValue(undefined), +})); + +const mockGetGuildOwnerId = vi.fn().mockResolvedValue("owner-1"); +vi.mock("../../../../src/server/shared/discordApi.js", () => ({ + isBotInGuild: vi.fn().mockResolvedValue(true), + getGuildOwnerId: (...args: unknown[]) => mockGetGuildOwnerId(...args), +})); + +const mockResolveUserPermissions = vi.fn(); +vi.mock("../../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions: (...args: unknown[]) => mockResolveUserPermissions(...args), + hasPermission: vi.fn().mockReturnValue(true), + invalidatePermissionCache: vi.fn(), + createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), +})); + +const mockPrisma = { + dashboardUserPermission: { + findMany: vi.fn().mockResolvedValue([]), + deleteMany: vi.fn().mockResolvedValue({ count: 0 }), + createMany: vi.fn().mockResolvedValue({ count: 0 }), + }, + dashboardAuditLog: { create: vi.fn(), findMany: vi.fn(), count: vi.fn() }, + $transaction: vi.fn(async (fn: (tx: unknown) => Promise) => fn(mockPrisma)), +}; + +vi.mock("@fluxcore/database", () => ({ getPrisma: () => mockPrisma })); +vi.mock("@fluxcore/utils", () => ({ + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})); + +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import { registerDashboardPermissionRoutes } from "../../../../src/server/features/permissions/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "test-secret" }); + registerDashboardPermissionRoutes(app); + await app.ready(); + return app; +} + +describe("PUT /api/guilds/:guildId/user-permissions/:userId — escalation guards", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + mockGetSession.mockResolvedValue(callerSession); + mockGetGuildOwnerId.mockResolvedValue("owner-1"); + mockResolveUserPermissions.mockResolvedValue({ + permissions: new Set(["dashboard.roles.manage"]), + isOwner: false, + }); + app = await buildApp(); + }); + + it("rejects self-grant with 403", async () => { + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/user-permissions/caller-1", + cookies: { session: app.signCookie("valid") }, + payload: { permissions: ["dashboard.roles.view"] }, + }); + expect(res.statusCode).toBe(403); + expect(res.json().error).toMatch(/self/i); + }); + + it("rejects non-owner attempting to grant a wildcard they do not literally hold", async () => { + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/user-permissions/target-1", + cookies: { session: app.signCookie("valid") }, + payload: { permissions: ["dashboard.*"] }, + }); + expect(res.statusCode).toBe(403); + }); + + it("rejects non-owner attempting to grant the global wildcard", async () => { + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/user-permissions/target-1", + cookies: { session: app.signCookie("valid") }, + payload: { permissions: ["*"] }, + }); + expect(res.statusCode).toBe(403); + }); + + it("rejects when caller does not literally hold the requested key", async () => { + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/user-permissions/target-1", + cookies: { session: app.signCookie("valid") }, + payload: { permissions: ["actions.rules.manage"] }, + }); + expect(res.statusCode).toBe(403); + }); + + it("allows owner to grant any permission, including self", async () => { + mockResolveUserPermissions.mockResolvedValue({ + permissions: new Set(["*"]), + isOwner: true, + }); + mockGetSession.mockResolvedValue({ ...callerSession, userId: "owner-1" }); + mockGetGuildOwnerId.mockResolvedValue("owner-1"); + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/user-permissions/target-1", + cookies: { session: app.signCookie("valid") }, + payload: { permissions: ["actions.rules.manage"] }, + }); + expect(res.statusCode).toBe(200); + }); +}); +``` + +- [ ] **Step 2: Run test to verify fail** + +```bash +pnpm --filter @fluxcore/dashboard test -- userPermissions +``` + +Expected: tests "rejects self-grant", "rejects non-owner attempting to grant a wildcard", "rejects non-owner attempting to grant the global wildcard" all FAIL (current code returns 200 because `matchPermission` accepts wildcards and self-grant is not blocked). + +- [ ] **Step 3: Implement the fix** + +Edit `apps/dashboard/src/server/features/permissions/routes.ts` — replace the body of the `PUT /api/guilds/:guildId/user-permissions/:userId` handler starting at line 103, swapping the existing escalation block (lines 124-136) with strict checks: + +```typescript + async (request, reply) => { + const { guildId, userId } = request.params as { guildId: string; userId: string }; + const { permissions } = request.body as { permissions: string[] }; + const session = request.session!; + const prisma = getPrisma(); + + // Cannot modify owner permissions + const ownerId = await getGuildOwnerId(guildId); + if (ownerId === userId) { + reply.code(400).send({ error: "Cannot modify guild owner permissions" }); + return; + } + + // Block self-grant: a user must never grant or modify their own permission set + if (session.userId === userId) { + reply.code(403).send({ error: "Cannot modify your own permissions" }); + return; + } + + // Validate keys + for (const perm of permissions) { + if (!isValidPermKey(perm)) { + reply.code(400).send({ error: `Invalid permission key: ${perm}` }); + return; + } + } + + // Strict escalation gate: non-owners must literally hold every key they grant. + // Wildcards (`*`, `module.*`, `module.feature.*`) may only be granted by the + // guild owner — match-by-wildcard is not enough. + if (!request.resolvedPermissions?.isOwner) { + const literalCallerPerms = request.resolvedPermissions!.permissions; + for (const perm of permissions) { + if (perm === "*" || perm.includes("*")) { + reply.code(403).send({ error: "Insufficient privileges to grant this permission" }); + return; + } + if (!literalCallerPerms.has(perm)) { + reply.code(403).send({ error: "Insufficient privileges to grant this permission" }); + return; + } + } + } + + // Replace all user permissions in a transaction + await prisma.$transaction(async (tx) => { + await tx.dashboardUserPermission.deleteMany({ where: { guildId, userId } }); + if (permissions.length > 0) { + await tx.dashboardUserPermission.createMany({ + data: permissions.map((perm) => ({ + guildId, + userId, + permission: perm, + grantedBy: session.userId, + })), + }); + } + }); + + invalidatePermissionCache(guildId, userId); + + await createDashboardAuditLog({ + guildId, + userId: session.userId, + username: session.username, + action: "dashboard.permissions.update", + targetType: "user", + targetId: userId, + details: { permissions }, + }); + + reply.send({ success: true, permissions }); + }, +``` + +Note: also remove the `import { matchPermission } from "@fluxcore/types"` if it becomes unused. + +- [ ] **Step 4: Run test to verify pass** + +```bash +pnpm --filter @fluxcore/dashboard test -- userPermissions +pnpm typecheck +``` + +All five tests should now pass. + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/features/permissions/routes.ts apps/dashboard/tests/server/features/permissions/userPermissions.test.ts +git commit -m "fix(permissions): block self-grant and wildcard escalation in user-permissions PUT" +``` diff --git a/docs/superpowers/plans/2026-04-07-sec-api-02-audit-log-userid-enumeration.md b/docs/superpowers/plans/2026-04-07-sec-api-02-audit-log-userid-enumeration.md new file mode 100644 index 0000000..90c83c9 --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-api-02-audit-log-userid-enumeration.md @@ -0,0 +1,177 @@ +# Audit Log userId Enumeration — 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:** Restrict the audit-log `userId` filter so non-owners can only query their own actions, preventing user-id enumeration and disclosure of other admins' activity. +**Architecture:** In `GET /api/guilds/:guildId/dashboard-audit`, branch on `request.resolvedPermissions.isOwner`. Owners may pass any `userId`. Non-owners get the filter forced to their own session userId regardless of what they passed (or receive 403 if they tried a different value). +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +`apps/dashboard/src/server/features/permissions/routes.ts:301` accepts `query.userId` and applies it to the Prisma `where` clause unconditionally. Anyone with `dashboard.audit.view` can iterate user IDs and observe what every other admin did, including failed permission attempts and target IDs — useful for both reconnaissance and harassment. The audit log is meant to be visible to admins, but per-user filtering by arbitrary IDs is enumeration. + +## Files +- Modify: `apps/dashboard/src/server/features/permissions/routes.ts:280-336` +- Test: `apps/dashboard/tests/server/features/permissions/auditLog.test.ts` (new file) + +## Tasks + +### Task 1: Restrict userId filter to own user unless owner + +- [ ] **Step 1: Write the failing test** + +Create `apps/dashboard/tests/server/features/permissions/auditLog.test.ts`: + +```typescript +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { + token: "test-token", + clientId: "test-client-id", + dashboardSessionSecret: "session-secret", + logLevel: "info", + }, +})); + +const MANAGE_GUILD = BigInt(0x20); +const callerSession = { + userId: "caller-1", + username: "caller", + guilds: [{ id: "guild-1", name: "Test", permissions: MANAGE_GUILD.toString() }], +}; + +const mockGetSession = vi.fn().mockResolvedValue(callerSession); +vi.mock("../../../../src/server/shared/session.js", () => ({ + getSession: (...args: unknown[]) => mockGetSession(...args), + touchSession: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("../../../../src/server/shared/discordApi.js", () => ({ + isBotInGuild: vi.fn().mockResolvedValue(true), + getGuildOwnerId: vi.fn().mockResolvedValue("owner-1"), +})); + +const mockResolveUserPermissions = vi.fn(); +vi.mock("../../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions: (...args: unknown[]) => mockResolveUserPermissions(...args), + hasPermission: vi.fn().mockReturnValue(true), + invalidatePermissionCache: vi.fn(), + createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), +})); + +const mockFindMany = vi.fn().mockResolvedValue([]); +const mockCount = vi.fn().mockResolvedValue(0); +vi.mock("@fluxcore/database", () => ({ + getPrisma: () => ({ + dashboardAuditLog: { findMany: mockFindMany, count: mockCount }, + dashboardUserPermission: { findMany: vi.fn() }, + }), +})); +vi.mock("@fluxcore/utils", () => ({ + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})); + +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import { registerDashboardPermissionRoutes } from "../../../../src/server/features/permissions/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "test-secret" }); + registerDashboardPermissionRoutes(app); + await app.ready(); + return app; +} + +describe("GET /api/guilds/:guildId/dashboard-audit — userId filter", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + mockGetSession.mockResolvedValue(callerSession); + mockResolveUserPermissions.mockResolvedValue({ + permissions: new Set(["dashboard.audit.view"]), + isOwner: false, + }); + mockFindMany.mockResolvedValue([]); + mockCount.mockResolvedValue(0); + app = await buildApp(); + }); + + it("forces userId filter to caller for non-owner", async () => { + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/dashboard-audit?userId=other-user", + cookies: { session: app.signCookie("valid") }, + }); + expect(res.statusCode).toBe(200); + expect(mockFindMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ guildId: "guild-1", userId: "caller-1" }), + }), + ); + }); + + it("allows owner to filter by any userId", async () => { + mockResolveUserPermissions.mockResolvedValue({ + permissions: new Set(["*"]), + isOwner: true, + }); + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/dashboard-audit?userId=other-user", + cookies: { session: app.signCookie("valid") }, + }); + expect(res.statusCode).toBe(200); + expect(mockFindMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ userId: "other-user" }), + }), + ); + }); +}); +``` + +- [ ] **Step 2: Run test to verify fail** + +```bash +pnpm --filter @fluxcore/dashboard test -- auditLog +``` + +Expected: "forces userId filter to caller for non-owner" FAILS — current code forwards `other-user` directly. + +- [ ] **Step 3: Implement the fix** + +Edit `apps/dashboard/src/server/features/permissions/routes.ts` lines 300-302 — replace the `where` construction so that non-owners are forced to their own userId: + +```typescript + const where: Record = { guildId }; + const isOwner = request.resolvedPermissions?.isOwner === true; + if (isOwner) { + if (query.userId) where.userId = query.userId; + } else { + // Non-owners may only view their own audit entries + where.userId = request.session!.userId; + } + if (query.action) where.action = { contains: query.action }; + if (query.targetType) where.targetType = query.targetType; +``` + +- [ ] **Step 4: Run test to verify pass** + +```bash +pnpm --filter @fluxcore/dashboard test -- auditLog +pnpm typecheck +``` + +Both tests pass. + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/features/permissions/routes.ts apps/dashboard/tests/server/features/permissions/auditLog.test.ts +git commit -m "fix(permissions): restrict audit log userId filter to caller for non-owners" +``` diff --git a/docs/superpowers/plans/2026-04-07-sec-api-03-audit-log-date-validation.md b/docs/superpowers/plans/2026-04-07-sec-api-03-audit-log-date-validation.md new file mode 100644 index 0000000..0019b29 --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-api-03-audit-log-date-validation.md @@ -0,0 +1,121 @@ +# Audit Log Date Validation — 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:** Reject malformed `from`/`to` query parameters with HTTP 400 instead of silently passing `Invalid Date` to Prisma. +**Architecture:** Parse each date with `new Date(...)`, then check `Number.isFinite(d.getTime())`. If parsing fails, return 400 with a clear error before constructing the `where` clause. +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +`apps/dashboard/src/server/features/permissions/routes.ts:304-308` does `new Date(query.from)` without validation. Invalid input (e.g. `from=lol`) yields `Invalid Date`, which Prisma may either error on or silently produce empty results — making the audit log filter unreliable and easy to abuse to confuse incident responders. + +## Files +- Modify: `apps/dashboard/src/server/features/permissions/routes.ts:304-308` +- Test: `apps/dashboard/tests/server/features/permissions/auditLog.test.ts` (extend existing or add a new file `auditLogDates.test.ts`) + +## Tasks + +### Task 1: Validate from/to and 400 on bad input + +- [ ] **Step 1: Write the failing test** + +Append to `apps/dashboard/tests/server/features/permissions/auditLog.test.ts` (created in finding 02 — if not yet present, use the same scaffolding/mocks): + +```typescript +describe("GET /api/guilds/:guildId/dashboard-audit — date validation", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + mockGetSession.mockResolvedValue(callerSession); + mockResolveUserPermissions.mockResolvedValue({ + permissions: new Set(["*"]), + isOwner: true, + }); + mockFindMany.mockResolvedValue([]); + mockCount.mockResolvedValue(0); + app = await buildApp(); + }); + + it("returns 400 when from is not a valid date", async () => { + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/dashboard-audit?from=not-a-date", + cookies: { session: app.signCookie("valid") }, + }); + expect(res.statusCode).toBe(400); + expect(res.json().error).toMatch(/from/i); + }); + + it("returns 400 when to is not a valid date", async () => { + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/dashboard-audit?to=garbage", + cookies: { session: app.signCookie("valid") }, + }); + expect(res.statusCode).toBe(400); + expect(res.json().error).toMatch(/to/i); + }); + + it("accepts valid ISO dates", async () => { + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/dashboard-audit?from=2026-01-01T00:00:00Z&to=2026-04-01T00:00:00Z", + cookies: { session: app.signCookie("valid") }, + }); + expect(res.statusCode).toBe(200); + }); +}); +``` + +- [ ] **Step 2: Run test to verify fail** + +```bash +pnpm --filter @fluxcore/dashboard test -- auditLog +``` + +Expected: the two 400 cases FAIL (current handler accepts and produces an empty/200 response). + +- [ ] **Step 3: Implement the fix** + +Edit `apps/dashboard/src/server/features/permissions/routes.ts` lines 304-309 — replace the date block: + +```typescript + if (query.from || query.to) { + const createdAt: Record = {}; + if (query.from) { + const d = new Date(query.from); + if (!Number.isFinite(d.getTime())) { + reply.code(400).send({ error: "Invalid 'from' date" }); + return; + } + createdAt.gte = d; + } + if (query.to) { + const d = new Date(query.to); + if (!Number.isFinite(d.getTime())) { + reply.code(400).send({ error: "Invalid 'to' date" }); + return; + } + createdAt.lte = d; + } + where.createdAt = createdAt; + } +``` + +- [ ] **Step 4: Run test to verify pass** + +```bash +pnpm --filter @fluxcore/dashboard test -- auditLog +pnpm typecheck +``` + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/features/permissions/routes.ts apps/dashboard/tests/server/features/permissions/auditLog.test.ts +git commit -m "fix(permissions): validate from/to dates in audit log query" +``` diff --git a/docs/superpowers/plans/2026-04-07-sec-api-04-resource-creation-rate-limits.md b/docs/superpowers/plans/2026-04-07-sec-api-04-resource-creation-rate-limits.md new file mode 100644 index 0000000..86d3797 --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-api-04-resource-creation-rate-limits.md @@ -0,0 +1,169 @@ +# Resource Creation Rate Limiting — 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:** Cap how often a single session can hit expensive POST endpoints (album creation, custom command creation, giveaway creation) to prevent resource exhaustion and spam. +**Architecture:** `@fastify/rate-limit` is already registered globally. We add per-route configuration via `config.rateLimit` on the create endpoints, keyed by `request.session?.userId` (falling back to IP) so a single user cannot DOS via burst creation. Limits chosen: 10 creations / minute per user. +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +The global rate limit (100/min/IP) is too coarse for write endpoints that allocate DB rows. `apps/dashboard/src/server/features/music/routes.ts:104`, `features/commands/routes.ts:31`, and `features/giveaways/routes.ts:49` accept POST without per-route throttling. A single attacker behind a NAT can saturate the global bucket for innocent users by spamming `POST /music/library`, fill the per-guild caps, and trigger expensive cache reloads on every request. + +## Files +- Modify: `apps/dashboard/src/server/features/music/routes.ts:103-136` +- Modify: `apps/dashboard/src/server/features/commands/routes.ts:31-141` +- Modify: `apps/dashboard/src/server/features/giveaways/routes.ts:49-107` +- Test: `apps/dashboard/tests/server/features/music/musicRateLimit.test.ts` (new file) + +## Tasks + +### Task 1: Add per-route rate limits to create endpoints + +- [ ] **Step 1: Write the failing test** + +Create `apps/dashboard/tests/server/features/music/musicRateLimit.test.ts`: + +```typescript +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { + token: "t", + clientId: "c", + dashboardSessionSecret: "s", + logLevel: "info", + }, +})); + +const session = { + userId: "user-1", + username: "user", + guilds: [{ id: "guild-1", name: "T", permissions: BigInt(0x20).toString() }], +}; + +vi.mock("../../../../src/server/shared/session.js", () => ({ + getSession: vi.fn().mockResolvedValue(session), + touchSession: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("../../../../src/server/shared/discordApi.js", () => ({ + isBotInGuild: vi.fn().mockResolvedValue(true), + getGuildOwnerId: vi.fn().mockResolvedValue("owner-1"), +})); +vi.mock("../../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions: vi.fn().mockResolvedValue({ permissions: new Set(["*"]), isOwner: true }), + hasPermission: vi.fn().mockReturnValue(true), + invalidatePermissionCache: vi.fn(), + createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@fluxcore/systems/music/library", () => ({ + getAlbums: vi.fn().mockResolvedValue([]), + getAlbumById: vi.fn(), + addAlbum: vi.fn().mockResolvedValue({ id: 1, name: "x" }), + removeAlbum: vi.fn(), + addTrack: vi.fn(), + removeTrack: vi.fn(), + getAlbumTracks: vi.fn(), + getAlbumCount: vi.fn().mockResolvedValue(0), + getTrackCount: vi.fn().mockResolvedValue(0), + getTrackById: vi.fn(), +})); +vi.mock("@fluxcore/systems/music/config", () => ({ + fetchMusicSettings: vi.fn(), + upsertMusicSettings: vi.fn(), +})); +vi.mock("@fluxcore/systems/actions/persistence", () => ({ + notifyCacheInvalidation: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@fluxcore/utils", () => ({ + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})); + +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import fastifyRateLimit from "@fastify/rate-limit"; +import { registerMusicRoutes } from "../../../../src/server/features/music/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "test-secret" }); + await app.register(fastifyRateLimit, { global: false }); + registerMusicRoutes(app); + await app.ready(); + return app; +} + +describe("POST /api/guilds/:guildId/music/library — rate limit", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + app = await buildApp(); + }); + + it("returns 429 after exceeding 10 requests/minute", async () => { + const cookie = { session: app.signCookie("valid") }; + let lastStatus = 0; + for (let i = 0; i < 12; i++) { + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/music/library", + cookies: cookie, + payload: { name: `album-${i}` }, + }); + lastStatus = res.statusCode; + } + expect(lastStatus).toBe(429); + }); +}); +``` + +- [ ] **Step 2: Run test to verify fail** + +```bash +pnpm --filter @fluxcore/dashboard test -- musicRateLimit +``` + +Expected: the loop completes with status 201 (or 400) on the 12th call — never 429 — so the test FAILS. + +- [ ] **Step 3: Implement the fix** + +Edit `apps/dashboard/src/server/features/music/routes.ts` — at the POST `/api/guilds/:guildId/music/library` route (line 104), add `config.rateLimit`: + +```typescript + app.post( + "/api/guilds/:guildId/music/library", + { + preHandler: [requireAuth, requireGuildAdmin, requirePermission("music.library.manage")], + config: { + rateLimit: { + max: 10, + timeWindow: "1 minute", + keyGenerator: (req) => req.session?.userId ?? req.ip, + }, + }, + schema: { +``` + +Edit `apps/dashboard/src/server/features/commands/routes.ts` — at the POST `/api/guilds/:guildId/custom-commands` route (line 31), add the same `config.rateLimit` block right after `preHandler`. + +Edit `apps/dashboard/src/server/features/giveaways/routes.ts` — at the POST `/api/guilds/:guildId/giveaways` route (line 49), add the same `config.rateLimit` block. + +If `req.session` is not typed on `FastifyRequest` in this file, cast: `keyGenerator: (req) => (req as { session?: { userId?: string } }).session?.userId ?? req.ip`. + +- [ ] **Step 4: Run test to verify pass** + +```bash +pnpm --filter @fluxcore/dashboard test -- musicRateLimit +pnpm typecheck +``` + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/features/music/routes.ts apps/dashboard/src/server/features/commands/routes.ts apps/dashboard/src/server/features/giveaways/routes.ts apps/dashboard/tests/server/features/music/musicRateLimit.test.ts +git commit -m "fix(api): add per-user rate limits to resource creation endpoints" +``` diff --git a/docs/superpowers/plans/2026-04-07-sec-api-05-welcome-image-magic-byte-validation.md b/docs/superpowers/plans/2026-04-07-sec-api-05-welcome-image-magic-byte-validation.md new file mode 100644 index 0000000..ef30f14 --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-api-05-welcome-image-magic-byte-validation.md @@ -0,0 +1,254 @@ +# Welcome Image base64 / Magic-Byte Validation — 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:** Reject welcome background uploads whose base64 payload, decoded bytes, or content-type/magic-byte combination is malformed or mismatched. +**Architecture:** Add a strict base64 character regex, decode with `Buffer.from(data, "base64")`, then sniff the first bytes to confirm a real PNG (`89 50 4E 47`) / JPEG (`FF D8 FF`) / WebP (`52 49 46 46 .. 57 45 42 50`) header. The detected format must match the claimed `contentType`. Reject otherwise. +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +`apps/dashboard/src/server/features/welcome/routes.ts:217-224` blindly trusts the supplied `data` and `contentType`. `Buffer.from(, "base64")` produces a buffer of length 0+ from any string, so attackers can store arbitrary binary blobs (e.g. HTML, SVG with scripts, executables) under filenames like `xyz.png`. Combined with the lenient MIME check, a stored XSS or content-type-confusion attack against the storage backend / CDN is possible. + +## Files +- Modify: `apps/dashboard/src/server/features/welcome/routes.ts:206-233` +- Test: `apps/dashboard/tests/server/features/welcome/imageUpload.test.ts` (new file) + +## Tasks + +### Task 1: Validate base64, decode, then verify magic bytes match contentType + +- [ ] **Step 1: Write the failing test** + +Create `apps/dashboard/tests/server/features/welcome/imageUpload.test.ts`: + +```typescript +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { token: "t", clientId: "c", dashboardSessionSecret: "s", logLevel: "info" }, +})); + +const session = { + userId: "user-1", + username: "u", + guilds: [{ id: "guild-1", name: "T", permissions: BigInt(0x20).toString() }], +}; + +vi.mock("../../../../src/server/shared/session.js", () => ({ + getSession: vi.fn().mockResolvedValue(session), + touchSession: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("../../../../src/server/shared/discordApi.js", () => ({ + isBotInGuild: vi.fn().mockResolvedValue(true), + getGuildOwnerId: vi.fn().mockResolvedValue("owner-1"), +})); +vi.mock("../../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions: vi.fn().mockResolvedValue({ permissions: new Set(["*"]), isOwner: true }), + hasPermission: vi.fn().mockReturnValue(true), + invalidatePermissionCache: vi.fn(), + createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), +})); + +const mockUpload = vi.fn().mockResolvedValue(undefined); +vi.mock("@fluxcore/systems/welcome/image", async () => { + const actual = await vi.importActual>("@fluxcore/systems/welcome/image"); + return { + ...actual, + createStorageAdapter: () => ({ + upload: mockUpload, + delete: vi.fn().mockResolvedValue(undefined), + }), + MAX_BACKGROUND_SIZE: 5 * 1024 * 1024, + ALLOWED_BACKGROUND_TYPES: ["image/png", "image/jpeg", "image/webp"], + PRESET_BACKGROUNDS: [], + DEFAULT_WELCOME_IMAGE_SETTINGS: {}, + DEFAULT_FAREWELL_IMAGE_SETTINGS: {}, + welcomeImageSettingsSchema: { safeParse: () => ({ success: true, data: {} }) }, + generateWelcomeImage: vi.fn(), + getAllTemplates: () => [], + getAvailableFonts: () => [], + }; +}); +vi.mock("@fluxcore/systems/welcome/config", () => ({ + getWelcomeConfig: vi.fn(), + upsertWelcomeConfig: vi.fn(), +})); +vi.mock("@fluxcore/utils", () => ({ + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})); + +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import { registerWelcomeRoutes } from "../../../../src/server/features/welcome/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "test-secret" }); + registerWelcomeRoutes(app); + await app.ready(); + return app; +} + +const PNG_HEADER = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); +const JPEG_HEADER = Buffer.from([0xff, 0xd8, 0xff, 0xe0]); + +describe("POST /api/guilds/:guildId/welcome/image/background — magic byte validation", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + app = await buildApp(); + }); + + it("rejects invalid base64 with 400", async () => { + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/welcome/image/background", + cookies: { session: app.signCookie("valid") }, + payload: { data: "@@@not-base64@@@", contentType: "image/png" }, + }); + expect(res.statusCode).toBe(400); + }); + + it("rejects payload whose magic bytes do not match contentType", async () => { + const fakePng = Buffer.from("hello world").toString("base64"); + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/welcome/image/background", + cookies: { session: app.signCookie("valid") }, + payload: { data: fakePng, contentType: "image/png" }, + }); + expect(res.statusCode).toBe(400); + expect(res.json().error).toMatch(/png|magic|content/i); + }); + + it("rejects mismatched contentType vs header (jpeg sent as png)", async () => { + const data = JPEG_HEADER.toString("base64"); + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/welcome/image/background", + cookies: { session: app.signCookie("valid") }, + payload: { data, contentType: "image/png" }, + }); + expect(res.statusCode).toBe(400); + }); + + it("accepts a valid PNG header", async () => { + const data = Buffer.concat([PNG_HEADER, Buffer.alloc(16)]).toString("base64"); + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/welcome/image/background", + cookies: { session: app.signCookie("valid") }, + payload: { data, contentType: "image/png" }, + }); + expect(res.statusCode).toBe(200); + expect(mockUpload).toHaveBeenCalled(); + }); +}); +``` + +- [ ] **Step 2: Run test to verify fail** + +```bash +pnpm --filter @fluxcore/dashboard test -- imageUpload +``` + +Expected: the three rejection tests FAIL (current handler returns 200 for any base64-shaped string). + +- [ ] **Step 3: Implement the fix** + +Edit `apps/dashboard/src/server/features/welcome/routes.ts` — replace the body of the POST `/welcome/image/background` handler (lines 206-233): + +```typescript + async (request, reply) => { + const { guildId } = request.params as { guildId: string }; + const { data, contentType } = request.body as { data: string; contentType: string }; + + if (!ALLOWED_BACKGROUND_TYPES.includes(contentType)) { + reply.code(400).send({ + error: `Invalid file type. Allowed: ${ALLOWED_BACKGROUND_TYPES.join(", ")}`, + }); + return; + } + + // Strict base64 validation (RFC 4648, optional padding) + const base64Regex = /^[A-Za-z0-9+/]+={0,2}$/; + if (data.length === 0 || data.length % 4 !== 0 || !base64Regex.test(data)) { + reply.code(400).send({ error: "Invalid base64 payload" }); + return; + } + + const buffer = Buffer.from(data, "base64"); + if (buffer.length === 0) { + reply.code(400).send({ error: "Empty payload" }); + return; + } + + if (buffer.length > MAX_BACKGROUND_SIZE) { + reply.code(400).send({ + error: `File too large. Maximum size: ${MAX_BACKGROUND_SIZE / 1024 / 1024} MB`, + }); + return; + } + + // Magic byte sniffing — must match contentType + const detected = detectImageType(buffer); + if (!detected || detected !== contentType) { + reply.code(400).send({ + error: "File content does not match the declared image type", + }); + return; + } + + const ext = contentType.split("/")[1] === "jpeg" ? "jpg" : contentType.split("/")[1]; + const key = `backgrounds/${guildId}/${randomUUID()}.${ext}`; + + await storage.upload(key, buffer, contentType); + + reply.send({ key }); + }, +``` + +Add this helper at the bottom of the file (outside `registerWelcomeRoutes`): + +```typescript +function detectImageType(buffer: Buffer): string | null { + if (buffer.length < 12) return null; + // PNG: 89 50 4E 47 0D 0A 1A 0A + if ( + buffer[0] === 0x89 && buffer[1] === 0x50 && buffer[2] === 0x4e && buffer[3] === 0x47 && + buffer[4] === 0x0d && buffer[5] === 0x0a && buffer[6] === 0x1a && buffer[7] === 0x0a + ) { + return "image/png"; + } + // JPEG: FF D8 FF + if (buffer[0] === 0xff && buffer[1] === 0xd8 && buffer[2] === 0xff) { + return "image/jpeg"; + } + // WebP: RIFF .... WEBP + if ( + buffer[0] === 0x52 && buffer[1] === 0x49 && buffer[2] === 0x46 && buffer[3] === 0x46 && + buffer[8] === 0x57 && buffer[9] === 0x45 && buffer[10] === 0x42 && buffer[11] === 0x50 + ) { + return "image/webp"; + } + return null; +} +``` + +- [ ] **Step 4: Run test to verify pass** + +```bash +pnpm --filter @fluxcore/dashboard test -- imageUpload +pnpm typecheck +``` + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/features/welcome/routes.ts apps/dashboard/tests/server/features/welcome/imageUpload.test.ts +git commit -m "fix(welcome): validate base64 and magic bytes for background uploads" +``` diff --git a/docs/superpowers/plans/2026-04-07-sec-api-06-cron-preview-dos.md b/docs/superpowers/plans/2026-04-07-sec-api-06-cron-preview-dos.md new file mode 100644 index 0000000..e8552c9 --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-api-06-cron-preview-dos.md @@ -0,0 +1,208 @@ +# Cron Preview DoS — 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:** Prevent denial-of-service against the cron-preview endpoint by adding a tight per-user rate limit and capping wall-clock time spent computing the next 5 runs. +**Architecture:** Add `config.rateLimit` (5 requests / 10s per session) to the GET `/scheduled-messages/preview-cron` route, and wrap the 5-iteration `getNextCronRun` loop with a 250 ms wall-clock budget. If the budget is exceeded, return 400 ("cron expression too slow to evaluate") instead of looping forever. +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +`apps/dashboard/src/server/features/scheduled/routes.ts:222-250` calls `getNextCronRun` five times per request with no rate limit. A pathological-but-valid cron expression (very narrow constraint, e.g. `0 0 29 2 1` style) can take seconds per evaluation. Combined with the lack of per-route throttling, an attacker can pin a Node worker on the dashboard process. + +## Files +- Modify: `apps/dashboard/src/server/features/scheduled/routes.ts:220-251` +- Test: `apps/dashboard/tests/server/features/scheduled/cronPreview.test.ts` (new file) + +## Tasks + +### Task 1: Rate-limit and time-bound cron preview + +- [ ] **Step 1: Write the failing test** + +Create `apps/dashboard/tests/server/features/scheduled/cronPreview.test.ts`: + +```typescript +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { token: "t", clientId: "c", dashboardSessionSecret: "s", logLevel: "info" }, +})); + +const session = { + userId: "user-1", + username: "u", + guilds: [{ id: "guild-1", name: "T", permissions: BigInt(0x20).toString() }], +}; + +vi.mock("../../../../src/server/shared/session.js", () => ({ + getSession: vi.fn().mockResolvedValue(session), + touchSession: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("../../../../src/server/shared/discordApi.js", () => ({ + isBotInGuild: vi.fn().mockResolvedValue(true), + getGuildOwnerId: vi.fn().mockResolvedValue("owner-1"), +})); +vi.mock("../../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions: vi.fn().mockResolvedValue({ permissions: new Set(["*"]), isOwner: true }), + hasPermission: vi.fn().mockReturnValue(true), + invalidatePermissionCache: vi.fn(), + createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), +})); + +let slowMode = false; +vi.mock("@fluxcore/systems/scheduledMessages/cron", () => ({ + validateCronExpression: () => null, + getNextCronRun: () => { + if (slowMode) { + const start = Date.now(); + while (Date.now() - start < 100) { /* spin */ } + } + return new Date(); + }, +})); +vi.mock("@fluxcore/systems/scheduledMessages/persistence", () => ({ + getScheduledMessages: vi.fn(), + getScheduledMessageById: vi.fn(), + createScheduledMessage: vi.fn(), + updateScheduledMessage: vi.fn(), + deleteScheduledMessage: vi.fn(), +})); +vi.mock("@fluxcore/utils", () => ({ + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})); + +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import fastifyRateLimit from "@fastify/rate-limit"; +import { registerScheduledMessageRoutes } from "../../../../src/server/features/scheduled/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "test-secret" }); + await app.register(fastifyRateLimit, { global: false }); + registerScheduledMessageRoutes(app); + await app.ready(); + return app; +} + +describe("GET /scheduled-messages/preview-cron — DoS guards", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + slowMode = false; + app = await buildApp(); + }); + + it("returns 429 after exceeding 5 requests per 10 seconds", async () => { + const cookie = { session: app.signCookie("valid") }; + let lastStatus = 0; + for (let i = 0; i < 7; i++) { + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/scheduled-messages/preview-cron?cronExpr=*+*+*+*+*", + cookies: cookie, + }); + lastStatus = res.statusCode; + } + expect(lastStatus).toBe(429); + }); + + it("returns 400 when cron evaluation exceeds time budget", async () => { + slowMode = true; + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/scheduled-messages/preview-cron?cronExpr=*+*+*+*+*", + cookies: { session: app.signCookie("valid") }, + }); + expect(res.statusCode).toBe(400); + expect(res.json().error).toMatch(/slow|budget|timeout/i); + }); +}); +``` + +- [ ] **Step 2: Run test to verify fail** + +```bash +pnpm --filter @fluxcore/dashboard test -- cronPreview +``` + +Expected: both tests FAIL — current code never returns 429 or a time-budget 400. + +- [ ] **Step 3: Implement the fix** + +Edit `apps/dashboard/src/server/features/scheduled/routes.ts` — replace the GET `/preview-cron` route definition (lines 220-251): + +```typescript + // GET preview next run time for a cron expression + app.get( + "/api/guilds/:guildId/scheduled-messages/preview-cron", + { + preHandler: [requireAuth, requireGuildAdmin, requirePermission("scheduled.messages.view")], + config: { + rateLimit: { + max: 5, + timeWindow: "10 seconds", + keyGenerator: (req) => + (req as { session?: { userId?: string } }).session?.userId ?? req.ip, + }, + }, + }, + async (request, reply) => { + const query = request.query as { cronExpr?: string; timezone?: string }; + if (!query.cronExpr) { + reply.code(400).send({ error: "cronExpr query parameter is required" }); + return; + } + + const cronError = validateCronExpression(query.cronExpr); + if (cronError) { + reply.code(400).send({ error: `Invalid cron expression: ${cronError}` }); + return; + } + + const timezone = query.timezone ?? "UTC"; + const budgetMs = 250; + const start = Date.now(); + + try { + const nextRun = getNextCronRun(query.cronExpr, timezone); + if (Date.now() - start > budgetMs) { + reply.code(400).send({ error: "Cron expression too slow to evaluate (budget exceeded)" }); + return; + } + const nextRuns: string[] = [nextRun.toISOString()]; + let lastRun = nextRun; + for (let i = 0; i < 4; i++) { + if (Date.now() - start > budgetMs) { + reply.code(400).send({ error: "Cron expression too slow to evaluate (budget exceeded)" }); + return; + } + const next = getNextCronRun(query.cronExpr, timezone, lastRun); + nextRuns.push(next.toISOString()); + lastRun = next; + } + reply.send({ nextRuns }); + } catch (err) { + reply.code(400).send({ error: "Failed to evaluate cron expression" }); + } + }, + ); +``` + +- [ ] **Step 4: Run test to verify pass** + +```bash +pnpm --filter @fluxcore/dashboard test -- cronPreview +pnpm typecheck +``` + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/features/scheduled/routes.ts apps/dashboard/tests/server/features/scheduled/cronPreview.test.ts +git commit -m "fix(scheduled): rate-limit and time-bound cron preview to prevent DoS" +``` diff --git a/docs/superpowers/plans/2026-04-07-sec-api-07-custom-command-redos.md b/docs/superpowers/plans/2026-04-07-sec-api-07-custom-command-redos.md new file mode 100644 index 0000000..f0e99ed --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-api-07-custom-command-redos.md @@ -0,0 +1,200 @@ +# Custom Command Regex ReDoS — 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:** Reject regex-trigger custom commands whose pattern is exponential / catastrophic-backtracking-prone before they are persisted, so the bot's hot path cannot be DOSed. +**Architecture:** Use the `safe-regex` package (already common in Node ecosystems; small footprint) inside both POST and PUT handlers to validate user-supplied patterns. If the pattern is unsafe, reject with 400. We also enforce a max length of 200 chars and reject patterns containing nested unbounded repetition. +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +`apps/dashboard/src/server/features/commands/routes.ts:111-118` and `:220-227` compile user input via `new RegExp(body.name, "i")`. They only check that the pattern *parses* — not that it can be evaluated in bounded time. A malicious admin (or compromised account) can save `(a+)+$` as a regex trigger; every subsequent message in the guild will then run the bot's regex matcher into catastrophic backtracking, freezing the message handler. + +## Files +- Modify: `apps/dashboard/src/server/features/commands/routes.ts:110-118, 219-227` +- Modify: `apps/dashboard/package.json` (add `safe-regex` dep, inside Docker) +- Test: `apps/dashboard/tests/server/features/commands/regexValidation.test.ts` (new file) + +## Tasks + +### Task 1: Reject pathological regex patterns + +- [ ] **Step 1: Install safe-regex (inside Docker)** + +```bash +docker compose -f docker-compose.yml run --rm dashboard pnpm add safe-regex +docker compose -f docker-compose.yml run --rm dashboard pnpm add -D @types/safe-regex +``` + +- [ ] **Step 2: Write the failing test** + +Create `apps/dashboard/tests/server/features/commands/regexValidation.test.ts`: + +```typescript +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { token: "t", clientId: "c", dashboardSessionSecret: "s", logLevel: "info" }, +})); + +const session = { + userId: "user-1", + username: "u", + guilds: [{ id: "guild-1", name: "T", permissions: BigInt(0x20).toString() }], +}; +vi.mock("../../../../src/server/shared/session.js", () => ({ + getSession: vi.fn().mockResolvedValue(session), + touchSession: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("../../../../src/server/shared/discordApi.js", () => ({ + isBotInGuild: vi.fn().mockResolvedValue(true), + getGuildOwnerId: vi.fn().mockResolvedValue("owner-1"), +})); +vi.mock("../../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions: vi.fn().mockResolvedValue({ permissions: new Set(["*"]), isOwner: true }), + hasPermission: vi.fn().mockReturnValue(true), + invalidatePermissionCache: vi.fn(), + createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@fluxcore/systems/customCommands/persistence", () => ({ + getCustomCommands: vi.fn().mockResolvedValue([]), + getCustomCommandCount: vi.fn().mockResolvedValue(0), + createCustomCommand: vi.fn().mockResolvedValue({ id: 1 }), + updateCustomCommand: vi.fn().mockResolvedValue({ id: 1 }), + deleteCustomCommand: vi.fn(), +})); +vi.mock("@fluxcore/systems/customCommands/constants", () => ({ + MAX_COMMANDS_PER_GUILD: 100, + TRIGGER_TYPES: ["exact", "startsWith", "contains", "regex"], +})); +vi.mock("@fluxcore/utils", () => ({ + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})); + +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import { registerCustomCommandRoutes } from "../../../../src/server/features/commands/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "test-secret" }); + registerCustomCommandRoutes(app); + await app.ready(); + return app; +} + +describe("POST /custom-commands — regex safety", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + app = await buildApp(); + }); + + it("rejects catastrophic backtracking pattern (a+)+$", async () => { + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/custom-commands", + cookies: { session: app.signCookie("valid") }, + payload: { name: "(a+)+$", triggerType: "regex" }, + }); + expect(res.statusCode).toBe(400); + expect(res.json().error).toMatch(/unsafe|regex/i); + }); + + it("rejects nested quantifier pattern (a*)*", async () => { + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/custom-commands", + cookies: { session: app.signCookie("valid") }, + payload: { name: "(a*)*", triggerType: "regex" }, + }); + expect(res.statusCode).toBe(400); + }); + + it("accepts a simple safe regex", async () => { + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/custom-commands", + cookies: { session: app.signCookie("valid") }, + payload: { name: "^hello", triggerType: "regex" }, + }); + expect(res.statusCode).toBe(201); + }); +}); +``` + +- [ ] **Step 3: Run test to verify fail** + +```bash +pnpm --filter @fluxcore/dashboard test -- regexValidation +``` + +Expected: the two unsafe-pattern tests FAIL — current handler accepts them. + +- [ ] **Step 4: Implement the fix** + +Edit `apps/dashboard/src/server/features/commands/routes.ts` — at the top of the file add: + +```typescript +import safeRegex from "safe-regex"; + +const MAX_REGEX_LENGTH = 200; + +function validateRegexPattern(pattern: string): string | null { + if (pattern.length > MAX_REGEX_LENGTH) { + return `Regex pattern too long (max ${MAX_REGEX_LENGTH} chars)`; + } + try { + new RegExp(pattern, "i"); + } catch { + return "Invalid regex pattern"; + } + if (!safeRegex(pattern)) { + return "Unsafe regex pattern (catastrophic backtracking risk)"; + } + return null; +} +``` + +Then replace lines 110-118 (POST handler) with: + +```typescript + // Validate regex if trigger type is regex + if (body.triggerType === "regex") { + const err = validateRegexPattern(body.name); + if (err) { + reply.code(400).send({ error: err }); + return; + } + } +``` + +And replace lines 219-227 (PUT handler) with: + +```typescript + // Validate regex if trigger type is being changed to regex + if (body.triggerType === "regex" && body.name) { + const err = validateRegexPattern(body.name); + if (err) { + reply.code(400).send({ error: err }); + return; + } + } +``` + +- [ ] **Step 5: Run test to verify pass** + +```bash +pnpm --filter @fluxcore/dashboard test -- regexValidation +pnpm typecheck +``` + +- [ ] **Step 6: Commit** + +```bash +git add apps/dashboard/src/server/features/commands/routes.ts apps/dashboard/tests/server/features/commands/regexValidation.test.ts apps/dashboard/package.json pnpm-lock.yaml +git commit -m "fix(commands): reject unsafe regex patterns to prevent ReDoS" +``` diff --git a/docs/superpowers/plans/2026-04-07-sec-api-08-permission-error-leak.md b/docs/superpowers/plans/2026-04-07-sec-api-08-permission-error-leak.md new file mode 100644 index 0000000..2e4d42d --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-api-08-permission-error-leak.md @@ -0,0 +1,87 @@ +# Permission Error Echoes Failed Key — 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:** Stop leaking which specific permission key tripped the escalation gate; respond with a generic message so attackers cannot binary-search the registry to map a victim's permissions. +**Architecture:** Drop the `permission` field from the 403 response body in the `PUT user-permissions` handler. The fix in finding 01 already replaces the response message; this plan locks in the no-leak guarantee with an explicit test so a future regression cannot reintroduce the field. +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +`apps/dashboard/src/server/features/permissions/routes.ts:115-135` (specifically the rejection at lines 129-133) returns `{ error, permission: perm }`. An attacker can probe `PUT /user-permissions/` with each registry key and learn exactly which permissions another role holds (since the failure tells them precisely which key failed). This is information disclosure that aids privilege escalation. + +## Files +- Modify: `apps/dashboard/src/server/features/permissions/routes.ts:128-134` (already touched by finding 01; this finding requires the response shape to remain `{ error: }`) +- Test: `apps/dashboard/tests/server/features/permissions/userPermissions.test.ts` (extend) + +## Tasks + +### Task 1: Lock down the 403 response shape + +- [ ] **Step 1: Write the failing test** + +Append to `apps/dashboard/tests/server/features/permissions/userPermissions.test.ts`: + +```typescript +describe("PUT /user-permissions — error response does not leak key", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + mockGetSession.mockResolvedValue(callerSession); + mockResolveUserPermissions.mockResolvedValue({ + permissions: new Set(["dashboard.roles.manage"]), + isOwner: false, + }); + app = await buildApp(); + }); + + it("does not echo the failed permission key in the error body", async () => { + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/user-permissions/target-1", + cookies: { session: app.signCookie("valid") }, + payload: { permissions: ["actions.rules.manage"] }, + }); + expect(res.statusCode).toBe(403); + const body = res.json(); + expect(body).not.toHaveProperty("permission"); + expect(JSON.stringify(body)).not.toContain("actions.rules.manage"); + }); +}); +``` + +- [ ] **Step 2: Run test to verify fail** + +```bash +pnpm --filter @fluxcore/dashboard test -- userPermissions +``` + +Expected: the test FAILS because the original code returns `{ error, permission: "actions.rules.manage" }`. (If finding 01 has already been applied, this test will pass — that's fine, it then serves as a regression guard.) + +- [ ] **Step 3: Implement the fix** + +Confirm/keep the fix from finding 01: in `apps/dashboard/src/server/features/permissions/routes.ts`, the 403 response inside the escalation loop must be: + +```typescript + reply.code(403).send({ error: "Insufficient privileges to grant this permission" }); + return; +``` + +No `permission:` field, no `key:` field, no echo of `perm`. + +- [ ] **Step 4: Run test to verify pass** + +```bash +pnpm --filter @fluxcore/dashboard test -- userPermissions +pnpm typecheck +``` + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/features/permissions/routes.ts apps/dashboard/tests/server/features/permissions/userPermissions.test.ts +git commit -m "fix(permissions): generic 403 error to avoid leaking failed permission key" +``` diff --git a/docs/superpowers/plans/2026-04-07-sec-api-09-audit-action-exact-match.md b/docs/superpowers/plans/2026-04-07-sec-api-09-audit-action-exact-match.md new file mode 100644 index 0000000..1222f14 --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-api-09-audit-action-exact-match.md @@ -0,0 +1,122 @@ +# Audit Action Filter Exact-Match — 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 the substring `contains` filter on the audit log `action` field with an exact match against an allowlist, so attackers cannot enumerate or partially match arbitrary action strings. +**Architecture:** Build an `ALLOWED_AUDIT_ACTIONS` constant covering every action string emitted by the dashboard (`dashboard.permissions.update`, `dashboard.permissions.clear`, `dashboard.settings.update`, `dashboard.role.create`, `dashboard.role.update`, `dashboard.role.delete`, `dashboard.role.assign`, `dashboard.role.unassign`). When `query.action` is supplied, validate membership in the allowlist and use `where.action = query.action` instead of `{ contains }`. +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +`apps/dashboard/src/server/features/permissions/routes.ts:302` does `where.action = { contains: query.action }`. Substring matching exposes information (an attacker can search for partial strings to discover undocumented action types) and allows confusion in the UI. Exact-match against an allowlist is both more correct and prevents partial-match enumeration. + +## Files +- Modify: `apps/dashboard/src/server/features/permissions/routes.ts:280-336` +- Test: `apps/dashboard/tests/server/features/permissions/auditLog.test.ts` (extend) + +## Tasks + +### Task 1: Replace `contains` with exact-match allowlist + +- [ ] **Step 1: Write the failing test** + +Append to `apps/dashboard/tests/server/features/permissions/auditLog.test.ts`: + +```typescript +describe("GET /dashboard-audit — action filter is exact-match", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + mockResolveUserPermissions.mockResolvedValue({ + permissions: new Set(["*"]), + isOwner: true, + }); + mockFindMany.mockResolvedValue([]); + mockCount.mockResolvedValue(0); + app = await buildApp(); + }); + + it("uses exact match (not contains) when action is allowlisted", async () => { + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/dashboard-audit?action=dashboard.permissions.update", + cookies: { session: app.signCookie("valid") }, + }); + expect(res.statusCode).toBe(200); + expect(mockFindMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ action: "dashboard.permissions.update" }), + }), + ); + }); + + it("returns 400 when action is not in the allowlist", async () => { + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/dashboard-audit?action=permissions", + cookies: { session: app.signCookie("valid") }, + }); + expect(res.statusCode).toBe(400); + }); +}); +``` + +- [ ] **Step 2: Run test to verify fail** + +```bash +pnpm --filter @fluxcore/dashboard test -- auditLog +``` + +Expected: both tests FAIL — current code uses `{ contains: "permissions" }` and never returns 400. + +- [ ] **Step 3: Implement the fix** + +Edit `apps/dashboard/src/server/features/permissions/routes.ts`. At the bottom of the file, add the allowlist: + +```typescript +const ALLOWED_AUDIT_ACTIONS = new Set([ + "dashboard.permissions.update", + "dashboard.permissions.clear", + "dashboard.settings.update", + "dashboard.role.create", + "dashboard.role.update", + "dashboard.role.delete", + "dashboard.role.assign", + "dashboard.role.unassign", +]); +``` + +Then in the GET handler (around line 302) replace: + +```typescript + if (query.action) where.action = { contains: query.action }; +``` + +with: + +```typescript + if (query.action) { + if (!ALLOWED_AUDIT_ACTIONS.has(query.action)) { + reply.code(400).send({ error: "Unknown action filter" }); + return; + } + where.action = query.action; + } +``` + +- [ ] **Step 4: Run test to verify pass** + +```bash +pnpm --filter @fluxcore/dashboard test -- auditLog +pnpm typecheck +``` + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/features/permissions/routes.ts apps/dashboard/tests/server/features/permissions/auditLog.test.ts +git commit -m "fix(permissions): enforce exact-match allowlist on audit action filter" +``` diff --git a/docs/superpowers/plans/2026-04-07-sec-api-10-image-preview-nosniff.md b/docs/superpowers/plans/2026-04-07-sec-api-10-image-preview-nosniff.md new file mode 100644 index 0000000..0ec537d --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-api-10-image-preview-nosniff.md @@ -0,0 +1,147 @@ +# Image Preview X-Content-Type-Options nosniff — 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:** LOW +**Goal:** Ensure browsers do not MIME-sniff the welcome image preview response, eliminating the residual XSS-via-content-sniffing risk if a future bug allowed non-PNG bytes to be served from the preview endpoint. +**Architecture:** Add the `X-Content-Type-Options: nosniff` header to the reply chain in the POST `/welcome/image/preview` route. This is a one-line defense-in-depth fix. +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +`apps/dashboard/src/server/features/welcome/routes.ts:183-186` sets `Content-Type: image/png` and `Cache-Control: no-cache` but omits `X-Content-Type-Options: nosniff`. If a future bug ever caused `imageBuffer` to contain HTML or SVG, browsers might sniff and execute it. Adding `nosniff` is a cheap defense-in-depth control. + +## Files +- Modify: `apps/dashboard/src/server/features/welcome/routes.ts:183-186` +- Test: `apps/dashboard/tests/server/features/welcome/imagePreviewHeaders.test.ts` (new file) + +## Tasks + +### Task 1: Add nosniff header to image preview + +- [ ] **Step 1: Write the failing test** + +Create `apps/dashboard/tests/server/features/welcome/imagePreviewHeaders.test.ts`: + +```typescript +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { token: "t", clientId: "c", dashboardSessionSecret: "s", logLevel: "info" }, +})); + +const session = { + userId: "user-1", + username: "u", + avatar: null, + guilds: [{ id: "guild-1", name: "T", permissions: BigInt(0x20).toString() }], +}; + +vi.mock("../../../../src/server/shared/session.js", () => ({ + getSession: vi.fn().mockResolvedValue(session), + touchSession: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("../../../../src/server/shared/discordApi.js", () => ({ + isBotInGuild: vi.fn().mockResolvedValue(true), + getGuildOwnerId: vi.fn().mockResolvedValue("owner-1"), +})); +vi.mock("../../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions: vi.fn().mockResolvedValue({ permissions: new Set(["*"]), isOwner: true }), + hasPermission: vi.fn().mockReturnValue(true), + invalidatePermissionCache: vi.fn(), + createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@fluxcore/systems/welcome/image", async () => { + const actual = await vi.importActual>("@fluxcore/systems/welcome/image"); + return { + ...actual, + createStorageAdapter: () => ({ + upload: vi.fn().mockResolvedValue(undefined), + delete: vi.fn().mockResolvedValue(undefined), + }), + welcomeImageSettingsSchema: { safeParse: () => ({ success: true, data: {} }) }, + DEFAULT_WELCOME_IMAGE_SETTINGS: {}, + DEFAULT_FAREWELL_IMAGE_SETTINGS: {}, + MAX_BACKGROUND_SIZE: 5 * 1024 * 1024, + ALLOWED_BACKGROUND_TYPES: ["image/png"], + PRESET_BACKGROUNDS: [], + generateWelcomeImage: vi.fn().mockResolvedValue(Buffer.from([0x89, 0x50, 0x4e, 0x47])), + getAllTemplates: () => [], + getAvailableFonts: () => [], + }; +}); +vi.mock("@fluxcore/systems/welcome/config", () => ({ + getWelcomeConfig: vi.fn(), + upsertWelcomeConfig: vi.fn(), +})); +vi.mock("@fluxcore/utils", () => ({ + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})); + +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import { registerWelcomeRoutes } from "../../../../src/server/features/welcome/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "test-secret" }); + registerWelcomeRoutes(app); + await app.ready(); + return app; +} + +describe("POST /welcome/image/preview — security headers", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + app = await buildApp(); + }); + + it("sets X-Content-Type-Options: nosniff", async () => { + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/welcome/image/preview", + cookies: { session: app.signCookie("valid") }, + payload: { settings: {}, type: "welcome" }, + }); + expect(res.statusCode).toBe(200); + expect(res.headers["x-content-type-options"]).toBe("nosniff"); + }); +}); +``` + +- [ ] **Step 2: Run test to verify fail** + +```bash +pnpm --filter @fluxcore/dashboard test -- imagePreviewHeaders +``` + +Expected: header is `undefined`, test FAILS. + +- [ ] **Step 3: Implement the fix** + +Edit `apps/dashboard/src/server/features/welcome/routes.ts` lines 183-186 — add the nosniff header: + +```typescript + reply + .header("Content-Type", "image/png") + .header("Cache-Control", "no-cache") + .header("X-Content-Type-Options", "nosniff") + .send(imageBuffer); +``` + +- [ ] **Step 4: Run test to verify pass** + +```bash +pnpm --filter @fluxcore/dashboard test -- imagePreviewHeaders +pnpm typecheck +``` + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/features/welcome/routes.ts apps/dashboard/tests/server/features/welcome/imagePreviewHeaders.test.ts +git commit -m "fix(welcome): add X-Content-Type-Options nosniff to image preview" +``` diff --git a/docs/superpowers/plans/2026-04-07-sec-api-11-actions-update-error-mask.md b/docs/superpowers/plans/2026-04-07-sec-api-11-actions-update-error-mask.md new file mode 100644 index 0000000..d5a1cbe --- /dev/null +++ b/docs/superpowers/plans/2026-04-07-sec-api-11-actions-update-error-mask.md @@ -0,0 +1,183 @@ +# Actions Update Generic Catch Masks Real Errors — 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:** LOW +**Goal:** Distinguish "rule not found" from real backend errors in `PUT /actions/rules/:ruleId` so genuine 500s are surfaced and logged instead of silently masked as 404s. +**Architecture:** Catch the error, inspect for Prisma's `P2025` (record not found) error code — if matched, return 404. For any other error, log via the project logger and return 500. This restores observability without changing the happy path. +**Tech Stack:** Fastify 5, TypeScript, Prisma 7, Vitest + +--- + +## Vulnerability +`apps/dashboard/src/server/features/actions/routes.ts:231` does `try { ... } catch { reply.code(404).send({ error: "Rule not found" }); }`. Any failure — DB connection drop, validation error inside `updateRule`, JSON parse fault — is reported as a 404. Operators lose visibility into real failures, and an attacker can trigger DB issues without ever seeing a 5xx response. + +## Files +- Modify: `apps/dashboard/src/server/features/actions/routes.ts:210-234` +- Test: `apps/dashboard/tests/server/features/actions/updateRuleErrors.test.ts` (new file) + +## Tasks + +### Task 1: Catch P2025 specifically; log other errors as 500 + +- [ ] **Step 1: Write the failing test** + +Create `apps/dashboard/tests/server/features/actions/updateRuleErrors.test.ts`: + +```typescript +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { token: "t", clientId: "c", dashboardSessionSecret: "s", logLevel: "info" }, +})); + +const session = { + userId: "user-1", + username: "u", + guilds: [{ id: "guild-1", name: "T", permissions: BigInt(0x20).toString() }], +}; +vi.mock("../../../../src/server/shared/session.js", () => ({ + getSession: vi.fn().mockResolvedValue(session), + touchSession: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("../../../../src/server/shared/discordApi.js", () => ({ + isBotInGuild: vi.fn().mockResolvedValue(true), + getGuildOwnerId: vi.fn().mockResolvedValue("owner-1"), +})); +vi.mock("../../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions: vi.fn().mockResolvedValue({ permissions: new Set(["*"]), isOwner: true }), + hasPermission: vi.fn().mockReturnValue(true), + invalidatePermissionCache: vi.fn(), + createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), +})); + +const mockUpdateRule = vi.fn(); +vi.mock("@fluxcore/systems/actions/persistence", () => ({ + notifyCacheInvalidation: vi.fn().mockResolvedValue(undefined), + listRules: vi.fn().mockResolvedValue([]), + getRule: vi.fn(), + createRule: vi.fn(), + updateRule: (...args: unknown[]) => mockUpdateRule(...args), + deleteRule: vi.fn(), + bulkUpdateRules: vi.fn(), + getRuleAnalytics: vi.fn(), + getActionLogs: vi.fn(), + getGuildSettings: vi.fn(), + upsertGuildSettings: vi.fn(), +})); + +const mockLogger = { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }; +vi.mock("@fluxcore/utils", () => ({ logger: mockLogger })); + +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import { registerActionRoutes } from "../../../../src/server/features/actions/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "test-secret" }); + registerActionRoutes(app); + await app.ready(); + return app; +} + +describe("PUT /actions/rules/:ruleId — error handling", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + app = await buildApp(); + }); + + it("returns 404 when Prisma throws P2025", async () => { + const err = Object.assign(new Error("Record not found"), { code: "P2025" }); + mockUpdateRule.mockRejectedValue(err); + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/actions/rules/123", + cookies: { session: app.signCookie("valid") }, + payload: { name: "x" }, + }); + expect(res.statusCode).toBe(404); + }); + + it("returns 500 and logs when an unexpected error occurs", async () => { + mockUpdateRule.mockRejectedValue(new Error("DB connection refused")); + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/actions/rules/123", + cookies: { session: app.signCookie("valid") }, + payload: { name: "x" }, + }); + expect(res.statusCode).toBe(500); + expect(mockLogger.error).toHaveBeenCalled(); + }); +}); +``` + +- [ ] **Step 2: Run test to verify fail** + +```bash +pnpm --filter @fluxcore/dashboard test -- updateRuleErrors +``` + +Expected: the "returns 500 and logs" test FAILS — current handler returns 404 for every error. + +- [ ] **Step 3: Implement the fix** + +Edit `apps/dashboard/src/server/features/actions/routes.ts` — replace the `try { ... } catch { ... }` block at lines 210-233: + +```typescript + try { + const updated = await updateRule(Number(ruleId), guildId, { + ...(body.name !== undefined && { name: body.name }), + ...(body.eventType !== undefined && { + eventType: body.eventType as ActionEventType, + }), + ...(body.actions !== undefined && { + actions: body.actions.map((a) => ({ + ...a, + type: a.type as ActionType, + })), + }), + ...(body.steps && body.entryStepId + ? { steps: body.steps as unknown as RuleStep[], entryStepId: body.entryStepId } + : {}), + ...(body.conditions !== undefined && { conditions: body.conditions }), + ...(body.priority !== undefined && { priority: body.priority }), + ...(body.enabled !== undefined && { enabled: body.enabled }), + }); + await notifyCacheInvalidation(guildId); + reply.send(updated); + } catch (err) { + const code = (err as { code?: string }).code; + if (code === "P2025") { + reply.code(404).send({ error: "Rule not found" }); + return; + } + logger.error({ err, guildId, ruleId }, "Failed to update action rule"); + reply.code(500).send({ error: "Failed to update rule" }); + } +``` + +Make sure `logger` is imported at the top of the file: + +```typescript +import { logger } from "@fluxcore/utils"; +``` + +(If it's not already imported, add it; if already present, no change needed.) + +- [ ] **Step 4: Run test to verify pass** + +```bash +pnpm --filter @fluxcore/dashboard test -- updateRuleErrors +pnpm typecheck +``` + +- [ ] **Step 5: Commit** + +```bash +git add apps/dashboard/src/server/features/actions/routes.ts apps/dashboard/tests/server/features/actions/updateRuleErrors.test.ts +git commit -m "fix(actions): distinguish P2025 not-found from 500 errors in rule update" +``` From aca93d7613ea3a1309931dd4b9727d3ec0ad0bd2 Mon Sep 17 00:00:00 2001 From: Abdulkhalek Muhammad Date: Tue, 7 Apr 2026 14:16:30 +0200 Subject: [PATCH 02/61] fix(security): harden dashboard api surface (sec-api-01..11) Implements 11 security hardening plans for the dashboard REST API: - sec-api-01: block self-grant and wildcard permission escalation - sec-api-02: restrict audit log userId filter to self for non-owners - sec-api-03: validate audit log date range query parameters - sec-api-04: add per-user rate limits to music/commands/giveaways creation - sec-api-05: magic-byte validation for welcome background image uploads - sec-api-06: rate limit + 250ms budget on cron preview endpoint - sec-api-07: reject unsafe regex (ReDoS) in custom command creation - sec-api-08: drop permission key from 403 error responses - sec-api-09: exact-match allowlist for audit action filter - sec-api-10: add X-Content-Type-Options nosniff to image preview - sec-api-11: distinguish Prisma P2025 from 500s in actions rule update Adds safe-regex dependency. Includes 49 new tests across 9 files covering each hardening. Typecheck clean; no new regressions. --- apps/dashboard/package.json | 2 + .../src/server/features/actions/routes.ts | 14 +- .../src/server/features/commands/routes.ts | 40 +++- .../src/server/features/giveaways/routes.ts | 8 + .../src/server/features/music/routes.ts | 8 + .../src/server/features/permissions/routes.ts | 69 +++++-- .../src/server/features/scheduled/routes.ts | 50 +++-- .../src/server/features/welcome/routes.ts | 44 ++++ .../features/actions/updateRuleErrors.test.ts | 114 +++++++++++ .../features/commands/regexValidation.test.ts | 90 ++++++++ .../features/music/musicRateLimit.test.ts | 90 ++++++++ .../features/permissions/auditLog.test.ts | 192 ++++++++++++++++++ .../permissions/userPermissions.test.ts | 164 +++++++++++++++ .../features/scheduled/cronPreview.test.ts | 99 +++++++++ .../welcome/imagePreviewHeaders.test.ts | 83 ++++++++ .../features/welcome/imageUpload.test.ts | 121 +++++++++++ .../server/features/welcome/welcome.test.ts | 8 +- 17 files changed, 1161 insertions(+), 35 deletions(-) create mode 100644 apps/dashboard/tests/server/features/actions/updateRuleErrors.test.ts create mode 100644 apps/dashboard/tests/server/features/commands/regexValidation.test.ts create mode 100644 apps/dashboard/tests/server/features/music/musicRateLimit.test.ts create mode 100644 apps/dashboard/tests/server/features/permissions/auditLog.test.ts create mode 100644 apps/dashboard/tests/server/features/permissions/userPermissions.test.ts create mode 100644 apps/dashboard/tests/server/features/scheduled/cronPreview.test.ts create mode 100644 apps/dashboard/tests/server/features/welcome/imagePreviewHeaders.test.ts create mode 100644 apps/dashboard/tests/server/features/welcome/imageUpload.test.ts diff --git a/apps/dashboard/package.json b/apps/dashboard/package.json index 92a8ab5..92a82e3 100644 --- a/apps/dashboard/package.json +++ b/apps/dashboard/package.json @@ -62,6 +62,7 @@ "react-dom": "^19.1.0", "react-i18next": "^15.5.2", "recharts": "^3.8.0", + "safe-regex": "^2.1.1", "sonner": "^2.0.7", "tailwind-merge": "^3.5.0", "zod": "^3.25.17" @@ -71,6 +72,7 @@ "@tailwindcss/vite": "^4.1.7", "@types/react": "^19.1.6", "@types/react-dom": "^19.1.6", + "@types/safe-regex": "^1.1.6", "@vitejs/plugin-react": "^4.5.2", "@vitest/coverage-v8": "^4.0.18", "concurrently": "^9.1.2", diff --git a/apps/dashboard/src/server/features/actions/routes.ts b/apps/dashboard/src/server/features/actions/routes.ts index d99b102..28592b1 100644 --- a/apps/dashboard/src/server/features/actions/routes.ts +++ b/apps/dashboard/src/server/features/actions/routes.ts @@ -28,6 +28,7 @@ import { } from "@fluxcore/systems/actions/constants"; import type { ActionEventType, ActionType, RuleStep } from "@fluxcore/systems/actions/types"; import { channelExistsInGuild } from "../../shared/discordApi.js"; +import { logger } from "@fluxcore/utils"; const validEventTypes = new Set(Object.keys(EVENT_TYPES)); const validActionTypes = new Set(Object.keys(ACTION_TYPES)); @@ -228,8 +229,17 @@ export function registerActionRoutes(app: FastifyInstance): void { }); await notifyCacheInvalidation(guildId); reply.send(updated); - } catch { - reply.code(404).send({ error: "Rule not found" }); + } catch (err) { + const code = (err as { code?: string }).code; + if (code === "P2025") { + reply.code(404).send({ error: "Rule not found" }); + return; + } + logger.error( + `Failed to update action rule ${ruleId} in guild ${guildId}`, + err instanceof Error ? err : new Error(String(err)), + ); + reply.code(500).send({ error: "Failed to update rule" }); } }, ); diff --git a/apps/dashboard/src/server/features/commands/routes.ts b/apps/dashboard/src/server/features/commands/routes.ts index 5173c2f..c7e1876 100644 --- a/apps/dashboard/src/server/features/commands/routes.ts +++ b/apps/dashboard/src/server/features/commands/routes.ts @@ -1,4 +1,5 @@ import type { FastifyInstance } from "fastify"; +import safeRegex from "safe-regex"; import { requireAuth, requireGuildAdmin, requirePermission } from "../../shared/middleware.js"; import { getCustomCommands, @@ -10,6 +11,23 @@ import { import { MAX_COMMANDS_PER_GUILD } from "@fluxcore/systems/customCommands/constants"; import { TRIGGER_TYPES } from "@fluxcore/systems/customCommands/constants"; +const MAX_REGEX_LENGTH = 200; + +function validateRegexPattern(pattern: string): string | null { + if (pattern.length > MAX_REGEX_LENGTH) { + return `Regex pattern too long (max ${MAX_REGEX_LENGTH} chars)`; + } + try { + new RegExp(pattern, "i"); + } catch { + return "Invalid regex pattern"; + } + if (!safeRegex(pattern)) { + return "Unsafe regex pattern (catastrophic backtracking risk)"; + } + return null; +} + function parseIntParam(value: string): number | null { const n = parseInt(value, 10); return Number.isFinite(n) && n > 0 ? n : null; @@ -32,6 +50,14 @@ export function registerCustomCommandRoutes(app: FastifyInstance): void { "/api/guilds/:guildId/custom-commands", { preHandler: [requireAuth, requireGuildAdmin, requirePermission("commands.list.manage")], + config: { + rateLimit: { + max: 10, + timeWindow: "1 minute", + keyGenerator: (req) => + (req as { session?: { userId?: string } }).session?.userId ?? req.ip, + }, + }, schema: { body: { type: "object", @@ -109,10 +135,9 @@ export function registerCustomCommandRoutes(app: FastifyInstance): void { // Validate regex if trigger type is regex if (body.triggerType === "regex") { - try { - new RegExp(body.name, "i"); - } catch { - reply.code(400).send({ error: "Invalid regex pattern" }); + const err = validateRegexPattern(body.name); + if (err) { + reply.code(400).send({ error: err }); return; } } @@ -218,10 +243,9 @@ export function registerCustomCommandRoutes(app: FastifyInstance): void { // Validate regex if trigger type is being changed to regex if (body.triggerType === "regex" && body.name) { - try { - new RegExp(body.name, "i"); - } catch { - reply.code(400).send({ error: "Invalid regex pattern" }); + const err = validateRegexPattern(body.name); + if (err) { + reply.code(400).send({ error: err }); return; } } diff --git a/apps/dashboard/src/server/features/giveaways/routes.ts b/apps/dashboard/src/server/features/giveaways/routes.ts index 7ed966c..ddb5831 100644 --- a/apps/dashboard/src/server/features/giveaways/routes.ts +++ b/apps/dashboard/src/server/features/giveaways/routes.ts @@ -50,6 +50,14 @@ export function registerGiveawayRoutes(app: FastifyInstance): void { "/api/guilds/:guildId/giveaways", { preHandler: [requireAuth, requireGuildAdmin, requirePermission("giveaways.list.manage")], + config: { + rateLimit: { + max: 10, + timeWindow: "1 minute", + keyGenerator: (req) => + (req as { session?: { userId?: string } }).session?.userId ?? req.ip, + }, + }, schema: { body: { type: "object", diff --git a/apps/dashboard/src/server/features/music/routes.ts b/apps/dashboard/src/server/features/music/routes.ts index c353156..799c249 100644 --- a/apps/dashboard/src/server/features/music/routes.ts +++ b/apps/dashboard/src/server/features/music/routes.ts @@ -105,6 +105,14 @@ export function registerMusicRoutes(app: FastifyInstance): void { "/api/guilds/:guildId/music/library", { preHandler: [requireAuth, requireGuildAdmin, requirePermission("music.library.manage")], + config: { + rateLimit: { + max: 10, + timeWindow: "1 minute", + keyGenerator: (req) => + (req as { session?: { userId?: string } }).session?.userId ?? req.ip, + }, + }, schema: { body: { type: "object", diff --git a/apps/dashboard/src/server/features/permissions/routes.ts b/apps/dashboard/src/server/features/permissions/routes.ts index 0ac2d25..4df98b8 100644 --- a/apps/dashboard/src/server/features/permissions/routes.ts +++ b/apps/dashboard/src/server/features/permissions/routes.ts @@ -3,7 +3,6 @@ import { getPrisma } from "@fluxcore/database"; import { PERMISSION_REGISTRY, ALL_PERMISSION_KEYS, - matchPermission, resolveEffectivePermissions, } from "@fluxcore/types"; import { requireAuth, requireGuildAdmin, requirePermission } from "../../shared/middleware.js"; @@ -113,6 +112,12 @@ export function registerDashboardPermissionRoutes(app: FastifyInstance): void { return; } + // Block self-grant: a user must never grant or modify their own permission set + if (session.userId === userId) { + reply.code(403).send({ error: "Cannot modify your own permissions" }); + return; + } + // Validate keys for (const perm of permissions) { if (!isValidPermKey(perm)) { @@ -121,15 +126,18 @@ export function registerDashboardPermissionRoutes(app: FastifyInstance): void { } } - // Escalation check + // Strict escalation gate: non-owners must literally hold every key they grant. + // Wildcards (`*`, `module.*`, `module.feature.*`) may only be granted by the + // guild owner — match-by-wildcard is not enough. if (!request.resolvedPermissions?.isOwner) { - const userPerms = request.resolvedPermissions!.permissions; + const literalCallerPerms = request.resolvedPermissions!.permissions; for (const perm of permissions) { - if (!matchPermission(userPerms, perm)) { - reply.code(403).send({ - error: "Cannot grant permissions you don't have", - permission: perm, - }); + if (perm === "*" || perm.includes("*")) { + reply.code(403).send({ error: "Insufficient privileges to grant this permission" }); + return; + } + if (!literalCallerPerms.has(perm)) { + reply.code(403).send({ error: "Insufficient privileges to grant this permission" }); return; } } @@ -298,13 +306,39 @@ export function registerDashboardPermissionRoutes(app: FastifyInstance): void { const skip = (page - 1) * limit; const where: Record = { guildId }; - if (query.userId) where.userId = query.userId; - if (query.action) where.action = { contains: query.action }; + const isOwner = request.resolvedPermissions?.isOwner === true; + if (isOwner) { + if (query.userId) where.userId = query.userId; + } else { + // Non-owners may only view their own audit entries + where.userId = request.session!.userId; + } + if (query.action) { + if (!ALLOWED_AUDIT_ACTIONS.has(query.action)) { + reply.code(400).send({ error: "Unknown action filter" }); + return; + } + where.action = query.action; + } if (query.targetType) where.targetType = query.targetType; if (query.from || query.to) { const createdAt: Record = {}; - if (query.from) createdAt.gte = new Date(query.from); - if (query.to) createdAt.lte = new Date(query.to); + if (query.from) { + const d = new Date(query.from); + if (!Number.isFinite(d.getTime())) { + reply.code(400).send({ error: "Invalid 'from' date" }); + return; + } + createdAt.gte = d; + } + if (query.to) { + const d = new Date(query.to); + if (!Number.isFinite(d.getTime())) { + reply.code(400).send({ error: "Invalid 'to' date" }); + return; + } + createdAt.lte = d; + } where.createdAt = createdAt; } @@ -339,6 +373,17 @@ export function registerDashboardPermissionRoutes(app: FastifyInstance): void { // ─── Helpers ─── +const ALLOWED_AUDIT_ACTIONS = new Set([ + "dashboard.permissions.update", + "dashboard.permissions.clear", + "dashboard.settings.update", + "dashboard.role.create", + "dashboard.role.update", + "dashboard.role.delete", + "dashboard.role.assign", + "dashboard.role.unassign", +]); + function isValidPermKey(key: string): boolean { if (ALL_PERMISSION_KEYS.includes(key)) return true; if (key === "*") return true; diff --git a/apps/dashboard/src/server/features/scheduled/routes.ts b/apps/dashboard/src/server/features/scheduled/routes.ts index 2c07de6..17c33f9 100644 --- a/apps/dashboard/src/server/features/scheduled/routes.ts +++ b/apps/dashboard/src/server/features/scheduled/routes.ts @@ -220,7 +220,17 @@ export function registerScheduledMessageRoutes(app: FastifyInstance): void { // GET preview next run time for a cron expression app.get( "/api/guilds/:guildId/scheduled-messages/preview-cron", - { preHandler: [requireAuth, requireGuildAdmin, requirePermission("scheduled.messages.view")] }, + { + preHandler: [requireAuth, requireGuildAdmin, requirePermission("scheduled.messages.view")], + config: { + rateLimit: { + max: 5, + timeWindow: "10 seconds", + keyGenerator: (req) => + (req as { session?: { userId?: string } }).session?.userId ?? req.ip, + }, + }, + }, async (request, reply) => { const query = request.query as { cronExpr?: string; timezone?: string }; if (!query.cronExpr) { @@ -235,18 +245,34 @@ export function registerScheduledMessageRoutes(app: FastifyInstance): void { } const timezone = query.timezone ?? "UTC"; - const nextRun = getNextCronRun(query.cronExpr, timezone); - - // Calculate the next 5 run times - const nextRuns: string[] = [nextRun.toISOString()]; - let lastRun = nextRun; - for (let i = 0; i < 4; i++) { - const next = getNextCronRun(query.cronExpr, timezone, lastRun); - nextRuns.push(next.toISOString()); - lastRun = next; - } + const budgetMs = 250; + const start = Date.now(); - reply.send({ nextRuns }); + try { + const nextRun = getNextCronRun(query.cronExpr, timezone); + if (Date.now() - start > budgetMs) { + reply + .code(400) + .send({ error: "Cron expression too slow to evaluate (budget exceeded)" }); + return; + } + const nextRuns: string[] = [nextRun.toISOString()]; + let lastRun = nextRun; + for (let i = 0; i < 4; i++) { + if (Date.now() - start > budgetMs) { + reply + .code(400) + .send({ error: "Cron expression too slow to evaluate (budget exceeded)" }); + return; + } + const next = getNextCronRun(query.cronExpr, timezone, lastRun); + nextRuns.push(next.toISOString()); + lastRun = next; + } + reply.send({ nextRuns }); + } catch { + reply.code(400).send({ error: "Failed to evaluate cron expression" }); + } }, ); } diff --git a/apps/dashboard/src/server/features/welcome/routes.ts b/apps/dashboard/src/server/features/welcome/routes.ts index 57fe966..666dd4a 100644 --- a/apps/dashboard/src/server/features/welcome/routes.ts +++ b/apps/dashboard/src/server/features/welcome/routes.ts @@ -183,6 +183,7 @@ export function registerWelcomeRoutes(app: FastifyInstance): void { reply .header("Content-Type", "image/png") .header("Cache-Control", "no-cache") + .header("X-Content-Type-Options", "nosniff") .send(imageBuffer); }, ); @@ -214,7 +215,18 @@ export function registerWelcomeRoutes(app: FastifyInstance): void { return; } + // Strict base64 validation (RFC 4648, optional padding) + const base64Regex = /^[A-Za-z0-9+/]+={0,2}$/; + if (data.length === 0 || data.length % 4 !== 0 || !base64Regex.test(data)) { + reply.code(400).send({ error: "Invalid base64 payload" }); + return; + } + const buffer = Buffer.from(data, "base64"); + if (buffer.length === 0) { + reply.code(400).send({ error: "Empty payload" }); + return; + } if (buffer.length > MAX_BACKGROUND_SIZE) { reply.code(400).send({ @@ -223,6 +235,15 @@ export function registerWelcomeRoutes(app: FastifyInstance): void { return; } + // Magic byte sniffing — must match contentType + const detected = detectImageType(buffer); + if (!detected || detected !== contentType) { + reply.code(400).send({ + error: "File content does not match the declared image type", + }); + return; + } + const ext = contentType.split("/")[1] === "jpeg" ? "jpg" : contentType.split("/")[1]; const key = `backgrounds/${guildId}/${randomUUID()}.${ext}`; @@ -296,3 +317,26 @@ export function registerWelcomeRoutes(app: FastifyInstance): void { }, ); } + +function detectImageType(buffer: Buffer): string | null { + if (buffer.length < 12) return null; + // PNG: 89 50 4E 47 0D 0A 1A 0A + if ( + buffer[0] === 0x89 && buffer[1] === 0x50 && buffer[2] === 0x4e && buffer[3] === 0x47 && + buffer[4] === 0x0d && buffer[5] === 0x0a && buffer[6] === 0x1a && buffer[7] === 0x0a + ) { + return "image/png"; + } + // JPEG: FF D8 FF + if (buffer[0] === 0xff && buffer[1] === 0xd8 && buffer[2] === 0xff) { + return "image/jpeg"; + } + // WebP: RIFF .... WEBP + if ( + buffer[0] === 0x52 && buffer[1] === 0x49 && buffer[2] === 0x46 && buffer[3] === 0x46 && + buffer[8] === 0x57 && buffer[9] === 0x45 && buffer[10] === 0x42 && buffer[11] === 0x50 + ) { + return "image/webp"; + } + return null; +} diff --git a/apps/dashboard/tests/server/features/actions/updateRuleErrors.test.ts b/apps/dashboard/tests/server/features/actions/updateRuleErrors.test.ts new file mode 100644 index 0000000..541c79c --- /dev/null +++ b/apps/dashboard/tests/server/features/actions/updateRuleErrors.test.ts @@ -0,0 +1,114 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { token: "t", clientId: "c", dashboardSessionSecret: "s", logLevel: "info" }, +})); + +vi.mock("../../../../src/server/shared/session.js", () => ({ + getSession: vi.fn().mockResolvedValue({ + userId: "user-1", + username: "u", + guilds: [{ id: "guild-1", name: "T", permissions: BigInt(0x20).toString() }], + }), + touchSession: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("../../../../src/server/shared/discordApi.js", () => ({ + isBotInGuild: vi.fn().mockResolvedValue(true), + channelExistsInGuild: vi.fn().mockResolvedValue(true), + getGuildOwnerId: vi.fn().mockResolvedValue("owner-1"), +})); +vi.mock("../../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions: vi + .fn() + .mockResolvedValue({ permissions: new Set(["*"]), isOwner: true }), + hasPermission: vi.fn().mockReturnValue(true), + invalidatePermissionCache: vi.fn(), + createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), +})); + +const { mockUpdateRule } = vi.hoisted(() => ({ mockUpdateRule: vi.fn() })); +vi.mock("@fluxcore/systems/actions/persistence", () => ({ + notifyCacheInvalidation: vi.fn().mockResolvedValue(undefined), + listRules: vi.fn().mockResolvedValue([]), + getRule: vi.fn(), + createRule: vi.fn(), + updateRule: (...args: unknown[]) => mockUpdateRule(...args), + deleteRule: vi.fn(), + bulkUpdateRules: vi.fn(), + bulkDeleteRules: vi.fn(), + getRuleAnalytics: vi.fn(), + getActionLogs: vi.fn(), + getGuildSettings: vi.fn(), + upsertGuildSettings: vi.fn(), + getRulesByGuild: vi.fn().mockResolvedValue([]), + countRules: vi.fn().mockResolvedValue(0), + getRecentLogs: vi.fn().mockResolvedValue([]), + getAnalytics: vi.fn().mockResolvedValue({}), + getLastFiredByGuild: vi.fn().mockResolvedValue(new Map()), +})); + +vi.mock("@fluxcore/systems/actions/config", () => ({ + getGuildSettingsOrDefault: vi + .fn() + .mockReturnValue({ maxRules: 25, globalEnabled: true, logChannelId: null }), + setGuildSettings: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@fluxcore/systems/actions/constants", () => ({ + EVENT_TYPES: { memberJoin: { label: "Member Join" } }, + ACTION_TYPES: { sendMessage: { label: "Send Message" } }, + MAX_ACTIONS_PER_RULE: 5, + ACTION_TYPE_FIELDS: {}, + EVENT_TYPE_VARIABLES: {}, + TEMPLATE_VARIABLES: [], +})); + +const { mockLogger } = vi.hoisted(() => ({ + mockLogger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})); +vi.mock("@fluxcore/utils", () => ({ logger: mockLogger })); + +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import { registerActionRoutes } from "../../../../src/server/features/actions/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "test-secret" }); + registerActionRoutes(app); + await app.ready(); + return app; +} + +describe("PUT /actions/rules/:ruleId — error handling", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + app = await buildApp(); + }); + + it("returns 404 when Prisma throws P2025", async () => { + const err = Object.assign(new Error("Record not found"), { code: "P2025" }); + mockUpdateRule.mockRejectedValue(err); + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/actions/rules/123", + cookies: { session: app.signCookie("valid") }, + payload: { name: "x" }, + }); + expect(res.statusCode).toBe(404); + }); + + it("returns 500 and logs when an unexpected error occurs", async () => { + mockUpdateRule.mockRejectedValue(new Error("DB connection refused")); + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/actions/rules/123", + cookies: { session: app.signCookie("valid") }, + payload: { name: "x" }, + }); + expect(res.statusCode).toBe(500); + expect(mockLogger.error).toHaveBeenCalled(); + }); +}); diff --git a/apps/dashboard/tests/server/features/commands/regexValidation.test.ts b/apps/dashboard/tests/server/features/commands/regexValidation.test.ts new file mode 100644 index 0000000..758518b --- /dev/null +++ b/apps/dashboard/tests/server/features/commands/regexValidation.test.ts @@ -0,0 +1,90 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { token: "t", clientId: "c", dashboardSessionSecret: "s", logLevel: "info" }, +})); + +vi.mock("../../../../src/server/shared/session.js", () => ({ + getSession: vi.fn().mockResolvedValue({ + userId: "user-1", + username: "u", + guilds: [{ id: "guild-1", name: "T", permissions: BigInt(0x20).toString() }], + }), + touchSession: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("../../../../src/server/shared/discordApi.js", () => ({ + isBotInGuild: vi.fn().mockResolvedValue(true), + getGuildOwnerId: vi.fn().mockResolvedValue("owner-1"), +})); +vi.mock("../../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions: vi.fn().mockResolvedValue({ permissions: new Set(["*"]), isOwner: true }), + hasPermission: vi.fn().mockReturnValue(true), + invalidatePermissionCache: vi.fn(), + createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@fluxcore/systems/customCommands/persistence", () => ({ + getCustomCommands: vi.fn().mockResolvedValue([]), + getCustomCommandCount: vi.fn().mockResolvedValue(0), + createCustomCommand: vi.fn().mockResolvedValue({ id: 1 }), + updateCustomCommand: vi.fn().mockResolvedValue({ id: 1 }), + deleteCustomCommand: vi.fn(), +})); +vi.mock("@fluxcore/systems/customCommands/constants", () => ({ + MAX_COMMANDS_PER_GUILD: 100, + TRIGGER_TYPES: ["exact", "startsWith", "contains", "regex"], +})); +vi.mock("@fluxcore/utils", () => ({ + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})); + +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import { registerCustomCommandRoutes } from "../../../../src/server/features/commands/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "test-secret" }); + registerCustomCommandRoutes(app); + await app.ready(); + return app; +} + +describe("POST /custom-commands — regex safety", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + app = await buildApp(); + }); + + it("rejects catastrophic backtracking pattern (a+)+$", async () => { + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/custom-commands", + cookies: { session: app.signCookie("valid") }, + payload: { name: "(a+)+$", triggerType: "regex" }, + }); + expect(res.statusCode).toBe(400); + expect(res.json().error).toMatch(/unsafe|regex/i); + }); + + it("rejects nested quantifier pattern (a*)*", async () => { + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/custom-commands", + cookies: { session: app.signCookie("valid") }, + payload: { name: "(a*)*", triggerType: "regex" }, + }); + expect(res.statusCode).toBe(400); + }); + + it("accepts a simple safe regex", async () => { + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/custom-commands", + cookies: { session: app.signCookie("valid") }, + payload: { name: "^hello", triggerType: "regex" }, + }); + expect(res.statusCode).toBe(201); + }); +}); diff --git a/apps/dashboard/tests/server/features/music/musicRateLimit.test.ts b/apps/dashboard/tests/server/features/music/musicRateLimit.test.ts new file mode 100644 index 0000000..d1e1bf9 --- /dev/null +++ b/apps/dashboard/tests/server/features/music/musicRateLimit.test.ts @@ -0,0 +1,90 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { + token: "t", + clientId: "c", + dashboardSessionSecret: "s", + logLevel: "info", + }, +})); + +vi.mock("../../../../src/server/shared/session.js", () => ({ + getSession: vi.fn().mockResolvedValue({ + userId: "user-1", + username: "user", + guilds: [{ id: "guild-1", name: "T", permissions: BigInt(0x20).toString() }], + }), + touchSession: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("../../../../src/server/shared/discordApi.js", () => ({ + isBotInGuild: vi.fn().mockResolvedValue(true), + getGuildOwnerId: vi.fn().mockResolvedValue("owner-1"), +})); +vi.mock("../../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions: vi.fn().mockResolvedValue({ permissions: new Set(["*"]), isOwner: true }), + hasPermission: vi.fn().mockReturnValue(true), + invalidatePermissionCache: vi.fn(), + createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@fluxcore/systems/music/library", () => ({ + getAlbums: vi.fn().mockResolvedValue([]), + getAlbumById: vi.fn(), + addAlbum: vi.fn().mockResolvedValue({ id: 1, name: "x" }), + removeAlbum: vi.fn(), + addTrack: vi.fn(), + removeTrack: vi.fn(), + getAlbumTracks: vi.fn(), + getAlbumCount: vi.fn().mockResolvedValue(0), + getTrackCount: vi.fn().mockResolvedValue(0), + getTrackById: vi.fn(), +})); +vi.mock("@fluxcore/systems/music/config", () => ({ + fetchMusicSettings: vi.fn(), + upsertMusicSettings: vi.fn(), +})); +vi.mock("@fluxcore/systems/actions/persistence", () => ({ + notifyCacheInvalidation: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@fluxcore/utils", () => ({ + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})); + +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import fastifyRateLimit from "@fastify/rate-limit"; +import { registerMusicRoutes } from "../../../../src/server/features/music/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "test-secret" }); + await app.register(fastifyRateLimit, { global: false }); + registerMusicRoutes(app); + await app.ready(); + return app; +} + +describe("POST /api/guilds/:guildId/music/library — rate limit", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + app = await buildApp(); + }); + + it("returns 429 after exceeding 10 requests/minute", async () => { + const cookie = { session: app.signCookie("valid") }; + let lastStatus = 0; + for (let i = 0; i < 12; i++) { + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/music/library", + cookies: cookie, + payload: { name: `album-${i}` }, + }); + lastStatus = res.statusCode; + } + expect(lastStatus).toBe(429); + }); +}); diff --git a/apps/dashboard/tests/server/features/permissions/auditLog.test.ts b/apps/dashboard/tests/server/features/permissions/auditLog.test.ts new file mode 100644 index 0000000..15034d5 --- /dev/null +++ b/apps/dashboard/tests/server/features/permissions/auditLog.test.ts @@ -0,0 +1,192 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { + token: "test-token", + clientId: "test-client-id", + dashboardSessionSecret: "session-secret", + logLevel: "info", + }, +})); + +const MANAGE_GUILD = BigInt(0x20); +const callerSession = { + userId: "caller-1", + username: "caller", + guilds: [{ id: "guild-1", name: "Test", permissions: MANAGE_GUILD.toString() }], +}; + +const mockGetSession = vi.fn().mockResolvedValue(callerSession); +vi.mock("../../../../src/server/shared/session.js", () => ({ + getSession: (...args: unknown[]) => mockGetSession(...args), + touchSession: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("../../../../src/server/shared/discordApi.js", () => ({ + isBotInGuild: vi.fn().mockResolvedValue(true), + getGuildOwnerId: vi.fn().mockResolvedValue("owner-1"), +})); + +const mockResolveUserPermissions = vi.fn(); +vi.mock("../../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions: (...args: unknown[]) => mockResolveUserPermissions(...args), + hasPermission: vi.fn().mockReturnValue(true), + invalidatePermissionCache: vi.fn(), + createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), +})); + +const mockFindMany = vi.fn().mockResolvedValue([]); +const mockCount = vi.fn().mockResolvedValue(0); +vi.mock("@fluxcore/database", () => ({ + getPrisma: () => ({ + dashboardAuditLog: { findMany: mockFindMany, count: mockCount }, + dashboardUserPermission: { findMany: vi.fn() }, + }), +})); +vi.mock("@fluxcore/utils", () => ({ + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})); + +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import { registerDashboardPermissionRoutes } from "../../../../src/server/features/permissions/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "test-secret" }); + registerDashboardPermissionRoutes(app); + await app.ready(); + return app; +} + +describe("GET /api/guilds/:guildId/dashboard-audit — userId filter", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + mockGetSession.mockResolvedValue(callerSession); + mockResolveUserPermissions.mockResolvedValue({ + permissions: new Set(["dashboard.audit.view"]), + isOwner: false, + }); + mockFindMany.mockResolvedValue([]); + mockCount.mockResolvedValue(0); + app = await buildApp(); + }); + + it("forces userId filter to caller for non-owner", async () => { + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/dashboard-audit?userId=other-user", + cookies: { session: app.signCookie("valid") }, + }); + expect(res.statusCode).toBe(200); + expect(mockFindMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ guildId: "guild-1", userId: "caller-1" }), + }), + ); + }); + + it("allows owner to filter by any userId", async () => { + mockResolveUserPermissions.mockResolvedValue({ + permissions: new Set(["*"]), + isOwner: true, + }); + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/dashboard-audit?userId=other-user", + cookies: { session: app.signCookie("valid") }, + }); + expect(res.statusCode).toBe(200); + expect(mockFindMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ userId: "other-user" }), + }), + ); + }); +}); + +describe("GET /api/guilds/:guildId/dashboard-audit — date validation", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + mockGetSession.mockResolvedValue(callerSession); + mockResolveUserPermissions.mockResolvedValue({ + permissions: new Set(["*"]), + isOwner: true, + }); + mockFindMany.mockResolvedValue([]); + mockCount.mockResolvedValue(0); + app = await buildApp(); + }); + + it("returns 400 when from is not a valid date", async () => { + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/dashboard-audit?from=not-a-date", + cookies: { session: app.signCookie("valid") }, + }); + expect(res.statusCode).toBe(400); + expect(res.json().error).toMatch(/from/i); + }); + + it("returns 400 when to is not a valid date", async () => { + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/dashboard-audit?to=garbage", + cookies: { session: app.signCookie("valid") }, + }); + expect(res.statusCode).toBe(400); + expect(res.json().error).toMatch(/to/i); + }); + + it("accepts valid ISO dates", async () => { + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/dashboard-audit?from=2026-01-01T00:00:00Z&to=2026-04-01T00:00:00Z", + cookies: { session: app.signCookie("valid") }, + }); + expect(res.statusCode).toBe(200); + }); +}); + +describe("GET /dashboard-audit — action filter is exact-match", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + mockGetSession.mockResolvedValue(callerSession); + mockResolveUserPermissions.mockResolvedValue({ + permissions: new Set(["*"]), + isOwner: true, + }); + mockFindMany.mockResolvedValue([]); + mockCount.mockResolvedValue(0); + app = await buildApp(); + }); + + it("uses exact match (not contains) when action is allowlisted", async () => { + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/dashboard-audit?action=dashboard.permissions.update", + cookies: { session: app.signCookie("valid") }, + }); + expect(res.statusCode).toBe(200); + expect(mockFindMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ action: "dashboard.permissions.update" }), + }), + ); + }); + + it("returns 400 when action is not in the allowlist", async () => { + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/dashboard-audit?action=permissions", + cookies: { session: app.signCookie("valid") }, + }); + expect(res.statusCode).toBe(400); + }); +}); diff --git a/apps/dashboard/tests/server/features/permissions/userPermissions.test.ts b/apps/dashboard/tests/server/features/permissions/userPermissions.test.ts new file mode 100644 index 0000000..7c05f84 --- /dev/null +++ b/apps/dashboard/tests/server/features/permissions/userPermissions.test.ts @@ -0,0 +1,164 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { + token: "test-token", + clientId: "test-client-id", + dashboardSessionSecret: "session-secret", + logLevel: "info", + }, +})); + +const MANAGE_GUILD = BigInt(0x20); +const callerSession = { + userId: "caller-1", + username: "caller", + guilds: [{ id: "guild-1", name: "Test", permissions: MANAGE_GUILD.toString() }], +}; + +const mockGetSession = vi.fn().mockResolvedValue(callerSession); +vi.mock("../../../../src/server/shared/session.js", () => ({ + getSession: (...args: unknown[]) => mockGetSession(...args), + touchSession: vi.fn().mockResolvedValue(undefined), +})); + +const mockGetGuildOwnerId = vi.fn().mockResolvedValue("owner-1"); +vi.mock("../../../../src/server/shared/discordApi.js", () => ({ + isBotInGuild: vi.fn().mockResolvedValue(true), + getGuildOwnerId: (...args: unknown[]) => mockGetGuildOwnerId(...args), +})); + +const mockResolveUserPermissions = vi.fn(); +vi.mock("../../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions: (...args: unknown[]) => mockResolveUserPermissions(...args), + hasPermission: vi.fn().mockReturnValue(true), + invalidatePermissionCache: vi.fn(), + createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), +})); + +const mockPrisma = { + dashboardUserPermission: { + findMany: vi.fn().mockResolvedValue([]), + deleteMany: vi.fn().mockResolvedValue({ count: 0 }), + createMany: vi.fn().mockResolvedValue({ count: 0 }), + }, + dashboardAuditLog: { create: vi.fn(), findMany: vi.fn(), count: vi.fn() }, + $transaction: vi.fn(async (fn: (tx: unknown) => Promise) => fn(mockPrisma)), +}; + +vi.mock("@fluxcore/database", () => ({ getPrisma: () => mockPrisma })); +vi.mock("@fluxcore/utils", () => ({ + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})); + +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import { registerDashboardPermissionRoutes } from "../../../../src/server/features/permissions/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "test-secret" }); + registerDashboardPermissionRoutes(app); + await app.ready(); + return app; +} + +describe("PUT /api/guilds/:guildId/user-permissions/:userId — escalation guards", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + mockGetSession.mockResolvedValue(callerSession); + mockGetGuildOwnerId.mockResolvedValue("owner-1"); + mockResolveUserPermissions.mockResolvedValue({ + permissions: new Set(["dashboard.roles.manage"]), + isOwner: false, + }); + app = await buildApp(); + }); + + it("rejects self-grant with 403", async () => { + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/user-permissions/caller-1", + cookies: { session: app.signCookie("valid") }, + payload: { permissions: ["dashboard.roles.view"] }, + }); + expect(res.statusCode).toBe(403); + expect(res.json().error).toMatch(/own/i); + }); + + it("rejects non-owner attempting to grant a wildcard they do not literally hold", async () => { + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/user-permissions/target-1", + cookies: { session: app.signCookie("valid") }, + payload: { permissions: ["dashboard.*"] }, + }); + expect(res.statusCode).toBe(403); + }); + + it("rejects non-owner attempting to grant the global wildcard", async () => { + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/user-permissions/target-1", + cookies: { session: app.signCookie("valid") }, + payload: { permissions: ["*"] }, + }); + expect(res.statusCode).toBe(403); + }); + + it("rejects when caller does not literally hold the requested key", async () => { + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/user-permissions/target-1", + cookies: { session: app.signCookie("valid") }, + payload: { permissions: ["actions.rules.manage"] }, + }); + expect(res.statusCode).toBe(403); + }); + + it("allows owner to grant any permission, including self", async () => { + mockResolveUserPermissions.mockResolvedValue({ + permissions: new Set(["*"]), + isOwner: true, + }); + mockGetSession.mockResolvedValue({ ...callerSession, userId: "owner-1" }); + mockGetGuildOwnerId.mockResolvedValue("owner-2"); + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/user-permissions/target-1", + cookies: { session: app.signCookie("valid") }, + payload: { permissions: ["actions.rules.manage"] }, + }); + expect(res.statusCode).toBe(200); + }); +}); + +describe("PUT /user-permissions — error response does not leak key", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + mockGetSession.mockResolvedValue(callerSession); + mockGetGuildOwnerId.mockResolvedValue("owner-1"); + mockResolveUserPermissions.mockResolvedValue({ + permissions: new Set(["dashboard.roles.manage"]), + isOwner: false, + }); + app = await buildApp(); + }); + + it("does not echo the failed permission key in the error body", async () => { + const res = await app.inject({ + method: "PUT", + url: "/api/guilds/guild-1/user-permissions/target-1", + cookies: { session: app.signCookie("valid") }, + payload: { permissions: ["actions.rules.manage"] }, + }); + expect(res.statusCode).toBe(403); + const body = res.json(); + expect(body).not.toHaveProperty("permission"); + expect(JSON.stringify(body)).not.toContain("actions.rules.manage"); + }); +}); diff --git a/apps/dashboard/tests/server/features/scheduled/cronPreview.test.ts b/apps/dashboard/tests/server/features/scheduled/cronPreview.test.ts new file mode 100644 index 0000000..20f810b --- /dev/null +++ b/apps/dashboard/tests/server/features/scheduled/cronPreview.test.ts @@ -0,0 +1,99 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { token: "t", clientId: "c", dashboardSessionSecret: "s", logLevel: "info" }, +})); + +vi.mock("../../../../src/server/shared/session.js", () => ({ + getSession: vi.fn().mockResolvedValue({ + userId: "user-1", + username: "u", + guilds: [{ id: "guild-1", name: "T", permissions: BigInt(0x20).toString() }], + }), + touchSession: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("../../../../src/server/shared/discordApi.js", () => ({ + isBotInGuild: vi.fn().mockResolvedValue(true), + getGuildOwnerId: vi.fn().mockResolvedValue("owner-1"), +})); +vi.mock("../../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions: vi + .fn() + .mockResolvedValue({ permissions: new Set(["*"]), isOwner: true }), + hasPermission: vi.fn().mockReturnValue(true), + invalidatePermissionCache: vi.fn(), + createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), +})); + +let slowMode = false; +vi.mock("@fluxcore/systems/scheduled-messages/cron", () => ({ + validateCronExpression: () => null, + getNextCronRun: () => { + if (slowMode) { + const start = Date.now(); + while (Date.now() - start < 100) { + /* spin */ + } + } + return new Date(); + }, +})); +vi.mock("@fluxcore/systems/scheduled-messages/persistence", () => ({ + getScheduledMessages: vi.fn(), + getScheduledMessageById: vi.fn(), + createScheduledMessage: vi.fn(), + updateScheduledMessage: vi.fn(), + deleteScheduledMessage: vi.fn(), +})); +vi.mock("@fluxcore/utils", () => ({ + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})); + +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import fastifyRateLimit from "@fastify/rate-limit"; +import { registerScheduledMessageRoutes } from "../../../../src/server/features/scheduled/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "test-secret" }); + await app.register(fastifyRateLimit, { global: false }); + registerScheduledMessageRoutes(app); + await app.ready(); + return app; +} + +describe("GET /scheduled-messages/preview-cron — DoS guards", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + slowMode = false; + app = await buildApp(); + }); + + it("returns 429 after exceeding 5 requests per 10 seconds", async () => { + const cookie = { session: app.signCookie("valid") }; + let lastStatus = 0; + for (let i = 0; i < 7; i++) { + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/scheduled-messages/preview-cron?cronExpr=*+*+*+*+*", + cookies: cookie, + }); + lastStatus = res.statusCode; + } + expect(lastStatus).toBe(429); + }); + + it("returns 400 when cron evaluation exceeds time budget", async () => { + slowMode = true; + const res = await app.inject({ + method: "GET", + url: "/api/guilds/guild-1/scheduled-messages/preview-cron?cronExpr=*+*+*+*+*", + cookies: { session: app.signCookie("valid") }, + }); + expect(res.statusCode).toBe(400); + expect(res.json().error).toMatch(/slow|budget|timeout/i); + }); +}); diff --git a/apps/dashboard/tests/server/features/welcome/imagePreviewHeaders.test.ts b/apps/dashboard/tests/server/features/welcome/imagePreviewHeaders.test.ts new file mode 100644 index 0000000..f713b54 --- /dev/null +++ b/apps/dashboard/tests/server/features/welcome/imagePreviewHeaders.test.ts @@ -0,0 +1,83 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { token: "t", clientId: "c", dashboardSessionSecret: "s", logLevel: "info" }, +})); + +vi.mock("../../../../src/server/shared/session.js", () => ({ + getSession: vi.fn().mockResolvedValue({ + userId: "123456789012345678", + username: "u", + avatar: "abc123", + guilds: [{ id: "guild-1", name: "T", permissions: BigInt(0x20).toString() }], + }), + touchSession: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("../../../../src/server/shared/discordApi.js", () => ({ + isBotInGuild: vi.fn().mockResolvedValue(true), + getGuildOwnerId: vi.fn().mockResolvedValue("owner-1"), +})); +vi.mock("../../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions: vi.fn().mockResolvedValue({ permissions: new Set(["*"]), isOwner: true }), + hasPermission: vi.fn().mockReturnValue(true), + invalidatePermissionCache: vi.fn(), + createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@fluxcore/systems/welcome/image", async () => { + const actual = await vi.importActual>("@fluxcore/systems/welcome/image"); + return { + ...actual, + createStorageAdapter: () => ({ + upload: vi.fn().mockResolvedValue(undefined), + delete: vi.fn().mockResolvedValue(undefined), + }), + welcomeImageSettingsSchema: { safeParse: () => ({ success: true, data: {} }) }, + DEFAULT_WELCOME_IMAGE_SETTINGS: {}, + DEFAULT_FAREWELL_IMAGE_SETTINGS: {}, + MAX_BACKGROUND_SIZE: 5 * 1024 * 1024, + ALLOWED_BACKGROUND_TYPES: ["image/png"], + PRESET_BACKGROUNDS: [], + generateWelcomeImage: vi.fn().mockResolvedValue(Buffer.from([0x89, 0x50, 0x4e, 0x47])), + getAllTemplates: () => [], + getAvailableFonts: () => [], + }; +}); +vi.mock("@fluxcore/systems/welcome/config", () => ({ + getWelcomeConfig: vi.fn(), + upsertWelcomeConfig: vi.fn(), +})); +vi.mock("@fluxcore/utils", () => ({ + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})); + +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import { registerWelcomeRoutes } from "../../../../src/server/features/welcome/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "test-secret" }); + registerWelcomeRoutes(app); + await app.ready(); + return app; +} + +describe("POST /welcome/image/preview — security headers", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + app = await buildApp(); + }); + + it("sets X-Content-Type-Options: nosniff", async () => { + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/welcome/image/preview", + cookies: { session: app.signCookie("valid") }, + payload: { settings: {}, type: "welcome" }, + }); + expect(res.statusCode).toBe(200); + expect(res.headers["x-content-type-options"]).toBe("nosniff"); + }); +}); diff --git a/apps/dashboard/tests/server/features/welcome/imageUpload.test.ts b/apps/dashboard/tests/server/features/welcome/imageUpload.test.ts new file mode 100644 index 0000000..c55b732 --- /dev/null +++ b/apps/dashboard/tests/server/features/welcome/imageUpload.test.ts @@ -0,0 +1,121 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@fluxcore/config", () => ({ + config: { token: "t", clientId: "c", dashboardSessionSecret: "s", logLevel: "info" }, +})); + +vi.mock("../../../../src/server/shared/session.js", () => ({ + getSession: vi.fn().mockResolvedValue({ + userId: "user-1", + username: "u", + guilds: [{ id: "guild-1", name: "T", permissions: BigInt(0x20).toString() }], + }), + touchSession: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("../../../../src/server/shared/discordApi.js", () => ({ + isBotInGuild: vi.fn().mockResolvedValue(true), + getGuildOwnerId: vi.fn().mockResolvedValue("owner-1"), +})); +vi.mock("../../../../src/server/shared/permissions.js", () => ({ + resolveUserPermissions: vi.fn().mockResolvedValue({ permissions: new Set(["*"]), isOwner: true }), + hasPermission: vi.fn().mockReturnValue(true), + invalidatePermissionCache: vi.fn(), + createDashboardAuditLog: vi.fn().mockResolvedValue(undefined), +})); + +const { mockUpload } = vi.hoisted(() => ({ mockUpload: vi.fn().mockResolvedValue(undefined) })); +vi.mock("@fluxcore/systems/welcome/image", async () => { + const actual = await vi.importActual>("@fluxcore/systems/welcome/image"); + return { + ...actual, + createStorageAdapter: () => ({ + upload: mockUpload, + delete: vi.fn().mockResolvedValue(undefined), + }), + MAX_BACKGROUND_SIZE: 5 * 1024 * 1024, + ALLOWED_BACKGROUND_TYPES: ["image/png", "image/jpeg", "image/webp"], + PRESET_BACKGROUNDS: [], + DEFAULT_WELCOME_IMAGE_SETTINGS: {}, + DEFAULT_FAREWELL_IMAGE_SETTINGS: {}, + welcomeImageSettingsSchema: { safeParse: () => ({ success: true, data: {} }) }, + generateWelcomeImage: vi.fn(), + getAllTemplates: () => [], + getAvailableFonts: () => [], + }; +}); +vi.mock("@fluxcore/systems/welcome/config", () => ({ + getWelcomeConfig: vi.fn(), + upsertWelcomeConfig: vi.fn(), +})); +vi.mock("@fluxcore/utils", () => ({ + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})); + +import Fastify from "fastify"; +import fastifyCookie from "@fastify/cookie"; +import { registerWelcomeRoutes } from "../../../../src/server/features/welcome/routes.js"; + +async function buildApp() { + const app = Fastify(); + await app.register(fastifyCookie, { secret: "test-secret" }); + registerWelcomeRoutes(app); + await app.ready(); + return app; +} + +const PNG_HEADER = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); +const JPEG_HEADER = Buffer.from([0xff, 0xd8, 0xff, 0xe0]); + +describe("POST /api/guilds/:guildId/welcome/image/background — magic byte validation", () => { + let app: Awaited>; + + beforeEach(async () => { + vi.clearAllMocks(); + app = await buildApp(); + }); + + it("rejects invalid base64 with 400", async () => { + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/welcome/image/background", + cookies: { session: app.signCookie("valid") }, + payload: { data: "@@@not-base64@@@", contentType: "image/png" }, + }); + expect(res.statusCode).toBe(400); + }); + + it("rejects payload whose magic bytes do not match contentType", async () => { + const fakePng = Buffer.from("hello world").toString("base64"); + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/welcome/image/background", + cookies: { session: app.signCookie("valid") }, + payload: { data: fakePng, contentType: "image/png" }, + }); + expect(res.statusCode).toBe(400); + expect(res.json().error).toMatch(/png|magic|content/i); + }); + + it("rejects mismatched contentType vs header (jpeg sent as png)", async () => { + const data = JPEG_HEADER.toString("base64"); + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/welcome/image/background", + cookies: { session: app.signCookie("valid") }, + payload: { data, contentType: "image/png" }, + }); + expect(res.statusCode).toBe(400); + }); + + it("accepts a valid PNG header", async () => { + const data = Buffer.concat([PNG_HEADER, Buffer.alloc(16)]).toString("base64"); + const res = await app.inject({ + method: "POST", + url: "/api/guilds/guild-1/welcome/image/background", + cookies: { session: app.signCookie("valid") }, + payload: { data, contentType: "image/png" }, + }); + expect(res.statusCode).toBe(200); + expect(mockUpload).toHaveBeenCalled(); + }); +}); diff --git a/apps/dashboard/tests/server/features/welcome/welcome.test.ts b/apps/dashboard/tests/server/features/welcome/welcome.test.ts index 1598526..8bf7d3c 100644 --- a/apps/dashboard/tests/server/features/welcome/welcome.test.ts +++ b/apps/dashboard/tests/server/features/welcome/welcome.test.ts @@ -373,7 +373,13 @@ describe("welcome routes", () => { method: "POST", url: "/api/guilds/guild-1/welcome/image/background", cookies: { session: app.signCookie("valid") }, - payload: { data: Buffer.from("fake-png").toString("base64"), contentType: "image/png" }, + payload: { + data: Buffer.concat([ + Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]), + Buffer.alloc(8), + ]).toString("base64"), + contentType: "image/png", + }, }); expect(res.statusCode).toBe(200); From eef01c93950d3f51e085c02336e2d9f4d1a4c2d1 Mon Sep 17 00:00:00 2001 From: Abdulkhalek Muhammad Date: Tue, 7 Apr 2026 14:17:29 +0200 Subject: [PATCH 03/61] 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 `