feat(queue-58): ForbidUntimedHttpClientRule — AST-aware Doctrine #8 timeout enforcement

CHANGELOG.md

@@ -8,6 +8,14 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and
 
 ### Added
 
+- `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).
+
+  **CONSERVATIVE BY DESIGN — biased to zero false positives.** Fires only when the ENTIRE chain from an entry point to the send verb is visible in a single expression. It DECLINES (never a false positive) on: split chains where the `PendingRequest` is built on one statement / helper and sent on another (`$req = $this->http->timeout(5); $req->get()`, or `$this->apiClient()->post()` returning a pre-timed request — a large fraction of the real fleet surface); raw `GuzzleHttp\Client` construction / per-call `['timeout' => N]` options; vendor SDKs that configure the timeout via their own `setConfig([...])`; and DI-bound pre-timed clients. **Consequence: this rule COMPLEMENTS the per-territory named-list tests; it does NOT replace them** — the helper-built split (which the named-list's whole-file scan does cover) is out of AST reach for a single-expression rule. The builder-side enforcement that would actually retire the named-lists is a separate follow-up (queue #58 continuation).
+
+  **Field-validated (spike, 2026-07-21):** run against kendo / emmie / ublgenie / BIO backends — **zero false positives** (compliant chains short-circuit on the timeout; split/helper chains decline); a synthetic positive control fires on exactly the untimed method against the *real* Illuminate `Factory`. **Versioning: MINOR** (new rule; zero violations on every swept consumer ⇒ no baseline on kendo/emmie/ublgenie/BIO). A not-yet-swept consumer carrying an untimed *direct* facade/Factory chain would see a new error and adopt on its own bump PR; per the pre-1.0 caret convention `^0.8` excludes the next minor, so tagging auto-adopts nobody. **NOT tagged** (release is ally-gated).
+
 - `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

@@ -35,6 +35,7 @@ Composer package distributing war-room-doctrine PHPStan rules across `script-dev
 | `EnforceResourceDataValidatorOptInRule` | ADR-0009 §EAGER_LOAD validator opt-in | `enforceResourceDataValidatorOptIn.missingValidatorCall` |
 | `EnforceFormRequestToDtoRule` | ADR-0012 §FormRequest → DTO Flow | `enforceFormRequestToDto.missingToDtoMethod` |
 | `EnforceCurrentUserAttributeRule` | War-room §Explicit over implicit | `enforceCurrentUserAttribute.useAttributeInsteadOfRequestUser` |
+| `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<T>` carve-out. Seed kendo PR #1653. on `main`, `[Unreleased]` — pending v0.8.0 tag (release PR #53)) |
 | `ConnectionTransactionReturnTypeExtension` | (type extension, no rule) | — |
@@ -113,6 +114,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; on `main`, `[Unreleased]`). These enforce war-room §Architectural Principles (some also touching numbered ADRs) — each rule's docblock "Doctrine source" line names its authority.
+- **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
 

extension.neon

@@ -142,6 +142,9 @@ services:
             auditModelNamespacePrefixes: %auditModelNamespacePrefixes%
             auditModelNameSuffixes: %auditModelNameSuffixes%
         tags: [phpstan.rules.rule]
+    -
+        class: ScriptDevelopment\PhpstanWarroomRules\Rules\ForbidUntimedHttpClientRule
+        tags: [phpstan.rules.rule]
     -
         class: ScriptDevelopment\PhpstanWarroomRules\Rules\ForbidRawExceptionMessageInResponseRule
         arguments: