ADR-028: PHPat architectural lock for HTTP client construction 

Table of contents

Status 

Accepted

Date 

2026-05-23

Context 

The vault enforces multiple layers of outbound HTTP defence:

These defences are only worth what the construction site is worth. Any code that calls new \GuzzleHttp\Client(...) directly bypasses every middleware the factory installs — and that's exactly the kind of regression that looks fine in code review ("just adding a small HTTP call") but silently undoes the SSRF guard.

History shows this isn't theoretical: PR #145 review found one such site in adapter code that escaped earlier review cycles. We need a mechanical fence, not just convention.

Decision 

Add a PHPat architectural rule to Tests/Architecture/ArchitectureTest.php (verbatim from testOnlySecureHttpClientFactoryInstantiatesGuzzleClient):

public function testOnlySecureHttpClientFactoryInstantiatesGuzzleClient(): BuildStep
{
    return PHPat::rule()
        ->classes(Selector::inNamespace('Netresearch\\NrVault'))
        ->excluding(
            Selector::classname(SecureHttpClientFactory::class),
            Selector::inNamespace('Netresearch\\NrVault\\Tests'),
        )
        ->shouldNot()
        ->dependOn()
        ->classes(Selector::classname(Client::class))
        ->because(
            'all outbound HTTP must flow through SecureHttpClientFactory; '
            . 'instantiating GuzzleHttp\\Client directly bypasses SSRF + '
            . 'DNS-rebinding + no-redirect defences (PR #145)',
        );
}
Copied!

The rule uses shouldNot()->dependOn() rather than a hypothetical shouldNotConstruct. dependOn is intentionally broader: it forbids any reference to GuzzleHttp\\Client (new, use, type-hint, static call) outside the allowed namespaces. That's the strictest fence PHPat offers and matches the intent — production code shouldn't even name the class.

Allowed namespaces (an explicit allowlist, not a regex):

  • SecureHttpClientFactory — the single legitimate constructor.
  • Netresearch\\NrVault\\Tests — test doubles and fixtures need to construct GuzzleHttp\\Client instances directly to wire MockHandler-driven flows. Tests don't ship in the distributed extension, so they can't widen the production attack surface.

The rule runs inside PHPStan (phpstan.neon includes phpat.neon, which tags the test class phpat.test), so violations fail the PHPStan job with a deterministic message naming the offending class.

Consequences 

Positive 

  • Future regressions get a hard "cannot instantiate" failure at PHPat time, not at a security-audit time three months later.
  • Code review for new HTTP-using classes becomes mechanical: either the diff calls $factory->create(...) or PHPat fails.
  • Documents the rule alongside the rest of the architectural contract — anyone surveying Tests/Architecture/ sees the constraint without hunting for tribal knowledge.

Negative 

  • Genuine edge cases (e.g., a one-off testing-only HTTP client that intentionally skips the middleware) need explicit allowlist additions with PR-level justification. This is intentional friction — the friction is the value.
  • PHPat takes  1 s to evaluate the rule. Negligible.

Verified 

  • Rule passes on the current codebase (post-#146 cleanup).
  • Manually verified: inserting new Client() into any adapter fails composer ci with a PHPat error pointing at the file.

References