Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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: <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

- `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.
Expand Down
3 changes: 2 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` |
Expand Down Expand Up @@ -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

Expand Down
Loading
Loading