From c14ce75a227772b255361df90c752b5aec19acc7 Mon Sep 17 00:00:00 2001 From: bernardhanna Date: Fri, 9 Oct 2026 14:46:22 +0100 Subject: [PATCH] Force bulk upload temps onto S3 even when Forge still has local. Deployed apps ignore BULK_UPLOAD_TEMP_DISK=local so validate/import share the same object across web nodes; verify the put before continuing. Co-authored-by: Cursor --- .../Controllers/BulkEventUploadController.php | 58 +++++++++++++------ app/Jobs/ProcessBulkEventImportJob.php | 2 +- app/Jobs/ValidateBulkEventUploadJob.php | 2 +- app/Services/BulkEventUploadCache.php | 20 +++++++ tests/Unit/BulkEventUploadCacheTest.php | 14 +++++ 5 files changed, 76 insertions(+), 20 deletions(-) diff --git a/app/Http/Controllers/BulkEventUploadController.php b/app/Http/Controllers/BulkEventUploadController.php index d41e72ae7..3747e828a 100644 --- a/app/Http/Controllers/BulkEventUploadController.php +++ b/app/Http/Controllers/BulkEventUploadController.php @@ -76,12 +76,38 @@ function ($attribute, $value, $fail) { ->withInput(); } - $tempDisk = $this->tempDisk(); - $path = $file->storeAs( - 'temp/bulk-events', - 'bulk_events_'.time().'_'.Str::random(8).'.'.$extension, - ['disk' => $tempDisk, 'visibility' => 'private'] - ); + $tempDisk = BulkEventUploadCache::tempDisk(); + $path = 'temp/bulk-events/bulk_events_'.time().'_'.Str::random(8).'.'.$extension; + + try { + $stored = Storage::disk($tempDisk)->put( + $path, + $file->get(), + ['visibility' => 'private'] + ); + } catch (\Throwable $e) { + Log::error('Bulk event upload: failed to store temp file', [ + 'disk' => $tempDisk, + 'path' => $path, + 'error' => $e->getMessage(), + ]); + + return redirect()->route('admin.bulk-upload.index') + ->withErrors(['file' => 'Could not store upload on '.$tempDisk.': '.$e->getMessage()]) + ->withInput(); + } + + if ($stored === false || ! Storage::disk($tempDisk)->exists($path)) { + Log::error('Bulk event upload: temp file missing immediately after store', [ + 'disk' => $tempDisk, + 'path' => $path, + 'stored' => $stored, + ]); + + return redirect()->route('admin.bulk-upload.index') + ->withErrors(['file' => 'Could not store upload on shared disk ('.$tempDisk.'). Check AWS credentials / BULK_UPLOAD_TEMP_DISK.']) + ->withInput(); + } $headerCheck = BulkEventUploadValidator::validateRequiredColumns($path, $tempDisk); if (isset($headerCheck['error'])) { @@ -188,16 +214,22 @@ public function import(Request $request): RedirectResponse } $path = (string) ($payload['path'] ?? ''); - $disk = (string) ($payload['disk'] ?? $this->tempDisk()); + $disk = BulkEventUploadCache::tempDisk($payload['disk'] ?? null); + if ($path === '' || ! Storage::disk($disk)->exists($path)) { Log::warning('Bulk event import: temp file missing', [ 'token' => $token, 'disk' => $disk, + 'cached_disk' => $payload['disk'] ?? null, 'path' => $path, ]); return redirect()->route('admin.bulk-upload.index') - ->withErrors(['import' => 'Uploaded file no longer available. Please upload again.']); + ->withErrors(['import' => 'Uploaded file no longer available (disk: '.$disk.'). Please upload again.']); + } + + if (($payload['disk'] ?? null) !== $disk) { + BulkEventUploadCache::merge($token, ['disk' => $disk]); } BulkEventUploadCache::merge($token, [ @@ -237,14 +269,4 @@ public function report(Request $request, ?string $token = null): View|RedirectRe return redirect()->route('admin.bulk-upload.index') ->with('info', 'Report no longer available. Run an import to see a new report.'); } - - /** - * Shared temp disk (Amazon S3). `local` is not shared across web nodes. - */ - private function tempDisk(): string - { - $disk = (string) config('filesystems.bulk_upload_temp_disk', 's3'); - - return $disk !== '' ? $disk : 's3'; - } } diff --git a/app/Jobs/ProcessBulkEventImportJob.php b/app/Jobs/ProcessBulkEventImportJob.php index b03c972dd..c15edc7ab 100644 --- a/app/Jobs/ProcessBulkEventImportJob.php +++ b/app/Jobs/ProcessBulkEventImportJob.php @@ -36,7 +36,7 @@ public function handle(): void } $path = (string) ($payload['path'] ?? ''); - $disk = (string) ($payload['disk'] ?? 'local'); + $disk = BulkEventUploadCache::tempDisk($payload['disk'] ?? null); $defaultCreatorEmail = $payload['default_creator_email'] ?? null; if ($path === '' || ! Storage::disk($disk)->exists($path)) { diff --git a/app/Jobs/ValidateBulkEventUploadJob.php b/app/Jobs/ValidateBulkEventUploadJob.php index e0eb4a637..e9a6cbb84 100644 --- a/app/Jobs/ValidateBulkEventUploadJob.php +++ b/app/Jobs/ValidateBulkEventUploadJob.php @@ -35,7 +35,7 @@ public function handle(): void } $path = (string) ($payload['path'] ?? ''); - $disk = (string) ($payload['disk'] ?? 'local'); + $disk = BulkEventUploadCache::tempDisk($payload['disk'] ?? null); $defaultCreatorEmail = $payload['default_creator_email'] ?? null; if ($path === '' || ! Storage::disk($disk)->exists($path)) { diff --git a/app/Services/BulkEventUploadCache.php b/app/Services/BulkEventUploadCache.php index e6c5901f2..4b40447b4 100644 --- a/app/Services/BulkEventUploadCache.php +++ b/app/Services/BulkEventUploadCache.php @@ -24,6 +24,26 @@ public static function key(string $token): string return 'bulk_upload_'.$token; } + /** + * Shared temp disk for validate → import. Never use `local` on deployed apps + * (Forge may still set BULK_UPLOAD_TEMP_DISK=local from the old example). + */ + public static function tempDisk(?string $cachedDisk = null): string + { + $configured = (string) ($cachedDisk ?: config('filesystems.bulk_upload_temp_disk', 's3')); + $cloud = (string) config('filesystems.cloud', 's3'); + + if (app()->environment('local', 'testing')) { + return $configured !== '' ? $configured : 'local'; + } + + if ($configured === '' || $configured === 'local') { + return $cloud !== '' ? $cloud : 's3'; + } + + return $configured; + } + /** * @return array|null */ diff --git a/tests/Unit/BulkEventUploadCacheTest.php b/tests/Unit/BulkEventUploadCacheTest.php index 7852dadd0..f9c82a9c7 100644 --- a/tests/Unit/BulkEventUploadCacheTest.php +++ b/tests/Unit/BulkEventUploadCacheTest.php @@ -29,4 +29,18 @@ public function test_preview_from_result_uses_failures_only_for_large_uploads(): $this->assertCount(1, $preview['row_statuses']); $this->assertSame(42, $preview['row_statuses'][0]['row']); } + + public function test_temp_disk_rejects_local_outside_local_environment(): void + { + config([ + 'filesystems.bulk_upload_temp_disk' => 'local', + 'filesystems.cloud' => 's3', + 'app.env' => 'production', + ]); + app()->detectEnvironment(fn () => 'production'); + + $this->assertSame('s3', BulkEventUploadCache::tempDisk()); + $this->assertSame('s3', BulkEventUploadCache::tempDisk('local')); + $this->assertSame('s3', BulkEventUploadCache::tempDisk('s3')); + } }