Skip to content

fix(functions): add apikey compatibility header#5509

Open
kallebysantos wants to merge 6 commits into
supabase:developfrom
kallebysantos:fix-functions-apikeys-compatibility
Open

fix(functions): add apikey compatibility header#5509
kallebysantos wants to merge 6 commits into
supabase:developfrom
kallebysantos:fix-functions-apikeys-compatibility

Conversation

@kallebysantos

Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

Currently, the API proxy is overwriting the Authorization header when forwarding to /functions

What is the new behavior?

Uses a custom sb-api-key header to handle the minted jwt

Additional context

Towards FUNC-681

@kallebysantos kallebysantos marked this pull request as ready for review June 8, 2026 17:08
@kallebysantos kallebysantos requested a review from a team as a code owner June 8, 2026 17:08
@coveralls

coveralls commented Jun 8, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 27219620930

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Warning

No base build found for commit dc2ba5d on develop.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 64.618%

Details

  • Patch coverage: No coverable lines changed in this PR.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →


Coverage Stats

Coverage Status
Relevant Lines: 16254
Covered Lines: 10503
Line Coverage: 64.62%
Coverage Strength: 7.11 hits per line

💛 - Coveralls

@kallebysantos kallebysantos requested review from aantti and tomaspozo June 8, 2026 17:15

@avallete avallete left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 }}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants