Skip to content

fix(security): close remaining SSRF (CWE-918) call sites via shared URL validator - #6520

Open
liuruibin with Copilot wants to merge 2 commits into
v2from
copilot/fix-ssrf-vulnerability
Open

fix(security): close remaining SSRF (CWE-918) call sites via shared URL validator#6520
liuruibin with Copilot wants to merge 2 commits into
v2from
copilot/fix-ssrf-vulnerability

Conversation

Copilot AI commented Jul 27, 2026

Copy link
Copy Markdown

PR #6455 fixed SSRF in application.py by replacing startswith() prefix checks with proper urlparse-based validation, but left 6 identical vulnerable call sites unpatched across tools/serializers/tool.py, tools/serializers/tool_workflow.py, and knowledge/serializers/knowledge_workflow.py.

Changes

New shared utility: apps/common/utils/url_validator.py

  • validate_trusted_url(url, allowed_hosts)urlparse-based validator: requires https scheme, exact hostname allowlist match, rejects userinfo (user:pass@host) and explicit ports
  • ALLOWED_DOWNLOAD_HOSTS = {"apps-assets.fit2cloud.com"}
  • ALLOWED_CALLBACK_HOSTS = {"apps.fit2cloud.com"}

apps/application/serializers/application.py

  • Replaced local _validate_trusted_url / _ALLOWED_* definitions with imports from the shared module (behavior unchanged)

apps/tools/serializers/tool.py

  • Replaced startswith() in Create.insert, AddStoreTool.add, and UpdateStoreTool.update_tool (download + callback)
  • UpdateStoreTool.update_tool's callback requests.get was completely unguarded — added validate_trusted_url check and allow_redirects=False

apps/tools/serializers/tool_workflow.py and apps/knowledge/serializers/knowledge_workflow.py

  • Replaced all startswith() checks with validate_trusted_url; added allow_redirects=False to all affected requests.get calls

Before / After

# Before — bypassable via https://apps-assets.fit2cloud.com@attacker.com/
if not download_url.startswith("https://apps-assets.fit2cloud.com/"):
    raise AppApiException(500, _("Illegal download url"))
res = requests.get(download_url, timeout=5)

# After
if not validate_trusted_url(download_url, ALLOWED_DOWNLOAD_HOSTS):
    raise AppApiException(500, _("Illegal download url"))
res = requests.get(download_url, timeout=5, allow_redirects=False)

The pickle deserialization sink in tool.py (RestrictedUnpickler(...).load()) is fed from the download response, making the unpatched call sites particularly high severity.

Copilot AI review requested due to automatic review settings July 27, 2026 10:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot AI review requested due to automatic review settings July 27, 2026 10:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot AI changed the title [WIP] Fix SSRF vulnerability in additional serializers fix(security): close remaining SSRF (CWE-918) call sites via shared URL validator Jul 27, 2026
Copilot AI requested a review from liuruibin July 27, 2026 10:21
@liuruibin
liuruibin marked this pull request as ready for review July 27, 2026 10:21
@shaohuzhang1

Copy link
Copy Markdown
Contributor

Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants