The issue asked for two things and only one had been done. A well-formed but absent id already answered 404 — the PUT route carries a comment saying so. A malformed one still reached Postgres as text, raised 22P02 on an integer column, and surfaced through the route's catch as a 500, telling the admin the server had broken when the truth is that no such item can exist. That half is now closed everywhere rather than on the three routes that happened to have it.
Guarded: DELETE an item, DELETE an image, unpublish, and every route in adminItemDrafts — publish, regenerate, discard, restore, and the two background-removal endpoints added by #281. The last of those were flagged in that feature's own final review as sharing this pre-existing shape, so they are fixed with the rest rather than left to be found again.
Routes carrying two ids guard both. A route can guard the first and forget the second, and the forgotten one fails exactly as loudly, so there is a case each way for both image endpoints and for DELETE image.
DELETE deliberately still answers 204 for a well-formed id that is absent. The method is idempotent and the caller's intent, that the item should not exist, is satisfied either way; what must not happen is a 500. There is a test pinning that so the distinction is a decision rather than an omission.
Also replaced the raw req.params.id and Number(req.params.id) uses that sat inside routes which had already computed a validated id. They were safe, because the guard above them made them safe, but a validated id and a raw one side by side in the same handler is how this bug comes back.
The test block named "a non-numeric id on every admin item route" covered two routes. It now covers every route that takes an id, which is what makes its name true.
The issue asked for two things and only one had been done. A well-formed but absent id already answered 404 — the PUT route carries a comment saying so. A malformed one still reached Postgres as text, raised 22P02 on an integer column, and surfaced through the route's catch as a 500, telling the admin the server had broken when the truth is that no such item can exist. That half is now closed everywhere rather than on the three routes that happened to have it.
Guarded: DELETE an item, DELETE an image, unpublish, and every route in adminItemDrafts — publish, regenerate, discard, restore, and the two background-removal endpoints added by #281. The last of those were flagged in that feature's own final review as sharing this pre-existing shape, so they are fixed with the rest rather than left to be found again.
Routes carrying two ids guard both. A route can guard the first and forget the second, and the forgotten one fails exactly as loudly, so there is a case each way for both image endpoints and for DELETE image.
DELETE deliberately still answers 204 for a well-formed id that is absent. The method is idempotent and the caller's intent, that the item should not exist, is satisfied either way; what must not happen is a 500. There is a test pinning that so the distinction is a decision rather than an omission.
Also replaced the raw req.params.id and Number(req.params.id) uses that sat inside routes which had already computed a validated id. They were safe, because the guard above them made them safe, but a validated id and a raw one side by side in the same handler is how this bug comes back.
The test block named "a non-numeric id on every admin item route" covered two routes. It now covers every route that takes an id, which is what makes its name true.
Closes #207
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The issue asked for two things and only one had been done. A well-formed but absent id already answered 404 — the PUT route carries a comment saying so. A malformed one still reached Postgres as text, raised 22P02 on an integer column, and surfaced through the route's catch as a 500, telling the admin the server had broken when the truth is that no such item can exist. That half is now closed everywhere rather than on the three routes that happened to have it.
Guarded: DELETE an item, DELETE an image, unpublish, and every route in adminItemDrafts — publish, regenerate, discard, restore, and the two background-removal endpoints added by #281. The last of those were flagged in that feature's own final review as sharing this pre-existing shape, so they are fixed with the rest rather than left to be found again.
Routes carrying two ids guard both. A route can guard the first and forget the second, and the forgotten one fails exactly as loudly, so there is a case each way for both image endpoints and for DELETE image.
DELETE deliberately still answers 204 for a well-formed id that is absent. The method is idempotent and the caller's intent, that the item should not exist, is satisfied either way; what must not happen is a 500. There is a test pinning that so the distinction is a decision rather than an omission.
Also replaced the raw req.params.id and Number(req.params.id) uses that sat inside routes which had already computed a validated id. They were safe, because the guard above them made them safe, but a validated id and a raw one side by side in the same handler is how this bug comes back.
The test block named "a non-numeric id on every admin item route" covered two routes. It now covers every route that takes an id, which is what makes its name true.
Closes#207
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
The issue asked for two things and only one had been done. A well-formed but absent id already answered 404 — the PUT route carries a comment saying so. A malformed one still reached Postgres as text, raised 22P02 on an integer column, and surfaced through the route's catch as a 500, telling the admin the server had broken when the truth is that no such item can exist. That half is now closed everywhere rather than on the three routes that happened to have it.
Guarded: DELETE an item, DELETE an image, unpublish, and every route in adminItemDrafts — publish, regenerate, discard, restore, and the two background-removal endpoints added by #281. The last of those were flagged in that feature's own final review as sharing this pre-existing shape, so they are fixed with the rest rather than left to be found again.
Routes carrying two ids guard both. A route can guard the first and forget the second, and the forgotten one fails exactly as loudly, so there is a case each way for both image endpoints and for DELETE image.
DELETE deliberately still answers 204 for a well-formed id that is absent. The method is idempotent and the caller's intent, that the item should not exist, is satisfied either way; what must not happen is a 500. There is a test pinning that so the distinction is a decision rather than an omission.
Also replaced the raw req.params.id and Number(req.params.id) uses that sat inside routes which had already computed a validated id. They were safe, because the guard above them made them safe, but a validated id and a raw one side by side in the same handler is how this bug comes back.
The test block named "a non-numeric id on every admin item route" covered two routes. It now covers every route that takes an id, which is what makes its name true.
Closes #207
Co-Authored-By: Claude Opus 5 noreply@anthropic.com