Expose forwarded PROXY v2 TLS facts as request.connectionInfo

[kriszyp]

Closes #1983 (items 3 + 4).

Summary

#1858 taught Harper to parse PROXY protocol v2 on the UDS mirror, but only wired the mTLS client cert chain (0xE2) to getPeerCertificate. The other TLVs symphony forwards under sourceAddressHeader: 'proxyProtocolV2' were decoded and thrown away — so an application component behind Harper had no way to read the client's JA3/JA4 fingerprint, negotiated ALPN, SNI, or TLS version/cipher once the fronting proxy terminated TLS. That's the blocking dependency @hdbjeff flagged for the WooCommerce/WAF fingerprint work.

This extends the decoder to capture the full v2 TLV set into a ConnectionInfo and exposes it as request.connectionInfo:

request.connectionInfo // { alpn?, authority?, tls?: { version?, cipher?, verified? }, ja3?, ja4?, clientCertChain? } | undefined

TLVs parsed: ALPN 0x01, authority (SNI) 0x02, SSL 0x20 (with version 0x21 / cipher 0x23 sub-TLVs and the client-cert-present + verify bits), JA3 0xE0, JA4 0xE1, client cert chain 0xE2. Also fixes request.protocol to report https on the plaintext UDS mirror when the SSL TLV shows the client connection was TLS (it previously always returned http there → wrong X-Forwarded-Proto, #1983 item 4).

Design note (new API surface — approved shape)

request.connectionInfo is a new public Request accessor; shape per #1983 ({ alpn, authority, tls, ja3, ja4, clientCertChain }). Chosen over synthesizing x-ja4 headers because the TLV carrier is spoof-safe and works under h2 (nothing to inject into the request stream), and headers are client-forgeable. connectionInfo is populated only from a decoded PROXY v2 header on the filesystem-permissioned UDS mirror — same trust boundary as the existing v1 source-address override — and never from a request header.

Where to look

• mTLS preservation. The cert path refactors from the old internal header.tls to header.connectionInfo.clientCertChain; authorized + lazy/memoized getPeerCertificate stay gated on the 0xE2 chain exactly as Decode PROXY protocol v2 with forwarded mTLS client certificates on UDS mirrors #1858 had them. HTTP (security/auth.ts) and MQTT (server/mqtt.ts) mTLS are unchanged.
• connectionInfo.tls.verified semantics. It reflects the SSL TLV's verify bit (cert presented + proxy-verified), and can be true even when no chain was forwarded (symphony omits an oversized chain but still sends the SSL TLV). Documented on the field; apps wanting the standard "verified mTLS client" gate should use request.authorized, which is chain-gated. (Surfaced by the review — a doc clarification, not a behavior change.)
• Bounds. parseSslSubTlvs and the main TLV loop are byte-bounds-guarded (offset+3 <= end, valueEnd > end breaks); covered by unit tests + fuzz.

Not in scope

• v2 parsing / 0xE2 mTLS cert (already shipped in Decode PROXY protocol v2 with forwarded mTLS client certificates on UDS mirrors #1858); trust-gating to the UDS listener (Parse PROXY protocol v2 and expose TLS facts (JA3/JA4, ALPN, SNI, mTLS cert) to the JS request layer #1983 item 5, already the case).
• The h2-capable UDS mirror (Parse PROXY protocol v2 and expose TLS facts (JA3/JA4, ALPN, SNI, mTLS cert) to the JS request layer #1983 secondary) — its own issue.

Testing

• unitTests/server/serverHelpers/proxyProtocol.test.js + Request.test.js + udsMirror.test.js: 123 passing. New coverage for every TLV (ALPN/authority/SSL sub-TLVs/JA3/JA4/cert), JA3/JA4-without-SSL, connectionInfo on the socket, v1-has-no-connectionInfo, and protocol https-via-SSL-TLV.
• Full test:unit:main not run locally (pre-existing env failure — getUser stub / storage-path, reproduces on clean main); relying on CI.

Review

Cross-model (thorough): Codex + Gemini + Grok (xAI, per request) outside legs + Harper domain adjudication. No blockers, no significant concerns — the bounds safety, #1858 mTLS equivalence, and trust boundary were independently traced. The two suggestions (the verified doc note above; a per-connection Object.keys allocation) are applied.

Generated by an LLM (Claude Opus 4.8) pairing with Kris.

[kriszyp]

Docs: this adds a public Request accessor (request.connectionInfo) and changes request.protocol on the UDS mirror, so a companion PR to HarperFast/documentation (the Request/globals API reference) should follow — will file once the shape is confirmed in review. cc @hdbjeff — this is the Harper-side consumer for the JA3/JA4 forwarding you filed in #1983; request.connectionInfo.ja4 (and .ja3, .alpn, .tls, .clientCertChain) is how an app reads it. Note it needs symphony configured with sourceAddressHeader: 'proxyProtocolV2' + forwardFingerprint on the route.

🤖 Claude Opus 4.8, on Kris's behalf

[claude]

Reviewed; no blockers found. The lazy-allocation refactor in ea8eef1ed is correctness-preserving (equivalent to the prior direct-field-check gating), and Ethan's PP2_CLIENT_SSL inference concern is a resolved thread already reflected in the current code (clientBits & PP2_CLIENT_SSL gate at proxyProtocol.ts:185).

[gemini-code-assist]

Code Review

This pull request extends the PROXY v2 header parsing to decode and forward additional TLS connection facts (such as ALPN, SNI authority, TLS version/cipher, and JA3/JA4 fingerprints) via a new ConnectionInfo interface. These facts are stashed on the socket and exposed through request.connectionInfo. Additionally, the Request class's protocol getter is updated to correctly report 'https' on plaintext UDS mirrors when a fronting proxy forwards TLS facts. Unit tests have been updated accordingly. There are no review comments, and I have no feedback to provide.

[kriszyp]

Docs companion opened: HarperFast/documentation#614 — documents request.connectionInfo (the ConnectionInfo shape, a JA4 usage example, and the tls.verified-vs-mTLS trust caveat) and the request.protocol clarification on the Request reference.

🤖 Claude Opus 4.8, on Kris's behalf

[hdbjeff]

Requesting changes for the PP2_CLIENT_SSL protocol-inference issue below.
Otherwise this is solid, and I traced the parts most likely to be dangerous rather than taking the description on trust:

• Trust boundary holds. Production reaches applyProxyHeader through two direct call sites, both restricted to UDS-backed paths: enableProxyProtocol (http.ts:1653) for the HTTP/1 mirror, and consumeProxyHeader (proxyProtocol.ts:302), used by withProxyProtocol for both the h2c front (http.ts:1696) and the raw-protocol/MQTT UDS listener (threads/threadServer.js:619). Request.connectionInfo is a pure read of socket.connectionInfo; no request-header path reaches it.
• Bounds are correct. decodeV2 returns incomplete when buffer.length < headerLength, so the TLV loop bounded by headerLength cannot read past the buffer. Both the main loop and parseSslSubTlvs guard offset + 3 <= end and break on valueEnd > end, and a zero-length TLV still advances the offset by 3, so neither can spin.
• mTLS equivalence with #1858 holds. Old code set authorized = verified only under certPresented && clientCertChain; new code sets it only under chain, to info.tls?.verified === true where verified === certPresented && verifyOk. Since clientCertChain is itself only assigned under certPresented, the new value reduces to verifyOk — identical. The chain-present-but-no-SSL-TLV case also behaves the same.
• The tls.verified doc clarification, and the pointer to chain-gated request.authorized as the real mTLS gate, is the right call-out.

request.connectionInfo gives HarperFast/harper-woo-cache#212 exactly what it needs for the eventual PROXY-v2 migration, with application-level fingerprint validation retained. No other blockers found.

[hdbjeff]

Blocking inline — proxyProtocol.ts, SSL TLV branch (lines 171–176)

▎ [P2] Honor PP2_CLIENT_SSL before reporting the connection as TLS
▎
▎ This branch creates connectionInfo.tls for every structurally valid SSL TLV but never checks the TLV's client & 0x01 (PP2_CLIENT_SSL) flag — PP2_CLIENT_CERT_CONN (0x02) is the only client-field constant defined in the file. Request.protocol then treats the existence of connectionInfo.tls as proof that the client connected over TLS:
▎
▎ if (this._nodeRequest.socket.encrypted || this._nodeRequest.socket.connectionInfo?.tls) return 'https';
▎
▎ So an SSL TLV with client = 0x00 decodes to { tls: { verified: false } } — a truthy object — and request.protocol returns "https" for a cleartext client. That misreports the scheme in the unsafe direction, and it feeds X-Forwarded-Proto, redirects, secure-cookie decisions, and other scheme-gated behavior. Since only a proxy on the UDS can send this, it's a correctness bug rather than a vulnerability — but a silent one, and it undercuts item 4 of the PR description, which is specifically the protocol fix.
▎
▎ Worth noting the bit is already understood at the test layer: the fixture at unitTests/server/serverHelpers/proxyProtocol.test.js:436 builds Buffer.from([0x01 | 0x02]) and comments it client: SSL | CERT_CONN. It is simply never read in production, which is why no existing test catches this.
▎
▎ Suggested fix — gate the whole branch rather than adding a field:
▎
▎ const PP2_CLIENT_SSL = 0x01;
▎ // ...
▎ const clientBits = buffer[valueStart];
▎ if ((clientBits & PP2_CLIENT_SSL) !== 0) {
▎ certPresented = (clientBits & PP2_CLIENT_CERT_CONN) !== 0;
▎ const verifyOk = buffer.readUInt32BE(valueStart + 1) === 0;
▎ tls = { verified: certPresented && verifyOk };
▎ parseSslSubTlvs(buffer, valueStart + 5, valueEnd, tls);
▎ }
▎
▎ This keeps connectionInfo.tls meaning "the proxy reported TLS was used", so Request.protocol can continue testing object presence and the approved ConnectionInfo shape doesn't grow. It also tightens the cert path slightly, which I think is correct: a header asserting CERT_CONN without CLIENT_SSL is internally inconsistent — a client certificate is a TLS construct — so it would no longer produce authorized. Worth a sentence in the changelog if you agree.
▎
▎ Please also add a regression test using an SSL TLV with client = 0x00.

[hdbjeff]

Optional inline — the ConnectionInfo interface (lines 64–81)

▎ Non-blocking: could the ConnectionInfo docs state that the string fields are raw, unvalidated TLV payloads?
▎
▎ ja3, ja4, and alpn are decoded with latin1 and authority with UTF-8, with no grammar or length validation beyond the enclosing PROXY-header bounds. That's reasonable for a transport decoder — I'm not suggesting Harper impose Symphony's fingerprint grammar here — but consumers should validate before placing these values in headers, logs, cache keys, or security decisions, and the type doesn't currently signal that.
▎
▎ Concretely: the motivating consumer (HarperFast/harper-woo-cache#212) re-emits ja4 into an outbound header toward an origin WAF, and will keep its strict validateJa3/validateJa4 checks after migrating off the HTTP-header carrier. A malformed value there breaks origin fetches rather than injecting anything — Headers.set() throws on CR/LF — but that's the consumer having chosen correctly, not the API making it hard to get wrong. One clause in the doc comment would make the contract explicit.

[Ethan-Arrowood]

TLV bounds checking, the trust gate (proxy protocol only on the UDS mirror), and mTLS equivalence through the refactor all check out, with unit coverage on every new TLV. Zero per-request cost. The two suggestions in my notes (PP2_CLIENT_SSL client-field bit, explicit connectionInfo member on the Bun/uWS request classes) are non-blocking.

_{sent with Claude Fable 5}

[Ethan-Arrowood]

Following up on my approval with the two suggestions inline (both non-blocking).

_{sent with Claude Fable 5}

[Ethan-Arrowood]

One more (can't anchor it inline — the lines aren't in the diff): the Bun and uWS request classes in server/serverHelpers/Request.ts (~L401 / ~L511 at head) have no explicit connectionInfo member. Reads are undefined, which is consistent with "no v2 header", but the neighboring capability gaps there are made explicit (authorized, peerCertificate return documented sentinels with comments). An explicit get connectionInfo() { return undefined; } in the same style would keep the API surface self-documenting and the TS types uniform. Non-blocking.

_{sent with Claude Fable 5}

[kriszyp]

Addressed the review feedback:

• @hdbjeff — blocking: gated the SSL TLV branch on PP2_CLIENT_SSL (0x01), not just presence, so an SSL TLV with client = 0x00 no longer produces a truthy connectionInfo.tls (and request.protocol no longer reports https for a cleartext client). Applied your suggested fix as written, including treating a CERT_CONN bit without CLIENT_SSL as internally inconsistent. Added regression tests for both cases (client = 0x00, and CERT_CONN without CLIENT_SSL).
• @hdbjeff — non-blocking: documented alpn/authority/ja3/ja4 as raw, unvalidated TLV payloads on ConnectionInfo and each field.
• @Ethan-Arrowood — non-blocking: added explicit connectionInfo getters (returning undefined) on BunRequest/UwsRequest, matching the existing authorized/peerCertificate parity getters.

Also applied a suggestion from independent pre-push review: ConnectionInfo is now allocated lazily in decodeV2, so a metadata-free v2 connection no longer allocates and discards an empty object.

125 relevant unit tests pass; build and lint are clean.

🤖 Claude Opus, on Kris's behalf