Skip to content

fix: Guard response usage in validate_credentials #288

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.

Sign up for GitHub

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

Open
Blackoutta wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Guard response usage in validate_credentials #288

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion python/dify_plugin/interfaces/model/openai_compatible/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ def validate_credentials(self, model: str, credentials: dict) -> None:
:param credentials: model credentials
:return:
"""
response = None
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Initializing response = None is a crucial improvement. It prevents UnboundLocalError in scenarios where requests.post might raise an exception before response is assigned, ensuring that the variable is always defined when accessed in the except blocks.

try:
headers = {"Content-Type": "application/json"}

Expand Down Expand Up @@ -255,9 +256,15 @@ def validate_credentials(self, model: str, credentials: dict) -> None:
)
except CredentialsValidateFailedError:
raise
except requests.RequestException as ex:
raise CredentialsValidateFailedError(
f"Credentials validation request failed: {ex!s}"
) from ex
Comment on lines +259 to +262
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Adding a specific except requests.RequestException handler is a good practice. It allows for more granular error handling for network-related issues, providing a clearer error message to the user compared to a generic Exception. This improves the diagnostic capabilities of the system.

except Exception as ex:
response_body = response.text if response is not None else "<no response>"
raise CredentialsValidateFailedError(
f"An error occurred during credentials validation: {ex!s}, response body {response.text}"
f"An error occurred during credentials validation: {ex!s}, "
f"response body {response_body}"
) from ex
Comment on lines 263 to 268
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

While the conditional assignment to response_body correctly prevents an UnboundLocalError, including the raw response body in error messages creates a critical Server-Side Request Forgery (SSRF) vulnerability. Since the endpoint_url is user-supplied, an attacker can exfiltrate sensitive data from internal services by providing a malicious URL and reading the response in the error message. To remediate, avoid including the full response body in error messages or strictly validate endpoint_url against an allow-list and block internal IP ranges.


def get_customizable_model_schema(self, model: str, credentials: dict) -> AIModelEntity:
Expand Down