fix: close 3 high-severity findings, gate lint + types in CI

[chriskehayias]

Closes all three high-severity TODOs from the coverage review, plus the CI gap that let a type error sit on dev unnoticed.

1. Unvalidated numeric fields reaching MP $filter strings

saveFamily now runs HouseholdSchema.safeParse() before the payload reaches FamilyService. A server action is a public POST endpoint and its TypeScript annotation is erased at runtime, so envelopeNo: number guaranteed nothing.

The rejection reports field paths only (members.0.envelopeNo), never the submitted values — rule 14 applies to returned error strings, not just logs. Authorization still runs first, so an unauthorized caller learns nothing about the schema.

Defence in depth where the string is actually built: resolveUniqueEnvelopeNo validates requested and excludeDonorId, and re-validates candidate each loop iteration so a future change to getNextEnvelopeNumber() can't reintroduce an unchecked value. upsertDonor validates the id it uses to target the Donors update. null and 0 stay valid as the "no donor" sentinels.

Correction to the original report — please read before assessing risk

The classic injection string was already blocked, but incidentally rather than by design. upsertDonor guards on envelopeNo > 0, and JS coerces "1 OR 1=1" to NaN, so that comparison is false and the crafted string never reached the filter.

What did get through: values that survive numeric coercion without being positive integers — 1.5, Infinity, 1e21 (interpolating as the malformed 1e+21), numeric strings. That's malformed filters and confusing 500s, not a filter-injection primitive. The practical severity was below "high" as filed.

The fix still stands: relying on an incidental coercion side effect to block injection is fragile, and a schema change or a new call path that drops the > 0 guard would turn it into the real thing with nothing to catch it.

2. removeGroup silently dropped a non-empty group's fields

The guard existed on groupedFields but not groupOrder, and buildSavePayload() iterates groupOrder — so the fields vanished from the save payload and from live MP page configuration, with no error shown to the admin. The guard now sits at the top of the callback, reading rendered state, so both updates agree.

Worth knowing for review: the obvious fix doesn't work. A let removed = false flag set inside the setGroupedFields updater is still false when setGroupOrder is reached, because React runs updaters during render, not at call time. It would have read as correct and kept the bug. I wrote it that way first; it's recorded in the TODO so it isn't retried.

3. Raw error objects logged household PII

docxtemplater attaches the live merge scope to err.properties.scope on a scope-parser failure — in this feature that scope is the household list, so console.error('...', error) wrote every printable name and mailing address for the batch into server logs.

New describeError() reduces a throw to { name, message } plus docxtemplater's own id/explanation, which carry no caller data. Applied to mergeTemplate, generateLabelPdf, generateLabelDocx.

One extra beyond the three highs: I also fixed the identical anti-pattern in addeditfamily/page.tsx (filed separately as medium, and a file this branch already touches). Shipping a redaction fix that skips the file next door would leave two standards in one codebase. Say the word if you'd rather it were split out.

CI gates

npm run typecheck and npm run lint now run as steps of the existing test job — steps, not new jobs, because branch protection requires the check named test, and separate jobs would be green-but-unrequired until someone also edited the protection rules. A gate that doesn't gate is worse than none.

Two more issues found in the same file:

• npm install → npm ci. npm install resolves fresh versions and rewrites package-lock.json inside CI, so CI could be testing a dependency tree no developer had. Verified with npm ci --dry-run.
• No concurrency group — superseded PR runs burned minutes. Now cancels in-flight runs per ref, except on main and dev, whose runs gate merges and releases and must not be killed.

Plus permissions: contents: read; the workflow only reads the repo, and Codecov uses its own token.

Review pointers

File	What to look at
familyService.ts	+20 lines of validation; check the 0/null sentinel handling
addeditfamily/actions.ts	schema parse; confirm the error leaks no values
use-field-order-state.ts	guard moved to top of callback, groupedFields added to deps
address-labels/actions.ts	new describeError()
.github/workflows/test.yml	steps vs jobs, and the concurrency exemption for the trunks

Everything else is tests (+28) and TODO/doc updates.

Status

TODOs: 9 open → 4 (2 medium, 2 low, all in src/components). No high-severity or security items remain open.

Gate	Result
npm run lint	0 problems
npm run typecheck	0 errors
npm run test:coverage	exit 0 — 116 files, 1,563 tests, 98.85% stmts / 99.70% lines
npm run build	exit 0

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!