diff --git a/app/Http/Middleware/TrustHosts.php b/app/Http/Middleware/TrustHosts.php index 5fca583d9..d44b6057a 100644 --- a/app/Http/Middleware/TrustHosts.php +++ b/app/Http/Middleware/TrustHosts.php @@ -30,14 +30,44 @@ public function handle(Request $request, $next) return $next($request); } + // Eagerly call hosts() to populate the cache (fixes circular dependency + // where handle() checked cache before hosts() could populate it via + // Cache::remember, causing host validation to never activate) + $this->hosts(); + // Skip host validation if no FQDN is configured (initial setup) $fqdnHost = Cache::get('instance_settings_fqdn_host'); if ($fqdnHost === '' || $fqdnHost === null) { return $next($request); } - // For all other routes, use parent's host validation - return parent::handle($request, $next); + // Validate the request host against trusted hosts explicitly. + // We check manually instead of relying on Symfony's lazy getHost() validation, + // which can be bypassed if getHost() was already called earlier in the pipeline. + $trustedHosts = array_filter($this->hosts()); + + // Collect all hosts to validate: the actual Host header, plus X-Forwarded-Host + // if present. We must check X-Forwarded-Host here because this middleware runs + // BEFORE TrustProxies, which would later apply X-Forwarded-Host to the request. + $hostsToValidate = [strtolower(trim($request->getHost()))]; + + $forwardedHost = $request->headers->get('X-Forwarded-Host'); + if ($forwardedHost) { + // X-Forwarded-Host can be a comma-separated list; validate the first (client-facing) value. + // Strip port if present (e.g. "coolify.example.com:443" → "coolify.example.com") + // to match the trusted hosts list which stores hostnames without ports. + $forwardedHostValue = strtolower(trim(explode(',', $forwardedHost)[0])); + $forwardedHostValue = preg_replace('/:\d+$/', '', $forwardedHostValue); + $hostsToValidate[] = $forwardedHostValue; + } + + foreach ($hostsToValidate as $hostToCheck) { + if (! $this->isHostTrusted($hostToCheck, $trustedHosts)) { + return response('Bad Host', 400); + } + } + + return $next($request); } /** @@ -100,4 +130,29 @@ public function hosts(): array return array_filter($trustedHosts); } + + /** + * Check if a host matches the trusted hosts list. + * + * Regex patterns (from allSubdomainsOfApplicationUrl, starting with ^) + * are matched with preg_match. Literal hostnames use exact comparison + * only — they are NOT passed to preg_match, which would treat unescaped + * dots as wildcards and match unanchored substrings. + * + * @param array $trustedHosts + */ + protected function isHostTrusted(string $host, array $trustedHosts): bool + { + foreach ($trustedHosts as $pattern) { + if (str_starts_with($pattern, '^')) { + if (@preg_match('{'.$pattern.'}i', $host)) { + return true; + } + } elseif ($host === $pattern) { + return true; + } + } + + return false; + } } diff --git a/app/Notifications/TransactionalEmails/ResetPassword.php b/app/Notifications/TransactionalEmails/ResetPassword.php index 179c8d948..511818e21 100644 --- a/app/Notifications/TransactionalEmails/ResetPassword.php +++ b/app/Notifications/TransactionalEmails/ResetPassword.php @@ -67,9 +67,12 @@ protected function resetUrl($notifiable) return call_user_func(static::$createUrlCallback, $notifiable, $this->token); } - return url(route('password.reset', [ + $path = route('password.reset', [ 'token' => $this->token, 'email' => $notifiable->getEmailForPasswordReset(), - ], false)); + ], false); + + // Use server-side config (FQDN / public IP) instead of request host + return rtrim(base_url(), '/').$path; } } diff --git a/tests/Feature/ResetPasswordUrlTest.php b/tests/Feature/ResetPasswordUrlTest.php new file mode 100644 index 000000000..03d1103f0 --- /dev/null +++ b/tests/Feature/ResetPasswordUrlTest.php @@ -0,0 +1,123 @@ +invoke($notification, $notifiable); +} + +it('generates reset URL using configured FQDN, not request host', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => 'https://coolify.example.com', 'public_ipv4' => '65.21.3.91'] + ); + Once::flush(); + + $user = User::factory()->create(); + $notification = new ResetPassword('test-token-abc', isTransactionalEmail: false); + + $url = callResetUrl($notification, $user); + + expect($url) + ->toStartWith('https://coolify.example.com/') + ->toContain('test-token-abc') + ->toContain(urlencode($user->email)) + ->not->toContain('localhost'); +}); + +it('generates reset URL using public IP when no FQDN is configured', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => null, 'public_ipv4' => '65.21.3.91'] + ); + Once::flush(); + + $user = User::factory()->create(); + $notification = new ResetPassword('test-token-abc', isTransactionalEmail: false); + + $url = callResetUrl($notification, $user); + + expect($url) + ->toContain('65.21.3.91') + ->toContain('test-token-abc') + ->not->toContain('evil.com'); +}); + +it('is immune to X-Forwarded-Host header poisoning when FQDN is set', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => 'https://coolify.example.com', 'public_ipv4' => '65.21.3.91'] + ); + Once::flush(); + + // Simulate a request with a spoofed X-Forwarded-Host header + $user = User::factory()->create(); + + $this->withHeaders([ + 'X-Forwarded-Host' => 'evil.com', + ])->get('/'); + + $notification = new ResetPassword('poisoned-token', isTransactionalEmail: false); + $url = callResetUrl($notification, $user); + + expect($url) + ->toStartWith('https://coolify.example.com/') + ->toContain('poisoned-token') + ->not->toContain('evil.com'); +}); + +it('is immune to X-Forwarded-Host header poisoning when using IP only', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => null, 'public_ipv4' => '65.21.3.91'] + ); + Once::flush(); + + $user = User::factory()->create(); + + $this->withHeaders([ + 'X-Forwarded-Host' => 'evil.com', + ])->get('/'); + + $notification = new ResetPassword('poisoned-token', isTransactionalEmail: false); + $url = callResetUrl($notification, $user); + + expect($url) + ->toContain('65.21.3.91') + ->toContain('poisoned-token') + ->not->toContain('evil.com'); +}); + +it('generates a valid route path in the reset URL', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => 'https://coolify.example.com'] + ); + Once::flush(); + + $user = User::factory()->create(); + $notification = new ResetPassword('my-token', isTransactionalEmail: false); + + $url = callResetUrl($notification, $user); + + // Should contain the password reset route path with token and email + expect($url) + ->toContain('/reset-password/') + ->toContain('my-token') + ->toContain(urlencode($user->email)); +}); diff --git a/tests/Feature/TrustHostsMiddlewareTest.php b/tests/Feature/TrustHostsMiddlewareTest.php index 5c60b30d6..a16698a6a 100644 --- a/tests/Feature/TrustHostsMiddlewareTest.php +++ b/tests/Feature/TrustHostsMiddlewareTest.php @@ -2,13 +2,16 @@ use App\Http\Middleware\TrustHosts; use App\Models\InstanceSettings; +use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Once; -uses(\Illuminate\Foundation\Testing\RefreshDatabase::class); +uses(RefreshDatabase::class); beforeEach(function () { - // Clear cache before each test to ensure isolation + // Clear cache and once() memoization to ensure isolation between tests Cache::forget('instance_settings_fqdn_host'); + Once::flush(); }); it('trusts the configured FQDN from InstanceSettings', function () { @@ -84,7 +87,7 @@ it('handles exception during InstanceSettings fetch', function () { // Drop the instance_settings table to simulate installation - \Schema::dropIfExists('instance_settings'); + Schema::dropIfExists('instance_settings'); $middleware = new TrustHosts($this->app); @@ -248,21 +251,144 @@ expect($response->status())->not->toBe(400); }); +it('populates cache on first request via handle() — no circular dependency', function () { + // Regression test: handle() used to check cache before hosts() could + // populate it, so host validation never activated. + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => 'https://coolify.example.com'] + ); + + // Clear cache to simulate cold start + Cache::forget('instance_settings_fqdn_host'); + + // Make a request — handle() should eagerly call hosts() to populate cache + $this->get('/', ['Host' => 'localhost']); + + // Cache should now be populated by the middleware + expect(Cache::get('instance_settings_fqdn_host'))->toBe('coolify.example.com'); +}); + +it('rejects host that is a superstring of trusted FQDN via suffix', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => 'https://coolify.example.com'] + ); + + // coolify.example.com.evil.com contains "coolify.example.com" as a substring — + // must NOT match. Literal hosts use exact comparison, not regex substring matching. + $response = $this->get('http://coolify.example.com.evil.com/'); + + expect($response->status())->toBe(400); +}); + +it('rejects host that is a superstring of trusted FQDN via prefix', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => 'https://coolify.example.com'] + ); + + // evil-coolify.example.com also contains the FQDN as a substring + $response = $this->get('http://evil-coolify.example.com/'); + + expect($response->status())->toBe(400); +}); + +it('rejects X-Forwarded-Host that is a superstring of trusted FQDN', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => 'https://coolify.example.com'] + ); + + $response = $this->get('/', [ + 'X-Forwarded-Host' => 'coolify.example.com.evil.com', + ]); + + expect($response->status())->toBe(400); +}); + +it('rejects host containing localhost as substring', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => 'https://coolify.example.com'] + ); + + // "evil-localhost" contains "localhost" — must not match the literal entry + $response = $this->get('http://evil-localhost/'); + + expect($response->status())->toBe(400); +}); + +it('allows subdomain of APP_URL via regex pattern', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => 'https://coolify.example.com'] + ); + + // sub.localhost should match ^(.+\.)?localhost$ from allSubdomainsOfApplicationUrl + $response = $this->get('http://sub.localhost/'); + + expect($response->status())->not->toBe(400); +}); + it('still enforces host validation for non-terminal routes', function () { InstanceSettings::updateOrCreate( ['id' => 0], ['fqdn' => 'https://coolify.example.com'] ); - // Regular routes should still validate Host header - $response = $this->get('/', [ - 'Host' => 'evil.com', - ]); + // Use full URL so Laravel's test client doesn't override Host with APP_URL + $response = $this->get('http://evil.com/'); // Should get 400 Bad Host for untrusted host expect($response->status())->toBe(400); }); +it('rejects requests with spoofed X-Forwarded-Host header', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => 'https://coolify.example.com'] + ); + + // Host header is trusted (localhost), but X-Forwarded-Host is spoofed. + // TrustHosts must reject this BEFORE TrustProxies can apply the spoofed host. + $response = $this->get('/', [ + 'X-Forwarded-Host' => 'evil.com', + ]); + + expect($response->status())->toBe(400); +}); + +it('allows legitimate X-Forwarded-Host from reverse proxy matching configured FQDN', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => 'https://coolify.example.com'] + ); + + // Legitimate request from Cloudflare/Traefik — X-Forwarded-Host matches the configured FQDN + $response = $this->get('/', [ + 'X-Forwarded-Host' => 'coolify.example.com', + ]); + + // Should NOT be rejected (would be 400 for Bad Host) + expect($response->status())->not->toBe(400); +}); + +it('allows X-Forwarded-Host with port matching configured FQDN', function () { + InstanceSettings::updateOrCreate( + ['id' => 0], + ['fqdn' => 'https://coolify.example.com'] + ); + + // Some proxies include the port in X-Forwarded-Host + $response = $this->get('/', [ + 'X-Forwarded-Host' => 'coolify.example.com:443', + ]); + + // Should NOT be rejected — port is stripped before matching + expect($response->status())->not->toBe(400); +}); + it('skips host validation for API routes', function () { // All API routes use token-based auth (Sanctum), not host validation // They should be accessible from any host (mobile apps, CLI tools, scripts) @@ -352,9 +478,10 @@ ]); expect($response->status())->not->toBe(400); - // Test Stripe webhook + // Test Stripe webhook — may return 400 from Stripe signature validation, + // but the response should NOT contain "Bad Host" (host validation error) $response = $this->post('/webhooks/payments/stripe/events', [], [ 'Host' => 'stripe-webhook-forwarder.local', ]); - expect($response->status())->not->toBe(400); + expect($response->content())->not->toContain('Bad Host'); });