ForbidCredentialCastBypassRule — cast resolution is PHP dispatch, not an ancestry merge (#217, supersedes #68)

CHANGELOG.md

@@ -8,6 +8,37 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and
 
 ### Added
 
+- `ForbidCredentialCastBypassRule` — new rule (war-room enforcement queue #217) forbidding a `hashed` / `encrypted` / `encrypted:*` cast column from appearing as a key in a **query-builder** write payload: `Model::query()->…->update([...])`, `->insert(...)`, `->insertOrIgnore(...)`, `->insertGetId(...)`, `->upsert(...)`, `->updateOrInsert(...)`, the Postgres-only `updateFrom` / `insertOrIgnoreReturning`, `incrementOrCreate`, the increment family loud and quiet, a relation-derived builder write, or a `DB::table('…')` write on a mapped table. Registered in `extension.neon` with one new parameter, `credentialCastTableModels` (default `[]`). Identifier: `forbidCredentialCastBypass.castBypassedByBuilderWrite`. Doctrine: war-room §Architectural Principles #1 (Explicit over implicit) + #10; ISO 27001 A.5.33 and AVG on the compliance territories. Seed: lokalekeuze PR #65.
+
+  **The bug class.** Attribute casts fire on the MODEL path only — `setAttribute()` runs the cast when you assign `$model->password = $plain` and `save()`. `Illuminate\Database\Eloquent\Builder::update()` delegates to `toBase()->update()`, which ships the array straight to SQL: the credential lands in the column verbatim, with no hash, no encryption, no exception, and a **green test suite** (a test that reads the column back gets exactly what it wrote). The failure is silent at every layer and is normally discovered by reading the database. In the seed, `ReissueVoucherAction` wrote through the model by CHOICE while `BlockVoucherAction`'s builder idiom sat one file away — nothing but author preference separated the safe site from the unsafe one.
+
+  **Model path stays silent STRUCTURALLY, not by exemption.** The receiver type must be an Eloquent `Builder`, a query `Builder`, or a `Relation`; a `Model` receiver never matches, so `$model->update([...])`, `$model->password = …; $model->save()` and `Model::create([...])` are silent because of what they are, not because they are listed. `create` / `updateOrCreate` / `firstOrCreate` / `createOrFirst` are deliberately absent from the write-verb list even on a builder receiver — they instantiate and `save()` a model, so casts fire and flagging them would criminalize the remediation. The increment family IS on the list because `Query\Builder::incrementEach()` is literally `update(array_merge($columns, $extra))` — its extra payload is an ordinary uncast write.
+
+  **And for that family only, a MODEL receiver is in scope too.** `Model::increment()` is `protected`, but `Model::__call()` names all eight increment methods and forwards to them, and `Model::incrementOrDecrement()` casts the in-memory attribute through `forceFill($extra)` while handing the SAME `$extra`, uncast, to the query builder — the object ends up right and the row ends up plaintext. So "the model path is safe" holds per VERB, never structurally; `MODEL_BYPASSING_METHODS` is the whole of the exception and it is pinned by fixtures on a `Model` receiver.
+
+  **Payload slots carry a parameter NAME as well as a position.** A named argument does not sit at its parameter's index once an earlier optional one is skipped — `increment('votes', extra: [...])` puts the payload at index 1, not 2 — so a position-only reading is a false-negative generator. Reading the slot rather than refusing whenever any argument is named keeps `upsert($values, uniqueBy: [...])` covered. The names are asserted against Laravel's own signatures by a test, because a rename upstream would disable the named lookup in silence; that test is teeth-proved against both a renamed slot and a shifted position.
+
+  **Model resolution — generic first, per union branch, config only for raw tables.** An Eloquent `Builder<TModel>` or `Relation<TRelatedModel, …>` yields the model from its generic argument (measured on plain PHPStan: `Model::query()->where(…)` resolves to `Builder<App\Models\User>`), so the dominant idiom needs no configuration. A UNION receiver is read branch by branch rather than collapsed to its first member — `Builder<User>|Builder<AuditLog>` is two different cast maps, and letting one speak for both misses a credential on one branch while inventing one on the other. `DB::table('users')` resolves to a bare `Illuminate\Database\Query\Builder` carrying **no** model, so it is resolved only by walking the chain back to the `table('…')` string literal and looking it up in `credentialCastTableModels`. That map is **empty by default**, so raw-table writes are silent until a consumer opts in — inferring a model from a table name by inflection is exactly the false-positive source a credential-flavoured rule cannot afford, and queue #217 names the per-territory Level-1 arch test as the interim for what this rule cannot see.
+
+  **Cast maps are resolved the way PHP resolves them, not merged.** Neither declaration shape is reachable through reflection alone: `protected function casts(): array` needs a method body, and invoking it would mean instantiating an Eloquent model inside the analyser — so the rule injects PHPStan's own `@defaultAnalysisParser` (cached: a model file is parsed once per run), locates the class by resolved `namespacedName`, and reads `'column' => 'cast'` string pairs from source. What it then does with them mirrors `HasAttributes::initializeHasAttributes()`, which builds the effective map exactly once as `array_merge($this->casts, $this->casts())`:
+
+  - `$casts` is a **property** — exactly ONE declaration survives, the most derived, REPLACING an ancestor's default rather than merging with it, and a class-declared default replacing a trait-imported one.
+  - `casts()` is a **method** read by a SINGLE virtual dispatch — only the nearest body runs. An ancestor's or a trait's body contributes NOTHING unless the body that runs calls `parent::casts()` **and captures the result**; a bare `parent::casts();` statement changes nothing at runtime and must not extend the walk. Both composition forms carry the parent call (`array_merge(parent::casts(), [...])`, `[...parent::casts(), …]`), a bare `return parent::casts();` needs no literal of its own, and one assigned to a variable first counts too.
+
+  Which body runs is resolved by **reflection**, not by searching the source: `getNativeReflection()->getMethod('casts')` is the declaration PHP would dispatch, and its file and start line locate it exactly — through a trait, and through a trait ADAPTATION. A first-match walk over the imported traits gets `use A, B { B::casts insteadof A; }` wrong whenever the excluded trait is listed first. The property half does walk the declaration chain, and that IS PHP's answer there: adaptations are method-only, and two sources declaring `$casts` with different defaults is a fatal error rather than an ambiguity.
+
+  A body with several returns has no single static answer, so every branch is read and the union taken — a column some branch casts as a credential IS cast on that path. Where two branches disagree about the same column the **credential** cast wins, because source order is not a fact about which branch runs.
+
+  Because the merge puts `casts()` second, the method half beats the property half on a shared column, whatever order the two appear in the file. This is spelled out because the obvious reading — merge every declaration in the ancestry, leaf wins — is wrong, and measurably so: against PHP's own answer it is wrong on **nine of the twenty-three declaration shapes** in this rule's shape table, eight of them inventing a credential cast the model does not have and the ninth reporting a readable declaration as unreadable. Resolving the method half by first match over the imported traits instead — the obvious next reading — is wrong on two others, so the table keeps shapes refuting BOTH mistakes. Each was masked in ordinary fixtures by a key collision. Payload keys come from the resolved **constant array type** rather than the AST literal, so `$p = ['password' => …]; $q->update($p);` is caught and a dynamic payload is silent.
+
+  **Three failure modes report under their own identifiers**, because MISSING, FAILED and MISCONFIGURED must not arrive as the same (silent) outcome, and each has a different remediation: `…modelSourceUnreadable` (a declaring source whose PHP cannot be located or parsed — fix the source), `…castMapIncomplete` (the source was read but a declaration carries no array literal at all, `return self::CASTS;` — restate the credential columns literally), `…configuredModelMissing` (`credentialCastTableModels` maps a table to a class that does not exist — fix the parameter; reachable only from the config map). All three fire regardless of the payload: with an incomplete map the rule cannot claim the payload is clean, and treating any of them as "declares no casts" would fail OPEN on exactly the models the rule exists to guard.
+
+  **Accepted false NEGATIVES, documented and pinned** — nothing is parked there to excuse a false positive: class-based casts (`AsEncryptedArrayObject::class` and friends appear as `::class` constant fetches, not the string values matched here); dynamic payloads and computed keys; `upsert()`'s third argument (an update-COLUMN list whose names are values, and every column named there already appears in the row payload that is read); a `DB::table('…')` builder hoisted into a variable (the variable's type carries no table name — not resolvable in principle); `Model::where(…)->update([...])` static-magic entry on plain PHPStan, where `__callStatic` is untypeable (consumers running larastan get `Builder<TModel>` there and the rule fires normally); raw SQL, which has no payload array; a composition mixing a readable contributor with a dynamic one; and casts added at RUNTIME via `mergeCasts()` / `withCasts()`. The last is documented rather than diagnosed on measured grounds: across the war-room fleet `mergeCasts()` appears in application code exactly once, inside a copy-pasted `newInstance()` override propagating a map the rule already reads, and `withCasts()` once, on a non-credential column — a diagnostic keyed on those calls has no true positive to find today and one false positive to produce.
+
+  **Teeth.** The shape table (`tests/Fixtures/CredentialCastBypass/CastDispatchShapes.php`) computes its expectation from **PHP itself** — the property default PHP resolved, merged under a real virtual dispatch of `casts()` — rather than from anyone's reading of Laravel, and asserts the two readings still disagree on enough rows to be measuring something. Against the merge-everything implementation it reds on seven spurious errors. Mutation controls: dropping a write verb, and collapsing the union receiver to its first branch, each red exactly the assertion that pins it. Flipping `'password' => 'hashed'` to `'string'` drops the `password` findings and leaves every other cast firing; injecting `'password'` into a clean-fixture write turns the green assertion red at that line. Denominator tests assert each fixture still carries its write sites and that the shape fixture and the shape table have not drifted apart, so no zero-expectation assertion can pass on an empty or unparsed file. `extension.neon` wiring was smoked through the real `phpstan analyse` entry point, not only `RuleTestCase`.
+
+  **Versioning: candidate MAJOR** — it surfaces new errors in code that previously passed wherever a consumer writes a credential column through a builder. Per the pre-1.0 caret convention `^0.8` excludes the next minor, so tagging auto-adopts nobody; each consumer adopts on its own pin-bump PR. **NOT tagged** (release is a General/Commander step).
+
 - `ForbidUntimedHttpClientRule` — new rule enforcing war-room **Architectural Principle #8** (explicit timeouts on outbound HTTP) at analysis time, the AST-aware successor to the per-territory `ExternalHttpTimeoutTest` named-list Pest tests (kendo / emmie). The named-list tests detect wrong-shape on *enrolled* classes but are blind to **omission** — a new untimed call nobody adds to the list; this rule closes the omission gap for the tractable call shapes. Registered in `extension.neon` (no parameters). Identifier: `forbidUntimedHttpClient.missingTimeout`. Doctrine: war-room §Architectural Principles #8. Seed: war-room enforcement queue #58 (spike branch `spike/wr-queue58-untimed-http-client`). **Review follow-up (bus #57 findings):** the `withOptions()` check is TYPE-aware, not AST-literal — a constant array type provably lacking `'timeout'` still fires (including through a variable holding a literal array, a widening over the inline-`Array_`-only first cut), while a computed/helper-built options expression (not a constant array type) is POSSIBLY timed and the chain DECLINES (the Major: flagging it was a false positive). A chain member outside the known `PendingRequest` builder surface — a Macroable extension (`Http::github()`, root or intermediate) or `when()`/`unless()` with their opaque closures — likewise declines (the Minor: a macro may return a pre-timed request); a genuine builder missing from the list costs only a false negative.
 
   **Detection (type-anchored, two entry points):** fires on a terminal send verb (`get`/`post`/`put`/`patch`/`delete`/`head`/`send`) reached without an explicit request timeout, where the entry point is either (1) the `Http` facade (`Illuminate\Support\Facades\Http` static-call root), or (2) an **injected `Illuminate\Http\Client\Factory`** receiver (`$this->http->…->get()`, anchored by TYPE so the property alias is irrelevant — the dominant fleet idiom, established by field survey of kendo/emmie/ublgenie/BIO). A timeout counts when the visible chain contains `->timeout(...)` **or** `->withOptions([... 'timeout' => ...])`; `connectTimeout()` alone does NOT (it bounds the handshake, not the response).