🔄 synced local 'tests/unit/' with remote 'tests/unit/'

This commit is contained in:
Shuchang Zheng 2026-07-07 23:09:20 +00:00
parent 3c4b36a841
commit a3f18fb0d2
10 changed files with 711 additions and 28 deletions

View file

@ -53,6 +53,108 @@ async def test_finalize_downloaded_files_renames_with_download_suffix(tmp_path)
rename_mock.assert_called_once_with(os.path.join(download_dir, "uuid-file.zip"), "req-123.zip")
@pytest.mark.asyncio
async def test_finalize_skips_rename_for_session_file_already_named_by_suffix(tmp_path) -> None:
# A session download is named at download time by download_suffix, so the watcher syncs it as
# ``s3://.../req-123.pdf``. Finalize must NOT re-suffix it (which would bump it to req-123_1.pdf).
agent = ForgeAgent()
task = _make_task()
task.browser_session_id = "pbs-1"
download_dir = tmp_path / "downloads"
download_dir.mkdir()
aws_client = MagicMock()
aws_client.download_file = AsyncMock(return_value=b"data")
rename_mock = MagicMock()
with (
patch("skyvern.forge.agent.get_path_for_workflow_download_directory", return_value=download_dir),
patch("skyvern.forge.agent.get_aws_client", return_value=aws_client),
patch("skyvern.forge.agent.rename_file", rename_mock),
patch("skyvern.forge.agent.skyvern_context.current", return_value=None),
patch("skyvern.forge.agent.app") as mock_app,
):
mock_app.STORAGE.list_downloaded_files_in_browser_session = AsyncMock(
return_value=["s3://bucket/o/pbs-1/req-123.pdf"]
)
await agent._finalize_downloaded_files_for_task(
task,
organization_id=task.organization_id,
download_suffix="req-123",
list_files_before=[],
randomize_if_missing=False,
)
rename_mock.assert_not_called()
assert (download_dir / "req-123.pdf").exists()
assert not (download_dir / "req-123_1.pdf").exists()
@pytest.mark.asyncio
async def test_finalize_dedupes_two_session_files_sharing_one_suffix(tmp_path) -> None:
agent = ForgeAgent()
task = _make_task()
task.browser_session_id = "pbs-1"
download_dir = tmp_path / "downloads"
download_dir.mkdir()
aws_client = MagicMock()
aws_client.download_file = AsyncMock(side_effect=[b"a", b"b"])
with (
patch("skyvern.forge.agent.get_path_for_workflow_download_directory", return_value=download_dir),
patch("skyvern.forge.agent.get_aws_client", return_value=aws_client),
patch("skyvern.forge.agent.skyvern_context.current", return_value=None),
patch("skyvern.forge.agent.app") as mock_app,
):
mock_app.STORAGE.list_downloaded_files_in_browser_session = AsyncMock(
return_value=[
"s3://bucket/o/pbs-1/aaaa.pdf",
"s3://bucket/o/pbs-1/bbbb.pdf",
]
)
await agent._finalize_downloaded_files_for_task(
task,
organization_id=task.organization_id,
download_suffix="req-123",
list_files_before=[],
randomize_if_missing=False,
)
assert {path.name for path in download_dir.iterdir()} == {"req-123.pdf", "req-123_1.pdf"}
assert not (download_dir / "aaaa.pdf").exists()
assert not (download_dir / "bbbb.pdf").exists()
@pytest.mark.asyncio
async def test_finalize_skips_rename_for_local_file_already_named_by_suffix(tmp_path) -> None:
# A run-directory download named at download time by download_suffix arrives as an absolute path
# from list_files_in_directory; finalize must not re-suffix it to req-123_1.pdf.
agent = ForgeAgent()
task = _make_task()
download_dir = tmp_path / "downloads"
download_dir.mkdir()
(download_dir / "req-123.pdf").write_bytes(b"x")
rename_mock = MagicMock()
with (
patch("skyvern.forge.agent.get_path_for_workflow_download_directory", return_value=download_dir),
patch("skyvern.forge.agent.rename_file", rename_mock),
patch("skyvern.forge.agent.skyvern_context.current", return_value=None),
):
await agent._finalize_downloaded_files_for_task(
task,
organization_id=task.organization_id,
download_suffix="req-123",
list_files_before=[],
randomize_if_missing=False,
)
rename_mock.assert_not_called()
assert (download_dir / "req-123.pdf").exists()
assert not (download_dir / "req-123_1.pdf").exists()
@pytest.mark.asyncio
async def test_cleanup_task_finalizes_downloads_before_saving(tmp_path) -> None:
agent = ForgeAgent()

View file

