fix(ssl): extend DISABLE_SSL_VERIFY coverage to codex provider and four embedding adapters#465
Conversation
|
The Smoke Tests CI failures here are pre-existing on `dev` and unrelated to this diff. Stash-bisect against pristine `origin/dev` at `72bcdd7` ("prepare v1.3.9 release"): ``` Same shape as the CI run:
None of those modules are in the diff for this PR — the change is scoped to `services/llm/` (new `ssl_utils.py` + 4 provider/adapter sites) and `services/embedding/` (4 adapter sites). Surfaces I touched stay green: ``` Looks like `tests/api/` and `tests/services/test_path_service.py` need their own follow-up; happy to file a separate issue if useful. |
280f3e3 to
67b9b05
Compare
…ur embedding adapters v1.3.10 added the `openai_http_client.disable_ssl_verify_enabled` helper and applied it to the SDK paths (`openai_sdk` embedding, `openai_compat_provider`, `azure_openai_provider`, tutorbot `openai_compat_provider`). Five raw-httpx call sites still ignored the flag: - `provider_core/openai_codex_provider.py:79` hardcoded `verify=True` on the first attempt before falling back on `CERTIFICATE_VERIFY_FAILED`. - `embedding/adapters/openai_compatible.py:193` constructed `httpx.AsyncClient(timeout=...)` with no `verify` kwarg. - `embedding/adapters/jina.py:89`, `ollama.py:61`, `cohere.py:115` same. Each site now consults the helper and passes `verify=not disable_ssl_verify_enabled()`. The codex provider's `CERTIFICATE_VERIFY_FAILED` retry path is preserved unchanged. Tests: - `tests/services/embedding/test_disable_ssl_verify.py` (5 adapter tests + 1 production-guard test) asserts each adapter passes `verify=False` when the flag is set and `verify=True` otherwise. - `tests/services/llm/test_codex_disable_ssl_verify.py` covers the default-true case, flag-flip-to-false case, and the retry-on-cert-failure fallback.
67b9b05 to
93d24f7
Compare
Closes #464.
v1.3.10 added the
openai_http_client.disable_ssl_verify_enabledhelper and wired it through the SDK paths (openai_sdkembedding,openai_compat_provider,azure_openai_provider, tutorbotopenai_compat_provider). Thank you for shipping that helper — the design and prod-guard semantics are exactly the shape this PR was reaching for.After v1.3.10, five raw-httpx call sites still ignore
DISABLE_SSL_VERIFY:services/llm/provider_core/openai_codex_provider.py:79verify=Trueon first attempt; only retries onCERTIFICATE_VERIFY_FAILEDservices/embedding/adapters/openai_compatible.py:193httpx.AsyncClient(timeout=...)with noverifykwargservices/embedding/adapters/jina.py:89services/embedding/adapters/ollama.py:61services/embedding/adapters/cohere.py:115This PR adds the missing coverage by consulting the v1.3.10 helper at each site:
verify=not disable_ssl_verify_enabled()on the first attempt; theCERTIFICATE_VERIFY_FAILEDretry fallback is preserved.httpx.AsyncClient(timeout=..., verify=not disable_ssl_verify_enabled()).Tests
tests/services/embedding/test_disable_ssl_verify.py— 5 adapter tests assert each adapter passesverify=FalsewhenDISABLE_SSL_VERIFYis set (covers1,true,yes,ontruthy values across the four adapters), 1 test assertsverify=Trueby default, 1 test asserts the production-guardLLMConfigErrorpropagates through embed calls.tests/services/llm/test_codex_disable_ssl_verify.py— covers default-true, flag-flip-to-false on first attempt, and the cert-failure retry fallback (asserts both attempts: first withverify=True, retry withverify=False).pytest tests/services/llm/ tests/services/embedding/— 184 passed.pre-commit runover the changed files — ruff, ruff-format, bandit, mypy, detect-secrets all pass.Scope
This is complementary to the v1.3.10 sweep, not duplicative. The SDK-path coverage you already shipped stays unchanged; this only adds the helper to the five raw-httpx sites that weren't part of that pass.