diff --git a/.claude/TODO/2026-09-30-step8-fork-specific-low-risk-items.md b/.claude/TODO/2026-09-30-step8-fork-specific-low-risk-items.md deleted file mode 100644 index 433c63e..0000000 --- a/.claude/TODO/2026-09-30-step8-fork-specific-low-risk-items.md +++ /dev/null @@ -1,28 +0,0 @@ ---- -title: "Step 8 — Fork-specific low-risk items: address-label caps, error boundaries, tool-params decode, codegen escaping" -severity: low -tags: [security, bug] -area: components -files: [src/components/address-labels/actions.ts, src/services/addressLabelService.ts, src/app/global-error.tsx, src/lib/tool-params.server.ts, src/lib/providers/ministry-platform/scripts/generate-types.ts, src/lib/providers/ministry-platform/db/] -discovered: 2026-09-30 -discovered_by: playbook-audit (port-security-review-2026-09-28.md, fork sweep) -status: open ---- - -## Problem -- Address labels: `mergeTemplate`/`generateLabel*` merge client-sent labels (not re-fetched) with no count cap; - the 5 MB `.docx` template is PizZip-decompressed with no size limit (zip bomb); the 5 MB check is dead because - Next's default server-action body limit is 1 MB. -- `global-error.tsx` is retry-button only — no plain ``, button not held until hydration. -- `tool-params.server.ts` `decodeURIComponent(recordDescription)` throws on a stray `%` and crashes the page. -- `generate-types.ts:361,414-421` interpolates MP metadata raw into generated source. -- `api_dev_GetProcedureDefinition` lacks `@DomainID INT` as first parameter (repo rule for `api_*` procs). -- `UserService.getUserProfile` returns `User_ID`, `Mobile_Phone`, roles and groups to the client — trim to what the UI uses. - -## Proposed fix -Count cap + re-fetch or validate labels server-side; cap uncompressed size / entry count before PizZip; align -`serverActions.bodySizeLimit` with the documented limit; `global-error` fallback link; safe decode; escape codegen -strings; add `@DomainID`; minimise profile DTO. - -## Impact if not fixed -Memory exhaustion from a crafted template; a malformed URL crashes a tool page; minor data over-exposure. diff --git a/.claude/TODO/INDEX.md b/.claude/TODO/INDEX.md index 3bef953..a4e22a0 100644 --- a/.claude/TODO/INDEX.md +++ b/.claude/TODO/INDEX.md @@ -18,10 +18,10 @@ Severity tiers: - **medium**: doc drift, missing test, refactor with real cost - **low**: nits, minor doc fixes, stylistic improvements -Total: **3 open TODOs**. +Total: **2 open TODOs**. -> **`step6`** and **`step8`** are the remaining steps from the 2026-09-30 MPNext -> playbook audit (one PR each). `held-*` items wait on an owner decision. +> **`step6`** is the remaining step from the 2026-09-30 MPNext playbook audit. +> `held-*` items wait on an owner decision. --- @@ -39,22 +39,19 @@ _none open_ | routing | security, drift | Step 6 — Headers, next.config, server-only, prerender + build in CI | [→](2026-09-30-step6-headers-csp-build-guards.md) | | routing | security, doc | Held — CSP notes for customer-deployed clones | [→](2026-09-30-held-csp-customer-deployments.md) | -### Low (1) -| Area | Tags | Title | File | -|---|---|---|---| -| components | security, bug | Step 8 — Address-label caps, error boundaries, tool-params decode, codegen escaping | [→](2026-09-30-step8-fork-specific-low-risk-items.md) | +### Low (0) +_none open_ --- ## By tag -### security (3) +### security (2) - step6-headers-csp-build-guards — medium - held-csp-customer-deployments — medium -- step8-fork-specific-low-risk-items — low -### bug (1) -- step8-fork-specific-low-risk-items — low +### bug (0) +_none open_ ### drift (1) - step6-headers-csp-build-guards — medium @@ -77,7 +74,7 @@ _none open_ | Area | Count | |---|---| -| components | 1 | +| components | 0 | | routing | 2 | | services | 0 | | auth | 0 | diff --git a/.claude/references/DECISIONS.md b/.claude/references/DECISIONS.md index 554d99d..9100dc8 100644 --- a/.claude/references/DECISIONS.md +++ b/.claude/references/DECISIONS.md @@ -1,7 +1,7 @@ --- title: Architectural Decisions type: adr -last_updated: 2026-04-17 +last_updated: 2026-10-10 --- # Architectural Decisions @@ -84,6 +84,7 @@ Architectural decisions captured by the context-engineering review at SHA `971c4 **Context:** A tempting design is to enrich the session object inside `customSession` with the user's full MP profile (roles, user groups, Contact_ID, Image_GUID). Every consumer would then read a single object. The downside is that `customSession` runs on every cache miss, and MP profile lookups require extra MP API calls (`dp_Users` + `dp_User_Roles` + `dp_User_User_Groups`). **Decision:** `customSession` in `src/lib/auth.ts:398-413` does only `firstName` / `lastName` splitting from `user.name` — no API calls. MP profile loading moves to the client, behind `UserProvider` (`src/contexts/user-context.tsx`), which calls the parameterless `getCurrentUserProfile()` server action on mount (the action re-derives the GUID from the session — a GUID parameter would be an IDOR) and exposes `useUser()`. **Consequences:** `getSession()` stays cheap. Sign-in does not break when MP is down. Client components that need roles/groups must mount under `UserProvider`; every page load incurs one extra round-trip. `UserService.getUserProfile()` issues three queries (profile + roles + groups). +**Note (2026-10-10):** `MPUserProfile` is now display-only (`First_Name`, `Nickname`, `Last_Name`, `Email_Address`) and `getUserProfile()` issues **one** `dp_Users` query; roles and user groups are no longer sent to the client. Security-role checks use `AuthorizationService` server-side. See `services/user-service.md`. **Alternatives considered:** - **Enrich in `customSession`** — would hit MP API on every JWT refresh and couple sign-in availability to MP uptime. - **Load profile server-side in layout** — blocks TTFB on every page render. diff --git a/.claude/references/GLOSSARY.md b/.claude/references/GLOSSARY.md index 2994f15..093a7ba 100644 --- a/.claude/references/GLOSSARY.md +++ b/.claude/references/GLOSSARY.md @@ -1,7 +1,7 @@ --- title: Glossary type: glossary -last_updated: 2026-04-17 +last_updated: 2026-10-10 --- # Glossary @@ -238,7 +238,7 @@ Public-API facade class for Ministry Platform operations; normalizes friendly pa ## MPUserProfile -Enriched client-side user profile combining `dp_Users` row + joined Contact fields + `roles[]` + `userGroups[]`; loaded via `UserService.getUserProfile(userGuid)` and cached in `UserContext`. +Display-only client-side user profile: `First_Name`, `Nickname`, `Last_Name`, `Email_Address` (joined from the `dp_Users` row's Contact) — no IDs, phone, image, roles, or user groups; loaded via `getCurrentUserProfile()` → `UserService.getUserProfile(userGuid)` (single query) and cached in `UserContext`. **Not to be confused with:** Better Auth `session.user` (name/email/id only). **Defined in:** `src/lib/providers/ministry-platform/types/user-profile.types.ts:1` diff --git a/.claude/references/GOTCHAS.md b/.claude/references/GOTCHAS.md index cd97456..b3e8b81 100644 --- a/.claude/references/GOTCHAS.md +++ b/.claude/references/GOTCHAS.md @@ -422,7 +422,7 @@ Retired 2026-10-10: applied only to the template editor's merge tokens; the edit **Symptom:** User configures IMb with a Mailer ID, prints labels, and sees POSTNET (or no barcode) on the output. **Root cause:** `preEncodeBarcodes` wraps `imbEncode` in a bare `try {} catch {}` with no logging, falling through to POSTNET. Also, if `config.mailerId` is empty (`''`), the `config.barcodeFormat === 'imb' && config.mailerId` guard skips IMb entirely and falls through silently. **Fix:** (a) Users: enter a valid 6- or 9-digit USPS Mailer ID. (b) Code: fixed — the fallback is logged (`console.warn`) and the Mailer ID is validated. -**Enforced where:** `src/lib/barcode-helpers.ts` (logged fallback). +**Enforced where:** `src/lib/barcode-helpers.ts` (logged fallback). Log payload is `{ labelIndex, error: }` — no label name, address, or routing digits (rule 14) — so locate the row by its index in the request. **Related:** [utils/](utils/), [components/address-labels.md](components/address-labels.md) ### GOTCHA-041: Avery label cells misaligned after adding new stock diff --git a/.claude/references/components/address-labels.md b/.claude/references/components/address-labels.md index 1efaf8a..4a60f44 100644 --- a/.claude/references/components/address-labels.md +++ b/.claude/references/components/address-labels.md @@ -30,7 +30,9 @@ last_verified: 2026-04-17 ## Purpose Print USPS-compliant mailing labels (PDF + Word) from MP Contacts/Selection data, with IMb/POSTNET barcodes, and merge addresses into a user-uploaded `.docx` template (one page per address with embedded barcode image) for window-envelope mailings. -## Files (13) +## Files + +Per-component `*.test.tsx` files are co-located and not listed. | File | Role | |------|------| @@ -44,9 +46,13 @@ Print USPS-compliant mailing labels (PDF + Word) from MP Contacts/Selection data | `src/components/address-labels/mail-merge-tab.tsx` | Client mail-merge UI: sample download, template upload, merge + download | | `src/components/address-labels/sample-template.ts` | `'use server'` — `generateSampleTemplate()` produces the sample `.docx` with merge tokens | | `src/components/address-labels/word-document.ts` | `buildWordDocument()` — Avery-style `.docx` via `docx` package with BMP barcodes | -| `src/components/address-labels/actions.ts` | Server actions (see API below) | +| `src/components/address-labels/actions.ts` | Server actions (see API below); shared `prepareLabels` validation helper | +| `src/components/address-labels/limits.ts` | Shared size caps: `MAX_TEMPLATE_BYTES` (2 MB) + `MAX_TEMPLATE_LABEL` (`'2MB'`), `MAX_LABELS` (10,000), `TEMPLATE_ZIP_LIMITS` (`maxEntries: 1_000`, `maxTotalBytes: 64 MB`). Imported by both client (`mail-merge-tab.tsx`) and server (`actions.ts`, `schemas.ts`) | +| `src/components/address-labels/schemas.ts` | Zod schemas + `parseLabelInput(labels, config)` — server-side validation of client-sent labels/config | +| `src/lib/zip-guard.ts` | `assertZipWithinLimits` / `ZipLimitError` — zip-bomb pre-flight run before PizZip | | `src/components/address-labels/actions.test.ts` | Vitest suite covering all four server actions | | `src/components/address-labels/word-document.test.ts` | Vitest suite for `buildWordDocument` geometry paths | +| `src/components/address-labels/merge-template.integration.test.ts` | Real (unmocked) `mergeTemplate` + PizZip run, incl. a forged-size zip bomb | Tool route: `src/app/(web)/tools/addresslabels/page.tsx` → `address-labels.tsx` (orchestrates form, fetch, summary, tabs, PDF/Word download). @@ -118,24 +124,26 @@ export async function fetchAddressLabels( config: LabelConfig, ): Promise; +type ActionResult = { success: true; data: string } | { success: false; error: string }; + // Renders LabelDocument to a PDF via @react-pdf/renderer; returns base64 or { success: false, error }. export async function generateLabelPdf( - labels: LabelData[], - config: LabelConfig, -): Promise<{ success: true; data: string } | { success: false; error: string }>; + rawLabels: LabelData[], + rawConfig: LabelConfig, +): Promise; // Builds an Avery-layout .docx via `docx` + BMP barcodes; returns base64 or error. export async function generateLabelDocx( - labels: LabelData[], - config: LabelConfig, -): Promise<{ success: true; data: string } | { success: false; error: string }>; + rawLabels: LabelData[], + rawConfig: LabelConfig, +): Promise; -// Merges user-uploaded .docx template (base64, ≤5MB) with address rows + embedded barcode images. +// Merges user-uploaded .docx template (base64, ≤ MAX_TEMPLATE_BYTES = 2MB) with address rows + embedded barcode images. export async function mergeTemplate( templateBase64: string, - labels: LabelData[], - config: LabelConfig, -): Promise<{ success: true; data: string } | { success: false; error: string }>; + rawLabels: LabelData[], + rawConfig: LabelConfig, +): Promise; ``` Other exported helpers: @@ -163,8 +171,20 @@ const SERVICE_TYPES = [{id:'040',…}, {id:'300',…}, {id:'044',…}, {id:'700' - Single-record mode (`params.recordID !== -1`): `AddressLabelService.getAddressForContact(recordID)`. - `filterAndTransform()` (`actions.ts:42`) iterates rows, applies skip rules in fixed order (opted_out → no_address → no_postal_code → no_barcode), dedups by `Household_ID` in household mode, sorts by `postalCode` ascending. +### prepareLabels (shared input validation) +Called by `generateLabelPdf`, `generateLabelDocx`, `mergeTemplate` after `requireAccess('Contacts', 'read')`. Labels/config are caller-shaped input (they round-trip through the client after `fetchAddressLabels`), so every render action re-validates them. Returns `{ ok: true, labels, config, stock }` or `{ ok: false, result }` (the action returns `result` as-is). Order: +1. `parseLabelInput(rawLabels, rawConfig)` (`schemas.ts`): + - array length > `MAX_LABELS` → `Too many labels (N); the limit is 10,000 per request` (checked before Zod) + - `labelsSchema` — bounded strings (`name`/`addressLine1`/`addressLine2` ≤256, `city` ≤128, `state` ≤64, `postalCode` ≤32, `barCode` ≤64, `deliveryPointCode` ≤16); `barStates`/`barType` are **stripped** (Zod `z.object` drops unknown keys) — only `preEncodeBarcodes` may set them. Failure → `Invalid label data` + - `labelConfigSchema` — `stockId` ≤64, `addressMode`/`barcodeFormat` enums, `startPosition` int ≥1, `mailerId` ≤9, `serviceType` ∈ `SERVICE_TYPES` ids. Failure → `Invalid label settings` + - Errors name the problem, never the payload. +2. `getLabelStock(config.stockId)` → `Unknown label stock: ${stockId}`. +3. `config.startPosition > stock.columns * stock.rows` → `Start position is beyond the end of the sheet` (it is a count of blank cells to emit, so unbounded = unbounded pages). +4. Empty labels → per-action message (`No labels to print` / `No labels to export` / `No addresses to merge`). +5. IMb with a Mailer ID → `validateMailerId`; failure returns its message (fallback `Invalid Mailer ID`). + ### generateLabelPdf -- Calls `preEncodeBarcodes(labels, config)` then renders `` through `pdf()` from `@react-pdf/renderer` → `toBlob()` → base64. Unknown `stockId` or empty labels return `{ success: false }`. +- `prepareLabels(..., 'No labels to print')`, then `preEncodeBarcodes(labels, config)`, then renders `` through `pdf()` from `@react-pdf/renderer` → `toBlob()` → base64. ### LabelDocument layout - `skipCount = startPosition - 1`; `labelsPerPage = stock.columns * stock.rows`; `pageCount = ceil((skipCount + labels.length) / labelsPerPage)`. @@ -175,13 +195,15 @@ const SERVICE_TYPES = [{id:'040',…}, {id:'300',…}, {id:'044',…}, {id:'700' - Barcodes embedded via `ImageRun({ type: 'bmp' })` using `imbBarcodeToBmp` / `postnetBarcodeToBmp` from `src/lib/barcode-image.ts`. ### mergeTemplate (.docx mail merge) -- Rejects `templateBase64` if `Math.ceil(length * 0.75) > 5 * 1024 * 1024`. +- Size check first, on the encoded string, before any decoding: non-string or `Math.ceil(length * 0.75) > MAX_TEMPLATE_BYTES` → `Template file exceeds 2MB limit`. +- `prepareLabels(..., 'No addresses to merge')`. +- Zip-bomb guard: `assertZipWithinLimits(Buffer.from(templateBase64, 'base64'), TEMPLATE_ZIP_LIMITS)` (`src/lib/zip-guard.ts`) **before** PizZip sees the archive. Walks the central directory, caps entry count (1,000) and summed declared uncompressed size (64 MB), rejects ZIP64 / encrypted / non-STORE-DEFLATE entries, then inflates each entry with `maxOutputLength` = declared size so a lying header fails at that size. `ZipLimitError` message is returned; any other throw → `Not a valid .docx file`. See `../data-flow/error-catalog.md` for the full message list. - `preEncodeBarcodes()` then builds `addresses[]` rows with `{ Name, AddressLine1, AddressLine2, City, State, PostalCode, Barcode: 'barcode_N', isNotLast }` plus a `Map` of BMP barcodes keyed by `barcode_N`. - `Docxtemplater` with `docxtemplater-image` — `getImage` resolves key strings to buffers; `getSize` returns `[200, 25]` for the `Barcode` tag. - Render errors containing "tag" are wrapped as `Template error: …`. ### Mail merge tab upload flow -- Only `.docx`, `file.size <= 5MB`; base64-encoded client-side via `FileReader`. +- Only `.docx`, `file.size <= MAX_TEMPLATE_BYTES` (2 MB, from `limits.ts` — same constant the server enforces; error `Template file must be under 2MB`); base64-encoded client-side via `FileReader`. Label text shows `max {MAX_TEMPLATE_LABEL}`. - Re-validates 6/9-digit Mailer ID before calling `mergeTemplate`. - Downloads result blob as `merged-letters.docx`. @@ -231,8 +253,9 @@ Tool page wiring (from `src/app/(web)/tools/addresslabels/address-labels.tsx:180 - **Mailer ID is user-supplied** (not an env var). `LabelConfig.mailerId` persists in `localStorage`; both the PDF (`address-labels.tsx:98`) and `mergeTemplate` (`mail-merge-tab.tsx:111`) reject IMb generation unless the length is exactly 6 or 9. `preEncodeBarcodes` silently falls back from IMb → POSTNET when `mailerId` is empty (`src/lib/barcode-helpers.ts:41`). - **Label geometry is in PDF points** (72pt = 1in). `buildWordDocument` converts via `convertInchesToTwip(ptToIn(...))`. Adding a new `LABEL_STOCKS` entry in the wrong unit silently produces misaligned pages. -- **React-pdf type cast**: `pdf(doc as any)` in `actions.ts:156` is a runtime-safe cast because `LabelDocument` returns `` at its root; `@react-pdf/renderer` types don't permit a wrapper with its own props. -- **PizZip/docxtemplater expect strings, not Buffers**, as image tag values — `mergeTemplate` keys the buffer map with `barcode_${i}` strings; passing the buffer directly makes the image module misinterpret it as a cached result (`actions.ts:227`). +- **React-pdf type cast**: `pdf(doc as any)` in `generateLabelPdf` (`actions.ts`) is a runtime-safe cast because `LabelDocument` returns `` at its root; `@react-pdf/renderer` types don't permit a wrapper with its own props. +- **PizZip/docxtemplater expect strings, not Buffers**, as image tag values — `mergeTemplate` keys the buffer map with `barcode_${i}` strings; passing the buffer directly makes the image module misinterpret it as a cached result (`mergeTemplate` in `actions.ts`). +- **Size limits are coupled to the request-body ceiling.** `next.config.ts` sets `experimental.serverActions.bodySizeLimit: "4mb"` (Next default 1 MB made the template limit unreachable) to stay under Vercel's 4.5 MB function request cap. A 2 MB template is ~2.7 MB as base64, leaving ~1.3 MB for the label list on a merge. Change `limits.ts` and `next.config.ts` together. - **Selection mode requires MP `User_ID`** (not Better Auth `user.id`). `getMPUserId()` does a GUID→ID lookup per call — no memoization. ## Related docs diff --git a/.claude/references/components/user-menu.md b/.claude/references/components/user-menu.md index 51789a8..149234e 100644 --- a/.claude/references/components/user-menu.md +++ b/.claude/references/components/user-menu.md @@ -26,7 +26,7 @@ Dropdown menu that displays the signed-in MP user's name + email and triggers RP ## Key concepts - `UserMenu` is a **client component** (`"use client"`) — wraps a caller-supplied trigger via `{children}`. -- `userProfile` prop is typed `MPUserProfile` (`src/lib/providers/ministry-platform/types/user-profile.types.ts:1-13`) — expects the enriched MP profile, typically supplied by `UserProvider` / `useUser()`. +- `userProfile` prop is typed `MPUserProfile` (`src/lib/providers/ministry-platform/types/user-profile.types.ts`) — display-only DTO (`First_Name`, `Nickname`, `Last_Name`, `Email_Address`), typically supplied by `UserProvider` / `useUser()`. Every field it carries is rendered here. - Display name uses `Nickname || First_Name`, then `Last_Name` (`user-menu.tsx:49-50`). - Sign-out is the only menu item currently (`userMenuItems` array, `user-menu.tsx:21-27`). Icon is `@heroicons/react/24/outline`'s `ArrowRightOnRectangleIcon`. - `onClose?` optional callback fires before the action — used by callers that render the trigger inside a larger popover/sidebar to close the outer surface. diff --git a/.claude/references/contexts/user-provider.md b/.claude/references/contexts/user-provider.md index 204692d..ed61eda 100644 --- a/.claude/references/contexts/user-provider.md +++ b/.claude/references/contexts/user-provider.md @@ -9,14 +9,14 @@ last_verified: 2026-09-13 --- ## Purpose -Client context that watches the Better Auth session, extracts `userGuid` (OIDC `sub` → MP `User_GUID`), and loads the enriched `MPUserProfile` (roles + groups) via the `getCurrentUserProfile` server action. `userGuid` gates *whether* the load fires; the action re-derives it server-side and takes no argument. +Client context that watches the Better Auth session, extracts `userGuid` (OIDC `sub` → MP `User_GUID`), and loads the display-only `MPUserProfile` (names + email; no IDs, roles, or groups) via the `getCurrentUserProfile` server action. `userGuid` gates *whether* the load fires; the action re-derives it server-side and takes no argument. ## Files - `src/contexts/user-context.tsx` — provider + hook + error handling - `src/contexts/user-context.test.tsx` — 6 test cases covering load/error/refresh/missing-guid - `src/app/providers.tsx` — mounts `` at the app shell - `src/components/shared-actions/user.ts` — `getCurrentUserProfile()` server action (no parameters; derives the GUID from the session) -- `src/services/userService.ts:81` — `UserService.getUserProfile(guid)` downstream lookup +- `src/services/userService.ts` — `UserService.getUserProfile(guid)` downstream lookup - `src/lib/providers/ministry-platform/types/user-profile.types.ts` — `MPUserProfile` shape ## Key concepts @@ -44,17 +44,10 @@ export function useUser(): UserContextValue; ```typescript // src/lib/providers/ministry-platform/types/user-profile.types.ts export interface MPUserProfile { - User_ID: number; - User_GUID: string; - Contact_ID: number; First_Name: string; Nickname: string; Last_Name: string; Email_Address: string | null; - Mobile_Phone: string | null; - Image_GUID: string | null; - roles: string[]; - userGroups: string[]; } ``` @@ -115,10 +108,8 @@ UserProvider mount └─ getCurrentUserProfile() (server action: src/components/shared-actions/user.ts:25) └─ auth.api.getSession({ headers }) (re-verifies session; throws Unauthorized unless user.userGuid is a non-empty string) └─ UserService.getInstance() - └─ UserService.getUserProfile(userGuid) (src/services/userService.ts:81 — GUID from the session, never the caller) - ├─ MP getTableRecords dp_Users (filter: User_GUID = '...') - ├─ MP getTableRecords dp_User_Roles (filter: User_ID = ...) - └─ MP getTableRecords dp_User_User_Groups (filter: User_ID = ...) + └─ UserService.getUserProfile(userGuid) (src/services/userService.ts — GUID from the session, never the caller) + └─ MP getTableRecords dp_Users (filter: User_GUID = '...'; single query, 4 Contact_ID_TABLE fields) └─ setUserProfile | setError └─ setIsLoading(false) ``` diff --git a/.claude/references/data-flow/call-graphs.md b/.claude/references/data-flow/call-graphs.md index 630f3ea..8a17b28 100644 --- a/.claude/references/data-flow/call-graphs.md +++ b/.claude/references/data-flow/call-graphs.md @@ -164,26 +164,24 @@ last_verified: 2026-04-17 6. `src/components/shared-actions/user.ts:25` — server action `getCurrentUserProfile()`; takes no parameters, so a caller cannot name another user (IDOR). 7. `src/components/shared-actions/user.ts:26-28` — `auth.api.getSession({ headers: await headers() })`, then derive `userGuid` from `session.user`; throws `'Unauthorized'` unless it is a non-empty string. 8. `src/components/shared-actions/user.ts:30-31` — `UserService.getInstance()` → `userService.getUserProfile(userGuid)`. -9. `src/services/userService.ts:81-110` — runs 3 MP queries: - - `mp.getTableRecords('dp_Users', { filter: "User_GUID = ''", select: "User_ID, User_GUID, Contact_ID_TABLE.First_Name, ..., Contact_ID_TABLE.dp_fileUniqueId AS Image_GUID", top: 1 })` at lines 86-91. - - `Promise.all([dp_User_Roles fetch, dp_User_User_Groups fetch])` at lines 92-103, keyed by the numeric `User_ID` from the first query. -10. `src/services/userService.ts:105-109` — returns `{ ...profile, roles: string[], userGroups: string[] }`. +9. `src/services/userService.ts` `getUserProfile` — runs 1 MP query: `mp.getTableRecords('dp_Users', { filter: "User_GUID = ''", select: "Contact_ID_TABLE.First_Name,Contact_ID_TABLE.Nickname,Contact_ID_TABLE.Last_Name,Contact_ID_TABLE.Email_Address", top: 1 })`. +10. Same method — maps the row explicitly to `{ First_Name, Nickname, Last_Name, Email_Address: ?? null }` (no spread, so nothing else MP returns reaches the client). 11. `src/contexts/user-context.tsx:42` — `setUserProfile(profile ?? null)`. **Side effects:** -- MP API calls: 1 + 2 (parallel) `GET /tables/...` per mount (first run per fresh process triggers token fetch). +- MP API calls: 1 `GET /tables/dp_Users` per mount (first run per fresh process triggers token fetch). - No cookie changes (session read from JWT cache). **Error paths:** - Server action throws → caught at `src/contexts/user-context.tsx:43-46` → sets `error` state, `userProfile = null`. - Session `userGuid` missing or empty → `Error('Unauthorized')` at `src/components/shared-actions/user.ts:28`. (Not keyed on `session.user.id` — that is Better Auth's internal ID and does not prove an MP identity.) -- Profile row missing → `userProfile = undefined` returned at `src/services/userService.ts:90`; normalized to `null` in context (line 42). +- Profile row missing → `userProfile = undefined` returned by `getUserProfile` (`if (!row) return undefined`); normalized to `null` in context (line 42). -**Return shape:** `MPUserProfile | null` in context state (with `roles: string[]`, `userGroups: string[]`). +**Return shape:** `MPUserProfile | null` in context state (`First_Name`, `Nickname`, `Last_Name`, `Email_Address` only). **Related:** - `../contexts/user-provider.md` — hook usage, lifecycle -- `../services/user-service.md` — `getUserProfile` and `getUserIdByGuid` contracts +- `../services/user-service.md` — `getUserProfile` contract - `../auth/sessions.md` — why `userGuid` (additionalField) is needed instead of `user.id` --- diff --git a/.claude/references/data-flow/error-catalog.md b/.claude/references/data-flow/error-catalog.md index c517a10..d42b817 100644 --- a/.claude/references/data-flow/error-catalog.md +++ b/.claude/references/data-flow/error-catalog.md @@ -85,12 +85,18 @@ These do NOT throw to the component; server action returns `{ success: false, er | Type | Thrown at (internal) | Caught at | User-facing | Logged | |---|---|---|---|---| -| `Unknown label stock: ${id}` (control-flow branch, not throw) | check at `src/components/address-labels/actions.ts:136,178` | same function returns `{success:false}` at `:137,:179` | mail-merge / PDF tab `setError` | no | -| `'No labels to print' / 'No labels to export' / 'No addresses to merge'` | `src/components/address-labels/actions.ts:141,183,213` (returns envelope) | tab components `setError` | inline | no | -| `'Template file exceeds 5MB limit'` | `src/components/address-labels/actions.ts:219` (returns envelope) | `mail-merge-tab.tsx:123` `setError` | inline | no | -| PDF render failure (`@react-pdf/renderer` `toBlob`) | throws inside try at `src/components/address-labels/actions.ts:157` | `:162-168` wraps as `{success:false, error: message}` | inline error | yes, `console.error('generateLabelPdf error:', error)` | -| Docx render failure (`Packer.toBuffer`) | throws inside try at `src/components/address-labels/actions.ts:190` | `:194-200` wraps as envelope | inline error | yes, `console.error('generateLabelDocx error:', error)` | -| Docxtemplater render error (tag mismatch) | throws inside `doc.render` at `src/components/address-labels/actions.ts:273` | `:279-286` wraps; if message includes `'tag'` returns prettier error | inline error | yes, `console.error('mergeTemplate error:', error)` | +| `'Too many labels (N); the limit is 10,000 per request'` / `'Invalid label data'` / `'Invalid label settings'` | `parseLabelInput` in `src/components/address-labels/schemas.ts` (Zod `safeParse`; count cap = `MAX_LABELS`), called from `prepareLabels` in `actions.ts` | `generateLabelPdf` / `generateLabelDocx` / `mergeTemplate` return the envelope | tab components `setError` | no (errors name the problem, never the payload) | +| `Unknown label stock: ${id}` (control-flow branch, not throw) | `prepareLabels` in `src/components/address-labels/actions.ts` | all three render actions return `{success:false}` | mail-merge / PDF tab `setError` | no | +| `'Start position is beyond the end of the sheet'` | `prepareLabels` — `config.startPosition > stock.columns * stock.rows` | all three render actions | inline | no | +| `'No labels to print' / 'No labels to export' / 'No addresses to merge'` | `prepareLabels` (`emptyMessage` arg, per action) | tab components `setError` | inline | no | +| Mailer ID validation message (`validateMailerId`) / fallback `'Invalid Mailer ID'` | `prepareLabels`, when `barcodeFormat === 'imb' && mailerId` | all three render actions | inline | no | +| `'Template file exceeds 2MB limit'` (`MAX_TEMPLATE_LABEL`) | `mergeTemplate` in `src/components/address-labels/actions.ts` — base64 length × 0.75 > `MAX_TEMPLATE_BYTES`, checked before decoding or `prepareLabels` | `mail-merge-tab.tsx` `handleMerge` → `setError(result.error)` | inline | no | +| `'Template file must be under 2MB'` (client pre-check) | `src/components/address-labels/mail-merge-tab.tsx` `handleFileChange`, `file.size > MAX_TEMPLATE_BYTES` | same function → `setError` early return | inline | no | +| `ZipLimitError`: `'Not a valid .docx file'` / `'ZIP64 templates are not supported'` / `'Template contains too many files'` / `'Encrypted templates are not supported'` / `'Template uses an unsupported compression method'` / `'Template is too large when uncompressed'` | `assertZipWithinLimits` in `src/lib/zip-guard.ts` (limits = `TEMPLATE_ZIP_LIMITS`), run by `mergeTemplate` before PizZip | `mergeTemplate` try/catch → envelope with `error.message`; any non-`ZipLimitError` → `'Not a valid .docx file'` | inline | no | +| Server-action body over `4mb` | Next.js (`experimental.serverActions.bodySizeLimit` in `next.config.ts`) rejects the request before the action body runs | client `await` rejects → `mail-merge-tab.tsx` `handleMerge` catch → `setError(err.message)` (fallback `'Merge failed'`) | inline | framework-side only | +| PDF render failure (`@react-pdf/renderer` `toBlob`) | throws inside try in `generateLabelPdf` (`src/components/address-labels/actions.ts`) | same function wraps as `{success:false, error: message}` | inline error | yes, `console.error('generateLabelPdf error:', describeError(error))` | +| Docx render failure (`Packer.toBuffer`) | throws inside try in `generateLabelDocx` | same function wraps as envelope | inline error | yes, `console.error('generateLabelDocx error:', describeError(error))` | +| Docxtemplater render error (tag mismatch) | throws inside `doc.render` in `mergeTemplate` | same function wraps; if message includes `'tag'` returns `Template error: ... Check that merge tokens are correctly formatted.` | inline error | yes, `console.error('mergeTemplate error:', describeError(error))` | | Profile fetch failure in `UserProvider` | `src/components/shared-actions/user.ts:28` (`Unauthorized` — session has no `userGuid`) or downstream MP error from `UserService.getUserProfile` | `src/contexts/user-context.tsx:43-46` try/catch sets `error` state | silent (consumers of `useUser()` inspect `error`) | no | | `Error("useUser must be used within a UserProvider")` | `src/contexts/user-context.tsx:79` | uncaught — surfaces to nearest React error boundary | white-screen error unless a boundary is present | no | | `Error("useFormField should be used within ")` | `src/components/ui/form.tsx:53` | uncaught — React error boundary | same as above | no | diff --git a/.claude/references/dto-constants/constants.md b/.claude/references/dto-constants/constants.md index 62274bc..e866ff9 100644 --- a/.claude/references/dto-constants/constants.md +++ b/.claude/references/dto-constants/constants.md @@ -2,8 +2,8 @@ title: Shared Constants & Validation Helpers domain: dto-constants type: reference -applies_to: [src/lib/dto/address-label.dto.ts, src/lib/label-stock.ts, src/lib/validation.ts, src/components/group-wizard/types.ts, src/services/addressLabelService.ts, src/services/toolService.ts] -symbols: [SERVICE_TYPES, LABEL_STOCKS, LabelStockConfig, LabelPosition, WIZARD_STEPS, WizardStepIndex, validateGuid, validatePositiveInt, validateColumnName, escapeFilterString, BATCH_SIZE, MAX_TEMPLATE_SIZE, MAX_FILE_SIZE] +applies_to: [src/lib/dto/address-label.dto.ts, src/lib/label-stock.ts, src/lib/validation.ts, src/components/group-wizard/types.ts, src/services/addressLabelService.ts, src/services/toolService.ts, src/components/address-labels/limits.ts] +symbols: [SERVICE_TYPES, LABEL_STOCKS, LabelStockConfig, LabelPosition, WIZARD_STEPS, WizardStepIndex, validateGuid, validatePositiveInt, validateColumnName, escapeFilterString, BATCH_SIZE, MAX_TEMPLATE_BYTES, MAX_TEMPLATE_LABEL, MAX_LABELS, TEMPLATE_ZIP_LIMITS] related: [dtos.md, ../services/README.md, ../utils/README.md] last_verified: 2026-04-17 --- @@ -121,8 +121,10 @@ Items found in `src/` outside `src/lib/dto/` and `src/lib/validation.ts`: | Literal | Location | Meaning | Extraction candidate? | |---------|----------|---------|-----------------------| | `BATCH_SIZE = 100` | `src/services/addressLabelService.ts:21` (module) + `src/services/toolService.ts:283` (class static) | MP record fetch batch size | Yes — two independent copies, merge into single export (e.g. `src/lib/constants.ts`) | -| `MAX_TEMPLATE_SIZE = 5 * 1024 * 1024` | `src/components/address-labels/actions.ts:203` | 5MB docx upload cap | Possibly | -| `MAX_FILE_SIZE = 5 * 1024 * 1024` | `src/components/address-labels/mail-merge-tab.tsx:13` | 5MB docx upload cap (client mirror) | Yes — duplicated literal | +| `MAX_TEMPLATE_BYTES = 2 * 1024 * 1024` / `MAX_TEMPLATE_LABEL = '2MB'` | `src/components/address-labels/limits.ts` | Mail-merge `.docx` template cap; one export used by client `mail-merge-tab.tsx` and server `actions.ts` (`mergeTemplate`) | Done — single shared constant | +| `MAX_LABELS = 10_000` | `src/components/address-labels/limits.ts` | Labels per render request (`parseLabelInput` in `schemas.ts`) | Done | +| `TEMPLATE_ZIP_LIMITS = { maxEntries: 1_000, maxTotalBytes: 64 MB }` | `src/components/address-labels/limits.ts` | Zip-bomb bounds passed to `assertZipWithinLimits` (`src/lib/zip-guard.ts`) | Done | +| `bodySizeLimit: "4mb"` | `next.config.ts` (`experimental.serverActions`) | Server-action request cap; sized with `limits.ts` under Vercel's 4.5 MB body cap | Coupled config — change with `limits.ts` | | `'040'` default | `src/app/(web)/tools/addresslabels/address-labels.tsx:53` | Default USPS service type | Consider `DEFAULT_SERVICE_TYPE = SERVICE_TYPES[0].id` | | `serialLength = mailerId.length === 6 ? 9 : 6` | `src/lib/barcode-helpers.ts:23` | USPS IMb rule: 6-digit MID → 9-digit serial, 9-digit MID → 6-digit serial | Domain rule — documented in comment, OK | | `barcodeId = '00'` | `src/lib/barcode-helpers.ts:22` | IMb header byte | OK (domain constant, inline) | @@ -144,10 +146,10 @@ filter: `Group_ID = ${validatePositiveInt(groupId)}` ``` ```typescript -// src/services/userService.ts:70-85 -return this.mp!.getTableRecords({ - table: 'dp_Users', - filter: `User_GUID = '${validateGuid(guid)}'`, +// src/services/userService.ts — UserService.getUserProfile +const records = await this.mp!.getTableRecords({ + table: "dp_Users", + filter: `User_GUID = '${validateGuid(id)}'`, // ... }); ``` @@ -155,7 +157,7 @@ return this.mp!.getTableRecords({ ## Gotchas - **Two `BATCH_SIZE = 100` constants.** Module-local in `addressLabelService.ts:21`; class-static in `toolService.ts:283`. Drift risk if MP API limit changes. See TODO `2026-04-17-dto-constants-batchsize-duplication.md`. -- **`MAX_TEMPLATE_SIZE` and `MAX_FILE_SIZE` are both 5MB** and mirror each other across server (`actions.ts:203`) and client (`mail-merge-tab.tsx:13`). Drift risk. +- **Template size cap is one constant now.** The former duplicated 5 MB literals (`MAX_TEMPLATE_SIZE` in `actions.ts`, `MAX_FILE_SIZE` in `mail-merge-tab.tsx`) were replaced by `MAX_TEMPLATE_BYTES` (2 MB) in `src/components/address-labels/limits.ts`. Resolved. Raising it means re-checking `next.config.ts` `bodySizeLimit` and Vercel's 4.5 MB cap together. - **`escapeFilterString` escapes `%` + `_` + `'`** — not generic SQL escaping, specific to MP OData/LIKE. Do NOT use it as a universal SQL sanitizer. - **`validatePositiveInt` rejects 0** (strictly positive). If you need non-negative int accept `>= 0`, write a new helper. - **`validateGuid` accepts upper- or lower-case hex** but NOT braces (no `{xxx-xxx}` form). Confirmed by `src/lib/validation.test.ts:11-13`. diff --git a/.claude/references/mp-provider/services/table.md b/.claude/references/mp-provider/services/table.md index 3e0a88b..44ce62d 100644 --- a/.claude/references/mp-provider/services/table.md +++ b/.claude/references/mp-provider/services/table.md @@ -99,7 +99,7 @@ const result = await this.mp!.updateTableRecords('Groups', [apiData], { $userId: userId, }); -// src/services/userService.ts:82 — filtered read +// src/services/userService.ts (getUserProfile) — filtered read const records = await this.mp!.getTableRecords({ ... }); ``` diff --git a/.claude/references/mp-provider/type-generation.md b/.claude/references/mp-provider/type-generation.md index 42f0353..74c147d 100644 --- a/.claude/references/mp-provider/type-generation.md +++ b/.claude/references/mp-provider/type-generation.md @@ -4,9 +4,13 @@ domain: mp-provider type: reference applies_to: - src/lib/providers/ministry-platform/scripts/generate-types.ts + - src/lib/providers/ministry-platform/scripts/codegen-escape.ts - src/lib/providers/ministry-platform/models/ symbols: - generate-types + - commentText + - propertyKey + - positiveInt related: - architecture.md - ../mp-schema/README.md @@ -18,6 +22,7 @@ last_verified: 2026-04-17 ## Files - `src/lib/providers/ministry-platform/scripts/generate-types.ts` — CLI +- `src/lib/providers/ministry-platform/scripts/codegen-escape.ts` — escaping helpers for MP metadata interpolated into generated source (+ `codegen-escape.test.ts`) - `src/lib/providers/ministry-platform/scripts/README.md` — human-facing usage - `src/lib/providers/ministry-platform/models/` — output: 603 `.ts` files (verified by snapshot: `.claude/references/_meta/facts/2026-04-17.md`) - `.claude/references/ministryplatform.schema.md` — schema doc written by the same script @@ -29,7 +34,11 @@ last_verified: 2026-04-17 - `npm run mp:generate:models` → `tsx .../generate-types.ts -o src/lib/providers/ministry-platform/models --zod --clean` (canonical) - **Data source:** MP `/tables` metadata endpoint via `MPHelper.getTables(search?)`, which hits `MetadataService` - **Column-driven typing is the default path;** `--detailed` falls back to record-sampling only if a table has no column metadata -- **Field-name safety:** non-identifier field names are quoted (e.g. `"Allow_Check-in"`, `"SSN/EIN"`) — see `generate-types.ts:163-166` +- **Field-name safety:** non-identifier field names are emitted as JSON string literals (e.g. `"Allow_Check-in"`, `"SSN/EIN"`) — `formatFieldName` → `propertyKey` (`codegen-escape.ts`), also applied to the `--detailed` fallback `${name}_ID` key +- **All MP metadata is escaped before interpolation** (the output is committed and compiled, so a hostile/odd value must not become code). One helper per context in `codegen-escape.ts`: + - `propertyKey(name)` — bare if `/^[a-zA-Z_$][a-zA-Z0-9_$]*$/`, else `JSON.stringify(name)` + - `commentText(value)` — line terminators (CR, LF, U+2028, U+2029) → space, `*/` → `*\/`; used for FK target table/column, `SpecialPermissions`, `AccessLevel`, and the table name in the JSDoc header + - `positiveInt(value)` — positive safe integer (number or digit-only string) else `undefined`; gates every `Size` interpolation (`/* max N chars */`, `Max length: N`, Zod `.max(N)`) — a non-numeric size drops the constraint instead of emitting it - **Environment load order:** `.env.local` → `.env.development` → `.env` (each loaded via `dotenv.config`, later files don't override earlier ones because `dotenv` skips already-set keys — so `.env.local` wins) — `generate-types.ts:9-20` ## API / Interface @@ -68,11 +77,11 @@ If missing, prints each missing var and `process.exit(1)`. ### TypeScript type mapping -From `mapDataTypeToTypeScript` (`generate-types.ts:168-222`): +From `mapDataTypeToTypeScript` (`generate-types.ts`): | MP `ParameterDataType` | TS (required) | Notes | |---|---|---| -| `String`/`Text`/`LargeString` | `string /* max N chars */` | size comment if present | +| `String`/`Text`/`LargeString` | `string /* max N chars */` | size comment only if `positiveInt(Size)` | | `Email` | `string /* email */` | | | `Phone` | `string /* phone number */` | | | `Url` | `string /* URL */` | | @@ -92,11 +101,11 @@ From `mapDataTypeToTypeScript` (`generate-types.ts:168-222`): Non-required fields append ` | null` and use `?:` instead of `:`. -Annotations emitted as inline comments (`generate-types.ts:359-365`): `Primary Key`, `Foreign Key -> {ReferencedTable}.{ReferencedColumn}`, `Read Only`, `Computed`, `Has Default`. +Annotations emitted as inline comments (`generateDetailedTypeDefinition` in `generate-types.ts`; FK names pass through `commentText`): `Primary Key`, `Foreign Key -> {ReferencedTable}.{ReferencedColumn}`, `Read Only`, `Computed`, `Has Default`. ### Zod type mapping -From `mapDataTypeToZod` (`generate-types.ts:224-287`): +From `mapDataTypeToZod` (`generate-types.ts`; `N` = `positiveInt(col.Size)`, `.max` omitted when undefined): | MP type | Zod | |---|---| diff --git a/.claude/references/mp-schema/data-model-map.md b/.claude/references/mp-schema/data-model-map.md index a200c49..cbbd604 100644 --- a/.claude/references/mp-schema/data-model-map.md +++ b/.claude/references/mp-schema/data-model-map.md @@ -19,9 +19,8 @@ Call site = one invocation of `getTableRecords`/`createTableRecords`/`updateTabl |---|---|---| | `Groups` | 4 | `src/services/groupService.ts:164` (searchGroups), `:174` (getGroup), `:232` (createGroup), `:245` (updateGroup) | | `Contacts` | 3 | `src/services/groupService.ts:153` (searchContacts), `src/services/addressLabelService.ts:79` (getAddressesForContacts), `:96` (getAddressForContact) | -| `dp_Users` | 2 | `src/services/userService.ts:69` (getUserIdByGuid), `:82` (getUserProfile) | -| `dp_User_Roles` | 1 | `src/services/userService.ts:93` (roles in getUserProfile) | -| `dp_User_User_Groups` | 1 | `src/services/userService.ts:98` (user groups in getUserProfile) | +| `dp_Users` | 2 | `src/services/authorizationService.ts` (`resolveUserId`), `src/services/userService.ts` (`getUserProfile`) | +| `dp_User_Roles` | 1 | `src/services/authorizationService.ts` (`loadSecurityRoles`) | | `dp_Roles` | 1 | `src/services/toolService.ts:232` (listRoles) | | `Group_Types` | 1 | `src/services/groupService.ts:63` (fetchAllLookups) | | `Ministries` | 1 | `src/services/groupService.ts:68` | @@ -38,7 +37,7 @@ Call site = one invocation of `getTableRecords`/`createTableRecords`/`updateTabl | `Group_Ended_Reasons` | 1 | `src/services/groupService.ts:127` | | `` | 1 | `src/services/toolService.ts:335` (resolveContactIds — table name supplied at runtime from `api_Tools_GetPageData` result) | -Unique tables touched (excluding the dynamic case): **19**. Lookup tables at `groupService.fetchAllLookups` dominate the count but each fires once per wizard load. +Unique tables touched (excluding the dynamic case): **18**. Lookup tables at `groupService.fetchAllLookups` dominate the count but each fires once per wizard load. ## FK traversals used @@ -46,14 +45,11 @@ Scanned from `$select`/`orderBy` strings using the `_TABLE` suffix convention. | FK chain (in `$select`/`orderBy`) | Base table | Consumer (file:line) | Purpose | |---|---|---|---| -| `Contact_ID_TABLE.First_Name` | `dp_Users` | `src/services/userService.ts:85` | User profile name (via linked Contact) | -| `Contact_ID_TABLE.Nickname` | `dp_Users` | `src/services/userService.ts:85` | Preferred display name | -| `Contact_ID_TABLE.Last_Name` | `dp_Users` | `src/services/userService.ts:85` | User profile surname | -| `Contact_ID_TABLE.Email_Address` | `dp_Users` | `src/services/userService.ts:85` | User profile email | -| `Contact_ID_TABLE.Mobile_Phone` | `dp_Users` | `src/services/userService.ts:85` | User profile phone | -| `Contact_ID_TABLE.dp_fileUniqueId AS Image_GUID` | `dp_Users` | `src/services/userService.ts:85` | User avatar image GUID | -| `Role_ID_TABLE.Role_Name` | `dp_User_Roles` | `src/services/userService.ts:96` | Role labels for the authenticated user | -| `User_Group_ID_TABLE.User_Group_Name` | `dp_User_User_Groups` | `src/services/userService.ts:101` | User-group labels for the authenticated user | +| `Contact_ID_TABLE.First_Name` | `dp_Users` | `src/services/userService.ts` (`getUserProfile`) | User profile name (via linked Contact) | +| `Contact_ID_TABLE.Nickname` | `dp_Users` | `src/services/userService.ts` (`getUserProfile`) | Preferred display name | +| `Contact_ID_TABLE.Last_Name` | `dp_Users` | `src/services/userService.ts` (`getUserProfile`) | User profile surname | +| `Contact_ID_TABLE.Email_Address` | `dp_Users` | `src/services/userService.ts` (`getUserProfile`) | User profile email | +| `Role_ID_TABLE.Role_Name` | `dp_User_Roles` | `src/services/authorizationService.ts` (`loadSecurityRoles`) | Security-role names for the authorization gate | | `Group_Type_ID_TABLE.Group_Type` | `Groups` | `src/services/groupService.ts:166` | Group search result badge | | `Household_ID_TABLE.Household_Name` | `Contacts` | `src/services/addressLabelService.ts:29` | Household label on mailing labels | | `Household_ID_TABLE.Bulk_Mail_Opt_Out` | `Contacts` | `src/services/addressLabelService.ts:30` | Opt-out filter for bulk mailings | @@ -88,7 +84,7 @@ Cross-referenced with `required-procs.md`. All call via `MPHelper.executeProcedu - `Bookable = 1` — `Rooms` filter. `src/services/groupService.ts:113`. - ` LIKE '%'` — prefix search (user-provided term is passed through `escapeFilterString`). `src/services/groupService.ts:156` (contacts), `:167` (groups); `src/services/toolService.ts:229` uses `%term%` for roles. - ` IN ()` — batched ID lookups (batch size 100). `src/services/addressLabelService.ts:82`, `src/services/toolService.ts:338`. -- `User_GUID = ''` — GUID lookup on `dp_Users` (passed through `validateGuid`). `src/services/userService.ts:72`, `:84`. +- `User_GUID = ''` — GUID lookup on `dp_Users` (passed through `validateGuid`). `src/services/userService.ts` (`getUserProfile`), `src/services/authorizationService.ts` (`resolveUserId`). - ` = ` — single-record lookup (input through `validatePositiveInt`). `src/services/groupService.ts:176`. All user-supplied strings in filters are escaped via `escapeFilterString` or validated via `validateGuid`/`validatePositiveInt`/`validateColumnName` from `src/lib/validation.ts`. diff --git a/.claude/references/security/README.md b/.claude/references/security/README.md index 379d350..ecc1e0e 100644 --- a/.claude/references/security/README.md +++ b/.claude/references/security/README.md @@ -434,7 +434,7 @@ segment, so one boundary cannot do every job: |---|---|---| | `src/app/(web)/error.tsx` | anything below the `(web)` layout | renders **inside** the shell, so the header and **sign-out survive** | | `src/app/error.tsx` | `/signin`, `/session-error`, `/auth-error` | those routes have no shell | -| `src/app/global-error.tsx` | a throw in the root layout itself | replaces it; imports nothing from the app | +| `src/app/global-error.tsx` | a throw in the root layout itself | replaces it; imports nothing from the app. Has a plain `` that works without JS, and "Try again" is disabled until hydration (`useSyncExternalStore`) so a pre-hydration click is not swallowed | Two details that bite: diff --git a/.claude/references/services/README.md b/.claude/references/services/README.md index 5139e85..29d45c9 100644 --- a/.claude/references/services/README.md +++ b/.claude/references/services/README.md @@ -12,7 +12,7 @@ Application service layer — singletons that wrap `MPHelper` and expose domain |------|---------|--------------| | `query-patterns.md` | MP REST query rules: `_TABLE_` FK traversal, ambiguous-column prefixing, quote escaping, `$userId` audit, batching | Before writing/modifying any MP query in a service | | `tool-service.md` | `ToolService` — pages, user tools, selection resolution, contact-ID resolver, dev-tool deploy | Editing `src/services/toolService.ts` or its callers | -| `user-service.md` | `UserService` — profile fetch with roles + groups, GUID→ID lookup | Editing user profile/session code | +| `user-service.md` | `UserService` — display-only profile fetch (names + email) | Editing user profile/session code | | `address-label-service.md` | `AddressLabelService` — Contacts→Households→Addresses multi-level FK join | Editing address-label feature | | `group-service.md` | `GroupService` — lookup fetching, contact/group search, group CRUD with date coercion | Editing group-wizard feature | | `field-management-service.md` | `FieldManagementService` — page field editor backing store, `api_MPNextTools_*` SPs | Editing field-management feature | diff --git a/.claude/references/services/query-patterns.md b/.claude/references/services/query-patterns.md index 1196a26..eac666d 100644 --- a/.claude/references/services/query-patterns.md +++ b/.claude/references/services/query-patterns.md @@ -30,7 +30,7 @@ Conventions for building safe MP REST queries inside service methods — FK trav | `TableName.ColumnName` | Disambiguate a column that appears in multiple joined tables | `Contacts.Contact_ID` | | `FKColumn_TABLE.ColumnName` | Follow one FK → pull a column from the related table | `Contact_ID_TABLE.First_Name` | | `FK1_TABLE_FK2_TABLE.ColumnName` | Multi-level FK traversal (underscores between levels, dot only before final field) | `Household_ID_TABLE_Address_ID_TABLE.City` | -| `FKColumn_TABLE.Column AS Alias` | FK traversal with alias | `Contact_ID_TABLE.dp_fileUniqueId AS Image_GUID` | +| `FKColumn_TABLE.Column AS Alias` | FK traversal with alias | `Parent_Group_TABLE.Group_Name AS Parent_Group_Name` | | `[Column/With-Special]` | Bracket notation for columns with `/`, `-`, space | `Household_ID_TABLE_Address_ID_TABLE.[State/Region]` | **Invalid:** `Building_ID_TABLE.Location_ID_TABLE.Column` (dots between intermediate FK levels). Use underscores: `Building_ID_TABLE_Location_ID_TABLE.Column`. @@ -40,7 +40,7 @@ Conventions for building safe MP REST queries inside service methods — FK trav | Column | Appears in | Resolution | |--------|-----------|------------| | `Contact_ID` | Contacts, dp_Users, Participants, Group_Participants, Household_Care_Log, many more | Prefix with table, e.g., `Contacts.Contact_ID` | -| `User_ID` | dp_Users, dp_User_Roles, dp_User_User_Groups | Prefix when joining, e.g., `dp_Users.User_ID` | +| `User_ID` | dp_Users, dp_User_Roles | Prefix when joining, e.g., `dp_Users.User_ID` | | `Household_ID` | Contacts, Households | Prefix, e.g., `Contacts.Household_ID` | | `Display_Name` | Contacts, Groups, Ministries, etc. | Prefix when joining creates shadow | @@ -48,11 +48,11 @@ Conventions for building safe MP REST queries inside service methods — FK trav ### 1-level FK traversal — `UserService.getUserProfile` ```typescript -// src/services/userService.ts:81-87 +// src/services/userService.ts — UserService.getUserProfile const records = await this.mp!.getTableRecords({ table: "dp_Users", filter: `User_GUID = '${validateGuid(id)}'`, - select: "User_ID, User_GUID, Contact_ID_TABLE.First_Name,Contact_ID_TABLE.Nickname,Contact_ID_TABLE.Last_Name,Contact_ID_TABLE.Email_Address,Contact_ID_TABLE.Mobile_Phone,Contact_ID_TABLE.dp_fileUniqueId AS Image_GUID", + select: "Contact_ID_TABLE.First_Name,Contact_ID_TABLE.Nickname,Contact_ID_TABLE.Last_Name,Contact_ID_TABLE.Email_Address", top: 1 }); ``` @@ -78,19 +78,14 @@ const SELECT_FIELDS = [ ].join(', '); ``` -### FK traversal in `$select` (role names) — `UserService` +### FK traversal in `$select` (role names) — `AuthorizationService` ```typescript -// src/services/userService.ts:93-102 -this.mp!.getTableRecords<{ Role_Name: string }>({ +// src/services/authorizationService.ts — loadSecurityRoles +const records = await mp.getTableRecords<{ Role_Name: string }>({ table: "dp_User_Roles", - filter: `User_ID = ${profile.User_ID}`, select: "Role_ID_TABLE.Role_Name", -}), -this.mp!.getTableRecords<{ User_Group_Name: string }>({ - table: "dp_User_User_Groups", - filter: `User_ID = ${profile.User_ID}`, - select: "User_Group_ID_TABLE.User_Group_Name", -}), + filter: `User_ID = ${userId}`, +}); ``` ## Quote escaping (SQL-injection prevention) @@ -140,7 +135,7 @@ export function validateColumnName(value: string): string { /* throws if ≠ /^[ | Where | Call | Purpose | |-------|------|---------| -| `userService.ts:72, 84` | `validateGuid(guid)` | Guard `User_GUID = '...'` filter | +| `userService.ts` (`getUserProfile`), `authorizationService.ts` (`resolveUserId`) | `validateGuid(...)` | Guard `User_GUID = '...'` filter | | `toolService.ts:307-334` | `validateColumnName`, `validateFkPath`, `validatePositiveInt` | Guard `resolveContactIds` — rejects bad page-metadata table/PK/contact-id-field and record-ID values | | `addressLabelService.ts:76` | `batch.forEach(validatePositiveInt)` | Guard batched `Contact_ID IN (...)` | | `groupService.ts:176` | `validatePositiveInt(groupId)` | Guard `Group_ID = ${id}` in `getGroup` | diff --git a/.claude/references/services/user-service.md b/.claude/references/services/user-service.md index 3549a9c..a7eb01c 100644 --- a/.claude/references/services/user-service.md +++ b/.claude/references/services/user-service.md @@ -9,15 +9,15 @@ last_verified: 2026-04-17 --- ## Purpose -Fetches the enriched MP user profile (by `User_GUID`) plus role names and user-group names; also resolves `User_GUID` → numeric `User_ID` for `$userId` audit use. +Fetches the signed-in user's display profile (`MPUserProfile`: names + email) by `User_GUID`. Single `dp_Users` query; no roles, user groups, IDs, phone, or image. ## Files - `src/services/userService.ts` — singleton -- `src/services/userService.test.ts` — 159 lines; covers singleton, `getUserProfile` happy/empty/error paths, and `getUserIdByGuid` (including malformed GUID). +- `src/services/userService.test.ts` — covers singleton, `getUserProfile` single-query select, explicit mapping (extra row fields never pass through), `Email_Address` → `null` normalization, empty result, malformed GUID, error propagation. ## Singleton ```typescript -// src/services/userService.ts:11-45 +// src/services/userService.ts — UserService export class UserService { private static instance: UserService; private mp: MPHelper | null = null; @@ -31,114 +31,59 @@ export class UserService { | Method | Returns | Purpose | |--------|---------|---------| -| `getUserProfile(id: string)` | `MPUserProfile \| undefined` | Full profile: Contact fields + roles + user groups | -| `getUserIdByGuid(guid: string)` | `number` | MP numeric `User_ID` (for `$userId` audit) | +| `getUserProfile(id: string)` | `MPUserProfile \| undefined` | Display profile: `First_Name`, `Nickname`, `Last_Name`, `Email_Address` | -Both methods guard input via `validateGuid()` from `src/lib/validation.ts`. +Input guarded via `validateGuid()` from `src/lib/validation.ts`. `User_GUID` → numeric `User_ID` resolution for `$userId` lives in `AuthorizationService` (`resolveUserId` in `src/services/authorizationService.ts`), not here. ## getUserProfile -Three queries: profile first (so the numeric `User_ID` is known), then roles and user-groups in parallel via `Promise.all`. +One query. The row is mapped field-by-field so nothing else MP returns can reach the client. ```typescript -// src/services/userService.ts:81-110 +// src/services/userService.ts — UserService.getUserProfile public async getUserProfile(id: string): Promise { const records = await this.mp!.getTableRecords({ table: "dp_Users", filter: `User_GUID = '${validateGuid(id)}'`, - select: "User_ID, User_GUID, Contact_ID_TABLE.First_Name,Contact_ID_TABLE.Nickname,Contact_ID_TABLE.Last_Name,Contact_ID_TABLE.Email_Address,Contact_ID_TABLE.Mobile_Phone,Contact_ID_TABLE.dp_fileUniqueId AS Image_GUID", + select: "Contact_ID_TABLE.First_Name,Contact_ID_TABLE.Nickname,Contact_ID_TABLE.Last_Name,Contact_ID_TABLE.Email_Address", top: 1 }); - const profile = records[0]; - if (!profile) return undefined; - - const [roleRecords, groupRecords] = await Promise.all([ - this.mp!.getTableRecords<{ Role_Name: string }>({ - table: "dp_User_Roles", - filter: `User_ID = ${profile.User_ID}`, - select: "Role_ID_TABLE.Role_Name", - }), - this.mp!.getTableRecords<{ User_Group_Name: string }>({ - table: "dp_User_User_Groups", - filter: `User_ID = ${profile.User_ID}`, - select: "User_Group_ID_TABLE.User_Group_Name", - }), - ]); + const row = records[0]; + if (!row) return undefined; return { - ...profile, - roles: roleRecords.map((r) => r.Role_Name), - userGroups: groupRecords.map((g) => g.User_Group_Name), + First_Name: row.First_Name, + Nickname: row.Nickname, + Last_Name: row.Last_Name, + Email_Address: row.Email_Address ?? null, }; } ``` -### Query 1 — `dp_Users` +### Query — `dp_Users` | Param | Value | |-------|-------| | `filter` | `User_GUID = '{validated guid}'` | -| `select` | `User_ID, User_GUID, Contact_ID_TABLE.First_Name, Contact_ID_TABLE.Nickname, Contact_ID_TABLE.Last_Name, Contact_ID_TABLE.Email_Address, Contact_ID_TABLE.Mobile_Phone, Contact_ID_TABLE.dp_fileUniqueId AS Image_GUID` | +| `select` | `Contact_ID_TABLE.First_Name, Contact_ID_TABLE.Nickname, Contact_ID_TABLE.Last_Name, Contact_ID_TABLE.Email_Address` | | `top` | `1` | -FK traversal: `dp_Users.Contact_ID` → `Contacts` row. `dp_fileUniqueId` aliased to `Image_GUID` for the profile picture. - -### Query 2 — `dp_User_Roles` (parallel) -| Param | Value | -|-------|-------| -| `filter` | `User_ID = {profile.User_ID}` | -| `select` | `Role_ID_TABLE.Role_Name` | - -Maps to `string[]` of role names. - -### Query 3 — `dp_User_User_Groups` (parallel) -| Param | Value | -|-------|-------| -| `filter` | `User_ID = {profile.User_ID}` | -| `select` | `User_Group_ID_TABLE.User_Group_Name` | - -Maps to `string[]` of user-group names. - -## getUserIdByGuid - -```typescript -// src/services/userService.ts:68-79 -public async getUserIdByGuid(guid: string): Promise { - const records = await this.mp!.getTableRecords<{ User_ID: number }>({ - table: 'dp_Users', - select: 'User_ID', - filter: `User_GUID = '${validateGuid(guid)}'`, - top: 1, - }); - if (!records || records.length === 0) { - throw new Error('User not found'); - } - return records[0].User_ID; -} -``` - -- Throws `Invalid GUID format: ...` for malformed input (via `validateGuid`) -- Throws `User not found` when `dp_Users` returns 0 rows or null +FK traversal: `dp_Users.Contact_ID` → `Contacts` row. ## Return type ```typescript -// MPUserProfile — from src/lib/providers/ministry-platform/types/user-profile.types.ts -{ - User_ID: number; - User_GUID: string; - Contact_ID: number; +// src/lib/providers/ministry-platform/types/user-profile.types.ts +export interface MPUserProfile { First_Name: string; Nickname: string; Last_Name: string; Email_Address: string | null; - Mobile_Phone: string | null; - Image_GUID: string | null; - roles: string[]; - userGroups: string[]; } ``` +Deliberately minimal (client-bound DTO). Formerly also carried `User_ID`, `User_GUID`, `Contact_ID`, `Mobile_Phone`, `Image_GUID`, `roles`, `userGroups` — none rendered. Add a field back only with the UI that needs it. + ## Caller contract > Pass `session.user.userGuid`, **not** `session.user.id`. Better Auth's `user.id` is the internal auth record ID, not the MP `User_GUID`. @@ -155,13 +100,10 @@ See `../auth/sessions.md` for `userGuid` vs `user.id`. | Action | File | Method | |--------|------|--------| | `getCurrentUserProfile` | `src/components/shared-actions/user.ts` | `getUserProfile` | -| `getUserTools` | `src/components/dev-panel/panels/user-tools-actions.ts` | `getUserIdByGuid` (then ToolService.getUserTools) | -| Group-wizard actions (create/update) | `src/components/group-wizard/actions.ts` | `getUserIdByGuid` (for `$userId` audit) | -| Address-label actions | `src/components/address-labels/actions.ts` | `getUserIdByGuid` | ## Gotchas - `getUserProfile` does NOT re-check the session; caller is responsible for `auth.api.getSession()`. -- If `dp_Users` has 0 rows for the GUID, `getUserProfile` returns `undefined` (not throw); `getUserIdByGuid` throws `User not found`. +- If `dp_Users` has 0 rows for the GUID, `getUserProfile` returns `undefined` (not throw). - The `select` string has no spaces after some commas (`...Nickname,Contact_ID_TABLE...`) — MP REST accepts this; do not "fix" unless the API starts rejecting it. ## Related docs diff --git a/.claude/references/utils/barcodes.md b/.claude/references/utils/barcodes.md index baaecca..9600ca5 100644 --- a/.claude/references/utils/barcodes.md +++ b/.claude/references/utils/barcodes.md @@ -90,7 +90,7 @@ export function preEncodeBarcodes( // - barcodeFormat === 'none' → labels returned unchanged // - barcodeFormat === 'imb' && config.mailerId → try IMb // on success: barStates = bars.join(''), barType = 'imb' -// on throw OR no mailerId: fall through to POSTNET +// on throw OR no mailerId: fall through to POSTNET (throw → console.warn { labelIndex, error: }) // - barcodeFormat === 'postnet' | 'imb' fallback: // tries full routingCode, then retries with first 5 digits // on success: barStates = JSON.stringify(bars), barType = 'postnet' @@ -187,8 +187,8 @@ expect(imbEncode('0123456709498765432101234567891').join('')).toBe( ``` ## Gotchas -- **Fallback swallows IMb errors silently.** In `preEncodeBarcodes` (`src/lib/barcode-helpers.ts:48`), an IMb throw falls through to POSTNET with no logging. If a mailer config is subtly wrong (bad service type, bad mailer ID length), labels silently downgrade to POSTNET. See glossary candidate "IMb→POSTNET fallback". -- **POSTNET `barStates` is JSON, IMb `barStates` is a raw string.** Consumers must branch on `barType` before rendering. Enforced at `src/lib/barcode-helpers.ts:47` (IMb) vs `:57,61` (POSTNET) and `src/lib/barcode-image.ts:64` (IMb expects plain string) vs `:98-103` (POSTNET parses JSON and silently falls back to a 1×1 BMP on parse failure). +- **IMb → POSTNET fallback is logged, but without record content.** In `preEncodeBarcodes` (`src/lib/barcode-helpers.ts`), each encode failure emits `console.warn('[preEncodeBarcodes] IMb encode failed, falling back to POSTNET', { labelIndex, error })` (POSTNET retry adds `routingLength`; final failure logs `... no barcode will be rendered`). `error` is the error's **name** only (`describeFailure`) — label name, address, ZIP/routing digits and the error message are deliberately omitted (CLAUDE.md rule 14). Find the bad row by `labelIndex` in the request. Users still see POSTNET silently on a subtly wrong mailer config. See [GOTCHA-040](../GOTCHAS.md). +- **POSTNET `barStates` is JSON, IMb `barStates` is a raw string.** Consumers must branch on `barType` before rendering. Enforced at `src/lib/barcode-helpers.ts:59` (IMb) vs `:70,78` (POSTNET) and `src/lib/barcode-image.ts:64` (IMb expects plain string) vs `:98-103` (POSTNET parses JSON and silently falls back to a 1×1 BMP on parse failure). - **`buildRoutingCode` treats DP `'00'` as absent.** Pass a real 1–2 digit delivery point; `'00'` returns a 5/9-digit routing, not 7/11-digit. See `src/lib/barcode-helpers.ts:13`. - **BMP invalid-input behavior differs.** `imbBarcodeToBmp` silently skips unknown bar chars; `postnetBarcodeToBmp` returns a 1×1 fallback on JSON parse failure — neither throws. diff --git a/.claude/references/utils/tool-params.md b/.claude/references/utils/tool-params.md index 81adbad..3f867e3 100644 --- a/.claude/references/utils/tool-params.md +++ b/.claude/references/utils/tool-params.md @@ -84,8 +84,8 @@ export interface ToolParams { export async function parseToolParams( searchParams: URLSearchParams | { [key: string]: string | string[] | undefined } ): Promise; -// - Integer params parsed via parseInt(_, 10) -// - recordDescription is decodeURIComponent()'d +// - Integer params parsed via parseIntOrUndefined (NaN → undefined) +// - recordDescription goes through safeDecode (decodeURIComponent, raw on URIError) // - pageData fetched via ToolService.getInstance().getPageData(pageID); // failures are caught and logged (pageData stays undefined) @@ -110,9 +110,9 @@ Standard shadcn helper for merging Tailwind class strings while deduplicating co ## How it works 1. `parseToolParams` normalizes `searchParams` reads through a single `getValue(key)` shim (handles both `URLSearchParams` and Next.js's awaited object shape; arrays collapse to first element). -2. Raw strings are converted: numeric fields via `parseInt(..., 10)`, `recordDescription` via `decodeURIComponent`. +2. Raw strings are converted: numeric fields via `parseIntOrUndefined` (finite integer or `undefined`), `recordDescription` via `safeDecode` (`decodeURIComponent`, falling back to the raw string on a malformed escape such as a stray `%`). 3. If `pageID` parsed as a number, `ToolService.getInstance()` + `getPageData(pageID)` is awaited. -4. Any error fetching page data is swallowed with `console.warn('Could not fetch page data for pageID:', ..., '- Stored procedure may not exist yet')` and `pageData` remains `undefined` — **tools must handle that case**. +4. Any error fetching page data is swallowed with `console.warn('tool_params.page_data_unavailable', { pageID })` and `pageData` remains `undefined` — **tools must handle that case**. 5. `isNewRecord` / `isEditMode` are pure predicates. ## Usage @@ -161,8 +161,8 @@ Client/UI (consume the `ToolParams` type and/or predicates): ## Gotchas - **`pageData` may be `undefined` even when `pageID` is set.** The fetch is wrapped in `try/catch` and swallowed — callers must guard. See `src/lib/tool-params.ts:54-60`. -- **`recordDescription` is URI-decoded; other strings are not.** Only `recordDescription` passes through `decodeURIComponent`. `q` and `addl` come through raw. -- **Integer parse errors silently produce `NaN` via `parseInt` semantics.** `parseInt('abc', 10)` returns `NaN` (note: `NaN === NaN` is `false` — use `Number.isNaN()` to test). The conditional `x ? parseInt(x, 10) : undefined` still assigns `NaN` back when `x` is a non-empty non-numeric string. Callers that check `typeof pageID === 'number'` see `NaN` (passes, since `typeof NaN === 'number'`), so pre-validation is the caller's responsibility. +- **`recordDescription` is URI-decoded; other strings are not.** Only `recordDescription` passes through `safeDecode`. `q` and `addl` come through raw. A malformed escape (`50% off`) used to throw `URIError` and crash the tool page; it now comes through raw. +- **Non-numeric integer params become `undefined`, not `NaN`.** `parseIntOrUndefined` returns `undefined` unless the parse is finite (GOTCHA-043). Callers still validate range (e.g. `> 0`) themselves. - **`searchParams` must be awaited first.** Next.js 16 dynamic APIs (`params`, `searchParams`) are `Promise`s; `await searchParams` in the page, then pass the result to `parseToolParams`. ## Related docs diff --git a/_INSTALL/.sql-hash b/_INSTALL/.sql-hash index e17814f..f6552c7 100644 --- a/_INSTALL/.sql-hash +++ b/_INSTALL/.sql-hash @@ -1 +1 @@ -f520e52514db5853536dff4afce89010a4e86d9cb9cbe5915bfde66d5dbb1904 \ No newline at end of file +de1ea26e0d700b7c869eba64e50c00e4d72e2ff672e8779dc7924bc84e007a05 \ No newline at end of file diff --git a/_INSTALL/ministryplatform-install.sql b/_INSTALL/ministryplatform-install.sql index 05afb6f..2c4a457 100644 --- a/_INSTALL/ministryplatform-install.sql +++ b/_INSTALL/ministryplatform-install.sql @@ -1,6 +1,6 @@ -- ============================================= -- Ministry Platform Database Install Script --- Generated: 2026-07-26T23:20:04.015Z +-- Generated: 2026-10-10T16:06:51.738Z -- Auto-generated - Do not edit manually -- ============================================= -- NOTE: Run this script against your Ministry Platform database @@ -890,10 +890,11 @@ GO -- api_dev_GetProcedureDefinition -- ============================================= -- Description: Returns the actual create SQL script for a stored procedure to enable LLM context --- Last Modified: 04/16/2026 +-- Last Modified: 10/10/2026 -- Chris Kehayias -- ============================================= CREATE PROCEDURE [dbo].[api_dev_GetProcedureDefinition] + @DomainID INT, -- required first parameter for MP API procs; MP injects it (unused here) @ProcedureName NVARCHAR(256), @SchemaName NVARCHAR(128) = 'dbo' AS diff --git a/next.config.ts b/next.config.ts index 2f73fde..9ac0c6d 100644 --- a/next.config.ts +++ b/next.config.ts @@ -37,6 +37,16 @@ const hstsHeader = { }; const nextConfig: NextConfig = { + experimental: { + serverActions: { + // Default is 1 MB, which made the address-label mail merge's documented + // template limit unreachable. 4 MB stays under Vercel's 4.5 MB function + // request-body cap so Next rejects first, with a clear error. Sized + // with the limits in `src/components/address-labels/limits.ts` — change + // them together. + bodySizeLimit: "4mb", + }, + }, async headers() { return [ { diff --git a/src/app/global-error.test.tsx b/src/app/global-error.test.tsx index 29445aa..debb23d 100644 --- a/src/app/global-error.test.tsx +++ b/src/app/global-error.test.tsx @@ -1,6 +1,7 @@ import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'; import { render, screen, cleanup } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import { renderToString } from 'react-dom/server'; import GlobalError from './global-error'; @@ -44,6 +45,25 @@ describe('GlobalError', () => { expect(retry).toHaveBeenCalledTimes(1); }); + it('offers a plain link home that works without JavaScript', () => { + render(); + + expect(screen.getByRole('link', { name: /go to home page/i })).toHaveAttribute('href', '/'); + }); + + it('enables the retry button once hydrated', () => { + render(); + + expect(screen.getByRole('button', { name: /try again/i })).toBeEnabled(); + }); + + it('server-renders the retry button disabled, so a pre-hydration click is not swallowed', () => { + const html = renderToString(); + + expect(html).toMatch(/]*disabled=""[^>]*>Try again<\/button>/); + expect(html).toContain('href="/"'); + }); + it('shows the digest reference when one is present', () => { render(); diff --git a/src/app/global-error.tsx b/src/app/global-error.tsx index d56160d..b62a3f3 100644 --- a/src/app/global-error.tsx +++ b/src/app/global-error.tsx @@ -1,6 +1,6 @@ "use client"; -import { useEffect } from "react"; +import { useEffect, useSyncExternalStore } from "react"; /** * Last-resort boundary: catches a throw in the ROOT LAYOUT itself, which it @@ -15,7 +15,20 @@ import { useEffect } from "react"; * The two are coupled: a nonce-based `style-src` would silently drop all of * this and render an unstyled page at the worst possible moment. If you change * one, check the other. + * + * Two controls, because this page can be the server-rendered HTML a user sees + * BEFORE (or without) hydration: + * - "Try again" calls `retry`, which needs React. It is disabled until the + * page hydrates, so a pre-hydration click is not silently swallowed. + * - "Go to home page" is a plain ``, which works with no JS at all. */ + +const subscribeNoop = () => () => {}; + +/** `false` in the server render and during hydration, `true` once hydrated. */ +function useHydrated(): boolean { + return useSyncExternalStore(subscribeNoop, () => true, () => false); +} export default function GlobalError({ error, retry, @@ -23,6 +36,8 @@ export default function GlobalError({ error: Error & { digest?: string }; retry: () => void; }) { + const hydrated = useHydrated(); + useEffect(() => { console.error("ui.render.error", { boundary: "global", @@ -61,7 +76,9 @@ export default function GlobalError({ The application failed to load. Please try again.