@ -1,4 +1,5 @@
import os
import socket
import tempfile
from typing import Any
from unittest.mock import AsyncMock, MagicMock, patch
@ -6,7 +7,89 @@ from unittest.mock import AsyncMock, MagicMock, patch
import aiohttp
import pytest
from skyvern.forge.sdk.core.aiohttp_helper import aiohttp_request
from skyvern.exceptions import BlockedHost, HttpException
from skyvern.forge.sdk.core.aiohttp_helper import SSRFGuardedResolver, aiohttp_request
from skyvern.utils.url_validators import MAX_SAFE_REDIRECTS, validate_fetch_url
@pytest.fixture(autouse=True)
def public_dns(monkeypatch: pytest.MonkeyPatch) -> None:
def resolves_public(host: str, port: int | None, *args: object, **kwargs: object) -> list[object]:
return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("93.184.216.34", port or 0))]
monkeypatch.setattr("skyvern.utils.url_validators.socket.getaddrinfo", resolves_public)
@pytest.mark.asyncio
async def test_ssrf_guarded_resolver_blocks_dns_rebind_after_validation(monkeypatch: pytest.MonkeyPatch) -> None:
dns_answers = iter(
[
[(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("93.184.216.34", 0))],
[(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("169.254.169.254", 0))],
]
)
def rebinding_dns(host: str, port: int | None, *args: object, **kwargs: object) -> list[object]:
return next(dns_answers)
monkeypatch.setattr("skyvern.utils.url_validators.socket.getaddrinfo", rebinding_dns)
assert validate_fetch_url("https://rebind.example.test/file.pdf") is not None
with pytest.raises(BlockedHost):
await SSRFGuardedResolver().resolve("rebind.example.test", 443)
@pytest.mark.asyncio
async def test_ssrf_guarded_resolver_uses_pinned_ips_without_resolving(monkeypatch: pytest.MonkeyPatch) -> None:
def unexpected_dns(host: str, port: int | None, *args: object, **kwargs: object) -> list[object]:
raise AssertionError("pinned host should not be re-resolved")
resolver = SSRFGuardedResolver()
resolver.pin_url_ips("https://rebind.example.test/file.pdf", ("93.184.216.34",))
monkeypatch.setattr("skyvern.utils.url_validators.socket.getaddrinfo", unexpected_dns)
results = await resolver.resolve("rebind.example.test", 443)
assert [result["host"] for result in results] == ["93.184.216.34"]
@pytest.mark.asyncio
async def test_aiohttp_request_trusts_internal_proxy_without_trusting_target() -> None:
captured_resolver: SSRFGuardedResolver | None = None
def capture_connector(resolver: SSRFGuardedResolver | None = None) -> object:
nonlocal captured_resolver
captured_resolver = resolver
return object()
mock_response = AsyncMock()
mock_response.status = 200
mock_response.headers = {"Content-Type": "application/json"}
mock_response.json = AsyncMock(return_value={"success": True})
mock_response.text = AsyncMock(return_value='{"success": true}')
mock_response.__aenter__ = AsyncMock(return_value=mock_response)
mock_response.__aexit__ = AsyncMock(return_value=None)
mock_session = MagicMock()
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
mock_session.__aexit__ = AsyncMock(return_value=None)
mock_session.request = MagicMock(return_value=mock_response)
with (
patch("skyvern.forge.sdk.core.aiohttp_helper.ssrf_guarded_tcp_connector", side_effect=capture_connector),
patch("skyvern.forge.sdk.core.aiohttp_helper.aiohttp.ClientSession", return_value=mock_session),
):
await aiohttp_request(
method="GET",
url="https://example.com/api",
proxy="http://127.0.0.1:8080",
)
assert captured_resolver is not None
proxy_results = await captured_resolver.resolve("127.0.0.1", 8080)
assert proxy_results
with pytest.raises(BlockedHost):
await SSRFGuardedResolver().resolve("127.0.0.1", 8080)
@pytest.mark.asyncio
@ -543,7 +626,7 @@ async def test_aiohttp_request_response_headers_dict() -> None:
@pytest.mark.asyncio
async def test_aiohttp_request_follow_redirects() -> None:
"""Test that follow_redirects parameter is passed correctly"""
"""Test that redirects are handled manually for SSRF validation."""
captured_args: list[Any] = []
captured_request_kwargs: dict[str, Any] = {}
@ -573,10 +656,202 @@ async def test_aiohttp_request_follow_redirects() -> None:
follow_redirects=False,
)
# Verify allow_redirects parameter was set correctly
# aiohttp automatic redirects stay disabled so each Location target is SSRF-checked first.
assert captured_request_kwargs["allow_redirects"] is False
@pytest.mark.asyncio
async def test_aiohttp_request_follows_safe_redirect() -> None:
redirect_response = AsyncMock()
redirect_response.status = 302
redirect_response.headers = {"Location": "https://example.com/final"}
redirect_response.__aenter__ = AsyncMock(return_value=redirect_response)
redirect_response.__aexit__ = AsyncMock(return_value=None)
final_response = AsyncMock()
final_response.status = 200
final_response.headers = {"Content-Type": "application/json"}
final_response.json = AsyncMock(return_value={"success": True})
final_response.text = AsyncMock(return_value='{"success": true}')
final_response.__aenter__ = AsyncMock(return_value=final_response)
final_response.__aexit__ = AsyncMock(return_value=None)
responses = [redirect_response, final_response]
requested_urls: list[str] = []
def capture_request(*args: Any, **kwargs: Any) -> AsyncMock:
requested_urls.append(kwargs["url"])
return responses.pop(0)
mock_session = MagicMock()
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
mock_session.__aexit__ = AsyncMock(return_value=None)
mock_session.request = MagicMock(side_effect=capture_request)
with patch("skyvern.forge.sdk.core.aiohttp_helper.aiohttp.ClientSession", return_value=mock_session):
status, _headers, body = await aiohttp_request(
method="GET",
url="https://example.com/start",
follow_redirects=True,
)
assert status == 200
assert body == {"success": True}
assert requested_urls == ["https://example.com/start", "https://example.com/final"]
@pytest.mark.asyncio
async def test_aiohttp_request_strips_credentials_on_cross_origin_redirect() -> None:
redirect_response = AsyncMock()
redirect_response.status = 302
redirect_response.headers = {"Location": "https://other.example.com/final"}
redirect_response.__aenter__ = AsyncMock(return_value=redirect_response)
redirect_response.__aexit__ = AsyncMock(return_value=None)
final_response = AsyncMock()
final_response.status = 200
final_response.headers = {"Content-Type": "application/json"}
final_response.json = AsyncMock(return_value={"success": True})
final_response.text = AsyncMock(return_value='{"success": true}')
final_response.__aenter__ = AsyncMock(return_value=final_response)
final_response.__aexit__ = AsyncMock(return_value=None)
responses = [redirect_response, final_response]
requested_headers: list[dict[str, str]] = []
requested_cookies: list[dict[str, str] | None] = []
def capture_request(*args: Any, **kwargs: Any) -> AsyncMock:
requested_headers.append(kwargs["headers"])
requested_cookies.append(kwargs["cookies"])
return responses.pop(0)
mock_session = MagicMock()
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
mock_session.__aexit__ = AsyncMock(return_value=None)
mock_session.request = MagicMock(side_effect=capture_request)
with patch("skyvern.forge.sdk.core.aiohttp_helper.aiohttp.ClientSession", return_value=mock_session):
await aiohttp_request(
method="GET",
url="https://example.com/start",
headers={"Authorization": "Bearer secret", "Cookie": "sid=abc", "X-Keep": "1"},
cookies={"session": "abc"},
follow_redirects=True,
)
assert requested_headers[0]["Authorization"] == "Bearer secret"
assert requested_headers[0]["Cookie"] == "sid=abc"
assert requested_cookies[0] == {"session": "abc"}
assert requested_headers[1] == {"X-Keep": "1"}
assert requested_cookies[1] is None
@pytest.mark.asyncio
async def test_aiohttp_request_rebuilds_multipart_for_307_redirect() -> None:
with tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".txt") as tmp_file:
tmp_file.write("test file content")
tmp_file_path = tmp_file.name
try:
redirect_response = AsyncMock()
redirect_response.status = 307
redirect_response.headers = {"Location": "https://example.com/final"}
redirect_response.__aenter__ = AsyncMock(return_value=redirect_response)
redirect_response.__aexit__ = AsyncMock(return_value=None)
final_response = AsyncMock()
final_response.status = 200
final_response.headers = {"Content-Type": "application/json"}
final_response.json = AsyncMock(return_value={"success": True})
final_response.text = AsyncMock(return_value='{"success": true}')
final_response.__aenter__ = AsyncMock(return_value=final_response)
final_response.__aexit__ = AsyncMock(return_value=None)
responses = [redirect_response, final_response]
multipart_bodies: list[aiohttp.FormData] = []
def capture_request(*args: Any, **kwargs: Any) -> AsyncMock:
multipart_bodies.append(kwargs["data"])
return responses.pop(0)
mock_session = MagicMock()
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
mock_session.__aexit__ = AsyncMock(return_value=None)
mock_session.request = MagicMock(side_effect=capture_request)
with patch("skyvern.forge.sdk.core.aiohttp_helper.aiohttp.ClientSession", return_value=mock_session):
status, _headers, body = await aiohttp_request(
method="POST",
url="https://example.com/start",
files={"file": tmp_file_path},
follow_redirects=True,
)
assert status == 200
assert body == {"success": True}
assert len(multipart_bodies) == 2
assert multipart_bodies[0] is not multipart_bodies[1]
finally:
if os.path.exists(tmp_file_path):
os.unlink(tmp_file_path)
@pytest.mark.asyncio
async def test_aiohttp_request_rejects_unsafe_redirect_target() -> None:
redirect_response = AsyncMock()
redirect_response.status = 302
redirect_response.headers = {"Location": "http://169.254.169.254/latest/meta-data"}
redirect_response.__aenter__ = AsyncMock(return_value=redirect_response)
redirect_response.__aexit__ = AsyncMock(return_value=None)
requested_urls: list[str] = []
def capture_request(*args: Any, **kwargs: Any) -> AsyncMock:
requested_urls.append(kwargs["url"])
return redirect_response
mock_session = MagicMock()
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
mock_session.__aexit__ = AsyncMock(return_value=None)
mock_session.request = MagicMock(side_effect=capture_request)
with patch("skyvern.forge.sdk.core.aiohttp_helper.aiohttp.ClientSession", return_value=mock_session):
with pytest.raises(BlockedHost):
await aiohttp_request(
method="GET",
url="https://example.com/start",
follow_redirects=True,
)
assert requested_urls == ["https://example.com/start"]
@pytest.mark.asyncio
async def test_aiohttp_request_redirect_limit_raises_http_exception() -> None:
redirect_response = AsyncMock()
redirect_response.status = 302
redirect_response.headers = {"Location": "https://example.com/next"}
redirect_response.__aenter__ = AsyncMock(return_value=redirect_response)
redirect_response.__aexit__ = AsyncMock(return_value=None)
mock_session = MagicMock()
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
mock_session.__aexit__ = AsyncMock(return_value=None)
mock_session.request = MagicMock(return_value=redirect_response)
with patch("skyvern.forge.sdk.core.aiohttp_helper.aiohttp.ClientSession", return_value=mock_session):
with pytest.raises(HttpException) as exc_info:
await aiohttp_request(
method="GET",
url="https://example.com/start",
follow_redirects=True,
)
assert exc_info.value.status_code == 400
assert "Too many redirects" in (exc_info.value.error_message or "")
assert mock_session.request.call_count == MAX_SAFE_REDIRECTS + 1
@pytest.mark.asyncio
async def test_aiohttp_request_proxy_passed_correctly() -> None:
"""Test that proxy parameter is passed correctly"""
@ -646,10 +921,12 @@ async def test_aiohttp_request_with_files_uses_multipart() -> None:
mock_session.__aexit__ = AsyncMock(return_value=None)
mock_session.request = MagicMock(side_effect=capture_request)
headers = {"Content-Type": "multipart/form-data", "X-Test": "value"}
with patch("skyvern.forge.sdk.core.aiohttp_helper.aiohttp.ClientSession", return_value=mock_session):
await aiohttp_request(
method="POST",
url="https://example.com/api/upload",
headers=headers,
files={"file": tmp_file_path},
)
@ -660,6 +937,7 @@ async def test_aiohttp_request_with_files_uses_multipart() -> None:
# Verify Content-Type header was removed (aiohttp will set it for multipart)
assert "Content-Type" not in captured_request_kwargs["headers"]
assert "content-type" not in captured_request_kwargs["headers"]
assert headers == {"Content-Type": "multipart/form-data", "X-Test": "value"}
finally:
# Clean up temporary file
if os.path.exists(tmp_file_path):

