Automated PR provided by Codex via Gitea API. Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de> Reviewed-on: #477
120 lines
17 KiB
Markdown
120 lines
17 KiB
Markdown
# Spec 406 Implementation Report - Governance Artifact Lifecycle & Retention
|
|
|
|
## 1. Candidate Gate Result
|
|
|
|
Result: `PASS WITH CONDITIONS` candidate selected by operator.
|
|
|
|
Spec 406 is approved as a bounded lifecycle hardening slice over existing governance artifacts. It does not approve a generic artifact registry, new portal, export center, legal/eDiscovery claim, purge platform, new navigation entry, or broad lifecycle framework.
|
|
|
|
## 2. Dirty State
|
|
|
|
- Branch: `406-governance-artifact-lifecycle-retention`
|
|
- Baseline HEAD: `686947d2 feat: harden json to jsonb data layer for trust payloads (#476)`
|
|
- Initial working tree: untracked `specs/406-governance-artifact-lifecycle-retention/`; no tracked file modifications before implementation edits.
|
|
- Initial `git diff --check`: pass.
|
|
- Completed-spec rewrite assertion: Specs 158, 262, 267, 400, 403, 404, and 405 are read-only context. Their completed task markers, implementation reports, browser proof, and validation history were not edited.
|
|
|
|
## 3. Spec 404/405 Carry-Forward
|
|
|
|
- Spec 404 remains `PASS WITH CONDITIONS` for external Staging/Dokploy validation. Spec 406 may prove local management-report PDF lifecycle/download behavior, but it does not claim Staging/Dokploy readiness.
|
|
- Spec 405 remains `PASS WITH CONDITIONS` for external Staging/Dokploy validation. Spec 406 touches no JSON/JSONB conversion path and carries no storage-engine readiness claim beyond existing local test proof.
|
|
- Neither remaining condition blocks the narrow Spec 406 local hardening of review-pack download consistency, review-pack lifecycle action authorization/audit, and matrix/report truth.
|
|
|
|
## 4. Governance Artifact Lifecycle Matrix
|
|
|
|
| Family | Current owner/source | File dependency | Customer-safe boundary | Lifecycle/file rules | Hold/delete support | Status | Risk/follow-up |
|
|
|---|---|---|---|---|---|---|---|
|
|
| Review packs | `ReviewPack`, `ReviewPackService`, `ReviewPackDownloadController`, `ReviewPackRenderedReportController`, `ReviewPackResource`, `PruneReviewPacksCommand` | Private `exports` disk, `file_disk`, `file_path`, `file_size`, `sha256` | `CustomerOutputGate` plus signed download/rendered report routes | Ready/non-expired/customer-safe packs may download or render only after current `exports`-disk existence, positive size, size match, and SHA-256 match. Expired/queued/failed/missing/tampered/wrong-disk files fail closed. Spec 406 hardens size/SHA/wrong-disk checks and action audit. | Expire supported now. Hard-delete via prune exists after grace. Hold remains `PRODUCT_DECISION_REQUIRED`; no hold columns/actions added. | `PASS WITH CONDITIONS` | Follow-up required for legal hold/export-before-delete/purge governance. |
|
|
| Stored reports / management PDFs | `StoredReport`, `ManagementReportPdfService`, `ManagementReportPdfDownloadController`, `StoredReportResource`, `PruneStoredReportsCommand` | Private `exports` disk, PDF bytes, `file_size`, `sha256` | Source review pack must be current, released, customer-safe, and downloadable by actor | Existing controller validates ready metadata, review-pack state/currentness, file existence, size, valid PDF bytes, and SHA before streaming. | Delete via retention prune exists. Hold is `PRODUCT_DECISION_REQUIRED`; no hold columns/actions added. | `PASS WITH CONDITIONS` | Local lifecycle/download proof exists; external Spec 404 Staging/Dokploy condition carries forward. |
|
|
| Evidence snapshots | `EvidenceSnapshot`, `EvidenceSnapshotService`, `EvidenceSnapshotResource`, evidence currentness/anchor services | Database-backed evidence rows; no downloadable artifact file in this slice | Customer-facing review output consumes summarized/released evidence, not raw evidence by default | Existing service supports expiration and audit; currentness remains separate from review-pack release proof. No file-download lifecycle added. | Expire supported now. Delete/hold/purge are `DEFERRED`. | `PASS WITH EXCEPTION` | Follow-up only if evidence deletion/hold becomes a product decision. |
|
|
| Customer review retained output | `CustomerReviewWorkspace`, review-pack rendered report/download routes, `CustomerOutputGate` | Review pack ZIP and management PDF when available | Customer/read-only surface must hide raw evidence, OperationRun internals, source keys, fingerprints, and provider payloads | Existing tests cover released/customer-safe visibility, unsafe/internal denial, and customer-safe copy boundaries. Spec 406 inherits this and adds review-pack file consistency proof. | No mutation surface. Hold/delete N/A on customer surface. | `PASS WITH CONDITIONS` | Browser proof still required for representative rendered path. |
|
|
| OperationRun proof | `OperationRun`, operation detail/resources, OperationRun UX contracts | No artifact file by default | Internal proof only unless an existing product contract exposes derived artifact output | Execution status/outcome remains execution truth and must not become artifact lifecycle truth. No lifecycle mutation implemented here. | Delete/hold/purge `DEFERRED`; not an artifact owner in this slice. | `DEFERRED` | Retain current internal proof posture; no new customer exposure. |
|
|
| Finding exceptions / accepted risks / governance inbox artifacts | `FindingException`, `FindingExceptionDecision`, finding exception services/resources, customer review summaries | Database-backed decisions and evidence references | Customer review output may show customer-safe accepted-risk summary only; internal rationale remains hidden | Existing workflows authorize lifecycle decisions and audit request/approve/reject/renew/revoke actions. No file-backed download lifecycle in this slice. | Decision lifecycle supported. Delete/hold/purge `DEFERRED`. | `PASS WITH EXCEPTION` | Follow-up only if accepted-risk deletion/retention semantics need dedicated product behavior. |
|
|
|
|
## 5. Runtime Changes
|
|
|
|
- `ReviewPackService` now owns review-pack file-download validation through `resolveDownloadableArtifact()` / `hasDownloadableArtifact()`. The rule requires `ready`, non-expired lifecycle state, `exports` disk, a present file path, positive stored size, present SHA-256, private-disk existence, exact size match, and SHA-256 byte match before any surface treats a ZIP as downloadable.
|
|
- `ReviewPackDownloadController` delegates byte validation to `ReviewPackService` and streams only the already validated bytes. Signed URLs continue to re-check current lifecycle, authorization, customer-output gate, file presence, size, disk, and hash at request time.
|
|
- `ReviewPackRenderedReportController` and `CustomerOutputGate` now use the same downloadable-artifact validation before rendering customer/internal report views or offering the toolbar download URL, so rendered-report signed URLs also fail closed after tampering, missing bytes, wrong disk, or expiry.
|
|
- `ReviewPackService::expire()` centralizes the ReviewPack expire transition. It requires tenant scope, `REVIEW_PACK_MANAGE`, actor tenant access, ready status, private-file delete verification, and audit proof through `review_pack.expired`.
|
|
- `ReviewPackResource` and `ViewReviewPack` now hide review-pack download actions when the private artifact fails validation and share the same customer-output/internal-preview URL parameter logic, so visible table downloads do not point at a server-denied stream mode. The destructive expire action remains a Filament action with confirmation, server-side capability enforcement, file delete, and audit logging.
|
|
- `CustomerReviewWorkspace` now uses the same downloadable-artifact validation for customer-safe download URLs and review-pack availability state, so a ready record with missing/tampered/wrong-disk bytes is not shown as a valid customer download.
|
|
- `AuditActionId` includes `review_pack.expired` labels/summaries.
|
|
- No database migrations, new tables, new lifecycle framework, new navigation, new panel, new assets, or new runtime dependencies were added.
|
|
|
|
## 6. Tests Added Or Updated
|
|
|
|
- `ReviewPackDownloadTest` covers invalid file metadata/bytes (`zero-byte`, size mismatch, SHA mismatch, missing SHA, wrong disk), verifies old signed download URLs re-check current lifecycle before streaming bytes, and verifies rendered-report signed URLs re-check artifact integrity before rendering.
|
|
- `ReviewPackRbacTest` covers owner expire behavior, confirmation state, export-disk file deletion, wrong-disk non-deletion, audit metadata, and direct service denial for a read-only actor.
|
|
- `ReviewPackResourceTest` now creates realistic export-disk fixtures for positive download UI tests, covers the table download URL for internal previews, and covers hidden list/detail download actions when a ready pack fails hash validation.
|
|
- `CustomerReviewWorkspacePackAccessTest` now creates realistic customer-safe artifact fixtures and covers a ready customer-safe pack with tampered bytes being shown as not configured without a download action.
|
|
- Fixtures remain feature-local. No global artifact matrix harness or broad artifact registry test framework was introduced.
|
|
|
|
## 7. Browser Proof
|
|
|
|
- Command: `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Browser/Spec379ManagementReportPdfSmokeTest.php`
|
|
- Result: `1 passed (18 assertions)`, re-run after the ReviewPack table download URL/internal-preview fix.
|
|
- Surface: existing ReviewPack detail / management-report PDF smoke path.
|
|
- Actor/role: authorized admin test user from the existing browser smoke.
|
|
- Artifact type/state: ready ReviewPack detail surface with management-report PDF action state.
|
|
- Expected/actual: rendered surface remained available and action layout remained valid after ReviewPack download visibility was tightened.
|
|
- Console/runtime/network: no failing browser assertions; no screenshot artifact emitted on pass.
|
|
- Held artifact browser proof: `N/A` for Spec 406 implementation. Hold support for review packs, stored reports, OperationRun proof, evidence snapshots, and accepted-risk/finding decision artifacts remains `PRODUCT_DECISION_REQUIRED` or `DEFERRED`; no hold/delete UI was added.
|
|
- Missing/tampered-file browser proof: covered by Feature/Livewire tests rather than an additional browser proof in this slice; no new visible surface was introduced, only existing download/rendered-report affordances are hidden or denied.
|
|
|
|
## 8. Product Surface Close-Out
|
|
|
|
- Runtime UI files changed: `ReviewPackResource`, `ViewReviewPack`, and `CustomerReviewWorkspace`.
|
|
- UI impact: existing ReviewPack download actions and rendered-report toolbar downloads become stricter and disappear when the private artifact is not actually downloadable. Existing customer workspace package availability now fails closed for tampered/missing/wrong-disk ZIPs.
|
|
- Product Surface exceptions: none.
|
|
- Human Product Sanity: pass. Purpose remains clear, the dominant next action is unchanged, technical diagnostics remain demoted, and canonical customer labels remain `Ready`, `Running`, `Failed`, `Expired`, `Blocked`, `Needs attention`, and `Not configured`.
|
|
- Visible complexity outcome: neutral. No new page, navigation entry, panel, card stack, portal, export center, or extra customer-facing action was added.
|
|
- No-legacy posture: canonical lifecycle hardening over current pre-production behavior.
|
|
- Route inventory / design coverage matrix: no update. Existing routes/surfaces were tightened; no new rendered route or page archetype was introduced.
|
|
|
|
## 9. Filament / Livewire / Deployment Close-Out
|
|
|
|
- Livewire v4 compliance: Livewire 4.1.4 confirmed by Laravel Boost application info.
|
|
- Provider registration location: Laravel 12 panel providers remain in `apps/platform/bootstrap/providers.php`; no provider registration changes.
|
|
- Global search posture: no new globally searchable resource. `ReviewPackResource` remains `$isGloballySearchable = false`; it has a View page, but global search stays disabled.
|
|
- Destructive/high-impact actions: `expire` remains `Action::make(...)->action(...)` with `requiresConfirmation()`, `REVIEW_PACK_MANAGE` authorization, tenant access checks, file delete verification, and audit proof. No destructive URL-only action was added.
|
|
- Asset strategy: no new assets, no `FilamentAsset::register()`, and no new on-demand asset. No additional `filament:assets` requirement beyond the existing deployment baseline.
|
|
- Deployment impact: code-only. No migrations, env vars, queues, scheduler changes, storage volume changes, reverse proxy changes, or Dokploy configuration changes. Existing private `exports` disk must remain persistent and private.
|
|
|
|
## 10. Validation Commands
|
|
|
|
- `php -l apps/platform/app/Services/ReviewPackService.php` - pass.
|
|
- `php -l apps/platform/app/Http/Controllers/ReviewPackDownloadController.php` - pass.
|
|
- `php -l apps/platform/app/Http/Controllers/ReviewPackRenderedReportController.php` - pass.
|
|
- `php -l apps/platform/app/Support/ReviewPacks/CustomerOutputGate.php` - pass.
|
|
- `php -l apps/platform/app/Filament/Resources/ReviewPackResource.php` - pass.
|
|
- `php -l apps/platform/app/Filament/Resources/ReviewPackResource/Pages/ViewReviewPack.php` - pass.
|
|
- `php -l apps/platform/tests/Feature/ReviewPack/ReviewPackDownloadTest.php` - pass.
|
|
- `php -l apps/platform/tests/Feature/ReviewPack/ReviewPackRbacTest.php` - pass.
|
|
- `php -l apps/platform/tests/Feature/ReviewPack/ReviewPackResourceTest.php` - pass.
|
|
- `php -l apps/platform/tests/Feature/Reviews/CustomerReviewWorkspacePackAccessTest.php` - pass.
|
|
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ReviewPack/ReviewPackDownloadTest.php --filter=Spec406` - `7 passed (14 assertions)`.
|
|
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ReviewPack/ReviewPackRbacTest.php --filter=Spec406` - `1 passed (6 assertions)`.
|
|
- `cd apps/platform && ./vendor/bin/sail artisan test --compact --filter=Spec406` - `8 passed (20 assertions)`.
|
|
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ReviewPack/ReviewPackResourceTest.php --filter='shows the download action for a ready pack'` - `1 passed (6 assertions)`.
|
|
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ReviewPack/ReviewPackResourceTest.php` - `23 passed (121 assertions)`.
|
|
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/ReviewPack/ReviewPackDownloadTest.php tests/Feature/ReviewPack/ReviewPackRbacTest.php tests/Feature/ReviewPack/ReviewPackResourceTest.php tests/Feature/Reviews/CustomerReviewWorkspacePackAccessTest.php` - `67 passed (352 assertions)`.
|
|
- Earlier focused runs before post-review hardening: `ReviewPackDownloadTest` - `22 passed (129 assertions)`, `ReviewPackResourceTest` - `23 passed (117 assertions)`, `CustomerReviewWorkspacePackAccessTest` - `8 passed (55 assertions)`, `ReviewPackRbacTest` - `12 passed (39 assertions)`, `--filter=Spec406` - `6 passed (12 assertions)`.
|
|
- `cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Browser/Spec379ManagementReportPdfSmokeTest.php` - `1 passed (18 assertions)`.
|
|
- `cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent` - pass.
|
|
- `git diff --check` - pass.
|
|
- PostgreSQL lane: `N/A`; no migrations, indexes, or constraints were added.
|
|
|
|
## 11. Findings And Gate Result
|
|
|
|
- Final gate result: `PASS WITH CONDITIONS`.
|
|
- Confirmed defect fixed: ready ReviewPack records with missing/tampered/wrong-disk/zero-byte artifacts could still be treated as downloadable by some UI paths or rendered-report signed URLs after URL creation. Runtime now fails closed and UI does not offer those as valid downloads or rendered reports.
|
|
- Confirmed defect fixed: ReviewPack list downloads for internal-only packs could render a visible action without the required `internal_preview=1` signed URL parameter. List and detail downloads now share the same stream-mode URL logic.
|
|
- Confirmed defect fixed: ReviewPack expire action logic was page-local and did not emit a dedicated lifecycle audit action. Runtime now uses a service transition with authorization, file delete verification, and `review_pack.expired` audit metadata.
|
|
- Local residual risk closed: focused browser proof was re-run after the final table-link fix and remains passing.
|
|
- Remaining condition: legal hold, export-before-delete, broad purge, and cross-family deletion governance remain product decisions. No hold columns/actions were added.
|
|
- Remaining condition: Spec 404 and Spec 405 external Staging/Dokploy validation conditions carry forward; Spec 406 does not claim production deployment proof.
|
|
- `PASS` without conditions is intentionally not claimed because SC-406-002 and T061 require `PASS WITH CONDITIONS` when high-risk artifact families remain `DEFERRED` / `PRODUCT_DECISION_REQUIRED` for hold/delete or when Spec 404/405 carry-forward conditions remain unresolved.
|
|
- Reports/logs/generated artifacts review: no secrets, tokens, raw credential payloads, sensitive provider payloads, private URLs, customer data, or stack traces were added to report/test fixtures.
|
|
- Post-implementation analysis: changes stayed bounded to existing ReviewPack/customer-workspace surfaces, avoided a generic artifact registry/workflow engine, preserved customer-safe boundaries, and did not rewrite completed historical specs.
|