From 0ecaa191db150bbc1ce120b00821517a4404990f Mon Sep 17 00:00:00 2001
From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com>
Date: Thu, 27 Nov 2025 10:57:24 +0100
Subject: [PATCH] fix: prevent SERVICE_FQDN/SERVICE_URL path duplication on
 FQDN updates
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Add endsWith() checks before appending template paths in serviceParser() to
prevent duplicate paths when parse() is called after FQDN updates. This fixes
the bug where services like Appwrite realtime would get `/v1/realtime/v1/realtime`.

Fixes #7363

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
---
 bootstrap/helpers/parsers.php                 |  13 +-
 tests/Feature/ServiceFqdnUpdatePathTest.php   | 220 ++++++++++++++++++
 .../Unit/ServiceParserPathDuplicationTest.php | 150 ++++++++++++
 3 files changed, 380 insertions(+), 3 deletions(-)
 create mode 100644 tests/Feature/ServiceFqdnUpdatePathTest.php
 create mode 100644 tests/Unit/ServiceParserPathDuplicationTest.php

diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php
index dfcc3e190..e7d875777 100644
--- a/bootstrap/helpers/parsers.php
+++ b/bootstrap/helpers/parsers.php
@@ -1644,9 +1644,16 @@ function serviceParser(Service $resource): Collection
                 if ($value && get_class($value) === \Illuminate\Support\Stringable::class && $value->startsWith('/')) {
                     $path = $value->value();
                     if ($path !== '/') {
-                        $fqdn = "$fqdn$path";
-                        $url = "$url$path";
-                        $fqdnValueForEnv = "$fqdnValueForEnv$path";
+                        // Only add path if it's not already present (prevents duplication on subsequent parse() calls)
+                        if (! str($fqdn)->endsWith($path)) {
+                            $fqdn = "$fqdn$path";
+                        }
+                        if (! str($url)->endsWith($path)) {
+                            $url = "$url$path";
+                        }
+                        if (! str($fqdnValueForEnv)->endsWith($path)) {
+                            $fqdnValueForEnv = "$fqdnValueForEnv$path";
+                        }
                     }
                 }
 
