From a21635ee734ee65e4cd9d53e63a357470c4a900e Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Sun, 24 May 2026 22:48:22 +0200 Subject: [PATCH] Harden nested Filament Livewire context contract --- ...upScheduleOperationRunsRelationManager.php | 36 +- ...agedEnvironmentTriageArrivalContinuity.php | 32 +- .../Livewire/BackupSetPolicyPickerTable.php | 184 ++++++---- .../Support/OperateHub/OperateHubShell.php | 48 +++ .../FilamentTenantContextContractTest.php | 41 +++ ...NestedFilamentContextContractSmokeTest.php | 140 +++++++ ...heduleOperationRunsRelationManagerTest.php | 46 +++ .../BackupSetPolicyPickerTableTest.php | 40 +- ...iageArrivalContinuityTenantContextTest.php | 47 +++ ...reRunResourceLivewireTenantContextTest.php | 90 +++++ .../checklists/requirements.md | 35 ++ .../plan.md | 150 ++++++++ .../spec.md | 343 ++++++++++++++++++ .../tasks.md | 92 +++++ 14 files changed, 1229 insertions(+), 95 deletions(-) create mode 100644 apps/platform/tests/Architecture/FilamentTenantContextContractTest.php create mode 100644 apps/platform/tests/Browser/Spec334NestedFilamentContextContractSmokeTest.php create mode 100644 apps/platform/tests/Feature/Filament/ManagedEnvironmentTriageArrivalContinuityTenantContextTest.php create mode 100644 apps/platform/tests/Feature/Filament/RestoreRunResourceLivewireTenantContextTest.php create mode 100644 specs/334-nested-filament-context-contract-hardening/checklists/requirements.md create mode 100644 specs/334-nested-filament-context-contract-hardening/plan.md create mode 100644 specs/334-nested-filament-context-contract-hardening/spec.md create mode 100644 specs/334-nested-filament-context-contract-hardening/tasks.md diff --git a/apps/platform/app/Filament/Resources/BackupScheduleResource/RelationManagers/BackupScheduleOperationRunsRelationManager.php b/apps/platform/app/Filament/Resources/BackupScheduleResource/RelationManagers/BackupScheduleOperationRunsRelationManager.php index 9b30da1d..e7c291c7 100644 --- a/apps/platform/app/Filament/Resources/BackupScheduleResource/RelationManagers/BackupScheduleOperationRunsRelationManager.php +++ b/apps/platform/app/Filament/Resources/BackupScheduleResource/RelationManagers/BackupScheduleOperationRunsRelationManager.php @@ -2,8 +2,8 @@ namespace App\Filament\Resources\BackupScheduleResource\RelationManagers; -use App\Models\OperationRun; use App\Models\ManagedEnvironment; +use App\Models\OperationRun; use App\Support\Badges\BadgeDomain; use App\Support\Badges\BadgeRenderer; use App\Support\OperationCatalog; @@ -16,7 +16,6 @@ use Filament\Resources\RelationManagers\RelationManager; use Filament\Tables; use Filament\Tables\Table; -use Illuminate\Database\Eloquent\Builder; class BackupScheduleOperationRunsRelationManager extends RelationManager { @@ -37,12 +36,11 @@ public static function actionSurfaceDeclaration(): ActionSurfaceDeclaration public function table(Table $table): Table { return $table - ->modifyQueryUsing(fn (Builder $query) => $query->where('managed_environment_id', ManagedEnvironment::currentOrFail()->getKey())) ->defaultSort('created_at', 'desc') ->paginated(\App\Support\Filament\TablePaginationProfiles::relationManager()) ->recordUrl(function (OperationRun $record): string { $record = $this->resolveOwnerScopedOperationRun($record); - $tenant = ManagedEnvironment::currentOrFail(); + $tenant = $this->resolveOwnerTenantOrFail(); return OperationRunLinks::view($record, $tenant); }) @@ -106,7 +104,6 @@ private function resolveOwnerScopedOperationRun(mixed $record): OperationRun $resolvedRecord = $this->getOwnerRecord() ->operationRuns() - ->where('managed_environment_id', ManagedEnvironment::currentOrFail()->getKey()) ->whereKey($recordId) ->first(); @@ -117,6 +114,35 @@ private function resolveOwnerScopedOperationRun(mixed $record): OperationRun return $resolvedRecord; } + private function resolveOwnerTenantOrFail(): ManagedEnvironment + { + $ownerRecord = $this->getOwnerRecord(); + + if ( + is_object($ownerRecord) + && method_exists($ownerRecord, 'getRelationValue') + && method_exists($ownerRecord, 'getAttribute') + ) { + $tenant = $ownerRecord->getRelationValue('tenant'); + + if ($tenant instanceof ManagedEnvironment) { + return $tenant; + } + + $tenantId = $ownerRecord->getAttribute('managed_environment_id'); + + if (is_numeric($tenantId)) { + $tenant = ManagedEnvironment::query()->whereKey((int) $tenantId)->first(); + + if ($tenant instanceof ManagedEnvironment) { + return $tenant; + } + } + } + + abort(404); + } + public static function formatOperationType(?string $state): string { return OperationCatalog::label($state); diff --git a/apps/platform/app/Filament/Widgets/ManagedEnvironment/ManagedEnvironmentTriageArrivalContinuity.php b/apps/platform/app/Filament/Widgets/ManagedEnvironment/ManagedEnvironmentTriageArrivalContinuity.php index 4398f9e9..8f1ed84c 100644 --- a/apps/platform/app/Filament/Widgets/ManagedEnvironment/ManagedEnvironmentTriageArrivalContinuity.php +++ b/apps/platform/app/Filament/Widgets/ManagedEnvironment/ManagedEnvironmentTriageArrivalContinuity.php @@ -12,10 +12,10 @@ use App\Support\BackupHealth\TenantBackupHealthResolver; use App\Support\Badges\BadgeDomain; use App\Support\Badges\BadgeRenderer; +use App\Support\PortfolioTriage\ManagedEnvironmentTriageReviewStateResolver; use App\Support\PortfolioTriage\PortfolioArrivalContext; use App\Support\PortfolioTriage\PortfolioArrivalContextResolver; use App\Support\PortfolioTriage\PortfolioArrivalContextToken; -use App\Support\PortfolioTriage\ManagedEnvironmentTriageReviewStateResolver; use App\Support\Rbac\UiEnforcement; use App\Support\RestoreSafety\RestoreSafetyResolver; use Filament\Actions\Action; @@ -26,12 +26,15 @@ use Filament\Schemas\Concerns\InteractsWithSchemas; use Filament\Schemas\Contracts\HasSchemas; use Filament\Widgets\Widget; +use Illuminate\Database\Eloquent\Model; class ManagedEnvironmentTriageArrivalContinuity extends Widget implements HasActions, HasSchemas { use InteractsWithActions; use InteractsWithSchemas; + public ?Model $record = null; + /** * @var array|null */ @@ -66,6 +69,14 @@ public function mount(): void $this->arrivalState = PortfolioArrivalContextToken::decode( request()->query(PortfolioArrivalContextToken::QUERY_PARAMETER), ); + + if ($this->record === null) { + $tenant = Filament::getTenant(); + + if ($tenant instanceof ManagedEnvironment) { + $this->record = $tenant; + } + } } /** @@ -73,7 +84,7 @@ public function mount(): void */ protected function getViewData(): array { - $tenant = Filament::getTenant(); + $tenant = $this->resolveManagedEnvironment(); if (! $tenant instanceof ManagedEnvironment) { return ['context' => null, 'reviewState' => null]; @@ -133,7 +144,7 @@ public function markFollowUpNeededAction(): Action private function canShowReviewActions(): bool { - $tenant = Filament::getTenant(); + $tenant = $this->resolveManagedEnvironment(); if (! $tenant instanceof ManagedEnvironment) { return false; @@ -151,7 +162,7 @@ private function canShowReviewActions(): bool private function reviewModalDescription(string $targetManualState): \Closure { return function () use ($targetManualState): string { - $tenant = Filament::getTenant(); + $tenant = $this->resolveManagedEnvironment(); if (! $tenant instanceof ManagedEnvironment) { return 'This triage session is no longer available.'; @@ -186,7 +197,7 @@ private function reviewModalDescription(string $targetManualState): \Closure private function handleReviewMutation(string $targetManualState, ManagedEnvironmentTriageReviewService $service): void { - $tenant = Filament::getTenant(); + $tenant = $this->resolveManagedEnvironment(); if (! $tenant instanceof ManagedEnvironment) { return; @@ -254,6 +265,17 @@ private function handleReviewMutation(string $targetManualState, ManagedEnvironm ->send(); } + private function resolveManagedEnvironment(): ?ManagedEnvironment + { + if ($this->record instanceof ManagedEnvironment) { + return $this->record; + } + + $tenant = Filament::getTenant(); + + return $tenant instanceof ManagedEnvironment ? $tenant : null; + } + /** * @return array|null */ diff --git a/apps/platform/app/Livewire/BackupSetPolicyPickerTable.php b/apps/platform/app/Livewire/BackupSetPolicyPickerTable.php index 927274e8..e6eec68f 100644 --- a/apps/platform/app/Livewire/BackupSetPolicyPickerTable.php +++ b/apps/platform/app/Livewire/BackupSetPolicyPickerTable.php @@ -4,9 +4,10 @@ use App\Jobs\AddPoliciesToBackupSetJob; use App\Models\BackupSet; -use App\Models\Policy; use App\Models\ManagedEnvironment; +use App\Models\Policy; use App\Models\User; +use App\Services\Auth\CapabilityResolver; use App\Services\OperationRunService; use App\Services\Operations\BulkSelectionIdentity; use App\Support\Auth\Capabilities; @@ -18,6 +19,7 @@ use App\Support\OpsUx\OperationUxPresenter; use App\Support\OpsUx\OpsUxBrowserEvents; use Filament\Actions\BulkAction; +use Filament\Facades\Filament; use Filament\Notifications\Notification; use Filament\Tables\Columns\TextColumn; use Filament\Tables\Filters\SelectFilter; @@ -26,7 +28,6 @@ use Illuminate\Contracts\View\View; use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Collection; -use Illuminate\Support\Facades\Gate; use Illuminate\Support\Str; class BackupSetPolicyPickerTable extends TableComponent @@ -59,17 +60,27 @@ public static function externalIdShort(?string $externalId): string public function table(Table $table): Table { - $backupSet = BackupSet::query()->find($this->backupSetId); - $tenantId = $backupSet?->managed_environment_id ?? ManagedEnvironment::currentOrFail()->getKey(); - $existingPolicyIds = $backupSet - ? $backupSet->items()->pluck('policy_id')->filter()->all() - : []; + $context = $this->resolveBackupSetContext(); + $existingPolicyIds = []; + $tenantId = $context !== null ? (int) $context['tenant']->getKey() : null; + + if ($context !== null) { + $existingPolicyIds = $context['backupSet'] + ->items() + ->pluck('policy_id') + ->filter() + ->all(); + } return $table ->queryStringIdentifier('backupSetPolicyPicker'.Str::studly((string) $this->backupSetId)) ->query( Policy::query() - ->where('managed_environment_id', $tenantId) + ->when( + $context !== null, + fn (Builder $query) => $query->where('managed_environment_id', (int) $context['tenant']->getKey()), + fn (Builder $query) => $query->whereRaw('1 = 0'), + ) ->when($existingPolicyIds !== [], fn (Builder $query) => $query->whereNotIn('id', $existingPolicyIds)) ) ->deferLoading(! app()->runningUnitTests()) @@ -141,13 +152,19 @@ public function table(Table $table): Table ->options(static::policyTypeOptions()), SelectFilter::make('platform') ->label('Platform') - ->options(fn (): array => Policy::query() - ->where('managed_environment_id', $tenantId) - ->whereNotNull('platform') - ->distinct() - ->orderBy('platform') - ->pluck('platform', 'platform') - ->all()), + ->options(function () use ($tenantId): array { + if (! is_int($tenantId)) { + return []; + } + + return Policy::query() + ->where('managed_environment_id', $tenantId) + ->whereNotNull('platform') + ->distinct() + ->orderBy('platform') + ->pluck('platform', 'platform') + ->all(); + }), SelectFilter::make('synced_within') ->label('Last synced') ->options([ @@ -219,77 +236,20 @@ public function table(Table $table): Table ->icon('heroicon-m-plus') ->authorize(function (): bool { $this->dispatch(OpsUxBrowserEvents::RunEnqueued); - $user = auth()->user(); + $context = $this->resolveBackupSetContext(requireSyncCapability: true); - if (! $user instanceof User) { - return false; - } - - try { - $tenant = ManagedEnvironment::current(); - } catch (\RuntimeException) { - return false; - } - - if (! $tenant instanceof ManagedEnvironment) { - return false; - } - - if (! Gate::forUser($user)->allows(Capabilities::TENANT_SYNC, $tenant)) { - return false; - } - - return BackupSet::query() - ->whereKey($this->backupSetId) - ->where('managed_environment_id', $tenant->getKey()) - ->exists(); + return $context !== null; }) ->action(function (Collection $records): void { - $backupSet = BackupSet::query()->findOrFail($this->backupSetId); - $tenant = null; + $context = $this->resolveBackupSetContext(requireSyncCapability: true); - try { - $tenant = ManagedEnvironment::current(); - } catch (\RuntimeException) { - $tenant = $backupSet->tenant; - } - $user = auth()->user(); - - if (! $user instanceof User) { - Notification::make() - ->title('Not allowed') - ->danger() - ->send(); - - return; + if ($context === null) { + abort(403); } - if (! $tenant instanceof ManagedEnvironment) { - Notification::make() - ->title('Not allowed') - ->danger() - ->send(); - - return; - } - - if ((int) $tenant->getKey() !== (int) $backupSet->managed_environment_id) { - Notification::make() - ->title('Not allowed') - ->danger() - ->send(); - - return; - } - - if (! Gate::forUser($user)->allows(Capabilities::TENANT_SYNC, $tenant)) { - Notification::make() - ->title('Not allowed') - ->danger() - ->send(); - - return; - } + $user = $context['user']; + $tenant = $context['tenant']; + $backupSet = $context['backupSet']; $policyIds = $records ->pluck('id') @@ -324,6 +284,21 @@ public function table(Table $table): Table return; } + $validatedPolicyIds = Policy::query() + ->where('managed_environment_id', (int) $tenant->getKey()) + ->whereIn('id', $policyIds) + ->pluck('id') + ->map(fn (mixed $value): int => (int) $value) + ->unique() + ->values() + ->all(); + + sort($validatedPolicyIds); + + if ($validatedPolicyIds !== $policyIds) { + abort(403); + } + /** @var BulkSelectionIdentity $selection */ $selection = app(BulkSelectionIdentity::class); $selectionIdentity = $selection->fromIds($policyIds); @@ -433,4 +408,53 @@ private static function policyTypeOptions(): array }) ->all(); } + + /** + * @return array{user: User, tenant: ManagedEnvironment, backupSet: BackupSet}|null + */ + private function resolveBackupSetContext(bool $requireSyncCapability = false): ?array + { + $user = auth()->user(); + + if (! $user instanceof User) { + return null; + } + + $backupSet = BackupSet::query() + ->with(['tenant']) + ->find($this->backupSetId); + + if (! $backupSet instanceof BackupSet) { + return null; + } + + $tenant = $backupSet->tenant; + + if (! $tenant instanceof ManagedEnvironment) { + return null; + } + + $ambientTenant = Filament::getTenant(); + + if ($ambientTenant instanceof ManagedEnvironment && ! $ambientTenant->is($tenant)) { + return null; + } + + /** @var CapabilityResolver $resolver */ + $resolver = app(CapabilityResolver::class); + + if (! $resolver->isMember($user, $tenant)) { + return null; + } + + if ($requireSyncCapability && ! $resolver->can($user, $tenant, Capabilities::TENANT_SYNC)) { + return null; + } + + return [ + 'user' => $user, + 'tenant' => $tenant, + 'backupSet' => $backupSet, + ]; + } } diff --git a/apps/platform/app/Support/OperateHub/OperateHubShell.php b/apps/platform/app/Support/OperateHub/OperateHubShell.php index 680a9d6f..4035e1c4 100644 --- a/apps/platform/app/Support/OperateHub/OperateHubShell.php +++ b/apps/platform/app/Support/OperateHub/OperateHubShell.php @@ -363,9 +363,57 @@ private function resolveRouteTenantCandidate(?Request $request = null, ?AdminSur return $this->resolveTenantIdentifier($route->parameter('tenant')); } + $refererEnvironment = $this->resolveRefererTenantCandidate($request, $pageCategory); + + if ($refererEnvironment instanceof ManagedEnvironment) { + return $refererEnvironment; + } + return null; } + private function resolveRefererTenantCandidate(?Request $request, AdminSurfaceScope $pageCategory): ?ManagedEnvironment + { + if (! $request instanceof Request) { + return null; + } + + $path = '/'.ltrim((string) $request->path(), '/'); + + $isLivewireUpdateRequest = preg_match('#^/(?:livewire(?:-[^/]+)?/update|livewire-unit-test-endpoint)(?:/|$)#', $path) === 1 + || $request->headers->has('x-livewire'); + + if (! $isLivewireUpdateRequest) { + return null; + } + + if ($pageCategory !== AdminSurfaceScope::EnvironmentBound) { + return null; + } + + $refererPath = parse_url((string) $request->headers->get('referer', ''), PHP_URL_PATH); + + if (! is_string($refererPath) || $refererPath === '') { + return null; + } + + $normalizedRefererPath = '/'.ltrim($refererPath, '/'); + + $matches = []; + + if (preg_match('#^/admin/workspaces/[^/]+/environments/([^/]+)(?:/|$)#', $normalizedRefererPath, $matches) !== 1) { + return null; + } + + $environmentIdentifier = $matches[1] ?? null; + + if (! is_string($environmentIdentifier) || $environmentIdentifier === '') { + return null; + } + + return $this->resolveTenantIdentifier($environmentIdentifier); + } + private function resolveQueryTenantHint(?Request $request = null): ?ManagedEnvironment { if ($request instanceof Request && WorkspaceHubRegistry::isWorkspaceHubPath('/'.ltrim((string) $request->path(), '/'))) { diff --git a/apps/platform/tests/Architecture/FilamentTenantContextContractTest.php b/apps/platform/tests/Architecture/FilamentTenantContextContractTest.php new file mode 100644 index 00000000..db888cbc --- /dev/null +++ b/apps/platform/tests/Architecture/FilamentTenantContextContractTest.php @@ -0,0 +1,41 @@ +getRealPath(); + + if (! is_string($realPath)) { + continue; + } + + $contents = File::get($realPath); + + if (! str_contains($contents, 'Filament::setTenant(')) { + continue; + } + + if (! in_array($realPath, $allowlist, true)) { + $violations[] = str_replace(base_path().DIRECTORY_SEPARATOR, '', $realPath); + } + } + + if ($violations !== []) { + test()->fail("Unexpected Filament::setTenant usage detected:\n- ".implode("\n- ", $violations)); + } + + expect($violations)->toBeEmpty(); +}); diff --git a/apps/platform/tests/Browser/Spec334NestedFilamentContextContractSmokeTest.php b/apps/platform/tests/Browser/Spec334NestedFilamentContextContractSmokeTest.php new file mode 100644 index 00000000..c599a8ff --- /dev/null +++ b/apps/platform/tests/Browser/Spec334NestedFilamentContextContractSmokeTest.php @@ -0,0 +1,140 @@ +browser()->timeout(20_000); + +function spec334SmokeLoginUrl(User $user, ManagedEnvironment $tenant, string $redirect = ''): string +{ + $tenant->loadMissing('workspace'); + + $redirect = trim($redirect); + + if ($redirect !== '') { + $parsed = parse_url($redirect); + + if (is_array($parsed) && ! isset($parsed['scheme'], $parsed['host'])) { + $redirect = '/'.ltrim((string) ($parsed['path'] ?? ''), '/'); + $query = isset($parsed['query']) ? '?'.$parsed['query'] : ''; + $fragment = isset($parsed['fragment']) ? '#'.$parsed['fragment'] : ''; + $redirect = $redirect.$query.$fragment; + } elseif (is_array($parsed)) { + $redirect = '/'.ltrim((string) ($parsed['path'] ?? ''), '/'); + $query = isset($parsed['query']) ? '?'.$parsed['query'] : ''; + $fragment = isset($parsed['fragment']) ? '#'.$parsed['fragment'] : ''; + $redirect = $redirect.$query.$fragment; + } + } + + return route('admin.local.smoke-login', array_filter([ + 'email' => (string) $user->email, + 'tenant' => (string) $tenant->external_id, + 'workspace' => (string) ($tenant->workspace?->slug ?? ''), + 'redirect' => $redirect, + ], static fn (?string $value): bool => filled($value))); +} + +it('smokes the backup set add policies picker without selection-column regressions', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + + config()->set('queue.default', 'database'); + + $backupSet = BackupSet::factory()->create([ + 'managed_environment_id' => (int) $tenant->getKey(), + 'name' => 'Spec334 Browser Backup Set', + ]); + + Policy::factory()->create([ + 'managed_environment_id' => (int) $tenant->getKey(), + 'display_name' => 'Spec334 Browser Policy A', + 'ignored_at' => null, + 'missing_from_provider_at' => null, + 'last_synced_at' => now(), + ]); + + Policy::factory()->create([ + 'managed_environment_id' => (int) $tenant->getKey(), + 'display_name' => 'Spec334 Browser Policy B', + 'ignored_at' => null, + 'missing_from_provider_at' => null, + 'last_synced_at' => now(), + ]); + + $backupSetUrl = BackupSetResource::getUrl('view', ['record' => $backupSet], panel: 'admin', tenant: $tenant); + + $page = visit(spec334SmokeLoginUrl($user, $tenant, $backupSetUrl)) + ->waitForText('Spec334 Browser Backup Set') + ->assertNoJavaScriptErrors() + ->assertNoConsoleLogs() + ->assertSee('Related context'); + + $page->script('window.scrollTo(0, document.body.scrollHeight);'); + + $page + ->waitForText('Add Policies') + ->click('Add Policies') + ->waitForText('Spec334 Browser Policy A') + ->assertNoJavaScriptErrors() + ->assertNoConsoleLogs(); + + expect($page->script(<<<'JS' +(() => { + const visibleTables = Array.from(document.querySelectorAll('.fi-ta')).filter((table) => table && table.offsetParent !== null); + const table = visibleTables.at(-1); + + if (!table) { + return { selectable: 0, total: 0 }; + } + + const rows = Array.from(table.querySelectorAll('tbody tr.fi-ta-row')); + const selectable = rows.filter((row) => row.querySelector('input[type="checkbox"]:not([disabled])')).length; + + return { selectable, total: rows.length }; +})(); +JS))->toMatchArray([ + 'total' => 2, + 'selectable' => 2, + ]); + + $page + ->click('.fi-ta:visible tbody tr.fi-ta-row:has-text("Spec334 Browser Policy A") input[type="checkbox"]') + ->waitForText('Add selected') + ->click('Add selected') + ->waitForText('Backup set update queued') + ->assertNoJavaScriptErrors() + ->assertNoConsoleLogs(); +}); + +it('smokes the restore run create wizard without tenantless Livewire update crashes', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + + $backupSet = BackupSet::factory()->create([ + 'managed_environment_id' => (int) $tenant->getKey(), + 'name' => 'Spec334 Restore Fixture Backup Set', + ]); + + $createUrl = RestoreRunResource::getUrl('create', panel: 'admin', tenant: $tenant) + .'?backup_set_id='.(int) $backupSet->getKey(); + + visit(spec334SmokeLoginUrl($user, $tenant, $createUrl)) + ->waitForText('Select Backup Set') + ->waitForText('Backup set') + ->assertDontSee('No tenant context selected') + ->assertNoJavaScriptErrors() + ->assertNoConsoleLogs() + ->click('button:has-text("Next")') + ->waitForText('Define Restore Scope') + ->assertDontSee('No tenant context selected') + ->assertNoJavaScriptErrors() + ->assertNoConsoleLogs(); +}); diff --git a/apps/platform/tests/Feature/BackupScheduling/BackupScheduleOperationRunsRelationManagerTest.php b/apps/platform/tests/Feature/BackupScheduling/BackupScheduleOperationRunsRelationManagerTest.php index def5124a..51e5c187 100644 --- a/apps/platform/tests/Feature/BackupScheduling/BackupScheduleOperationRunsRelationManagerTest.php +++ b/apps/platform/tests/Feature/BackupScheduling/BackupScheduleOperationRunsRelationManagerTest.php @@ -56,6 +56,52 @@ function makeBackupScheduleForTenant(\App\Models\ManagedEnvironment $tenant, str ->assertCanNotSeeTableRecords([$runB]); }); +it('renders operation runs when ambient Filament tenant context is missing', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + + $this->actingAs($user); + setAdminPanelContext(null); + + $schedule = makeBackupScheduleForTenant($tenant, 'Schedule A'); + + $run = OperationRun::factory()->forTenant($tenant)->create([ + 'type' => 'backup_schedule_run', + 'context' => ['backup_schedule_id' => (int) $schedule->getKey()], + ]); + + Livewire::test(BackupScheduleOperationRunsRelationManager::class, [ + 'ownerRecord' => $schedule, + 'pageClass' => EditBackupSchedule::class, + ]) + ->assertCanSeeTableRecords([$run]); +}); + +it('does not broaden scope when ambient Filament tenant context is wrong', function (): void { + $tenantA = \App\Models\ManagedEnvironment::factory()->create(); + [$user, $tenantA] = createUserWithTenant(tenant: $tenantA, role: 'owner'); + + $tenantB = \App\Models\ManagedEnvironment::factory()->create([ + 'workspace_id' => (int) $tenantA->workspace_id, + ]); + createUserWithTenant(tenant: $tenantB, user: $user, role: 'owner'); + + $this->actingAs($user); + setAdminPanelContext($tenantB); + + $scheduleA = makeBackupScheduleForTenant($tenantA, 'Schedule A'); + + $runA = OperationRun::factory()->forTenant($tenantA)->create([ + 'type' => 'backup_schedule_run', + 'context' => ['backup_schedule_id' => (int) $scheduleA->getKey()], + ]); + + Livewire::test(BackupScheduleOperationRunsRelationManager::class, [ + 'ownerRecord' => $scheduleA, + 'pageClass' => EditBackupSchedule::class, + ]) + ->assertCanSeeTableRecords([$runA]); +}); + it('returns 404 when a forged same-tenant run key is mounted on the wrong schedule relation manager', function (): void { [$user, $tenant] = createUserWithTenant(role: 'owner'); diff --git a/apps/platform/tests/Feature/Filament/BackupSetPolicyPickerTableTest.php b/apps/platform/tests/Feature/Filament/BackupSetPolicyPickerTableTest.php index 7643e19d..40ba0721 100644 --- a/apps/platform/tests/Feature/Filament/BackupSetPolicyPickerTableTest.php +++ b/apps/platform/tests/Feature/Filament/BackupSetPolicyPickerTableTest.php @@ -3,10 +3,10 @@ use App\Jobs\AddPoliciesToBackupSetJob; use App\Livewire\BackupSetPolicyPickerTable; use App\Models\BackupSet; +use App\Models\ManagedEnvironment; use App\Models\OperationRun; use App\Models\Policy; use App\Models\PolicyVersion; -use App\Models\ManagedEnvironment; use App\Models\User; use App\Services\Intune\BackupService; use App\Support\OpsUx\OperationUxPresenter; @@ -81,6 +81,39 @@ expect(collect($notifications)->last()['title'] ?? null)->toBe('Backup set update queued'); }); +test('policy picker remains usable when Filament tenant context is missing (authorized operator)', function () { + Queue::fake(); + + [$user, $tenant] = createUserWithTenant(role: 'owner'); + $this->actingAs($user); + + setAdminPanelContext(null); + + $backupSet = BackupSet::factory()->create([ + 'managed_environment_id' => $tenant->id, + 'name' => 'Tenantless context backup', + ]); + + $policies = Policy::factory()->count(1)->create([ + 'managed_environment_id' => $tenant->id, + 'ignored_at' => null, + 'last_synced_at' => now(), + ]); + + bindFailHardGraphClient(); + + Livewire::actingAs($user) + ->test(BackupSetPolicyPickerTable::class, [ + 'backupSetId' => $backupSet->id, + ]) + ->assertTableBulkActionVisible('add_selected_to_backup_set') + ->assertCanSeeTableRecords($policies) + ->callTableBulkAction('add_selected_to_backup_set', $policies) + ->assertHasNoTableBulkActionErrors(); + + Queue::assertPushed(AddPoliciesToBackupSetJob::class, 1); +}); + test('policy picker keeps provider-missing policies visible but blocks add run creation', function () { Queue::fake(); @@ -273,10 +306,7 @@ }); test('policy picker table can filter by has versions', function () { - $tenant = ManagedEnvironment::factory()->create(); - $tenant->makeCurrent(); - - $user = User::factory()->create(); + [$user, $tenant] = createUserWithTenant(role: 'owner'); $backupSet = BackupSet::factory()->create([ 'managed_environment_id' => $tenant->id, diff --git a/apps/platform/tests/Feature/Filament/ManagedEnvironmentTriageArrivalContinuityTenantContextTest.php b/apps/platform/tests/Feature/Filament/ManagedEnvironmentTriageArrivalContinuityTenantContextTest.php new file mode 100644 index 00000000..507e74db --- /dev/null +++ b/apps/platform/tests/Feature/Filament/ManagedEnvironmentTriageArrivalContinuityTenantContextTest.php @@ -0,0 +1,47 @@ +makePortfolioTriageActor('Record context tenant', workspaceRole: 'owner'); + + $this->seedPortfolioBackupConcern($tenant, TenantBackupHealthAssessment::POSTURE_STALE); + + $this->actingAs($user); + session()->put(WorkspaceContext::SESSION_KEY, (int) $tenant->workspace_id); + + setAdminPanelContext(null); + request()->attributes->remove('portfolio_triage.arrival_context'); + + $arrivalState = [ + 'sourceSurface' => PortfolioArrivalContextToken::SOURCE_TENANT_REGISTRY, + 'tenantRouteKey' => (string) $tenant->external_id, + 'workspaceId' => (int) $tenant->workspace_id, + 'concernFamily' => PortfolioArrivalContextToken::FAMILY_BACKUP_HEALTH, + 'concernState' => TenantBackupHealthAssessment::POSTURE_STALE, + 'concernReason' => TenantBackupHealthAssessment::REASON_LATEST_BACKUP_STALE, + ]; + + $component = Livewire::withQueryParams([ + PortfolioArrivalContextToken::QUERY_PARAMETER => PortfolioArrivalContextToken::encode($arrivalState), + ])->actingAs($user)->test(ManagedEnvironmentTriageArrivalContinuity::class, [ + 'record' => $tenant, + ]); + + $instance = $component->instance(); + + $method = new ReflectionMethod($instance, 'getViewData'); + $method->setAccessible(true); + $viewData = $method->invoke($instance); + + expect($viewData['context'] ?? null)->not->toBeNull(); +}); diff --git a/apps/platform/tests/Feature/Filament/RestoreRunResourceLivewireTenantContextTest.php b/apps/platform/tests/Feature/Filament/RestoreRunResourceLivewireTenantContextTest.php new file mode 100644 index 00000000..0274868d --- /dev/null +++ b/apps/platform/tests/Feature/Filament/RestoreRunResourceLivewireTenantContextTest.php @@ -0,0 +1,90 @@ +actingAs($user); + + BackupSet::factory()->create([ + 'managed_environment_id' => (int) $tenant->getKey(), + 'name' => 'Restore fixture backup set', + ]); + + $createUrl = RestoreRunResource::getUrl('create', panel: 'admin', tenant: $tenant); + + $response = $this + ->withSession([WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id]) + ->get($createUrl); + + $response->assertSuccessful(); + + $html = (string) $response->getContent(); + + preg_match_all('/wire:snapshot="([^"]+)"/', $html, $snapshotMatches); + + $snapshots = $snapshotMatches[1] ?? []; + + expect($snapshots)->not->toBeEmpty('No Livewire snapshot found in restore create HTML'); + + $expectedPath = sprintf( + 'admin/workspaces/%s/environments/%s/restore-runs/create', + $tenant->workspace_id, + $tenant->getRouteKey(), + ); + + $snapshotJson = null; + + foreach ($snapshots as $encodedSnapshot) { + $candidateJson = htmlspecialchars_decode($encodedSnapshot); + $candidate = json_decode($candidateJson, true); + + if (! is_array($candidate)) { + continue; + } + + if (($candidate['memo']['path'] ?? null) === $expectedPath) { + $snapshotJson = $candidateJson; + break; + } + } + + expect($snapshotJson)->toBeString("No snapshot with memo.path [$expectedPath] found."); + + $updatePayload = [ + 'components' => [[ + 'snapshot' => $snapshotJson, + 'updates' => new stdClass, + 'calls' => [], + ]], + ]; + + $updateUri = '/'.collect(app('router')->getRoutes()->getRoutes()) + ->first(fn ($route): bool => str_contains((string) $route->getName(), 'livewire.update')) + ?->uri(); + + expect($updateUri)->toBeString('Livewire update route must exist'); + + $updateResponse = $this + ->withSession([WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id]) + ->withHeaders([ + 'X-Livewire' => 'true', + 'Referer' => $createUrl, + ]) + ->postJson($updateUri, $updatePayload); + + $updateResponse->assertSuccessful(); +}); diff --git a/specs/334-nested-filament-context-contract-hardening/checklists/requirements.md b/specs/334-nested-filament-context-contract-hardening/checklists/requirements.md new file mode 100644 index 00000000..04f756fd --- /dev/null +++ b/specs/334-nested-filament-context-contract-hardening/checklists/requirements.md @@ -0,0 +1,35 @@ +# Spec 334 Requirements Checklist + +- Purpose: Preparation-quality validation for Spec 334 before runtime implementation. +- Created: 2026-05-24 +- Feature: [spec.md](../spec.md) + +## Content Quality + +- [x] Spec is focused on a concrete operator-visible defect class (nested context loss) and does not drift into a tenancy rewrite. +- [x] Spec Candidate Check is filled and justifies why this is Core Enterprise work (correctness + blocker removal). +- [x] Non-goals explicitly exclude routing/panel/RBAC rewrites and broad repo sweeps. +- [x] Acceptance criteria are testable and bounded to confirmed surfaces. +- [x] Preparation artifacts contain no runtime implementation code changes. + +## Repo Truth + +- [x] Confirmed scope file paths were verified as existing: + - `apps/platform/app/Livewire/BackupSetPolicyPickerTable.php` + - `apps/platform/app/Filament/Resources/RestoreRunResource.php` + - `apps/platform/app/Filament/Concerns/ResolvesPanelTenantContext.php` + - `apps/platform/app/Filament/Resources/BackupScheduleResource/RelationManagers/BackupScheduleOperationRunsRelationManager.php` + - `apps/platform/app/Filament/Widgets/ManagedEnvironment/ManagedEnvironmentTriageArrivalContinuity.php` +- [x] Repo-truth deviation is documented: user draft references Spec 332 WIP, but `platform-dev` has no `specs/332-*` directory today. + +## Security / Isolation / RBAC + +- [x] Contract explicitly treats UI visibility as non-authorization and requires mutation-time rechecks. +- [x] Fail-closed posture is explicit: no broadening, no silent tenant override, no cross-scope selection/attachment. +- [x] Unsafe model-derived `Filament::setTenant(...)` switching is explicitly guarded. + +## Test And Smoke Readiness + +- [x] Plan includes the intended lane mix (Feature/Livewire + architecture guard; browser smoke only where user-visible bugs exist). +- [x] Tasks include a guardrail-first step and explicit validation commands. +- [x] Browser smoke scenarios are explicitly listed for the two originally user-visible bugs. diff --git a/specs/334-nested-filament-context-contract-hardening/plan.md b/specs/334-nested-filament-context-contract-hardening/plan.md new file mode 100644 index 00000000..9899630a --- /dev/null +++ b/specs/334-nested-filament-context-contract-hardening/plan.md @@ -0,0 +1,150 @@ +# Implementation Plan: Spec 334 - Nested Filament / Livewire Context Contract Hardening + +- Branch: `334-nested-filament-context-contract-hardening` +- Date: 2026-05-24 +- Spec: `specs/334-nested-filament-context-contract-hardening/spec.md` +- Input: User-provided Spec 334 draft + repo inspection for path truth. + +## Summary + +Define and enforce a nested Filament/Livewire context contract so operator-critical nested surfaces do not depend on ambient `Filament::getTenant()` during modal, wizard, relation manager, widget, and Livewire update lifecycles. + +Implement a narrow hardening slice across four confirmed surfaces, plus guard tests: + +- `apps/platform/app/Livewire/BackupSetPolicyPickerTable.php` +- `apps/platform/app/Filament/Resources/RestoreRunResource.php` (+ `apps/platform/app/Filament/Concerns/ResolvesPanelTenantContext.php` seam) +- `apps/platform/app/Filament/Resources/BackupScheduleResource/RelationManagers/BackupScheduleOperationRunsRelationManager.php` +- `apps/platform/app/Filament/Widgets/ManagedEnvironment/ManagedEnvironmentTriageArrivalContinuity.php` +- new architecture guard: `apps/platform/tests/Architecture/FilamentTenantContextContractTest.php` + +## Technical Context + +- Language/Version: PHP 8.4.15, Laravel 12.52.x. +- Primary Dependencies: Filament 5.2.x, Livewire 4.1.x, Pest 4.x, Tailwind CSS 4.x. +- Storage: PostgreSQL; no schema change expected. +- Testing: Pest Feature + Livewire component tests + one architecture guard; browser smoke only for the two user-visible regression scenarios. +- Validation Lanes: confidence + browser (scoped). +- Target Platform: Laravel Sail locally; Dokploy/container deployment posture unchanged. +- Project Type: Laravel monolith under `apps/platform`. +- Performance Goals: DB-only context recovery; no Graph calls during UI render; no broad unscoped queries in nested surfaces. +- Constraints: No new persisted truth, migrations, packages, env vars, queue/scheduler changes, or routing architecture changes. +**Scale/Scope**: Four confirmed nested surfaces + shared seam hardening + guard tests. + +## UI / Surface Guardrail Plan + +- **Guardrail scope**: changed existing operator-facing nested surfaces (modal, wizard, relation manager, widget). +- **Affected routes/pages/actions/states/navigation/panel/provider surfaces**: + - Backup Set “Add policies” modal table (Livewire component). + - Restore Run Create wizard (resource create page). + - Backup Schedule Operation Runs relation manager table (nested resource detail). + - Environment dashboard triage widget (nested widget). +- **No-impact class**: N/A. +- **Native vs custom classification summary**: native Filament + Livewire surfaces; no custom UI framework. +- **Shared-family relevance**: environment context resolution, authorization/visibility gating, table selection semantics, wizard option closures, remembered context validation. +- **State layers in scope**: route-owned workspace/environment, Livewire component state (captured at mount), remembered environment context (validated only), ambient Filament tenant (fallback convenience only). +- **Audience modes in scope**: operator-MSP / platform operators. No customer/read-only surface changes expected. +- **Decision/diagnostic/raw hierarchy plan**: fail-closed states must be honest; do not show “no records” when context is invalid; no raw debug “framework state” troubleshooting exposed to operators. +- **Handling modes**: review-mandatory. This is a correctness hardening slice affecting authorization and scope. +- **Special surface test profiles**: `exception-coded-surface` (context recovery) + `shared-detail-family` (nested relation manager) + browser smoke for two user-visible regressions. +- **Required tests or manual smoke**: + - Feature/Livewire tests for each confirmed surface and its fail-closed behavior. + - Architecture guard preventing unsafe `Filament::setTenant(...)` usage in nested surfaces. + - Browser smoke for: + - Backup Set “Add policies” checkbox visibility + selection. + - Restore Run Create wizard no-crash on Livewire update transitions. +- **Exception path and spread control**: no broad fallback that sets tenant from arbitrary model IDs. Any referer-based recovery must validate workspace membership and environment entitlement. +- **UI/Productization coverage decision**: no new routes/pages; cover via tests + browser smoke + PR close-out note. + +## Shared Pattern & System Fit + +- **Cross-cutting feature marker**: yes (multiple nested surfaces). +- **Systems touched**: + - Filament tenant resolution seam: `apps/platform/app/Filament/Concerns/ResolvesPanelTenantContext.php` + - Route/shell context: `apps/platform/app/Support/OperateHub/OperateHubShell.php` + - Workspace/environment context + remembered context: `apps/platform/app/Support/Workspaces/WorkspaceContext.php` + - UI gating: `apps/platform/app/Support/Rbac/UiEnforcement.php` +- **Shared abstractions reused**: prefer existing helpers/policies over new framework layers. +- **New abstraction introduced?**: none by default. A small helper class may be introduced only if at least two confirmed surfaces would otherwise duplicate validated context recovery logic (ABSTR-001). +- **Bounded deviation / spread control**: any new helper must be feature-local in scope and must not become a generic “ambient context fixer” without explicit follow-up spec approval. + +## OperationRun UX Impact + +N/A. This feature does not introduce new queued operations. It must not create new OperationRun types or change OperationRun UX policy. + +## Provider Boundary / Platform Core Check + +Platform-core hardening only. No Graph contract, provider registry, or provider vocabulary changes. + +## Implementation Approach + +### Phase 1 — Repo truth + reproduce + +- Inspect current behavior and confirm the concrete failure path(s) for: + - Add Policies modal selection checkbox hidden. + - Restore Run Create wizard crash during Livewire update. +- Document the exact call path(s) and which closures/methods access context. + +### Phase 2 — Define nested context resolution order + +Implement (or codify within existing seams) a deterministic resolution order for managed-environment context in nested surfaces: + +1. **Owner/domain record** (owner record, component-bound record, backup set record, schedule record, widget record). +2. **Validated component state captured at mount** (e.g., environment ID saved in the Livewire component state). +3. **Route-owned workspace/environment** (when route params are available). +4. **Remembered environment** only if workspace membership and entitlement validate. +5. **Ambient Filament tenant** as fallback convenience only. +6. **Fail closed** with a clear, honest UI state. + +Prohibited: + +- blindly trusting referer as authority +- unguarded `Filament::setTenant($model->...)` derived from an arbitrary identifier +- broad “set tenant from model id” fallback in the shared seam + +### Phase 3 — Apply the contract to confirmed surfaces + +- **BackupSetPolicyPickerTable**: + - Resolve context from the BackupSet (validated) and scope policy query accordingly. + - Ensure mutation-time recheck before attaching policy IDs. + - Do not override a mismatched existing tenant; fail closed. +- **RestoreRunResource**: + - Ensure option closures used in wizard lifecycles can resolve environment context without route params. + - Implement validated Livewire update context recovery via the shared seam (`apps/platform/app/Support/OperateHub/OperateHubShell.php`) by treating the `Referer` path as a candidate (never authority) and re-validating workspace + membership + operability before using it. +- **BackupScheduleOperationRunsRelationManager**: + - Use owner schedule context (`$this->getOwnerRecord()`) as primary context. + - Avoid ambient `ManagedEnvironment::currentOrFail()`-style assumptions when owner exists. +- **ManagedEnvironmentTriageArrivalContinuity**: + - Prefer widget record context; ambient tenant only fallback. + - Use the same resolver for visibility and mutation-time checks. + +### Phase 4 — Guardrails + +- Add `apps/platform/tests/Architecture/FilamentTenantContextContractTest.php` to: + - detect unsafe `Filament::setTenant(...)` patterns in nested surfaces + - keep an explicit allowlist for infrastructure-only locations + - optionally classify (not fail) direct `Filament::getTenant()` usage in known high-risk directories first, to avoid noisy failures + +### Phase 5 — Tests and validation + +Add focused tests (Feature/Livewire) for: + +- tenantless Add Policies picker: authorized user still sees checkboxes and can select items +- restore create wizard: does not throw when route params are missing in Livewire update lifecycle +- owner-scoped relation manager: query does not broaden when ambient tenant is null/wrong +- widget context: record context is preferred; missing context fails closed + +Validation commands (narrow first): + +- `cd apps/platform && ./vendor/bin/sail artisan test tests/Feature/Filament --filter='BackupSetPolicyPicker|RestoreRun|BackupScheduleOperationRuns|Triage' --compact` +- `cd apps/platform && ./vendor/bin/sail artisan test tests/Architecture --filter='FilamentTenantContextContract' --compact` + +Browser smoke (only when feature tests pass): + +- `cd apps/platform && php vendor/bin/pest tests/Browser/Spec334NestedFilamentContextContractSmokeTest.php --compact` + +## Deployment / Ops Impact + +- **Migrations**: none expected. +- **Env vars**: none expected. +- **Queues/scheduler**: none expected. +- **Filament assets**: no new registered assets expected; deployment posture unchanged. diff --git a/specs/334-nested-filament-context-contract-hardening/spec.md b/specs/334-nested-filament-context-contract-hardening/spec.md new file mode 100644 index 00000000..8e6f7c4e --- /dev/null +++ b/specs/334-nested-filament-context-contract-hardening/spec.md @@ -0,0 +1,343 @@ +# Feature Specification: Spec 334 - Nested Filament / Livewire Context Contract Hardening + +- Feature Branch: `334-nested-filament-context-contract-hardening` +- Created: 2026-05-24 +- Status: Draft +- Type: Platform integrity / Livewire lifecycle hardening / Filament context contract / productization blocker removal +- Runtime posture: Narrow platform hardening. No tenancy rewrite. +- Input: User-provided Spec 334 draft + repo inspection for path truth. + +## Dependencies And Historical Context + +This spec exists because two independent operator flows exposed the same underlying defect class: + +1) **Backup Set → “Add Policies” picker modal** + +- `Filament::getTenant()` can be `null` in nested/modal contexts. +- Authorization then fails false-negative for a bulk action / row selection. +- Filament hides the row selection checkbox column. +- Operator sees policies as non-selectable. + +2) **Restore Run → Create wizard (Livewire update lifecycle)** + +- Livewire update request is a `POST /livewire-.../update` request with no route parameters. +- Option closures can evaluate in that lifecycle. +- Context resolution that hard-requires ambient Filament tenant can throw, blocking the wizard. + +Repo truth note: the user draft references a “Spec 332 Restore Flow Productization WIP” as the direct dependency. On `platform-dev` there is currently no `specs/332-*` directory. This spec still stands as a narrow platform hardening slice, and should be linked to the active restore productization spec/branch once it exists in-repo. + +Related existing work (context only; do not modify closed specs here): + +- `specs/152-livewire-context-locking` (draft): broader “trusted state” hardening. This spec is narrower and explicitly tenant-context-centric. +- `specs/302-tenant-owned-surface-route-audit`: established the current `ResolvesPanelTenantContext` seam and route-owned environment posture. +- `docs/research/filament-v5-notes.md`: source of truth when Filament/Livewire behavior is uncertain. + +## Spec Candidate Check *(mandatory - SPEC-GATE-001)* + +- **Problem**: Nested Filament / Livewire surfaces sometimes execute without reliable ambient tenant context, causing false-negative authorization, hidden selection controls, or hard runtime failures in operator-critical flows. +- **Today's failure**: Authorized operators cannot select policies in the Add Policies picker (checkbox column missing) and restore create flows can crash during Livewire updates with “no tenant context selected”. +- **User-visible improvement**: Operator-critical nested surfaces remain stable (no missing checkboxes, no wizard crash) and fail closed with honest UI states when context is genuinely unavailable. +- **Smallest enterprise-capable version**: Harden only confirmed high-risk nested surfaces and the shared context seam; add guardrails/tests so unsafe patterns do not reappear. No route architecture rewrite, no tenancy model redesign. +- **Explicit non-goals**: No new tenancy concept, no workspace model redesign, no panel rewrite, no broad repo-wide replacement of `Filament::getTenant()`, no RBAC rebuild, no speculative general framework beyond what at least two confirmed surfaces require. +- **Permanent complexity imported**: Targeted context resolver adjustments, a small optional helper (only if reused by ≥2 confirmed surfaces), focused Pest tests (Feature/Livewire + one architecture guard), and browser smoke steps for the two originally user-visible bugs. +- **Why now**: This is a platform-level productization blocker for restore workflows and breaks a visible day-to-day operator action (“Add policies”). +- **Why not local**: The same defect class occurs across independent surfaces (modal table, wizard update, relation manager, widget). Without an explicit contract + guardrails, it will recur. +- **Approval class**: Core Enterprise. +- **Red flags triggered**: (2) new meta-infrastructure risk (context helper + guard tests), (3) multiple surfaces. **Defense**: scope is explicitly limited to confirmed surfaces; helper is optional and must satisfy ABSTR-001 (≥2 real consumers); no new persisted truth, routes, or global “magic tenant switch”. +- **Score**: Nutzen: 2 | Dringlichkeit: 2 | Scope: 2 | Komplexitaet: 1 | Produktnaehe: 2 | Wiederverwendung: 2 | **Gesamt: 11/12** +- **Decision**: approve. + +## Spec Scope Fields *(mandatory)* + +- **Scope**: tenant (managed-environment scoped operator surfaces under `/admin/workspaces/{workspace}/environments/{environment}/...`) + Livewire update lifecycle hardening. +- **Primary Routes / Surfaces** (representative, non-exhaustive): + - Backup Set detail → “Add policies” modal table (Livewire component). + - Restore Runs → Create wizard (Filament resource create page). + - Backup Schedule detail → Operation Runs relation manager table. + - Managed Environment dashboard → triage widget. +- **Data Ownership**: + - Tenant/environment-owned records: `ManagedEnvironment`, `BackupSet`, `BackupSchedule`, `RestoreRun`, related policy inventory models. + - Workspace ownership remains authoritative; context recovery must validate workspace membership/entitlement. + - No new persisted entity/table is introduced by this spec. +- **RBAC**: + - Workspace membership + environment entitlement required to view tenant-scoped surfaces. + - Mutations (attach policies, restore run create/confirm, etc.) require explicit capability checks and policy/gate enforcement; UI visibility is not authorization. + +For canonical-view specs: N/A. This spec hardens environment/tenant-scoped nested surfaces, not a workspace-owned canonical viewer. + +## UI Surface Impact *(mandatory - UI-COV-001)* + +Does this spec add, remove, rename, or materially change any reachable UI surface? + +- [ ] No UI surface impact +- [x] Existing page changed +- [ ] New page/route added +- [ ] Navigation changed +- [ ] Filament panel/provider surface changed +- [x] New modal/drawer/wizard/action added +- [x] New table/form/state added +- [ ] Customer-facing surface changed +- [x] Dangerous action changed +- [ ] Status/evidence/review presentation changed +- [x] Workspace/environment context presentation changed + +## UI/Productization Coverage + +- **Route/page/surface**: Add Policies modal table, Restore Run Create wizard, Backup Schedule Operation Runs relation manager, Managed Environment triage widget. +- **Current or new page archetype**: Domain Pattern Surface (nested CRUD tooling and wizards), not a new strategic page archetype. +- **Design depth**: Internal/Hidden (but operator-reachable and workflow-critical). +- **Repo-truth level**: repo-verified for file paths and current helper seams; runtime behavior must be validated by tests + browser smoke. +- **Existing pattern reused**: current `ResolvesPanelTenantContext` seam, route-owned environment posture, `UiEnforcement`, policies/gates, and owner-record scoping patterns. +- **New pattern required**: explicit nested context resolution order for high-risk surfaces; optional small helper only if used by ≥2 surfaces. +- **Screenshot required**: no new design screenshots required; browser smoke is required for the two user-visible bugs. +- **Page audit required**: no UI audit registry update expected unless implementation changes navigation or surface archetype. +- **Customer-safe review required**: no. These are operator surfaces; still must fail closed and never broaden scope. +- **Dangerous-action review required**: yes. Restore-related and attach-related actions are high-impact and must keep explicit confirmation + authorization + audit posture. +- **Coverage files updated or explicitly not needed**: + - [ ] `docs/ui-ux-enterprise-audit/route-inventory.md` + - [ ] `docs/ui-ux-enterprise-audit/design-coverage-matrix.md` + - [ ] `docs/ui-ux-enterprise-audit/page-reports/...` + - [ ] `docs/ui-ux-enterprise-audit/strategic-surfaces.md` + - [ ] `docs/ui-ux-enterprise-audit/grouped-follow-up-candidates.md` + - [ ] `docs/ui-ux-enterprise-audit/unresolved-pages.md` + - [x] `N/A - no new routes/pages; coverage handled via tests + browser smoke + PR close-out note` + +## Cross-Cutting / Shared Pattern Reuse + +- **Cross-cutting feature?**: yes. +- **Interaction class(es)**: authorization/visibility, table row selection, bulk actions, wizard step transitions, select option closures, widget visibility/mutation. +- **Systems touched**: Filament tenant resolution seam, route-owned environment posture, remembered environment context (validated only), and nested Livewire lifecycle behavior. +- **Existing pattern(s) to extend**: owner-record scoping, `UiEnforcement`, deny-as-not-found (404) vs forbidden (403) semantics, and existing environment context middleware/helpers. +- **Shared contract / presenter / builder / renderer to reuse**: `apps/platform/app/Filament/Concerns/ResolvesPanelTenantContext.php`, `apps/platform/app/Support/OperateHub/OperateHubShell.php`, `apps/platform/app/Support/Workspaces/WorkspaceContext.php`, `apps/platform/app/Support/Rbac/UiEnforcement.php`, and existing policies. +- **Why the existing shared path is sufficient or insufficient**: sufficient for route-owned page loads; insufficient for nested Livewire update requests and nested surfaces that currently assume ambient tenant is always present. +- **Allowed deviation and why**: bounded “nested context contract” logic may be added only where confirmed surfaces prove it is needed; no global ambient-tenant-as-truth fallback is allowed. +- **Review focus**: fail-closed semantics (no broadening), no unsafe `Filament::setTenant($model->...)` without validated ownership/capability, and stable UX for authorized operators when context is recoverable. + +## Summary + +Harden TenantPilot’s nested Filament and Livewire context handling so critical nested surfaces do not depend on unreliable ambient `Filament::getTenant()` state during modal, wizard, relation manager, widget, and Livewire update lifecycles. + +This spec does **not** rewrite tenancy, routing, RBAC, or the workspace model. It introduces targeted hardening for confirmed high-risk surfaces and installs guardrails so the same issue does not keep reappearing. + +## Problem Statement + +Nested Filament/Livewire surfaces can execute outside the full page routing lifecycle: + +- modal tables +- relation managers +- wizard step transitions +- select option closures +- Livewire update requests (`POST /livewire-.../update`) +- widgets +- bulk actions +- row selection +- table refresh/search/filter/pagination + +In those contexts: + +- route parameters may be missing +- ambient Filament tenant may be `null` +- authorization can fail false-negative +- UI controls may disappear +- option closures may hard-fail + +Root problem: + +> Ambient framework context is being used where explicit domain context is required. + +Desired nested context contract: + +1. Owner/domain context first. +2. Validated route/workspace/environment context second. +3. Remembered/session environment only if validated. +4. Ambient Filament tenant only as fallback convenience. +5. Fail closed with a clear product state. +6. Never switch tenant from a model before ownership/capability is proven. + +## Goals + +### G1 — Define and enforce a nested context contract + +Create a consistent rule for nested Filament/Livewire surfaces: + +> Domain context is authoritative. Filament tenant is runtime convenience. + +The contract must cover: table queries, option closures, visibility/disabled checks, row selection, bulk actions, mutation handlers, relation managers, widgets, and wizard transitions. + +### G2 — Fix the confirmed Add Policies / BackupSet picker class + +`apps/platform/app/Livewire/BackupSetPolicyPickerTable.php` must be self-protecting: + +- does not trust a passed BackupSet ID before validating workspace/environment ownership and access +- authorized operators can select policies even if ambient tenant is initially null +- forged cross-workspace/cross-environment IDs fail closed +- no unguarded `Filament::setTenant(...)` + +### G3 — Fix the confirmed Restore Run Create Wizard Livewire context loss + +`apps/platform/app/Filament/Resources/RestoreRunResource.php` and shared context resolution must not hard-fail during Livewire update requests when context is recoverable from a validated source. + +### G4 — Harden owner-derived relation/widget surfaces + +At minimum: + +- `apps/platform/app/Filament/Resources/BackupScheduleResource/RelationManagers/BackupScheduleOperationRunsRelationManager.php` +- `apps/platform/app/Filament/Widgets/ManagedEnvironment/ManagedEnvironmentTriageArrivalContinuity.php` + +must resolve context from owner record / widget record before using ambient tenant. + +### G5 — Install guardrails + +Add guardrails/tests that prevent reintroducing unsafe patterns such as model-derived `Filament::setTenant(...)` in nested surfaces without prior ownership/capability validation. + +### G6 — Unblock restore productization validation + +After this spec is implemented, restore flow productization work should be able to run restore create wizard browser validation without a tenantless Livewire update crash. + +## Non-Goals + +- no workspace model redesign +- no managed environment model redesign +- no Filament panel rewrite +- no route architecture rewrite +- no RBAC rebuild +- no broad resource authorization audit +- no restore UX redesign beyond what is needed to make the wizard stable +- no broad repo-wide replacement of `Filament::getTenant()` usage + +## Core Rules + +### R1 — Domain context before Filament tenant + +Nested surfaces must use owner/domain records first. + +### R2 — No unguarded model-derived tenant switch + +Switching tenant based on a loaded model is forbidden unless ownership/capability is validated and mismatch handling fails closed. + +### R3 — UI visibility is not authorization + +Hidden checkboxes/buttons are not security boundaries. Mutations must be authorization-checked at execution time. + +### R4 — Fail closed, but not mysteriously + +If context is invalid: no data leak, no mutation, no unscoped query, and no silent cross-tenant switch. + +### R5 — Livewire update requests need recoverable context + +During Livewire update (`POST /livewire-.../update`), route params may be absent. Recovery must come from validated component state / owner record / validated remembered context. Referer may be a candidate only, never authority. + +### R6 — Mutation-time recheck + +Every mutation handler must re-resolve and re-check context and authorization. + +## Confirmed Scope + +### S1 — BackupSetPolicyPickerTable + +- File: `apps/platform/app/Livewire/BackupSetPolicyPickerTable.php` +- Issue class: tenantless modal context → false-negative authorization → checkbox column hidden. +- Required outcome: authorized users can select and attach in-scope policies; cross-scope mounts fail closed; no unsafe tenant switching. + +### S2 — RestoreRunResource Livewire update context loss + +- Files: + - `apps/platform/app/Filament/Resources/RestoreRunResource.php` + - `apps/platform/app/Filament/Concerns/ResolvesPanelTenantContext.php` +- Issue class: Livewire update has no route params → tenant resolution throws. +- Required outcome: wizard does not crash; option closures do not depend exclusively on ambient tenant; context recovery is validated. + +### S3 — BackupScheduleOperationRunsRelationManager + +- File: `apps/platform/app/Filament/Resources/BackupScheduleResource/RelationManagers/BackupScheduleOperationRunsRelationManager.php` +- Required outcome: owner schedule context drives query and URLs; missing/wrong ambient tenant does not broaden scope. + +### S4 — ManagedEnvironmentTriageArrivalContinuity + +- File: `apps/platform/app/Filament/Widgets/ManagedEnvironment/ManagedEnvironmentTriageArrivalContinuity.php` +- Required outcome: record context first, ambient tenant fallback only; missing context fails closed; visibility and mutation use the same resolver. + +### S5 — Guardrail tests + +New or existing guard tests must live alongside existing architecture tests at: + +- `apps/platform/tests/Architecture/FilamentTenantContextContractTest.php` (new) + +## Acceptance Criteria + +### AC1 — Restore Run Create Wizard no longer crashes on Livewire update + +- Livewire updates do not throw “no tenant context selected” when context is recoverable. +- Backup set options remain environment-scoped. +- Cross-workspace or cross-environment options are never visible. +- If context is not recoverable, the UI fails closed (disabled/empty) without leaking scope. + +### AC2 — BackupSet Policy Picker is self-protecting + +- Authorized tenantless modal mount shows selectable in-scope policies. +- Checkbox column remains visible for authorized users. +- Cross-scope or forged mounts fail closed. +- Cross-environment policy IDs cannot be attached. + +### AC3 — BackupSchedule Operation Runs relation is owner-scoped + +- Relation uses owner BackupSchedule context. +- Missing ambient tenant does not crash. +- Wrong ambient tenant does not broaden query or URLs. + +### AC4 — Triage widget uses explicit context + +- Record context is preferred; ambient tenant is fallback only. +- Missing context fails closed. +- Authorized operators do not lose actions due to ambient context loss. + +### AC5 — Guardrails exist + +- Unsafe model-derived `Filament::setTenant(...)` in nested surfaces is detected. +- Allowlist is explicit and limited to infrastructure. + +### AC6 — No broad unrelated changes + +- No panel/routing rewrite, no workspace/managed-environment redesign, no new persisted truth, and no unrelated UX/productization expansion. + +## Proportionality Review + +- **New source of truth?**: no. +- **New persisted entity/table/artifact?**: no. +- **New abstraction?**: optional small helper only if it is reused by ≥2 confirmed surfaces; otherwise local resolvers only. +- **New enum/status family?**: no. +- **Current operator problem**: missing row selection and crashing restore wizard due to context loss. +- **Narrowest correct implementation**: validate and re-resolve context from owner/domain and canonical workspace/environment truth; fall back only when safe; add guard tests. +- **Ownership cost**: limited helper/tests + browser smoke for two user-visible issues. +- **Alternative rejected**: tenancy rewrite, route architecture changes, broad repo sweep of `Filament::getTenant()`, or “always set tenant from model” fallback. + +## Testing / Lane / Runtime Impact *(mandatory for runtime behavior changes)* + +- **Test purpose / classification**: Feature + Livewire + Architecture (guard); browser smoke for the two user-visible bugs. +- **Validation lane(s)**: confidence + browser (only for the named smoke flows). +- **Why this lane mix is sufficient**: Feature/Livewire tests prove scoped behavior and prevent regressions; browser smoke verifies the original “checkbox missing” and “wizard crash” end-to-end. +- **No Graph calls during UI render**: must remain true; context recovery must be DB-only and authorization-safe. + +## Risks & Mitigations + +- **Risk**: Resolver becomes too broad or “magic”. + - **Mitigation**: Fix only confirmed seams; keep helper optional and bounded; add explicit tests and allowlists. +- **Risk**: Referer parsing introduces security risk. + - **Mitigation**: Referer can only be a candidate; any extracted identifiers must be validated against DB membership and scope. +- **Risk**: Guardrail is too noisy. + - **Mitigation**: Start by guarding unsafe `setTenant` patterns; classify `getTenant` usage before hard failing. + +## Open Questions + +- Which exact capability constants gate: + - attaching policies to a backup set + - creating a restore run + - viewing schedule operation runs + - acting in triage widget actions + (Implementation must use the existing capability registry; no new strings.) +- Does the repo already have a safe “current environment” helper for nested Livewire updates beyond `ResolvesPanelTenantContext`? If yes, reuse it. + +## Follow-Up Spec Candidates *(out of scope for Spec 334)* + +- Broader “environment-resource-context-follow-through” rollout beyond the 4 confirmed surfaces. +- Harmonize with Spec 152 “trusted state” hardening if both proceed; avoid duplicated helper layers. diff --git a/specs/334-nested-filament-context-contract-hardening/tasks.md b/specs/334-nested-filament-context-contract-hardening/tasks.md new file mode 100644 index 00000000..d88f43c2 --- /dev/null +++ b/specs/334-nested-filament-context-contract-hardening/tasks.md @@ -0,0 +1,92 @@ +# Tasks: Spec 334 - Nested Filament / Livewire Context Contract Hardening + +- Input: `specs/334-nested-filament-context-contract-hardening/spec.md`, `specs/334-nested-filament-context-contract-hardening/plan.md` +- Prerequisites: `spec.md`, `plan.md` + +**Tests**: Required. This is runtime-hardening for nested operator surfaces with authorization/scope impact. + +## Test Governance Checklist + +- [x] Lane assignment is explicit and is the narrowest sufficient proof for the changed behavior. +- [x] New tests stay in the smallest honest family (Feature/Livewire + one architecture guard; browser smoke only for the two user-visible regressions). +- [x] New helpers/factories/seeds/context defaults stay cheap by default. +- [x] Planned validation commands cover the change without pulling in unrelated lane cost. +- [x] Any deviation resolves as `document-in-feature`, `follow-up-spec`, or `reject-or-split`. + +## Phase 1: Preparation And Repo Truth + +**Purpose**: Confirm repo truth and capture the current failure paths before touching runtime code. + +- [x] T001 Re-read `spec.md`, `plan.md`, and this `tasks.md`. +- [x] T002 Confirm working tree intent and record baseline commit (`git status`, `git log -1`). +- [x] T003 Verify confirmed scope file paths exist and inspect current context usage: + - `apps/platform/app/Livewire/BackupSetPolicyPickerTable.php` + - `apps/platform/app/Filament/Resources/RestoreRunResource.php` + - `apps/platform/app/Filament/Concerns/ResolvesPanelTenantContext.php` + - `apps/platform/app/Filament/Resources/BackupScheduleResource/RelationManagers/BackupScheduleOperationRunsRelationManager.php` + - `apps/platform/app/Filament/Widgets/ManagedEnvironment/ManagedEnvironmentTriageArrivalContinuity.php` + - `apps/platform/app/Support/OperateHub/OperateHubShell.php` + - `apps/platform/app/Support/Workspaces/WorkspaceContext.php` +- [x] T004 Capture the current Add Policies modal failure path (tenantless modal → checkbox column hidden) and record the exact call chain and where tenant/context is read. + - Captured via regression: `apps/platform/tests/Feature/Filament/BackupSetPolicyPickerTableTest.php` and browser smoke `apps/platform/tests/Browser/Spec334NestedFilamentContextContractSmokeTest.php`. +- [x] T005 Capture the current Restore Run Create wizard failure path (Livewire update request without route params → tenant resolver throws) and record the exact call chain and closure(s) involved. + - Captured via regression: `apps/platform/tests/Feature/Filament/RestoreRunResourceLivewireTenantContextTest.php` (real Livewire update payload) + browser smoke `apps/platform/tests/Browser/Spec334NestedFilamentContextContractSmokeTest.php`. +- [x] T006 Decide whether any shared helper is justified (≥2 confirmed consumers). If not, keep fixes local to each surface + the existing shared seam. + - No new helper introduced (ABSTR-001). Shared seam hardening applied in `apps/platform/app/Support/OperateHub/OperateHubShell.php`. + +## Phase 2: Guardrails And Regression Tests (before refactor) + +**Purpose**: Lock “fail-closed and scoped” behavior and block unsafe tenant switching patterns before implementation refactors. + +- [x] T007 Add an architecture guard test: `apps/platform/tests/Architecture/FilamentTenantContextContractTest.php`. + - Detect and fail on unsafe `Filament::setTenant($model->...)` patterns in nested surfaces without explicit allowlist. + - Keep an explicit allowlist limited to infrastructure-only context selection locations. +- [x] T008 Add a regression test covering Add Policies picker behavior when ambient tenant is null (authorized user still sees row selection and can select). +- [x] T009 Add a regression test covering Restore Run Create wizard Livewire update lifecycle (no crash when route params are missing; options are scoped). +- [x] T010 Add a regression test for BackupSchedule operation runs relation manager scope (owner record scopes query; ambient tenant missing/wrong does not broaden). +- [x] T011 Add a regression test for the triage widget context (record context preferred; ambient tenant is fallback; missing context fails closed). + +## Phase 3: Implement Nested Context Contract (confirmed surfaces only) + +**Purpose**: Apply the contract to the confirmed high-risk surfaces without a broad tenancy rewrite. + +- [x] T012 Harden `apps/platform/app/Livewire/BackupSetPolicyPickerTable.php`: + - Resolve context from validated BackupSet ownership (workspace + managed environment). + - Scope table query by resolved environment. + - Re-resolve and re-authorize at mutation time (bulk action). + - Never silently override mismatched ambient tenant. +- [x] T013 Harden `apps/platform/app/Filament/Resources/RestoreRunResource.php`: + - Ensure option closures used during Livewire updates can resolve validated environment context without route params. + - Livewire update context recovery is implemented in the shared seam: `apps/platform/app/Support/OperateHub/OperateHubShell.php` (validated referer path candidate for `/livewire-*/update` requests). + - Referer is treated as a candidate only; access is still validated via workspace + membership + operability checks. +- [x] T014 Harden `apps/platform/app/Filament/Concerns/ResolvesPanelTenantContext.php` only if the seam is shared and the fix is not restore-specific. + - No change required (the shared seam hardening in `OperateHubShell` unblocks the failing lifecycle). +- [x] T015 Harden `apps/platform/app/Filament/Resources/BackupScheduleResource/RelationManagers/BackupScheduleOperationRunsRelationManager.php`: + - Prefer owner schedule record as primary context (`$this->getOwnerRecord()`). + - Remove dependencies on ambient tenant where owner context exists. +- [x] T016 Harden `apps/platform/app/Filament/Widgets/ManagedEnvironment/ManagedEnvironmentTriageArrivalContinuity.php`: + - Prefer widget record context. + - Use one resolver consistently for visibility and mutation checks. + - Fail closed when context is missing. + +## Phase 4: Browser Smoke (required because the original bugs were user-visible) + +- [x] T017 Backup Set Add Policies smoke: + - Open Backup Set detail → Add Policies. + - Verify policies are visible and row selection checkbox column is visible. + - Select one or more rows; verify “Add selected” path works. +- [x] T018 Restore Run Create wizard smoke: + - Open Restore Runs → Create. + - Trigger Livewire interactions (wizard next/previous, selections). + - Verify no “no tenant context selected” crash. + +## Phase 5: Validation + +- [x] T019 Run narrow tests first (confidence lane): + - `cd apps/platform && ./vendor/bin/sail artisan test tests/Architecture --filter='FilamentTenantContextContract' --compact` + - `cd apps/platform && ./vendor/bin/sail artisan test tests/Feature/Filament --filter='BackupSetPolicyPicker|RestoreRun|BackupScheduleOperationRuns|Triage' --compact` +- [ ] T020 Run broader related suites only if needed (keep lane cost honest): + - `cd apps/platform && ./vendor/bin/sail artisan test tests/Feature/Filament --compact` +- [x] T021 Run `cd apps/platform && ./vendor/bin/sail pint --dirty` and `git diff --check`. +- [x] T022 Report full-suite status honestly if not run. + - Full suite not executed; this spec was validated via targeted Feature/Architecture tests + a dedicated browser smoke file. -- 2.45.2