View file

@ -151,7 +151,8 @@ async def test_rename_happens_before_s3_upload(setup, tmp_path):
refs = setup(
get_side_effect=[[], ["invoice.pdf"]],
save_side_effect=track_save,
list_files_side_effect=[[], [fake_file], [fake_file]], # before, local verify, rename
# before, local verify, rename local_files_after, existing_names dedup lookup
list_files_side_effect=[[], [fake_file], [fake_file], [fake_file]],
rename_mock=rename_mock,
)
try:
@ -174,7 +175,8 @@ async def test_download_suffix_rename_uses_file_path_directly(setup, tmp_path):
rename_mock = MagicMock(return_value=str(download_dir / "invoice.pdf"))
refs = setup(
get_side_effect=[[], ["invoice.pdf"]],
list_files_side_effect=[[], [abs_path], [abs_path]], # before, local verify, rename
# before, local verify, rename local_files_after, existing_names dedup lookup
list_files_side_effect=[[], [abs_path], [abs_path], [abs_path]],
rename_mock=rename_mock,
)
try:
@ -373,7 +375,8 @@ async def test_rename_skips_crdownload_files(setup, tmp_path):
rename_mock = MagicMock(return_value=str(download_dir / "renamed.pdf"))
refs = setup(
get_side_effect=[[], ["invoice.pdf"]],
list_files_side_effect=[[], [incomplete, complete], [incomplete, complete]], # before, local verify, rename
# before, local verify, rename local_files_after, existing_names dedup lookup
list_files_side_effect=[[], [incomplete, complete], [incomplete, complete], [incomplete, complete]],
rename_mock=rename_mock,
)
try:
@ -398,7 +401,8 @@ async def test_rename_handles_name_collision(setup, tmp_path):
rename_mock = MagicMock(return_value=str(download_dir / "invoice_1.pdf"))
refs = setup(
get_side_effect=[[], ["invoice_1.pdf"]],
list_files_side_effect=[[], [new_file], [new_file]], # before, local verify, rename
# before, local verify, rename local_files_after, existing_names dedup lookup (already has invoice.pdf)
list_files_side_effect=[[], [new_file], [new_file], [new_file, str(download_dir / "invoice.pdf")]],
rename_mock=rename_mock,
)
try:

