diff --git a/Dockerfile b/Dockerfile index 6021cae..01279b5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM node:24-bookworm-slim@sha256:0e0ff40c39bc087845bfb27465a0df4ea419520094bc35842ff83dd8cbe6f9b6 AS build +FROM --platform=$BUILDPLATFORM node:24-bookworm-slim@sha256:0e0ff40c39bc087845bfb27465a0df4ea419520094bc35842ff83dd8cbe6f9b6 AS build WORKDIR /app COPY package.json package-lock.json ./ COPY packages/shared/package.json packages/shared/package.json @@ -10,7 +10,7 @@ RUN npm run build:shared && npm run build -w @seahaven-ap/api ARG GIT_SHA=unknown RUN GIT_SHA="$GIT_SHA" node -e "require('node:fs').writeFileSync('packages/api/dist/build-info.js', 'export const BUILD_GIT_SHA = ' + JSON.stringify(process.env.GIT_SHA || 'unknown') + ';\\n')" -FROM node:24-alpine@sha256:ebfe2f90462722a7a4de65e91990e97fe0d401c70e0e762c5b53302f905ec1c1 +FROM --platform=linux/amd64 node:24-alpine@sha256:ebfe2f90462722a7a4de65e91990e97fe0d401c70e0e762c5b53302f905ec1c1 RUN addgroup -S app && adduser -S -G app app WORKDIR /app COPY package.json package-lock.json ./ diff --git a/packages/api/src/app.ts b/packages/api/src/app.ts index eb647df..a4a95a7 100644 --- a/packages/api/src/app.ts +++ b/packages/api/src/app.ts @@ -12,7 +12,7 @@ import { createUserRoutes } from "./routes/users.js"; import { createInvoiceRoutes } from "./routes/invoices.js"; import { createApprovalRoutes } from "./routes/approvals.js"; import { createDocumentsStore, type DocumentsStore } from "./documents.js"; -import { errorJson } from "./http.js"; +import { ApiError, errorJson } from "./http.js"; import { cloudFrontOriginAllowed } from "./auth/origin-verify.js"; import { csrfAllowed, isMutating } from "./auth/oauth.js"; import type { CognitoTokenClient } from "./auth/cognito.js"; @@ -58,6 +58,9 @@ export function createApp(env: ApiEnv, handle: Db, deps: AppDeps = {}) { app.notFound((c) => errorJson(c, 404, "NOT_FOUND", "Not found.")); app.onError((error, c) => { + if (error instanceof ApiError) { + return errorJson(c, error.status, error.code, error.message); + } console.error(error); return errorJson(c, 500, "INTERNAL_ERROR", "Internal server error."); }); diff --git a/packages/api/src/approvals.ts b/packages/api/src/approvals.ts index 509b94d..89afeb6 100644 --- a/packages/api/src/approvals.ts +++ b/packages/api/src/approvals.ts @@ -1,5 +1,5 @@ import { eq } from "drizzle-orm"; -import type { Db } from "./db/client.js"; +import type { DbHandle } from "./db/client.js"; import { activityLog, approvalPolicies, approvalSteps, invoices } from "./db/schema/index.js"; import { moneyCents, rowsOf } from "./routes/helpers.js"; @@ -8,7 +8,7 @@ type PolicyRow = typeof approvalPolicies.$inferSelect; type StepRow = typeof approvalSteps.$inferSelect; export async function appendActivity( - handle: Db, + handle: DbHandle, invoiceId: string, actorUserId: string, message: string, @@ -17,7 +17,7 @@ export async function appendActivity( } export async function applyMatchingPolicy( - handle: Db, + handle: DbHandle, invoice: InvoiceRow, actorUserId: string, ): Promise { @@ -68,7 +68,28 @@ export async function applyMatchingPolicy( return updated ?? { ...invoice, status: "approved" }; } -export async function remainingPending(handle: Db, invoiceId: string): Promise { +export async function remainingPending(handle: DbHandle, invoiceId: string): Promise { const rows = await rowsOf(handle, approvalSteps); return rows.filter((row) => row.invoiceId === invoiceId && row.status === "pending"); } + +export async function closePendingSteps( + handle: DbHandle, + invoiceId: string, + actorUserId: string | null, +): Promise { + const pending = await remainingPending(handle, invoiceId); + const actedAt = new Date(); + for (const step of pending) { + await handle.db + .update(approvalSteps) + .set({ + id: step.id, + status: "skipped", + actedByUserId: actorUserId, + actedAt, + }) + .where(eq(approvalSteps.id, step.id)) + .returning(); + } +} diff --git a/packages/api/src/auth/middleware.ts b/packages/api/src/auth/middleware.ts index d6d9426..f5707aa 100644 --- a/packages/api/src/auth/middleware.ts +++ b/packages/api/src/auth/middleware.ts @@ -23,12 +23,13 @@ export type AuthDeps = { verifyToken?: TokenVerifier; }; -function roleFromClaims(claims: Record, fallback: UserRole): UserRole { +function roleFromClaims(claims: Record): UserRole | undefined { const raw = (typeof claims["custom:role"] === "string" && claims["custom:role"]) || (typeof claims.role === "string" && claims.role) || - fallback; - return isUserRole(raw) ? raw : fallback; + undefined; + if (raw === undefined) return undefined; + return isUserRole(raw) ? raw : undefined; } function audienceMatches(payload: JWTPayload, expected: string): boolean { @@ -141,7 +142,7 @@ export function createAuthMiddleware(env: ApiEnv, handle: Db, deps: AuthDeps = { cognitoSub: identity.sub, email: identity.email, name: identity.name, - role: roleFromClaims(payload as Record, "viewer"), + role: roleFromClaims(payload as Record), }); } catch (error) { if (error instanceof IdentityConflictError) { diff --git a/packages/api/src/auth/upsert-user.test.ts b/packages/api/src/auth/upsert-user.test.ts index 8eceaa8..6afb9e9 100644 --- a/packages/api/src/auth/upsert-user.test.ts +++ b/packages/api/src/auth/upsert-user.test.ts @@ -5,7 +5,7 @@ import { IdentityConflictError, upsertUserFromIdentity } from "./upsert-user.js" function createDb(options: { bySub?: Record | null; byEmail?: Record | null; -}): Db { +}): { handle: Db; setSpy: ReturnType } { const findFirst = vi.fn(async (_args: { where: unknown }) => { // drizzle eq objects aren't introspectable here; alternate by call order. if (findFirst.mock.calls.length === 1) { @@ -22,30 +22,35 @@ function createDb(options: { role: "admin" as const, }; + const setSpy = vi.fn((patch: Record) => ({ + where: vi.fn(() => ({ + returning: vi.fn(async () => [{ ...returningRow, ...patch, role: patch.role ?? returningRow.role }]), + })), + })); + return { - driver: "postgres", - pool: { end: vi.fn(async () => undefined) } as never, - db: { - query: { users: { findFirst } }, - update: vi.fn(() => ({ - set: vi.fn(() => ({ - where: vi.fn(() => ({ + handle: { + driver: "postgres", + pool: { end: vi.fn(async () => undefined) } as never, + db: { + query: { users: { findFirst } }, + update: vi.fn(() => ({ + set: setSpy, + })), + insert: vi.fn(() => ({ + values: vi.fn(() => ({ returning: vi.fn(async () => [returningRow]), })), })), - })), - insert: vi.fn(() => ({ - values: vi.fn(() => ({ - returning: vi.fn(async () => [returningRow]), - })), - })), - } as never, + } as never, + }, + setSpy, }; } describe("upsertUserFromIdentity", () => { it("updates an existing row matched by cognito sub", async () => { - const handle = createDb({ + const { handle } = createDb({ bySub: { id: "11111111-1111-4111-8111-111111111111", cognitoSub: "seed-sub-admin", @@ -66,8 +71,52 @@ describe("upsertUserFromIdentity", () => { expect(handle.db.update).toHaveBeenCalled(); }); + it("preserves the stored role when the token omits a role claim", async () => { + const { handle, setSpy } = createDb({ + bySub: { + id: "11111111-1111-4111-8111-111111111111", + cognitoSub: "seed-sub-admin", + email: "admin@seahavenind.com", + name: "Dev Admin", + role: "admin", + }, + }); + + const user = await upsertUserFromIdentity(handle, { + cognitoSub: "seed-sub-admin", + email: "admin@seahavenind.com", + name: "Dev Admin", + }); + + expect(setSpy).toHaveBeenCalledWith( + expect.not.objectContaining({ role: expect.anything() }), + ); + expect(user.role).toBe("admin"); + }); + + it("writes an explicit role claim over the stored role", async () => { + const { handle, setSpy } = createDb({ + bySub: { + id: "11111111-1111-4111-8111-111111111111", + cognitoSub: "seed-sub-admin", + email: "admin@seahavenind.com", + name: "Dev Admin", + role: "viewer", + }, + }); + + await upsertUserFromIdentity(handle, { + cognitoSub: "seed-sub-admin", + email: "admin@seahavenind.com", + name: "Dev Admin", + role: "approver", + }); + + expect(setSpy).toHaveBeenCalledWith(expect.objectContaining({ role: "approver" })); + }); + it("refuses to rebind an email owned by a different cognito sub", async () => { - const handle = createDb({ + const { handle } = createDb({ bySub: null, byEmail: { id: "11111111-1111-4111-8111-111111111111", diff --git a/packages/api/src/auth/upsert-user.ts b/packages/api/src/auth/upsert-user.ts index d0373a5..fdf3201 100644 --- a/packages/api/src/auth/upsert-user.ts +++ b/packages/api/src/auth/upsert-user.ts @@ -7,7 +7,8 @@ export type AuthIdentity = { cognitoSub: string; email: string; name: string; - role: UserRole; + /** Present only when the token carries an explicit role claim. */ + role?: UserRole; }; export type AuthUser = { @@ -46,6 +47,7 @@ function toAuthUser(row: { /** * Upsert by Cognito subject only. Never rebind an existing email to a new * subject — that would allow account takeover if email claims collide. + * Missing role claims leave the stored role unchanged. */ export async function upsertUserFromIdentity( handle: Db, @@ -56,14 +58,22 @@ export async function upsertUserFromIdentity( }); if (bySub) { + const patch: { + email: string; + name: string; + role?: UserRole; + updatedAt: Date; + } = { + email: identity.email, + name: identity.name, + updatedAt: new Date(), + }; + if (identity.role !== undefined) { + patch.role = identity.role; + } const [updated] = await handle.db .update(users) - .set({ - email: identity.email, - name: identity.name, - role: identity.role, - updatedAt: new Date(), - }) + .set(patch) .where(eq(users.id, bySub.id)) .returning(); return toAuthUser(updated); @@ -83,7 +93,7 @@ export async function upsertUserFromIdentity( cognitoSub: identity.cognitoSub, email: identity.email, name: identity.name, - role: identity.role, + role: identity.role ?? "viewer", }) .returning(); diff --git a/packages/api/src/db/client.ts b/packages/api/src/db/client.ts index cddde03..12195e9 100644 --- a/packages/api/src/db/client.ts +++ b/packages/api/src/db/client.ts @@ -10,6 +10,17 @@ export type PostgresDb = ReturnType; export type DataApiDb = ReturnType; export type Db = PostgresDb | DataApiDb; +type PostgresTx = Parameters[0]>[0]; +type DataApiTx = Parameters[0]>[0]; + +/** + * Root connection or a transaction-scoped handle. Query helpers accept this so + * callers can pass `{ db: tx }` without casting a transaction to Db. + */ +export type DbHandle = { + db: Db["db"] | PostgresTx | DataApiTx; +}; + function createPostgresDb(env: ApiEnv) { const pool = new pg.Pool({ connectionString: env.databaseUrl }); return { diff --git a/packages/api/src/http.ts b/packages/api/src/http.ts index 3afbd8c..bee88b8 100644 --- a/packages/api/src/http.ts +++ b/packages/api/src/http.ts @@ -11,6 +11,17 @@ export type ErrorCode = | "INTERNAL_ERROR" | "DATABASE_UNAVAILABLE"; +export class ApiError extends Error { + constructor( + readonly status: ContentfulStatusCode, + readonly code: ErrorCode, + message: string, + ) { + super(message); + this.name = "ApiError"; + } +} + export const CORRELATION_HEADER = "x-correlation-id"; export type ErrorEnvelope = { diff --git a/packages/api/src/routes/approvals.test.ts b/packages/api/src/routes/approvals.test.ts index 4a50cc3..eafb6d8 100644 --- a/packages/api/src/routes/approvals.test.ts +++ b/packages/api/src/routes/approvals.test.ts @@ -3,7 +3,7 @@ import { createApp } from "../app.js"; import { createMemoryDocumentsStore } from "../documents.js"; import { loadEnv } from "../env.js"; import type { ErrorEnvelope } from "../http.js"; -import { createFakeDb, SEED } from "../test/fake-db.js"; +import { createFakeDb, emptyStore, SEED } from "../test/fake-db.js"; function expectEnvelope(body: unknown, code: string) { const envelope = body as ErrorEnvelope; @@ -77,6 +77,56 @@ describe("approval stubs", () => { expectEnvelope(await response.json(), "FORBIDDEN"); }); + it("returns 403 when the caller is not the assignee for the step", async () => { + const store = emptyStore(); + store.approvalSteps[0] = { + ...SEED.step, + assigneeUserId: "22222222-2222-4222-8222-222222222222", + }; + const api = createApp(envFor("admin"), createFakeDb(store), { + documents: createMemoryDocumentsStore(), + }); + const response = await api.request(`/api/approval-steps/${SEED.step.id}/decisions`, { + method: "POST", + headers: jsonHeaders, + body: JSON.stringify({ action: "approve" }), + }); + expect(response.status).toBe(403); + expectEnvelope(await response.json(), "FORBIDDEN"); + expect(store.approvalSteps[0]?.status).toBe("pending"); + }); + + it("voids an invoice, closes pending steps, and rejects later decisions", async () => { + const store = emptyStore(); + const api = createApp(envFor("admin"), createFakeDb(store), { + documents: createMemoryDocumentsStore(), + }); + const voided = await api.request(`/api/invoices/${SEED.invoice.id}`, { + method: "PATCH", + headers: jsonHeaders, + body: JSON.stringify({ status: "void" }), + }); + expect(voided.status).toBe(200); + await expect(voided.json()).resolves.toMatchObject({ status: "void" }); + expect(store.approvalSteps[0]?.status).toBe("skipped"); + + const inbox = await api.request("/api/inbox"); + expect(inbox.status).toBe(200); + const listed = (await inbox.json()) as { items: Array<{ id: string }> }; + expect(listed.items.some((item) => item.id === SEED.step.id)).toBe(false); + + // Re-open a pending step to prove voided invoices stay sealed even if a step remains. + store.approvalSteps[0] = { ...store.approvalSteps[0]!, status: "pending" }; + const decision = await api.request(`/api/approval-steps/${SEED.step.id}/decisions`, { + method: "POST", + headers: jsonHeaders, + body: JSON.stringify({ action: "approve" }), + }); + expect(decision.status).toBe(409); + expectEnvelope(await decision.json(), "CONFLICT"); + expect(store.invoices[0]?.status).toBe("void"); + }); + it("lists the caller inbox and accepts a comment", async () => { const api = app("admin"); const inbox = await api.request("/api/inbox"); diff --git a/packages/api/src/routes/approvals.ts b/packages/api/src/routes/approvals.ts index c7f55e3..cd48c7d 100644 --- a/packages/api/src/routes/approvals.ts +++ b/packages/api/src/routes/approvals.ts @@ -169,12 +169,20 @@ export function createApprovalRoutes(handle: Db) { if (step.status !== "pending") { return errorJson(c, 409, "CONFLICT", "Step is not pending."); } + const user = caller(c); + if (!inboxVisible(step, user)) { + return errorJson(c, 403, "FORBIDDEN", "You are not an assignee for this approval step."); + } + const invoice = await firstById(handle, invoices, step.invoiceId); + if (!invoice) return errorJson(c, 404, "NOT_FOUND", "Invoice not found."); + if (invoice.status === "void") { + return errorJson(c, 409, "CONFLICT", "Cannot act on a voided invoice."); + } const body = await parseJsonBody(c); const action = asString(body.action); if (!isDecision(action)) { return errorJson(c, 400, "VALIDATION_ERROR", "action must be approve, reject, or skip."); } - const user = caller(c); const nextStatus = action === "approve" ? "approved" : action === "reject" ? "rejected" : "skipped"; const [updated] = await handle.db diff --git a/packages/api/src/routes/helpers.ts b/packages/api/src/routes/helpers.ts index d12dfd1..ef75037 100644 --- a/packages/api/src/routes/helpers.ts +++ b/packages/api/src/routes/helpers.ts @@ -1,9 +1,9 @@ import { randomUUID } from "node:crypto"; import type { Context } from "hono"; -import type { Db } from "../db/client.js"; +import type { DbHandle } from "../db/client.js"; import type { UserRole } from "../env.js"; import { can, type RbacAction } from "../auth/rbac.js"; -import { errorJson } from "../http.js"; +import { ApiError, errorJson } from "../http.js"; import type { AppBindings } from "../auth/middleware.js"; const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; @@ -43,8 +43,12 @@ export function newId(): string { return randomUUID(); } -export function parseJsonBody(c: Context): Promise> { - return c.req.json>(); +export async function parseJsonBody(c: Context): Promise> { + try { + return await c.req.json>(); + } catch { + throw new ApiError(400, "VALIDATION_ERROR", "Request body must be valid JSON."); + } } export function asMoney(value: unknown): string | null { @@ -65,12 +69,12 @@ export function isDateOnly(value: string): boolean { return /^\d{4}-\d{2}-\d{2}$/.test(value); } -export async function rowsOf(handle: Db, table: unknown): Promise { +export async function rowsOf(handle: DbHandle, table: unknown): Promise { return (await handle.db.select().from(table as never)) as T[]; } export async function firstById( - handle: Db, + handle: DbHandle, table: unknown, id: string, ): Promise { diff --git a/packages/api/src/routes/invoices.test.ts b/packages/api/src/routes/invoices.test.ts index fe84394..aab0993 100644 --- a/packages/api/src/routes/invoices.test.ts +++ b/packages/api/src/routes/invoices.test.ts @@ -168,6 +168,21 @@ describe("invoice stubs", () => { expectEnvelope(await response.json(), "VALIDATION_ERROR"); }); + it("returns 400 VALIDATION_ERROR for a malformed JSON body", async () => { + const app = createApp(envFor("admin"), createFakeDb(), { + documents: createMemoryDocumentsStore(), + }); + const response = await app.request(`/api/invoices/${SEED.invoice.id}`, { + method: "PATCH", + headers: jsonHeaders, + body: "{not-json", + }); + expect(response.status).toBe(400); + const body = (await response.json()) as ErrorEnvelope; + expect(body.error.code).toBe("VALIDATION_ERROR"); + expect(body.error.message).toBe("Request body must be valid JSON."); + }); + it("keeps existing lines when a replace insert fails", async () => { const store = emptyStore(); const handle = createFakeDb(store); diff --git a/packages/api/src/routes/invoices.ts b/packages/api/src/routes/invoices.ts index ac76206..4f54d5d 100644 --- a/packages/api/src/routes/invoices.ts +++ b/packages/api/src/routes/invoices.ts @@ -1,11 +1,11 @@ import { eq } from "drizzle-orm"; import { Hono } from "hono"; -import type { Db } from "../db/client.js"; +import type { Db, DbHandle } from "../db/client.js"; import type { DocumentsStore } from "../documents.js"; import { documents, invoiceLines, invoices, vendors } from "../db/schema/index.js"; import type { AppBindings } from "../auth/middleware.js"; import { errorJson } from "../http.js"; -import { applyMatchingPolicy } from "../approvals.js"; +import { applyMatchingPolicy, closePendingSteps } from "../approvals.js"; import { asMoney, asString, @@ -103,13 +103,13 @@ function linesSumToAmount(lines: Array<{ amount: string }>, amount: string): boo return sum === moneyCents(amount); } -async function linesFor(handle: Db, invoiceId: string): Promise { +async function linesFor(handle: DbHandle, invoiceId: string): Promise { const rows = await rowsOf(handle, invoiceLines); return rows.filter((row) => row.invoiceId === invoiceId); } async function activeDuplicate( - handle: Db, + handle: DbHandle, vendorId: string, invoiceNumber: string, exceptId?: string, @@ -214,7 +214,7 @@ export function createInvoiceRoutes(handle: Db, store: DocumentsStore) { let currentLines: LineRow[] = []; try { await handle.db.transaction(async (tx) => { - const scoped = { ...handle, db: tx } as typeof handle; + const scoped: DbHandle = { db: tx }; const [row] = await tx .insert(invoices) .values({ @@ -335,6 +335,9 @@ export function createInvoiceRoutes(handle: Db, store: DocumentsStore) { } let row: InvoiceRow; try { + if (patch.status === "void") { + await closePendingSteps(handle, id, caller(c).id); + } [row] = await handle.db.update(invoices).set(patch).where(eq(invoices.id, id)).returning(); } catch (error) { if (isUniqueViolation(error)) {