fix(customer): return 404 (not 200+null) on master-path GET miss#242
Merged
CryptoJones merged 1 commit intoMay 19, 2026
Merged
Conversation
…sses on the master path
The master-key branch of `getCustomerById` called the `findAndRespond`
helper without first verifying the customer existed:
```js
if (isAuthKeyMasterKey) {
return findAndRespond(customerId, res);
}
```
And `findAndRespond` unconditionally wrapped `findByPk(id)` in a 200
envelope:
```js
const customer = await Customer.findByPk(customerId);
return res.status(200).json({ ..., customers: customer });
```
So a master key requesting `/v1/customer/<nonexistent>` got back:
```json
{ "message": "Successfully retrieved...", "customers": null }
```
A success status code wrapping an empty body — any client reading
`body.customers.custId` crashes; SDK generators that branch on
HTTP status mark the request as successful and propagate the null
downstream. The scoped (non-master) branch never hit this path
because `GetCustomerCompanyId(id) === -1` short-circuits to 403 for
absent rows, but the master path skipped that guard.
Fix: have `findAndRespond` short-circuit to 404 when `findByPk`
returns null. Both paths now agree that "no such customer id" is
not-found, not a success. Soft-deleted rows (which `defaultScope`
filters at the model layer) correctly surface as 404 too.
Pinned in `tests/api/customer.test.js` with a master-path test that
drives `getCustomerById` directly through the controller export and
uses the P5-M `auth._setDbForTesting` seam to make `IsMaster`
resolve to `true`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CryptoJones
deleted the
fix/customer-getbyid-404-on-non-existent-master-path
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #241.
Summary
Master-key
GET /v1/customer/<nonexistent>returned200 { customers: null }— a success status wrapping an empty body. Clients readingbody.customers.custIdcrashed; SDK generators that branched on status marked it successful.Fix:
findAndRespondshort-circuits to 404 whenfindByPkreturns null. Both master and scoped paths now agree on the not-found shape.Test plan
npm run lintcleannpm test— 686 → 687 (+1 master-path 404 test using the P5-M auth._setDbForTesting seam)Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/