github/github-spec-kit
1
0
Fork 0
mirror of https://github.com/github/spec-kit.git synced 2026-08-03 06:26:30 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(extensions): make shipped scripts executable after install (#3723)

Browse Source 
Extension archives are unpacked with zipfile.extractall and directory installs
are copied; neither restores a stripped Unix mode. A bundled *.sh therefore
lands non-executable, so a documented `.specify/extensions/<id>/scripts/bash/foo.sh`
invocation fails with "Permission denied" — e.g. a CI step that runs an
extension's gate. It only worked incidentally, after a later `specify init`.

Restore permissions at the shared sink. Every extension install route funnels
through ExtensionManager.install_from_directory (install_from_zip delegates to
it; extension add, extension update, and bundle installs all reach it), so
calling the existing ensure_executable_scripts() there covers every route —
present and future — by construction rather than by patching each command.

The helper already makes .specify scripts executable (init, migrate, and
integration-install all call it); it is called plainly, re-establishing the same
idempotent "scripts are executable" invariant those flows restore. Deliberately
the whole-project call rather than a scoped one: a scan-scope argument would only
spare re-walking already-correct files — negligible beside the copy/extract just
performed — while widening a simple, widely-used interface for a single caller.
Existing callers were audited: init's end-of-init call still covers core
.specify/scripts and is untouched; integration-install and migrate do no manager
install. Nothing is removed. No-op on Windows; best-effort per file; does not
change which files are executable or their mode.

Tests: a manager-level regression test asserts a mode-0644 script comes out
executable via both install_from_directory and install_from_zip(force=True) (the
latter also covering the remove-then-reinstall shape of extension update), plus
an end-to-end `extension add --dev` test. Both fail without the change; skipped
on Windows.

Fixes #3722.
This commit is contained in:
Oscar
2026-07-27 14:40:43 +02:00
committed by GitHub
parent 446ee329b1
commit 2fb94e0f9c
2 changed files with 108 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
src/specify_cli/extensions/__init__.py
View File

@@ -2021,6 +2021,24 @@ class ExtensionManager:
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(

90
tests/test_extensions.py
View File

@@ -1103,6 +1103,52 @@ class TestExtensionManager:
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
):
@@ -6317,6 +6363,50 @@ class TestExtensionAddCLI:
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