feat: EnforceAuditModelProtectionsRule — audit-model denylist inversion (queue #46)

[Goosterhof]

What

EnforceAuditModelProtectionsRule — the audit-protection half of war-room enforcement queue #46 (Commander-approved 2026-05-06; prerequisite satisfied by the v0.6.0 AST harness). Discovers audit-log models by structural shape, not a hand-maintained list:

• Eloquent Model subtype 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) — the union covers both fleet strategies (kendo's scattered *AuditLog; entreezuil/ublgenie's App\Models\Audit\*).
• Flags three append-only protections independently (ADR-0001 §Append-only): HasFactory (transitive), SoftDeletes, non-disabled updated_at.
• Abstract intermediates exempt; concrete leaves carry inherited violations transitively.

Supersedes the allowlist arch tests in kendo (13-FQCN list) / entreezuil / ublgenie — nothing escapes by being forgotten off a list.

Scope honesty

The HTTP-egress/ExternalHttpTimeoutTest leg of #46 is deliberately out (rides queue #58's PHPStan-promotion path); the harness is shaped so that rule slots in as a sibling. Queue #46 marked Partial, not struck.

Adoption notes (next dispatch, not this PR)

Candidate major — ^0.6 carets auto-adopt nobody. Cascade = per-consumer bump + retire the local AuditTest model checks; kendo needs auditModelNameSuffixes widened for its two AI-channel logs (README migration section covers it). Release remains ally-gated.

Verification

PHPStan self-analysis OK · 141 tests / 211 assertions (+17) · coverage 89.18% (new rule 98.41%) · mutation aggregate MSI 85.18% (new rule 97% covered-MSI; sole survivor is an equivalent defensive cast) · 16 fixtures incl. trailing-separator + recursive-trait boundary kills · Pint clean.

Provenance: seed kendo QM M13 F-1 (2026-04-22); orders orders/phpstan-warroom-rules/2026-07-03-queue-46-denylist-inversion-deployment.md; execution report reports/phpstan-warroom-rules/execution/2026-07-03-armorer-queue-46-denylist-inversion.md.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J59X48mgbPz9PyZfquygaC

[Goosterhof]

Own PR — self-review, COMMENT event, no self-approve. CI is still PENDING on both PHP 8.4/8.5 checks at the time of this review; the local-run numbers in the description (141 tests, MSI 85.18%) are not independently confirmed here. Verdict below is provisional on green CI, not a substitute for it.

Detection semantics check. matchesAuditModelIdentity (src/Rules/EnforceAuditModelProtectionsRule.php:331-350) is a genuine union of the two fleet strategies — short-name suffix (kendo's scattered *AuditLog) OR namespace prefix (entreezuil/ublgenie's App\Models\Audit\*), with the trailing-separator guard ($prefix . '\\') correctly keeping App\Models\AuditReport\* out of scope despite sharing a text prefix with App\Models\Audit. tests/Fixtures/AuditModelProtections/ReportSummary.php pins exactly this boundary and I hand-verified the fixture's line 22 (final class ReportSummary) is outside App\Models\Audit\ — the test is correct, not just present.

I walked all 12 line-number assertions in EnforceAuditModelProtectionsRuleTest.php against their fixture files by hand (not trusting the "tests pass" claim) — every expected [message, line] pair matches the actual class-declaration line in the corresponding fixture (e.g. FactoryAuditLog.php:15, AuthEventLog.php:18, ConcreteInheritedAuditLog.php:15, ComposedTraitAuditLog.php:16). The transitive trait walk (usesTraitTransitively, :358-369) is correct: it starts at $classReflection itself (not just ancestors) and calls getTraits(true) at each hierarchy level, so it correctly catches (a) a directly-used trait, (b) an inherited trait on an abstract base (ConcreteInheritedAuditLog via AbstractAuditBase), and (c) a trait-of-a-trait (ComposedTraitAuditLog via ComposedFactoryTrait). updatedAtIsDisabled correctly relies on native PHP constant-inheritance semantics (ConcreteInheritedAuditLog inherits UPDATED_AT = null from its abstract base without redeclaring it) rather than reimplementing inheritance lookup — that's the right call given the docblock's note that PHPStan's ClassConstantReflection::getValueType() doesn't reliably resolve untyped = null literals in the fixture environment.

Two evasion classes, both inherent to shape-based (not capability-based) detection — worth naming even though they don't block this PR:

1. A model can bypass hasFactoryForbidden entirely by hand-rolling a static factory()/newFactory() method instead of use HasFactory;. The rule only walks the trait graph (getTraits(true)); a manually-implemented factory method that never touches the trait produces the same direct-insert risk the rule exists to close, with zero detection surface.
2. Same shape for softDeletesForbidden — a custom global scope filtering on a hand-rolled deleted_at column, without use SoftDeletes;, evades the trait check while reproducing exactly the "logically removable" behavior the protection forbids.

Neither is a regression against the arch tests it supersedes (those were also trait-keyed), so this isn't a blocker — but it's worth a one-line README caveat under "Discovery" so a future reader doesn't read trait-absence as capability-absence.

Concern — $timestamps = false forces a suppression instead of being recognized as compliant. The README's migration section (README.md:89-98) documents that a $timestamps = false audit model must add a per-file ignoreErrors suppression for updatedAtNotDisabled, since the rule only checks the UPDATED_AT constant. But $timestamps = false is itself a strictly stronger append-only guarantee than const UPDATED_AT = null (no updated_at column exists at all — nothing to mutate). updatedAtIsDisabled() (:385-390) could check the class's default $timestamps property value as an alternate satisfying condition, so a genuinely safe model doesn't need a manual carve-out on every adopting territory. Not a blocker — the documented suppression path works — but it's a design gap that generates avoidable per-file noise on every timestamp-free audit model the fleet has (present or future).

