From 4b8c50a54638180e3ba63cb1ce3a27d57bdae85f Mon Sep 17 00:00:00 2001
From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com>
Date: Thu, 24 Sep 2026 12:28:12 +0200
Subject: [PATCH] Improve file storage metadata handling

---
 app/Helpers/SslHelper.php              | 15 +++++++------
 app/Models/LocalFileVolume.php         |  6 ++----
 tests/Unit/FileStorageSecurityTest.php | 30 ++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/app/Helpers/SslHelper.php b/app/Helpers/SslHelper.php
index 6397c330db..edae3db580 100644
--- a/app/Helpers/SslHelper.php
+++ b/app/Helpers/SslHelper.php
@@ -191,35 +191,38 @@ class SslHelper
                     });
 
                 if ($isPemKeyFileRequired) {
-                    $model->fileStorages()->create([
+                    $fileStorage = $model->fileStorages()->make([
                         'fs_path' => $configurationDir.'/ssl/server.pem',
                         'mount_path' => $mountPath.'/server.pem',
                         'content' => $certificateStr."\n".$privateKeyStr,
                         'is_directory' => false,
-                        'chmod' => '600',
                         'resource_type' => $resourceType,
                         'resource_id' => $resourceId,
                     ]);
+                    $fileStorage->chmod = '600';
+                    $fileStorage->save();
                 } else {
-                    $model->fileStorages()->create([
+                    $fileStorage = $model->fileStorages()->make([
                         'fs_path' => $configurationDir.'/ssl/server.crt',
                         'mount_path' => $mountPath.'/server.crt',
                         'content' => $certificateStr,
                         'is_directory' => false,
-                        'chmod' => '644',
                         'resource_type' => $resourceType,
                         'resource_id' => $resourceId,
                     ]);
+                    $fileStorage->chmod = '644';
+                    $fileStorage->save();
 
-                    $model->fileStorages()->create([
+                    $fileStorage = $model->fileStorages()->make([
                         'fs_path' => $configurationDir.'/ssl/server.key',
                         'mount_path' => $mountPath.'/server.key',
                         'content' => $privateKeyStr,
                         'is_directory' => false,
-                        'chmod' => '600',
                         'resource_type' => $resourceType,
                         'resource_id' => $resourceId,
                     ]);
+                    $fileStorage->chmod = '600';
+                    $fileStorage->save();
                 }
             }
 
diff --git a/app/Models/LocalFileVolume.php b/app/Models/LocalFileVolume.php
index 8be4f8f2a1..f7048d8838 100644
--- a/app/Models/LocalFileVolume.php
+++ b/app/Models/LocalFileVolume.php
@@ -41,8 +41,6 @@ class LocalFileVolume extends BaseModel
         'resource_id',
         'is_directory',
         'is_host_file',
-        'chown',
-        'chmod',
         'is_based_on_git',
         'is_preview_suffix_enabled',
     ];
@@ -318,10 +316,10 @@ class LocalFileVolume extends BaseModel
             }
             $commands->push("chmod +x {$escapedPath}");
             if ($chown) {
-                $commands->push("chown $chown {$escapedPath}");
+                $commands->push('chown -- '.escapeshellarg($chown)." {$escapedPath}");
             }
             if ($chmod) {
-                $commands->push("chmod $chmod {$escapedPath}");
+                $commands->push('chmod -- '.escapeshellarg($chmod)." {$escapedPath}");
             }
         } elseif ($isDir === 'NOK' && $this->is_directory) {
             $commands->push("mkdir -p {$escapedPath} > /dev/null 2>&1 || true");
diff --git a/tests/Unit/FileStorageSecurityTest.php b/tests/Unit/FileStorageSecurityTest.php
index 0623c9ac72..eb7177527a 100644
--- a/tests/Unit/FileStorageSecurityTest.php
+++ b/tests/Unit/FileStorageSecurityTest.php
@@ -1,5 +1,7 @@
 <?php
 
+use App\Models\LocalFileVolume;
+
 /**
  * File Storage Security Tests
  *
@@ -216,3 +218,31 @@ test('host file mounts are bind-only and skipped by server storage writes', func
         ->and($source)->toContain('return;')
         ->and($source)->toContain('tee {$escapedPath}');
 });
+
+test('file storage quotes owner and mode as single command arguments', function () {
+    $source = file_get_contents(__DIR__.'/../../app/Models/LocalFileVolume.php');
+
+    expect($source)
+        ->toContain("'chown -- '.escapeshellarg(\$chown).\" {\$escapedPath}\"")
+        ->toContain("'chmod -- '.escapeshellarg(\$chmod).\" {\$escapedPath}\"")
+        ->not->toContain('"chown $chown {$escapedPath}"')
+        ->not->toContain('"chmod $chmod {$escapedPath}"');
+});
+
+test('file storage permissions cannot be set by mass assignment', function () {
+    $volume = new LocalFileVolume;
+    $volume->fill(['chown' => 'root', 'chmod' => '777']);
+
+    expect($volume->chown)->toBeNull()
+        ->and($volume->chmod)->toBeNull();
+});
+
+test('internal SSL files retain their private and public modes', function () {
+    $source = file_get_contents(__DIR__.'/../../app/Helpers/SslHelper.php');
+
+    expect($source)
+        ->toContain("\$fileStorage->chmod = '600';")
+        ->toContain("\$fileStorage->chmod = '644';")
+        ->toContain('$fileStorage->save();')
+        ->not->toContain("'chmod' =>");
+});