From e31249dd097b8b3b526808c378fbc5db1bc941fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Mon, 13 Apr 2026 23:57:59 +0200 Subject: [PATCH 1/6] Prevent mkdir() race conditions in FilesystemTenancyBootstrapper (#1453) This prevents race conditions that may occur if there are two concurrent processes trying to create the storage path for the tenant. The storagePath() method runs during bootstrap() which can easily happen in two places at once. The race condition specifically occurs in between the is_dir() check and the mkdir() call, the latter producing an exception if the dir already exist. We simply ignore any error coming out of mkdir() and then check for success separately. We could omit that success check since failure is unlikely and would only occur due to a server misconfiguration that would manifest itself in other ways as well, but this way the simple TOC/TOU race condition is prevented while other errors are still reported. We apply the same change to the mkdir() in scopeSessions() as the logic is similar. Resolves #1452 --- .../FilesystemTenancyBootstrapper.php | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/Bootstrappers/FilesystemTenancyBootstrapper.php b/src/Bootstrappers/FilesystemTenancyBootstrapper.php index af2b809f..56091600 100644 --- a/src/Bootstrappers/FilesystemTenancyBootstrapper.php +++ b/src/Bootstrappers/FilesystemTenancyBootstrapper.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Stancl\Tenancy\Bootstrappers; +use Exception; use Illuminate\Foundation\Application; use Illuminate\Session\FileSessionHandler; use Illuminate\Support\Facades\Storage; @@ -75,8 +76,13 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper : $this->originalStoragePath . '/framework/cache'; if (! is_dir($path)) { - // Create tenant framework/cache directory if it does not exist - mkdir($path, 0750, true); + // Create tenant framework/cache directory if it does not exist. + // We ignore errors due to TOCTOU race conditions, instead we check for success below. + @mkdir($path, 0750, true); + + if (! is_dir($path)) { + throw new Exception("Unable to create tenant storage directory [{$path}]."); + } } if ($suffix === false) { @@ -222,8 +228,13 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper : $this->originalStoragePath . '/framework/sessions'; if (! is_dir($path)) { - // Create tenant framework/sessions directory if it does not exist - mkdir($path, 0750, true); + // Create tenant framework/sessions directory if it does not exist. + // We ignore errors due to TOCTOU race conditions, instead we check for success below. + @mkdir($path, 0750, true); + + if (! is_dir($path)) { + throw new Exception("Unable to create tenant session directory [{$path}]."); + } } $this->app['config']['session.files'] = $path; From c32f52ce7cb9e705cbf1f5a5e884e466c8dde319 Mon Sep 17 00:00:00 2001 From: Samuel Stancl Date: Wed, 15 Apr 2026 11:09:22 +0200 Subject: [PATCH 2/6] phpstan fix: Scope generics phpstan started failing with '... implements generic interface Illuminate\Database\Eloquent\Scope but does not specify its types: TModel'. We solve this by adding an implements docblock to the scopes implementing that interface. They're fairly generic - we just use the Model type itself in the code - so we use Model for the type parameter. --- src/Database/Concerns/PendingScope.php | 1 + src/Database/ParentModelScope.php | 1 + src/Database/TenantScope.php | 1 + 3 files changed, 3 insertions(+) diff --git a/src/Database/Concerns/PendingScope.php b/src/Database/Concerns/PendingScope.php index 712de6c7..52b8eb19 100644 --- a/src/Database/Concerns/PendingScope.php +++ b/src/Database/Concerns/PendingScope.php @@ -8,6 +8,7 @@ use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Scope; +/** @implements Scope */ class PendingScope implements Scope { /** diff --git a/src/Database/ParentModelScope.php b/src/Database/ParentModelScope.php index ee1a200e..44f4ac12 100644 --- a/src/Database/ParentModelScope.php +++ b/src/Database/ParentModelScope.php @@ -8,6 +8,7 @@ use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Scope; +/** @implements Scope */ class ParentModelScope implements Scope { /** diff --git a/src/Database/TenantScope.php b/src/Database/TenantScope.php index 050da365..94ff4572 100644 --- a/src/Database/TenantScope.php +++ b/src/Database/TenantScope.php @@ -9,6 +9,7 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Scope; use Stancl\Tenancy\Tenancy; +/** @implements Scope */ class TenantScope implements Scope { /** From ab2a4d84385b2fd857cf9f8fb9e950f208898d22 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Wed, 22 Apr 2026 14:32:53 +0200 Subject: [PATCH 3/6] Fix chaining `withoutPending()` with `where()` (#1457) At the moment, `where()` cannot be used correctly while using `withoutPending()`. For example, if we have a single non-pending tenant in our DB (with ID 'foo'), queries like `Tenant::withoutPending()->where('id', 'nonexistent')->first()`will incorrectly return the non-pending tenant ('foo'). This is because `withoutPending()` does `$builder->whereNull('data->pending_since')->orWhereNull('data')`. These two aren't grouped, so `withoutPending()->where('id', 'nonexistent')` basically translates to "WHERE data->pending_since IS NULL **OR (data IS NULL AND id = 'nonexistent')**". So the query will include all tenants whose `pending_since` is null (= all non-pending tenants). Grouping `->whereNull('data->pending_since')->orWhereNull('data')` in a closure passed to a separate `where()` fixes this issue. --- src/Database/Concerns/PendingScope.php | 6 ++++-- tests/PendingTenantsTest.php | 12 ++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/Database/Concerns/PendingScope.php b/src/Database/Concerns/PendingScope.php index 52b8eb19..99a5ef59 100644 --- a/src/Database/Concerns/PendingScope.php +++ b/src/Database/Concerns/PendingScope.php @@ -58,8 +58,10 @@ class PendingScope implements Scope { $builder->macro('withoutPending', function (Builder $builder) { $builder->withoutGlobalScope(static::class) - ->whereNull($builder->getModel()->getColumnForQuery('pending_since')) - ->orWhereNull($builder->getModel()->getDataColumn()); + ->where(function (Builder $query) { + $query->whereNull($query->getModel()->getColumnForQuery('pending_since')) + ->orWhereNull($query->getModel()->getDataColumn()); + }); return $builder; }); diff --git a/tests/PendingTenantsTest.php b/tests/PendingTenantsTest.php index a90aceed..433b85fb 100644 --- a/tests/PendingTenantsTest.php +++ b/tests/PendingTenantsTest.php @@ -111,6 +111,18 @@ test('a new tenant gets created while pulling a pending tenant if the pending po expect(Tenant::withPending()->get()->count())->toBe(1); // All tenants }); +test('withoutPending chained with where clauses returns correct results', function () { + $tenant = Tenant::create(); + $pendingTenant = Tenant::createPending(); + + // The query returned the correct tenant + expect(Tenant::withoutPending()->where('id', $tenant->id)->first()->id)->toBe($tenant->id); + // No tenant with this ID exists, the query returns null + expect(Tenant::withoutPending()->where('id', Str::random(8) . 'nonexistent-id')->first())->toBeNull(); + // withoutPending() correctly excludes the pending tenant from the query + expect(Tenant::withoutPending()->where('id', $pendingTenant->id)->first())->toBeNull(); +}); + test('pending tenants are included in all queries based on the include_in_queries config', function () { Tenant::createPending(); From 984911946a31351e1cb63bcb13d53b2390b5baf2 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Wed, 22 Apr 2026 16:45:54 +0200 Subject: [PATCH 4/6] Change tenant storage listeners into jobs (#1446) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `CreateTenantStorage` and `DeleteTenantStorage` listeners were used alongside JobPipelines. When the `TenantCreated` JobPipeline had `shouldBeQueued(true)` and the `Listeners\CreateTenantStorage` was uncommented, the listener would throw an exception (`Stancl\Tenancy\Database\Exceptions\TenantDatabaseDoesNotExistException Database tenantX.sqlite does not exist.`) because at the time of executing the listener, the tenant DB wasn't created yet. The same issue could likely also occur in the `DeleteTenantStorage` listener as it uses `tenancy()->run()` to resolve the tenant's storage path which wouldn't work if the tenant's database (or other resources) was already deleted, making initialization impossible. This PR changes `DeleteTenantStorage` into a job and puts it (commented) into the job pipeline, so that it can be queued with the rest of the jobs. It also removes `CreateTenantStorage` because it should be redundant with the FilesystemTenancyBootstrapper creating the same paths automatically when storage path is suffixed. The old classes are kept but deprecated for backwards compatibility. We've also added some edge case hardening to `DeleteTenantStorage` to make sure it never deletes the central storage path directory, which previously could in theory occur due to a misconfiguration if a user enabled this job/listener but disabled storage path suffixing. Co-authored-by: Samuel Štancl Co-authored-by: github-actions[bot] Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- assets/TenancyServiceProvider.stub.php | 5 +- src/Jobs/DeleteTenantStorage.php | 43 ++++++++++++++ src/Listeners/CreateTenantStorage.php | 6 +- src/Listeners/DeleteTenantStorage.php | 21 ++++++- .../FilesystemTenancyBootstrapperTest.php | 57 ++++++++++++++++--- 5 files changed, 112 insertions(+), 20 deletions(-) create mode 100644 src/Jobs/DeleteTenantStorage.php diff --git a/assets/TenancyServiceProvider.stub.php b/assets/TenancyServiceProvider.stub.php index 1cb358de..915e80c2 100644 --- a/assets/TenancyServiceProvider.stub.php +++ b/assets/TenancyServiceProvider.stub.php @@ -53,8 +53,6 @@ class TenancyServiceProvider extends ServiceProvider ])->send(function (Events\TenantCreated $event) { return $event->tenant; })->shouldBeQueued(false), - - // Listeners\CreateTenantStorage::class, ], Events\SavingTenant::class => [], Events\TenantSaved::class => [], @@ -63,12 +61,11 @@ class TenancyServiceProvider extends ServiceProvider Events\DeletingTenant::class => [ JobPipeline::make([ Jobs\DeleteDomains::class, + // Jobs\DeleteTenantStorage::class, // Jobs\RemoveStorageSymlinks::class, ])->send(function (Events\DeletingTenant $event) { return $event->tenant; })->shouldBeQueued(false), - - // Listeners\DeleteTenantStorage::class, ], Events\TenantDeleted::class => [ JobPipeline::make([ diff --git a/src/Jobs/DeleteTenantStorage.php b/src/Jobs/DeleteTenantStorage.php new file mode 100644 index 00000000..36a0d326 --- /dev/null +++ b/src/Jobs/DeleteTenantStorage.php @@ -0,0 +1,43 @@ +central(fn () => storage_path()); + $tenantStoragePath = tenancy()->run($this->tenant, fn () => storage_path()); + + if ($tenantStoragePath === $centralStoragePath) { + // Check again to ensure the tenant storage path is distinct from the central storage path + // to avoid any accidental central storage path deletion + return; + } + + if (is_dir($tenantStoragePath)) { + File::deleteDirectory($tenantStoragePath); + } + } +} diff --git a/src/Listeners/CreateTenantStorage.php b/src/Listeners/CreateTenantStorage.php index 3bebb731..0ffdef60 100644 --- a/src/Listeners/CreateTenantStorage.php +++ b/src/Listeners/CreateTenantStorage.php @@ -7,11 +7,7 @@ namespace Stancl\Tenancy\Listeners; use Stancl\Tenancy\Events\Contracts\TenantEvent; /** - * Can be used to manually create framework directories in the tenant storage when storage_path() is scoped. - * - * Useful when using real-time facades which use the framework/cache directory. - * - * Generally not needed anymore as the directory is also created by the FilesystemTenancyBootstrapper. + * @deprecated FilesystemTenancyBootstrapper creates the path automatically when suffix_storage_path is enabled. */ class CreateTenantStorage { diff --git a/src/Listeners/DeleteTenantStorage.php b/src/Listeners/DeleteTenantStorage.php index ec360073..06f20454 100644 --- a/src/Listeners/DeleteTenantStorage.php +++ b/src/Listeners/DeleteTenantStorage.php @@ -7,14 +7,29 @@ namespace Stancl\Tenancy\Listeners; use Illuminate\Support\Facades\File; use Stancl\Tenancy\Events\Contracts\TenantEvent; +/** + * @deprecated Use Stancl\Tenancy\Jobs\DeleteTenantStorage in a job pipeline instead. + */ class DeleteTenantStorage { public function handle(TenantEvent $event): void { - $path = tenancy()->run($event->tenant, fn () => storage_path()); + if (config('tenancy.filesystem.suffix_storage_path') === false) { + // Skip storage deletion if path suffixing is disabled + return; + } - if (is_dir($path)) { - File::deleteDirectory($path); + $centralStoragePath = tenancy()->central(fn () => storage_path()); + $tenantStoragePath = tenancy()->run($event->tenant, fn () => storage_path()); + + if ($tenantStoragePath === $centralStoragePath) { + // Check again to ensure the tenant storage path is distinct from the central storage path + // to avoid any accidental central storage path deletion + return; + } + + if (is_dir($tenantStoragePath)) { + File::deleteDirectory($tenantStoragePath); } } } diff --git a/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php b/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php index 628b974e..4e834917 100644 --- a/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php +++ b/tests/Bootstrappers/FilesystemTenancyBootstrapperTest.php @@ -13,7 +13,7 @@ use Stancl\Tenancy\Events\TenancyInitialized; use Stancl\Tenancy\Jobs\CreateStorageSymlinks; use Stancl\Tenancy\Jobs\RemoveStorageSymlinks; use Stancl\Tenancy\Listeners\BootstrapTenancy; -use Stancl\Tenancy\Listeners\DeleteTenantStorage; +use Stancl\Tenancy\Jobs\DeleteTenantStorage; use Stancl\Tenancy\Listeners\RevertToCentralContext; use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; use function Stancl\Tenancy\Tests\pest; @@ -184,21 +184,63 @@ test('create and delete storage symlinks jobs work', function() { $this->assertDirectoryDoesNotExist(public_path("public-$tenantKey")); }); -test('tenant storage can get deleted after the tenant when DeletingTenant listens to DeleteTenantStorage', function() { - Event::listen(DeletingTenant::class, DeleteTenantStorage::class); +test('tenant storage gets deleted during tenant deletion when the DeletingTenant pipeline contains DeleteTenantStorage', function() { + Event::listen(DeletingTenant::class, + JobPipeline::make([DeleteTenantStorage::class])->send(function (DeletingTenant $event) { + return $event->tenant; + })->shouldBeQueued(false)->toListener() + ); + + $centralStoragePath = storage_path(); + tenancy()->initialize(Tenant::create()); + + // FilesystemTenancyBootstrapper not enabled, + // tenant and central storage path is the same, + // the storage deletion will be skipped. + $tenantStoragePath = storage_path(); + expect($tenantStoragePath)->toBe($centralStoragePath); + expect(File::isDirectory($centralStoragePath))->toBeTrue(); + tenant()->delete(); + + expect(File::isDirectory($centralStoragePath))->toBeTrue(); + + config([ + 'tenancy.bootstrappers' => [FilesystemTenancyBootstrapper::class], + 'tenancy.filesystem.suffix_storage_path' => false, + ]); + + tenancy()->initialize(Tenant::create()); + + $tenantStoragePath = storage_path(); + + // FilesystemTenancyBootstrapper enabled, + // but tenant and central storage path is still the same + // because suffix_storage_path is false. + // The storage deletion will be skipped. + expect($tenantStoragePath)->toBe($centralStoragePath); + expect(File::isDirectory($centralStoragePath))->toBeTrue(); + tenant()->delete(); + + expect(File::isDirectory($centralStoragePath))->toBeTrue(); + + config([ + 'tenancy.bootstrappers' => [FilesystemTenancyBootstrapper::class], + 'tenancy.filesystem.suffix_storage_path' => true, + ]); tenancy()->initialize(Tenant::create()); $tenantStoragePath = storage_path(); - Storage::fake('test'); - + // FilesystemTenancyBootstrapper enabled, + // suffix_storage_path enabled, so the two paths are distinct. + // Tenant storage will be deleted. + expect($tenantStoragePath)->not()->toBe($centralStoragePath); expect(File::isDirectory($tenantStoragePath))->toBeTrue(); - Storage::put('test.txt', 'testing file'); - tenant()->delete(); expect(File::isDirectory($tenantStoragePath))->toBeFalse(); + expect(File::isDirectory($centralStoragePath))->toBeTrue(); }); test('the framework/cache directory is created when storage_path is scoped', function (bool $suffixStoragePath) { @@ -256,4 +298,3 @@ test('scoped disks are scoped per tenant', function () { expect(file_get_contents(storage_path() . "/app/public/scoped_disk_prefix/foo.txt"))->toBe('central2'); expect(file_get_contents(storage_path() . "/tenant{$tenant->id}/app/public/scoped_disk_prefix/foo.txt"))->toBe('tenant'); }); - From 53f44762cab15f5171e1196245cf644a64e1e8b5 Mon Sep 17 00:00:00 2001 From: Samuel Stancl Date: Fri, 1 May 2026 15:48:11 +0200 Subject: [PATCH 5/6] docker: change mssql env yaml syntax --- docker-compose.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 70a68019..6cc03e12 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -80,8 +80,8 @@ services: mssql: image: mcr.microsoft.com/mssql/server:2022-latest environment: - - ACCEPT_EULA=Y - - SA_PASSWORD=P@ssword # must be the same as TENANCY_TEST_SQLSRV_PASSWORD + ACCEPT_EULA: "Y" + SA_PASSWORD: "P@ssword" # must be the same as TENANCY_TEST_SQLSRV_PASSWORD healthcheck: # https://github.com/Microsoft/mssql-docker/issues/133#issuecomment-1995615432 test: timeout 2 bash -c 'cat < /dev/null > /dev/tcp/127.0.0.1/1433' interval: 10s From 41701aff5f1105c025d99f091b5d7fe850330d27 Mon Sep 17 00:00:00 2001 From: Samuel Stancl Date: Fri, 1 May 2026 16:07:18 +0200 Subject: [PATCH 6/6] phpstan fix: Model covariants in Scope generics Builds on changes in recent commit: Commit ID: c32f52ce7cb9e705cbf1f5a5e884e466c8dde319 Change ID: qsnosyvyulxzrnzorpxqwqqztmqorsmk --- src/Database/Concerns/PendingScope.php | 2 +- src/Database/ParentModelScope.php | 2 +- src/Database/TenantScope.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Database/Concerns/PendingScope.php b/src/Database/Concerns/PendingScope.php index 99a5ef59..e8805d8a 100644 --- a/src/Database/Concerns/PendingScope.php +++ b/src/Database/Concerns/PendingScope.php @@ -14,7 +14,7 @@ class PendingScope implements Scope /** * Apply the scope to a given Eloquent query builder. * - * @param Builder $builder + * @param Builder $builder * * @return void */ diff --git a/src/Database/ParentModelScope.php b/src/Database/ParentModelScope.php index 44f4ac12..9268fea9 100644 --- a/src/Database/ParentModelScope.php +++ b/src/Database/ParentModelScope.php @@ -12,7 +12,7 @@ use Illuminate\Database\Eloquent\Scope; class ParentModelScope implements Scope { /** - * @param Builder $builder + * @param Builder $builder */ public function apply(Builder $builder, Model $model): void { diff --git a/src/Database/TenantScope.php b/src/Database/TenantScope.php index 94ff4572..c6ce5f09 100644 --- a/src/Database/TenantScope.php +++ b/src/Database/TenantScope.php @@ -13,7 +13,7 @@ use Stancl\Tenancy\Tenancy; class TenantScope implements Scope { /** - * @param Builder $builder + * @param Builder $builder */ public function apply(Builder $builder, Model $model) {