feat: ForbidSentinelFallbackOnNarrowingHelperRule (psr-0002)

CHANGELOG.md

@@ -8,6 +8,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and
 
 ### Added
 
+- `ForbidSentinelFallbackOnNarrowingHelperRule` — new rule that flags a **literal sentinel fallback** (`?? ''`, `?? 0`, `?: 'unknown'`, `?? false`, `?? []`, `?? Foo::BAR`) on the result of a **narrowing helper**: a first-party boundary helper that takes `mixed` external data and returns a nullable scalar, where `null` is the helper's way of saying "this input was unreadable". The sentinel converts that failure signal into a plausible-looking value, which then gets persisted, compared, or used to unlock a branch — and the original unreadable input is unrecoverable. **Confirmed damage (tc-api PR #360):** `?? ''` seeded `['']`, a one-element array holding an empty string that reads as non-empty and unlocked a `forceDelete()` wipe; a gender was written to the database as an empty `SET` string. A third bug in the same PR — `Carbon::parse(text($leaf) ?? null)` parsing a missing node as `now()` and writing today's date as a person's date of birth — is the **motivation** for this rule but explicitly **out of its scope**: there is no literal sentinel there (`?? null` is the correct half of the shape), and the damage happens one call later, in a sink that swallows the preserved `null`. Catching it needs sink-aware analysis (which callees are null-swallowing), not this rule's fallback-shape matcher. The remediation is always one of: skip the write, fail closed, or handle `null` explicitly. **Detection is SHAPE-KEYED, never a helper-name list** (a territory's helper inventory changes, the shape does not) — a left-hand `MethodCall` / `NullsafeMethodCall` / `StaticCall` / `FuncCall` (the `?->` arm is fixture-pinned and needs no null-stripping of its own: PHPStan analyses the left operand of `??` / `?:` in isset-ish context, so a nullable receiver — promoted property, parameter, method-call receiver, chained `?->…?->`, array offset — is already narrowed to the bare class by the time the rule resolves the method) fires only when its resolved reflection satisfies ALL of: (1) the declaring class (method/static call) or function FQN (plain function) sits under a configured namespace prefix — the new optional `narrowingHelperNamespacePrefixes` parameter (`listOf(string())`, default `['App']`), matched on a namespace BOUNDARY so `App` and `App\` behave identically and `Application\Foo` is never swept in; (2) at least one **required** parameter resolves to `MixedType` — explicit `mixed` or untyped, both being the same boundary contract from the caller's side, while an OPTIONAL mixed parameter (`ParameterReflection::isOptional()`) is skipped because a lookup helper's `get(string $key, mixed $default = null): ?string` takes the caller's own default there, not unvalidated external data, and its `?? ''` is idiomatic; (3) the return type is a nullable scalar or nullable union of scalars (`?string`, `?int`, `?float`, `?bool`, `string|int|null`), checked via the PHPStan Type API (`TypeCombinator::containsNull()` + a per-member scalar probe on the non-null remainder, with an explicit `NeverType` reject so a `null`-only return cannot match through the bottom type), never by string-comparing type names. The namespace gate is also the vendor gate: `filter_var($x, FILTER_VALIDATE_EMAIL) ?? ''` and every framework call are structurally out of scope, because a vendor helper's null contract is not ours to reason about. **Never flagged:** `?? null` (it PRESERVES the failure signal — the remediation), coalesces on properties / array offsets (`$row['city'] ?? ''` — the idiomatic absent-key default, no helper call), helpers with no `mixed` parameter, non-nullable returns, and nullable non-scalar returns (a null-object fallback is a different, legitimate pattern). **Deliberate misses:** a non-literal fallback (`?? $default`, `?? $this->fetch($leaf)`) — the false-positive-rich half of the shape, where the fallback may itself carry the failure forward; the long ternary (`text($leaf) !== null ? … : ''` — an explicit null test IS the remediation); and a helper result laundered through a variable before the coalesce (provenance is gone at the coalesce; closing it needs data-flow tracking, not a wider matcher). **Known residual false positive:** a first-party helper taking a REQUIRED `mixed` (or untyped) key AND returning a nullable scalar still matches even when it is a lookup rather than a boundary reader — `get(mixed $key, string $default = ''): ?string` is indistinguishable from a narrowing helper by signature alone; the required/optional split removes the common lookup shape, and for the rest the escape hatch is an inline `@phpstan-ignore forbidSentinelFallbackOnNarrowingHelper.sentinelFallback`. ADR-0021 posture — false negatives acceptable, false positives are not. `getNodeType()` is `Expr` because `Coalesce` (a `BinaryOp`) and the short `Ternary` have no common ancestor below it; both are matched by an instanceof guard on the first line and the AST-only sentinel check runs before any reflection, so the per-node cost on non-matching expressions is one instanceof. Identifier: `forbidSentinelFallbackOnNarrowingHelper.sentinelFallback`. Doctrine: war-room §Architectural Principles — Explicit over implicit (a swallowed parse failure is the implicit path); fail-closed data-integrity posture for the boundary/import surface. **Versioning: candidate MAJOR** (surfaces new errors in already-clean consumer code wherever an import/boundary helper is followed by a sentinel). Per the pre-1.0 caret convention the next minor auto-adopts nobody — each consumer remediates and goes green on its own pin bump. **NOT tagged** (release is ally-gated). Seed: tc-api PR #360.
 - `ForbidRawExceptionMessageInResponseRule` — new rule (war-room enforcement queue #140) that flags a raw `Throwable::getMessage()` — or a `Throwable` expression itself — flowing into a **client-facing response sink**. A raw exception message is internal detail (stack-trace fragments, SQL, file paths, driver errors); when it reaches an API response it is an information-disclosure leak (ISO 27001 A.5.33 / general defence-in-depth for the ISO 27001 / AVG / NEN 7510 consumer territories). The remediation is always the same: **log** the raw message server-side (`Log::`, `report()`) and hand the client a stable, app-authored message. The rule is the durable Level-2 backstop for the raw-exception-message leak family (the confirmed point-fix sites — ublgenie's 8 MCP tools + codebook `DeleteChapterTool`, each returning `Response::error('...' . $e->getMessage())` — and each consumer's release-pin adoption are separate, separately-tracked steps). **Sink model:** a sink is a `FQCN::method` signature, matched in BOTH call forms — a `StaticCall` whose resolved class equals the FQCN (`Response::error(...)`) and a `MethodCall` whose receiver type is a subtype of the FQCN (an injected persist-sink service). The built-in default sink is `Laravel\Mcp\Response::error` (the confirmed dominant shape, always armed); a consumer adds its own PERSIST sinks (an invoice-log setter, a `MarkInvoiceFailed` Action) via the new optional `rawExceptionMessageSinks` PHPStan parameter (`listOf(string())`, default `[]` — so the rule is safe to adopt with only the MCP shape armed). **Argument detection:** a matched sink call is flagged when any argument is, directly OR via string concatenation (`'context: ' . $e->getMessage()`), a `->getMessage()` call on an expression whose type is a subtype of `\Throwable`, or a `\Throwable` expression passed directly. **Type-aware discrimination is load-bearing:** `$validator->getMessage()` on a non-`Throwable` receiver does NOT fire — only a message pulled off an actual exception is a leak. **Mandatory false-positive exclusions (the remediation pattern, never the violation):** `Log::` / `logger()->` / PSR `LoggerInterface` log-level calls (`info` / `warning` / `error` / `critical` / `debug` / `log` / `notice` / `alert` / `emergency`) and `report()` — server-side logging of the raw message is exactly where it is *supposed* to go. Because a sink is keyed on `FQCN::method` a logger is never a sink under the default config; the exclusion additionally short-circuits BEFORE sink matching, so a consumer that adds a broad sink can never turn a logger into a false positive (pinned by tests that configure a logger method AS a sink and assert it still stays silent). **Exemptions (narrowest first):** the `safeMessageExceptionClasses` parameter (`listOf(string())`, default `[]`) lists exception FQCNs whose message discipline is proven app-authored — arch-test-pinned in the consuming territory, the codebook `DependentModelRelationException` shape — so a prove-safe class costs ONE config line, not a per-call-site annotation (type-aware, subtypes inherit; covers the **message only** — the Throwable itself still fires, `__toString` carries class/file/trace regardless of message discipline); a `// @leak-safe: <rationale>` comment on the sink call line (or in the contiguous comment block directly above it) suppresses a proven-safe call site the class list cannot express (the codebook `SendCodyReportAction` shape); the standard PHPStan inline-ignore mechanism on the identifier is the alternative. `$e?->getMessage()` (a `NullsafeMethodCall` — a distinct AST node) is matched like its unconditional sibling. Identifier: `forbidRawExceptionMessageInResponse.rawMessageInResponse`. **Deliberate misses (v1 scope):** `getTraceAsString()` / `__toString()` and other Throwable accessors (a future minor can widen the accessor set), a Throwable laundered through a helper/formatter call whose return type is no longer `Throwable`, and plain local-variable extraction (`$msg = $e->getMessage(); Response::error($msg);` — the type at the sink is `string`, provenance gone; closing it needs data-flow tracking) (ADR-0021 posture — false negatives acceptable, false positives are not). Doctrine: war-room §Architectural Principles — Explicit over implicit (#1); information-disclosure hardening. **Versioning: candidate MAJOR** (surfaces new errors in already-clean consumer code wherever a raw exception message reaches a response sink — the confirmed ublgenie/codebook MCP-tool sites). Per the pre-1.0 caret convention `^0.7` excludes the next minor, so tagging auto-adopts nobody — each consumer remediates and goes green on its own bump PR (a suppress-only / baseline-absorb posture). **NOT tagged** (release is ally-gated). Seed: war-room enforcement queue #140.
 
 ### Changed

CLAUDE.md

@@ -27,6 +27,7 @@ Composer package distributing war-room-doctrine PHPStan rules across `script-dev
 | `ForbidResourceWrappedInJsonResponseRule` | War-room §Explicit over implicit + ADR-0009 | `forbidResourceWrappedInJsonResponse.resourceWrapped` (type-aware; bans wrapping a `JsonResource` in `response()->json()` / `new JsonResponse()` in `App\Http\Controllers\*`. Named-envelope nesting excluded. shipped v0.5.0) |
 | `ForbidInlineArrayJsonResponseInControllersRule` | ADR-0009 | `forbidInlineArrayJsonResponseInControllers.arrayPayload` (type-aware; bans constructing the base `JsonResponse` (exact-FQCN, NOT subclasses) / `response()->json()` with an ARRAY payload in `App\Http\Controllers\*`. Inverse of `ForbidResourceWrappedInJsonResponseRule`. `fromJsonString` a deliberate miss. Seed kendo PR #1653. on `main`, `[Unreleased]` — pending v0.8.0 tag (release PR #53)) |
 | `ForbidRawExceptionMessageInResponseRule` | War-room §Explicit over implicit + info-disclosure hardening | `forbidRawExceptionMessageInResponse.rawMessageInResponse` (flags a raw `Throwable::getMessage()` — directly or via string concat — or the `Throwable` itself flowing into a client-facing response sink. Default sink `Laravel\Mcp\Response::error`; additional `FQCN::method` sinks via the `rawExceptionMessageSinks` param, default `[]`. Type-aware — only a `getMessage()` on a `\Throwable` receiver fires. Server-side logging (`Log::`/`logger()`/PSR `LoggerInterface`/`report()`) never flags. `// @leak-safe:` comment exemption. on `main`, `[Unreleased]`) |
+| `ForbidSentinelFallbackOnNarrowingHelperRule` | War-room §Explicit over implicit + fail-closed data integrity | `forbidSentinelFallbackOnNarrowingHelper.sentinelFallback` (shape-keyed; flags a literal sentinel fallback — `?? ''` / `?? 0` / `?: 'unknown'` / `?? false` / `?? []` / `?? Foo::BAR` — on a first-party helper that takes a `mixed` param and returns a nullable scalar. Namespace prefixes via `narrowingHelperNamespacePrefixes`, default `['App']`. `?? null` never fires; non-literal fallbacks and the long ternary are deliberate misses. Seed tc-api PR #360. on `main`, `[Unreleased]`) |
 | `LogRule` | ADR-0001 §Append-only | `logRule.logModification` (covers instance `update`/`delete`/`forceDelete`/`forceDeleteQuietly`; static `Model::destroy()` / `Model::forceDestroy()` shipped in v0.3.0) |
 | `LogBuilderTruncateRule` | ADR-0001 §Append-only | `logRule.logModification` (shared with `LogRule`; covers `Builder->truncate()` on Log-named tables — shipped in v0.3.0) |
 | `EnforceAuditSnapshotOnRetryRule` | ADR-0001 §Snapshot-on-Retry Safety | `enforceAuditSnapshotOnRetry.firstStatementMustResetState` |