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
36 changes: 34 additions & 2 deletions src/specify_cli/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,13 @@ def atomic_write_json(target_file: Path, payload: dict[str, Any]) -> None:
else:
log("Skipped merge (preserved existing settings)", "yellow")
else:
shutil.copy2(sub_item, dest_file)
shutil.copyfile(sub_item, dest_file)
log("Copied (no existing settings.json):", "blue")

except Exception as e:
log(f"Warning: Could not merge settings: {e}", "yellow")
if not dest_file.exists():
shutil.copy2(sub_item, dest_file)
shutil.copyfile(sub_item, dest_file)


def merge_json_files(existing_path: Path, new_content: Any, verbose: bool = False) -> dict[str, Any] | None:
Expand Down Expand Up @@ -227,6 +227,38 @@ def deep_merge_polite(base: dict[str, Any], update: dict[str, Any]) -> dict[str,
return merged


def ensure_writable_tree(path: Path) -> None:
"""Add owner write+execute to every directory under path.

shutil.copytree always calls copystat() on directories, propagating
read-only bits from any read-only source. Call this after copytree to
guarantee destination directories accept writes regardless of source
permissions.
"""
Comment thread
rayhem marked this conversation as resolved.
if os.name == "nt":
return
for dirpath, _, _ in os.walk(path):
dp = Path(dirpath)
try:
dp.chmod((dp.stat().st_mode & 0o7777) | 0o300)
except OSError:
pass


def copy_file_preserving_exec(src: str, dst: str) -> None:
"""Copy file content without propagating source mtime or read-only bits.

shutil.copyfile transfers only content, leaving destination permissions at
the umask default (typically 0o644). This wrapper additionally re-applies
any execute bits present on the source so that installed shell scripts
remain executable while still being owner-writable.
"""
shutil.copyfile(src, dst)
if os.name != "nt":
exec_bits = os.stat(src).st_mode & 0o111
os.chmod(dst, 0o644 | exec_bits)

Comment on lines +248 to +260

def _display_project_path(project_root: Path, path: str | Path) -> str:
"""Return a stable POSIX-style display path for paths under a project."""
path_obj = Path(path)
Expand Down
2 changes: 1 addition & 1 deletion src/specify_cli/commands/init.py
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ def init(
project_path / ".specify" / "workflows" / "speckit"
)
dest_wf.mkdir(parents=True, exist_ok=True)
_shutil.copy2(
_shutil.copyfile(
bundled_wf / "workflow.yml",
dest_wf / "workflow.yml",
)
Expand Down
4 changes: 3 additions & 1 deletion src/specify_cli/extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

from ._init_options import is_ai_skills_enabled
from ._invocation_style import is_slash_skills_agent
from ._utils import copy_file_preserving_exec, ensure_writable_tree
from .catalogs import CatalogEntry as BaseCatalogEntry
from .catalogs import CatalogStackBase

Expand Down Expand Up @@ -1361,7 +1362,8 @@ def install_from_directory(
shutil.rmtree(dest_dir)

ignore_fn = self._load_extensionignore(source_dir)
shutil.copytree(source_dir, dest_dir, ignore=ignore_fn)
shutil.copytree(source_dir, dest_dir, ignore=ignore_fn, copy_function=copy_file_preserving_exec)
ensure_writable_tree(dest_dir)
Comment thread
rayhem marked this conversation as resolved.

# Register commands with AI agents
registered_commands = {}
Expand Down
6 changes: 3 additions & 3 deletions src/specify_cli/integrations/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ def copy_command_to_directory(
"""
dest_dir.mkdir(parents=True, exist_ok=True)
dst = dest_dir / filename
shutil.copy2(src, dst)
shutil.copyfile(src, dst)
return dst

@staticmethod
Expand Down Expand Up @@ -504,9 +504,9 @@ def install_scripts(
if not src_script.is_file():
continue
dst_script = scripts_dest / src_script.name
shutil.copy2(src_script, dst_script)
shutil.copyfile(src_script, dst_script)
if dst_script.suffix == ".sh":
dst_script.chmod(dst_script.stat().st_mode | 0o111)
dst_script.chmod((dst_script.stat().st_mode & 0o7777) | 0o111)
self.record_file_in_manifest(dst_script, project_root, manifest)
created.append(dst_script)

Expand Down
2 changes: 1 addition & 1 deletion src/specify_cli/integrations/copilot/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ def _setup_default(
# remove the user's settings file on uninstall.
self._merge_vscode_settings(settings_src, dst_settings)
else:
shutil.copy2(settings_src, dst_settings)
shutil.copyfile(settings_src, dst_settings)
self.record_file_in_manifest(dst_settings, project_root, manifest)
created.append(dst_settings)

Expand Down
4 changes: 3 additions & 1 deletion src/specify_cli/presets/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

from ..extensions import REINSTALL_COMMAND, ExtensionRegistry, normalize_priority
from .._init_options import is_ai_skills_enabled
from .._utils import copy_file_preserving_exec, ensure_writable_tree
from ..integrations.base import IntegrationBase


Expand Down Expand Up @@ -1534,7 +1535,8 @@ def install_from_directory(
if dest_dir.exists():
shutil.rmtree(dest_dir)

shutil.copytree(source_dir, dest_dir)
shutil.copytree(source_dir, dest_dir, copy_function=copy_file_preserving_exec)
ensure_writable_tree(dest_dir)

# Pre-register the preset so that composition resolution can see it
# in the priority stack when resolving composed command content.
Expand Down
29 changes: 29 additions & 0 deletions tests/integrations/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,14 @@ def test_copy_command_to_directory(self, tmp_path):
assert result == dest_dir / "speckit.plan.md"
assert result.read_text(encoding="utf-8") == "content"

def test_copy_command_to_directory_readonly_source_is_writable(self, tmp_path):
src = tmp_path / "source.md"
src.write_text("content", encoding="utf-8")
src.chmod(0o444)
dest_dir = tmp_path / "output"
result = IntegrationBase.copy_command_to_directory(src, dest_dir, "speckit.plan.md")
assert result.stat().st_mode & 0o200, "destination must be owner-writable"

Comment on lines +152 to +159
def test_record_file_in_manifest(self, tmp_path):
f = tmp_path / "f.txt"
f.write_text("hello", encoding="utf-8")
Expand All @@ -164,6 +172,27 @@ def test_write_file_and_record(self, tmp_path):
assert dest.read_text(encoding="utf-8") == "content"
assert "sub/f.txt" in m.files

def test_install_scripts_readonly_source_files_are_writable(self, tmp_path, monkeypatch):
scripts_src = tmp_path / "scripts_src"
scripts_src.mkdir()
helper = scripts_src / "helper.sh"
helper.write_text("#!/bin/sh\necho hi\n")
helper.chmod(0o444)
data = scripts_src / "data.txt"
data.write_text("payload")
data.chmod(0o444)

project = tmp_path / "project"
project.mkdir()
i = StubIntegration()
m = IntegrationManifest("stub", project)
monkeypatch.setattr(i, "integration_scripts_dir", lambda: scripts_src)
created = i.install_scripts(project, m)

assert len(created) == 2
for dst in created:
assert dst.stat().st_mode & 0o200, f"{dst.name} must be owner-writable"

Comment on lines +175 to +195
def test_setup_copies_shared_templates(self, tmp_path):
i = StubIntegration()
m = IntegrationManifest("stub", tmp_path)
Expand Down
14 changes: 14 additions & 0 deletions tests/integrations/test_integration_copilot.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,20 @@ def test_setup_creates_vscode_settings_new(self, tmp_path):
assert settings in created
assert any("settings.json" in k for k in m.files)

def test_setup_vscode_settings_readonly_source_is_writable(self, tmp_path, monkeypatch):
from specify_cli.integrations.copilot import CopilotIntegration
import json as _json
copilot = CopilotIntegration()
# Simulate a bundled settings file with read-only store permissions.
ro_src = tmp_path / "ro_settings.json"
ro_src.write_text(_json.dumps({"foo": "bar"}))
ro_src.chmod(0o444)
monkeypatch.setattr(copilot, "_vscode_settings_path", lambda: ro_src)
m = IntegrationManifest("copilot", tmp_path)
copilot.setup(tmp_path, m)
settings = tmp_path / ".vscode" / "settings.json"
assert settings.stat().st_mode & 0o200, ".vscode/settings.json must be owner-writable"

Comment on lines +70 to +83
def test_setup_merges_existing_vscode_settings(self, tmp_path):
from specify_cli.integrations.copilot import CopilotIntegration
copilot = CopilotIntegration()
Expand Down
86 changes: 86 additions & 0 deletions tests/test_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import json
import os
import platform
import stat
import tempfile
import shutil
import tomllib
Expand Down Expand Up @@ -984,6 +985,91 @@ 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 mode bits are not stable on Windows")
def test_install_from_directory_readonly_source_produces_writable_dest(
self, extension_dir, project_dir
):
"""Directories copied from a read-only source must be owner-writable."""
# Lock the source tree to simulate a Nix store / read-only mount
for dirpath, _, filenames in os.walk(extension_dir):
dp = Path(dirpath)
for fn in filenames:
(dp / fn).chmod(0o444)
dp.chmod(0o555)

try:
manager = ExtensionManager(project_dir)
manager.install_from_directory(
extension_dir, "0.1.0", register_commands=False
)

ext_dir = project_dir / ".specify" / "extensions" / "test-ext"
for dirpath, _, _ in os.walk(ext_dir):
mode = stat.S_IMODE(Path(dirpath).stat().st_mode)
assert mode & 0o200, (
f"{dirpath} is not owner-writable: {oct(mode)}"
)
finally:
# Restore writable perms so the temp_dir fixture can clean up
for dirpath, _, filenames in os.walk(extension_dir):
dp = Path(dirpath)
dp.chmod(0o755)
for fn in filenames:
(dp / fn).chmod(0o644)

@pytest.mark.skipif(os.name == "nt", reason="POSIX mode bits are not stable on Windows")
def test_install_from_directory_readonly_source_files_are_writable(
self, extension_dir, project_dir
):
"""Files copied via copyfile should inherit default umask, not source perms."""
for dirpath, _, filenames in os.walk(extension_dir):
dp = Path(dirpath)
for fn in filenames:
(dp / fn).chmod(0o444)
dp.chmod(0o555)

try:
manager = ExtensionManager(project_dir)
manager.install_from_directory(
extension_dir, "0.1.0", register_commands=False
)

ext_dir = project_dir / ".specify" / "extensions" / "test-ext"
for dirpath, _, filenames in os.walk(ext_dir):
for fn in filenames:
fp = Path(dirpath) / fn
# Functional check: the file must be openable for writing
with open(fp, "a") as fh:
fh.write("")
finally:
for dirpath, _, filenames in os.walk(extension_dir):
dp = Path(dirpath)
dp.chmod(0o755)
for fn in filenames:
(dp / fn).chmod(0o644)

@pytest.mark.skipif(os.name == "nt", reason="POSIX mode bits are not stable on Windows")
def test_install_from_directory_preserves_executable_bits(
self, extension_dir, project_dir
):
"""Shell scripts inside an extension must remain executable after install."""
scripts_dir = extension_dir / "scripts" / "bash"
scripts_dir.mkdir(parents=True)
script = scripts_dir / "run.sh"
script.write_text("#!/bin/sh\necho ok\n")
script.chmod(0o755)

manager = ExtensionManager(project_dir)
manager.install_from_directory(extension_dir, "0.1.0", register_commands=False)

installed_script = (
project_dir / ".specify" / "extensions" / "test-ext" / "scripts" / "bash" / "run.sh"
)
assert installed_script.exists()
mode = stat.S_IMODE(installed_script.stat().st_mode)
assert mode & 0o111, f"Installed script lost execute bits: {oct(mode)}"
assert mode & 0o200, f"Installed script is not owner-writable: {oct(mode)}"

def test_install_from_directory_explicitly_recovers_active_skills_dir(
self, extension_dir, project_dir, monkeypatch
):
Expand Down
58 changes: 58 additions & 0 deletions tests/test_presets.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
import pytest
import io
import json
import os
import stat
import tempfile
import shutil
import warnings
Expand Down Expand Up @@ -592,6 +594,62 @@ def test_install_from_directory(self, project_dir, pack_dir):
assert (installed_dir / "preset.yml").exists()
assert (installed_dir / "templates" / "spec-template.md").exists()

@pytest.mark.skipif(os.name == "nt", reason="POSIX mode bits are not stable on Windows")
def test_install_from_directory_readonly_source_produces_writable_dest(
self, project_dir, pack_dir
):
"""Directories copied from a read-only source must be owner-writable."""
for dirpath, _, filenames in os.walk(pack_dir):
dp = Path(dirpath)
for fn in filenames:
(dp / fn).chmod(0o444)
dp.chmod(0o555)

try:
manager = PresetManager(project_dir)
manager.install_from_directory(pack_dir, "0.1.5")

installed_dir = project_dir / ".specify" / "presets" / "test-pack"
for dirpath, _, _ in os.walk(installed_dir):
mode = stat.S_IMODE(Path(dirpath).stat().st_mode)
assert mode & 0o200, (
f"{dirpath} is not owner-writable: {oct(mode)}"
)
finally:
for dirpath, _, filenames in os.walk(pack_dir):
dp = Path(dirpath)
dp.chmod(0o755)
for fn in filenames:
(dp / fn).chmod(0o644)

@pytest.mark.skipif(os.name == "nt", reason="POSIX mode bits are not stable on Windows")
def test_install_from_directory_readonly_source_files_are_writable(
self, project_dir, pack_dir
):
"""Files copied via copyfile should inherit default umask, not source perms."""
for dirpath, _, filenames in os.walk(pack_dir):
dp = Path(dirpath)
for fn in filenames:
(dp / fn).chmod(0o444)
dp.chmod(0o555)

try:
manager = PresetManager(project_dir)
manager.install_from_directory(pack_dir, "0.1.5")

installed_dir = project_dir / ".specify" / "presets" / "test-pack"
for dirpath, _, filenames in os.walk(installed_dir):
for fn in filenames:
fp = Path(dirpath) / fn
with open(fp, "a") as fh:
fh.write("")
finally:
for dirpath, _, filenames in os.walk(pack_dir):
dp = Path(dirpath)
dp.chmod(0o755)
for fn in filenames:
(dp / fn).chmod(0o644)

def test_install_already_installed(self, project_dir, pack_dir):
"""Test installing an already-installed pack raises error."""
manager = PresetManager(project_dir)
Expand Down
Loading