---
title: "ADR-028: PHPat architectural lock for HTTP client construction"
manual: "nr-vault"
version: "1.0"
permalink: "https://docs.typo3.org/permalink/netresearch/nr-vault:adr-028-phpat-http-client-lock@1.0"
source: "Developer/Adr/ADR-028-PhpatHttpClientLock.rst"
rendered: "2026-09-18T07:37:50+00:00"
---

# ADR-028: PHPat architectural lock for HTTP client construction {#adr-028-phpat-architectural-lock-for-http-client-construction}

**Table of contents**

-   [Status](https://docs.typo3.org/permalink/netresearch/nr-vault:status@1.0)
-   [Date](https://docs.typo3.org/permalink/netresearch/nr-vault:date@1.0)
-   [Context](https://docs.typo3.org/permalink/netresearch/nr-vault:context@1.0)
-   [Decision](https://docs.typo3.org/permalink/netresearch/nr-vault:decision@1.0)
-   [Consequences](https://docs.typo3.org/permalink/netresearch/nr-vault:consequences@1.0)
-   [Verified](https://docs.typo3.org/permalink/netresearch/nr-vault:verified@1.0)
-   [References](https://docs.typo3.org/permalink/netresearch/nr-vault:references@1.0)

## Status {#status}

Accepted

## Date {#date}

2026-05-23

## Context {#context}

The vault enforces multiple layers of outbound HTTP defence:

-   `SecureHttpClientFactory::create()` configures the
    `isHostAllowed` allowlist, the DNS-rebinding middleware
    ([ADR-026: DNS-rebinding defence via CURLOPT_RESOLVE](https://docs.typo3.org/permalink/netresearch/nr-vault:adr-026-dns-rebinding-defence@1.0)), retry policy, and timeouts.
-   `OAuthTokenManager` ([ADR-027: OAuth token requests use the secure HTTP client](https://docs.typo3.org/permalink/netresearch/nr-vault:adr-027-oauth-client-unification@1.0))
    routes through the same factory so token endpoints inherit the
    same protections.

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 {#decision}

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

```php
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)',
        );
}
```

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 {#consequences}

### Positive {#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 {#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 {#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 {#references}

-   Pull request: [#146](https://github.com/netresearch/t3x-nr-vault/pull/146)
-   Related: [ADR-010: Secure Outbound inside nr-vault](https://docs.typo3.org/permalink/netresearch/nr-vault:adr-010-secure-outbound@1.0),
    [ADR-026: DNS-rebinding defence via CURLOPT_RESOLVE](https://docs.typo3.org/permalink/netresearch/nr-vault:adr-026-dns-rebinding-defence@1.0),
    [ADR-027: OAuth token requests use the secure HTTP client](https://docs.typo3.org/permalink/netresearch/nr-vault:adr-027-oauth-client-unification@1.0)
-   PHPat docs: [https://github.com/carlosas/phpat](https://github.com/carlosas/phpat)
