fix(admin): PUT /admin/items/:id returns 200 with an empty body for an id that does not exist #207

New Issue

2026-08-28T11:54:25-05:00

bermudalamb commented

2026-08-28 11:54:25 -05:00

backend/src/routes/admin.ts:419 ends the update route with res.json(full[0]). Its sibling, the create route at :371, uses requireRow(full, 'the item just inserted').

So for an id that does not exist: the UPDATE matches zero rows, the transaction commits successfully, the follow-up SELECT returns zero rows, and the route responds 200 with an empty body. The admin client gets a success it can do nothing with, and nothing anywhere records that the item was not found.

requireRow exists precisely for this and is used on the other half of the same pair, so this reads as an oversight rather than a decision.

Fix

Return 404 for a missing id. Worth checking the same shape across the other admin routes while in there — requireRow usage looks inconsistent rather than deliberate.

Note the ordering wrinkle: a non-numeric :id never reaches line 419, because the UPDATE ... WHERE id=$4 at :395 binds the same raw string and Postgres raises 22P02 invalid input syntax for type integer first, which the catch turns into a 500. So a garbage id gives 500 and a well-formed but absent id gives 200 — neither of which is 404. Both are worth fixing together.

Not a security issue

Raised during the S2077 review (#180/#202), which confirmed req.params.id is bound rather than interpolated and cannot inject. This is correctness only.

Found by

Code review of 66ff354 (#180), raised as Minor 4. Pre-existing, not introduced by that change.

`backend/src/routes/admin.ts:419` ends the update route with `res.json(full[0])`. Its sibling, the create route at `:371`, uses `requireRow(full, 'the item just inserted')`. So for an id that does not exist: the `UPDATE` matches zero rows, the transaction commits successfully, the follow-up `SELECT` returns zero rows, and the route responds **200 with an empty body**. The admin client gets a success it can do nothing with, and nothing anywhere records that the item was not found. `requireRow` exists precisely for this and is used on the other half of the same pair, so this reads as an oversight rather than a decision. ## Fix Return 404 for a missing id. Worth checking the same shape across the other admin routes while in there — `requireRow` usage looks inconsistent rather than deliberate. Note the ordering wrinkle: a non-numeric `:id` never reaches line 419, because the `UPDATE ... WHERE id=$4` at `:395` binds the same raw string and Postgres raises `22P02 invalid input syntax for type integer` first, which the catch turns into a 500. So a garbage id gives 500 and a well-formed but absent id gives 200 — neither of which is 404. Both are worth fixing together. ## Not a security issue Raised during the S2077 review (#180/#202), which confirmed `req.params.id` is bound rather than interpolated and cannot inject. This is correctness only. ## Found by Code review of `66ff354` (#180), raised as Minor 4. Pre-existing, not introduced by that change.

bermudalamb referenced this issue from a commit

2026-09-02 09:06:47 -05:00

fix(admin): answer 404 for an item id that does not exist (#207)

bermudalamb referenced this issue from a commit

2026-09-02 09:33:15 -05:00

fix(admin): answer 404 for an item id that does not exist (#207)

bermudalamb referenced this issue from a commit

2026-09-03 10:49:51 -05:00

chore: clear the six code smells SonarQube reported (#181)

bermudalamb referenced this issue

2026-09-03 10:50:42 -05:00

chore: clear the six code smells SonarQube reported (#181) #282