fix(ssl): stop shadowing the Windows trust store for child processes - #9370
Conversation
Design Review (Fable 5, fork) — ✅ PASSDesign-level review of The patch is a three-line platform guard plus two pinning tests; the base file and PR description fully support each other. I've verified the design gate: real harm (Windows corporate fleets behind TLS inspection lose chat entirely), root-cause fix at the layer that owns the export, the operator escape hatch preserved, trivially reversible, extensive manual verification on the affected host, and no test pins the opposite behavior. No design-level findings survived the kill-filter. Design-Verdict: PASS Removes the export at the layer that owns it, preserves the operator override, and pins placement with mirrored tests — minimal, verified, reversible. [DESIGN-REVIEWED] 50ef491 |
First Principles Review (Fable 5, fork) — ✅ PASSPremise-level review of The review contract, PR intent, patch, and base files are all read; the change is a three-line win32 guard in First-Principles-Verdict: PASS Verify the load-bearing vendor claim on the shipped kiro-cli: rustls-native-certs treats The description carries its own reproduction (three-run What this change shipsIntent: make chat work on Windows hosts behind TLS-inspecting proxies by not overriding kiro-cli's trust store — a FIX. Inventory (3 items)
[FIRST-PRINCIPLES-REVIEWED] 50ef491 |
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed |
GPT 5.6 Review (fork) — ✅ no blocking findingsReviewed Review detailsNo findings. |
|
Inherited failure, not from this change. |
_ensure_ssl_certs() has no Windows-aware branch, so on win32 the Unix CA candidates never exist, get_default_verify_paths().cafile is None, and the certifi fallback always exports SSL_CERT_FILE + REQUESTS_CA_BUNDLE. rustls-native-certs checks SSL_CERT_FILE first on every platform and loads that file INSTEAD of the platform store, so the export silently removed the Windows ROOT store from kiro-cli's trust while leaving the gateway's own HTTPS intact -- CPython reads the Windows store directly and treats the variable as an addition. Behind a TLS-inspecting proxy whose root is installed in Windows ROOT, every turn failed with "dispatch failure" on the intercepted q.<region>.amazonaws.com endpoint while sign-in and the usage API, which are not intercepted, kept working. Return before discovery on win32. Nothing there needs the export: CPython and requests are already covered, and Node children read NODE_EXTRA_CA_CERTS. An explicit SSL_CERT_FILE is still honoured above.
c0fb167 to
50ef491
Compare
cixuuz
left a comment
There was a problem hiding this comment.
Reviewed the complete current-head diff. The Windows guard preserves explicit SSL_CERT_FILE overrides, avoids shadowing the native trust store, and is covered by focused regression tests.
_ensure_ssl_certs() has no Windows-aware branch, so on win32 the Unix CA candidates never exist, get_default_verify_paths().cafile is None, and the certifi fallback always exports SSL_CERT_FILE + REQUESTS_CA_BUNDLE.
rustls-native-certs checks SSL_CERT_FILE first on every platform and loads that file INSTEAD of the platform store, so the export silently removed the Windows ROOT store from kiro-cli's trust while leaving the gateway's own HTTPS intact -- CPython reads the Windows store directly and treats the variable as an addition. Behind a TLS-inspecting proxy whose root is installed in Windows ROOT, every turn failed with "dispatch failure" on the intercepted q..amazonaws.com endpoint while sign-in and the usage API, which are not intercepted, kept working.
Return before discovery on win32. Nothing there needs the export: CPython and requests are already covered, and Node children read NODE_EXTRA_CA_CERTS. An explicit SSL_CERT_FILE is still honoured above.
Problem / Motivation
On Windows, Kiro Crew chat fails on every turn with
An unknown error occurred: dispatch failure, while sign-in and the usage/credit pill keep working. The naturaldiagnostic — run
kiro-cliin a terminal — passes, because a plain shell has noSSL_CERT_FILEset.The split is explained by which endpoints a TLS-inspecting proxy intercepts:
q.us-east-1.amazonaws.comZscaler Intermediate Root CA (zscalertwo.net)oidc.us-east-1.amazonaws.comAmazon RSA 2048 M04codewhisperer.us-east-1.amazonaws.comAmazon RSA 2048 M04Only the model endpoint is inspected, so only chat dies.
The Zscaler root is installed in both
Cert:\LocalMachine\RootandCert:\CurrentUser\Root, so every ordinary Windows application is fine. The shippeddesktop backend's certifi bundle holds 121 public roots and contains no Zscaler entry.
_ensure_ssl_certs()runs at import ofcli.py/__main__.py(and frommcp_gateway/gatewayd.py), so it is in the gateway process before any kiro-cli spawn.On Windows
ssl.get_default_verify_paths().cafileisNoneand all three_CA_CANDIDATESare Linux paths, so control always reaches the certifi fallback andexports
SSL_CERT_FILE+REQUESTS_CA_BUNDLE. kiro-cli inherits it.Why it matters
Kiro Crew is unusable for chat on any Windows host behind TLS inspection -- most
corporate Windows fleets. The failure is silent and misattributed: it presents as a
network fault or a Kiro CLI bug, and the obvious check (
kiro-cliin a terminal)passes, so the gateway is the last place anyone looks.
What changed (motivation → approach → change)
The asymmetry is what makes this invisible.
SSL_CERT_FILEmeans different things tothe two consumers:
create_default_context()on Windows loads the Windows ROOT storeand honours
SSL_CERT_FILE. A union. The gateway's own HTTPS keeps working,which hides the problem.
SSL_CERT_FILEfirst, on every platform, and when set loads that fileinstead of the platform store. A replacement.
So exporting a public-roots-only bundle subtracts every private CA the machine
trusts from the child, while leaving the parent working.
The Windows branch was never intentional. The module docstring scopes the file-based
bootstrap to "Linux distributions whose Python default points at the wrong CA
location"; Windows falls into it only because the Unix candidate paths don't exist.
Fix: return before discovery on
win32. Nothing there needs the export —are already covered.
requestsnever reads the Windows store, but its default is certifi, so pointingREQUESTS_CA_BUNDLEat certifi changes nothing.NODE_EXTRA_CA_CERTS, never this variable.Placement is deliberate: below the explicit-override check, so an operator's own
SSL_CERT_FILEstill wins (the current workaround for affected users, and thesupported way to add a private CA); above the discovery block, so no future edit
can reintroduce an export on that path.
Linux behaviour is unchanged. That branch is the module's stated purpose, the paths it
finds are real system bundles rather than public-roots-only files, and
rustls-native-certs reads
/etc/ssl/...there anyway.Tests
Two cases added to
test/test_ssl_certs.py::TestEnsureSslCerts:test_win32_exports_nothing_even_when_a_bundle_is_discoverable— the mirror of theexisting
test_falls_back_to_certifi_when_no_system_path_exists: identical setup,only the platform differs, opposite outcome. Both discovery paths are made to
succeed (an existing candidate path and a resolvable certifi bundle), so it pins
that the guard sits above discovery rather than merely that certifi goes
unreached. Confirmed to fail without the guard, exporting the bundle.
test_win32_still_honours_an_explicit_ssl_cert_file— pins that an operator'sexplicit bundle survives the guard. This passes without the fix too (the override
check already existed); it guards the placement rather than catching the defect.
Both override the file's autouse
_reset_ssl_bootstrapfixture, which forcessys.platform = "linux".Manual verification
Measured by the reporter on the affected Windows host:
TLS probe against the intercepted endpoint, trusting one bundle at a time:
End-to-end against the real
kiro-clibinary, three runs in one shell:The failing run also lost the model name from its status bar (
kiro_default · ◔ 4%versus
kiro_default · claude-opus-5), so model listing was failing on the samehandshake.
Prelude behaviour on that host, post-patch:
On Linux (this PR's development host):
test_ssl_certs.pypasses 20/20; black,isort, flake8 and mypy report nothing on the changed files; the repo black and
subprocess-encoding gates pass.
Related Issues
Fixes #NNNN (filed alongside this PR — no existing issue covered the Windows case).
Related: #5602 — the macOS sibling of this class, same TLS-inspection context but
about this process's own trust rather than the child's. Still open after #5903
landed the Keychain work; see Open question 1.
Open questions for maintainers
macOS may carry the same defect, and Bundled Python ignores macOS Keychain trust store — SSL handshakes to MCP endpoints fail behind Zscaler/TLS-inspection proxies #5602 is where it would land. The
darwinbranch calls_inject_macos_system_trust()(added by fix(tls): honor macOS Keychain trust #5903) and thenfalls through to discovery — there is no return.
inject_into_ssl()isprocess-local, so the export exists deliberately to give children something they
cannot inherit as a monkey-patch. But if discovery lands on certifi, a rustls child
skips the Keychain for exactly the Windows reason — which would explain why Bundled Python ignores macOS Keychain trust store — SSL handshakes to MCP endpoints fail behind Zscaler/TLS-inspection proxies #5602
is still open after fix(tls): honor macOS Keychain trust #5903. Whether it fires depends on whether
get_default_verify_paths().cafileexists on a given macOS install, which was nottestable from the affected host. Flagging rather than fixing: widening this diff
onto an unverifiable platform is how a three-line fix stalls. Happy to take it as a
follow-up against Bundled Python ignores macOS Keychain trust store — SSL handshakes to MCP endpoints fail behind Zscaler/TLS-inspection proxies #5602 if you agree with the reading.
Want a
windows-install.mdnote? No spec covers the SSL bootstrap (greppeddocs/**andsrc/kiro_crew/docs/**for_ssl_compat,_ensure_ssl_certs,SSL_CERT_FILE,REQUESTS_CA_BUNDLE— no hits), so nothing goes stale. A short"corporate TLS inspection" section explaining that
SSL_CERT_FILEis the supportedway to add a private CA might help affected users find the workaround, but that is
scope you did not ask for. Say the word and I'll add it here or as a follow-up.
Interaction with fix(acp): classify prose-spelled dispatch failure as transient #9330. That PR classified the prose spelling of
dispatch failureas transient, i.e. retry-eligible. A TLS trust failure is permanent, so onan affected host each turn now burns the retry ladder before surfacing. It does not
change this fix, but the two share a symptom and it is worth knowing they interact.
Pattern harvest
Checklist
Contribution License Agreement