M21: [Major] API input hardening — NaN limits, JSON parse, settings/role/rule validation, error shape

[curtyo18]

Summary

Four related gaps across packages/engine/src/api/server.ts. Bundled because they're all about the API's defenses against malformed input and inconsistent error responses; one PR can address them.

A. Non-finite limit guards

Some endpoints already use the safe Number.isFinite(rawLimit) ? rawLimit : default pattern (server.ts:329, 331, 550, 552), but two don't:

• server.ts:203-216 — /api/files: limit = Math.min(parseInt(...), 1000) with no Number.isFinite guard. ?limit=abc produces NaN, binds NaN into LIMIT ?, which sqlite coerces to 0 (silently empty) or throws.
• server.ts:735 — /api/batches: same shape.

B. Unhandled JSON parse failures

Several handlers do await c.req.json() with no .catch. Malformed JSON throws past the route, bypasses the per-route error shape, and falls into Hono's default error path with a generic 500. Inconsistent with server.ts:449, 677, 716 which already use .catch(() => ({})). Sites missing the catch: server.ts:336-352, 488, 522, 544, 563, 594, 629, 699-712.

C. PUT /api/settings / POST /api/roles / POST /api/rules accept unvalidated bodies

• server.ts:425-430 — PUT settings writes body.settings verbatim into the catalog with no schema validation; a malformed payload corrupts throttleProfiles and propagates through onSettingsChanged to the scheduler.
• server.ts:489, 503 — Role create/update bodies cast as CreateRoleInput/UpdateRoleInput with no runtime validation. A malformed body like drivePriority: "not-an-array" reaches SQL.
• server.ts:522-526 — POST /api/rules has no try/catch; a rules.create() validation throw becomes a 500 instead of a 400 with the schema reason.

D. Internal-error leak + inconsistent error shape

• server.ts:687-689, 497, 511 — (err as Error).message returned to the client for any thrown error. Leaks internal stack-bearing detail and conflates "user error" with "server error."
• server.ts:587-591, 622-626, 667-672 — Non-DriveError handlers throw err, which surfaces as a default Hono 500 with no JSON body. Different shape for "operation failed" depending on the route.

Background

From the 2026-05-17 multi-agent full-repo review.

Acceptance criteria

A. Limit guards

• /api/files and /api/batches use the same Number.isFinite(rawLimit) ? rawLimit : default pattern as the other endpoints.
• Test added: GET /api/files?limit=abc returns either a documented default or 400 — never NaN-bound SQL.

B. JSON parse

• Extract a parseJsonBody<T>(c): Promise<T | { error: 'invalid-json' }> helper that does await c.req.json().catch(() => null).
• All POST/PUT/PATCH handlers go through the helper. A null parse result returns 400 { error: 'invalid-json' }.
• Test added: send Content-Type: application/json body that isn't valid JSON to one representative endpoint — assert 400.

C. Settings/roles/rules validation

• Validate PUT settings body against the Settings shape (zod or hand-written guard) before save. On failure: 400 with the path that failed.
• Role create/update routes call a runtime validator instead of casting. Same shape on failure.
• POST /api/rules wraps rules.create() in try/catch and translates RuleError/ValidationError to 400; falls through to generic 500 only for unknown errors.

D. Error shape consistency

• Install app.onError(...) at the framework level: known typed errors (Rule/Drive/Quarantine/Integrity/Catalog/Scan) → mapped statuses with a structured body { code, error }; unknown errors → 500 with { error: 'internal' } and the original logged at error level.
• The (err as Error).message-to-client pattern at :497, :511, :687-689 is replaced by the centralized handler.
• Test added: throw an Error('internal secret detail') from one of the routes — assert response body does NOT contain the message verbatim.
• Test added: throw a RuleError('INVALID_GLOB', ...) — assert response is 400 with code: 'INVALID_GLOB'.

Files affected (likely)

• packages/engine/src/api/server.ts
• packages/engine/src/api/server.test.ts
• Possibly packages/shared/src/types.ts — validators for Settings, CreateRoleInput, UpdateRoleInput
• packages/engine/package.json — if zod is chosen, add as dep (already a candidate in standards/dependency-discipline.md)

Suggested approach

Order: D (the central handler) → B (helper) → C (validators) → A (one-line fixes). Doing D first means C and B can be smaller.

Out of scope

• AbortSignal/timeout on requests (the UI's api/client.ts lack of timeout is a separate, UI-scoped issue — excluded from this issue per instruction).
• Auth (loopback-only; not in scope per spec §11).
• Rate limiting (loopback-only).

References

• Code: packages/engine/src/api/server.ts
• Spec §11 — API design