fix(files): support a fully private S3 media bucket

src/Models/Extension.php

@@ -187,7 +187,8 @@ public function getAuthorNameAttribute()
      */
     public function getIconUrlAttribute()
     {
-        return static::attributeFromCache($this, 'file.url', 'https://s3.ap-southeast-1.amazonaws.com/flb-assets/static/no-avatar.png');
+        // short TTL: file.url is a signed URL that expires, a day-long cache would serve dead links
+        return static::attributeFromCache($this, 'file.url', 'https://s3.ap-southeast-1.amazonaws.com/flb-assets/static/no-avatar.png', 30 * 60);
     }
 
     /**

src/Models/File.php

@@ -147,30 +147,102 @@ public function getUrlAttribute()
         /** @var Storage $filesystem */
         $filesystem = $this->getFilesystem();
 
-        $cacheKey      = "file_url_{$this->uuid}";
-        $bufferTime    = 5; // Buffer time in minutes
+        if ($disk === 's3' || $disk === 'gcs') {
+            return static::cachedTemporaryUrl($filesystem, $this->path, "file_url_{$this->uuid}");
+        }
+
+        $url = $filesystem->url($this->path);
+
+        if ($disk === 'local') {
+            return asset($url, !app()->environment(['development', 'local']));
+        }
+
+        return $url;
+    }
+
+    /**
+     * Generate a signed URL for an object, cached for slightly less than its lifetime.
+     */
+    protected static function cachedTemporaryUrl($filesystem, string $path, string $cacheKey): string
+    {
+        // Cache for half the signature's lifetime, so every URL handed out has at least an hour left.
+        // Callers (browser tabs, short-lived caches) hold the string after we return it.
+        $bufferTime    = 60; // Buffer time in minutes
         $urlExpiration = 120; // URL expiration time in minutes (2 hours)
 
-        if ($disk === 's3' || $disk === 'gcs') {
-            // Check if the URL is already cached
-            if (Cache::has($cacheKey)) {
-                return Cache::get($cacheKey);
-            }
+        // Check if the URL is already cached
+        if (Cache::has($cacheKey)) {
+            return Cache::get($cacheKey);
+        }
+
+        // Generate a new temporary URL
+        $url = $filesystem->temporaryUrl($path, now()->addMinutes($urlExpiration));
+
+        // Cache the URL with a reduced expiration time for buffer
+        Cache::put($cacheKey, $url, now()->addMinutes($urlExpiration - $bufferTime));
+
+        return $url;
+    }
 
-            // Generate a new temporary URL
-            $url = $filesystem->temporaryUrl($this->path, now()->addMinutes($urlExpiration));
+    /**
+     * Re-sign an absolute URL that was stored as a string and points into the configured S3 bucket.
+     *
+     * Some columns (e.g. legacy `avatar_url` values, cart item image URLs) hold a URL rather than a
+     * File reference. Plain bucket URLs only work while the bucket is publicly readable, and stored
+     * signed URLs stop working once their signature expires, so both are turned back into an object
+     * key and signed afresh. Anything else (other hosts, flb-assets, relative paths, UUIDs) is
+     * returned unchanged.
+     */
+    public static function signStoredUrl(?string $url): ?string
+    {
+        $key = static::s3KeyFromUrl($url);
+        if ($key === null) {
+            return $url;
+        }
 
-            // Cache the URL with a reduced expiration time for buffer
-            Cache::put($cacheKey, $url, now()->addMinutes($urlExpiration - $bufferTime));
+        return static::cachedTemporaryUrl(Storage::disk('s3'), $key, 'file_url_key_' . sha1($key));
+    }
+
+    /**
+     * Extract the object key from a URL that points into the configured S3 bucket, or null.
+     *
+     * Recognises virtual-hosted (`bucket.s3.region.amazonaws.com/key`, `bucket.s3-region...`),
+     * path-style (`s3.region.amazonaws.com/bucket/key`) and the disk's configured `url` (AWS_URL).
+     */
+    public static function s3KeyFromUrl(?string $url): ?string
+    {
+        if (!is_string($url) || !preg_match('#^https?://#i', $url)) {
+            return null;
+        }
+
+        $bucket = config('filesystems.disks.s3.bucket');
+        if (!is_string($bucket) || $bucket === '') {
+            return null;
+        }
+
+        $key        = null;
+        $withoutQs  = preg_split('/[?#]/', $url, 2)[0];
+        $configured = config('filesystems.disks.s3.url');
+
+        if (is_string($configured) && $configured !== '' && Str::startsWith($withoutQs, rtrim($configured, '/') . '/')) {
+            $key = Str::after($withoutQs, rtrim($configured, '/') . '/');
         } else {
-            $url = $filesystem->url($this->path);
+            $host   = strtolower((string) parse_url($withoutQs, PHP_URL_HOST));
+            $path   = ltrim((string) parse_url($withoutQs, PHP_URL_PATH), '/');
+            $quoted = preg_quote(strtolower($bucket), '#');
+
+            if (preg_match('#^' . $quoted . '\.s3([.-][a-z0-9-]+)?\.amazonaws\.com$#', $host)) {
+                $key = $path;
+            } elseif (preg_match('#^s3([.-][a-z0-9-]+)?\.amazonaws\.com$#', $host) && Str::startsWith($path, $bucket . '/')) {
+                $key = Str::after($path, $bucket . '/');
+            }
         }
 
-        if ($disk === 'local') {
-            return asset($url, !app()->environment(['development', 'local']));
+        if ($key === null || $key === '') {
+            return null;
         }
 
-        return $url;
+        return rawurldecode($key);
     }
 
     /**

src/Services/TemplateRenderService.php

@@ -2,6 +2,7 @@
 
 namespace Fleetbase\Services;
 
+use Fleetbase\Models\File;
 use Fleetbase\Models\Template;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Support\Carbon;
@@ -297,7 +298,8 @@ protected function renderElement(array $element): string
                 return "<div style=\"{$styleStr}\">{$content}</div>\n";
 
             case 'image':
-                $src = data_get($element, 'src', '');
+                // the builder stores the upload's URL; re-sign it so old (expired or unsigned) bucket URLs still render
+                $src = File::signStoredUrl((string) data_get($element, 'src', ''));
 
                 return "<img src=\"{$src}\" style=\"{$styleStr}\" alt=\"\" />\n";
 

src/Support/Utils.php

@@ -1791,8 +1791,9 @@ public static function urlToStorefrontFile($url, $type = 'source', ?Model $owner
         $bucketPath = 'uploads/storefront/' . $owner->uuid . '/' . Str::slug($type) . '/' . $fileName;
         $pathInfo   = pathinfo($bucketPath);
 
-        // upload to bucket
-        Storage::disk('s3')->put($bucketPath, $contents, 'public');
+        // upload to bucket. No 'public' visibility: the media bucket is private and enforces
+        // bucket-owner object ownership, which rejects any request carrying an ACL.
+        Storage::disk('s3')->put($bucketPath, $contents);
 
         $fileInfo = [
             'company_uuid'  => $owner->company_uuid ?? null,

tests/Unit/Models/FileModelTest.php

@@ -343,6 +343,53 @@ function bind_file_model_filesystem(array $config = []): FileModelFilesystemFake
     expect($file->url)->toBe('https://cdn.example.test/cached-report.csv');
 });
 
+it('extracts object keys only from urls that point into the configured s3 bucket', function () {
+    bind_file_model_filesystem([
+        'filesystems.disks.s3.bucket' => 'fleetbase-production-media',
+        'filesystems.disks.s3.url'    => 'https://media.example.test/assets',
+    ]);
+
+    expect(File::s3KeyFromUrl('https://fleetbase-production-media.s3.amazonaws.com/uploads/a/photo.png'))->toBe('uploads/a/photo.png')
+        ->and(File::s3KeyFromUrl('https://fleetbase-production-media.s3.ap-southeast-1.amazonaws.com/uploads/a/photo.png?X-Amz-Signature=old'))->toBe('uploads/a/photo.png')
+        ->and(File::s3KeyFromUrl('https://fleetbase-production-media.s3-ap-southeast-1.amazonaws.com/custom-avatars/vehicles/c/My%20Van.png'))->toBe('custom-avatars/vehicles/c/My Van.png')
+        ->and(File::s3KeyFromUrl('https://s3.ap-southeast-1.amazonaws.com/fleetbase-production-media/uploads/b/logo.png'))->toBe('uploads/b/logo.png')
+        ->and(File::s3KeyFromUrl('https://media.example.test/assets/uploads/c/doc.pdf'))->toBe('uploads/c/doc.pdf')
+        // other buckets, other hosts and non-urls are left alone
+        ->and(File::s3KeyFromUrl('https://flb-assets.s3.ap-southeast-1.amazonaws.com/static/no-avatar.png'))->toBeNull()
+        ->and(File::s3KeyFromUrl('https://s3.ap-southeast-1.amazonaws.com/flb-assets/static/no-avatar.png'))->toBeNull()
+        ->and(File::s3KeyFromUrl('https://evil.example.test/fleetbase-production-media.s3.amazonaws.com/x.png'))->toBeNull()
+        ->and(File::s3KeyFromUrl('https://fleetbase-production-media.s3.amazonaws.com.evil.test/x.png'))->toBeNull()
+        ->and(File::s3KeyFromUrl('https://fleetbase-production-media.s3.amazonaws.com/'))->toBeNull()
+        ->and(File::s3KeyFromUrl('5f1c2a10-0000-4000-8000-000000000000'))->toBeNull()
+        ->and(File::s3KeyFromUrl(null))->toBeNull();
+
+    // config is shared across tests: drop the url override here as well
+    bind_file_model_filesystem(['filesystems.disks.s3.bucket' => null, 'filesystems.disks.s3.url' => null]);
+
+    expect(File::s3KeyFromUrl('https://fleetbase-production-media.s3.amazonaws.com/uploads/a/photo.png'))->toBeNull();
+});
+
+it('re-signs stored bucket urls and leaves every other value unchanged', function () {
+    $filesystem = bind_file_model_filesystem([
+        'filesystems.disks.s3.bucket' => 'fleetbase-production-media',
+    ]);
+
+    $stored = 'https://fleetbase-production-media.s3.ap-southeast-1.amazonaws.com/custom-avatars/vehicles/c/van.png?X-Amz-Expires=7200&X-Amz-Signature=expired';
+
+    expect(File::signStoredUrl($stored))->toBe('https://s3.example.test/custom-avatars/vehicles/c/van.png?temporary=1')
+        ->and($filesystem->disk('s3')->temporaryUrls)->toHaveKey('custom-avatars/vehicles/c/van.png')
+        ->and(File::signStoredUrl('https://flb-assets.s3.ap-southeast-1.amazonaws.com/static/vehicle-icons/mini_bus.svg'))
+        ->toBe('https://flb-assets.s3.ap-southeast-1.amazonaws.com/static/vehicle-icons/mini_bus.svg')
+        ->and(File::signStoredUrl(null))->toBeNull()
+        ->and(File::signStoredUrl(''))->toBe('');
+
+    // the signed url is cached per object key, like File::url
+    $filesystem->disk('s3')->temporaryUrls = [];
+
+    expect(File::signStoredUrl($stored))->toBe('https://s3.example.test/custom-avatars/vehicles/c/van.png?temporary=1')
+        ->and($filesystem->disk('s3')->temporaryUrls)->toBe([]);
+});
+
 it('assigns uploaders subjects and file types through model mutators', function () {
     bind_test_container();