diff --git a/docs/superpowers/plans/2026-09-16-phase-1b-design-system-and-board.md b/docs/superpowers/plans/2026-09-16-phase-1b-design-system-and-board.md index d24130c..b297729 100644 --- a/docs/superpowers/plans/2026-09-16-phase-1b-design-system-and-board.md +++ b/docs/superpowers/plans/2026-09-16-phase-1b-design-system-and-board.md @@ -1556,48 +1556,121 @@ export function BoardColumn({ } ``` -- [ ] **Step 3: Create the board route** +- [ ] **Step 3: Create the client card controls** -`src/app/p/[clientSlug]/[projectSlug]/page.tsx`: +`src/components/board/client-card-controls.tsx`: ```tsx -import { notFound, redirect } from "next/navigation"; +"use client"; -import { AppShell } from "@/components/shell/app-shell"; -import { TopBar } from "@/components/shell/top-bar"; -import { Board } from "@/components/board/board"; -import { getSession } from "@/lib/auth/session"; -import { loadBoard } from "@/lib/data/board"; +import { useState } from "react"; -export default async function BoardPage({ - params, +import { + Dropdown, + DropdownContent, + DropdownItem, + DropdownLabel, + DropdownTrigger, +} from "@/components/ui/dropdown"; +import type { CardRecord, Priority } from "@/lib/pb/types"; + +const PRIORITIES: Array<{ value: Priority; label: string }> = [ + { value: "critical", label: "Critical" }, + { value: "high", label: "High" }, + { value: "medium", label: "Medium" }, + { value: "low", label: "Low" }, +]; + +/** + * The only two fields a client manager may change. + * + * Both go through PATCH /api/client/cards/:id, never straight to PocketBase. + * Their token cannot write to the cards collection at all — see Plan 1a — so + * a direct SDK call from here would simply fail. The route is the path. + */ +export function ClientCardControls({ + card, + onPatched, }: { - params: Promise<{ clientSlug: string; projectSlug: string }>; + card: CardRecord; + onPatched: (card: CardRecord) => void; }) { - const session = await getSession(); - if (!session) redirect("/login"); + const [saving, setSaving] = useState(false); + const [error, setError] = useState(null); - const { clientSlug, projectSlug } = await params; - const board = await loadBoard(clientSlug, projectSlug); + async function patch(body: Record) { + setSaving(true); + setError(null); - // loadBoard returns null both for "does not exist" and "not yours". Rendering - // the same 404 for both is deliberate: a distinct 403 would confirm to one - // client that another client's project is real. - if (!board) notFound(); + const previous = { ...card }; + onPatched({ ...card, ...body } as CardRecord); + + const response = await fetch(`/api/client/cards/${card.id}`, { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); + + setSaving(false); + + if (!response.ok) { + onPatched(previous); + setError("Could not save"); + return; + } + + const { card: updated } = await response.json(); + onPatched({ ...card, ...updated } as CardRecord); + } + + const currentPriority = + PRIORITIES.find((p) => p.value === card.priority) ?? PRIORITIES[2]; return ( - - - - +
+ + + + + + Priority + {PRIORITIES.map((option) => ( + patch({ priority: option.value })} + > + {option.label} + + ))} + + + + + + {error && ( + + {error} + + )} +
); } ``` @@ -1605,20 +1678,123 @@ export default async function BoardPage({ - [ ] **Step 4: Commit** ```bash -git add src/components/board src/app/p -git commit -m "feat(board): column and card tile presentation" +git add src/components/board +git commit -m "feat(board): column, card tile and client control presentation" ``` -The build will fail until Task 10 creates `Board`. That is expected — commit the presentation layer on its own so it can be reviewed separately. - --- -## Task 10: Drag, drop and persistence +## Task 10: Owner card move route + +**Files:** +- Create: `src/app/api/owner/cards/[id]/move/route.ts` + +**Why this route exists.** Plan 1a stores the auth token in an **httpOnly** cookie, which JavaScript cannot read. That is the right call for a token, and it has a consequence worth stating plainly: the browser-side PocketBase SDK has no credentials and cannot write anything. An owner drag that called `pbBrowser.collection("cards").update(...)` would fail silently with a 403 and the card would snap back. + +The two ways out are to make the cookie readable by JavaScript, or to move the write server-side. The first hands a long-lived token to any XSS on the page; the second costs one small route. This plan takes the second. + +`src/lib/pb/browser.ts` from Plan 1a therefore stays unauthenticated and is used for nothing in this plan. Leave it in place — Plan 1c may authenticate it deliberately for realtime, with that tradeoff made explicitly. + +- [ ] **Step 1: Write the route** + +`src/app/api/owner/cards/[id]/move/route.ts`: + +```ts +import { getServerPb } from "@/lib/pb/server"; +import { requireSession } from "@/lib/auth/session"; +import { AccessError } from "@/lib/auth/access"; +import { jsonError, jsonOk } from "@/lib/http/json"; +import type { CardRecord } from "@/lib/pb/types"; + +/** + * PATCH /api/owner/cards/:id/move — the owner drops a card in a new place. + * + * Unlike the /api/client/* routes, this one uses the caller's OWN PocketBase + * token, not a superuser's. The owner-only write rule from Plan 1a is what + * authorises the update, so a client manager reaching this route is refused by + * the database even though the handler itself is simple. + * + * That is why there is no field whitelist here: PocketBase is already + * enforcing who may write. Only the two positional fields are forwarded + * regardless, so a stray body field cannot ride along. + */ +export async function PATCH( + request: Request, + context: { params: Promise<{ id: string }> } +): Promise { + try { + const session = await requireSession(); + const { id } = await context.params; + + if (session.role !== "owner") { + return jsonError("Card not found", 404); + } + + let body: unknown; + try { + body = await request.json(); + } catch { + return jsonError("Invalid request body", 400); + } + + const record = body as Record | null; + const column = record?.column; + const position = record?.position; + + if (typeof column !== "string" || !column) { + return jsonError("A destination column is required", 400); + } + if (typeof position !== "number" || !Number.isFinite(position)) { + return jsonError("A numeric position is required", 400); + } + + const pb = await getServerPb(); + + const updated = await pb + .collection("cards") + .update(id, { column, position }); + + return jsonOk({ + card: { + id: updated.id, + column: updated.column, + position: updated.position, + }, + }); + } catch (error) { + if (error instanceof AccessError) { + return jsonError(error.message, error.status); + } + console.error("[owner-move] failed", error); + return jsonError("Could not move the card", 500); + } +} +``` + +- [ ] **Step 2: Verify the build** + +```bash +npm run build +``` + +Expected: build succeeds; the route list includes `/api/owner/cards/[id]/move`. + +- [ ] **Step 3: Commit** + +```bash +git add src/app/api/owner +git commit -m "feat(api): owner card move route using the caller's own token" +``` + +--- + +## Task 11: Drag, drop and persistence **Files:** - Create: `src/components/board/board.tsx` +- Create: `src/app/p/[clientSlug]/[projectSlug]/page.tsx` -**Who may drag.** The owner only. A client manager reads the board and edits two fields through the controls in Task 11; the spec is explicit that they may not move cards between columns. `@dnd-kit` is given no sensors at all for a client manager, so there is nothing to disable and nothing to bypass. This is presentation convenience, not the security boundary — the boundary is the PocketBase write rule from Plan 1a, which refuses their token outright. +**Who may drag.** The owner only. A client manager reads the board and edits two fields through the controls from Task 9; the spec is explicit that they may not move cards between columns. `@dnd-kit` is given no sensors at all for a client manager, so there is nothing to disable and nothing to bypass. This is presentation convenience, not the security boundary — the boundary is the PocketBase write rule from Plan 1a, which refuses their token outright. - [ ] **Step 1: Install @dnd-kit** diff --git a/src/lib/cards/client-patch.test.ts b/src/lib/cards/client-patch.test.ts new file mode 100644 index 0000000..539e4b9 --- /dev/null +++ b/src/lib/cards/client-patch.test.ts @@ -0,0 +1,94 @@ +import { describe, it, expect } from "vitest"; +import { + CLIENT_EDITABLE_CARD_FIELDS, + pickClientCardPatch, + pickClientCardCreate, +} from "@/lib/cards/client-patch"; + +describe("pickClientCardPatch", () => { + it("accepts priority and target_date", () => { + const { patch, dropped } = pickClientCardPatch({ + priority: "high", + target_date: "2026-10-01", + }); + expect(patch).toEqual({ priority: "high", target_date: "2026-10-01" }); + expect(dropped).toEqual([]); + }); + + it("drops every field outside the whitelist", () => { + const { patch, dropped } = pickClientCardPatch({ + priority: "low", + column: "col_done", + title: "hacked", + accepted: true, + archived: true, + scope_rule: "stock", + }); + expect(patch).toEqual({ priority: "low" }); + expect(dropped.sort()).toEqual( + ["accepted", "archived", "column", "scope_rule", "title"].sort() + ); + }); + + it("drops a priority outside the allowed set", () => { + const { patch, dropped } = pickClientCardPatch({ priority: "urgent" }); + expect(patch).toEqual({}); + expect(dropped).toEqual(["priority"]); + }); + + it("accepts null target_date as clearing the date", () => { + const { patch } = pickClientCardPatch({ target_date: null }); + expect(patch).toEqual({ target_date: "" }); + }); + + it("drops a malformed target_date", () => { + const { patch, dropped } = pickClientCardPatch({ target_date: "next tuesday" }); + expect(patch).toEqual({}); + expect(dropped).toEqual(["target_date"]); + }); + + it("returns an empty patch for a non-object body", () => { + expect(pickClientCardPatch(null).patch).toEqual({}); + expect(pickClientCardPatch("nope").patch).toEqual({}); + expect(pickClientCardPatch(42).patch).toEqual({}); + }); + + it("exposes exactly two editable fields", () => { + expect([...CLIENT_EDITABLE_CARD_FIELDS]).toEqual(["priority", "target_date"]); + }); +}); + +describe("pickClientCardCreate", () => { + it("accepts title and description only", () => { + const { input, dropped } = pickClientCardCreate({ + title: "Add a POS system", + description: "We need card payments at the counter.", + }); + expect(input).toEqual({ + title: "Add a POS system", + description: "We need card payments at the counter.", + }); + expect(dropped).toEqual([]); + }); + + it("drops column, accepted and priority on create", () => { + const { input, dropped } = pickClientCardCreate({ + title: "x", + column: "col_inprogress", + accepted: true, + priority: "critical", + }); + expect(input).toEqual({ title: "x", description: "" }); + expect(dropped.sort()).toEqual(["accepted", "column", "priority"].sort()); + }); + + it("rejects a missing or blank title", () => { + expect(pickClientCardCreate({ description: "no title" }).input).toBeNull(); + expect(pickClientCardCreate({ title: " " }).input).toBeNull(); + }); + + it("trims the title and caps it at 200 characters", () => { + const { input } = pickClientCardCreate({ title: ` ${"a".repeat(250)} ` }); + expect(input!.title).toHaveLength(200); + }); +}); \ No newline at end of file diff --git a/src/lib/cards/client-patch.ts b/src/lib/cards/client-patch.ts new file mode 100644 index 0000000..fc528a8 --- /dev/null +++ b/src/lib/cards/client-patch.ts @@ -0,0 +1,125 @@ +/** + * The field whitelist for client-manager writes. + * + * This module is the entire reason /api/client/* exists. PocketBase API rules + * are record-level: there is no rule that says "this user may update priority + * but not column". So a client manager is denied all writes at the database, + * and their requests pass through here instead, where only known-good fields + * survive. + * + * The whitelist is positive, never a blocklist. A new card field added in a + * later phase is inaccessible to clients by default, which is the failure + * direction we want. + */ + +import type { Priority } from "@/lib/pb/types"; + +export const CLIENT_EDITABLE_CARD_FIELDS = ["priority", "target_date"] as const; + +const PRIORITIES: readonly Priority[] = ["critical", "high", "medium", "low"]; + +/** PocketBase stores dates as strings; "" means unset. */ +const ISO_DATE = /^\d{4}-\d{2}-\d{2}$/; + +export interface ClientCardPatch { + priority?: Priority; + target_date?: string; +} + +export interface ClientCardCreate { + title: string; + description: string; +} + +function asRecord(body: unknown): Record | null { + if (typeof body !== "object" || body === null || Array.isArray(body)) { + return null; + } + return body as Record; +} + +/** + * Split an untrusted PATCH body into the fields we will apply and the names + * of everything we refused. Dropped names are returned so the caller can log + * them — a client repeatedly sending `column` is worth noticing. + */ +export function pickClientCardPatch(body: unknown): { + patch: ClientCardPatch; + dropped: string[]; +} { + const record = asRecord(body); + if (!record) return { patch: {}, dropped: [] }; + + const patch: ClientCardPatch = {}; + const dropped: string[] = []; + + for (const [key, value] of Object.entries(record)) { + if (key === "priority") { + if (typeof value === "string" && PRIORITIES.includes(value as Priority)) { + patch.priority = value as Priority; + } else { + dropped.push(key); + } + continue; + } + + if (key === "target_date") { + if (value === null || value === "") { + patch.target_date = ""; + } else if (typeof value === "string" && ISO_DATE.test(value)) { + patch.target_date = value; + } else { + dropped.push(key); + } + continue; + } + + dropped.push(key); + } + + return { patch, dropped }; +} + +/** + * Split an untrusted create body the same way. + * + * Note what is absent: no column, no priority, no accepted. The route handler + * sets those itself — Inbox, medium, false — so a client cannot create a card + * straight into In Progress at critical priority. + * + * `input` is null when the body has no usable title. + */ +export function pickClientCardCreate(body: unknown): { + input: ClientCardCreate | null; + dropped: string[]; +} { + const record = asRecord(body); + if (!record) return { input: null, dropped: [] }; + + const dropped: string[] = []; + let title = ""; + let description = ""; + + for (const [key, value] of Object.entries(record)) { + if (key === "title") { + if (typeof value === "string") title = value.trim().slice(0, 200); + else dropped.push(key); + continue; + } + + if (key === "description") { + if (typeof value === "string") description = value.slice(0, 50000); + else dropped.push(key); + continue; + } + + // `project` is read from the body by the route handler before calling + // this, so it is expected here and is not a violation. + if (key === "project") continue; + + dropped.push(key); + } + + if (!title) return { input: null, dropped }; + return { input: { title, description }, dropped }; +} \ No newline at end of file