fix(functions): add apikey compatibility header#5509
Conversation
Coverage Report for CI Build 27219620930Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Warning No base build found for commit Coverage: 64.618%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
avallete
left a comment
avallete
left a comment
There was a problem hiding this comment.
With the latest commit this looks like it resolves the original bug for the CLI stackAuthorization is preserved, the mint moves to sb-api-key, and it's now stripped before the user worker (prepareUserRequest → worker.fetch(userReq)). Two small nits inline.
| add: | ||
| headers: | ||
| - "Authorization: {{ .BearerToken }}" | ||
| - "sb-api-key: {{ .BearerToken }}" |
There was a problem hiding this comment.
remark(not-blocking):
this mints on every /functions request regardless of verify_jwt. Harmless now that it goes to sb-api-key and is stripped before the worker (and only read when verify_jwt is on), but it's wider than necessary could scope it later. (Underlying expression is in https://github.com/supabase/cli/blob/develop/apps/cli-go/internal/start/start.go#L451-L463 )
| const sbApiKeyCompatibilityToken = req.headers.get("sb-api-key") | ||
|
|
||
| // NOTE:(kallebysantos) Kong on legacy CLI stack pass it down as 'Bearer Token' format | ||
| const cleanSbApiKeyCompatibilityToken = sbApiKeyCompatibilityToken.replace('Bearer', '').trim() |
There was a problem hiding this comment.
nitpick
sbApiKeyCompatibilityToken is string | null (headers.get returns null when absent), so .replace(...) throws if sb-api-key is missing. Masked today only because Kong always sets the header on this route but a request reaching the runtime without it (direct-to-runtime, tests) would 500. A ?? guard would match the ingress behavior.
There was a problem hiding this comment.
Yha! I noticed it after pushing the last commit...
I'm already fixing it, and I also figured out that edge-runtime-main.ts needs to be updated as well 😅
3 participants
What kind of change does this PR introduce?
Bug fix
What is the current behavior?
Currently, the API proxy is overwriting the
Authorizationheader when forwarding to/functionsWhat is the new behavior?
Uses a custom
sb-api-keyheader to handle the minted jwtAdditional context
Towards FUNC-681