Skip to content

Commit 706fb91

Browse files
committed
feat: enforce ADR-0029 closure-scope discipline (queue #86)
Adds EnforceAuditTransactionScopeRule enforcing the success-side of ADR-0029 (Audit Row Durability Contract): non-transactional state mutations (StatefulGuard / Session / Cache / Bus / Queue / Mail / Notification / Broadcast / Storage facades + contracts, mutation methods only) MUST NOT happen 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). Failure-side (sentinel-return; throws inside closure) is enforcement queue #85 — separate Pest arch tests, not bundled. Algorithm: namespace-gate App\Actions\*; locate execute()'s top-level transaction() calls; for each, walk the closure body recursively including nested closures and nested transactions; flag every MethodCall|StaticCall whose receiver-type + method match the blocklist. Instance-call detection matches the constructor-property's declared FQCN against blocklist keys; static-facade detection uses Scope::resolveName(). Reads (Auth::user(), Session::get(), Cache::get()) are deliberately permitted. CHANGELOG [Unreleased] grows; no version bump in this PR.
1 parent 3555946 commit 706fb91

22 files changed

Lines changed: 1271 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+
- `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.**
1112
- `EnforceResourceDataValidatorOptInRule` — flags classes extending `App\Http\Resources\ResourceData` that declare a non-empty `EAGER_LOAD_COUNT` or `EAGER_LOAD_SUM` constant but do not call `validateRelationsLoaded()` anywhere in their method bodies. Without the call, missing eager-load aggregates fail open as `0` / `null` instead of throwing — silently re-introducing the silent-zero bug closed by kendo PR #1079 (KD-0494). Doctrine: ADR-0009 §EAGER_LOAD validator opt-in. Identifier: `enforceResourceDataValidatorOptIn.missingValidatorCall`. Promoted from kendo PR #1084's Pest arch test (Armorer, merged 2026-05-07 at `db20ea9cf`) — the third instance of the "arch test detects misuse but not omission" enforcement shape, dispositioned for Phase-2 promotion under war-room enforcement queue #55 by the Commander on 2026-05-07. Inheritance is matched via PHPStan reflection (FQCN ancestor traversal) — short-name collisions in unrelated namespaces do NOT match. The base FQCN is parameterizable via the `resourceDataBaseClass` PHPStan parameter (default: `App\Http\Resources\ResourceData`); territories whose `ResourceData` lives elsewhere can override per consumer `phpstan.neon`. Compliant call shapes: `self::validateRelationsLoaded($model)`, `static::validateRelationsLoaded($model)`, `$this->validateRelationsLoaded($model)` (instance form accepted for liberal compatibility with the source-of-truth Pest matcher, even though the base method is `protected static`). Empty-array constants (`EAGER_LOAD_COUNT = []`) do NOT fire — they are no-ops. **Versioning: per ADR-0021 §Versioning, candidate Major bump (the rule surfaces new errors in already-clean code wherever a consumer territory has a `ResourceData` subclass declaring the aggregate constants without the validator call). The release PR will determine whether v0.3.0 collapses this rule into the same Major bump as the staged `LogRule` static-call expansion, or cuts a separate Major.** **Pre-cascade audit required across emmie, kendo, entreezuil, ublgenie, brick-inventory before tagging** — the kendo arch test already closed kendo's standing proof point (`ProjectGithubRepoResourceData`) in PR #1084, but other consumer territories may carry undetected violators. Sister extractions for the FormRequest `toDto()` omission shape (queue #55 instance #2) and the routes `->can()` middleware omission shape (queue #55 instance #1) are deferred to separate dispatches.
1213

1314
### Security

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ Composer package distributing war-room-doctrine PHPStan rules across `script-dev
2626
| `LogRule` | ADR-0001 §Append-only | `logRule.logModification` (covers instance `update`/`delete`/`forceDelete`/`forceDeleteQuietly`; static `Model::destroy()` / `Model::forceDestroy()` ship with v0.3.0 per `[Unreleased]`) |
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` |
29+
| `EnforceAuditTransactionScopeRule` | ADR-0029 | `enforceAuditTransactionScope.nonTransactionalMutationInClosure` |
2930
| `EnforceResourceDataValidatorOptInRule` | ADR-0009 §EAGER_LOAD validator opt-in | `enforceResourceDataValidatorOptIn.missingValidatorCall` |
3031
| `ConnectionTransactionReturnTypeExtension` | (type extension, no rule) ||
3132

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ includes:
4343
| `LogRule` | `logRule.logModification` | `update()` / `delete()` calls | If the receiver type's class name contains `"Log"` or `"logs"` (case-insensitive), error. |
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. |
46+
| `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. |
4647
| `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. |
4748

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

extension.neon

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ services:
2727
-
2828
class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceAuditSnapshotOnRetryRule
2929
tags: [phpstan.rules.rule]
30+
-
31+
class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceAuditTransactionScopeRule
32+
tags: [phpstan.rules.rule]
3033
-
3134
class: ScriptDevelopment\PhpstanWarroomRules\Rules\EnforceResourceDataValidatorOptInRule
3235
arguments:

0 commit comments

Comments
 (0)