Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
18 changes: 18 additions & 0 deletions src/specify_cli/extensions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2021,6 +2021,24 @@ def _restore_stranded_config_file(
except OSError:
pass # Best-effort; install already committed to the registry.

# Restore execute bits on shipped POSIX scripts. copytree here (and the
# zipfile.extractall in install_from_zip, which delegates to this method) does
# not restore a stripped Unix mode, so a bundled *.sh would land non-executable
# and a documented `.specify/extensions/<id>/scripts/...` invocation would fail
# with "Permission denied". This is the single sink every install route funnels
# through (extension add / update / bundle), so fixing it here covers them all.
# No-op on Windows (helper returns early).
#
# Deliberately the whole-project call, not a scoped one. The helper's contract
# is "make .specify scripts executable" — the same idempotent invariant that
# init and migrate restore — so re-establishing it after an install is exactly
# its job. A scoped variant would only spare re-walking already-correct files,
# a handful of stats that are negligible beside the copy/extract this method just
# did, and it would cost a per-caller scan-scope argument on an otherwise simple,
# widely-used interface. The simpler call wins.
from .. import ensure_executable_scripts
ensure_executable_scripts(self.project_root)

return manifest

def install_from_zip(
Expand Down
90 changes: 90 additions & 0 deletions tests/test_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1103,6 +1103,52 @@ def test_install_from_directory(self, extension_dir, project_dir):
assert (ext_dir / "extension.yml").exists()
assert (ext_dir / "commands" / "hello.md").exists()

@pytest.mark.skipif(
os.name == "nt", reason="POSIX execute bits are not meaningful on Windows"
)
def test_install_restores_execute_bit_on_shipped_scripts(
self, extension_dir, project_dir
):
"""Every install route restores execute bits on shipped POSIX scripts.

``install_from_directory`` is the single sink all routes funnel through
(``install_from_zip`` delegates to it; ``extension add``, ``extension
update``, and bundle installs all call these two methods). copytree /
zipfile.extractall do not restore a stripped Unix mode, so a shipped
``*.sh`` would land non-executable and a documented
``.specify/extensions/<id>/scripts/...`` invocation would fail with
"Permission denied". Guards that every route restores the bit; fails
without the ensure_executable_scripts() call in install_from_directory.
"""
import zipfile
import tempfile

scripts = extension_dir / "scripts"
scripts.mkdir()
script = scripts / "gate.sh"
script.write_text("#!/usr/bin/env bash\necho hi\n")
script.chmod(0o644) # non-executable, as extractall/copytree may leave it

installed = (
project_dir / ".specify" / "extensions" / "test-ext" / "scripts" / "gate.sh"
)
manager = ExtensionManager(project_dir)

# Route 1 — install_from_directory (extension add --dev, bundle dir install)
manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)
assert os.access(installed, os.X_OK), "directory install left script non-executable"

# Route 2 — install_from_zip, force=True: the ZIP path (extension add --from,
# bundle zip) and the remove-then-reinstall shape of `extension update`.
with tempfile.TemporaryDirectory() as tmpdir:
zip_path = Path(tmpdir) / "test-ext.zip"
with zipfile.ZipFile(zip_path, "w") as zf:
for f in extension_dir.rglob("*"):
if f.is_file():
zf.write(f, f.relative_to(extension_dir))
manager.install_from_zip(zip_path, "0.1.0", force=True)
assert os.access(installed, os.X_OK), "zip/update install left script non-executable"

def test_install_from_directory_explicitly_recovers_active_skills_dir(
self, extension_dir, project_dir, monkeypatch
):
Expand Down Expand Up @@ -6317,6 +6363,50 @@ def test_add_dev_links_copilot_agent_when_supported(
else:
assert not agent_file.is_symlink()

@pytest.mark.skipif(
os.name == "nt", reason="POSIX execute bits are not meaningful on Windows"
)
def test_add_makes_shipped_scripts_executable(self, extension_dir, project_dir):
"""extension add must restore execute bits on bundled POSIX scripts.

Archives are unpacked with zipfile.extractall and --dev installs copy the
tree; neither restores a stripped Unix mode, so a shipped *.sh can land
non-executable and a documented `.specify/extensions/<id>/scripts/...`
invocation then fails with "Permission denied". init / migrate /
integration-install already call ensure_executable_scripts(); this guards
that `extension add` does too.
"""
import stat

scripts_dir = extension_dir / "scripts"
scripts_dir.mkdir()
script = scripts_dir / "gate.sh"
script.write_text("#!/usr/bin/env bash\necho hi\n")
script.chmod(0o644) # non-executable, as an unpacked/copied script may be
assert not os.access(script, os.X_OK)

from typer.testing import CliRunner
from unittest.mock import patch
from specify_cli import app

runner = CliRunner()
with patch.object(Path, "cwd", return_value=project_dir):
result = runner.invoke(
app,
["extension", "add", str(extension_dir), "--dev"],
catch_exceptions=True,
)

assert result.exit_code == 0, result.output
installed = (
project_dir / ".specify" / "extensions" / "test-ext" / "scripts" / "gate.sh"
)
assert installed.exists(), result.output
assert os.access(installed, os.X_OK), (
f"installed script not executable: mode="
f"{stat.S_IMODE(installed.stat().st_mode):o}"
)

def test_add_dev_writes_codex_skills_as_files(self, extension_dir, project_dir):
"""Codex dev skills should be written as files so Codex can load them."""
from typer.testing import CliRunner
Expand Down