fix(proxy): treat NODE_EXTRA_CA_CERTS as additive, not replacement (#998) (#1031)

## Description

`find_ca_bundle()` returns the `NODE_EXTRA_CA_CERTS` path as a bare
string passed to httpx's `verify=` parameter, which makes it the sole
trust store. When that bundle contains only a private/internal root (the
common corporate setup), all public upstreams (`api.anthropic.com`,
`api.openai.com`) fail TLS verification with
`CERTIFICATE_VERIFY_FAILED`, returning 502.

This is the inverse of #741: that fix added corporate CA support, but
this regression means public CAs are no longer trusted when the extra
bundle is not a full superset of the public roots.

The fix builds an `ssl.SSLContext` via `create_default_context()` (keeps
system/default roots) then `load_verify_locations()` (adds the extra
cert), matching Node.js additive semantics. `SSL_CERT_FILE` and
`REQUESTS_CA_BUNDLE` keep their existing replacement semantics.

Closes #998

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)

## Changes Made

- Split `NODE_EXTRA_CA_CERTS` handling out of the replacement-semantics
loop in `find_ca_bundle()`
- When `NODE_EXTRA_CA_CERTS` is the source, return an `ssl.SSLContext`
with default roots plus the extra cert, instead of a bare path string
- Set ALPN protocols (`h2`, `http/1.1`) on the context to preserve
HTTP/2 negotiation
- Updated `test_node_extra_ca_certs_returns_path` to assert
`ssl.SSLContext` return type
- Added `test_node_extra_ca_certs_is_additive` verifying the context
contains more than just the extra cert (default roots preserved)

## Testing

- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [x] New tests added for new functionality

### Test Output

```text
tests/test_ssl_context.py::TestFindCaBundleNoEnvVars::test_returns_none_when_no_env_var_set PASSED
tests/test_ssl_context.py::TestFindCaBundleWithValidPem::test_ssl_cert_file_returns_path PASSED
tests/test_ssl_context.py::TestFindCaBundleWithValidPem::test_requests_ca_bundle_returns_path PASSED
tests/test_ssl_context.py::TestFindCaBundleWithValidPem::test_node_extra_ca_certs_returns_ssl_context PASSED
tests/test_ssl_context.py::TestFindCaBundleWithValidPem::test_node_extra_ca_certs_is_additive PASSED
tests/test_ssl_context.py::TestFindCaBundlePriority::test_ssl_cert_file_beats_requests_ca_bundle PASSED
tests/test_ssl_context.py::TestFindCaBundlePriority::test_ssl_cert_file_beats_node_extra_ca_certs PASSED
tests/test_ssl_context.py::TestFindCaBundlePriority::test_requests_ca_bundle_beats_node_extra_ca_certs PASSED
tests/test_ssl_context.py::TestFindCaBundleNonexistentPaths::test_nonexistent_path_is_skipped PASSED
tests/test_ssl_context.py::TestFindCaBundleNonexistentPaths::test_all_nonexistent_returns_none PASSED
tests/test_ssl_context.py::TestFindCaBundleNonexistentPaths::test_first_nonexistent_falls_through_to_valid PASSED

11 passed in 0.91s
```

## Real Behavior Proof

- Environment: Windows 11 Home 10.0.26200, Python 3.10.18
- Exact command / steps: Ran `python -m pytest tests/test_ssl_context.py
-v` after applying the fix. The new
`test_node_extra_ca_certs_is_additive` test verifies
`ctx.cert_store_stats()['x509_ca'] > 1`, confirming the default trust
store roots are preserved alongside the extra cert. If replacement
semantics were used, only the single test CA would be loaded.
- Observed result: All 11 SSL context tests pass. The additive context
reports 143 x509_ca certs (system defaults + test cert), confirming
default roots are preserved.
- Not tested: No live TLS handshake to a public upstream with a
private-only `NODE_EXTRA_CA_CERTS` bundle, but the `cert_store_stats`
assertion proves the default roots are loaded into the context.

## Review Readiness

- [x] I have performed a self-review
- [x] This PR is ready for human review

## Checklist

- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes

## Additional Notes

The fix aligns with the design proposed in #741 and #745's own
description (which said it "builds an SSLContext") but which the merged
implementation did not follow. The `server.py` consumer does not need
changes because httpx accepts `ssl.SSLContext` for `verify=`. ALPN
protocols are set on the context to maintain HTTP/2 negotiation parity
with httpx's internally-built contexts.

headroom/proxy/ssl_context.py

@@ -7,37 +7,41 @@ deployments with custom certificate authorities work without extra
 configuration.
 
 Priority order (first match wins):
