diff --git a/CHANGELOG.md b/CHANGELOG.md index c4eefb5..77f15c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and ## [Unreleased] +### Added + +- `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: ` 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 - `ForbidEloquentMutationInControllersRule` — registration moved off `Class_` onto `CallLike` (per-node), closing a receiver-type blind spot. The rule previously registered on `Class_` and manually walked every method body, resolving each receiver via `$scope->getType($node->var)` against the **class-entry** scope — a scope carrying no flow-derived knowledge of method-local variables, so any receiver born inside the method body (`$m = new Model; $m->save();`; `$m = Model::query()->…->firstOrFail(); $m->delete();`; a `Builder` held in a local var) resolved to `mixed` and **silently never fired** — only receivers typed from a method signature (typed parameters) matched. Empirically proven on tc-api PR #133 (PHPStan-baseline entries for exactly these shapes came back "unmatched" — the rule never emitted them). Now registers on `CallLike` and branches `MethodCall` / `StaticCall` (mirrors `EnforceCurrentUserAttributeRule` / `LogRule`), so PHPStan supplies a method-level flow scope and local-variable receivers resolve to their real Model / Builder types. The namespace gate (`controllerNamespacePrefixes`, unchanged), the `checkInstanceCall` / `checkStaticCall` type-matching, the blocklist, the error identifier `forbidEloquentMutationInControllers.eloquentMutationInController`, and the message format are all preserved byte-for-byte; the containing-controller FQCN for the message now comes from `$scope->getClassReflection()?->getName()` at the call site (the manual `Class_`-scope `resolveClassFqcn` + `walkNodes` helpers are deleted). The static-call path (`Model::destroy($id)`) is unaffected — it never depended on flow scope. Every pre-existing fixture still fires at the same line; new positive fixtures cover the three local-receiver shapes and a new compliant fixture pins that a local var of a NON-Model class stays clean (the type gate still discriminates under flow scope). Nullsafe `$m?->delete()` calls are covered without a dedicated branch: PHPStan's `NodeScopeResolver` emits a synthetic non-null-narrowed `MethodCall` node (attribute `virtualNullsafeMethodCall`) for every `?->` call, so the plain `MethodCall` branch already fires once — an explicit `NullsafeMethodCall` branch double-reports (the real node plus its synthetic twin). `TypeCombinator::removeNull()` is applied to the receiver before the type gate for the DISTINCT plain-call-on-nullable shape (a plain `->delete()` on a `?Post` from `->first()`: `Post|null` is only a `maybe()` Model supertype, never `yes()`, so without the strip it never fires; a nullable Model receiver carries the same audit-bypass risk). Per-node registration additionally reaches trait bodies analysed through a using class: a mutation in a trait declared under a controllers namespace (e.g. `App\Http\Controllers\Concerns\*`) now fires, message naming the using class — intended coverage the old `Class_` walk structurally never had (a trait file has no `Class_` node). These are pinned by new fixtures (`ViolationNullsafeDelete` — single-fire, no double-report; `ViolationPlainNullableDelete` — the `removeNull` path; `ViolationInTraitFile`). **Versioning: candidate MAJOR per ADR-0021 §Versioning** — surfaces new errors in previously-clean consumer code (known: tc-api `EducationController::store` / `destroy`; the nullsafe + trait paths are additional new-error surface under the same candidacy); per the pre-1.0 caret convention `^0.7` excludes the next minor, so tagging auto-adopts nobody — each consumer runs a pre-cascade audit and remediates on its own pin-bump PR. Seed: tc-api PR #133 baseline-unmatched finding. diff --git a/CLAUDE.md b/CLAUDE.md index cb50ae0..d06919e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -26,6 +26,7 @@ Composer package distributing war-room-doctrine PHPStan rules across `script-dev | `ForbidHttpExceptionInActionsRule` | War-room §Explicit over implicit + §FormRequest → DTO → Action | `forbidHttpExceptionInActions.httpExceptionInAction` (type-aware sibling of `ForbidAbortHelperRule`; bans throwing the `Symfony\…\HttpException` family from `App\Actions\*`. `Illuminate\Validation\ValidationException` out of scope. shipped v0.5.0) | | `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]`) | | `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` | @@ -111,7 +112,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), and `ForbidResourceWrappedInJsonResponseRule` (bans wrapping a `JsonResource` in `response()->json()` / `new JsonResponse()` inside controllers per Principle #1 + ADR-0009; shipped v0.5.0). These enforce war-room §Architectural Principles (the last two also touching numbered ADRs) — each rule's docblock "Doctrine source" line names its authority. +- **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. ### War-room internal ADRs diff --git a/README.md b/README.md index a9fe262..ea09b80 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,7 @@ includes: | `EnforceAuditTransactionScopeRule` | `enforceAuditTransactionScope.nonTransactionalMutationInClosure` | `App\Actions\*` whose `execute()` calls `transaction(...)` with a literal closure | Mutating `StatefulGuard` / `Session` / `Cache` / `Bus` / `Queue` / `Mailer` / `Notification` / `Broadcaster` / `Filesystem` state (or their `Illuminate\Support\Facades\*` counterparts) inside the closure is an error. Reads (`Auth::user()`, `Session::get()`, `Cache::get()`) are permitted. Doctrine: ADR-0029 (Audit Row Durability Contract) §Decision rule 3. | | `ForbidEloquentMutationInControllersRule` | `forbidEloquentMutationInControllers.eloquentMutationInController` | `App\Http\Controllers\*` (including sub-namespaces; configurable via `controllerNamespacePrefixes`) | Calling Eloquent persistence APIs (`save`, `update`, `delete`, `create`, `destroy`, `forceDelete`, `forceFill`, `push`, `restore`, `touch`, and their `*OrFail` / `*Quietly` / `*OrCreate` variants — 24-method blocklist) on `Illuminate\Database\Eloquent\Model` subclasses or `Illuminate\Database\Eloquent\Builder` chains is an error. Reads (`find`, `where`, `get`, `first`, `paginate`, `pluck`, `count`, `exists`, `query`) are permitted. Delegate mutations to an Action. Doctrine: ADR-0011 (Action Class Architecture) + ADR-0019 (Explicit Model Hydration). | | `ForbidInlineArrayJsonResponseInControllersRule` | `forbidInlineArrayJsonResponseInControllers.arrayPayload` | `App\Http\Controllers\*` (including sub-namespaces; configurable via `controllerNamespacePrefixes`) | Constructing the base `Illuminate\Http\JsonResponse` — exact-FQCN, **NOT subclasses** — or its `response()->json(...)` factory twin with an **array** payload is an error. Type-aware: fires when the first argument's resolved type `isArray()->yes()`, catching both inline literals (`new JsonResponse(['enabled' => …])`) and array-typed variables (`new JsonResponse($result)` — the same violation laundered through a variable). Passes on Resource / DTO / `JsonSerializable` / mixed / unknown payloads, `null` (`new JsonResponse(null, 204)`), no-args, and any JsonResponse **subclass** (`NoContentResponse`, … — the compliant fix; matching by supertype would criminalize it). Response shapes belong to a Resource/ResourceData or a dedicated JsonResponse subclass. Deliberate miss: `JsonResponse::fromJsonString(...)`. Sibling/inverse of `ForbidResourceWrappedInJsonResponseRule` (same JsonResponse × payload boundary, opposite direction — that rule fires on a Resource payload, this one on an array). Doctrine: ADR-0009 (Unified ResourceData Pattern). Seed: kendo PR #1653. | +| `ForbidRawExceptionMessageInResponseRule` | `forbidRawExceptionMessageInResponse.rawMessageInResponse` | Calls to a configured client-facing response sink (default `Laravel\Mcp\Response::error`; add more via `rawExceptionMessageSinks`) | Passing a raw `Throwable::getMessage()` — directly or via string concat (`'x: ' . $e->getMessage()`) — or the `Throwable` itself into a response sink is an error: it leaks internal detail (stack traces, SQL, file paths) to the API client. Log the raw message server-side (`Log::` / `report()`) and return a stable, app-authored message. **Type-aware:** only a `getMessage()` on an actual `\Throwable` receiver fires (`$validator->getMessage()` is silent). **Never flags** server-side logging — `Log::` / `logger()->` / PSR `LoggerInterface` log-level calls and `report()` are the remediation, not the leak. Exempt a proven-safe app-authored message per exception CLASS via `safeMessageExceptionClasses` (arch-test-pinned; message only — the Throwable itself still fires) or per call site with a `// @leak-safe: ` comment on/above the sink line. Doctrine: war-room §Explicit over implicit (#1); information-disclosure hardening. | | `EnforceResourceDataValidatorOptInRule` | `enforceResourceDataValidatorOptIn.missingValidatorCall` | Classes extending `App\Http\Resources\ResourceData` | If the class declares a non-empty `EAGER_LOAD_COUNT` / `EAGER_LOAD_SUM` constant but never calls `validateRelationsLoaded()` in any method, error. | | `EnforceFormRequestToDtoRule` | `enforceFormRequestToDto.missingToDtoMethod` | Concrete classes extending `Illuminate\Foundation\Http\FormRequest` | If the class neither declares nor inherits a `toDto()` method, error. Abstract intermediates (`BaseFormRequest`) are exempt. Hand Actions a typed DTO, not `$request->validated()` arrays. Doctrine: ADR-0012 (FormRequest → DTO Flow). | | `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). | @@ -229,6 +230,47 @@ parameters: path: app/Models/Audit/SomeProjectionLog.php ``` +### `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 `[]`: + +```neon +parameters: + rawExceptionMessageSinks: + # single backslashes — NEON keeps them literal outside double quotes + - 'App\Support\InvoiceLog::recordError' + - 'App\Actions\Invoice\MarkInvoiceFailed::execute' +``` + +A signature matches BOTH call forms — a static call whose resolved class equals the FQCN, and an instance call whose receiver is a subtype of the FQCN — so an injected persist sink is caught without a rule change. + +**Server-side logging is never flagged** — `Log::`, `logger()->`, PSR `LoggerInterface` log-level calls, and `report()` are the remediation. Log the raw message; return a stable, app-authored message to the client: + +```php +} catch (\Throwable $e) { + logger()->error('invoice.show failed', ['exception' => $e->getMessage()]); // fine — server-side + return Response::error('Could not load the invoice.'); // fine — app-authored + // return Response::error('Failed: ' . $e->getMessage()); // ERROR — raw leak +} +``` + +**Exempting a proven-safe exception CLASS** — when a domain exception's message discipline is proven app-authored (the codebook `DependentModelRelationException` shape, pinned by an arch test in the consuming territory), list it in `safeMessageExceptionClasses` (default `[]`) instead of annotating every call site. Type-aware — subtypes inherit the allowance; the exemption covers the **message only** (passing the Throwable itself still fires: `__toString` carries class, file, and trace regardless of message discipline). List a class here only when an arch test pins its message discipline — config without the pin is a hole, not an exemption: + +```neon +parameters: + safeMessageExceptionClasses: + - 'App\Exceptions\DependentModelRelationException' +``` + +**Exempting a proven-safe call site** — when the exception message is app-authored and carries no raw payload but a class-level listing does not fit (the codebook `SendCodyReportAction` shape), mark it with a `// @leak-safe: ` comment on the sink line or in the comment block directly above it: + +```php +// @leak-safe: SendCodyReportException carries only an app-authored, payload-free message +return Response::error('Report failed: ' . $e->getMessage()); +``` + +The standard PHPStan inline-ignore on `forbidRawExceptionMessageInResponse.rawMessageInResponse` is the alternative. `getTraceAsString()` / `__toString()` and a Throwable laundered through a formatter call are deliberate v1 misses. + ### Action namespace assumption `EnforceActionTransactionsRule` and `ForbidDatabaseManagerInActionsRule` only fire on classes whose namespace starts with `App\Actions`. This matches the Laravel convention used in every `script-development` territory. Territories using a different actions namespace should open a PR to make this configurable. diff --git a/extension.neon b/extension.neon index 866a555..a9137f1 100644 --- a/extension.neon +++ b/extension.neon @@ -53,6 +53,20 @@ parameters: auditModelNameSuffixes: - 'AuditLog' + # `ForbidRawExceptionMessageInResponseRule`: ADDITIONAL client-facing sink + # signatures, each in `FQCN::method` form, that must not receive a raw + # `Throwable::getMessage()` (or the Throwable itself). These are ADDED to the + # always-armed built-in default sink `Laravel\Mcp\Response::error`; the + # default is empty so the rule is safe to adopt with only the MCP shape + # armed. A signature matches BOTH call forms — a static call whose resolved + # class equals the FQCN, and an instance call whose receiver is a subtype of + # the FQCN — so a consumer names its persist-error sink (an invoice-log + # setter, a MarkInvoiceFailed Action) here without a rule change. Logger / + # report() calls are excluded structurally and never need listing. Each FQCN + # uses single backslashes — see the NEON-quoting note above. + rawExceptionMessageSinks: [] + safeMessageExceptionClasses: [] + parametersSchema: resourceDataBaseClass: string() formRequestBaseClass: string() @@ -60,6 +74,8 @@ parametersSchema: controllerNamespacePrefixes: listOf(string()) auditModelNamespacePrefixes: listOf(string()) auditModelNameSuffixes: listOf(string()) + rawExceptionMessageSinks: listOf(string()) + safeMessageExceptionClasses: listOf(string()) services: - @@ -126,6 +142,12 @@ services: auditModelNamespacePrefixes: %auditModelNamespacePrefixes% auditModelNameSuffixes: %auditModelNameSuffixes% tags: [phpstan.rules.rule] + - + class: ScriptDevelopment\PhpstanWarroomRules\Rules\ForbidRawExceptionMessageInResponseRule + arguments: + rawExceptionMessageSinks: %rawExceptionMessageSinks% + safeMessageExceptionClasses: %safeMessageExceptionClasses% + tags: [phpstan.rules.rule] - class: ScriptDevelopment\PhpstanWarroomRules\Type\ConnectionTransactionReturnTypeExtension tags: [phpstan.broker.dynamicMethodReturnTypeExtension] diff --git a/src/Rules/ForbidRawExceptionMessageInResponseRule.php b/src/Rules/ForbidRawExceptionMessageInResponseRule.php new file mode 100644 index 0000000..b6ff84f --- /dev/null +++ b/src/Rules/ForbidRawExceptionMessageInResponseRule.php @@ -0,0 +1,463 @@ +getMessage())` (ublgenie's 8 MCP tools + + * codebook `DeleteChapterTool` — every one concatenates the raw message into + * the error response). `Laravel\Mcp\Response::error` is therefore the built-in + * default sink; a consumer adds its own PERSIST sinks (an invoice-log setter, + * a `MarkInvoiceFailed` Action) via the `rawExceptionMessageSinks` parameter, + * default `[]`, so the rule is safe to adopt with only the MCP shape armed. + * + * A sink is a `FQCN::method` signature. It is matched in BOTH call forms: + * + * - a `StaticCall` whose resolved class equals the sink FQCN + * (`Response::error(...)`), and + * - a `MethodCall` whose receiver type is a subtype of the sink FQCN + * (`$this->response->error(...)`, an injected persist-sink service). + * + * A matched sink call is flagged when ANY argument is, directly OR via string + * concatenation (`'context: ' . $e->getMessage()`): + * + * - a `->getMessage()` `MethodCall` on an expression whose type is a subtype + * of `\Throwable`, or + * - a `\Throwable` expression passed directly into the sink. + * + * Type-aware discrimination is load-bearing: `$validator->getMessage()` (a + * non-`Throwable` receiver) does NOT fire — only a message pulled off an actual + * exception is a leak. + * + * NEVER flagged (mandatory false-positive exclusions — the remediation pattern, + * not 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 CORRECT — it is where the raw message is *supposed* to go. + * Because a sink is keyed on `FQCN::method`, a logger is never a sink under + * the default config; this exclusion additionally short-circuits BEFORE + * sink matching, so a consumer that adds a broad sink can never turn a + * logger into a false positive. + * + * Exemptions, narrowest first: + * + * - `safeMessageExceptionClasses` (config, the CLASS-level path): exception + * FQCNs whose messages are proven app-authored (the codebook + * `DependentModelRelationException` shape — its message discipline is + * pinned by an arch test in the consuming territory). A `getMessage()` + * whose receiver is a subtype of a listed class never fires, so a + * prove-safe class needs ONE config line, not an annotation at every call + * site. The allowlist covers the MESSAGE only — passing the Throwable + * itself into a sink still fires (`__toString` carries class, file, and + * trace regardless of message discipline). List a class here only when the + * consuming territory pins its message discipline with an arch test; + * config without the pin is a hole, not an exemption. + * - `// @leak-safe: ` comment on the sink call line (or in the + * contiguous comment block directly above it) — the per-call-site path for + * a proven-safe case the class-level list cannot express (the codebook + * `SendCodyReportAction` shape). The standard PHPStan inline-ignore on the + * identifier `forbidRawExceptionMessageInResponse.rawMessageInResponse` is + * the alternative. + * + * Out of scope (deliberately, for v1): + * + * - `getTraceAsString()` / `__toString()` / other Throwable accessors — the + * confirmed leak surface is `getMessage()` and the Throwable itself; a + * future minor can widen the accessor set. + * - Sinks passed a Throwable through a helper/formatter call + * (`Response::error($this->format($e))`) — the type at the sink boundary is + * the formatter's return, not a Throwable; a false negative is accepted + * (ADR-0021 posture: false negatives acceptable, false positives are not). + * - Plain local-variable extraction (`$msg = $e->getMessage(); + * Response::error($msg);`) — the mundane sibling of the formatter gap: the + * type at the sink is `string`, the provenance is gone. Same accepted- + * false-negative posture; closing it needs data-flow tracking, not a + * wider matcher. + * + * @implements Rule + */ +final class ForbidRawExceptionMessageInResponseRule implements Rule +{ + /** + * Built-in default sink — the Laravel-MCP `Response::error(...)` shape that + * dominates the confirmed leak surface. Always armed; consumer-configured + * sinks are added to it, never replace it. + */ + private const string DEFAULT_SINK = 'Laravel\Mcp\Response::error'; + + private const string THROWABLE = Throwable::class; + + /** Log-level method names — a call to any of these on a logger is the remediation, never a leak. */ + private const array LOGGER_METHODS = [ + 'info', 'warning', 'error', 'critical', 'debug', 'log', 'notice', 'alert', 'emergency', + ]; + + /** Logger receivers whose log-level calls are excluded. */ + private const string LOG_FACADE = Log::class; + + private const string PSR_LOGGER = LoggerInterface::class; + + private const string LOGGER_HELPER = 'logger'; + + /** + * Parsed sink signatures — each `['class' => FQCN, 'method' => name]`. + * + * @var list + */ + private array $sinks; + + /** + * @param list $rawExceptionMessageSinks additional client-facing + * sink signatures in + * `FQCN::method` form (e.g. a + * consumer's persist-error + * setter). Merged with the + * built-in `Response::error` + * default; empty by default so + * the rule is safe to adopt. + * @param list $safeMessageExceptionClasses exception FQCNs whose + * messages are proven + * app-authored (arch-test- + * pinned in the consuming + * territory) — their + * `getMessage()` is exempt; + * the Throwable itself never + * is. Empty by default. + */ + public function __construct( + array $rawExceptionMessageSinks = [], + private readonly array $safeMessageExceptionClasses = [], + ) { + $this->sinks = []; + + foreach ([self::DEFAULT_SINK, ...$rawExceptionMessageSinks] as $signature) { + $parsed = $this->parseSinkSignature($signature); + + if ($parsed !== null) { + $this->sinks[] = $parsed; + } + } + } + + public function getNodeType(): string + { + return CallLike::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!$node instanceof StaticCall && !$node instanceof MethodCall) { + return []; + } + + // Mandatory exclusion — a logger call is the remediation, not a leak. + // Short-circuits before sink matching so a broad consumer sink can never + // criminalize server-side logging. (`report()` is a FuncCall, never + // examined here, so it is structurally out of scope already.) + if ($this->isLoggerCall($node, $scope)) { + return []; + } + + if (!$this->isConfiguredSink($node, $scope)) { + return []; + } + + if ($this->hasLeakSafeMarker($node, $scope)) { + return []; + } + + foreach ($node->getArgs() as $arg) { + if ($this->exprCarriesRawExceptionMessage($arg->value, $scope)) { + return [$this->buildError($node)]; + } + } + + return []; + } + + /** + * @return array{class: string, method: string}|null + */ + private function parseSinkSignature(string $signature): ?array + { + $parts = explode('::', $signature); + + if (count($parts) !== 2 || $parts[0] === '' || $parts[1] === '') { + return null; + } + + return ['class' => $parts[0], 'method' => $parts[1]]; + } + + /** + * True when the call matches a configured sink in either form: a static + * call whose resolved class equals the sink FQCN, or an instance call whose + * receiver type is a subtype of the sink FQCN. Method name must match. + */ + private function isConfiguredSink(MethodCall|StaticCall $node, Scope $scope): bool + { + if (!$node->name instanceof Identifier) { + return false; + } + + $methodName = $node->name->toString(); + + foreach ($this->sinks as $sink) { + if ($sink['method'] !== $methodName) { + continue; + } + + if ($node instanceof StaticCall) { + if ($node->class instanceof Name && $scope->resolveName($node->class) === $sink['class']) { + return true; + } + + continue; + } + + if ((new ObjectType($sink['class']))->isSuperTypeOf($scope->getType($node->var))->yes()) { + return true; + } + } + + return false; + } + + /** + * True when `$expr` is, directly or through string concatenation, a raw + * exception message: a `Throwable::getMessage()` call or a `Throwable` + * expression itself. + */ + private function exprCarriesRawExceptionMessage(Expr $expr, Scope $scope): bool + { + if ($expr instanceof Concat) { + return $this->exprCarriesRawExceptionMessage($expr->left, $scope) + || $this->exprCarriesRawExceptionMessage($expr->right, $scope); + } + + if ($this->isThrowableGetMessageCall($expr, $scope)) { + return true; + } + + // The Throwable itself passed into the sink (a `getMessage()` call + // returns string, so this branch never double-counts the call above). + return $this->typeIsThrowable($scope->getType($expr)); + } + + private function isThrowableGetMessageCall(Expr $expr, Scope $scope): bool + { + // NullsafeMethodCall is a distinct node — `$e?->getMessage()` leaks + // exactly as its unconditional sibling does when `$e` is non-null. + if (!$expr instanceof MethodCall && !$expr instanceof NullsafeMethodCall) { + return false; + } + + if (!$expr->name instanceof Identifier || $expr->name->toString() !== 'getMessage') { + return false; + } + + $receiverType = $scope->getType($expr->var); + + return $this->typeIsThrowable($receiverType) + && !$this->isSafeMessageException($receiverType); + } + + /** + * True when the receiver's type is a subtype of a configured + * safe-message exception class — its `getMessage()` is proven + * app-authored, so the message (and ONLY the message) is exempt. + */ + private function isSafeMessageException(Type $type): bool + { + $type = TypeCombinator::removeNull($type); + + if ($type instanceof NeverType) { + return false; + } + + foreach ($this->safeMessageExceptionClasses as $class) { + if ((new ObjectType($class))->isSuperTypeOf($type)->yes()) { + return true; + } + } + + return false; + } + + private function typeIsThrowable(Type $type): bool + { + // Strip null so a nullsafe receiver (`$e?->getMessage()` — the receiver + // types as `Throwable|null` inside the NullsafeMethodCall) still + // resolves; a pure-null type (NeverType after the strip) never is one. + $type = TypeCombinator::removeNull($type); + + if ($type instanceof NeverType) { + return false; + } + + return (new ObjectType(self::THROWABLE))->isSuperTypeOf($type)->yes(); + } + + /** + * Recognise a logger call so it is never treated as a leak. Covers + * `Log::error(...)`, `logger()->error(...)`, and a PSR `LoggerInterface` + * instance call. (`report(...)` is a FuncCall, already out of scope.). + */ + private function isLoggerCall(MethodCall|StaticCall $node, Scope $scope): bool + { + if (!$node->name instanceof Identifier) { + return false; + } + + $methodName = $node->name->toString(); + + if ($node instanceof StaticCall) { + return in_array($methodName, self::LOGGER_METHODS, true) + && $node->class instanceof Name + && $scope->resolveName($node->class) === self::LOG_FACADE; + } + + if (!in_array($methodName, self::LOGGER_METHODS, true)) { + return false; + } + + // `logger()->error(...)` — receiver is the `logger()` helper FuncCall. + if ( + $node->var instanceof FuncCall + && $node->var->name instanceof Name + && $node->var->name->toString() === self::LOGGER_HELPER + ) { + return true; + } + + // `$this->logger->error(...)` — receiver typed as a PSR logger. + return (new ObjectType(self::PSR_LOGGER))->isSuperTypeOf($scope->getType($node->var))->yes(); + } + + /** + * Honour a `// @leak-safe: ` marker on the sink call line or in + * the contiguous comment block directly above it. Mirrors + * `EnforceAuditSnapshotOnRetryRule::hasExemptionMarker()` — PHPStan does not + * propagate a `parent` attribute onto nodes, so the raw-source scan is the + * reliable path. + */ + private function hasLeakSafeMarker(MethodCall|StaticCall $node, Scope $scope): bool + { + foreach ($node->getComments() as $comment) { + if (str_contains($comment->getText(), '@leak-safe')) { + return true; + } + } + + $file = $scope->getFile(); + + if ($file === '' || !is_file($file)) { + return false; + } + + $source = @file_get_contents($file); + + if ($source === false) { + return false; + } + + $lines = explode("\n", $source); + $startLine = $node->getStartLine(); + + // Same-line trailing comment (`Response::error(...); // @leak-safe: ...`). + if (isset($lines[$startLine - 1]) && str_contains($lines[$startLine - 1], '@leak-safe')) { + return true; + } + + // Contiguous comment block immediately above the sink call. + $idx = $startLine - 2; + + while ($idx >= 0) { + $line = mb_trim($lines[$idx]); + + if ($line === '') { + $idx--; + + continue; + } + + $isCommentLine = str_starts_with($line, '//') + || str_starts_with($line, '*') + || str_starts_with($line, '/*'); + + if (!$isCommentLine) { + return false; + } + + if (str_contains($line, '@leak-safe')) { + return true; + } + + $idx--; + } + + return false; + } + + private function buildError(MethodCall|StaticCall $node): IdentifierRuleError + { + return RuleErrorBuilder::message( + 'Raw exception message reaches a client-facing response sink. ' + . 'Passing Throwable::getMessage() (or the Throwable itself) to a response leaks internal detail ' + . '(stack-trace fragments, SQL, file paths) to the API client. Log the raw message server-side ' + . '(Log::/report()) and return a stable, app-authored message. ' + . 'Suppress a proven-safe app-authored message with a `// @leak-safe: ` comment on the sink line, ' + . 'or list an arch-test-pinned exception class in `safeMessageExceptionClasses`.', + ) + ->identifier('forbidRawExceptionMessageInResponse.rawMessageInResponse') + ->line($node->getStartLine()) + ->build(); + } +} diff --git a/tests/Fixtures/RawExceptionMessageInResponse/AppAuthoredLiteralMessage.php b/tests/Fixtures/RawExceptionMessageInResponse/AppAuthoredLiteralMessage.php new file mode 100644 index 0000000..8057d82 --- /dev/null +++ b/tests/Fixtures/RawExceptionMessageInResponse/AppAuthoredLiteralMessage.php @@ -0,0 +1,19 @@ +getMessage()); + } +} diff --git a/tests/Fixtures/RawExceptionMessageInResponse/ErrorSinkDirectGetMessage.php b/tests/Fixtures/RawExceptionMessageInResponse/ErrorSinkDirectGetMessage.php new file mode 100644 index 0000000..fb6610c --- /dev/null +++ b/tests/Fixtures/RawExceptionMessageInResponse/ErrorSinkDirectGetMessage.php @@ -0,0 +1,18 @@ +getMessage()); + } +} diff --git a/tests/Fixtures/RawExceptionMessageInResponse/LeakSafeExempted.php b/tests/Fixtures/RawExceptionMessageInResponse/LeakSafeExempted.php new file mode 100644 index 0000000..fcb918c --- /dev/null +++ b/tests/Fixtures/RawExceptionMessageInResponse/LeakSafeExempted.php @@ -0,0 +1,18 @@ +getMessage()); + } +} diff --git a/tests/Fixtures/RawExceptionMessageInResponse/LeakSafeSameLineExempted.php b/tests/Fixtures/RawExceptionMessageInResponse/LeakSafeSameLineExempted.php new file mode 100644 index 0000000..3e147e7 --- /dev/null +++ b/tests/Fixtures/RawExceptionMessageInResponse/LeakSafeSameLineExempted.php @@ -0,0 +1,16 @@ +getMessage()); // @leak-safe: app-authored, payload-free + } +} diff --git a/tests/Fixtures/RawExceptionMessageInResponse/LogFacadeDirectGetMessage.php b/tests/Fixtures/RawExceptionMessageInResponse/LogFacadeDirectGetMessage.php new file mode 100644 index 0000000..5298869 --- /dev/null +++ b/tests/Fixtures/RawExceptionMessageInResponse/LogFacadeDirectGetMessage.php @@ -0,0 +1,19 @@ +getMessage())` — direct raw message to the Log facade. + // Server-side logging; the static-logger exclusion must hold it silent + // EVEN when Log::error is configured as a sink. + Log::error($e->getMessage()); + } +} diff --git a/tests/Fixtures/RawExceptionMessageInResponse/LoggerHelperGetMessage.php b/tests/Fixtures/RawExceptionMessageInResponse/LoggerHelperGetMessage.php new file mode 100644 index 0000000..7c86e46 --- /dev/null +++ b/tests/Fixtures/RawExceptionMessageInResponse/LoggerHelperGetMessage.php @@ -0,0 +1,18 @@ +error(...)` — the helper form of server-side logging. The + // logger exclusion must hold this silent EVEN when the logger method is + // configured as a sink (the exclusion short-circuits before sink match). + logger()->error($e->getMessage()); + } +} diff --git a/tests/Fixtures/RawExceptionMessageInResponse/LogsGetMessage.php b/tests/Fixtures/RawExceptionMessageInResponse/LogsGetMessage.php new file mode 100644 index 0000000..3737d65 --- /dev/null +++ b/tests/Fixtures/RawExceptionMessageInResponse/LogsGetMessage.php @@ -0,0 +1,21 @@ + $e->getMessage()]); + logger()->error($e->getMessage()); + report($e); + } +} diff --git a/tests/Fixtures/RawExceptionMessageInResponse/NonThrowableGetMessage.php b/tests/Fixtures/RawExceptionMessageInResponse/NonThrowableGetMessage.php new file mode 100644 index 0000000..bc11df3 --- /dev/null +++ b/tests/Fixtures/RawExceptionMessageInResponse/NonThrowableGetMessage.php @@ -0,0 +1,18 @@ +getMessage()); + } +} diff --git a/tests/Fixtures/RawExceptionMessageInResponse/NullsafeGetMessage.php b/tests/Fixtures/RawExceptionMessageInResponse/NullsafeGetMessage.php new file mode 100644 index 0000000..96de279 --- /dev/null +++ b/tests/Fixtures/RawExceptionMessageInResponse/NullsafeGetMessage.php @@ -0,0 +1,19 @@ +` is a distinct + // AST node (NullsafeMethodCall) but leaks identically when non-null. + // Fires. + return Response::error('Invalid input: ' . $e?->getMessage()); + } +} diff --git a/tests/Fixtures/RawExceptionMessageInResponse/PersistSinkGetMessage.php b/tests/Fixtures/RawExceptionMessageInResponse/PersistSinkGetMessage.php new file mode 100644 index 0000000..675febe --- /dev/null +++ b/tests/Fixtures/RawExceptionMessageInResponse/PersistSinkGetMessage.php @@ -0,0 +1,23 @@ +log->recordError('failed: ' . $e->getMessage()); + } +} diff --git a/tests/Fixtures/RawExceptionMessageInResponse/PsrLoggerGetMessage.php b/tests/Fixtures/RawExceptionMessageInResponse/PsrLoggerGetMessage.php new file mode 100644 index 0000000..6ca6467 --- /dev/null +++ b/tests/Fixtures/RawExceptionMessageInResponse/PsrLoggerGetMessage.php @@ -0,0 +1,24 @@ +reporter->logger->error($e->getMessage()); + } +} diff --git a/tests/Fixtures/RawExceptionMessageInResponse/SafeMessageExceptionPassthrough.php b/tests/Fixtures/RawExceptionMessageInResponse/SafeMessageExceptionPassthrough.php new file mode 100644 index 0000000..7a33f50 --- /dev/null +++ b/tests/Fixtures/RawExceptionMessageInResponse/SafeMessageExceptionPassthrough.php @@ -0,0 +1,20 @@ +getMessage()); + } +} diff --git a/tests/Fixtures/RawExceptionMessageInResponse/SafeMessageExceptionThrowableItself.php b/tests/Fixtures/RawExceptionMessageInResponse/SafeMessageExceptionThrowableItself.php new file mode 100644 index 0000000..0cc3c3b --- /dev/null +++ b/tests/Fixtures/RawExceptionMessageInResponse/SafeMessageExceptionThrowableItself.php @@ -0,0 +1,19 @@ +` / PSR `LoggerInterface` +// log-level call, or `report()`, is never a leak. +// +// composer.json requires none of laravel/mcp, psr/log, or illuminate/support, +// so the sink base, the PSR logger contract, the Log facade, the `logger()` / +// `report()` helpers, and a non-Throwable object carrying a `getMessage()` +// method are stubbed here with their real FQCNs — enough for PHPStan's FQCN +// resolution + subtype inference without pulling the real packages. `Throwable` +// and its exception hierarchy (`Exception`, `RuntimeException`, +// `InvalidArgumentException`) are PHP core, so fixtures use them directly with +// no stub. Mirrors the sibling rules' fixture-stub approach. + +namespace Laravel\Mcp { + // The MCP tool response. `error()` is the confirmed client-facing sink — + // ublgenie's 8 MCP tools + codebook DeleteChapterTool all return + // `Response::error('...' . $e->getMessage())`. + final class Response + { + public static function error(string $message): self + { + return new self; + } + } +} + +namespace Psr\Log { + // Minimal PSR-3 logger contract — the instance-logger exclusion resolves a + // receiver's subtype against this. + interface LoggerInterface + { + public function error(string $message, array $context = []): void; + + public function info(string $message, array $context = []): void; + } +} + +namespace Illuminate\Support\Facades { + // The Log facade — static log-level calls resolve against this FQCN. + class Log + { + public static function error(string $message, array $context = []): void {} + + public static function info(string $message, array $context = []): void {} + } +} + +namespace App\Support { + use Psr\Log\LoggerInterface; + + // A consumer-side PERSIST sink — records a failure message to a store. Not + // a logger; a configured `App\Support\InvoiceLog::recordError` sink. + final class InvoiceLog + { + public function recordError(string $message): void {} + } + + // A non-Throwable object that happens to expose getMessage() — the type + // gate must NOT treat its getMessage() as a leak. + final class ValidatorBag + { + public function getMessage(): string + { + return 'app-authored validation summary'; + } + } + + // A class holding an injected PSR logger, for the instance-logger exclusion. + final class ReportsToLogger + { + public function __construct( + public LoggerInterface $logger, + ) {} + } +} + +namespace { + use Psr\Log\LoggerInterface; + + // The `logger()` helper — normally in illuminate/foundation's helpers file. + if (!\function_exists('logger')) { + function logger(): LoggerInterface + { + return new class implements LoggerInterface { + public function error(string $message, array $context = []): void {} + + public function info(string $message, array $context = []): void {} + }; + } + } + + // The `report()` helper — normally in illuminate/foundation's helpers file. + if (!\function_exists('report')) { + function report(Throwable $throwable): void {} + } +} + +namespace App\Exceptions { + // A domain exception whose message discipline the consuming territory pins + // as app-authored (the codebook DeleteChapterTool / + // DependentModelRelationException shape) — the fixture target for the + // `safeMessageExceptionClasses` allowlist. + class DependentModelRelationException extends \RuntimeException {} +} diff --git a/tests/Rules/ForbidRawExceptionMessageInResponseRuleTest.php b/tests/Rules/ForbidRawExceptionMessageInResponseRuleTest.php new file mode 100644 index 0000000..b511ccb --- /dev/null +++ b/tests/Rules/ForbidRawExceptionMessageInResponseRuleTest.php @@ -0,0 +1,278 @@ + + */ +final class ForbidRawExceptionMessageInResponseRuleTest extends RuleTestCase +{ + private const string MESSAGE = 'Raw exception message reaches a client-facing response sink. ' + . 'Passing Throwable::getMessage() (or the Throwable itself) to a response leaks internal detail ' + . '(stack-trace fragments, SQL, file paths) to the API client. Log the raw message server-side ' + . '(Log::/report()) and return a stable, app-authored message. ' + . 'Suppress a proven-safe app-authored message with a `// @leak-safe: ` comment on the sink line, ' + . 'or list an arch-test-pinned exception class in `safeMessageExceptionClasses`.'; + + private const string STUBS = __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/_stubs.php'; + + /** + * Override hook: when set, `getRule()` returns this instance instead of the + * default. Lets a single test reconfigure `rawExceptionMessageSinks`. + */ + private ?Rule $ruleOverride = null; + + public function testFlagsConcatGetMessageIntoResponseError(): void + { + // The dominant MCP-tool shape: Response::error('x: ' . $e->getMessage()). + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/ErrorSinkConcatGetMessage.php'], + [[self::MESSAGE, 16]], + ); + } + + public function testFlagsDirectGetMessageIntoResponseError(): void + { + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/ErrorSinkDirectGetMessage.php'], + [[self::MESSAGE, 16]], + ); + } + + public function testFlagsThrowableItselfIntoSink(): void + { + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/ThrowableItselfIntoSink.php'], + [[self::MESSAGE, 16]], + ); + } + + public function testFlagsConfiguredPersistSink(): void + { + // The persist sink fires ONLY when its signature is configured. + $this->ruleOverride = new ForbidRawExceptionMessageInResponseRule( + ['App\Support\InvoiceLog::recordError'], + ); + + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/PersistSinkGetMessage.php'], + [[self::MESSAGE, 21]], + ); + } + + public function testIgnoresPersistSinkWhenNotConfigured(): void + { + // Same fixture under the default config — the persist sink is not armed, + // so the raw message flowing into it is silent. Pins "empty param = + // only Response::error built-in". + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/PersistSinkGetMessage.php'], + [], + ); + } + + public function testIgnoresLoggingOfRawMessage(): void + { + // Log::error / logger()->error / report() — the remediation, never a leak. + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/LogsGetMessage.php'], + [], + ); + } + + public function testIgnoresAppAuthoredLiteralMessage(): void + { + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/AppAuthoredLiteralMessage.php'], + [], + ); + } + + public function testIgnoresLeakSafeExemptedSink(): void + { + // `// @leak-safe:` marker in the comment block above the sink call. + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/LeakSafeExempted.php'], + [], + ); + } + + public function testIgnoresGetMessageOnNonThrowable(): void + { + // getMessage() on a non-Throwable receiver — the type gate keeps it silent. + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/NonThrowableGetMessage.php'], + [], + ); + } + + public function testIgnoresInstancePsrLoggerUnderDefaultConfig(): void + { + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/PsrLoggerGetMessage.php'], + [], + ); + } + + public function testLoggerExclusionOverridesAConfiguredLoggerSink(): void + { + // Even if a consumer (mis)configures a logger method as a sink, the + // logger/report exclusion short-circuits first — server-side logging is + // never criminalized. Gives the mandatory exclusion real teeth. + $this->ruleOverride = new ForbidRawExceptionMessageInResponseRule( + ['Psr\Log\LoggerInterface::error'], + ); + + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/PsrLoggerGetMessage.php'], + [], + ); + } + + public function testStaticLoggerExclusionOverridesAConfiguredLogFacadeSink(): void + { + // Configure the Log facade method AS a sink; the static-logger exclusion + // must still hold `Log::error($e->getMessage())` silent. Gives the + // static-call branch of the exclusion real teeth. + $this->ruleOverride = new ForbidRawExceptionMessageInResponseRule( + ['Illuminate\Support\Facades\Log::error'], + ); + + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/LogFacadeDirectGetMessage.php'], + [], + ); + } + + public function testHelperLoggerExclusionOverridesAConfiguredLoggerSink(): void + { + // Configure the PSR logger method AS a sink; `logger()->error(...)` (the + // helper's return type IS LoggerInterface, so it would match the sink) + // must still be held silent by the logger()-helper exclusion branch. + $this->ruleOverride = new ForbidRawExceptionMessageInResponseRule( + ['Psr\Log\LoggerInterface::error'], + ); + + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/LoggerHelperGetMessage.php'], + [], + ); + } + + public function testIgnoresSameLineLeakSafeMarker(): void + { + // `// @leak-safe:` trailing comment on the sink call line itself. + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/LeakSafeSameLineExempted.php'], + [], + ); + } + + public function testFlagsNullsafeGetMessageIntoResponseError(): void + { + // `$e?->getMessage()` is a NullsafeMethodCall — a distinct AST node the + // MethodCall-only matcher missed (the #59 review's Nit). Same leak. + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/NullsafeGetMessage.php'], + [[self::MESSAGE, 17]], + ); + } + + public function testFlagsSafeMessageExceptionUnderDefaultConfig(): void + { + // No allowlist configured — a prove-safe class is still an exception + // like any other. Pins "empty param = no class-level exemption". + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/SafeMessageExceptionPassthrough.php'], + [[self::MESSAGE, 18]], + ); + } + + public function testIgnoresConfiguredSafeMessageExceptionGetMessage(): void + { + // The #59 review's Major: the rule's own motivating example (codebook + // DeleteChapterTool passing DependentModelRelationException::getMessage(), + // arch-test-pinned app-authored) needed a config-level exemption, not a + // per-call-site annotation. Type-aware: subtypes inherit the allowance. + $this->ruleOverride = new ForbidRawExceptionMessageInResponseRule( + [], + ['App\Exceptions\DependentModelRelationException'], + ); + + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/SafeMessageExceptionPassthrough.php'], + [], + ); + } + + public function testSafeMessageExceptionDoesNotExemptTheThrowableItself(): void + { + // The allowlist covers the MESSAGE only — the Throwable stringifies + // with class, file, and trace regardless of message discipline. + $this->ruleOverride = new ForbidRawExceptionMessageInResponseRule( + [], + ['App\Exceptions\DependentModelRelationException'], + ); + + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/SafeMessageExceptionThrowableItself.php'], + [[self::MESSAGE, 17]], + ); + } + + public function testMalformedSinkSignaturesAreSkippedNotFatal(): void + { + // Garbage sink signatures (no `::`, too many `::`, empty class, empty + // method) are skipped by the signature parser — they neither crash nor + // arm a bogus sink, and the always-armed Response::error default still + // fires. Pins the parser's guard. + $this->ruleOverride = new ForbidRawExceptionMessageInResponseRule( + ['NoSeparatorHere', 'A::b::c', '::emptyClass', 'EmptyMethod::'], + ); + + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/ErrorSinkConcatGetMessage.php'], + [[self::MESSAGE, 16]], + ); + } + + public function testRuleResolvesFromExtensionNeonAndFiresOnDefaultSink(): void + { + // End-to-end pin on the extension.neon registration path consumers + // actually use: resolve the rule from the PHPStan container so the + // shipped `rawExceptionMessageSinks` default ([]) + the built-in + // Response::error sink wiring are exercised — NOT the PHP constructor + // default. A NEON regression would silently no-op the rule; this asserts + // the canonical Response::error leak still flags under the shipped wiring. + $this->ruleOverride = self::getContainer()->getByType(ForbidRawExceptionMessageInResponseRule::class); + + $this->analyse( + [self::STUBS, __DIR__ . '/../Fixtures/RawExceptionMessageInResponse/ErrorSinkConcatGetMessage.php'], + [[self::MESSAGE, 16]], + ); + } + + /** + * Load the shipped extension.neon so the container-resolved test can pull + * the rule out with its NEON-configured `rawExceptionMessageSinks`. + * + * @return array + */ + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/../../extension.neon', + ]; + } + + protected function getRule(): Rule + { + return $this->ruleOverride ?? new ForbidRawExceptionMessageInResponseRule; + } +}