Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and

### Added

- `EnforceCurrentUserAttributeRule` — flags calls to `Request::user()` / `Auth::user()` / `auth()->user()` inside classes extending `Illuminate\Routing\Controller`. The canonical fix is Laravel's `#[\Illuminate\Container\Attributes\CurrentUser]` container attribute on the method parameter — eliminates the implicit-nullable-then-assert dance (`$user = $request->user(); assert($user instanceof User);`) introduced by emmie PR #263 (EMMIE-0197) and recurring across the war-room fleet. Doctrine: war-room §Architectural Principles — Explicit over implicit. Identifier: `enforceCurrentUserAttribute.useAttributeInsteadOfRequestUser`. Detection branches on three call shapes via `CallLike` registration (mirrors `LogRule` v0.3.0 shape): `MethodCall` on `Illuminate\Http\Request` subtype receiver (type-based via `ObjectType::isSuperTypeOf()`); `MethodCall` whose receiver is a `FuncCall('auth')` (AST-shape match — the helper's return type is unloaded in stub-only environments); `StaticCall` resolving to `Illuminate\Support\Facades\Auth` (FQCN comparison via `$scope->resolveName()`). Scoped to Controller descendants via `ClassReflection::isSubclassOf(Illuminate\Routing\Controller)` — FormRequest descendants (where `$this->user()` is canonical because container-attribute injection does not apply to `FormRequest::rules()` / `toDto()` / `authorize()` invocations), middleware, services, Actions, jobs, and console commands are silent. Origin: war-room cross-territory recon 2026-05-22 (50+ violations across codebook, ublgenie, entreezuil, emmie; kendo already clean with 30 adopted sites). **Versioning: per ADR-0021 §Versioning, candidate Major bump (the rule surfaces new errors in already-clean code wherever a consumer territory has un-migrated controllers). The release PR will determine whether this collapses into the existing v0.3.0 [Unreleased] block (already a Major) or cuts as a separate Major (v0.4.0) after v0.3.0 ships.** **Pre-cascade audit required before tagging** — consuming territories will need either Medic dispatches to migrate (ublgenie 6 sites, entreezuil 3 sites, emmie 2 sites) or PHPStan-baseline staging (codebook ~40+ sites — safer than mass-edit on lightly-staffed AVG/NEN-7510-downstream territory). Kendo gets a constraint bump only (zero violations). Multi-guard ergonomics (`#[CurrentUser] Client $client` resolves via client guard, `#[CurrentUser] User $user` via user guard — verified in emmie's `ClientController::me`) work as expected: Laravel dispatches by typed parameter. Out of scope v1: `Auth::guard('name')->user()` and other guard-specific resolution paths — rare, substitution is more nuanced (`#[CurrentUser(guard: 'name')]`), do not appear in the recon yield.

- `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.**
- `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.**

Expand Down
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ Composer package distributing war-room-doctrine PHPStan rules across `script-dev
| `EnforceAuditTransactionScopeRule` | ADR-0029 | `enforceAuditTransactionScope.nonTransactionalMutationInClosure` |
| `ForbidEloquentMutationInControllersRule` | ADR-0011 + ADR-0019 | `forbidEloquentMutationInControllers.eloquentMutationInController` |
| `EnforceResourceDataValidatorOptInRule` | ADR-0009 §EAGER_LOAD validator opt-in | `enforceResourceDataValidatorOptIn.missingValidatorCall` |
| `EnforceCurrentUserAttributeRule` | War-room §Explicit over implicit | `enforceCurrentUserAttribute.useAttributeInsteadOfRequestUser` |
| `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. `EnforceExplicitHydrationRule` (ADR-0019) is the next Phase 2 candidate.
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ includes:
| `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. |
| `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). |
| `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. |
| `EnforceCurrentUserAttributeRule` | `enforceCurrentUserAttribute.useAttributeInsteadOfRequestUser` | `Request::user()` / `Auth::user()` / `auth()->user()` calls inside Controller descendants | Use `#[\Illuminate\Container\Attributes\CurrentUser] User $user` on the method parameter. FormRequest descendants, middleware, services, Actions, jobs, and console commands are silent — container-attribute injection does not apply to FormRequest methods, and the other contexts have their own canonical resolution paths. |

### `EnforceActionTransactionsRule` — write-method list

Expand Down
3 changes: 3 additions & 0 deletions extension.neon
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ services:
arguments:
resourceDataBaseClass: %resourceDataBaseClass%
tags: [phpstan.rules.rule]
-
class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceCurrentUserAttributeRule
tags: [phpstan.rules.rule]
-
class: ScriptDevelopment\PhpstanWarroomRules\Type\ConnectionTransactionReturnTypeExtension
tags: [phpstan.broker.dynamicMethodReturnTypeExtension]
197 changes: 197 additions & 0 deletions src/Rules/EnforceCurrentUserAttributeRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
<?php

declare(strict_types = 1);

namespace ScriptDevelopment\PhpstanWarroomRules\Rules;

use Illuminate\Http\Request;
use Illuminate\Support\Facades\Auth;
use PhpParser\Node;
use PhpParser\Node\Expr\CallLike;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\StaticCall;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Rules\IdentifierRuleError;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\ObjectType;

use function sprintf;

