management: the API has no error handler, so unauthenticated writes answer 500 instead of 401 #70

New issue

Closed

opened 2026-08-28 02:18:40 +00:00 by yuris · 0 comments

yuris commented

2026-08-28 02:18:40 +00:00

Owner

Found by Author59 while fixing #59/#60 on PR #68, and by Reviewer56r1 during PR #56. Explicitly declared out of scope by issue #60, which named it as the reason overflow surfaced badly and asked for it to be filed separately.

The defect

The Hono app mounted at apps/management/app/api/[[...route]]/route.ts has no error handler, so any thrown error flattens to a bare 500 with no readable body. Two measured consequences:

An unauthenticated REST write answers 500 instead of 401. A caller cannot distinguish "you are not signed in" from "the server is broken".
Driver-level errors surface as 500s with Postgres internals rather than a readable rejection. PR #68 closed this per-field by validating at the boundary, but any path that throws for a reason nobody anticipated still lands here.

Why it was deferred, and why that was right

It touches every endpoint, so fixing it inside a field-validation PR would have made that PR unreviewable and would have widened a clean verdict's blast radius. #60 recorded the deferral honestly rather than describing the endpoints as safe — which is the distinction that matters: an inaccurate "this is fine" note stops the next reader from checking, whereas a named deferral invites it.

Scope

Add an onError handler to the Hono app that maps a thrown error to a status and a readable body, without leaking Postgres internals or stack traces to a caller.
Auth failures must answer 401, not 500.
Validation failures already answer readable 4xx after PR #68; confirm the handler does not regress them or swallow the field names.
Decide what an unanticipated throw should say publicly. It must be readable and must not echo driver text.

Boundary validation on the four inventory write paths landed in PR #68 (#59, #60).
The remaining unvalidated write path, createStockAdjustment, is filed separately and produces exactly this bare-500 shape when called outside a browser.
apps/store/lib/catalog.ts has the mirror-image defect on the consumer side — a bare catch that rewrites every throw into one friendly message, masking a Next static-generation bailout. Filed as #66. Worth reading together: one endpoint says nothing useful, one client refuses to hear anything specific.

Verification

Call each write endpoint unauthenticated and assert 401 with a readable body.
Force a driver error and assert the response is readable and leaks no Postgres internals.
Re-run the PR #68 rejection matrix and confirm every readable 4xx and field name survives.

Acceptance

No endpoint answers a bare 500 for an anticipated condition; unauthenticated writes answer 401; no response body leaks driver internals or stack traces; PR #68's readable field-named rejections are unchanged.

Found by `Author59` while fixing #59/#60 on PR #68, and by `Reviewer56r1` during PR #56. Explicitly declared out of scope by issue #60, which named it as the reason overflow surfaced badly and asked for it to be filed separately. ## The defect The Hono app mounted at `apps/management/app/api/[[...route]]/route.ts` has **no error handler**, so any thrown error flattens to a **bare 500** with no readable body. Two measured consequences: - An **unauthenticated** REST write answers **500 instead of 401**. A caller cannot distinguish "you are not signed in" from "the server is broken". - Driver-level errors surface as 500s with Postgres internals rather than a readable rejection. PR #68 closed this per-field by validating at the boundary, but any path that throws for a reason nobody anticipated still lands here. ## Why it was deferred, and why that was right It touches **every endpoint**, so fixing it inside a field-validation PR would have made that PR unreviewable and would have widened a clean verdict's blast radius. #60 recorded the deferral honestly rather than describing the endpoints as safe — which is the distinction that matters: an inaccurate "this is fine" note stops the next reader from checking, whereas a named deferral invites it. ## Scope - Add an `onError` handler to the Hono app that maps a thrown error to a status and a readable body, without leaking Postgres internals or stack traces to a caller. - Auth failures must answer **401**, not 500. - Validation failures already answer readable 4xx after PR #68; confirm the handler does not regress them or swallow the field names. - Decide what an unanticipated throw should say publicly. It must be readable and must not echo driver text. ## Related - Boundary validation on the four inventory write paths landed in PR #68 (#59, #60). - The remaining unvalidated write path, `createStockAdjustment`, is filed separately and produces exactly this bare-500 shape when called outside a browser. - `apps/store/lib/catalog.ts` has the mirror-image defect on the consumer side — a bare `catch` that rewrites *every* throw into one friendly message, masking a Next static-generation bailout. Filed as #66. **Worth reading together: one endpoint says nothing useful, one client refuses to hear anything specific.** ## Verification - Call each write endpoint unauthenticated and assert 401 with a readable body. - Force a driver error and assert the response is readable and leaks no Postgres internals. - Re-run the PR #68 rejection matrix and confirm every readable 4xx and field name survives. ## Acceptance No endpoint answers a bare 500 for an anticipated condition; unauthenticated writes answer 401; no response body leaks driver internals or stack traces; PR #68's readable field-named rejections are unchanged.

yuris referenced this issue

2026-08-28 02:49:24 +00:00

fix(management): refuse unparseable numeric input in both inventory forms; bound buy_price, stock and weight_grams #68

yuris referenced this issue

2026-08-28 02:50:01 +00:00

fix(management): refuse unparseable numeric input in both inventory forms; bound buy_price, stock and weight_grams #68

yuris referenced this issue

2026-08-28 07:21:31 +00:00

management: numeric bounds sit on the transports, so the WhatsApp bot writes inventory unbounded from LLM/OCR output #71

yuris referenced this issue from a pull request that will close it,

2026-08-29 06:36:41 +00:00

fix(management): map thrown API errors to readable statuses instead of a bare 500 #76