View file

@ -864,7 +864,7 @@ async def test_handle_action_prefers_observed_file_over_download_event_copy(
patch("skyvern.webeye.actions.handler.get_download_dir", return_value=primary_dir),
patch(
"skyvern.webeye.actions.handler.skyvern_context.current",
return_value=MagicMock(run_id="pbs-1"),
return_value=MagicMock(run_id="pbs-1", download_suffix=None),
),
patch(
"skyvern.webeye.actions.handler.check_downloading_files_and_wait_for_download_to_complete",
@ -964,7 +964,7 @@ async def test_handle_action_copies_download_event_when_no_observed_file_appears
patch("skyvern.webeye.actions.handler.get_download_dir", return_value=primary_dir),
patch(
"skyvern.webeye.actions.handler.skyvern_context.current",
return_value=MagicMock(run_id="pbs-1"),
return_value=MagicMock(run_id="pbs-1", download_suffix=None),
),
patch(
"skyvern.webeye.actions.handler.check_downloading_files_and_wait_for_download_to_complete",
@ -1067,7 +1067,7 @@ async def test_handle_action_ignores_empty_download_event_fallback_file(
patch("skyvern.webeye.actions.handler.get_download_dir", return_value=primary_dir),
patch(
"skyvern.webeye.actions.handler.skyvern_context.current",
return_value=MagicMock(run_id="pbs-1"),
return_value=MagicMock(run_id="pbs-1", download_suffix=None),
),
patch(
"skyvern.webeye.actions.handler.check_downloading_files_and_wait_for_download_to_complete",
@ -1166,7 +1166,7 @@ async def test_handle_action_stops_after_download_event_fallback_failure(
patch("skyvern.webeye.actions.handler.get_download_dir", return_value=primary_dir),
patch(
"skyvern.webeye.actions.handler.skyvern_context.current",
return_value=MagicMock(run_id="pbs-1"),
return_value=MagicMock(run_id="pbs-1", download_suffix=None),
),
patch(
"skyvern.webeye.actions.handler.check_downloading_files_and_wait_for_download_to_complete",
@ -1266,7 +1266,7 @@ async def test_handle_action_removes_late_zero_byte_duplicate_after_download_wai
patch("skyvern.webeye.actions.handler.get_download_dir", return_value=primary_dir),
patch(
"skyvern.webeye.actions.handler.skyvern_context.current",
return_value=MagicMock(run_id="pbs-1"),
return_value=MagicMock(run_id="pbs-1", download_suffix=None),
),
patch(
"skyvern.webeye.actions.handler.check_downloading_files_and_wait_for_download_to_complete",
@ -1342,7 +1342,7 @@ async def test_handle_action_removes_download_listener_when_inner_action_raises(
patch("skyvern.webeye.actions.handler.get_download_dir", return_value=primary_dir),
patch(
"skyvern.webeye.actions.handler.skyvern_context.current",
return_value=MagicMock(run_id="pbs-1"),
return_value=MagicMock(run_id="pbs-1", download_suffix=None),
),
patch("skyvern.webeye.actions.handler.app", mock_app),
):
@ -1398,7 +1398,10 @@ async def test_handle_action_discards_xhr_staging_when_native_file_present(
patch.object(ActionHandler, "_handle_action", side_effect=mock_inner),
patch("skyvern.webeye.actions.handler.get_download_dir", return_value=primary_dir),
patch("skyvern.webeye.actions.handler.make_temp_directory", return_value=staging),
patch("skyvern.webeye.actions.handler.skyvern_context.current", return_value=MagicMock(run_id="pbs-1")),
patch(
"skyvern.webeye.actions.handler.skyvern_context.current",
return_value=MagicMock(run_id="pbs-1", download_suffix=None),
),
patch(
"skyvern.webeye.actions.handler.check_downloading_files_and_wait_for_download_to_complete",
new=AsyncMock(),
@ -1451,7 +1454,10 @@ async def test_handle_action_uses_xhr_staging_fallback_when_no_native_file(
patch.object(ActionHandler, "_handle_action", side_effect=mock_inner),
patch("skyvern.webeye.actions.handler.get_download_dir", return_value=primary_dir),
patch("skyvern.webeye.actions.handler.make_temp_directory", return_value=staging),
patch("skyvern.webeye.actions.handler.skyvern_context.current", return_value=MagicMock(run_id="pbs-1")),
patch(
"skyvern.webeye.actions.handler.skyvern_context.current",
return_value=MagicMock(run_id="pbs-1", download_suffix=None),
),
patch(
"skyvern.webeye.actions.handler.check_downloading_files_and_wait_for_download_to_complete",
new=AsyncMock(),
@ -1507,7 +1513,10 @@ async def test_handle_action_moves_multiple_staged_xhr_files_as_fallback(
patch.object(ActionHandler, "_handle_action", side_effect=mock_inner),
patch("skyvern.webeye.actions.handler.get_download_dir", return_value=primary_dir),
patch("skyvern.webeye.actions.handler.make_temp_directory", return_value=staging),
patch("skyvern.webeye.actions.handler.skyvern_context.current", return_value=MagicMock(run_id="pbs-1")),
patch(
"skyvern.webeye.actions.handler.skyvern_context.current",
return_value=MagicMock(run_id="pbs-1", download_suffix=None),
),
patch(
"skyvern.webeye.actions.handler.check_downloading_files_and_wait_for_download_to_complete",
new=AsyncMock(),
@ -1559,7 +1568,10 @@ async def test_handle_action_cleans_staging_on_exception(
patch.object(ActionHandler, "_handle_action", side_effect=mock_inner),
patch("skyvern.webeye.actions.handler.get_download_dir", return_value=primary_dir),
patch("skyvern.webeye.actions.handler.make_temp_directory", return_value=staging),
patch("skyvern.webeye.actions.handler.skyvern_context.current", return_value=MagicMock(run_id="pbs-1")),
patch(
"skyvern.webeye.actions.handler.skyvern_context.current",
return_value=MagicMock(run_id="pbs-1", download_suffix=None),
),
patch(
"skyvern.webeye.actions.handler.check_downloading_files_and_wait_for_download_to_complete",
new=AsyncMock(),
@ -1624,7 +1636,10 @@ async def test_handle_action_logs_warning_when_late_native_appears_after_xhr_fal
patch("skyvern.webeye.actions.handler.get_download_dir", return_value=primary_dir),
patch("skyvern.webeye.actions.handler.make_temp_directory", return_value=staging),
patch("skyvern.webeye.actions.handler.BROWSER_DOWNLOAD_NO_SIGNAL_GRACE_TIME", 0),
patch("skyvern.webeye.actions.handler.skyvern_context.current", return_value=MagicMock(run_id="pbs-1")),
patch(
"skyvern.webeye.actions.handler.skyvern_context.current",
return_value=MagicMock(run_id="pbs-1", download_suffix=None),
),
patch(
"skyvern.webeye.actions.handler.check_downloading_files_and_wait_for_download_to_complete",
new=AsyncMock(side_effect=mock_settle),
@ -1729,7 +1744,10 @@ async def test_handle_action_adopted_session_lands_download_via_eager_save(
with (
patch.object(ActionHandler, "_handle_action", side_effect=mock_inner_handle_action),
patch("skyvern.webeye.actions.handler.get_download_dir", return_value=primary_dir),
patch("skyvern.webeye.actions.handler.skyvern_context.current", return_value=MagicMock(run_id="bs-1")),
patch(
"skyvern.webeye.actions.handler.skyvern_context.current",
return_value=MagicMock(run_id="bs-1", download_suffix=None),
),
patch(
"skyvern.webeye.actions.handler.check_downloading_files_and_wait_for_download_to_complete",
new=wait_for_downloads,
@ -1831,7 +1849,10 @@ async def test_handle_action_adopted_session_refetches_when_save_as_target_close
with (
patch.object(ActionHandler, "_handle_action", side_effect=mock_inner_handle_action),
patch("skyvern.webeye.actions.handler.get_download_dir", return_value=primary_dir),
patch("skyvern.webeye.actions.handler.skyvern_context.current", return_value=MagicMock(run_id="bs-1")),
patch(
"skyvern.webeye.actions.handler.skyvern_context.current",
return_value=MagicMock(run_id="bs-1", download_suffix=None),
),
patch(
"skyvern.webeye.actions.handler.check_downloading_files_and_wait_for_download_to_complete",
new=wait_for_downloads,
@ -1947,7 +1968,10 @@ async def test_handle_action_adopted_session_falls_through_to_session_folder_whe
with (
patch.object(ActionHandler, "_handle_action", side_effect=mock_inner_handle_action),
patch("skyvern.webeye.actions.handler.get_download_dir", return_value=primary_dir),
patch("skyvern.webeye.actions.handler.skyvern_context.current", return_value=MagicMock(run_id="bs-1")),
patch(
"skyvern.webeye.actions.handler.skyvern_context.current",
return_value=MagicMock(run_id="bs-1", download_suffix=None),
),
patch(
"skyvern.webeye.actions.handler.check_downloading_files_and_wait_for_download_to_complete",
new=wait_for_downloads,
@ -2046,7 +2070,10 @@ async def test_handle_action_adopted_session_helper_failure_does_not_short_circu
with (
patch.object(ActionHandler, "_handle_action", side_effect=mock_inner_handle_action),
patch("skyvern.webeye.actions.handler.get_download_dir", return_value=primary_dir),
patch("skyvern.webeye.actions.handler.skyvern_context.current", return_value=MagicMock(run_id="bs-1")),
patch(
"skyvern.webeye.actions.handler.skyvern_context.current",
return_value=MagicMock(run_id="bs-1", download_suffix=None),
),
patch(
"skyvern.webeye.actions.handler.check_downloading_files_and_wait_for_download_to_complete",
new=wait_for_downloads,
@ -2155,7 +2182,10 @@ async def test_handle_action_adopted_session_xhr_staging_recovered_when_helper_f
patch("skyvern.webeye.actions.handler.get_download_dir", return_value=primary_dir),
patch("skyvern.webeye.actions.handler.make_temp_directory", return_value=staging_dir),
patch("skyvern.webeye.actions.handler.ScopedXhrDownloadCapture", return_value=mock_xhr),
patch("skyvern.webeye.actions.handler.skyvern_context.current", return_value=MagicMock(run_id="bs-1")),
patch(
"skyvern.webeye.actions.handler.skyvern_context.current",
return_value=MagicMock(run_id="bs-1", download_suffix=None),
),
patch(
"skyvern.webeye.actions.handler.check_downloading_files_and_wait_for_download_to_complete",
new=wait_for_downloads,

View file

@ -1,6 +1,9 @@
from __future__ import annotations
import os
import socket
from collections.abc import AsyncIterator
from pathlib import Path
from types import SimpleNamespace
from unittest.mock import AsyncMock, MagicMock
@ -8,7 +11,9 @@ import pytest
from skyvern.config import settings
from skyvern.constants import DOWNLOAD_FILE_PREFIX
from skyvern.exceptions import BlockedHost, SkyvernHTTPException
from skyvern.forge.sdk.api import files
from skyvern.utils.url_validators import MAX_SAFE_REDIRECTS
ATTACKER_ORG_ID = "o_attacker"
VICTIM_ORG_ID = "o_victim"
@ -242,3 +247,122 @@ async def test_download_file_routes_gcs_uri_to_managed_storage(storage: SimpleNa
assert f.read() == b"tenant-secret-bytes"
finally:
os.unlink(path)
@pytest.mark.asyncio
async def test_download_file_blocks_hostname_resolving_to_private_ip(monkeypatch: pytest.MonkeyPatch) -> None:
def resolves_private(host: str, port: int | None, *args: object, **kwargs: object) -> list[object]:
return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("10.0.0.42", port or 0))]
client_session = MagicMock()
monkeypatch.setattr("skyvern.utils.url_validators.socket.getaddrinfo", resolves_private)
monkeypatch.setattr(files.aiohttp, "ClientSession", client_session)
with pytest.raises(BlockedHost):
await files.download_file("https://evil.example.test/secret.pdf")
client_session.assert_not_called()
@pytest.mark.asyncio
async def test_download_file_rejects_unsafe_redirect_target(monkeypatch: pytest.MonkeyPatch) -> None:
def resolves_public(host: str, port: int | None, *args: object, **kwargs: object) -> list[object]:
return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("93.184.216.34", port or 0))]
redirect_response = AsyncMock()
redirect_response.status = 302
redirect_response.headers = {"Location": "http://169.254.169.254/latest/meta-data"}
redirect_response.__aenter__ = AsyncMock(return_value=redirect_response)
redirect_response.__aexit__ = AsyncMock(return_value=None)
mock_session = MagicMock()
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
mock_session.__aexit__ = AsyncMock(return_value=None)
mock_session.get = MagicMock(return_value=redirect_response)
monkeypatch.setattr("skyvern.utils.url_validators.socket.getaddrinfo", resolves_public)
monkeypatch.setattr(files.aiohttp, "ClientSession", MagicMock(return_value=mock_session))
with pytest.raises(BlockedHost):
await files.download_file("https://example.com/start.pdf")
mock_session.get.assert_called_once()
@pytest.mark.asyncio
async def test_download_file_strips_credentials_on_cross_origin_redirect(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
def resolves_public(host: str, port: int | None, *args: object, **kwargs: object) -> list[object]:
return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("93.184.216.34", port or 0))]
class Content:
async def iter_chunked(self, chunk_size: int) -> AsyncIterator[bytes]:
yield b"file-bytes"
redirect_response = AsyncMock()
redirect_response.status = 302
redirect_response.headers = {"Location": "https://other.example.com/final.pdf"}
redirect_response.__aenter__ = AsyncMock(return_value=redirect_response)
redirect_response.__aexit__ = AsyncMock(return_value=None)
final_response = AsyncMock()
final_response.status = 200
final_response.headers = {}
final_response.content_length = len(b"file-bytes")
final_response.content = Content()
final_response.__aenter__ = AsyncMock(return_value=final_response)
final_response.__aexit__ = AsyncMock(return_value=None)
responses = [redirect_response, final_response]
requested_headers: list[dict[str, str]] = []
def capture_get(*args: object, **kwargs: object) -> AsyncMock:
headers = kwargs["headers"]
assert isinstance(headers, dict)
requested_headers.append(headers)
return responses.pop(0)
mock_session = MagicMock()
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
mock_session.__aexit__ = AsyncMock(return_value=None)
mock_session.get = MagicMock(side_effect=capture_get)
monkeypatch.setattr("skyvern.utils.url_validators.socket.getaddrinfo", resolves_public)
monkeypatch.setattr(files.aiohttp, "ClientSession", MagicMock(return_value=mock_session))
result = await files.download_file(
"https://example.com/start.pdf",
headers={"Authorization": "Bearer secret", "Cookie": "sid=abc", "X-Keep": "1"},
output_dir=str(tmp_path),
)
assert Path(result).read_bytes() == b"file-bytes"
assert requested_headers[0]["Authorization"] == "Bearer secret"
assert requested_headers[0]["Cookie"] == "sid=abc"
assert requested_headers[1] == {"X-Keep": "1"}
@pytest.mark.asyncio
async def test_download_file_redirect_limit_raises_http_exception(monkeypatch: pytest.MonkeyPatch) -> None:
def resolves_public(host: str, port: int | None, *args: object, **kwargs: object) -> list[object]:
return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("93.184.216.34", port or 0))]
redirect_response = AsyncMock()
redirect_response.status = 302
redirect_response.headers = {"Location": "https://example.com/next.pdf"}
redirect_response.__aenter__ = AsyncMock(return_value=redirect_response)
redirect_response.__aexit__ = AsyncMock(return_value=None)
mock_session = MagicMock()
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
mock_session.__aexit__ = AsyncMock(return_value=None)
mock_session.get = MagicMock(return_value=redirect_response)
monkeypatch.setattr("skyvern.utils.url_validators.socket.getaddrinfo", resolves_public)
monkeypatch.setattr(files.aiohttp, "ClientSession", MagicMock(return_value=mock_session))
with pytest.raises(SkyvernHTTPException, match="Too many redirects"):
await files.download_file("https://example.com/start.pdf")
assert mock_session.get.call_count == MAX_SAFE_REDIRECTS + 1

View file

@ -17,6 +17,7 @@ class _FakeDownloadResponse:
self._data = data
# aiohttp exposes headers as a case-insensitive CIMultiDictProxy; mirror that.
self.headers = CIMultiDictProxy(CIMultiDict(headers or {}))
self.status = 200
self.content_length = len(data) if advertise_length else None
self.content = self
self.body_read = False
@ -36,7 +37,9 @@ class _FakeDownloadSession:
def __init__(self, response: _FakeDownloadResponse) -> None:
self._response = response
def get(self, url: object, headers: dict[str, str] | None = None) -> _FakeDownloadResponse:
def get(
self, url: object, headers: dict[str, str] | None = None, allow_redirects: bool = True
) -> _FakeDownloadResponse:
return self._response
async def __aenter__(self) -> _FakeDownloadSession:

View file

@ -145,10 +145,10 @@ def test_debug_fields_handles_none_inputs() -> None:
async def test_resolve_org_from_api_key_logs_decode_error_reason(monkeypatch) -> None:
logged: dict[str, object] = {}
def fake_error(_message: str, **kwargs: object) -> None:
def fake_warning(_message: str, **kwargs: object) -> None:
logged.update(kwargs)
monkeypatch.setattr(org_auth_service.LOG, "error", fake_error)
monkeypatch.setattr(org_auth_service.LOG, "warning", fake_warning)
with pytest.raises(HTTPException) as exc_info:
await org_auth_service.resolve_org_from_api_key("definitely-not-a-jwt", SimpleNamespace(), ())

View file

@ -1,8 +1,16 @@
import socket
import pytest
from skyvern.config import settings
from skyvern.exceptions import BlockedHost
from skyvern.utils.url_validators import encode_url, is_blocked_host, validate_url
from skyvern.utils.url_validators import (
encode_url,
is_blocked_host,
validate_fetch_url,
validate_redirect_url,
validate_url,
)
def test_encode_url_basic():
@ -121,3 +129,60 @@ def test_is_blocked_host_allowed_hosts_normalize_brackets_and_mapped(
@pytest.mark.parametrize("host", ["LOCALHOST", "LocalHost", "localhost"])
def test_is_blocked_host_blocked_hosts_case_insensitive(host: str) -> None:
assert is_blocked_host(host) is True
def test_validate_fetch_url_blocks_hostname_resolving_private_ip(monkeypatch: pytest.MonkeyPatch) -> None:
def resolves_to_private(host: str, port: int | None, *args: object, **kwargs: object) -> list[object]:
return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("10.0.0.42", port or 0))]
monkeypatch.setattr("skyvern.utils.url_validators.socket.getaddrinfo", resolves_to_private)
with pytest.raises(BlockedHost):
validate_fetch_url("https://evil.example.test/file.pdf")
def test_validate_fetch_url_fails_closed_on_dns_error(monkeypatch: pytest.MonkeyPatch) -> None:
def fails_dns(host: str, port: int | None, *args: object, **kwargs: object) -> list[object]:
raise OSError("dns unavailable")
monkeypatch.setattr("skyvern.utils.url_validators.socket.getaddrinfo", fails_dns)
with pytest.raises(BlockedHost):
validate_fetch_url("https://unresolvable.example.test/file.pdf")
def test_validate_url_does_not_resolve_dns(monkeypatch: pytest.MonkeyPatch) -> None:
def unexpected_dns(host: str, port: int | None, *args: object, **kwargs: object) -> list[object]:
raise AssertionError("general URL validation should not resolve DNS")
monkeypatch.setattr("skyvern.utils.url_validators.socket.getaddrinfo", unexpected_dns)
assert validate_url("https://webhook.example.com/receive") is not None
@pytest.mark.parametrize(
"url",
[
"https://metadata.google.internal/computeMetadata/v1/",
"https://kubernetes.default.svc/api",
"https://my-service.namespace.svc.cluster.local/api",
"https://internal.local/api",
],
)
def test_validate_url_blocks_internal_hostnames(url: str, monkeypatch: pytest.MonkeyPatch) -> None:
def unexpected_dns(host: str, port: int | None, *args: object, **kwargs: object) -> list[object]:
raise AssertionError("internal hostname should be blocked before DNS")
monkeypatch.setattr("skyvern.utils.url_validators.socket.getaddrinfo", unexpected_dns)
with pytest.raises(BlockedHost):
validate_url(url)
def test_is_blocked_host_allows_public_svc_subdomain() -> None:
assert is_blocked_host("api.svc.example.com") is False
def test_validate_redirect_url_rejects_private_redirect_target() -> None:
with pytest.raises(BlockedHost):
validate_redirect_url("https://example.com/file.pdf", "http://169.254.169.254/latest/meta-data")

View file

@ -0,0 +1,51 @@
from __future__ import annotations
from pathlib import Path
from skyvern.forge.sdk.core import skyvern_context
from skyvern.forge.sdk.core.skyvern_context import SkyvernContext
from skyvern.webeye.actions.handler import _download_target_path
from skyvern.webeye.cdp_download_interceptor import download_filename_from_suffix
_SITE_UUID = "0faafbe5-fc0e-4cb6-9947-332fe1405073.pdf"
def test_suffix_without_extension_appends_source_extension() -> None:
assert download_filename_from_suffix("REQ-1", ".pdf", set()) == "REQ-1.pdf"
def test_suffix_with_extension_is_used_verbatim() -> None:
# A suffix that already carries an extension must not be double-suffixed.
assert download_filename_from_suffix("report.pdf", ".pdf", set()) == "report.pdf"
def test_suffix_collisions_are_deduped() -> None:
assert download_filename_from_suffix("REQ-1", ".pdf", {"REQ-1.pdf"}) == "REQ-1_1.pdf"
assert download_filename_from_suffix("report.pdf", ".pdf", {"report.pdf"}) == "report_1.pdf"
def test_suffix_dedup_normalizes_full_path_existing_names() -> None:
# A caller that passes absolute paths must not defeat dedup (would silently overwrite the first file).
assert download_filename_from_suffix("REQ-1", ".pdf", {"/downloads/REQ-1.pdf"}) == "REQ-1_1.pdf"
def test_download_target_path_prefers_download_suffix(tmp_path: Path) -> None:
with skyvern_context.scoped(SkyvernContext(download_suffix="REQ-1")):
target = _download_target_path(tmp_path, _SITE_UUID)
# The site's UUID name is replaced by the request-based name.
assert target.name == "REQ-1.pdf"
assert target.parent == tmp_path
def test_download_target_path_without_suffix_keeps_site_stem(tmp_path: Path) -> None:
with skyvern_context.scoped(SkyvernContext(download_suffix=None)):
target = _download_target_path(tmp_path, "invoice.pdf")
assert target.name.endswith("-invoice.pdf")
assert target.name != "invoice.pdf"
def test_download_target_path_dedupes_same_suffix_in_dir(tmp_path: Path) -> None:
(tmp_path / "REQ-1.pdf").write_bytes(b"x")
with skyvern_context.scoped(SkyvernContext(download_suffix="REQ-1")):
target = _download_target_path(tmp_path, "site-uuid.pdf")
assert target.name == "REQ-1_1.pdf"

View file

@ -1,6 +1,7 @@
import json
import socket
from datetime import datetime, timezone
from unittest.mock import AsyncMock
from unittest.mock import AsyncMock, MagicMock
import pytest
@ -193,6 +194,31 @@ class TestSecretResponsePaths:
class TestHttpRequestBlockSecretResponsePaths:
@pytest.mark.asyncio
async def test_execute_rejects_hostname_resolving_to_private_ip_before_request(
self, monkeypatch: pytest.MonkeyPatch
) -> None:
context = _make_context()
block = _http_block(url="https://evil.example.test/api")
db_mock = AsyncMock()
client_session = MagicMock()
def resolves_private(host: str, port: int | None, *args: object, **kwargs: object) -> list[object]:
return [(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("10.0.0.42", port or 0))]
monkeypatch.setattr(HttpRequestBlock, "get_workflow_run_context", lambda _self, _workflow_run_id: context)
monkeypatch.setattr("skyvern.utils.url_validators.socket.getaddrinfo", resolves_private)
monkeypatch.setattr("skyvern.forge.sdk.core.aiohttp_helper.aiohttp.ClientSession", client_session)
monkeypatch.setattr(block_module.app, "DATABASE", db_mock)
result = await block.execute(workflow_run_id="wr-1", workflow_run_block_id="wrb-1")
assert result.success is False
assert result.status == BlockStatus.failed
assert result.failure_reason is not None
assert "blocked" in result.failure_reason
client_session.assert_not_called()
@pytest.mark.asyncio
async def test_execute_records_placeholder_and_masks_duplicate_echo(self, monkeypatch: pytest.MonkeyPatch) -> None:
context = _make_context()