From 46b11d37707cc9bce1043d6622c40ef52e5b92c4 Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Wed, 18 Mar 2026 13:56:09 +0100 Subject: [PATCH] feat: harden findings workflow and audit backstop --- .github/agents/copilot-instructions.md | 3 +- app/Filament/Resources/FindingResource.php | 14 +- .../FindingResource/Pages/ListFindings.php | 2 +- app/Jobs/CompareBaselineToTenantJob.php | 21 +- app/Models/AuditLog.php | 15 +- app/Models/Finding.php | 38 -- app/Policies/FindingPolicy.php | 45 ++- app/Services/Audit/AuditRecorder.php | 49 ++- .../Baselines/BaselineAutoCloseService.php | 28 +- .../EntraAdminRolesFindingGenerator.php | 69 ++-- .../Findings/FindingWorkflowService.php | 228 ++++++++--- app/Services/Intune/AuditLogger.php | 15 +- .../PermissionPostureFindingGenerator.php | 71 ++-- app/Support/Audit/AuditActionId.php | 29 +- app/Support/Audit/AuditContextSanitizer.php | 13 + database/factories/FindingFactory.php | 79 ++++ docs/product/spec-candidates.md | 221 ++++++++++- .../checklists/requirements.md | 34 ++ .../contracts/finding-audit-event.schema.yaml | 139 +++++++ .../contracts/findings-workflow.openapi.yaml | 357 ++++++++++++++++++ .../data-model.md | 137 +++++++ specs/151-findings-workflow-backstop/plan.md | 177 +++++++++ .../quickstart.md | 121 ++++++ .../research.md | 70 ++++ specs/151-findings-workflow-backstop/spec.md | 172 +++++++++ specs/151-findings-workflow-backstop/tasks.md | 197 ++++++++++ .../Audit/FindingAuditVisibilityTest.php | 193 ++++++++++ .../EntraAdminRolesFindingGeneratorTest.php | 6 +- .../InteractsWithFindingsWorkflow.php | 62 +++ .../Findings/FindingAuditBackstopTest.php | 130 +++++++ .../FindingAutomationWorkflowTest.php | 330 ++++++++++++++++ tests/Feature/Findings/FindingRbacTest.php | 33 ++ .../FindingWorkflowConcurrencyTest.php | 183 +++++++++ .../Findings/FindingWorkflowGuardTest.php | 51 +++ .../Findings/FindingWorkflowServiceTest.php | 98 +++++ .../FindingWorkflowUiEnforcementTest.php | 90 +++++ tests/Feature/Models/FindingResolvedTest.php | 38 +- .../PermissionPostureFindingGeneratorTest.php | 6 +- tests/Pest.php | 4 + 39 files changed, 3321 insertions(+), 247 deletions(-) create mode 100644 specs/151-findings-workflow-backstop/checklists/requirements.md create mode 100644 specs/151-findings-workflow-backstop/contracts/finding-audit-event.schema.yaml create mode 100644 specs/151-findings-workflow-backstop/contracts/findings-workflow.openapi.yaml create mode 100644 specs/151-findings-workflow-backstop/data-model.md create mode 100644 specs/151-findings-workflow-backstop/plan.md create mode 100644 specs/151-findings-workflow-backstop/quickstart.md create mode 100644 specs/151-findings-workflow-backstop/research.md create mode 100644 specs/151-findings-workflow-backstop/spec.md create mode 100644 specs/151-findings-workflow-backstop/tasks.md create mode 100644 tests/Feature/Audit/FindingAuditVisibilityTest.php create mode 100644 tests/Feature/Findings/Concerns/InteractsWithFindingsWorkflow.php create mode 100644 tests/Feature/Findings/FindingAuditBackstopTest.php create mode 100644 tests/Feature/Findings/FindingAutomationWorkflowTest.php create mode 100644 tests/Feature/Findings/FindingWorkflowConcurrencyTest.php create mode 100644 tests/Feature/Findings/FindingWorkflowGuardTest.php create mode 100644 tests/Feature/Findings/FindingWorkflowServiceTest.php create mode 100644 tests/Feature/Findings/FindingWorkflowUiEnforcementTest.php diff --git a/.github/agents/copilot-instructions.md b/.github/agents/copilot-instructions.md index e43c3d4..f31bd41 100644 --- a/.github/agents/copilot-instructions.md +++ b/.github/agents/copilot-instructions.md @@ -88,6 +88,7 @@ ## Active Technologies - PHP 8.4.15 + Laravel 12, Filament 5, Livewire 4, Pest 4, existing `OperationRunService`, `TrackOperationRun`, `ProviderOperationStartGate`, `TenantOperabilityService`, `CapabilityResolver`, and `WriteGateInterface` seams (149-queued-execution-reauthorization) - PostgreSQL-backed application data plus queue-serialized `OperationRun` context; no schema migration planned for the first implementation slice (149-queued-execution-reauthorization) - PHP 8.4.15 + Laravel 12, Filament v5, Livewire v4, PostgreSQL, Pest 4 (150-tenant-owned-query-canon-and-wrong-tenant-guards) +- PostgreSQL with existing `findings` and `audit_logs` tables; no new storage engine or external log store (151-findings-workflow-backstop) - PHP 8.4.15 (feat/005-bulk-operations) @@ -107,8 +108,8 @@ ## Code Style PHP 8.4.15: Follow standard conventions ## Recent Changes +- 151-findings-workflow-backstop: Added PHP 8.4.15 + Laravel 12, Filament v5, Livewire v4, Laravel Sail, Pest v4, PHPUnit v12 - 150-tenant-owned-query-canon-and-wrong-tenant-guards: Added PHP 8.4.15 + Laravel 12, Filament v5, Livewire v4, PostgreSQL, Pest 4 - 149-queued-execution-reauthorization: Added PHP 8.4.15 + Laravel 12, Filament 5, Livewire 4, Pest 4, existing `OperationRunService`, `TrackOperationRun`, `ProviderOperationStartGate`, `TenantOperabilityService`, `CapabilityResolver`, and `WriteGateInterface` seams -- 148-central-tenant-operability-policy: Added PHP 8.4.15 + Laravel 12, Filament 5, Livewire 4, Pest 4, existing support-layer helpers such as `UiEnforcement`, `CapabilityResolver`, `WorkspaceContext`, `OperateHubShell`, `TenantOperabilityService`, and `TenantActionPolicySurface` diff --git a/app/Filament/Resources/FindingResource.php b/app/Filament/Resources/FindingResource.php index 5af9174..a955704 100644 --- a/app/Filament/Resources/FindingResource.php +++ b/app/Filament/Resources/FindingResource.php @@ -114,7 +114,8 @@ public static function canView(Model $record): bool } if ($record instanceof Finding) { - return (int) $record->tenant_id === (int) $tenant->getKey(); + return (int) $record->tenant_id === (int) $tenant->getKey() + && (int) $record->workspace_id === (int) $tenant->workspace_id; } return true; @@ -1326,6 +1327,7 @@ public static function closeAction(): Actions\Action ->label('Close') ->icon('heroicon-o-x-circle') ->color('danger') + ->visible(fn (Finding $record): bool => static::freshWorkflowRecord($record)->hasOpenStatus()) ->requiresConfirmation() ->form([ Textarea::make('closed_reason') @@ -1360,6 +1362,7 @@ public static function riskAcceptAction(): Actions\Action ->label('Risk accept') ->icon('heroicon-o-shield-check') ->color('warning') + ->visible(fn (Finding $record): bool => static::freshWorkflowRecord($record)->hasOpenStatus()) ->requiresConfirmation() ->form([ Textarea::make('closed_reason') @@ -1432,6 +1435,15 @@ private static function runWorkflowMutation(Finding $record, string $successTitl return; } + if ((int) $record->workspace_id !== (int) $tenant->workspace_id) { + Notification::make() + ->title('Finding belongs to a different workspace') + ->danger() + ->send(); + + return; + } + try { $callback($record, $tenant, $user); } catch (InvalidArgumentException $e) { diff --git a/app/Filament/Resources/FindingResource/Pages/ListFindings.php b/app/Filament/Resources/FindingResource/Pages/ListFindings.php index 5fecee2..5d0a509 100644 --- a/app/Filament/Resources/FindingResource/Pages/ListFindings.php +++ b/app/Filament/Resources/FindingResource/Pages/ListFindings.php @@ -39,7 +39,7 @@ class ListFindings extends ListRecords */ public function mountAction(string $name, array $arguments = [], array $context = []): mixed { - if (($context['table'] ?? false) === true && filled($context['recordKey'] ?? null) && in_array($name, ['triage', 'assign', 'resolve', 'close', 'reopen'], true)) { + if (($context['table'] ?? false) === true && filled($context['recordKey'] ?? null) && in_array($name, ['triage', 'start_progress', 'assign', 'resolve', 'close', 'risk_accept', 'reopen'], true)) { try { FindingResource::resolveScopedRecordOrFail($context['recordKey']); } catch (ModelNotFoundException) { diff --git a/app/Jobs/CompareBaselineToTenantJob.php b/app/Jobs/CompareBaselineToTenantJob.php index 4df48e8..ce3b73b 100644 --- a/app/Jobs/CompareBaselineToTenantJob.php +++ b/app/Jobs/CompareBaselineToTenantJob.php @@ -29,6 +29,7 @@ use App\Services\Drift\Normalizers\ScopeTagsNormalizer; use App\Services\Drift\Normalizers\SettingsNormalizer; use App\Services\Findings\FindingSlaPolicy; +use App\Services\Findings\FindingWorkflowService; use App\Services\Intune\AuditLogger; use App\Services\Intune\IntuneRoleDefinitionNormalizer; use App\Services\OperationRunService; @@ -2130,20 +2131,14 @@ private function upsertFindings( : null; if ($resolvedAt === null || $observedAt->greaterThan($resolvedAt)) { - $severity = (string) $driftItem['severity']; - $slaDays = $slaPolicy->daysForSeverity($severity, $tenant); + $finding->save(); - $finding->forceFill([ - 'status' => Finding::STATUS_REOPENED, - 'reopened_at' => $observedAt, - 'resolved_at' => null, - 'resolved_reason' => null, - 'closed_at' => null, - 'closed_reason' => null, - 'closed_by_user_id' => null, - 'sla_days' => $slaDays, - 'due_at' => $slaPolicy->dueAtForSeverity($severity, $tenant, $observedAt), - ]); + app(FindingWorkflowService::class)->reopenBySystem( + finding: $finding, + tenant: $tenant, + reopenedAt: $observedAt, + operationRunId: (int) $this->operationRun->getKey(), + ); $reopenedCount++; } else { diff --git a/app/Models/AuditLog.php b/app/Models/AuditLog.php index 89a02a3..8a87adb 100644 --- a/app/Models/AuditLog.php +++ b/app/Models/AuditLog.php @@ -18,6 +18,14 @@ class AuditLog extends Model { use HasFactory; + /** + * @var array + */ + private const INTERNAL_METADATA_KEYS = [ + '_actor_type', + '_dedupe_key', + ]; + protected $guarded = []; protected $casts = [ @@ -202,7 +210,12 @@ public function contextItems(): array } foreach ($metadata as $key => $value) { - if (in_array($key, $seen, true) || in_array($key, ['before', 'after'], true)) { + if ( + in_array($key, $seen, true) + || in_array($key, ['before', 'after'], true) + || str_starts_with((string) $key, '_') + || in_array((string) $key, self::INTERNAL_METADATA_KEYS, true) + ) { continue; } diff --git a/app/Models/Finding.php b/app/Models/Finding.php index b011f40..704d4ff 100644 --- a/app/Models/Finding.php +++ b/app/Models/Finding.php @@ -160,44 +160,6 @@ public function hasOpenStatus(): bool return self::isOpenStatus($this->status); } - public function acknowledge(User $user): void - { - if ($this->status === self::STATUS_ACKNOWLEDGED) { - return; - } - - $this->forceFill([ - 'status' => self::STATUS_ACKNOWLEDGED, - 'acknowledged_at' => now(), - 'acknowledged_by_user_id' => $user->getKey(), - ]); - - $this->save(); - } - - /** - * Auto-resolve the finding. - */ - public function resolve(string $reason): void - { - $this->status = self::STATUS_RESOLVED; - $this->resolved_at = now(); - $this->resolved_reason = $reason; - $this->save(); - } - - /** - * Re-open a resolved finding. - */ - public function reopen(array $evidence): void - { - $this->status = self::STATUS_NEW; - $this->resolved_at = null; - $this->resolved_reason = null; - $this->evidence_jsonb = $evidence; - $this->save(); - } - public function resolvedSubjectDisplayName(): ?string { $displayName = $this->getAttribute('subject_display_name'); diff --git a/app/Policies/FindingPolicy.php b/app/Policies/FindingPolicy.php index 2e74382..c84c2f0 100644 --- a/app/Policies/FindingPolicy.php +++ b/app/Policies/FindingPolicy.php @@ -33,17 +33,9 @@ public function viewAny(User $user): bool public function view(User $user, Finding $finding): Response|bool { - $tenant = $this->resolvedTenant(); + $tenant = $this->authorizedTenantOrNull($user, $finding); - if (! $tenant) { - return Response::denyAsNotFound(); - } - - if (! $user->canAccessTenant($tenant)) { - return Response::denyAsNotFound(); - } - - if ((int) $finding->tenant_id !== (int) $tenant->getKey()) { + if (! $tenant instanceof Tenant) { return Response::denyAsNotFound(); } @@ -98,20 +90,12 @@ private function canMutateWithCapability(User $user, Finding $finding, string $c */ private function canMutateWithAnyCapability(User $user, Finding $finding, array $capabilities): Response|bool { - $tenant = $this->resolvedTenant(); + $tenant = $this->authorizedTenantOrNull($user, $finding); if (! $tenant instanceof Tenant) { return Response::denyAsNotFound(); } - if (! $user->canAccessTenant($tenant)) { - return Response::denyAsNotFound(); - } - - if ((int) $finding->tenant_id !== (int) $tenant->getKey()) { - return Response::denyAsNotFound(); - } - /** @var CapabilityResolver $resolver */ $resolver = app(CapabilityResolver::class); @@ -124,6 +108,29 @@ private function canMutateWithAnyCapability(User $user, Finding $finding, array return Response::deny(); } + private function authorizedTenantOrNull(User $user, Finding $finding): ?Tenant + { + $tenant = $this->resolvedTenant(); + + if (! $tenant instanceof Tenant) { + return null; + } + + if (! $user->canAccessTenant($tenant)) { + return null; + } + + if ((int) $finding->tenant_id !== (int) $tenant->getKey()) { + return null; + } + + if ((int) $finding->workspace_id !== (int) $tenant->workspace_id) { + return null; + } + + return $tenant; + } + private function resolvedTenant(): ?Tenant { if (Filament::getCurrentPanel()?->getId() === 'admin') { diff --git a/app/Services/Audit/AuditRecorder.php b/app/Services/Audit/AuditRecorder.php index c5c1c32..91f76e6 100644 --- a/app/Services/Audit/AuditRecorder.php +++ b/app/Services/Audit/AuditRecorder.php @@ -36,19 +36,42 @@ public function record( ): AuditLog { $actionValue = $action instanceof AuditActionId ? $action->value : trim($action); - return AuditLog::query()->create( - $this->builder->buildRecordAttributes( - action: $actionValue, - context: $context, - workspace: $workspace, - tenant: $tenant, - actor: $actor, - target: $target, - outcome: $outcome, - recordedAt: $recordedAt, - summary: $summary, - operationRunId: $operationRunId, - ), + $metadata = is_array($context['metadata'] ?? null) ? $context['metadata'] : []; + $dedupeKey = is_string($metadata['_dedupe_key'] ?? null) ? trim((string) $metadata['_dedupe_key']) : null; + + if ($dedupeKey !== '') { + $metadata['_dedupe_key'] = $dedupeKey; + $context['metadata'] = $metadata; + } + + $attributes = $this->builder->buildRecordAttributes( + action: $actionValue, + context: $context, + workspace: $workspace, + tenant: $tenant, + actor: $actor, + target: $target, + outcome: $outcome, + recordedAt: $recordedAt, + summary: $summary, + operationRunId: $operationRunId, ); + + if ($dedupeKey !== null && $dedupeKey !== '') { + $existing = AuditLog::query() + ->where('tenant_id', $attributes['tenant_id']) + ->where('action', $attributes['action']) + ->where('resource_type', $attributes['resource_type']) + ->where('resource_id', $attributes['resource_id']) + ->whereRaw("metadata ->> '_dedupe_key' = ?", [$dedupeKey]) + ->latest('id') + ->first(); + + if ($existing instanceof AuditLog) { + return $existing; + } + } + + return AuditLog::query()->create($attributes); } } diff --git a/app/Services/Baselines/BaselineAutoCloseService.php b/app/Services/Baselines/BaselineAutoCloseService.php index 3773edc..987e7fb 100644 --- a/app/Services/Baselines/BaselineAutoCloseService.php +++ b/app/Services/Baselines/BaselineAutoCloseService.php @@ -8,14 +8,17 @@ use App\Models\OperationRun; use App\Models\Tenant; use App\Models\Workspace; +use App\Services\Findings\FindingWorkflowService; use App\Services\Settings\SettingsResolver; use App\Support\OperationRunOutcome; use App\Support\OperationRunStatus; +use Carbon\CarbonImmutable; final class BaselineAutoCloseService { public function __construct( private readonly SettingsResolver $settingsResolver, + private readonly ?FindingWorkflowService $findingWorkflowService = null, ) {} public function shouldAutoClose(Tenant $tenant, OperationRun $run): bool @@ -73,7 +76,7 @@ public function resolveStaleFindings( int $currentOperationRunId, ): int { $scopeKey = 'baseline_profile:'.$baselineProfileId; - $resolvedAt = now(); + $resolvedAt = CarbonImmutable::now(); $resolvedCount = 0; $query = Finding::query() @@ -88,18 +91,22 @@ public function resolveStaleFindings( $query->whereNotIn('fingerprint', array_values(array_unique($seenFingerprints))); } - $query->chunkById(100, function ($findings) use (&$resolvedCount, $resolvedAt, $currentOperationRunId): void { + $query->chunkById(100, function ($findings) use ($tenant, &$resolvedCount, $resolvedAt, $currentOperationRunId): void { foreach ($findings as $finding) { if (! $finding instanceof Finding) { continue; } - $finding->forceFill([ - 'status' => Finding::STATUS_RESOLVED, - 'resolved_at' => $resolvedAt, - 'resolved_reason' => 'no_longer_drifting', - 'current_operation_run_id' => $currentOperationRunId, - ])->save(); + $this->findingWorkflowService()->resolveBySystem( + finding: $finding, + tenant: $tenant, + reason: 'no_longer_drifting', + resolvedAt: $resolvedAt, + operationRunId: $currentOperationRunId, + mutate: function (Finding $record) use ($currentOperationRunId): void { + $record->current_operation_run_id = $currentOperationRunId; + }, + ); $resolvedCount++; } @@ -118,4 +125,9 @@ private function resolveWorkspace(Tenant $tenant): ?Workspace return Workspace::query()->whereKey($workspaceId)->first(); } + + private function findingWorkflowService(): FindingWorkflowService + { + return $this->findingWorkflowService ?? app(FindingWorkflowService::class); + } } diff --git a/app/Services/EntraAdminRoles/EntraAdminRolesFindingGenerator.php b/app/Services/EntraAdminRoles/EntraAdminRolesFindingGenerator.php index 36e51aa..b41f755 100644 --- a/app/Services/EntraAdminRoles/EntraAdminRolesFindingGenerator.php +++ b/app/Services/EntraAdminRoles/EntraAdminRolesFindingGenerator.php @@ -9,6 +9,7 @@ use App\Models\OperationRun; use App\Models\Tenant; use App\Services\Findings\FindingSlaPolicy; +use App\Services\Findings\FindingWorkflowService; use Carbon\CarbonImmutable; final class EntraAdminRolesFindingGenerator @@ -21,6 +22,7 @@ final class EntraAdminRolesFindingGenerator public function __construct( private readonly HighPrivilegeRoleCatalog $catalog, private readonly ?FindingSlaPolicy $slaPolicy = null, + private readonly ?FindingWorkflowService $findingWorkflowService = null, ) {} /** @@ -173,19 +175,14 @@ private function upsertFinding( $resolvedAt = $existing->resolved_at; if ($resolvedAt === null || $observedAt->greaterThan(CarbonImmutable::instance($resolvedAt))) { - $slaDays = $slaPolicy->daysForSeverity($severity, $tenant); + $existing->save(); - $existing->forceFill([ - 'status' => Finding::STATUS_REOPENED, - 'reopened_at' => $observedAt, - 'resolved_at' => null, - 'resolved_reason' => null, - 'closed_at' => null, - 'closed_reason' => null, - 'closed_by_user_id' => null, - 'sla_days' => $slaDays, - 'due_at' => $slaPolicy->dueAtForSeverity($severity, $tenant, $observedAt), - ])->save(); + $this->findingWorkflowService()->reopenBySystem( + finding: $existing, + tenant: $tenant, + reopenedAt: $observedAt, + operationRunId: $operationRun?->getKey(), + ); return 'reopened'; } @@ -268,22 +265,19 @@ private function handleGaAggregate( $resolvedAt = $existing->resolved_at; if ($resolvedAt === null || $observedAt->greaterThan(CarbonImmutable::instance($resolvedAt))) { - $slaDays = $slaPolicy->daysForSeverity(Finding::SEVERITY_HIGH, $tenant); + $existing->save(); - $existing->forceFill([ - 'status' => Finding::STATUS_REOPENED, - 'reopened_at' => $observedAt, - 'resolved_at' => null, - 'resolved_reason' => null, - 'closed_at' => null, - 'closed_reason' => null, - 'closed_by_user_id' => null, - 'sla_days' => $slaDays, - 'due_at' => $slaPolicy->dueAtForSeverity(Finding::SEVERITY_HIGH, $tenant, $observedAt), - ]); + $this->findingWorkflowService()->reopenBySystem( + finding: $existing, + tenant: $tenant, + reopenedAt: $observedAt, + operationRunId: $operationRun?->getKey(), + ); $reopened++; $this->produceAlertEvent($tenant, $gaFingerprint, $evidence); + + return $resolved; } } @@ -322,11 +316,12 @@ private function handleGaAggregate( ->first(); if ($existing instanceof Finding) { - $existing->forceFill([ - 'status' => Finding::STATUS_RESOLVED, - 'resolved_at' => $observedAt, - 'resolved_reason' => 'ga_count_within_threshold', - ])->save(); + $this->findingWorkflowService()->resolveBySystem( + finding: $existing, + tenant: $tenant, + reason: 'ga_count_within_threshold', + resolvedAt: $observedAt, + ); $resolved++; } } @@ -353,11 +348,12 @@ private function resolveStaleFindings(Tenant $tenant, array $currentFingerprints continue; } - $finding->forceFill([ - 'status' => Finding::STATUS_RESOLVED, - 'resolved_at' => $observedAt, - 'resolved_reason' => 'role_assignment_removed', - ])->save(); + $this->findingWorkflowService()->resolveBySystem( + finding: $finding, + tenant: $tenant, + reason: 'role_assignment_removed', + resolvedAt: $observedAt, + ); $resolved++; } @@ -463,4 +459,9 @@ private function resolvePrincipalType(array $principal): string default => 'unknown', }; } + + private function findingWorkflowService(): FindingWorkflowService + { + return $this->findingWorkflowService ?? app(FindingWorkflowService::class); + } } diff --git a/app/Services/Findings/FindingWorkflowService.php b/app/Services/Findings/FindingWorkflowService.php index fb8f7dd..2107150 100644 --- a/app/Services/Findings/FindingWorkflowService.php +++ b/app/Services/Findings/FindingWorkflowService.php @@ -10,6 +10,8 @@ use App\Models\User; use App\Services\Auth\CapabilityResolver; use App\Services\Intune\AuditLogger; +use App\Support\Audit\AuditActionId; +use App\Support\Audit\AuditActorType; use App\Support\Auth\Capabilities; use Carbon\CarbonImmutable; use Illuminate\Auth\Access\AuthorizationException; @@ -48,7 +50,7 @@ public function triage(Finding $finding, Tenant $tenant, User $actor): Finding finding: $finding, tenant: $tenant, actor: $actor, - action: 'finding.triaged', + action: AuditActionId::FindingTriaged, context: [ 'metadata' => [ 'triaged_at' => $now->toIso8601String(), @@ -78,7 +80,7 @@ public function startProgress(Finding $finding, Tenant $tenant, User $actor): Fi finding: $finding, tenant: $tenant, actor: $actor, - action: 'finding.in_progress', + action: AuditActionId::FindingInProgress, context: [ 'metadata' => [ 'in_progress_at' => $now->toIso8601String(), @@ -112,7 +114,7 @@ public function assign( finding: $finding, tenant: $tenant, actor: $actor, - action: 'finding.assigned', + action: AuditActionId::FindingAssigned, context: [ 'metadata' => [ 'assignee_user_id' => $assigneeUserId, @@ -141,7 +143,7 @@ public function resolve(Finding $finding, Tenant $tenant, User $actor, string $r finding: $finding, tenant: $tenant, actor: $actor, - action: 'finding.resolved', + action: AuditActionId::FindingResolved, context: [ 'metadata' => [ 'resolved_reason' => $reason, @@ -167,7 +169,7 @@ public function close(Finding $finding, Tenant $tenant, User $actor, string $rea finding: $finding, tenant: $tenant, actor: $actor, - action: 'finding.closed', + action: AuditActionId::FindingClosed, context: [ 'metadata' => [ 'closed_reason' => $reason, @@ -194,7 +196,7 @@ public function riskAccept(Finding $finding, Tenant $tenant, User $actor, string finding: $finding, tenant: $tenant, actor: $actor, - action: 'finding.risk_accepted', + action: AuditActionId::FindingRiskAccepted, context: [ 'metadata' => [ 'closed_reason' => $reason, @@ -229,7 +231,7 @@ public function reopen(Finding $finding, Tenant $tenant, User $actor): Finding finding: $finding, tenant: $tenant, actor: $actor, - action: 'finding.reopened', + action: AuditActionId::FindingReopened, context: [ 'metadata' => [ 'reopened_at' => $now->toIso8601String(), @@ -251,6 +253,97 @@ public function reopen(Finding $finding, Tenant $tenant, User $actor): Finding ); } + public function resolveBySystem( + Finding $finding, + Tenant $tenant, + string $reason, + CarbonImmutable $resolvedAt, + ?int $operationRunId = null, + ?callable $mutate = null, + ): Finding { + $this->assertFindingOwnedByTenant($finding, $tenant); + + if (! $finding->hasOpenStatus()) { + throw new InvalidArgumentException('Only open findings can be resolved.'); + } + + $reason = $this->validatedReason($reason, 'resolved_reason'); + + return $this->mutateAndAudit( + finding: $finding, + tenant: $tenant, + actor: null, + action: AuditActionId::FindingResolved, + context: [ + 'metadata' => [ + 'resolved_reason' => $reason, + 'resolved_at' => $resolvedAt->toIso8601String(), + 'system_origin' => true, + ], + ], + mutate: function (Finding $record) use ($mutate, $reason, $resolvedAt): void { + if ($mutate !== null) { + $mutate($record); + } + + $record->status = Finding::STATUS_RESOLVED; + $record->resolved_reason = $reason; + $record->resolved_at = $resolvedAt; + }, + actorType: AuditActorType::System, + operationRunId: $operationRunId, + ); + } + + public function reopenBySystem( + Finding $finding, + Tenant $tenant, + CarbonImmutable $reopenedAt, + ?int $operationRunId = null, + ?callable $mutate = null, + ): Finding { + $this->assertFindingOwnedByTenant($finding, $tenant); + + if (! in_array((string) $finding->status, Finding::terminalStatuses(), true)) { + throw new InvalidArgumentException('Only terminal findings can be reopened.'); + } + + $slaDays = $this->slaPolicy->daysForFinding($finding, $tenant); + $dueAt = $this->slaPolicy->dueAtForSeverity((string) $finding->severity, $tenant, $reopenedAt); + + return $this->mutateAndAudit( + finding: $finding, + tenant: $tenant, + actor: null, + action: AuditActionId::FindingReopened, + context: [ + 'metadata' => [ + 'reopened_at' => $reopenedAt->toIso8601String(), + 'sla_days' => $slaDays, + 'due_at' => $dueAt->toIso8601String(), + 'system_origin' => true, + ], + ], + mutate: function (Finding $record) use ($mutate, $reopenedAt, $slaDays, $dueAt): void { + if ($mutate !== null) { + $mutate($record); + } + + $record->status = Finding::STATUS_REOPENED; + $record->reopened_at = $reopenedAt; + $record->resolved_at = null; + $record->resolved_reason = null; + $record->closed_at = null; + $record->closed_reason = null; + $record->closed_by_user_id = null; + $record->sla_days = $slaDays; + $record->due_at = $dueAt; + }, + actorType: AuditActorType::System, + operationRunId: $operationRunId, + ); + } + /** * @param array $capabilities */ @@ -260,13 +353,7 @@ private function authorize(Finding $finding, Tenant $tenant, User $actor, array throw new NotFoundHttpException; } - if ((int) $finding->tenant_id !== (int) $tenant->getKey()) { - throw new NotFoundHttpException; - } - - if ((int) $finding->workspace_id !== (int) $tenant->workspace_id) { - throw new NotFoundHttpException; - } + $this->assertFindingOwnedByTenant($finding, $tenant); foreach ($capabilities as $capability) { if ($this->capabilityResolver->can($actor, $tenant, $capability)) { @@ -277,6 +364,17 @@ private function authorize(Finding $finding, Tenant $tenant, User $actor, array throw new AuthorizationException('Missing capability for finding workflow action.'); } + private function assertFindingOwnedByTenant(Finding $finding, Tenant $tenant): void + { + if ((int) $finding->tenant_id !== (int) $tenant->getKey()) { + throw new NotFoundHttpException; + } + + if ((int) $finding->workspace_id !== (int) $tenant->workspace_id) { + throw new NotFoundHttpException; + } + } + private function assertTenantMemberOrNull(Tenant $tenant, ?int $userId, string $field): void { if ($userId === null) { @@ -318,41 +416,53 @@ private function validatedReason(string $reason, string $field): string private function mutateAndAudit( Finding $finding, Tenant $tenant, - User $actor, - string $action, + ?User $actor, + string|AuditActionId $action, array $context, callable $mutate, + ?AuditActorType $actorType = null, + ?int $operationRunId = null, ): Finding { - $before = $this->auditSnapshot($finding); + $metadata = is_array($context['metadata'] ?? null) ? $context['metadata'] : []; + $resolvedFinding = DB::transaction(function () use ($finding, $tenant, $actor, $action, $metadata, $mutate, $actorType, $operationRunId): Finding { + /** @var Finding $record */ + $record = Finding::query() + ->whereKey($finding->getKey()) + ->lockForUpdate() + ->firstOrFail(); - DB::transaction(function () use ($finding, $mutate): void { - $mutate($finding); - $finding->save(); + $before = $this->auditSnapshot($record); + + $mutate($record); + $record->save(); + + $after = $this->auditSnapshot($record); + $auditMetadata = array_merge($metadata, [ + 'finding_id' => (int) $record->getKey(), + 'before_status' => $before['status'] ?? null, + 'after_status' => $after['status'] ?? null, + 'before' => $before, + 'after' => $after, + '_dedupe_key' => $this->dedupeKey($action, $record, $before, $after, $metadata, $actor, $actorType), + ]); + + $this->auditLogger->log( + tenant: $tenant, + action: $action, + actorId: $actor?->getKey() !== null ? (int) $actor->getKey() : null, + actorEmail: $actor?->email, + actorName: $actor?->name, + resourceType: 'finding', + resourceId: (string) $record->getKey(), + context: ['metadata' => $auditMetadata], + actorType: $actorType, + operationRunId: $operationRunId, + ); + + return $record; }); - $finding->refresh(); - - $metadata = is_array($context['metadata'] ?? null) ? $context['metadata'] : []; - $metadata = array_merge($metadata, [ - 'finding_id' => (int) $finding->getKey(), - 'before_status' => $before['status'] ?? null, - 'after_status' => $finding->status, - 'before' => $before, - 'after' => $this->auditSnapshot($finding), - ]); - - $this->auditLogger->log( - tenant: $tenant, - action: $action, - actorId: (int) $actor->getKey(), - actorEmail: $actor->email, - actorName: $actor->name, - resourceType: 'finding', - resourceId: (string) $finding->getKey(), - context: ['metadata' => $metadata], - ); - - return $finding; + return $resolvedFinding->refresh(); } /** @@ -377,4 +487,36 @@ private function auditSnapshot(Finding $finding): array 'closed_by_user_id' => $finding->closed_by_user_id, ]; } + + /** + * @param array $before + * @param array $after + * @param array $metadata + */ + private function dedupeKey( + string|AuditActionId $action, + Finding $finding, + array $before, + array $after, + array $metadata, + ?User $actor, + ?AuditActorType $actorType = null, + ): string { + $payload = [ + 'action' => $action instanceof AuditActionId ? $action->value : $action, + 'finding_id' => (int) $finding->getKey(), + 'actor_id' => $actor?->getKey() !== null ? (int) $actor->getKey() : null, + 'actor_type' => $actorType?->value, + 'before' => $before, + 'after' => $after, + 'assignee_user_id' => $metadata['assignee_user_id'] ?? null, + 'owner_user_id' => $metadata['owner_user_id'] ?? null, + 'resolved_reason' => $metadata['resolved_reason'] ?? null, + 'closed_reason' => $metadata['closed_reason'] ?? null, + ]; + + $encoded = json_encode($payload); + + return hash('sha256', is_string($encoded) ? $encoded : serialize($payload)); + } } diff --git a/app/Services/Intune/AuditLogger.php b/app/Services/Intune/AuditLogger.php index 87af6e4..3a6ff44 100644 --- a/app/Services/Intune/AuditLogger.php +++ b/app/Services/Intune/AuditLogger.php @@ -36,6 +36,14 @@ public function log( ): \App\Models\AuditLog { $workspaceId = is_numeric($tenant->workspace_id) ? (int) $tenant->workspace_id : null; + $metadata = is_array($context['metadata'] ?? null) ? $context['metadata'] : []; + + if ($actorType instanceof AuditActorType) { + $metadata['_actor_type'] = $actorType->value; + } + + $context['metadata'] = $metadata; + if ($workspaceId === null) { throw new InvalidArgumentException('Tenant-scoped audit events require tenant workspace_id.'); } @@ -58,7 +66,12 @@ public function log( ), outcome: $status, recordedAt: CarbonImmutable::now(), - summary: $summary, + summary: $summary ?? AuditActionId::summaryFor( + action: $action, + targetLabel: $targetLabel, + targetType: $resourceType, + context: $metadata, + ), operationRunId: $operationRunId, ); } diff --git a/app/Services/PermissionPosture/PermissionPostureFindingGenerator.php b/app/Services/PermissionPosture/PermissionPostureFindingGenerator.php index c69f6e0..c18a9dc 100644 --- a/app/Services/PermissionPosture/PermissionPostureFindingGenerator.php +++ b/app/Services/PermissionPosture/PermissionPostureFindingGenerator.php @@ -10,6 +10,7 @@ use App\Models\StoredReport; use App\Models\Tenant; use App\Services\Findings\FindingSlaPolicy; +use App\Services\Findings\FindingWorkflowService; use Carbon\CarbonImmutable; /** @@ -21,6 +22,7 @@ final class PermissionPostureFindingGenerator implements FindingGeneratorContrac public function __construct( private readonly PostureScoreCalculator $scoreCalculator, private readonly FindingSlaPolicy $slaPolicy, + private readonly ?FindingWorkflowService $findingWorkflowService = null, ) {} /** @@ -146,22 +148,15 @@ private function handleMissingPermission( $resolvedAt = $finding->resolved_at; if ($resolvedAt === null || $observedAt->greaterThan(CarbonImmutable::instance($resolvedAt))) { - $slaDays = $this->slaPolicy->daysForSeverity($severity, $tenant); - - $finding->forceFill([ - 'status' => Finding::STATUS_REOPENED, - 'reopened_at' => $observedAt, - 'resolved_at' => null, - 'resolved_reason' => null, - 'closed_at' => null, - 'closed_reason' => null, - 'closed_by_user_id' => null, - 'sla_days' => $slaDays, - 'due_at' => $this->slaPolicy->dueAtForSeverity($severity, $tenant, $observedAt), - ]); - $finding->save(); + $this->findingWorkflowService()->reopenBySystem( + finding: $finding, + tenant: $tenant, + reopenedAt: $observedAt, + operationRunId: $operationRun?->getKey(), + ); + return 'reopened'; } } @@ -229,19 +224,16 @@ private function handleErrorPermission( $resolvedAt = $existing->resolved_at; if ($resolvedAt === null || $observedAt->greaterThan(CarbonImmutable::instance($resolvedAt))) { - $slaDays = $this->slaPolicy->daysForSeverity($severity, $tenant); + $existing->save(); - $existing->forceFill([ - 'status' => Finding::STATUS_REOPENED, - 'reopened_at' => $observedAt, - 'resolved_at' => null, - 'resolved_reason' => null, - 'closed_at' => null, - 'closed_reason' => null, - 'closed_by_user_id' => null, - 'sla_days' => $slaDays, - 'due_at' => $this->slaPolicy->dueAtForSeverity($severity, $tenant, $observedAt), - ]); + $this->findingWorkflowService()->reopenBySystem( + finding: $existing, + tenant: $tenant, + reopenedAt: $observedAt, + operationRunId: $operationRun?->getKey(), + ); + + return; } } @@ -287,11 +279,12 @@ private function resolveExistingFinding(Tenant $tenant, string $key, string $rea return false; } - $finding->forceFill([ - 'status' => Finding::STATUS_RESOLVED, - 'resolved_at' => $observedAt, - 'resolved_reason' => $reason, - ])->save(); + $this->findingWorkflowService()->resolveBySystem( + finding: $finding, + tenant: $tenant, + reason: $reason, + resolvedAt: $observedAt, + ); return true; } @@ -320,11 +313,12 @@ private function resolveStaleFindings(Tenant $tenant, array $processedPermission } if ($permissionKey !== null && ! in_array($permissionKey, $processedPermissionKeys, true)) { - $finding->forceFill([ - 'status' => Finding::STATUS_RESOLVED, - 'resolved_at' => $observedAt, - 'resolved_reason' => 'permission_removed_from_registry', - ])->save(); + $this->findingWorkflowService()->resolveBySystem( + finding: $finding, + tenant: $tenant, + reason: 'permission_removed_from_registry', + resolvedAt: $observedAt, + ); $resolved++; } } @@ -473,4 +467,9 @@ private function errorFingerprint(Tenant $tenant, string $permissionKey): string { return substr(hash('sha256', 'permission_posture:'.$tenant->getKey().':'.$permissionKey.':error'), 0, 64); } + + private function findingWorkflowService(): FindingWorkflowService + { + return $this->findingWorkflowService ?? app(FindingWorkflowService::class); + } } diff --git a/app/Support/Audit/AuditActionId.php b/app/Support/Audit/AuditActionId.php index 49d1ea4..b84b338 100644 --- a/app/Support/Audit/AuditActionId.php +++ b/app/Support/Audit/AuditActionId.php @@ -72,6 +72,14 @@ enum AuditActionId: string case BaselineAssignmentUpdated = 'baseline_assignment.updated'; case BaselineAssignmentDeleted = 'baseline_assignment.deleted'; + case FindingTriaged = 'finding.triaged'; + case FindingInProgress = 'finding.in_progress'; + case FindingAssigned = 'finding.assigned'; + case FindingResolved = 'finding.resolved'; + case FindingClosed = 'finding.closed'; + case FindingRiskAccepted = 'finding.risk_accepted'; + case FindingReopened = 'finding.reopened'; + // Workspace selection / switch events (Spec 107). case WorkspaceAutoSelected = 'workspace.auto_selected'; case WorkspaceSelected = 'workspace.selected'; @@ -182,13 +190,13 @@ private static function labels(): array self::BaselineAssignmentDeleted->value => 'Baseline assignment deleted', self::WorkspaceAutoSelected->value => 'Workspace auto-selected', self::WorkspaceSelected->value => 'Workspace selected', - 'finding.triaged' => 'Finding triaged', - 'finding.in_progress' => 'Finding moved to in progress', - 'finding.assigned' => 'Finding assignment updated', - 'finding.resolved' => 'Finding resolved', - 'finding.closed' => 'Finding closed', - 'finding.risk_accepted' => 'Finding risk accepted', - 'finding.reopened' => 'Finding reopened', + self::FindingTriaged->value => 'Finding triaged', + self::FindingInProgress->value => 'Finding moved to in progress', + self::FindingAssigned->value => 'Finding assignment updated', + self::FindingResolved->value => 'Finding resolved', + self::FindingClosed->value => 'Finding closed', + self::FindingRiskAccepted->value => 'Finding risk accepted', + self::FindingReopened->value => 'Finding reopened', 'baseline.capture.started' => 'Baseline capture started', 'baseline.capture.completed' => 'Baseline capture completed', 'baseline.capture.failed' => 'Baseline capture failed', @@ -247,6 +255,13 @@ private static function summaries(): array self::AlertRuleCreated->value => 'Alert rule created', self::AlertRuleUpdated->value => 'Alert rule updated', self::AlertRuleDeleted->value => 'Alert rule deleted', + self::FindingTriaged->value => 'Finding triaged', + self::FindingInProgress->value => 'Finding moved to in progress', + self::FindingAssigned->value => 'Finding assignment updated', + self::FindingResolved->value => 'Finding resolved', + self::FindingClosed->value => 'Finding closed', + self::FindingRiskAccepted->value => 'Finding risk accepted', + self::FindingReopened->value => 'Finding reopened', ]; } diff --git a/app/Support/Audit/AuditContextSanitizer.php b/app/Support/Audit/AuditContextSanitizer.php index 26946bb..fa7856a 100644 --- a/app/Support/Audit/AuditContextSanitizer.php +++ b/app/Support/Audit/AuditContextSanitizer.php @@ -10,6 +10,15 @@ final class AuditContextSanitizer { private const REDACTED = '[REDACTED]'; + /** + * @var array + */ + private const DROPPED_FIELDS = [ + 'evidence_jsonb', + 'raw_evidence', + 'snapshot_payload', + ]; + private const MAX_ITEMS = 50; private const MAX_STRING_LENGTH = 500; @@ -29,6 +38,10 @@ public static function sanitize(mixed $value): mixed break; } + if (is_string($key) && in_array($key, self::DROPPED_FIELDS, true)) { + continue; + } + if (is_string($key) && self::classifier()->protectsField('audit', $key)) { $sanitized[$key] = self::REDACTED; diff --git a/database/factories/FindingFactory.php b/database/factories/FindingFactory.php index 6abf42c..f693e1f 100644 --- a/database/factories/FindingFactory.php +++ b/database/factories/FindingFactory.php @@ -73,6 +73,45 @@ public function permissionPosture(): static ]); } + /** + * State for legacy acknowledged findings. + */ + public function acknowledged(): static + { + return $this->state(fn (array $attributes): array => [ + 'status' => Finding::STATUS_ACKNOWLEDGED, + 'acknowledged_at' => now(), + 'acknowledged_by_user_id' => null, + ]); + } + + /** + * State for triaged findings. + */ + public function triaged(): static + { + return $this->state(fn (array $attributes): array => [ + 'status' => Finding::STATUS_TRIAGED, + 'triaged_at' => now(), + ]); + } + + /** + * State for in-progress findings. + */ + public function inProgress(): static + { + return $this->state(function (array $attributes): array { + $triagedAt = now()->subMinute(); + + return [ + 'status' => Finding::STATUS_IN_PROGRESS, + 'triaged_at' => $attributes['triaged_at'] ?? $triagedAt, + 'in_progress_at' => now(), + ]; + }); + } + /** * State for resolved findings. */ @@ -85,6 +124,46 @@ public function resolved(): static ]); } + /** + * State for reopened findings. + */ + public function reopened(): static + { + return $this->state(fn (array $attributes): array => [ + 'status' => Finding::STATUS_REOPENED, + 'reopened_at' => now(), + 'resolved_at' => null, + 'resolved_reason' => null, + 'closed_at' => null, + 'closed_reason' => null, + 'closed_by_user_id' => null, + ]); + } + + /** + * State for closed findings. + */ + public function closed(): static + { + return $this->state(fn (array $attributes): array => [ + 'status' => Finding::STATUS_CLOSED, + 'closed_at' => now(), + 'closed_reason' => 'duplicate', + ]); + } + + /** + * State for risk accepted findings. + */ + public function riskAccepted(): static + { + return $this->state(fn (array $attributes): array => [ + 'status' => Finding::STATUS_RISK_ACCEPTED, + 'closed_at' => now(), + 'closed_reason' => 'accepted_risk', + ]); + } + /** * State for Entra admin roles findings. */ diff --git a/docs/product/spec-candidates.md b/docs/product/spec-candidates.md index 2922244..21dffb3 100644 --- a/docs/product/spec-candidates.md +++ b/docs/product/spec-candidates.md @@ -5,7 +5,7 @@ # Spec Candidates > > **Flow**: Inbox → Qualified → Planned → Spec created → removed from this file -**Last reviewed**: 2026-03-17 (Enterprise App / SP Governance, SharePoint Sharing Governance, Entra Role Governance, Security Posture Signals added) +**Last reviewed**: 2026-03-18 (Help/guidance capability line refactored into 4 bounded candidates) --- @@ -21,6 +21,9 @@ ## Inbox - Inventory landing page may be redundant — consider pure navigation section - Settings change history → explainable change tracking - Workspace chooser v2: search, sort, favorites, pins, environment badges, last activity +- Workspace-level PII override for review packs (deferred from Spec 109 — controls whether PII is included/redacted in tenant review pack exports at workspace scope) +- CSV export for filtered run metadata (deferred from Spec 114 — allow operators to export filtered operation run lists from the system console as CSV) +- Raw error/context drilldowns for system console (deferred from Spec 114 — in-product drilldown into raw error payloads and execution context for failed/stuck runs in the system console) --- @@ -84,9 +87,28 @@ ### Evidence Domain Foundation - **Problem**: Review pack export (Spec 109) and permission posture reports (104/105) exist as separate output artifacts. There is no first-class evidence domain model that curates, bundles, and tracks these artifacts as a coherent compliance deliverable for external audit submission. - **Why it matters**: Enterprise customers need a single, versioned, auditor-ready package — not a collection of separate exports assembled manually. The gap is not export packaging (Spec 109 handles that); it is the absence of an evidence domain layer that owns curation, completeness tracking, and audit-trail linkage. - **Proposed direction**: Evidence domain model with curated artifact references (review packs, posture reports, findings summaries, baseline governance snapshots). Completeness metadata. Immutable snapshots with generation timestamp and actor. Not a re-implementation of export — a higher-order assembly layer. +- **Explicit non-goals**: Not a presentation or reporting layer — this candidate owns data curation, completeness tracking, artifact storage, and immutable snapshots. Executive summaries, framework-oriented readiness views, management-ready outputs, and stakeholder-facing packaging belong to the Compliance Readiness & Executive Review Packs candidate, which consumes this foundation. Not a replacement for Spec 109's export packaging. Not a generic BI or data warehouse initiative. +- **Boundary with Compliance Readiness**: Evidence Domain Foundation = lower-level data assembly (what artifacts exist, are they complete, are they immutable). Compliance Readiness = upper-level presentation (how to arrange evidence into framework-oriented, stakeholder-facing deliverables). This candidate is a prerequisite; Compliance Readiness is a downstream consumer. - **Dependencies**: Review pack export (109), permission posture (104/105) - **Priority**: high +### Compliance Readiness & Executive Review Packs +- **Type**: feature +- **Source**: roadmap-to-spec coverage audit 2026-03-18, R2 theme completion, product positioning for German midmarket / MSP governance +- **Problem**: TenantPilot is building a strong evidence/data foundation (Evidence Domain Foundation candidate, StoredReports, review pack export via Spec 109, findings, baselines), but there is no product-level capability that assembles this data into management-ready, customer-facing, or auditor-oriented readiness views. Enterprise customers, MSP account managers, and CISOs need structured governance outputs for recurring tenant reviews, audit preparation, and compliance conversations — not raw artifact collections or manual export assembly. The gap is not data availability; it is the absence of a dedicated readiness presentation and packaging layer that turns existing governance evidence into actionable, consumable deliverables. +- **Why it matters**: This is a core product differentiator and revenue-relevant capability for the MSP and German midmarket audience. Without it, TenantPilot remains an operator tool — powerful but invisible to the stakeholders who sign off on governance, approve budgets, and evaluate vendor value. Structured readiness outputs (lightweight BSI/NIS2/CIS-oriented views, executive summaries, customer review packs) make TenantPilot sellable as a governance review platform, not just a backup and configuration tool. This directly strengthens the MSP sales story for quarterly reviews, security health checks, and audit preparation. +- **Proposed direction**: + - A dedicated readiness/review presentation layer that consumes evidence domain artifacts, findings summaries, baseline/drift posture, permission posture signals, and operational health data + - Management-ready output surfaces: executive summary views, customer-facing review dashboards, structured compliance readiness pages oriented toward frameworks such as BSI Grundschutz, NIS2, and CIS — in a lightweight, non-certification sense (governance evidence, not formal compliance claims) + - Exportable review packs that combine multiple evidence dimensions into a single coherent deliverable (PDF or structured export) for external stakeholders + - Tenant-scoped and workspace-scoped views — individual tenant reviews as well as portfolio-level readiness summaries + - Clear separation from the Evidence Domain Foundation: evidence foundation owns curation, completeness tracking, and artifact storage; compliance readiness owns presentation, assembly, and stakeholder-facing output + - Readiness views should be composable: an operator selects which dimensions to include in a review pack (e.g. baseline posture + findings summary + permission evidence + operational health) rather than a monolithic fixed report +- **Explicit non-goals**: Not a formal certification engine — TenantPilot does not certify compliance or issue attestations. Not a legal or compliance advice system. Not a replacement for the Evidence Domain Foundation (which owns the data layer). Not a generic BI dashboard or data warehouse initiative. Not a PDF-only export task — the primary value is the structured readiness view, with export as a secondary delivery mechanism. Not a reimplementation of review pack export (Spec 109 handles CSV/ZIP). Not a customer-facing analytics suite. +- **Boundary with Evidence Domain Foundation**: Evidence Domain Foundation = curation, completeness tracking, artifact storage, immutable snapshots. Compliance Readiness = presentation, assembly, framework-oriented views, stakeholder-facing outputs. Evidence Foundation is a prerequisite; Compliance Readiness is a consumer. +- **Dependencies**: Evidence Domain Foundation (data layer), review pack export (Spec 109), findings workflow (Spec 111), baseline/drift engine (Specs 116–119), permission posture (Specs 104/105), audit log foundation (Spec 134) +- **Priority**: medium (high strategic value, but depends on evidence foundation maturity) + ### Enterprise App / Service Principal Governance - **Type**: feature - **Source**: platform domain coverage planning, governance gap analysis @@ -123,6 +145,63 @@ ### Security Posture Signals Foundation - **Dependencies**: StoredReports/Evidence direction, signal ingestion foundations, reporting/export maturity - **Priority**: medium +### Security Suite Layer — Posture Score, Blast Radius, High-Risk Opt-In Controls +- **Type**: feature +- **Source**: roadmap-to-spec coverage audit 2026-03-18, 0800-future-features brainstorming, roadmap "Security Suite Layer" long-term theme +- **Problem**: TenantPilot's security-related capabilities are growing — findings, baselines, drift detection, permission posture — but they remain siloed as individual data outputs. The Security Posture Signals Foundation candidate addresses lower-level signal ingestion and evidence collection, but there is no product layer that aggregates, interprets, and prioritizes these signals into actionable security posture surfaces for operators. An MSP operator managing twenty tenants cannot currently answer: which tenants have the weakest security posture? Which single misconfiguration has the widest blast radius across users and devices? Which high-risk settings are intentionally enabled vs. accidentally exposed? The gap is not signal availability — it is the absence of a higher-level interpretation and prioritization layer that turns raw posture data into operator-facing security value. +- **Why it matters**: Raw signals and individual findings are necessary but insufficient. Operators, CISOs, and MSP account managers need aggregated, prioritized, and contextualized security views that surface the most consequential risks first. Without this, security-relevant data is scattered across findings tables, drift reports, permission posture views, and evidence exports — forcing operators to mentally assemble a posture picture themselves. A productized security posture layer is the difference between "we collect security data" and "we help you act on the most important risks." This is a strategic differentiator for MSP positioning and enterprise customer conversations where security posture is a recurring review topic. +- **Proposed direction**: + - **Posture scoring or posture rollups**: Tenant-level and optionally workspace-level security posture summaries that aggregate signals from findings, baselines, drift state, permission posture, and (when available) external posture signals into a structured posture indicator. Not a single arbitrary number — a structured rollup showing posture dimensions (configuration compliance, identity hygiene, protection coverage, exposure risk) with clear drill-down paths. The goal is "where should I focus?" not "what is my score?" + - **Blast-radius and impact-oriented interpretation**: For high-severity findings, misconfigurations, or risky conditions, show the scope of impact — how many users, devices, or groups are affected? Which policies target broad populations with permissive or risky settings? Impact context helps operators prioritize consequential risks over technically-severe-but-narrow ones. This is interpretation layered on top of existing assignment and scope data, not a separate data collection effort. + - **High-risk opt-in and guarded enablement surfaces**: Where tenants have intentionally enabled high-risk settings (e.g. broad sharing, disabled MFA for service accounts, permissive conditional access), make these visible as explicit, acknowledged decisions rather than hidden configuration details. Support opt-in acknowledgement patterns where operators confirm that a high-risk condition is intentional versus accidental. This is about operator awareness and explicit decision capture, not about enforcing or blocking configurations. + - **Security prioritization surfaces**: Operator-facing views that rank and filter posture conditions by severity, blast radius, and recency — helping operators focus on the few conditions that matter most rather than reviewing flat lists. Supports "top 5 risks across my portfolio" and "highest-impact unresolved findings" patterns. + - **Tenant-scoped and portfolio-aware**: Security posture is evaluated per tenant; portfolio-level aggregation surfaces which tenants are strongest and weakest for MSP operators managing fleets. Supports fleet-level security posture comparisons and trend tracking over time. +- **Explicit non-goals**: Not a SIEM, SOC, or XDR platform — this is posture interpretation for governance operators, not a security operations center tool. Not a vulnerability management system — TenantPilot does not own vulnerability remediation workflows or patch management. Not a generic security analytics platform or BI dashboard. Not a replacement for Security Posture Signals Foundation — this candidate consumes signals; the foundation candidate collects them. Not a compliance certification engine (Compliance Readiness handles audit-ready reporting). Not a threat detection or incident response system. Not a catch-all security backlog bucket — scope is bounded to posture aggregation, interpretation, prioritization, and guarded-visibility patterns. Not a broad platform hardening initiative — infrastructure, delivery, and application-level hardening are separate candidates. +- **Boundary with Security Posture Signals Foundation**: Security Posture Signals Foundation = signal ingestion, historization, correlation, and evidence-layer representation of security-relevant data (Defender exposure, backup health, etc.). Security Suite Layer = aggregation, interpretation, prioritization, and operator-facing posture value built on top of those signals (and other existing governance data like findings, baselines, and drift). Foundation is the substrate; Suite Layer is the product interpretation. Foundation answers "what signals exist?" Suite Layer answers "what do they mean, and what should I act on first?" +- **Boundary with Compliance Readiness & Executive Review Packs**: Compliance Readiness = framework-oriented, stakeholder-facing reporting and evidence assembly for audit conversations. Security Suite Layer = operator-facing, action-oriented posture interpretation for day-to-day security prioritization. Compliance readiness produces reports; security suite layer guides operational focus. Posture data may feed into compliance readiness outputs, but the two serve different audiences and decision patterns. +- **Boundary with Script & Secrets Governance**: Script & Secrets Governance = lifecycle controls, diff, review, and scanning for high-risk content (scripts, secrets). Security Suite Layer may consume secrets governance findings as posture inputs, but does not own the scanning, diffing, or lifecycle management of scripts and secrets. +- **Boundary with Findings and Baselines**: Findings (Spec 111+) and baselines (Specs 116–119) produce governance data points. Security Suite Layer aggregates and reinterprets those data points through a security-prioritization lens. Findings workflow owns the individual finding lifecycle; security suite layer owns the cross-finding posture picture. +- **Dependencies**: Security Posture Signals Foundation (primary signal source), findings workflow (Spec 111+), baseline/drift engine (Specs 116–119), permission posture (Specs 104/105), RBAC/capability system (066+), audit log foundation (Spec 134) +- **Priority**: medium (high strategic value for MSP positioning and enterprise security conversations, but realistically sequenced after signal foundations and current governance hardening work are stable) + +### Recovery Confidence — Automated Restore Testing & Readiness Reporting +- **Type**: feature +- **Source**: roadmap-to-spec coverage audit 2026-03-18, 0800-future-features brainstorming ("Killer Feature"), product positioning for enterprise trust and MSP differentiation +- **Problem**: TenantPilot has a mature backup and restore pipeline — including restore preview, dry-run, risk checking, and audit logging — but there is no product-level capability that answers the question "how confident are we that restores will actually succeed when needed?" Backup existence proves data is captured; restore execution proves the mechanism works when manually triggered. Neither proves ongoing recoverability. Operators cannot answer: when was recoverability last validated for a given tenant or policy family? Which restore paths have never been exercised? Which tenants have backup coverage but zero restore confidence? What is the overall recovery posture across the portfolio? The gap is not restore capability — it is the absence of a confidence and readiness layer that continuously proves, measures, and reports on recoverability. +- **Why it matters**: Backup without proven recoverability is a false safety net. Enterprise customers, auditors, and MSP account managers increasingly ask not "do you have backups?" but "can you prove you can recover?" Recovery confidence is the difference between a backup tool and a trusted governance platform. It directly strengthens audit conversations (proving restore paths work), MSP differentiation (recovery readiness as a reportable SLA dimension), and operator trust (visibility into which restore paths are validated vs. assumed). This was identified as a "killer feature" in product brainstorming because it shifts TenantPilot from reactive restore capability to proactive recovery assurance — a category few competitors occupy. +- **Proposed direction**: + - **Automated restore confidence checks**: scheduled or operator-triggered restore validation runs that exercise restore paths without modifying the production tenant — leveraging existing dry-run/preview infrastructure, targeted at proving that backed-up configurations can be successfully restored. Confidence checks produce structured results (pass/fail/partial, coverage, blockers) rather than just logs. + - **Recoverability tracking model**: per-tenant, per-policy-family tracking of when each restore path was last validated, what the result was, and which paths remain unexercised. This is the persistent readiness state, not a one-time report. Tracks coverage (which policy families have been validated), freshness (how recently), and result quality (clean pass vs. partial vs. failed). + - **Restore readiness summaries and reporting**: tenant-level and workspace-level views that show recovery posture — coverage gaps, stale validations, unexercised restore paths, confidence scores or readiness indicators. Exportable for audit evidence, customer reviews, and management reporting. Integrates with the evidence/reporting direction as a high-value signal source. + - **Preflight scoring**: before a real restore is needed, operators can see a structured readiness assessment — which policy families are covered by recent successful validation, which have known blockers, which have never been tested. This turns restore from a "hope it works" moment into a predictable, pre-validated operation. + - **Validation evidence trail**: each confidence check produces an immutable evidence artifact — what was tested, when, by whom, what the result was, what blockers were found. This evidence feeds into review packs, audit conversations, and compliance readiness outputs. The validation run itself is an auditable governance event. + - **Tenant-scoped and portfolio-aware**: recovery confidence is evaluated per tenant; portfolio-level aggregation surfaces which tenants have strong vs. weak recovery posture for MSP operators managing fleets. +- **Explicit non-goals**: Not a rewrite or replacement of the restore engine (Spec 011 and related specs handle restore execution; this handles confidence measurement and readiness reporting on top of that foundation). Not a full disaster-recovery orchestration platform or automated failover system. Not a synthetic test lab that provisions isolated test tenants and deploys configurations into them — confidence checks leverage existing dry-run/preview/validation infrastructure, not a separate execution environment. Not a generic backup-health dashboard — backup health (coverage, freshness, size) is a prerequisite signal, not the same problem as restore confidence (proven recoverability). Not a vague "resilience" umbrella — this is specifically about proving and reporting on restore path readiness. Not a replacement for the Evidence Domain Foundation (which owns artifact curation) or Compliance Readiness (which owns presentation assembly) — recovery confidence produces evidence artifacts that those layers consume. +- **Boundary with restore execution (Spec 011, restore pipeline)**: Restore execution = the mechanism to restore configurations from backup to a tenant. Recovery confidence = the layer that exercises, measures, tracks, and reports on whether those mechanisms are ready and reliable. Execution is the tool; confidence is the proof. +- **Boundary with Security Posture Signals Foundation**: Security Posture Signals = ingestion and historization of external posture data (Defender, backup success/failure signals) as evidence inputs. Recovery Confidence = active validation of restore paths and structured readiness reporting. Posture signals may include backup-health inputs; recovery confidence actively exercises restore paths and produces readiness-specific evidence. Backup health signals are passive; restore confidence checks are active. They are complementary: posture signals feed portfolio health views; recovery confidence proves operational readiness. +- **Boundary with Evidence Domain Foundation**: Evidence Foundation = curation, completeness tracking, immutable artifact storage. Recovery Confidence = produces validation evidence artifacts (confidence check results) that Evidence Foundation may curate and bundle. Recovery confidence is a producer; evidence foundation is a curator/consumer. +- **Boundary with MSP Portfolio Dashboard**: Portfolio Dashboard = fleet-level health aggregation and SLA reporting. Recovery confidence signals (per-tenant readiness posture) are a high-value input to the portfolio dashboard, not a replacement for it. The dashboard consumes; this candidate produces the recovery-specific signal. +- **Dependencies**: Restore pipeline stable (Spec 011 and follow-ups), backup infrastructure mature, dry-run/preview infrastructure (restore preview), audit log foundation (Spec 134), RBAC/capability system (066+), evidence/reporting direction for downstream consumption +- **Priority**: medium (high strategic value and strong product differentiation potential, but depends on restore pipeline maturity and is realistically sequenced after current hardening work) + +### MSP Multi-Tenant Portfolio Dashboard & SLA Reporting +- **Type**: feature +- **Source**: roadmap-to-spec coverage audit 2026-03-18, 0800-future-features brainstorming (pillar #1 — MSP Portfolio & Operations), product positioning for MSP portfolio owners +- **Problem**: TenantPilot provides strong per-tenant governance, monitoring, and operational surfaces, but MSP operators and portfolio owners managing 10–100+ tenants across workspaces have no fleet-level view that answers "how is my portfolio doing?" There is no cross-tenant health summary, no SLA/compliance risk overview, no portfolio-level operational monitoring, and no structured reporting surface that supports recurring customer portfolio reviews. Operators must navigate tenant by tenant to assemble a portfolio picture, which does not scale and prevents proactive governance. +- **Why it matters**: MSP portfolio visibility is the #1 brainstorming priority and a core product differentiator. Without it, TenantPilot serves individual tenant management well but cannot position itself as the operational cockpit for MSP businesses. Portfolio-level health, SLA tracking, compliance risk summaries, and cross-tenant operational monitoring are the capabilities that justify platform-level pricing and recurring MSP engagement. This is the difference between a per-tenant tool and an MSP operations platform. +- **Proposed direction**: + - Workspace-level portfolio dashboard: aggregated health, governance, and operational status across all managed tenants in a workspace + - Key portfolio signals: backup health (last successful backup age, coverage), sync health (last successful sync, staleness), drift/findings posture (open findings count, severity distribution, trend), operational health (recent failures, stuck runs, throttling indicators), provider connection status (consent/verification posture across fleet) + - SLA/compliance risk summary views: which tenants are below operational health thresholds, which tenants have governance gaps, which tenants need attention — sortable, filterable, visually prioritized + - Cross-tenant operational monitoring: portfolio-level view of recent operation runs, failure clustering, and common error patterns across tenants + - Structured portfolio reporting: exportable portfolio health summaries for MSP-internal use, customer-facing SLA reports, and recurring review preparation + - Workspace-scoped, RBAC-gated: portfolio views respect workspace membership and capability authorization +- **Explicit non-goals**: Not a replacement for per-tenant dashboards or detail views (those remain the primary tenant-level surfaces). Not a generic BI/data warehouse initiative or a drag-and-drop report builder. Not a customer-facing analytics suite — this is an operator/MSP-internal tool. Not a cross-tenant compare/diff/promotion surface (that is the Cross-Tenant Compare & Promotion candidate). Not a system-console-level platform triage view (that is the System Console Multi-Workspace Operator UX candidate). Not a replacement for alerting (Specs 099/100 handle event-driven notifications; this is a review/monitoring surface). +- **Boundary with Cross-Tenant Compare & Promotion**: Portfolio Dashboard = fleet-level monitoring, health aggregation, SLA reporting, operational overview. Cross-Tenant Compare = policy-level diff, staging-to-production promotion, configuration comparison. They share the multi-tenant dimension but solve fundamentally different problems. +- **Boundary with System Console Multi-Workspace Operator UX**: Portfolio Dashboard = workspace-scoped MSP operator view, health/SLA/governance focus. System Console = platform-level triage, cross-workspace operator tooling, infrastructure focus. Different audiences, different panels. +- **Dependencies**: Per-tenant operational health signals (backup, sync, drift, findings, provider connection status), workspace model, tenant inventory, alerting foundations (Specs 099/100), RBAC/capability system (066+) +- **Priority**: medium (high strategic value, significant data aggregation effort; depends on per-tenant signal maturity) + ### Policy Lifecycle / Ghost Policies (Spec 900 refresh) - **Type**: feature - **Source**: Spec 900 draft (2025-12-22), HANDOVER risk #9 @@ -132,6 +211,23 @@ ### Policy Lifecycle / Ghost Policies (Spec 900 refresh) - **Dependencies**: Inventory sync stable - **Priority**: medium +### Standardization & Policy Quality — Linting, Company Standards, Hygiene +- **Type**: feature +- **Source**: roadmap-to-spec coverage audit 2026-03-18, 0800-future-features brainstorming (pillar #3 — Standardization & Policy Quality / "Intune Linting") +- **Problem**: TenantPilot captures, versions, and governs Intune policy configurations, but provides no capability to evaluate whether those configurations meet quality, consistency, or organizational standards. Operators cannot answer questions like: "Do all policies follow our naming convention?", "Are there duplicate or near-duplicate policies?", "Which policies have no assignments?", "Are scope tags applied consistently?", "Does this tenant meet our company's minimum configuration standard?" Today, quality and hygiene assessment is manual, tenant-by-tenant, and invisible to governance workflows. +- **Why it matters**: Configuration quality is a distinct governance dimension from baseline drift and compliance findings. Drift detection answers "has something changed?"; standardization answers "is it correct and well-structured?" Enterprise customers and MSPs need both. Policy linting, hygiene checks, and company standards create a repeatable quality layer that reduces configuration debt, catches structural problems early, and supports standardization across managed tenants. This is the #3 brainstorming priority and a natural complement to the existing governance stack. +- **Proposed direction**: + - **Policy linting / quality checks**: rule-based evaluation of policy configurations against defined quality criteria — naming conventions, scope tag requirements, assignment presence, setting completeness, structural validity. Rules should be composable and extensible per workspace or tenant. + - **Company standards as reusable reference packs**: operators or MSPs define their own configuration standards ("Company Standard 2026") as reference expectations that policies can be evaluated against. Distinct from Microsoft baselines — these are organization-defined, not vendor-defined. A standard pack is a set of expected configuration postures, not a deployable template. + - **Hygiene checks**: automated detection of structural problems — duplicate or near-duplicate policies, unassigned policies, orphaned scope tags or filters, policies with no settings or empty payloads, stale policies not updated in extended periods, inconsistent naming patterns across policy families. + - **Quality findings integration**: hygiene and linting results should produce structured findings or quality signals that integrate with the existing findings workflow, not a separate parallel reporting system. + - **Tenant-scoped and portfolio-aware**: quality evaluation runs per tenant; portfolio views can aggregate quality posture across tenants for MSP operators. +- **Explicit non-goals**: Not a full compliance framework or certification engine (compliance readiness is a separate candidate). Not a generic recommendation engine or AI assistant. Not a replacement for baseline/drift detection (which answers "has it changed from a known-good state?" — standardization answers "is it well-structured and consistent?"). Not a policy deployment or remediation engine — this is evaluation and visibility, not automated correction. Not a replacement for the existing findings workflow — quality signals should flow into findings, not bypass them. +- **Boundary with baseline/drift engine**: Baselines compare current state against a snapshot of known-good state. Standardization evaluates current state against quality rules and organizational expectations. A policy can be drift-free (unchanged from baseline) but still fail quality checks (bad naming, missing assignments, no scope tags). These are complementary, not overlapping. +- **Boundary with Policy Setting Explorer**: Policy Setting Explorer = reverse lookup ("where is this setting defined?"). Standardization = quality evaluation ("is this policy well-structured and consistent?"). Different questions, different surfaces. +- **Dependencies**: Inventory sync stable, policy versioning, tenant context model, findings workflow (Spec 111) for quality findings integration, RBAC/capability system (066+) +- **Priority**: medium (high strategic value, incremental delivery possible starting with high-value hygiene checks) + ### Schema-driven Secret Classification - **Type**: hardening - **Source**: Spec 120 deferred follow-up @@ -141,6 +237,26 @@ ### Schema-driven Secret Classification - **Dependencies**: Secret redaction (120) stable, registry completeness (095) - **Priority**: medium +### Script & Secrets Governance — Diff, Review, Scanning, and Lifecycle Controls for High-Risk Content +- **Type**: feature +- **Source**: roadmap-to-spec coverage audit 2026-03-18, 0800-future-features brainstorming (Script & Secrets Governance pillar), platform hardening direction +- **Problem**: TenantPilot governs a wide range of Intune policy configurations, but a subset of these configurations carries disproportionate operational risk: PowerShell remediation scripts, detection scripts, custom compliance scripts, proactive remediations, and policy artifacts that embed or reference secrets (pre-shared keys, certificate data, credentials, API tokens). These artifacts are fundamentally different from declarative policy settings — they contain executable logic or sensitive material where a single change can have outsized blast radius, and where silent or unreviewed modification creates real security and operational exposure. Today, TenantPilot treats script-bearing and secret-sensitive artifacts with the same governance depth as any other policy: they are versioned and backed up, but there is no dedicated diff/review surface for script content, no approval or guarded workflow for high-risk script changes, no scanning or policy checks for obviously unsafe secret-handling patterns, and no structured visibility into which configurations carry elevated risk because they contain executable or secret-sensitive content. +- **Why it matters**: Script and secret governance is a distinct risk dimension that cuts across policy families. A naming convention violation in a device configuration policy is a hygiene problem; an unreviewed script change in a remediation policy is a potential security incident. Enterprise customers and MSP operators need to trust that high-risk content changes are visible, reviewable, and governable — not just captured as another version snapshot. This capability strengthens audit conversations (proving that script changes are reviewed), operator safety (preventing silent high-risk modifications from going unnoticed), and platform credibility (demonstrating that TenantPilot understands which parts of Intune configuration carry elevated risk). Without it, backup and versioning give a false sense of governance completeness — the most dangerous artifacts receive the same governance treatment as the least dangerous ones. +- **Proposed direction**: + - **Script-aware diff and review surfaces**: dedicated diff views for script-bearing policy artifacts that render script content changes readably — not just JSON diff of the enclosing policy payload, but structured presentation of the script text itself (before/after, syntax-highlighted where practical, change summary). These surfaces make script changes reviewable by operators rather than buried in raw payload diffs. + - **Risk classification for script/secret-bearing artifacts**: extend the inventory or governance metadata so that policy artifacts containing scripts or secret-sensitive fields are identifiable as elevated-risk items. This classification enables filtering, alerting, and governance workflow differentiation — operators can see "which of my policies are script-bearing?" or "which versions changed script content?" without manually inspecting payloads. + - **Guarded change workflows for high-risk content**: optional governance gates for script-bearing or secret-sensitive changes — such as requiring explicit acknowledgment, capability-gated approval, or elevated audit logging when a versioned change involves script content or secret-sensitive fields. These are governance-layer controls, not Intune-side mutation blocks (TenantPilot observes configuration, it does not control the Intune mutation path). The gates apply to how TenantPilot classifies and routes detected changes. + - **Scanning and policy checks for secret-handling patterns**: lightweight rule-based checks that flag obviously unsafe patterns in script or configuration content — hardcoded credentials, plaintext secrets, overly broad credential scopes, known-bad patterns. Not a full SAST engine — focused, high-signal checks that catch the most common and most dangerous mistakes. Results integrate with the findings workflow as governance signals, not a parallel detection system. + - **Rollback and auditability expectations**: script and secret-sensitive changes should have clear rollback visibility (which version introduced the script change, who triggered restore, what was the before/after state). Audit trail expectations should be elevated for this content class — change, review, approval, and rollback events should be distinctly traceable in audit logs. + - **Operator visibility into script/secret risk posture**: tenant-level and portfolio-level views that surface which tenants have unreviewed script changes, which script-bearing policies have never been reviewed, and where secret-handling patterns have been flagged. This is the governance visibility layer, not a generic dashboard initiative. +- **Explicit non-goals**: Not a replacement for external secret vault or key management systems (Azure Key Vault, HashiCorp Vault, etc.) — TenantPilot does not store or manage secrets as a vault; it governs configurations that may contain or reference sensitive material. Not a full code-signing or binary-signing platform — governance focus is on reviewability and risk visibility, not cryptographic attestation. Not a SIEM, DLP, or broad security-monitoring system — this is governance of specific high-risk content classes within the existing policy governance architecture, not a generic security operations capability. Not a catch-all bucket for every security topic — this is bounded to script-bearing and secret-sensitive configuration artifacts. Not a replacement for the baseline/drift engine (which detects *any* configuration change) — this adds risk-aware governance specifically for the content classes where changes carry elevated operational risk. Not a policy deployment or remediation engine — this is detection, review, and governance, not automated correction. Not a full static analysis (SAST) engine for arbitrary scripts. +- **Boundary with Schema-driven Secret Classification**: Schema-driven Secret Classification = improving the *redaction mechanism's* reliability by using schema metadata to classify which fields contain secrets (a backend classification improvement for the existing redaction pipeline). Script & Secrets Governance = lifecycle governance around script-bearing and secret-sensitive *artifacts* — diff, review, scanning, approval workflows, risk visibility. Classification makes redaction more accurate; governance adds reviewability and lifecycle controls. Schema-driven classification may inform governance risk tagging (which fields are secret-sensitive), but the problems and deliverables are distinct. +- **Boundary with Standardization & Policy Quality**: Standardization = evaluating whether policies are well-structured, consistently named, properly assigned, and hygienically maintained. Script & Secrets Governance = evaluating whether high-risk content (scripts, secrets) is reviewed, safe, and governable. A policy can pass all quality checks (good naming, proper assignments, scope tags) but still have an unreviewed script change or a hardcoded credential. These are complementary governance dimensions, not overlapping. +- **Boundary with Security Posture Signals Foundation**: Security Posture Signals = ingesting and historizing external posture data (Defender, backup health) as evidence inputs for reporting. Script & Secrets Governance = internal governance of the product's own high-risk configuration content. Different data sources, different governance problems. Posture signals are external evidence; script governance is internal safety. +- **Boundary with baseline/drift engine (Specs 116–119)**: Drift detection = detecting that *something changed*. Script & Secrets Governance = applying differentiated governance treatment *because of what changed* (script content, secret-sensitive fields). Drift is content-agnostic detection; script governance is risk-aware response. They compose: drift detection finds the change, script governance classifies and routes it based on risk. +- **Dependencies**: Inventory sync stable, policy versioning and snapshot infrastructure, secret redaction (Spec 120) stable, findings workflow (Spec 111) for governance signal integration, audit log foundation (Spec 134), RBAC/capability system (066+), GraphContractRegistry maturity for field-level metadata +- **Priority**: medium (high security-governance value and clear product differentiation, but realistically sequenced after current hardening work and dependent on inventory/versioning/findings maturity) + ### Cross-Tenant Compare & Promotion - **Type**: feature - **Source**: Spec 043 draft, 0800-future-features @@ -148,6 +264,7 @@ ### Cross-Tenant Compare & Promotion - **Why it matters**: Core MSP/enterprise workflow. Identified as top revenue lever in brainstorming. - **Proposed direction**: Compare/diff UI, group/scope-tag mapping, promotion plan (preview → dry-run → cutover → verify) - **Dependencies**: Inventory sync, backup/restore mature +- **Spec 043 relationship**: Spec 043 (`specs/043-cross-tenant-compare-and-promotion/spec.md`) is a lightweight draft (scenarios + FRs, created 2026-01-07, status: Draft) that covers the core compare/promotion contract. This candidate captures the expanded strategic direction and scope refinements accumulated since the draft was written. When this candidate is promoted, it should refresh and supersede the existing Spec 043 draft rather than creating a parallel spec. - **Priority**: medium (high value, high effort) ### System Console Scope Hardening @@ -423,16 +540,70 @@ ### Policy Setting Explorer — Reverse Lookup for Tenant Configuration ### Help Center / Documentation Surface - **Type**: feature - **Source**: product planning, operator support friction analysis -- **Problem**: TenantPilot lacks a first-class in-product knowledge surface for operators. As the platform grows in governance depth, operators need contextual guidance, workflow explanations, role/capability explanations, remediation help, and product documentation without leaving the admin experience. Today, knowledge is fragmented across specs, internal docs, and implicit operator expectations. -- **Why it matters**: Reduces support friction, improves operator onboarding, enables self-service resolution, and makes advanced governance features more understandable and adoptable. Provides a canonical product-help layer distinct from audit/evidence/reporting artifacts. This is a product maturity and support-efficiency capability, not a content management system. +- **Problem**: TenantPilot lacks a central, searchable, in-product knowledge surface for operators. Product documentation, glossary content, workflow walkthroughs, and conceptual explanations are fragmented across specs, internal docs, and implicit operator expectations. Operators who need to understand a concept, look up a term, or read a walkthrough must leave the admin experience entirely. +- **Why it matters**: A canonical in-product documentation surface reduces support friction, enables self-service resolution, and makes advanced governance features more understandable and adoptable. This is the product's central knowledge layer — distinct from contextual inline help (separate candidate), page-level instructional panels (separate candidate), and onboarding next-step guidance (separate candidate). It is also distinct from audit/evidence/reporting artifacts. This is a product maturity and support-efficiency capability, not a content management system. - **Proposed direction**: - - Markdown-based documentation stored in-repo, rendered inside the Filament admin product - - Global documentation search - - Contextual help entry points on relevant resources/pages (modal / slideover preview where appropriate) + - Markdown-based documentation stored in-repo, rendered inside the Filament admin panel as a dedicated help center surface + - Global documentation search across all help articles + - Structured article hierarchy: conceptual docs, walkthroughs, glossary, role/capability explanations, domain-specific governance guidance - Clear separation between product help/knowledge and audit/report/evidence exports - Workspace/tenant context awareness only where helpful for navigation, not to turn docs into tenant data -- **Explicit non-goals**: Not a customer support ticket system. Not an audit pack feature. Not a generic CMS. Not a replacement for external knowledge bases if those exist separately. + - Native first-party rendering approach — no external CMS dependency, no third-party documentation platform required +- **Explicit non-goals**: Not a customer support ticket system. Not an audit pack feature. Not a generic CMS. Not a replacement for external knowledge bases if those exist separately. Not the delivery mechanism for contextual inline help or page-level guidance panels — those are separate capabilities that may link into this surface but are independently spec-able. Not a video-first help strategy. Not a forced guided tour infrastructure. v1 does not include a `/system` help-management UI. Content authoring, editing, and lifecycle remain code/repo-driven. Any future `/system` surface is limited to governance/observability concerns, not CMS behavior. - **Dependencies**: Filament panel infrastructure, existing navigation/information architecture +- **Related candidates**: Contextual Help and Inline Guidance Framework, Page-Level Guidance Patterns, Onboarding Guidance and Next-Step Surfaces, Documentation Generation Pipeline and Editorial Workflow +- **Priority**: medium + +### Contextual Help and Inline Guidance Framework +- **Type**: feature +- **Source**: product planning, operator support friction analysis, governance UX complexity +- **Problem**: As TenantPilot's governance surface area grows — findings workflows, RBAC capabilities, provider connection lifecycle, restore risk assessment, compliance baselines — operators encounter complex concepts and multi-step workflows where the purpose, consequences, or next steps are not self-evident from the UI alone. There is no standard mechanism for surfacing short, targeted, inline explanations at the point of need. Operators must either already know the domain or leave the product to find help. +- **Why it matters**: Contextual help reduces cognitive load on high-complexity governance surfaces, decreases support escalations, and makes advanced features accessible to operators who are not domain experts. This is distinct from the central documentation surface (which is a browsable knowledge base) and from page-level instructional panels (which provide section-level orientation). Contextual help is the layer that answers "what does this mean?" and "what happens if I do this?" at the point of interaction. +- **Proposed direction**: + - Standardized inline help entry points (e.g. `?` icon actions, help affordances on action buttons, info popovers on complex form fields) integrated with Filament's action and component patterns + - Short-form help content rendered in slideover or modal surfaces — not tooltips, not a tooltip explosion + - Help content is text-first: concise explanation of what the feature does, why it matters, and what happens next + - Content sourced from markdown or structured help definitions stored in-repo, maintainable alongside code + - Optional deep-link from contextual help into the central Help Center for extended reading + - Clean integration with Filament v5 actions, render hooks, and CSS hook classes — no internal view publishing + - First-party, native approach — no third-party guided-tour or walkthrough library dependency +- **Explicit non-goals**: Not a tooltip explosion across every field. Not a video-first help strategy. Not a forced guided tour or product walkthrough system. Not a replacement for the central Help Center documentation surface. Not intercom-style chat or conversational help. Not an onboarding checklist (separate candidate). Not a generic CMS feature. v1 does not include a `/system` help-management UI. Content authoring, editing, and lifecycle remain code/repo-driven. Any future `/system` surface is limited to governance/observability concerns, not CMS behavior. +- **Dependencies**: Filament panel infrastructure, action system (Filament v5 actions), Help Center / Documentation Surface (for deep-link target, but functionally independent) +- **Related candidates**: Help Center / Documentation Surface, Page-Level Guidance Patterns, Admin Visual Language Canon +- **Priority**: medium + +### Page-Level Guidance Patterns +- **Type**: feature +- **Source**: product planning, governance UX complexity analysis +- **Problem**: Several TenantPilot admin pages serve governance-heavy, interpretation-heavy, or consequence-heavy functions — findings review, RBAC capability management, provider connection lifecycle, restore dry-run results, compliance baseline comparison, drift analysis. These pages present data and actions that require domain context to interpret correctly, but operators arrive without orientation. There is no standard pattern for providing page-level introductory guidance, "learn more" affordances, or instructional panels that help operators understand what a page shows, why it matters, and how to use it effectively. +- **Why it matters**: Page-level guidance reduces operator confusion on the product's most complex and highest-stakes surfaces. It bridges the gap between contextual inline help (which explains individual concepts) and the central documentation surface (which is a browsable reference). It provides section-level orientation without requiring operators to leave the page or consult external resources. For governance and compliance surfaces, clear page-level framing increases operator confidence and reduces misinterpretation of presented data. +- **Proposed direction**: + - Standardized page-level instructional panel pattern: optional intro/help section at the top of qualifying pages, rendered via Filament render hooks or section components + - "Learn more" affordances linking from the page context into the central Help Center for extended documentation + - Pattern supports dismissibility and operator preference (e.g. collapsible, "don't show again" per-page or per-user) + - Content stored as markdown or structured definitions in-repo, not hardcoded in Blade templates + - Visual pattern consistent with the Admin Visual Language Canon direction — does not introduce a new visual system + - Qualifying page criteria: governance-heavy pages, consequence-heavy action pages, interpretation-heavy data review pages. Not every list page or simple CRUD form. +- **Explicit non-goals**: Not a banner/notification system. Not contextual inline help for individual fields or actions (separate candidate). Not onboarding step-by-step flow (separate candidate). Not a forced walkthrough or modal tutorial. Not applied universally to every admin page — only where governance complexity warrants it. v1 does not include a `/system` help-management UI. Content definitions remain code/repo-driven. Any future `/system` surface is limited to governance/observability concerns, not CMS behavior. +- **Dependencies**: Filament panel infrastructure, render hooks, Admin Visual Language Canon (for visual consistency), Help Center / Documentation Surface (for "learn more" link targets) +- **Related candidates**: Help Center / Documentation Surface, Contextual Help and Inline Guidance Framework, Admin Visual Language Canon +- **Priority**: low + +### Onboarding Guidance and Next-Step Surfaces +- **Type**: feature +- **Source**: product planning, operator time-to-value analysis +- **Problem**: New operators and new tenants arrive in TenantPilot without structured product-level guidance about what to do next. The managed-tenant onboarding wizard handles the technical setup flow (consent, connection, initial sync), but after onboarding completes — or for operators exploring governance features for the first time — there is no product-level guidance surface that helps operators understand what is available, what to configure next, or how to reach productive use of governance workflows. Empty states, action labels, and page descriptions do not consistently communicate next steps or paths to value. +- **Why it matters**: Reduces time-to-value for new operators without introducing a forced product tour. Improves discoverability of governance features (findings, baselines, RBAC, drift, reporting) by surfacing actionable next steps rather than relying on operators to discover capabilities through sidebar exploration. Improves the quality of empty states and first-use experiences across the product. This is a product adoption and operator efficiency capability, not a marketing or onboarding conversion feature. +- **Proposed direction**: + - Smart empty states with actionable next-step guidance on high-value surfaces (e.g. findings list with no findings yet, backup history with no backups, dashboard with no synced tenants) + - Optional post-onboarding next-step surface or checklist-style orientation (not a forced wizard — an opt-in guidance panel or dedicated page) + - Descriptive action labels and page descriptions that reduce confusion about what features do, especially on first encounter + - Progress-aware guidance: surfaces that adapt based on what the operator has already configured (e.g. "sync complete — next: review your compliance baseline" vs. "no sync yet — start here") + - Lightweight, maintainable: guidance content stored as structured definitions in-repo, not hardcoded across scattered Blade templates + - Native Filament integration via empty states, info sections, and render hooks +- **Explicit non-goals**: Not a forced guided tour or product walkthrough. Not a video-first onboarding strategy. Not a step-by-step wizard for every product feature. Not a tooltip explosion. Not a replacement for the managed-tenant onboarding wizard (which handles technical setup). Not a marketing/conversion funnel. Not a third-party onboarding library integration (e.g. no Appcues, no Intercom tours). Not the same as contextual inline help (separate candidate) or page-level instructional panels (separate candidate). v1 does not include a `/system` help-management UI. Guidance definitions remain code/repo-driven. Any future `/system` surface is limited to governance/observability concerns, not CMS behavior. +- **Dependencies**: Filament panel infrastructure, empty-state patterns, Admin Visual Language Canon (for visual consistency), managed-tenant onboarding wizard (Spec 001 and follow-ups — this candidate covers life after technical onboarding) +- **Related candidates**: Help Center / Documentation Surface, Contextual Help and Inline Guidance Framework, Page-Level Guidance Patterns - **Priority**: medium ### Documentation Generation Pipeline and Editorial Workflow @@ -465,6 +636,23 @@ ### Drift Notifications Settings Surface - **Dependencies**: Alerting v1 direction, drift detection foundation (Spec 044), tenant/workspace context model - **Priority**: medium +### Drift Change Governance — Approval Workflows, Freeze Windows, Tamper Detection +- **Type**: feature +- **Source**: roadmap-to-spec coverage audit 2026-03-18, 0800-future-features brainstorming (pillar #2 — Drift & Change Governance / "Zahlhebel #1") +- **Problem**: TenantPilot's drift/baseline engine (Specs 116–119) detects configuration changes and surfaces them as findings. But detection alone does not govern _when_ and _how_ changes are allowed. There is no approval workflow for high-risk configuration changes, no protected time windows during which changes are blocked or escalated, and no tamper detection that distinguishes authorized changes from suspicious or unauthorized modifications. The drift engine answers "what changed?"; this capability answers "was the change allowed, and should it have happened now?" +- **Why it matters**: Change governance is the #2 revenue lever identified in product brainstorming. Enterprise customers and MSPs managing production tenants need controlled change processes — especially for high-risk policy families (endpoint security, compliance, conditional access). Without approval workflows and freeze windows, every detected drift is reactive: something already happened, the only question is whether to accept or revert it. Adding a governance layer turns drift from a detection feature into a change-control platform — the core value proposition for regulated environments, MSP SLA enforcement, and enterprise change management. +- **Proposed direction**: + - **Change approval workflows**: define which policy families, change types, or severity levels require explicit approval before being accepted in the governance record. Approval can be gated by capability (e.g. `drift.change.approve`), with structured approval/rejection, justification, actor, and timestamp. Approval workflows are governance-layer constructs — they do not block changes in Intune (TenantPilot does not control the mutation path in the source tenant), but they govern how TenantPilot treats detected changes: approved (accepted into baseline), rejected (escalated as governance violation), pending (awaiting review). + - **Protected / frozen windows**: workspace- or tenant-level configuration of time periods during which detected changes are automatically escalated or flagged (e.g. "no high-risk changes accepted during weekend maintenance windows" or "freeze all baseline-covered policy families during audit preparation"). Freeze windows do not prevent changes in Intune — they elevate the governance response when changes are detected during protected periods. + - **Tamper / suspicious change detection**: heuristic or rule-based identification of changes that look unauthorized or anomalous — changes outside business hours, changes by unexpected actors, bulk changes across multiple policy families, changes to policies that haven't been modified in extended periods. These produce elevated findings or alerts, not automated blocks. + - **Integration with existing drift/findings pipeline**: change governance operates on top of the drift detection pipeline. It consumes drift findings and applies governance rules (approval requirements, freeze window evaluation, tamper heuristics) to classify and route them. It does not replace the detection engine. +- **Explicit non-goals**: Not a rewrite or replacement of the drift/baseline engine (Specs 116–119 handle detection; this handles governance response). Not a DevOps CI/CD pipeline or deployment system — TenantPilot does not deploy to Intune tenants. Not a mutation-path control mechanism — TenantPilot cannot prevent changes in the source tenant; it can only govern how detected changes are classified and acted upon. Not a generic security hardening bucket. Not a real-time blocking proxy between operators and Intune. +- **Boundary with drift/baseline engine (Specs 116–119)**: Drift engine = detect changes, capture content, produce findings. Change governance = classify detected changes against approval/freeze/tamper rules, route them through governance workflows. The engine feeds this layer; this layer does not modify detection behavior. +- **Boundary with Drift Notifications Settings Surface**: Drift notifications = operator-facing configuration of alert delivery for drift events. Change governance = approval workflows, freeze windows, tamper classification. Notifications deliver signals; governance adds workflow, classification, and control semantics. +- **Boundary with Exception / Risk-Acceptance Workflow for Findings**: Risk acceptance = post-hoc acknowledgment that a known finding is intentionally accepted. Change governance = pre/peri-change classification of whether a detected change was authorized and occurred under acceptable conditions. Different lifecycle positions, complementary capabilities. +- **Dependencies**: Drift/baseline engine (Specs 116–119) fully shipped, findings workflow (Spec 111), alerting foundations (Specs 099/100), audit log foundation (Spec 134), RBAC/capability system (066+) +- **Priority**: medium (high strategic and revenue value, but depends on drift engine and findings workflow maturity) + ### User Invitations and Directory-based User Selection - **Type**: feature - **Source**: product planning, access-management UX analysis @@ -621,6 +809,25 @@ ### Admin Visual Language Canon — First-Party UI Convention Codification and D - **Risks if ignored**: Slow visual drift across surfaces, increasing review friction for new surfaces, divergent local conventions that become expensive to reconcile, weakened enterprise UX credibility as surface count grows, and higher cost of eventual systematic alignment. - **Priority**: medium +### Infrastructure & Platform Debt — CI, Static Analysis, Test Parity, Release Process +- **Type**: hardening +- **Source**: roadmap-to-spec coverage audit 2026-03-18, Infrastructure & Platform Debt table in `docs/product/roadmap.md` +- **Problem**: TenantPilot's product architecture and governance domain have matured significantly, but the surrounding delivery infrastructure has not kept pace. The roadmap acknowledges six open infrastructure debt items — no CI pipeline, no static analysis (PHPStan/Larastan), SQLite-for-tests vs. PostgreSQL-in-production schema drift risk, no `.env.example`, no formal release process, and Dokploy configuration external to the repo — but none of these has a planning home or a specifiable path to resolution. Individually, each is a small-to-medium task. Collectively, they represent a real delivery confidence and maintainability gap: regressions are caught manually, schema drift between test and runtime is a known risk, deploys are manual, there is no static analysis baseline, and developer onboarding has unnecessary friction. As surface area and contributor count grow, this gap becomes more expensive and more dangerous. +- **Why it matters**: Delivery infrastructure is the foundation that makes product-level correctness sustainable. Without CI, regressions that product architecture hardening work has eliminated can silently return. Without static analysis, type-safety gains from PHP 8.4 and strict Filament/Livewire patterns are unenforced. Test/runtime environment parity gaps mean that passing tests do not prove production correctness — a particularly dangerous problem for a product that governs enterprise tenant configurations. No formal release process means deploy confidence depends on human discipline, which degrades as velocity increases. These are not individually urgent, but they are collectively a prerequisite for scaling the product safely. A platform that governs enterprise Intune tenants should have its own delivery governance in order. +- **Proposed direction**: + - **CI pipeline**: establish a CI configuration (compatible with Gitea runners or external CI) that runs the test suite, Pint formatting checks, and (once added) static analysis on every push/PR. Start with a minimal pipeline that provides a pass/fail quality gate rather than a complex multi-stage build system. The goal is "every merge request is automatically validated" — not a full platform engineering initiative. + - **Static analysis baseline**: introduce PHPStan or Larastan at a pragmatic starting level (e.g. level 5–6), baselined against the current codebase. Focus on catching type errors, undefined method calls, and incorrect return types. Do not aim for level-max compliance as a first step — establish the tool, baseline the noise, and raise the level incrementally. + - **Test/runtime environment parity**: resolve the SQLite-for-tests vs. PostgreSQL-in-production gap. The existing `phpunit.pgsql.xml` suggests this work is partially started. The goal is that the default test suite runs against the same database engine used in production, so that schema-level and query-level differences do not create silent correctness gaps. This is particularly important for JSONB-dependent domains (policy snapshots, backup payloads, operation context). + - **Developer onboarding hygiene**: add `.env.example` with documented defaults. Small but persistent friction item that affects new contributor experience and reduces setup-related support burden. + - **Release process formalization**: define a lightweight, documented release process covering version tagging, migration verification, asset compilation (`filament:assets`, `npm run build`), and staging-to-production promotion checks. Not a full release engineering overhaul — a minimal repeatable process that replaces purely manual deploys with a documented, verifiable workflow. + - **Deployment configuration traceability**: evaluate bringing essential Dokploy/deploy configuration into the repo (or at minimum documenting the external configuration surface) so that environment drift between staging and production is detectable rather than discovered after deployment. +- **Explicit non-goals**: Not a full platform engineering or DevOps transformation initiative. Not a rewrite of deployment architecture or infrastructure provisioning. Not a generic "clean up the repo" bucket for unrelated code quality tasks. Not a replacement for product-level architecture hardening work (queued execution reauthorization, Livewire context locking, etc. are distinct product-safety concerns). Not a mandate to achieve maximum static analysis strictness immediately. Not a CI/CD feature-flag or canary-deployment system. Not an internal developer tooling platform with custom CLIs, dashboards, or abstraction layers. The scope is bounded to the six concrete items identified in the roadmap's Infrastructure & Platform Debt table, plus the minimal CI/release process that connects them into an actionable delivery improvement. +- **Boundary with product architecture hardening (Queued Execution Reauthorization, Livewire Context Locking, etc.)**: Product hardening candidates address trust, authorization, and isolation correctness in the running application. Infrastructure debt addresses delivery confidence — the tooling and process that ensures correctness is verified continuously and shipped reliably. These are complementary layers: product hardening fixes what the code does; infrastructure maturity ensures the fixes stay fixed. +- **Boundary with Operations Naming Harmonization**: Operations naming is about operator-facing terminology consistency across product surfaces. Infrastructure debt is about developer-facing delivery tooling and process. Different audiences, different concerns. +- **Boundary with Admin Visual Language Canon**: The visual language canon mentions lightweight CI enforcement as a possible delivery mechanism for visual convention compliance. If this infrastructure candidate delivers CI, the visual canon can use it — but the CI pipeline itself is infrastructure scope, not visual-canon scope. +- **Dependencies**: None — this is foundational work that other candidates can build on. CI pipeline benefits every future spec by providing automated regression coverage. Static analysis benefits every future hardening spec by enforcing type-safety contractually. +- **Priority**: medium (high cumulative value for delivery confidence and maintainability, but individual items are execution-level tasks rather than product-architecture blockers; should be prioritized pragmatically alongside product work rather than treated as urgent or deferred indefinitely) + --- ## Covered / Absorbed diff --git a/specs/151-findings-workflow-backstop/checklists/requirements.md b/specs/151-findings-workflow-backstop/checklists/requirements.md new file mode 100644 index 0000000..662ed1c --- /dev/null +++ b/specs/151-findings-workflow-backstop/checklists/requirements.md @@ -0,0 +1,34 @@ +# Specification Quality Checklist: Findings Workflow Enforcement and Audit Backstop + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-03-18 +**Feature**: [spec.md](/Users/ahmeddarrazi/Documents/projects/TenantAtlas/specs/151-findings-workflow-backstop/spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- Validation pass complete. The spec intentionally extends Spec 111 and Spec 134 without redefining findings statuses, findings capabilities, or the broader audit UI contract. \ No newline at end of file diff --git a/specs/151-findings-workflow-backstop/contracts/finding-audit-event.schema.yaml b/specs/151-findings-workflow-backstop/contracts/finding-audit-event.schema.yaml new file mode 100644 index 0000000..40a38e8 --- /dev/null +++ b/specs/151-findings-workflow-backstop/contracts/finding-audit-event.schema.yaml @@ -0,0 +1,139 @@ +openapi: 3.1.0 +info: + title: Finding Audit Event Metadata Schema + version: 0.1.0 +components: + schemas: + FindingAuditMetadata: + type: object + required: + - finding_id + - before_status + - after_status + - before + - after + properties: + finding_id: + type: integer + before_status: + type: + - string + - 'null' + after_status: + type: string + system_origin: + type: + - boolean + - 'null' + description: True when the lifecycle mutation was triggered by automation rather than a human actor. + resolved_reason: + type: + - string + - 'null' + closed_reason: + type: + - string + - 'null' + assignee_user_id: + type: + - integer + - 'null' + owner_user_id: + type: + - integer + - 'null' + triaged_at: + type: + - string + - 'null' + format: date-time + in_progress_at: + type: + - string + - 'null' + format: date-time + reopened_at: + type: + - string + - 'null' + format: date-time + due_at: + type: + - string + - 'null' + format: date-time + sla_days: + type: + - integer + - 'null' + before: + $ref: '#/components/schemas/FindingAuditSnapshot' + after: + $ref: '#/components/schemas/FindingAuditSnapshot' + additionalProperties: false + FindingAuditSnapshot: + type: object + properties: + status: + type: + - string + - 'null' + severity: + type: + - string + - 'null' + due_at: + type: + - string + - 'null' + format: date-time + sla_days: + type: + - integer + - 'null' + assignee_user_id: + type: + - integer + - 'null' + owner_user_id: + type: + - integer + - 'null' + triaged_at: + type: + - string + - 'null' + format: date-time + in_progress_at: + type: + - string + - 'null' + format: date-time + reopened_at: + type: + - string + - 'null' + format: date-time + resolved_at: + type: + - string + - 'null' + format: date-time + resolved_reason: + type: + - string + - 'null' + closed_at: + type: + - string + - 'null' + format: date-time + closed_reason: + type: + - string + - 'null' + closed_by_user_id: + type: + - integer + - 'null' + additionalProperties: false \ No newline at end of file diff --git a/specs/151-findings-workflow-backstop/contracts/findings-workflow.openapi.yaml b/specs/151-findings-workflow-backstop/contracts/findings-workflow.openapi.yaml new file mode 100644 index 0000000..b6cfbad --- /dev/null +++ b/specs/151-findings-workflow-backstop/contracts/findings-workflow.openapi.yaml @@ -0,0 +1,357 @@ +openapi: 3.1.0 +info: + title: Findings Workflow Semantic Contract + version: 0.1.0 + description: | + Semantic contract for tenant-context findings workflow operations and audit review. + These endpoints describe the canonical server-side behavior required by the feature, + even if concrete implementation remains behind Filament and Livewire action handlers. + Human-driven and system-driven lifecycle mutations share the same server-side + transition gateway and audit contract. +servers: + - url: http://localhost +paths: + /admin/t/{tenant}/findings/{finding}: + get: + summary: View a tenant-scoped finding + operationId: viewFinding + parameters: + - $ref: '#/components/parameters/TenantId' + - $ref: '#/components/parameters/FindingId' + responses: + '200': + description: Finding returned within the entitled tenant scope + content: + application/json: + schema: + $ref: '#/components/schemas/Finding' + '403': + $ref: '#/components/responses/Forbidden' + '404': + $ref: '#/components/responses/NotFound' + /admin/t/{tenant}/findings/{finding}/workflow/triage: + post: + summary: Triage a finding + operationId: triageFinding + parameters: + - $ref: '#/components/parameters/TenantId' + - $ref: '#/components/parameters/FindingId' + responses: + '200': + $ref: '#/components/responses/FindingMutationSucceeded' + '403': + $ref: '#/components/responses/Forbidden' + '404': + $ref: '#/components/responses/NotFound' + '409': + $ref: '#/components/responses/InvalidTransition' + /admin/t/{tenant}/findings/{finding}/workflow/start-progress: + post: + summary: Move a finding to in progress + operationId: startFindingProgress + parameters: + - $ref: '#/components/parameters/TenantId' + - $ref: '#/components/parameters/FindingId' + responses: + '200': + $ref: '#/components/responses/FindingMutationSucceeded' + '403': + $ref: '#/components/responses/Forbidden' + '404': + $ref: '#/components/responses/NotFound' + '409': + $ref: '#/components/responses/InvalidTransition' + /admin/t/{tenant}/findings/{finding}/workflow/assign: + post: + summary: Assign finding ownership or assignee + operationId: assignFinding + parameters: + - $ref: '#/components/parameters/TenantId' + - $ref: '#/components/parameters/FindingId' + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + assignee_user_id: + type: + - integer + - 'null' + owner_user_id: + type: + - integer + - 'null' + responses: + '200': + $ref: '#/components/responses/FindingMutationSucceeded' + '403': + $ref: '#/components/responses/Forbidden' + '404': + $ref: '#/components/responses/NotFound' + '409': + $ref: '#/components/responses/InvalidTransition' + '422': + $ref: '#/components/responses/ValidationError' + /admin/t/{tenant}/findings/{finding}/workflow/resolve: + post: + summary: Resolve a finding + operationId: resolveFinding + parameters: + - $ref: '#/components/parameters/TenantId' + - $ref: '#/components/parameters/FindingId' + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [reason] + properties: + reason: + type: string + responses: + '200': + $ref: '#/components/responses/FindingMutationSucceeded' + '403': + $ref: '#/components/responses/Forbidden' + '404': + $ref: '#/components/responses/NotFound' + '409': + $ref: '#/components/responses/InvalidTransition' + '422': + $ref: '#/components/responses/ValidationError' + /admin/t/{tenant}/findings/{finding}/workflow/close: + post: + summary: Close a finding + operationId: closeFinding + parameters: + - $ref: '#/components/parameters/TenantId' + - $ref: '#/components/parameters/FindingId' + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [reason] + properties: + reason: + type: string + responses: + '200': + $ref: '#/components/responses/FindingMutationSucceeded' + '403': + $ref: '#/components/responses/Forbidden' + '404': + $ref: '#/components/responses/NotFound' + '409': + $ref: '#/components/responses/InvalidTransition' + '422': + $ref: '#/components/responses/ValidationError' + /admin/t/{tenant}/findings/{finding}/workflow/risk-accept: + post: + summary: Mark a finding as risk accepted + operationId: riskAcceptFinding + parameters: + - $ref: '#/components/parameters/TenantId' + - $ref: '#/components/parameters/FindingId' + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [reason] + properties: + reason: + type: string + responses: + '200': + $ref: '#/components/responses/FindingMutationSucceeded' + '403': + $ref: '#/components/responses/Forbidden' + '404': + $ref: '#/components/responses/NotFound' + '409': + $ref: '#/components/responses/InvalidTransition' + '422': + $ref: '#/components/responses/ValidationError' + /admin/t/{tenant}/findings/{finding}/workflow/reopen: + post: + summary: Reopen a terminal finding + operationId: reopenFinding + parameters: + - $ref: '#/components/parameters/TenantId' + - $ref: '#/components/parameters/FindingId' + responses: + '200': + $ref: '#/components/responses/FindingMutationSucceeded' + '403': + $ref: '#/components/responses/Forbidden' + '404': + $ref: '#/components/responses/NotFound' + '409': + $ref: '#/components/responses/InvalidTransition' + /admin/t/{tenant}/findings/workflow/bulk: + post: + summary: Execute a bulk workflow mutation over multiple findings + operationId: bulkMutateFindings + parameters: + - $ref: '#/components/parameters/TenantId' + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [operation, finding_ids] + properties: + operation: + type: string + enum: [triage, assign, resolve, close, risk_accept, reopen] + finding_ids: + type: array + items: + type: integer + payload: + type: object + additionalProperties: true + responses: + '200': + description: Bulk mutation completed with per-record audit history + content: + application/json: + schema: + type: object + required: [processed, succeeded, failed] + properties: + processed: + type: integer + succeeded: + type: integer + failed: + type: integer + '403': + $ref: '#/components/responses/Forbidden' + '404': + $ref: '#/components/responses/NotFound' + '409': + $ref: '#/components/responses/InvalidTransition' + /admin/audit-log: + get: + summary: Review finding-related audit history on the canonical audit surface + operationId: listFindingAuditEvents + parameters: + - name: resource_type + in: query + schema: + type: string + enum: [finding] + - name: tenant_id + in: query + schema: + type: integer + responses: + '200': + description: Audit events visible within current workspace and tenant entitlement + content: + application/json: + schema: + type: array + items: + $ref: '#/components/schemas/FindingAuditEvent' + '403': + $ref: '#/components/responses/Forbidden' + '404': + $ref: '#/components/responses/NotFound' +components: + parameters: + TenantId: + name: tenant + in: path + required: true + schema: + type: integer + FindingId: + name: finding + in: path + required: true + schema: + type: integer + responses: + FindingMutationSucceeded: + description: Workflow mutation completed and durable audit history was written + content: + application/json: + schema: + type: object + required: [finding, audit_event] + properties: + finding: + $ref: '#/components/schemas/Finding' + audit_event: + $ref: '#/components/schemas/FindingAuditEvent' + Forbidden: + description: Viewer is in scope but lacks required capability + NotFound: + description: Viewer is not entitled to the requested workspace or tenant scope, or the record is outside that scope, including workspace-mismatched findings in canonical tenant context + InvalidTransition: + description: Requested lifecycle mutation is not valid from the current state + ValidationError: + description: Request payload is syntactically valid but missing required business fields + schemas: + Finding: + type: object + required: [id, tenant_id, workspace_id, status, severity] + properties: + id: + type: integer + tenant_id: + type: integer + workspace_id: + type: integer + status: + type: string + enum: [new, acknowledged, triaged, in_progress, reopened, resolved, closed, risk_accepted] + severity: + type: string + enum: [low, medium, high, critical] + due_at: + type: + - string + - 'null' + format: date-time + resolved_reason: + type: + - string + - 'null' + closed_reason: + type: + - string + - 'null' + FindingAuditEvent: + type: object + required: [action, resource_type, resource_id, tenant_id, workspace_id, metadata] + properties: + action: + type: string + example: finding.resolved + resource_type: + type: string + enum: [finding] + resource_id: + type: string + tenant_id: + type: + - integer + - 'null' + workspace_id: + type: integer + metadata: + $ref: './finding-audit-event.schema.yaml#/components/schemas/FindingAuditMetadata' + actor_type: + type: string + enum: [human, system, scheduled, integration, platform] + description: Distinguishes system-origin workflow mutations from human-initiated actions on the canonical audit surface. \ No newline at end of file diff --git a/specs/151-findings-workflow-backstop/data-model.md b/specs/151-findings-workflow-backstop/data-model.md new file mode 100644 index 0000000..d8ff30d --- /dev/null +++ b/specs/151-findings-workflow-backstop/data-model.md @@ -0,0 +1,137 @@ +# Data Model: Findings Workflow Enforcement and Audit Backstop + +## Overview + +This feature does not introduce a new primary persistence model. It hardens the existing relationship between tenant-owned `Finding` records and workspace-owned audit history so that lifecycle truth is enforced consistently across UI and automation paths. + +## Entities + +### Finding + +**Type**: Existing tenant-owned aggregate +**Storage**: `findings` table +**Ownership**: `workspace_id` + `tenant_id` required via tenant ownership rules + +**Relevant fields**: + +- `id` +- `workspace_id` +- `tenant_id` +- `status` +- `severity` +- `due_at` +- `sla_days` +- `assignee_user_id` +- `owner_user_id` +- `triaged_at` +- `in_progress_at` +- `reopened_at` +- `resolved_at` +- `resolved_reason` +- `closed_at` +- `closed_reason` +- `closed_by_user_id` +- `first_seen_at` +- `last_seen_at` +- `times_seen` +- `evidence_jsonb` + +**Validation rules**: + +- `tenant_id` must match the active tenant scope for any tenant-context mutation. +- `workspace_id` must match the owning tenant workspace. +- `status` must remain within the canonical Spec 111 lifecycle state set, with legacy `acknowledged` treated as compatibility input rather than a preferred new write target. +- `resolved_reason` is required for resolve operations. +- `closed_reason` is required for close and risk-accept operations. +- `assignee_user_id` and `owner_user_id`, when present, must reference current tenant members. + +**State transitions**: + +| From | To | Trigger | Notes | +|---|---|---|---| +| `new` | `triaged` | Human triage | Canonical replacement for legacy acknowledge semantics | +| `reopened` | `triaged` | Human triage | Same workflow rule as Spec 111 | +| `acknowledged` | `triaged` | Human triage or compatibility normalization | Legacy compatibility path | +| `triaged` | `in_progress` | Human start progress | Existing workflow rule | +| `acknowledged` | `in_progress` | Human start progress | Legacy compatibility path | +| `new`,`triaged`,`in_progress`,`reopened`,`acknowledged` | `resolved` | Human resolve or system auto-resolve | Reason required; system-origin audit must remain explicit | +| `new`,`triaged`,`in_progress`,`reopened`,`acknowledged` | `closed` | Human close | Reason required | +| `new`,`triaged`,`in_progress`,`reopened`,`acknowledged` | `risk_accepted` | Human risk accept | Reason required | +| `resolved`,`closed`,`risk_accepted` | `reopened` | Human reopen or recurrence reopen | Recomputes SLA and due date | + +**Out-of-scope transitions**: + +- Any direct write that changes lifecycle truth without using the canonical gateway +- Any new active status value beyond the Spec 111 set + +### Finding Workflow Mutation + +**Type**: Domain command, not a table +**Purpose**: Encapsulates the requested lifecycle change and the context needed to validate and audit it + +**Fields**: + +- `finding_id` +- `tenant_id` +- `workspace_id` +- `requested_transition` +- `actor_kind` (`human` or `system`) +- `actor_id` or system-origin identity +- `reason_text` when required +- `assignee_user_id` and `owner_user_id` when assignment changes +- `observation_timestamp` for automation paths that depend on current observation truth + +**Validation rules**: + +- Must resolve to one owned `Finding` within the current tenant scope. +- Must satisfy the allowed transition matrix. +- Must carry all required metadata for the requested transition. +- Must reject no-op transitions that do not materially change lifecycle truth. + +### Audit Log Entry for Findings Workflow + +**Type**: Existing workspace-owned historical record +**Storage**: `audit_logs` table + +**Relevant fields**: + +- `workspace_id` +- `tenant_id` +- `actor_id` +- `actor_email` +- `actor_name` +- `action` +- `resource_type` +- `resource_id` +- `status` +- `metadata` +- `recorded_at` + +**Required metadata shape for this feature**: + +- `finding_id` +- `before_status` +- `after_status` +- `before` +- `after` +- transition-specific reason or assignment fields when applicable +- system-origin marker when the mutation was not user-driven + +**Constraints**: + +- Must never expose `evidence_jsonb`, secrets, or oversized raw payloads. +- Must remain intelligible even if the live `Finding` later changes or becomes inaccessible. +- Must not duplicate history for the same successful covered mutation. + +## Relationships + +- One `Finding` belongs to one `Tenant` and one `Workspace` through tenant ownership. +- Many audit log entries may reference one `Finding` over time. +- One workflow mutation command targets exactly one `Finding` but may be invoked repeatedly across the lifecycle. + +## Consistency Rules + +- A lifecycle mutation is valid only if both the persisted `Finding` and the requested transition still satisfy the gateway rules at execution time. +- Human-driven and system-driven lifecycle changes share the same transition rules but differ in actor semantics and audit labeling. +- Assignment changes are part of workflow history and must be auditable, but they do not introduce a separate lifecycle state. +- Legacy `acknowledged` values remain readable and migratable, but new writes should converge on canonical statuses. \ No newline at end of file diff --git a/specs/151-findings-workflow-backstop/plan.md b/specs/151-findings-workflow-backstop/plan.md new file mode 100644 index 0000000..2548961 --- /dev/null +++ b/specs/151-findings-workflow-backstop/plan.md @@ -0,0 +1,177 @@ +# Implementation Plan: Findings Workflow Enforcement and Audit Backstop + +**Branch**: `151-findings-workflow-backstop` | **Date**: 2026-03-18 | **Spec**: [specs/151-findings-workflow-backstop/spec.md](spec.md) +**Input**: Feature specification from `specs/151-findings-workflow-backstop/spec.md` + +## Summary + +Harden findings lifecycle truth by routing all meaningful finding mutations through one service-owned workflow gateway, eliminating known bypass paths, and guaranteeing durable audit history for both human-driven and system-driven lifecycle changes. + +The implementation keeps Spec 111 as the source of lifecycle semantics and Spec 134 as the source of audit semantics. The design centers on one canonical mutation gateway for findings workflow changes, removes or neutralizes direct model mutation shortcuts, adopts the same gateway for automated reopen and auto-resolve flows, normalizes finding audit action taxonomy, and adds regression guards that fail when covered code paths bypass workflow enforcement or audit coverage. + +## Technical Context + +**Language/Version**: PHP 8.4.15 +**Primary Dependencies**: Laravel 12, Filament v5, Livewire v4, Laravel Sail, Pest v4, PHPUnit v12 +**Storage**: PostgreSQL with existing `findings` and `audit_logs` tables; no new storage engine or external log store +**Testing**: Pest feature and Livewire tests run through `vendor/bin/sail artisan test --compact`; focused browser tests are optional for this slice and not the primary verification path +**Target Platform**: Laravel web application with tenant-context Filament resources and canonical workspace-scoped audit review +**Project Type**: Web application +**Performance Goals**: Single-record findings workflow mutations remain DB-only and transactional; list and view renders remain DB-only; bulk workflow actions continue to scale to 100+ findings while writing per-record audit history deterministically +**Constraints**: No silent lifecycle mutation without server-side authorization and audit history; non-members remain `404`; in-scope members missing capability remain `403`; existing finding-status badges remain centralized; no new Graph calls; no ad hoc audit strings or raw capability strings +**Scale/Scope**: Cross-cutting findings hardening across `Finding`, `FindingWorkflowService`, automated generators and auto-close services, Findings Filament surfaces, audit taxonomy, and focused regression suites + +### Filament v5 Implementation Notes + +- **Livewire v4.0+ compliance**: Maintained. Findings list and view actions remain Livewire-backed Filament actions and stay within the Filament v5 + Livewire v4 contract. +- **Provider registration location**: No new panel is introduced. Existing panel providers remain registered in `bootstrap/providers.php`. +- **Global search rule**: This feature does not add a new globally searchable resource or alter global-search exposure rules. +- **Destructive actions**: `Resolve`, `Close`, `Risk accept`, and `Reopen` remain explicit, capability-gated, and confirmation-protected; this plan keeps them on existing action surfaces rather than inventing hidden mutation paths. +- **Asset strategy**: No new Filament assets are planned. Existing deployment practice still includes `php artisan filament:assets`, but this feature adds no asset registrations. +- **Testing plan**: Add focused Pest feature and Livewire coverage for allowed transitions, invalid transitions, service-path bypass prevention, audit backstop behavior, automation-driven lifecycle paths, and tenant-scope authorization semantics. + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +- Inventory-first: PASS. Findings remain tenant-owned observed-state artifacts; the feature hardens lifecycle truth and does not redefine inventory versus snapshot ownership. +- Read/write separation: PASS. This is a write hardening feature, so every covered mutation stays explicit, server-authorized, auditable, and test-covered. +- Graph contract path: PASS. No new Graph calls or contract-registry additions are introduced. +- Deterministic capabilities: PASS. Existing findings capability constants remain canonical; no raw capability strings are introduced in feature code. +- RBAC-UX plane separation: PASS. Findings mutations stay in the tenant/admin plane, while audit review stays on canonical `/admin` monitoring surfaces with tenant-safe filtering. +- Workspace isolation: PASS. Workspace context remains required for findings and audit review, and historical finding audit detail remains workspace-scoped. +- Tenant isolation: PASS. Findings remain tenant-owned and every list, view, action, and audit drill-down stays tenant-safe. +- Destructive confirmation standard: PASS WITH WORK. Existing destructive-like findings actions remain confirmed; any newly introduced system-safe operator affordance must preserve the same rule. +- Global search tenant safety: PASS. No global-search change is in scope. +- Run observability: PASS. The feature is DB-only for lifecycle enforcement and audit history; no new `OperationRun` usage is introduced. +- Data minimization: PASS WITH WORK. Audit metadata must continue to exclude `evidence_jsonb`, secrets, and oversized payloads while preserving meaningful before/after lifecycle history. +- BADGE-001: PASS. No new finding status values are introduced; existing centralized finding badge semantics remain authoritative. +- UI-NAMING-001: PASS WITH WORK. Audit prose and workflow affordances keep the existing `Triage`, `Start progress`, `Assign`, `Resolve`, `Close`, `Risk accept`, and `Reopen` vocabulary. +- Filament Action Surface Contract: PASS. Existing Findings list and view surfaces already define inspection and grouped workflow actions; the implementation must preserve this while changing only the enforcement path behind them. +- Filament UX-001: PASS. No new create or edit screens are introduced; existing Findings list and detail surfaces remain within the current layout model. + +## Project Structure + +### Documentation (this feature) + +```text +specs/151-findings-workflow-backstop/ +├── plan.md +├── research.md +├── data-model.md +├── quickstart.md +├── contracts/ +│ ├── findings-workflow.openapi.yaml +│ └── finding-audit-event.schema.yaml +└── tasks.md +``` + +### Source Code (repository root) + +```text +app/ +├── Filament/ +│ └── Resources/ +│ └── FindingResource.php +├── Models/ +│ ├── Finding.php +│ └── AuditLog.php +├── Policies/ +│ └── FindingPolicy.php +├── Services/ +│ ├── Audit/ +│ ├── Baselines/ +│ ├── EntraAdminRoles/ +│ ├── Findings/ +│ └── PermissionPosture/ +└── Support/ + ├── Audit/ + ├── Auth/ + └── Badges/ + +database/ +├── factories/ +└── migrations/ + +tests/ +├── Feature/ +│ ├── Audit/ +│ ├── Findings/ +│ └── WorkspaceIsolation/ +└── Unit/ +``` + +**Structure Decision**: Keep the current Laravel and Filament structure. The implementation concentrates findings lifecycle enforcement in the existing findings service layer and audit support layer, then updates automated generator services and existing Filament action surfaces to call the same gateway instead of adding a parallel workflow stack. + +## Complexity Tracking + +| Violation | Why Needed | Simpler Alternative Rejected Because | +|-----------|------------|-------------------------------------| +| N/A | N/A | N/A | + +## Phase 0 — Research (complete) + +- Output: [specs/151-findings-workflow-backstop/research.md](research.md) +- Confirmed that the current findings workflow has multiple real bypass paths: public mutation methods on `Finding`, direct `forceFill()` lifecycle writes in automated services, and no explicit backstop beyond `FindingWorkflowService`. +- Selected a service-owned gateway strategy with regression guards instead of observer-heavy implicit enforcement. +- Confirmed that existing audit history for findings is currently service-path only and uses free-form `finding.*` action IDs with structured metadata snapshots. + +## Phase 1 — Design & Contracts (complete) + +- Output: [specs/151-findings-workflow-backstop/data-model.md](data-model.md) +- Output: [specs/151-findings-workflow-backstop/quickstart.md](quickstart.md) +- Output: [specs/151-findings-workflow-backstop/contracts/findings-workflow.openapi.yaml](contracts/findings-workflow.openapi.yaml) +- Output: [specs/151-findings-workflow-backstop/contracts/finding-audit-event.schema.yaml](contracts/finding-audit-event.schema.yaml) + +### Post-design Constitution Re-check + +- PASS: Findings remain tenant-owned and audit review remains workspace-scoped with tenant-safe filtering. +- PASS: No new Graph or `OperationRun` contract is introduced; the feature stays DB-only and audit-focused. +- PASS WITH WORK: All human-driven and system-driven lifecycle mutations must converge on one service-owned workflow gateway. +- PASS WITH WORK: Existing destructive-like workflow actions must continue to use explicit confirmation and capability gating on Findings surfaces. +- PASS WITH WORK: Regression coverage must actively block direct status mutation and missing-audit regressions rather than relying on coding convention. + +## Phase 2 — Implementation Planning + +`tasks.md` should cover: + +- Consolidating findings lifecycle mutation entry points behind one canonical gateway. +- Retiring or neutralizing direct `Finding` lifecycle mutators and other silent bypass paths. +- Adopting the same gateway for auto-resolve and recurrence flows. +- Normalizing finding audit action taxonomy and audit metadata expectations. +- Preserving existing Filament action surfaces while rewiring list, bulk, and view actions to the unified gateway. +- Adding focused regression coverage for invalid transitions, bypass attempts, duplicate-audit prevention, automated lifecycle flows, and tenant-safe audit visibility. + +### Contract Implementation Note + +- The OpenAPI contract is semantic rather than transport-prescriptive. It documents the canonical server-side workflow operations the UI and automation must honor, even if existing implementation continues to use Filament and Livewire action handlers instead of standalone REST controllers. +- The audit-event schema captures the durable history shape expected from every successful covered workflow mutation and can be used as the validation target for tests. +- No automation-path exemption is planned in this feature. All covered automated reopen and auto-resolve flows must converge on the same gateway as interactive actions. + +### Deployment Sequencing Note + +- Schema changes should be avoided unless the chosen implementation requires a small additive support field; the preferred design is code-path hardening on top of existing findings and audit tables. +- Existing legacy `acknowledged` data remains readable during rollout; any change to write semantics must preserve backward-compatible interpretation during the transition. +- Focused findings and audit tests should pass before expanding the gateway to additional automation services. + +## Final Implementation Status + +- Completed service-owned workflow hardening for human-driven mutations, including owner reassignment and reason-bearing transitions. +- Removed direct `Finding` lifecycle mutators and moved covered automation paths onto the same canonical workflow gateway. +- Added system-origin resolve and reopen support so baseline auto-close, permission posture, Entra admin roles, and baseline recurrence share one audit contract. +- Preserved the existing Filament action surface while tightening visibility and deny semantics for single-record list and view actions. +- Enforced tenant and workspace scope consistency in both policy and resource-layer checks. + +## Final Validation Snapshot + +- `vendor/bin/sail bin pint --dirty --format agent` +- Focused human workflow and audit slice: 28 tests passed, 229 assertions. +- Focused automation and recurrence slice: 45 tests passed, 230 assertions. +- Focused UI and RBAC single-record slice: 14 tests passed, 151 assertions. + +## Rollout Conclusion + +- No schema migration was required. +- No new Filament assets or panel registrations were introduced. +- No unapproved automation-path exemptions remain in the implemented slice. +- Legacy `acknowledged` state remains readable for compatibility, while new workflow truth converges on the canonical statuses and gateway. diff --git a/specs/151-findings-workflow-backstop/quickstart.md b/specs/151-findings-workflow-backstop/quickstart.md new file mode 100644 index 0000000..2cba7f5 --- /dev/null +++ b/specs/151-findings-workflow-backstop/quickstart.md @@ -0,0 +1,121 @@ +# Quickstart: Findings Workflow Enforcement and Audit Backstop + +## Goal + +Verify that findings lifecycle mutations now follow one canonical workflow path and always produce durable audit history for covered changes. + +## Prerequisites + +1. Start Sail if it is not already running. +2. Use a workspace and tenant with at least one authorized findings operator. +3. Ensure test factories can create findings in multiple statuses. + +## Focused Verification Flow + +### 1. Run the focused post-implementation findings workflow suites + +```bash +vendor/bin/sail artisan test --compact \ + tests/Feature/Findings/FindingWorkflowGuardTest.php \ + tests/Feature/Findings/FindingWorkflowServiceTest.php \ + tests/Feature/Findings/FindingWorkflowUiEnforcementTest.php \ + tests/Feature/Findings/FindingWorkflowRowActionsTest.php \ + tests/Feature/Findings/FindingWorkflowViewActionsTest.php \ + tests/Feature/Findings/FindingAuditBackstopTest.php \ + tests/Feature/Audit/FindingAuditVisibilityTest.php \ + tests/Feature/Findings/FindingRbacTest.php +``` + +Expected outcome: + +- workflow mutations succeed only for allowed transitions +- list and view action surfaces still execute the canonical workflow gateway +- owner reassignment and reason-bearing workflow mutations follow the same gateway and validation rules +- audit entries are written exactly once per covered mutation +- non-members get `404` +- in-scope users without capability get `403` + +### 2. Run the automation and recurrence suites + +```bash +vendor/bin/sail artisan test --compact \ + tests/Feature/Findings/DriftStaleAutoResolveTest.php \ + tests/Feature/PermissionPosture/PermissionPostureFindingGeneratorTest.php \ + tests/Feature/EntraAdminRoles/EntraAdminRolesFindingGeneratorTest.php \ + tests/Feature/Findings/FindingRecurrenceTest.php \ + tests/Feature/Findings/FindingAutomationWorkflowTest.php \ + tests/Feature/Findings/FindingWorkflowConcurrencyTest.php +``` + +Expected outcome: + +- system-origin resolve and reopen mutations emit canonical audit rows +- recurrence and stale-auto-resolve paths use the same lifecycle truth as human actions +- older automated observations do not override newer human terminal decisions + +### 3. Run the existing findings compatibility suites + +```bash +vendor/bin/sail artisan test --compact \ + tests/Feature/Findings/FindingAuditLogTest.php \ + tests/Feature/Findings/FindingBulkActionsTest.php +``` + +Expected outcome: + +- legacy findings flows still pass on top of the canonical gateway and audit backstop + +### 4. Run the Filament action-surface contract guard + +```bash +vendor/bin/sail artisan test --compact tests/Feature/Guards/ActionSurfaceContractTest.php +``` + +Expected outcome: + +- the Findings resource still satisfies the required Filament action surface slots after workflow rewiring +- grouped actions, inspection affordances, and mutation visibility rules remain intact + +### 5. Verify tenant-context UI actions + +1. Open the Findings page for an entitled tenant. +2. Execute `Triage`, `Start progress`, `Assign`, `Resolve`, `Close`, `Risk accept`, and `Reopen` on representative findings, including an owner reassignment case and a reason-required transition. +3. Confirm that disabled actions remain visible for members without capability and execute as `403` if forced server-side. + +Expected outcome: + +- existing Findings action surfaces remain unchanged from the operator perspective +- destructive-like workflow actions still require confirmation +- `Close` and `Risk accept` are only visible for open findings; `Reopen` is only visible for terminal findings +- invalid transitions fail safely and do not mutate the record +- no workflow mutation bypasses the existing Findings action surface contract + +### 6. Verify audit review + +1. Open the canonical Audit Log surface. +2. Filter to finding-related audit rows. +3. Confirm that workflow history shows actor, tenant scope, before state, after state, owner or assignee changes, and transition reason where applicable. +4. Confirm that an authorized audit viewer can still understand the historical event when the live finding is deleted, merged away, or no longer directly accessible. + +Expected outcome: + +- finding audit rows remain readable even if the live finding is no longer directly visible +- unauthorized audit viewers do not see finding-related tenant details outside their entitlement scope +- internal bookkeeping metadata such as `_actor_type` and `_dedupe_key` do not render in the inspection view + +## Implementation Sequence + +1. Consolidate lifecycle mutations behind the canonical findings workflow gateway. +2. Neutralize direct `Finding` lifecycle mutators and other bypass paths in covered services. +3. Cover owner reassignment and reason-bearing lifecycle updates with the same gateway and audit contract. +4. Adopt the same gateway for auto-resolve and recurrence flows. +5. Normalize audit action registration and metadata expectations. +6. Add regression guards and focused test coverage, including the Filament action-surface guard. + +## Rollout Notes + +- Prefer additive code-path hardening over schema changes. +- Keep legacy `acknowledged` rows readable during rollout. +- Human and system-origin lifecycle mutations now converge on the same workflow gateway and audit contract. +- Workspace mismatch remains deny-as-not-found on the policy path and deny-safe on the Filament action path. +- Run the focused findings and audit suites before broader tenant workflow or baseline suites. \ No newline at end of file diff --git a/specs/151-findings-workflow-backstop/research.md b/specs/151-findings-workflow-backstop/research.md new file mode 100644 index 0000000..bbe7dfa --- /dev/null +++ b/specs/151-findings-workflow-backstop/research.md @@ -0,0 +1,70 @@ +# Research: Findings Workflow Enforcement and Audit Backstop + +## Decision 1: Use a service-owned workflow gateway as the single mutation path + +**Decision**: Keep `FindingWorkflowService` as the implementation nucleus and evolve it into the single approved gateway for all meaningful findings lifecycle mutations, including automated reopen and auto-resolve paths. + +**Rationale**: The current repo already concentrates the intended human-driven lifecycle logic in `FindingWorkflowService`, including capability checks, transaction handling, and audit writes. The architectural gap is not the absence of a service but the existence of bypass paths that do not use it. Promoting the existing service into the sole mutation gateway preserves established behavior while reducing new abstraction churn. + +**Alternatives considered**: + +- Add model observers to infer audit events and transition validity from any `save()`. Rejected because observers are implicit, brittle for bulk or query updates, and make transition intent harder to reason about. +- Move lifecycle rules into the `Finding` model. Rejected because the model already exposes unsafe public mutators and would still need separate authorization and audit coordination. +- Accept service-path discipline without stronger guardrails. Rejected because the repo already contains concrete bypass paths in automation and model methods. + +## Decision 2: Add explicit regression guards instead of relying on implicit ORM magic + +**Decision**: Enforce the single-gateway rule with focused regression tests that fail when covered code paths mutate findings lifecycle truth directly, rather than attempting to guarantee correctness purely through hidden ORM hooks. + +**Rationale**: The codebase already uses test-driven guardrails for other cross-cutting invariants. A grep or semantic guard for direct lifecycle writes, combined with focused feature tests, is explicit and maintainable. It also matches the repo constitution preference for actionable CI failures over convention-only guidance. + +**Alternatives considered**: + +- Rely exclusively on code review discipline. Rejected because current bypasses already made it through. +- Introduce a custom repository layer just for findings. Rejected because it adds structural overhead without solving automation paths by itself. +- Use database triggers. Rejected because they would hide application intent, complicate testing, and not encode authorization semantics. + +## Decision 3: Route automated lifecycle changes through the same gateway with system-origin semantics + +**Decision**: Baseline auto-close, permission posture cleanup, Entra admin roles cleanup, recurrence reopen, and similar automation paths should call the same workflow gateway while explicitly recording that the actor is system-originated. + +**Rationale**: The strongest remaining workflow-truth risk is that automation still mutates findings directly through `forceFill()` or duplicate reopen logic. Bringing automation under the same gateway preserves one lifecycle model and one audit contract. The audit layer already supports non-human actor semantics. + +**Alternatives considered**: + +- Leave automation paths unaudited because they are “internal”. Rejected because system-driven lifecycle changes are still meaningful governance history. +- Give automation a second lightweight service. Rejected because that would reintroduce split lifecycle truth. +- Audit automation separately but leave status mutation inline. Rejected because audit without transition enforcement still permits invalid state drift. + +## Decision 4: Keep legacy `acknowledged` readable but stop treating it as an active write target + +**Decision**: Preserve compatibility for existing `acknowledged` rows and capability aliases, but normalize future lifecycle writes to the canonical Spec 111 status set instead of continuing to write new `acknowledged` states. + +**Rationale**: The repo still contains `STATUS_ACKNOWLEDGED` and alias capability handling, but Spec 111 already established `triaged` as the canonical workflow state. Continuing to write `acknowledged` would prolong dual-state ambiguity and complicate enforcement, reopening, and audit expectations. + +**Alternatives considered**: + +- Remove `acknowledged` immediately from the database contract. Rejected because existing data and compatibility surfaces still rely on it. +- Leave `acknowledged` as a full first-class active state indefinitely. Rejected because it weakens the goal of one clean lifecycle model. + +## Decision 5: Normalize findings audit taxonomy through the canonical audit naming layer + +**Decision**: Keep the established `finding.*` action vocabulary but register and document it as part of the canonical audit taxonomy instead of leaving it as an ungoverned free-form string set. + +**Rationale**: The existing audit history and tests already use action IDs such as `finding.triaged` and `finding.resolved`. Preserving that vocabulary minimizes migration churn, but the plan should stop treating it as ad hoc. Bringing those actions under the canonical audit naming layer aligns findings with the broader audit foundation and improves summary consistency. + +**Alternatives considered**: + +- Replace all findings action IDs with a new naming scheme. Rejected because the audit value is in stronger governance, not vocabulary churn. +- Leave findings action IDs as raw service strings forever. Rejected because it leaves the same long-term drift risk seen in other audit surfaces. + +## Decision 6: Prefer additive code-path hardening over schema changes in the first slice + +**Decision**: The first implementation slice should avoid new tables or nonessential schema changes and focus on consolidating mutation paths, normalizing audit action registration, and strengthening tests. + +**Rationale**: The core problem is behavioral inconsistency, not missing storage primitives. Existing `findings` and `audit_logs` tables already contain the fields needed for lifecycle truth and durable history. Additive schema work would slow delivery without addressing the primary bypass risk. + +**Alternatives considered**: + +- Add a dedicated findings transition history table. Rejected because Spec 134 already established the audit log as the canonical history layer. +- Add versioning to the `findings` table. Rejected because it duplicates audit history rather than fixing mutation discipline. \ No newline at end of file diff --git a/specs/151-findings-workflow-backstop/spec.md b/specs/151-findings-workflow-backstop/spec.md new file mode 100644 index 0000000..85c377d --- /dev/null +++ b/specs/151-findings-workflow-backstop/spec.md @@ -0,0 +1,172 @@ +# Feature Specification: Findings Workflow Enforcement and Audit Backstop + +**Feature Branch**: `151-findings-workflow-backstop` +**Created**: 2026-03-18 +**Status**: Draft +**Input**: User description: "nimm strategisch das beste und sinnvollste aus spec-candidate" + +## Spec Scope Fields *(mandatory)* + +- **Scope**: tenant + canonical-view +- **Primary Routes**: + - Tenant-context Findings list and detail pages under `/admin/t/{tenant}/...` + - Existing Findings workflow action endpoints triggered from tenant-context Filament list, bulk, and view surfaces + - Canonical audit review surface at `/admin/audit-log` where resulting workflow history is reviewed +- **Data Ownership**: + - Tenant-owned: `Finding` lifecycle truth, assignment metadata, transition reasons, and recurrence-driven status changes + - Workspace-owned: audit events that record meaningful workflow mutations and preserve historical reviewability + - Existing review-pack, reporting, and monitoring outputs remain downstream consumers of the same finding truth and are not redefined by this feature +- **RBAC**: + - Tenant membership remains the isolation boundary for finding visibility and workflow mutation + - Findings view and mutation capabilities remain tenant-context capability-gated through the canonical capability registry + - Audit review remains workspace-scoped and capability-gated through existing audit visibility rules + - Non-members remain deny-as-not-found; in-scope members without the required capability remain forbidden + +For canonical-view specs, the spec MUST define: + +- **Default filter behavior when tenant-context is active**: The canonical audit review surface remains at `/admin/audit-log`. When an operator arrives from tenant context, audit review may prefilter to the active tenant, but it must remain on the shared audit route and must not imply broader tenant visibility than the viewer is entitled to see. +- **Explicit entitlement checks preventing cross-tenant leakage**: Audit rows, filter options, drill-down labels, and finding-related links must be assembled only after workspace and tenant entitlement checks succeed for the referenced tenant-bound finding. Unauthorized viewers must not learn about finding existence, prior status, assignee, or tenant identity through audit metadata. + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Prevent invalid workflow bypasses (Priority: P1) + +As a tenant operator, I need every meaningful finding workflow mutation to pass through one enforced transition contract, so that invalid status changes or direct persistence shortcuts cannot create workflow truth that the product did not explicitly allow. + +**Why this priority**: This closes the main architectural gap. Spec 111 defined the workflow, but the repo still relies too much on disciplined service usage instead of making bypasses impossible or immediately detectable. + +**Independent Test**: Can be fully tested by attempting valid and invalid status changes through single-record actions, bulk actions, and direct persistence-style paths, then confirming that only allowed transitions succeed. + +**Acceptance Scenarios**: + +1. **Given** a finding is in an open workflow state, **When** an allowed workflow action is executed, **Then** the finding transitions successfully through the canonical workflow path. +2. **Given** a finding is in a state that does not permit the requested next state, **When** a workflow mutation is attempted, **Then** the mutation is refused before persistence. +3. **Given** a covered workflow mutation path attempts to bypass the canonical transition path, **When** the mutation executes, **Then** the system fails closed instead of silently accepting the change. + +--- + +### User Story 2 - Trust the audit history for findings lifecycle changes (Priority: P1) + +As a compliance-focused operator, I need every meaningful findings workflow change to leave one complete, understandable audit trail, so that I can answer who changed what, from which state, for which tenant, and why. + +**Why this priority**: A workflow that can change state without durable historical evidence is not enterprise-grade. Auditability is the required backstop when UI discipline or service discipline fails. + +**Independent Test**: Can be fully tested by executing representative workflow changes and verifying that each one produces exactly one readable audit history entry with the correct scope, actor, transition, and reason metadata. + +**Acceptance Scenarios**: + +1. **Given** a user triages, resolves, closes, risk-accepts, reopens, or reassigns a finding, **When** the mutation succeeds, **Then** one audit entry records the meaningful before-and-after lifecycle change. +2. **Given** a system-driven workflow change such as automatic reopen or automatic resolve occurs, **When** the mutation succeeds, **Then** the audit trail records that the change was system-driven rather than a human action. +3. **Given** the finding is later deleted, renamed, or no longer directly accessible, **When** the operator reviews the audit history, **Then** the workflow event remains intelligible from stored summary data. + +--- + +### User Story 3 - Keep recurrence and reopen semantics consistent across all mutation paths (Priority: P2) + +As a product owner, I need recurrence, reopen, and other system-driven lifecycle behaviors to use the same enforced workflow truth as interactive UI actions, so that no second lifecycle model emerges for automated findings changes. + +**Why this priority**: The workflow becomes fragile again if human actions are hardened but recurrence, reopen, or auto-resolve logic still write state through side paths. + +**Independent Test**: Can be fully tested by exercising recurrence detection, reopen behavior, and auto-resolve behavior through representative automated flows and confirming they follow the same transition validity and audit rules. + +**Acceptance Scenarios**: + +1. **Given** a previously resolved finding is detected again, **When** recurrence handling runs, **Then** the reopen uses the same canonical transition truth as a user-driven reopen. +2. **Given** a drift finding is no longer detected and qualifies for automatic resolution, **When** system-driven closure logic runs, **Then** the resulting state change follows the same transition guard and audit contract. +3. **Given** a concurrent user action and automated lifecycle update target the same finding, **When** both flows are evaluated, **Then** the final persisted result remains valid under one workflow contract and the audit trail remains coherent. + +### Edge Cases + +- A bulk action targets findings that span multiple current statuses, some of which permit the transition and some of which do not. +- A direct update changes assignment or lifecycle-reason metadata without using the canonical workflow mutation path. +- A recurrence or auto-resolve flow attempts to change status after a human user already moved the finding to a different terminal state. +- A no-op save re-submits the current status and reason fields without any meaningful lifecycle change. +- A workflow mutation succeeds, but the normal audit-writing path is unavailable or skipped; the feature must still prevent silent history loss. +- An audit viewer is entitled to the workspace but no longer entitled to the tenant that owned the historical finding. + +## Requirements *(mandatory)* + +**Constitution alignment (required):** This feature introduces no new Microsoft Graph calls and no new product workflow states. It hardens existing write behavior around findings lifecycle truth and meaningful audit history. Because it governs security-relevant DB-only mutations, it must define one canonical transition path, fail-closed behavior for bypass attempts, tenant isolation, and durable `AuditLog` coverage instead of relying on UI discipline alone. Existing queue-backed runbooks such as findings lifecycle backfill remain outside the primary scope except where they must obey the same lifecycle truth rules for any state changes they perform. + +**Constitution alignment (OPS-UX):** This feature does not create a new `OperationRun` type and does not change the Ops-UX 3-surface contract. Existing operational runbooks that already use `OperationRun` keep their current behavior. This spec is about workflow truth and auditability for findings mutations, not about new start surfaces, progress surfaces, or terminal run notifications. + +**Constitution alignment (RBAC-UX):** This feature operates in the tenant/admin plane for findings workflow actions and in the canonical admin monitoring plane for audit review. Cross-plane access remains deny-as-not-found. Non-members or users not entitled to the workspace or tenant scope remain `404`. In-scope members lacking the required capability remain `403`. Server-side authorization remains mandatory for every findings mutation path, including bulk actions and any automated or service-owned workflow entry point that runs on behalf of a human actor. The capability registry remains canonical, global search behavior is unchanged, and destructive-like actions such as resolve, close, risk accept, and reopen remain confirmation-gated where already required. + +**Constitution alignment (OPS-EX-AUTH-001):** Not applicable. This feature does not alter authentication handshake behavior or Monitoring page outbound work. + +**Constitution alignment (BADGE-001):** The set of finding status values does not change. Existing centralized finding-status badge semantics remain the only allowed presentation source, and tests must continue to prove that audit and findings surfaces do not introduce ad hoc status label mappings. + +**Constitution alignment (UI-NAMING-001):** The target object is the finding lifecycle change. Operator-facing verbs remain `Triage`, `Start progress`, `Assign`, `Resolve`, `Close`, `Risk accept`, and `Reopen`. Audit prose must preserve the same domain vocabulary across workflow buttons, confirmations, and audit summaries, and must avoid implementation-first language such as internal transition engine terms or persistence jargon. + +**Constitution alignment (Filament Action Surfaces):** This feature modifies existing Findings Filament surfaces by enforcing one transition contract behind list, bulk, and view actions and by requiring audit-safe outcomes for those mutations. The Action Surface Contract is satisfied because destructive-like actions remain explicit, confirmed, capability-gated, and auditable. + +**Constitution alignment (UX-001 — Layout & Information Architecture):** This feature does not add new create or edit screens. Existing Findings list and detail screens keep their current layout. UX impact is limited to preserving consistent workflow actions, keeping the existing explicit empty state, and ensuring view and list surfaces reflect canonical status truth rather than hidden side-path mutations. + +### Functional Requirements + +- **FR-151-001**: The system MUST define one canonical findings workflow mutation path for all in-scope lifecycle changes. +- **FR-151-002**: The canonical findings workflow mutation path MUST enforce the allowed lifecycle transitions already defined by Spec 111 rather than redefining the findings lifecycle. +- **FR-151-003**: In-scope lifecycle changes MUST include at least status transitions, manual reopen, automated reopen, automated resolve, assignee or owner changes, and terminal-reason updates that materially change workflow history. +- **FR-151-004**: Any in-scope mutation attempt that does not satisfy the canonical transition contract MUST be refused before persistence. +- **FR-151-005**: Covered findings list actions, bulk actions, and detail-page actions MUST use the same canonical workflow mutation path rather than separate local transition logic. +- **FR-151-006**: Automated findings lifecycle paths that reopen, resolve, or otherwise materially advance workflow truth MUST use the same canonical workflow mutation path as interactive actions. No narrow or audited exemption is declared for this feature slice. +- **FR-151-007**: The system MUST prevent silent direct status mutation in covered flows. A covered bypass attempt MUST either be blocked or be forced through the canonical workflow mutation path before persistence. +- **FR-151-008**: Every successful in-scope findings workflow mutation MUST produce exactly one meaningful audit event describing actor class, workspace scope, tenant scope, target finding, before state, after state, and reason metadata when applicable. +- **FR-151-009**: The audit backstop MUST ensure that a meaningful workflow mutation cannot complete successfully without durable audit history, even if a preferred audit-writing path is skipped. +- **FR-151-010**: The audit backstop MUST avoid duplicate audit history for the same successful findings workflow mutation. +- **FR-151-011**: Audit metadata for findings workflow changes MUST remain summary-first and MUST NOT store raw evidence payloads, secrets, or oversized snapshots. +- **FR-151-012**: System-driven lifecycle mutations MUST be auditable as system-originated changes and MUST remain distinguishable from human-initiated workflow actions. +- **FR-151-013**: Reopen, recurrence, and auto-resolve behavior defined by Spec 111 MUST remain valid after this hardening and MUST not create a second lifecycle truth model. +- **FR-151-014**: When concurrent workflow mutations target the same finding, the persisted result MUST still satisfy the canonical lifecycle rules and the resulting audit history MUST remain internally coherent. +- **FR-151-015**: Findings workflow audit history MUST remain understandable even when the finding is later deleted, merged, consolidated, or no longer directly viewable to the current operator. +- **FR-151-016**: Audit review surfaces MUST continue to enforce workspace and tenant entitlement before showing finding-related historical detail, labels, or links. +- **FR-151-017**: The feature MUST add negative-path regression coverage for invalid transitions, bypass attempts, missing-audit scenarios, and recurrence edge cases. +- **FR-151-018**: The feature MUST add positive-path regression coverage for representative human-driven and system-driven workflow mutations. +- **FR-151-019**: This feature MUST NOT introduce new finding statuses, new findings capabilities, or a new exception or risk-acceptance entity lifecycle. +- **FR-151-020**: This feature MUST document the narrow boundary between workflow enforcement and audit backstop so future lifecycle-heavy domains can reuse the pattern without copying findings-specific semantics. + +## UI Action Matrix *(mandatory when Filament is changed)* + +| Surface | Location | Header Actions | Inspect Affordance (List/Table) | Row Actions (max 2 visible) | Bulk Actions (grouped) | Empty-State CTA(s) | View Header Actions | Create/Edit Save+Cancel | Audit log? | Notes / Exemptions | +|---|---|---|---|---|---|---|---|---|---|---| +| Findings Resource | Tenant-context Findings list and detail pages | Existing lifecycle utilities only; no new top-level destructive action required for this spec | Existing view affordance and linked record navigation remain | `View`, `Workflow` | `Workflow` group | `Clear filters` | `Workflow` | N/A | Yes | `Workflow` remains the operator surface for `Triage`, `Start progress`, `Assign`, `Resolve`, `Close`, `Risk accept`, and `Reopen`. All destructive-like transitions remain confirmation-gated and capability-gated. | + +### Key Entities *(include if feature involves data)* + +- **Finding lifecycle mutation**: A meaningful change to a finding's workflow truth, such as status progression, reopen, automatic resolution, or ownership change that operators rely on for governance. +- **Workflow transition rule**: The allowed before-and-after lifecycle relationship that makes a findings mutation valid. +- **Findings workflow audit event**: The durable historical record that explains who or what changed a finding lifecycle state, in which tenant scope, and why. +- **System-originated lifecycle change**: A recurrence, auto-resolve, or other product-driven update that changes workflow truth without a direct user click. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-151-001**: 100% of covered findings workflow mutations in automated tests produce exactly one audit event with actor class, tenant scope, before state, and after state. +- **SC-151-002**: 0 covered invalid findings lifecycle transitions succeed in automated regression tests. +- **SC-151-003**: Representative human-driven and system-driven findings lifecycle mutations both pass through one enforced transition contract in acceptance testing. +- **SC-151-004**: In regression tests where the target finding later becomes inaccessible, authorized audit viewers can still understand the historical workflow event without opening the live record. +- **SC-151-005**: CI includes at least one negative guard that fails when a covered findings workflow path bypasses the canonical transition contract or loses audit coverage. + +## Non-Goals + +- Introducing a new findings workflow state model +- Creating a first-class risk exception entity or approval workflow +- Redesigning the Findings UI information architecture beyond what enforcement requires +- Replacing the broader audit foundation with findings-specific history storage +- Turning all low-signal finding field updates into auditable lifecycle events + +## Assumptions + +- Spec 111 remains the product source of truth for allowed findings statuses and core lifecycle semantics. +- Spec 134 remains the source of truth for canonical audit review, event readability, and audit visibility rules. +- Existing findings capabilities and confirmation rules remain adequate for this hardening slice. +- The current product already has representative human-driven and automated findings mutation paths worth hardening under one pattern. +- No automation-path exemption from the canonical workflow gateway is allowed in this slice; any future exemption would require a follow-up spec change. + +## Dependencies + +- Spec 111 - Findings Workflow V2 + SLA +- Spec 134 - Audit Log Foundation +- Existing tenant isolation and capability enforcement for Findings actions +- Existing findings recurrence, detection, and auto-resolve behavior that must be brought under the same lifecycle truth model diff --git a/specs/151-findings-workflow-backstop/tasks.md b/specs/151-findings-workflow-backstop/tasks.md new file mode 100644 index 0000000..33e84f3 --- /dev/null +++ b/specs/151-findings-workflow-backstop/tasks.md @@ -0,0 +1,197 @@ +# Tasks: Findings Workflow Enforcement and Audit Backstop + +**Input**: Design documents from `/specs/151-findings-workflow-backstop/` +**Prerequisites**: plan.md, spec.md, research.md, data-model.md, contracts/, quickstart.md + +**Tests**: Tests are REQUIRED for this feature because it changes runtime behavior, authorization-sensitive mutation paths, and audit guarantees. +**Operations**: This feature does not introduce new long-running or queued work. Existing runbooks remain out of scope unless they mutate findings lifecycle truth, in which case they must use `AuditLog` coverage instead of new `OperationRun` behavior. +**RBAC**: Findings mutations remain tenant/admin plane behavior. Non-members or users outside workspace or tenant scope must resolve as `404`; in-scope users lacking capability must resolve as `403`. No raw capability strings or role-string checks may be introduced. +**UI Naming**: Existing operator-facing wording remains `Triage`, `Start progress`, `Assign`, `Resolve`, `Close`, `Risk accept`, and `Reopen` across UI and audit surfaces. +**Filament UI Action Surfaces**: Existing Findings list and view surfaces remain the primary UI contract. Tasks below preserve grouped workflow actions, confirmation requirements, audit logging, and inspection affordances. +**Badges**: No new badge domain is introduced. Existing finding status badges remain centralized and must not be remapped locally. + +## Phase 1: Setup (Shared Infrastructure) + +**Purpose**: Prepare shared fixtures and helper coverage used by all stories. + +- [X] T001 Update lifecycle-ready finding factory states in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/database/factories/FindingFactory.php +- [X] T002 [P] Add shared findings workflow test helpers in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Pest.php and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Findings/Concerns/InteractsWithFindingsWorkflow.php + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +**Purpose**: Establish the shared workflow and audit foundation before any user-story slice is implemented. + +**⚠️ CRITICAL**: No user story work should begin until this phase is complete. + +- [X] T003 Normalize findings workflow audit action registration in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Support/Audit/AuditActionId.php +- [X] T004 Establish the service-owned findings workflow gateway for status, owner-assignment, and reason-bearing lifecycle mutations in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Services/Findings/FindingWorkflowService.php +- [X] T005 [P] Add workflow bypass regression guard coverage in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Findings/FindingWorkflowGuardTest.php +- [X] T006 [P] Tighten findings audit sanitization and summary handling in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Support/Audit/AuditContextSanitizer.php and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Services/Intune/AuditLogger.php + +**Checkpoint**: Shared workflow and audit primitives are ready. User stories can now be implemented independently. + +--- + +## Phase 3: User Story 1 - Prevent Invalid Workflow Bypasses (Priority: P1) 🎯 MVP + +**Goal**: Ensure every covered findings lifecycle mutation goes through one enforced transition path and fails closed on invalid or bypassed writes. + +**Independent Test**: Attempt valid and invalid transitions through service, list action, bulk action, and direct mutation-style paths; only valid transitions should persist. + +### Tests for User Story 1 + +- [X] T007 [P] [US1] Add transition validation tests for status changes, owner reassignment, and required-reason mutations in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Findings/FindingWorkflowServiceTest.php +- [X] T008 [P] [US1] Add Livewire findings action enforcement tests that preserve the existing action surface contract in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Findings/FindingWorkflowUiEnforcementTest.php + +### Implementation for User Story 1 + +- [X] T009 [US1] Remove or neutralize direct lifecycle mutators in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Models/Finding.php +- [X] T010 [US1] Rewire single-record workflow actions to the canonical gateway in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Filament/Resources/FindingResource.php and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Filament/Resources/FindingResource/Pages/ViewFinding.php +- [X] T011 [US1] Rewire bulk workflow actions to the canonical gateway in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Filament/Resources/FindingResource.php and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Filament/Resources/FindingResource/Pages/ListFindings.php +- [X] T012 [US1] Tighten tenant-scope and capability enforcement for covered mutations in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Policies/FindingPolicy.php and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Services/Findings/FindingWorkflowService.php + +**Checkpoint**: User Story 1 is complete when invalid transitions and direct bypass attempts no longer mutate findings and existing Findings UI actions still function for authorized users. + +--- + +## Phase 4: User Story 2 - Trust the Audit History for Findings Lifecycle Changes (Priority: P1) + +**Goal**: Guarantee that every covered findings lifecycle mutation produces one durable, readable, tenant-safe audit event. + +**Independent Test**: Execute representative lifecycle changes and confirm exactly one audit row is created per mutation with the correct actor, scope, before-state, after-state, and reason metadata. + +### Tests for User Story 2 + +- [X] T013 [P] [US2] Add findings audit backstop tests in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Findings/FindingAuditBackstopTest.php +- [X] T014 [P] [US2] Add finding audit visibility, scope, and deleted-or-inaccessible record readability tests in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Audit/FindingAuditVisibilityTest.php + +### Implementation for User Story 2 + +- [X] T015 [US2] Ensure each covered gateway mutation, including owner reassignment and reason-bearing updates, writes one canonical audit event in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Services/Findings/FindingWorkflowService.php +- [X] T016 [US2] Register findings workflow audit summaries and labels in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Support/Audit/AuditActionId.php and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Support/Navigation/RelatedNavigationResolver.php +- [X] T017 [US2] Enforce findings audit metadata minimization in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Support/Audit/AuditContextSanitizer.php and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Models/AuditLog.php +- [X] T018 [US2] Add duplicate-audit prevention and backstop behavior in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Services/Audit/AuditRecorder.php and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Services/Intune/AuditLogger.php + +**Checkpoint**: User Story 2 is complete when covered lifecycle mutations always emit one readable audit event and audit review remains tenant-safe. + +--- + +## Phase 5: User Story 3 - Keep Recurrence and Reopen Semantics Consistent Across All Mutation Paths (Priority: P2) + +**Goal**: Bring system-driven reopen and auto-resolve flows under the same lifecycle truth and audit rules as interactive workflow actions. + +**Independent Test**: Run recurrence and auto-resolve scenarios through baseline, permission posture, and Entra-admin flows and confirm they use the same workflow rules and audit outputs as UI actions. + +### Tests for User Story 3 + +- [X] T019 [P] [US3] Add automation workflow regression tests in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Findings/FindingAutomationWorkflowTest.php +- [X] T020 [P] [US3] Add concurrency edge-case tests for human and automation mutations in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/tests/Feature/Findings/FindingWorkflowConcurrencyTest.php + +### Implementation for User Story 3 + +- [X] T021 [US3] Route baseline auto-close through the canonical gateway in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Services/Baselines/BaselineAutoCloseService.php +- [X] T022 [US3] Route permission posture lifecycle updates through the canonical gateway in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Services/PermissionPosture/PermissionPostureFindingGenerator.php +- [X] T023 [US3] Route Entra admin roles lifecycle updates through the canonical gateway in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Services/EntraAdminRoles/EntraAdminRolesFindingGenerator.php +- [X] T024 [US3] Unify recurrence reopen handling in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Jobs/CompareBaselineToTenantJob.php and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/app/Services/Findings/FindingWorkflowService.php + +**Checkpoint**: User Story 3 is complete when automation paths no longer mutate findings lifecycle truth through private side channels. + +--- + +## Phase 6: Polish & Cross-Cutting Concerns + +**Purpose**: Finalize documentation alignment and end-to-end verification across all stories. + +- [X] T025 [P] Update verification notes and final command set, including the Filament action-surface guard, in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/specs/151-findings-workflow-backstop/quickstart.md and /Users/ahmeddarrazi/Documents/projects/TenantAtlas/specs/151-findings-workflow-backstop/contracts/findings-workflow.openapi.yaml +- [X] T026 Record final rollout notes and confirm that no unapproved implementation exemptions were introduced in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/specs/151-findings-workflow-backstop/plan.md after running focused Pint and Pest validation + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +- **Setup (Phase 1)**: No dependencies; can start immediately. +- **Foundational (Phase 2)**: Depends on Setup completion; blocks all user stories. +- **User Story 1 (Phase 3)**: Depends on Foundational completion; provides the MVP enforcement slice. +- **User Story 2 (Phase 4)**: Depends on Foundational completion and should follow US1 because it hardens the same gateway with audit guarantees. +- **User Story 3 (Phase 5)**: Depends on Foundational completion and benefits from US1 gateway consolidation before routing automation paths. +- **Polish (Phase 6)**: Depends on all desired user stories being complete. + +### User Story Dependencies + +- **US1**: Can start immediately after Foundational; no dependency on other stories. +- **US2**: Can begin after Foundational, but is lowest-risk after US1 establishes the canonical gateway. +- **US3**: Can begin after Foundational, but depends conceptually on the gateway introduced in US1 and the audit guarantees reinforced in US2. + +### Within Each User Story + +- Tests should be written first and verified to fail before implementation. +- Gateway or policy changes should land before UI or automation rewiring. +- Story-specific verification should pass before moving to the next story. + +### Parallel Opportunities + +- T001 and T002 can run in parallel. +- T005 and T006 can run in parallel once T003 and T004 are underway. +- Within each story, the test tasks marked `[P]` can run in parallel. +- US3 service rewiring tasks T021, T022, and T023 can run in parallel once the canonical gateway behavior is stable. + +--- + +## Parallel Example: User Story 1 + +```bash +# Write the two US1 test files in parallel +Task: T007 [US1] tests/Feature/Findings/FindingWorkflowServiceTest.php +Task: T008 [US1] tests/Feature/Findings/FindingWorkflowUiEnforcementTest.php + +# After the tests are in place, rewire the two UI surfaces in parallel +Task: T010 [US1] app/Filament/Resources/FindingResource.php + app/Filament/Resources/FindingResource/Pages/ViewFinding.php +Task: T011 [US1] app/Filament/Resources/FindingResource.php + app/Filament/Resources/FindingResource/Pages/ListFindings.php +``` + +## Parallel Example: User Story 2 + +```bash +# Add audit backstop and visibility tests in parallel +Task: T013 [US2] tests/Feature/Findings/FindingAuditBackstopTest.php +Task: T014 [US2] tests/Feature/Audit/FindingAuditVisibilityTest.php + +# Then split audit taxonomy and sanitizer work +Task: T016 [US2] app/Support/Audit/AuditActionId.php + app/Support/Navigation/RelatedNavigationResolver.php +Task: T017 [US2] app/Support/Audit/AuditContextSanitizer.php + app/Models/AuditLog.php +``` + +## Parallel Example: User Story 3 + +```bash +# Route automation services independently once the gateway contract is stable +Task: T021 [US3] app/Services/Baselines/BaselineAutoCloseService.php +Task: T022 [US3] app/Services/PermissionPosture/PermissionPostureFindingGenerator.php +Task: T023 [US3] app/Services/EntraAdminRoles/EntraAdminRolesFindingGenerator.php +``` + +--- + +## Implementation Strategy + +### MVP First + +Implement **User Story 1** first to establish a single enforced workflow mutation path and remove the most dangerous bypasses. This delivers immediate risk reduction even before audit and automation cleanup are complete. + +### Incremental Delivery + +1. Complete Setup and Foundational phases. +2. Deliver US1 as the first mergeable slice. +3. Add US2 to guarantee durable historical evidence for the same gateway. +4. Extend the gateway to automation paths in US3. +5. Finish with cross-cutting validation and documentation updates. + +### Suggested MVP Scope + +- **MVP**: Phase 1 + Phase 2 + Phase 3 (User Story 1) +- **Production-ready hardening slice**: MVP + Phase 4 (User Story 2) +- **Full feature scope**: MVP + US2 + US3 + Polish \ No newline at end of file diff --git a/tests/Feature/Audit/FindingAuditVisibilityTest.php b/tests/Feature/Audit/FindingAuditVisibilityTest.php new file mode 100644 index 0000000..c9008ef --- /dev/null +++ b/tests/Feature/Audit/FindingAuditVisibilityTest.php @@ -0,0 +1,193 @@ +for($tenant)->create(); + + $audit = AuditLog::query()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'tenant_id' => (int) $tenant->getKey(), + 'actor_email' => 'owner@example.com', + 'actor_name' => 'Owner', + 'actor_type' => 'human', + 'action' => 'finding.resolved', + 'status' => 'success', + 'resource_type' => 'finding', + 'resource_id' => (string) $finding->getKey(), + 'target_label' => 'Drift finding #'.$finding->getKey(), + 'summary' => 'Finding resolved for Drift finding #'.$finding->getKey(), + 'metadata' => [ + 'finding_id' => (int) $finding->getKey(), + 'before_status' => Finding::STATUS_TRIAGED, + 'after_status' => Finding::STATUS_RESOLVED, + ], + 'recorded_at' => now(), + ]); + + $this->actingAs($user); + Filament::setTenant(null, true); + session()->put(WorkspaceContext::SESSION_KEY, (int) $tenant->workspace_id); + + Livewire::actingAs($user)->test(AuditLogPage::class) + ->assertCanSeeTableRecords([$audit]) + ->callTableAction('inspect', $audit) + ->assertSet('selectedAuditLogId', (int) $audit->getKey()) + ->assertSee('Drift finding #'.$finding->getKey()) + ->assertSee('Open finding'); +}); + +it('keeps deleted findings readable while suppressing finding drill-down links', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + + $finding = Finding::factory()->for($tenant)->create(); + $findingId = (int) $finding->getKey(); + $finding->delete(); + + $audit = AuditLog::query()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'tenant_id' => (int) $tenant->getKey(), + 'actor_email' => 'owner@example.com', + 'actor_name' => 'Owner', + 'actor_type' => 'human', + 'action' => 'finding.closed', + 'status' => 'success', + 'resource_type' => 'finding', + 'resource_id' => (string) $findingId, + 'target_label' => 'Permission posture finding #'.$findingId, + 'summary' => 'Finding closed for Permission posture finding #'.$findingId, + 'metadata' => [ + 'finding_id' => $findingId, + 'before_status' => Finding::STATUS_REOPENED, + 'after_status' => Finding::STATUS_CLOSED, + 'closed_reason' => 'duplicate', + ], + 'recorded_at' => now(), + ]); + + $this->actingAs($user); + Filament::setTenant(null, true); + session()->put(WorkspaceContext::SESSION_KEY, (int) $tenant->workspace_id); + + Livewire::actingAs($user)->test(AuditLogPage::class) + ->assertCanSeeTableRecords([$audit]) + ->callTableAction('inspect', $audit) + ->assertSet('selectedAuditLogId', (int) $audit->getKey()) + ->assertSee('Permission posture finding #'.$findingId) + ->assertDontSee('Open finding'); +}); + +it('does not render internal audit bookkeeping metadata in the inspection view', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + + $finding = Finding::factory()->for($tenant)->resolved()->create(); + + $audit = AuditLog::query()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + 'tenant_id' => (int) $tenant->getKey(), + 'actor_email' => 'owner@example.com', + 'actor_name' => 'Owner', + 'actor_type' => 'human', + 'action' => 'finding.closed', + 'status' => 'success', + 'resource_type' => 'finding', + 'resource_id' => (string) $finding->getKey(), + 'target_label' => 'Drift finding #'.$finding->getKey(), + 'summary' => 'Finding closed for Drift finding #'.$finding->getKey(), + 'metadata' => [ + 'finding_id' => (int) $finding->getKey(), + 'before_status' => Finding::STATUS_RESOLVED, + 'after_status' => Finding::STATUS_CLOSED, + '_actor_type' => 'hidden-actor-marker', + '_dedupe_key' => 'internal-bookkeeping-marker', + ], + 'recorded_at' => now(), + ]); + + $this->actingAs($user); + Filament::setTenant(null, true); + session()->put(WorkspaceContext::SESSION_KEY, (int) $tenant->workspace_id); + + Livewire::actingAs($user)->test(AuditLogPage::class) + ->assertCanSeeTableRecords([$audit]) + ->callTableAction('inspect', $audit) + ->assertSet('selectedAuditLogId', (int) $audit->getKey()) + ->assertDontSee('_dedupe_key') + ->assertDontSee('internal-bookkeeping-marker') + ->assertDontSee('_actor_type') + ->assertDontSee('hidden-actor-marker'); +}); + +it('hides finding audit rows for tenants outside the viewer entitlement scope', function (): void { + [$user, $tenantA] = createUserWithTenant(role: 'owner'); + + $tenantB = Tenant::factory()->create([ + 'workspace_id' => (int) $tenantA->workspace_id, + ]); + + $findingA = Finding::factory()->for($tenantA)->create(); + $findingB = Finding::factory()->for($tenantB)->create(); + + $visible = AuditLog::query()->create([ + 'workspace_id' => (int) $tenantA->workspace_id, + 'tenant_id' => (int) $tenantA->getKey(), + 'actor_email' => 'owner@example.com', + 'actor_name' => 'Owner', + 'actor_type' => 'human', + 'action' => 'finding.triaged', + 'status' => 'success', + 'resource_type' => 'finding', + 'resource_id' => (string) $findingA->getKey(), + 'target_label' => 'Drift finding #'.$findingA->getKey(), + 'summary' => 'Finding triaged for Drift finding #'.$findingA->getKey(), + 'metadata' => [ + 'finding_id' => (int) $findingA->getKey(), + 'before_status' => Finding::STATUS_NEW, + 'after_status' => Finding::STATUS_TRIAGED, + ], + 'recorded_at' => now(), + ]); + + $hidden = AuditLog::query()->create([ + 'workspace_id' => (int) $tenantB->workspace_id, + 'tenant_id' => (int) $tenantB->getKey(), + 'actor_email' => 'owner@example.com', + 'actor_name' => 'Owner', + 'actor_type' => 'human', + 'action' => 'finding.reopened', + 'status' => 'success', + 'resource_type' => 'finding', + 'resource_id' => (string) $findingB->getKey(), + 'target_label' => 'Drift finding #'.$findingB->getKey(), + 'summary' => 'Finding reopened for Drift finding #'.$findingB->getKey(), + 'metadata' => [ + 'finding_id' => (int) $findingB->getKey(), + 'before_status' => Finding::STATUS_RESOLVED, + 'after_status' => Finding::STATUS_REOPENED, + ], + 'recorded_at' => now(), + ]); + + $this->actingAs($user) + ->withSession([WorkspaceContext::SESSION_KEY => (int) $tenantA->workspace_id]) + ->get(route('admin.monitoring.audit-log').'?event='.(int) $hidden->getKey()) + ->assertNotFound(); + + $this->actingAs($user); + Filament::setTenant(null, true); + session()->put(WorkspaceContext::SESSION_KEY, (int) $tenantA->workspace_id); + + Livewire::actingAs($user)->test(AuditLogPage::class) + ->assertCanSeeTableRecords([$visible]) + ->assertCanNotSeeTableRecords([$hidden]); +}); diff --git a/tests/Feature/EntraAdminRoles/EntraAdminRolesFindingGeneratorTest.php b/tests/Feature/EntraAdminRoles/EntraAdminRolesFindingGeneratorTest.php index f3ba8f8..b2dbf3e 100644 --- a/tests/Feature/EntraAdminRoles/EntraAdminRolesFindingGeneratorTest.php +++ b/tests/Feature/EntraAdminRoles/EntraAdminRolesFindingGeneratorTest.php @@ -461,7 +461,11 @@ function makeGenerator(): EntraAdminRolesFindingGenerator ->where('tenant_id', $tenant->getKey()) ->where('subject_external_id', 'user-1:def-ga') ->first(); - $finding->acknowledge($user); + $finding->forceFill([ + 'status' => Finding::STATUS_ACKNOWLEDGED, + 'acknowledged_at' => now(), + 'acknowledged_by_user_id' => $user->getKey(), + ])->save(); expect($finding->fresh()->status)->toBe(Finding::STATUS_ACKNOWLEDGED); diff --git a/tests/Feature/Findings/Concerns/InteractsWithFindingsWorkflow.php b/tests/Feature/Findings/Concerns/InteractsWithFindingsWorkflow.php new file mode 100644 index 0000000..d048910 --- /dev/null +++ b/tests/Feature/Findings/Concerns/InteractsWithFindingsWorkflow.php @@ -0,0 +1,62 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + return [$user, $tenant]; + } + + /** + * @param array $attributes + */ + protected function makeFindingForWorkflow(Tenant $tenant, string $status = Finding::STATUS_NEW, array $attributes = []): Finding + { + $factory = Finding::factory()->for($tenant); + + $factory = match ($status) { + Finding::STATUS_ACKNOWLEDGED => $factory->acknowledged(), + Finding::STATUS_TRIAGED => $factory->triaged(), + Finding::STATUS_IN_PROGRESS => $factory->inProgress(), + Finding::STATUS_REOPENED => $factory->reopened(), + Finding::STATUS_RESOLVED => $factory->resolved(), + Finding::STATUS_CLOSED => $factory->closed(), + Finding::STATUS_RISK_ACCEPTED => $factory->riskAccepted(), + default => $factory, + }; + + return $factory->create($attributes); + } + + protected function latestFindingAudit(Finding $finding, string|AuditActionId $action): ?AuditLog + { + $actionValue = $action instanceof AuditActionId ? $action->value : $action; + + return AuditLog::query() + ->where('tenant_id', (int) $finding->tenant_id) + ->where('resource_type', 'finding') + ->where('resource_id', (string) $finding->getKey()) + ->where('action', $actionValue) + ->latest('id') + ->first(); + } +} diff --git a/tests/Feature/Findings/FindingAuditBackstopTest.php b/tests/Feature/Findings/FindingAuditBackstopTest.php new file mode 100644 index 0000000..45ff9f4 --- /dev/null +++ b/tests/Feature/Findings/FindingAuditBackstopTest.php @@ -0,0 +1,130 @@ +actingAsFindingOperator('owner'); + + $service = app(FindingWorkflowService::class); + $finding = $this->makeFindingForWorkflow($tenant, Finding::STATUS_NEW); + + $service->triage($finding, $tenant, $user); + $service->assign($finding->refresh(), $tenant, $user, null, (int) $user->getKey()); + $service->resolve($finding->refresh(), $tenant, $user, 'patched'); + $service->reopen($finding->refresh(), $tenant, $user); + $service->close($finding->refresh(), $tenant, $user, 'duplicate'); + + expect(AuditLog::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('resource_type', 'finding') + ->where('resource_id', (string) $finding->getKey()) + ->count())->toBe(5); + + $closedAudit = $this->latestFindingAudit($finding, AuditActionId::FindingClosed); + + expect($closedAudit)->not->toBeNull(); + expect($closedAudit->actorSnapshot()->type)->toBe(AuditActorType::Human) + ->and($closedAudit->targetDisplayLabel())->toContain('finding') + ->and(data_get($closedAudit->metadata, 'before_status'))->toBe(Finding::STATUS_REOPENED) + ->and(data_get($closedAudit->metadata, 'after_status'))->toBe(Finding::STATUS_CLOSED) + ->and(data_get($closedAudit->metadata, 'closed_reason'))->toBe('duplicate') + ->and(data_get($closedAudit->metadata, 'before.evidence_jsonb'))->toBeNull() + ->and(data_get($closedAudit->metadata, 'after.evidence_jsonb'))->toBeNull(); +}); + +it('deduplicates repeated finding audit writes for the same successful mutation payload', function (): void { + $tenant = Tenant::factory()->create(); + $finding = Finding::factory()->for($tenant)->resolved()->create(); + + $recorder = app(AuditRecorder::class); + + $payload = [ + 'metadata' => [ + 'finding_id' => (int) $finding->getKey(), + 'before_status' => Finding::STATUS_TRIAGED, + 'after_status' => Finding::STATUS_RESOLVED, + 'before' => ['status' => Finding::STATUS_TRIAGED], + 'after' => ['status' => Finding::STATUS_RESOLVED], + '_dedupe_key' => 'finding-resolved-'.$finding->getKey(), + ], + ]; + + $first = $recorder->record( + action: AuditActionId::FindingResolved, + context: $payload, + workspace: $tenant->workspace, + tenant: $tenant, + actor: AuditActorSnapshot::fromLegacy(AuditActorType::Human, 1, 'owner@example.com', 'Owner'), + target: new AuditTargetSnapshot(type: 'finding', id: (string) $finding->getKey(), label: 'Permission posture finding #'.$finding->getKey()), + outcome: 'success', + ); + + $second = $recorder->record( + action: AuditActionId::FindingResolved, + context: $payload, + workspace: $tenant->workspace, + tenant: $tenant, + actor: AuditActorSnapshot::fromLegacy(AuditActorType::Human, 1, 'owner@example.com', 'Owner'), + target: new AuditTargetSnapshot(type: 'finding', id: (string) $finding->getKey(), label: 'Permission posture finding #'.$finding->getKey()), + outcome: 'success', + ); + + expect((int) $first->getKey())->toBe((int) $second->getKey()) + ->and(AuditLog::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('action', AuditActionId::FindingResolved->value) + ->where('resource_type', 'finding') + ->where('resource_id', (string) $finding->getKey()) + ->count())->toBe(1); +}); + +it('stores system-origin audit metadata without raw evidence payloads', function (): void { + $tenant = Tenant::factory()->create(); + $finding = Finding::factory()->for($tenant)->resolved()->create([ + 'evidence_jsonb' => [ + 'secret' => 'should-never-appear', + 'payload' => ['a' => 'b'], + ], + ]); + + $audit = app(AuditRecorder::class)->record( + action: AuditActionId::FindingReopened, + context: [ + 'metadata' => [ + 'finding_id' => (int) $finding->getKey(), + 'before_status' => Finding::STATUS_RESOLVED, + 'after_status' => Finding::STATUS_REOPENED, + 'before' => [ + 'status' => Finding::STATUS_RESOLVED, + 'evidence_jsonb' => ['secret' => 'leak'], + ], + 'after' => [ + 'status' => Finding::STATUS_REOPENED, + 'evidence_jsonb' => ['secret' => 'leak'], + ], + 'system_origin' => true, + '_actor_type' => AuditActorType::System->value, + ], + ], + workspace: $tenant->workspace, + tenant: $tenant, + actor: AuditActorSnapshot::fromLegacy(AuditActorType::System), + target: new AuditTargetSnapshot(type: 'finding', id: (string) $finding->getKey(), label: 'Drift finding #'.$finding->getKey()), + outcome: 'success', + ); + + expect($audit->actorSnapshot()->type)->toBe(AuditActorType::System) + ->and(data_get($audit->metadata, 'system_origin'))->toBeTrue() + ->and(data_get($audit->metadata, 'before.evidence_jsonb'))->toBeNull() + ->and(data_get($audit->metadata, 'after.evidence_jsonb'))->toBeNull(); +}); diff --git a/tests/Feature/Findings/FindingAutomationWorkflowTest.php b/tests/Feature/Findings/FindingAutomationWorkflowTest.php new file mode 100644 index 0000000..84e72f5 --- /dev/null +++ b/tests/Feature/Findings/FindingAutomationWorkflowTest.php @@ -0,0 +1,330 @@ + + */ + function automationBaselineCompareDriftItem(int $baselineProfileId, int $compareOperationRunId, string $subjectKey): array + { + return [ + 'change_type' => 'different_version', + 'severity' => Finding::SEVERITY_MEDIUM, + 'subject_type' => 'policy', + 'subject_external_id' => $subjectKey, + 'subject_key' => $subjectKey, + 'policy_type' => 'deviceConfiguration', + 'baseline_hash' => 'baseline', + 'current_hash' => 'current', + 'evidence_fidelity' => 'meta', + 'evidence' => [ + 'change_type' => 'different_version', + 'policy_type' => 'deviceConfiguration', + 'subject_key' => $subjectKey, + 'summary' => ['kind' => 'policy_snapshot'], + 'baseline' => ['policy_version_id' => null, 'hash' => 'baseline'], + 'current' => ['policy_version_id' => null, 'hash' => 'current'], + 'fidelity' => 'meta', + 'provenance' => [ + 'baseline_profile_id' => $baselineProfileId, + 'baseline_snapshot_id' => 1, + 'compare_operation_run_id' => $compareOperationRunId, + 'inventory_sync_run_id' => null, + ], + ], + ]; + } +} + +if (! function_exists('invokeAutomationBaselineCompareUpsertFindings')) { + /** + * @param array> $driftResults + * @return array{processed_count:int,created_count:int,reopened_count:int,unchanged_count:int,seen_fingerprints:array} + */ + function invokeAutomationBaselineCompareUpsertFindings( + CompareBaselineToTenantJob $job, + \App\Models\Tenant $tenant, + BaselineProfile $profile, + string $scopeKey, + array $driftResults, + ): array { + $reflection = new ReflectionMethod($job, 'upsertFindings'); + + /** @var array{processed_count:int,created_count:int,reopened_count:int,unchanged_count:int,seen_fingerprints:array} $result */ + $result = $reflection->invoke($job, $tenant, $profile, $scopeKey, $driftResults); + + return $result; + } +} + +it('writes a system-origin audit row when stale drift findings auto-resolve', function (): void { + [, $tenant] = createUserWithTenant(role: 'manager'); + + $profile = BaselineProfile::factory()->active()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + ]); + + $scopeKey = 'baseline_profile:'.$profile->getKey(); + $run = OperationRun::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + 'type' => 'baseline_compare', + ]); + + $observedAt = CarbonImmutable::parse('2026-03-18T09:00:00Z'); + CarbonImmutable::setTestNow($observedAt); + + $finding = Finding::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'source' => 'baseline.compare', + 'scope_key' => $scopeKey, + 'fingerprint' => 'drift-stale-audit', + 'recurrence_key' => 'drift-stale-audit', + 'status' => Finding::STATUS_NEW, + ]); + + expect(app(BaselineAutoCloseService::class)->resolveStaleFindings( + tenant: $tenant, + baselineProfileId: (int) $profile->getKey(), + seenFingerprints: [], + currentOperationRunId: (int) $run->getKey(), + ))->toBe(1); + + $audit = AuditLog::query() + ->where('resource_type', 'finding') + ->where('resource_id', (string) $finding->getKey()) + ->where('action', AuditActionId::FindingResolved->value) + ->latest('id') + ->first(); + + expect($audit)->not->toBeNull() + ->and($audit->actorSnapshot()->type)->toBe(AuditActorType::System) + ->and(data_get($audit->metadata, 'system_origin'))->toBeTrue() + ->and(data_get($audit->metadata, 'resolved_reason'))->toBe('no_longer_drifting'); +}); + +it('writes system-origin audit rows for permission posture auto-resolve and recurrence reopen', function (): void { + [, $tenant] = createUserWithTenant(); + + CarbonImmutable::setTestNow(CarbonImmutable::parse('2026-03-18T10:00:00Z')); + $generator = app(PermissionPostureFindingGenerator::class); + + $generator->generate($tenant, [ + 'overall_status' => 'missing', + 'permissions' => [[ + 'key' => 'Perm.Automation', + 'type' => 'application', + 'status' => 'missing', + 'features' => ['policy-sync'], + ]], + 'last_refreshed_at' => now()->toIso8601String(), + ]); + + CarbonImmutable::setTestNow(CarbonImmutable::parse('2026-03-18T11:00:00Z')); + $generator->generate($tenant, [ + 'overall_status' => 'granted', + 'permissions' => [[ + 'key' => 'Perm.Automation', + 'type' => 'application', + 'status' => 'granted', + 'features' => ['policy-sync'], + ]], + 'last_refreshed_at' => now()->toIso8601String(), + ]); + + CarbonImmutable::setTestNow(CarbonImmutable::parse('2026-03-18T12:00:00Z')); + $generator->generate($tenant, [ + 'overall_status' => 'missing', + 'permissions' => [[ + 'key' => 'Perm.Automation', + 'type' => 'application', + 'status' => 'missing', + 'features' => ['policy-sync'], + ]], + 'last_refreshed_at' => now()->toIso8601String(), + ]); + + $finding = Finding::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('finding_type', Finding::FINDING_TYPE_PERMISSION_POSTURE) + ->firstOrFail(); + + $resolvedAudit = AuditLog::query() + ->where('resource_type', 'finding') + ->where('resource_id', (string) $finding->getKey()) + ->where('action', AuditActionId::FindingResolved->value) + ->latest('id') + ->first(); + + $reopenedAudit = AuditLog::query() + ->where('resource_type', 'finding') + ->where('resource_id', (string) $finding->getKey()) + ->where('action', AuditActionId::FindingReopened->value) + ->latest('id') + ->first(); + + expect($resolvedAudit)->not->toBeNull() + ->and($resolvedAudit->actorSnapshot()->type)->toBe(AuditActorType::System) + ->and(data_get($resolvedAudit->metadata, 'resolved_reason'))->toBe('permission_granted') + ->and($reopenedAudit)->not->toBeNull() + ->and($reopenedAudit->actorSnapshot()->type)->toBe(AuditActorType::System) + ->and(data_get($reopenedAudit->metadata, 'after_status'))->toBe(Finding::STATUS_REOPENED); +}); + +it('writes system-origin audit rows for entra admin role auto-resolve and recurrence reopen', function (): void { + [, $tenant] = createUserWithTenant(); + + $generator = new EntraAdminRolesFindingGenerator(new HighPrivilegeRoleCatalog); + + $generator->generate($tenant, [ + 'measured_at' => '2026-03-18T10:00:00Z', + 'role_definitions' => [[ + 'id' => 'def-ga', + 'displayName' => 'Global Administrator', + 'templateId' => '62e90394-69f5-4237-9190-012177145e10', + 'isBuiltIn' => true, + ]], + 'role_assignments' => [[ + 'id' => 'a1', + 'roleDefinitionId' => 'def-ga', + 'principalId' => 'user-1', + 'directoryScopeId' => '/', + 'principal' => [ + '@odata.type' => '#microsoft.graph.user', + 'displayName' => 'Alice', + ], + ]], + ]); + + $generator->generate($tenant, [ + 'measured_at' => '2026-03-18T11:00:00Z', + 'role_definitions' => [[ + 'id' => 'def-ga', + 'displayName' => 'Global Administrator', + 'templateId' => '62e90394-69f5-4237-9190-012177145e10', + 'isBuiltIn' => true, + ]], + 'role_assignments' => [], + ]); + + $generator->generate($tenant, [ + 'measured_at' => '2026-03-18T12:00:00Z', + 'role_definitions' => [[ + 'id' => 'def-ga', + 'displayName' => 'Global Administrator', + 'templateId' => '62e90394-69f5-4237-9190-012177145e10', + 'isBuiltIn' => true, + ]], + 'role_assignments' => [[ + 'id' => 'a2', + 'roleDefinitionId' => 'def-ga', + 'principalId' => 'user-1', + 'directoryScopeId' => '/', + 'principal' => [ + '@odata.type' => '#microsoft.graph.user', + 'displayName' => 'Alice Reactivated', + ], + ]], + ]); + + $finding = Finding::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('finding_type', Finding::FINDING_TYPE_ENTRA_ADMIN_ROLES) + ->where('subject_external_id', 'user-1:def-ga') + ->firstOrFail(); + + expect(AuditLog::query() + ->where('resource_type', 'finding') + ->where('resource_id', (string) $finding->getKey()) + ->where('action', AuditActionId::FindingResolved->value) + ->latest('id') + ->first()?->actorSnapshot()->type)->toBe(AuditActorType::System) + ->and(AuditLog::query() + ->where('resource_type', 'finding') + ->where('resource_id', (string) $finding->getKey()) + ->where('action', AuditActionId::FindingReopened->value) + ->latest('id') + ->first()?->actorSnapshot()->type)->toBe(AuditActorType::System); +}); + +it('writes a system-origin reopen audit row for baseline recurrence handling', function (): void { + [, $tenant] = createUserWithTenant(role: 'manager'); + + $profile = BaselineProfile::factory()->active()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + ]); + + $scopeKey = 'baseline_profile:'.$profile->getKey(); + $run1 = OperationRun::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + 'type' => 'baseline_compare', + ]); + + CarbonImmutable::setTestNow(CarbonImmutable::parse('2026-03-18T08:00:00Z')); + invokeAutomationBaselineCompareUpsertFindings( + new CompareBaselineToTenantJob($run1), + $tenant, + $profile, + $scopeKey, + [automationBaselineCompareDriftItem((int) $profile->getKey(), (int) $run1->getKey(), 'policy-audit-recur')], + ); + + $finding = Finding::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('source', 'baseline.compare') + ->where('subject_external_id', 'policy-audit-recur') + ->firstOrFail(); + + $finding->forceFill([ + 'status' => Finding::STATUS_RESOLVED, + 'resolved_at' => CarbonImmutable::parse('2026-03-18T09:00:00Z'), + 'resolved_reason' => 'fixed', + ])->save(); + + $run2 = OperationRun::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + 'type' => 'baseline_compare', + ]); + + CarbonImmutable::setTestNow(CarbonImmutable::parse('2026-03-18T10:00:00Z')); + invokeAutomationBaselineCompareUpsertFindings( + new CompareBaselineToTenantJob($run2), + $tenant, + $profile, + $scopeKey, + [automationBaselineCompareDriftItem((int) $profile->getKey(), (int) $run2->getKey(), 'policy-audit-recur')], + ); + + $audit = AuditLog::query() + ->where('resource_type', 'finding') + ->where('resource_id', (string) $finding->getKey()) + ->where('action', AuditActionId::FindingReopened->value) + ->latest('id') + ->first(); + + expect($audit)->not->toBeNull() + ->and($audit->actorSnapshot()->type)->toBe(AuditActorType::System) + ->and(data_get($audit->metadata, 'system_origin'))->toBeTrue(); +}); diff --git a/tests/Feature/Findings/FindingRbacTest.php b/tests/Feature/Findings/FindingRbacTest.php index ac4b401..b77756d 100644 --- a/tests/Feature/Findings/FindingRbacTest.php +++ b/tests/Feature/Findings/FindingRbacTest.php @@ -6,9 +6,11 @@ use App\Filament\Resources\FindingResource\Pages\ListFindings; use App\Models\Finding; use App\Services\Findings\FindingWorkflowService; +use App\Support\Workspaces\WorkspaceContext; use Filament\Facades\Filament; use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Gate; use Livewire\Livewire; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -83,3 +85,34 @@ expect($foreignFinding->fresh()?->status)->toBe(Finding::STATUS_NEW); }); + +it('denies finding view and triage as not found when tenant context workspace does not match the record workspace', function (): void { + $tenant = \App\Models\Tenant::factory()->create(); + [$user, $tenant] = createUserWithTenant(tenant: $tenant, role: 'owner'); + + $finding = Finding::make([ + 'tenant_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id + 999, + 'status' => Finding::STATUS_NEW, + ]); + + $this->actingAs($user); + Filament::setCurrentPanel('admin'); + Filament::setTenant(null, true); + Filament::bootCurrentPanel(); + + session()->put(WorkspaceContext::SESSION_KEY, (int) $tenant->workspace_id); + session()->put(WorkspaceContext::LAST_TENANT_IDS_SESSION_KEY, [ + (string) $tenant->workspace_id => (int) $tenant->getKey(), + ]); + + expect(Gate::forUser($user)->allows('view', $finding))->toBeFalse(); + + try { + Gate::forUser($user)->authorize('triage', $finding); + + $this->fail('Expected workspace-mismatched finding authorization to be denied as not found.'); + } catch (AuthorizationException $exception) { + expect($exception->status())->toBe(404); + } +}); diff --git a/tests/Feature/Findings/FindingWorkflowConcurrencyTest.php b/tests/Feature/Findings/FindingWorkflowConcurrencyTest.php new file mode 100644 index 0000000..1759fa1 --- /dev/null +++ b/tests/Feature/Findings/FindingWorkflowConcurrencyTest.php @@ -0,0 +1,183 @@ + + */ + function concurrencyBaselineCompareDriftItem(int $baselineProfileId, int $compareOperationRunId, string $subjectKey): array + { + return [ + 'change_type' => 'different_version', + 'severity' => Finding::SEVERITY_MEDIUM, + 'subject_type' => 'policy', + 'subject_external_id' => $subjectKey, + 'subject_key' => $subjectKey, + 'policy_type' => 'deviceConfiguration', + 'baseline_hash' => 'baseline', + 'current_hash' => 'current', + 'evidence_fidelity' => 'meta', + 'evidence' => [ + 'change_type' => 'different_version', + 'policy_type' => 'deviceConfiguration', + 'subject_key' => $subjectKey, + 'summary' => ['kind' => 'policy_snapshot'], + 'baseline' => ['policy_version_id' => null, 'hash' => 'baseline'], + 'current' => ['policy_version_id' => null, 'hash' => 'current'], + 'fidelity' => 'meta', + 'provenance' => [ + 'baseline_profile_id' => $baselineProfileId, + 'baseline_snapshot_id' => 1, + 'compare_operation_run_id' => $compareOperationRunId, + 'inventory_sync_run_id' => null, + ], + ], + ]; + } +} + +if (! function_exists('invokeConcurrencyBaselineCompareUpsertFindings')) { + /** + * @param array> $driftResults + * @return array{processed_count:int,created_count:int,reopened_count:int,unchanged_count:int,seen_fingerprints:array} + */ + function invokeConcurrencyBaselineCompareUpsertFindings( + CompareBaselineToTenantJob $job, + \App\Models\Tenant $tenant, + BaselineProfile $profile, + string $scopeKey, + array $driftResults, + ): array { + $reflection = new ReflectionMethod($job, 'upsertFindings'); + + /** @var array{processed_count:int,created_count:int,reopened_count:int,unchanged_count:int,seen_fingerprints:array} $result */ + $result = $reflection->invoke($job, $tenant, $profile, $scopeKey, $driftResults); + + return $result; + } +} + +it('does not reopen when a later human resolve beats an older automated recurrence observation', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'manager'); + + $profile = BaselineProfile::factory()->active()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + ]); + + $scopeKey = 'baseline_profile:'.$profile->getKey(); + $run1 = OperationRun::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + 'type' => 'baseline_compare', + ]); + + CarbonImmutable::setTestNow(CarbonImmutable::parse('2026-03-18T08:00:00Z')); + invokeConcurrencyBaselineCompareUpsertFindings( + new CompareBaselineToTenantJob($run1), + $tenant, + $profile, + $scopeKey, + [concurrencyBaselineCompareDriftItem((int) $profile->getKey(), (int) $run1->getKey(), 'policy-concurrency')], + ); + + $finding = Finding::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('source', 'baseline.compare') + ->firstOrFail(); + + CarbonImmutable::setTestNow(CarbonImmutable::parse('2026-03-18T10:00:00Z')); + app(FindingWorkflowService::class)->resolve($finding, $tenant, $user, 'human-won'); + + $run2 = OperationRun::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + 'type' => 'baseline_compare', + ]); + + CarbonImmutable::setTestNow(CarbonImmutable::parse('2026-03-18T09:00:00Z')); + $result = invokeConcurrencyBaselineCompareUpsertFindings( + new CompareBaselineToTenantJob($run2), + $tenant, + $profile, + $scopeKey, + [concurrencyBaselineCompareDriftItem((int) $profile->getKey(), (int) $run2->getKey(), 'policy-concurrency')], + ); + + expect($result['reopened_count'])->toBe(0) + ->and($finding->fresh()->status)->toBe(Finding::STATUS_RESOLVED) + ->and(AuditLog::query() + ->where('resource_type', 'finding') + ->where('resource_id', (string) $finding->getKey()) + ->where('action', AuditActionId::FindingReopened->value) + ->count())->toBe(0); +}); + +it('keeps closed findings terminal when recurrence is detected after a human close', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'manager'); + + $profile = BaselineProfile::factory()->active()->create([ + 'workspace_id' => (int) $tenant->workspace_id, + ]); + + $scopeKey = 'baseline_profile:'.$profile->getKey(); + $run1 = OperationRun::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + 'type' => 'baseline_compare', + ]); + + CarbonImmutable::setTestNow(CarbonImmutable::parse('2026-03-18T08:00:00Z')); + invokeConcurrencyBaselineCompareUpsertFindings( + new CompareBaselineToTenantJob($run1), + $tenant, + $profile, + $scopeKey, + [concurrencyBaselineCompareDriftItem((int) $profile->getKey(), (int) $run1->getKey(), 'policy-closed')], + ); + + $finding = Finding::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('source', 'baseline.compare') + ->where('subject_external_id', 'policy-closed') + ->firstOrFail(); + + CarbonImmutable::setTestNow(CarbonImmutable::parse('2026-03-18T09:00:00Z')); + app(FindingWorkflowService::class)->close($finding, $tenant, $user, 'accepted'); + + $run2 = OperationRun::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + 'type' => 'baseline_compare', + ]); + + CarbonImmutable::setTestNow(CarbonImmutable::parse('2026-03-18T10:00:00Z')); + $result = invokeConcurrencyBaselineCompareUpsertFindings( + new CompareBaselineToTenantJob($run2), + $tenant, + $profile, + $scopeKey, + [concurrencyBaselineCompareDriftItem((int) $profile->getKey(), (int) $run2->getKey(), 'policy-closed')], + ); + + expect($result['reopened_count'])->toBe(0) + ->and($finding->fresh()->status)->toBe(Finding::STATUS_CLOSED) + ->and(AuditLog::query() + ->where('resource_type', 'finding') + ->where('resource_id', (string) $finding->getKey()) + ->where('action', AuditActionId::FindingReopened->value) + ->count())->toBe(0); +}); diff --git a/tests/Feature/Findings/FindingWorkflowGuardTest.php b/tests/Feature/Findings/FindingWorkflowGuardTest.php new file mode 100644 index 0000000..3365974 --- /dev/null +++ b/tests/Feature/Findings/FindingWorkflowGuardTest.php @@ -0,0 +1,51 @@ +toContain(AuditActionId::FindingTriaged->value) + ->toContain(AuditActionId::FindingInProgress->value) + ->toContain(AuditActionId::FindingAssigned->value) + ->toContain(AuditActionId::FindingResolved->value) + ->toContain(AuditActionId::FindingClosed->value) + ->toContain(AuditActionId::FindingRiskAccepted->value) + ->toContain(AuditActionId::FindingReopened->value); +}); + +it('does not expose direct finding lifecycle mutators', function (): void { + expect(method_exists(Finding::class, 'acknowledge'))->toBeFalse() + ->and(method_exists(Finding::class, 'resolve'))->toBeFalse() + ->and(method_exists(Finding::class, 'reopen'))->toBeFalse(); +}); + +it('rolls back finding workflow persistence when audit logging fails', function (): void { + [$user, $tenant] = $this->actingAsFindingOperator('owner'); + + $finding = $this->makeFindingForWorkflow($tenant, Finding::STATUS_NEW); + + mock(AuditLogger::class, function (MockInterface $mock): void { + $mock->shouldReceive('log') + ->once() + ->andThrow(new \RuntimeException('Audit write unavailable.')); + }); + + expect(fn () => app(FindingWorkflowService::class)->triage($finding, $tenant, $user)) + ->toThrow(\RuntimeException::class, 'Audit write unavailable.'); + + expect($finding->refresh()->status)->toBe(Finding::STATUS_NEW) + ->and(AuditLog::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('resource_type', 'finding') + ->where('resource_id', (string) $finding->getKey()) + ->count())->toBe(0); +}); diff --git a/tests/Feature/Findings/FindingWorkflowServiceTest.php b/tests/Feature/Findings/FindingWorkflowServiceTest.php new file mode 100644 index 0000000..f656ca6 --- /dev/null +++ b/tests/Feature/Findings/FindingWorkflowServiceTest.php @@ -0,0 +1,98 @@ +makeFindingForWorkflow($tenant, Finding::STATUS_NEW); + $triagedFinding = $service->triage($newFinding, $tenant, $user); + + expect($triagedFinding->status)->toBe(Finding::STATUS_TRIAGED) + ->and($this->latestFindingAudit($triagedFinding, AuditActionId::FindingTriaged))->not->toBeNull(); + + $inProgressFinding = $service->startProgress($triagedFinding, $tenant, $user); + + expect($inProgressFinding->status)->toBe(Finding::STATUS_IN_PROGRESS) + ->and($this->latestFindingAudit($inProgressFinding, AuditActionId::FindingInProgress))->not->toBeNull(); + + $resolvedFinding = $service->resolve($inProgressFinding, $tenant, $user, 'patched'); + + expect($resolvedFinding->status)->toBe(Finding::STATUS_RESOLVED) + ->and($resolvedFinding->resolved_reason)->toBe('patched') + ->and($this->latestFindingAudit($resolvedFinding, AuditActionId::FindingResolved))->not->toBeNull(); + + $reopenedFinding = $service->reopen($resolvedFinding, $tenant, $user); + + expect($reopenedFinding->status)->toBe(Finding::STATUS_REOPENED) + ->and($reopenedFinding->reopened_at)->not->toBeNull() + ->and($this->latestFindingAudit($reopenedFinding, AuditActionId::FindingReopened))->not->toBeNull(); + + expect(fn () => $service->startProgress($this->makeFindingForWorkflow($tenant, Finding::STATUS_NEW), $tenant, $user)) + ->toThrow(\InvalidArgumentException::class, 'Finding cannot be moved to in-progress from the current status.'); +}); + +it('enforces tenant-member validation for owner and assignee reassignment', function (): void { + [$owner, $tenant] = createUserWithTenant(role: 'owner'); + $assignee = User::factory()->create(); + createUserWithTenant(tenant: $tenant, user: $assignee, role: 'operator'); + $outsider = User::factory()->create(); + + $finding = $this->makeFindingForWorkflow($tenant, Finding::STATUS_TRIAGED); + $service = app(FindingWorkflowService::class); + + $assignedFinding = $service->assign( + finding: $finding, + tenant: $tenant, + actor: $owner, + assigneeUserId: (int) $assignee->getKey(), + ownerUserId: (int) $owner->getKey(), + ); + + expect((int) $assignedFinding->assignee_user_id)->toBe((int) $assignee->getKey()) + ->and((int) $assignedFinding->owner_user_id)->toBe((int) $owner->getKey()) + ->and($this->latestFindingAudit($assignedFinding, AuditActionId::FindingAssigned))->not->toBeNull(); + + expect(fn () => $service->assign( + finding: $assignedFinding, + tenant: $tenant, + actor: $owner, + assigneeUserId: (int) $outsider->getKey(), + ownerUserId: (int) $owner->getKey(), + ))->toThrow(\InvalidArgumentException::class, 'assignee_user_id must reference a current tenant member.'); +}); + +it('requires explicit reasons for resolve close and risk accept mutations', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + + $service = app(FindingWorkflowService::class); + + expect(fn () => $service->resolve($this->makeFindingForWorkflow($tenant, Finding::STATUS_NEW), $tenant, $user, ' ')) + ->toThrow(\InvalidArgumentException::class, 'resolved_reason is required.'); + + expect(fn () => $service->close($this->makeFindingForWorkflow($tenant, Finding::STATUS_NEW), $tenant, $user, ' ')) + ->toThrow(\InvalidArgumentException::class, 'closed_reason is required.'); + + expect(fn () => $service->riskAccept($this->makeFindingForWorkflow($tenant, Finding::STATUS_NEW), $tenant, $user, ' ')) + ->toThrow(\InvalidArgumentException::class, 'closed_reason is required.'); +}); + +it('returns 403 for in-scope members without the required workflow capability', function (): void { + [$owner, $tenant] = createUserWithTenant(role: 'owner'); + [$readonly] = createUserWithTenant(tenant: $tenant, role: 'readonly'); + $finding = $this->makeFindingForWorkflow($tenant, Finding::STATUS_NEW); + + expect(fn () => app(FindingWorkflowService::class)->resolve($finding, $tenant, $readonly, 'patched')) + ->toThrow(AuthorizationException::class); + + expect(app(FindingWorkflowService::class)->resolve($finding, $tenant, $owner, 'patched')->status) + ->toBe(Finding::STATUS_RESOLVED); +}); diff --git a/tests/Feature/Findings/FindingWorkflowUiEnforcementTest.php b/tests/Feature/Findings/FindingWorkflowUiEnforcementTest.php new file mode 100644 index 0000000..917fcf7 --- /dev/null +++ b/tests/Feature/Findings/FindingWorkflowUiEnforcementTest.php @@ -0,0 +1,90 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $finding = $this->makeFindingForWorkflow($tenant, Finding::STATUS_NEW); + + Livewire::test(ListFindings::class) + ->assertTableActionVisible('triage', $finding) + ->assertTableActionDisabled('triage', $finding) + ->assertTableActionVisible('resolve', $finding) + ->assertTableActionDisabled('resolve', $finding); + + Livewire::test(ViewFinding::class, ['record' => $finding->getKey()]) + ->assertActionVisible('triage') + ->assertActionDisabled('triage') + ->assertActionVisible('resolve') + ->assertActionDisabled('resolve'); +}); + +it('preserves the expected workflow action surface by finding status', function (): void { + [$user, $tenant] = $this->actingAsFindingOperator('owner'); + + $newFinding = $this->makeFindingForWorkflow($tenant, Finding::STATUS_NEW); + $triagedFinding = $this->makeFindingForWorkflow($tenant, Finding::STATUS_TRIAGED); + $resolvedFinding = $this->makeFindingForWorkflow($tenant, Finding::STATUS_RESOLVED); + + Livewire::test(ListFindings::class) + ->assertTableActionVisible('triage', $newFinding) + ->assertTableActionHidden('start_progress', $newFinding) + ->assertTableActionVisible('start_progress', $triagedFinding) + ->assertTableActionHidden('reopen', $triagedFinding) + ->filterTable('open', false) + ->assertTableActionVisible('reopen', $resolvedFinding) + ->assertTableActionHidden('triage', $resolvedFinding) + ->assertTableActionHidden('close', $resolvedFinding) + ->assertTableActionHidden('risk_accept', $resolvedFinding); + + Livewire::test(ViewFinding::class, ['record' => $resolvedFinding->getKey()]) + ->assertActionVisible('reopen') + ->assertActionHidden('triage') + ->assertActionHidden('close') + ->assertActionHidden('risk_accept'); +}); + +it('returns 404 when forged foreign-tenant workflow actions are mounted for protected actions', function (): void { + $tenantA = Tenant::factory()->create(); + [$user, $tenantA] = createUserWithTenant(tenant: $tenantA, role: 'owner'); + + $tenantB = Tenant::factory()->create([ + 'workspace_id' => (int) $tenantA->workspace_id, + ]); + + createUserWithTenant(tenant: $tenantB, user: $user, role: 'owner'); + + $foreignFinding = $this->makeFindingForWorkflow($tenantB, Finding::STATUS_TRIAGED); + + $this->actingAs($user); + Filament::setCurrentPanel('admin'); + Filament::setTenant(null, true); + Filament::bootCurrentPanel(); + + session()->put(WorkspaceContext::SESSION_KEY, (int) $tenantA->workspace_id); + session()->put(WorkspaceContext::LAST_TENANT_IDS_SESSION_KEY, [ + (string) $tenantA->workspace_id => (int) $tenantA->getKey(), + ]); + + $component = Livewire::actingAs($user)->test(ListFindings::class); + + expect(fn () => $component->instance()->mountTableAction('start_progress', (string) $foreignFinding->getKey())) + ->toThrow(NotFoundHttpException::class); + + expect(fn () => $component->instance()->mountTableAction('risk_accept', (string) $foreignFinding->getKey())) + ->toThrow(NotFoundHttpException::class); + + expect($foreignFinding->refresh()->status)->toBe(Finding::STATUS_TRIAGED); +}); diff --git a/tests/Feature/Models/FindingResolvedTest.php b/tests/Feature/Models/FindingResolvedTest.php index ac90740..d0361ce 100644 --- a/tests/Feature/Models/FindingResolvedTest.php +++ b/tests/Feature/Models/FindingResolvedTest.php @@ -3,15 +3,16 @@ declare(strict_types=1); use App\Models\Finding; -use App\Models\User; +use App\Services\Findings\FindingWorkflowService; use Illuminate\Foundation\Testing\RefreshDatabase; uses(RefreshDatabase::class); it('resolves a finding with reason', function (): void { - $finding = Finding::factory()->permissionPosture()->create(); + [$user, $tenant] = createUserWithTenant(role: 'owner'); + $finding = Finding::factory()->for($tenant)->permissionPosture()->create(); - $finding->resolve('permission_granted'); + $finding = app(FindingWorkflowService::class)->resolve($finding, $tenant, $user, 'permission_granted'); expect($finding->status)->toBe(Finding::STATUS_RESOLVED) ->and($finding->resolved_at)->not->toBeNull() @@ -22,24 +23,16 @@ ->and($fresh->resolved_at)->not->toBeNull(); }); -it('reopens a resolved finding (legacy model helper compatibility)', function (): void { - $finding = Finding::factory()->permissionPosture()->resolved()->create(); +it('reopens a resolved finding through the workflow service', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + $finding = Finding::factory()->for($tenant)->permissionPosture()->resolved()->create(); - $newEvidence = [ - 'permission_key' => 'DeviceManagementConfiguration.ReadWrite.All', - 'permission_type' => 'application', - 'expected_status' => 'granted', - 'actual_status' => 'missing', - 'blocked_features' => ['policy-sync'], - 'checked_at' => now()->toIso8601String(), - ]; + $finding = app(FindingWorkflowService::class)->reopen($finding, $tenant, $user); - $finding->reopen($newEvidence); - - expect($finding->status)->toBe(Finding::STATUS_NEW) + expect($finding->status)->toBe(Finding::STATUS_REOPENED) + ->and($finding->reopened_at)->not->toBeNull() ->and($finding->resolved_at)->toBeNull() - ->and($finding->resolved_reason)->toBeNull() - ->and($finding->evidence_jsonb)->toBe($newEvidence); + ->and($finding->resolved_reason)->toBeNull(); }); it('exposes v2 open and terminal status helpers', function (): void { @@ -68,13 +61,14 @@ }); it('preserves acknowledged metadata when resolving an acknowledged finding', function (): void { - $user = User::factory()->create(); - $finding = Finding::factory()->permissionPosture()->create(); + [$user, $tenant] = createUserWithTenant(role: 'owner'); + $finding = Finding::factory()->for($tenant)->permissionPosture()->acknowledged()->create([ + 'acknowledged_by_user_id' => $user->getKey(), + ]); - $finding->acknowledge($user); expect($finding->status)->toBe(Finding::STATUS_ACKNOWLEDGED); - $finding->resolve('permission_granted'); + $finding = app(FindingWorkflowService::class)->resolve($finding, $tenant, $user, 'permission_granted'); expect($finding->status)->toBe(Finding::STATUS_RESOLVED) ->and($finding->acknowledged_at)->not->toBeNull() diff --git a/tests/Feature/PermissionPosture/PermissionPostureFindingGeneratorTest.php b/tests/Feature/PermissionPosture/PermissionPostureFindingGeneratorTest.php index a8a1310..f40f628 100644 --- a/tests/Feature/PermissionPosture/PermissionPostureFindingGeneratorTest.php +++ b/tests/Feature/PermissionPosture/PermissionPostureFindingGeneratorTest.php @@ -113,7 +113,11 @@ function errorPermission(string $key, array $features = []): array $finding = Finding::query()->where('tenant_id', $tenant->getKey())->first(); $ackUser = User::factory()->create(); - $finding->acknowledge($ackUser); + $finding->forceFill([ + 'status' => Finding::STATUS_ACKNOWLEDGED, + 'acknowledged_at' => now(), + 'acknowledged_by_user_id' => $ackUser->getKey(), + ])->save(); $result = $generator->generate($tenant, buildComparison([grantedPermission('Perm.A')], 'granted')); diff --git a/tests/Pest.php b/tests/Pest.php index f3dc653..3232a1c 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -19,6 +19,7 @@ use App\Support\Workspaces\WorkspaceContext; use Illuminate\Foundation\Testing\RefreshDatabase; use Pest\PendingCalls\TestCall; +use Tests\Feature\Findings\Concerns\InteractsWithFindingsWorkflow; use Tests\Support\AssertsNoOutboundHttp; use Tests\Support\FailHardGraphClient; @@ -37,6 +38,9 @@ ->use(RefreshDatabase::class) ->in('Feature'); +pest()->use(InteractsWithFindingsWorkflow::class) + ->in('Feature/Findings', 'Feature/Audit'); + pest()->extend(Tests\TestCase::class) ->use(RefreshDatabase::class) ->in('Browser');