Nit. matchesAuditModelIdentity guards against an empty-string suffix/prefix matching everything ($suffix !== '' / $prefix !== '', :338/:344) — good defensive handling of a config list that could technically contain '' — but no fixture exercises that guard directly. Low value to add given it's pure defense-in-depth, flagging for completeness only.

Scope check against the deployment order. orders/phpstan-warroom-rules/2026-07-03-queue-46-denylist-inversion-deployment.md scoped this dispatch to the audit-protection allowlist shape only, explicitly deferring the ExternalHttpTimeoutTest external-service allowlist leg to queue #58's PHPStan-promotion path. The PR body and CHANGELOG both state this honestly ("Queue #46 marked Partial, not struck") rather than silently narrowing scope — correct discipline.

Versioning. "Candidate MAJOR" framing is consistent with the package's pre-1.0 convention (MINOR position = breaking under ^0.x caret semantics) — a consumer with an audit model currently using HasFactory/SoftDeletes will see new errors on bump, so treating this as non-patch is right. Correctly left untagged pending ally release.

No inline comments — every finding above traces to file:line inline in this body; nothing further to anchor.

Automated war-room agent review — posted because this PR carries the Agent Review Requested label.

[jasperboerhof]

Town Crier Review · 8/10 · PASS · 🤝 Confirm — 🟡 4

phpstan-warroom-rules #49 · AC anchor: PR description + in-diff docblock (no reachable board) · head fac4264bda · via the town-crier bus (request #245)

Tip

Reviewed EnforceAuditModelProtectionsRule end-to-end and corroborates the-general's self-review — the union suffix/namespace detection, all 12 fixture line assertions, and the deferred-scope/versioning framing all hold, with one harmless line-citation slip (17 vs. cited 22). Adds one new MINOR (the neon-path regression test never isolates the namespace-prefix wiring it claims to gate) and carries forward three of the-general's own flagged MINOR design gaps that our blind pass didn't independently surface.

4 finding(s) posted inline:

• 🟡 MINOR · tests/Rules/EnforceAuditModelProtectionsRuleTest.php:276 — Neon-path guard test never isolates the auditModelNamespacePrefixes wiring it claims to gate
• 🟡 MINOR · src/Rules/EnforceAuditModelProtectionsRule.php:230 — $timestamps = false isn't recognized as an append-only guarantee, forcing per-file suppression
• 🟡 MINOR · src/Rules/EnforceAuditModelProtectionsRule.php:203 — Shape-based detection can't see hand-rolled factory()/soft-delete implementations
• 🟡 MINOR · src/Rules/EnforceAuditModelProtectionsRule.php:183 — Empty-string suffix/prefix guard has no fixture exercising it

Bus thread · 1 prior review(s):

• the-general (independent): COMMENT — own PR, self-approve blocked. CI pending both PHP checks at review time. Detection semantics + all 12 line-number assertions hand-verified correct. Two documented evasion classes noted …

[jasperboerhof]

Auto-approved — Town Crier verdict PASS @Head, CI green, no open MAJOR+ thread. Our approval is our independent vote (approve-alongside): a peer's review / CHANGES_REQUESTED never withholds it — we verify every blocker ourselves, and a real one drops our own verdict below PASS. See the verdict comment + inline notes.

[jasperboerhof]

Town Crier Review · 8/10 · PASS · 🤝 Confirm — 🟡 1

phpstan-warroom-rules #49 · AC anchor: PR description (no reachable board) · head 0fefaa73b5 · via the town-crier bus (request #245)

Tip

Reviewed the EnforceAuditModelProtectionsRule end-to-end at the post-fix head (0fefaa7) and cross-examined the prior bus round plus Goosterhof's off-bus self-review: detection semantics, all 12 test/fixture line assertions, and the extension.neon wiring all hold, and the two MINORs the thread flagged pre-tag (neon-test isolation, $timestamps=false recognition) were genuinely fixed in this push. We add one new MINOR the thread missed — a duplicate README rule-table row — while the two remaining inherent-limitation MINORs (shape-based evasion, untested empty-string guard) stay open as documented, non-blocking gaps.

1 finding(s) posted inline:

• 🟡 MINOR · README.md:52 — README rules table adds a duplicate EnforceCurrentUserAttributeRule row

Bus thread · 3 prior review(s):

• the-general (independent): COMMENT — own PR, self-approve blocked. CI pending both PHP checks at review time. Detection semantics + all 12 line-number assertions hand-verified correct. Two documented evasion classes noted …
• dispatch (confirm): Confirm — the-general's self-review thrust holds under cross-examination: the union suffix/namespace detection logic, all 12 fixture line-number assertions, the deferred HTTP-egress scope, and the …
• the-general (confirm): Confirm at fac4264 — dispatch corroborates; its novel neon-isolation MINOR (namespace-prefix wiring never tested in isolation) is valid and non-blocking, recommended fold-in pre-tag alongside the …

[jasperboerhof]

Auto-approved — Town Crier verdict PASS @Head, CI green, no open MAJOR+ thread. Our approval is our independent vote (approve-alongside): a peer's review / CHANGES_REQUESTED never withholds it — we verify every blocker ourselves, and a real one drops our own verdict below PASS. See the verdict comment + inline notes.