-
Notifications
You must be signed in to change notification settings - Fork 20
feat: add verifySSL param #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request adds an optional boolean parameter, Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Client as InfisicalSDKClient
participant Req as InfisicalRequests
participant API as API Server
User->>Client: Instantiate with verifySSL option
Client->>Req: Pass host, token, cache_ttl, verifySSL
Req->>API: Execute HTTP request (using verifySSL setting)
API-->>Req: Return response
Req-->>Client: Return processed data
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
infisical_sdk/client.py (1)
10-18
: Consider updating constructor docstringWhile the new parameter is well-documented in the README, consider also documenting it in the constructor docstring for developers viewing the source code directly.
:param str host: The host URL for your Infisical instance. Will default to `https://app.infisical.com` if not specified. :param str token: The authentication token for the client. If not specified, you can use the `auth` methods to authenticate. :param int cache_ttl: The time to live for the secrets cache. This is the number of seconds that secrets fetched from the API will be cached for. Set to `None` to disable caching. Defaults to `60` seconds. + :param bool verifySSL: Whether to verify SSL certificates. Set to `False` to disable verification for self-signed certificates. Defaults to `True`. """
infisical_sdk/infisical_requests.py (2)
178-178
: Fix docstring in delete methodThis is a pre-existing issue unrelated to your changes, but the docstring for the delete method incorrectly states it makes a PATCH request.
- """Make a PATCH request with JSON data""" + """Make a DELETE request with JSON data"""
54-193
: Consider adding a warning about security implicationsSince disabling SSL verification can pose security risks in production environments, consider adding a warning in the docstring about the security implications.
def __init__(self, host: str, token: Optional[str] = None, verifySSL: bool = True): + """ + Initialize the Infisical requests client. + + :param str host: The host URL for your Infisical instance. + :param str token: Optional authentication token for the client. + :param bool verifySSL: Whether to verify SSL certificates. Set to `False` to disable + verification for self-signed certificates. Warning: Disabling + SSL verification may expose you to man-in-the-middle attacks. + Only use in development or with trusted networks. Defaults to `True`. + """ self.host = host.rstrip("/") self.session = requests.Session() self.verifySSL = verifySSL🧰 Tools
🪛 Ruff (0.8.2)
87-91: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
93-93: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
95-95: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md
(1 hunks)infisical_sdk/client.py
(2 hunks)infisical_sdk/infisical_requests.py
(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
infisical_sdk/client.py (1)
infisical_sdk/infisical_requests.py (1)
InfisicalRequests
(53-193)
🔇 Additional comments (7)
infisical_sdk/client.py (3)
10-10
: Well-implemented signature update with sensible defaultThe addition of the
verifySSL
parameter with a default value ofTrue
is a good practice as it maintains secure behavior by default while allowing flexibility for users with self-signed certificates.
21-21
: LGTM: Properly storing the parameterGood practice to store the parameter as an instance variable for potential future use.
23-23
: Correctly passing parameter to the requests classProperly propagating the
verifySSL
parameter to theInfisicalRequests
class ensures consistent SSL verification behavior throughout the SDK.README.md (1)
54-54
: Clear and helpful documentationThe documentation for the new parameter clearly explains its purpose, type, and default value, which is important for users to understand the security implications.
infisical_sdk/infisical_requests.py (3)
54-54
: Well-implemented parameter additionThe addition of the
verifySSL
parameter with a default value ofTrue
is consistent with the client class and maintains secure behavior by default.
57-57
: LGTM: Properly storing the parameterGood practice to store the parameter as an instance variable for use in the request methods.
112-112
: Consistently applying SSL verification across all HTTP methodsThe implementation correctly uses the
verify
parameter of the underlyingrequests
library in all HTTP methods, ensuring consistent behavior.Also applies to: 136-136, 160-160, 184-184
@DanielHougaard any chance you have time to see this? |
This PR adds a verifySSL bool parameter to the InfisicalSDKClient so requests to self-hosted servers with self-signed certificates work.
Summary by CodeRabbit
New Features
Documentation