-
Notifications
You must be signed in to change notification settings - Fork 8.1k
fix(tools): block private URLs in SeleniumScrapingTool #6870
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,8 @@ | |
| from crewai.tools import BaseTool | ||
| from pydantic import BaseModel, Field, field_validator | ||
|
|
||
| from crewai_tools.security.safe_path import validate_url | ||
|
|
||
|
|
||
| class FixedSeleniumScrapingToolSchema(BaseModel): | ||
| """Input for SeleniumScrapingTool.""" | ||
|
|
@@ -45,7 +47,9 @@ def validate_website_url(cls, v: str) -> str: | |
| if re.search(r"\s", v): | ||
| raise ValueError("URL cannot contain whitespace") | ||
|
|
||
| return v | ||
| # Align with Firecrawl/Scrapfly: block private/reserved SSRF targets | ||
| # before the local Chrome WebDriver navigates to the URL. | ||
| return validate_url(v) | ||
|
|
||
|
|
||
| class SeleniumScrapingTool(BaseTool): | ||
|
|
@@ -121,9 +125,9 @@ def __init__( | |
| self.css_element = css_element | ||
|
|
||
| if website_url is not None: | ||
| self.website_url = website_url | ||
| self.website_url = validate_url(website_url) | ||
| self.description = ( | ||
| f"A tool that can be used to read {website_url}'s content." | ||
| f"A tool that can be used to read {self.website_url}'s content." | ||
| ) | ||
| self.args_schema = FixedSeleniumScrapingToolSchema | ||
|
|
||
|
|
@@ -195,6 +199,9 @@ def _make_request( | |
| if not re.match(r"^https?://", url): | ||
| raise ValueError("URL must start with http:// or https://") | ||
|
|
||
| # Defense in depth for the fixed-URL schema path (no pydantic re-check). | ||
| url = validate_url(url) | ||
|
|
||
|
Comment on lines
+202
to
+204
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg 'selenium_scraping_tool\.py$|selenium' || true
echo "== target outline =="
ast-grep outline lib/crewai-tools/src/crewai_tools/tools/selenium_scraping_tool/selenium_scraping_tool.py --view expanded || true
echo "== relevant source =="
sed -n '1,260p' lib/crewai-tools/src/crewai_tools/tools/selenium_scraping_tool/selenium_scraping_tool.py
echo "== validate_url definitions/usages =="
rg -n "def validate_url|validate_url|SeleniumBase|Crawler|driver\.get|driver\.add_" lib/crewai-tools/src/crewai_tools/tools/selenium_scraping_tool -SRepository: crewAIInc/crewAI Length of output: 17379 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== safe_path outline =="
ast-grep outline lib/crewai-tools/src/crewai_tools/security/safe_path.py --view expanded || true
echo "== safe_path source =="
sed -n '1,220p' lib//crewai_tools/src/crewai_tools/security/safe_path.py || true
echo "== validate_url search =="
rg -rn "def validate_url|resolve_hostname|ipaddress|ALLOWED|BLOCK|private|reserved|safe_path" lib/crewai-tools/src/crewai_tools -SRepository: crewAIInc/crewAI Length of output: 9401 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== safe_path exact source =="
sed -n '140,210p' lib/crewai-tools/src/crewai_tools/security/safe_path.py
echo "== selenium source around driver navigation =="
sed -n '193,215p' lib/crewai-tools/src/crewai_tools/tools/selenium_scraping_tool/selenium_scraping_tool.pyRepository: crewAIInc/crewAI Length of output: 3379 Enforce the SSRF policy at Chrome egress.
🤖 Prompt for AI Agents |
||
| if self.driver is None: | ||
| raise RuntimeError("Driver not initialized. Call _run first.") | ||
| sleep_time = wait_time or 0 | ||
|
|
||
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.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: crewAIInc/crewAI
Length of output: 11238
🏁 Script executed:
Repository: crewAIInc/crewAI
Length of output: 50372
Validate fixed URLs before creating the Selenium WebDriver.
SeleniumScrapingTool.__init__createswebdriver.Chromebeforevalidate_url(website_url)runs for a fixed URL. Move the validation before driver initialization, or close the driver when validation fails. The fixed-URL constructor regression test should also assertmocked_chrome.assert_not_called()after confirming theValueError.📍 Affects 2 files
lib/crewai-tools/src/crewai_tools/tools/selenium_scraping_tool/selenium_scraping_tool.py#L128-L130(this comment)lib/crewai-tools/tests/tools/selenium_scraping_tool_test.py#L156-L165🤖 Prompt for AI Agents