Skip to content

Conversation

@ramusbucket
Copy link
Contributor

@ramusbucket ramusbucket commented Sep 10, 2025

When specify command is run behind a proxy (enterprise proxy like zscalar, or anti-virus web scan proxy, etc.), it hangs on Fetch latest release step, saying "contacting GitHub API". This PR fixes that issue with the use of local truststore with httpx and makes it work provided python cert settings are configured according to the proxy provider's documentation. For example: https://help.zscaler.com/zia/adding-custom-certificate-application-specific-trust-store

@ramusbucket ramusbucket changed the title Refactor HTTP client usage to utilize truststore for SSL context fix: Refactor HTTP client usage to utilize truststore for SSL context Sep 10, 2025
@nomagicln
Copy link

Good jobs, but I think it would be better if there were an option to skip TLS verification.

@localden localden self-assigned this Sep 10, 2025
@localden localden added enhancement merge-candidate Reasonable change that is going to be merged after a review. labels Sep 10, 2025
@localden
Copy link
Collaborator

This sounds like a reasonable change, but I am curious as @nomagicln called out - is it better to introduce a new package dependency vs. disable TLS verification altogether? Seems like the latter might be preferred.

@ramusbucket
Copy link
Contributor Author

Taken @localden @nomagicln, disabling TLS verification altogether is one option. To provide more flexibility, I've implemented the --skip-tls cli option. As a secure alternative for users with enterprise proxy certificates or self-signed certificates, I've also retained a package dependency that enables the use of a custom truststore.

@localden
Copy link
Collaborator

@ramusbucket I am more curious if just disabling TLS verification is enough? Seems like the threat vector of doing it is that if you are getting MITM-d, then you may download a bad package from someone that is injecting bad payloads. But even then, the CLI doesn't execute anything and just extracts the content so the risk is somewhat contained.

Also, looks like pip itself is using truststore, so I am less concerned about this particular dependency.

I'll review this PR and we can merge it soon.

@localden localden merged commit 0f0e19d into github:main Sep 11, 2025
jellydn pushed a commit to jellydn/spec-kit that referenced this pull request Sep 30, 2025
fix: Refactor HTTP client usage to utilize truststore for SSL context
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement merge-candidate Reasonable change that is going to be merged after a review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants