fix(versioninfo): reject reads with unknown authKey, not just missing-header#160
Merged
Merged
Conversation
…-header `GET /v1/versioninfo/:id` and `GET /v1/versioninfo` originally gated only on `if (!authKey)` — header-present-check, no DB validation. Anyone with ANY non-empty string in the `authKey` header got 200 with the schema/build version records. The README's endpoint table says "yes (authKey)" for these reads, which the rest of the API consistently means "any VALID authKey, master or scoped". A literal presence-check breaks that contract: the same string `authKey: anything` that reliably 403s every other v1 endpoint silently let through on versioninfo. Add a `requireValidAuth(req, res)` helper that reads the already- resolved `req.authKey` / `req.isMaster` / `req.companyId` populated by `attachAuth` (mounted at /v1/*). Master keys pass; non-master keys pass when they resolved to a real company id; an unknown header is the new reject case (403 with the same "Invalid Authorization Key." message the other endpoints emit). No extra DB round-trip — the helper reads req state attachAuth already produced upstream. Two new tests pin the tightening: GET 403 for unknown authKey, GET list 403 for unknown authKey. The existing missing-header 403 tests continue to pass; the mounting tests that send `authKey: any` continue to pass because they only assert on body shape, not status code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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 #159.
Summary
GET /v1/versioninfo/:idandGET /v1/versioninfooriginally gated only onif (!authKey)— header-present-check, no DB validation. Anyone sending any non-empty string inauthKeygot 200 with the schema/build version records, even though the README's endpoint table promises "yes (authKey)" with the rest-of-codebase semantics of "any valid authKey, master or scoped".Add a
requireValidAuth(req, res)helper that reads the already-resolvedreq.authKey/req.isMaster/req.companyIdpopulated byattachAuth(mounted at /v1/*). Master keys pass; non-master keys pass when they resolved to a real company id; an unknown header is the reject case (403 "Invalid Authorization Key.").No extra DB round-trip —
attachAuthalready did the lookups upstream.Test plan
npm run lint— cleannpm test— 614 passed (was 612 + 2 new for unknown-authKey rejection on both reads), 15 skippedauthKey: anyand only assert on body shape, so they continue to pass; the missing-header 403 tests are still 403Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/