diff --git a/src/data/users.test.ts b/src/data/users.test.ts index dc6294b..2576238 100644 --- a/src/data/users.test.ts +++ b/src/data/users.test.ts @@ -208,4 +208,19 @@ describe("guest row exclusion", () => { expect(users.countUsers()).toBe(1); // alice only expect(users.listUsers().some((u) => u.id === GUEST_USER_ID)).toBe(false); }); + + it("setRoleIfNotLastAdmin refuses to re-role the reserved guest principal", () => { + // The guest row is seeded by createDatabase via ensureGuestUser. + expect(users.findById(GUEST_USER_ID)!.role).toBe("guest"); // sanity + expect(users.setRoleIfNotLastAdmin(GUEST_USER_ID, "admin")).toBe("not_found"); + // The guest row's role is unchanged. + expect(users.findById(GUEST_USER_ID)!.role).toBe("guest"); + }); + + it("deleteUserIfNotLastAdmin refuses to delete the reserved guest principal", () => { + expect(users.findById(GUEST_USER_ID)).not.toBeNull(); // sanity + expect(users.deleteUserIfNotLastAdmin(GUEST_USER_ID)).toBe("not_found"); + // The guest row still exists. + expect(users.findById(GUEST_USER_ID)).not.toBeNull(); + }); }); diff --git a/src/data/users.ts b/src/data/users.ts index 4a6d3b9..dbb530b 100644 --- a/src/data/users.ts +++ b/src/data/users.ts @@ -137,6 +137,7 @@ export function createUserStore(db: Database.Database): UserStore { const tx = db.transaction(() => { const row = findByIdStmt.get(id) as UserRow | undefined; if (!row) return "not_found" as const; + if (row.role === "guest") return "not_found" as const; // reserved synthetic principal if (row.role === newRole) return "ok" as const; // no-op if (row.role === "admin" && newRole === "member") { const adminCount = (countAdminsStmt.get() as { n: number }).n; @@ -161,6 +162,7 @@ export function createUserStore(db: Database.Database): UserStore { const tx = db.transaction(() => { const row = findByIdStmt.get(id) as UserRow | undefined; if (!row) return "not_found" as const; + if (row.role === "guest") return "not_found" as const; // reserved synthetic principal if (row.role === "admin") { const adminCount = (countAdminsStmt.get() as { n: number }).n; if (adminCount <= 1) return "would_orphan" as const; diff --git a/src/web/api/session.ts b/src/web/api/session.ts index beaef21..4f4d5a0 100644 --- a/src/web/api/session.ts +++ b/src/web/api/session.ts @@ -141,7 +141,16 @@ export function createSessionRouter( res.status(403).json({ error: "guest mode disabled" }); return; } - const { token } = sessions.createSession(GUEST_USER_ID, { ttlMs: GUEST_SESSION_TTL_MS, skipCap: true }); + let token: string; + try { + // If the reserved guest row is somehow missing, the session FK would + // throw; surface a clean 503 rather than letting it become a 500. + ({ token } = sessions.createSession(GUEST_USER_ID, { ttlMs: GUEST_SESSION_TTL_MS, skipCap: true })); + } catch (err) { + logger.error({ err }, "guest session creation failed"); + res.status(503).json({ error: "guest unavailable" }); + return; + } setSessionCookie(res, token); res.json({ id: GUEST_USER_ID, username: GUEST_USERNAME, role: "guest" }); }); diff --git a/src/web/api/users.test.ts b/src/web/api/users.test.ts index c422907..1920fe4 100644 --- a/src/web/api/users.test.ts +++ b/src/web/api/users.test.ts @@ -4,7 +4,7 @@ import cookieParser from "cookie-parser"; import request from "supertest"; import pino from "pino"; import { createDatabase, type BotDatabase } from "../../data/database.js"; -import { createUserStore, type UserStore } from "../../data/users.js"; +import { createUserStore, GUEST_USER_ID, type UserStore } from "../../data/users.js"; import { createSessionStore, type SessionStore } from "../../data/sessions.js"; import { createAuditStore, type AuditStore } from "../../data/audit.js"; import { createPermissionStore, type PermissionStore } from "../../data/permissions.js"; @@ -342,4 +342,49 @@ describe("users router", () => { expect(perms.status).toBe(200); expect(perms.body).toEqual({ capabilities: [], bots: [] }); }); + + // --- reserved guest principal is never mutable/visible via user mgmt ------- + // The synthetic __guest__ row is seeded by createDatabase. findById has no + // role filter, so without the 404-guard these by-id handlers would operate + // on it (privilege-escalation / DoS / cred-login holes). + describe("reserved __guest__ principal is 404 on every by-id handler", () => { + it("DELETE /:id → 404 and the guest row survives", async () => { + const res = await request(app).delete(`/api/users/${GUEST_USER_ID}`).set("Cookie", aliceCookie); + expect(res.status).toBe(404); + expect(users.findById(GUEST_USER_ID)).not.toBeNull(); + expect(users.findById(GUEST_USER_ID)!.role).toBe("guest"); + }); + + it("PATCH /:id/role {role:'admin'} → 404 and the guest role is unchanged", async () => { + const res = await request(app) + .patch(`/api/users/${GUEST_USER_ID}/role`) + .set("Cookie", aliceCookie) + .send({ role: "admin" }); + expect(res.status).toBe(404); + expect(users.findById(GUEST_USER_ID)!.role).toBe("guest"); + }); + + it("POST /:id/reset-password → 404 (cannot give the guest a login)", async () => { + const res = await request(app) + .post(`/api/users/${GUEST_USER_ID}/reset-password`) + .set("Cookie", aliceCookie) + .send({ newPassword: "guest-new-pw" }); + expect(res.status).toBe(404); + }); + + it("GET /:id/permissions → 404", async () => { + const res = await request(app) + .get(`/api/users/${GUEST_USER_ID}/permissions`) + .set("Cookie", aliceCookie); + expect(res.status).toBe(404); + }); + + it("PUT /:id/permissions → 404", async () => { + const res = await request(app) + .put(`/api/users/${GUEST_USER_ID}/permissions`) + .set("Cookie", aliceCookie) + .send({ capabilities: ["player.control"], bots: "all" }); + expect(res.status).toBe(404); + }); + }); }); diff --git a/src/web/api/users.ts b/src/web/api/users.ts index 7d38236..0e736fc 100644 --- a/src/web/api/users.ts +++ b/src/web/api/users.ts @@ -1,7 +1,7 @@ import { Router } from "express"; import type { Logger } from "../../logger.js"; import type { UserStore } from "../../data/users.js"; -import { UsernameTakenError } from "../../data/users.js"; +import { UsernameTakenError, GUEST_USER_ID } from "../../data/users.js"; import type { SessionStore } from "../../data/sessions.js"; import type { AuditStore } from "../../data/audit.js"; import { isCapability, BASIC_TIER_CAPABILITIES, type PermissionStore } from "../../data/permissions.js"; @@ -63,6 +63,7 @@ export function createUsersRouter( router.delete("/:id", (req, res) => { const targetId = req.params.id; + if (targetId === GUEST_USER_ID) { res.status(404).json({ error: "not found" }); return; } // Snapshot target's username BEFORE deletion for audit const target = users.findById(targetId); if (!target) { @@ -104,6 +105,7 @@ export function createUsersRouter( return; } const targetId = req.params.id; + if (targetId === GUEST_USER_ID) { res.status(404).json({ error: "not found" }); return; } const target = users.findById(targetId); if (!target) { res.status(404).json({ error: "not found" }); @@ -130,6 +132,7 @@ export function createUsersRouter( router.patch("/:id/role", (req, res) => { const targetId = req.params.id; + if (targetId === GUEST_USER_ID) { res.status(404).json({ error: "not found" }); return; } const { role: newRole } = req.body ?? {}; if (newRole !== "admin" && newRole !== "member") { res.status(400).json({ error: "invalid role" }); @@ -168,6 +171,7 @@ export function createUsersRouter( }); router.get("/:id/permissions", (req, res) => { + if (req.params.id === GUEST_USER_ID) { res.status(404).json({ error: "not found" }); return; } const user = users.findById(req.params.id); if (!user) { res.status(404).json({ error: "not_found" }); @@ -180,6 +184,7 @@ export function createUsersRouter( }); router.put("/:id/permissions", (req, res) => { + if (req.params.id === GUEST_USER_ID) { res.status(404).json({ error: "not found" }); return; } const user = users.findById(req.params.id); if (!user) { res.status(404).json({ error: "not_found" });