Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
21 changes: 20 additions & 1 deletion src/specify_cli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3642,9 +3642,28 @@ def extension_add(
console.print(f"Downloading from {from_url}...")

# Download ZIP to temp location
import uuid as _uuid
download_dir = project_root / ".specify" / "extensions" / ".cache" / "downloads"
download_dir.mkdir(parents=True, exist_ok=True)
zip_path = download_dir / f"{extension}-url-download.zip"

# Reject symlinked ancestors (consistent with shared_infra.py)
_check = project_root
for _part in download_dir.relative_to(project_root).parts:
_check = _check / _part
if _check.is_symlink():
console.print("[red]Error:[/red] Refusing to use symlinked download cache directory")
raise typer.Exit(1)
Comment thread
mnriem marked this conversation as resolved.
Outdated
Comment thread
mnriem marked this conversation as resolved.
Outdated
Comment thread
mnriem marked this conversation as resolved.
Outdated

safe_name = Path(extension).name.replace("/", "_").replace("\\", "_") or "download"
safe_name = safe_name[:64] # cap length to avoid filesystem errors
zip_path = download_dir / f"{safe_name}-{_uuid.uuid4().hex[:8]}.zip"

# Guard: resolved path must stay inside download_dir (CWE-22)
try:
zip_path.resolve().relative_to(download_dir.resolve())
Comment thread
mnriem marked this conversation as resolved.
Outdated
except ValueError:
Comment thread
mnriem marked this conversation as resolved.
Outdated
console.print("[red]Error:[/red] Invalid extension name (path traversal detected)")
raise typer.Exit(1)

try:
from specify_cli.authentication.http import open_url as _open_url
Expand Down
103 changes: 103 additions & 0 deletions tests/test_extension_add_path_traversal.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
"""Tests for path traversal guard in `specify extension add --from` (GHSA-67q9-p54f-7cpr).

The extension argument is used to construct a temporary ZIP download path.
Without sanitisation, absolute paths or ``../`` segments in the argument can
escape the intended cache directory, causing arbitrary file writes and deletes.
"""

from unittest.mock import MagicMock, patch

import pytest
from typer.testing import CliRunner

from specify_cli import app

runner = CliRunner()

TRAVERSAL_PAYLOADS = [
"../pwned",
"../../etc/passwd",
"subdir/../../escape",
Comment thread
mnriem marked this conversation as resolved.
"..\\pwned",
"..\\..\\etc\\passwd",
"/tmp/evil",
Comment thread
mnriem marked this conversation as resolved.
]


@pytest.fixture()
def project_dir(tmp_path, monkeypatch):
proj = tmp_path / "project"
proj.mkdir()
(proj / ".specify").mkdir()
monkeypatch.chdir(proj)
return proj


def _mock_open_url():
"""Return a mock open_url that yields fake ZIP bytes."""
mock_response = MagicMock()
mock_response.read.return_value = b"PK\x03\x04fake"
mock_response.__enter__ = MagicMock(return_value=mock_response)
mock_response.__exit__ = MagicMock(return_value=False)
return mock_response


class TestExtensionAddFromPathTraversal:
"""Path traversal payloads in the extension name must not escape the download cache."""

@pytest.mark.parametrize("bad_name", TRAVERSAL_PAYLOADS)
def test_traversal_payload_writes_inside_cache(self, project_dir, bad_name):
"""The zip_path passed to install_from_zip must resolve inside the cache dir."""
cache_dir = project_dir / ".specify" / "extensions" / ".cache" / "downloads"
with patch("specify_cli.authentication.http.open_url", return_value=_mock_open_url()), \
patch("specify_cli.extensions.ExtensionManager.install_from_zip", return_value=MagicMock(id="x", name="X", version="1.0.0")) as mock_install:
result = runner.invoke(
app,
["extension", "add", bad_name, "--from", "https://example.com/ext.zip"],
)
assert result.exit_code == 0, result.output
mock_install.assert_called_once()
zip_arg = mock_install.call_args[0][0] # positional arg: zip_path
zip_arg.resolve().relative_to(cache_dir.resolve()) # raises ValueError if outside

@pytest.mark.parametrize("bad_name", TRAVERSAL_PAYLOADS)
def test_traversal_payload_cannot_delete_outside_cache(self, project_dir, bad_name):
"""The finally-block cleanup must not delete files outside the cache dir."""
# Place a sentinel at the path the pre-fix code would have constructed
cache_dir = project_dir / ".specify" / "extensions" / ".cache" / "downloads"
cache_dir.mkdir(parents=True, exist_ok=True)
pre_fix_path = cache_dir / f"{bad_name}-url-download.zip"
try:
pre_fix_path.parent.mkdir(parents=True, exist_ok=True)
pre_fix_path.write_text("sentinel")
except OSError:
# Absolute payloads like /tmp/evil can't be created relative to cache
pre_fix_path = None
Comment thread
mnriem marked this conversation as resolved.
Outdated

with patch("specify_cli.authentication.http.open_url", return_value=_mock_open_url()), \
patch("specify_cli.extensions.ExtensionManager.install_from_zip", return_value=MagicMock(id="x", name="X", version="1.0.0")):
runner.invoke(
app,
["extension", "add", bad_name, "--from", "https://example.com/ext.zip"],
)
Comment thread
mnriem marked this conversation as resolved.
Outdated
if pre_fix_path is not None:
assert pre_fix_path.exists(), f"Sentinel deleted by cleanup: {pre_fix_path}"
assert pre_fix_path.read_text() == "sentinel"

def test_clean_name_is_unaffected(self, project_dir):
"""A normal extension name should not trigger the guard."""
with patch("specify_cli.authentication.http.open_url", return_value=_mock_open_url()), \
patch("specify_cli.extensions.ExtensionManager.install_from_zip") as mock_install:
mock_install.return_value = MagicMock(id="my-ext", name="My Ext", version="1.0.0")
result = runner.invoke(
app,
["extension", "add", "my-ext", "--from", "https://example.com/ext.zip"],
)

assert result.exit_code == 0, result.output
mock_install.assert_called_once()
# Verify the zip path is inside the cache
zip_arg = mock_install.call_args[0][0]
cache_dir = project_dir / ".specify" / "extensions" / ".cache" / "downloads"
zip_arg.resolve().relative_to(cache_dir.resolve())
assert "path traversal" not in (result.output or "").lower()
Loading