+

+ {/* A plain , not next/link, on purpose: it must work before or + without hydration, and a full document load is the recovery we + want when the root layout itself has failed. */} + {/* eslint-disable-next-line @next/next/no-html-link-for-pages */} + + Go to home page + +

{error.digest ? (

Reference: {error.digest} diff --git a/src/components/address-labels/actions.test.ts b/src/components/address-labels/actions.test.ts index f03cf83..eb6c8d3 100644 --- a/src/components/address-labels/actions.test.ts +++ b/src/components/address-labels/actions.test.ts @@ -8,6 +8,7 @@ const mockToBlob = vi.hoisted(() => vi.fn()); const mockDocxtemplaterRender = vi.hoisted(() => vi.fn()); const mockDocxtemplaterGetZip = vi.hoisted(() => vi.fn()); const mockImageModuleCtor = vi.hoisted(() => vi.fn()); +const mockAssertZip = vi.hoisted(() => vi.fn()); vi.mock('@/lib/auth', () => ({ auth: { @@ -91,6 +92,13 @@ vi.mock('pizzip', () => ({ default: class {}, })); +// The zip guard has its own tests against real archives (src/lib/zip-guard.test.ts); +// here it is a seam so the fake templates below need not be valid ZIPs. +vi.mock('@/lib/zip-guard', () => ({ + assertZipWithinLimits: mockAssertZip, + ZipLimitError: class ZipLimitError extends Error {}, +})); + vi.mock('docxtemplater-image', () => ({ default: class { constructor(...args: unknown[]) { @@ -111,6 +119,9 @@ vi.mock('@/lib/barcode-image', () => ({ })); import { fetchAddressLabels, generateLabelPdf, mergeTemplate } from './actions'; +import { preEncodeBarcodes } from '@/lib/barcode-helpers'; +import { ZipLimitError } from '@/lib/zip-guard'; +import { MAX_LABELS } from './limits'; import type { LabelConfig, LabelData } from '@/lib/dto'; import type { ToolParams } from '@/lib/tool-params'; @@ -463,11 +474,12 @@ describe('mergeTemplate', () => { }); it('should return error for oversized template', async () => { - const hugeBase64 = Buffer.alloc(6 * 1024 * 1024).toString('base64'); + const hugeBase64 = Buffer.alloc(3 * 1024 * 1024).toString('base64'); const labels: LabelData[] = [{ name: 'T', addressLine1: 'A', city: 'C', state: 'S', postalCode: '12345' }]; const result = await mergeTemplate(hugeBase64, labels, mergeConfig); expect(result.success).toBe(false); - if (!result.success) expect(result.error).toContain('5MB'); + if (!result.success) expect(result.error).toContain('2MB'); + expect(mockAssertZip).not.toHaveBeenCalled(); }); it('returns error wrapped for template tag mismatch', async () => { @@ -568,6 +580,84 @@ describe('mergeTemplate', () => { }); }); +describe('label input validation (shared by every render action)', () => { + const config: LabelConfig = { + stockId: '5160', + addressMode: 'household', + startPosition: 1, + includeMissingBarcodes: true, + barcodeFormat: 'postnet', + mailerId: '', + serviceType: '040', + }; + const label: LabelData = { name: 'T', addressLine1: 'A', city: 'C', state: 'S', postalCode: '12345' }; + const template = Buffer.from('x').toString('base64'); + + beforeEach(() => { + mockAssertZip.mockReset(); + mockDocxtemplaterRender.mockClear(); + vi.mocked(preEncodeBarcodes).mockClear(); + }); + + it('rejects more than MAX_LABELS labels before rendering', async () => { + const labels = Array.from({ length: MAX_LABELS + 1 }, () => label); + const result = await generateLabelPdf(labels, config); + expect(result).toEqual({ success: false, error: expect.stringContaining('Too many labels') }); + expect(preEncodeBarcodes).not.toHaveBeenCalled(); + }); + + it('rejects a label with an oversized field', async () => { + const result = await generateLabelPdf([{ ...label, name: 'x'.repeat(10_000) }], config); + expect(result).toEqual({ success: false, error: 'Invalid label data' }); + }); + + it('rejects a non-array labels payload', async () => { + const result = await generateLabelPdf('nope' as unknown as LabelData[], config); + expect(result).toEqual({ success: false, error: 'Invalid label data' }); + }); + + it('rejects an unknown service type', async () => { + const result = await generateLabelPdf([label], { ...config, serviceType: '999' }); + expect(result).toEqual({ success: false, error: 'Invalid label settings' }); + }); + + it.each([0, 31, 1e9, 1.5])('rejects start position %s outside the 30-up sheet', async (startPosition) => { + const result = await generateLabelPdf([label], { ...config, startPosition }); + expect(result.success).toBe(false); + expect(preEncodeBarcodes).not.toHaveBeenCalled(); + }); + + it('drops caller-supplied barStates/barType so only server-encoded bars render', async () => { + const hostile = { ...label, barStates: 'FFFF', barType: 'imb' as const }; + await mergeTemplate(template, [hostile], config); + const passed = vi.mocked(preEncodeBarcodes).mock.calls[0][0]; + expect(passed[0]).not.toHaveProperty('barStates'); + expect(passed[0]).not.toHaveProperty('barType'); + }); + + it('runs the zip guard before PizZip and returns its message on a limit breach', async () => { + mockAssertZip.mockImplementationOnce(() => { + throw new ZipLimitError('Template is too large when uncompressed'); + }); + const result = await mergeTemplate(template, [label], config); + expect(result).toEqual({ success: false, error: 'Template is too large when uncompressed' }); + expect(mockDocxtemplaterRender).not.toHaveBeenCalled(); + }); + + it('maps any other zip-guard failure to a generic invalid-file error', async () => { + mockAssertZip.mockImplementationOnce(() => { + throw new RangeError('out of bounds'); + }); + const result = await mergeTemplate(template, [label], config); + expect(result).toEqual({ success: false, error: 'Not a valid .docx file' }); + }); + + it('rejects a non-string template', async () => { + const result = await mergeTemplate(42 as unknown as string, [label], config); + expect(result).toEqual({ success: false, error: 'Template file exceeds 2MB limit' }); + }); +}); + describe('generateLabelDocx', () => { const docxConfig: LabelConfig = { stockId: '5160', diff --git a/src/components/address-labels/actions.ts b/src/components/address-labels/actions.ts index 7f509a0..01d8686 100644 --- a/src/components/address-labels/actions.ts +++ b/src/components/address-labels/actions.ts @@ -19,6 +19,10 @@ import { buildWordDocument } from './word-document'; import { getLabelStock } from '@/lib/label-stock'; import { preEncodeBarcodes } from '@/lib/barcode-helpers'; import { validateMailerId } from '@/lib/validation'; +import { assertZipWithinLimits, ZipLimitError } from '@/lib/zip-guard'; +import type { LabelStockConfig } from '@/lib/label-stock'; +import { parseLabelInput } from './schemas'; +import { MAX_TEMPLATE_BYTES, MAX_TEMPLATE_LABEL, TEMPLATE_ZIP_LIMITS } from './limits'; import Docxtemplater from 'docxtemplater'; import PizZip from 'pizzip'; import ImageModule from 'docxtemplater-image'; @@ -172,19 +176,36 @@ function describeError(error: unknown): Record { return described; } -export async function generateLabelPdf( - labels: LabelData[], - config: LabelConfig -): Promise<{ success: true; data: string } | { success: false; error: string }> { - await requireAccess('Contacts', 'read'); +type ActionResult = { success: true; data: string } | { success: false; error: string }; + +type PreparedLabels = + | { ok: true; labels: LabelData[]; config: LabelConfig; stock: LabelStockConfig } + | { ok: false; result: ActionResult }; + +/** + * Validate the caller-supplied labels and config shared by every render + * action: schema + count cap (`parseLabelInput`), a known stock, a start + * position that fits on one sheet, and a well-formed Mailer ID for IMb. + * + * `startPosition` matters: it is a count of blank cells to emit, so an + * unbounded value is an unbounded number of pages. + */ +function prepareLabels(rawLabels: unknown, rawConfig: unknown, emptyMessage: string): PreparedLabels { + const parsed = parseLabelInput(rawLabels, rawConfig); + if (!parsed.success) return { ok: false, result: parsed }; + const { labels, config } = parsed; const stock = getLabelStock(config.stockId); if (!stock) { - return { success: false, error: `Unknown label stock: ${config.stockId}` }; + return { ok: false, result: { success: false, error: `Unknown label stock: ${config.stockId}` } }; + } + + if (config.startPosition > stock.columns * stock.rows) { + return { ok: false, result: { success: false, error: 'Start position is beyond the end of the sheet' } }; } if (labels.length === 0) { - return { success: false, error: 'No labels to print' }; + return { ok: false, result: { success: false, error: emptyMessage } }; } if (config.barcodeFormat === 'imb' && config.mailerId) { @@ -192,12 +213,25 @@ export async function generateLabelPdf( validateMailerId(config.mailerId); } catch (error) { return { - success: false, - error: error instanceof Error ? error.message : 'Invalid Mailer ID', + ok: false, + result: { success: false, error: error instanceof Error ? error.message : 'Invalid Mailer ID' }, }; } } + return { ok: true, labels, config, stock }; +} + +export async function generateLabelPdf( + rawLabels: LabelData[], + rawConfig: LabelConfig +): Promise { + await requireAccess('Contacts', 'read'); + + const prepared = prepareLabels(rawLabels, rawConfig, 'No labels to print'); + if (!prepared.ok) return prepared.result; + const { labels, config, stock } = prepared; + try { const labelsWithBars = preEncodeBarcodes(labels, config); @@ -226,30 +260,14 @@ export async function generateLabelPdf( } export async function generateLabelDocx( - labels: LabelData[], - config: LabelConfig -): Promise<{ success: true; data: string } | { success: false; error: string }> { + rawLabels: LabelData[], + rawConfig: LabelConfig +): Promise { await requireAccess('Contacts', 'read'); - const stock = getLabelStock(config.stockId); - if (!stock) { - return { success: false, error: `Unknown label stock: ${config.stockId}` }; - } - - if (labels.length === 0) { - return { success: false, error: 'No labels to export' }; - } - - if (config.barcodeFormat === 'imb' && config.mailerId) { - try { - validateMailerId(config.mailerId); - } catch (error) { - return { - success: false, - error: error instanceof Error ? error.message : 'Invalid Mailer ID', - }; - } - } + const prepared = prepareLabels(rawLabels, rawConfig, 'No labels to export'); + if (!prepared.ok) return prepared.result; + const { labels, config, stock } = prepared; try { const labelsWithBars = preEncodeBarcodes(labels, config); @@ -268,34 +286,32 @@ export async function generateLabelDocx( } } -const MAX_TEMPLATE_SIZE = 5 * 1024 * 1024; // 5MB - export async function mergeTemplate( templateBase64: string, - labels: LabelData[], - config: LabelConfig -): Promise<{ success: true; data: string } | { success: false; error: string }> { + rawLabels: LabelData[], + rawConfig: LabelConfig +): Promise { await requireAccess('Contacts', 'read'); - if (labels.length === 0) { - return { success: false, error: 'No addresses to merge' }; + // Size check on the encoded string first, before decoding anything. + if (typeof templateBase64 !== 'string' || Math.ceil(templateBase64.length * 0.75) > MAX_TEMPLATE_BYTES) { + return { success: false, error: `Template file exceeds ${MAX_TEMPLATE_LABEL} limit` }; } - // Validate template size - const templateSize = Math.ceil(templateBase64.length * 0.75); - if (templateSize > MAX_TEMPLATE_SIZE) { - return { success: false, error: 'Template file exceeds 5MB limit' }; - } + const prepared = prepareLabels(rawLabels, rawConfig, 'No addresses to merge'); + if (!prepared.ok) return prepared.result; + const { labels, config } = prepared; - if (config.barcodeFormat === 'imb' && config.mailerId) { - try { - validateMailerId(config.mailerId); - } catch (error) { - return { - success: false, - error: error instanceof Error ? error.message : 'Invalid Mailer ID', - }; - } + // Bound decompression BEFORE PizZip sees the archive (zip-bomb guard). + const templateBuffer = Buffer.from(templateBase64, 'base64'); + try { + assertZipWithinLimits(templateBuffer, TEMPLATE_ZIP_LIMITS); + } catch (error) { + // Anything else is a malformed archive tripping a bounds check. + return { + success: false, + error: error instanceof ZipLimitError ? error.message : 'Not a valid .docx file', + }; } try { @@ -326,8 +342,6 @@ export async function mergeTemplate( }; }); - // Parse template - const templateBuffer = Buffer.from(templateBase64, 'base64'); const zip = new PizZip(templateBuffer); // Configure image module diff --git a/src/components/address-labels/limits.ts b/src/components/address-labels/limits.ts new file mode 100644 index 0000000..0c027cb --- /dev/null +++ b/src/components/address-labels/limits.ts @@ -0,0 +1,22 @@ +/** + * Size limits for the address-label server actions, shared by the client + * (early, friendly errors) and the server (the actual enforcement). + * + * They are sized against the request-body ceilings, not picked freely: + * Vercel rejects function request bodies over 4.5 MB, and + * `experimental.serverActions.bodySizeLimit` in `next.config.ts` is set to + * 4 MB so Next fails first with a clear error. A 2 MB template is ~2.7 MB as + * base64, which leaves ~1.3 MB for the label list on a mail merge. + * Raising any of these means re-checking all three together. + */ +export const MAX_TEMPLATE_BYTES = 2 * 1024 * 1024; +export const MAX_TEMPLATE_LABEL = '2MB'; + +/** Upper bound on labels per request. Bounds PDF/DOCX render memory. */ +export const MAX_LABELS = 10_000; + +/** Bounds PizZip's decompression of an uploaded template — see `src/lib/zip-guard.ts`. */ +export const TEMPLATE_ZIP_LIMITS = { + maxEntries: 1_000, + maxTotalBytes: 64 * 1024 * 1024, +} as const; diff --git a/src/components/address-labels/mail-merge-tab.test.tsx b/src/components/address-labels/mail-merge-tab.test.tsx index ed73768..fd8fbaa 100644 --- a/src/components/address-labels/mail-merge-tab.test.tsx +++ b/src/components/address-labels/mail-merge-tab.test.tsx @@ -103,14 +103,14 @@ describe('MailMergeTab', () => { expect(await screen.findByText('Please select a .docx file')).toBeInTheDocument(); }); - it('rejects a file over the 5MB limit', async () => { + it('rejects a file over the 2MB limit', async () => { render(); const input = screen.getByLabelText(/upload your template/i); - const file = makeDocxFile('big.docx', 6 * 1024 * 1024); + const file = makeDocxFile('big.docx', 3 * 1024 * 1024); fireEvent.change(input, { target: { files: [file] } }); - expect(await screen.findByText('Template file must be under 5MB')).toBeInTheDocument(); + expect(await screen.findByText('Template file must be under 2MB')).toBeInTheDocument(); }); it('accepts a valid .docx file, shows its name, and enables Merge', async () => { diff --git a/src/components/address-labels/mail-merge-tab.tsx b/src/components/address-labels/mail-merge-tab.tsx index 0f2d4d9..c66d991 100644 --- a/src/components/address-labels/mail-merge-tab.tsx +++ b/src/components/address-labels/mail-merge-tab.tsx @@ -8,9 +8,9 @@ import { Loader2, Download, Upload, FileText } from 'lucide-react'; import { AddressLabelsSummary } from './address-labels-summary'; import { generateSampleTemplate } from './sample-template'; import { mergeTemplate } from './actions'; +import { MAX_TEMPLATE_BYTES, MAX_TEMPLATE_LABEL } from './limits'; import type { LabelData, SkipRecord, LabelConfig } from '@/lib/dto'; -const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB interface MailMergeTabProps { printable: LabelData[]; @@ -88,8 +88,8 @@ export function MailMergeTab({ printable, skipped, config }: MailMergeTabProps) return; } - if (file.size > MAX_FILE_SIZE) { - setError('Template file must be under 5MB'); + if (file.size > MAX_TEMPLATE_BYTES) { + setError(`Template file must be under ${MAX_TEMPLATE_LABEL}`); return; } @@ -154,7 +154,7 @@ export function MailMergeTab({ printable, skipped, config }: MailMergeTabProps)

- + ({ + AuthorizationService: { + getInstance: () => ({ requireSecurityRole: vi.fn(async () => 1) }), + }, + UnauthorizedError: class UnauthorizedError extends Error {}, +})); + +import PizZip from 'pizzip'; +import { generateSampleTemplate } from './sample-template'; +import { mergeTemplate } from './actions'; +import type { LabelConfig } from '@/lib/dto'; + +/** + * End-to-end through the REAL zip guard, PizZip and docxtemplater (actions.test.ts + * mocks all three). Proves the guard accepts a genuine Word archive and that a + * bomb is stopped before PizZip is reached. + */ +const config: LabelConfig = { + stockId: '5160', + addressMode: 'household', + startPosition: 1, + includeMissingBarcodes: true, + barcodeFormat: 'none', + mailerId: '', + serviceType: '040', +}; +const labels = [{ name: 'Jane Doe', addressLine1: '1 Main St', city: 'Springfield', state: 'IL', postalCode: '62704' }]; + +describe('mergeTemplate (integration)', () => { + it('merges the bundled sample template', async () => { + const template = await generateSampleTemplate(); + const result = await mergeTemplate(template, labels, config); + expect(result.success).toBe(true); + if (!result.success) return; + const xml = new PizZip(Buffer.from(result.data, 'base64')).file('word/document.xml')!.asText(); + expect(xml).toContain('Jane Doe'); + }); + + it('rejects a zip bomb before decompressing it', async () => { + // An entry whose header understates its size: the shape that gets past a + // declared-size check and would make PizZip inflate it in full. (The + // honest-but-huge case is covered in src/lib/zip-guard.test.ts.) + const zip = new PizZip(); + zip.file('word/document.xml', Buffer.alloc(4 * 1024 * 1024)); + const buf = zip.generate({ type: 'nodebuffer', compression: 'DEFLATE' }); + const central = buf.indexOf(Buffer.from([0x50, 0x4b, 0x01, 0x02])); + buf.writeUInt32LE(100, central + 24); + const template = buf.toString('base64'); + const result = await mergeTemplate(template, labels, config); + expect(result).toEqual({ success: false, error: 'Template is too large when uncompressed' }); + }); +}); diff --git a/src/components/address-labels/schemas.ts b/src/components/address-labels/schemas.ts new file mode 100644 index 0000000..cc7d58e --- /dev/null +++ b/src/components/address-labels/schemas.ts @@ -0,0 +1,63 @@ +import { z } from 'zod'; +import { SERVICE_TYPES } from '@/lib/dto'; +import type { LabelConfig, LabelData } from '@/lib/dto'; +import { MAX_LABELS } from './limits'; + +/** + * Server-side validation for what the address-label actions receive. + * + * Labels come back from the client after `fetchAddressLabels`, so they are + * caller-shaped input: a server action is a POST endpoint anyone with access + * can call with any payload. This bounds the count and every string, and + * DROPS `barStates`/`barType` — those are computed server-side by + * `preEncodeBarcodes` and must never be accepted from the caller. + * + * Field caps are generous (well above MP's column sizes) so real data never + * trips them; they exist to bound memory, not to re-validate addresses. + */ +const text = (max: number) => z.string().max(max); + +export const labelDataSchema = z.object({ + name: text(256), + addressLine1: text(256), + addressLine2: text(256).optional(), + city: text(128), + state: text(64), + postalCode: text(32), + barCode: text(64).optional(), + deliveryPointCode: text(16).optional(), +}); + +export const labelsSchema = z.array(labelDataSchema).max(MAX_LABELS); + +const serviceTypeIds = SERVICE_TYPES.map((t) => t.id) as [string, ...string[]]; + +export const labelConfigSchema = z.object({ + stockId: text(64), + addressMode: z.enum(['household', 'individual']), + // Upper bound is the stock's labels-per-sheet, checked once the stock is known. + startPosition: z.number().int().min(1), + includeMissingBarcodes: z.boolean(), + barcodeFormat: z.enum(['imb', 'postnet', 'none']), + mailerId: text(9), + serviceType: z.enum(serviceTypeIds), +}); + +export type ParsedLabelInput = + | { success: true; labels: LabelData[]; config: LabelConfig } + | { success: false; error: string }; + +/** Parse labels + config; the error names the problem, never the payload. */ +export function parseLabelInput(labels: unknown, config: unknown): ParsedLabelInput { + if (Array.isArray(labels) && labels.length > MAX_LABELS) { + return { + success: false, + error: `Too many labels (${labels.length.toLocaleString('en-US')}); the limit is ${MAX_LABELS.toLocaleString('en-US')} per request`, + }; + } + const parsedLabels = labelsSchema.safeParse(labels); + if (!parsedLabels.success) return { success: false, error: 'Invalid label data' }; + const parsedConfig = labelConfigSchema.safeParse(config); + if (!parsedConfig.success) return { success: false, error: 'Invalid label settings' }; + return { success: true, labels: parsedLabels.data, config: parsedConfig.data }; +} diff --git a/src/components/shared-actions/user.test.ts b/src/components/shared-actions/user.test.ts index 28ccd74..a2c57c3 100644 --- a/src/components/shared-actions/user.test.ts +++ b/src/components/shared-actions/user.test.ts @@ -35,17 +35,10 @@ describe('getCurrentUserProfile', () => { it("should look up the profile using the session's userGuid", async () => { const mockProfile = { - User_ID: 1, - User_GUID: SESSION_GUID, - Contact_ID: 100, First_Name: 'John', Nickname: 'Johnny', Last_Name: 'Doe', Email_Address: 'john@example.com', - Mobile_Phone: null, - Image_GUID: null, - roles: ['Admin'], - userGroups: ['Staff'], }; mockGetUserProfile.mockResolvedValueOnce(mockProfile); diff --git a/src/components/user-menu/user-menu.test.tsx b/src/components/user-menu/user-menu.test.tsx index 53911dd..58c4c00 100644 --- a/src/components/user-menu/user-menu.test.tsx +++ b/src/components/user-menu/user-menu.test.tsx @@ -69,17 +69,10 @@ import { UserMenu } from "./user-menu"; function makeProfile(overrides: Partial = {}): MPUserProfile { return { - User_ID: 1, - User_GUID: "guid-1", - Contact_ID: 101, First_Name: "Jonathan", Nickname: "Jon", Last_Name: "Doe", Email_Address: "jon@example.com", - Mobile_Phone: null, - Image_GUID: null, - roles: [], - userGroups: [], ...overrides, }; } diff --git a/src/lib/barcode-helpers.test.ts b/src/lib/barcode-helpers.test.ts index 60a047a..90cb9a9 100644 --- a/src/lib/barcode-helpers.test.ts +++ b/src/lib/barcode-helpers.test.ts @@ -101,7 +101,9 @@ describe('barcode-helpers', () => { expect(result[0].barStates).toBeDefined(); expect(warnSpy).toHaveBeenCalledTimes(1); expect(warnSpy.mock.calls[0][0]).toContain('IMb encode failed'); - expect(warnSpy.mock.calls[0][0]).toContain('Test'); + // Identifiers and shape only — never the label's name or address (rule 14). + expect(warnSpy.mock.calls[0][1]).toEqual({ labelIndex: 0, error: 'Error' }); + expect(JSON.stringify(warnSpy.mock.calls)).not.toContain(baseLabel.name); }); it('should log warning and produce no barcode when IMb fails and POSTNET also fails (invalid ZIP)', () => { @@ -124,6 +126,7 @@ describe('barcode-helpers', () => { expect(warnSpy.mock.calls[0][0]).toContain('IMb encode failed'); expect(warnSpy.mock.calls[1][0]).toContain('POSTNET encode failed'); expect(warnSpy.mock.calls[2][0]).toContain('POSTNET encode also failed'); + expect(JSON.stringify(warnSpy.mock.calls)).not.toContain('ABCDE'); }); it('should log warning and fall back to 5-digit ZIP when full-routing POSTNET throws', () => { diff --git a/src/lib/barcode-helpers.ts b/src/lib/barcode-helpers.ts index e53f823..879b0ff 100644 --- a/src/lib/barcode-helpers.ts +++ b/src/lib/barcode-helpers.ts @@ -32,14 +32,23 @@ export function buildImbTrackingCode(mailerId: string, serviceType: string, seri * Attempts IMb first (if configured), falls back to POSTNET. * Returns a new array with `barStates` and `barType` populated where possible. */ +/** + * Log shape for an encode failure. CLAUDE.md rule 14: identifiers and shape + * only. The label's position in the batch identifies it; its name, address + * and ZIP/routing digits are record content and stay out of logs. The error + * message is dropped too, since encoder errors can echo their input. + */ +function describeFailure(index: number, error: unknown): { labelIndex: number; error: string } { + return { labelIndex: index, error: error instanceof Error ? error.name : typeof error }; +} + export function preEncodeBarcodes(labels: LabelData[], config: LabelConfig): LabelData[] { let serialCounter = 0; - return labels.map((label) => { + return labels.map((label, index) => { if (config.barcodeFormat === 'none') return label; const routingCode = buildRoutingCode(label.postalCode, label.deliveryPointCode); - const labelId = label.name || label.addressLine1 || '(unnamed)'; if (config.barcodeFormat === 'imb' && config.mailerId) { try { @@ -50,10 +59,7 @@ export function preEncodeBarcodes(labels: LabelData[], config: LabelConfig): Lab return { ...label, barStates: bars.join(''), barType: 'imb' as const }; } catch (err) { // Fall through to POSTNET — log the downgrade so operators see misconfigured mailer IDs / ZIPs. - console.warn( - `[preEncodeBarcodes] IMb encode failed for "${labelId}", falling back to POSTNET:`, - err instanceof Error ? err.message : err - ); + console.warn('[preEncodeBarcodes] IMb encode failed, falling back to POSTNET', describeFailure(index, err)); } } @@ -64,16 +70,16 @@ export function preEncodeBarcodes(labels: LabelData[], config: LabelConfig): Lab return { ...label, barStates: JSON.stringify(bars), barType: 'postnet' as const }; } catch (err) { console.warn( - `[preEncodeBarcodes] POSTNET encode failed for "${labelId}" with routing "${routingCode}", retrying with 5-digit ZIP:`, - err instanceof Error ? err.message : err + '[preEncodeBarcodes] POSTNET encode failed, retrying with 5-digit ZIP', + { ...describeFailure(index, err), routingLength: routingCode.length } ); try { const bars = postnetEncode(routingCode.substring(0, 5)); return { ...label, barStates: JSON.stringify(bars), barType: 'postnet' as const }; } catch (err2) { console.warn( - `[preEncodeBarcodes] POSTNET encode also failed with 5-digit ZIP for "${labelId}", no barcode will be rendered:`, - err2 instanceof Error ? err2.message : err2 + '[preEncodeBarcodes] POSTNET encode also failed with 5-digit ZIP, no barcode will be rendered', + describeFailure(index, err2) ); } } diff --git a/src/lib/providers/ministry-platform/db/api_dev_GetProcedureDefinition.sql b/src/lib/providers/ministry-platform/db/api_dev_GetProcedureDefinition.sql index f4c6e83..abe6119 100644 --- a/src/lib/providers/ministry-platform/db/api_dev_GetProcedureDefinition.sql +++ b/src/lib/providers/ministry-platform/db/api_dev_GetProcedureDefinition.sql @@ -12,10 +12,11 @@ GO -- api_dev_GetProcedureDefinition -- ============================================= -- Description: Returns the actual create SQL script for a stored procedure to enable LLM context --- Last Modified: 04/16/2026 +-- Last Modified: 10/10/2026 -- Chris Kehayias -- ============================================= CREATE PROCEDURE [dbo].[api_dev_GetProcedureDefinition] + @DomainID INT, -- required first parameter for MP API procs; MP injects it (unused here) @ProcedureName NVARCHAR(256), @SchemaName NVARCHAR(128) = 'dbo' AS diff --git a/src/lib/providers/ministry-platform/scripts/codegen-escape.test.ts b/src/lib/providers/ministry-platform/scripts/codegen-escape.test.ts new file mode 100644 index 0000000..d4c783e --- /dev/null +++ b/src/lib/providers/ministry-platform/scripts/codegen-escape.test.ts @@ -0,0 +1,48 @@ +import { describe, it, expect } from 'vitest'; +import { commentText, propertyKey, positiveInt } from './codegen-escape'; + +describe('commentText', () => { + it('passes ordinary text through', () => { + expect(commentText('Contacts.Contact_ID')).toBe('Contacts.Contact_ID'); + }); + + it('cannot close a block comment', () => { + const out = commentText('Evil */ export const x = 1; /*'); + expect(out).not.toContain('*/'); + }); + + it.each(['\n', '\r\n', '\u2028', '\u2029'])('cannot end a line comment with %j', (nl) => { + const out = commentText(`Table${nl}export const pwned = true;`); + expect(out).not.toMatch(/[\r\n\u2028\u2029]/); + }); + + it('stringifies null/undefined to empty', () => { + expect(commentText(undefined)).toBe(''); + expect(commentText(null)).toBe(''); + }); +}); + +describe('propertyKey', () => { + it('leaves valid identifiers bare', () => { + expect(propertyKey('Contact_ID')).toBe('Contact_ID'); + }); + + it('quotes names with special characters', () => { + expect(propertyKey('Allow_Check-in')).toBe('"Allow_Check-in"'); + }); + + it.each(['a"b', 'a\\b', 'a\nb', '"; export const x = 1; "'])('produces a single valid string literal for %j', (name) => { + const key = propertyKey(name); + expect(JSON.parse(key)).toBe(name); + }); +}); + +describe('positiveInt', () => { + it.each([[50, 50], ['50', 50]])('accepts %j', (input, expected) => { + expect(positiveInt(input)).toBe(expected); + }); + + it.each([0, -1, 1.5, NaN, Infinity, '1); process.exit(1', '', null, undefined, {}])('rejects %j', (input) => { + expect(positiveInt(input)).toBeUndefined(); + }); +}); diff --git a/src/lib/providers/ministry-platform/scripts/codegen-escape.ts b/src/lib/providers/ministry-platform/scripts/codegen-escape.ts new file mode 100644 index 0000000..05a6329 --- /dev/null +++ b/src/lib/providers/ministry-platform/scripts/codegen-escape.ts @@ -0,0 +1,37 @@ +/** + * Escaping for MP metadata interpolated into generated TypeScript source + * (`generate-types.ts`). + * + * Table names, column names, FK targets, permission strings and sizes all come + * from the MP API at generation time, and the output is committed and compiled + * into the app. A value carrying a newline, a `*\/`, a quote or a non-numeric + * "size" could otherwise close a comment or string early and turn metadata + * into code. Each helper below matches one interpolation context. + */ + +const IDENTIFIER = /^[a-zA-Z_$][a-zA-Z0-9_$]*$/; + +/** + * For text inside a `//` line comment or a `/* *\/` block comment: no line + * terminators (they end a `//` comment) and no `*\/` (it ends a block comment). + */ +export function commentText(value: unknown): string { + return String(value ?? '') + .replace(/[\r\n\u2028\u2029]+/g, ' ') + .replace(/\*\//g, '*\\/'); +} + +/** An object-literal / interface property key: bare if a valid identifier, else a JSON string literal. */ +export function propertyKey(name: string): string { + return IDENTIFIER.test(name) ? name : JSON.stringify(name); +} + +/** + * A size interpolated as a number literal (`.max(${n})`, `max ${n} chars`). + * Returns `undefined` for anything that is not a positive safe integer, so a + * caller can simply omit the constraint. + */ +export function positiveInt(value: unknown): number | undefined { + const n = typeof value === 'number' ? value : typeof value === 'string' && /^\d+$/.test(value) ? Number(value) : NaN; + return Number.isSafeInteger(n) && n > 0 ? n : undefined; +} diff --git a/src/lib/providers/ministry-platform/scripts/generate-types.ts b/src/lib/providers/ministry-platform/scripts/generate-types.ts index 76e9b64..e993cc9 100644 --- a/src/lib/providers/ministry-platform/scripts/generate-types.ts +++ b/src/lib/providers/ministry-platform/scripts/generate-types.ts @@ -21,6 +21,7 @@ envFiles.forEach(file => { import { MPHelper } from "../helper"; import type { TableMetadata, ParameterDataType } from "../types"; +import { commentText, positiveInt, propertyKey } from "./codegen-escape"; interface ColumnMetadata { Name: string; @@ -153,31 +154,27 @@ function sanitizeTypeName(name: string): string { return result; } -function isValidIdentifier(name: string): boolean { - // Check if the name is a valid JavaScript identifier - // Must start with letter, underscore, or dollar sign - // Can contain letters, digits, underscores, or dollar signs - return /^[a-zA-Z_$][a-zA-Z0-9_$]*$/.test(name); -} - +// Every MP metadata value interpolated into generated source goes through +// ./codegen-escape: propertyKey() for keys, commentText() for comment text, +// positiveInt() for sizes. See that file for why. function formatFieldName(name: string): string { - // Quote field names that aren't valid JavaScript identifiers - return isValidIdentifier(name) ? name : `"${name}"`; + return propertyKey(name); } -function mapDataTypeToTypeScript(dataType: ParameterDataType, isRequired: boolean, size?: number): string { +function mapDataTypeToTypeScript(dataType: ParameterDataType, isRequired: boolean, rawSize?: unknown): string { + const size = positiveInt(rawSize); const baseType = (() => { switch (dataType) { case "String": case "Text": case "LargeString": - return size && size > 0 ? `string /* max ${size} chars */` : "string"; + return size ? `string /* max ${size} chars */` : "string"; case "Email": - return size && size > 0 ? `string /* email, max ${size} chars */` : "string /* email */"; + return size ? `string /* email, max ${size} chars */` : "string /* email */"; case "Phone": return "string /* phone number */"; case "Url": - return size && size > 0 ? `string /* URL, max ${size} chars */` : "string /* URL */"; + return size ? `string /* URL, max ${size} chars */` : "string /* URL */"; case "Integer16": return "number /* 16-bit integer */"; case "Integer32": @@ -223,20 +220,22 @@ function mapDataTypeToTypeScript(dataType: ParameterDataType, isRequired: boolea function mapDataTypeToZod(col: ColumnMetadata): string { let zodType = ""; - + // `.max(${size})` is executable code in the generated schema, not a comment. + const size = positiveInt(col.Size); + switch (col.DataType) { case "String": case "Text": case "LargeString": zodType = "z.string()"; - if (col.Size > 0) { - zodType += `.max(${col.Size})`; + if (size) { + zodType += `.max(${size})`; } break; case "Email": zodType = "z.string().email()"; - if (col.Size > 0) { - zodType += `.max(${col.Size})`; + if (size) { + zodType += `.max(${size})`; } break; case "Phone": @@ -244,8 +243,8 @@ function mapDataTypeToZod(col: ColumnMetadata): string { break; case "Url": zodType = "z.string().url()"; - if (col.Size > 0) { - zodType += `.max(${col.Size})`; + if (size) { + zodType += `.max(${size})`; } break; case "Integer16": @@ -352,13 +351,13 @@ function generateDetailedTypeDefinition( const fields = table.Columns .filter(col => col.DataType !== "Separator") // Skip separator fields .map(col => { - const fieldType = mapDataTypeToTypeScript(col.DataType, col.IsRequired, col.Size > 0 ? col.Size : undefined); + const fieldType = mapDataTypeToTypeScript(col.DataType, col.IsRequired, col.Size); const optionalMarker = col.IsRequired ? "" : "?"; const commentParts = []; if (col.IsPrimaryKey) commentParts.push("Primary Key"); if (col.IsForeignKey && col.ReferencedTable) { - commentParts.push(`Foreign Key -> ${col.ReferencedTable}.${col.ReferencedColumn}`); + commentParts.push(`Foreign Key -> ${commentText(col.ReferencedTable)}.${commentText(col.ReferencedColumn)}`); } if (col.IsReadOnly) commentParts.push("Read Only"); if (col.IsComputed) commentParts.push("Computed"); @@ -366,8 +365,9 @@ function generateDetailedTypeDefinition( // Add JSDoc comment for additional constraints let jsDocComment = ""; - if (col.Size > 0 && ["String", "Text", "Email", "Url"].includes(col.DataType)) { - jsDocComment = `\n /**\n * Max length: ${col.Size} characters\n */`; + const maxLength = positiveInt(col.Size); + if (maxLength && ["String", "Text", "Email", "Url"].includes(col.DataType)) { + jsDocComment = `\n /**\n * Max length: ${maxLength} characters\n */`; } const inlineComment = commentParts.length > 0 ? ` // ${commentParts.join(", ")}` : ""; @@ -404,16 +404,17 @@ function generateDetailedTypeDefinition( fieldsDefinition = fields.join("\n"); } else { // Basic fallback - fieldsDefinition = ` ${name}_ID?: number; // Primary key (assuming standard naming convention) + fieldsDefinition = ` ${formatFieldName(`${name}_ID`)}?: number; // Primary key (assuming standard naming convention) [key: string]: unknown; // Allow for additional fields`; } - const permissions = table.SpecialPermissions ? `Special Permissions: ${table.SpecialPermissions}` : ""; - const accessLevel = `Access Level: ${table.AccessLevel}`; - + const permissions = table.SpecialPermissions ? `Special Permissions: ${commentText(table.SpecialPermissions)}` : ""; + const accessLevel = `Access Level: ${commentText(table.AccessLevel)}`; + const nameInComment = commentText(name); + return `/** - * Interface for ${name} -* Table: ${name} + * Interface for ${nameInComment} +* Table: ${nameInComment} * ${accessLevel} * ${permissions} * ${sampleRecords ? `Generated from ${sampleRecords.length} sample records` : "Generated from column metadata"} diff --git a/src/lib/providers/ministry-platform/types/user-profile.types.ts b/src/lib/providers/ministry-platform/types/user-profile.types.ts index d1f834a..2b59f2a 100644 --- a/src/lib/providers/ministry-platform/types/user-profile.types.ts +++ b/src/lib/providers/ministry-platform/types/user-profile.types.ts @@ -1,13 +1,15 @@ +/** + * The signed-in user's profile as sent to the CLIENT (via + * `getCurrentUserProfile`). Deliberately minimal: only what the UI renders. + * + * It used to carry `User_ID`, `User_GUID`, `Contact_ID`, `Mobile_Phone`, + * `Image_GUID`, and the user's security roles and user groups. None of that + * was displayed, and roles/groups describe the domain's security setup. + * Add a field back only together with the UI that needs it. + */ export interface MPUserProfile { - User_ID: number; - User_GUID: string; - Contact_ID: number; First_Name: string; Nickname: string; Last_Name: string; Email_Address: string | null; - Mobile_Phone: string | null; - Image_GUID: string | null; - roles: string[]; - userGroups: string[]; } diff --git a/src/lib/tool-params.server.ts b/src/lib/tool-params.server.ts index 4a1907b..5d2a09a 100644 --- a/src/lib/tool-params.server.ts +++ b/src/lib/tool-params.server.ts @@ -25,6 +25,20 @@ function parseIntOrUndefined(value: string | undefined): number | undefined { return Number.isFinite(parsed) ? parsed : undefined; } +/** + * `decodeURIComponent` throws `URIError` on a malformed escape (a stray `%`, + * e.g. "50% off"), which used to crash the whole tool page. Fall back to the + * raw value: it is display text, and the raw string is the best rendering of + * whatever MP sent. + */ +function safeDecode(value: string): string { + try { + return decodeURIComponent(value); + } catch { + return value; + } +} + export async function parseToolParams(searchParams: URLSearchParams | { [key: string]: string | string[] | undefined }): Promise { const getValue = (key: string): string | undefined => { if (searchParams instanceof URLSearchParams) { @@ -70,7 +84,7 @@ export async function parseToolParams(searchParams: URLSearchParams | { [key: st q: q || undefined, v: parseIntOrUndefined(v), recordID: parseIntOrUndefined(recordID), - recordDescription: recordDescription ? decodeURIComponent(recordDescription) : undefined, + recordDescription: recordDescription ? safeDecode(recordDescription) : undefined, addl: addl || undefined, pageData: pageData, }; diff --git a/src/lib/tool-params.test.ts b/src/lib/tool-params.test.ts index d5b080c..bc117d7 100644 --- a/src/lib/tool-params.test.ts +++ b/src/lib/tool-params.test.ts @@ -145,6 +145,14 @@ describe('tool-params', () => { expect(params.recordDescription).toBe('Plain Text'); }); + it.each(['50% off', 'Smith %E0%A4%A', '%'])( + 'keeps a malformed escape (%s) raw instead of throwing', + async (raw) => { + const params = await parseToolParams({ recordDescription: raw }); + expect(params.recordDescription).toBe(raw); + }, + ); + it('returns undefined when recordDescription absent', async () => { const params = await parseToolParams({}); expect(params.recordDescription).toBeUndefined(); diff --git a/src/lib/zip-guard.test.ts b/src/lib/zip-guard.test.ts new file mode 100644 index 0000000..8459680 --- /dev/null +++ b/src/lib/zip-guard.test.ts @@ -0,0 +1,130 @@ +import { describe, it, expect } from 'vitest'; +import PizZip from 'pizzip'; +import { assertZipWithinLimits, ZipLimitError } from './zip-guard'; + +const LIMITS = { maxEntries: 10, maxTotalBytes: 1024 * 1024 }; + +function makeZip(files: Record, compression: 'DEFLATE' | 'STORE' = 'DEFLATE'): Buffer { + const zip = new PizZip(); + for (const [name, content] of Object.entries(files)) zip.file(name, content); + return zip.generate({ type: 'nodebuffer', compression }); +} + +/** Offset of the first central-directory record (signature PK\x01\x02). */ +function centralOffset(buf: Buffer): number { + return buf.indexOf(Buffer.from([0x50, 0x4b, 0x01, 0x02])); +} + +function expectRejected(buf: Buffer, message: string, limits = LIMITS) { + let caught: unknown; + try { + assertZipWithinLimits(buf, limits); + } catch (error) { + caught = error; + } + expect(caught).toBeInstanceOf(ZipLimitError); + expect((caught as Error).message).toBe(message); +} + +describe('assertZipWithinLimits', () => { + it('accepts a small, honest deflated archive', () => { + const buf = makeZip({ 'word/document.xml': '' + 'a'.repeat(5000) + '', '[Content_Types].xml': '' }); + expect(() => assertZipWithinLimits(buf, LIMITS)).not.toThrow(); + }); + + it('accepts a stored (uncompressed) archive', () => { + const buf = makeZip({ 'a.xml': 'hello' }, 'STORE'); + expect(() => assertZipWithinLimits(buf, LIMITS)).not.toThrow(); + }); + + it('accepts an archive with an empty entry', () => { + const buf = makeZip({ 'empty.xml': '', 'a.xml': 'x' }); + expect(() => assertZipWithinLimits(buf, LIMITS)).not.toThrow(); + }); + + it('rejects an archive whose declared total exceeds the cap (classic bomb)', () => { + // 2 MB of zeros deflates to a couple of KB. + const buf = makeZip({ 'bomb.xml': Buffer.alloc(2 * 1024 * 1024) }); + expect(buf.length).toBeLessThan(20_000); + expectRejected(buf, 'Template is too large when uncompressed'); + }); + + it('rejects an entry that lies about its uncompressed size', () => { + const buf = makeZip({ 'liar.xml': Buffer.alloc(512 * 1024) }); + // Patch the central directory's declared uncompressed size down to 10 bytes. + buf.writeUInt32LE(10, centralOffset(buf) + 24); + expectRejected(buf, 'Template is too large when uncompressed'); + }); + + it('rejects an entry that declares more than it actually contains', () => { + const buf = makeZip({ 'short.xml': 'a'.repeat(1000) }); + buf.writeUInt32LE(5000, centralOffset(buf) + 24); + expectRejected(buf, 'Not a valid .docx file'); + }); + + it('rejects too many entries', () => { + const files = Object.fromEntries(Array.from({ length: 11 }, (_, i) => [`f${i}.xml`, 'x'])); + expectRejected(makeZip(files), 'Template contains too many files'); + }); + + it('rejects a buffer that is not a ZIP at all', () => { + expectRejected(Buffer.from('this is not a zip file, just some text padding it out'), 'Not a valid .docx file'); + }); + + it('rejects a buffer shorter than an end-of-central-directory record', () => { + expectRejected(Buffer.from('PK'), 'Not a valid .docx file'); + }); + + it('rejects ZIP64 markers', () => { + const buf = makeZip({ 'a.xml': 'x' }); + const eocd = buf.lastIndexOf(Buffer.from([0x50, 0x4b, 0x05, 0x06])); + buf.writeUInt16LE(0xffff, eocd + 10); + expectRejected(buf, 'ZIP64 templates are not supported'); + }); + + it('rejects encrypted entries', () => { + const buf = makeZip({ 'a.xml': 'x' }); + const cd = centralOffset(buf); + buf.writeUInt16LE(buf.readUInt16LE(cd + 8) | 0x1, cd + 8); + expectRejected(buf, 'Encrypted templates are not supported'); + }); + + it('rejects unsupported compression methods', () => { + const buf = makeZip({ 'a.xml': 'x' }); + buf.writeUInt16LE(12, centralOffset(buf) + 10); // bzip2 + expectRejected(buf, 'Template uses an unsupported compression method'); + }); + + it('rejects a corrupt deflate stream', () => { + const buf = makeZip({ 'a.xml': 'a'.repeat(2000) }); + const local = 0; + const dataStart = local + 30 + buf.readUInt16LE(local + 26) + buf.readUInt16LE(local + 28); + buf.fill(0xff, dataStart, dataStart + 8); + expectRejected(buf, 'Not a valid .docx file'); + }); + + it('rejects a stored entry whose sizes disagree', () => { + const buf = makeZip({ 'a.xml': 'hello' }, 'STORE'); + buf.writeUInt32LE(4, centralOffset(buf) + 20); // compressed size + expectRejected(buf, 'Not a valid .docx file'); + }); + + it('rejects a central directory that points past the end of the buffer', () => { + const buf = makeZip({ 'a.xml': 'x' }); + const eocd = buf.lastIndexOf(Buffer.from([0x50, 0x4b, 0x05, 0x06])); + buf.writeUInt32LE(buf.length + 100, eocd + 16); + expectRejected(buf, 'Not a valid .docx file'); + }); + + it('rejects a local header offset that does not point at a local header', () => { + const buf = makeZip({ 'a.xml': 'x' }); + buf.writeUInt32LE(5, centralOffset(buf) + 42); + expectRejected(buf, 'Not a valid .docx file'); + }); + + it('rejects entry data that runs past the end of the buffer', () => { + const buf = makeZip({ 'a.xml': 'a'.repeat(100) }); + buf.writeUInt32LE(buf.length * 2, centralOffset(buf) + 20); + expectRejected(buf, 'Not a valid .docx file'); + }); +}); diff --git a/src/lib/zip-guard.ts b/src/lib/zip-guard.ts new file mode 100644 index 0000000..5efa51b --- /dev/null +++ b/src/lib/zip-guard.ts @@ -0,0 +1,132 @@ +import { inflateRawSync } from 'zlib'; + +/** + * Pre-flight check for an uploaded ZIP container (a `.docx` template) BEFORE + * it is handed to PizZip. + * + * PizZip trusts the sizes declared in the archive's own headers and inflates + * each entry fully into memory before it compares the result against them, so + * a few-hundred-KB "zip bomb" can expand to gigabytes inside the server + * process. This walks the central directory independently and: + * + * 1. caps the entry count and the SUM of declared uncompressed sizes, then + * 2. actually inflates every entry with `maxOutputLength` set to its declared + * size, so an entry that lies about its size fails at that size instead of + * expanding without bound. + * + * After this passes, PizZip's own decompression is bounded by `maxTotalBytes`. + * ZIP64 archives, encryption and compression methods other than STORE/DEFLATE + * are rejected — a Word template never needs them, and PizZip cannot read the + * last two anyway. + * + * Errors name the failure mode only, never archive content. + */ +export interface ZipLimits { + maxEntries: number; + maxTotalBytes: number; +} + +export class ZipLimitError extends Error { + constructor(message: string) { + super(message); + this.name = 'ZipLimitError'; + } +} + +const EOCD_SIGNATURE = 0x06054b50; +const CENTRAL_SIGNATURE = 0x02014b50; +const LOCAL_SIGNATURE = 0x04034b50; +const EOCD_MIN_LENGTH = 22; +const MAX_COMMENT_LENGTH = 0xffff; +const METHOD_STORE = 0; +const METHOD_DEFLATE = 8; +const FLAG_ENCRYPTED = 0x1; + +function findEndOfCentralDirectory(buf: Buffer): number { + const stop = Math.max(0, buf.length - EOCD_MIN_LENGTH - MAX_COMMENT_LENGTH); + for (let i = buf.length - EOCD_MIN_LENGTH; i >= stop; i--) { + if (buf.readUInt32LE(i) === EOCD_SIGNATURE) return i; + } + throw new ZipLimitError('Not a valid .docx file'); +} + +export function assertZipWithinLimits(buf: Buffer, limits: ZipLimits): void { + if (buf.length < EOCD_MIN_LENGTH) throw new ZipLimitError('Not a valid .docx file'); + + const eocd = findEndOfCentralDirectory(buf); + const entryCount = buf.readUInt16LE(eocd + 10); + const directoryOffset = buf.readUInt32LE(eocd + 16); + + if (entryCount === 0xffff || directoryOffset === 0xffffffff) { + throw new ZipLimitError('ZIP64 templates are not supported'); + } + if (entryCount > limits.maxEntries) { + throw new ZipLimitError('Template contains too many files'); + } + + // Pass 1: declared sizes only — cheap, and bounds pass 2. + const entries: Array<{ method: number; compressed: number; uncompressed: number; localOffset: number }> = []; + let declaredTotal = 0; + let cursor = directoryOffset; + for (let i = 0; i < entryCount; i++) { + if (cursor + 46 > buf.length || buf.readUInt32LE(cursor) !== CENTRAL_SIGNATURE) { + throw new ZipLimitError('Not a valid .docx file'); + } + const flags = buf.readUInt16LE(cursor + 8); + const method = buf.readUInt16LE(cursor + 10); + const compressed = buf.readUInt32LE(cursor + 20); + const uncompressed = buf.readUInt32LE(cursor + 24); + const nameLength = buf.readUInt16LE(cursor + 28); + const extraLength = buf.readUInt16LE(cursor + 30); + const commentLength = buf.readUInt16LE(cursor + 32); + const localOffset = buf.readUInt32LE(cursor + 42); + + if (flags & FLAG_ENCRYPTED) throw new ZipLimitError('Encrypted templates are not supported'); + if (method !== METHOD_STORE && method !== METHOD_DEFLATE) { + throw new ZipLimitError('Template uses an unsupported compression method'); + } + + declaredTotal += uncompressed; + if (declaredTotal > limits.maxTotalBytes) { + throw new ZipLimitError('Template is too large when uncompressed'); + } + + entries.push({ method, compressed, uncompressed, localOffset }); + cursor += 46 + nameLength + extraLength + commentLength; + } + + // Pass 2: prove each entry's real size matches its declared size. + for (const entry of entries) { + const local = entry.localOffset; + if (local + 30 > buf.length || buf.readUInt32LE(local) !== LOCAL_SIGNATURE) { + throw new ZipLimitError('Not a valid .docx file'); + } + const dataStart = local + 30 + buf.readUInt16LE(local + 26) + buf.readUInt16LE(local + 28); + const dataEnd = dataStart + entry.compressed; + if (dataEnd > buf.length) throw new ZipLimitError('Not a valid .docx file'); + + if (entry.method === METHOD_STORE) { + if (entry.compressed !== entry.uncompressed) throw new ZipLimitError('Not a valid .docx file'); + continue; + } + + let actual: number; + try { + // +1 so "exactly the declared size" succeeds and anything larger throws. + actual = inflateRawSync(buf.subarray(dataStart, dataEnd), { + maxOutputLength: entry.uncompressed + 1, + }).length; + } catch (error) { + // zlib throws a RangeError when output would pass maxOutputLength; + // anything else is a corrupt stream. + throw new ZipLimitError( + error instanceof RangeError ? 'Template is too large when uncompressed' : 'Not a valid .docx file', + ); + } + if (actual !== entry.uncompressed) { + throw new ZipLimitError( + actual > entry.uncompressed ? 'Template is too large when uncompressed' : 'Not a valid .docx file', + ); + } + } +} diff --git a/src/services/userService.test.ts b/src/services/userService.test.ts index 205b1e6..4848471 100644 --- a/src/services/userService.test.ts +++ b/src/services/userService.test.ts @@ -28,48 +28,48 @@ describe('UserService', () => { }); describe('getUserProfile', () => { - it('should fetch user profile with roles and groups', async () => { - const mockProfile = { - User_ID: 1, - User_GUID: '550e8400-e29b-41d4-a716-446655440000', - Contact_ID: 100, - First_Name: 'John', - Nickname: 'Johnny', - Last_Name: 'Doe', - Email_Address: 'john@example.com', - Mobile_Phone: '555-1234', - Image_GUID: 'img-guid-456', - }; - mockGetTableRecords - .mockResolvedValueOnce([mockProfile]) - .mockResolvedValueOnce([{ Role_Name: 'Admin' }, { Role_Name: 'Editor' }]) - .mockResolvedValueOnce([{ User_Group_Name: 'Staff' }]); + const row = { + First_Name: 'John', + Nickname: 'Johnny', + Last_Name: 'Doe', + Email_Address: 'john@example.com', + }; + + it('fetches only the display fields, in a single query', async () => { + mockGetTableRecords.mockResolvedValueOnce([row]); const service = await UserService.getInstance(); const result = await service.getUserProfile('550e8400-e29b-41d4-a716-446655440000'); - expect(mockGetTableRecords).toHaveBeenCalledTimes(3); + expect(mockGetTableRecords).toHaveBeenCalledTimes(1); expect(mockGetTableRecords).toHaveBeenCalledWith({ table: 'dp_Users', filter: "User_GUID = '550e8400-e29b-41d4-a716-446655440000'", - select: expect.stringContaining('User_ID'), + select: 'Contact_ID_TABLE.First_Name,Contact_ID_TABLE.Nickname,Contact_ID_TABLE.Last_Name,Contact_ID_TABLE.Email_Address', top: 1, }); - expect(mockGetTableRecords).toHaveBeenCalledWith({ - table: 'dp_User_Roles', - filter: 'User_ID = 1', - select: 'Role_ID_TABLE.Role_Name', - }); - expect(mockGetTableRecords).toHaveBeenCalledWith({ - table: 'dp_User_User_Groups', - filter: 'User_ID = 1', - select: 'User_Group_ID_TABLE.User_Group_Name', - }); - expect(result).toEqual({ - ...mockProfile, - roles: ['Admin', 'Editor'], - userGroups: ['Staff'], - }); + expect(result).toEqual(row); + }); + + it('never passes through fields beyond the profile DTO', async () => { + mockGetTableRecords.mockResolvedValueOnce([ + { ...row, User_ID: 1, Mobile_Phone: '555-1234', roles: ['Administrators'] }, + ]); + + const service = await UserService.getInstance(); + const result = await service.getUserProfile('550e8400-e29b-41d4-a716-446655440000'); + + expect(Object.keys(result!).sort()).toEqual(['Email_Address', 'First_Name', 'Last_Name', 'Nickname']); + }); + + it('normalizes a missing email to null', async () => { + const { Email_Address: _omit, ...noEmail } = row; + mockGetTableRecords.mockResolvedValueOnce([noEmail]); + + const service = await UserService.getInstance(); + const result = await service.getUserProfile('550e8400-e29b-41d4-a716-446655440000'); + + expect(result!.Email_Address).toBeNull(); }); it('should return undefined when user not found', async () => { @@ -82,31 +82,10 @@ describe('UserService', () => { expect(mockGetTableRecords).toHaveBeenCalledTimes(1); }); - it('should return empty arrays when user has no roles or groups', async () => { - const mockProfile = { - User_ID: 2, - User_GUID: '660e8400-e29b-41d4-a716-446655440001', - Contact_ID: 200, - First_Name: 'Jane', - Nickname: 'Jane', - Last_Name: 'Smith', - Email_Address: 'jane@example.com', - Mobile_Phone: null, - Image_GUID: null, - }; - mockGetTableRecords - .mockResolvedValueOnce([mockProfile]) - .mockResolvedValueOnce([]) - .mockResolvedValueOnce([]); - + it('rejects a malformed GUID before querying', async () => { const service = await UserService.getInstance(); - const result = await service.getUserProfile('660e8400-e29b-41d4-a716-446655440001'); - - expect(result).toEqual({ - ...mockProfile, - roles: [], - userGroups: [], - }); + await expect(service.getUserProfile("x' OR 1=1 --")).rejects.toThrow(); + expect(mockGetTableRecords).not.toHaveBeenCalled(); }); it('should propagate errors from MPHelper', async () => { diff --git a/src/services/userService.ts b/src/services/userService.ts index 3c3e974..9b7c468 100644 --- a/src/services/userService.ts +++ b/src/services/userService.ts @@ -45,47 +45,32 @@ export class UserService { } /** - * Retrieves a user profile by User GUID from Ministry Platform - * - * Fetches user information including: - * - User GUID - * - Contact details (First Name, Nickname, Last Name) - * - Email Address - * - Mobile Phone - * - Profile Image GUID - * + * Retrieves the profile shown to the signed-in user, by User GUID. + * + * Returns only the fields the UI renders (see `MPUserProfile`): names and + * email. The row is mapped explicitly so nothing else the query returns can + * reach the client. + * * @param id - The User GUID to search for - * @returns Promise - The user profile data from Ministry Platform + * @returns Promise - The profile, or undefined if no such user * @throws Will throw an error if the Ministry Platform query fails */ public async getUserProfile(id: string): Promise { const records = await this.mp!.getTableRecords({ table: "dp_Users", filter: `User_GUID = '${validateGuid(id)}'`, - select: "User_ID, User_GUID, Contact_ID_TABLE.First_Name,Contact_ID_TABLE.Nickname,Contact_ID_TABLE.Last_Name,Contact_ID_TABLE.Email_Address,Contact_ID_TABLE.Mobile_Phone,Contact_ID_TABLE.dp_fileUniqueId AS Image_GUID", + select: "Contact_ID_TABLE.First_Name,Contact_ID_TABLE.Nickname,Contact_ID_TABLE.Last_Name,Contact_ID_TABLE.Email_Address", top: 1 }); - const profile = records[0]; - if (!profile) return undefined; - - const [roleRecords, groupRecords] = await Promise.all([ - this.mp!.getTableRecords<{ Role_Name: string }>({ - table: "dp_User_Roles", - filter: `User_ID = ${profile.User_ID}`, - select: "Role_ID_TABLE.Role_Name", - }), - this.mp!.getTableRecords<{ User_Group_Name: string }>({ - table: "dp_User_User_Groups", - filter: `User_ID = ${profile.User_ID}`, - select: "User_Group_ID_TABLE.User_Group_Name", - }), - ]); + const row = records[0]; + if (!row) return undefined; return { - ...profile, - roles: roleRecords.map((r) => r.Role_Name), - userGroups: groupRecords.map((g) => g.User_Group_Name), + First_Name: row.First_Name, + Nickname: row.Nickname, + Last_Name: row.Last_Name, + Email_Address: row.Email_Address ?? null, }; } -} \ No newline at end of file +}