From f68da3b15fd3f7c37b561a2fd1d56f40d71e9938 Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Mon, 12 Jan 2026 00:57:24 +0100 Subject: [PATCH 1/6] spec(044): define generic findings pipeline for drift MVP --- specs/044-drift-mvp/spec.md | 97 +++++++++++++++++++++++++++++++++++-- 1 file changed, 92 insertions(+), 5 deletions(-) diff --git a/specs/044-drift-mvp/spec.md b/specs/044-drift-mvp/spec.md index b367de3..05bb9ab 100644 --- a/specs/044-drift-mvp/spec.md +++ b/specs/044-drift-mvp/spec.md @@ -10,6 +10,54 @@ ## Purpose This MVP focuses on reporting and triage, not automatic remediation. +## Clarifications + +### Session 2026-01-12 + +- Q: How should Drift pick the baseline run for a given tenant + scope? → A: Baseline = previous successful inventory run for the same scope; compare against the latest successful run. +- Q: Should Drift findings be persisted or computed on demand? → A: Persist findings in DB per comparison (baseline_run_id + current_run_id), including a deterministic fingerprint for stable identity + triage. +- Q: How define the fingerprint (Stable ID) for a drift finding? → A: `sha256(tenant_id + scope_key + subject_type + subject_external_id + change_type + baseline_hash + current_hash)` (normalized; excludes volatile fields). +- Q: Which inventory entities/types are in scope for Drift MVP? → A: Policies + Assignments. +- Q: When should drift findings be generated? → A: On-demand when opening Drift: if findings for (baseline,current,scope) don’t exist yet, dispatch an async job to generate them. + +## Pinned Decisions (MVP defaults) + +- Drift is implemented as a generator that writes persisted Finding rows (not only an in-memory/on-demand diff). +- Baseline selection: baseline = previous successful inventory run for the same scope_key; comparison = latest successful inventory run for the same scope_key. +- Scope is first-class via `scope_key` and must be deterministic to support future pinned baselines and compare workflows. +- Fingerprints are deterministic and stable for triage/audit workflows. +- Drift MVP only uses `finding_type=drift` and `status` in {`new`, `acknowledged`}. +- Default severity: `medium` (until a rule engine exists). +- UI must not perform render-time Graph calls. Graph access (if any) is limited to background sync/jobs. + +## Key Entities / Generic Findings (Future-proof) + +### Finding (generic) + +We want Drift MVP to remain MVP-sized, while making it easy to add future generators (Security Suite Audits, Cross-tenant Compare) without inventing a new model. + +Rationale: +- Drift = delta engine over runs. +- Audit = rule engine over inventory. +- Both write Findings with the same semantics: deterministic fingerprint + triage + minimized evidence. + +- `finding_type` (enum): `drift` (MVP), later `audit`, `compare` +- `tenant_id` +- `scope_key` (string): deterministic scope identifier (see Scope Definition / FR1) +- `baseline_run_id` (nullable; e.g. audit/compare) +- `current_run_id` (nullable; e.g. audit) +- `fingerprint` (string): deterministic; unique per tenant+scope+subject+change +- `subject_type` (string): e.g. policy type (or other inventory entity type) +- `subject_external_id` (string): Graph external id +- `severity` (enum): `low` / `medium` / `high` (MVP default: `medium`) +- `status` (enum): `new` / `acknowledged` (later: `snoozed` / `assigned` / `commented`) +- `acknowledged_at` (nullable) +- `acknowledged_by_user_id` (nullable) +- `evidence_jsonb` (jsonb): sanitized, small, secrets-free (no raw payload dumps) +- Optional/nullable for later (prepared; out of MVP): `rule_id`, `control_id`, `expected_value`, `source` + +MVP implementation scope: only `finding_type=drift`, statuses `new/acknowledged`, and no rule engine. + ## User Scenarios & Testing ### Scenario 1: View drift summary @@ -29,24 +77,63 @@ ### Scenario 3: Acknowledge/triage ## Functional Requirements -- FR1: Define a baseline concept (e.g., last completed run for a selection scope). -- FR2: Produce drift findings for adds/removals/metadata changes based on inventory/run state. -- FR3: Provide drift UI with summary and details. -- FR4: Allow acknowledgement/triage states. +- FR1: Baseline + scope + - Define `scope_key` as a deterministic string derived from the Inventory Selection. + - Example: `scope_key = sha256(normalized selection payload)`. + - Must remain stable across equivalent selections (normalization), and allow future pinned baselines / compare baselines. + - Baseline run (MVP) = previous successful inventory run for the same `scope_key`. + - Comparison run (MVP) = latest successful inventory run for the same `scope_key`. + +- FR2: Finding generation (Drift MVP) + - Findings are persisted per (`baseline_run_id`, `current_run_id`, `scope_key`). + - Findings cover adds, removals, and metadata changes for supported entities (Policies + Assignments). + - Findings are deterministic: same baseline/current + scope_key ⇒ same set of fingerprints. + +- FR2a: Fingerprint definition (MVP) + - Fingerprint = `sha256(tenant_id + scope_key + subject_type + subject_external_id + change_type + baseline_hash + current_hash)`. + - `baseline_hash` / `current_hash` are hashes over normalized, sanitized comparison data (exclude volatile fields like timestamps). + - Goal: stable identity for triage + audit compatibility. + +- FR2b: Drift MVP scope includes Policies and their Assignments. + - Assignment drift includes target changes (e.g., groupId) and intent changes. + +- FR3: Provide Drift UI with summary and details. + +- FR4: Triage (MVP) + - Admin can acknowledge a finding; record `acknowledged_by_user_id` + `acknowledged_at`. + - Findings are never deleted in the MVP. ## Non-Functional Requirements - NFR1: Drift generation must be deterministic for the same baseline and scope. - NFR2: Drift must remain tenant-scoped and safe to display. +- NFR3: Evidence minimization + - `evidence_jsonb` must be sanitized (no tokens/secrets) and kept small. + - MVP drift evidence should include only: + - `change_type` + - changed_fields / metadata summary (counts, field list) + - run refs (baseline_run_id/current_run_id, timestamps) + - No raw payload dumps. + +## Dependencies / Name Resolution + +- Drift/Audit UI should resolve labels via Inventory + Foundations (047) + Groups Cache (051) where applicable. +- No render-time Graph calls (Graph only in background sync/jobs, never in UI render). ## Success Criteria -- SC1: Admins can identify drift across supported types in under 3 minutes. +- SC1: Admins can identify drift across supported types (Policies + Assignments) in under 3 minutes. - SC2: Drift results are consistent across repeated generation for the same baseline. ## Out of Scope - Automatic revert/promotion. +- Rule engine in MVP (Audit later), but the data model is prepared via `rule_id` / `control_id` / `expected_value`. + +## Future Work (non-MVP) + +- Security Suite Audits: add rule-based generators that write Findings (no new Finding model). +- Cross-tenant Compare: may write Findings (`finding_type=compare`) or emit a compatible format that can be stored as Findings. ## Related Specs -- 2.45.2 From df18cb1a0d30edd1da2380ba7491ce1147dfd084 Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Tue, 13 Jan 2026 23:28:02 +0100 Subject: [PATCH 2/6] spec(044): finalize drift MVP docs --- .../044-drift-mvp/checklists/requirements.md | 35 ++++ .../contracts/admin-findings.openapi.yaml | 146 +++++++++++++++++ specs/044-drift-mvp/data-model.md | 57 +++++++ specs/044-drift-mvp/plan.md | 117 +++++++++++-- specs/044-drift-mvp/quickstart.md | 30 ++++ specs/044-drift-mvp/research.md | 72 ++++++++ specs/044-drift-mvp/spec.md | 26 ++- specs/044-drift-mvp/tasks.md | 155 +++++++++++++++++- 8 files changed, 614 insertions(+), 24 deletions(-) create mode 100644 specs/044-drift-mvp/checklists/requirements.md create mode 100644 specs/044-drift-mvp/contracts/admin-findings.openapi.yaml create mode 100644 specs/044-drift-mvp/data-model.md create mode 100644 specs/044-drift-mvp/quickstart.md create mode 100644 specs/044-drift-mvp/research.md diff --git a/specs/044-drift-mvp/checklists/requirements.md b/specs/044-drift-mvp/checklists/requirements.md new file mode 100644 index 0000000..5e432f1 --- /dev/null +++ b/specs/044-drift-mvp/checklists/requirements.md @@ -0,0 +1,35 @@ +# Specification Quality Checklist: Drift MVP (044) + +**Purpose**: Validate specification completeness and quality before proceeding to implementation +**Created**: 2026-01-12 +**Feature**: [specs/044-drift-mvp/spec.md](../spec.md) + +## Content Quality + +- [ ] No implementation details (languages, frameworks, APIs) +- [ ] Focused on user value and business needs +- [ ] Written for non-technical stakeholders +- [ ] All mandatory sections completed + +## Requirement Completeness + +- [ ] No [NEEDS CLARIFICATION] markers remain +- [ ] Requirements are testable and unambiguous +- [ ] Success criteria are measurable +- [ ] Success criteria are technology-agnostic (no implementation details) +- [ ] All acceptance scenarios are defined +- [ ] Edge cases are identified +- [ ] Scope is clearly bounded +- [ ] Dependencies and assumptions identified + +## Feature Readiness + +- [ ] All functional requirements have clear acceptance criteria +- [ ] User scenarios cover primary flows +- [ ] Feature meets measurable outcomes defined in Success Criteria +- [ ] No implementation details leak into specification + +## Notes + +- Items marked incomplete require spec updates before `/speckit.clarify` or `/speckit.plan`. +- Constitution gate: this checklist must exist for features that change runtime behavior. diff --git a/specs/044-drift-mvp/contracts/admin-findings.openapi.yaml b/specs/044-drift-mvp/contracts/admin-findings.openapi.yaml new file mode 100644 index 0000000..3c742bd --- /dev/null +++ b/specs/044-drift-mvp/contracts/admin-findings.openapi.yaml @@ -0,0 +1,146 @@ +openapi: 3.0.3 +info: + title: Admin Findings API (Internal) + version: 0.1.0 + description: | + Internal contracts for the generic Findings pipeline. + Drift MVP is the first generator (finding_type=drift). + +servers: + - url: /admin/api + +paths: + /findings: + get: + summary: List findings + parameters: + - in: query + name: finding_type + schema: + type: string + enum: [drift, audit, compare] + - in: query + name: status + schema: + type: string + enum: [new, acknowledged] + - in: query + name: scope_key + schema: + type: string + - in: query + name: current_run_id + schema: + type: integer + responses: + '200': + description: OK + content: + application/json: + schema: + type: object + properties: + data: + type: array + items: + $ref: '#/components/schemas/Finding' + + /findings/{id}: + get: + summary: Get finding detail + parameters: + - in: path + name: id + required: true + schema: + type: integer + responses: + '200': + description: OK + content: + application/json: + schema: + type: object + properties: + data: + $ref: '#/components/schemas/Finding' + + /findings/{id}/acknowledge: + post: + summary: Acknowledge a finding + parameters: + - in: path + name: id + required: true + schema: + type: integer + responses: + '200': + description: OK + content: + application/json: + schema: + type: object + properties: + data: + $ref: '#/components/schemas/Finding' + + /drift/generate: + post: + summary: Generate drift findings (async) + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + scope_key: + type: string + description: Inventory selection hash + required: [scope_key] + responses: + '202': + description: Accepted + +components: + schemas: + Finding: + type: object + properties: + id: + type: integer + finding_type: + type: string + enum: [drift, audit, compare] + tenant_id: + type: integer + scope_key: + type: string + baseline_run_id: + type: integer + nullable: true + current_run_id: + type: integer + nullable: true + fingerprint: + type: string + subject_type: + type: string + subject_external_id: + type: string + severity: + type: string + enum: [low, medium, high] + status: + type: string + enum: [new, acknowledged] + acknowledged_at: + type: string + nullable: true + acknowledged_by_user_id: + type: integer + nullable: true + evidence_jsonb: + type: object + additionalProperties: true diff --git a/specs/044-drift-mvp/data-model.md b/specs/044-drift-mvp/data-model.md new file mode 100644 index 0000000..80a2b73 --- /dev/null +++ b/specs/044-drift-mvp/data-model.md @@ -0,0 +1,57 @@ +# Phase 1 Design: Data Model (044) + +## Entities + +### Finding + +New table: `findings` + +**Purpose**: Generic, persisted pipeline for analytic findings (Drift now; Audit/Compare later). + +**Core fields (MVP)** +- `id` (pk) +- `tenant_id` (fk tenants) +- `finding_type` (`drift` in MVP; later `audit`/`compare`) +- `scope_key` (string; deterministic; reuse Inventory selection hash) +- `baseline_run_id` (nullable fk inventory_sync_runs) +- `current_run_id` (nullable fk inventory_sync_runs) +- `fingerprint` (string; deterministic) +- `subject_type` (string; e.g. policy type) +- `subject_external_id` (string; Graph external id) +- `severity` (`low|medium|high`; MVP default `medium`) +- `status` (`new|acknowledged`) +- `acknowledged_at` (nullable) +- `acknowledged_by_user_id` (nullable fk users) +- `evidence_jsonb` (jsonb; sanitized, small; allowlist) + +**Prepared for later (nullable, out of MVP)** +- `rule_id`, `control_id`, `expected_value`, `source` + +## Constraints & Indexes + +**Uniqueness** +- Unique: `(tenant_id, fingerprint)` + +**Lookup indexes (suggested)** +- `(tenant_id, finding_type, status)` +- `(tenant_id, scope_key)` +- `(tenant_id, current_run_id)` +- `(tenant_id, baseline_run_id)` +- `(tenant_id, subject_type, subject_external_id)` + +## Relationships + +- `Finding` belongs to `Tenant`. +- `Finding` belongs to `User` via `acknowledged_by_user_id`. +- `Finding` belongs to `InventorySyncRun` via `baseline_run_id` (nullable) and `current_run_id` (nullable). + +## Evidence shape (MVP allowlist) + +For Drift MVP, `evidence_jsonb` should contain only: +- `change_type` +- `changed_fields` (list) and/or `change_counts` +- `run`: + - `baseline_run_id`, `current_run_id` + - `baseline_finished_at`, `current_finished_at` + +No raw policy payload dumps; exclude secrets/tokens; exclude volatile fields for hashing. diff --git a/specs/044-drift-mvp/plan.md b/specs/044-drift-mvp/plan.md index 6bffb81..81fe07e 100644 --- a/specs/044-drift-mvp/plan.md +++ b/specs/044-drift-mvp/plan.md @@ -1,24 +1,113 @@ -# Implementation Plan: Drift MVP +# Implementation Plan: Drift MVP (044) -**Date**: 2026-01-07 -**Spec**: `specs/044-drift-mvp/spec.md` +**Branch**: `feat/044-drift-mvp` | **Date**: 2026-01-12 | **Spec**: `specs/044-drift-mvp/spec.md` +**Input**: Feature specification from `specs/044-drift-mvp/spec.md` ## Summary -Add drift findings generation and UI using inventory and sync run metadata. +Introduce a generic, persisted Finding pipeline and implement Drift as the first generator. -## Dependencies +- Drift compares Inventory Sync Runs for the same selection scope (`scope_key`). +- Baseline run = previous successful run for the same scope; comparison run = latest successful run. +- Findings are persisted with deterministic fingerprints and support MVP triage (`new` → `acknowledged`). +- UI is DB-only for label/name resolution (no render-time Graph calls). -- Inventory core + run tracking (Spec 040) -- Inventory UI patterns (Spec 041) +## Technical Context -## Deliverables +**Language/Version**: PHP 8.4.x +**Framework**: Laravel 12 +**Admin UI**: Filament v4 + Livewire v3 +**Storage**: PostgreSQL (JSONB) +**Testing**: Pest v4 +**Target Platform**: Docker (Sail-first local), Dokploy container deployments +**Project Type**: Laravel monolith +**Performance Goals**: +- Drift generation happens async (job), with deterministic output +- Drift listing remains filterable and index-backed +**Constraints**: +- Tenant isolation for all reads/writes +- No render-time Graph calls; labels resolved from DB caches +- Evidence minimization (sanitized allowlist; no raw payload dumps) +**Scale/Scope**: +- Tenants may have large inventories; findings must be indexed for typical filtering -- Baseline definition and drift finding generation -- Drift summary + detail UI -- Acknowledge/triage actions +## Constitution Check -## Risks +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* -- False positives if baseline definition is unclear -- Data volume for large tenants +- Inventory-first: Drift is derived from Inventory Sync Runs and Inventory Items (“last observed” state). +- Read/write separation: Drift generation is analytical; writes are limited to triage acknowledgement and must be audited + tested. +- Graph contract path: Drift UI performs no Graph calls; Graph calls remain isolated in existing Inventory/Graph client layers. +- Deterministic capabilities: drift scope derives from existing selection hashing and inventory type registries. +- Tenant isolation: all reads/writes tenant-scoped; no cross-tenant leakage. +- Automation: drift generation is queued; jobs are deduped/locked per scope+run pair and observable. +- Data minimization: store only minimized evidence JSON; logs contain no secrets/tokens. + +## Project Structure + +### Documentation (this feature) + +```text +specs/044-drift-mvp/ +├── plan.md # This file (/speckit.plan output) +├── research.md # Phase 0 output +├── data-model.md # Phase 1 output +├── quickstart.md # Phase 1 output +├── contracts/ # Phase 1 output +│ └── admin-findings.openapi.yaml +└── tasks.md # Phase 2 output (/speckit.tasks) +``` + +### Source Code (repository root) + +```text +app/ +├── Filament/ +│ ├── Pages/ +│ │ └── DriftLanding.php # drift landing page (summary + generation status) +│ └── Resources/ +│ └── FindingResource/ # list/detail + acknowledge action (tenant-scoped) +├── Jobs/ +│ └── GenerateDriftFindingsJob.php # async generator (on-demand) +├── Models/ +│ └── Finding.php # generic finding model +└── Services/ + └── Drift/ + ├── DriftFindingGenerator.php # computes deterministic findings for baseline/current + ├── DriftHasher.php # baseline_hash/current_hash helpers + └── DriftScopeKey.php # scope_key is InventorySyncRun.selection_hash (single canonical definition) + +database/migrations/ +└── 2026_.._.._create_findings_table.php + +tests/Feature/Drift/ +├── DriftGenerationDeterminismTest.php +├── DriftTenantIsolationTest.php +└── DriftAcknowledgeTest.php +``` + +**Structure Decision**: Laravel monolith using Filament pages/resources and queued jobs. + +## Complexity Tracking + +> **Fill ONLY if Constitution Check has violations that must be justified** + +| Violation | Why Needed | Simpler Alternative Rejected Because | +|-----------|------------|-------------------------------------| +| (none) | | | + +## Phase 0 Output (Research) + +Completed in `specs/044-drift-mvp/research.md`. + +## Phase 1 Output (Design) + +Completed in: + +- `specs/044-drift-mvp/data-model.md` +- `specs/044-drift-mvp/contracts/` +- `specs/044-drift-mvp/quickstart.md` + +## Phase 2 Planning Notes + +Next step is expanding `specs/044-drift-mvp/tasks.md` (via `/speckit.tasks`) with phased, test-first implementation tasks. diff --git a/specs/044-drift-mvp/quickstart.md b/specs/044-drift-mvp/quickstart.md new file mode 100644 index 0000000..2e709a3 --- /dev/null +++ b/specs/044-drift-mvp/quickstart.md @@ -0,0 +1,30 @@ +# Quickstart: Drift MVP (044) + +## Run locally (Sail) + +```bash +./vendor/bin/sail up -d +./vendor/bin/sail artisan queue:work --tries=1 +``` + +## Prepare data + +1. Open the admin panel and select a tenant context. +2. Navigate to Inventory and run an Inventory Sync **twice** with the same selection (same `selection_hash`). + +## Use Drift + +1. Navigate to the new Drift area. +2. On first open, Drift will dispatch a background job to generate findings for: + - baseline = previous successful run for the same `scope_key` + - current = latest successful run for the same `scope_key` +3. Refresh the page once the job finishes. + +## Triage + +- Acknowledge a finding; it should move out of the “new” view but remain visible/auditable. + +## Notes + +- UI must remain DB-only for label resolution (no render-time Graph calls). +- Findings store minimal, sanitized evidence only. diff --git a/specs/044-drift-mvp/research.md b/specs/044-drift-mvp/research.md new file mode 100644 index 0000000..0ef8180 --- /dev/null +++ b/specs/044-drift-mvp/research.md @@ -0,0 +1,72 @@ +# Phase 0 Output: Research (044) + +## Decisions + +### 1) `scope_key` reuse + +- Decision: Use the existing Inventory selection hash as `scope_key`. + - Concretely: `scope_key = InventorySyncRun.selection_hash`. +- Rationale: + - Inventory already normalizes + hashes selection payload deterministically (via `InventorySelectionHasher`). + - It is already used for concurrency/deduping inventory runs, so it’s the right stable scope identifier. +- Alternatives considered: + - Compute a second hash (duplicate of selection_hash) → adds drift without benefit. + - Store the raw selection payload as the primary key → not stable without strict normalization. + +### 2) Baseline selection (MVP) + +- Decision: Baseline run = previous successful inventory sync run for the same `scope_key`; comparison run = latest successful inventory sync run for the same `scope_key`. +- Rationale: + - Matches “run at least twice” scenario. + - Deterministic and explainable. +- Alternatives considered: + - User-pinned baselines → valuable, but deferred (design must allow later via `scope_key`). + +### 3) Persisted generic Findings + +- Decision: Persist Findings in a generic `findings` table. +- Rationale: + - Enables stable triage (`acknowledged`) without recomputation drift. + - Reusable pipeline for Drift now, Audit/Compare later. +- Alternatives considered: + - Compute-on-demand and store only acknowledgements by fingerprint → harder operationally and can surprise users when diff rules evolve. + +### 4) Generation trigger (MVP) + +- Decision: On opening Drift, if findings for (tenant, `scope_key`, baseline_run_id, current_run_id) do not exist, dispatch an async job to generate them. +- Rationale: + - Avoids long request times. + - Avoids scheduled complexity in MVP. +- Alternatives considered: + - Generate after every inventory run → may be expensive; can be added later. + - Nightly schedule → hides immediacy and complicates operations. + +### 5) Fingerprint and state hashing + +- Decision: Use a deterministic fingerprint that changes when the underlying state changes. + - Fingerprint = `sha256(tenant_id + scope_key + subject_type + subject_external_id + change_type + baseline_hash + current_hash)`. + - baseline_hash/current_hash are computed over normalized, sanitized comparison data (exclude volatile fields like timestamps). +- Rationale: + - Stable identity for triage and audit. + - Supports future generators (audit/compare) using same semantics. +- Alternatives considered: + - Fingerprint without baseline/current hash → cannot distinguish changed vs unchanged findings. + +### 6) Evidence minimization + +- Decision: Store small, sanitized `evidence_jsonb` with an allowlist shape; no raw payload dumps. +- Rationale: + - Aligns with data minimization + safe logging. + - Avoids storing secrets/tokens. + +### 7) Name resolution and Graph safety + +- Decision: UI resolves human-readable labels using DB-backed Inventory + Foundations (047) + Groups Cache (051). No render-time Graph calls. +- Rationale: + - Works offline / when tokens are broken. + - Keeps UI safe and predictable. + +## Notes / Follow-ups for Phase 1 + +- Define the `findings` table indexes carefully for tenant-scoped filtering (status, type, scope_key, run_ids). +- Consider using existing observable run patterns (BulkOperationRun + AuditLogger) for drift generation jobs. diff --git a/specs/044-drift-mvp/spec.md b/specs/044-drift-mvp/spec.md index 05bb9ab..27031cb 100644 --- a/specs/044-drift-mvp/spec.md +++ b/specs/044-drift-mvp/spec.md @@ -20,6 +20,14 @@ ### Session 2026-01-12 - Q: Which inventory entities/types are in scope for Drift MVP? → A: Policies + Assignments. - Q: When should drift findings be generated? → A: On-demand when opening Drift: if findings for (baseline,current,scope) don’t exist yet, dispatch an async job to generate them. +### Session 2026-01-13 + +- Q: What should Drift do if there are fewer than two successful inventory runs for the same `scope_key`? → A: Show a blocked/empty state (“Need at least 2 successful runs for this scope to calculate drift”) and do not dispatch drift generation. +- Q: Should acknowledgement carry forward across comparisons? → A: No; acknowledgement is per comparison (`baseline_run_id` + `current_run_id` + `scope_key`). The same drift may re-appear as `new` in later comparisons. +- Q: Which `change_type` values are supported in Drift MVP? → A: `added`, `removed`, `modified` (assignment target/intent changes are covered under `modified`). +- Q: What is the default UI behavior for `new` vs `acknowledged` findings? → A: Default UI shows only `new`; `acknowledged` is accessible via an explicit filter. +- Q: What should the UI do if drift generation fails for a comparison? → A: Show an explicit error state (safe message + reference/run ids) and do not show findings for that comparison until a successful generation exists. + ## Pinned Decisions (MVP defaults) - Drift is implemented as a generator that writes persisted Finding rows (not only an in-memory/on-demand diff). @@ -65,6 +73,8 @@ ### Scenario 1: View drift summary - When the admin opens Drift - Then they see a summary of changes since the last baseline +- If there are fewer than two successful runs for the same `scope_key`, Drift shows a blocked/empty state and does not start drift generation. + ### Scenario 2: Drill into a drift finding - Given a drift finding exists - When the admin opens the finding @@ -75,19 +85,23 @@ ### Scenario 3: Acknowledge/triage - When the admin marks it acknowledged - Then it is hidden from “new” lists but remains auditable +- Acknowledgement is per comparison; later comparisons may still surface the same drift as `new`. + ## Functional Requirements - FR1: Baseline + scope - - Define `scope_key` as a deterministic string derived from the Inventory Selection. - - Example: `scope_key = sha256(normalized selection payload)`. - - Must remain stable across equivalent selections (normalization), and allow future pinned baselines / compare baselines. + - Define `scope_key` as the deterministic Inventory selection identifier. + - MVP definition: `scope_key = InventorySyncRun.selection_hash`. + - Rationale: selection hashing already normalizes equivalent selections; reusing it keeps drift scope stable and consistent across the product. - Baseline run (MVP) = previous successful inventory run for the same `scope_key`. - Comparison run (MVP) = latest successful inventory run for the same `scope_key`. - FR2: Finding generation (Drift MVP) - Findings are persisted per (`baseline_run_id`, `current_run_id`, `scope_key`). - - Findings cover adds, removals, and metadata changes for supported entities (Policies + Assignments). + - Findings cover adds, removals, and changes for supported entities (Policies + Assignments). + - MVP `change_type` values: `added`, `removed`, `modified`. - Findings are deterministic: same baseline/current + scope_key ⇒ same set of fingerprints. + - If fewer than two successful inventory runs exist for a given `scope_key`, Drift does not generate findings and must surface a clear blocked/empty state in the UI. - FR2a: Fingerprint definition (MVP) - Fingerprint = `sha256(tenant_id + scope_key + subject_type + subject_external_id + change_type + baseline_hash + current_hash)`. @@ -98,9 +112,13 @@ ## Functional Requirements - Assignment drift includes target changes (e.g., groupId) and intent changes. - FR3: Provide Drift UI with summary and details. + - Default lists and the Drift landing summary show only `status=new` by default. + - The UI must provide a filter to include `acknowledged` findings. + - If drift generation fails for a comparison, the UI must surface an explicit error state (no secrets), including reference identifiers (e.g., run ids), and must not fall back to stale/previous results. - FR4: Triage (MVP) - Admin can acknowledge a finding; record `acknowledged_by_user_id` + `acknowledged_at`. + - Acknowledgement does not carry forward across comparisons in the MVP. - Findings are never deleted in the MVP. ## Non-Functional Requirements diff --git a/specs/044-drift-mvp/tasks.md b/specs/044-drift-mvp/tasks.md index 7ccfb97..481f990 100644 --- a/specs/044-drift-mvp/tasks.md +++ b/specs/044-drift-mvp/tasks.md @@ -1,7 +1,150 @@ -# Tasks: Drift MVP +--- -- [ ] T001 Define baseline and scope rules -- [ ] T002 Drift finding generation (deterministic) -- [ ] T003 Drift summary + detail UI -- [ ] T004 Acknowledge/triage state -- [ ] T005 Tests for determinism and tenant scoping +description: "Task list for feature 044 drift MVP" + +--- + +# Tasks: Drift MVP (044) + +**Input**: Design documents from `specs/044-drift-mvp/` +**Prerequisites**: `plan.md` (required), `spec.md` (required), plus `research.md`, `data-model.md`, `contracts/`, `quickstart.md` + +**Tests**: REQUIRED (Pest) - feature introduces runtime behavior + new persistence. + +**Organization**: Tasks are grouped by user story (Scenario 1/2/3 in spec). + +## Format: `[ID] [P?] [Story] Description` + +- **[P]**: Can run in parallel (different files, no dependencies) +- **[Story]**: Which user story this task belongs to (e.g., US1, US2, US3) +- Include exact file paths in descriptions + +--- + +## Phase 1: Setup (Shared Infrastructure) + +**Purpose**: Project wiring for Drift MVP. + +- [ ] T001 Create/update constitution gate checklist in `specs/044-drift-mvp/checklists/requirements.md` +- [ ] T002 Confirm spec/plan artifacts are current in `specs/044-drift-mvp/{plan.md,spec.md,research.md,data-model.md,quickstart.md,contracts/admin-findings.openapi.yaml}` +- [ ] T003 Add Drift landing page shell in `app/Filament/Pages/DriftLanding.php` +- [ ] T004 [P] Add Finding resource shells in `app/Filament/Resources/FindingResource.php` and `app/Filament/Resources/FindingResource/Pages/` + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +**Purpose**: Persistence + authorization + deterministic IDs that all stories depend on. + +**Checkpoint**: DB schema exists, tenant scoping enforced, and tests can create Finding rows. + +- [ ] T005 Create `findings` migration in `database/migrations/*_create_findings_table.php` with Finding fields aligned to `specs/044-drift-mvp/spec.md`: + (tenant_id, finding_type, scope_key, baseline_run_id, current_run_id, subject_type, subject_external_id, severity, status, fingerprint unique, evidence_jsonb, acknowledged_at, acknowledged_by_user_id) +- [ ] T006 Create `Finding` model in `app/Models/Finding.php` (casts for `evidence_jsonb`, enums/constants for `finding_type`/`severity`/`status`, `acknowledged_at` handling) +- [ ] T007 [P] Add `FindingFactory` in `database/factories/FindingFactory.php` +- [ ] T008 Ensure `InventorySyncRunFactory` exists (or add it) in `database/factories/InventorySyncRunFactory.php` +- [ ] T009 Add authorization policy in `app/Policies/FindingPolicy.php` and wire it in `app/Providers/AuthServiceProvider.php` (or project equivalent) +- [ ] T010 Add Drift permissions in `config/intune_permissions.php` (view + acknowledge) and wire them into Filament navigation/actions +- [ ] T011 Enforce tenant scoping for Finding queries in `app/Models/Finding.php` and/or `app/Filament/Resources/FindingResource.php` +- [ ] T012 Implement fingerprint helper in `app/Services/Drift/DriftHasher.php` (sha256 per spec) +- [ ] T013 Pin `scope_key = InventorySyncRun.selection_hash` in `app/Services/Drift/DriftScopeKey.php` (single canonical definition) +- [ ] T014 Implement evidence allowlist builder in `app/Services/Drift/DriftEvidence.php` (new file) + +--- + +## Phase 3: User Story 1 - View drift summary (Priority: P1) MVP + +**Goal**: Opening Drift generates (async) and displays a summary of new drift findings for the latest scope. + +**Independent Test**: With 2 successful inventory runs for the same selection hash, opening Drift dispatches generation if missing and then shows summary counts. + +### Tests (write first) + +- [ ] T015 [P] [US1] Baseline selection tests in `tests/Feature/Drift/DriftBaselineSelectionTest.php` +- [ ] T016 [P] [US1] Generation dispatch tests in `tests/Feature/Drift/DriftGenerationDispatchTest.php` +- [ ] T017 [P] [US1] Tenant isolation tests in `tests/Feature/Drift/DriftTenantIsolationTest.php` +- [ ] T018 [P] [US1] Assignment drift detection test (targets + intent changes per FR2b) in `tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php` + +### Implementation + +- [ ] T019 [US1] Implement run selection service in `app/Services/Drift/DriftRunSelector.php` +- [ ] T020 [US1] Implement generator job in `app/Jobs/GenerateDriftFindingsJob.php` (dedupe/lock by tenant+scope+baseline+current) +- [ ] T021 [US1] Implement generator service in `app/Services/Drift/DriftFindingGenerator.php` (idempotent) +- [ ] T022 [US1] Implement landing behavior (dispatch + status UI) in `app/Filament/Pages/DriftLanding.php` +- [ ] T023 [US1] Implement summary queries/widgets in `app/Filament/Pages/DriftLanding.php` + +--- + +## Phase 4: User Story 2 - Drill into a drift finding (Priority: P2) + +**Goal**: Admin can view a finding and see sanitized evidence + run references (DB-only label resolution). + +**Independent Test**: A persisted finding renders details without Graph calls. + +### Tests (write first) + +- [ ] T024 [P] [US2] Finding detail test in `tests/Feature/Drift/DriftFindingDetailTest.php` +- [ ] T025 [P] [US2] Evidence minimization test in `tests/Feature/Drift/DriftEvidenceMinimizationTest.php` + +### Implementation + +- [ ] T026 [US2] Implement list UI in `app/Filament/Resources/FindingResource.php` (filters: status, scope_key, run) +- [ ] T027 [US2] Implement detail UI in `app/Filament/Resources/FindingResource/Pages/ViewFinding.php` +- [ ] T028 [US2] Implement DB-only name resolution in `app/Filament/Resources/FindingResource.php` (inventory/foundations caches) + +--- + +## Phase 5: User Story 3 - Acknowledge/triage (Priority: P3) + +**Goal**: Admin can acknowledge findings; new lists hide acknowledged but records remain auditable. + +**Independent Test**: Acknowledging sets `acknowledged_at` + `acknowledged_by_user_id` and flips status. + +### Tests (write first) + +- [ ] T029 [P] [US3] Acknowledge action test in `tests/Feature/Drift/DriftAcknowledgeTest.php` +- [ ] T030 [P] [US3] Acknowledge authorization test in `tests/Feature/Drift/DriftAcknowledgeAuthorizationTest.php` + +### Implementation + +- [ ] T031 [US3] Add acknowledge actions in `app/Filament/Resources/FindingResource.php` +- [ ] T032 [US3] Implement `acknowledge()` domain method in `app/Models/Finding.php` +- [ ] T033 [US3] Ensure Drift summary excludes acknowledged by default in `app/Filament/Pages/DriftLanding.php` + +--- + +## Phase 6: Polish & Cross-Cutting Concerns + +- [ ] T034 Add DB indexes in `database/migrations/*_create_findings_table.php` (tenant_id+status, tenant_id+scope_key, tenant_id+baseline_run_id, tenant_id+current_run_id) +- [ ] T035 [P] Add determinism test in `tests/Feature/Drift/DriftGenerationDeterminismTest.php` (same baseline/current => same fingerprints) +- [ ] T036 Add job observability logs in `app/Jobs/GenerateDriftFindingsJob.php` (no secrets) +- [ ] T037 Add idempotency/upsert strategy in `app/Services/Drift/DriftFindingGenerator.php` +- [ ] T038 Ensure volatile fields excluded from hashing in `app/Services/Drift/DriftHasher.php` and cover in `tests/Feature/Drift/DriftHasherTest.php` +- [ ] T039 Validate and update `specs/044-drift-mvp/quickstart.md` after implementation + +--- + +## Dependencies & Execution Order + +- Setup (Phase 1) -> Foundational (Phase 2) -> US1 -> US2 -> US3 -> Polish + +### Parallel execution examples + +- Foundational: T007, T008, T012, T013, T014 +- US1 tests: T015, T016, T017, T018 +- US2 tests: T024, T025 +- US3 tests: T029, T030 + +--- + +## Implementation Strategy + +### MVP scope + +- MVP = Phase 1 + Phase 2 + US1. + +### Format validation + +- All tasks use `- [ ] T###` format +- Story tasks include `[US1]`/`[US2]`/`[US3]` +- All tasks include file paths -- 2.45.2 From 242881c04e2a604059fea44c8496bcef4ec7bfe0 Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Tue, 13 Jan 2026 23:48:16 +0100 Subject: [PATCH 3/6] feat(044): add drift findings foundation --- app/Filament/Pages/DriftLanding.php | 83 +++++++++++++++++++ app/Filament/Resources/FindingResource.php | 65 +++++++++++++++ .../FindingResource/Pages/ListFindings.php | 11 +++ .../FindingResource/Pages/ViewFinding.php | 11 +++ app/Jobs/GenerateDriftFindingsJob.php | 30 +++++++ app/Models/Finding.php | 65 +++++++++++++++ app/Policies/FindingPolicy.php | 66 +++++++++++++++ app/Providers/AppServiceProvider.php | 5 ++ app/Services/Drift/DriftEvidence.php | 31 +++++++ app/Services/Drift/DriftHasher.php | 33 ++++++++ app/Services/Drift/DriftRunSelector.php | 40 +++++++++ app/Services/Drift/DriftScopeKey.php | 13 +++ database/factories/FindingFactory.php | 37 +++++++++ ...026_01_13_223311_create_findings_table.php | 56 +++++++++++++ .../filament/pages/drift-landing.blade.php | 15 ++++ .../Drift/DriftBaselineSelectionTest.php | 60 ++++++++++++++ .../Drift/DriftGenerationDispatchTest.php | 60 ++++++++++++++ 17 files changed, 681 insertions(+) create mode 100644 app/Filament/Pages/DriftLanding.php create mode 100644 app/Filament/Resources/FindingResource.php create mode 100644 app/Filament/Resources/FindingResource/Pages/ListFindings.php create mode 100644 app/Filament/Resources/FindingResource/Pages/ViewFinding.php create mode 100644 app/Jobs/GenerateDriftFindingsJob.php create mode 100644 app/Models/Finding.php create mode 100644 app/Policies/FindingPolicy.php create mode 100644 app/Services/Drift/DriftEvidence.php create mode 100644 app/Services/Drift/DriftHasher.php create mode 100644 app/Services/Drift/DriftRunSelector.php create mode 100644 app/Services/Drift/DriftScopeKey.php create mode 100644 database/factories/FindingFactory.php create mode 100644 database/migrations/2026_01_13_223311_create_findings_table.php create mode 100644 resources/views/filament/pages/drift-landing.blade.php create mode 100644 tests/Feature/Drift/DriftBaselineSelectionTest.php create mode 100644 tests/Feature/Drift/DriftGenerationDispatchTest.php diff --git a/app/Filament/Pages/DriftLanding.php b/app/Filament/Pages/DriftLanding.php new file mode 100644 index 0000000..cfdcb44 --- /dev/null +++ b/app/Filament/Pages/DriftLanding.php @@ -0,0 +1,83 @@ +user(); + if (! $user instanceof User) { + abort(403, 'Not allowed'); + } + + $latestSuccessful = InventorySyncRun::query() + ->where('tenant_id', $tenant->getKey()) + ->where('status', InventorySyncRun::STATUS_SUCCESS) + ->whereNotNull('finished_at') + ->orderByDesc('finished_at') + ->first(); + + if (! $latestSuccessful instanceof InventorySyncRun) { + return; + } + + $scopeKey = (string) $latestSuccessful->selection_hash; + + $selector = app(DriftRunSelector::class); + $comparison = $selector->selectBaselineAndCurrent($tenant, $scopeKey); + + if ($comparison === null) { + return; + } + + $baseline = $comparison['baseline']; + $current = $comparison['current']; + + $exists = Finding::query() + ->where('tenant_id', $tenant->getKey()) + ->where('finding_type', Finding::FINDING_TYPE_DRIFT) + ->where('scope_key', $scopeKey) + ->where('baseline_run_id', $baseline->getKey()) + ->where('current_run_id', $current->getKey()) + ->exists(); + + if ($exists) { + return; + } + + GenerateDriftFindingsJob::dispatch( + tenantId: (int) $tenant->getKey(), + userId: (int) $user->getKey(), + baselineRunId: (int) $baseline->getKey(), + currentRunId: (int) $current->getKey(), + scopeKey: $scopeKey, + ); + } + + public function getFindingsUrl(): string + { + return FindingResource::getUrl('index', tenant: Tenant::current()); + } +} diff --git a/app/Filament/Resources/FindingResource.php b/app/Filament/Resources/FindingResource.php new file mode 100644 index 0000000..bfd097e --- /dev/null +++ b/app/Filament/Resources/FindingResource.php @@ -0,0 +1,65 @@ +columns([ + Tables\Columns\TextColumn::make('finding_type')->badge()->label('Type'), + Tables\Columns\TextColumn::make('status')->badge(), + Tables\Columns\TextColumn::make('severity')->badge(), + Tables\Columns\TextColumn::make('subject_type')->label('Subject')->searchable(), + Tables\Columns\TextColumn::make('subject_external_id')->label('External ID')->toggleable(isToggledHiddenByDefault: true), + Tables\Columns\TextColumn::make('scope_key')->label('Scope')->toggleable(isToggledHiddenByDefault: true), + Tables\Columns\TextColumn::make('created_at')->since()->label('Created'), + ]) + ->actions([ + Actions\ViewAction::make(), + ]) + ->bulkActions([]); + } + + public static function getEloquentQuery(): Builder + { + $tenantId = Tenant::current()->getKey(); + + return parent::getEloquentQuery() + ->when($tenantId, fn (Builder $query) => $query->where('tenant_id', $tenantId)); + } + + public static function getPages(): array + { + return [ + 'index' => Pages\ListFindings::route('/'), + 'view' => Pages\ViewFinding::route('/{record}'), + ]; + } +} diff --git a/app/Filament/Resources/FindingResource/Pages/ListFindings.php b/app/Filament/Resources/FindingResource/Pages/ListFindings.php new file mode 100644 index 0000000..cb872dd --- /dev/null +++ b/app/Filament/Resources/FindingResource/Pages/ListFindings.php @@ -0,0 +1,11 @@ + */ + use HasFactory; + + public const string FINDING_TYPE_DRIFT = 'drift'; + + public const string SEVERITY_LOW = 'low'; + + public const string SEVERITY_MEDIUM = 'medium'; + + public const string SEVERITY_HIGH = 'high'; + + public const string STATUS_NEW = 'new'; + + public const string STATUS_ACKNOWLEDGED = 'acknowledged'; + + protected $guarded = []; + + protected $casts = [ + 'acknowledged_at' => 'datetime', + 'evidence_jsonb' => 'array', + ]; + + public function tenant(): BelongsTo + { + return $this->belongsTo(Tenant::class); + } + + public function baselineRun(): BelongsTo + { + return $this->belongsTo(InventorySyncRun::class, 'baseline_run_id'); + } + + public function currentRun(): BelongsTo + { + return $this->belongsTo(InventorySyncRun::class, 'current_run_id'); + } + + public function acknowledgedByUser(): BelongsTo + { + return $this->belongsTo(User::class, 'acknowledged_by_user_id'); + } + + 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(), + ]); + } +} diff --git a/app/Policies/FindingPolicy.php b/app/Policies/FindingPolicy.php new file mode 100644 index 0000000..6a779e0 --- /dev/null +++ b/app/Policies/FindingPolicy.php @@ -0,0 +1,66 @@ +canAccessTenant($tenant); + } + + public function view(User $user, Finding $finding): bool + { + $tenant = Tenant::current(); + + if (! $tenant) { + return false; + } + + if (! $user->canAccessTenant($tenant)) { + return false; + } + + return (int) $finding->tenant_id === (int) $tenant->getKey(); + } + + public function update(User $user, Finding $finding): bool + { + $tenant = Tenant::current(); + + if (! $tenant) { + return false; + } + + if (! $user->canAccessTenant($tenant)) { + return false; + } + + if ((int) $finding->tenant_id !== (int) $tenant->getKey()) { + return false; + } + + $role = $user->tenantRole($tenant); + + return match ($role) { + TenantRole::Owner, + TenantRole::Manager, + TenantRole::Operator => true, + default => false, + }; + } +} diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index a605e4a..8e59c1e 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -3,10 +3,14 @@ namespace App\Providers; use App\Models\BackupSchedule; +use App\Models\BulkOperationRun; +use App\Models\Finding; use App\Models\Tenant; use App\Models\User; use App\Models\UserTenantPreference; use App\Policies\BackupSchedulePolicy; +use App\Policies\BulkOperationRunPolicy; +use App\Policies\FindingPolicy; use App\Services\Graph\GraphClientInterface; use App\Services\Graph\MicrosoftGraphClient; use App\Services\Graph\NullGraphClient; @@ -108,5 +112,6 @@ public function boot(): void Gate::policy(BackupSchedule::class, BackupSchedulePolicy::class); Gate::policy(BulkOperationRun::class, BulkOperationRunPolicy::class); + Gate::policy(Finding::class, FindingPolicy::class); } } diff --git a/app/Services/Drift/DriftEvidence.php b/app/Services/Drift/DriftEvidence.php new file mode 100644 index 0000000..f5c4d96 --- /dev/null +++ b/app/Services/Drift/DriftEvidence.php @@ -0,0 +1,31 @@ + $payload + * @return array + */ + public function sanitize(array $payload): array + { + $allowedKeys = [ + 'change_type', + 'summary', + 'baseline', + 'current', + 'diff', + 'notes', + ]; + + $safe = []; + foreach ($allowedKeys as $key) { + if (array_key_exists($key, $payload)) { + $safe[$key] = $payload[$key]; + } + } + + return $safe; + } +} diff --git a/app/Services/Drift/DriftHasher.php b/app/Services/Drift/DriftHasher.php new file mode 100644 index 0000000..6ca3aee --- /dev/null +++ b/app/Services/Drift/DriftHasher.php @@ -0,0 +1,33 @@ +normalize($scopeKey), + $this->normalize($subjectType), + $this->normalize($subjectExternalId), + $this->normalize($changeType), + $this->normalize($baselineHash), + $this->normalize($currentHash), + ]; + + return hash('sha256', implode('|', $parts)); + } + + private function normalize(string $value): string + { + return trim(mb_strtolower($value)); + } +} diff --git a/app/Services/Drift/DriftRunSelector.php b/app/Services/Drift/DriftRunSelector.php new file mode 100644 index 0000000..a320f5d --- /dev/null +++ b/app/Services/Drift/DriftRunSelector.php @@ -0,0 +1,40 @@ +where('tenant_id', $tenant->getKey()) + ->where('selection_hash', $scopeKey) + ->where('status', InventorySyncRun::STATUS_SUCCESS) + ->whereNotNull('finished_at') + ->orderByDesc('finished_at') + ->limit(2) + ->get(); + + if ($runs->count() < 2) { + return null; + } + + $current = $runs->first(); + $baseline = $runs->last(); + + if (! $baseline instanceof InventorySyncRun || ! $current instanceof InventorySyncRun) { + return null; + } + + return [ + 'baseline' => $baseline, + 'current' => $current, + ]; + } +} diff --git a/app/Services/Drift/DriftScopeKey.php b/app/Services/Drift/DriftScopeKey.php new file mode 100644 index 0000000..f6e9405 --- /dev/null +++ b/app/Services/Drift/DriftScopeKey.php @@ -0,0 +1,13 @@ +selection_hash; + } +} diff --git a/database/factories/FindingFactory.php b/database/factories/FindingFactory.php new file mode 100644 index 0000000..408ce4e --- /dev/null +++ b/database/factories/FindingFactory.php @@ -0,0 +1,37 @@ + + */ +class FindingFactory extends Factory +{ + /** + * Define the model's default state. + * + * @return array + */ + public function definition(): array + { + return [ + 'tenant_id' => Tenant::factory(), + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'scope_key' => hash('sha256', fake()->uuid()), + 'baseline_run_id' => null, + 'current_run_id' => null, + 'fingerprint' => hash('sha256', fake()->uuid()), + 'subject_type' => 'assignment', + 'subject_external_id' => fake()->uuid(), + 'severity' => Finding::SEVERITY_MEDIUM, + 'status' => Finding::STATUS_NEW, + 'acknowledged_at' => null, + 'acknowledged_by_user_id' => null, + 'evidence_jsonb' => [], + ]; + } +} diff --git a/database/migrations/2026_01_13_223311_create_findings_table.php b/database/migrations/2026_01_13_223311_create_findings_table.php new file mode 100644 index 0000000..5307a51 --- /dev/null +++ b/database/migrations/2026_01_13_223311_create_findings_table.php @@ -0,0 +1,56 @@ +id(); + + $table->foreignId('tenant_id')->constrained(); + + $table->string('finding_type'); + $table->string('scope_key'); + + $table->foreignId('baseline_run_id')->nullable()->constrained('inventory_sync_runs'); + $table->foreignId('current_run_id')->nullable()->constrained('inventory_sync_runs'); + + $table->string('fingerprint', 64); + + $table->string('subject_type'); + $table->string('subject_external_id'); + + $table->string('severity'); + $table->string('status'); + + $table->timestampTz('acknowledged_at')->nullable(); + $table->foreignId('acknowledged_by_user_id')->nullable()->constrained('users'); + + $table->jsonb('evidence_jsonb')->nullable(); + + $table->timestamps(); + + $table->unique(['tenant_id', 'fingerprint']); + + $table->index(['tenant_id', 'status']); + $table->index(['tenant_id', 'scope_key']); + $table->index(['tenant_id', 'baseline_run_id']); + $table->index(['tenant_id', 'current_run_id']); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::dropIfExists('findings'); + } +}; diff --git a/resources/views/filament/pages/drift-landing.blade.php b/resources/views/filament/pages/drift-landing.blade.php new file mode 100644 index 0000000..9b631d5 --- /dev/null +++ b/resources/views/filament/pages/drift-landing.blade.php @@ -0,0 +1,15 @@ + + +
+
+ Review new drift findings between the last two inventory sync runs for the current scope. +
+ +
+ + Findings + +
+
+ + diff --git a/tests/Feature/Drift/DriftBaselineSelectionTest.php b/tests/Feature/Drift/DriftBaselineSelectionTest.php new file mode 100644 index 0000000..c0e273b --- /dev/null +++ b/tests/Feature/Drift/DriftBaselineSelectionTest.php @@ -0,0 +1,60 @@ +actingAs($user); + + $scopeKey = hash('sha256', 'scope-a'); + + InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(3), + ]); + + $baseline = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'status' => InventorySyncRun::STATUS_FAILED, + 'finished_at' => now(), + ]); + + $selector = app(DriftRunSelector::class); + + $selected = $selector->selectBaselineAndCurrent($tenant, $scopeKey); + + expect($selected)->not->toBeNull(); + expect($selected['baseline']->getKey())->toBe($baseline->getKey()); + expect($selected['current']->getKey())->toBe($current->getKey()); +}); + +test('it returns null when fewer than two successful runs exist for scope', function () { + [$user, $tenant] = createUserWithTenant(role: 'manager'); + $this->actingAs($user); + + $scopeKey = hash('sha256', 'scope-b'); + + InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $selector = app(DriftRunSelector::class); + + expect($selector->selectBaselineAndCurrent($tenant, $scopeKey))->toBeNull(); +}); diff --git a/tests/Feature/Drift/DriftGenerationDispatchTest.php b/tests/Feature/Drift/DriftGenerationDispatchTest.php new file mode 100644 index 0000000..3e54f9a --- /dev/null +++ b/tests/Feature/Drift/DriftGenerationDispatchTest.php @@ -0,0 +1,60 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $scopeKey = hash('sha256', 'scope-dispatch'); + + $baseline = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + Livewire::test(DriftLanding::class); + + Queue::assertPushed(GenerateDriftFindingsJob::class, function (GenerateDriftFindingsJob $job) use ($tenant, $user, $baseline, $current, $scopeKey): bool { + return $job->tenantId === (int) $tenant->getKey() + && $job->userId === (int) $user->getKey() + && $job->baselineRunId === (int) $baseline->getKey() + && $job->currentRunId === (int) $current->getKey() + && $job->scopeKey === $scopeKey; + }); +}); + +test('opening Drift does not dispatch generation when fewer than two successful runs exist', function () { + Queue::fake(); + + [$user, $tenant] = createUserWithTenant(role: 'manager'); + $this->actingAs($user); + Filament::setTenant($tenant, true); + + $scopeKey = hash('sha256', 'scope-blocked'); + + InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + Livewire::test(DriftLanding::class); + + Queue::assertNothingPushed(); +}); -- 2.45.2 From 68ab61b5c01688c1e7ac8686d641d29caa7d7f44 Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Tue, 13 Jan 2026 23:55:41 +0100 Subject: [PATCH 4/6] feat(044): generate assignment drift findings --- app/Jobs/GenerateDriftFindingsJob.php | 28 +++- app/Services/Drift/DriftFindingGenerator.php | 127 ++++++++++++++++++ .../DriftAssignmentDriftDetectionTest.php | 76 +++++++++++ .../Drift/DriftTenantIsolationTest.php | 80 +++++++++++ 4 files changed, 309 insertions(+), 2 deletions(-) create mode 100644 app/Services/Drift/DriftFindingGenerator.php create mode 100644 tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php create mode 100644 tests/Feature/Drift/DriftTenantIsolationTest.php diff --git a/app/Jobs/GenerateDriftFindingsJob.php b/app/Jobs/GenerateDriftFindingsJob.php index 8c0de42..645dae4 100644 --- a/app/Jobs/GenerateDriftFindingsJob.php +++ b/app/Jobs/GenerateDriftFindingsJob.php @@ -2,11 +2,15 @@ namespace App\Jobs; +use App\Models\InventorySyncRun; +use App\Models\Tenant; +use App\Services\Drift\DriftFindingGenerator; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; +use RuntimeException; class GenerateDriftFindingsJob implements ShouldQueue { @@ -23,8 +27,28 @@ public function __construct( /** * Execute the job. */ - public function handle(): void + public function handle(DriftFindingGenerator $generator): void { - // Implemented in later tasks (T020/T021). + $tenant = Tenant::query()->find($this->tenantId); + if (! $tenant instanceof Tenant) { + throw new RuntimeException('Tenant not found.'); + } + + $baseline = InventorySyncRun::query()->find($this->baselineRunId); + if (! $baseline instanceof InventorySyncRun) { + throw new RuntimeException('Baseline run not found.'); + } + + $current = InventorySyncRun::query()->find($this->currentRunId); + if (! $current instanceof InventorySyncRun) { + throw new RuntimeException('Current run not found.'); + } + + $generator->generate( + tenant: $tenant, + baseline: $baseline, + current: $current, + scopeKey: $this->scopeKey, + ); } } diff --git a/app/Services/Drift/DriftFindingGenerator.php b/app/Services/Drift/DriftFindingGenerator.php new file mode 100644 index 0000000..1fc88fa --- /dev/null +++ b/app/Services/Drift/DriftFindingGenerator.php @@ -0,0 +1,127 @@ +finished_at || ! $current->finished_at) { + throw new RuntimeException('Baseline/current run must be finished.'); + } + + /** @var array $selection */ + $selection = is_array($current->selection_payload) ? $current->selection_payload : []; + + $policyTypes = Arr::get($selection, 'policy_types'); + if (! is_array($policyTypes)) { + $policyTypes = []; + } + + $policyTypes = array_values(array_filter(array_map('strval', $policyTypes))); + + $created = 0; + + Policy::query() + ->where('tenant_id', $tenant->getKey()) + ->whereIn('policy_type', $policyTypes) + ->orderBy('id') + ->chunk(200, function ($policies) use ($tenant, $baseline, $current, $scopeKey, &$created): void { + foreach ($policies as $policy) { + if (! $policy instanceof Policy) { + continue; + } + + $baselineVersion = $this->versionForRun($policy, $baseline); + $currentVersion = $this->versionForRun($policy, $current); + + if (! $baselineVersion instanceof PolicyVersion || ! $currentVersion instanceof PolicyVersion) { + continue; + } + + $baselineAssignmentsHash = $baselineVersion->assignments_hash ?? null; + $currentAssignmentsHash = $currentVersion->assignments_hash ?? null; + + if ($baselineAssignmentsHash === $currentAssignmentsHash) { + continue; + } + + $fingerprint = $this->hasher->fingerprint( + tenantId: (int) $tenant->getKey(), + scopeKey: $scopeKey, + subjectType: 'assignment', + subjectExternalId: (string) $policy->external_id, + changeType: 'modified', + baselineHash: (string) ($baselineAssignmentsHash ?? ''), + currentHash: (string) ($currentAssignmentsHash ?? ''), + ); + + $rawEvidence = [ + 'change_type' => 'modified', + 'summary' => 'Policy assignments changed', + 'baseline' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $baselineVersion->getKey(), + 'assignments_hash' => $baselineAssignmentsHash, + ], + 'current' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $currentVersion->getKey(), + 'assignments_hash' => $currentAssignmentsHash, + ], + ]; + + Finding::query()->updateOrCreate( + [ + 'tenant_id' => $tenant->getKey(), + 'fingerprint' => $fingerprint, + ], + [ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'scope_key' => $scopeKey, + 'baseline_run_id' => $baseline->getKey(), + 'current_run_id' => $current->getKey(), + 'subject_type' => 'assignment', + 'subject_external_id' => (string) $policy->external_id, + 'severity' => Finding::SEVERITY_MEDIUM, + 'status' => Finding::STATUS_NEW, + 'acknowledged_at' => null, + 'acknowledged_by_user_id' => null, + 'evidence_jsonb' => $this->evidence->sanitize($rawEvidence), + ], + ); + + $created++; + } + }); + + return $created; + } + + private function versionForRun(Policy $policy, InventorySyncRun $run): ?PolicyVersion + { + if (! $run->finished_at) { + return null; + } + + return PolicyVersion::query() + ->where('tenant_id', $policy->tenant_id) + ->where('policy_id', $policy->getKey()) + ->where('captured_at', '<=', $run->finished_at) + ->latest('captured_at') + ->first(); + } +} diff --git a/tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php b/tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php new file mode 100644 index 0000000..5a186ea --- /dev/null +++ b/tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php @@ -0,0 +1,76 @@ +for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['settingsCatalogPolicy']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['settingsCatalogPolicy']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'policy_type' => 'settingsCatalogPolicy', + ]); + + $baselineAssignments = [ + [ + 'target' => [ + '@odata.type' => '#microsoft.graph.groupAssignmentTarget', + 'groupId' => 'group-a', + ], + ], + ]; + + $currentAssignments = [ + [ + 'target' => [ + '@odata.type' => '#microsoft.graph.groupAssignmentTarget', + 'groupId' => 'group-b', + ], + ], + ]; + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 1, + 'policy_type' => $policy->policy_type, + 'captured_at' => $baseline->finished_at->copy()->subMinute(), + 'assignments' => $baselineAssignments, + 'assignments_hash' => hash('sha256', json_encode($baselineAssignments)), + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 2, + 'policy_type' => $policy->policy_type, + 'captured_at' => $current->finished_at->copy()->subMinute(), + 'assignments' => $currentAssignments, + 'assignments_hash' => hash('sha256', json_encode($currentAssignments)), + ]); + + $generator = app(DriftFindingGenerator::class); + $created = $generator->generate($tenant, $baseline, $current, $scopeKey); + + expect($created)->toBe(1); + + $finding = Finding::query()->where('tenant_id', $tenant->getKey())->first(); + expect($finding)->not->toBeNull(); + expect($finding->subject_type)->toBe('assignment'); + expect($finding->subject_external_id)->toBe($policy->external_id); + expect($finding->evidence_jsonb)->toHaveKey('change_type', 'modified'); +}); diff --git a/tests/Feature/Drift/DriftTenantIsolationTest.php b/tests/Feature/Drift/DriftTenantIsolationTest.php new file mode 100644 index 0000000..a6661a2 --- /dev/null +++ b/tests/Feature/Drift/DriftTenantIsolationTest.php @@ -0,0 +1,80 @@ +for($tenantA)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['settingsCatalogPolicy']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $currentA = InventorySyncRun::factory()->for($tenantA)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['settingsCatalogPolicy']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policyA = Policy::factory()->for($tenantA)->create([ + 'policy_type' => 'settingsCatalogPolicy', + ]); + + $baselineAssignments = [['target' => ['groupId' => 'group-a'], '@odata.type' => '#microsoft.graph.groupAssignmentTarget']]; + $currentAssignments = [['target' => ['groupId' => 'group-b'], '@odata.type' => '#microsoft.graph.groupAssignmentTarget']]; + + PolicyVersion::factory()->for($tenantA)->for($policyA)->create([ + 'version_number' => 1, + 'policy_type' => $policyA->policy_type, + 'captured_at' => $baselineA->finished_at->copy()->subMinute(), + 'assignments' => $baselineAssignments, + 'assignments_hash' => hash('sha256', json_encode($baselineAssignments)), + ]); + + PolicyVersion::factory()->for($tenantA)->for($policyA)->create([ + 'version_number' => 2, + 'policy_type' => $policyA->policy_type, + 'captured_at' => $currentA->finished_at->copy()->subMinute(), + 'assignments' => $currentAssignments, + 'assignments_hash' => hash('sha256', json_encode($currentAssignments)), + ]); + + $policyB = Policy::factory()->for($tenantB)->create([ + 'policy_type' => 'settingsCatalogPolicy', + ]); + + $baselineAssignmentsB = [['target' => ['groupId' => 'group-x'], '@odata.type' => '#microsoft.graph.groupAssignmentTarget']]; + $currentAssignmentsB = [['target' => ['groupId' => 'group-y'], '@odata.type' => '#microsoft.graph.groupAssignmentTarget']]; + + PolicyVersion::factory()->for($tenantB)->for($policyB)->create([ + 'version_number' => 1, + 'policy_type' => $policyB->policy_type, + 'captured_at' => now()->subDays(2)->subMinute(), + 'assignments' => $baselineAssignmentsB, + 'assignments_hash' => hash('sha256', json_encode($baselineAssignmentsB)), + ]); + + PolicyVersion::factory()->for($tenantB)->for($policyB)->create([ + 'version_number' => 2, + 'policy_type' => $policyB->policy_type, + 'captured_at' => now()->subDay()->subMinute(), + 'assignments' => $currentAssignmentsB, + 'assignments_hash' => hash('sha256', json_encode($currentAssignmentsB)), + ]); + + $generator = app(DriftFindingGenerator::class); + $generator->generate($tenantA, $baselineA, $currentA, $scopeKey); + + expect(Finding::query()->where('tenant_id', $tenantA->getKey())->count())->toBe(1); + expect(Finding::query()->where('tenant_id', $tenantB->getKey())->count())->toBe(0); +}); -- 2.45.2 From 2214d8acc6821bd56cdc37c91bd1eb36b548aaec Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Wed, 14 Jan 2026 00:12:37 +0100 Subject: [PATCH 5/6] spec(044): record BulkOperationRun decision --- .../044-drift-mvp/checklists/requirements.md | 32 +++++++++---------- specs/044-drift-mvp/spec.md | 8 ++++- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/specs/044-drift-mvp/checklists/requirements.md b/specs/044-drift-mvp/checklists/requirements.md index 5e432f1..04dab55 100644 --- a/specs/044-drift-mvp/checklists/requirements.md +++ b/specs/044-drift-mvp/checklists/requirements.md @@ -6,28 +6,28 @@ # Specification Quality Checklist: Drift MVP (044) ## Content Quality -- [ ] No implementation details (languages, frameworks, APIs) -- [ ] Focused on user value and business needs -- [ ] Written for non-technical stakeholders -- [ ] All mandatory sections completed +- [ ] No implementation details (languages, frameworks, APIs) (T002) +- [x] Focused on user value and business needs (spec.md: Purpose, User Scenarios & Testing, Success Criteria) +- [ ] Written for non-technical stakeholders (T002) +- [x] All mandatory sections completed (spec.md includes Purpose, Scenarios, FR/NFR, Success Criteria, Out of Scope) ## Requirement Completeness -- [ ] No [NEEDS CLARIFICATION] markers remain -- [ ] Requirements are testable and unambiguous -- [ ] Success criteria are measurable -- [ ] Success criteria are technology-agnostic (no implementation details) -- [ ] All acceptance scenarios are defined -- [ ] Edge cases are identified -- [ ] Scope is clearly bounded -- [ ] Dependencies and assumptions identified +- [x] No [NEEDS CLARIFICATION] markers remain (spec.md: no "[NEEDS CLARIFICATION]" markers) +- [x] Requirements are testable and unambiguous (spec.md: FR1–FR4; tasks.md defines tests for key behaviors T015–T018, T024–T025, T029–T030, T035, T038) +- [x] Success criteria are measurable (spec.md: SC1 "under 3 minutes", SC2 deterministic consistency) +- [x] Success criteria are technology-agnostic (no implementation details) (spec.md: SC1–SC2) +- [x] All acceptance scenarios are defined (spec.md: Scenario 1/2/3) +- [x] Edge cases are identified (spec.md: <2 runs blocked state; generation failure explicit error state; acknowledgement per comparison) +- [x] Scope is clearly bounded (spec.md: FR2b + Out of Scope) +- [x] Dependencies and assumptions identified (spec.md: Dependencies / Name Resolution; NFR2; "No render-time Graph calls") ## Feature Readiness -- [ ] All functional requirements have clear acceptance criteria -- [ ] User scenarios cover primary flows -- [ ] Feature meets measurable outcomes defined in Success Criteria -- [ ] No implementation details leak into specification +- [x] All functional requirements have clear acceptance criteria (spec.md: FR1–FR4 + Scenario 1/2/3) +- [x] User scenarios cover primary flows (spec.md: Scenario 1/2/3) +- [ ] Feature meets measurable outcomes defined in Success Criteria (T022, T023, T026, T027, T031, T033, T035) +- [ ] No implementation details leak into specification (T002) ## Notes diff --git a/specs/044-drift-mvp/spec.md b/specs/044-drift-mvp/spec.md index 27031cb..a3a2a61 100644 --- a/specs/044-drift-mvp/spec.md +++ b/specs/044-drift-mvp/spec.md @@ -28,6 +28,10 @@ ### Session 2026-01-13 - Q: What is the default UI behavior for `new` vs `acknowledged` findings? → A: Default UI shows only `new`; `acknowledged` is accessible via an explicit filter. - Q: What should the UI do if drift generation fails for a comparison? → A: Show an explicit error state (safe message + reference/run ids) and do not show findings for that comparison until a successful generation exists. +### Session 2026-01-14 + +- Q: How should Drift track generation status/errors/idempotency for a comparison? → A: Use `BulkOperationRun` as the canonical run container (status, failures, idempotency_key, and consistent UI/ops patterns). + ## Pinned Decisions (MVP defaults) - Drift is implemented as a generator that writes persisted Finding rows (not only an in-memory/on-demand diff). @@ -37,6 +41,7 @@ ## Pinned Decisions (MVP defaults) - Drift MVP only uses `finding_type=drift` and `status` in {`new`, `acknowledged`}. - Default severity: `medium` (until a rule engine exists). - UI must not perform render-time Graph calls. Graph access (if any) is limited to background sync/jobs. +- Drift generation is tracked via `BulkOperationRun` to persist status/errors across refresh and to enforce idempotency per (tenant, scope_key, baseline_run_id, current_run_id). ## Key Entities / Generic Findings (Future-proof) @@ -101,6 +106,7 @@ ## Functional Requirements - Findings cover adds, removals, and changes for supported entities (Policies + Assignments). - MVP `change_type` values: `added`, `removed`, `modified`. - Findings are deterministic: same baseline/current + scope_key ⇒ same set of fingerprints. + - Drift generation must be tracked via `BulkOperationRun` with an idempotency key derived from (tenant_id, scope_key, baseline_run_id, current_run_id). - If fewer than two successful inventory runs exist for a given `scope_key`, Drift does not generate findings and must surface a clear blocked/empty state in the UI. - FR2a: Fingerprint definition (MVP) @@ -114,7 +120,7 @@ ## Functional Requirements - FR3: Provide Drift UI with summary and details. - Default lists and the Drift landing summary show only `status=new` by default. - The UI must provide a filter to include `acknowledged` findings. - - If drift generation fails for a comparison, the UI must surface an explicit error state (no secrets), including reference identifiers (e.g., run ids), and must not fall back to stale/previous results. + - If drift generation fails for a comparison, the UI must surface an explicit error state (no secrets), including reference identifiers (e.g., run ids and the `BulkOperationRun` id), and must not fall back to stale/previous results. - FR4: Triage (MVP) - Admin can acknowledge a finding; record `acknowledged_by_user_id` + `acknowledged_at`. -- 2.45.2 From 66b46955811e25d9c32b8de33f1877b10bd07476 Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Thu, 15 Jan 2026 00:12:55 +0100 Subject: [PATCH 6/6] feat(044): drift findings UI + bulk acknowledge --- app/Filament/Pages/DriftLanding.php | 152 ++++++++ app/Filament/Resources/FindingResource.php | 329 +++++++++++++++++- .../FindingResource/Pages/ListFindings.php | 160 +++++++++ app/Jobs/GenerateDriftFindingsJob.php | 64 +++- app/Models/Finding.php | 2 + .../Drift/DriftFindingDiffBuilder.php | 304 ++++++++++++++++ app/Services/Drift/DriftFindingGenerator.php | 250 +++++++++++-- app/Services/Drift/DriftHasher.php | 68 ++++ .../Normalizers/AssignmentsNormalizer.php | 113 ++++++ .../Drift/Normalizers/ScopeTagsNormalizer.php | 136 ++++++++ .../Drift/Normalizers/SettingsNormalizer.php | 19 + config/intune_permissions.php | 6 +- .../entries/assignments-diff.blade.php | 114 ++++++ .../entries/scope-tags-diff.blade.php | 111 ++++++ .../filament/pages/drift-landing.blade.php | 92 +++++ .../044-drift-mvp/checklists/requirements.md | 26 +- .../contracts/admin-findings.openapi.yaml | 21 ++ specs/044-drift-mvp/plan.md | 2 + specs/044-drift-mvp/quickstart.md | 8 +- specs/044-drift-mvp/spec.md | 185 +++------- specs/044-drift-mvp/tasks.md | 113 +++--- .../DriftAcknowledgeAuthorizationTest.php | 31 ++ tests/Feature/Drift/DriftAcknowledgeTest.php | 28 ++ .../DriftAssignmentDriftDetectionTest.php | 2 - ...AcknowledgeAllMatchingConfirmationTest.php | 34 ++ .../DriftBulkAcknowledgeAllMatchingTest.php | 51 +++ .../DriftBulkAcknowledgeAuthorizationTest.php | 60 ++++ .../Drift/DriftBulkAcknowledgeTest.php | 34 ++ .../DriftCompletedRunWithZeroFindingsTest.php | 71 ++++ .../Drift/DriftEvidenceMinimizationTest.php | 24 ++ ...tFindingDetailShowsAssignmentsDiffTest.php | 151 ++++++++ ...iftFindingDetailShowsScopeTagsDiffTest.php | 100 ++++++ ...riftFindingDetailShowsSettingsDiffTest.php | 100 ++++++ .../Feature/Drift/DriftFindingDetailTest.php | 48 +++ .../Drift/DriftGenerationDeterminismTest.php | 76 ++++ .../Drift/DriftGenerationDispatchTest.php | 73 +++- tests/Feature/Drift/DriftHasherTest.php | 39 +++ .../DriftLandingShowsComparisonInfoTest.php | 33 ++ .../DriftPolicySnapshotDriftDetectionTest.php | 73 ++++ ...otMetadataOnlyDoesNotCreateFindingTest.php | 62 ++++ .../Drift/DriftScopeTagDriftDetectionTest.php | 84 +++++ ...gLegacyDefaultDoesNotCreateFindingTest.php | 69 ++++ 42 files changed, 3276 insertions(+), 242 deletions(-) create mode 100644 app/Services/Drift/DriftFindingDiffBuilder.php create mode 100644 app/Services/Drift/Normalizers/AssignmentsNormalizer.php create mode 100644 app/Services/Drift/Normalizers/ScopeTagsNormalizer.php create mode 100644 app/Services/Drift/Normalizers/SettingsNormalizer.php create mode 100644 resources/views/filament/infolists/entries/assignments-diff.blade.php create mode 100644 resources/views/filament/infolists/entries/scope-tags-diff.blade.php create mode 100644 tests/Feature/Drift/DriftAcknowledgeAuthorizationTest.php create mode 100644 tests/Feature/Drift/DriftAcknowledgeTest.php create mode 100644 tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingConfirmationTest.php create mode 100644 tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingTest.php create mode 100644 tests/Feature/Drift/DriftBulkAcknowledgeAuthorizationTest.php create mode 100644 tests/Feature/Drift/DriftBulkAcknowledgeTest.php create mode 100644 tests/Feature/Drift/DriftCompletedRunWithZeroFindingsTest.php create mode 100644 tests/Feature/Drift/DriftEvidenceMinimizationTest.php create mode 100644 tests/Feature/Drift/DriftFindingDetailShowsAssignmentsDiffTest.php create mode 100644 tests/Feature/Drift/DriftFindingDetailShowsScopeTagsDiffTest.php create mode 100644 tests/Feature/Drift/DriftFindingDetailShowsSettingsDiffTest.php create mode 100644 tests/Feature/Drift/DriftFindingDetailTest.php create mode 100644 tests/Feature/Drift/DriftGenerationDeterminismTest.php create mode 100644 tests/Feature/Drift/DriftHasherTest.php create mode 100644 tests/Feature/Drift/DriftLandingShowsComparisonInfoTest.php create mode 100644 tests/Feature/Drift/DriftPolicySnapshotDriftDetectionTest.php create mode 100644 tests/Feature/Drift/DriftPolicySnapshotMetadataOnlyDoesNotCreateFindingTest.php create mode 100644 tests/Feature/Drift/DriftScopeTagDriftDetectionTest.php create mode 100644 tests/Feature/Drift/DriftScopeTagLegacyDefaultDoesNotCreateFindingTest.php diff --git a/app/Filament/Pages/DriftLanding.php b/app/Filament/Pages/DriftLanding.php index cfdcb44..3b7c1d6 100644 --- a/app/Filament/Pages/DriftLanding.php +++ b/app/Filament/Pages/DriftLanding.php @@ -2,13 +2,18 @@ namespace App\Filament\Pages; +use App\Filament\Resources\BulkOperationRunResource; use App\Filament\Resources\FindingResource; +use App\Filament\Resources\InventorySyncRunResource; use App\Jobs\GenerateDriftFindingsJob; +use App\Models\BulkOperationRun; use App\Models\Finding; use App\Models\InventorySyncRun; use App\Models\Tenant; use App\Models\User; +use App\Services\BulkOperationService; use App\Services\Drift\DriftRunSelector; +use App\Support\RunIdempotency; use BackedEnum; use Filament\Pages\Page; use UnitEnum; @@ -23,6 +28,30 @@ class DriftLanding extends Page protected string $view = 'filament.pages.drift-landing'; + public ?string $state = null; + + public ?string $message = null; + + public ?string $scopeKey = null; + + public ?int $baselineRunId = null; + + public ?int $currentRunId = null; + + public ?string $baselineFinishedAt = null; + + public ?string $currentFinishedAt = null; + + public ?int $bulkOperationRunId = null; + + /** @var array|null */ + public ?array $statusCounts = null; + + public static function canAccess(): bool + { + return FindingResource::canAccess(); + } + public function mount(): void { $tenant = Tenant::current(); @@ -40,21 +69,45 @@ public function mount(): void ->first(); if (! $latestSuccessful instanceof InventorySyncRun) { + $this->state = 'blocked'; + $this->message = 'No successful inventory runs found yet.'; + return; } $scopeKey = (string) $latestSuccessful->selection_hash; + $this->scopeKey = $scopeKey; $selector = app(DriftRunSelector::class); $comparison = $selector->selectBaselineAndCurrent($tenant, $scopeKey); if ($comparison === null) { + $this->state = 'blocked'; + $this->message = 'Need at least 2 successful runs for this scope to calculate drift.'; + return; } $baseline = $comparison['baseline']; $current = $comparison['current']; + $this->baselineRunId = (int) $baseline->getKey(); + $this->currentRunId = (int) $current->getKey(); + + $this->baselineFinishedAt = $baseline->finished_at?->toDateTimeString(); + $this->currentFinishedAt = $current->finished_at?->toDateTimeString(); + + $idempotencyKey = RunIdempotency::buildKey( + tenantId: (int) $tenant->getKey(), + operationType: 'drift.generate', + targetId: $scopeKey, + context: [ + 'scope_key' => $scopeKey, + 'baseline_run_id' => (int) $baseline->getKey(), + 'current_run_id' => (int) $current->getKey(), + ], + ); + $exists = Finding::query() ->where('tenant_id', $tenant->getKey()) ->where('finding_type', Finding::FINDING_TYPE_DRIFT) @@ -64,15 +117,87 @@ public function mount(): void ->exists(); if ($exists) { + $this->state = 'ready'; + $newCount = (int) Finding::query() + ->where('tenant_id', $tenant->getKey()) + ->where('finding_type', Finding::FINDING_TYPE_DRIFT) + ->where('scope_key', $scopeKey) + ->where('baseline_run_id', $baseline->getKey()) + ->where('current_run_id', $current->getKey()) + ->where('status', Finding::STATUS_NEW) + ->count(); + + $this->statusCounts = [Finding::STATUS_NEW => $newCount]; + return; } + $latestRun = BulkOperationRun::query() + ->where('tenant_id', $tenant->getKey()) + ->where('idempotency_key', $idempotencyKey) + ->latest('id') + ->first(); + + $activeRun = RunIdempotency::findActiveBulkOperationRun((int) $tenant->getKey(), $idempotencyKey); + if ($activeRun instanceof BulkOperationRun) { + $this->state = 'generating'; + $this->bulkOperationRunId = (int) $activeRun->getKey(); + + return; + } + + if ($latestRun instanceof BulkOperationRun && $latestRun->status === 'completed') { + $this->state = 'ready'; + $this->bulkOperationRunId = (int) $latestRun->getKey(); + + $newCount = (int) Finding::query() + ->where('tenant_id', $tenant->getKey()) + ->where('finding_type', Finding::FINDING_TYPE_DRIFT) + ->where('scope_key', $scopeKey) + ->where('baseline_run_id', $baseline->getKey()) + ->where('current_run_id', $current->getKey()) + ->where('status', Finding::STATUS_NEW) + ->count(); + + $this->statusCounts = [Finding::STATUS_NEW => $newCount]; + + if ($newCount === 0) { + $this->message = 'No drift findings for this comparison. If you changed settings after the current run, run Inventory Sync again to capture a newer snapshot.'; + } + + return; + } + + if ($latestRun instanceof BulkOperationRun && in_array($latestRun->status, ['failed', 'aborted'], true)) { + $this->state = 'error'; + $this->message = 'Drift generation failed for this comparison. See the run for details.'; + $this->bulkOperationRunId = (int) $latestRun->getKey(); + + return; + } + + $bulkOperationService = app(BulkOperationService::class); + $run = $bulkOperationService->createRun( + tenant: $tenant, + user: $user, + resource: 'drift', + action: 'generate', + itemIds: [$scopeKey], + totalItems: 1, + ); + + $run->update(['idempotency_key' => $idempotencyKey]); + + $this->state = 'generating'; + $this->bulkOperationRunId = (int) $run->getKey(); + GenerateDriftFindingsJob::dispatch( tenantId: (int) $tenant->getKey(), userId: (int) $user->getKey(), baselineRunId: (int) $baseline->getKey(), currentRunId: (int) $current->getKey(), scopeKey: $scopeKey, + bulkOperationRunId: (int) $run->getKey(), ); } @@ -80,4 +205,31 @@ public function getFindingsUrl(): string { return FindingResource::getUrl('index', tenant: Tenant::current()); } + + public function getBaselineRunUrl(): ?string + { + if (! is_int($this->baselineRunId)) { + return null; + } + + return InventorySyncRunResource::getUrl('view', ['record' => $this->baselineRunId], tenant: Tenant::current()); + } + + public function getCurrentRunUrl(): ?string + { + if (! is_int($this->currentRunId)) { + return null; + } + + return InventorySyncRunResource::getUrl('view', ['record' => $this->currentRunId], tenant: Tenant::current()); + } + + public function getBulkRunUrl(): ?string + { + if (! is_int($this->bulkOperationRunId)) { + return null; + } + + return BulkOperationRunResource::getUrl('view', ['record' => $this->bulkOperationRunId], tenant: Tenant::current()); + } } diff --git a/app/Filament/Resources/FindingResource.php b/app/Filament/Resources/FindingResource.php index bfd097e..a78c03a 100644 --- a/app/Filament/Resources/FindingResource.php +++ b/app/Filament/Resources/FindingResource.php @@ -4,14 +4,28 @@ use App\Filament\Resources\FindingResource\Pages; use App\Models\Finding; +use App\Models\InventoryItem; +use App\Models\PolicyVersion; use App\Models\Tenant; +use App\Models\User; +use App\Services\Drift\DriftFindingDiffBuilder; use BackedEnum; use Filament\Actions; +use Filament\Actions\BulkAction; +use Filament\Actions\BulkActionGroup; +use Filament\Forms\Components\TextInput; +use Filament\Infolists\Components\TextEntry; +use Filament\Infolists\Components\ViewEntry; +use Filament\Notifications\Notification; use Filament\Resources\Resource; +use Filament\Schemas\Components\Section; use Filament\Schemas\Schema; use Filament\Tables; use Filament\Tables\Table; use Illuminate\Database\Eloquent\Builder; +use Illuminate\Support\Arr; +use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Gate; use UnitEnum; class FindingResource extends Resource @@ -29,22 +43,326 @@ public static function form(Schema $schema): Schema return $schema; } + public static function infolist(Schema $schema): Schema + { + return $schema + ->schema([ + Section::make('Finding') + ->schema([ + TextEntry::make('finding_type')->badge()->label('Type'), + TextEntry::make('status')->badge(), + TextEntry::make('severity')->badge(), + TextEntry::make('fingerprint')->label('Fingerprint')->copyable(), + TextEntry::make('scope_key')->label('Scope')->copyable(), + TextEntry::make('subject_display_name')->label('Subject')->placeholder('—'), + TextEntry::make('subject_type')->label('Subject type'), + TextEntry::make('subject_external_id')->label('External ID')->copyable(), + TextEntry::make('baseline_run_id') + ->label('Baseline run') + ->url(fn (Finding $record): ?string => $record->baseline_run_id + ? InventorySyncRunResource::getUrl('view', ['record' => $record->baseline_run_id], tenant: Tenant::current()) + : null) + ->openUrlInNewTab(), + TextEntry::make('current_run_id') + ->label('Current run') + ->url(fn (Finding $record): ?string => $record->current_run_id + ? InventorySyncRunResource::getUrl('view', ['record' => $record->current_run_id], tenant: Tenant::current()) + : null) + ->openUrlInNewTab(), + TextEntry::make('acknowledged_at')->dateTime()->placeholder('—'), + TextEntry::make('acknowledged_by_user_id')->label('Acknowledged by')->placeholder('—'), + TextEntry::make('created_at')->label('Created')->dateTime(), + ]) + ->columns(2) + ->columnSpanFull(), + + Section::make('Diff') + ->schema([ + ViewEntry::make('settings_diff') + ->label('') + ->view('filament.infolists.entries.normalized-diff') + ->state(function (Finding $record): array { + $tenant = Tenant::current(); + if (! $tenant) { + return ['summary' => ['message' => 'No tenant context'], 'added' => [], 'removed' => [], 'changed' => []]; + } + + $baselineId = Arr::get($record->evidence_jsonb ?? [], 'baseline.policy_version_id'); + $currentId = Arr::get($record->evidence_jsonb ?? [], 'current.policy_version_id'); + + $baselineVersion = is_numeric($baselineId) + ? PolicyVersion::query()->where('tenant_id', $tenant->getKey())->find((int) $baselineId) + : null; + + $currentVersion = is_numeric($currentId) + ? PolicyVersion::query()->where('tenant_id', $tenant->getKey())->find((int) $currentId) + : null; + + $diff = app(DriftFindingDiffBuilder::class)->buildSettingsDiff($baselineVersion, $currentVersion); + + $addedCount = (int) Arr::get($diff, 'summary.added', 0); + $removedCount = (int) Arr::get($diff, 'summary.removed', 0); + $changedCount = (int) Arr::get($diff, 'summary.changed', 0); + + if (($addedCount + $removedCount + $changedCount) === 0) { + Arr::set( + $diff, + 'summary.message', + 'No normalized changes were found. This drift finding may be based on fields that are intentionally excluded from normalization.' + ); + } + + return $diff; + }) + ->visible(fn (Finding $record): bool => Arr::get($record->evidence_jsonb ?? [], 'summary.kind') === 'policy_snapshot') + ->columnSpanFull(), + + ViewEntry::make('scope_tags_diff') + ->label('') + ->view('filament.infolists.entries.scope-tags-diff') + ->state(function (Finding $record): array { + $tenant = Tenant::current(); + if (! $tenant) { + return ['summary' => ['message' => 'No tenant context'], 'added' => [], 'removed' => [], 'changed' => []]; + } + + $baselineId = Arr::get($record->evidence_jsonb ?? [], 'baseline.policy_version_id'); + $currentId = Arr::get($record->evidence_jsonb ?? [], 'current.policy_version_id'); + + $baselineVersion = is_numeric($baselineId) + ? PolicyVersion::query()->where('tenant_id', $tenant->getKey())->find((int) $baselineId) + : null; + + $currentVersion = is_numeric($currentId) + ? PolicyVersion::query()->where('tenant_id', $tenant->getKey())->find((int) $currentId) + : null; + + return app(DriftFindingDiffBuilder::class)->buildScopeTagsDiff($baselineVersion, $currentVersion); + }) + ->visible(fn (Finding $record): bool => Arr::get($record->evidence_jsonb ?? [], 'summary.kind') === 'policy_scope_tags') + ->columnSpanFull(), + + ViewEntry::make('assignments_diff') + ->label('') + ->view('filament.infolists.entries.assignments-diff') + ->state(function (Finding $record): array { + $tenant = Tenant::current(); + if (! $tenant) { + return ['summary' => ['message' => 'No tenant context'], 'added' => [], 'removed' => [], 'changed' => []]; + } + + $baselineId = Arr::get($record->evidence_jsonb ?? [], 'baseline.policy_version_id'); + $currentId = Arr::get($record->evidence_jsonb ?? [], 'current.policy_version_id'); + + $baselineVersion = is_numeric($baselineId) + ? PolicyVersion::query()->where('tenant_id', $tenant->getKey())->find((int) $baselineId) + : null; + + $currentVersion = is_numeric($currentId) + ? PolicyVersion::query()->where('tenant_id', $tenant->getKey())->find((int) $currentId) + : null; + + return app(DriftFindingDiffBuilder::class)->buildAssignmentsDiff($tenant, $baselineVersion, $currentVersion); + }) + ->visible(fn (Finding $record): bool => Arr::get($record->evidence_jsonb ?? [], 'summary.kind') === 'policy_assignments') + ->columnSpanFull(), + ]) + ->collapsed() + ->columnSpanFull(), + + Section::make('Evidence (Sanitized)') + ->schema([ + ViewEntry::make('evidence_jsonb') + ->label('') + ->view('filament.infolists.entries.snapshot-json') + ->state(fn (Finding $record) => $record->evidence_jsonb ?? []) + ->columnSpanFull(), + ]) + ->columnSpanFull(), + ]); + } + public static function table(Table $table): Table { return $table + ->defaultSort('created_at', 'desc') ->columns([ Tables\Columns\TextColumn::make('finding_type')->badge()->label('Type'), Tables\Columns\TextColumn::make('status')->badge(), Tables\Columns\TextColumn::make('severity')->badge(), - Tables\Columns\TextColumn::make('subject_type')->label('Subject')->searchable(), + Tables\Columns\TextColumn::make('subject_display_name')->label('Subject')->placeholder('—'), + Tables\Columns\TextColumn::make('subject_type')->label('Subject type')->searchable(), Tables\Columns\TextColumn::make('subject_external_id')->label('External ID')->toggleable(isToggledHiddenByDefault: true), Tables\Columns\TextColumn::make('scope_key')->label('Scope')->toggleable(isToggledHiddenByDefault: true), Tables\Columns\TextColumn::make('created_at')->since()->label('Created'), ]) + ->filters([ + Tables\Filters\SelectFilter::make('status') + ->options([ + Finding::STATUS_NEW => 'New', + Finding::STATUS_ACKNOWLEDGED => 'Acknowledged', + ]) + ->default(Finding::STATUS_NEW), + Tables\Filters\SelectFilter::make('finding_type') + ->options([ + Finding::FINDING_TYPE_DRIFT => 'Drift', + ]) + ->default(Finding::FINDING_TYPE_DRIFT), + Tables\Filters\Filter::make('scope_key') + ->form([ + TextInput::make('scope_key') + ->label('Scope key') + ->placeholder('Inventory selection hash') + ->maxLength(255), + ]) + ->query(function (Builder $query, array $data): Builder { + $scopeKey = $data['scope_key'] ?? null; + + if (! is_string($scopeKey) || $scopeKey === '') { + return $query; + } + + return $query->where('scope_key', $scopeKey); + }), + Tables\Filters\Filter::make('run_ids') + ->label('Run IDs') + ->form([ + TextInput::make('baseline_run_id') + ->label('Baseline run id') + ->numeric(), + TextInput::make('current_run_id') + ->label('Current run id') + ->numeric(), + ]) + ->query(function (Builder $query, array $data): Builder { + $baselineRunId = $data['baseline_run_id'] ?? null; + if (is_numeric($baselineRunId)) { + $query->where('baseline_run_id', (int) $baselineRunId); + } + + $currentRunId = $data['current_run_id'] ?? null; + if (is_numeric($currentRunId)) { + $query->where('current_run_id', (int) $currentRunId); + } + + return $query; + }), + ]) ->actions([ + Actions\Action::make('acknowledge') + ->label('Acknowledge') + ->icon('heroicon-o-check') + ->color('gray') + ->requiresConfirmation() + ->visible(fn (Finding $record): bool => $record->status === Finding::STATUS_NEW) + ->authorize(function (Finding $record): bool { + $user = auth()->user(); + + if (! $user instanceof User) { + return false; + } + + return $user->can('update', $record); + }) + ->action(function (Finding $record): void { + $tenant = Tenant::current(); + $user = auth()->user(); + + if (! $tenant || ! $user instanceof User) { + return; + } + + if ((int) $record->tenant_id !== (int) $tenant->getKey()) { + Notification::make() + ->title('Finding belongs to a different tenant') + ->danger() + ->send(); + + return; + } + + $record->acknowledge($user); + + Notification::make() + ->title('Finding acknowledged') + ->success() + ->send(); + }), Actions\ViewAction::make(), ]) - ->bulkActions([]); + ->bulkActions([ + BulkActionGroup::make([ + BulkAction::make('acknowledge_selected') + ->label('Acknowledge selected') + ->icon('heroicon-o-check') + ->color('gray') + ->authorize(function (): bool { + $tenant = Tenant::current(); + $user = auth()->user(); + + if (! $tenant || ! $user instanceof User) { + return false; + } + + $probe = new Finding(['tenant_id' => $tenant->getKey()]); + + return $user->can('update', $probe); + }) + ->authorizeIndividualRecords('update') + ->requiresConfirmation() + ->action(function (Collection $records): void { + $tenant = Tenant::current(); + $user = auth()->user(); + + if (! $tenant || ! $user instanceof User) { + return; + } + + $firstRecord = $records->first(); + if ($firstRecord instanceof Finding) { + Gate::authorize('update', $firstRecord); + } + + $acknowledgedCount = 0; + $skippedCount = 0; + + foreach ($records as $record) { + if (! $record instanceof Finding) { + $skippedCount++; + + continue; + } + + if ((int) $record->tenant_id !== (int) $tenant->getKey()) { + $skippedCount++; + + continue; + } + + if ($record->status !== Finding::STATUS_NEW) { + $skippedCount++; + + continue; + } + + $record->acknowledge($user); + $acknowledgedCount++; + } + + $body = "Acknowledged {$acknowledgedCount} finding".($acknowledgedCount === 1 ? '' : 's').'.'; + if ($skippedCount > 0) { + $body .= " Skipped {$skippedCount}."; + } + + Notification::make() + ->title('Bulk acknowledge completed') + ->body($body) + ->success() + ->send(); + }) + ->deselectRecordsAfterCompletion(), + ]), + ]); } public static function getEloquentQuery(): Builder @@ -52,6 +370,13 @@ public static function getEloquentQuery(): Builder $tenantId = Tenant::current()->getKey(); return parent::getEloquentQuery() + ->addSelect([ + 'subject_display_name' => InventoryItem::query() + ->select('display_name') + ->whereColumn('inventory_items.tenant_id', 'findings.tenant_id') + ->whereColumn('inventory_items.external_id', 'findings.subject_external_id') + ->limit(1), + ]) ->when($tenantId, fn (Builder $query) => $query->where('tenant_id', $tenantId)); } diff --git a/app/Filament/Resources/FindingResource/Pages/ListFindings.php b/app/Filament/Resources/FindingResource/Pages/ListFindings.php index cb872dd..0322a04 100644 --- a/app/Filament/Resources/FindingResource/Pages/ListFindings.php +++ b/app/Filament/Resources/FindingResource/Pages/ListFindings.php @@ -3,9 +3,169 @@ namespace App\Filament\Resources\FindingResource\Pages; use App\Filament\Resources\FindingResource; +use App\Models\Finding; +use App\Models\Tenant; +use App\Models\User; +use Filament\Actions; +use Filament\Forms\Components\TextInput; +use Filament\Notifications\Notification; use Filament\Resources\Pages\ListRecords; +use Illuminate\Database\Eloquent\Builder; +use Illuminate\Support\Arr; +use Illuminate\Support\Facades\Gate; class ListFindings extends ListRecords { protected static string $resource = FindingResource::class; + + protected function getHeaderActions(): array + { + return [ + Actions\Action::make('acknowledge_all_matching') + ->label('Acknowledge all matching') + ->icon('heroicon-o-check') + ->color('gray') + ->requiresConfirmation() + ->authorize(function (): bool { + $tenant = Tenant::current(); + $user = auth()->user(); + + if (! $tenant || ! $user instanceof User) { + return false; + } + + $probe = new Finding(['tenant_id' => $tenant->getKey()]); + + return $user->can('update', $probe); + }) + ->visible(fn (): bool => $this->getStatusFilterValue() === Finding::STATUS_NEW) + ->modalDescription(function (): string { + $count = $this->getAllMatchingCount(); + + return "You are about to acknowledge {$count} finding".($count === 1 ? '' : 's').' matching the current filters.'; + }) + ->form(function (): array { + $count = $this->getAllMatchingCount(); + + if ($count <= 100) { + return []; + } + + return [ + TextInput::make('confirmation') + ->label('Type ACKNOWLEDGE to confirm') + ->required() + ->in(['ACKNOWLEDGE']) + ->validationMessages([ + 'in' => 'Please type ACKNOWLEDGE to confirm.', + ]), + ]; + }) + ->action(function (array $data): void { + $tenant = Tenant::current(); + $user = auth()->user(); + + if (! $tenant || ! $user instanceof User) { + return; + } + + $query = $this->buildAllMatchingQuery(); + $count = (clone $query)->count(); + + if ($count === 0) { + Notification::make() + ->title('No matching findings') + ->body('There are no new findings matching the current filters.') + ->warning() + ->send(); + + return; + } + + $firstRecord = (clone $query)->first(); + if ($firstRecord instanceof Finding) { + Gate::authorize('update', $firstRecord); + } + + $updated = $query->update([ + 'status' => Finding::STATUS_ACKNOWLEDGED, + 'acknowledged_at' => now(), + 'acknowledged_by_user_id' => $user->getKey(), + ]); + + $this->deselectAllTableRecords(); + $this->resetPage(); + + Notification::make() + ->title('Bulk acknowledge completed') + ->body("Acknowledged {$updated} finding".($updated === 1 ? '' : 's').'.') + ->success() + ->send(); + }), + ]; + } + + protected function buildAllMatchingQuery(): Builder + { + $tenant = Tenant::current(); + + $query = Finding::query(); + + if (! $tenant) { + return $query->whereRaw('1 = 0'); + } + + $query->where('tenant_id', $tenant->getKey()); + + $query->where('status', Finding::STATUS_NEW); + + $findingType = $this->getFindingTypeFilterValue(); + if (is_string($findingType) && $findingType !== '') { + $query->where('finding_type', $findingType); + } + + $scopeKeyState = $this->getTableFilterState('scope_key') ?? []; + $scopeKey = Arr::get($scopeKeyState, 'scope_key'); + if (is_string($scopeKey) && $scopeKey !== '') { + $query->where('scope_key', $scopeKey); + } + + $runIdsState = $this->getTableFilterState('run_ids') ?? []; + $baselineRunId = Arr::get($runIdsState, 'baseline_run_id'); + if (is_numeric($baselineRunId)) { + $query->where('baseline_run_id', (int) $baselineRunId); + } + + $currentRunId = Arr::get($runIdsState, 'current_run_id'); + if (is_numeric($currentRunId)) { + $query->where('current_run_id', (int) $currentRunId); + } + + return $query; + } + + protected function getAllMatchingCount(): int + { + return (int) $this->buildAllMatchingQuery()->count(); + } + + protected function getStatusFilterValue(): string + { + $state = $this->getTableFilterState('status') ?? []; + $value = Arr::get($state, 'value'); + + return is_string($value) && $value !== '' + ? $value + : Finding::STATUS_NEW; + } + + protected function getFindingTypeFilterValue(): string + { + $state = $this->getTableFilterState('finding_type') ?? []; + $value = Arr::get($state, 'value'); + + return is_string($value) && $value !== '' + ? $value + : Finding::FINDING_TYPE_DRIFT; + } } diff --git a/app/Jobs/GenerateDriftFindingsJob.php b/app/Jobs/GenerateDriftFindingsJob.php index 645dae4..353cb77 100644 --- a/app/Jobs/GenerateDriftFindingsJob.php +++ b/app/Jobs/GenerateDriftFindingsJob.php @@ -2,15 +2,19 @@ namespace App\Jobs; +use App\Models\BulkOperationRun; use App\Models\InventorySyncRun; use App\Models\Tenant; +use App\Services\BulkOperationService; use App\Services\Drift\DriftFindingGenerator; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; +use Illuminate\Support\Facades\Log; use RuntimeException; +use Throwable; class GenerateDriftFindingsJob implements ShouldQueue { @@ -22,13 +26,22 @@ public function __construct( public int $baselineRunId, public int $currentRunId, public string $scopeKey, + public int $bulkOperationRunId, ) {} /** * Execute the job. */ - public function handle(DriftFindingGenerator $generator): void + public function handle(DriftFindingGenerator $generator, BulkOperationService $bulkOperationService): void { + Log::info('GenerateDriftFindingsJob: started', [ + 'tenant_id' => $this->tenantId, + 'baseline_run_id' => $this->baselineRunId, + 'current_run_id' => $this->currentRunId, + 'scope_key' => $this->scopeKey, + 'bulk_operation_run_id' => $this->bulkOperationRunId, + ]); + $tenant = Tenant::query()->find($this->tenantId); if (! $tenant instanceof Tenant) { throw new RuntimeException('Tenant not found.'); @@ -44,11 +57,48 @@ public function handle(DriftFindingGenerator $generator): void throw new RuntimeException('Current run not found.'); } - $generator->generate( - tenant: $tenant, - baseline: $baseline, - current: $current, - scopeKey: $this->scopeKey, - ); + $run = BulkOperationRun::query() + ->where('tenant_id', $tenant->getKey()) + ->find($this->bulkOperationRunId); + + if (! $run instanceof BulkOperationRun) { + throw new RuntimeException('Bulk operation run not found.'); + } + + $bulkOperationService->start($run); + + try { + $created = $generator->generate( + tenant: $tenant, + baseline: $baseline, + current: $current, + scopeKey: $this->scopeKey, + ); + + Log::info('GenerateDriftFindingsJob: completed', [ + 'tenant_id' => $this->tenantId, + 'baseline_run_id' => $this->baselineRunId, + 'current_run_id' => $this->currentRunId, + 'scope_key' => $this->scopeKey, + 'bulk_operation_run_id' => $this->bulkOperationRunId, + 'created_findings_count' => $created, + ]); + + $bulkOperationService->recordSuccess($run); + $bulkOperationService->complete($run); + } catch (Throwable $e) { + Log::error('GenerateDriftFindingsJob: failed', [ + 'tenant_id' => $this->tenantId, + 'baseline_run_id' => $this->baselineRunId, + 'current_run_id' => $this->currentRunId, + 'scope_key' => $this->scopeKey, + 'bulk_operation_run_id' => $this->bulkOperationRunId, + 'error' => $e->getMessage(), + ]); + + $bulkOperationService->fail($run, $e->getMessage()); + + throw $e; + } } } diff --git a/app/Models/Finding.php b/app/Models/Finding.php index c0e8173..5f94e31 100644 --- a/app/Models/Finding.php +++ b/app/Models/Finding.php @@ -61,5 +61,7 @@ public function acknowledge(User $user): void 'acknowledged_at' => now(), 'acknowledged_by_user_id' => $user->getKey(), ]); + + $this->save(); } } diff --git a/app/Services/Drift/DriftFindingDiffBuilder.php b/app/Services/Drift/DriftFindingDiffBuilder.php new file mode 100644 index 0000000..1196947 --- /dev/null +++ b/app/Services/Drift/DriftFindingDiffBuilder.php @@ -0,0 +1,304 @@ + + */ + public function buildSettingsDiff(?PolicyVersion $baselineVersion, ?PolicyVersion $currentVersion): array + { + $policyType = $currentVersion?->policy_type ?? $baselineVersion?->policy_type ?? ''; + $platform = $currentVersion?->platform ?? $baselineVersion?->platform; + + $from = $baselineVersion + ? $this->settingsNormalizer->normalizeForDiff(is_array($baselineVersion->snapshot) ? $baselineVersion->snapshot : [], (string) $policyType, $platform) + : []; + + $to = $currentVersion + ? $this->settingsNormalizer->normalizeForDiff(is_array($currentVersion->snapshot) ? $currentVersion->snapshot : [], (string) $policyType, $platform) + : []; + + $result = $this->versionDiff->compare($from, $to); + $result['policy_type'] = $policyType; + + return $result; + } + + /** + * @return array + */ + public function buildAssignmentsDiff(Tenant $tenant, ?PolicyVersion $baselineVersion, ?PolicyVersion $currentVersion, int $limit = 200): array + { + $baseline = $baselineVersion ? $this->assignmentsNormalizer->normalizeForDiff($baselineVersion->assignments) : []; + $current = $currentVersion ? $this->assignmentsNormalizer->normalizeForDiff($currentVersion->assignments) : []; + + $baselineMap = []; + foreach ($baseline as $row) { + $baselineMap[$row['key']] = $row; + } + + $currentMap = []; + foreach ($current as $row) { + $currentMap[$row['key']] = $row; + } + + $allKeys = array_values(array_unique(array_merge(array_keys($baselineMap), array_keys($currentMap)))); + sort($allKeys); + + $added = []; + $removed = []; + $changed = []; + + foreach ($allKeys as $key) { + $from = $baselineMap[$key] ?? null; + $to = $currentMap[$key] ?? null; + + if ($from === null && is_array($to)) { + $added[] = $to; + + continue; + } + + if ($to === null && is_array($from)) { + $removed[] = $from; + + continue; + } + + if (! is_array($from) || ! is_array($to)) { + continue; + } + + $diffFields = [ + 'filter_type', + 'filter_id', + 'intent', + 'mode', + ]; + + $fieldChanges = []; + + foreach ($diffFields as $field) { + $fromValue = $from[$field] ?? null; + $toValue = $to[$field] ?? null; + + if ($fromValue !== $toValue) { + $fieldChanges[$field] = [ + 'from' => $fromValue, + 'to' => $toValue, + ]; + } + } + + if ($fieldChanges !== []) { + $changed[] = [ + 'key' => $key, + 'include_exclude' => $to['include_exclude'], + 'target_type' => $to['target_type'], + 'target_id' => $to['target_id'], + 'from' => $from, + 'to' => $to, + 'changes' => $fieldChanges, + ]; + } + } + + $truncated = false; + + $total = count($added) + count($removed) + count($changed); + if ($total > $limit) { + $truncated = true; + + $budget = $limit; + + $changed = array_slice($changed, 0, min(count($changed), $budget)); + $budget -= count($changed); + + $added = array_slice($added, 0, min(count($added), $budget)); + $budget -= count($added); + + $removed = array_slice($removed, 0, min(count($removed), $budget)); + } + + $labels = $this->groupLabelsForDiff($tenant, $added, $removed, $changed); + + $decorateAssignment = function (array $row) use ($labels): array { + $row['target_label'] = $this->targetLabel($row, $labels); + + return $row; + }; + + $decorateChanged = function (array $row) use ($decorateAssignment): array { + $row['from'] = is_array($row['from'] ?? null) ? $decorateAssignment($row['from']) : $row['from']; + $row['to'] = is_array($row['to'] ?? null) ? $decorateAssignment($row['to']) : $row['to']; + $row['target_label'] = is_array($row['to'] ?? null) ? ($row['to']['target_label'] ?? null) : null; + + return $row; + }; + + return [ + 'summary' => [ + 'added' => count($added), + 'removed' => count($removed), + 'changed' => count($changed), + 'message' => sprintf('%d added, %d removed, %d changed', count($added), count($removed), count($changed)), + 'truncated' => $truncated, + 'limit' => $limit, + ], + 'added' => array_map($decorateAssignment, $added), + 'removed' => array_map($decorateAssignment, $removed), + 'changed' => array_map($decorateChanged, $changed), + ]; + } + + /** + * @return array + */ + public function buildScopeTagsDiff(?PolicyVersion $baselineVersion, ?PolicyVersion $currentVersion): array + { + $baselineIds = $baselineVersion ? ($this->scopeTagsNormalizer->normalizeIdsForHash($baselineVersion->scope_tags) ?? []) : []; + $currentIds = $currentVersion ? ($this->scopeTagsNormalizer->normalizeIdsForHash($currentVersion->scope_tags) ?? []) : []; + + $baselineLabels = $baselineVersion ? $this->scopeTagsNormalizer->labelsById($baselineVersion->scope_tags) : []; + $currentLabels = $currentVersion ? $this->scopeTagsNormalizer->labelsById($currentVersion->scope_tags) : []; + + $baselineSet = array_fill_keys($baselineIds, true); + $currentSet = array_fill_keys($currentIds, true); + + $addedIds = array_values(array_diff($currentIds, $baselineIds)); + $removedIds = array_values(array_diff($baselineIds, $currentIds)); + + sort($addedIds); + sort($removedIds); + + $decorate = static function (array $ids, array $labels): array { + $rows = []; + + foreach ($ids as $id) { + if (! is_string($id) || $id === '') { + continue; + } + + $rows[] = [ + 'id' => $id, + 'name' => $labels[$id] ?? ($id === '0' ? 'Default' : $id), + ]; + } + + return $rows; + }; + + return [ + 'summary' => [ + 'added' => count($addedIds), + 'removed' => count($removedIds), + 'changed' => 0, + 'message' => sprintf('%d added, %d removed', count($addedIds), count($removedIds)), + 'baseline_count' => count($baselineSet), + 'current_count' => count($currentSet), + ], + 'added' => $decorate($addedIds, $currentLabels), + 'removed' => $decorate($removedIds, $baselineLabels), + 'baseline' => $decorate($baselineIds, $baselineLabels), + 'current' => $decorate($currentIds, $currentLabels), + 'changed' => [], + ]; + } + + /** + * @param array> $added + * @param array> $removed + * @param array> $changed + * @return array + */ + private function groupLabelsForDiff(Tenant $tenant, array $added, array $removed, array $changed): array + { + $groupIds = []; + + foreach ([$added, $removed] as $items) { + foreach ($items as $row) { + $targetType = $row['target_type'] ?? null; + $targetId = $row['target_id'] ?? null; + + if (! is_string($targetType) || ! is_string($targetId)) { + continue; + } + + if (! str_contains($targetType, 'groupassignmenttarget')) { + continue; + } + + $groupIds[] = $targetId; + } + } + + foreach ($changed as $row) { + $targetType = $row['target_type'] ?? null; + $targetId = $row['target_id'] ?? null; + + if (! is_string($targetType) || ! is_string($targetId)) { + continue; + } + + if (! str_contains($targetType, 'groupassignmenttarget')) { + continue; + } + + $groupIds[] = $targetId; + } + + $groupIds = array_values(array_unique($groupIds)); + + if ($groupIds === []) { + return []; + } + + return $this->groupLabelResolver->resolveMany($tenant, $groupIds); + } + + /** + * @param array $assignment + * @param array $groupLabels + */ + private function targetLabel(array $assignment, array $groupLabels): string + { + $targetType = $assignment['target_type'] ?? null; + $targetId = $assignment['target_id'] ?? null; + + if (! is_string($targetType) || ! is_string($targetId)) { + return 'Unknown target'; + } + + if (str_contains($targetType, 'alldevicesassignmenttarget')) { + return 'All devices'; + } + + if (str_contains($targetType, 'allusersassignmenttarget')) { + return 'All users'; + } + + if (str_contains($targetType, 'groupassignmenttarget')) { + return $groupLabels[$targetId] ?? EntraGroupLabelResolver::formatLabel(null, $targetId); + } + + return sprintf('%s (%s)', $targetType, $targetId); + } +} diff --git a/app/Services/Drift/DriftFindingGenerator.php b/app/Services/Drift/DriftFindingGenerator.php index 1fc88fa..1944925 100644 --- a/app/Services/Drift/DriftFindingGenerator.php +++ b/app/Services/Drift/DriftFindingGenerator.php @@ -7,6 +7,8 @@ use App\Models\Policy; use App\Models\PolicyVersion; use App\Models\Tenant; +use App\Services\Drift\Normalizers\ScopeTagsNormalizer; +use App\Services\Drift\Normalizers\SettingsNormalizer; use Illuminate\Support\Arr; use RuntimeException; @@ -15,6 +17,8 @@ class DriftFindingGenerator public function __construct( private readonly DriftHasher $hasher, private readonly DriftEvidence $evidence, + private readonly SettingsNormalizer $settingsNormalizer, + private readonly ScopeTagsNormalizer $scopeTagsNormalizer, ) {} public function generate(Tenant $tenant, InventorySyncRun $baseline, InventorySyncRun $current, string $scopeKey): int @@ -48,48 +52,139 @@ public function generate(Tenant $tenant, InventorySyncRun $baseline, InventorySy $baselineVersion = $this->versionForRun($policy, $baseline); $currentVersion = $this->versionForRun($policy, $current); + if ($baselineVersion instanceof PolicyVersion || $currentVersion instanceof PolicyVersion) { + $policyType = (string) ($policy->policy_type ?? ''); + $platform = is_string($policy->platform ?? null) ? $policy->platform : null; + + $baselineSnapshot = $baselineVersion instanceof PolicyVersion && is_array($baselineVersion->snapshot) + ? $baselineVersion->snapshot + : []; + $currentSnapshot = $currentVersion instanceof PolicyVersion && is_array($currentVersion->snapshot) + ? $currentVersion->snapshot + : []; + + $baselineNormalized = $this->settingsNormalizer->normalizeForDiff($baselineSnapshot, $policyType, $platform); + $currentNormalized = $this->settingsNormalizer->normalizeForDiff($currentSnapshot, $policyType, $platform); + + $baselineSnapshotHash = $this->hasher->hashNormalized($baselineNormalized); + $currentSnapshotHash = $this->hasher->hashNormalized($currentNormalized); + + if ($baselineSnapshotHash !== $currentSnapshotHash) { + $changeType = match (true) { + $baselineVersion instanceof PolicyVersion && ! $currentVersion instanceof PolicyVersion => 'removed', + ! $baselineVersion instanceof PolicyVersion && $currentVersion instanceof PolicyVersion => 'added', + default => 'modified', + }; + + $fingerprint = $this->hasher->fingerprint( + tenantId: (int) $tenant->getKey(), + scopeKey: $scopeKey, + subjectType: 'policy', + subjectExternalId: (string) $policy->external_id, + changeType: $changeType, + baselineHash: $baselineSnapshotHash, + currentHash: $currentSnapshotHash, + ); + + $rawEvidence = [ + 'change_type' => $changeType, + 'summary' => [ + 'kind' => 'policy_snapshot', + 'changed_fields' => ['snapshot_hash'], + ], + 'baseline' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $baselineVersion?->getKey(), + 'snapshot_hash' => $baselineSnapshotHash, + ], + 'current' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $currentVersion?->getKey(), + 'snapshot_hash' => $currentSnapshotHash, + ], + ]; + + $finding = Finding::query()->firstOrNew([ + 'tenant_id' => $tenant->getKey(), + 'fingerprint' => $fingerprint, + ]); + + $wasNew = ! $finding->exists; + + $finding->forceFill([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'scope_key' => $scopeKey, + 'baseline_run_id' => $baseline->getKey(), + 'current_run_id' => $current->getKey(), + 'subject_type' => 'policy', + 'subject_external_id' => (string) $policy->external_id, + 'severity' => Finding::SEVERITY_MEDIUM, + 'evidence_jsonb' => $this->evidence->sanitize($rawEvidence), + ]); + + if ($wasNew) { + $finding->forceFill([ + 'status' => Finding::STATUS_NEW, + 'acknowledged_at' => null, + 'acknowledged_by_user_id' => null, + ]); + } + + $finding->save(); + + if ($wasNew) { + $created++; + } + } + } + if (! $baselineVersion instanceof PolicyVersion || ! $currentVersion instanceof PolicyVersion) { continue; } - $baselineAssignmentsHash = $baselineVersion->assignments_hash ?? null; - $currentAssignmentsHash = $currentVersion->assignments_hash ?? null; + $baselineAssignments = is_array($baselineVersion->assignments) ? $baselineVersion->assignments : []; + $currentAssignments = is_array($currentVersion->assignments) ? $currentVersion->assignments : []; - if ($baselineAssignmentsHash === $currentAssignmentsHash) { - continue; - } + $baselineAssignmentsHash = $this->hasher->hashNormalized($baselineAssignments); + $currentAssignmentsHash = $this->hasher->hashNormalized($currentAssignments); - $fingerprint = $this->hasher->fingerprint( - tenantId: (int) $tenant->getKey(), - scopeKey: $scopeKey, - subjectType: 'assignment', - subjectExternalId: (string) $policy->external_id, - changeType: 'modified', - baselineHash: (string) ($baselineAssignmentsHash ?? ''), - currentHash: (string) ($currentAssignmentsHash ?? ''), - ); + if ($baselineAssignmentsHash !== $currentAssignmentsHash) { + $fingerprint = $this->hasher->fingerprint( + tenantId: (int) $tenant->getKey(), + scopeKey: $scopeKey, + subjectType: 'assignment', + subjectExternalId: (string) $policy->external_id, + changeType: 'modified', + baselineHash: (string) ($baselineAssignmentsHash ?? ''), + currentHash: (string) ($currentAssignmentsHash ?? ''), + ); - $rawEvidence = [ - 'change_type' => 'modified', - 'summary' => 'Policy assignments changed', - 'baseline' => [ - 'policy_id' => $policy->external_id, - 'policy_version_id' => $baselineVersion->getKey(), - 'assignments_hash' => $baselineAssignmentsHash, - ], - 'current' => [ - 'policy_id' => $policy->external_id, - 'policy_version_id' => $currentVersion->getKey(), - 'assignments_hash' => $currentAssignmentsHash, - ], - ]; + $rawEvidence = [ + 'change_type' => 'modified', + 'summary' => [ + 'kind' => 'policy_assignments', + 'changed_fields' => ['assignments_hash'], + ], + 'baseline' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $baselineVersion->getKey(), + 'assignments_hash' => $baselineAssignmentsHash, + ], + 'current' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $currentVersion->getKey(), + 'assignments_hash' => $currentAssignmentsHash, + ], + ]; - Finding::query()->updateOrCreate( - [ + $finding = Finding::query()->firstOrNew([ 'tenant_id' => $tenant->getKey(), 'fingerprint' => $fingerprint, - ], - [ + ]); + + $wasNew = ! $finding->exists; + + $finding->forceFill([ 'finding_type' => Finding::FINDING_TYPE_DRIFT, 'scope_key' => $scopeKey, 'baseline_run_id' => $baseline->getKey(), @@ -97,14 +192,97 @@ public function generate(Tenant $tenant, InventorySyncRun $baseline, InventorySy 'subject_type' => 'assignment', 'subject_external_id' => (string) $policy->external_id, 'severity' => Finding::SEVERITY_MEDIUM, + 'evidence_jsonb' => $this->evidence->sanitize($rawEvidence), + ]); + + if ($wasNew) { + $finding->forceFill([ + 'status' => Finding::STATUS_NEW, + 'acknowledged_at' => null, + 'acknowledged_by_user_id' => null, + ]); + } + + $finding->save(); + + if ($wasNew) { + $created++; + } + } + + $baselineScopeTagIds = $this->scopeTagsNormalizer->normalizeIdsForHash($baselineVersion->scope_tags); + $currentScopeTagIds = $this->scopeTagsNormalizer->normalizeIdsForHash($currentVersion->scope_tags); + + if ($baselineScopeTagIds === null || $currentScopeTagIds === null) { + continue; + } + + $baselineScopeTagsHash = $this->hasher->hashNormalized($baselineScopeTagIds); + $currentScopeTagsHash = $this->hasher->hashNormalized($currentScopeTagIds); + + if ($baselineScopeTagsHash === $currentScopeTagsHash) { + continue; + } + + $fingerprint = $this->hasher->fingerprint( + tenantId: (int) $tenant->getKey(), + scopeKey: $scopeKey, + subjectType: 'scope_tag', + subjectExternalId: (string) $policy->external_id, + changeType: 'modified', + baselineHash: $baselineScopeTagsHash, + currentHash: $currentScopeTagsHash, + ); + + $rawEvidence = [ + 'change_type' => 'modified', + 'summary' => [ + 'kind' => 'policy_scope_tags', + 'changed_fields' => ['scope_tags_hash'], + ], + 'baseline' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $baselineVersion->getKey(), + 'scope_tags_hash' => $baselineScopeTagsHash, + ], + 'current' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $currentVersion->getKey(), + 'scope_tags_hash' => $currentScopeTagsHash, + ], + ]; + + $finding = Finding::query()->firstOrNew([ + 'tenant_id' => $tenant->getKey(), + 'fingerprint' => $fingerprint, + ]); + + $wasNew = ! $finding->exists; + + $finding->forceFill([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'scope_key' => $scopeKey, + 'baseline_run_id' => $baseline->getKey(), + 'current_run_id' => $current->getKey(), + 'subject_type' => 'scope_tag', + 'subject_external_id' => (string) $policy->external_id, + 'severity' => Finding::SEVERITY_MEDIUM, + 'evidence_jsonb' => $this->evidence->sanitize($rawEvidence), + ]); + + if ($wasNew) { + $finding->forceFill([ 'status' => Finding::STATUS_NEW, 'acknowledged_at' => null, 'acknowledged_by_user_id' => null, - 'evidence_jsonb' => $this->evidence->sanitize($rawEvidence), - ], - ); + ]); + } - $created++; + $finding->save(); + + if ($wasNew) { + $created++; + } } }); diff --git a/app/Services/Drift/DriftHasher.php b/app/Services/Drift/DriftHasher.php index 6ca3aee..6f9ec9d 100644 --- a/app/Services/Drift/DriftHasher.php +++ b/app/Services/Drift/DriftHasher.php @@ -4,6 +4,24 @@ class DriftHasher { + /** + * @param array $volatileKeys + */ + public function hashNormalized(mixed $value, array $volatileKeys = [ + '@odata.context', + '@odata.etag', + 'createdDateTime', + 'lastModifiedDateTime', + 'modifiedDateTime', + 'createdAt', + 'updatedAt', + ]): string + { + $normalized = $this->normalizeValue($value, $volatileKeys); + + return hash('sha256', json_encode($normalized, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)); + } + public function fingerprint( int $tenantId, string $scopeKey, @@ -30,4 +48,54 @@ private function normalize(string $value): string { return trim(mb_strtolower($value)); } + + /** + * @param array $volatileKeys + */ + private function normalizeValue(mixed $value, array $volatileKeys): mixed + { + if (is_array($value)) { + if ($this->isList($value)) { + $items = array_map(fn ($item) => $this->normalizeValue($item, $volatileKeys), $value); + + usort($items, function ($a, $b): int { + return strcmp( + json_encode($a, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE), + json_encode($b, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) + ); + }); + + return $items; + } + + $result = []; + + foreach ($value as $key => $item) { + if (is_string($key) && in_array($key, $volatileKeys, true)) { + continue; + } + + $result[$key] = $this->normalizeValue($item, $volatileKeys); + } + + ksort($result); + + return $result; + } + + if (is_string($value)) { + return trim($value); + } + + return $value; + } + + private function isList(array $value): bool + { + if ($value === []) { + return true; + } + + return array_keys($value) === range(0, count($value) - 1); + } } diff --git a/app/Services/Drift/Normalizers/AssignmentsNormalizer.php b/app/Services/Drift/Normalizers/AssignmentsNormalizer.php new file mode 100644 index 0000000..77b85c6 --- /dev/null +++ b/app/Services/Drift/Normalizers/AssignmentsNormalizer.php @@ -0,0 +1,113 @@ + + */ + public function normalizeForDiff(mixed $assignments): array + { + if (! is_array($assignments)) { + return []; + } + + $rows = []; + + foreach ($assignments as $assignment) { + if (! is_array($assignment)) { + continue; + } + + $target = $assignment['target'] ?? null; + if (! is_array($target)) { + continue; + } + + $rawType = $target['@odata.type'] ?? null; + $targetType = $this->normalizeOdataType(is_string($rawType) ? $rawType : ''); + + $includeExclude = str_contains($targetType, 'exclusion') ? 'exclude' : 'include'; + $targetId = $this->extractTargetId($targetType, $target); + + if ($targetId === '') { + continue; + } + + $filterId = $target['deviceAndAppManagementAssignmentFilterId'] ?? null; + $filterType = $target['deviceAndAppManagementAssignmentFilterType'] ?? 'none'; + + $intent = $assignment['intent'] ?? null; + $mode = $assignment['mode'] ?? null; + + $row = [ + 'key' => implode('|', [ + $includeExclude, + $targetType, + $targetId, + ]), + 'include_exclude' => $includeExclude, + 'target_type' => $targetType, + 'target_id' => $targetId, + 'filter_type' => is_string($filterType) && $filterType !== '' ? strtolower(trim($filterType)) : 'none', + 'filter_id' => is_string($filterId) && $filterId !== '' ? $filterId : null, + 'intent' => is_string($intent) && $intent !== '' ? strtolower(trim($intent)) : null, + 'mode' => is_string($mode) && $mode !== '' ? strtolower(trim($mode)) : null, + ]; + + $rows[] = $row; + } + + usort($rows, function (array $a, array $b): int { + return strcmp($a['key'], $b['key']); + }); + + return array_values($rows); + } + + private function normalizeOdataType(string $odataType): string + { + $value = trim($odataType); + $value = ltrim($value, '#'); + + if ($value === '') { + return 'unknown'; + } + + if (str_contains($value, '.')) { + $value = (string) strrchr($value, '.'); + $value = ltrim($value, '.'); + } + + return strtolower(trim($value)); + } + + /** + * @param array $target + */ + private function extractTargetId(string $targetType, array $target): string + { + if (str_contains($targetType, 'alldevicesassignmenttarget')) { + return 'all_devices'; + } + + if (str_contains($targetType, 'allusersassignmenttarget')) { + return 'all_users'; + } + + $groupId = Arr::get($target, 'groupId'); + if (is_string($groupId) && $groupId !== '') { + return $groupId; + } + + $collectionId = Arr::get($target, 'collectionId'); + if (is_string($collectionId) && $collectionId !== '') { + return $collectionId; + } + + return ''; + } +} diff --git a/app/Services/Drift/Normalizers/ScopeTagsNormalizer.php b/app/Services/Drift/Normalizers/ScopeTagsNormalizer.php new file mode 100644 index 0000000..a543d15 --- /dev/null +++ b/app/Services/Drift/Normalizers/ScopeTagsNormalizer.php @@ -0,0 +1,136 @@ + + */ + public function normalizeIds(mixed $scopeTags): array + { + return $this->normalizeIdsForHash($scopeTags) ?? []; + } + + /** + * For drift hashing/comparison we need stable, reliable IDs. + * + * Legacy policy versions may have only `names` without `ids`. In that case we: + * - infer `Default` as id `0` + * - otherwise return null (unknown/unreliable; should not create drift) + * + * @return array|null + */ + public function normalizeIdsForHash(mixed $scopeTags): ?array + { + if (! is_array($scopeTags)) { + return []; + } + + $ids = $scopeTags['ids'] ?? null; + if (is_array($ids)) { + $normalized = []; + + foreach ($ids as $id) { + if (! is_string($id)) { + continue; + } + + $id = trim($id); + + if ($id === '') { + continue; + } + + $normalized[] = $id; + } + + $normalized = array_values(array_unique($normalized)); + sort($normalized); + + return $normalized; + } + + $names = $scopeTags['names'] ?? null; + if (! is_array($names) || $names === []) { + return []; + } + + $normalizedNames = []; + + foreach ($names as $name) { + if (! is_string($name)) { + continue; + } + + $name = trim($name); + + if ($name === '') { + continue; + } + + $normalizedNames[] = strtolower($name); + } + + $normalizedNames = array_values(array_unique($normalizedNames)); + sort($normalizedNames); + + if ($normalizedNames === ['default']) { + return ['0']; + } + + return null; + } + + /** + * @return array + */ + public function labelsById(mixed $scopeTags): array + { + if (! is_array($scopeTags)) { + return []; + } + + $ids = is_array($scopeTags['ids'] ?? null) ? $scopeTags['ids'] : null; + $names = is_array($scopeTags['names'] ?? null) ? $scopeTags['names'] : []; + + if (! is_array($ids)) { + $inferred = $this->normalizeIdsForHash($scopeTags); + + if ($inferred === ['0'] && $names !== []) { + return ['0' => 'Default']; + } + + return []; + } + + $labels = []; + + foreach ($ids as $index => $id) { + if (! is_string($id)) { + continue; + } + + $id = trim($id); + + if ($id === '') { + continue; + } + + $name = $names[$index] ?? ''; + $name = is_string($name) ? trim($name) : ''; + + if ($name === '') { + $name = $id === '0' ? 'Default' : $id; + } + + if (! array_key_exists($id, $labels) || $labels[$id] === $id) { + $labels[$id] = $name; + } + } + + ksort($labels); + + return $labels; + } +} diff --git a/app/Services/Drift/Normalizers/SettingsNormalizer.php b/app/Services/Drift/Normalizers/SettingsNormalizer.php new file mode 100644 index 0000000..38b352e --- /dev/null +++ b/app/Services/Drift/Normalizers/SettingsNormalizer.php @@ -0,0 +1,19 @@ +|null $snapshot + * @return array + */ + public function normalizeForDiff(?array $snapshot, string $policyType, ?string $platform = null): array + { + return $this->policyNormalizer->flattenForDiff($snapshot ?? [], $policyType, $platform); + } +} diff --git a/config/intune_permissions.php b/config/intune_permissions.php index 609aedc..96677d8 100644 --- a/config/intune_permissions.php +++ b/config/intune_permissions.php @@ -6,13 +6,13 @@ 'key' => 'DeviceManagementConfiguration.ReadWrite.All', 'type' => 'application', 'description' => 'Read and write Intune device configuration policies.', - 'features' => ['policy-sync', 'backup', 'restore', 'settings-normalization'], + 'features' => ['policy-sync', 'backup', 'restore', 'settings-normalization', 'drift'], ], [ 'key' => 'DeviceManagementConfiguration.Read.All', 'type' => 'application', 'description' => 'Read Intune device configuration policies (least-privilege for inventory).', - 'features' => ['policy-sync', 'backup', 'settings-normalization'], + 'features' => ['policy-sync', 'backup', 'settings-normalization', 'drift'], ], [ 'key' => 'DeviceManagementApps.ReadWrite.All', @@ -72,7 +72,7 @@ 'key' => 'Group.Read.All', 'type' => 'application', 'description' => 'Read group information for resolving assignment group names and cross-tenant group mapping.', - 'features' => ['assignments', 'group-mapping', 'backup-metadata', 'directory-groups', 'group-directory-cache'], + 'features' => ['assignments', 'group-mapping', 'backup-metadata', 'directory-groups', 'group-directory-cache', 'drift'], ], [ 'key' => 'DeviceManagementScripts.ReadWrite.All', diff --git a/resources/views/filament/infolists/entries/assignments-diff.blade.php b/resources/views/filament/infolists/entries/assignments-diff.blade.php new file mode 100644 index 0000000..1118937 --- /dev/null +++ b/resources/views/filament/infolists/entries/assignments-diff.blade.php @@ -0,0 +1,114 @@ +@php + $diff = $getState() ?? []; + $summary = $diff['summary'] ?? []; + + $added = is_array($diff['added'] ?? null) ? $diff['added'] : []; + $removed = is_array($diff['removed'] ?? null) ? $diff['removed'] : []; + $changed = is_array($diff['changed'] ?? null) ? $diff['changed'] : []; + + $renderRow = static function (array $row): array { + return [ + 'include_exclude' => (string) ($row['include_exclude'] ?? 'include'), + 'target_label' => (string) ($row['target_label'] ?? 'Unknown target'), + 'filter_type' => (string) ($row['filter_type'] ?? 'none'), + 'filter_id' => $row['filter_id'] ?? null, + 'intent' => $row['intent'] ?? null, + 'mode' => $row['mode'] ?? null, + ]; + }; +@endphp + +
+ +
+ + {{ (int) ($summary['added'] ?? 0) }} added + + + {{ (int) ($summary['removed'] ?? 0) }} removed + + + {{ (int) ($summary['changed'] ?? 0) }} changed + + + @if (($summary['truncated'] ?? false) === true) + + Truncated to {{ (int) ($summary['limit'] ?? 0) }} items + + @endif +
+ + + @if ($changed !== []) + +
+ @foreach ($changed as $row) + @php + $to = is_array($row['to'] ?? null) ? $renderRow($row['to']) : $renderRow([]); + $from = is_array($row['from'] ?? null) ? $renderRow($row['from']) : $renderRow([]); + @endphp + +
+
+ {{ $to['target_label'] }} +
+ +
+
+
From
+
+
Type: {{ $from['include_exclude'] }}
+
Filter: {{ $from['filter_type'] }}@if($from['filter_id']) ({{ $from['filter_id'] }})@endif
+
+
+
+
To
+
+
Type: {{ $to['include_exclude'] }}
+
Filter: {{ $to['filter_type'] }}@if($to['filter_id']) ({{ $to['filter_id'] }})@endif
+
+
+
+
+ @endforeach +
+ + @endif + + @if ($added !== []) + +
+ @foreach ($added as $row) + @php $row = $renderRow(is_array($row) ? $row : []); @endphp + +
+
{{ $row['target_label'] }}
+
+ {{ $row['include_exclude'] }} · filter: {{ $row['filter_type'] }} +
+
+ @endforeach +
+ + @endif + + @if ($removed !== []) + +
+ @foreach ($removed as $row) + @php $row = $renderRow(is_array($row) ? $row : []); @endphp + +
+
{{ $row['target_label'] }}
+
+ {{ $row['include_exclude'] }} · filter: {{ $row['filter_type'] }} +
+
+ @endforeach +
+ + @endif +
diff --git a/resources/views/filament/infolists/entries/scope-tags-diff.blade.php b/resources/views/filament/infolists/entries/scope-tags-diff.blade.php new file mode 100644 index 0000000..324f574 --- /dev/null +++ b/resources/views/filament/infolists/entries/scope-tags-diff.blade.php @@ -0,0 +1,111 @@ +@php + $diff = $getState() ?? []; + $summary = $diff['summary'] ?? []; + + $added = is_array($diff['added'] ?? null) ? $diff['added'] : []; + $removed = is_array($diff['removed'] ?? null) ? $diff['removed'] : []; + $baseline = is_array($diff['baseline'] ?? null) ? $diff['baseline'] : []; + $current = is_array($diff['current'] ?? null) ? $diff['current'] : []; +@endphp + +
+ +
+ + {{ (int) ($summary['added'] ?? 0) }} added + + + {{ (int) ($summary['removed'] ?? 0) }} removed + + + Baseline: {{ (int) ($summary['baseline_count'] ?? 0) }} + + + Current: {{ (int) ($summary['current_count'] ?? 0) }} + +
+ + + @if ($added !== []) + +
+ @foreach ($added as $row) + @php + $name = (string) ($row['name'] ?? 'Unknown'); + $id = (string) ($row['id'] ?? ''); + @endphp + +
+
{{ $name }}
+ @if ($id !== '') +
{{ $id }}
+ @endif +
+ @endforeach +
+ + @endif + + @if ($removed !== []) + +
+ @foreach ($removed as $row) + @php + $name = (string) ($row['name'] ?? 'Unknown'); + $id = (string) ($row['id'] ?? ''); + @endphp + +
+
{{ $name }}
+ @if ($id !== '') +
{{ $id }}
+ @endif +
+ @endforeach +
+ + @endif + + @if ($current !== []) + +
+ @foreach ($current as $row) + @php + $name = (string) ($row['name'] ?? 'Unknown'); + $id = (string) ($row['id'] ?? ''); + @endphp + +
+
{{ $name }}
+ @if ($id !== '') +
{{ $id }}
+ @endif +
+ @endforeach +
+ + @endif + + @if ($baseline !== []) + +
+ @foreach ($baseline as $row) + @php + $name = (string) ($row['name'] ?? 'Unknown'); + $id = (string) ($row['id'] ?? ''); + @endphp + +
+
{{ $name }}
+ @if ($id !== '') +
{{ $id }}
+ @endif +
+ @endforeach +
+ + @endif +
diff --git a/resources/views/filament/pages/drift-landing.blade.php b/resources/views/filament/pages/drift-landing.blade.php index 9b631d5..d1703ed 100644 --- a/resources/views/filament/pages/drift-landing.blade.php +++ b/resources/views/filament/pages/drift-landing.blade.php @@ -5,6 +5,98 @@ Review new drift findings between the last two inventory sync runs for the current scope. + @if (filled($scopeKey)) +
+ Scope: {{ $scopeKey }} + @if ($baselineRunId && $currentRunId) + · Baseline + @if ($this->getBaselineRunUrl()) + + #{{ $baselineRunId }} + + @else + #{{ $baselineRunId }} + @endif + @if (filled($baselineFinishedAt)) + ({{ $baselineFinishedAt }}) + @endif + · Current + @if ($this->getCurrentRunUrl()) + + #{{ $currentRunId }} + + @else + #{{ $currentRunId }} + @endif + @if (filled($currentFinishedAt)) + ({{ $currentFinishedAt }}) + @endif + @endif +
+ @endif + + @if ($state === 'blocked') + + Blocked + + + @if (filled($message)) +
+ {{ $message }} +
+ @endif + @elseif ($state === 'generating') + + Generating + + +
+ Drift generation has been queued. Refresh this page once it finishes. +
+ + @if ($this->getBulkRunUrl()) +
+ + View run #{{ $bulkOperationRunId }} + +
+ @endif + @elseif ($state === 'error') + + Error + + + @if (filled($message)) +
+ {{ $message }} +
+ @endif + + @if ($this->getBulkRunUrl()) +
+ + View run #{{ $bulkOperationRunId }} + +
+ @endif + @elseif ($state === 'ready') +
+ + New: {{ (int) ($statusCounts['new'] ?? 0) }} + +
+ + @if (filled($message)) +
+ {{ $message }} +
+ @endif + @else + + Ready + + @endif +
Findings diff --git a/specs/044-drift-mvp/checklists/requirements.md b/specs/044-drift-mvp/checklists/requirements.md index 04dab55..ceb096c 100644 --- a/specs/044-drift-mvp/checklists/requirements.md +++ b/specs/044-drift-mvp/checklists/requirements.md @@ -6,28 +6,28 @@ # Specification Quality Checklist: Drift MVP (044) ## Content Quality -- [ ] No implementation details (languages, frameworks, APIs) (T002) -- [x] Focused on user value and business needs (spec.md: Purpose, User Scenarios & Testing, Success Criteria) -- [ ] Written for non-technical stakeholders (T002) -- [x] All mandatory sections completed (spec.md includes Purpose, Scenarios, FR/NFR, Success Criteria, Out of Scope) +- [x] No implementation details (languages, frameworks, APIs) (spec.md contains scenarios/rules/states/acceptance only) +- [x] Focused on user value and business needs (spec.md: Purpose, User Scenarios, Acceptance Criteria) +- [x] Written for non-technical stakeholders (spec.md uses plain language; avoids code/framework terms) +- [x] All mandatory sections completed (spec.md includes Purpose, User Scenarios, Rules, Acceptance Criteria) ## Requirement Completeness - [x] No [NEEDS CLARIFICATION] markers remain (spec.md: no "[NEEDS CLARIFICATION]" markers) -- [x] Requirements are testable and unambiguous (spec.md: FR1–FR4; tasks.md defines tests for key behaviors T015–T018, T024–T025, T029–T030, T035, T038) -- [x] Success criteria are measurable (spec.md: SC1 "under 3 minutes", SC2 deterministic consistency) -- [x] Success criteria are technology-agnostic (no implementation details) (spec.md: SC1–SC2) +- [x] Requirements are testable and unambiguous (spec.md: Rules + Acceptance Criteria) +- [x] Success criteria are measurable (spec.md: Acceptance Criteria) +- [x] Success criteria are technology-agnostic (no implementation details) (spec.md: Acceptance Criteria) - [x] All acceptance scenarios are defined (spec.md: Scenario 1/2/3) -- [x] Edge cases are identified (spec.md: <2 runs blocked state; generation failure explicit error state; acknowledgement per comparison) -- [x] Scope is clearly bounded (spec.md: FR2b + Out of Scope) -- [x] Dependencies and assumptions identified (spec.md: Dependencies / Name Resolution; NFR2; "No render-time Graph calls") +- [x] Edge cases are identified (spec.md: blocked state; error state; acknowledgement per comparison) +- [x] Scope is clearly bounded (spec.md: Rules → Coverage (MVP)) +- [x] Dependencies and assumptions identified (spec.md: Rules → UI states; Run tracking) ## Feature Readiness -- [x] All functional requirements have clear acceptance criteria (spec.md: FR1–FR4 + Scenario 1/2/3) +- [x] All functional requirements have clear acceptance criteria (spec.md: Rules + Acceptance Criteria) - [x] User scenarios cover primary flows (spec.md: Scenario 1/2/3) -- [ ] Feature meets measurable outcomes defined in Success Criteria (T022, T023, T026, T027, T031, T033, T035) -- [ ] No implementation details leak into specification (T002) +- [x] Feature meets measurable outcomes defined in Success Criteria (spec.md: Acceptance Criteria are measurable and testable) +- [x] No implementation details leak into specification (spec.md avoids implementation and names a generic “persisted run record” only) ## Notes diff --git a/specs/044-drift-mvp/contracts/admin-findings.openapi.yaml b/specs/044-drift-mvp/contracts/admin-findings.openapi.yaml index 3c742bd..7d20e3c 100644 --- a/specs/044-drift-mvp/contracts/admin-findings.openapi.yaml +++ b/specs/044-drift-mvp/contracts/admin-findings.openapi.yaml @@ -102,9 +102,30 @@ paths: responses: '202': description: Accepted + content: + application/json: + schema: + type: object + properties: + data: + $ref: '#/components/schemas/DriftGenerateAccepted' components: schemas: + DriftGenerateAccepted: + type: object + properties: + bulk_operation_run_id: + type: integer + description: Canonical async run record (status/errors/idempotency) + scope_key: + type: string + baseline_run_id: + type: integer + current_run_id: + type: integer + required: [bulk_operation_run_id, scope_key, baseline_run_id, current_run_id] + Finding: type: object properties: diff --git a/specs/044-drift-mvp/plan.md b/specs/044-drift-mvp/plan.md index 81fe07e..a448d66 100644 --- a/specs/044-drift-mvp/plan.md +++ b/specs/044-drift-mvp/plan.md @@ -11,6 +11,7 @@ ## Summary - Baseline run = previous successful run for the same scope; comparison run = latest successful run. - Findings are persisted with deterministic fingerprints and support MVP triage (`new` → `acknowledged`). - UI is DB-only for label/name resolution (no render-time Graph calls). +- Drift generation is tracked via `BulkOperationRun` for status/errors across refresh and idempotency. ## Technical Context @@ -28,6 +29,7 @@ ## Technical Context - Tenant isolation for all reads/writes - No render-time Graph calls; labels resolved from DB caches - Evidence minimization (sanitized allowlist; no raw payload dumps) +- Drift generation is job-only and uses `BulkOperationRun` (`resource=drift`, `action=generate`) as the canonical run record **Scale/Scope**: - Tenants may have large inventories; findings must be indexed for typical filtering diff --git a/specs/044-drift-mvp/quickstart.md b/specs/044-drift-mvp/quickstart.md index 2e709a3..b38df8b 100644 --- a/specs/044-drift-mvp/quickstart.md +++ b/specs/044-drift-mvp/quickstart.md @@ -15,14 +15,16 @@ ## Prepare data ## Use Drift 1. Navigate to the new Drift area. -2. On first open, Drift will dispatch a background job to generate findings for: +2. On first open, Drift will queue background generation and record status in a persisted run record. +3. Generation produces findings for: - baseline = previous successful run for the same `scope_key` - current = latest successful run for the same `scope_key` -3. Refresh the page once the job finishes. +4. Refresh the page once generation finishes. ## Triage -- Acknowledge a finding; it should move out of the “new” view but remain visible/auditable. +- Acknowledge a finding; it moves out of the default “new” view but remains visible/auditable. +- Use the status filter to include acknowledged findings. ## Notes diff --git a/specs/044-drift-mvp/spec.md b/specs/044-drift-mvp/spec.md index a3a2a61..e6c04e0 100644 --- a/specs/044-drift-mvp/spec.md +++ b/specs/044-drift-mvp/spec.md @@ -1,166 +1,81 @@ -# Feature Specification: Drift MVP - -**Feature Branch**: `feat/044-drift-mvp` -**Created**: 2026-01-07 -**Status**: Draft +# Feature Specification: Drift MVP (044) ## Purpose -Detect and report drift between expected and observed states using inventory and run metadata. +Help admins quickly spot and triage configuration “drift”: what changed between two inventory snapshots. -This MVP focuses on reporting and triage, not automatic remediation. +This MVP is about visibility and acknowledgement (triage), not automatic fixes. -## Clarifications - -### Session 2026-01-12 - -- Q: How should Drift pick the baseline run for a given tenant + scope? → A: Baseline = previous successful inventory run for the same scope; compare against the latest successful run. -- Q: Should Drift findings be persisted or computed on demand? → A: Persist findings in DB per comparison (baseline_run_id + current_run_id), including a deterministic fingerprint for stable identity + triage. -- Q: How define the fingerprint (Stable ID) for a drift finding? → A: `sha256(tenant_id + scope_key + subject_type + subject_external_id + change_type + baseline_hash + current_hash)` (normalized; excludes volatile fields). -- Q: Which inventory entities/types are in scope for Drift MVP? → A: Policies + Assignments. -- Q: When should drift findings be generated? → A: On-demand when opening Drift: if findings for (baseline,current,scope) don’t exist yet, dispatch an async job to generate them. - -### Session 2026-01-13 - -- Q: What should Drift do if there are fewer than two successful inventory runs for the same `scope_key`? → A: Show a blocked/empty state (“Need at least 2 successful runs for this scope to calculate drift”) and do not dispatch drift generation. -- Q: Should acknowledgement carry forward across comparisons? → A: No; acknowledgement is per comparison (`baseline_run_id` + `current_run_id` + `scope_key`). The same drift may re-appear as `new` in later comparisons. -- Q: Which `change_type` values are supported in Drift MVP? → A: `added`, `removed`, `modified` (assignment target/intent changes are covered under `modified`). -- Q: What is the default UI behavior for `new` vs `acknowledged` findings? → A: Default UI shows only `new`; `acknowledged` is accessible via an explicit filter. -- Q: What should the UI do if drift generation fails for a comparison? → A: Show an explicit error state (safe message + reference/run ids) and do not show findings for that comparison until a successful generation exists. - -### Session 2026-01-14 - -- Q: How should Drift track generation status/errors/idempotency for a comparison? → A: Use `BulkOperationRun` as the canonical run container (status, failures, idempotency_key, and consistent UI/ops patterns). - -## Pinned Decisions (MVP defaults) - -- Drift is implemented as a generator that writes persisted Finding rows (not only an in-memory/on-demand diff). -- Baseline selection: baseline = previous successful inventory run for the same scope_key; comparison = latest successful inventory run for the same scope_key. -- Scope is first-class via `scope_key` and must be deterministic to support future pinned baselines and compare workflows. -- Fingerprints are deterministic and stable for triage/audit workflows. -- Drift MVP only uses `finding_type=drift` and `status` in {`new`, `acknowledged`}. -- Default severity: `medium` (until a rule engine exists). -- UI must not perform render-time Graph calls. Graph access (if any) is limited to background sync/jobs. -- Drift generation is tracked via `BulkOperationRun` to persist status/errors across refresh and to enforce idempotency per (tenant, scope_key, baseline_run_id, current_run_id). - -## Key Entities / Generic Findings (Future-proof) - -### Finding (generic) - -We want Drift MVP to remain MVP-sized, while making it easy to add future generators (Security Suite Audits, Cross-tenant Compare) without inventing a new model. - -Rationale: -- Drift = delta engine over runs. -- Audit = rule engine over inventory. -- Both write Findings with the same semantics: deterministic fingerprint + triage + minimized evidence. - -- `finding_type` (enum): `drift` (MVP), later `audit`, `compare` -- `tenant_id` -- `scope_key` (string): deterministic scope identifier (see Scope Definition / FR1) -- `baseline_run_id` (nullable; e.g. audit/compare) -- `current_run_id` (nullable; e.g. audit) -- `fingerprint` (string): deterministic; unique per tenant+scope+subject+change -- `subject_type` (string): e.g. policy type (or other inventory entity type) -- `subject_external_id` (string): Graph external id -- `severity` (enum): `low` / `medium` / `high` (MVP default: `medium`) -- `status` (enum): `new` / `acknowledged` (later: `snoozed` / `assigned` / `commented`) -- `acknowledged_at` (nullable) -- `acknowledged_by_user_id` (nullable) -- `evidence_jsonb` (jsonb): sanitized, small, secrets-free (no raw payload dumps) -- Optional/nullable for later (prepared; out of MVP): `rule_id`, `control_id`, `expected_value`, `source` - -MVP implementation scope: only `finding_type=drift`, statuses `new/acknowledged`, and no rule engine. - -## User Scenarios & Testing +## User Scenarios ### Scenario 1: View drift summary -- Given inventory sync has run at least twice -- When the admin opens Drift -- Then they see a summary of changes since the last baseline -- If there are fewer than two successful runs for the same `scope_key`, Drift shows a blocked/empty state and does not start drift generation. +- Given the system has at least two successful inventory snapshots for the same selection/scope +- When an admin opens Drift +- Then they see a summary of what was added, removed, or changed since the previous snapshot ### Scenario 2: Drill into a drift finding + +- Given drift findings exist for a comparison +- When an admin opens a specific finding +- Then they can see what changed and which two snapshots were compared + +### Scenario 3: Acknowledge / triage + - Given a drift finding exists -- When the admin opens the finding -- Then they see what changed, when, and which run observed it +- When an admin acknowledges it +- Then it no longer appears in “new” views, but remains available for audit/history -### Scenario 3: Acknowledge/triage -- Given a drift finding exists -- When the admin marks it acknowledged -- Then it is hidden from “new” lists but remains auditable +## Rules -- Acknowledgement is per comparison; later comparisons may still surface the same drift as `new`. +### Coverage (MVP) -## Functional Requirements +- Drift findings cover **policies, their assignments, and scope tags** for the selected scope. -- FR1: Baseline + scope - - Define `scope_key` as the deterministic Inventory selection identifier. - - MVP definition: `scope_key = InventorySyncRun.selection_hash`. - - Rationale: selection hashing already normalizes equivalent selections; reusing it keeps drift scope stable and consistent across the product. - - Baseline run (MVP) = previous successful inventory run for the same `scope_key`. - - Comparison run (MVP) = latest successful inventory run for the same `scope_key`. +### Baseline and comparison selection -- FR2: Finding generation (Drift MVP) - - Findings are persisted per (`baseline_run_id`, `current_run_id`, `scope_key`). - - Findings cover adds, removals, and changes for supported entities (Policies + Assignments). - - MVP `change_type` values: `added`, `removed`, `modified`. - - Findings are deterministic: same baseline/current + scope_key ⇒ same set of fingerprints. - - Drift generation must be tracked via `BulkOperationRun` with an idempotency key derived from (tenant_id, scope_key, baseline_run_id, current_run_id). - - If fewer than two successful inventory runs exist for a given `scope_key`, Drift does not generate findings and must surface a clear blocked/empty state in the UI. +- Drift always compares two successful inventory snapshots for the same selection/scope. +- The “current” snapshot is the latest successful snapshot for that scope. +- The “baseline” snapshot is the previous successful snapshot for that scope. -- FR2a: Fingerprint definition (MVP) - - Fingerprint = `sha256(tenant_id + scope_key + subject_type + subject_external_id + change_type + baseline_hash + current_hash)`. - - `baseline_hash` / `current_hash` are hashes over normalized, sanitized comparison data (exclude volatile fields like timestamps). - - Goal: stable identity for triage + audit compatibility. +### Change types -- FR2b: Drift MVP scope includes Policies and their Assignments. - - Assignment drift includes target changes (e.g., groupId) and intent changes. +Each drift finding must be categorized as one of: -- FR3: Provide Drift UI with summary and details. - - Default lists and the Drift landing summary show only `status=new` by default. - - The UI must provide a filter to include `acknowledged` findings. - - If drift generation fails for a comparison, the UI must surface an explicit error state (no secrets), including reference identifiers (e.g., run ids and the `BulkOperationRun` id), and must not fall back to stale/previous results. +- **added**: the item exists in current but not in baseline +- **removed**: the item exists in baseline but not in current +- **modified**: the item exists in both but differs (including assignment target and/or intent changes) -- FR4: Triage (MVP) - - Admin can acknowledge a finding; record `acknowledged_by_user_id` + `acknowledged_at`. - - Acknowledgement does not carry forward across comparisons in the MVP. - - Findings are never deleted in the MVP. +### Acknowledgement -## Non-Functional Requirements +- Acknowledgement is **per comparison** (baseline + current within a scope). +- Acknowledgement does **not** carry forward to later comparisons. -- NFR1: Drift generation must be deterministic for the same baseline and scope. -- NFR2: Drift must remain tenant-scoped and safe to display. -- NFR3: Evidence minimization - - `evidence_jsonb` must be sanitized (no tokens/secrets) and kept small. - - MVP drift evidence should include only: - - `change_type` - - changed_fields / metadata summary (counts, field list) - - run refs (baseline_run_id/current_run_id, timestamps) - - No raw payload dumps. +### UI states -## Dependencies / Name Resolution +- **blocked**: If fewer than two successful snapshots exist for the same scope, Drift shows a clear blocked state and does not attempt generation. +- **error**: If drift generation fails for a comparison, Drift shows a clear error state with safe information and reference identifiers to the recorded run. -- Drift/Audit UI should resolve labels via Inventory + Foundations (047) + Groups Cache (051) where applicable. -- No render-time Graph calls (Graph only in background sync/jobs, never in UI render). +### Default views -## Success Criteria +- Default Drift summary and default finding lists show **new** findings only. +- Acknowledged findings are accessible via an explicit filter. -- SC1: Admins can identify drift across supported types (Policies + Assignments) in under 3 minutes. -- SC2: Drift results are consistent across repeated generation for the same baseline. +### Run tracking (status, errors, idempotency) -## Out of Scope +- Drift generation status and errors must be recorded in a **persisted run record** so that progress/failure survives refresh and can be inspected later. +- Re-opening Drift for the same comparison must be idempotent (it should not create duplicate work for the same comparison). -- Automatic revert/promotion. -- Rule engine in MVP (Audit later), but the data model is prepared via `rule_id` / `control_id` / `expected_value`. +### Determinism and stable identity -## Future Work (non-MVP) +- For the same scope + baseline + current, Drift must produce the same set of findings. +- Each finding must have a stable identifier (“fingerprint”) so triage actions can reliably reference the same drift item within a comparison. -- Security Suite Audits: add rule-based generators that write Findings (no new Finding model). -- Cross-tenant Compare: may write Findings (`finding_type=compare`) or emit a compatible format that can be stored as Findings. +## Acceptance Criteria -## Related Specs - -- Program: `specs/039-inventory-program/spec.md` -- Core: `specs/040-inventory-core/spec.md` -- Compare: `specs/043-cross-tenant-compare-and-promotion/spec.md` +- With two successful snapshots for the same scope, Drift shows a summary of **added/removed/modified** items for that comparison. +- With fewer than two successful snapshots for the same scope, Drift shows **blocked** and does not start generation. +- If generation fails, Drift shows **error** and provides reference identifiers to the persisted run record. +- Default views exclude acknowledged findings, and acknowledged findings remain available via filter. +- Acknowledging a finding records who/when acknowledged and hides it from “new” views. +- Re-running generation for the same comparison does not create duplicate work and produces consistent results. diff --git a/specs/044-drift-mvp/tasks.md b/specs/044-drift-mvp/tasks.md index 481f990..f6dd823 100644 --- a/specs/044-drift-mvp/tasks.md +++ b/specs/044-drift-mvp/tasks.md @@ -25,10 +25,10 @@ ## Phase 1: Setup (Shared Infrastructure) **Purpose**: Project wiring for Drift MVP. -- [ ] T001 Create/update constitution gate checklist in `specs/044-drift-mvp/checklists/requirements.md` -- [ ] T002 Confirm spec/plan artifacts are current in `specs/044-drift-mvp/{plan.md,spec.md,research.md,data-model.md,quickstart.md,contracts/admin-findings.openapi.yaml}` -- [ ] T003 Add Drift landing page shell in `app/Filament/Pages/DriftLanding.php` -- [ ] T004 [P] Add Finding resource shells in `app/Filament/Resources/FindingResource.php` and `app/Filament/Resources/FindingResource/Pages/` +- [x] T001 Create/update constitution gate checklist in `specs/044-drift-mvp/checklists/requirements.md` +- [x] T002 Confirm spec/plan artifacts are current in `specs/044-drift-mvp/{plan.md,spec.md,research.md,data-model.md,quickstart.md,contracts/admin-findings.openapi.yaml}` +- [x] T003 Add Drift landing page shell in `app/Filament/Pages/DriftLanding.php` +- [x] T004 [P] Add Finding resource shells in `app/Filament/Resources/FindingResource.php` and `app/Filament/Resources/FindingResource/Pages/` --- @@ -38,17 +38,17 @@ ## Phase 2: Foundational (Blocking Prerequisites) **Checkpoint**: DB schema exists, tenant scoping enforced, and tests can create Finding rows. -- [ ] T005 Create `findings` migration in `database/migrations/*_create_findings_table.php` with Finding fields aligned to `specs/044-drift-mvp/spec.md`: +- [x] T005 Create `findings` migration in `database/migrations/*_create_findings_table.php` with Finding fields aligned to `specs/044-drift-mvp/spec.md`: (tenant_id, finding_type, scope_key, baseline_run_id, current_run_id, subject_type, subject_external_id, severity, status, fingerprint unique, evidence_jsonb, acknowledged_at, acknowledged_by_user_id) -- [ ] T006 Create `Finding` model in `app/Models/Finding.php` (casts for `evidence_jsonb`, enums/constants for `finding_type`/`severity`/`status`, `acknowledged_at` handling) -- [ ] T007 [P] Add `FindingFactory` in `database/factories/FindingFactory.php` -- [ ] T008 Ensure `InventorySyncRunFactory` exists (or add it) in `database/factories/InventorySyncRunFactory.php` -- [ ] T009 Add authorization policy in `app/Policies/FindingPolicy.php` and wire it in `app/Providers/AuthServiceProvider.php` (or project equivalent) -- [ ] T010 Add Drift permissions in `config/intune_permissions.php` (view + acknowledge) and wire them into Filament navigation/actions -- [ ] T011 Enforce tenant scoping for Finding queries in `app/Models/Finding.php` and/or `app/Filament/Resources/FindingResource.php` -- [ ] T012 Implement fingerprint helper in `app/Services/Drift/DriftHasher.php` (sha256 per spec) -- [ ] T013 Pin `scope_key = InventorySyncRun.selection_hash` in `app/Services/Drift/DriftScopeKey.php` (single canonical definition) -- [ ] T014 Implement evidence allowlist builder in `app/Services/Drift/DriftEvidence.php` (new file) +- [x] T006 Create `Finding` model in `app/Models/Finding.php` (casts for `evidence_jsonb`, enums/constants for `finding_type`/`severity`/`status`, `acknowledged_at` handling) +- [x] T007 [P] Add `FindingFactory` in `database/factories/FindingFactory.php` +- [x] T008 Ensure `InventorySyncRunFactory` exists (or add it) in `database/factories/InventorySyncRunFactory.php` +- [x] T009 Add authorization policy in `app/Policies/FindingPolicy.php` and wire it in `app/Providers/AuthServiceProvider.php` (or project equivalent) +- [x] T010 Add Drift permissions in `config/intune_permissions.php` (view + acknowledge) and wire them into Filament navigation/actions +- [x] T011 Enforce tenant scoping for Finding queries in `app/Models/Finding.php` and/or `app/Filament/Resources/FindingResource.php` +- [x] T012 Implement fingerprint helper in `app/Services/Drift/DriftHasher.php` (sha256 per spec) +- [x] T013 Pin `scope_key = InventorySyncRun.selection_hash` in `app/Services/Drift/DriftScopeKey.php` (single canonical definition) +- [x] T014 Implement evidence allowlist builder in `app/Services/Drift/DriftEvidence.php` (new file) --- @@ -60,18 +60,18 @@ ## Phase 3: User Story 1 - View drift summary (Priority: P1) MVP ### Tests (write first) -- [ ] T015 [P] [US1] Baseline selection tests in `tests/Feature/Drift/DriftBaselineSelectionTest.php` -- [ ] T016 [P] [US1] Generation dispatch tests in `tests/Feature/Drift/DriftGenerationDispatchTest.php` -- [ ] T017 [P] [US1] Tenant isolation tests in `tests/Feature/Drift/DriftTenantIsolationTest.php` -- [ ] T018 [P] [US1] Assignment drift detection test (targets + intent changes per FR2b) in `tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php` +- [x] T015 [P] [US1] Baseline selection tests in `tests/Feature/Drift/DriftBaselineSelectionTest.php` +- [x] T016 [P] [US1] Generation dispatch tests in `tests/Feature/Drift/DriftGenerationDispatchTest.php` +- [x] T017 [P] [US1] Tenant isolation tests in `tests/Feature/Drift/DriftTenantIsolationTest.php` +- [x] T018 [P] [US1] Assignment drift detection test (targets + intent changes per FR2b) in `tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php` ### Implementation -- [ ] T019 [US1] Implement run selection service in `app/Services/Drift/DriftRunSelector.php` -- [ ] T020 [US1] Implement generator job in `app/Jobs/GenerateDriftFindingsJob.php` (dedupe/lock by tenant+scope+baseline+current) -- [ ] T021 [US1] Implement generator service in `app/Services/Drift/DriftFindingGenerator.php` (idempotent) -- [ ] T022 [US1] Implement landing behavior (dispatch + status UI) in `app/Filament/Pages/DriftLanding.php` -- [ ] T023 [US1] Implement summary queries/widgets in `app/Filament/Pages/DriftLanding.php` +- [x] T019 [US1] Implement run selection service in `app/Services/Drift/DriftRunSelector.php` +- [x] T021 [US1] Implement generator service in `app/Services/Drift/DriftFindingGenerator.php` (idempotent) +- [x] T020 [US1] Implement generator job in `app/Jobs/GenerateDriftFindingsJob.php` (dedupe/lock by tenant+scope+baseline+current) +- [x] T022 [US1] Implement landing behavior (dispatch + status UI) in `app/Filament/Pages/DriftLanding.php` +- [x] T023 [US1] Implement summary queries/widgets in `app/Filament/Pages/DriftLanding.php` --- @@ -83,14 +83,17 @@ ## Phase 4: User Story 2 - Drill into a drift finding (Priority: P2) ### Tests (write first) -- [ ] T024 [P] [US2] Finding detail test in `tests/Feature/Drift/DriftFindingDetailTest.php` -- [ ] T025 [P] [US2] Evidence minimization test in `tests/Feature/Drift/DriftEvidenceMinimizationTest.php` +- [x] T024 [P] [US2] Finding detail test in `tests/Feature/Drift/DriftFindingDetailTest.php` +- [x] T025 [P] [US2] Evidence minimization test in `tests/Feature/Drift/DriftEvidenceMinimizationTest.php` +- [x] T041 [P] [US2] Finding detail shows normalized settings diff (DB-only) in `tests/Feature/Drift/DriftFindingDetailShowsSettingsDiffTest.php` +- [x] T042 [P] [US2] Finding detail shows assignments diff + cached group labels (DB-only) in `tests/Feature/Drift/DriftFindingDetailShowsAssignmentsDiffTest.php` ### Implementation -- [ ] T026 [US2] Implement list UI in `app/Filament/Resources/FindingResource.php` (filters: status, scope_key, run) -- [ ] T027 [US2] Implement detail UI in `app/Filament/Resources/FindingResource/Pages/ViewFinding.php` -- [ ] T028 [US2] Implement DB-only name resolution in `app/Filament/Resources/FindingResource.php` (inventory/foundations caches) +- [x] T026 [US2] Implement list UI in `app/Filament/Resources/FindingResource.php` (filters: status, scope_key, run) +- [x] T027 [US2] Implement detail UI in `app/Filament/Resources/FindingResource/Pages/ViewFinding.php` +- [x] T028 [US2] Implement DB-only name resolution in `app/Filament/Resources/FindingResource.php` (inventory/foundations caches) +- [x] T043 [US2] Add real diffs to Finding detail (settings + assignments) in `app/Filament/Resources/FindingResource.php`, `app/Services/Drift/*`, and `resources/views/filament/infolists/entries/assignments-diff.blade.php` --- @@ -102,25 +105,57 @@ ## Phase 5: User Story 3 - Acknowledge/triage (Priority: P3) ### Tests (write first) -- [ ] T029 [P] [US3] Acknowledge action test in `tests/Feature/Drift/DriftAcknowledgeTest.php` -- [ ] T030 [P] [US3] Acknowledge authorization test in `tests/Feature/Drift/DriftAcknowledgeAuthorizationTest.php` +- [x] T029 [P] [US3] Acknowledge action test in `tests/Feature/Drift/DriftAcknowledgeTest.php` +- [x] T030 [P] [US3] Acknowledge authorization test in `tests/Feature/Drift/DriftAcknowledgeAuthorizationTest.php` ### Implementation -- [ ] T031 [US3] Add acknowledge actions in `app/Filament/Resources/FindingResource.php` -- [ ] T032 [US3] Implement `acknowledge()` domain method in `app/Models/Finding.php` -- [ ] T033 [US3] Ensure Drift summary excludes acknowledged by default in `app/Filament/Pages/DriftLanding.php` +- [x] T031 [US3] Add acknowledge actions in `app/Filament/Resources/FindingResource.php` +- [x] T032 [US3] Implement `acknowledge()` domain method in `app/Models/Finding.php` +- [x] T033 [US3] Ensure Drift summary excludes acknowledged by default in `app/Filament/Pages/DriftLanding.php` + +### Bulk triage (post-MVP UX) + +**Goal**: Admin can acknowledge many findings safely and quickly. + +### Tests (write first) + +- [x] T048 [P] [US3] Bulk acknowledge selected test in `tests/Feature/Drift/DriftBulkAcknowledgeTest.php` +- [x] T049 [P] [US3] Acknowledge all matching current filters test in `tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingTest.php` +- [x] T050 [P] [US3] Acknowledge all matching requires type-to-confirm when >100 in `tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingConfirmationTest.php` +- [x] T051 [P] [US3] Bulk acknowledge authorization test in `tests/Feature/Drift/DriftBulkAcknowledgeAuthorizationTest.php` + +### Implementation + +- [x] T052 [US3] Add bulk triage actions to Findings list in `app/Filament/Resources/FindingResource.php` and `app/Filament/Resources/FindingResource/Pages/ListFindings.php` --- ## Phase 6: Polish & Cross-Cutting Concerns -- [ ] T034 Add DB indexes in `database/migrations/*_create_findings_table.php` (tenant_id+status, tenant_id+scope_key, tenant_id+baseline_run_id, tenant_id+current_run_id) -- [ ] T035 [P] Add determinism test in `tests/Feature/Drift/DriftGenerationDeterminismTest.php` (same baseline/current => same fingerprints) -- [ ] T036 Add job observability logs in `app/Jobs/GenerateDriftFindingsJob.php` (no secrets) -- [ ] T037 Add idempotency/upsert strategy in `app/Services/Drift/DriftFindingGenerator.php` -- [ ] T038 Ensure volatile fields excluded from hashing in `app/Services/Drift/DriftHasher.php` and cover in `tests/Feature/Drift/DriftHasherTest.php` -- [ ] T039 Validate and update `specs/044-drift-mvp/quickstart.md` after implementation +- [x] T034 Add DB indexes in `database/migrations/*_create_findings_table.php` (tenant_id+status, tenant_id+scope_key, tenant_id+baseline_run_id, tenant_id+current_run_id) +- [x] T035 [P] Add determinism test in `tests/Feature/Drift/DriftGenerationDeterminismTest.php` (same baseline/current => same fingerprints) +- [x] T036 Add job observability logs in `app/Jobs/GenerateDriftFindingsJob.php` (no secrets) +- [x] T037 Add idempotency/upsert strategy in `app/Services/Drift/DriftFindingGenerator.php` +- [x] T038 Ensure volatile fields excluded from hashing in `app/Services/Drift/DriftHasher.php` and cover in `tests/Feature/Drift/DriftHasherTest.php` +- [x] T039 Validate and update `specs/044-drift-mvp/quickstart.md` after implementation +- [x] T040 Fix drift evidence summary shape in `app/Services/Drift/DriftFindingGenerator.php` and cover in `tests/Feature/Drift/DriftPolicySnapshotDriftDetectionTest.php` (snapshot_hash vs assignments_hash) + +--- + +## Phase 7: Scope Tags Drift (Post-MVP Fix) + +**Goal**: Detect and display drift caused by `roleScopeTagIds` changes on a policy version. + +### Tests (write first) + +- [x] T044 [P] [US1] Scope-tag drift detection test in `tests/Feature/Drift/DriftScopeTagDriftDetectionTest.php` +- [x] T045 [P] [US2] Finding detail shows scope-tags diff (DB-only) in `tests/Feature/Drift/DriftFindingDetailShowsScopeTagsDiffTest.php` + +### Implementation + +- [x] T046 [US1] Implement scope-tag drift detection in `app/Services/Drift/DriftFindingGenerator.php` (deterministic hashing; kind=`policy_scope_tags`) +- [x] T047 [US2] Implement scope-tags diff builder + UI in `app/Services/Drift/DriftFindingDiffBuilder.php`, `app/Filament/Resources/FindingResource.php`, and `resources/views/filament/infolists/entries/scope-tags-diff.blade.php` --- diff --git a/tests/Feature/Drift/DriftAcknowledgeAuthorizationTest.php b/tests/Feature/Drift/DriftAcknowledgeAuthorizationTest.php new file mode 100644 index 0000000..c687664 --- /dev/null +++ b/tests/Feature/Drift/DriftAcknowledgeAuthorizationTest.php @@ -0,0 +1,31 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $finding = Finding::factory()->for($tenant)->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'status' => Finding::STATUS_NEW, + ]); + + $thrown = null; + + try { + Livewire::test(ListFindings::class) + ->callTableAction('acknowledge', $finding); + } catch (Throwable $exception) { + $thrown = $exception; + } + + expect($thrown)->not->toBeNull(); + + $finding->refresh(); + expect($finding->status)->toBe(Finding::STATUS_NEW); +}); diff --git a/tests/Feature/Drift/DriftAcknowledgeTest.php b/tests/Feature/Drift/DriftAcknowledgeTest.php new file mode 100644 index 0000000..05f8784 --- /dev/null +++ b/tests/Feature/Drift/DriftAcknowledgeTest.php @@ -0,0 +1,28 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $finding = Finding::factory()->for($tenant)->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'status' => Finding::STATUS_NEW, + 'acknowledged_at' => null, + 'acknowledged_by_user_id' => null, + ]); + + Livewire::test(ListFindings::class) + ->callTableAction('acknowledge', $finding); + + $finding->refresh(); + + expect($finding->status)->toBe(Finding::STATUS_ACKNOWLEDGED); + expect($finding->acknowledged_at)->not->toBeNull(); + expect((int) $finding->acknowledged_by_user_id)->toBe((int) $user->getKey()); +}); diff --git a/tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php b/tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php index 5a186ea..dee9bc6 100644 --- a/tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php +++ b/tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php @@ -52,7 +52,6 @@ 'policy_type' => $policy->policy_type, 'captured_at' => $baseline->finished_at->copy()->subMinute(), 'assignments' => $baselineAssignments, - 'assignments_hash' => hash('sha256', json_encode($baselineAssignments)), ]); PolicyVersion::factory()->for($tenant)->for($policy)->create([ @@ -60,7 +59,6 @@ 'policy_type' => $policy->policy_type, 'captured_at' => $current->finished_at->copy()->subMinute(), 'assignments' => $currentAssignments, - 'assignments_hash' => hash('sha256', json_encode($currentAssignments)), ]); $generator = app(DriftFindingGenerator::class); diff --git a/tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingConfirmationTest.php b/tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingConfirmationTest.php new file mode 100644 index 0000000..936ff40 --- /dev/null +++ b/tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingConfirmationTest.php @@ -0,0 +1,34 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $findings = Finding::factory() + ->count(101) + ->for($tenant) + ->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'status' => Finding::STATUS_NEW, + ]); + + Livewire::test(ListFindings::class) + ->mountAction('acknowledge_all_matching') + ->callMountedAction(); + + $findings->each(fn (Finding $finding) => expect($finding->refresh()->status)->toBe(Finding::STATUS_NEW)); + + Livewire::test(ListFindings::class) + ->mountAction('acknowledge_all_matching') + ->setActionData(['confirmation' => 'ACKNOWLEDGE']) + ->callMountedAction() + ->assertHasNoActionErrors(); + + $findings->each(fn (Finding $finding) => expect($finding->refresh()->status)->toBe(Finding::STATUS_ACKNOWLEDGED)); +}); diff --git a/tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingTest.php b/tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingTest.php new file mode 100644 index 0000000..b283d35 --- /dev/null +++ b/tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingTest.php @@ -0,0 +1,51 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $scopeA = 'scope-a'; + $scopeB = 'scope-b'; + + $matching = Finding::factory() + ->count(2) + ->for($tenant) + ->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'status' => Finding::STATUS_NEW, + 'scope_key' => $scopeA, + ]); + + $nonMatching = Finding::factory() + ->for($tenant) + ->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'status' => Finding::STATUS_NEW, + 'scope_key' => $scopeB, + ]); + + Livewire::test(ListFindings::class) + ->set('tableFilters', [ + 'status' => ['value' => Finding::STATUS_NEW], + 'finding_type' => ['value' => Finding::FINDING_TYPE_DRIFT], + 'scope_key' => ['scope_key' => $scopeA], + ]) + ->callAction('acknowledge_all_matching'); + + $matching->each(function (Finding $finding) use ($user): void { + $finding->refresh(); + + expect($finding->status)->toBe(Finding::STATUS_ACKNOWLEDGED); + expect((int) $finding->acknowledged_by_user_id)->toBe((int) $user->getKey()); + }); + + $nonMatching->refresh(); + expect($nonMatching->status)->toBe(Finding::STATUS_NEW); + expect($nonMatching->acknowledged_at)->toBeNull(); +}); diff --git a/tests/Feature/Drift/DriftBulkAcknowledgeAuthorizationTest.php b/tests/Feature/Drift/DriftBulkAcknowledgeAuthorizationTest.php new file mode 100644 index 0000000..4903049 --- /dev/null +++ b/tests/Feature/Drift/DriftBulkAcknowledgeAuthorizationTest.php @@ -0,0 +1,60 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $findings = Finding::factory() + ->count(2) + ->for($tenant) + ->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'status' => Finding::STATUS_NEW, + ]); + + $thrown = null; + + try { + Livewire::test(ListFindings::class) + ->callTableBulkAction('acknowledge_selected', $findings); + } catch (Throwable $exception) { + $thrown = $exception; + } + + expect($thrown)->not->toBeNull(); + + $findings->each(fn (Finding $finding) => expect($finding->refresh()->status)->toBe(Finding::STATUS_NEW)); +}); + +test('readonly users cannot acknowledge all matching findings', function () { + [$user, $tenant] = createUserWithTenant(role: 'readonly'); + $this->actingAs($user); + Filament::setTenant($tenant, true); + + $findings = Finding::factory() + ->count(2) + ->for($tenant) + ->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'status' => Finding::STATUS_NEW, + ]); + + $thrown = null; + + try { + Livewire::test(ListFindings::class) + ->callAction('acknowledge_all_matching'); + } catch (Throwable $exception) { + $thrown = $exception; + } + + expect($thrown)->not->toBeNull(); + + $findings->each(fn (Finding $finding) => expect($finding->refresh()->status)->toBe(Finding::STATUS_NEW)); +}); diff --git a/tests/Feature/Drift/DriftBulkAcknowledgeTest.php b/tests/Feature/Drift/DriftBulkAcknowledgeTest.php new file mode 100644 index 0000000..ea613bf --- /dev/null +++ b/tests/Feature/Drift/DriftBulkAcknowledgeTest.php @@ -0,0 +1,34 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $findings = Finding::factory() + ->count(3) + ->for($tenant) + ->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'status' => Finding::STATUS_NEW, + 'acknowledged_at' => null, + 'acknowledged_by_user_id' => null, + ]); + + Livewire::test(ListFindings::class) + ->callTableBulkAction('acknowledge_selected', $findings) + ->assertHasNoTableBulkActionErrors(); + + $findings->each(function (Finding $finding) use ($user): void { + $finding->refresh(); + + expect($finding->status)->toBe(Finding::STATUS_ACKNOWLEDGED); + expect($finding->acknowledged_at)->not->toBeNull(); + expect((int) $finding->acknowledged_by_user_id)->toBe((int) $user->getKey()); + }); +}); diff --git a/tests/Feature/Drift/DriftCompletedRunWithZeroFindingsTest.php b/tests/Feature/Drift/DriftCompletedRunWithZeroFindingsTest.php new file mode 100644 index 0000000..f8832aa --- /dev/null +++ b/tests/Feature/Drift/DriftCompletedRunWithZeroFindingsTest.php @@ -0,0 +1,71 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $scopeKey = hash('sha256', 'scope-zero-findings'); + + $baseline = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['settingsCatalogPolicy']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['settingsCatalogPolicy']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $idempotencyKey = RunIdempotency::buildKey( + tenantId: (int) $tenant->getKey(), + operationType: 'drift.generate', + targetId: $scopeKey, + context: [ + 'scope_key' => $scopeKey, + 'baseline_run_id' => (int) $baseline->getKey(), + 'current_run_id' => (int) $current->getKey(), + ], + ); + + BulkOperationRun::factory()->for($tenant)->for($user)->create([ + 'resource' => 'drift', + 'action' => 'generate', + 'status' => 'completed', + 'idempotency_key' => $idempotencyKey, + 'item_ids' => [$scopeKey], + 'total_items' => 1, + 'processed_items' => 1, + 'succeeded' => 1, + 'failed' => 0, + 'skipped' => 0, + ]); + + Livewire::test(DriftLanding::class) + ->assertSet('state', 'ready') + ->assertSet('scopeKey', $scopeKey); + + Queue::assertNothingPushed(); + + expect(BulkOperationRun::query() + ->where('tenant_id', $tenant->getKey()) + ->where('idempotency_key', $idempotencyKey) + ->count())->toBe(1); + + Queue::assertNotPushed(GenerateDriftFindingsJob::class); +}); diff --git a/tests/Feature/Drift/DriftEvidenceMinimizationTest.php b/tests/Feature/Drift/DriftEvidenceMinimizationTest.php new file mode 100644 index 0000000..82bf47b --- /dev/null +++ b/tests/Feature/Drift/DriftEvidenceMinimizationTest.php @@ -0,0 +1,24 @@ + 'modified', + 'summary' => ['changed_fields' => ['assignments_hash']], + 'baseline' => ['hash' => 'a'], + 'current' => ['hash' => 'b'], + 'diff' => ['a' => 'b'], + 'notes' => 'ok', + 'access_token' => 'should-not-leak', + 'client_secret' => 'should-not-leak', + 'raw_payload' => ['big' => 'blob'], + ]; + + $safe = app(DriftEvidence::class)->sanitize($payload); + + expect($safe)->toHaveKeys(['change_type', 'summary', 'baseline', 'current', 'diff', 'notes']); + expect($safe)->not->toHaveKey('access_token'); + expect($safe)->not->toHaveKey('client_secret'); + expect($safe)->not->toHaveKey('raw_payload'); +}); diff --git a/tests/Feature/Drift/DriftFindingDetailShowsAssignmentsDiffTest.php b/tests/Feature/Drift/DriftFindingDetailShowsAssignmentsDiffTest.php new file mode 100644 index 0000000..605617c --- /dev/null +++ b/tests/Feature/Drift/DriftFindingDetailShowsAssignmentsDiffTest.php @@ -0,0 +1,151 @@ +for($tenant)->create([ + 'selection_hash' => hash('sha256', 'scope-assignments-diff'), + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $baseline->selection_hash, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'external_id' => 'policy-456', + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows10', + ]); + + $group1 = '76b787af-cae9-4a8e-89e9-b8cc67f81779'; + $group2 = '6b0bc3d7-91f3-4e4b-8181-8236d908d2dd'; + $group3 = 'cbd8d685-0d95-4de0-8fce-140a5cad8ddc'; + + EntraGroup::factory()->for($tenant)->create([ + 'entra_id' => strtolower($group1), + 'display_name' => 'Group One', + ]); + + EntraGroup::factory()->for($tenant)->create([ + 'entra_id' => strtolower($group3), + 'display_name' => 'Group Three', + ]); + + $baselineVersion = PolicyVersion::factory()->for($tenant)->create([ + 'policy_id' => $policy->getKey(), + 'version_number' => 1, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $baseline->finished_at->copy()->subHour(), + 'assignments' => [ + [ + 'intent' => 'apply', + 'target' => [ + '@odata.type' => '#microsoft.graph.groupAssignmentTarget', + 'groupId' => $group1, + 'deviceAndAppManagementAssignmentFilterId' => null, + 'deviceAndAppManagementAssignmentFilterType' => 'none', + ], + ], + [ + 'target' => [ + '@odata.type' => '#microsoft.graph.exclusionGroupAssignmentTarget', + 'groupId' => $group2, + 'deviceAndAppManagementAssignmentFilterId' => null, + 'deviceAndAppManagementAssignmentFilterType' => 'none', + ], + ], + ], + ]); + + $currentVersion = PolicyVersion::factory()->for($tenant)->create([ + 'policy_id' => $policy->getKey(), + 'version_number' => 2, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $current->finished_at->copy()->subHour(), + 'assignments' => [ + [ + 'intent' => 'apply', + 'target' => [ + '@odata.type' => '#microsoft.graph.groupAssignmentTarget', + 'groupId' => $group1, + 'deviceAndAppManagementAssignmentFilterId' => '62fb77d0-8f85-4ba0-a1c7-fd71d418521d', + 'deviceAndAppManagementAssignmentFilterType' => 'include', + ], + ], + [ + 'intent' => 'apply', + 'target' => [ + '@odata.type' => '#microsoft.graph.groupAssignmentTarget', + 'groupId' => $group3, + 'deviceAndAppManagementAssignmentFilterId' => null, + 'deviceAndAppManagementAssignmentFilterType' => 'none', + ], + ], + ], + ]); + + $finding = Finding::factory()->for($tenant)->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'scope_key' => (string) $current->selection_hash, + 'baseline_run_id' => $baseline->getKey(), + 'current_run_id' => $current->getKey(), + 'subject_type' => 'assignment', + 'subject_external_id' => $policy->external_id, + 'evidence_jsonb' => [ + 'change_type' => 'modified', + 'summary' => [ + 'kind' => 'policy_assignments', + 'changed_fields' => ['assignments_hash'], + ], + 'baseline' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $baselineVersion->getKey(), + 'assignments_hash' => 'baseline-assignments-hash', + ], + 'current' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $currentVersion->getKey(), + 'assignments_hash' => 'current-assignments-hash', + ], + ], + ]); + + InventoryItem::factory()->for($tenant)->create([ + 'external_id' => $finding->subject_external_id, + 'display_name' => 'My Policy 456', + ]); + + $expectedGroup1 = EntraGroupLabelResolver::formatLabel('Group One', $group1); + $expectedGroup2 = EntraGroupLabelResolver::formatLabel(null, $group2); + $expectedGroup3 = EntraGroupLabelResolver::formatLabel('Group Three', $group3); + + $this->actingAs($user) + ->get(FindingResource::getUrl('view', ['record' => $finding], tenant: $tenant)) + ->assertOk() + ->assertSee('Assignments diff') + ->assertSee('1 added') + ->assertSee('1 removed') + ->assertSee('1 changed') + ->assertSee($expectedGroup1) + ->assertSee($expectedGroup2) + ->assertSee($expectedGroup3) + ->assertSee('include') + ->assertSee('none'); +}); diff --git a/tests/Feature/Drift/DriftFindingDetailShowsScopeTagsDiffTest.php b/tests/Feature/Drift/DriftFindingDetailShowsScopeTagsDiffTest.php new file mode 100644 index 0000000..3d905d3 --- /dev/null +++ b/tests/Feature/Drift/DriftFindingDetailShowsScopeTagsDiffTest.php @@ -0,0 +1,100 @@ +for($tenant)->create([ + 'selection_hash' => hash('sha256', 'scope-scope-tags-diff'), + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $baseline->selection_hash, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'external_id' => 'policy-scope-tags-1', + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows10', + ]); + + $baselineVersion = PolicyVersion::factory()->for($tenant)->create([ + 'policy_id' => $policy->getKey(), + 'version_number' => 17, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $baseline->finished_at->copy()->subHour(), + 'snapshot' => ['customSettingFoo' => 'Same value'], + 'assignments' => [], + 'scope_tags' => [ + 'ids' => ['0'], + 'names' => ['Default'], + ], + ]); + + $currentVersion = PolicyVersion::factory()->for($tenant)->create([ + 'policy_id' => $policy->getKey(), + 'version_number' => 18, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $current->finished_at->copy()->subHour(), + 'snapshot' => ['customSettingFoo' => 'Same value'], + 'assignments' => [], + 'scope_tags' => [ + 'ids' => ['0', 'a1b2c3'], + 'names' => ['Default', 'Verbund-1'], + ], + ]); + + $finding = Finding::factory()->for($tenant)->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'scope_key' => (string) $current->selection_hash, + 'baseline_run_id' => $baseline->getKey(), + 'current_run_id' => $current->getKey(), + 'subject_type' => 'scope_tag', + 'subject_external_id' => $policy->external_id, + 'evidence_jsonb' => [ + 'change_type' => 'modified', + 'summary' => [ + 'kind' => 'policy_scope_tags', + 'changed_fields' => ['scope_tags_hash'], + ], + 'baseline' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $baselineVersion->getKey(), + 'scope_tags_hash' => 'baseline-scope-tags-hash', + ], + 'current' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $currentVersion->getKey(), + 'scope_tags_hash' => 'current-scope-tags-hash', + ], + ], + ]); + + InventoryItem::factory()->for($tenant)->create([ + 'external_id' => $finding->subject_external_id, + 'display_name' => 'My Policy Scope Tags', + ]); + + $this->actingAs($user) + ->get(FindingResource::getUrl('view', ['record' => $finding], tenant: $tenant)) + ->assertOk() + ->assertSee('Scope tags diff') + ->assertSee('1 added') + ->assertSee('0 removed') + ->assertSee('Verbund-1') + ->assertSee('Default'); +}); diff --git a/tests/Feature/Drift/DriftFindingDetailShowsSettingsDiffTest.php b/tests/Feature/Drift/DriftFindingDetailShowsSettingsDiffTest.php new file mode 100644 index 0000000..ffc4136 --- /dev/null +++ b/tests/Feature/Drift/DriftFindingDetailShowsSettingsDiffTest.php @@ -0,0 +1,100 @@ +for($tenant)->create([ + 'selection_hash' => hash('sha256', 'scope-settings-diff'), + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $baseline->selection_hash, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'external_id' => 'policy-123', + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows10', + ]); + + $baselineVersion = PolicyVersion::factory()->for($tenant)->create([ + 'policy_id' => $policy->getKey(), + 'version_number' => 1, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $baseline->finished_at->copy()->subHour(), + 'snapshot' => [ + 'displayName' => 'My Policy', + 'description' => 'Old description', + 'customSettingFoo' => 'Old value', + ], + ]); + + $currentVersion = PolicyVersion::factory()->for($tenant)->create([ + 'policy_id' => $policy->getKey(), + 'version_number' => 2, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $current->finished_at->copy()->subHour(), + 'snapshot' => [ + 'displayName' => 'My Policy', + 'description' => 'New description', + 'customSettingFoo' => 'New value', + ], + ]); + + $finding = Finding::factory()->for($tenant)->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'scope_key' => (string) $current->selection_hash, + 'baseline_run_id' => $baseline->getKey(), + 'current_run_id' => $current->getKey(), + 'subject_type' => 'policy', + 'subject_external_id' => $policy->external_id, + 'evidence_jsonb' => [ + 'change_type' => 'modified', + 'summary' => [ + 'kind' => 'policy_snapshot', + 'changed_fields' => ['snapshot_hash'], + ], + 'baseline' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $baselineVersion->getKey(), + 'snapshot_hash' => 'baseline-hash', + ], + 'current' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $currentVersion->getKey(), + 'snapshot_hash' => 'current-hash', + ], + ], + ]); + + InventoryItem::factory()->for($tenant)->create([ + 'external_id' => $finding->subject_external_id, + 'display_name' => 'My Policy 123', + ]); + + $this->actingAs($user) + ->get(FindingResource::getUrl('view', ['record' => $finding], tenant: $tenant)) + ->assertOk() + ->assertSee('Normalized diff') + ->assertSee('1 changed') + ->assertSee('Custom Setting Foo') + ->assertSee('From') + ->assertSee('To') + ->assertSee('Old value') + ->assertSee('New value'); +}); diff --git a/tests/Feature/Drift/DriftFindingDetailTest.php b/tests/Feature/Drift/DriftFindingDetailTest.php new file mode 100644 index 0000000..b216dc4 --- /dev/null +++ b/tests/Feature/Drift/DriftFindingDetailTest.php @@ -0,0 +1,48 @@ +for($tenant)->create([ + 'selection_hash' => hash('sha256', 'scope-detail'), + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $baseline->selection_hash, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $finding = Finding::factory()->for($tenant)->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'scope_key' => (string) $current->selection_hash, + 'baseline_run_id' => $baseline->getKey(), + 'current_run_id' => $current->getKey(), + 'subject_type' => 'deviceConfiguration', + 'subject_external_id' => 'policy-123', + 'evidence_jsonb' => [ + 'change_type' => 'modified', + 'summary' => ['changed_fields' => ['assignments_hash']], + ], + ]); + + $inventoryItem = InventoryItem::factory()->for($tenant)->create([ + 'external_id' => $finding->subject_external_id, + 'display_name' => 'My Policy 123', + ]); + + $this->actingAs($user) + ->get(FindingResource::getUrl('view', ['record' => $finding], tenant: $tenant)) + ->assertOk() + ->assertSee($finding->fingerprint) + ->assertSee($inventoryItem->display_name); +}); diff --git a/tests/Feature/Drift/DriftGenerationDeterminismTest.php b/tests/Feature/Drift/DriftGenerationDeterminismTest.php new file mode 100644 index 0000000..c5ea172 --- /dev/null +++ b/tests/Feature/Drift/DriftGenerationDeterminismTest.php @@ -0,0 +1,76 @@ +for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['settingsCatalogPolicy']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['settingsCatalogPolicy']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'policy_type' => 'settingsCatalogPolicy', + ]); + + $baselineAssignments = [ + ['target' => ['@odata.type' => '#microsoft.graph.groupAssignmentTarget', 'groupId' => 'group-a']], + ['target' => ['@odata.type' => '#microsoft.graph.groupAssignmentTarget', 'groupId' => 'group-b']], + ]; + + $currentAssignments = [ + ['target' => ['@odata.type' => '#microsoft.graph.groupAssignmentTarget', 'groupId' => 'group-c']], + ]; + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 1, + 'policy_type' => $policy->policy_type, + 'captured_at' => $baseline->finished_at->copy()->subMinute(), + 'assignments' => $baselineAssignments, + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 2, + 'policy_type' => $policy->policy_type, + 'captured_at' => $current->finished_at->copy()->subMinute(), + 'assignments' => $currentAssignments, + ]); + + $generator = app(DriftFindingGenerator::class); + + $created1 = $generator->generate($tenant, $baseline, $current, $scopeKey); + $fingerprints1 = Finding::query() + ->where('tenant_id', $tenant->getKey()) + ->pluck('fingerprint') + ->sort() + ->values() + ->all(); + + $created2 = $generator->generate($tenant, $baseline, $current, $scopeKey); + $fingerprints2 = Finding::query() + ->where('tenant_id', $tenant->getKey()) + ->pluck('fingerprint') + ->sort() + ->values() + ->all(); + + expect($created1)->toBeGreaterThan(0); + expect($created2)->toBe(0); + expect($fingerprints2)->toBe($fingerprints1); +}); diff --git a/tests/Feature/Drift/DriftGenerationDispatchTest.php b/tests/Feature/Drift/DriftGenerationDispatchTest.php index 3e54f9a..09fc6fa 100644 --- a/tests/Feature/Drift/DriftGenerationDispatchTest.php +++ b/tests/Feature/Drift/DriftGenerationDispatchTest.php @@ -2,7 +2,9 @@ use App\Filament\Pages\DriftLanding; use App\Jobs\GenerateDriftFindingsJob; +use App\Models\BulkOperationRun; use App\Models\InventorySyncRun; +use App\Support\RunIdempotency; use Filament\Facades\Filament; use Illuminate\Support\Facades\Queue; use Livewire\Livewire; @@ -30,15 +32,81 @@ Livewire::test(DriftLanding::class); - Queue::assertPushed(GenerateDriftFindingsJob::class, function (GenerateDriftFindingsJob $job) use ($tenant, $user, $baseline, $current, $scopeKey): bool { + $idempotencyKey = RunIdempotency::buildKey( + tenantId: (int) $tenant->getKey(), + operationType: 'drift.generate', + targetId: $scopeKey, + context: [ + 'scope_key' => $scopeKey, + 'baseline_run_id' => (int) $baseline->getKey(), + 'current_run_id' => (int) $current->getKey(), + ], + ); + + $bulkRun = BulkOperationRun::query() + ->where('tenant_id', $tenant->getKey()) + ->where('idempotency_key', $idempotencyKey) + ->latest('id') + ->first(); + + expect($bulkRun)->not->toBeNull(); + expect($bulkRun->resource)->toBe('drift'); + expect($bulkRun->action)->toBe('generate'); + expect($bulkRun->status)->toBe('pending'); + + Queue::assertPushed(GenerateDriftFindingsJob::class, function (GenerateDriftFindingsJob $job) use ($tenant, $user, $baseline, $current, $scopeKey, $bulkRun): bool { return $job->tenantId === (int) $tenant->getKey() && $job->userId === (int) $user->getKey() && $job->baselineRunId === (int) $baseline->getKey() && $job->currentRunId === (int) $current->getKey() - && $job->scopeKey === $scopeKey; + && $job->scopeKey === $scopeKey + && $job->bulkOperationRunId === (int) $bulkRun->getKey(); }); }); +test('opening Drift is idempotent while a run is pending', function () { + Queue::fake(); + + [$user, $tenant] = createUserWithTenant(role: 'manager'); + $this->actingAs($user); + Filament::setTenant($tenant, true); + + $scopeKey = hash('sha256', 'scope-idempotent'); + + $baseline = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + Livewire::test(DriftLanding::class); + Livewire::test(DriftLanding::class); + + Queue::assertPushed(GenerateDriftFindingsJob::class, 1); + + $idempotencyKey = RunIdempotency::buildKey( + tenantId: (int) $tenant->getKey(), + operationType: 'drift.generate', + targetId: $scopeKey, + context: [ + 'scope_key' => $scopeKey, + 'baseline_run_id' => (int) $baseline->getKey(), + 'current_run_id' => (int) $current->getKey(), + ], + ); + + expect(BulkOperationRun::query() + ->where('tenant_id', $tenant->getKey()) + ->where('idempotency_key', $idempotencyKey) + ->count())->toBe(1); +}); + test('opening Drift does not dispatch generation when fewer than two successful runs exist', function () { Queue::fake(); @@ -57,4 +125,5 @@ Livewire::test(DriftLanding::class); Queue::assertNothingPushed(); + expect(BulkOperationRun::query()->where('tenant_id', $tenant->getKey())->count())->toBe(0); }); diff --git a/tests/Feature/Drift/DriftHasherTest.php b/tests/Feature/Drift/DriftHasherTest.php new file mode 100644 index 0000000..2d39eeb --- /dev/null +++ b/tests/Feature/Drift/DriftHasherTest.php @@ -0,0 +1,39 @@ + 'abc', + 'createdDateTime' => '2020-01-01T00:00:00Z', + 'lastModifiedDateTime' => '2020-01-02T00:00:00Z', + 'target' => ['groupId' => 'group-a'], + ]; + + $b = [ + 'id' => 'abc', + 'createdDateTime' => '2025-01-01T00:00:00Z', + 'lastModifiedDateTime' => '2026-01-02T00:00:00Z', + 'target' => ['groupId' => 'group-a'], + ]; + + expect($hasher->hashNormalized($a))->toBe($hasher->hashNormalized($b)); +}); + +test('normalized hashing is order-insensitive for lists', function () { + $hasher = app(DriftHasher::class); + + $listA = [ + ['target' => ['groupId' => 'group-a']], + ['target' => ['groupId' => 'group-b']], + ]; + + $listB = [ + ['target' => ['groupId' => 'group-b']], + ['target' => ['groupId' => 'group-a']], + ]; + + expect($hasher->hashNormalized($listA))->toBe($hasher->hashNormalized($listB)); +}); diff --git a/tests/Feature/Drift/DriftLandingShowsComparisonInfoTest.php b/tests/Feature/Drift/DriftLandingShowsComparisonInfoTest.php new file mode 100644 index 0000000..3927771 --- /dev/null +++ b/tests/Feature/Drift/DriftLandingShowsComparisonInfoTest.php @@ -0,0 +1,33 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $scopeKey = hash('sha256', 'scope-landing-comparison-info'); + + $baseline = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + Livewire::test(DriftLanding::class) + ->assertSet('scopeKey', $scopeKey) + ->assertSet('baselineRunId', (int) $baseline->getKey()) + ->assertSet('currentRunId', (int) $current->getKey()) + ->assertSet('baselineFinishedAt', $baseline->finished_at->toDateTimeString()) + ->assertSet('currentFinishedAt', $current->finished_at->toDateTimeString()); +}); diff --git a/tests/Feature/Drift/DriftPolicySnapshotDriftDetectionTest.php b/tests/Feature/Drift/DriftPolicySnapshotDriftDetectionTest.php new file mode 100644 index 0000000..34ecbf3 --- /dev/null +++ b/tests/Feature/Drift/DriftPolicySnapshotDriftDetectionTest.php @@ -0,0 +1,73 @@ +for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['deviceConfiguration']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['deviceConfiguration']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows10', + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 1, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $baseline->finished_at->copy()->subMinute(), + 'snapshot' => ['customSettingFoo' => 'Old value'], + 'assignments' => [], + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 2, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $current->finished_at->copy()->subMinute(), + 'snapshot' => ['customSettingFoo' => 'New value'], + 'assignments' => [], + ]); + + $generator = app(DriftFindingGenerator::class); + $created = $generator->generate($tenant, $baseline, $current, $scopeKey); + + expect($created)->toBe(1); + + $finding = Finding::query() + ->where('tenant_id', $tenant->getKey()) + ->where('finding_type', Finding::FINDING_TYPE_DRIFT) + ->where('scope_key', $scopeKey) + ->where('subject_type', 'policy') + ->first(); + + expect($finding)->not->toBeNull(); + expect($finding->subject_external_id)->toBe($policy->external_id); + expect($finding->evidence_jsonb)->toHaveKey('change_type', 'modified'); + expect($finding->evidence_jsonb) + ->toHaveKey('summary.changed_fields') + ->and($finding->evidence_jsonb['summary']['changed_fields'])->toContain('snapshot_hash') + ->and($finding->evidence_jsonb)->toHaveKey('baseline.snapshot_hash') + ->and($finding->evidence_jsonb)->toHaveKey('current.snapshot_hash') + ->and($finding->evidence_jsonb)->not->toHaveKey('baseline.assignments_hash') + ->and($finding->evidence_jsonb)->not->toHaveKey('current.assignments_hash'); +}); diff --git a/tests/Feature/Drift/DriftPolicySnapshotMetadataOnlyDoesNotCreateFindingTest.php b/tests/Feature/Drift/DriftPolicySnapshotMetadataOnlyDoesNotCreateFindingTest.php new file mode 100644 index 0000000..539a343 --- /dev/null +++ b/tests/Feature/Drift/DriftPolicySnapshotMetadataOnlyDoesNotCreateFindingTest.php @@ -0,0 +1,62 @@ +for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['deviceConfiguration']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['deviceConfiguration']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows10', + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 1, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $baseline->finished_at->copy()->subMinute(), + 'snapshot' => [ + 'displayName' => 'My Policy', + 'description' => 'Old description', + ], + 'assignments' => [], + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 2, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $current->finished_at->copy()->subMinute(), + 'snapshot' => [ + 'displayName' => 'My Policy', + 'description' => 'New description', + ], + 'assignments' => [], + ]); + + $generator = app(DriftFindingGenerator::class); + $created = $generator->generate($tenant, $baseline, $current, $scopeKey); + + expect($created)->toBe(0); + expect(Finding::query()->where('tenant_id', $tenant->getKey())->count())->toBe(0); +}); diff --git a/tests/Feature/Drift/DriftScopeTagDriftDetectionTest.php b/tests/Feature/Drift/DriftScopeTagDriftDetectionTest.php new file mode 100644 index 0000000..15b8b8f --- /dev/null +++ b/tests/Feature/Drift/DriftScopeTagDriftDetectionTest.php @@ -0,0 +1,84 @@ +for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['deviceConfiguration']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['deviceConfiguration']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows10', + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 1, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $baseline->finished_at->copy()->subMinute(), + 'snapshot' => ['customSettingFoo' => 'Same value'], + 'assignments' => [], + 'scope_tags' => [ + 'ids' => ['0'], + 'names' => ['Default'], + ], + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 2, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $current->finished_at->copy()->subMinute(), + 'snapshot' => ['customSettingFoo' => 'Same value'], + 'assignments' => [], + 'scope_tags' => [ + 'ids' => ['0', 'a1b2c3'], + 'names' => ['Default', 'Verbund-1'], + ], + ]); + + $generator = app(DriftFindingGenerator::class); + $created = $generator->generate($tenant, $baseline, $current, $scopeKey); + + expect($created)->toBe(1); + + $finding = Finding::query() + ->where('tenant_id', $tenant->getKey()) + ->where('finding_type', Finding::FINDING_TYPE_DRIFT) + ->where('scope_key', $scopeKey) + ->where('subject_type', 'scope_tag') + ->first(); + + expect($finding)->not->toBeNull(); + expect($finding->subject_external_id)->toBe($policy->external_id); + expect($finding->evidence_jsonb)->toHaveKey('change_type', 'modified'); + expect($finding->evidence_jsonb) + ->toHaveKey('summary.kind', 'policy_scope_tags') + ->toHaveKey('summary.changed_fields') + ->and($finding->evidence_jsonb['summary']['changed_fields'])->toContain('scope_tags_hash') + ->and($finding->evidence_jsonb)->toHaveKey('baseline.scope_tags_hash') + ->and($finding->evidence_jsonb)->toHaveKey('current.scope_tags_hash') + ->and($finding->evidence_jsonb)->not->toHaveKey('baseline.snapshot_hash') + ->and($finding->evidence_jsonb)->not->toHaveKey('current.snapshot_hash') + ->and($finding->evidence_jsonb)->not->toHaveKey('baseline.assignments_hash') + ->and($finding->evidence_jsonb)->not->toHaveKey('current.assignments_hash'); +}); diff --git a/tests/Feature/Drift/DriftScopeTagLegacyDefaultDoesNotCreateFindingTest.php b/tests/Feature/Drift/DriftScopeTagLegacyDefaultDoesNotCreateFindingTest.php new file mode 100644 index 0000000..be29a5a --- /dev/null +++ b/tests/Feature/Drift/DriftScopeTagLegacyDefaultDoesNotCreateFindingTest.php @@ -0,0 +1,69 @@ +for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['deviceConfiguration']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['deviceConfiguration']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows10', + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 1, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $baseline->finished_at->copy()->subMinute(), + 'snapshot' => ['customSettingFoo' => 'Same value'], + 'assignments' => [], + 'scope_tags' => [ + // legacy data shape (missing ids) + 'names' => ['Default'], + ], + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 2, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $current->finished_at->copy()->subMinute(), + 'snapshot' => ['customSettingFoo' => 'Same value'], + 'assignments' => [], + 'scope_tags' => [ + 'ids' => ['0'], + 'names' => ['Default'], + ], + ]); + + $generator = app(DriftFindingGenerator::class); + $created = $generator->generate($tenant, $baseline, $current, $scopeKey); + + expect($created)->toBe(0); + + expect(Finding::query() + ->where('tenant_id', $tenant->getKey()) + ->where('finding_type', Finding::FINDING_TYPE_DRIFT) + ->where('scope_key', $scopeKey) + ->count())->toBe(0); +}); -- 2.45.2