From 1aaab1bf66efa3407d05300febf4b147e77f84b2 Mon Sep 17 00:00:00 2001 From: elibrachas Date: Sat, 22 Aug 2026 15:41:31 -0300 Subject: [PATCH 1/3] Read TRUSTED_PROXIES late enough for it to be seen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The value was read with env() inside the withMiddleware closure in bootstrap/app.php. That closure runs when the HTTP kernel is resolved, which is before the dotenv bootstrapper reads .env — so on every web request env() returned null for anything set in .env, and the proxy was never trusted. It worked when the value came from a real environment variable, which is why the Docker compose path was fine and the manual install described in INSTALL.md, where we tell people to put it in .env, was not. Artisan bootstraps in the other order, so a check from the command line reported the setting as working the whole time. Behind a TLS-terminating proxy the consequence is not subtle. Laravel falls back to the connecting address and the plain scheme, builds every link and redirect with http:// while the browser is on https://, and marks the session cookie non-secure. The browser then declines to send that cookie to what it reads as a different, less secure origin, the session arrives empty, and the first write fails with a 419 that reads as "your session expired" — most often on the create-your-admin form, which is the first thing a new install submits. Afterwards each redirect leaves and re-enters over the wrong scheme, which is the random bounce back to the login screen people report as flakiness. Moved to config/trustedproxy.php, the key the framework's TrustProxies middleware already falls back to on its own. Config files load after dotenv, so the value is there whether it comes from .env or from the environment. This was also the only env() read outside config/, which means config:cache is no longer dangerous on this application. Co-Authored-By: Claude Opus 5 (1M context) --- bootstrap/app.php | 19 ++-- config/trustedproxy.php | 12 +++ tests/Feature/Platform/TrustedProxiesTest.php | 88 +++++++++++++++++++ 3 files changed, 108 insertions(+), 11 deletions(-) create mode 100644 config/trustedproxy.php create mode 100644 tests/Feature/Platform/TrustedProxiesTest.php diff --git a/bootstrap/app.php b/bootstrap/app.php index cb526408..12dceeb7 100644 --- a/bootstrap/app.php +++ b/bootstrap/app.php @@ -32,17 +32,14 @@ return Application::configure(basePath: dirname(__DIR__)) health: '/up', ) ->withMiddleware(function (Middleware $middleware) { - // Unset = trust nothing, which is correct for the shipped topology - // (nginx talks to PHP-FPM directly and passes the real REMOTE_ADDR). - // Behind anything else — a load balancer, Cloudflare, the hosted - // Cloud ingress — this MUST name the proxy, or every client appears - // to come from it: per-IP throttles collapse into one shared bucket - // and the download IP log records the proxy instead of the client. - $proxies = env('TRUSTED_PROXIES'); - - if (is_string($proxies) && $proxies !== '') { - $middleware->trustProxies(at: $proxies === '*' ? '*' : explode(',', $proxies)); - } + // Trusted proxies are configured in config/trustedproxy.php, NOT + // here. This closure runs when the HTTP kernel is resolved, which is + // before the dotenv bootstrapper has read .env, so env() returns null + // here for anything that is not already a real environment variable — + // silently, and only on web requests (artisan bootstraps in the other + // order, so a CLI check reports the setting as working). The framework's + // TrustProxies middleware is in the global stack either way and falls + // back to that config key on its own. $middleware->web(append: [ // Binds every session to the password hash it was created under, diff --git a/config/trustedproxy.php b/config/trustedproxy.php new file mode 100644 index 00000000..52a26f58 --- /dev/null +++ b/config/trustedproxy.php @@ -0,0 +1,12 @@ + env('TRUSTED_PROXIES'), +]; diff --git a/tests/Feature/Platform/TrustedProxiesTest.php b/tests/Feature/Platform/TrustedProxiesTest.php new file mode 100644 index 00000000..12fbf925 --- /dev/null +++ b/tests/Feature/Platform/TrustedProxiesTest.php @@ -0,0 +1,88 @@ + response()->json([ + 'root' => request()->getSchemeAndHttpHost(), + 'ip' => request()->ip(), + 'secure' => request()->isSecure(), + ])); +}); + +test('forwarded scheme, host and client address are honoured when the proxy is trusted', function () { + config()->set('trustedproxy.proxies', '*'); + + $this->get('/__proxy-probe', [ + 'X-Forwarded-Proto' => 'https', + 'X-Forwarded-Host' => 'files.example.com', + 'X-Forwarded-For' => '203.0.113.9', + ])->assertOk()->assertJson([ + 'root' => 'https://files.example.com', + 'ip' => '203.0.113.9', + 'secure' => true, + ]); +}); + +test('forwarded headers are ignored when no proxy is trusted', function () { + config()->set('trustedproxy.proxies', null); + + // Asserted against the forwarded values rather than a literal expected + // host: the test environment's APP_URL supplies the host here, and + // isSecure() is already true from it, so neither is a signal on its own. + // What discriminates is that the proxy's claims are not adopted. + $json = $this->get('/__proxy-probe', [ + 'X-Forwarded-Proto' => 'https', + 'X-Forwarded-Host' => 'files.example.com', + 'X-Forwarded-For' => '203.0.113.9', + ])->assertOk()->json(); + + expect($json['ip'])->toBe('127.0.0.1') + ->and($json['root'])->not->toContain('files.example.com'); +}); + +test('the config key the framework falls back to is wired to TRUSTED_PROXIES', function () { + // Illuminate\Http\Middleware\TrustProxies reads `trustedproxy.proxies` + // when nothing called trustProxies(at:). That is the only path that sees + // a value coming from .env, so the key has to stay spelled this way and + // has to keep reading that variable. Evaluated directly rather than + // through config(), which already holds the value loaded at boot. + $_ENV['TRUSTED_PROXIES'] = '10.0.0.1,10.0.0.2'; + $_SERVER['TRUSTED_PROXIES'] = '10.0.0.1,10.0.0.2'; + + try { + expect(require base_path('config/trustedproxy.php')) + ->toBe(['proxies' => '10.0.0.1,10.0.0.2']); + } finally { + unset($_ENV['TRUSTED_PROXIES'], $_SERVER['TRUSTED_PROXIES']); + } +}); + +test('bootstrap/app.php does not read TRUSTED_PROXIES from the environment', function () { + // Reintroducing this read is the regression: it works when the value is + // a real environment variable (Docker `environment:`), and silently does + // nothing when it comes from .env, which is the documented way to set it. + expect(file_get_contents(base_path('bootstrap/app.php'))) + ->not->toContain('TRUSTED_PROXIES'); +}); From 2a82335e07b36477d51bf33ba8d1324b31758b6f Mon Sep 17 00:00:00 2001 From: elibrachas Date: Sat, 22 Aug 2026 15:41:49 -0300 Subject: [PATCH 2/3] Move the shared public-listing helpers to tests/Helpers.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit publicListingFile() and publicListingImageFile() were defined in PublicGroupsTest.php and used from PublicFilePreviewTest.php too. Pest declares a test file's functions as ordinary globals, so that works only once the defining file has been loaded — which under --parallel depends on how the runner happens to distribute files across processes. Adding any unrelated test file anywhere in the suite reshuffles that and takes PublicFilePreviewTest.php down with "Call to undefined function", and running it on its own with --filter never worked at all. tests/Helpers.php exists for exactly this and its docblock describes this failure; these two had just been missed. publicPageProps() stays where it is, since only one file uses it. Co-Authored-By: Claude Opus 5 (1M context) --- tests/Feature/Groups/PublicGroupsTest.php | 34 --------------------- tests/Helpers.php | 36 +++++++++++++++++++++++ 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/tests/Feature/Groups/PublicGroupsTest.php b/tests/Feature/Groups/PublicGroupsTest.php index cb6c8e7f..5b9b5250 100644 --- a/tests/Feature/Groups/PublicGroupsTest.php +++ b/tests/Feature/Groups/PublicGroupsTest.php @@ -11,10 +11,8 @@ use App\Modules\Files\Models\Folder; use App\Modules\Groups\Models\Group; use App\Modules\Platform\Settings\Setting; use App\Modules\Platform\Settings\Settings; -use Illuminate\Http\UploadedFile; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Storage; -use Illuminate\Support\Str; use Illuminate\Testing\TestResponse; /** @@ -27,38 +25,6 @@ function publicPageProps(TestResponse $response): array return $page['props']; } -function publicListingFile(array $overrides = []): File -{ - return File::factory()->create(array_merge([ - 'uploaded_by' => User::factory()->create()->id, - 'name' => 'Report', - 'original_name' => 'report.pdf', - 'path' => '2026/08/'.Str::uuid()->toString().'.pdf', - 'mime_type' => 'application/pdf', - 'size' => 2048, - 'public' => true, - ], $overrides)); -} - -/** - * A real, thumbnailable public image on the faked "files" disk — unlike - * publicListingFile()'s bare PDF row, this one has actual bytes GD can - * decode, needed to exercise the thumbnail-generation path. - */ -function publicListingImageFile(User $uploader): File -{ - test()->actingAs($uploader)->post('/files', [ - 'file' => UploadedFile::fake()->image('photo.jpg', 200, 100), - 'name' => '', - 'description' => '', - ]); - - $file = File::query()->latest('id')->firstOrFail(); - $file->update(['public' => true]); - - return $file; -} - beforeEach(function () { Storage::fake('files'); diff --git a/tests/Helpers.php b/tests/Helpers.php index 3e43bd02..76447a15 100644 --- a/tests/Helpers.php +++ b/tests/Helpers.php @@ -209,3 +209,39 @@ function activateExternalStorage(): void ])->save(); app(ExternalStorageConfigApplier::class)->flush(); } + +/** + * A public file row, with no bytes behind it — enough for any listing or + * permission assertion that never opens the file. + */ +function publicListingFile(array $overrides = []): File +{ + return File::factory()->create(array_merge([ + 'uploaded_by' => User::factory()->create()->id, + 'name' => 'Report', + 'original_name' => 'report.pdf', + 'path' => '2026/08/'.Str::uuid()->toString().'.pdf', + 'mime_type' => 'application/pdf', + 'size' => 2048, + 'public' => true, + ], $overrides)); +} + +/** + * A real, thumbnailable public image on the faked "files" disk — unlike + * publicListingFile()'s bare PDF row, this one has actual bytes GD can + * decode, needed to exercise the thumbnail-generation path. + */ +function publicListingImageFile(User $uploader): File +{ + test()->actingAs($uploader)->post('/files', [ + 'file' => UploadedFile::fake()->image('photo.jpg', 200, 100), + 'name' => '', + 'description' => '', + ]); + + $file = File::query()->latest('id')->firstOrFail(); + $file->update(['public' => true]); + + return $file; +} From 027e8532d2b775fb2ba61e489c79268e9ce4a917 Mon Sep 17 00:00:00 2001 From: elibrachas Date: Sat, 22 Aug 2026 15:41:49 -0300 Subject: [PATCH 3/3] Name the 419 as a symptom of an untrusted proxy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DOCKER.md said getting TRUSTED_PROXIES wrong gives you wrong client IPs or wrong links but never affects whether a request succeeds. It does: it is what makes the create-your-admin form come back as a 419, which is the first thing a new install behind a proxy hits and gives no hint about the cause. Said so, and kept the point that a 502 is a different problem. INSTALL.md told operators never to run config:cache, and the only reason it gave was that doing so disabled TRUSTED_PROXIES. That read now goes through the config layer, so the reason is gone and the section claimed something untrue. Replaced with the caveat that does apply to a cached config: re-run it after editing .env. Also a troubleshooting entry under the symptom people search for — 419 on login, or being returned to the login screen at random — since the existing proxy entry only covered rate limiting and the download log. Co-Authored-By: Claude Opus 5 (1M context) --- DOCKER.md | 17 ++++++++++++----- INSTALL.md | 43 ++++++++++++++++++++----------------------- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/DOCKER.md b/DOCKER.md index c858b3cc..262738a3 100644 --- a/DOCKER.md +++ b/DOCKER.md @@ -121,11 +121,18 @@ Without it every visitor appears to come from the proxy. The login rate limiter your users as one attacker, and the download log records the proxy's address instead of the person's. `compose.example.yaml` already sets this. -What it does **not** affect is whether a request succeeds at all. `TRUSTED_PROXIES` only changes how -the application reads `X-Forwarded-*` headers on a request that already arrived, so getting it wrong -gives you wrong client IPs, wrong generated links, or a redirect loop — never a `502`. A 502 means -your proxy could not get a usable response out of the container, which is a different problem with a -different fix. +Leaving it unset does not cause a `502` — that means your proxy could not get a usable response out +of the container at all, which is a different problem with a different fix. It does cause a **419 +"page expired"**. Without it the application never learns the proxy terminated TLS, so it builds +its links and redirects with `http://` while the browser is on `https://`, and marks the session +cookie as non-secure. The browser declines to send that cookie back to what it now reads as a +different, less secure origin, the session arrives empty, and the first thing you submit — usually +the create-your-admin-account form — is rejected as a stale token. After that you get returned to +the login screen at random, because each redirect leaves and re-enters over the wrong scheme. + +Your proxy also has to pass the original `Host` header through, or the links come out naming the +container instead of your domain. Most do by default: `passHostHeader=true` in Traefik, +`proxy_set_header Host $host;` in nginx. ### Give the proxy header headroom diff --git a/INSTALL.md b/INSTALL.md index 55c40994..821f49f0 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -463,28 +463,16 @@ sudo -u www-data php artisan event:cache You only run these once: `projectsend:update` notices they are in place and rebuilds them for you after every update. If you change your mind, `php artisan optimize:clear` undoes all three. -#### One command to skip: `config:cache` +#### `config:cache` and your `.env` -Every Laravel deployment guide on the internet lists `php artisan config:cache` alongside those -three, and `php artisan optimize` runs it for you. **Don't** — not on this application. +Every Laravel deployment guide also lists `php artisan config:cache`, and `php artisan optimize` +runs it for you. It is safe here, with one thing to remember. -Here is why. Caching the configuration writes every resolved setting into one PHP file, and from -then on the framework stops reading your `.env` at all, on the entirely reasonable grounds that -everything in it has already been baked in. That holds for settings read the normal way, through -`config()`. ProjectSend reads one value earlier than that — `TRUSTED_PROXIES`, which has to be -known before the middleware stack is assembled, so it is read straight from the environment. Cache -the config and that read returns nothing. - -Nothing breaks loudly. The site comes up, you log in, everything looks fine. But if there is a -proxy or CDN in front of the server, ProjectSend goes back to believing every visitor is the proxy: -the login rate limiter now counts all of your users as one attacker and locks the whole site out -after five wrong passwords, and every row in the download log records the proxy's address instead -of the person who actually downloaded the file. Both are the kind of thing you discover weeks -later, from a complaint. - -If you have already run it — or ran `php artisan optimize`, which includes it — `php artisan -config:clear` puts things back immediately, and every update clears it too, saying why. The three commands above are safe and give you nearly -all of the speed anyway; `config:cache` was always the smallest win of the four. +Caching the configuration writes every resolved setting into one PHP file, and from then on the +framework stops reading your `.env` at all — everything in it has already been baked in. So +**re-run `php artisan config:cache` every time you edit `.env`**, or the edit does nothing and you +are left staring at a setting that is plainly there and plainly ignored. `php artisan config:clear` +goes back to reading `.env` directly, and every update clears it too, saying why. --- @@ -577,13 +565,22 @@ applies to a non-standard port; behind a TLS proxy on 443 you do not need it. **A change I made in `.env` has no effect.** Run `php artisan optimize:clear`, then restart PHP-FPM and the worker. Both hold the old values until they are restarted. If it *still* has no effect, someone has run `php artisan config:cache` -(or `optimize`) on this install — see [One command to skip](#one-command-to-skip-configcache). +(or `optimize`) on this install — re-run it to pick the new value up, or `php artisan config:clear` +to go back to reading `.env` directly. **Everyone is locked out of the login form at once, or the download log shows the same IP for every download.** ProjectSend is seeing your proxy or CDN instead of your visitors. Set `TRUSTED_PROXIES` in `.env` -(step 3) — and make sure `config:cache` has not been run, which stops that value from being read -at all. Same section as above. +(step 3) and restart PHP-FPM. + +**Behind a reverse proxy: 419 "page expired" when you log in or save a form, or you land back on +the login screen at random.** +`TRUSTED_PROXIES` again (step 3). Without it ProjectSend never learns the proxy terminated TLS, so +it builds its links and redirects with `http://` while the browser is on `https://`, and marks the +session cookie as non-secure. The browser then declines to send that cookie back, the session +arrives empty, and the write fails with a 419 that reads as an expired session. Make sure your +proxy passes the original `Host` header through as well — `proxy_set_header Host $host;` in nginx, +`passHostHeader=true` in Traefik (its default). Still stuck? Ask in the [community forum](https://www.projectsend.org/) or open an issue on [GitHub](https://github.com/projectsend/projectsend/issues), and include the last few lines of