From fc18946bf5375e461a1b53134e8bd4014996bdca Mon Sep 17 00:00:00 2001 From: Gerard Oosterhof Date: Fri, 3 Jul 2026 11:34:10 +0200 Subject: [PATCH 1/2] Add EnforceAuditModelProtectionsRule (denylist inversion, queue #46) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Author the audit-protection denylist-inversion rule: discover audit-log models by SHAPE — Eloquent Model subclasses whose short name ends with a configured suffix (auditModelNameSuffixes, default AuditLog) OR whose FQCN sits under a configured namespace prefix (auditModelNamespacePrefixes, default App\Models\Audit) — and flag any missing append-only protection, without a hand-maintained class list. Three independent checks: HasFactory present (hasFactoryForbidden), SoftDeletes present (softDeletesForbidden), updated_at not disabled (updatedAtNotDisabled). Supersedes the allowlist arch tests in kendo/entreezuil/ublgenie, which enumerate audit models by FQCN list or namespace sweep and thereby exempt any future model added outside the list. Discovery is a union of both fleet identification strategies (kendo's scattered *AuditLog suffix + entreezuil/ublgenie App\Models\Audit\* namespace incl. channel logs like AuthEventLog). Trait detection is transitive (parent-class + trait-of-a-trait); abstract intermediates are exempt; non-model *AuditLog classes excluded by the Model type gate. Configuration expresses patterns, never class names. - src/Rules/EnforceAuditModelProtectionsRule.php (InClassNode, pure ClassReflection; native reflection only inline for the UPDATED_AT literal) - extension.neon: two listOf(string()) params + schema + service wiring - 16 fixtures + 17 tests (incl. NEON container-resolution pin and boundary fixtures killing the trailing-separator + recursive-trait mutants) - README rule row + section (discovery, protections, migration recipe, false-positive suppression); CLAUDE.md rules table + ADR-0001 projection; CHANGELOG [Unreleased] (candidate MAJOR) Gates green on a CI-faithful fresh resolve: phpstan [OK], test 141/211, coverage 89.18% (new rule 98.41%), mutation aggregate MSI 85.18% / Covered 85% (new rule 97% Covered MSI; the 1 residual is the defensive (string) cast, a true equivalent mutant), audit clean, Pint clean. War-room enforcement queue #46. Seed: kendo Quartermaster M13 F-1. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01J59X48mgbPz9PyZfquygaC --- CHANGELOG.md | 3 +- CLAUDE.md | 3 +- README.md | 48 +++ extension.neon | 24 ++ .../EnforceAuditModelProtectionsRule.php | 244 ++++++++++++++ .../AbstractAuditBase.php | 22 ++ .../AuditModelProtections/AuthEventLog.php | 23 ++ .../AuditModelProtections/CleanAuditLog.php | 17 + .../ComposedFactoryTrait.php | 18 ++ .../ComposedTraitAuditLog.php | 21 ++ .../ConcreteInheritedAuditLog.php | 15 + .../AuditModelProtections/FactoryAuditLog.php | 20 ++ .../AuditModelProtections/FakeAuditLog.php | 19 ++ .../AuditModelProtections/LifecycleTrail.php | 22 ++ .../AuditModelProtections/MutableAuditLog.php | 15 + .../AuditModelProtections/PaymentRecord.php | 23 ++ .../AuditModelProtections/RegularPost.php | 22 ++ .../AuditModelProtections/ReportSummary.php | 22 ++ .../ScatteredAuditLog.php | 23 ++ .../SoftDeleteAuditLog.php | 20 ++ .../TripleViolationAuditLog.php | 20 ++ .../EnforceAuditModelProtectionsRuleTest.php | 304 ++++++++++++++++++ 22 files changed, 946 insertions(+), 2 deletions(-) create mode 100644 src/Rules/EnforceAuditModelProtectionsRule.php create mode 100644 tests/Fixtures/AuditModelProtections/AbstractAuditBase.php create mode 100644 tests/Fixtures/AuditModelProtections/AuthEventLog.php create mode 100644 tests/Fixtures/AuditModelProtections/CleanAuditLog.php create mode 100644 tests/Fixtures/AuditModelProtections/ComposedFactoryTrait.php create mode 100644 tests/Fixtures/AuditModelProtections/ComposedTraitAuditLog.php create mode 100644 tests/Fixtures/AuditModelProtections/ConcreteInheritedAuditLog.php create mode 100644 tests/Fixtures/AuditModelProtections/FactoryAuditLog.php create mode 100644 tests/Fixtures/AuditModelProtections/FakeAuditLog.php create mode 100644 tests/Fixtures/AuditModelProtections/LifecycleTrail.php create mode 100644 tests/Fixtures/AuditModelProtections/MutableAuditLog.php create mode 100644 tests/Fixtures/AuditModelProtections/PaymentRecord.php create mode 100644 tests/Fixtures/AuditModelProtections/RegularPost.php create mode 100644 tests/Fixtures/AuditModelProtections/ReportSummary.php create mode 100644 tests/Fixtures/AuditModelProtections/ScatteredAuditLog.php create mode 100644 tests/Fixtures/AuditModelProtections/SoftDeleteAuditLog.php create mode 100644 tests/Fixtures/AuditModelProtections/TripleViolationAuditLog.php create mode 100644 tests/Rules/EnforceAuditModelProtectionsRuleTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index a33518c..e158cd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,12 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and ## [Unreleased] -**Release-as-a-whole: backward-compatible MINOR** — one new optional PHPStan parameter (`controllerNamespacePrefixes`, default `['App\Http\Controllers']`) shared by all three controller-scoped rules (`ForbidEloquentMutationInControllersRule`, `EnforceCurrentUserAttributeRule`, `ForbidResourceWrappedInJsonResponseRule`). The default reproduces the prior hardcoded `App\Http\Controllers` gate byte-for-byte ⇒ zero new errors in any existing consumer on adoption (NOT a candidate-major — contrast the v0.5.0 rules that surfaced errors in clean code); new violations appear only when a consumer opts a divergent controller namespace in by configuring the param. A consumer adopts on its own `^0.6 → ^0.7` pin bump. Seed: war-room WR-0283 / WR-0275 DayUpdate pilot — emmie ships controllers under `App\Http\Client\Controllers` (4) + `App\Http\Admin\Controllers` (9), outside the hardcoded prefix, so their inline Eloquent mutations, `$request->user()` calls, and resource-wrapped JSON responses were rule-invisible. +**Release-as-a-whole: candidate MAJOR** — two entries. The shared optional `controllerNamespacePrefixes` parameter is a backward-compatible MINOR on its own (default reproduces the prior hardcoded gate byte-for-byte — see its bullet); the new `EnforceAuditModelProtectionsRule` is the candidate-MAJOR driver (it surfaces new errors in any consumer with an unprotected audit model — see its bullet), so the release as a whole classifies as candidate MAJOR. Per the pre-1.0 caret convention `^0.6` excludes the next minor, so tagging auto-adopts nobody — each consumer adopts on its own pin-bump PR. Seeds: war-room WR-0283 / WR-0275 DayUpdate pilot (controller prefixes); kendo Quartermaster M13 F-1 + war-room enforcement queue #46 (audit rule). ### Added - `ForbidEloquentMutationInControllersRule` + `EnforceCurrentUserAttributeRule` + `ForbidResourceWrappedInJsonResponseRule` — new shared optional `controllerNamespacePrefixes` PHPStan parameter (default `['App\Http\Controllers']`): a list of namespace prefixes whose classes are treated as controllers. A class is in scope when its namespace `str_starts_with` **any** listed prefix, so sub-namespaces (kendo's `App\Http\Controllers\Central\*`) still match the canonical prefix naturally — the default is behaviour-identical to the prior hardcoded `private const CONTROLLER_NAMESPACE_PREFIX = 'App\Http\Controllers'` + single-prefix `str_starts_with` gate all three rules carried before. Wired through `extension.neon` (`controllerNamespacePrefixes: ['App\Http\Controllers']` parameter + `listOf(string())` schema + `controllerNamespacePrefixes: %controllerNamespacePrefixes%` on all three rules' service registrations), mirroring the `formRequestBaseClass` / `resourceDataBaseClass` / `formRequestToDtoExemptClasses` parameter precedent. A consumer with divergent controller namespaces (emmie's `App\Http\Client\Controllers` + `App\Http\Admin\Controllers`) brings them into scope by adding the prefixes in its `phpstan.neon`. No consumer namespace is ever hardcoded in a rule body — the prefix list is *config*, preserving the package's "never by name inside the rule" convention. This resolves the `ForbidEloquentMutationInControllersRule` docblock's own standing TODO ("if a future consumer ships controllers under a divergent namespace, lift this into a `controllerNamespacePrefixes` parameter") — now done, and applied uniformly to all three controller-scoped rules so the one knob governs the whole controller surface. README documents the param + a "Covering sub-namespaced controllers" recipe using emmie's real namespaces. The default-unchanged invariant is pinned end-to-end by a container-resolved test per rule (`testRuleResolvesFromExtensionNeonAndFiresOnDefaultPrefix` — resolves the rule from the PHPStan container so the shipped NEON default + `%controllerNamespacePrefixes%` wiring are exercised, then asserts a canonical/sub-namespaced controller still flags) plus every pre-existing fixture and test (the kendo `App\Http\Controllers\Central\*` sub-namespace still flags under the default). A new sub-namespaced-controller fixture per rule proves the emmie shape is CLEAN under the default and FLAGGED once the sub-namespace prefix is configured. **Versioning: backward-compatible MINOR** (new optional parameter, default reproduces current behaviour ⇒ zero new errors in existing consumers — the default-list means no consumer sees behaviour change until it opts in). Do NOT push / tag from this entry — the release PR + tag is a separate ally-gated step. +- `EnforceAuditModelProtectionsRule` — new rule enforcing ADR-0001 §Append-only on audit-log models, discovered by SHAPE rather than a hand-maintained class list (a **denylist inversion**, war-room enforcement queue #46). An Eloquent `Model` subclass is treated as an audit record when its short name ends with any configured suffix (`auditModelNameSuffixes`, default `['AuditLog']`) **OR** its FQCN sits under any configured namespace prefix (`auditModelNamespacePrefixes`, default `['App\Models\Audit']`) — a union covering both fleet identification strategies: kendo's `*AuditLog` models scattered across `App\Models` + `App\Models\Central` (suffix), and the entreezuil / ublgenie `App\Models\Audit\*` collection including non-`AuditLog`-suffixed channel logs like `AuthEventLog` / `SmsEventLog` (namespace). The rule then flags three append-only protections, each firing independently at the class line: `HasFactory` present (`enforceAuditModelProtections.hasFactoryForbidden` — a factory is a direct-insert path bypassing the hash-chained writer), `SoftDeletes` present (`.softDeletesForbidden` — audit rows are never removed), and a mutable `updated_at` (`.updatedAtNotDisabled` — the model does not declare `public const UPDATED_AT = null`). Trait detection is transitive (inherited / composed traits count); abstract intermediates are exempt (their concrete leaves carry inherited violations); non-model classes named `*AuditLog` are excluded by the Eloquent `Model` type gate. **This is the inverse of the allowlist arch tests it supersedes** (kendo `tests/Arch/AuditTest.php`'s 13-FQCN `HasFactory` / `SoftDeletes` lists; entreezuil `tests/Architecture/AuditTest.php` + ublgenie `tests/Arch/AuditTest.php` namespace sweeps + `UPDATED_AT` reflection checks) — a hand-maintained list silently exempts every future audit model added outside it, the exact omission-escape the inversion closes; a territory retires its local model-side checks by moving the discovery convention into the two parameters. Configuration expresses patterns, never enumerated class names — no consumer class name is hardcoded in the rule body. Wired through `extension.neon` (`auditModelNamespacePrefixes` / `auditModelNameSuffixes` parameters + `listOf(string())` schemas + `%...%` arguments on the service registration). README documents discovery, the three protections, the migration recipe, and the `$timestamps = false` false-positive suppression path. **Versioning: candidate MAJOR bump** (the rule surfaces new errors in any consumer territory that has an audit model using `HasFactory` / `SoftDeletes` or missing `const UPDATED_AT = null`). Per the pre-1.0 caret convention `^0.6` excludes the next minor, so tagging auto-adopts nobody — each consumer remediates and goes green on its own bump PR (a suppress-only / baseline-absorb posture: real gaps parked, rule bugs fixed upstream). Seed: kendo Quartermaster M13 F-1 (2026-04-22), war-room enforcement queue #46. **NOT tagged** (release is ally-gated). ## [0.6.1] — 2026-07-01 diff --git a/CLAUDE.md b/CLAUDE.md index 303a7f3..be7534c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -33,6 +33,7 @@ Composer package distributing war-room-doctrine PHPStan rules across `script-dev | `EnforceResourceDataValidatorOptInRule` | ADR-0009 §EAGER_LOAD validator opt-in | `enforceResourceDataValidatorOptIn.missingValidatorCall` | | `EnforceFormRequestToDtoRule` | ADR-0012 §FormRequest → DTO Flow | `enforceFormRequestToDto.missingToDtoMethod` | | `EnforceCurrentUserAttributeRule` | War-room §Explicit over implicit | `enforceCurrentUserAttribute.useAttributeInsteadOfRequestUser` | +| `EnforceAuditModelProtectionsRule` | ADR-0001 §Append-only | `enforceAuditModelProtections.hasFactoryForbidden` / `.softDeletesForbidden` / `.updatedAtNotDisabled` (denylist-inversion; discovers audit models by shape — `auditModelNameSuffixes` default `AuditLog` OR `auditModelNamespacePrefixes` default `App\Models\Audit` — and flags `HasFactory` / `SoftDeletes` / missing `const UPDATED_AT = null`. `[Unreleased]`) | | `ConnectionTransactionReturnTypeExtension` | (type extension, no rule) | — | Phase 2 expands the rule set: `EnforceAuditSnapshotOnRetryRule` (ADR-0001 §Snapshot-on-Retry Safety) was the first Phase 2 addition, promoted from cross-territory Pest arch tests (emmie PR #187, entreezuil PR #139, ublgenie PR #166, kendo PR #1029). `EnforceResourceDataValidatorOptInRule` (ADR-0009 §EAGER_LOAD validator opt-in) is the second Phase 2 addition, promoted from kendo PR #1084 under war-room enforcement queue #55. `EnforceFormRequestToDtoRule` (ADR-0012) is the third Phase 2 addition, promoted from entreezuil's `tests/Arch/FormRequestsTest.php` under the same queue #55 (instance 2). `EnforceExplicitHydrationRule` (ADR-0019) is the next Phase 2 candidate. @@ -93,7 +94,7 @@ SemVer per ADR-0021: > Each bullet's rule→doctrine mapping is authoritative per the rule class's docblock "Doctrine source" line (ADR-0021 §Doctrine source in docblock). -- ADR-0001 (Audit Logging) — package distributes `LogRule` + `LogBuilderTruncateRule` (both §Append-only) + `EnforceAuditSnapshotOnRetryRule` (§Snapshot-on-Retry Safety); does not itself maintain audit logs. +- ADR-0001 (Audit Logging) — package distributes `LogRule` + `LogBuilderTruncateRule` (both §Append-only), `EnforceAuditSnapshotOnRetryRule` (§Snapshot-on-Retry Safety), and `EnforceAuditModelProtectionsRule` (§Append-only — flags audit-log models, discovered by shape, that use `HasFactory` / `SoftDeletes` or fail to disable `updated_at`; a denylist inversion of the consumer-side audit-model arch tests, `[Unreleased]`); does not itself maintain audit logs. - ADR-0002 (Cascade Deletion) — no application surface. - ADR-0009 (Unified ResourceData Pattern) — package distributes `EnforceResourceDataValidatorOptInRule` (§EAGER_LOAD validator opt-in, shipped in v0.3.0) and `ForbidResourceWrappedInJsonResponseRule` (resources own their own response serialization — bans wrapping a `JsonResource` in `response()->json()` / `new JsonResponse()` inside controllers, `[Unreleased]`); does not itself ship API resources. - ADR-0011 (Action Class Architecture) — package distributes `EnforceActionTransactionsRule` + `ForbidDatabaseManagerInActionsRule`, and `ForbidEloquentMutationInControllersRule` (ADR-0011 + ADR-0019, `[Unreleased]`); itself has no Actions. diff --git a/README.md b/README.md index 95fcfc7..70669e3 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,8 @@ includes: | `EnforceResourceDataValidatorOptInRule` | `enforceResourceDataValidatorOptIn.missingValidatorCall` | Classes extending `App\Http\Resources\ResourceData` | If the class declares a non-empty `EAGER_LOAD_COUNT` / `EAGER_LOAD_SUM` constant but never calls `validateRelationsLoaded()` in any method, error. | | `EnforceFormRequestToDtoRule` | `enforceFormRequestToDto.missingToDtoMethod` | Concrete classes extending `Illuminate\Foundation\Http\FormRequest` | If the class neither declares nor inherits a `toDto()` method, error. Abstract intermediates (`BaseFormRequest`) are exempt. Hand Actions a typed DTO, not `$request->validated()` arrays. Doctrine: ADR-0012 (FormRequest → DTO Flow). | | `EnforceCurrentUserAttributeRule` | `enforceCurrentUserAttribute.useAttributeInsteadOfRequestUser` | `Request::user()` / `Auth::user()` / `auth()->user()` calls inside `App\Http\Controllers\*` classes (namespace prefix, incl. sub-namespaces; configurable via `controllerNamespacePrefixes`) | Use `#[\Illuminate\Container\Attributes\CurrentUser] User $user` on the method parameter. Scope is decided by namespace, not class ancestry — a base-less `final` controller in `App\Http\Controllers` fires; FormRequests (`App\Http\Requests`), middleware (`App\Http\Middleware`), services, Actions (`App\Actions`), jobs, and console commands are silent because their namespaces do not start with the controller prefix (container-attribute injection does not apply to FormRequest methods regardless). | +| `EnforceCurrentUserAttributeRule` | `enforceCurrentUserAttribute.useAttributeInsteadOfRequestUser` | `Request::user()` / `Auth::user()` / `auth()->user()` calls inside `App\Http\Controllers\*` classes (namespace prefix, incl. sub-namespaces) | Use `#[\Illuminate\Container\Attributes\CurrentUser] User $user` on the method parameter. Scope is decided by namespace, not class ancestry — a base-less `final` controller in `App\Http\Controllers` fires; FormRequests (`App\Http\Requests`), middleware (`App\Http\Middleware`), services, Actions (`App\Actions`), jobs, and console commands are silent because their namespaces do not start with the controller prefix (container-attribute injection does not apply to FormRequest methods regardless). | +| `EnforceAuditModelProtectionsRule` | `enforceAuditModelProtections.hasFactoryForbidden` / `.softDeletesForbidden` / `.updatedAtNotDisabled` | Eloquent models recognised as audit records by SHAPE — short name ends with a configured suffix (default `AuditLog`) OR FQCN sits under a configured namespace (default `App\Models\Audit`) | Three append-only protections, each firing independently: using `HasFactory` (a factory is a direct-insert path bypassing the hash-chained writer), using `SoftDeletes` (audit rows are never removed), or not disabling `updated_at` (an audit row is written once and never mutated — declare `public const UPDATED_AT = null;`) is an error. Discovery is by pattern, never a hand-maintained class list — a denylist inversion, so a newly-added audit model cannot escape the protections by omission. Abstract intermediates are exempt (their concrete leaves carry inherited violations). Non-model classes named `*AuditLog` are excluded by the Eloquent `Model` type gate. Doctrine: ADR-0001 §Append-only. | ### `EnforceActionTransactionsRule` — write-method list @@ -178,6 +180,52 @@ parameters: ``` All three rules then flag inline Eloquent mutations, `Request::user()` / `Auth::user()` / `auth()->user()` calls, and `JsonResource`-wrapped JSON responses in those namespaces too. Prefixes are *config* — no consumer namespace is ever hardcoded in a rule body, preserving the "never by name inside the rule" convention. (Each backslash is single — NEON only unescapes `\\` inside double quotes; single-quoted `\\` stays two literal characters and would match nothing.) +### `EnforceAuditModelProtectionsRule` — configurable discovery (denylist inversion) + +This rule is the **inverse** of an allowlist arch test. The Pest predecessors it supersedes (kendo `tests/Arch/AuditTest.php`, entreezuil `tests/Architecture/AuditTest.php`, ublgenie `tests/Arch/AuditTest.php`) enumerate audit models — by a hand-maintained FQCN list or a namespace directory sweep — and assert each lacks `HasFactory` / `SoftDeletes` / a mutable `updated_at`. A hand-maintained list silently exempts every future audit model added outside it. This rule scans for the audit-model *shape* and flags any that lacks a protection, so nothing escapes by being forgotten. + +**Discovery** — an Eloquent `Model` subclass is an audit record if its short name ends with any configured suffix **OR** its FQCN sits under any configured namespace prefix. The two signals are a union covering both fleet strategies. Defaults: + +```neon +parameters: + auditModelNamespacePrefixes: + - 'App\Models\Audit' # entreezuil / ublgenie convention (incl. channel logs: AuthEventLog, SmsEventLog) + auditModelNameSuffixes: + - 'AuditLog' # kendo *AuditLog models, scattered across App\Models + App\Models\Central +``` + +A consumer whose audit models use a different family widens either list — for example, to bring a kendo-style channel-log pair (`AiOutboundLog`, `AiMcpLog`) into scope alongside the `*AuditLog` entity models: + +```neon +parameters: + auditModelNameSuffixes: + - 'AuditLog' + - 'OutboundLog' + - 'McpLog' +``` + +Configuration expresses **patterns**, never enumerated class names — no consumer class name is ever hardcoded in the rule body, and a non-model class named `*AuditLog` (a DTO, a service) is excluded by the Eloquent `Model` type gate. + +**Protections** — three checks fire independently (a model missing several yields several errors at the class line): + +| Identifier | Fires when | +|---|---| +| `enforceAuditModelProtections.hasFactoryForbidden` | the model uses `HasFactory` (transitively — an inherited trait on an abstract base counts). A factory is a direct-insert path that bypasses the hash-chained audit writer. | +| `enforceAuditModelProtections.softDeletesForbidden` | the model uses `SoftDeletes`. Audit rows are append-only and never removed. | +| `enforceAuditModelProtections.updatedAtNotDisabled` | the model does not declare `public const UPDATED_AT = null`. The framework `Model` base sets `UPDATED_AT = 'updated_at'`, so a model that never overrides it keeps a mutable timestamp — an audit row is written once and never mutated. | + +Abstract intermediates (`abstract class BaseAuditLog`) are exempt — the concrete leaf carries any inherited violation. + +**Migrating off the local arch test** — move the arch test's model-discovery convention into the parameters above, delete the local `HasFactory` / `SoftDeletes` / `updated_at` model checks, and the package rule becomes the single enforcement authority. (The append-only `update()` / `delete()` ban on `*Log` classes is a separate concern already covered by `LogRule`.) A genuine non-audit false positive — e.g. a `$timestamps = false` model with no `UPDATED_AT = null` — is suppressed per-file via `ignoreErrors` keyed on the specific identifier, with a rationale comment: + +```neon +parameters: + ignoreErrors: + - + identifier: enforceAuditModelProtections.updatedAtNotDisabled + # timestamps disabled entirely; no updated_at column to null out + path: app/Models/Audit/SomeTimestampFreeLog.php +``` ### Action namespace assumption diff --git a/extension.neon b/extension.neon index 037c187..578c5cb 100644 --- a/extension.neon +++ b/extension.neon @@ -36,12 +36,30 @@ parameters: # above. controllerNamespacePrefixes: - 'App\Http\Controllers' + # `EnforceAuditModelProtectionsRule`: how an audit-log model is recognised + # by SHAPE (a denylist-inversion — no hand-maintained model list). A model + # is an audit record if its FQCN sits under one of these namespace prefixes + # OR its short name ends with one of the `auditModelNameSuffixes` below. The + # namespace default matches the entreezuil / ublgenie `App\Models\Audit\*` + # convention (incl. channel logs like `AuthEventLog`); the suffix default + # matches the kendo `*AuditLog` models scattered across `App\Models` + + # `App\Models\Central`. Override per consumer to add a channel-log family. + # Each prefix uses single backslashes — see the NEON-quoting note above. + auditModelNamespacePrefixes: + - 'App\Models\Audit' + + # `EnforceAuditModelProtectionsRule`: class short-name suffixes that mark a + # model as an audit record (union with the namespace prefixes above). + auditModelNameSuffixes: + - 'AuditLog' parametersSchema: resourceDataBaseClass: string() formRequestBaseClass: string() formRequestToDtoExemptClasses: listOf(string()) controllerNamespacePrefixes: listOf(string()) + auditModelNamespacePrefixes: listOf(string()) + auditModelNameSuffixes: listOf(string()) services: - @@ -94,6 +112,12 @@ services: arguments: controllerNamespacePrefixes: %controllerNamespacePrefixes% tags: [phpstan.rules.rule] + - + class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceAuditModelProtectionsRule + arguments: + auditModelNamespacePrefixes: %auditModelNamespacePrefixes% + auditModelNameSuffixes: %auditModelNameSuffixes% + tags: [phpstan.rules.rule] - class: ScriptDevelopment\PhpstanWarroomRules\Type\ConnectionTransactionReturnTypeExtension tags: [phpstan.broker.dynamicMethodReturnTypeExtension] diff --git a/src/Rules/EnforceAuditModelProtectionsRule.php b/src/Rules/EnforceAuditModelProtectionsRule.php new file mode 100644 index 0000000..f32edb3 --- /dev/null +++ b/src/Rules/EnforceAuditModelProtectionsRule.php @@ -0,0 +1,244 @@ + enforceAuditModelProtections.hasFactoryForbidden + * - `SoftDeletes` present -> enforceAuditModelProtections.softDeletesForbidden + * - `UPDATED_AT` not disabled (null) -> enforceAuditModelProtections.updatedAtNotDisabled + * + * Trait detection is transitive: a trait used by the model, by any ancestor + * class, or by a trait-of-a-trait all count, so an inherited `HasFactory` on an + * abstract base is caught at the concrete leaf. `UPDATED_AT` is read via + * reflection constant lookup — the framework `Model` base defines + * `const UPDATED_AT = 'updated_at'`, so a model that never overrides it to + * `null` is flagged (the append-only tell every seed model declares). + * + * Configuration expresses PATTERNS, never enumerated class names — no consumer + * class name is ever hardcoded in this rule body, preserving the package's + * "never by name inside the rule" convention. A consumer whose audit models use + * a different suffix (e.g. a channel-log family) or namespace overrides the + * parameters; a genuine non-audit false positive (e.g. a `$timestamps = false` + * model with no `UPDATED_AT = null`) is suppressed per-file via `ignoreErrors` + * keyed on the specific identifier. + * + * @implements Rule + */ +final class EnforceAuditModelProtectionsRule implements Rule +{ + /** + * @param list $auditModelNamespacePrefixes FQCN namespace prefixes + * whose models are audit + * records (default + * `App\Models\Audit`) + * @param list $auditModelNameSuffixes class short-name + * suffixes marking an audit + * record (default + * `AuditLog`) + */ + public function __construct( + private array $auditModelNamespacePrefixes = ['App\Models\Audit'], + private array $auditModelNameSuffixes = ['AuditLog'], + ) {} + + public function getNodeType(): string + { + return InClassNode::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $classNode = $node->getOriginalNode(); + + if (!$classNode instanceof Class_) { + return []; + } + + if ($classNode->isAbstract()) { + return []; + } + + $classReflection = $node->getClassReflection(); + + if (!$this->matchesAuditModelIdentity($classReflection)) { + return []; + } + + // Type gate: a class merely NAMED like an audit log (a DTO, a service, + // an enum) is not an audit model. Only Eloquent models carry the + // trait / timestamp surface these protections govern. + if (!$classReflection->isSubclassOf(Model::class)) { + return []; + } + + $name = $classReflection->getName(); + $errors = []; + + if ($this->usesTraitTransitively($classReflection, HasFactory::class)) { + $errors[] = $this->buildError( + sprintf( + '%s is an audit model but uses the HasFactory trait — audit rows are written exclusively through the hash-chained audit writer inside a transaction; a factory offers a direct-insert path that bypasses the chain. Remove HasFactory. See ADR-0001 §Append-only.', + $name, + ), + 'enforceAuditModelProtections.hasFactoryForbidden', + $classNode->getStartLine(), + ); + } + + if ($this->usesTraitTransitively($classReflection, SoftDeletes::class)) { + $errors[] = $this->buildError( + sprintf( + '%s is an audit model but uses the SoftDeletes trait — audit logs are append-only and must never be soft-deleted. Remove SoftDeletes. See ADR-0001 §Append-only.', + $name, + ), + 'enforceAuditModelProtections.softDeletesForbidden', + $classNode->getStartLine(), + ); + } + + if (!$this->updatedAtIsDisabled($classReflection)) { + $errors[] = $this->buildError( + sprintf( + '%s is an audit model but does not disable updated_at — an audit row is written once and never mutated. Declare `public const UPDATED_AT = null;`. See ADR-0001 §Append-only.', + $name, + ), + 'enforceAuditModelProtections.updatedAtNotDisabled', + $classNode->getStartLine(), + ); + } + + return $errors; + } + + /** + * Structural-identity gate: the class short-name ends with a configured + * suffix, OR its FQCN sits under a configured namespace prefix. The trailing + * separator on the namespace match keeps `App\Models\AuditReport\X` from + * matching an `App\Models\Audit` prefix. + */ + private function matchesAuditModelIdentity(ClassReflection $classReflection): bool + { + $name = $classReflection->getName(); + $parts = explode('\\', $name); + $shortName = (string) end($parts); + + foreach ($this->auditModelNameSuffixes as $suffix) { + if ($suffix !== '' && str_ends_with($shortName, $suffix)) { + return true; + } + } + + foreach ($this->auditModelNamespacePrefixes as $prefix) { + if ($prefix !== '' && str_starts_with($name, $prefix . '\\')) { + return true; + } + } + + return false; + } + + /** + * True iff `$traitFqcn` is used by the class, any ancestor class, or any + * trait transitively used by those (trait-of-a-trait). `getTraits(true)` + * flattens traits-used-by-traits; walking `getParentClass()` catches an + * inherited `HasFactory` on an abstract base. + */ + private function usesTraitTransitively(ClassReflection $classReflection, string $traitFqcn): bool + { + for ($current = $classReflection; $current !== null; $current = $current->getParentClass()) { + foreach ($current->getTraits(true) as $trait) { + if ($trait->getName() === $traitFqcn) { + return true; + } + } + } + + return false; + } + + /** + * The framework `Model` base defines `const UPDATED_AT = 'updated_at'`, so + * every Eloquent model resolves the constant; an audit model must override + * it to `null` to opt out of the mutable timestamp. A missing constant + * (defensive — not reachable for a real Model subtype) is treated as + * not-disabled. + * + * Reads the literal constant value via native reflection (consumed inline — + * no generic-typed parameter, so the `ReflectionClass` invariance + * trap does not apply). PHPStan's `ClassConstantReflection::getValueType()` + * does not reliably resolve an untyped `= null` literal to a `NullType` in + * the fixture analysis environment, so the native literal value is the + * dependable read. + */ + private function updatedAtIsDisabled(ClassReflection $classReflection): bool + { + $native = $classReflection->getNativeReflection(); + + return $native->hasConstant('UPDATED_AT') && $native->getConstant('UPDATED_AT') === null; + } + + private function buildError(string $message, string $identifier, int $line): IdentifierRuleError + { + return RuleErrorBuilder::message($message) + ->identifier($identifier) + ->line($line) + ->build(); + } +} diff --git a/tests/Fixtures/AuditModelProtections/AbstractAuditBase.php b/tests/Fixtures/AuditModelProtections/AbstractAuditBase.php new file mode 100644 index 0000000..21780cd --- /dev/null +++ b/tests/Fixtures/AuditModelProtections/AbstractAuditBase.php @@ -0,0 +1,22 @@ + + */ +final class EnforceAuditModelProtectionsRuleTest extends RuleTestCase +{ + private const string HAS_FACTORY = '%s is an audit model but uses the HasFactory trait — audit rows are written exclusively through the hash-chained audit writer inside a transaction; a factory offers a direct-insert path that bypasses the chain. Remove HasFactory. See ADR-0001 §Append-only.'; + + private const string SOFT_DELETES = '%s is an audit model but uses the SoftDeletes trait — audit logs are append-only and must never be soft-deleted. Remove SoftDeletes. See ADR-0001 §Append-only.'; + + private const string UPDATED_AT = '%s is an audit model but does not disable updated_at — an audit row is written once and never mutated. Declare `public const UPDATED_AT = null;`. See ADR-0001 §Append-only.'; + + /** + * Override hook: when set, `getRule()` returns this instance instead of the + * default. Lets a single test reconfigure the discovery parameters. + */ + private ?Rule $ruleOverride = null; + + public function testCleanAuditModelIsNotFlagged(): void + { + // Extends Model, `const UPDATED_AT = null`, no HasFactory/SoftDeletes — + // the canonical compliant shape. Discovered by both signals, fires nothing. + $this->analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/CleanAuditLog.php'], + [], + ); + } + + public function testHasFactoryIsFlagged(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/FactoryAuditLog.php'], + [ + [ + sprintf(self::HAS_FACTORY, 'App\Models\Audit\FactoryAuditLog'), + 15, + ], + ], + ); + } + + public function testSoftDeletesIsFlagged(): void + { + $this->analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/SoftDeleteAuditLog.php'], + [ + [ + sprintf(self::SOFT_DELETES, 'App\Models\Audit\SoftDeleteAuditLog'), + 15, + ], + ], + ); + } + + public function testMutableUpdatedAtIsFlagged(): void + { + // Forgot `const UPDATED_AT = null` — inherits the framework default + // `'updated_at'`. The "forgot a protection" omission the inversion catches. + $this->analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/MutableAuditLog.php'], + [ + [ + sprintf(self::UPDATED_AT, 'App\Models\Audit\MutableAuditLog'), + 15, + ], + ], + ); + } + + public function testAllThreeProtectionsFireIndependently(): void + { + // HasFactory + SoftDeletes + no UPDATED_AT on one class — three separate + // errors at the class line, in rule-emission order. + $this->analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/TripleViolationAuditLog.php'], + [ + [ + sprintf(self::HAS_FACTORY, 'App\Models\Audit\TripleViolationAuditLog'), + 16, + ], + [ + sprintf(self::SOFT_DELETES, 'App\Models\Audit\TripleViolationAuditLog'), + 16, + ], + [ + sprintf(self::UPDATED_AT, 'App\Models\Audit\TripleViolationAuditLog'), + 16, + ], + ], + ); + } + + public function testNamespaceSignalDiscoversNonAuditLogSuffix(): void + { + // `AuthEventLog` does not end in `AuditLog`; it is discovered purely by + // the `App\Models\Audit` namespace signal (the entreezuil/ublgenie + // channel-log shape). Proves the namespace leg catches what the suffix + // leg misses. + $this->analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/AuthEventLog.php'], + [ + [ + sprintf(self::HAS_FACTORY, 'App\Models\Audit\AuthEventLog'), + 18, + ], + ], + ); + } + + public function testSuffixSignalDiscoversModelOutsideAuditNamespace(): void + { + // `App\Models\ScatteredAuditLog` sits outside `App\Models\Audit` (the + // kendo scattered shape); discovered purely by the `AuditLog` suffix. + $this->analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/ScatteredAuditLog.php'], + [ + [ + sprintf(self::HAS_FACTORY, 'App\Models\ScatteredAuditLog'), + 18, + ], + ], + ); + } + + public function testNonModelNamedLikeAuditLogIsNotFlagged(): void + { + // `App\Support\FakeAuditLog` matches the `AuditLog` suffix but is NOT an + // Eloquent Model — the type gate excludes it. Without the gate it would + // wrongly fire updatedAtNotDisabled (it declares no UPDATED_AT). + $this->analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/FakeAuditLog.php'], + [], + ); + } + + public function testAbstractBaseIsExemptAndConcreteLeafInheritsViolation(): void + { + // The abstract base carries HasFactory but is exempt (never a record); + // the concrete leaf declares no traits of its own yet is flagged via the + // transitive trait walk. It inherits `const UPDATED_AT = null` from the + // base, so ONLY the HasFactory error fires, at the leaf. + $this->analyse( + [ + __DIR__ . '/../Fixtures/AuditModelProtections/AbstractAuditBase.php', + __DIR__ . '/../Fixtures/AuditModelProtections/ConcreteInheritedAuditLog.php', + ], + [ + [ + sprintf(self::HAS_FACTORY, 'App\Models\Audit\ConcreteInheritedAuditLog'), + 15, + ], + ], + ); + } + + public function testSiblingNamespaceSharingTextPrefixIsNotFlagged(): void + { + // `App\Models\AuditReport\ReportSummary` shares the text prefix + // `App\Models\Audit` but is not under the `App\Models\Audit\` namespace. + // The trailing separator keeps it out of scope despite its HasFactory — + // the namespace match is on a namespace boundary, not a text prefix. + $this->analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/ReportSummary.php'], + [], + ); + } + + public function testTraitOfATraitHasFactoryIsCaughtTransitively(): void + { + // HasFactory is reached only through a composed trait + // (`ComposedFactoryTrait use HasFactory`), never directly nor via a + // parent class. The recursive trait walk must catch it. + $this->analyse( + [ + __DIR__ . '/../Fixtures/AuditModelProtections/ComposedFactoryTrait.php', + __DIR__ . '/../Fixtures/AuditModelProtections/ComposedTraitAuditLog.php', + ], + [ + [ + sprintf(self::HAS_FACTORY, 'App\Models\Audit\ComposedTraitAuditLog'), + 16, + ], + ], + ); + } + + public function testRegularModelWithHasFactoryIsNotFlagged(): void + { + // A normal model using HasFactory + SoftDeletes + mutable updated_at + // matches neither the suffix nor the namespace — the structural-identity + // gate keeps the rule off the ordinary model surface. + $this->analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/RegularPost.php'], + [], + ); + } + + public function testCustomSuffixDefaultIsClean(): void + { + // `App\Models\LifecycleTrail` matches no default signal. + $this->analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/LifecycleTrail.php'], + [], + ); + } + + public function testCustomSuffixParameterBringsModelIntoScope(): void + { + // Configure `auditModelNameSuffixes: ['Trail']` — the model now matches + // and its HasFactory violation fires. Proves the suffix parameter is + // honoured end-to-end (default namespace prefixes retained). + $this->ruleOverride = new EnforceAuditModelProtectionsRule(auditModelNameSuffixes: ['Trail']); + + $this->analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/LifecycleTrail.php'], + [ + [ + sprintf(self::HAS_FACTORY, 'App\Models\LifecycleTrail'), + 17, + ], + ], + ); + } + + public function testCustomNamespaceDefaultIsClean(): void + { + // `App\Ledger\PaymentRecord` matches no default signal. + $this->analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/PaymentRecord.php'], + [], + ); + } + + public function testCustomNamespaceParameterBringsModelIntoScope(): void + { + // Configure `auditModelNamespacePrefixes: ['App\Ledger']` — the model now + // matches and its HasFactory violation fires. Proves the namespace-prefix + // parameter is honoured end-to-end (default suffixes retained). + $this->ruleOverride = new EnforceAuditModelProtectionsRule(auditModelNamespacePrefixes: ['App\Ledger']); + + $this->analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/PaymentRecord.php'], + [ + [ + sprintf(self::HAS_FACTORY, 'App\Ledger\PaymentRecord'), + 18, + ], + ], + ); + } + + public function testRuleResolvesFromExtensionNeonAndFires(): void + { + // End-to-end pin on the extension.neon registration path consumers use: + // resolve the rule from the PHPStan container so the shipped + // `auditModelNamespacePrefixes` / `auditModelNameSuffixes` defaults and + // the `%...%` argument wiring are exercised — NOT the PHP constructor + // default. A NEON quoting regression in a shipped default (e.g. the + // double-backslash single-quoted form) silently no-ops discovery while + // every direct-instantiation test stays green; this is the only gate + // that catches it. + $this->ruleOverride = self::getContainer()->getByType(EnforceAuditModelProtectionsRule::class); + + $this->analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/FactoryAuditLog.php'], + [ + [ + sprintf(self::HAS_FACTORY, 'App\Models\Audit\FactoryAuditLog'), + 15, + ], + ], + ); + } + + /** + * Load the shipped extension.neon so testRuleResolvesFromExtensionNeonAndFires + * can pull the rule out of the container with its NEON-configured discovery + * parameters applied. + * + * @return array + */ + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/../../extension.neon', + ]; + } + + protected function getRule(): Rule + { + return $this->ruleOverride ?? new EnforceAuditModelProtectionsRule; + } +} From 0fefaa73b5890f2b05a567bff1c81e8f54674852 Mon Sep 17 00:00:00 2001 From: Gerard Oosterhof Date: Fri, 3 Jul 2026 12:18:27 +0200 Subject: [PATCH 2/2] feat(audit-rule): recognise $timestamps=false natively + isolate NEON discovery defaults in the container test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bus-review MINORs folded in pre-tag (town-crier request #245, reviews by the-general + dispatch at fac4264): - updatedAtIsDisabled() now also reads the native $timestamps property default — a model disabling timestamps wholesale never writes updated_at, so it satisfies the protection natively instead of forcing a per-file ignoreErrors suppression on every genuinely append-only $timestamps=false model. Error message + README recipe updated; pinned by the new TimestamplessAuditLog fixture. - testRuleResolvesFromExtensionNeonAndFires now analyses the two single-signal fixtures (ScatteredAuditLog = suffix-only, AuthEventLog = namespace-only) instead of the dual-matched FactoryAuditLog, so a NEON quoting regression in either shipped discovery default fails in isolation rather than hiding behind the other signal. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01GpcHhKiY83TxVnhrbqokyr --- CHANGELOG.md | 2 +- README.md | 10 +++--- .../EnforceAuditModelProtectionsRule.php | 36 +++++++++++++------ .../TimestamplessAuditLog.php | 20 +++++++++++ .../EnforceAuditModelProtectionsRuleTest.php | 33 ++++++++++++++--- 5 files changed, 80 insertions(+), 21 deletions(-) create mode 100644 tests/Fixtures/AuditModelProtections/TimestamplessAuditLog.php diff --git a/CHANGELOG.md b/CHANGELOG.md index e158cd5..89a344d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and ### Added - `ForbidEloquentMutationInControllersRule` + `EnforceCurrentUserAttributeRule` + `ForbidResourceWrappedInJsonResponseRule` — new shared optional `controllerNamespacePrefixes` PHPStan parameter (default `['App\Http\Controllers']`): a list of namespace prefixes whose classes are treated as controllers. A class is in scope when its namespace `str_starts_with` **any** listed prefix, so sub-namespaces (kendo's `App\Http\Controllers\Central\*`) still match the canonical prefix naturally — the default is behaviour-identical to the prior hardcoded `private const CONTROLLER_NAMESPACE_PREFIX = 'App\Http\Controllers'` + single-prefix `str_starts_with` gate all three rules carried before. Wired through `extension.neon` (`controllerNamespacePrefixes: ['App\Http\Controllers']` parameter + `listOf(string())` schema + `controllerNamespacePrefixes: %controllerNamespacePrefixes%` on all three rules' service registrations), mirroring the `formRequestBaseClass` / `resourceDataBaseClass` / `formRequestToDtoExemptClasses` parameter precedent. A consumer with divergent controller namespaces (emmie's `App\Http\Client\Controllers` + `App\Http\Admin\Controllers`) brings them into scope by adding the prefixes in its `phpstan.neon`. No consumer namespace is ever hardcoded in a rule body — the prefix list is *config*, preserving the package's "never by name inside the rule" convention. This resolves the `ForbidEloquentMutationInControllersRule` docblock's own standing TODO ("if a future consumer ships controllers under a divergent namespace, lift this into a `controllerNamespacePrefixes` parameter") — now done, and applied uniformly to all three controller-scoped rules so the one knob governs the whole controller surface. README documents the param + a "Covering sub-namespaced controllers" recipe using emmie's real namespaces. The default-unchanged invariant is pinned end-to-end by a container-resolved test per rule (`testRuleResolvesFromExtensionNeonAndFiresOnDefaultPrefix` — resolves the rule from the PHPStan container so the shipped NEON default + `%controllerNamespacePrefixes%` wiring are exercised, then asserts a canonical/sub-namespaced controller still flags) plus every pre-existing fixture and test (the kendo `App\Http\Controllers\Central\*` sub-namespace still flags under the default). A new sub-namespaced-controller fixture per rule proves the emmie shape is CLEAN under the default and FLAGGED once the sub-namespace prefix is configured. **Versioning: backward-compatible MINOR** (new optional parameter, default reproduces current behaviour ⇒ zero new errors in existing consumers — the default-list means no consumer sees behaviour change until it opts in). Do NOT push / tag from this entry — the release PR + tag is a separate ally-gated step. -- `EnforceAuditModelProtectionsRule` — new rule enforcing ADR-0001 §Append-only on audit-log models, discovered by SHAPE rather than a hand-maintained class list (a **denylist inversion**, war-room enforcement queue #46). An Eloquent `Model` subclass is treated as an audit record when its short name ends with any configured suffix (`auditModelNameSuffixes`, default `['AuditLog']`) **OR** its FQCN sits under any configured namespace prefix (`auditModelNamespacePrefixes`, default `['App\Models\Audit']`) — a union covering both fleet identification strategies: kendo's `*AuditLog` models scattered across `App\Models` + `App\Models\Central` (suffix), and the entreezuil / ublgenie `App\Models\Audit\*` collection including non-`AuditLog`-suffixed channel logs like `AuthEventLog` / `SmsEventLog` (namespace). The rule then flags three append-only protections, each firing independently at the class line: `HasFactory` present (`enforceAuditModelProtections.hasFactoryForbidden` — a factory is a direct-insert path bypassing the hash-chained writer), `SoftDeletes` present (`.softDeletesForbidden` — audit rows are never removed), and a mutable `updated_at` (`.updatedAtNotDisabled` — the model does not declare `public const UPDATED_AT = null`). Trait detection is transitive (inherited / composed traits count); abstract intermediates are exempt (their concrete leaves carry inherited violations); non-model classes named `*AuditLog` are excluded by the Eloquent `Model` type gate. **This is the inverse of the allowlist arch tests it supersedes** (kendo `tests/Arch/AuditTest.php`'s 13-FQCN `HasFactory` / `SoftDeletes` lists; entreezuil `tests/Architecture/AuditTest.php` + ublgenie `tests/Arch/AuditTest.php` namespace sweeps + `UPDATED_AT` reflection checks) — a hand-maintained list silently exempts every future audit model added outside it, the exact omission-escape the inversion closes; a territory retires its local model-side checks by moving the discovery convention into the two parameters. Configuration expresses patterns, never enumerated class names — no consumer class name is hardcoded in the rule body. Wired through `extension.neon` (`auditModelNamespacePrefixes` / `auditModelNameSuffixes` parameters + `listOf(string())` schemas + `%...%` arguments on the service registration). README documents discovery, the three protections, the migration recipe, and the `$timestamps = false` false-positive suppression path. **Versioning: candidate MAJOR bump** (the rule surfaces new errors in any consumer territory that has an audit model using `HasFactory` / `SoftDeletes` or missing `const UPDATED_AT = null`). Per the pre-1.0 caret convention `^0.6` excludes the next minor, so tagging auto-adopts nobody — each consumer remediates and goes green on its own bump PR (a suppress-only / baseline-absorb posture: real gaps parked, rule bugs fixed upstream). Seed: kendo Quartermaster M13 F-1 (2026-04-22), war-room enforcement queue #46. **NOT tagged** (release is ally-gated). +- `EnforceAuditModelProtectionsRule` — new rule enforcing ADR-0001 §Append-only on audit-log models, discovered by SHAPE rather than a hand-maintained class list (a **denylist inversion**, war-room enforcement queue #46). An Eloquent `Model` subclass is treated as an audit record when its short name ends with any configured suffix (`auditModelNameSuffixes`, default `['AuditLog']`) **OR** its FQCN sits under any configured namespace prefix (`auditModelNamespacePrefixes`, default `['App\Models\Audit']`) — a union covering both fleet identification strategies: kendo's `*AuditLog` models scattered across `App\Models` + `App\Models\Central` (suffix), and the entreezuil / ublgenie `App\Models\Audit\*` collection including non-`AuditLog`-suffixed channel logs like `AuthEventLog` / `SmsEventLog` (namespace). The rule then flags three append-only protections, each firing independently at the class line: `HasFactory` present (`enforceAuditModelProtections.hasFactoryForbidden` — a factory is a direct-insert path bypassing the hash-chained writer), `SoftDeletes` present (`.softDeletesForbidden` — audit rows are never removed), and a mutable `updated_at` (`.updatedAtNotDisabled` — the model does not declare `public const UPDATED_AT = null`). Trait detection is transitive (inherited / composed traits count); abstract intermediates are exempt (their concrete leaves carry inherited violations); non-model classes named `*AuditLog` are excluded by the Eloquent `Model` type gate. **This is the inverse of the allowlist arch tests it supersedes** (kendo `tests/Arch/AuditTest.php`'s 13-FQCN `HasFactory` / `SoftDeletes` lists; entreezuil `tests/Architecture/AuditTest.php` + ublgenie `tests/Arch/AuditTest.php` namespace sweeps + `UPDATED_AT` reflection checks) — a hand-maintained list silently exempts every future audit model added outside it, the exact omission-escape the inversion closes; a territory retires its local model-side checks by moving the discovery convention into the two parameters. Configuration expresses patterns, never enumerated class names — no consumer class name is hardcoded in the rule body. Wired through `extension.neon` (`auditModelNamespacePrefixes` / `auditModelNameSuffixes` parameters + `listOf(string())` schemas + `%...%` arguments on the service registration). README documents discovery, the three protections, and the migration recipe. A model that disables timestamps wholesale (`public $timestamps = false;`) is recognised natively as satisfying the updated_at protection — no `ignoreErrors` suppression needed; the rule reads the native `$timestamps` default alongside the `UPDATED_AT` constant, matching Eloquent's own decision points — pinned by the `TimestamplessAuditLog` fixture. The container-resolved NEON test exercises the two shipped discovery defaults in ISOLATION (`ScatteredAuditLog` = suffix-only, `AuthEventLog` = namespace-only), so a quoting regression in either `extension.neon` parameter fails on its own instead of hiding behind the other signal. **Versioning: candidate MAJOR bump** (the rule surfaces new errors in any consumer territory that has an audit model using `HasFactory` / `SoftDeletes` or missing `const UPDATED_AT = null`). Per the pre-1.0 caret convention `^0.6` excludes the next minor, so tagging auto-adopts nobody — each consumer remediates and goes green on its own bump PR (a suppress-only / baseline-absorb posture: real gaps parked, rule bugs fixed upstream). Seed: kendo Quartermaster M13 F-1 (2026-04-22), war-room enforcement queue #46. **NOT tagged** (release is ally-gated). ## [0.6.1] — 2026-07-01 diff --git a/README.md b/README.md index 70669e3..ecea121 100644 --- a/README.md +++ b/README.md @@ -212,19 +212,19 @@ Configuration expresses **patterns**, never enumerated class names — no consum |---|---| | `enforceAuditModelProtections.hasFactoryForbidden` | the model uses `HasFactory` (transitively — an inherited trait on an abstract base counts). A factory is a direct-insert path that bypasses the hash-chained audit writer. | | `enforceAuditModelProtections.softDeletesForbidden` | the model uses `SoftDeletes`. Audit rows are append-only and never removed. | -| `enforceAuditModelProtections.updatedAtNotDisabled` | the model does not declare `public const UPDATED_AT = null`. The framework `Model` base sets `UPDATED_AT = 'updated_at'`, so a model that never overrides it keeps a mutable timestamp — an audit row is written once and never mutated. | +| `enforceAuditModelProtections.updatedAtNotDisabled` | the model does not declare `public const UPDATED_AT = null`. The framework `Model` base sets `UPDATED_AT = 'updated_at'`, so a model that never overrides it keeps a mutable timestamp — an audit row is written once and never mutated. A model that disables timestamps wholesale (`public $timestamps = false;`) never writes `updated_at` at all and is recognised natively as compliant. | Abstract intermediates (`abstract class BaseAuditLog`) are exempt — the concrete leaf carries any inherited violation. -**Migrating off the local arch test** — move the arch test's model-discovery convention into the parameters above, delete the local `HasFactory` / `SoftDeletes` / `updated_at` model checks, and the package rule becomes the single enforcement authority. (The append-only `update()` / `delete()` ban on `*Log` classes is a separate concern already covered by `LogRule`.) A genuine non-audit false positive — e.g. a `$timestamps = false` model with no `UPDATED_AT = null` — is suppressed per-file via `ignoreErrors` keyed on the specific identifier, with a rationale comment: +**Migrating off the local arch test** — move the arch test's model-discovery convention into the parameters above, delete the local `HasFactory` / `SoftDeletes` / `updated_at` model checks, and the package rule becomes the single enforcement authority. (The append-only `update()` / `delete()` ban on `*Log` classes is a separate concern already covered by `LogRule`.) A `$timestamps = false` model needs no suppression — the rule recognises disabled-wholesale timestamps natively. A remaining genuine non-audit false positive is suppressed per-file via `ignoreErrors` keyed on the specific identifier, with a rationale comment: ```neon parameters: ignoreErrors: - - identifier: enforceAuditModelProtections.updatedAtNotDisabled - # timestamps disabled entirely; no updated_at column to null out - path: app/Models/Audit/SomeTimestampFreeLog.php + identifier: enforceAuditModelProtections.hasFactoryForbidden + # seeded read-model projection, not an audit record; factory is test-only + path: app/Models/Audit/SomeProjectionLog.php ``` ### Action namespace assumption diff --git a/src/Rules/EnforceAuditModelProtectionsRule.php b/src/Rules/EnforceAuditModelProtectionsRule.php index f32edb3..a204d40 100644 --- a/src/Rules/EnforceAuditModelProtectionsRule.php +++ b/src/Rules/EnforceAuditModelProtectionsRule.php @@ -75,9 +75,12 @@ * class name is ever hardcoded in this rule body, preserving the package's * "never by name inside the rule" convention. A consumer whose audit models use * a different suffix (e.g. a channel-log family) or namespace overrides the - * parameters; a genuine non-audit false positive (e.g. a `$timestamps = false` - * model with no `UPDATED_AT = null`) is suppressed per-file via `ignoreErrors` - * keyed on the specific identifier. + * parameters. A model that disables timestamps wholesale + * (`public $timestamps = false;`) never writes `updated_at` at all and is + * recognised natively as satisfying the updated_at protection — no + * `ignoreErrors` suppression needed; a remaining genuine non-audit false + * positive is suppressed per-file via `ignoreErrors` keyed on the specific + * identifier. * * @implements Rule */ @@ -156,7 +159,7 @@ public function processNode(Node $node, Scope $scope): array if (!$this->updatedAtIsDisabled($classReflection)) { $errors[] = $this->buildError( sprintf( - '%s is an audit model but does not disable updated_at — an audit row is written once and never mutated. Declare `public const UPDATED_AT = null;`. See ADR-0001 §Append-only.', + '%s is an audit model but does not disable updated_at — an audit row is written once and never mutated. Declare `public const UPDATED_AT = null;` (or disable timestamps wholesale with `public $timestamps = false;`). See ADR-0001 §Append-only.', $name, ), 'enforceAuditModelProtections.updatedAtNotDisabled', @@ -220,18 +223,31 @@ private function usesTraitTransitively(ClassReflection $classReflection, string * (defensive — not reachable for a real Model subtype) is treated as * not-disabled. * - * Reads the literal constant value via native reflection (consumed inline — - * no generic-typed parameter, so the `ReflectionClass` invariance - * trap does not apply). PHPStan's `ClassConstantReflection::getValueType()` - * does not reliably resolve an untyped `= null` literal to a `NullType` in - * the fixture analysis environment, so the native literal value is the + * A model that disables timestamps wholesale (`public $timestamps = false;` + * — the framework base declares `public $timestamps = true`) never writes + * `updated_at` either, so it satisfies the protection natively rather than + * needing a per-file `ignoreErrors` suppression. Both reads use the default + * property/constant value, matching Eloquent's own decision points + * (`usesTimestamps()` / `getUpdatedAtColumn()`). + * + * Reads the literal values via native reflection (consumed inline — no + * generic-typed parameter, so the `ReflectionClass` invariance trap + * does not apply). PHPStan's `ClassConstantReflection::getValueType()` does + * not reliably resolve an untyped `= null` literal to a `NullType` in the + * fixture analysis environment, so the native literal value is the * dependable read. */ private function updatedAtIsDisabled(ClassReflection $classReflection): bool { $native = $classReflection->getNativeReflection(); - return $native->hasConstant('UPDATED_AT') && $native->getConstant('UPDATED_AT') === null; + if ($native->hasConstant('UPDATED_AT') && $native->getConstant('UPDATED_AT') === null) { + return true; + } + + $defaults = $native->getDefaultProperties(); + + return ($defaults['timestamps'] ?? true) === false; } private function buildError(string $message, string $identifier, int $line): IdentifierRuleError diff --git a/tests/Fixtures/AuditModelProtections/TimestamplessAuditLog.php b/tests/Fixtures/AuditModelProtections/TimestamplessAuditLog.php new file mode 100644 index 0000000..00bc75e --- /dev/null +++ b/tests/Fixtures/AuditModelProtections/TimestamplessAuditLog.php @@ -0,0 +1,20 @@ +analyse( + [__DIR__ . '/../Fixtures/AuditModelProtections/TimestamplessAuditLog.php'], + [], + ); + } + public function testHasFactoryIsFlagged(): void { $this->analyse( @@ -269,15 +280,27 @@ public function testRuleResolvesFromExtensionNeonAndFires(): void // default. A NEON quoting regression in a shipped default (e.g. the // double-backslash single-quoted form) silently no-ops discovery while // every direct-instantiation test stays green; this is the only gate - // that catches it. + // that catches it. The two fixtures each isolate ONE signal — + // ScatteredAuditLog matches only the suffix default, AuthEventLog only + // the namespace default — so a quoting regression in EITHER shipped + // parameter fails this test on its own; a dual-matched fixture + // (FactoryAuditLog) would let a broken namespace default hide behind + // the still-working suffix signal. $this->ruleOverride = self::getContainer()->getByType(EnforceAuditModelProtectionsRule::class); $this->analyse( - [__DIR__ . '/../Fixtures/AuditModelProtections/FactoryAuditLog.php'], + [ + __DIR__ . '/../Fixtures/AuditModelProtections/ScatteredAuditLog.php', + __DIR__ . '/../Fixtures/AuditModelProtections/AuthEventLog.php', + ], [ [ - sprintf(self::HAS_FACTORY, 'App\Models\Audit\FactoryAuditLog'), - 15, + sprintf(self::HAS_FACTORY, 'App\Models\ScatteredAuditLog'), + 18, + ], + [ + sprintf(self::HAS_FACTORY, 'App\Models\Audit\AuthEventLog'), + 18, ], ], );