/**
* Flags Request::user() / Auth::user() / auth()->user() calls inside controller methods.
* Use the #[\Illuminate\Container\Attributes\CurrentUser] container attribute on the
* method parameter instead — eliminates the nullable-then-assert dance.
*
* Scoped to classes extending Illuminate\Routing\Controller. FormRequest descendants
* are excluded by design — container-attribute injection does not apply to
* FormRequest::rules() / toDto() / authorize() invocations. Middleware, services,
* Actions, jobs, and console commands are likewise out of scope: each context has
* its own canonical resolution path (constructor DI for Actions, authenticated
* payload threading for jobs, etc.).
*
* Detection (three call shapes branch in `processNode`):
* 1. `MethodCall` — receiver type is a (subtype of) `Illuminate\Http\Request`
* and method name is `user`. Type-based via `ObjectType::isSuperTypeOf()`,
* mirroring `EnforceAuditSnapshotOnRetryRule::receiverIsConnectionInterface()`.
* 2. `MethodCall` — receiver is a `FuncCall` to `auth()` and method name is
* `user`. AST-shape match (the helper has no class to type-check against
* in a stub-only fixture environment).
* 3. `StaticCall` — class name resolves to `Illuminate\Support\Facades\Auth`
* and method name is `user`. FQCN comparison via `$scope->resolveName()`
* handles aliased imports.
*
* Containing-class gate (applied to all three branches): walk
* `$scope->getClassReflection()` ancestry and fire only if the class extends
* `Illuminate\Routing\Controller`. Out-of-scope classes return silently.
*
* Implementation note: `getNodeType()` returns `CallLike::class` so the rule
* sees both `MethodCall` and `StaticCall` in a single registration — mirrors
* the `LogRule` v0.3.0 expansion shape.
*
* Doctrine source: war-room §Architectural Principles — Explicit over implicit.
*
* @implements Rule<CallLike>
*/
final class EnforceCurrentUserAttributeRule implements Rule
{
private const string TARGET_METHOD = 'user';

private const string CONTROLLER_BASE_FQCN = 'Illuminate\Routing\Controller';

private const string REQUEST_FQCN = Request::class;

private const string AUTH_FACADE_FQCN = Auth::class;

public function getNodeType(): string
{
return CallLike::class;
}

public function processNode(Node $node, Scope $scope): array
{
if (!$this->insideControllerMethod($scope)) {
return [];
}

if ($node instanceof MethodCall) {
return $this->processMethodCall($node, $scope);
}

if ($node instanceof StaticCall) {
return $this->processStaticCall($node, $scope);
}

return [];
}

/**
* Containing-class gate: the rule fires only inside methods of a class
* that extends `Illuminate\Routing\Controller`. FormRequest descendants,
* middleware, services, Actions, jobs, and console commands are silent
* because their class hierarchies do not include Controller.
*/
private function insideControllerMethod(Scope $scope): bool
{
$classReflection = $scope->getClassReflection();

if (!$classReflection instanceof ClassReflection) {
return false;
}

if ($classReflection->getName() === self::CONTROLLER_BASE_FQCN) {
return false;
}

return $classReflection->isSubclassOf(self::CONTROLLER_BASE_FQCN);
}

/**
* @return list<IdentifierRuleError>
*/
private function processMethodCall(MethodCall $node, Scope $scope): array
{
if (!$node->name instanceof Identifier || $node->name->toString() !== self::TARGET_METHOD) {
return [];
}

// Shape 1: $request->user() — receiver type is Illuminate\Http\Request
if ($this->receiverIsRequest($node, $scope)) {
return [$this->buildError('$request->user()')];
}

// Shape 2: auth()->user() — receiver is FuncCall('auth')
if ($this->receiverIsAuthHelper($node)) {
return [$this->buildError('auth()->user()')];
}

return [];
}

/**
* @return list<IdentifierRuleError>
*/
private function processStaticCall(StaticCall $node, Scope $scope): array
{
if (!$node->name instanceof Identifier || $node->name->toString() !== self::TARGET_METHOD) {
return [];
}

if (!$node->class instanceof Name) {
return [];
}

if ($scope->resolveName($node->class) !== self::AUTH_FACADE_FQCN) {
return [];
}

return [$this->buildError('Auth::user()')];
}

/**
* Type-based receiver gate for shape 1: `$scope->getType($node->var)` must
* be a (subtype of) `Illuminate\Http\Request`. Mirrors the canonical type
* pattern in `EnforceAuditSnapshotOnRetryRule::receiverIsConnectionInterface()`.
*/
private function receiverIsRequest(MethodCall $node, Scope $scope): bool
{
$receiverType = $scope->getType($node->var);
$requestType = new ObjectType(self::REQUEST_FQCN);

return $requestType->isSuperTypeOf($receiverType)->yes();
}

/**
* AST-shape gate for shape 2: the receiver is a `FuncCall` to `auth()`.
* No type-based check because the `auth()` helper's return type
* (AuthManager / Guard) is not loaded in stub-only fixture environments;
* the AST shape is unambiguous on its own.
*/
private function receiverIsAuthHelper(MethodCall $node): bool
{
if (!$node->var instanceof FuncCall) {
return false;
}

if (!$node->var->name instanceof Name) {
return false;
}

return $node->var->name->toString() === 'auth';
}

private function buildError(string $callShape): IdentifierRuleError
{
return RuleErrorBuilder::message(sprintf(
'Authenticated-user resolution in controller methods uses the #[CurrentUser] container attribute. '
. 'Add `#[\Illuminate\Container\Attributes\CurrentUser] User $user` to the method signature instead of calling %s inside the body.',
$callShape,
))
->identifier('enforceCurrentUserAttribute.useAttributeInsteadOfRequestUser')
->build();
}
}
16 changes: 16 additions & 0 deletions tests/Fixtures/CurrentUserAttribute/AuthFacadeInController.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

declare(strict_types = 1);

namespace App\Http\Controllers;

use Illuminate\Routing\Controller;
use Illuminate\Support\Facades\Auth;

final class AuthFacadeInController extends Controller
{
public function store(): ?object
{
return Auth::user();
}
}
Loading
Loading