fix(cli): jwt errors#5862
Conversation
|
👋 Thanks! The linked issue hasn't been marked A maintainer adds that label once an issue is triaged and ready to be worked on. Please wait for the label before opening a pull request. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
imo this shouldn't really happen for the members ? 😅 |
It shouldn't, sadly if your affiliation is "private" on https://github.com/orgs/supabase/people If that's not a bother, could you try to your supabase membership to "public" ? This should allows your PR's to pass the gate without issues. |
avallete
left a comment
There was a problem hiding this comment.
I think this might actually be a BC need to double check.
|
Alright, tell me if I'm mistaken @kallebysantos @7ttp but the Hosted platform does not uses Looking at the code, I think this is the wrong fix, and we actually need a contract between edge-functions/stdio/cli for error codes, they seems to solely rely on The official docs shows "error, message" contract: https://github.com/supabase/supabase/blob/master/apps/docs/content/troubleshooting/edge-function-401-error-response.mdx And it look like the only place where we rely on I think we should standardize toward flowchart LR
UI["Studio Test UI"]
SP["Studio test proxy<br/>currently assumes body.error"]
GW["Gateway<br/>Kong / Envoy / hosted router"]
JW["JWT admission layer<br/>hosted ingress / local main worker"]
UF["User function"]
UI --> SP --> GW --> JW --> UF
GW -. "503: {message} or text" .-> SP
JW -. "401: hosted {code,message}<br/>local legacy {msg}" .-> SP
UF -. "arbitrary status/body/headers" .-> SP
The bug I think is https://github.com/supabase/supabase/blob/master/apps/studio/pages/api/edge-functions/test.ts#L92-L107 :
That assumption is impossible to satisfy universally:
I think a minimal cleaner fix: I would make the Studio test endpoint a transparent response adapter. A completed upstream request even if it returns 401, 404, or 503 is a successful execution of the test operation. It should return: The Studio proxy itself should return HTTP 200 for that envelope. Reserve proxy-level 4xx/5xx for cases where Studio could not execute the test at all: invalid URL, network failure, malformed request, etc. Concretely, in
This fits the existing architecture unusually well:
#47855 (https://github.com/supabase/supabase/pull/47855)’s tolerant extractor is an acceptable hotfix, but it remains schema guessing and still loses response headers and structured bodies. Transparent relay is cleaner. Tests should cover hosted {code,message}, Kong {message}, legacy {msg}, nested JSON, arbitrary JSON, plain text, invalid JSON, headers, and a genuine fetch exception. Where JWT mirror code should liveJWT errors belong at the layer that performs JWT admission not in Studio and not in the generic Edge Runtime engine. The canonical contract should be the hosted ingress contract: Canonical codes:
The mirrors should live beside JWT verification in each independently shipped bootstrap:
Each bootstrap should have a small local JwtFailure mapping and one unauthorized(failure) response helper. Backward-compatible migrationLocal {msg} has shipped since 2023, so replace it additively: Also emit Recommended sequence:
The important boundary is to standardize all Supabase-owned JWT errors, preserve all other HTTP responses transparently. Trying to standardize gateway failures and user-function responses would require rewriting responses that Supabase does not own. cc @SaxonF @joshenlim since it seems you authored: https://github.com/supabase/supabase/blame/master/apps/studio/pages/api/edge-functions/test.ts#L92-L107 would you give it a look, tell me if you think I'm off ? |
|
You're right... I missed it, and I agree that we should use a single pattern across all environments. We've been refactoring Platform error codes, and yes! we're now using export enum AuthErrors {
UnsupportedTokenAlgorithm = 'UNAUTHORIZED_UNSUPPORTED_TOKEN_ALGORITHM',
MissingAuthHeader = 'UNAUTHORIZED_NO_AUTH_HEADER',
// ....
}// in case JWT validation fails:
const error = {
code: AuthErrors.UnsupportedTokenAlgorithm,
message: `Unsupported JWT algorithm ${jwt.alg}`
}
return Response.json(error, {
status: Status.Unauthorized,
headers: {
'sb-error-code': AuthErrors.UnsupportedTokenAlgorithm
}
}) |
2af6b28 to
fd9e5a1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd9e5a1a7d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 921984a8e4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7cfdaeed5a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
I’ve rescoped this PR to the but b4 this will patch the studio test proxy so it preserves the upstream status, headers, and body instead of keying only off |
TL;DR
uses the
errorkey for JWT verification failures in the local functions worker andregenerates the embedded Go template
Ref