fix(security): require auth on GET /api/v1/reauth and stop leaking SAT in body - #293
Open
birme wants to merge 3 commits into
Open
fix(security): require auth on GET /api/v1/reauth and stop leaking SAT in body#293birme wants to merge 3 commits into
birme wants to merge 3 commits into
Conversation
The reauth endpoint was registered with a schema only - no preHandler, no onRequest, no auth - so any unauthenticated caller could mint a valid OSC service access token. - Add requireReAuth, mirroring requireWhipAuth in api_whip.ts: Bearer header, constant-time timingSafeEqual comparison, 401 + WWW-Authenticate: Bearer realm="reauth", and auth disabled when no key is configured (existing installations keep working). - Configure via REAUTH_AUTH_KEY, falling back to WHIP_AUTH_KEY. - Defense in depth: stop returning the token in the JSON response body; the httpOnly cookie remains the delivery path. Closes #264
QA review of #283: auth-off-by-default is the right call for backwards compatibility, but it must not be silent. An install with OSC_ACCESS_TOKEN set and no effective key still hands out a service access token with no signal at all. A whitespace-only REAUTH_AUTH_KEY is worse: it looks configured but is falsy after trim, so auth is off while the operator believes it is on - the warning distinguishes that case as a configuration error. Also adds 401 coverage for empty Bearer, malformed header without the Bearer prefix, and a token that is a proper prefix of the key.
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
2 participants
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.
Summary
requireReAuthguard toGET /api/v1/reauth(mirrorsrequireWhipAuth): Bearer header with constant-timetimingSafeEqual,401+WWW-Authenticate: Bearer realm="reauth". Auth is disabled when no key is configured so existing installs keep working.REAUTH_AUTH_KEY, falling back toWHIP_AUTH_KEY.{ ok: true }); thehttpOnlycookie remains the sole delivery path./reauthis effectively unauthenticated (incl. whitespace-only key), so auth-off-by-default is never silent.Test plan
npm test)npm run typecheck)npm run lint)GET /api/v1/reauthwith no/invalid Bearer returns 401 whenREAUTH_AUTH_KEY/WHIP_AUTH_KEYis settokenvalue;satcookie is still setCloses #264
🤖 Generated with Claude Code
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com