diff --git a/CHANGELOG.md b/CHANGELOG.md index dc12edd..80fd55f 100644 --- a/CHANGELOG.md +++ b/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` or `Relation` yields the model from its generic argument (measured on plain PHPStan: `Model::query()->where(…)` resolves to `Builder`), so the dominant idiom needs no configuration. A UNION receiver is read branch by branch rather than collapsed to its first member — `Builder|Builder` 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` 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). diff --git a/CLAUDE.md b/CLAUDE.md index bf61980..2af98a3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -38,6 +38,7 @@ Composer package distributing war-room-doctrine PHPStan rules across `script-dev | `ForbidUntimedHttpClientRule` | War-room §Explicit HTTP timeouts (#8) | `forbidUntimedHttpClient.missingTimeout` (type-aware; flags an `Http` facade OR injected `Illuminate\Http\Client\Factory` chain that reaches a send verb with no explicit `->timeout()` / `withOptions(['timeout'])`. Conservative — fires only on fully-visible single-expression chains; declines split/helper-built chains + Guzzle/SDK surfaces to hold FP at zero. COMPLEMENTS, does not replace, the per-territory `ExternalHttpTimeoutTest`. on `main`, `[Unreleased]`) | | `EnforceAuditModelProtectionsRule` | ADR-0001 §Append-only | `enforceAuditModelProtections.hasFactoryForbidden` / `.softDeletesForbidden` / `.updatedAtNotDisabled` (denylist-inversion; discovers audit models by shape — `auditModelNameSuffixes` default `AuditLog` OR `auditModelNamespacePrefixes` default `App\Models\Audit` — and flags `HasFactory` / `SoftDeletes` / missing `const UPDATED_AT = null`. shipped v0.7.0) | | `EnforceActionResultDtoRule` | ADR-0020 + ADR-0011 | `enforceActionResultDto.arrayReturnFromExecute` (signature-only; flags an `array` / `?array` / `array\|Dto` union / `iterable` native return type on `App\Actions\*` `execute()`. Phpdoc-only `@return array{...}` is a deliberate miss; no `list` carve-out. Seed kendo PR #1653. shipped v0.8.0) | +| `ForbidCredentialCastBypassRule` | War-room §Explicit over implicit (#1) + §Rotation-invariant credential handling (#10) | `forbidCredentialCastBypass.castBypassedByBuilderWrite` / `.modelSourceUnreadable` / `.castMapIncomplete` / `.configuredModelMissing` (flags a `hashed` / `encrypted` / `encrypted:*` cast column appearing as a key in a QUERY-BUILDER write payload — `update`/`insert`/`insertOrIgnore`/`insertGetId`/`upsert`/`updateOrInsert` plus the increment family (`increment`/`decrement`/`incrementEach`/`decrementEach`, whose EXTRA payload reaches SQL via `update(array_merge($columns, $extra))`) on an Eloquent `Builder`, query `Builder` or `Relation`. Casts fire on the model path only, so a builder write stores the credential in plaintext with a green suite. The model path and the model-routing builder verbs (`create`/`updateOrCreate`/`firstOrCreate`/`createOrFirst`) are silent STRUCTURALLY — the receiver type gate, not an exemption list. Model comes from the builder/relation generic, read PER UNION BRANCH; `DB::table('…')` carries none and resolves only via the opt-in `credentialCastTableModels` map, default `[]`. Cast maps read from model SOURCE via the injected `@defaultAnalysisParser` — both `casts()` and `$casts` — then resolved as PHP resolves them, NOT merged across the ancestry: `$casts` is a property so ONE declaration survives (most derived, replacing), `casts()` is a SINGLE virtual dispatch so only the nearest body runs and an ancestor contributes only through an explicit `parent::casts()` (carried by `array_merge(parent::casts(), …)`, a spread, or a bare pass-through). Laravel's `array_merge($this->casts, $this->casts())` makes the METHOD half win on a shared column regardless of source order. ⚠ The merge-everything reading is wrong on 9 of the 23 shapes in `CastDispatchShapes.php`, 8 of them FALSE POSITIVES inventing a cast the model does not have; a first-match trait walk is wrong on 2 others (`insteadof`, discarded `parent::casts()`). The shape test computes its expectation from PHP itself. THREE fail-open shapes are each reported under their own identifier rather than reading as castless: `.modelSourceUnreadable` (source cannot be located/parsed), `.castMapIncomplete` (read, but a declaration carries no array literal at all — `return self::CASTS;`), `.configuredModelMissing` (`credentialCastTableModels` names a nonexistent class). Payload keys read from the CONSTANT ARRAY TYPE, so a hoisted variable is caught. Seed lokalekeuze PR #65. on `main`, `[Unreleased]`) | | `ConnectionTransactionReturnTypeExtension` | (type extension, no rule) | — (resolves `ConnectionInterface::transaction()` to the closure's return type. **Load-bearing on `illuminate/* ^12` only** — it annotates `@return mixed`; Laravel 13 annotates `@template TReturn`/`@return TReturn` and infers the same type unaided. Its teeth are measured by the `check-lowest-laravel` CI job, not by the unit suite on a Laravel-13 tree. WR-0855, WR-0860) | 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. `EnforceFormRequestToDtoRule` (ADR-0012) is the third Phase 2 addition, promoted from entreezuil's `tests/Arch/FormRequestsTest.php` under the same queue #55 (instance 2). `EnforceExplicitHydrationRule` (ADR-0019) is the next Phase 2 candidate. @@ -135,6 +136,7 @@ SemVer per ADR-0021: ### War-room Architectural Principle rules (no published ADR) - **Explicit over implicit** — package distributes `ForbidAbortHelperRule` (bans `abort()` / `abort_if()` / `abort_unless()`; shipped), `EnforceCurrentUserAttributeRule` (flags `Request::user()` / `Auth::user()` / `auth()->user()` in `App\Http\Controllers`, steering to the `#[CurrentUser]` container attribute per Architectural Principle #9; shipped v0.4.0), `ForbidHttpExceptionInActionsRule` (type-aware sibling of `ForbidAbortHelperRule` — bans throwing the `Symfony\…\HttpException` family from `App\Actions\*`; HTTP status concerns belong to the HTTP layer per Principles #1 + #3; `ValidationException` deliberately out of scope; shipped v0.5.0), `ForbidResourceWrappedInJsonResponseRule` (bans wrapping a `JsonResource` in `response()->json()` / `new JsonResponse()` inside controllers per Principle #1 + ADR-0009; shipped v0.5.0), and `ForbidRawExceptionMessageInResponseRule` (bans a raw `Throwable::getMessage()` — or the `Throwable` itself — reaching a client-facing response sink per Principle #1 + information-disclosure hardening for the ISO 27001 / AVG / NEN 7510 consumers; default sink `Laravel\Mcp\Response::error`, configurable via `rawExceptionMessageSinks`; server-side logging never flags; `// @leak-safe:` exemption; shipped v0.8.0). These enforce war-room §Architectural Principles (some also touching numbered ADRs) — each rule's docblock "Doctrine source" line names its authority. +- **Rotation-invariant credential handling (#10) + Explicit over implicit (#1)** — package distributes `ForbidCredentialCastBypassRule` (flags a `hashed` / `encrypted` / `encrypted:*` cast column named as a key in a query-builder write payload, where the cast never fires and the raw credential reaches SQL; the model path is structurally silent because a `Model` receiver never matches the builder/relation type gate. `DB::table()` resolution is opt-in via `credentialCastTableModels` — a model is never inferred from a table name. Cast resolution mirrors PHP's own member resolution (one surviving `$casts` property declaration; a single virtual dispatch for `casts()`, walking upward only through `parent::casts()`), because merging every declaration in the ancestry invents casts that do not exist at runtime. ISO 27001 A.5.33 / AVG relevance on the compliance consumers. Seed lokalekeuze PR #65, war-room enforcement queue #217; on `main`, `[Unreleased]`). - **Explicit HTTP timeouts (#8)** — package distributes `ForbidUntimedHttpClientRule` (flags an `Http` facade / injected `Illuminate\Http\Client\Factory` chain reaching a send verb without an explicit request timeout, per Architectural Principle #8; the AST-aware, omission-closing successor to the per-territory `ExternalHttpTimeoutTest` named-list Pest tests — conservative single-expression detection, declines split/helper-built chains + Guzzle/SDK surfaces; COMPLEMENTS the named-lists rather than replacing them; on `main`, `[Unreleased]`). Seed: war-room enforcement queue #58. ### War-room internal ADRs diff --git a/README.md b/README.md index ea09b80..2cfd52a 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,7 @@ includes: | `EnforceCurrentUserAttributeRule` | `enforceCurrentUserAttribute.useAttributeInsteadOfRequestUser` | `Request::user()` / `Auth::user()` / `auth()->user()` calls inside `App\Http\Controllers\*` classes (namespace prefix, incl. sub-namespaces; configurable via `controllerNamespacePrefixes`) | Use `#[\Illuminate\Container\Attributes\CurrentUser] User $user` on the method parameter. Scope is decided by namespace, not class ancestry — a base-less `final` controller in `App\Http\Controllers` fires; FormRequests (`App\Http\Requests`), middleware (`App\Http\Middleware`), services, Actions (`App\Actions`), jobs, and console commands are silent because their namespaces do not start with the controller prefix (container-attribute injection does not apply to FormRequest methods regardless). | | `EnforceCurrentUserAttributeRule` | `enforceCurrentUserAttribute.useAttributeInsteadOfRequestUser` | `Request::user()` / `Auth::user()` / `auth()->user()` calls inside `App\Http\Controllers\*` classes (namespace prefix, incl. sub-namespaces) | Use `#[\Illuminate\Container\Attributes\CurrentUser] User $user` on the method parameter. Scope is decided by namespace, not class ancestry — a base-less `final` controller in `App\Http\Controllers` fires; FormRequests (`App\Http\Requests`), middleware (`App\Http\Middleware`), services, Actions (`App\Actions`), jobs, and console commands are silent because their namespaces do not start with the controller prefix (container-attribute injection does not apply to FormRequest methods regardless). | | `EnforceAuditModelProtectionsRule` | `enforceAuditModelProtections.hasFactoryForbidden` / `.softDeletesForbidden` / `.updatedAtNotDisabled` | Eloquent models recognised as audit records by SHAPE — short name ends with a configured suffix (default `AuditLog`) OR FQCN sits under a configured namespace (default `App\Models\Audit`) | Three append-only protections, each firing independently: using `HasFactory` (a factory is a direct-insert path bypassing the hash-chained writer), using `SoftDeletes` (audit rows are never removed), or not disabling `updated_at` (an audit row is written once and never mutated — declare `public const UPDATED_AT = null;`) is an error. Discovery is by pattern, never a hand-maintained class list — a denylist inversion, so a newly-added audit model cannot escape the protections by omission. Abstract intermediates are exempt (their concrete leaves carry inherited violations). Non-model classes named `*AuditLog` are excluded by the Eloquent `Model` type gate. Doctrine: ADR-0001 §Append-only. | +| `ForbidCredentialCastBypassRule` | `forbidCredentialCastBypass.castBypassedByBuilderWrite` | Write calls (`update`, `insert`, `insertOrIgnore`, `insertGetId`, `upsert`, `updateOrInsert`, `updateFrom`, `insertOrIgnoreReturning`, `incrementOrCreate`, and the increment family loud and quiet) whose receiver is an `Illuminate\Database\Eloquent\Builder`, `Illuminate\Database\Query\Builder`, or `Illuminate\Database\Eloquent\Relations\Relation` | Naming a column that carries a `hashed`, `encrypted` or `encrypted:*` cast as a key in the write payload is an error. Casts fire on the MODEL path only; a builder write delegates to `toBase()->update()` and ships the raw value to SQL — no hash, no encryption, no exception, and a green test suite. The model path is **structurally** silent (a `Model` receiver never matches), and so are the builder methods that route through a model (`create`, `updateOrCreate`, `firstOrCreate`, `createOrFirst`) — they are the remediation. The increment family is included because `Query\Builder::incrementEach()` is literally `update(array_merge($columns, $extra))` — its extra payload is an ordinary uncast write — and for that family a **`Model` receiver is in scope too**: `Model::__call()` re-exposes the protected increment methods, and `Model::incrementOrDecrement()` casts the in-memory attribute via `forceFill($extra)` while passing the same `$extra` uncast to the query builder. Payload arguments are addressed by parameter NAME as well as position, since `increment('votes', extra: [...])` puts the payload at index 1 rather than 2. The model comes from the builder's/relation's generic type argument, read per UNION branch so `Builder|Builder` is checked against both cast maps; a `DB::table('…')` chain carries none and resolves only through the opt-in `credentialCastTableModels` map (empty by default ⇒ silent, never inferred from the table name). The cast map is read from the model SOURCE — both the `casts()` method and a `$casts` property — and then resolved the way PHP resolves them rather than merged: `$casts` is a property, so ONE declaration survives (most derived, replacing not merging), while `casts()` is a single virtual dispatch, so only the nearest body runs and an ancestor contributes only through an explicit `parent::casts()`. Laravel merges the two as `array_merge($this->casts, $this->casts())`, so the method half wins on a shared column whatever order the file declares them in. Three fail-open shapes each get their OWN identifier rather than reading as "declares no casts" — `forbidCredentialCastBypass.modelSourceUnreadable` (source cannot be located or parsed), `.castMapIncomplete` (source read, but a declaration carries no array literal at all — `return self::CASTS;`), and `.configuredModelMissing` (`credentialCastTableModels` names a class that does not exist). Payload keys are read from the resolved **constant array type**, so a payload hoisted into a variable is caught and a dynamic payload is silent. Deliberate misses: class-based casts (`AsEncryptedArrayObject::class`), dynamic keys, `upsert()`'s third argument, `Model::where(...)` static-magic entry without larastan, and a `DB::table()` builder hoisted into a variable (the variable's type carries no table name). Doctrine: war-room §Architectural Principles #1 + #10; ISO 27001 A.5.33 / AVG. Seed: lokalekeuze PR #65. | ### `EnforceActionTransactionsRule` — write-method list @@ -230,6 +231,114 @@ parameters: path: app/Models/Audit/SomeProjectionLog.php ``` +### `ForbidCredentialCastBypassRule` — resolving `DB::table()` writes + +A builder or relation write carries its model in its type — `Voucher::query()->…` +is a `Builder`, `$user->apiKeys()->…` is a `HasMany` — so the +rule finds the cast map on its own and needs no configuration. + +`DB::table('users')->update([...])` carries no model at all. The rule will not +guess one from the table name (an inflection guess is exactly the false-positive +source a credential-flavoured rule cannot afford), so raw-table writes are +**silent by default**. Opt a table in by mapping it: + +```neon +parameters: + credentialCastTableModels: + users: 'App\Models\User' + api_keys: 'App\Models\ApiKey' +``` + +Only tables whose model declares a `hashed` / `encrypted` / `encrypted:*` cast +are worth listing; a mapped table whose model has no credential cast changes +nothing. Single backslashes in the FQCN — NEON only unescapes `\\` inside +double quotes. + +Chain forms all resolve, including `DB::connection('mysql')->table('users')` and +any number of intermediate hops (`->where(...)->limit(...)`). What does **not** +resolve is a builder hoisted into a variable: + +```php +$query = DB::table('users'); +$query->update(['password' => $plain]); // silent — see below +``` + +The walk needs the `table('…')` string literal, and the variable's type is a bare +`Illuminate\Database\Query\Builder` carrying no table name, so there is nothing +left to read. This is a limitation of the query builder's type rather than of the +walk, and it is a false negative, never a false positive. + +### `ForbidCredentialCastBypassRule` — how the cast map is resolved + +Laravel builds a model's effective cast map exactly once, in +`HasAttributes::initializeHasAttributes()`: + +```php +$this->casts = array_merge($this->casts, $this->casts()); +``` + +Two halves, two different PHP rules, and the difference decides whether a write +is flagged: + +| Shape | Effective at runtime | Rule | +|---|---|---| +| `casts()` on the model, or inherited with no override | the declared map | flagged | +| `casts()` override that does **not** call `parent::casts()` | ONLY the override's map — the ancestor's body never runs | not flagged | +| `casts()` override calling `parent::casts()` (directly, via `array_merge`, or via a spread) | both, nearer wins | flagged | +| `$casts` property, own or inherited | the declared map | flagged | +| `$casts` property redeclared in a child | ONLY the child's — a property redeclaration replaces | not flagged | +| both `$casts` and `casts()` on one class | the METHOD wins, whatever the source order | per the method | +| trait `casts()` with a class-declared `casts()` too | ONLY the class's — the trait's body never runs | not flagged | +| trait `casts()` or `$casts` with no class declaration | the trait's map | flagged | + +The practical consequence: **cutting the `casts()` chain removes a cast.** A +subclass that overrides `casts()` without calling its parent does not inherit the +parent's `hashed` column — at runtime *or* here — so a builder write to that +column is not a cast bypass, and this rule will not claim it is. If you meant to +keep the parent's casts, compose them: + +```php +protected function casts(): array +{ + return array_merge(parent::casts(), ['api_token' => 'encrypted']); +} +``` + +### `ForbidCredentialCastBypassRule` — when the cast map cannot be read in full + +Three different things can stop the rule from reading a complete cast map, and +each reports under its own identifier — MISSING, FAILED and MISCONFIGURED must +not arrive as the same silent outcome, and the remediation differs: + +| Identifier | Cause | Fix | +|---|---|---| +| `forbidCredentialCastBypass.modelSourceUnreadable` | a declaring class or trait's PHP cannot be located or parsed | fix the source | +| `forbidCredentialCastBypass.castMapIncomplete` | the source was read, but a `casts()` return or a `$casts` default carries no array literal at all (`return self::CASTS;`, `return $this->buildCasts();`) | restate the credential columns as literal string pairs | +| `forbidCredentialCastBypass.configuredModelMissing` | `credentialCastTableModels` maps a table to a class that does not exist (a typo, or a stale rename) | fix the FQCN, or drop the mapping | + +All three are deliberately independent of the payload: with an incomplete map, a +credential column in that payload would go unreported. Suppress an identifier +alone (per file or per line) if a write is known safe; the real check stays armed. + +**Composed and pass-through cast maps are read, not reported.** +`return array_merge(parent::casts(), ['password' => 'hashed']);`, +`return [...parent::casts(), 'password' => 'hashed'];` and a bare +`return parent::casts();` all resolve: the first two contribute their literal and +the parent call continues the chain walk, and the third needs no literal of its +own. None triggers `castMapIncomplete`. A composition mixing a readable +contributor with a *dynamic* one (`array_merge($this->dynamicCasts(), [...])`, +`array_merge(parent::casts(), self::EXTRA)`) reads the readable half and stays +silent about the rest — flagging it would mean flagging every model that composes +at all. + +**Casts added at runtime are invisible.** `mergeCasts()` and +`withCasts()` declare nothing for a static analyser to read, so a column cast +only that way is a false negative. This is documented rather than diagnosed on +measured grounds: across the war-room fleet `mergeCasts()` appears in application +code once, inside a copy-pasted `newInstance()` override propagating a map the +rule already reads, and `withCasts()` once, on a non-credential column — so a +diagnostic keyed on those calls would produce a false positive and catch nothing. + ### `ForbidRawExceptionMessageInResponseRule` — configurable sinks + `@leak-safe` exemption The rule flags a raw `Throwable::getMessage()` (or the `Throwable` itself) reaching a **client-facing response sink** — an information-disclosure leak. The built-in default sink `Laravel\Mcp\Response::error` is always armed; a consumer adds more (a persist-error setter, a `MarkInvoiceFailed` Action) via the `rawExceptionMessageSinks` parameter — a list of `FQCN::method` signatures, default `[]`: diff --git a/extension.neon b/extension.neon index 9f94230..dc3fa87 100644 --- a/extension.neon +++ b/extension.neon @@ -67,6 +67,16 @@ parameters: rawExceptionMessageSinks: [] safeMessageExceptionClasses: [] + # `ForbidCredentialCastBypassRule`: map of raw TABLE NAME to model FQCN, + # used ONLY to resolve a `DB::table('…')->update([...])` chain, which + # carries no model in its type the way `Model::query()` does. Empty by + # default, so `DB::table()` writes are silent until a consumer opts in — + # inferring a model from a table name would be a false-positive source this + # rule cannot afford. Builder and relation writes need no entry here; their + # model comes out of the generic type argument. Each FQCN uses single + # backslashes — see the NEON-quoting note above. + credentialCastTableModels: [] + parametersSchema: resourceDataBaseClass: string() formRequestBaseClass: string() @@ -76,6 +86,7 @@ parametersSchema: auditModelNameSuffixes: listOf(string()) rawExceptionMessageSinks: listOf(string()) safeMessageExceptionClasses: listOf(string()) + credentialCastTableModels: arrayOf(string()) services: - @@ -151,6 +162,12 @@ services: rawExceptionMessageSinks: %rawExceptionMessageSinks% safeMessageExceptionClasses: %safeMessageExceptionClasses% tags: [phpstan.rules.rule] + - + class: ScriptDevelopment\PhpstanWarroomRules\Rules\ForbidCredentialCastBypassRule + arguments: + parser: @defaultAnalysisParser + credentialCastTableModels: %credentialCastTableModels% + tags: [phpstan.rules.rule] - class: ScriptDevelopment\PhpstanWarroomRules\Type\ConnectionTransactionReturnTypeExtension tags: [phpstan.broker.dynamicMethodReturnTypeExtension] diff --git a/src/Rules/ForbidCredentialCastBypassRule.php b/src/Rules/ForbidCredentialCastBypassRule.php new file mode 100644 index 0000000..84e97ee --- /dev/null +++ b/src/Rules/ForbidCredentialCastBypassRule.php @@ -0,0 +1,1558 @@ +…->update([...])`, + * `->insert(...)`, `->upsert(...)`, a relation-derived builder write, or a + * `DB::table('…')` write on a table mapped to a model. + * + * Doctrine source: war-room §Architectural Principles #1 (Explicit over + * implicit) + #10 (credential handling / rotation-invariance); ISO 27001 A.5.33 + * and AVG on the compliance territories. + * + * **Why this is a security rule, not a style rule.** Attribute casts fire on + * the MODEL path only — `setAttribute()` runs the `hashed` / `encrypted` cast + * when you assign `$model->password = $plain` and `save()`. A query-builder + * write skips the model entirely: `Builder::update()` delegates to + * `toBase()->update()`, which ships the array straight to SQL. The value lands + * in the column verbatim — no hash, no encryption, no exception, and a green + * test suite, because a test that reads the column back gets exactly what it + * wrote. The failure is silent at every layer and is only ever discovered by + * reading the database. Seed: lokalekeuze PR #65, where `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. + * + * Detection has three independent halves, and the rule stays SILENT unless all + * three resolve — a credential-flavoured false positive spends the gate's + * authority faster than almost any other kind (ADR-0021 posture). Read that as + * the DISPOSITION it is, not as a guarantee: it says what the rule does when it + * cannot resolve something, not that everything it does resolve is right. The + * false positives this rule has actually shipped were all cases where it + * resolved something confidently and wrongly, and each is now a fixture: + * + * 1. **Write verb + payload.** The call is one of the verbs in + * `WRITE_METHODS` — `update`, `insert`, `insertOrIgnore`, `insertGetId`, + * `upsert`, `updateOrInsert`, the Postgres-only `updateFrom` and + * `insertOrIgnoreReturning`, `incrementOrCreate`, and the increment family + * loud and quiet, whose extra payload reaches SQL through + * `update(array_merge($columns, $extra))` — and the payload argument + * resolves to a CONSTANT array type, so its keys are statically known. + * Each payload slot carries a parameter NAME as well as a position, + * because a named argument does not sit at its parameter's index once an + * earlier optional one is skipped (`increment('votes', extra: […])`); the + * names are checked against Laravel's own signatures by a test, since a + * rename upstream would otherwise disable the named lookup in silence. A payload built dynamically (`$data` of unknown + * shape, computed keys) is not a constant array type and is silent. + * Because the check is TYPE-based rather than AST-literal, a payload + * hoisted into a variable (`$p = ['password' => …]; $q->update($p);`) is + * still caught. A list-of-rows payload (`insert([[…], […]])`, `upsert`) + * is recognised by shape — all-integer keys over constant-array values — + * and each row's keys are collected. + * + * Deliberately ABSENT from the verb list: `create`, `updateOrCreate`, + * `firstOrCreate`, `createOrFirst`. Those are Eloquent-builder methods + * that instantiate a model and `save()` it, so casts DO fire — flagging + * them would criminalize the remediation. + * + * 2. **Receiver is a builder — and, for eight verbs, a model too.** The + * receiver type must be a subtype of + * `Illuminate\Database\Eloquent\Builder`, + * `Illuminate\Database\Query\Builder`, or + * `Illuminate\Database\Eloquent\Relations\Relation`. A `Model` receiver is + * excluded for every OTHER verb, which is what keeps the model path + * silent: `$model->update([...])` routes through `fill()` → + * `setAttribute()` and casts fire, so it is correct and must not fire. + * + * **The exception is the increment family** (`MODEL_BYPASSING_METHODS`), + * and it is not a carve-out for convenience — `Model::increment()` is + * `protected`, but `Model::__call()` names all eight 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" is true per VERB, never + * structurally, and this list is the whole of the exception. + * + * 3. **Model resolution.** From an Eloquent `Builder` or a + * `Relation` the model comes out of the GENERIC + * argument — the first type argument that is a `Model` subtype, per UNION + * BRANCH: `Builder|Builder` is two cast maps, and letting + * one branch speak for both is wrong in both directions. For a + * relation the first template parameter is `TRelatedModel`, which is the + * model whose table the write targets, so the same "first Model-subtype + * argument" reading is correct for both. A bare + * `Illuminate\Database\Query\Builder` (what `DB::table('x')` yields) + * carries NO model in its type, so it is resolved by walking the fluent + * chain back to the `table('…')` call with a string-literal argument and + * looking that table name up in the configured + * `credentialCastTableModels` map. That map is EMPTY by default, so + * `DB::table()` writes are silent until a consumer opts in — guessing a + * model from a table name by inflection would be exactly the + * false-positive source this rule cannot afford. + * + * **Reading the casts.** The model's cast map is read from SOURCE, because + * neither shape is reachable through reflection alone: the modern + * `protected function casts(): array` form needs a method body, and invoking it + * would mean instantiating an Eloquent model inside the analyser. The rule + * injects PHPStan's own analysis parser (`@defaultAnalysisParser` — cached, so + * a model file is parsed once per run), parses the file + * `ClassReflection::getFileName()` names, locates the class by resolved + * `namespacedName`, and collects `'column' => 'cast'` string pairs. + * + * **What it does with them is PHP's own resolution, not a merge.** Laravel + * builds the effective map once — `array_merge($this->casts, $this->casts())` + * in `HasAttributes::initializeHasAttributes()` — and the two halves resolve + * differently: + * + * - `$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, so it must not + * extend the walk either. + * + * The method half therefore beats the property half on a shared column, + * whatever order the two appear in the file. + * + * **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, and an `as` alias the same way. 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 branches disagree about the same column the CREDENTIAL + * cast wins, because source order is not a fact about which branch runs. + * + * Why this is spelled out at this length: merging every declaration in the + * ancestry and letting the leaf win reads plausible and is wrong on NINE of the + * twenty-three shapes in `CastDispatchShapes.php` — eight inventing a credential + * cast the model does not have, the ninth calling a readable declaration + * unreadable. Resolving the method half by first match over the imported traits + * instead is wrong on two OTHERS, which is the point of keeping shapes for both + * mistakes: a table that only refutes the reading you have already abandoned + * measures nothing. The test beside that fixture computes its expectation from + * PHP itself rather than from anyone's reading of Laravel. + * + * A cast map composed rather than returned literally is READ, not missed: + * `return array_merge(parent::casts(), ['password' => 'hashed']);` and + * `return [...parent::casts(), 'password' => 'hashed'];` both contribute their + * literal AND capture the parent's map, so the chain walk continues upward and + * the merged map is complete. A bare `return parent::casts();` carries no + * literal at all and needs none, for the same reason, and one assigned to a + * variable first (`$c = parent::casts(); return array_merge($c, […]);`) counts + * too — what does NOT count is a call whose result is discarded. Array literals are collected from anywhere inside a returned + * expression — but never from inside an already-collected array, so a + * nested-array cast value stays a value rather than becoming a second cast map. + * + * Three failure modes are each reported under their OWN identifier, 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 `casts()` return or + * a `$casts` default carries no array literal at all (`return self::CASTS;`, + * `return $this->buildCasts();`). 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: an FQCN taken from a resolved generic type always exists. + * + * All three are reported REGARDLESS of the payload, because with an incomplete + * map the rule cannot claim the payload is clean. Treating any of them as + * "declares no casts" would fail OPEN on exactly the models this rule exists to + * guard, and would make MISSING indistinguishable from FAILED. + * + * A cast counts as credential-bearing when its value is exactly `hashed`, + * exactly `encrypted`, or begins with `encrypted:` (`encrypted:array`, + * `encrypted:collection`, `encrypted:object`). + * + * Suppression: standard PHPStan inline-ignore mechanism on the rule's + * identifier `forbidCredentialCastBypass.castBypassedByBuilderWrite`. + * + * Out of scope. Every entry is an accepted false NEGATIVE — a write this rule + * knowingly stays silent on. Nothing may be parked here to excuse a false + * POSITIVE: a rule that invents a credential cast blocks a correct write, and on + * a security rule that spends the gate's authority faster than a missed catch. + * Three shapes that once lived here as "documented limits" were false positives + * and were fixed instead. + * + * - **Class-based encrypted casts** — `AsEncryptedArrayObject::class`, + * `AsEncryptedCollection::class` and friends appear as `::class` constant + * fetches rather than the string values this rule matches. They carry the + * same bypass risk; a consumer needing them covered restates the column in + * string form or relies on the per-territory arch test. + * - **Dynamic payloads and dynamic keys** — not a constant array type, so + * the keys are not statically known. + * - **`upsert()`'s third argument** (the update-column list) — its column + * names are VALUES, not keys, and every column named there must already + * appear in the row payload this rule does read. + * - **A `DB::table('…')` builder hoisted into a variable** — `$q = + * DB::table('users'); $q->update([...]);`. The chain walk needs the + * `table('…')` literal, and the variable's TYPE is a bare + * `Illuminate\Database\Query\Builder` that carries no table name, so once + * the builder is behind a variable there is nothing left to resolve. Chain + * forms ARE covered, including `DB::connection('…')->table('…')` and any + * number of intermediate hops. This is a limitation of the query builder's + * type, not of the walk. + * - **Static-magic builder entry** (`Model::where(...)->update([...])` + * without larastan) — plain PHPStan cannot type `Model::__callStatic`, so + * the receiver resolves to an error type and the rule declines. Consumers + * running larastan get `Builder` there and the rule fires normally; + * `Model::query()->…` resolves on plain PHPStan either way. + * - **Raw SQL** (`DB::update('update users set …')`) — no payload array. + * - **A composition mixing a readable contributor with a dynamic one** — + * `return array_merge($this->dynamicCasts(), ['password' => 'hashed']);`, or + * `array_merge(parent::casts(), self::EXTRA_CASTS)`. The literal and the + * parent chain ARE read, so the map is not reported as incomplete, and + * whatever the dynamic half contributes stays invisible. Reporting here + * would mean flagging every model that composes at all, including the ones + * read in full; the readable half being covered is the honest limit. + * - **Casts added at RUNTIME through `mergeCasts()` / `withCasts()`** — no + * declaration exists to read, so a column cast only that way is invisible. + * 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 this rule already + * reads, and `withCasts()` once, on a non-credential column and query-time + * only. A diagnostic keyed on those calls would have no true positive to + * find today and one false positive to produce. + * + * @implements Rule + */ +final class ForbidCredentialCastBypassRule implements Rule +{ + private const string IDENTIFIER = 'forbidCredentialCastBypass.castBypassedByBuilderWrite'; + + /** + * Reported when a declaring source could not be read, so the cast map is + * INCOMPLETE and the rule cannot vouch for the payload. A distinct + * identifier because MISSING and FAILED must not arrive as the same + * (silent) outcome — a consumer can suppress this one alone without + * disarming the real check. + */ + private const string UNREADABLE_IDENTIFIER = 'forbidCredentialCastBypass.modelSourceUnreadable'; + + /** + * Reported when a declaring source WAS read but one of its cast + * declarations could not be interpreted — a `casts()` return or a `$casts` + * default that contributes no array literal at all (`return self::CASTS;`, + * `return $this->buildCasts();`). Distinct from UNREADABLE_IDENTIFIER + * because the remediation is different: the file is fine, the declaration + * shape is what this rule cannot read, and restating the credential columns + * in literal form fixes it. Composition forms that DO carry a literal + * (`array_merge(parent::casts(), [...])`, `[...parent::casts(), ...]`) are + * read, not reported. + */ + private const string INCOMPLETE_IDENTIFIER = 'forbidCredentialCastBypass.castMapIncomplete'; + + /** + * Reported when `credentialCastTableModels` maps a table to a class that + * does not exist. A typo or a stale rename would otherwise be + * indistinguishable from "this table is not mapped", which silently and + * permanently disarms the rule for every write on that table — the same + * fail-open shape UNREADABLE_IDENTIFIER exists to prevent, arriving through + * the configuration instead of the source. + */ + private const string CONFIG_IDENTIFIER = 'forbidCredentialCastBypass.configuredModelMissing'; + + /** + * Builder write verbs that ship their payload to SQL without routing + * through `Model::setAttribute()`, mapped to the payload SLOTS carrying a + * `column => value` array — each slot the parameter's NAME and its position, + * because a named argument does not sit at its position once an earlier + * optional parameter is skipped. + * + * `create` / `updateOrCreate` / `firstOrCreate` / `createOrFirst` are + * deliberately absent — they build and `save()` a model, so casts fire. + * + * @var array> + */ + private const array WRITE_METHODS = [ + 'update' => [['values', 0]], + 'insert' => [['values', 0]], + 'insertOrIgnore' => [['values', 0]], + 'insertGetId' => [['values', 0]], + 'upsert' => [['values', 0]], + 'updateOrInsert' => [['attributes', 0], ['values', 1]], + // Postgres-only builder writes that take a payload like `update()` and + // reach SQL by the same route. Absent from `Eloquent\Builder`, but its + // `__call` forwards them, so a model query reaches them too. + 'updateFrom' => [['values', 0]], + 'insertOrIgnoreReturning' => [['values', 0]], + // The increment family ships an EXTRA payload to SQL by the same route: + // `Query\Builder::incrementEach()` is literally + // `update(array_merge($columns, $extra))`, and `increment()` / + // `decrement()` delegate to it. A credential column named in either + // array lands raw in the column with no model in the path. + 'increment' => [['extra', 2]], + 'decrement' => [['extra', 2]], + 'incrementEach' => [['columns', 0], ['extra', 1]], + 'decrementEach' => [['columns', 0], ['extra', 1]], + // `Eloquent\Builder::incrementOrCreate()` routes its `$attributes` + // through `firstOrCreate()` — a model save, so casts fire there and it + // is deliberately NOT read — then hands `$extra` to `Model::increment()`. + 'incrementOrCreate' => [['extra', 4]], + // Model-only, re-exposed through `Model::__call`. Same payload slot as + // their loud counterparts. + 'incrementQuietly' => [['extra', 2]], + 'decrementQuietly' => [['extra', 2]], + 'incrementEachQuietly' => [['columns', 0], ['extra', 1]], + 'decrementEachQuietly' => [['columns', 0], ['extra', 1]], + ]; + + /** + * The verbs whose payload bypasses casts even on a MODEL receiver, so the + * receiver type gate must NOT exclude them. + * + * `Model::increment()` is `protected`, but `Model::__call()` names all eight + * explicitly and forwards to them, so `$model->increment(…)` is reachable + * from anywhere. `Model::incrementOrDecrement()` then casts the in-memory + * attribute through `forceFill($extra)` and passes the SAME `$extra`, + * uncast, to the query builder — so the object is right and the row is + * plaintext. Verified against `Illuminate\Database\Eloquent\Model`. + * + * @var list + */ + private const array MODEL_BYPASSING_METHODS = [ + 'increment', + 'decrement', + 'incrementEach', + 'decrementEach', + 'incrementQuietly', + 'decrementQuietly', + 'incrementEachQuietly', + 'decrementEachQuietly', + ]; + + /** The fluent-chain method whose string argument names the table. */ + private const string TABLE_SETTING_METHOD = 'table'; + + /** + * The member name Eloquent reads casts from — the same string names the + * `casts()` method and the `$casts` property, which is why both halves of + * `declaredPropertyCasts()` and `dispatchedMethodCasts()` match on it. + */ + private const string CASTS_MEMBER = 'casts'; + + /** + * Cast values that mean "the model layer transforms this value on write". + * `encrypted:array` / `encrypted:collection` / `encrypted:object` are + * matched by the prefix entry. + * + * @var list + */ + private const array CREDENTIAL_CASTS = ['hashed', 'encrypted']; + + /** + * Cast resolutions already computed this run, keyed by model FQCN. A model + * is parsed once even when a hundred call sites write to it. + * + * @var array, unreadable: list, incomplete: list, missing: bool}> + */ + private array $castCache = []; + + /** + * Per-SOURCE `$casts` property declarations already read this run, keyed by + * class-or-trait FQCN. The property walk visits every ancestor and its + * traits, and one model is written to from many call sites. + * + * @var array|null, complete: bool}|null> + */ + private array $declarationCache = []; + + /** + * @param array $credentialCastTableModels map of raw table + * name to model + * FQCN, used ONLY + * to resolve + * `DB::table('…')` + * chains, which + * carry no model in + * their type. Empty + * by default — an + * unmapped table is + * silent, never + * guessed. + */ + public function __construct( + private ReflectionProvider $reflectionProvider, + private Parser $parser, + private array $credentialCastTableModels = [], + ) {} + + public function getNodeType(): string + { + return MethodCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!$node->name instanceof Identifier) { + return []; + } + + $method = $node->name->toString(); + + if (!array_key_exists($method, self::WRITE_METHODS)) { + return []; + } + + $modelFqcns = $this->resolveModels($node, $scope, $method); + + if ($modelFqcns === []) { + return []; + } + + $columns = $this->payloadColumns($node, $scope, self::WRITE_METHODS[$method]); + $errors = []; + + foreach ($modelFqcns as $modelFqcn) { + $resolution = $this->castResolutionFor($modelFqcn); + + // Reported REGARDLESS of the payload, and deliberately: when a + // declaring source could not be read, this rule does not know which + // columns carry a credential cast, so it cannot say the payload is + // clean. Staying silent here would be a fail-open on the one rule + // whose whole value is catching a silent plaintext write. + if ($resolution['missing']) { + $errors[] = $this->buildConfiguredModelMissingError($node, $modelFqcn); + } + + if ($resolution['unreadable'] !== []) { + $errors[] = $this->buildUnreadableSourceError($node, $modelFqcn, $resolution['unreadable']); + } + + if ($resolution['incomplete'] !== []) { + $errors[] = $this->buildIncompleteCastMapError($node, $modelFqcn, $resolution['incomplete']); + } + + foreach ($columns as $column) { + if (!array_key_exists($column, $resolution['casts'])) { + continue; + } + + $errors[] = $this->buildError($node, $modelFqcn, $column, $resolution['casts'][$column], $method); + } + } + + return $errors; + } + + /** + * Every model whose table this write could target — normally one, more when + * the receiver's type is a UNION of builders. Empty when no model can be + * established statically. + * + * A `Model` receiver returns nothing on purpose: the model path fires casts + * and is the remediation, not the violation. + * + * A union is checked branch by branch rather than collapsed to its first + * member. `Builder|Builder` is two different cast maps, and + * picking one to speak for both is wrong in both directions — it misses a + * credential the other branch casts, and it reports one the branch in hand + * does not. Two errors on one line for a genuinely ambiguous receiver is the + * honest answer: each branch is a real write. + * + * @return list + */ + private function resolveModels(MethodCall $node, Scope $scope, string $method): array + { + $receiverType = TypeCombinator::removeNull($scope->getType($node->var)); + + if ((new ObjectType(Model::class))->isSuperTypeOf($receiverType)->yes()) { + // The model path fires casts for every verb EXCEPT these — see + // MODEL_BYPASSING_METHODS. This is the one place the receiver type + // gate is not sufficient on its own. + if (!in_array($method, self::MODEL_BYPASSING_METHODS, true)) { + return []; + } + + $models = []; + + foreach ($receiverType->getObjectClassReflections() as $classReflection) { + $models[$classReflection->getName()] = true; + } + + return array_keys($models); + } + + $isEloquentBuilder = (new ObjectType(EloquentBuilder::class))->isSuperTypeOf($receiverType)->yes(); + $isRelation = (new ObjectType(Relation::class))->isSuperTypeOf($receiverType)->yes(); + + if ($isEloquentBuilder || $isRelation) { + $fromGenerics = $this->modelsFromGenerics($receiverType); + + if ($fromGenerics !== []) { + return $fromGenerics; + } + } + + if (!(new ObjectType(QueryBuilder::class))->isSuperTypeOf($receiverType)->yes()) { + return []; + } + + $fromTable = $this->modelFromChainTable($node->var); + + return $fromTable === null ? [] : [$fromTable]; + } + + /** + * The models named by `Builder` / `Relation` + * generic arguments — per union branch, the first argument that is a Model + * subtype. For a relation `TRelatedModel` comes first, and it is the model + * whose table the write targets, so one reading serves both shapes. + * + * @return list + */ + private function modelsFromGenerics(Type $receiverType): array + { + $modelType = new ObjectType(Model::class); + $models = []; + + foreach ($receiverType->getObjectClassReflections() as $classReflection) { + // The ACTIVE template map holds the arguments this particular + // instance was parameterized with, in template-declaration order — + // `TModel` for a Builder, `TRelatedModel` first for a Relation. + foreach ($classReflection->getActiveTemplateTypeMap()->getTypes() as $typeArgument) { + if (!$modelType->isSuperTypeOf($typeArgument)->yes()) { + continue; + } + + $referenced = $typeArgument->getReferencedClasses(); + + if ($referenced === []) { + continue; + } + + $models[$referenced[0]] = true; + + // One model per branch: the remaining template arguments of a + // Relation (TDeclaringModel, …) are not write targets. + break; + } + } + + return array_keys($models); + } + + /** + * Walk the fluent chain back to the nearest `table('…')` call with a + * string-literal argument and look the table name up in the configured + * map. An unmapped table, a non-literal argument, or no `table()` call at + * all all yield null — the rule declines rather than guessing a model from + * a table name. + */ + private function modelFromChainTable(Expr $receiver): ?string + { + $current = $receiver; + + while ($current instanceof MethodCall || $current instanceof StaticCall) { + if ( + $current->name instanceof Identifier + && $current->name->toString() === self::TABLE_SETTING_METHOD + ) { + return $this->mappedModelForTableArg($current); + } + + if ($current instanceof MethodCall) { + $current = $current->var; + + continue; + } + + // A StaticCall is the chain root — its own arguments were inspected + // above, and there are no earlier hops to walk. + return null; + } + + return null; + } + + private function mappedModelForTableArg(MethodCall|StaticCall $call): ?string + { + if (!isset($call->args[0]) || !$call->args[0] instanceof Node\Arg) { + return null; + } + + $value = $call->args[0]->value; + + if (!$value instanceof String_) { + return null; + } + + return $this->credentialCastTableModels[$value->value] ?? null; + } + + /** + * Distinct column names appearing as keys in the payload arguments named by + * `$payloadSlots`. Only constant array types are read; anything else + * contributes nothing. + * + * Deduplicated on purpose: a multi-row `insert([['password' => …], + * ['password' => …]])` names one offending column at one call site, and + * reporting it once per row would put N identical errors on one line. + * + * @param list $payloadSlots + * + * @return list + */ + private function payloadColumns(MethodCall $node, Scope $scope, array $payloadSlots): array + { + $seen = []; + + foreach ($payloadSlots as [$name, $position]) { + $argument = $this->argumentAt($node, $name, $position); + + if ($argument === null) { + continue; + } + + foreach ($this->constantArrayKeys($scope->getType($argument->value)) as $column) { + $seen[$column] = true; + } + } + + return array_keys($seen); + } + + /** + * One argument, addressed by NAME first and by position second. + * + * A named argument does not sit at its parameter's position: + * `increment('votes', extra: [...])` puts the payload at index 1, so reading + * index 2 finds nothing and the write passes silently. Since PHP 8.0 any + * caller may name any argument, so a position-only reading is a + * false-negative generator on every verb here — and on the increment family + * especially, whose payload is the THIRD parameter and whose second + * (`$amount`) has a default worth skipping. + */ + private function argumentAt(MethodCall $node, string $name, int $position): ?Node\Arg + { + foreach ($node->args as $argument) { + if ( + $argument instanceof Node\Arg + && $argument->name instanceof Identifier + && $argument->name->toString() === $name + ) { + return $argument; + } + } + + // PHP requires every positional argument before the first named one, so + // positional slots are contiguous from zero and this index is only + // meaningful when the argument sitting there is itself positional. + // Checking the slot rather than refusing whenever ANY argument is named + // keeps `upsert($values, uniqueBy: […])` covered. + $argument = $node->args[$position] ?? null; + + return $argument instanceof Node\Arg && $argument->name === null ? $argument : null; + } + + /** + * String keys of a constant array type. A list of constant arrays (the + * `insert([[…], […]])` / `upsert` row shape) is recognised by having no + * string keys of its own while every value is itself a constant array, and + * is descended into. + * + * @return list + */ + private function constantArrayKeys(Type $type): array + { + $columns = []; + + foreach ($type->getConstantArrays() as $constantArray) { + foreach ($this->keysOfConstantArray($constantArray) as $column) { + $columns[] = $column; + } + } + + return $columns; + } + + /** + * @return list + */ + private function keysOfConstantArray(ConstantArrayType $type): array + { + $columns = []; + $nested = []; + + foreach ($type->getKeyTypes() as $index => $keyType) { + $constantStrings = $keyType->getConstantStrings(); + + if ($constantStrings !== []) { + $columns[] = $constantStrings[0]->getValue(); + + continue; + } + + $valueType = $type->getValueTypes()[$index] ?? null; + + if ($valueType !== null) { + foreach ($valueType->getConstantArrays() as $row) { + $nested[] = $row; + } + } + } + + // A payload is EITHER a `column => value` map or a list of such maps. + // Descending into rows only when the outer array contributed no column + // names of its own keeps a map with one array-valued column (a `json` + // cast written as an array) from being misread as a row list. + if ($columns !== []) { + return $columns; + } + + foreach ($nested as $row) { + foreach ($this->keysOfConstantArray($row) as $column) { + $columns[] = $column; + } + } + + return $columns; + } + + /** + * The model's credential-bearing casts as `column => cast`, resolved the way + * PHP and Laravel actually resolve them, plus the declaring sources whose + * PHP could not be read or interpreted. Memoized per FQCN. + * + * Laravel builds the effective map exactly once, in + * `HasAttributes::initializeHasAttributes()`: + * + * $this->casts = array_merge($this->casts, $this->casts()); + * + * Two halves, two DIFFERENT PHP resolution rules, and reading either as a + * merge across every declaration produces a FALSE POSITIVE on a column the + * model does not cast: + * + * - `$casts` is a PROPERTY. Exactly ONE declaration survives — the most + * derived — and a redeclaration REPLACES the ancestor's default rather + * than merging with it. A class-declared default likewise replaces a + * trait-imported one. + * - `casts()` is a METHOD, and reading it is a SINGLE virtual dispatch. + * Only the nearest declaration's body runs. An ancestor's or a trait's + * body contributes NOTHING unless the body that does run calls + * `parent::casts()` — the one construct that makes an ancestor's map + * part of the answer, and the only reason to walk upward at all. + * + * Because the merge puts `casts()` second, the method half beats the + * property half for any column both declare — regardless of the order the + * two appear in the source file. + * + * Measured against PHP's own answer over the twenty-three declaration shapes + * in `CastDispatchShapes.php` (war-room enforcement #217): reading this as + * "merge every declaration, leaf wins" is wrong on nine of them — eight + * inventing a credential cast, one calling a readable declaration + * unreadable — each masked in the obvious fixtures by a key collision. + * + * @return array{casts: array, unreadable: list, incomplete: list, missing: bool} + */ + private function castResolutionFor(string $modelFqcn): array + { + if (array_key_exists($modelFqcn, $this->castCache)) { + return $this->castCache[$modelFqcn]; + } + + $empty = ['casts' => [], 'unreadable' => [], 'incomplete' => [], 'missing' => false]; + $this->castCache[$modelFqcn] = $empty; + + // The class cannot be absent on the GENERIC path — that FQCN came out + // of a resolved type — so this branch is reachable only from the + // `credentialCastTableModels` map, where it means the configured class + // does not exist. Returning the "no mapping" answer here would let a + // typo disarm the rule permanently and silently. + if (!$this->reflectionProvider->hasClass($modelFqcn)) { + $resolution = ['casts' => [], 'unreadable' => [], 'incomplete' => [], 'missing' => true]; + $this->castCache[$modelFqcn] = $resolution; + + return $resolution; + } + + $classReflection = $this->reflectionProvider->getClass($modelFqcn); + $chain = $this->declarationChain($classReflection); + + $unreadable = []; + $incomplete = []; + + $casts = array_merge( + $this->propertyCasts($chain, $unreadable, $incomplete), + $this->dispatchedMethodCasts($classReflection, $unreadable, $incomplete), + ); + + $credentialCasts = []; + + foreach ($casts as $column => $cast) { + if ($this->isCredentialCast($cast)) { + $credentialCasts[$column] = $cast; + } + } + + $resolution = [ + 'casts' => $credentialCasts, + // Both halves walk the same sources, so a source that cannot be + // read is reached twice and would otherwise be named twice in one + // message. + 'unreadable' => array_values(array_unique($unreadable)), + 'incomplete' => array_values(array_unique($incomplete)), + 'missing' => false, + ]; + $this->castCache[$modelFqcn] = $resolution; + + return $resolution; + } + + /** + * Every source that could carry a cast declaration, in PHP's own + * member-resolution order: nearest ancestor first and, within one ancestor, + * the class body before the traits it imports. + * + * The walk stops AT `Illuminate\Database\Eloquent\Model`. The framework's + * own `$casts = []` and `casts(): array { return []; }` contribute nothing, + * so parsing vendor source to discover that is wasted work — and it keeps + * the rule from reporting the framework as an unreadable declaring source + * on a consumer tree that ships no vendor PHP. + * + * @return list}> + */ + private function declarationChain(ClassReflection $classReflection): array + { + $chain = []; + + foreach ([$classReflection, ...$classReflection->getParents()] as $ancestor) { + if ($ancestor->getName() === Model::class) { + break; + } + + $chain[] = [ + 'class' => $ancestor, + 'sources' => [$ancestor, ...$this->importedTraits($ancestor)], + ]; + } + + return $chain; + } + + /** + * The traits ONE class-like imports, depth first, the importing trait before + * the traits it imports itself — PHP's precedence, since a trait's own + * member beats one it pulled in. + * + * Measured, and the reason this is hand-rolled rather than + * `ClassReflection::getTraits(true)`: that helper walks the PARENT CHAIN as + * well, so a model importing no traits at all reports twelve of them — + * Laravel's own `HasAttributes` among them, which declares BOTH + * `$casts = []` and `casts(): array`. Under a resolution that stops at the + * first declaration it finds, inheriting the framework's empty declaration + * into every subclass silently answers "this model casts nothing". + * + * @param array $seen guards a diamond import, where two + * imported traits pull in a third + * + * @return list + */ + private function importedTraits(ClassReflection $classReflection, array &$seen = []): array + { + $traits = []; + + foreach ($classReflection->getTraits() as $trait) { + $name = $trait->getName(); + + if (array_key_exists($name, $seen)) { + continue; + } + + $seen[$name] = true; + $traits[] = $trait; + + foreach ($this->importedTraits($trait, $seen) as $nested) { + $traits[] = $nested; + } + } + + return $traits; + } + + /** + * The ONE `$casts` property default that survives PHP's property + * resolution: the most derived declaration, class body before trait, + * replacing rather than merging whatever an ancestor declared. + * + * Two traits declaring `$casts` with different defaults, or a class + * redeclaring a trait's with a different default, is a PHP FATAL + * ("definition differs and is considered incompatible"), so an ambiguous + * property cannot reach this walk from valid code. + * + * @param list}> $chain + * @param list $unreadable + * @param list $incomplete + * + * @return array + */ + private function propertyCasts(array $chain, array &$unreadable, array &$incomplete): array + { + foreach ($chain as $entry) { + foreach ($entry['sources'] as $source) { + $declared = $this->declaredPropertyCasts($source); + + if ($declared === null) { + $unreadable[] = $source->getName(); + + continue; + } + + if ($declared['property'] === null) { + continue; + } + + if (!$declared['complete']) { + $incomplete[] = $source->getName(); + } + + return $declared['property']; + } + } + + return []; + } + + /** + * The map a `$model->casts()` call would actually produce. + * + * The declaration that runs is resolved by REFLECTION, not by searching the + * ancestry: `getNativeReflection()->getMethod('casts')` is the method PHP + * would dispatch, and its `getFileName()` / `getStartLine()` locate that + * body exactly — through a trait, and through a trait ADAPTATION. Verified: + * on `use A, B { B::casts insteadof A; }` reflection points at B's body, + * which a first-match walk over the imported traits gets wrong whenever the + * excluded trait is listed first. An `as` alias has the same shape. + * + * The walk continues upward only when the body that runs uses + * `parent::casts()` — and then from the parent of the class that DECLARES + * that body, not of the class the write targeted, because `parent::` inside + * an inherited body resolves against that body's own class. + * + * @param list $unreadable + * @param list $incomplete + * + * @return array + */ + private function dispatchedMethodCasts( + ClassReflection $classReflection, + array &$unreadable, + array &$incomplete, + ): array { + $maps = []; + $current = $classReflection; + $visited = []; + + // BOUNDED, not merely guarded. The walk only ever moves upward, so the + // ancestry depth is its ceiling — and an unbounded loop here would hang + // the consumer's analysis with no error at all rather than reporting + // something wrong. Mutation testing makes the difference visible: turning + // the visited-guard `break` below into `continue` spins forever, which is + // a timeout in CI and a mystery in a consumer's pipeline. + $remaining = count($classReflection->getParents()) + 1; + + while ($current !== null && $remaining-- > 0) { + $native = $current->getNativeReflection(); + + if (!$native->hasMethod(self::CASTS_MEMBER)) { + break; + } + + $method = $native->getMethod(self::CASTS_MEMBER); + $declaringClass = $method->getDeclaringClass()->getName(); + + // Laravel's own `casts(): array { return []; }`. Reaching it means + // nothing in the consumer's hierarchy declares one. + if ($declaringClass === Model::class) { + break; + } + + // A `parent::casts()` chain cannot revisit a class; guard anyway so + // a pathological hierarchy cannot spin here. + if (array_key_exists($declaringClass, $visited)) { + break; + } + + $visited[$declaringClass] = true; + + $file = $method->getFileName(); + $node = $file === false + ? null + : $this->castsMethodNodeAt($file, $method->getStartLine()); + + if ($node === null) { + $unreadable[] = $declaringClass; + + break; + } + + $complete = true; + $maps[] = $this->castsFromReturns($node, $complete); + + if (!$complete) { + $incomplete[] = $declaringClass; + } + + if (!$this->contributesParentCasts($node)) { + break; + } + + $current = $this->reflectionProvider->hasClass($declaringClass) + ? $this->reflectionProvider->getClass($declaringClass)->getParentClass() + : null; + } + + $casts = []; + + // Nearest declaration wins, so merge oldest-first. + foreach (array_reverse($maps) as $map) { + $casts = array_merge($casts, $map); + } + + return $casts; + } + + /** + * The `casts()` declaration whose body starts at `$line` in `$file`. + * + * Matched on the EXACT start line rather than by locating the class, because + * the body that runs may live in a trait the class only imports — and after + * an `insteadof` it is not even the first trait declaring it. Measured: + * php-parser and PHP reflection agree on this line, docblock and attribute + * included. No match means the source moved under us, which is a read + * FAILURE and reported as one, never a silent empty map. + */ + private function castsMethodNodeAt(string $file, false|int $line): ?ClassMethod + { + if ($line === false) { + return null; + } + + try { + $stmts = $this->parser->parseFile($file); + } catch (ParserErrorsException) { + return null; + } + + foreach ($this->castsMethodNodes($stmts) as $candidate) { + if ($candidate->getStartLine() === $line) { + return $candidate; + } + } + + return null; + } + + /** + * Every `casts()` method declaration among parsed statements, at any depth — + * one file can hold several classes and traits declaring it. + * + * @param array $nodes + * + * @return list + */ + private function castsMethodNodes(array $nodes): array + { + $found = []; + + foreach ($nodes as $node) { + if ($node instanceof ClassMethod && $node->name->toString() === self::CASTS_MEMBER) { + $found[] = $node; + } + + foreach ($this->castsMethodNodes($this->childNodes($node)) as $nested) { + $found[] = $nested; + } + } + + return $found; + } + + /** + * The `column => cast` pairs one `casts()` body contributes. + * + * A body with SEVERAL returns (`if (…) { return [...]; } return [...];`) has + * no single static answer, so every branch is read and the union is taken. + * That is a deliberate bias: a column some branch casts as a credential IS + * cast on that path, and a builder write to it is unsafe there. Where two + * branches disagree about the SAME column, the CREDENTIAL cast wins rather + * than whichever appears last — source order is not a fact about which + * branch runs. + * + * @return array + */ + private function castsFromReturns(ClassMethod $method, bool &$complete): array + { + $casts = []; + + foreach ($this->returnedArrays($method, $complete) as $array) { + foreach ($this->stringPairs($array) as $column => $cast) { + if ( + array_key_exists($column, $casts) + && $this->isCredentialCast($casts[$column]) + && !$this->isCredentialCast($cast) + ) { + continue; + } + + $casts[$column] = $cast; + } + } + + return $casts; + } + + /** + * Whether this body's returned value actually DEPENDS on `parent::casts()`. + * + * A call whose result is thrown away — `parent::casts();` as a statement of + * its own — contributes nothing at runtime, so inheriting the parent's map + * on the strength of it invents casts the child does not have. Anything that + * CAPTURES the result counts: returned directly, composed into a literal or + * an `array_merge`, or assigned to a variable first. + */ + private function contributesParentCasts(ClassMethod $method): bool + { + foreach ($this->childNodes($method) as $node) { + if ($this->capturesParentCastsCall($node)) { + return true; + } + } + + return false; + } + + /** + * @param Node $node a node whose own context is NOT a discarded expression + * statement + */ + private function capturesParentCastsCall(Node $node): bool + { + if ($node instanceof FunctionLike || $node instanceof Class_) { + return false; + } + + // `parent::casts();` alone: the call is the whole statement, so its + // result goes nowhere. + if ($node instanceof Node\Stmt\Expression && $this->isParentCastsCall($node->expr)) { + return false; + } + + if ($this->isParentCastsCall($node)) { + return true; + } + + foreach ($this->childNodes($node) as $child) { + if ($this->capturesParentCastsCall($child)) { + return true; + } + } + + return false; + } + + private function isParentCastsCall(Node $node): bool + { + return $node instanceof StaticCall + && $node->class instanceof Node\Name + && $node->class->toLowerString() === 'parent' + && $node->name instanceof Identifier + && $node->name->toString() === self::CASTS_MEMBER; + } + + /** + * Whether a cast value means "the model layer transforms this on write" — + * exactly `hashed`, exactly `encrypted`, or an `encrypted:` variant + * (`encrypted:array`, `encrypted:collection`, `encrypted:object`). + */ + private function isCredentialCast(string $cast): bool + { + foreach (self::CREDENTIAL_CASTS as $credentialCast) { + if ($cast === $credentialCast || str_starts_with($cast, $credentialCast . ':')) { + return true; + } + } + + return false; + } + + /** + * The `$casts` PROPERTY default declared by ONE source, read from its PHP + * because a property default is not what reflection hands back on an + * unconstructed analyser-side class. + * + * Property-only by design. The `casts()` METHOD half is resolved by + * reflection in `dispatchedMethodCasts()`, which handles trait adaptations + * a source walk cannot see. A property has no such adaptation — `insteadof` + * and `as` are method-only, and two sources declaring `$casts` with + * different defaults is a PHP fatal, not an ambiguity — so first-match over + * the declaration chain IS PHP's answer here. + * + * Returns NULL — never an empty shape — when the source cannot be located or + * parsed, so the caller can tell "declares nothing" from "we could not + * look". Within the shape, `property` is NULL when this source declares no + * `$casts` default at all, and an ARRAY (possibly empty) when it does: the + * difference is what stops the walk. `complete` is FALSE when a `$casts` + * default IS declared but carries no array literal to read + * (`protected $casts = self::CASTS;`). + * + * @return array{property: array|null, complete: bool}|null + */ + private function declaredPropertyCasts(ClassReflection $classReflection): ?array + { + $name = $classReflection->getName(); + + if (array_key_exists($name, $this->declarationCache)) { + return $this->declarationCache[$name]; + } + + $this->declarationCache[$name] = null; + + $file = $classReflection->getFileName(); + + if ($file === null) { + return null; + } + + try { + $stmts = $this->parser->parseFile($file); + } catch (ParserErrorsException) { + return null; + } + + $classNode = $this->findClassNode($stmts, $name); + + if ($classNode === null) { + return null; + } + + $property = null; + $complete = true; + + foreach ($classNode->stmts as $stmt) { + if (!$stmt instanceof Property) { + continue; + } + + foreach ($stmt->props as $prop) { + if ($prop->name->toString() !== self::CASTS_MEMBER || $prop->default === null) { + continue; + } + + if (!$prop->default instanceof Expr\Array_) { + $property = []; + $complete = false; + + continue; + } + + $property = $this->stringPairs($prop->default); + } + } + + $declaration = ['property' => $property, 'complete' => $complete]; + $this->declarationCache[$name] = $declaration; + + return $declaration; + } + + /** + * Locate the class-like declaration for `$fqcn` among parsed statements — + * a class OR a trait, since Laravel models routinely compose their cast map + * from traits. The injected parser resolves names, so `namespacedName` is + * populated and the match is exact rather than by short name. + * + * @param array $nodes + */ + private function findClassNode(array $nodes, string $fqcn): ?ClassLike + { + foreach ($nodes as $node) { + if ($node instanceof ClassLike && $node->namespacedName?->toString() === $fqcn) { + return $node; + } + + $found = $this->findClassNode($this->childNodes($node), $fqcn); + + if ($found !== null) { + return $found; + } + } + + return null; + } + + /** + * A node's direct child NODES. Sub-node slots also hold strings, ints, + * booleans and nulls (`Class_::$flags`, `Identifier::$name`, …), so the + * slot values are filtered rather than assumed traversable. + * + * @return list + */ + private function childNodes(Node $node): array + { + $children = []; + + foreach ($node->getSubNodeNames() as $subNodeName) { + $subNode = $node->{$subNodeName}; + + foreach (is_array($subNode) ? $subNode : [$subNode] as $candidate) { + if ($candidate instanceof Node) { + $children[] = $candidate; + } + } + } + + return $children; + } + + /** + * Every array literal contributed by a `return` in a method body, including + * returns nested inside conditionals and literals nested inside a + * composition expression (`return array_merge(parent::casts(), [...]);`). + * + * `$complete` is set to FALSE when a return statement contributes no array + * literal at all, so the caller can report an incomplete cast map instead + * of silently reading it as "declares nothing". + * + * @return list + */ + private function returnedArrays(ClassMethod $method, bool &$complete): array + { + $arrays = []; + + $this->collectReturnedArrays($this->childNodes($method), $arrays, $complete); + + return $arrays; + } + + /** + * @param list $nodes + * @param list $arrays + */ + private function collectReturnedArrays(array $nodes, array &$arrays, bool &$complete): void + { + foreach ($nodes as $node) { + if ($node instanceof Return_) { + if ($node->expr === null) { + continue; + } + + $returned = []; + + $this->collectArrayLiterals([$node->expr], $returned); + + if ($returned === []) { + // `return parent::casts();` carries no literal of its own + // and needs none — the ancestor's declaration is a + // resolvable chain link that dispatchedMethodCasts() walks. + // Reporting it as uninterpretable would be a false positive + // on the idiomatic pass-through override. + if (!$this->isParentCastsCall($node->expr) && !$this->capturesParentCastsCall($node->expr)) { + $complete = false; + } + + continue; + } + + foreach ($returned as $array) { + $arrays[] = $array; + } + + continue; + } + + // A nested function-like (closure, arrow function, nested function + // declaration) or anonymous class carries its own returns, which + // are not this method's cast map. + if ($node instanceof FunctionLike || $node instanceof Class_) { + continue; + } + + $this->collectReturnedArrays($this->childNodes($node), $arrays, $complete); + } + } + + /** + * Array literals reachable inside one expression WITHOUT descending into a + * collected array — so `['a' => ['b' => 'c']]` contributes the outer array + * only, and `'b' => 'c'` never becomes a cast pair of its own. Composition + * expressions DO contribute: `array_merge(parent::casts(), [...])` and a + * ternary over two literals both yield the literals they carry. + * + * A literal inside a function-like (a closure or arrow function passed as an + * argument, an anonymous class) is NOT collected — it is a callback's return + * value, not this cast map, and harvesting it would be a false positive on a + * column the model never casts. + * + * @param list $nodes + * @param list $arrays + */ + private function collectArrayLiterals(array $nodes, array &$arrays): void + { + foreach ($nodes as $node) { + if ($node instanceof FunctionLike || $node instanceof Class_) { + continue; + } + + if ($node instanceof Expr\Array_) { + $arrays[] = $node; + + continue; + } + + $this->collectArrayLiterals($this->childNodes($node), $arrays); + } + } + + /** + * `'key' => 'value'` pairs of an array literal. Non-string keys and + * non-string values (a `::class` constant fetch, a computed expression) are + * skipped — see the class docblock's out-of-scope list. + * + * @return array + */ + private function stringPairs(Expr $expr): array + { + if (!$expr instanceof Expr\Array_) { + return []; + } + + $pairs = []; + + foreach ($expr->items as $item) { + if ($item->key instanceof String_ && $item->value instanceof String_) { + $pairs[$item->key->value] = $item->value->value; + } + } + + return $pairs; + } + + /** + * @param list $unreadable + */ + private function buildUnreadableSourceError( + MethodCall $node, + string $modelFqcn, + array $unreadable, + ): IdentifierRuleError { + $message = sprintf( + 'Cannot verify this write against %s: the PHP declaring %s could not be located or parsed, so the credential-cast map is incomplete and a hashed/encrypted column in this payload would go unreported. Fix the source, or suppress %s here if the write is known safe.', + $modelFqcn, + implode(', ', $unreadable), + self::UNREADABLE_IDENTIFIER, + ); + + return RuleErrorBuilder::message($message) + ->identifier(self::UNREADABLE_IDENTIFIER) + ->line($node->getStartLine()) + ->build(); + } + + /** + * @param list $incomplete + */ + private function buildIncompleteCastMapError( + MethodCall $node, + string $modelFqcn, + array $incomplete, + ): IdentifierRuleError { + $message = sprintf( + 'Cannot verify this write against %s: %s declares casts in a form this rule cannot read (a casts() return or a $casts default carrying no array literal, such as a class constant or a helper call), so the credential-cast map is incomplete and a hashed/encrypted column in this payload would go unreported. Restate the credential columns as literal string pairs, or suppress %s here if the write is known safe.', + $modelFqcn, + implode(', ', $incomplete), + self::INCOMPLETE_IDENTIFIER, + ); + + return RuleErrorBuilder::message($message) + ->identifier(self::INCOMPLETE_IDENTIFIER) + ->line($node->getStartLine()) + ->build(); + } + + private function buildConfiguredModelMissingError(MethodCall $node, string $modelFqcn): IdentifierRuleError + { + $message = sprintf( + 'Cannot verify this write: credentialCastTableModels maps this table to %s, which does not exist, so no credential-cast map could be read and a hashed/encrypted column in this payload would go unreported. Fix the FQCN in the parameter, or remove the mapping if the table is no longer covered.', + $modelFqcn, + ); + + return RuleErrorBuilder::message($message) + ->identifier(self::CONFIG_IDENTIFIER) + ->line($node->getStartLine()) + ->build(); + } + + private function buildError( + MethodCall $node, + string $modelFqcn, + string $column, + string $cast, + string $method, + ): IdentifierRuleError { + $message = sprintf( + "Attribute '%s' on %s carries the '%s' cast, but %s() is a query-builder write that bypasses the cast and stores the raw value. Write it through the model path instead (assign the attribute and save the model).", + $column, + $modelFqcn, + $cast, + $method, + ); + + return RuleErrorBuilder::message($message) + ->identifier(self::IDENTIFIER) + ->line($node->getStartLine()) + ->build(); + } +} diff --git a/tests/Fixtures/CredentialCastBypass/AbstractCredentialHolder.php b/tests/Fixtures/CredentialCastBypass/AbstractCredentialHolder.php new file mode 100644 index 0000000..6379ee0 --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/AbstractCredentialHolder.php @@ -0,0 +1,24 @@ + + */ + protected function casts(): array + { + return [ + 'passphrase' => 'hashed', + ]; + } +} diff --git a/tests/Fixtures/CredentialCastBypass/ApiKey.php b/tests/Fixtures/CredentialCastBypass/ApiKey.php new file mode 100644 index 0000000..d260b2c --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/ApiKey.php @@ -0,0 +1,21 @@ + */ + protected $casts = [ + 'secret' => 'encrypted', + 'label' => 'string', + ]; +} diff --git a/tests/Fixtures/CredentialCastBypass/Article.php b/tests/Fixtures/CredentialCastBypass/Article.php new file mode 100644 index 0000000..5b047fa --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/Article.php @@ -0,0 +1,25 @@ + + */ + protected function casts(): array + { + return [ + 'published_at' => 'datetime', + ]; + } +} diff --git a/tests/Fixtures/CredentialCastBypass/BuilderWrites.php b/tests/Fixtures/CredentialCastBypass/BuilderWrites.php new file mode 100644 index 0000000..108cbc6 --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/BuilderWrites.php @@ -0,0 +1,241 @@ +where('id', 1)->update(['password' => 'plaintext']); + } + + public function encryptedColumn(): void + { + User::query()->where('id', 1)->update(['api_token' => 'raw-token']); + } + + public function encryptedVariantColumn(): void + { + User::query()->where('id', 1)->update(['recovery_codes' => ['a', 'b']]); + } + + public function castsPropertyForm(): void + { + ApiKey::query()->update(['secret' => 'raw']); + } + + public function inheritedFromAbstractBase(): void + { + Vault::query()->update(['passphrase' => 'raw']); + } + + public function payloadHoistedIntoVariable(): void + { + $payload = ['password' => 'plaintext']; + + User::query()->update($payload); + } + + public function insertRowList(): void + { + User::query()->insert([ + ['password' => 'one'], + ['password' => 'two'], + ]); + } + + public function upsertRows(): void + { + User::query()->upsert([['email' => 'a@b.c', 'password' => 'raw']], ['email']); + } + + public function updateOrInsertSecondArgument(): void + { + User::query()->updateOrInsert(['email' => 'a@b.c'], ['password' => 'raw']); + } + + public function relationDerivedWrite(User $user): void + { + $user->apiKeys()->update(['secret' => 'raw']); + } + + public function castDeclaredInATraitViaMethod(): void + { + TraitCastModel::query()->update(['trait_secret' => 'raw']); + } + + public function castDeclaredInATraitViaProperty(): void + { + TraitCastModel::query()->update(['trait_notes' => 'raw']); + } + + public function twoCredentialColumnsInOnePayload(): void + { + User::query()->update(['password' => 'p', 'api_token' => 't']); + } + + public function castComposedWithArrayMerge(): void + { + ComposedCastModel::query()->update(['composed_secret' => 'raw']); + } + + public function castInheritedThroughAComposingLeaf(): void + { + ComposedCastModel::query()->update(['passphrase' => 'raw']); + } + + public function castComposedWithArraySpread(): void + { + SpreadCastModel::query()->update(['spread_secret' => 'raw']); + } + + public function castDeclaredAlongsideANestedLiteral(): void + { + NestedLiteralCastModel::query()->update(['real_secret' => 'raw']); + } + + /** + * `insertOrIgnore` and `insertGetId` carry the same payload shape as + * `insert`, and were on the verb list with no site of their own — so a + * regression that dropped either would have been invisible. + */ + public function insertOrIgnorePayload(): void + { + User::query()->insertOrIgnore(['password' => 'raw']); + } + + public function insertGetIdPayload(): void + { + User::query()->insertGetId(['password' => 'raw']); + } + + /** + * The increment family ships its EXTRA payload straight to + * `Query\Builder::update()` — `incrementEach()` is literally + * `update(array_merge($columns, $extra))`. The counter column is innocent; + * the extra array is an ordinary uncast write. + */ + public function incrementWithCredentialInExtra(): void + { + User::query()->increment('login_count', 1, ['password' => 'raw']); + } + + public function decrementWithCredentialInExtra(): void + { + User::query()->decrement('login_count', 1, ['password' => 'raw']); + } + + public function incrementEachWithCredentialInExtra(): void + { + User::query()->incrementEach(['login_count' => 1], ['password' => 'raw']); + } + + public function decrementEachWithCredentialInExtra(): void + { + User::query()->decrementEach(['login_count' => 1], ['password' => 'raw']); + } + + /** + * A NAMED argument only sits at its parameter's position when no earlier + * optional parameter was skipped. Here `$amount` is omitted, so the payload + * is argument 1 rather than 2 and a position-only reading finds nothing. + * + * The `update(values: …)` site below is the OTHER half of the pair: with one + * argument the named form lands at index 0 anyway, so it was already covered + * incidentally. It is pinned to keep it that way, not as a fix. + */ + public function incrementWithNamedExtraArgument(): void + { + User::query()->increment('login_count', extra: ['password' => 'raw']); + } + + public function updateWithNamedValuesArgument(): void + { + User::query()->update(values: ['password' => 'raw']); + } + + /** + * A named argument AFTER the payload must not blind the positional read of + * the payload itself. + */ + public function upsertWithNamedUniqueByArgument(): void + { + User::query()->upsert([['email' => 'a@b.c', 'password' => 'raw']], uniqueBy: ['email']); + } + + /** + * `Query\Builder::updateFrom()` and `insertOrIgnoreReturning()` take a + * payload like `update()` and reach SQL the same way. Absent from + * `Eloquent\Builder`, but its `__call` forwards them. + */ + public function updateFromPayload(): void + { + User::query()->updateFrom(['password' => 'raw']); + } + + public function insertOrIgnoreReturningPayload(): void + { + User::query()->insertOrIgnoreReturning(['password' => 'raw']); + } + + /** + * `incrementOrCreate()` routes `$attributes` through `firstOrCreate()` — a + * model save, so casts fire there and it is deliberately not read — then + * hands `$extra` to `Model::increment()`, which does not. + */ + public function incrementOrCreateExtraPayload(): void + { + User::query()->incrementOrCreate(['email' => 'a@b.c'], 'login_count', 1, 1, ['password' => 'raw']); + } + + /** + * A MODEL receiver, and still a bypass — the one family where it is. + * `Model::__call()` re-exposes the protected increment methods, and + * `Model::incrementOrDecrement()` casts the in-memory attribute via + * `forceFill($extra)` while passing the SAME `$extra` uncast to the query + * builder. The object is right; the row is plaintext. + */ + public function modelIncrementExtraPayload(User $user): void + { + $user->increment('login_count', 1, ['password' => 'raw']); + } + + public function modelDecrementEachExtraPayload(User $user): void + { + $user->decrementEach(['login_count' => 1], ['password' => 'raw']); + } + + public function modelIncrementQuietlyExtraPayload(User $user): void + { + $user->incrementQuietly('login_count', 1, ['password' => 'raw']); + } + + /** + * A UNION receiver: `Builder
|Builder`, two different cast + * maps behind one variable. Every branch is a real write, so every branch is + * checked — and the castless model is deliberately FIRST, because reading + * only the first branch answers "nothing here" while `User::password` goes + * to SQL in plaintext on the other one. + */ + public function unionOfBuildersForDifferentModels(bool $flag): void + { + $query = $flag ? Article::query() : User::query(); + + $query->update(['password' => 'raw']); + } +} diff --git a/tests/Fixtures/CredentialCastBypass/CastDispatchShapes.php b/tests/Fixtures/CredentialCastBypass/CastDispatchShapes.php new file mode 100644 index 0000000..f5da3f4 --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/CastDispatchShapes.php @@ -0,0 +1,451 @@ +casts = array_merge($this->casts, $this->casts()); + * + * so `$casts` is a PROPERTY (one surviving declaration, most derived wins, + * replacing not merging) and `casts()` is a METHOD reached by a SINGLE virtual + * dispatch (only the nearest body runs, and an ancestor contributes only through + * an explicit `parent::casts()`). + * + * `ForbidCredentialCastBypassRuleTest::testTheRuleAgreesWithPhpsOwnCastResolution…` + * computes the expectation for every class here from PHP itself rather than from + * a hand-written list, and asserts the two readings disagree on enough rows that + * the table is actually exercising the difference. + * + * **Every class here is LOADED, not merely parsed** — that is what makes the + * expectation PHP's own answer instead of someone's reading of Laravel, and it + * constrains what can live here: a shape must be composable on the package's + * MINIMUM PHP, not just the newest. One shape is absent for exactly that reason. + * A trait declaring a non-empty `$casts` DEFAULT is a fatal composition error on + * PHP 8.4 — `Model` already declares `protected $casts = []` through + * `HasAttributes`, and 8.4 requires an inherited and a trait-imported property + * to agree on their default ("the definition differs and is considered + * incompatible"); PHP 8.5 accepts it. Measured on CI, where 8.5 passed and both + * 8.4 legs died. The trait-`$casts` shape is therefore pinned by the + * analysis-only fixtures instead (`HasEncryptedNotesProperty` on + * `TraitCastModel`), which PHPStan parses and never composes — the reason the + * incompatibility went unnoticed there. + */ +trait DeclaresSecretViaMethod +{ + /** + * @return array + */ + protected function casts(): array + { + return ['trait_method_secret' => 'hashed']; + } +} + +trait DeclaresPlainPasswordViaMethod +{ + /** + * @return array + */ + protected function casts(): array + { + return ['password' => 'string']; + } +} + +trait DeclaresPlainPasswordViaTraitToBeExcluded +{ + /** + * @return array + */ + protected function casts(): array + { + return ['password' => 'string']; + } +} + +trait DeclaresHashedPasswordViaTraitToBeExcluded +{ + /** + * @return array + */ + protected function casts(): array + { + return ['password' => 'hashed']; + } +} + +class MethodBase extends Model +{ + /** + * @return array + */ + protected function casts(): array + { + return ['password' => 'hashed']; + } +} + +/** + * The control: the shape every other row is measured against. + */ +class LeafMethod extends Model +{ + /** + * @return array + */ + protected function casts(): array + { + return ['password' => 'hashed']; + } +} + +/** + * Declares nothing, so dispatch passes through to the parent's body. + */ +class InheritedMethod extends MethodBase {} + +/** + * Overrides `casts()` WITHOUT calling the parent, and never mentions the + * parent's credential column. Dispatch runs this body only, so `password` + * carries no cast at all — flagging it is a false positive on a model whose + * every write is equally raw. + */ +class ReplacingOverride extends MethodBase +{ + /** + * @return array + */ + protected function casts(): array + { + return ['nickname' => 'string']; + } +} + +/** Same replacement, but naming the parent's column — a key collision masks a + * merge-everything reading's error here, which is why the row above exists. */ +class ReplacingOverrideSameColumn extends MethodBase +{ + /** + * @return array + */ + protected function casts(): array + { + return ['password' => 'string']; + } +} + +/** + * The idiomatic pass-through: carries no literal and needs none. + */ +class PassThroughOverride extends MethodBase +{ + /** + * @return array + */ + protected function casts(): array + { + return parent::casts(); + } +} + +class ComposingOverride extends MethodBase +{ + /** + * @return array + */ + protected function casts(): array + { + return array_merge(parent::casts(), ['api_token' => 'encrypted']); + } +} + +class SpreadingOverride extends MethodBase +{ + /** + * @return array + */ + protected function casts(): array + { + return [...parent::casts(), 'api_token' => 'encrypted']; + } +} + +class PropertyBase extends Model +{ + /** @var array */ + protected $casts = ['password' => 'hashed']; +} + +/** + * A property IS inherited, unlike a replaced `casts()` body. + */ +class InheritedProperty extends PropertyBase {} + +/** + * Redeclares the property. PHP keeps ONE declaration — this one — so the + * parent's `password` entry does not exist at runtime. + */ +class RedeclaringProperty extends PropertyBase +{ + /** @var array */ + protected $casts = ['nickname' => 'string']; +} + +/** + * Both forms on one class. `array_merge($this->casts, $this->casts())` puts the + * METHOD second, so the method wins whichever order the file declares them in. + * + * Only ONE order appears here, and not by choice: the canonical Pint config + * carries `ordered_class_elements`, which rewrites a method-before-property + * class into this shape — a second fixture in the other order was silently + * reformatted into a byte-identical twin of this one, with every gate green. + * `TraitMethodAndClassProperty` below carries the same disagreement across a + * trait boundary, where no formatter can collapse it, and the implementation + * reads the two forms into separate buckets rather than folding them in + * statement order, so source order is structurally unreachable. + */ +class PropertyThenMethod extends Model +{ + /** @var array */ + protected $casts = ['password' => 'hashed']; + + /** + * @return array + */ + protected function casts(): array + { + return ['password' => 'string']; + } +} + +class TraitMethodAndClassProperty extends Model +{ + use DeclaresPlainPasswordViaMethod; + + /** @var array */ + protected $casts = ['password' => 'hashed']; +} + +/** A class-declared `casts()` beats the trait's, and the trait's body never + * runs — so the trait's credential column is not cast here. */ +class TraitMethodOverridden extends Model +{ + use DeclaresSecretViaMethod; + + /** + * @return array + */ + protected function casts(): array + { + return ['nickname' => 'string']; + } +} + +class TraitMethodInherited extends Model +{ + use DeclaresSecretViaMethod; +} + +class GrandMethodBase extends Model +{ + /** + * @return array + */ + protected function casts(): array + { + return ['grand_secret' => 'hashed']; + } +} + +/** + * Composes with the parent AND shadows the parent's credential column. The + * nearer declaration wins, so `password` is a plain string here. + * + * Pins the merge DIRECTION of the parent-call chain rather than merely that a + * chain exists — a mutation removing the reversal survived every other row. + */ +class ComposingOverrideShadowingParent extends MethodBase +{ + /** + * @return array + */ + protected function casts(): array + { + return array_merge(parent::casts(), ['password' => 'string']); + } +} + +/** + * Declares nothing, so dispatch passes straight through it. + */ +class SilentMiddle extends MethodBase {} + +/** + * TWO hops from the declaration: dispatch skips the silent middle entirely and + * runs `MethodBase::casts()`. Pins that the ancestry walk is not capped at the + * first parent — every other row resolves within one hop. + */ +class TwoHopInherited extends SilentMiddle {} + +/** + * Replaces `casts()` and composes from a FOREIGN class's static `casts()`. Only + * `parent::casts()` extends the dispatch walk: a call that merely shares the + * method NAME is not a parent call, so the parent's `password` cast stays + * unreachable. Same method name on purpose — a detector keyed on the name alone + * passes every other row in this table. + */ +class ReplacingOverrideWithForeignStaticCall extends MethodBase +{ + /** + * @return array + */ + protected function casts(): array + { + return array_merge(ForeignCastSource::casts(), ['nickname' => 'string']); + } +} + +final class ForeignCastSource +{ + /** + * @return array + */ + public static function casts(): array + { + return ['unrelated' => 'string']; + } +} + +/** + * Cuts the chain: the grandparent's body is unreachable from here down. + */ +class MidReplacing extends GrandMethodBase +{ + /** + * @return array + */ + protected function casts(): array + { + return ['mid_plain' => 'string']; + } +} + +/** + * Composes with `parent::casts()` — but the parent it reaches is the one that + * CUT the chain, so the grandparent's credential column stays unreachable. The + * row that separates "walk up on a parent call" from "walk the whole ancestry". + */ +class LeafComposingOverMidReplacing extends MidReplacing +{ + /** + * @return array + */ + protected function casts(): array + { + return array_merge(parent::casts(), ['leaf_secret' => 'hashed']); + } +} + +/** + * A trait ADAPTATION: `insteadof` excludes the hashed declaration, so the body + * PHP dispatches is the plain one and `password` carries no credential cast. + * + * The excluded trait is listed FIRST on purpose — a first-match walk over the + * imported traits picks it and reports a cast that does not exist. Reflection + * resolves the adaptation, which is why the method half is resolved that way and + * the property half is not: a property has no adaptation, and a conflicting one + * is a PHP fatal rather than an ambiguity. + */ +class TraitMethodExcludedByInsteadOf extends Model +{ + use DeclaresHashedPasswordViaTraitToBeExcluded, DeclaresPlainPasswordViaTraitToBeExcluded { + DeclaresPlainPasswordViaTraitToBeExcluded::casts insteadof DeclaresHashedPasswordViaTraitToBeExcluded; + } +} + +/** + * Calls `parent::casts()` and THROWS THE RESULT AWAY. None of the parent's map + * reaches the returned value, so `password` is not cast here — walking upward on + * the strength of the call merely appearing in the body invents a cast. + */ +class DiscardedParentCastsCall extends MethodBase +{ + /** + * @return array + */ + protected function casts(): array + { + parent::casts(); + + return ['nickname' => 'string']; + } +} + +/** + * Captures `parent::casts()` in a VARIABLE before composing it. The result IS + * used, so the parent's map is part of the answer — the row that keeps the + * discarded-call fix from turning into a fail-open on a credential column. + */ +class ParentCastsCapturedInVariable extends MethodBase +{ + /** + * @return array + */ + protected function casts(): array + { + $inherited = parent::casts(); + + return array_merge($inherited, ['api_token' => 'encrypted']); + } +} + +/** + * Two returns disagreeing about the SAME column. No single call produces both, + * so every branch is read and the CREDENTIAL cast wins: a column some branch + * hashes is hashed on that path, and source order is not a fact about which + * branch runs. + */ +class ConditionalReturnsDisagreeing extends Model +{ + /** + * @return array + */ + protected function casts(): array + { + if ($this->exists) { + return ['password' => 'string']; + } + + return ['password' => 'hashed']; + } +} + +/** + * `mergeCasts()` at construct time — a real Laravel API, and an accepted false + * NEGATIVE: no declaration exists to read. Excluded from the truth table + * because its effective map only exists after construction. See the rule's + * out-of-scope list for the fleet measurement behind the ruling. + */ +class MergesCastsInConstructor extends Model +{ + /** + * @param array $attributes + */ + public function __construct(array $attributes = []) + { + parent::__construct($attributes); + + $this->mergeCasts(['password' => 'hashed']); + } +} diff --git a/tests/Fixtures/CredentialCastBypass/CastDispatchWrites.php b/tests/Fixtures/CredentialCastBypass/CastDispatchWrites.php new file mode 100644 index 0000000..3e232b5 --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/CastDispatchWrites.php @@ -0,0 +1,173 @@ +update(['password' => 'raw']); + } + + public function inheritedMethod(): void + { + InheritedMethod::query()->update(['password' => 'raw']); + } + + public function replacingOverride(): void + { + ReplacingOverride::query()->update(['password' => 'raw']); + } + + public function replacingOverrideSameColumn(): void + { + ReplacingOverrideSameColumn::query()->update(['password' => 'raw']); + } + + public function passThroughOverride(): void + { + PassThroughOverride::query()->update(['password' => 'raw']); + } + + public function composingOverride(): void + { + ComposingOverride::query()->update(['password' => 'raw', 'api_token' => 'raw']); + } + + public function spreadingOverride(): void + { + SpreadingOverride::query()->update(['password' => 'raw', 'api_token' => 'raw']); + } + + public function composingOverrideShadowingParent(): void + { + ComposingOverrideShadowingParent::query()->update(['password' => 'raw']); + } + + public function twoHopInherited(): void + { + TwoHopInherited::query()->update(['password' => 'raw']); + } + + public function replacingOverrideWithForeignStaticCall(): void + { + ReplacingOverrideWithForeignStaticCall::query()->update(['password' => 'raw']); + } + + public function traitMethodExcludedByInsteadOf(): void + { + TraitMethodExcludedByInsteadOf::query()->update(['password' => 'raw']); + } + + public function discardedParentCastsCall(): void + { + DiscardedParentCastsCall::query()->update(['password' => 'raw']); + } + + public function parentCastsCapturedInVariable(): void + { + ParentCastsCapturedInVariable::query()->update(['password' => 'raw', 'api_token' => 'raw']); + } + + public function conditionalReturnsDisagreeing(): void + { + ConditionalReturnsDisagreeing::query()->update(['password' => 'raw']); + } + + public function propertyBase(): void + { + PropertyBase::query()->update(['password' => 'raw']); + } + + public function inheritedProperty(): void + { + InheritedProperty::query()->update(['password' => 'raw']); + } + + public function redeclaringProperty(): void + { + RedeclaringProperty::query()->update(['password' => 'raw']); + } + + public function propertyThenMethod(): void + { + PropertyThenMethod::query()->update(['password' => 'raw']); + } + + public function traitMethodAndClassProperty(): void + { + TraitMethodAndClassProperty::query()->update(['password' => 'raw']); + } + + public function traitMethodOverridden(): void + { + TraitMethodOverridden::query()->update(['trait_method_secret' => 'raw']); + } + + public function traitMethodInherited(): void + { + TraitMethodInherited::query()->update(['trait_method_secret' => 'raw']); + } + + public function grandMethodBase(): void + { + GrandMethodBase::query()->update(['grand_secret' => 'raw']); + } + + public function midReplacing(): void + { + MidReplacing::query()->update(['grand_secret' => 'raw']); + } + + public function leafComposingOverMidReplacing(): void + { + LeafComposingOverMidReplacing::query()->update([ + 'grand_secret' => 'raw', + 'mid_plain' => 'raw', + 'leaf_secret' => 'raw', + ]); + } + + public function mergesCastsInConstructor(): void + { + MergesCastsInConstructor::query()->update(['password' => 'raw']); + } +} diff --git a/tests/Fixtures/CredentialCastBypass/CleanWrites.php b/tests/Fixtures/CredentialCastBypass/CleanWrites.php new file mode 100644 index 0000000..e4a497a --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/CleanWrites.php @@ -0,0 +1,143 @@ +password = 'plaintext'; + $user->save(); + } + + /** + * `Model::update()` fills through `setAttribute()`, so casts fire. + */ + public function modelUpdate(User $user): void + { + $user->update(['password' => 'plaintext']); + } + + /** + * `Model::create()` instantiates and saves — casts fire. + */ + public function modelCreate(): void + { + User::create(['password' => 'plaintext']); + } + + /** + * Builder `create()` also builds and saves a model — casts fire. + */ + public function builderCreate(): void + { + User::query()->create(['password' => 'plaintext']); + } + + /** + * Builder `updateOrCreate()` routes through the model too. + */ + public function builderUpdateOrCreate(): void + { + User::query()->updateOrCreate(['email' => 'a@b.c'], ['password' => 'plaintext']); + } + + /** + * A builder write to columns carrying no credential cast. + */ + public function builderWriteToNonCastColumn(): void + { + User::query()->update(['login_count' => 3]); + } + + /** + * A model with no credential casts at all. + */ + public function builderWriteOnUncastModel(): void + { + Article::query()->update(['published_at' => 'now']); + } + + /** + * `DB::table()` carries no model in its type, and no table is mapped here. + */ + public function unmappedRawTableWrite(): void + { + DB::table('users')->update(['password' => 'plaintext']); + } + + /** + * The child REDECLARES the abstract parent's `passphrase` cast as `string`. + * Child wins, so this is no longer a credential column. Pins the merge + * DIRECTION, not merely the fact that a merge happens. + */ + public function childOverridesInheritedCredentialCast(): void + { + OverridingVault::query()->update(['passphrase' => 'plain']); + } + + /** + * Cast values that only look like credential casts (`encryptedish`). + */ + public function nearMissCastNames(): void + { + NearMissCastModel::query()->update(['blob' => 'x', 'digest' => 'y']); + } + + /** + * The class REDECLARES the trait's `trait_secret` cast as `string`. PHP + * resolves a class-declared member over a trait-imported one, so this is no + * longer a credential column. + */ + public function classDeclarationBeatsTraitImportedCast(): void + { + TraitOverriddenCastModel::query()->update(['trait_secret' => 'plain']); + } + + /** + * A payload of unknown shape — keys are not statically known. + */ + public function dynamicPayload(string $column, string $value): void + { + User::query()->update([$column => $value]); + } + + /** + * The non-credential entry from the SAME composed cast map. Reading a + * composed declaration must not turn every column on that model into a + * finding — this is the false-positive direction of crit round 2, issue 1. + */ + public function nonCredentialColumnOfAComposedCastMap(): void + { + ComposedCastModel::query()->update(['composed_count' => 3]); + } + + /** + * The two literals a composed cast map must NOT contribute: a value nested + * inside the map itself, and a literal inside a callback passed as an + * argument. Both would be findings on columns the model never casts. + */ + public function nestedAndCallbackLiteralsAreNotCastPairs(): void + { + NestedLiteralCastModel::query()->update(['nested_secret' => 'x', 'decoy_secret' => 'y']); + } +} diff --git a/tests/Fixtures/CredentialCastBypass/ComposedCastModel.php b/tests/Fixtures/CredentialCastBypass/ComposedCastModel.php new file mode 100644 index 0000000..e57b592 --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/ComposedCastModel.php @@ -0,0 +1,28 @@ + + */ + protected function casts(): array + { + return array_merge(parent::casts(), [ + 'composed_secret' => 'hashed', + 'composed_count' => 'integer', + ]); + } +} diff --git a/tests/Fixtures/CredentialCastBypass/ComposesHashedSecret.php b/tests/Fixtures/CredentialCastBypass/ComposesHashedSecret.php new file mode 100644 index 0000000..41c9c25 --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/ComposesHashedSecret.php @@ -0,0 +1,14 @@ + */ + private const array CASTS = ['constant_secret' => 'hashed']; + + protected $table = 'constant_cast_models'; + + /** + * @return array + */ + protected function casts(): array + { + return self::CASTS; + } +} diff --git a/tests/Fixtures/CredentialCastBypass/ConstantCastPropertyModel.php b/tests/Fixtures/CredentialCastBypass/ConstantCastPropertyModel.php new file mode 100644 index 0000000..7a07210 --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/ConstantCastPropertyModel.php @@ -0,0 +1,26 @@ + */ + private const array CASTS = ['constant_property_secret' => 'encrypted']; + + protected $table = 'constant_cast_property_models'; + + /** @var array */ + protected $casts = self::CASTS; +} diff --git a/tests/Fixtures/CredentialCastBypass/HasEncryptedNotesProperty.php b/tests/Fixtures/CredentialCastBypass/HasEncryptedNotesProperty.php new file mode 100644 index 0000000..60e7d08 --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/HasEncryptedNotesProperty.php @@ -0,0 +1,29 @@ + */ + protected $casts = [ + 'trait_notes' => 'encrypted', + ]; +} diff --git a/tests/Fixtures/CredentialCastBypass/HasHashedSecret.php b/tests/Fixtures/CredentialCastBypass/HasHashedSecret.php new file mode 100644 index 0000000..cfab058 --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/HasHashedSecret.php @@ -0,0 +1,24 @@ + + */ + protected function casts(): array + { + return [ + 'trait_secret' => 'hashed', + ]; + } +} diff --git a/tests/Fixtures/CredentialCastBypass/NearMissCastModel.php b/tests/Fixtures/CredentialCastBypass/NearMissCastModel.php new file mode 100644 index 0000000..0eb571d --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/NearMissCastModel.php @@ -0,0 +1,28 @@ + + */ + protected function casts(): array + { + return [ + 'blob' => 'encryptedish', + 'digest' => 'hashedish', + ]; + } +} diff --git a/tests/Fixtures/CredentialCastBypass/NestedLiteralCastModel.php b/tests/Fixtures/CredentialCastBypass/NestedLiteralCastModel.php new file mode 100644 index 0000000..d5c43dc --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/NestedLiteralCastModel.php @@ -0,0 +1,45 @@ + cast` pairs: + * + * - a literal nested INSIDE the cast map (`'meta' => ['nested_secret' => …]`). + * Collecting array literals from anywhere inside a returned expression must + * stop at an array it already collected, or a nested value becomes a second + * cast map and every column named in it turns into a finding. + * - a literal inside a callback passed as an argument. It is that callback's + * return value, not this model's cast map. + * + * `real_secret` IS a cast and must still fire, so the fixture cannot pass by the + * rule simply going blind on this model. + */ +class NestedLiteralCastModel extends AbstractCredentialHolder +{ + protected $table = 'nested_literal_cast_models'; + + /** + * @return array + */ + protected function casts(): array + { + return array_merge(parent::casts(), $this->decorate(static fn(): array => ['decoy_secret' => 'hashed']), [ + 'real_secret' => 'hashed', + 'meta' => ['nested_secret' => 'hashed'], + ]); + } + + /** + * @param callable(): array $callback + * + * @return array + */ + private function decorate(callable $callback): array + { + return []; + } +} diff --git a/tests/Fixtures/CredentialCastBypass/OverridingVault.php b/tests/Fixtures/CredentialCastBypass/OverridingVault.php new file mode 100644 index 0000000..ed91740 --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/OverridingVault.php @@ -0,0 +1,26 @@ + + */ + protected function casts(): array + { + return [ + 'passphrase' => 'string', + ]; + } +} diff --git a/tests/Fixtures/CredentialCastBypass/RawTableWrites.php b/tests/Fixtures/CredentialCastBypass/RawTableWrites.php new file mode 100644 index 0000000..229b2c4 --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/RawTableWrites.php @@ -0,0 +1,57 @@ +update(['password' => 'plaintext']); + } + + public function mappedTableWithIntermediateChainHops(): void + { + DB::table('users')->where('id', 1)->limit(1)->update(['api_token' => 'raw']); + } + + public function mappedTableNonCastColumn(): void + { + DB::table('users')->update(['login_count' => 1]); + } + + public function connectionScopedTableWrite(): void + { + DB::connection('mysql')->table('users')->update(['password' => 'plaintext']); + } + + /** + * Hoisting the builder into a variable defeats the chain walk — the + * variable's TYPE is a bare `Query\Builder` carrying no table name, so + * there is nothing left to resolve. Documented, tested silence. + */ + public function hoistedTableBuilder(): void + { + $query = DB::table('users'); + + $query->update(['password' => 'plaintext']); + } + + public function unmappedTable(): void + { + DB::table('articles')->update(['password' => 'plaintext']); + } + + public function nonLiteralTableName(string $table): void + { + DB::table($table)->update(['password' => 'plaintext']); + } +} diff --git a/tests/Fixtures/CredentialCastBypass/SingleUncastWrite.php b/tests/Fixtures/CredentialCastBypass/SingleUncastWrite.php new file mode 100644 index 0000000..cfe7bbc --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/SingleUncastWrite.php @@ -0,0 +1,20 @@ +update(['published_at' => 'now']); + } +} diff --git a/tests/Fixtures/CredentialCastBypass/SpreadCastModel.php b/tests/Fixtures/CredentialCastBypass/SpreadCastModel.php new file mode 100644 index 0000000..0f5c2e3 --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/SpreadCastModel.php @@ -0,0 +1,24 @@ + + */ + protected function casts(): array + { + return [...parent::casts(), 'spread_secret' => 'encrypted']; + } +} diff --git a/tests/Fixtures/CredentialCastBypass/TraitCastModel.php b/tests/Fixtures/CredentialCastBypass/TraitCastModel.php new file mode 100644 index 0000000..438239b --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/TraitCastModel.php @@ -0,0 +1,19 @@ + + */ + protected function casts(): array + { + return [ + 'trait_secret' => 'string', + ]; + } +} diff --git a/tests/Fixtures/CredentialCastBypass/UninterpretableCastWrites.php b/tests/Fixtures/CredentialCastBypass/UninterpretableCastWrites.php new file mode 100644 index 0000000..51c5cd4 --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/UninterpretableCastWrites.php @@ -0,0 +1,28 @@ +update(['unrelated' => 'value']); + } + + public function propertyDefaultingToAClassConstant(): void + { + ConstantCastPropertyModel::query()->update(['unrelated' => 'value']); + } +} diff --git a/tests/Fixtures/CredentialCastBypass/User.php b/tests/Fixtures/CredentialCastBypass/User.php new file mode 100644 index 0000000..fa8d80a --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/User.php @@ -0,0 +1,38 @@ + + */ + public function apiKeys(): HasMany + { + return $this->hasMany(ApiKey::class); + } + + /** + * @return array + */ + protected function casts(): array + { + return [ + 'password' => 'hashed', + 'api_token' => 'encrypted', + 'recovery_codes' => 'encrypted:array', + 'login_count' => 'integer', + ]; + } +} diff --git a/tests/Fixtures/CredentialCastBypass/Vault.php b/tests/Fixtures/CredentialCastBypass/Vault.php new file mode 100644 index 0000000..3ecec8e --- /dev/null +++ b/tests/Fixtures/CredentialCastBypass/Vault.php @@ -0,0 +1,10 @@ + + */ +final class ForbidCredentialCastBypassRuleTest extends RuleTestCase +{ + private const string MESSAGE = "Attribute '%s' on %s carries the '%s' cast, but %s() is a query-builder write that bypasses the cast and stores the raw value. Write it through the model path instead (assign the attribute and save the model)."; + + private const string BUILDER_WRITES = __DIR__ . '/../Fixtures/CredentialCastBypass/BuilderWrites.php'; + + private const string CLEAN_WRITES = __DIR__ . '/../Fixtures/CredentialCastBypass/CleanWrites.php'; + + private const string RAW_TABLE_WRITES = __DIR__ . '/../Fixtures/CredentialCastBypass/RawTableWrites.php'; + + private const string SINGLE_UNCAST_WRITE = __DIR__ . '/../Fixtures/CredentialCastBypass/SingleUncastWrite.php'; + + private const string UNREADABLE_MESSAGE = 'Cannot verify this write against %s: the PHP declaring %s could not be located or parsed, so the credential-cast map is incomplete and a hashed/encrypted column in this payload would go unreported. Fix the source, or suppress forbidCredentialCastBypass.modelSourceUnreadable here if the write is known safe.'; + + private const string INCOMPLETE_MESSAGE = 'Cannot verify this write against %s: %s declares casts in a form this rule cannot read (a casts() return or a $casts default carrying no array literal, such as a class constant or a helper call), so the credential-cast map is incomplete and a hashed/encrypted column in this payload would go unreported. Restate the credential columns as literal string pairs, or suppress forbidCredentialCastBypass.castMapIncomplete here if the write is known safe.'; + + private const string CONFIGURED_MODEL_MISSING_MESSAGE = 'Cannot verify this write: credentialCastTableModels maps this table to %s, which does not exist, so no credential-cast map could be read and a hashed/encrypted column in this payload would go unreported. Fix the FQCN in the parameter, or remove the mapping if the table is no longer covered.'; + + private const string UNINTERPRETABLE_CAST_WRITES = __DIR__ . '/../Fixtures/CredentialCastBypass/UninterpretableCastWrites.php'; + + private const string CAST_DISPATCH_WRITES = __DIR__ . '/../Fixtures/CredentialCastBypass/CastDispatchWrites.php'; + + private const string DISPATCH_NAMESPACE = 'App\Models\CredentialCastBypass\Dispatch\\'; + + /** + * Verbs that postdate this package's MINIMUM Laravel, mapped to the release + * that introduced them. Absent on the lowest-supported leg and present on + * the newest, and the test asserts both directions rather than treating an + * absence as permission to stop checking. + * + * ENUMERATED rather than guessed, against both supported majors: every one + * of the rule's 17 verbs resolves on `illuminate/database` 13.20, and + * exactly these three are absent on 12.68. `updateFrom`, + * `incrementOrCreate` and the non-`Each` quiet variants are already present + * on 12, which is why probing one quiet verb and generalising from it was + * wrong. + * + * @var array + */ + private const array VERSION_GATED_METHODS = [ + 'insertOrIgnoreReturning' => 'illuminate/database 13.x', + 'incrementEachQuietly' => 'illuminate/database 13.x', + 'decrementEachQuietly' => 'illuminate/database 13.x', + ]; + + /** The one write site in the shape fixture with no table row — see its test. */ + private const string DOCUMENTED_FALSE_NEGATIVE = 'MergesCastsInConstructor'; + + /** + * The cast-declaration shape table, in the fixture's own order. + * + * `payload` is fixture CONTENT — the columns that shape's write names. + * `naive` is what a merge-every-declaration reading of the ancestry would + * flag, recorded ONLY as the denominator for + * `testTheRuleAgreesWithPhpsOwnCastResolutionForEveryDeclarationShape`: it + * is never asserted as behaviour, and its job is to fail loudly if the table + * ever stops distinguishing the two readings. The EXPECTATION is computed + * from PHP, never written here. + * + * @var array, naive: list}> + */ + private const array CAST_DISPATCH_TABLE = [ + // Both readings agree: the declaration that runs is the only one there is. + 'LeafMethod' => ['payload' => ['password'], 'naive' => ['password']], + 'InheritedMethod' => ['payload' => ['password'], 'naive' => ['password']], + // Dispatch stops at the override, so the parent's `password` cast does + // not exist — the merge reading invents it. + 'ReplacingOverride' => ['payload' => ['password'], 'naive' => ['password']], + // The same replacement naming the parent's own column: a key collision + // hides the merge reading's error, which is why the row above exists. + 'ReplacingOverrideSameColumn' => ['payload' => ['password'], 'naive' => []], + 'PassThroughOverride' => ['payload' => ['password'], 'naive' => ['password']], + 'ComposingOverride' => ['payload' => ['password', 'api_token'], 'naive' => ['password', 'api_token']], + 'SpreadingOverride' => ['payload' => ['password', 'api_token'], 'naive' => ['password', 'api_token']], + // Nearest wins on a shared column: the leaf downgrades what it composed. + 'ComposingOverrideShadowingParent' => ['payload' => ['password'], 'naive' => ['password']], + // Two hops from the declaration, through a silent middle. + 'TwoHopInherited' => ['payload' => ['password'], 'naive' => ['password']], + // A foreign static call is not `parent::casts()`, so the walk stops. + 'ReplacingOverrideWithForeignStaticCall' => ['payload' => ['password'], 'naive' => ['password']], + // Trait adaptation: a first-match walk over the imported traits picks the + // EXCLUDED declaration and reports a cast the model does not have. + 'TraitMethodExcludedByInsteadOf' => ['payload' => ['password'], 'naive' => ['password']], + // A `parent::casts()` whose result is discarded contributes nothing. + 'DiscardedParentCastsCall' => ['payload' => ['password'], 'naive' => ['password']], + // ...but one captured in a variable does — the fail-open guard. + 'ParentCastsCapturedInVariable' => ['payload' => ['password', 'api_token'], 'naive' => ['password', 'api_token']], + // Branches disagreeing on one column: the credential cast wins. + 'ConditionalReturnsDisagreeing' => ['payload' => ['password'], 'naive' => ['password']], + 'PropertyBase' => ['payload' => ['password'], 'naive' => ['password']], + // A property IS inherited, unlike a replaced method body. + 'InheritedProperty' => ['payload' => ['password'], 'naive' => ['password']], + // PHP keeps ONE property declaration; the merge reading keeps both. + 'RedeclaringProperty' => ['payload' => ['password'], 'naive' => ['password']], + // `array_merge($this->casts, $this->casts())` — the method always wins, + // whichever form the author wrote first. + 'PropertyThenMethod' => ['payload' => ['password'], 'naive' => []], + // The method half wins even when the method comes from a TRAIT and the + // property from the class — a shape no formatter can reorder away. + 'TraitMethodAndClassProperty' => ['payload' => ['password'], 'naive' => ['password']], + // A class-declared `casts()` means the trait's body never runs. + 'TraitMethodOverridden' => ['payload' => ['trait_method_secret'], 'naive' => ['trait_method_secret']], + 'TraitMethodInherited' => ['payload' => ['trait_method_secret'], 'naive' => ['trait_method_secret']], + 'GrandMethodBase' => ['payload' => ['grand_secret'], 'naive' => ['grand_secret']], + 'MidReplacing' => ['payload' => ['grand_secret'], 'naive' => ['grand_secret']], + // Composes with a parent that CUT the chain: walking up on a parent call + // is not walking the whole ancestry. + 'LeafComposingOverMidReplacing' => [ + 'payload' => ['grand_secret', 'mid_plain', 'leaf_secret'], + 'naive' => ['grand_secret', 'leaf_secret'], + ], + ]; + + /** + * Table-to-model map used by the `DB::table()` tests. Left null so the + * default (empty map, `DB::table()` silent) is what every other test sees. + * + * @var array|null + */ + private ?array $tableModelOverride = null; + + /** + * When set, `getRule()` injects a parser that throws for any file whose path + * ends with this suffix — the only way to exercise the unreadable-source + * branch without shipping a syntactically broken fixture (which would break + * the classmap for the whole suite). + */ + private ?string $unparsableFileSuffix = null; + + // ---------------------------------------------------------------- RED --- + + public function testHashedCastColumnInBuilderUpdateIsFlagged(): void + { + $this->analyse([self::BUILDER_WRITES], [ + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'update'), 24], + [sprintf(self::MESSAGE, 'api_token', 'App\Models\CredentialCastBypass\User', 'encrypted', 'update'), 29], + [sprintf(self::MESSAGE, 'recovery_codes', 'App\Models\CredentialCastBypass\User', 'encrypted:array', 'update'), 34], + [sprintf(self::MESSAGE, 'secret', 'App\Models\CredentialCastBypass\ApiKey', 'encrypted', 'update'), 39], + [sprintf(self::MESSAGE, 'passphrase', 'App\Models\CredentialCastBypass\Vault', 'hashed', 'update'), 44], + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'update'), 51], + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'insert'), 56], + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'upsert'), 64], + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'updateOrInsert'), 69], + [sprintf(self::MESSAGE, 'secret', 'App\Models\CredentialCastBypass\ApiKey', 'encrypted', 'update'), 74], + [sprintf(self::MESSAGE, 'trait_secret', 'App\Models\CredentialCastBypass\TraitCastModel', 'hashed', 'update'), 79], + [sprintf(self::MESSAGE, 'trait_notes', 'App\Models\CredentialCastBypass\TraitCastModel', 'encrypted', 'update'), 84], + [sprintf(self::MESSAGE, 'api_token', 'App\Models\CredentialCastBypass\User', 'encrypted', 'update'), 89], + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'update'), 89], + // crit round 2, issue 1 — the leaf's OWN cast, added through + // `array_merge(parent::casts(), …)`. Silent before the fix while + // line 97's inherited cast fired, so the same model was + // half-enforced. + [sprintf(self::MESSAGE, 'composed_secret', 'App\Models\CredentialCastBypass\ComposedCastModel', 'hashed', 'update'), 94], + [sprintf(self::MESSAGE, 'passphrase', 'App\Models\CredentialCastBypass\ComposedCastModel', 'hashed', 'update'), 99], + [sprintf(self::MESSAGE, 'spread_secret', 'App\Models\CredentialCastBypass\SpreadCastModel', 'encrypted', 'update'), 104], + // Composed maps must contribute their OWN pairs and nothing else — + // the nested and callback literals on this model are pinned clean in + // testModelPathAndNonCredentialWritesAreClean. + [sprintf(self::MESSAGE, 'real_secret', 'App\Models\CredentialCastBypass\NestedLiteralCastModel', 'hashed', 'update'), 109], + // Verbs that were on the list with no site of their own, so a + // regression dropping either would not have shown up anywhere. + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'insertOrIgnore'), 119], + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'insertGetId'), 124], + // The increment family: the counter column is innocent, the EXTRA + // payload is an ordinary uncast write that reaches SQL through + // `update(array_merge($columns, $extra))`. + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'increment'), 135], + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'decrement'), 140], + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'incrementEach'), 145], + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'decrementEach'), 150], + // A NAMED argument sits at a different index than its parameter's + // position, so a position-only reading is silent here. + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'increment'), 164], + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'update'), 169], + // A named argument AFTER the payload must not blind the payload's own + // positional read. + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'upsert'), 178], + // Postgres-only payload writes, forwarded by Eloquent's __call. + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'updateFrom'), 188], + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'insertOrIgnoreReturning'), 193], + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'incrementOrCreate'), 203], + // MODEL receivers — the one family where the model path bypasses + // casts, so the receiver type gate must not exclude them. + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'increment'), 215], + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'decrementEach'), 220], + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'incrementQuietly'), 225], + // Union receiver, castless branch first: reading only the first + // branch reports nothing while the other branch writes plaintext. + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'update'), 239], + ]); + } + + public function testRawTableWriteIsFlaggedOnlyForMappedTables(): void + { + $this->tableModelOverride = ['users' => 'App\Models\CredentialCastBypass\User']; + + $this->analyse([self::RAW_TABLE_WRITES], [ + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'update'), 18], + [sprintf(self::MESSAGE, 'api_token', 'App\Models\CredentialCastBypass\User', 'encrypted', 'update'), 23], + [sprintf(self::MESSAGE, 'password', 'App\Models\CredentialCastBypass\User', 'hashed', 'update'), 33], + ]); + } + + // -------------------------------------------------------------- GREEN --- + + public function testModelPathAndNonCredentialWritesAreClean(): void + { + $this->analyse([self::CLEAN_WRITES], []); + } + + public function testRawTableWritesAreSilentWithTheDefaultEmptyMap(): void + { + $this->analyse([self::RAW_TABLE_WRITES], []); + } + + /** + * crit round 1, issue 3. A declaring source that cannot be parsed yields the + * same empty cast set as a model that genuinely declares none, so treating + * the two alike makes the rule fail OPEN — silently passing every write to a + * model it can no longer read. Doctrine requires MISSING to be a distinct + * outcome from FAILED, so this reports under its own identifier. + * + * Both directions in ONE fixture: the same write site is silent under a + * working parser (below) and loud when the model's PHP cannot be read. + */ + public function testUnreadableModelSourceIsReportedRatherThanSilentlySkipped(): void + { + $this->unparsableFileSuffix = 'CredentialCastBypass/Article.php'; + + $this->analyse([self::SINGLE_UNCAST_WRITE], [ + [ + sprintf( + self::UNREADABLE_MESSAGE, + 'App\Models\CredentialCastBypass\Article', + 'App\Models\CredentialCastBypass\Article', + ), + 18, + ], + ]); + } + + public function testTheSameWriteIsSilentWhenTheModelSourceParses(): void + { + $this->analyse([self::SINGLE_UNCAST_WRITE], []); + } + + /** + * crit round 2, issue 1 — the OTHER direction. `declaredCasts()` requires an + * array literal; a `casts()` return or `$casts` default that carries none + * (`return self::CASTS;`) was read as "declares nothing", so the write + * passed silently. The source is readable, so `modelSourceUnreadable` never + * fired either: the declaration SHAPE is what cannot be read, and it gets + * its own identifier because the remediation is different. + * + * Reported regardless of payload, like the unreadable-source diagnostic — + * neither payload below names a credential column, which is the point: with + * an incomplete map the rule cannot claim the payload is clean. + */ + public function testCastDeclarationsCarryingNoArrayLiteralAreReportedRatherThanReadAsCastless(): void + { + $this->analyse([self::UNINTERPRETABLE_CAST_WRITES], [ + [ + sprintf( + self::INCOMPLETE_MESSAGE, + 'App\Models\CredentialCastBypass\ConstantCastModel', + 'App\Models\CredentialCastBypass\ConstantCastModel', + ), + 21, + ], + [ + sprintf( + self::INCOMPLETE_MESSAGE, + 'App\Models\CredentialCastBypass\ConstantCastPropertyModel', + 'App\Models\CredentialCastBypass\ConstantCastPropertyModel', + ), + 26, + ], + ]); + } + + /** + * crit round 2, issue 2. `hasClass()` returning false was answered with the + * same empty cast set as "this table is not mapped", so a typo or a stale + * rename in `credentialCastTableModels` silently and permanently disarmed + * the rule for that table — the exact fail-open shape the unreadable-source + * identifier exists to prevent, arriving through configuration instead of + * source. + * + * The three `users` sites fire; `articles` (unmapped) and the non-literal + * table argument stay silent, so this also pins that a bad mapping does not + * spray the diagnostic over tables it was never configured for. + */ + public function testAMistypedConfiguredModelIsReportedRatherThanTreatedAsUnmapped(): void + { + $this->tableModelOverride = ['users' => 'App\Models\CredentialCastBypass\Uzer']; + + $message = sprintf(self::CONFIGURED_MODEL_MISSING_MESSAGE, 'App\Models\CredentialCastBypass\Uzer'); + + $this->analyse([self::RAW_TABLE_WRITES], [ + [$message, 18], + [$message, 23], + [$message, 28], + [$message, 33], + ]); + } + + // ------------------------------------------------- DECLARATION SHAPES --- + + /** + * The rule's cast map must equal the one PHP and Laravel would actually + * build, for every shape a consumer model can declare casts in. Nothing here + * asserts a hand-written expectation: for each shape the expectation is + * COMPUTED from PHP — the property default PHP itself resolved + * (`getDefaultProperties()`, which honours a redeclaration replacing its + * parent's and a class default replacing a trait's) merged under a real + * virtual dispatch of `casts()`, in the order + * `HasAttributes::initializeHasAttributes()` merges them. + * + * Why the expectation is computed rather than written: a hand-written one + * encodes the author's reading of Laravel, which is exactly what was wrong. + * A merge-every-declaration reading of the ancestry, leaf wins, is wrong on + * NINE of these shapes — eight inventing a credential cast the model does not + * have, one calling a readable declaration unreadable — each masked in the + * older fixtures by a key collision. Resolving the method half by first match + * over the imported traits is wrong on two OTHERS (a trait `insteadof`, and a + * discarded `parent::casts()`), which is why shapes refuting both readings are + * kept: a table that only refutes an abandoned reading measures nothing. A rule + * that invents a credential cast blocks a consumer's CI on a correct write; + * on a security rule that spends the gate's authority faster than a missed + * catch. + * + * The `naive` column of the table records what that merge-everything reading + * would flag. It is never asserted as behaviour — it is the DENOMINATOR: if + * the two readings stopped disagreeing on most of these rows, this test + * would be measuring nothing, and the count assertion below fails rather + * than passing quietly. + */ + public function testTheRuleAgreesWithPhpsOwnCastResolutionForEveryDeclarationShape(): void + { + $rowsExpectingAnError = 0; + $rowsWhereTheTwoReadingsDisagree = 0; + + foreach (self::CAST_DISPATCH_TABLE as $model => $row) { + $flagged = array_keys($this->credentialCastsInPayloadOrder($model, $row['payload'])); + + if ($flagged !== []) { + $rowsExpectingAnError++; + } + + if ($flagged !== array_values(array_intersect($row['payload'], $row['naive']))) { + $rowsWhereTheTwoReadingsDisagree++; + } + } + + self::assertGreaterThanOrEqual( + 8, + $rowsExpectingAnError, + 'The shape table stopped expecting errors, so a rule reporting nothing at all would pass it.', + ); + + self::assertGreaterThanOrEqual( + 6, + $rowsWhereTheTwoReadingsDisagree, + 'The shape table no longer distinguishes PHP\'s resolution from a merge-every-declaration reading, so it can no longer catch the defect it exists for.', + ); + + $this->analyse([self::CAST_DISPATCH_WRITES], $this->expectedCastDispatchErrors()); + } + + /** + * `mergeCasts()` at construct time is an accepted false NEGATIVE: no + * declaration exists to read, so this write is silent even though the + * constructed model really does carry the cast. 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 that propagates a map the rule already reads, and + * `withCasts()` once, on a non-credential column. A diagnostic here would + * fire on neither a real bypass nor nothing at all: its only fleet target + * today is a false positive. + * + * Pinned so that changing it is a visible decision rather than a drift. + */ + public function testMergeCastsAtConstructTimeIsAnAcceptedFalseNegative(): void + { + $lines = $this->castDispatchWriteLines(); + + $reflection = new ReflectionClass(self::DISPATCH_NAMESPACE . self::DOCUMENTED_FALSE_NEGATIVE); + + // The declaration halves the rule CAN read are both empty here, which is + // why it stays silent — not because the fixture forgot to declare a cast. + self::assertSame([], $reflection->getDefaultProperties()['casts'] ?? null); + self::assertSame([], $this->credentialCastsAccordingToPhp($reflection->getName())); + self::assertArrayHasKey(self::DOCUMENTED_FALSE_NEGATIVE, $lines); + + $this->analyse([self::CAST_DISPATCH_WRITES], $this->expectedCastDispatchErrors()); + } + + /** + * Every payload slot names a parameter that EXISTS on the Laravel method, at + * the position the rule reads. + * + * The rule addresses a payload by name first and position second, so each + * slot is two claims about `illuminate/database`: that a parameter of that + * name exists, and that it sits at that index. A Laravel rename or a + * reordered signature would break the named lookup silently — the rule would + * simply stop seeing named payloads, with no error anywhere — and a shifted + * position would make it read the wrong argument. Neither shows up in any + * other test here, because the fixtures call these methods positionally with + * the arguments the rule already expects. + * + * Verbs are matched against whichever of the three receiver classes declares + * them; the increment family exists on more than one, and the parameter names + * agree, so the first hit is authoritative. + */ + public function testEveryPayloadSlotMatchesTheLaravelParameterItNames(): void + { + $declarers = [QueryBuilder::class, EloquentBuilder::class, Model::class]; + $checked = 0; + $skipped = []; + + foreach ($this->writeMethodSlots() as $method => $slots) { + $reflection = null; + + foreach ($declarers as $class) { + if ((new ReflectionClass($class))->hasMethod($method)) { + $reflection = (new ReflectionClass($class))->getMethod($method); + + break; + } + } + + if ($reflection === null) { + // An absent verb is only acceptable when it is one this package + // knows postdates its minimum Laravel. Anything else means the + // rule reads a payload from a method that does not exist. + self::assertArrayHasKey( + $method, + self::VERSION_GATED_METHODS, + sprintf( + 'The rule reads payloads from %s(), which no Laravel receiver class declares and which is not listed as version-gated.', + $method, + ), + ); + + $skipped[] = $method; + + continue; + } + + $parameters = $reflection->getParameters(); + + foreach ($slots as [$name, $position]) { + self::assertArrayHasKey( + $position, + $parameters, + sprintf('%s() has no parameter at position %d.', $method, $position), + ); + self::assertSame( + $name, + $parameters[$position]->getName(), + sprintf( + '%s() parameter %d is $%s, not $%s — the rule\'s named-argument lookup would silently never match.', + $method, + $position, + $parameters[$position]->getName(), + $name, + ), + ); + $checked++; + } + } + + // Reconciled, not a floor picked by hand: every slot of every present + // verb must have been checked. A hand-set minimum is calibrated on + // whichever Laravel the author happened to run, and silently wrong on + // the other — which is exactly how this assertion first failed. + $expected = 0; + + foreach ($this->writeMethodSlots() as $method => $slots) { + if (!in_array($method, $skipped, true)) { + $expected += count($slots); + } + } + + self::assertGreaterThan(0, $expected, 'The slot map is empty, so this test measures nothing.'); + self::assertSame( + $expected, + $checked, + 'Fewer slots were checked than the present verbs declare, so part of the map went unread.', + ); + + // On the NEWEST supported Laravel every verb must be present, so a + // rename there cannot hide behind the version gate — which is the only + // thing that would make skipping safe on the older leg. + if ($this->installedIlluminateMajor() >= 13) { + self::assertSame( + [], + $skipped, + 'These verbs are absent on the newest supported Laravel, so they were renamed or removed rather than merely postdating the minimum.', + ); + } + } + + // ---------------------------------------------------------- DENOMINATOR --- + + /** + * § Null-Result corollary 1 — the green fixtures above report zero both when + * the rule is correctly silent and when the fixture file is empty, renamed + * or stopped being parsed. Assert the population the clean assertions are + * measuring is non-zero, and that the red fixture really does carry a write + * per flagged line. + */ + public function testFixturePopulationIsNonZero(): void + { + $writeCalls = static function(string $file): int { + $source = file_get_contents($file); + + self::assertNotFalse($source, sprintf('Fixture %s could not be read.', $file)); + + return preg_match_all('/(->|::)(update|insert|insertOrIgnore|insertGetId|upsert|updateOrInsert|updateFrom|insertOrIgnoreReturning|increment|decrement|incrementEach|decrementEach|incrementQuietly|decrementQuietly|incrementEachQuietly|decrementEachQuietly|incrementOrCreate|create|updateOrCreate|save)\(/', $source); + }; + + self::assertGreaterThanOrEqual(34, $writeCalls(self::BUILDER_WRITES), 'The violating fixture lost write sites.'); + self::assertGreaterThanOrEqual(14, $writeCalls(self::CLEAN_WRITES), 'The clean fixture lost write sites, so its zero proves nothing.'); + self::assertGreaterThanOrEqual(7, $writeCalls(self::RAW_TABLE_WRITES), 'The raw-table fixture lost write sites.'); + } + + /** + * This rule is the first in the package to resolve a class FQCN to a FILE + * and parse it, so it is uniquely sensitive to an FQCN declared twice in the + * fixture corpus: reflection picks one declaration, and which one is + * classmap-order dependent. Measured — the model fixtures were first written + * in `App\Models`, where `User` already collided with three other rules' + * stubs; the suite passed on one installed tree and failed on another purely + * because `composer update` reordered the classmap. + * + * A comment saying "do not collide" would not survive; this does. + */ + public function testEveryFixtureModelResolvesToThisRuleSOwnFixtureDirectory(): void + { + $models = [ + 'App\Models\CredentialCastBypass\User', + 'App\Models\CredentialCastBypass\ApiKey', + 'App\Models\CredentialCastBypass\Vault', + 'App\Models\CredentialCastBypass\Article', + 'App\Models\CredentialCastBypass\OverridingVault', + 'App\Models\CredentialCastBypass\NearMissCastModel', + 'App\Models\CredentialCastBypass\AbstractCredentialHolder', + 'App\Models\CredentialCastBypass\TraitCastModel', + 'App\Models\CredentialCastBypass\TraitOverriddenCastModel', + 'App\Models\CredentialCastBypass\HasHashedSecret', + 'App\Models\CredentialCastBypass\HasEncryptedNotesProperty', + 'App\Models\CredentialCastBypass\ComposesHashedSecret', + 'App\Models\CredentialCastBypass\ComposedCastModel', + 'App\Models\CredentialCastBypass\SpreadCastModel', + 'App\Models\CredentialCastBypass\ConstantCastModel', + 'App\Models\CredentialCastBypass\ConstantCastPropertyModel', + 'App\Models\CredentialCastBypass\NestedLiteralCastModel', + // The shape table's classes share ONE file, so a duplicate FQCN + // would redirect every row of it at once rather than one model. + ...array_map( + static fn(string $shape): string => self::DISPATCH_NAMESPACE . $shape, + [ + ...array_keys(self::CAST_DISPATCH_TABLE), + self::DOCUMENTED_FALSE_NEGATIVE, + 'MethodBase', + 'SilentMiddle', + 'ForeignCastSource', + 'PropertyBase', + 'GrandMethodBase', + 'MidReplacing', + 'DeclaresSecretViaMethod', + 'DeclaresPlainPasswordViaMethod', + 'DeclaresPlainPasswordViaTraitToBeExcluded', + 'DeclaresHashedPasswordViaTraitToBeExcluded', + ], + ), + ]; + + $reflectionProvider = self::createReflectionProvider(); + $expectedDirectory = realpath(__DIR__ . '/../Fixtures/CredentialCastBypass'); + + self::assertNotFalse($expectedDirectory, 'The fixture directory is missing.'); + + foreach ($models as $model) { + self::assertTrue( + $reflectionProvider->hasClass($model), + sprintf('Fixture model %s does not resolve at all.', $model), + ); + + self::assertSame( + $expectedDirectory, + dirname((string) $reflectionProvider->getClass($model)->getFileName()), + sprintf( + '%s resolves to a file outside this rule\'s fixture directory, so another fixture declares the same FQCN. The rule reads casts from the resolved FILE, so it would silently read the wrong model.', + $model, + ), + ); + } + } + + protected function getRule(): Rule + { + $parser = self::getContainer()->getService('defaultAnalysisParser'); + + self::assertInstanceOf(Parser::class, $parser); + + if ($this->unparsableFileSuffix !== null) { + $parser = new ThrowingParser($parser, $this->unparsableFileSuffix); + } + + return new ForbidCredentialCastBypassRule( + self::createReflectionProvider(), + $parser, + $this->tableModelOverride ?? [], + ); + } + + /** + * The major version of the installed `illuminate/database`. + * + * Read from Composer rather than from a class-existence probe, because the + * question here is "which Laravel is this?" and answering it by looking for + * one of the methods under test would make the guard argue with itself. + */ + private function installedIlluminateMajor(): int + { + $version = InstalledVersions::getVersion('illuminate/database'); + + self::assertIsString($version, 'illuminate/database is not installed, so the signature check is measuring nothing.'); + + return (int) $version; + } + + /** + * The rule's own payload-slot map, read off the rule rather than restated — + * a copy here would drift and this test would then verify the copy. + * + * @return array> + */ + private function writeMethodSlots(): array + { + $slots = (new ReflectionClass(ForbidCredentialCastBypassRule::class))->getConstant('WRITE_METHODS'); + + self::assertIsArray($slots); + + return $slots; + } + + /** + * PHP's own answer for one model, as `column => cast`, restricted to the + * credential casts. Built the way Laravel builds it — property default + * first, a real dispatch of `casts()` second — and deliberately NOT by + * asking the rule. + * + * `newInstanceWithoutConstructor()` because a constructed Eloquent model + * boots its traits, which needs a container this suite does not have; the + * `casts()` body of a fixture model touches no state. + * + * @return array + */ + private function credentialCastsAccordingToPhp(string $fqcn): array + { + $reflection = new ReflectionClass($fqcn); + + $property = $reflection->getDefaultProperties()['casts'] ?? []; + + self::assertIsArray($property); + + $dispatched = $reflection->getMethod('casts')->invoke($reflection->newInstanceWithoutConstructor()); + + self::assertIsArray($dispatched); + + $credentialCasts = []; + + foreach (array_merge($property, $dispatched) as $column => $cast) { + if (!is_string($column) || !is_string($cast)) { + continue; + } + + if ($cast === 'hashed' || $cast === 'encrypted' || str_starts_with($cast, 'encrypted:')) { + $credentialCasts[$column] = $cast; + } + } + + return $credentialCasts; + } + + /** + * `short model name => line` for every write site in the shape fixture, + * reconciled against the table so a pattern that silently stops matching + * cannot shrink the population instead of failing. + * + * @return array + */ + private function castDispatchWriteLines(): array + { + $source = file_get_contents(self::CAST_DISPATCH_WRITES); + + self::assertNotFalse($source, 'The shape write fixture could not be read.'); + + $lines = []; + + foreach (explode("\n", $source) as $index => $line) { + if (preg_match('/^\s+([A-Za-z]+)::query\(\)->update\(/', $line, $matches) === 1) { + $lines[$matches[1]] = $index + 1; + } + } + + self::assertSame( + [...array_keys(self::CAST_DISPATCH_TABLE), self::DOCUMENTED_FALSE_NEGATIVE], + array_keys($lines), + 'The shape fixture and the shape table have drifted apart — every write site must have a table row, in order, and the only site without one is the documented false negative.', + ); + + return $lines; + } + + /** + * The credential casts PHP resolves for one shape, restricted to the + * columns its write payload names and ordered the way the rule walks that + * payload — which is the order it emits errors within one line. + * + * @param list $payload + * + * @return array + */ + private function credentialCastsInPayloadOrder(string $model, array $payload): array + { + $truth = $this->credentialCastsAccordingToPhp(self::DISPATCH_NAMESPACE . $model); + $flagged = []; + + foreach ($payload as $column) { + if (array_key_exists($column, $truth)) { + $flagged[$column] = $truth[$column]; + } + } + + return $flagged; + } + + /** + * @return list + */ + private function expectedCastDispatchErrors(): array + { + $lines = $this->castDispatchWriteLines(); + $expected = []; + + foreach (self::CAST_DISPATCH_TABLE as $model => $row) { + foreach ($this->credentialCastsInPayloadOrder($model, $row['payload']) as $column => $cast) { + $expected[] = [ + sprintf(self::MESSAGE, $column, self::DISPATCH_NAMESPACE . $model, $cast, 'update'), + $lines[$model], + ]; + } + } + + return $expected; + } +} diff --git a/tests/Support/ThrowingParser.php b/tests/Support/ThrowingParser.php new file mode 100644 index 0000000..c915200 --- /dev/null +++ b/tests/Support/ThrowingParser.php @@ -0,0 +1,46 @@ + + */ + public function parseFile(string $file): array + { + if (str_ends_with($file, $this->failingFileSuffix)) { + throw new ParserErrorsException([new Error('Simulated parse failure')], $file); + } + + return $this->inner->parseFile($file); + } + + /** + * @return array + */ + public function parseString(string $sourceCode): array + { + return $this->inner->parseString($sourceCode); + } +}