Skip to content

Commit f8a7456

Browse files
mnriemCopilot
andcommitted
Harden extension URL download cache
Assisted-by: GitHub Copilot (model: MAI-Code-1-Flash, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8f71e02a-bc64-4593-b305-2554debe96f6
1 parent fb163b9 commit f8a7456

3 files changed

Lines changed: 44 additions & 19 deletions

File tree

‎src/specify_cli/extensions/_commands.py‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -751,27 +751,35 @@ def extension_add(
751751
)
752752
raise typer.Exit(1)
753753

754-
download_file = os.fdopen(download_fd, "w+b")
755-
download_fd = -1
756-
download_file.write(zip_data)
757-
download_file.flush()
758-
download_file.seek(0)
754+
try:
755+
download_file = os.fdopen(download_fd, "w+b")
756+
download_fd = -1
757+
download_file.write(zip_data)
758+
download_file.flush()
759+
download_file.seek(0)
760+
except OSError as exc:
761+
console.print(
762+
"[red]Error:[/red] Could not safely write download file: "
763+
f"{_escape_markup(str(exc))}"
764+
)
765+
raise typer.Exit(1)
759766

760767
# Consume the inode reserved above rather than reopening
761768
# the mutable cache pathname during extraction.
762-
manifest = manager.install_from_zip(
763-
zip_path,
764-
speckit_version,
765-
priority=priority,
766-
force=force,
767-
archive_file=download_file,
768-
)
769-
except OSError as exc:
770-
console.print(
771-
"[red]Error:[/red] Could not safely write download file: "
772-
f"{_escape_markup(str(exc))}"
773-
)
774-
raise typer.Exit(1)
769+
try:
770+
manifest = manager.install_from_zip(
771+
zip_path,
772+
speckit_version,
773+
priority=priority,
774+
force=force,
775+
archive_file=download_file,
776+
)
777+
except OSError as exc:
778+
console.print(
779+
"[red]Error:[/red] Could not install extension from downloaded archive: "
780+
f"{_escape_markup(str(exc))}"
781+
)
782+
raise typer.Exit(1)
775783
finally:
776784
if download_file is not None:
777785
try:

‎tests/test_extension_add_path_traversal.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@
2323

2424

2525
def _require_secure_dir_fd() -> None:
26-
if not getattr(os, "O_NOFOLLOW", 0) or os.open not in os.supports_dir_fd:
26+
if (
27+
not getattr(os, "O_NOFOLLOW", 0)
28+
or os.open not in os.supports_dir_fd
29+
or os.mkdir not in os.supports_dir_fd
30+
):
2731
pytest.skip("requires dir_fd and O_NOFOLLOW support")
2832

2933

‎tests/test_extensions.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,13 @@ def _open_test_download_zip(project_root, download_dir, zip_filename):
5858
)
5959

6060

61+
def _validate_safe_cache_dir_test_stand_in(project_root):
62+
"""Cross-platform stand-in for the secure cache validator."""
63+
download_dir = project_root / ".specify" / "extensions" / ".cache" / "downloads"
64+
download_dir.mkdir(parents=True, exist_ok=True)
65+
return download_dir
66+
67+
6168
def can_create_symlink(tmp_path: Path) -> bool:
6269
"""Return True when the current platform/user can create file symlinks."""
6370
target = tmp_path / "symlink-target.txt"
@@ -7416,6 +7423,7 @@ def fake_install_from_zip(
74167423
runner = CliRunner()
74177424
with patch.object(Path, "cwd", return_value=project_dir), \
74187425
patch("typer.confirm", return_value=True), \
7426+
patch("specify_cli.extensions._commands._validate_safe_cache_dir", side_effect=_validate_safe_cache_dir_test_stand_in), \
74197427
patch("specify_cli.authentication.http.open_url", return_value=FakeResponse(_MINIMAL_ZIP_BYTES)), \
74207428
patch("specify_cli.extensions._commands._safe_open_download_zip", side_effect=_open_test_download_zip), \
74217429
patch.object(ExtensionManager, "install_from_zip", fake_install_from_zip), \
@@ -7465,6 +7473,7 @@ def test_add_from_url_escapes_download_exception_markup(self, tmp_path):
74657473
runner = CliRunner()
74667474
with patch.object(Path, "cwd", return_value=project_dir), \
74677475
patch("typer.confirm", return_value=True), \
7476+
patch("specify_cli.extensions._commands._validate_safe_cache_dir", side_effect=_validate_safe_cache_dir_test_stand_in), \
74687477
patch(
74697478
"specify_cli.authentication.http.open_url",
74707479
side_effect=urllib.error.URLError("bad [red]download[/red]"),
@@ -7506,6 +7515,7 @@ def __exit__(self, exc_type, exc, tb):
75067515
runner = CliRunner()
75077516
with patch.object(Path, "cwd", return_value=project_dir), \
75087517
patch("typer.confirm", return_value=True), \
7518+
patch("specify_cli.extensions._commands._validate_safe_cache_dir", side_effect=_validate_safe_cache_dir_test_stand_in), \
75097519
patch(
75107520
"specify_cli.authentication.http.open_url",
75117521
return_value=FakeResponse(b"<!DOCTYPE html><html>Sign in</html>"),
@@ -7556,6 +7566,7 @@ def reject_oversized(*_args, **_kwargs):
75567566
runner = CliRunner()
75577567
with patch.object(Path, "cwd", return_value=project_dir), \
75587568
patch("typer.confirm", return_value=True), \
7569+
patch("specify_cli.extensions._commands._validate_safe_cache_dir", side_effect=_validate_safe_cache_dir_test_stand_in), \
75597570
patch(
75607571
"specify_cli.authentication.http.open_url",
75617572
return_value=FakeResponse(_MINIMAL_ZIP_BYTES),
@@ -7627,6 +7638,7 @@ def fake_install(
76277638
runner = CliRunner()
76287639
with patch.object(Path, "cwd", return_value=project_dir), \
76297640
patch("typer.confirm", return_value=True), \
7641+
patch("specify_cli.extensions._commands._validate_safe_cache_dir", side_effect=_validate_safe_cache_dir_test_stand_in), \
76307642
patch("specify_cli.authentication.http.github_provider_hosts", return_value=("ghes.example",)), \
76317643
patch("specify_cli.authentication.http.open_url", side_effect=fake_open_url), \
76327644
patch("specify_cli.extensions._commands._safe_open_download_zip", side_effect=_open_test_download_zip), \
@@ -7728,6 +7740,7 @@ def fake_install_from_zip(
77287740
runner = CliRunner()
77297741
with patch.object(Path, "cwd", return_value=project_dir), \
77307742
patch("typer.confirm", return_value=True), \
7743+
patch("specify_cli.extensions._commands._validate_safe_cache_dir", side_effect=_validate_safe_cache_dir_test_stand_in), \
77317744
patch("specify_cli.authentication.http.open_url", return_value=FakeResponse(_MINIMAL_ZIP_BYTES)), \
77327745
patch("specify_cli.extensions._commands._safe_open_download_zip", side_effect=_open_test_download_zip), \
77337746
patch.object(ExtensionManager, "install_from_zip", fake_install_from_zip):

0 commit comments

Comments
 (0)