fix(cli): default SSL cert file to certifi#20
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to make the CLI’s HTTPS certificate verification more reliable (especially in the frozen binary) by defaulting SSL_CERT_FILE to the CA bundle shipped with certifi, while still allowing users to override via SSL_CERT_FILE.
Changes:
- Default
SSL_CERT_FILEtocertifi.where()in the CLI entrypoint when the user hasn’t set it. - Add
certifias a direct runtime dependency. - Add unit tests covering defaulting behavior and preserving a user-provided
SSL_CERT_FILE.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/agentrun_cli/main.py |
Sets a default CA bundle path via SSL_CERT_FILE during module import. |
tests/unit/test_main.py |
Adds unit tests validating default and override behavior for SSL_CERT_FILE. |
pyproject.toml |
Declares certifi as a runtime dependency to support the new defaulting behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+28
to
+32
| monkeypatch.setattr(main_module.certifi, "where", lambda: "/tmp/cacert.pem") | ||
|
|
||
| importlib.reload(main_module) | ||
|
|
||
| assert main_module.os.environ["SSL_CERT_FILE"] == "/custom/ca.pem" |
Signed-off-by: 117503445 <t117503445@gmail.com>
117503445
force-pushed
the
fix-default-certifi-ca
branch
from
56fb50b to
aa54213
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SSL_CERT_FILEtocertifi.where()when the user has not set itcertifias a direct runtime dependencyRoot Cause
The v0.1.2 standalone binary can fail FC TempBucket HTTPS requests with
CERTIFICATE_VERIFY_FAILEDunlessSSL_CERT_FILEis manually set. The CLI should have a stable CA bundle by default while still allowing users to override it with the standardSSL_CERT_FILEenvironment variable.Validation
.venv/bin/ruff check src/ tests/.venv/bin/ruff format --check src/ tests/.venv/bin/mypy src/agentrun_cli.venv/bin/pytest tests/unit/test_main.py tests/integration/test_main.py.venv/bin/pytest tests/unit tests/integration(590 passed)make buildSSL_CERT_FILE/REQUESTS_CA_BUNDLEunset:skill createreturnedCREATING, first poll returnedREADY, then the test skill was deletedSupersedes #19 with a simpler business-code implementation.