mirror of
https://github.com/Skyvern-AI/skyvern.git
synced 2026-07-09 16:09:13 +00:00
fix(SKY-11917): validate script deploy file paths (#7145)
This commit is contained in:
parent
fe5f183952
commit
75df48f09e
11 changed files with 212 additions and 13 deletions
|
|
@ -726,7 +726,17 @@ async def run(parameters):
|
|||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"file_path",
|
||||
["../outside.py", "/main.py", "dir//main.py", "dir/../main.py", "dir\\main.py", "./main.py"],
|
||||
[
|
||||
"../outside.py",
|
||||
"/main.py",
|
||||
"dir//main.py",
|
||||
"dir/../main.py",
|
||||
"dir\\main.py",
|
||||
"./main.py",
|
||||
"%2e%2e/outside.py",
|
||||
"main.py%00.txt",
|
||||
"C:/main.py",
|
||||
],
|
||||
)
|
||||
async def test_dry_run_rejects_unsafe_file_paths(file_path: str) -> None:
|
||||
source = """
|
||||
|
|
@ -742,7 +752,7 @@ async def run(parameters):
|
|||
encoding=FileEncoding.BASE64,
|
||||
mime_type="text/x-python",
|
||||
),
|
||||
ScriptFileCreate(
|
||||
ScriptFileCreate.model_construct(
|
||||
path=file_path,
|
||||
content=_b64("x = 1\n"),
|
||||
encoding=FileEncoding.BASE64,
|
||||
|
|
|
|||
|
|
@ -336,6 +336,27 @@ async def test_deploy_script_via_mcp(monkeypatch):
|
|||
assert called_files[0].path == "main.py"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_deploy_script_rejects_unsafe_file_path_via_mcp(monkeypatch):
|
||||
fake_client = SimpleNamespace(deploy_script=AsyncMock())
|
||||
monkeypatch.setattr(script_tools, "get_skyvern", lambda: fake_client)
|
||||
|
||||
files = json.dumps([{"path": "%2e%2e/main.py", "content": "ZA==", "encoding": "base64"}])
|
||||
|
||||
async with Client(mcp) as client:
|
||||
result = await client.call_tool(
|
||||
"skyvern_script_deploy",
|
||||
{
|
||||
"script_id": "s_abc",
|
||||
"files": files,
|
||||
},
|
||||
)
|
||||
|
||||
assert result.data["ok"] is False
|
||||
assert "relative POSIX path" in result.data["error"]["message"]
|
||||
fake_client.deploy_script.assert_not_awaited()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Scenario 6: Workflow create surfaces caching fields when caller opts in
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
82
tests/unit/test_script_file_paths.py
Normal file
82
tests/unit/test_script_file_paths.py
Normal file
|
|
@ -0,0 +1,82 @@
|
|||
from urllib.parse import quote
|
||||
|
||||
import pytest
|
||||
from pydantic import ValidationError
|
||||
|
||||
from skyvern.schemas.scripts import FileEncoding, ScriptFileCreate
|
||||
from skyvern.utils.script_file_paths import build_script_file_storage_uri, normalize_script_file_path
|
||||
|
||||
|
||||
def _script_file(path: str) -> ScriptFileCreate:
|
||||
return ScriptFileCreate(path=path, content="ZA==", encoding=FileEncoding.BASE64)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"file_path",
|
||||
[
|
||||
"",
|
||||
"../outside.py",
|
||||
"src/../outside.py",
|
||||
"/main.py",
|
||||
"\\main.py",
|
||||
"C:/main.py",
|
||||
"src//main.py",
|
||||
"src/./main.py",
|
||||
"src\\main.py",
|
||||
"main.py\x00.txt",
|
||||
"main.py%00.txt",
|
||||
"main.py%2500.txt",
|
||||
"%2e%2e/outside.py",
|
||||
"src/%2e%2e/outside.py",
|
||||
"%2Fmain.py",
|
||||
"src%5Cmain.py",
|
||||
],
|
||||
)
|
||||
def test_script_file_create_rejects_unsafe_paths_at_input_time(file_path: str) -> None:
|
||||
with pytest.raises(ValidationError) as exc:
|
||||
_script_file(file_path)
|
||||
|
||||
assert "relative POSIX path" in str(exc.value)
|
||||
|
||||
|
||||
def test_script_file_create_normalizes_encoded_storage_path() -> None:
|
||||
script_file = _script_file("src%2Fmain.py")
|
||||
|
||||
assert script_file.path == "src/main.py"
|
||||
|
||||
|
||||
def test_script_file_create_rejects_deeply_encoded_traversal_path() -> None:
|
||||
file_path = "%2e%2e/outside.py"
|
||||
for _ in range(6):
|
||||
file_path = quote(file_path, safe="")
|
||||
|
||||
with pytest.raises(ValidationError) as exc:
|
||||
_script_file(file_path)
|
||||
|
||||
assert "relative POSIX path" in str(exc.value)
|
||||
|
||||
|
||||
def test_build_script_file_storage_uri_pins_normalized_path_under_base() -> None:
|
||||
uri = build_script_file_storage_uri(
|
||||
"file:///tmp/artifacts/local/org/",
|
||||
script_id="s_test",
|
||||
script_version=2,
|
||||
file_path="src%2Fmain.py",
|
||||
)
|
||||
|
||||
assert uri == "file:///tmp/artifacts/local/org/scripts/s_test/2/src/main.py"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("file_path", ["../main.py", "%2e%2e/main.py", "main.py%00.txt", "C:/main.py"])
|
||||
def test_build_script_file_storage_uri_rejects_unsafe_paths(file_path: str) -> None:
|
||||
with pytest.raises(ValueError):
|
||||
build_script_file_storage_uri(
|
||||
"s3://bucket/v1/local/org",
|
||||
script_id="s_test",
|
||||
script_version=2,
|
||||
file_path=file_path,
|
||||
)
|
||||
|
||||
|
||||
def test_normalize_script_file_path_preserves_safe_posix_path() -> None:
|
||||
assert normalize_script_file_path("src/helpers/main.py") == "src/helpers/main.py"
|
||||
Loading…
Add table
Add a link
Reference in a new issue