You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Add a new env var - STORAGE_PUBLIC_URL that will allow the users of self-hosted Supabase to avoid various complicated gymnastics around proper configuration for resumable uploads.
What is the current behavior?
Current configuration options for building the Location header properly for TUS include REQUEST_ALLOW_X_FORWARDED_PATH and TUS_URL_PATH. (Also, one more platform-specific option).
While manageable in a default no-proxy configuration, when adding a proxy on top of the API gateway (currently, Kong), it becomes quite hard to either explain concisely, or debug various edge cases that people are running into. See, e.g., issue supabase#40686.
Arguably, it was possible to have a fairly straightforward workaround with proxying the traffic to Storage directly (e.g., see here), and rely on REQUEST_ALLOW_X_FORWARDED_PATH to build the Location properly in response, with the new API keys / new asymmetric auth being added to self-hosted Supabase - this isn't going to be feasible anymore, because traffic to Storage will have to pass via the API gateway for proper header substitution. The problem of handling various combinations of external protocol and port against those configured for Kong becomes quite painful again.
What is the new behavior?
Add STORAGE_PUBLIC_URL for self-hosted Supabase and CLI only that could be configured in .env. Works similarly to how Studio uses SUPABASE_PUBLIC_URL to rewrite internal URLs for the browser - allows Storage to always construct a proper Location header without depending on X-Forwarded-Proto, X-Forwarded-Host, or X-Forwarded-Port headers, which Kong can overwrite with internal values when sitting behind a proxy. Does not replace REQUEST_ALLOW_X_FORWARDED_PATH or TUS_URL_PATH - works alongside them for the path prefix.
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 5322c0a7-d283-4df1-9ace-9ffff77aed6e
📥 Commits
Reviewing files that changed from the base of the PR and between 53462ff and cf00249.
📒 Files selected for processing (2)
src/config.ts
src/http/routes/tus/lifecycle.ts
📝 Walkthrough
Summary by CodeRabbit
New Features
Added support for configuring a public storage URL, enabling proper URL generation when storage is accessed through a proxy or reverse proxy setup.
Walkthrough
The changes add support for an optional storage public URL configuration. A new storagePublicUrl field is introduced in the storage configuration and populated from the STORAGE_PUBLIC_URL environment variable. In the TUS lifecycle module, when this public URL is configured, it is parsed and used to determine the protocol and host for generated URLs, taking precedence over x-forwarded headers. The x-forwarded header handling is skipped when an explicit public URL is provided, while the existing production environment behavior remains intact.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
Add a new env var -
STORAGE_PUBLIC_URLthat will allow the users of self-hosted Supabase to avoid various complicated gymnastics around proper configuration for resumable uploads.What is the current behavior?
Current configuration options for building the
Locationheader properly for TUS includeREQUEST_ALLOW_X_FORWARDED_PATHandTUS_URL_PATH. (Also, one more platform-specific option).While manageable in a default no-proxy configuration, when adding a proxy on top of the API gateway (currently, Kong), it becomes quite hard to either explain concisely, or debug various edge cases that people are running into. See, e.g., issue supabase#40686.
Arguably, it was possible to have a fairly straightforward workaround with proxying the traffic to Storage directly (e.g., see here), and rely on
REQUEST_ALLOW_X_FORWARDED_PATHto build theLocationproperly in response, with the new API keys / new asymmetric auth being added to self-hosted Supabase - this isn't going to be feasible anymore, because traffic to Storage will have to pass via the API gateway for proper header substitution. The problem of handling various combinations of external protocol and port against those configured for Kong becomes quite painful again.What is the new behavior?
Add
STORAGE_PUBLIC_URLfor self-hosted Supabase and CLI only that could be configured in.env. Works similarly to how Studio usesSUPABASE_PUBLIC_URLto rewrite internal URLs for the browser - allows Storage to always construct a proper Location header without depending onX-Forwarded-Proto,X-Forwarded-Host, orX-Forwarded-Portheaders, which Kong can overwrite with internal values when sitting behind a proxy. Does not replaceREQUEST_ALLOW_X_FORWARDED_PATHorTUS_URL_PATH- works alongside them for the path prefix.