OMS Web

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

GATE 5 — QUALITY & SECURITY AUDIT (Next.js/Tailwind standalone)

Ticket : #3248 — OMS UI/UX and functional

Branch : feat/3248-oms-ui-ux-and-functional

Commits : 5214e01 (Data Layer) .. e8fb724 (Dead Code) [4 commits]

Node : W14_gate_quality_audit

Result : PASS (0 unresolved P1 IN THIS TICKET'S AUDITED SCOPE)

^^ read section 5 before trusting this word — 2 P1s exist in this

repo right now, both OUTSIDE this ticket's changed files.

Counts : P1=2 (both out-of-scope) P2=11 (7 in-scope, 4 out) P3=12

(+ 2 clean-audit records, 1 positives record)

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

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

0. SKILL RESOLUTION — READ THIS FIRST

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

The gate's §"Skill resolution" defines three paths per skill. ALL THREE FAILED

FOR ALL SEVEN SKILLS in this checkout — identical to the #3246 run, re-verified

rather than assumed:

path 1 /nextjs-tailwind-<name> -> no such slash command in this session

path 2 /<name> (repo's own) -> no such slash command in this session

path 3 read_file(".claude/skills/nextjs-tailwind-<name>/SKILL.md")

$ ls /opt/drcarrot-deploy/cross-repo-orchestration/oms-web/.claude

-> No such file or directory (oms-web has NO .claude/ at all)

$ find /opt/drcarrot-deploy -type d -name "nextjs-tailwind-*"

-> gates/nextjs-tailwind-standalone (this gate file's own dir)

-> nothing else

$ find /opt/drcarrot-deploy -type d -name skills -not -path "*/node_modules/*"

-> cross-repo-orchestration/dr.carrot-v3-react/.claude/skills

-> cross-repo-orchestration/emr_v1_l12/.claude/skills

-> /opt/drcarrot-deploy/.claude/skills

none of the three contains ANY nextjs-tailwind-* audit skill

(listed in full; the closest is dr.carrot-v3-react's generic

`security-audit`, which is a different stack's skill)

Per the gate's resolution rule 4 ("say so in your report ... do not skip it

silently"), this is recorded rather than papered over. Each of the seven audit

DIMENSIONS was then executed directly against the real files, using the

dimension list the gate file itself enumerates in §"Skills invoked". That is

NOT equivalent to running the skills' own tuned checklists — a finding those

SKILL.md files would have caught and this pass did not is a plausible gap.

Treat the coverage below as gate-file-scoped, not skill-scoped.

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

1. REAL COMMANDS RUN (actual output, not predicted)

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

$ npx tsc --noEmit -p tsconfig.json

-> exit 0, zero diagnostics. tsconfig has "strict": true.

$ npx next lint --max-warnings=0

-> "✔ No ESLint warnings or errors"

$ npx vitest run --reporter=basic

-> Test Files 86 passed (86)

Tests 630 passed (630)

Duration 226.22s

(no failures, no stderr warnings)

$ npx next build

-> ✓ Compiled successfully; 23/23 static pages generated.

First Load JS shared by all 87.3 kB

Middleware 30.5 kB

Routes relevant to the findings below:

/expense-summary 1.55 kB 112 kB First Load

/expense-summary/[project] 1.95 kB 112 kB

/expenses 2.29 kB 112 kB

/orders 2.29 kB 112 kB

/orders/[id] 3.92 kB 91.2 kB

/admin/items 538 B 114 kB

/admin/items/[id] 2.39 kB 93.5 kB

/admin/users 538 B 114 kB

/admin/users/[id] 137 B 91.3 kB

$ ls -la .next/static/css/ ; gzip -c .next/static/css/*.css | wc -c

-> one stylesheet, 26,836 B raw / 5,619 B gzipped, whole app

SCOPE NOTE. `feature_files` (56 entries) is the audit input. This ticket's real

diff (5214e01^..e8fb724, 94 files) also touches data-layer files that are NOT in

feature_files but ARE this ticket's own code: lib/actions/uom.ts,

lib/queries/masters.ts, lib/quick-create/registry.ts, lib/validation/schemas.ts,

prisma/schema.prisma + migration, vitest.config.ts. Because the gate calls the

security audit "first-party, not a second-hand contract check" for this repo,

those were audited too and are tagged `diff-scope`. Findings in files touched by

NEITHER list are tagged `out-of-scope` and are explicitly excluded from

gate_status — see section 5.

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

2. MERGED FINDINGS

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

### [security] SEC-1 — P1 — OUT-OF-SCOPE (pre-existing)

file: app/api/uploads/route.ts:96-101 (POST)

scope: `git log --oneline 5214e01^..e8fb724 -- app/api/uploads/route.ts` -> 0

commits. The two call sites (ExpenseForm.handleUpload,

OrderFulfillmentForm.handleUpload) ARE in feature_files but their upload

code paths are untouched by this ticket's diff.

problem:

Missing OBJECT-LEVEL authorization before a database write. The route checks

`checkPermission("attachment:upload")` — a coarse "may this user upload at

all" — then creates an Attachment row using the caller-supplied `orderId` /

`expenseItemId` verbatim. Nothing verifies the caller may write to THAT order

or THAT expense item. Any holder of `attachment:upload` can attach an

arbitrary file to any order or any expense item id they can guess or observe.

This is precisely the case the gate calls out: "missing authorization checks

before a database write".

fixed: NO — the gate's Step 3 forbids auto-fixing an authorization gap

("never guess at the intended permission"). Needs the intended policy: is it

order:update? expense:update? ownership-scoped? That is a product decision.

### [security] SEC-2 — P1 — OUT-OF-SCOPE (carried forward from the #3246 gate, STILL OPEN)

file: lib/actions/users.ts:55-75 (updateUser)

scope: not in feature_files; 0 commits in this ticket's diff.

problem:

docs/gate-evidence/quality-audit-3246-user-management.txt raised this as its

single blocking P1 ("updateUser has NO last-active-Super-Admin guard"). It is

still unresolved. #3247 hardened the NEIGHBOURING paths but not this one:

- deleteUser (users.ts:89-92) gained an absolute "Super Admin accounts

cannot be deleted" check on the TARGET's current role.

- updateUser gained only `assertNotSuperAdminRole(data.roleId)` — which

inspects the INCOMING roleId, i.e. it blocks PROMOTING someone

to Super Admin. It does not look at the target's CURRENT role

and does not look at `isActive` at all.

So both bricking paths the #3246 evidence documented remain open: submitting a

normal roleId for a user who currently holds the Super Admin role demotes

them, and submitting isActive=false deactivates them. Verified: `sed -n

'55,76p' lib/actions/users.ts | grep "isSuperAdmin\|isActive\|target"` returns

nothing, and there is no DB-level protection either (`grep -rin superadmin

prisma/schema.prisma prisma/migrations/` finds only the plain

`isSuperAdmin BOOLEAN NOT NULL DEFAULT false` column, no CHECK, no trigger).

Why it bricks the app is unchanged — see the #3246 file, section SEC-1.

fixed: NO — authorization/policy rule, never auto-fixed.

### [code-quality] CQ-1 — P2 — diff-scope

file: vitest.config.ts:14-96 (coverage.include)

problem:

`coverage.include` is a hand-maintained allowlist of ~80 individual file paths

with hand-escaped bracket regexes. Anything absent from it is silently

excluded from the coverage report — coverage reads "high" while untracked

modules contribute nothing. Verified absent:

lib/actions/uom.ts <- CHANGED this ticket (quick-create wiring)

lib/quick-create/registry.ts<- CHANGED this ticket (uom entry added)

lib/actions/orders.ts lib/actions/expenses.ts

app/api/uploads/route.ts middleware.ts

(`grep -c` for all six against vitest.config.ts -> 0)

Concrete consequence: this ticket ADDED tests/actions-uom.test.ts (177 lines)

and extended tests/quick-create-registry.test.ts, and neither moves the

reported coverage number by a single line, because their subjects aren't in

the include list. The repo's own CLAUDE.md rule — "when a script can drop

coverage ... make it log/warning that, so silent truncation never reads as

'everything succeeded'" — is the same principle.

fixed: NO — changing the include list changes the reported coverage number and

could fail a downstream coverage gate; that is an observable-behaviour change,

not a safe auto-fix.

### [code-quality] CQ-2 — P2 — in-scope

file: components/forms/OrderFulfillmentForm.tsx:59-67 (handleCancel)

problem:

`cancelOrder` returns `ActionState` (`{ok:false, message}` on failure — see

lib/actions/orders.ts:79-85), and handleCancel discards the return value

entirely, then calls `router.refresh()` unconditionally. A failed cancel is

indistinguishable from a successful one: no message, no error state, the row

just re-renders unchanged. Every other action surface in this ticket wires

failures through (`state?.message` -> `.field-error`); this one does not.

Secondary: "Cancel Order" is destructive and fires immediately on click, while

every row-level delete in the same ticket routes through DeleteConfirmModal

(components/ui/RowActions.tsx:51-58). Inconsistent destructive-action UX.

fixed: NO — adding error surfacing/confirmation changes observable behaviour.

### [dry-architecture] DRY-1 — P2 — in-scope

files: 9 list pages, all in feature_files —

app/admin/{items,users,projects,suppliers,uom}/page.tsx

app/{orders,expenses}/page.tsx

app/expense-summary/page.tsx, app/expense-summary/[project]/page.tsx

problem:

The identical `buildHref` closure is pasted into all nine (18 grep hits for

`buildHref`, 9 for `const buildHref`):

const buildHref = (extra) => {

const p = new URLSearchParams();

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

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

return `<route>?${p.toString()}`;

};

Only the route literal differs. lib/params.ts is already the shared home for

exactly this concern (it owns parseListParams/skipTake/orderBy) — a

`buildListHref(pathname, params)` there would delete nine copies.

fixed: NO — mechanical, but nine call sites in a file set this gate is auditing

rather than authoring; left as a reported finding.

### [dry-architecture] DRY-2 — P2 — in-scope

files: components/forms/{Supplier,Project,Uom,Item}Form.tsx (all in feature_files)

problem:

The four quick-create-capable forms carry a byte-identical embedded scaffold:

- `const initialState: ActionState = { ok: true };` 8 copies repo-wide

- the onSuccess effect, character-for-character identical in all 4:

`if (embedded && state.ok && state.data && onSuccess) onSuccess(state.data);`

each preceded by its own `// eslint-disable-next-line react-hooks/exhaustive-deps`

- `const fieldWidth = embedded ? "w-full" : "w-full sm:w-[350px]";`

- `{embedded && <input type="hidden" name="_quickCreate" value="1" />}`

- the `{!embedded && <h2 …>X Details</h2>}` heading

- the Cancel/Submit footer, differing only in label and redirect path

A `useEmbeddedForm({embedded, state, onSuccess})` hook plus an

`<EmbeddedFormShell>` would collapse this to one definition. The cost is

already visible: the eslint suppression is duplicated 4× rather than justified

once (see CQ-7).

fixed: NO — a shared hook + shell touches four live form components' render

output; too broad for an unreviewed auto-fix.

### [dry-architecture] DRY-3 — P2 — in-scope

files: components/forms/ExpenseForm.tsx:36-44 (in feature_files)

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

lib/validation/schemas.ts:134-142 (diff-scope)

problem:

The expense-type list exists in THREE places: twice as an identical

value/label option array, and a third time as the Zod `z.enum([...])` that

actually validates it. Adding a type means editing three files; missing one

yields either a type the UI offers but the server rejects, or a stored type

the summary screen renders as a raw enum string

(`EXPENSE_TYPE_OPTIONS.find(...)?.label ?? r.type` — the fallback silently

papers over exactly that drift).

fixed: NO — the schema copy is the validation source of truth; deriving the

other two from it is the right fix but changes what the Zod enum exports.

### [performance] PERF-1 — P2 — in-scope

files: components/ui/ListToolbar.tsx:2, components/ui/FilterBar.tsx:7-8

(both in feature_files)

problem, with measured evidence:

ListToolbar statically imports FilterBar; FilterBar statically imports

Listbox and DatePicker (a hand-rolled ~270-line calendar). ListToolbar only

RENDERS FilterBar when `filters && filters.length > 0` — but a runtime

conditional does not prune a module-scope client import from the route's

client bundle.

Measured against the build above:

app baseline (First Load JS shared by all) 87.3 kB

/expense-summary 112 kB <- passes NO filters

/admin/items (passes filters) 114 kB

/admin/items/[id] (no ListToolbar) 93.5 kB

`/expense-summary` renders `<ListToolbar search={...} />` with no `filters`

prop (app/expense-summary/page.tsx:45) and can never display a filter field,

yet ships ~25 kB of First Load JS over baseline — within 2 kB of the fully

filtered list pages. `next/dynamic` on FilterBar (or on DatePicker inside it)

would let the routes that never filter stop paying for the calendar.

fixed: NO — converting to a dynamic import changes hydration/loading behaviour

on every list screen; wants a ui_verify pass behind it.

### [performance] PERF-2 — P2 — in-scope

files: app/orders/[id]/page.tsx:7-14 (in feature_files)

app/expense-summary/[project]/page.tsx:21-33 (in feature_files)

problem:

Both routes fetch the same record twice per render — once in

`generateMetadata`, once in the page body:

orders/[id] getOrder(params.id) at :8 and :14

(getOrder includes items -> item, uom, attachments)

expense-summary/[p] prisma.project.findUnique at :22 and :33

Next.js dedupes `fetch()` across metadata and render, but NOT Prisma calls.

Neither `getOrder` nor the inline findUnique is wrapped in React `cache()`.

This is not a pattern the codebase is unaware of — lib/queries/rbac.ts:38

wraps `getUserWithPermissions` in `cache()` precisely for this reason, and

documents it. These two are the only `generateMetadata` implementations in

the app (`grep -rln generateMetadata app/` -> exactly these 2 files), so the

fix is two `cache()` wrappers.

fixed: NO — trivially correct, but it changes request-scoped caching semantics

for a query used on a live route; reported for review instead.

### [performance] PERF-3 — P2 — OUT-OF-SCOPE (pre-existing)

file: lib/queries/expenses.ts:49-80 (expenseSummaryByProject)

consumed by app/expense-summary/page.tsx:18 (in feature_files)

problem:

Unbounded full-table read on every page view:

- `prisma.expenseItem.groupBy({ by:["expenseId"], _sum:{amount:true} })`

with NO `where` — every ExpenseItem row in the database.

- `prisma.expense.findMany({ select:{...} })` with NO `where`, NO `take` —

every Expense row.

- the project rollup, the `params.q` search and the sort then all run in JS

over the full set.

The consuming page destructures only `{ rows }` and renders no `<Pagination>`,

so the whole table is also rendered. Cost grows linearly with total expense

volume forever. Compare the sibling `expenseItemsForProject` (same file,

:84-113) which does it correctly — filtered `where`, `skipTake`, and a SQL

`aggregate` for the total. A `groupBy(["projectId"])` on Expense joined to a

SQL sum would fix it.

scope note: surfaced through an in-scope file but the query itself is untouched

by this ticket (`app/expense-summary/page.tsx` is +2 lines: the ListToolbar

swap). Not counted against gate_status.

fixed: NO — rewriting an aggregation query is a behaviour change needing data

verification.

### [security] SEC-3 — P2 — OUT-OF-SCOPE (pre-existing)

files: app/api/uploads/route.ts:73-82, lib/permissions/registry.ts:104-125

problem:

Uploaded files are written to `public/uploads/` and served as static assets.

`ROUTE_PERMISSIONS` has an entry for `/api/uploads` (the write path) but NONE

for `/uploads/...` (the read path). middleware.ts's matcher does cover

`/uploads/*`, so a session is required — but `resolveRoutePermission()`

returns null, so NO permission is required. Any authenticated user of any role

can fetch any invoice/receipt by URL, regardless of their order/expense

permissions. Filenames are `Date.now()-<sanitised original>`, which is

guessable-adjacent, and URLs leak through the rendered AttachmentChip hrefs.

Related deployment note: writing to `public/` at runtime is not durable under

a containerised/standalone or multi-replica deploy — `public/` is captured at

build time.

fixed: NO — authorization policy.

### [security] SEC-4 — P2 — OUT-OF-SCOPE (pre-existing)

files: lib/actions/orders.ts:12-16 + :48-49, lib/actions/expenses.ts:11-15

problem:

`JSON.parse((raw.itemsJson as string) || "[]")` and the `linesJson`

equivalent run BEFORE Zod and are unguarded. Malformed JSON throws inside the

Server Action before `safeParse` ever sees it, so the caller gets an

unhandled-exception 500 instead of the `{ok:false, fieldErrors}` contract

every other failure path returns. The inputs are hidden form fields

(OrderForm.tsx:110, ExpenseForm.tsx:197, OrderFulfillmentForm.tsx:126) — i.e.

fully attacker-controlled on a public Server Action entry point. Not an

injection (Prisma is parameterised throughout — see the positives record), but

a trivially-reachable unhandled error path.

fixed: NO — wrapping in try/catch changes the response shape on a live path.

### [security] SEC-5 — P2 — OUT-OF-SCOPE (pre-existing)

file: lib/actions/orders.ts:79-85 (cancelOrder)

problem:

No status precondition and no existence check — it goes straight to

`prisma.order.update({ data:{ status: CANCELLED } })`. A FULFILLED order can

be cancelled. The client hides the button for CANCELLED orders only

(`readOnly = order.status === "CANCELLED"`, OrderFulfillmentForm.tsx:57), and

a Server Action is a public entry point regardless of what the form renders —

the same reasoning lib/actions/users.ts:27-32 already spells out for the

Super Admin role. The state machine is enforced nowhere.

fixed: NO — the legal transition set is a product decision.

### [code-quality] CQ-3 — P3 — in-scope

files: components/forms/ExpenseForm.tsx:87-111,

components/forms/OrderFulfillmentForm.tsx:73-94

problem:

`handleUpload` wraps `fetch` in `try { … } finally { … }` with no `catch`, and

is invoked un-awaited from an onChange handler

(`if (file) handleUpload(index, file);`). On a network failure the promise

rejects with nobody attached: an unhandled rejection, `uploadError` never set,

and the spinner state does reset — so the UI silently returns to "Upload" as

though nothing happened. The non-2xx path IS handled; only the thrown path

is not.

### [code-quality] CQ-4 — P3 — in-scope

file: app/expense-summary/[project]/page.tsx:39-61, 77-82

problem:

`buildHref` is defined and passed to DataTable as `buildSortHref`, but not one

of the four columns sets `sortable: true`. DataTable only calls buildSortHref

inside `col.sortable ? … : …` (DataTable.tsx:55-65), so the closure is

constructed on every render and never invoked. Either dead code, or a missing

`sortable` flag — the neighbouring app/expense-summary/page.tsx DOES mark its

projectName column sortable, which suggests the latter. Flagging rather than

guessing which. (The W13 dead-code gate did not catch this: the symbol IS

referenced, just never reachable.)

### [code-quality] CQ-5 — P3 — in-scope

files: components/ui/fields/TextField.tsx:34-36+48-56,

components/ui/fields/SelectField.tsx:40-43 (both in feature_files)

problem:

Both accept `required` and use it ONLY to append " *" to the visible label.

Neither sets the `required` attribute nor `aria-required` on the control. So

the asterisk is decorative: no native browser validation, and a screen reader

gets no required semantics — it hears the literal "*" in the label if

anything. Validation is genuinely server-side (Zod -> fieldErrors), which is

correct as the boundary, but the accessibility signal is simply missing.

Affects every field on every master/user form in this ticket.

### [code-quality] CQ-6 — P3 — in-scope

files: the 9 list pages of DRY-1, vs components/ui/FilterBar.tsx:31-44

problem:

Inconsistent pagination reset. `buildHref` spreads `...params`, which

parseListParams populates with the CURRENT `page` (lib/params.ts:34,41-50), so

a sort link built on page 3 emits `page=3` — re-sorting keeps you on page 3 of

a completely different ordering. FilterBar, by contrast, explicitly calls

`params.set("page", "1")` on both `setParam` and `clearAll`. Same list screen,

two different rules for what a control does to the current page.

### [code-quality] CQ-7 — P3 — in-scope

files: components/forms/{Supplier,Project,Uom,Item}Form.tsx

problem:

4 identical `// eslint-disable-next-line react-hooks/exhaustive-deps`

suppressions on the same `[state]` effect. Only SupplierForm.tsx:46 actually

explains why ("onSuccess/embedded are stable per mount"); the other three

copy the directive without the justification. A suppression duplicated four

times is a signal the pattern wants extracting (see DRY-2), and three of the

four now read as unexplained.

### [dry-architecture] DRY-4 — P3 — in-scope

files: app/admin/{items,users,projects,suppliers,uom}/page.tsx

problem:

The Active/Inactive status filter descriptor is re-declared inline in each

admin list page — identical `{name:"status", label:"Status", type:"select",

options:[{value:"active",…},{value:"inactive",…}]}`. A shared

`STATUS_FILTER_FIELD` constant would do. (3 verbatim hits for the option pair;

the other two differ only in surrounding whitespace.)

### [css-dry] CSSDRY-1 — P3 — in-scope

files: components/forms/{Supplier,Project,Uom,Item,User}Form.tsx

problem:

`className="flex flex-wrap gap-x-6 gap-y-4 max-w-[772px]"` — the de-carded

field-grid — repeated verbatim 5×, including the arbitrary `max-w-[772px]`.

app/globals.css already hosts exactly this kind of shared form primitive

(`.field-input`, `.field-label`, `.field-error`, `.field-control`); a

`.field-grid` there would be consistent with the file's own established

pattern and put the 772px design constant in one place.

### [css-dry] CSSDRY-2 — P3 — in-scope

files: the 5 forms above (6 occurrences of the literal)

problem:

`w-full sm:w-[350px]` is hardcoded 6× — 5 as the `fieldWidth` ternary's

non-embedded branch, once as UserForm.tsx:56's unconditional `const

fieldWidth`. Second undocumented design constant duplicated across files

(see CSSDRY-1). Both TextField and SelectField document this prop as

"Ticket #3248 de-carded form layout", so the value is load-bearing, not

incidental.

### [css-dry] CSSDRY-3 — P3 — in-scope

files: 11 occurrences across components/forms/* and components/ui/*

problem:

`text-sm font-semibold text-accent` — the de-carded section heading — repeated

11×. Same remedy as CSSDRY-1: a `.section-title` in the `@layer components`

block that already exists.

### [css-dry] CSSDRY-4 — P3 — in-scope

files: components/ui/RowActions.tsx:33,37,44 (3×);

app/expenses/ExpenseRowActions.tsx:13; app/orders/OrderRowActions.tsx:12

problem:

The icon-action hit-target cluster `p-1.5 -m-1.5 text-<token>` is repeated 6×

across three components that this ticket's own doc comments describe as

deliberately mirroring each other ("mirrors the 5 admin masters'

*RowActions.tsx icon-button pattern"). The negative-margin/padding trick that

makes the touch target work is the fragile part, and it is copy-pasted rather

than named.

### [css-performance] CSSPERF-1 — P3 — in-scope

file: tailwind.config.ts:59-60 (in feature_files)

problem:

Two design tokens are defined and referenced nowhere:

accent.selected "#e2cdb0" ("selected tab-pill segment")

accent.subtleHover "#f0e8dc" ("secondary/Cancel button hover")

Verified: `grep -rn "accent-selected\|accent-subtleHover"` across all

.ts/.tsx/.css outside node_modules/.next -> NO REFERENCES (only the config

definitions themselves). Confirmed zero runtime cost — `grep -c

"e2cdb0\|#f0e8dc\|226,205,176\|240,232,220" .next/static/css/*.css` -> 0,

i.e. Tailwind's JIT correctly never emitted them. So this is dead

configuration, not CSS bloat — hence P3, not P2. Both were introduced by

#3247 for design elements that apparently never got built.

(The W13 dead-code gate would not catch these: they are config values, not

imported symbols.)

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

3. CLEAN-AUDIT RECORDS (audits that ran and found nothing to report)

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

### [animation-performance] — CLEAN

Exhaustive grep for `transition|animate-|duration-|@keyframes` across

app/globals.css + components/ui + components/forms + app/{admin,expenses,orders}

returns exactly three animated things in the whole audited surface:

app/globals.css:13 `.btn { … transition-colors … }`

components/ui/fields/Listbox.tsx:178 `transition-transform` + `rotate-180`

components/ui/fields/DatePicker.tsx:270 `transition-colors` on day cells

`transition-colors` animates paint-only properties; `transition-transform` with

`rotate-180` is compositor-only. There are no width/height/top/left/margin

transitions, no `@keyframes`, no JS-driven animation loops, no

scroll/resize-driven style writes, and therefore no layout-thrash path. Nothing

to report — this is not an unrun audit.

### [css-performance] — CLEAN apart from CSSPERF-1

Whole-app stylesheet: 26,836 B raw / 5,619 B gzipped, one file, no

render-blocking imports beyond it. No `!important` anywhere; no ID selectors;

no descendant-selector chains — all styling is JIT-generated utilities plus 9

hand-written classes in a single `@layer components` block, which is the

correct Tailwind layering (they compile into the components layer, so utilities

still win without specificity hacks). 20 arbitrary-value utilities (`[NNNpx]`)

across the audited .tsx files; each generates one rule, so they cost bytes but

create no specificity or unused-CSS problem. No unused-CSS finding is possible

by construction here — Tailwind emits only what `content:` matches, and

CSSPERF-1 confirms that mechanism working correctly.

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

4. POSITIVES — verified, not assumed (security audit, in-scope)

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

These were actively checked because the gate calls this repo's security audit

first-party. Each is a real command, not an assumption:

* NO first-party SQL-injection surface.

`grep -rn '\$queryRaw|\$executeRaw|queryRawUnsafe' lib app` -> none.

Every database access in the repo goes through Prisma's parameterised

client. The gate's "including raw SQL escapes if this repo ever drops to

raw queries" case does not arise — it has not dropped to raw queries.

* NO markup-injection surface.

`grep -rn 'dangerouslySetInnerHTML|eval\(|new Function' app components lib`

-> none.

* NO secret leakage to the client.

Every `"use client"` module cross-checked for `process.env` -> none. The

only `process.env` reads are AUTH_SECRET in lib/auth.ts:89 and

lib/actions/auth.ts:21, both server-only, both fail-closed with an explicit

error rather than a silent fallback. No NEXT_PUBLIC_* anywhere.

* Every mutating Server Action in scope opens with `requirePermission(...)`

before touching Prisma — verified line-by-line in items.ts, uom.ts,

users.ts, orders.ts, expenses.ts.

* app/api/lookups/route.ts (CHANGED this ticket) is correctly gated: it

validates `entity` against the registry keys BEFORE use (so the

`QUICK_CREATE_REGISTRY[entity]` index cannot be steered to an arbitrary

property), then requires the matching per-entity `*:read` permission and

returns 401 otherwise. The permission is looked up from a closed

`READ_PERMISSION` record, not derived from user input.

* The ticket's headline new capability — UOM quick-create — inherits

authorization rather than re-implementing it: QUICK_CREATE_REGISTRY's `uom`

entry points at the existing `createUom`, which already calls

`requirePermission("uom:create")`. The registry entry adds no new

unauthenticated path. ItemForm additionally refuses to offer the trigger

while embedded (`canCreate={canCreateUom && !embedded}`), avoiding

modal-from-modal, and every `canCreate*` prop defaults to `false` so an

unpassed prop degrades to "no quick-create" rather than an open control.

* Super Admin role assignment is blocked at the real boundary, not just the

UI: listRolesForSelect excludes it from the dropdown AND

createUser/updateUser both call `assertNotSuperAdminRole` server-side.

(What is missing is the TARGET-side check — that is SEC-2, above.)

* Permission resolution is request-memoised (`cache()` in

lib/queries/rbac.ts:38), so the `<Can>` component appearing many times per

list page does NOT produce an N+1 of permission queries. Checked

specifically because it looked like one.

* The middleware/guard split is genuine defence-in-depth, not duplicated

theatre: the Edge layer reads the cookie snapshot (can only be MORE

restrictive), the Node layer re-reads from the database. Removing either

does not expose a mutation.

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

5. WHY THIS IS A PASS — AND WHAT THAT DOES NOT MEAN

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

The gate rule is "PASS only if zero unresolved P1 findings remain across all

seven audits", and the audits are defined as running "against this ticket's

changed files".

In-scope P1s (feature_files + this ticket's diff): 0 -> PASS

Out-of-scope P1s (files this ticket never touched): 2 -> SEC-1, SEC-2

Both P1s were found by deliberately auditing PAST the ticket boundary, because

the gate flags this repo's security audit as first-party. They are real,

verified, and NOT fixed. Neither is attributable to #3248:

SEC-1 app/api/uploads/route.ts — 0 commits in this ticket's range

SEC-2 lib/actions/users.ts — 0 commits in this ticket's range, and

already the #3246 gate's blocking P1

Reporting them as P1 while passing the gate is a deliberate call, recorded here

so an operator can overrule it: failing #3248 for a defect #3246 introduced and

#3247 failed to close would block this ticket without moving the actual defect,

and the gate's own Step 3 forbids this agent from fixing either one ("A missing

authorization check is never an auto-fix"). SEC-2 in particular has now survived

two gates — it wants its own ticket, not a third silent carry-forward.

If the orchestrator's policy is that ANY open P1 blocks regardless of

attribution, this result should be read as FAIL with 2 P1s.

STEP 3 (auto-fix) WAS NOT PERFORMED — no source file was modified. This node was

invoked read-only by design ("You have no write access, by design"), so even the

clearly-safe candidates (CSSPERF-1's two dead tokens, CQ-4's dead closure) were

left as reported findings rather than applied. The only file written by this

gate is this evidence file. Every finding above is therefore `fixed: NO`, and

that reflects the execution mode, not a judgement that each was unsafe to fix.

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

6. REDMINE NOTE — NOT POSTED

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

The Step 5 curl needs $REDMINE_URL, $TASK_ID and $REDMINE_API_KEY. Only

REDMINE_API_KEY is present in this environment (`env` check: REDMINE_URL and

TASK_ID are both unset), so the documented request cannot be constructed. No URL

and no ticket id were guessed. The note that would have been posted:

"Claude - Opus: Gate 5 — Quality & Security Audit: PASS (0 P1, 11 P2, 12 P3)"

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

7. VERDICT

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

PASS — 0 unresolved P1 in the audited scope; 11 P2 and 12 P3 reported, none

fixed (read-only invocation). Typecheck, lint, 630 unit tests and a production

build all pass against the real tree. Two pre-existing P1 security findings

outside this ticket's scope are recorded in full and need their own ticket;

SEC-2 is the #3246 gate's blocking P1, still open after two subsequent tickets.

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