Confine Message::$params to the transport boundary

[Firehed]

Context

RFC 1 §8.1 designates a static rule as the enforcement mechanism for §4.8:

4.8 Capability negotiation — Static rule: raw initialize params reachable only within the negotiation component.

RawInitializeCapabilitiesRule (slice S1.1, #364) implements a narrow version of this: it reports reads of a literal capabilities key outside Firehed\PhpLsp\Capability. This issue originally tracked broadening that rule key-by-key. A survey of the codebase suggests a better mechanism is available for less work.

Note: the SessionCapabilities::fromMessage() escape hatch found during review of #364 was closed by construction in that PR — the value no longer exposes any Message-based constructor — so it is not part of this issue.

What is actually there

Excluding php-parser's unrelated Node->params (AST parameter lists), src/ contains exactly three reads of Message::$params:

Location	Verdict
src/Capability/CapabilityNegotiator.php:46	Legitimate — the negotiation tier
src/Protocol/TextDocumentPositionParams.php:22	Legitimate — this is the typed boundary object
src/Handler/TextDocumentSyncHandler.php:40	The one holdout

DefinitionHandler, HoverHandler, CompletionHandler, and SignatureHelpHandler all go through TextDocumentPositionParams::tryFromMessage($message) and never touch $params. The typed-params pattern is already established and already applied to four of the five handler families.

The stronger mechanism

Because there is only one holdout, the rule does not need to guess at key names:

Message::$params may only be read in src/Protocol/ and src/Capability/.

This keys on the property access rather than the subscript, so in one stroke it covers clientInfo, rootUri, initializationOptions, workspaceFolders, processId, variable keys, destructuring, and array_key_exists — every gap this issue originally enumerated separately. It also needs no mixed-vs-typed discrimination, because reading the server's own InitializeResult never touches Message::$params, so the false-positive class disappears rather than being filtered.

$params['clientInfo']['name'] is the case worth emphasising: it is the canonical per-client quirk conditional that §4.10 and §8 conformance item 8 name explicitly, it is plausible to write by accident, and nothing guards it today.

Why this was rejected before, and why that no longer holds

#364 considered and rejected this framing:

Framing it instead as "no Message::$params access outside a parser" would have needed an allowlist entry for the pre-existing TextDocumentSyncHandler / TextDocumentPositionParams readers, which Step Z forbids carrying.

That reasoning has a gap. TextDocumentPositionParams lives in src/Protocol/ — it is the boundary, not an exemption. And TextDocumentSyncHandler does not need exempting; it needs converting. With the conversion done, the allowlist that killed this idea never has to exist.

Prerequisite: convert TextDocumentSyncHandler

handle() passes $message->params down as a raw array, and the private handlers narrow it with assert():

$uri = $textDocument['uri'] ?? '';
$languageId = $textDocument['languageId'] ?? '';
assert(is_string($uri));
assert(is_string($languageId));

This is off-pattern by two separate house rules:

• assert() compiles out under zend.assertions=-1, the normal production setting. A malformed didOpen is then coerced silently or fails later inside DocumentManager, rather than being rejected at the boundary. RFC §9 requires malformed input to yield an error response, not a crash. Compare TextDocumentPositionParams::tryFromMessage(), which returns null on malformed input.
• The project's PHPStan guidance explicitly says not to use assert() to override an inferred type.

Converting this to typed params objects (DidOpenTextDocumentParams / DidChangeTextDocumentParams / DidCloseTextDocumentParams, or a shared TextDocumentIdentifier-based set) fixes the robustness bug and removes the last blocker to the rule above.

What remains after that

Two smaller items from the original scope survive independently of which rule shape is chosen:

1. The exemption is keyed on the declared namespace. $scope->getNamespace() === 'Firehed\PhpLsp\Capability' means any file anywhere can opt out by declaring that namespace. A path-based check matches §8.1's "within the negotiation component" more closely — and a path check is the natural shape for the src/Protocol/ + src/Capability/ rule anyway.
2. The rule's error identifier is not asserted. Changing ->identifier('phpLsp.rawInitializeCapabilities') leaves both rule tests green; PHPStan's RuleTestCase::analyse() compares message and line only. Low impact while the project uses no baseline and no @phpstan-ignore, but a silent change would break any future suppression with no signal.

If the Message::$params rule lands, RawInitializeCapabilitiesRule is subsumed by it and should be deleted rather than broadened.

Scheduling

• TextDocumentSyncHandler is already booked for surgery in S2.5 (migrate to SymbolSink). The typed-params conversion either folds into that slice or should be sequenced against it deliberately.
• The assert()-at-the-boundary robustness problem is S1.4-adjacent (malformed-frame robustness).
• None of this is in S1.1's scope.

Review corroboration (S1.1 cleanroom pass, #364)

A cleanroom review panel for #364 — given only the slice's acceptance criteria and the RFC, not this issue — independently reproduced its central conclusion:

• The Message::$params framing is the panel's own recommendation. Asked to judge RawInitializeCapabilitiesRule as a §8.1 mechanism, a reviewer concluded unprompted that "a provenance-based check (reachable-from-Message::params) would not have this hole" — the exact rule proposed above. The mechanism swap is corroborated by an independent read, not only the original survey.
• The destructuring bypass is empirically confirmed, not just reasoned. A throwaway RuleTestCase over a fixture reading ['capabilities' => $x] = $message->params ?? []; in a non-Capability namespace produced zero errors from the rule (the expected diagnostic never fires; the destructure key is an ArrayItem inside an Array_ assignment target, never the ArrayDimFetch the rule visits). The fixture was reverted after confirming.
• The MixedType discriminator's inline rationale is false. RawInitializeCapabilitiesRule.php:47–51 states "a read that resolves to a concrete type is indexing one of the server's own typed structures." That is untrue: narrowing the raw params to a concrete shape (e.g. a helper typed @param array{capabilities: array<string,mixed>}) also yields a non-Mixed type and is silently exempted. When the Message::$params rule replaces this one the comment is deleted with it; until then it should not be cited as sound. This is why the panel flagged the discriminator as "too clever" — a fresh reader could not independently justify why "resolves to mixed" is the right test, which was the pre-agreed signal to replace rather than harden.

None of this widens the issue's scope; it confirms the plan. The narrow rule that shipped in S1.1 is adequate for the direct-read case, the confinement additionally holds by construction (SessionCapabilities exposes no Message constructor), and this issue remains the tracked broadening.

---

This issue body was written by AI; its framing was directed by a human. The
"Review corroboration" section was added by AI from a review pass at a human's
direction.