diff --git a/tests/Feature/ServiceFqdnUpdatePathTest.php b/tests/Feature/ServiceFqdnUpdatePathTest.php
new file mode 100644
index 000000000..4c0c4238f
--- /dev/null
+++ b/tests/Feature/ServiceFqdnUpdatePathTest.php
@@ -0,0 +1,220 @@
+<?php
+
+/**
+ * Feature tests to verify that FQDN updates don't cause path duplication
+ * for services with path-based SERVICE_URL/SERVICE_FQDN template variables.
+ *
+ * This tests the fix for GitHub issue #7363 where Appwrite and MindsDB services
+ * had their paths duplicated (e.g., /v1/realtime/v1/realtime) after FQDN updates.
+ *
+ * IMPORTANT: These tests require database access and must be run inside Docker:
+ * docker exec coolify php artisan test --filter ServiceFqdnUpdatePathTest
+ */
+
+use App\Models\Server;
+use App\Models\Service;
+use App\Models\ServiceApplication;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+
+uses(RefreshDatabase::class);
+
+test('Appwrite realtime service does not duplicate path on FQDN update', function () {
+    // Create a server
+    $server = Server::factory()->create([
+        'name' => 'test-server',
+        'ip' => '127.0.0.1',
+    ]);
+
+    // Load Appwrite template
+    $appwriteTemplate = file_get_contents(base_path('templates/compose/appwrite.yaml'));
+
+    // Create Appwrite service
+    $service = Service::factory()->create([
+        'server_id' => $server->id,
+        'name' => 'appwrite-test',
+        'docker_compose_raw' => $appwriteTemplate,
+    ]);
+
+    // Create the appwrite-realtime service application
+    $serviceApp = ServiceApplication::factory()->create([
+        'service_id' => $service->id,
+        'name' => 'appwrite-realtime',
+        'fqdn' => 'https://test.abc/v1/realtime',
+    ]);
+
+    // Parse the service (simulates initial setup)
+    $service->parse();
+
+    // Get environment variable
+    $urlVar = $service->environment_variables()
+        ->where('key', 'SERVICE_URL_APPWRITE')
+        ->first();
+
+    // Initial setup should have path once
+    expect($urlVar)->not->toBeNull()
+        ->and($urlVar->value)->not->toContain('/v1/realtime/v1/realtime')
+        ->and($urlVar->value)->toContain('/v1/realtime');
+
+    // Simulate user updating FQDN
+    $serviceApp->fqdn = 'https://newdomain.com/v1/realtime';
+    $serviceApp->save();
+
+    // Call parse again (this is where the bug occurred)
+    $service->parse();
+
+    // Check that path is not duplicated
+    $urlVar = $service->environment_variables()
+        ->where('key', 'SERVICE_URL_APPWRITE')
+        ->first();
+
+    expect($urlVar)->not->toBeNull()
+        ->and($urlVar->value)->not->toContain('/v1/realtime/v1/realtime')
+        ->and($urlVar->value)->toContain('/v1/realtime');
+})->skip('Requires database and Appwrite template - run in Docker');
+
+test('Appwrite console service does not duplicate /console path', function () {
+    $server = Server::factory()->create();
+    $appwriteTemplate = file_get_contents(base_path('templates/compose/appwrite.yaml'));
+    $service = Service::factory()->create([
+        'server_id' => $server->id,
+        'docker_compose_raw' => $appwriteTemplate,
+    ]);
+
+    $serviceApp = ServiceApplication::factory()->create([
+        'service_id' => $service->id,
+        'name' => 'appwrite-console',
+        'fqdn' => 'https://test.abc/console',
+    ]);
+
+    // Parse service
+    $service->parse();
+
+    // Update FQDN
+    $serviceApp->fqdn = 'https://newdomain.com/console';
+    $serviceApp->save();
+
+    // Parse again
+    $service->parse();
+
+    // Verify no duplication
+    $urlVar = $service->environment_variables()
+        ->where('key', 'SERVICE_URL_APPWRITE')
+        ->first();
+
+    expect($urlVar)->not->toBeNull()
+        ->and($urlVar->value)->not->toContain('/console/console')
+        ->and($urlVar->value)->toContain('/console');
+})->skip('Requires database and Appwrite template - run in Docker');
+
+test('MindsDB service does not duplicate /api path', function () {
+    $server = Server::factory()->create();
+    $mindsdbTemplate = file_get_contents(base_path('templates/compose/mindsdb.yaml'));
+    $service = Service::factory()->create([
+        'server_id' => $server->id,
+        'docker_compose_raw' => $mindsdbTemplate,
+    ]);
+
+    $serviceApp = ServiceApplication::factory()->create([
+        'service_id' => $service->id,
+        'name' => 'mindsdb',
+        'fqdn' => 'https://test.abc/api',
+    ]);
+
+    // Parse service
+    $service->parse();
+
+    // Update FQDN multiple times
+    $serviceApp->fqdn = 'https://domain1.com/api';
+    $serviceApp->save();
+    $service->parse();
+
+    $serviceApp->fqdn = 'https://domain2.com/api';
+    $serviceApp->save();
+    $service->parse();
+
+    // Verify no duplication after multiple updates
+    $urlVar = $service->environment_variables()
+        ->where('key', 'SERVICE_URL_API')
+        ->orWhere('key', 'LIKE', 'SERVICE_URL_%')
+        ->first();
+
+    expect($urlVar)->not->toBeNull()
+        ->and($urlVar->value)->not->toContain('/api/api');
+})->skip('Requires database and MindsDB template - run in Docker');
+
+test('service without path declaration is not affected', function () {
+    $server = Server::factory()->create();
+
+    // Create a simple service without path in template
+    $simpleTemplate = <<<'YAML'
+services:
+  redis:
+    image: redis:7
+    environment:
+      - SERVICE_FQDN_REDIS
+YAML;
+
+    $service = Service::factory()->create([
+        'server_id' => $server->id,
+        'docker_compose_raw' => $simpleTemplate,
+    ]);
+
+    $serviceApp = ServiceApplication::factory()->create([
+        'service_id' => $service->id,
+        'name' => 'redis',
+        'fqdn' => 'https://redis.test.abc',
+    ]);
+
+    // Parse service
+    $service->parse();
+
+    $fqdnBefore = $service->environment_variables()
+        ->where('key', 'SERVICE_FQDN_REDIS')
+        ->first()?->value;
+
+    // Update FQDN
+    $serviceApp->fqdn = 'https://redis.newdomain.com';
+    $serviceApp->save();
+
+    // Parse again
+    $service->parse();
+
+    $fqdnAfter = $service->environment_variables()
+        ->where('key', 'SERVICE_FQDN_REDIS')
+        ->first()?->value;
+
+    // Should work normally without issues
+    expect($fqdnAfter)->toBe('redis.newdomain.com')
+        ->and($fqdnAfter)->not->toContain('//');
+})->skip('Requires database - run in Docker');
+
+test('multiple FQDN updates never cause path duplication', function () {
+    $server = Server::factory()->create();
+    $appwriteTemplate = file_get_contents(base_path('templates/compose/appwrite.yaml'));
+    $service = Service::factory()->create([
+        'server_id' => $server->id,
+        'docker_compose_raw' => $appwriteTemplate,
+    ]);
+
+    $serviceApp = ServiceApplication::factory()->create([
+        'service_id' => $service->id,
+        'name' => 'appwrite-realtime',
+        'fqdn' => 'https://test.abc/v1/realtime',
+    ]);
+
+    // Update FQDN 10 times and parse each time
+    for ($i = 1; $i <= 10; $i++) {
+        $serviceApp->fqdn = "https://domain{$i}.com/v1/realtime";
+        $serviceApp->save();
+        $service->parse();
+
+        // Check path is never duplicated
+        $urlVar = $service->environment_variables()
+            ->where('key', 'SERVICE_URL_APPWRITE')
+            ->first();
+
+        expect($urlVar)->not->toBeNull()
+            ->and($urlVar->value)->not->toContain('/v1/realtime/v1/realtime')
+            ->and($urlVar->value)->toContain('/v1/realtime');
+    }
+})->skip('Requires database and Appwrite template - run in Docker');
diff --git a/tests/Unit/ServiceParserPathDuplicationTest.php b/tests/Unit/ServiceParserPathDuplicationTest.php
new file mode 100644
index 000000000..74ee1d215
--- /dev/null
+++ b/tests/Unit/ServiceParserPathDuplicationTest.php
@@ -0,0 +1,150 @@
+<?php
+
+/**
+ * Unit tests to verify that serviceParser() correctly handles path appending
+ * to prevent duplication when SERVICE_URL/SERVICE_FQDN variables have path values.
+ *
+ * This tests the fix for GitHub issue #7363 where paths like /v1/realtime
+ * were being duplicated on subsequent parse() calls after FQDN updates.
+ */
+test('path is added when FQDN does not already contain it', function () {
+    $fqdn = 'https://test.abc';
+    $path = '/v1/realtime';
+
+    // Simulate the logic in serviceParser()
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+
+    expect($fqdn)->toBe('https://test.abc/v1/realtime');
+});
+
+test('path is not added when FQDN already contains it', function () {
+    $fqdn = 'https://test.abc/v1/realtime';
+    $path = '/v1/realtime';
+
+    // Simulate the logic in serviceParser()
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+
+    expect($fqdn)->toBe('https://test.abc/v1/realtime');
+});
+
+test('multiple parse calls with same path do not cause duplication', function () {
+    $fqdn = 'https://test.abc';
+    $path = '/v1/realtime';
+
+    // First parse (initial creation)
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+    expect($fqdn)->toBe('https://test.abc/v1/realtime');
+
+    // Second parse (after FQDN update)
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+    expect($fqdn)->toBe('https://test.abc/v1/realtime');
+
+    // Third parse (another update)
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+    expect($fqdn)->toBe('https://test.abc/v1/realtime');
+});
+
+test('different paths for different services work correctly', function () {
+    // Appwrite main service (/)
+    $fqdn1 = 'https://test.abc';
+    $path1 = '/';
+    if ($path1 !== '/' && ! str($fqdn1)->endsWith($path1)) {
+        $fqdn1 = "$fqdn1$path1";
+    }
+    expect($fqdn1)->toBe('https://test.abc');
+
+    // Appwrite console (/console)
+    $fqdn2 = 'https://test.abc';
+    $path2 = '/console';
+    if ($path2 !== '/' && ! str($fqdn2)->endsWith($path2)) {
+        $fqdn2 = "$fqdn2$path2";
+    }
+    expect($fqdn2)->toBe('https://test.abc/console');
+
+    // Appwrite realtime (/v1/realtime)
+    $fqdn3 = 'https://test.abc';
+    $path3 = '/v1/realtime';
+    if ($path3 !== '/' && ! str($fqdn3)->endsWith($path3)) {
+        $fqdn3 = "$fqdn3$path3";
+    }
+    expect($fqdn3)->toBe('https://test.abc/v1/realtime');
+});
+
+test('nested paths are handled correctly', function () {
+    $fqdn = 'https://test.abc';
+    $path = '/api/v1/endpoint';
+
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+
+    expect($fqdn)->toBe('https://test.abc/api/v1/endpoint');
+
+    // Second call should not duplicate
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+
+    expect($fqdn)->toBe('https://test.abc/api/v1/endpoint');
+});
+
+test('MindsDB /api path is handled correctly', function () {
+    $fqdn = 'https://test.abc';
+    $path = '/api';
+
+    // First parse
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+    expect($fqdn)->toBe('https://test.abc/api');
+
+    // Second parse should not duplicate
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+    expect($fqdn)->toBe('https://test.abc/api');
+});
+
+test('fqdnValueForEnv path handling works correctly', function () {
+    $fqdnValueForEnv = 'test.abc';
+    $path = '/v1/realtime';
+
+    // First append
+    if (! str($fqdnValueForEnv)->endsWith($path)) {
+        $fqdnValueForEnv = "$fqdnValueForEnv$path";
+    }
+    expect($fqdnValueForEnv)->toBe('test.abc/v1/realtime');
+
+    // Second attempt should not duplicate
+    if (! str($fqdnValueForEnv)->endsWith($path)) {
+        $fqdnValueForEnv = "$fqdnValueForEnv$path";
+    }
+    expect($fqdnValueForEnv)->toBe('test.abc/v1/realtime');
+});
+
+test('url path handling works correctly', function () {
+    $url = 'https://test.abc';
+    $path = '/v1/realtime';
+
+    // First append
+    if (! str($url)->endsWith($path)) {
+        $url = "$url$path";
+    }
+    expect($url)->toBe('https://test.abc/v1/realtime');
+
+    // Second attempt should not duplicate
+    if (! str($url)->endsWith($path)) {
+        $url = "$url$path";
+    }
+    expect($url)->toBe('https://test.abc/v1/realtime');
+});