RefreshTokenGrant leaks raw internal error text to API clients in the msg field
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 78/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- go
- Domain
- authentication, backend, security
Research direction
Start in internal/tokens/service.go at RefreshTokenGrant and inspect the five NewInternalServerError call sites described in the issue. Use the existing generic-message and WithInternalError convention, then run the regression test mentioned in the issue; done means raw internal errors are absent from the API msg field while their causes remain available for server-side logging.
Written by the indexing model from the issue text.
Description
Summary
RefreshTokenGrant in internal/tokens/service.go sends raw internal Go error text straight back to the API caller in the "msg" field of the 500 response, instead of keeping it server side. This happens in five places inside this one function, and nowhere else in the codebase does the same thing.
Mechanism
Everywhere else in this codebase, an internal server error is built like this:
return apierrors.NewInternalServerError("Error creating identity").WithInternalError(terr)
The first argument is a generic, safe message that goes to the client. WithInternalError stores the real error separately in a field tagged json:"-", so it never gets serialized into the response, it is only used for server side logging through Cause().
RefreshTokenGrant does not follow this pattern in five places. It does this instead:
return nil, apierrors.NewInternalServerError("%s", err.Error())
Here the raw error text becomes the actual Message field of the HTTPError, and Message is tagged json:"msg", so it is sent directly to the client. The InternalError field is left nil, so the real cause is also lost from server side logs, since Cause() falls back to the HTTPError itself when InternalError is nil.
I found five of these in internal/tokens/service.go:
// line 218, before the transaction
return nil, apierrors.NewInternalServerError("%s", err.Error())
// line 295, inside the transaction
return apierrors.NewInternalServerError("%s", terr.Error())
// line 329, inside the SinglePerUser branch
return apierrors.NewInternalServerError("%s", terr.Error())
// line 379, looking up the currently active refresh token
return apierrors.NewInternalServerError("%s", terr.Error())
// line 403, revoking a token family
return apierrors.NewInternalServerError("%s", err.Error())
I checked the rest of the codebase. There are 303 calls to NewInternalServerError in total, and only these 5 use this pattern. Everywhere else the generic message plus WithInternalError convention is followed correctly.
Observed
I reproduced this against a real, freshly migrated Postgres database. I set up a session with a v2 refresh token, then corrupted the session's refresh_token_hmac_key column so it can no longer be base64 decoded, simulating what would happen if that value got corrupted on disk or the encryption key protecting it was rotated away. Calling the refresh token grant with that token gives back:
HTTPStatus: 500
Message: "illegal base64 data at input byte 3"
InternalError: nil
The exact text of the Go decode error ends up in the response body's "msg" field, which is what gets sent to the caller, while the field meant to carry it for server side logging is empty.
Reproduction
- Issue a refresh token with
RefreshTokenAlgorithmVersionset to 2. - Corrupt the session's
refresh_token_hmac_keycolumn directly (or simulate an encryption key rotation that leaves an old value undecryptable). - Call the refresh token grant with the original refresh token.
- The response is a 500 whose "msg" field contains the literal underlying Go error text instead of a generic message.
The same thing happens for the other 4 call sites listed above, for example a database error while looking up a user's sessions, or while revoking a token family, would also leak its raw text to the client.
Not the issue
This is not about the identity linking race fixed in #2810 and PR #2811, that one is a different function and a different kind of bug (a missing conflict check). This is specifically about RefreshTokenGrant sending raw error text to API clients instead of a generic message, which is a small information disclosure risk and also a loss of debugging signal for whoever operates the service, since the real error text should be in server side logs via InternalError and is not.
Suggested fix
Replace all five apierrors.NewInternalServerError("%s", err.Error()) style calls in RefreshTokenGrant with a generic, descriptive message plus .WithInternalError(err), matching the convention used everywhere else in the codebase. I have a fix and a regression test ready and will open a PR for it.
- Dominant language
- Go
- Stars
- 2.6k
- Forks
- 764
- Avg merge
- 5d 2h
- Merged PRs (30d)
- 38
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from supabase/auth
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
-
Difficulty 1/5 Under an hour Newbie friendliness 86/100
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 80/100
Similar issues
-
Difficulty 1/5 Under an hour Newbie friendliness 84/100
-
enhancement needs triage
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
-
kind/cleanup
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
kubernetes-sigs/kueue#15947 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
sympozium-ai/sympozium#627 ·
-
priority: p3
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
googleapis/librarian#7636 ·