RefreshTokenGrant leaks raw internal error text to API clients in the msg field

Open Beginner friendly
#2,812 0 comments 0 reactions 0 assignees View on GitHub

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

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

  1. Issue a refresh token with RefreshTokenAlgorithmVersion set to 2.
  2. Corrupt the session's refresh_token_hmac_key column directly (or simulate an encryption key rotation that leaves an old value undecryptable).
  3. Call the refresh token grant with the original refresh token.
  4. 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from supabase/auth

All issues in supabase/auth

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.