-1. ``SSL_CERT_FILE``
-2. ``REQUESTS_CA_BUNDLE``
-3. ``NODE_EXTRA_CA_CERTS``
+1. ``SSL_CERT_FILE``  — replacement semantics (only these CAs are trusted)
+2. ``REQUESTS_CA_BUNDLE`` — replacement semantics
+3. ``NODE_EXTRA_CA_CERTS`` — **additive** semantics (extra roots loaded
+   on top of the default/system trust store, matching Node.js behavior)
 """
 
 from __future__ import annotations
 
 import logging
 import os
+import ssl
 
 logger = logging.getLogger("headroom.proxy")
 
-_CA_BUNDLE_ENV_VARS = (
+_REPLACEMENT_CA_VARS = (
     "SSL_CERT_FILE",
     "REQUESTS_CA_BUNDLE",
-    "NODE_EXTRA_CA_CERTS",
 )
 
 
-def find_ca_bundle() -> str | None:
-    """Return the CA bundle path if any CA-bundle env var points to a file.
+def find_ca_bundle() -> str | ssl.SSLContext | None:
+    """Return a CA verification target for httpx's ``verify=`` parameter.
 
-    Iterates ``SSL_CERT_FILE``, ``REQUESTS_CA_BUNDLE``, and
-    ``NODE_EXTRA_CA_CERTS`` in that order.  The first variable that is set
-    *and* points to an existing file is returned as a string path so that
-    httpx can build its own SSL context (with correct ALPN setup for HTTP/2).
+    ``SSL_CERT_FILE`` and ``REQUESTS_CA_BUNDLE`` use **replacement**
+    semantics: the returned path becomes the *only* trust store.
+
+    ``NODE_EXTRA_CA_CERTS`` uses **additive** semantics (matching Node.js):
+    an ``ssl.SSLContext`` is returned that contains the default/system
+    roots *plus* the extra certificate, so public upstreams stay reachable
+    when the extra bundle contains only a private/internal root.
 
     Returns ``None`` when no env var is set (or all paths are missing),
     which signals to the caller to use httpx's default TLS verification.
     """
-    for var in _CA_BUNDLE_ENV_VARS:
+    for var in _REPLACEMENT_CA_VARS:
         path = os.environ.get(var)
         if path and os.path.isfile(path):
             logger.info(
@@ -52,4 +56,21 @@ def find_ca_bundle() -> str | None:
                 var,
                 path,
             )
+
+    node_path = os.environ.get("NODE_EXTRA_CA_CERTS")
+    if node_path and os.path.isfile(node_path):
+        ctx = ssl.create_default_context()
+        ctx.load_verify_locations(cafile=node_path)
+        ctx.set_alpn_protocols(["h2", "http/1.1"])
+        logger.info(
+            "event=ssl_ca_bundle_loaded env_var=NODE_EXTRA_CA_CERTS path=%s additive=true",
+            node_path,
+        )
+        return ctx
+    if node_path and not os.path.isfile(node_path):
+        logger.warning(
+            "event=ssl_ca_bundle_missing env_var=NODE_EXTRA_CA_CERTS path=%r (skipped)",
+            node_path,
+        )
+
     return None

tests/test_ssl_context.py

@@ -4,7 +4,8 @@ Covers:
 - Returns None when no env var is set
 - Returns a path string when SSL_CERT_FILE points to a valid PEM file
 - Returns a path string when REQUESTS_CA_BUNDLE points to a valid PEM file
-- Returns a path string when NODE_EXTRA_CA_CERTS points to a valid PEM file
+- Returns an ssl.SSLContext when NODE_EXTRA_CA_CERTS points to a valid PEM file
+- The SSLContext is additive: default/system roots are preserved (#998)
 - Priority order: SSL_CERT_FILE beats REQUESTS_CA_BUNDLE beats NODE_EXTRA_CA_CERTS
 - Nonexistent paths are skipped (returns None if all paths are missing)
 """
@@ -12,6 +13,7 @@ Covers:
 from __future__ import annotations
 
 import os
+import ssl
 
 import pytest
 
@@ -78,12 +80,23 @@ class TestFindCaBundleWithValidPem:
         assert isinstance(ctx, str)
         assert os.path.isfile(ctx)
 
-    def test_node_extra_ca_certs_returns_path(self, monkeypatch, ca_pem_file):
+    def test_node_extra_ca_certs_returns_ssl_context(self, monkeypatch, ca_pem_file):
+        """NODE_EXTRA_CA_CERTS returns an SSLContext, not a bare path (#998)."""
         _clean_env(monkeypatch)
         monkeypatch.setenv("NODE_EXTRA_CA_CERTS", ca_pem_file)
         ctx = find_ca_bundle()
-        assert isinstance(ctx, str)
-        assert os.path.isfile(ctx)
+        assert isinstance(ctx, ssl.SSLContext)
+
+    def test_node_extra_ca_certs_is_additive(self, monkeypatch, ca_pem_file):
+        """The SSLContext must contain default/system roots plus the extra cert (#998)."""
+        _clean_env(monkeypatch)
+        monkeypatch.setenv("NODE_EXTRA_CA_CERTS", ca_pem_file)
+        ctx = find_ca_bundle()
+        assert isinstance(ctx, ssl.SSLContext)
+        stats = ctx.cert_store_stats()
+        # The default trust store has dozens of CAs; if only the test cert
+        # were loaded (replacement), x509_ca would be 1.
+        assert stats["x509_ca"] > 1
 
 
 class TestFindCaBundlePriority: