refactor: improve docker compose validation and transaction handling in StackForm

app/Livewire/Project/Service/StackForm.php

@@ -5,6 +5,7 @@ namespace App\Livewire\Project\Service;
 use App\Models\Service;
 use App\Support\ValidationPatterns;
 use Illuminate\Support\Collection;
+use Illuminate\Support\Facades\DB;
 use Livewire\Component;
 
 class StackForm extends Component
@@ -22,7 +23,7 @@ class StackForm extends Component
 
     public string $dockerComposeRaw;
 
-    public string $dockerCompose;
+    public ?string $dockerCompose = null;
 
     public ?bool $connectToDockerNetwork = null;
 
@@ -30,7 +31,7 @@ class StackForm extends Component
     {
         $baseRules = [
             'dockerComposeRaw' => 'required',
-            'dockerCompose' => 'required',
+            'dockerCompose' => 'nullable',
             'name' => ValidationPatterns::nameRules(),
             'description' => ValidationPatterns::descriptionRules(),
             'connectToDockerNetwork' => 'nullable',
@@ -140,18 +141,26 @@ class StackForm extends Component
             $this->validate();
             $this->syncData(true);
 
-            // Validate for command injection BEFORE saving to database
+            // Validate for command injection BEFORE any database operations
             validateDockerComposeForInjection($this->service->docker_compose_raw);
 
-            $this->service->save();
+            // Use transaction to ensure atomicity - if parse fails, save is rolled back
-            $this->service->saveExtraFields($this->fields);
+            DB::transaction(function () {
-            $this->service->parse();
+                $this->service->save();
-            $this->service->refresh();
+                $this->service->saveExtraFields($this->fields);
-            $this->service->saveComposeConfigs();
+                $this->service->parse();
+                $this->service->refresh();
+                $this->service->saveComposeConfigs();
+            });
+
             $this->dispatch('refreshEnvs');
             $this->dispatch('refreshServices');
             $notify && $this->dispatch('success', 'Service saved.');
         } catch (\Throwable $e) {
+            // On error, refresh from database to restore clean state
+            $this->service->refresh();
+            $this->syncData(false);
+
             return handleError($e, $this);
         } finally {
             if (is_null($this->service->config_hash)) {

bootstrap/helpers/parsers.php

@@ -59,11 +59,13 @@ function validateDockerComposeForInjection(string $composeYaml): void
                     if (isset($volume['source'])) {
                         $source = $volume['source'];
                         if (is_string($source)) {
-                            // Allow simple env vars and env vars with defaults (validated in parseDockerVolumeString)
+                            // Allow env vars and env vars with defaults (validated in parseDockerVolumeString)
+                            // Also allow env vars followed by safe path concatenation (e.g., ${VAR}/path)
                             $isSimpleEnvVar = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}$/', $source);
                             $isEnvVarWithDefault = preg_match('/^\$\{[^}]+:-[^}]*\}$/', $source);
+                            $isEnvVarWithPath = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}[\/\w\.\-]*$/', $source);
 
-                            if (! $isSimpleEnvVar && ! $isEnvVarWithDefault) {
+                            if (! $isSimpleEnvVar && ! $isEnvVarWithDefault && ! $isEnvVarWithPath) {
                                 try {
                                     validateShellSafePath($source, 'volume source');
                                 } catch (\Exception $e) {
@@ -310,15 +312,17 @@ function parseDockerVolumeString(string $volumeString): array
     // Validate source path for command injection attempts
     // We validate the final source value after environment variable processing
     if ($source !== null) {
-        // Allow simple environment variables like ${VAR_NAME} or ${VAR}
+        // Allow environment variables like ${VAR_NAME} or ${VAR}
-        // but validate everything else for shell metacharacters
+        // Also allow env vars followed by safe path concatenation (e.g., ${VAR}/path)
         $sourceStr = is_string($source) ? $source : $source;
 
         // Skip validation for simple environment variable references
-        // Pattern: ${WORD_CHARS} with no special characters inside
+        // Pattern 1: ${WORD_CHARS} with no special characters inside
+        // Pattern 2: ${WORD_CHARS}/path/to/file (env var with path concatenation)
         $isSimpleEnvVar = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}$/', $sourceStr);
+        $isEnvVarWithPath = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}[\/\w\.\-]*$/', $sourceStr);
 
-        if (! $isSimpleEnvVar) {
+        if (! $isSimpleEnvVar && ! $isEnvVarWithPath) {
             try {
                 validateShellSafePath($sourceStr, 'volume source');
             } catch (\Exception $e) {
@@ -711,9 +715,12 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
                     // Validate source and target for command injection (array/long syntax)
                     if ($source !== null && ! empty($source->value())) {
                         $sourceValue = $source->value();
-                        // Allow simple environment variable references
+                        // Allow environment variable references and env vars with path concatenation
                         $isSimpleEnvVar = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}$/', $sourceValue);
-                        if (! $isSimpleEnvVar) {
+                        $isEnvVarWithDefault = preg_match('/^\$\{[^}]+:-[^}]*\}$/', $sourceValue);
+                        $isEnvVarWithPath = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}[\/\w\.\-]*$/', $sourceValue);
+
+                        if (! $isSimpleEnvVar && ! $isEnvVarWithDefault && ! $isEnvVarWithPath) {
                             try {
                                 validateShellSafePath($sourceValue, 'volume source');
                             } catch (\Exception $e) {
@@ -1812,9 +1819,12 @@ function serviceParser(Service $resource): Collection
                     // Validate source and target for command injection (array/long syntax)
                     if ($source !== null && ! empty($source->value())) {
                         $sourceValue = $source->value();
-                        // Allow simple environment variable references
+                        // Allow environment variable references and env vars with path concatenation
                         $isSimpleEnvVar = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}$/', $sourceValue);
-                        if (! $isSimpleEnvVar) {
+                        $isEnvVarWithDefault = preg_match('/^\$\{[^}]+:-[^}]*\}$/', $sourceValue);
+                        $isEnvVarWithPath = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}[\/\w\.\-]*$/', $sourceValue);
+
+                        if (! $isSimpleEnvVar && ! $isEnvVarWithDefault && ! $isEnvVarWithPath) {
                             try {
                                 validateShellSafePath($sourceValue, 'volume source');
                             } catch (\Exception $e) {

tests/Unit/VolumeArrayFormatSecurityTest.php

@@ -194,6 +194,36 @@ YAML;
         ->not->toThrow(Exception::class);
 });
 
+test('array-format with environment variable and path concatenation', function () {
+    // This is the reported issue #7127 - ${VAR}/path should be allowed
+    $dockerComposeYaml = <<<'YAML'
+services:
+  web:
+    image: nginx
+    volumes:
+      - type: bind
+        source: '${VOLUMES_PATH}/mysql'
+        target: /var/lib/mysql
+      - type: bind
+        source: '${DATA_PATH}/config'
+        target: /etc/config
+      - type: bind
+        source: '${VOLUME_PATH}/app_data'
+        target: /app/data
+YAML;
+
+    $parsed = Yaml::parse($dockerComposeYaml);
+
+    // Verify all three volumes have the correct source format
+    expect($parsed['services']['web']['volumes'][0]['source'])->toBe('${VOLUMES_PATH}/mysql');
+    expect($parsed['services']['web']['volumes'][1]['source'])->toBe('${DATA_PATH}/config');
+    expect($parsed['services']['web']['volumes'][2]['source'])->toBe('${VOLUME_PATH}/app_data');
+
+    // The validation should allow this - the reported bug was that it was blocked
+    expect(fn () => validateDockerComposeForInjection($dockerComposeYaml))
+        ->not->toThrow(Exception::class);
+});
+
 test('array-format with malicious environment variable default', function () {
     $dockerComposeYaml = <<<'YAML'
 services:

tests/Unit/VolumeSecurityTest.php

@@ -94,6 +94,27 @@ test('parseDockerVolumeString accepts simple environment variables', function ()
     }
 });
 
+test('parseDockerVolumeString accepts environment variables with path concatenation', function () {
+    $volumes = [
+        '${VOLUMES_PATH}/mysql:/var/lib/mysql',
+        '${DATA_PATH}/config:/etc/config',
+        '${VOLUME_PATH}/app_data:/app',
+        '${MY_VAR_123}/deep/nested/path:/data',
+        '${VAR}/path:/app',
+        '${VAR}_suffix:/app',
+        '${VAR}-suffix:/app',
+        '${VAR}.ext:/app',
+        '${VOLUMES_PATH}/mysql:/var/lib/mysql:ro',
+        '${DATA_PATH}/config:/etc/config:rw',
+    ];
+
+    foreach ($volumes as $volume) {
+        $result = parseDockerVolumeString($volume);
+        expect($result)->toBeArray();
+        expect($result['source'])->not->toBeNull();
+    }
+});
+
 test('parseDockerVolumeString rejects environment variables with command injection in default', function () {
     $maliciousVolumes = [
         '${VAR:-`whoami`}:/app',