Skip to content

Commit 1a222eb

Browse files
Goosterhofclaude
andcommitted
feat: add ForbidEloquentMutationInControllersRule (queue #87)
New PHPStan rule banning Eloquent persistence-API method calls (save/update/delete/create/destroy/forceDelete/forceFill/push/restore/touch and their *OrFail / *Quietly / *OrCreate variants — 24-method blocklist) on `Illuminate\Database\Eloquent\Model` subclasses and `Illuminate\Database\Eloquent\Builder` chains when the call site is inside an `App\Http\Controllers\*` class (including sub-namespaces via `str_starts_with`). Reads (`find`, `where`, `get`, `first`, `paginate`, `pluck`, `count`, `exists`, `query`) deliberately permitted — controllers reading Models is necessary for route-model binding, ResourceData hydration, and policy checks. Identifier: `forbidEloquentMutationInControllers.eloquentMutationInController`. Doctrine source: ADR-0011 (Action Class Architecture) + ADR-0019 (Explicit Model Hydration). Algorithm: - Namespace gate (`App\Http\Controllers` prefix) - Recursively walk every `ClassMethod` body collecting `MethodCall`/`StaticCall` - For `MethodCall`: type-aware `ObjectType::isSuperTypeOf()` against `Model` OR `Builder` matches the receiver; method name in blocklist fires - For `StaticCall`: `Scope::resolveName()` resolves to Model subclass FQCN; method name in blocklist fires Builder coverage uses resolution (a) per order §A7 — type-aware `ObjectType::isSuperTypeOf()` handles `Builder<User>` as a subtype of the unparameterized `Builder` cleanly, no brittle generic introspection needed. Verified empirically by `ViolationBuilderUpdate.php` fixture firing at the `->update([...])` line of a `User::query()->where(...)->update([...])` chain. Supersedes consumer-side string-match Pest arch tests in kendo, ublgenie, entreezuil, and the ISMS bridge subset from PR #10. Cross-territory cascade is the General's follow-up dispatch after this lands; emmie + BIO pick up coverage automatically on next composer update. 14 tests, 14 assertions; PHPStan max self-analysis clean (10 services); Pint clean; line coverage 87.98% (new rule 91.01%) ≥83 gate; MSI 82%, Covered Code MSI 82% (both ≥75 gate). The 7 escaped mutants on the new rule are all in the recognizable Standing Concern #29 walkNodes() parity family + MBString- equivalent + defensive `??` defaults — same shape as the precedent on the other four rules. Closes war-room enforcement queue #87. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f6e3757 commit 1a222eb

21 files changed

Lines changed: 993 additions & 0 deletions

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and
88

99
### Added
1010

11+
- `ForbidEloquentMutationInControllersRule` — bans Eloquent persistence APIs (`save`, `update`, `delete`, `create`, `destroy`, `forceDelete`, `forceFill`, `push`, `restore`, `touch`, and their `*OrFail` / `*Quietly` / `*OrCreate` variants — 24-method blocklist) on `Illuminate\Database\Eloquent\Model` subclasses and `Illuminate\Database\Eloquent\Builder` chains when the call site is inside an `App\Http\Controllers\*` class (including sub-namespaces like kendo's `App\Http\Controllers\Central\*`, matched via `str_starts_with`). Reads (`find`, `where`, `get`, `first`, `paginate`, `pluck`, `count`, `exists`, `query`) are deliberately permitted — route-model binding, ResourceData hydration, and policy checks need controller-level Model access; the doctrine line is "Controllers may READ Models, but MUST NOT mutate them." Identifier: `forbidEloquentMutationInControllers.eloquentMutationInController`. Doctrine: ADR-0011 (Action Class Architecture) — Actions are the chokepoint for mutations — combined with ADR-0019 (Explicit Model Hydration) — `Model::create()` / `fill()` / `forceFill()` / `update()` banned application-wide; this rule enforces the controller surface where the violations have been historically common. Algorithm: namespace gate (`App\Http\Controllers`) → recursively walk every `ClassMethod` body collecting `MethodCall` + `StaticCall` nodes → for `MethodCall`, fire if `ObjectType::isSuperTypeOf()` against `Model` OR `Builder` matches the receiver type and the method name is in the blocklist; for `StaticCall`, fire if `Scope::resolveName()` resolves to a Model subclass FQCN and the method name is in the blocklist. Builder coverage is type-aware (`User::query()->where(...)->update([...])` fires) — the generic parameter is not unwrapped because `ObjectType` matches `Builder<User>` as a subtype of the unparameterized `Builder` cleanly, no brittle generic introspection needed. Supersedes the consumer-side string-match Pest arch tests in kendo (`backend/tests/Arch/ControllersTest.php` `controllers must not call Eloquent write methods directly`), ublgenie + entreezuil (`tests/Arch/ControllersTest.php` of the same shape), and the bridge subset in ISMS (`backend/tests/Architecture/ControllerCurrentUserTest.php` from PR #10, 2026-05-28). The string-match shape catches `->save(`, `->update([`, `->delete(`, `->forceDelete(` but cannot discriminate `Model::create()` from `Response::create()`, `Collection::push()` from `Model::push()`, or `->update($vars)` without an inline array literal — the type-aware AST inspection here closes those gaps. Cross-territory cascade post-merge: consumer Pest tests deleted; emmie + brick-inventory-orchestrator pick up coverage automatically on next composer update. Out of scope: non-`App\Http\Controllers\*` namespaces (Actions/Services/Jobs/Middleware are allowed to call persistence APIs), non-Eloquent receivers, dynamic method names (`$model->{$var}()` — value-flow analysis), variable class names in static calls (`$class::create(...)`). Closes war-room enforcement queue #87. **Versioning: per ADR-0021 §Versioning, candidate Major bump (the rule surfaces new errors in already-clean code wherever a consumer territory has a controller calling Eloquent persistence APIs directly — the three territories currently running the string-match Pest test caught the bulk of these, but the type-aware shape will surface additional violations the string-match shape missed: `Model::create()`, `Model::destroy()`, chained Builder mutations, `*Quietly` variants, etc.). Pre-cascade audit required across ISMS, kendo, emmie, entreezuil, ublgenie, brick-inventory before tagging — three territories' Pest tests already closed the string-match-visible violators; the type-aware additional surface (Builder chains, `Model::create()`, `*Quietly` variants) may carry undetected violators.**
1112
- `EnforceAuditTransactionScopeRule` — enforces ADR-0029 (Audit Row Durability Contract) §Decision rule 3. Flags non-transactional state mutations (`StatefulGuard` / `Session` / `Cache` / `Bus` / `Queue` / `Mailer` / `Notification` / `Broadcaster` / `Filesystem` and their `Illuminate\Support\Facades\*` counterparts, mutation methods only) inside `transaction(...)` closures in `App\Actions\*` classes. Identifier: `enforceAuditTransactionScope.nonTransactionalMutationInClosure`. Doctrine: ADR-0029 §Decision rule 3. Seed: ISMS-0003 PR #7 commit `f1d357b` (2026-05-28) — three Auth Actions (`AuthenticateWorkerAction`, `VerifyTwoFactorChallengeAction`, `LogoutWorkerAction`) mutated `StatefulGuard` + `Session` state inside the transaction closure before the audit row write; an audit-write failure would have rolled back the audit row while leaving the session/guard mutation intact (A.8.15 / A.5.33 violation). Reads (`Auth::user()`, `Session::get()`, `Cache::get()`, etc.) are deliberately permitted — only mutations carry the rollback-vs-side-effect asymmetry. Instance-call detection matches the constructor-property's declared FQCN against the blocklist keys; static-call detection resolves the facade name via `Scope::resolveName()`. Nested `transaction(...)` calls inside an outer closure are walked transitively — a nested mutation is still inside the outermost transaction's rollback scope; top-level transaction discovery deduplicates so each violation reports exactly once. Out of scope: manual transaction management (`DB::beginTransaction()` / `commit()`); non-`App\Actions\*` namespaces; the failure-side discipline (sentinel-return; throw-inside-closure detection) which lives as per-territory Pest arch tests under enforcement queue #85. **Versioning: per ADR-0021 §Versioning, candidate Major bump (the rule surfaces new errors in already-clean code wherever a consumer territory has an `App\Actions\*` class mutating non-transactional state inside a `transaction(...)` closure). Pre-cascade audit required across ISMS, kendo, emmie, entreezuil, ublgenie, brick-inventory before tagging — ISMS-0003 PR #7 commit `f1d357b` already closed ISMS's known violators; other consumer territories may carry undetected violators.**
1213

1314
## [0.3.0] — 2026-05-13

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ Composer package distributing war-room-doctrine PHPStan rules across `script-dev
2727
| `LogBuilderTruncateRule` | ADR-0001 §Append-only | `logRule.logModification` (shared with `LogRule`; covers `Builder->truncate()` on Log-named tables — ships with v0.3.0 per `[Unreleased]`) |
2828
| `EnforceAuditSnapshotOnRetryRule` | ADR-0001 §Snapshot-on-Retry Safety | `enforceAuditSnapshotOnRetry.firstStatementMustResetState` |
2929
| `EnforceAuditTransactionScopeRule` | ADR-0029 | `enforceAuditTransactionScope.nonTransactionalMutationInClosure` |
30+
| `ForbidEloquentMutationInControllersRule` | ADR-0011 + ADR-0019 | `forbidEloquentMutationInControllers.eloquentMutationInController` |
3031
| `EnforceResourceDataValidatorOptInRule` | ADR-0009 §EAGER_LOAD validator opt-in | `enforceResourceDataValidatorOptIn.missingValidatorCall` |
3132
| `ConnectionTransactionReturnTypeExtension` | (type extension, no rule) ||
3233

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ includes:
4444
| `LogBuilderTruncateRule` | `logRule.logModification` | `Builder->truncate()` calls | If the fluent chain's most recent `table()` call targets a Log-named table (string-literal argument matching `"log"` / `"logs"`, case-insensitive), error. Sibling rule to `LogRule`; shares the `logRule.logModification` identifier so a single `ignoreErrors` entry covers both. Eloquent `from()` chains and Model-`$table`-property-driven tables are acceptable misses. Doctrine: ADR-0001 §Append-only. |
4545
| `EnforceAuditSnapshotOnRetryRule` | `enforceAuditSnapshotOnRetry.firstStatementMustResetState` | `App\Actions\*` whose constructor injects an entity audit logger | The first statement inside `$connection->transaction(...)` must reset the model's in-memory state (`$model->refresh()`, fresh fetch, or fresh instantiation). Doctrine: ADR-0001 §Snapshot-on-Retry Safety. |
4646
| `EnforceAuditTransactionScopeRule` | `enforceAuditTransactionScope.nonTransactionalMutationInClosure` | `App\Actions\*` whose `execute()` calls `transaction(...)` with a literal closure | Mutating `StatefulGuard` / `Session` / `Cache` / `Bus` / `Queue` / `Mailer` / `Notification` / `Broadcaster` / `Filesystem` state (or their `Illuminate\Support\Facades\*` counterparts) inside the closure is an error. Reads (`Auth::user()`, `Session::get()`, `Cache::get()`) are permitted. Doctrine: ADR-0029 (Audit Row Durability Contract) §Decision rule 3. |
47+
| `ForbidEloquentMutationInControllersRule` | `forbidEloquentMutationInControllers.eloquentMutationInController` | `App\Http\Controllers\*` (including sub-namespaces) | Calling Eloquent persistence APIs (`save`, `update`, `delete`, `create`, `destroy`, `forceDelete`, `forceFill`, `push`, `restore`, `touch`, and their `*OrFail` / `*Quietly` / `*OrCreate` variants — 24-method blocklist) on `Illuminate\Database\Eloquent\Model` subclasses or `Illuminate\Database\Eloquent\Builder` chains is an error. Reads (`find`, `where`, `get`, `first`, `paginate`, `pluck`, `count`, `exists`, `query`) are permitted. Delegate mutations to an Action. Doctrine: ADR-0011 (Action Class Architecture) + ADR-0019 (Explicit Model Hydration). |
4748
| `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. |
4849

4950
### `EnforceActionTransactionsRule` — write-method list

extension.neon

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ services:
3030
-
3131
class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceAuditTransactionScopeRule
3232
tags: [phpstan.rules.rule]
33+
-
34+
class: ScriptDevelopment\PhpstanWarroomRules\Rules\ForbidEloquentMutationInControllersRule
35+
tags: [phpstan.rules.rule]
3336
-
3437
class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceResourceDataValidatorOptInRule
3538
arguments:

0 commit comments

Comments
 (0)