Merge pull request #1674 from projectsend/fix/trusted-proxies-read-from-env

Read TRUSTED_PROXIES late enough for it to be seen

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
 

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

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,

config/trustedproxy.php

@@ -0,0 +1,12 @@
+<?php
+
+// Read here rather than in bootstrap/app.php's withMiddleware closure: that
+// closure runs when the HTTP kernel is resolved, which under PHP-FPM is
+// BEFORE the dotenv bootstrapper loads .env, so env('TRUSTED_PROXIES') is
+// null there on every web request (it works in artisan, which bootstraps
+// first — the discrepancy is invisible in CLI testing). Config files load
+// after dotenv, and the framework's TrustProxies middleware falls back to
+// this key on its own.
+return [
+    'proxies' => env('TRUSTED_PROXIES'),
+];

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');
 

tests/Feature/Platform/TrustedProxiesTest.php

@@ -0,0 +1,88 @@
+<?php
+
+declare(strict_types=1);
+
+use Illuminate\Support\Facades\Route;
+
+/**
+ * The trusted proxy list has to be read from configuration, not from
+ * bootstrap/app.php.
+ *
+ * The `withMiddleware` closure runs when the HTTP kernel is resolved, and
+ * that happens *before* the dotenv bootstrapper reads .env. Anything set
+ * only in .env is therefore invisible to `env()` in that closure, on every
+ * web request — while artisan, which bootstraps in the other order, reports
+ * the setting as working. That combination is what makes the bug so hard to
+ * see from the outside: the operator sets TRUSTED_PROXIES, a CLI check
+ * agrees it is set, and the web app ignores it anyway.
+ *
+ * With the proxy untrusted, Laravel falls back to the connecting address and
+ * the plain scheme, so behind a TLS-terminating proxy every generated URL
+ * and every redirect comes out as `http://` on a page the browser loaded
+ * over `https://`. The browser then refuses to send the session cookie to
+ * that other origin, the session looks empty, and the write fails with a 419
+ * that reads as "your session expired".
+ */
+beforeEach(function () {
+    Route::get('/__proxy-probe', fn () => 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');
+});

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;
+}