fix: ensure installed files are owner-writable regardless of source permissions by rayhem · Pull Request #2884 · github/spec-kit

rayhem · 2026-06-06T16:44:32Z

Description

shutil.copy2 and shutil.copytree (which uses copy2 by default) propagate permission bits from source to destination. When the source lives on a read-only filesystem — the Nix store, any read-only mount, or a permission-restricted directory — installed files land at 0o444 and directories at 0o555. Any subsequent write to .specify/ (re-running specify init, upgrading, editing installed config) then fails with PermissionError.

An install operation should produce owner-writable destinations. The installed file's mtime should also reflect when it was installed, not when the bundled asset was built — so the copy2 → copyfile change is correct on both counts.

I encountered this problem attempting to package spec-kit on NixOS. The repository gets added read-only to the Nix store. specify assumes it can write to the files it creates, but the copy utilities preserve the read-only permissions resulting in an error.

Testing

Tested locally with uv run specify --help
Ran existing tests with uv sync && uv run pytest
Tested with a sample project (if applicable)

AI Disclosure

I did not use AI assistance for this contribution
I did use AI assistance (describe below)

Fix devised and generated by Claude Opus 4.6

mnriem

Please add some positive and negative tests

Copilot

Pull request overview

This PR addresses a packaging/installation pitfall where shutil.copy2()/copytree() preserve source permission bits, causing files and directories installed from read-only sources (e.g., Nix store) to become non-writable in the project and later fail with PermissionError. It switches key copy operations to copyfile() (to avoid propagating perms/mtime) and introduces a helper to make copied directory trees owner-writable.

Changes:

Add ensure_writable_tree() and apply it after copytree() installs to force destination directories to be owner-writable.
Replace copy2 usage with copyfile in several install/copy paths to avoid inheriting read-only perms and source mtimes.
Add regression tests covering read-only source trees for extensions/presets and unit tests for ensure_writable_tree().

Show a summary per file

File	Description
`src/specify_cli/_utils.py`	Adds `ensure_writable_tree()` and switches settings copy fallback from `copy2` to `copyfile`.
`src/specify_cli/presets.py`	Uses `copytree(..., copy_function=copyfile)` and applies `ensure_writable_tree()` after preset installs.
`src/specify_cli/extensions.py`	Uses `copytree(..., copy_function=copyfile)` and applies `ensure_writable_tree()` after extension installs.
`src/specify_cli/commands/init.py`	Switches bundled workflow copy from `copy2` to `copyfile` to avoid inheriting perms/mtime.
`tests/test_utils.py`	New unit tests for `ensure_writable_tree()` behavior (POSIX-focused).
`tests/test_presets.py`	Adds regression tests ensuring preset installs from read-only sources yield writable destinations.
`tests/test_extensions.py`	Adds regression tests ensuring extension installs from read-only sources yield writable destinations.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 7/7 changed files
Comments generated: 1

Copilot

Copilot's findings

Files reviewed: 11/11 changed files
Comments generated: 4

mnriem · 2026-06-16T22:45:24Z

Please address Copilot feedback and rebase on upstream/main

…ermissions shutil.copy2 and copytree (which uses copy2 by default) propagate permission bits from source to destination. When the source lives on a read-only filesystem — the Nix store, any read-only mount, or a permission-restricted directory — installed files land at 0o444 and directories at 0o555, causing PermissionError on any subsequent write to .specify/ (re-running specify init, upgrading, editing installed config). Changes: - Replace shutil.copy2 with shutil.copyfile in all install paths (_utils.py, commands/init.py, integrations/base.py, integrations/copilot) so mtime and permission bits are never propagated from the source - Add copy_file_preserving_exec() to _utils.py: copies content via copyfile then sets 0o644 | source_exec_bits, so data files land at rw-r--r-- and shell scripts at rwxr-xr-x regardless of source perms - Use copy_file_preserving_exec as copy_function in extensions.py and presets/__init__.py so bundled shell scripts remain executable after install - Add ensure_writable_tree() to _utils.py and call it after copytree in extensions and presets: walks the destination tree and ORs 0o300 onto every directory, since copytree always calls copystat() on directories even when copy_function skips it for files - Mask st_mode with & 0o7777 before all chmod calls (ensure_writable_tree and install_scripts) to strip file-type bits; passing them to chmod is undefined per POSIX and raises EINVAL on some platforms - Add unit tests for ensure_writable_tree (positive and negative) and copy_file_preserving_exec, plus regression tests asserting read-only source trees produce owner-writable, executable-where-appropriate destinations for extensions, presets, integrations, and copilot Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Copilot's findings

Files reviewed: 11/11 changed files
Comments generated: 4

+    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"
+


+    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"
+


+    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"
+


+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)
+


mnriem · 2026-06-17T13:00:51Z

Please address Copilot feedback and rebase on upstream/main

rayhem requested a review from mnriem as a code owner June 6, 2026 16:44

mnriem requested changes Jun 8, 2026

View reviewed changes

mnriem requested a review from Copilot June 16, 2026 14:06

Copilot started reviewing on behalf of mnriem June 16, 2026 14:06 View session

Copilot AI reviewed Jun 16, 2026

View reviewed changes

Comment thread src/specify_cli/_utils.py

mnriem requested a review from Copilot June 16, 2026 21:19

Copilot started reviewing on behalf of mnriem June 16, 2026 21:20 View session

Copilot AI reviewed Jun 16, 2026

View reviewed changes

Comment thread src/specify_cli/_utils.py Outdated

Comment thread src/specify_cli/integrations/base.py Outdated

Comment thread src/specify_cli/extensions.py

Comment thread src/specify_cli/presets.py Outdated

rayhem force-pushed the main branch from 49bf5e2 to 6d24eeb Compare June 17, 2026 02:02

mnriem requested a review from Copilot June 17, 2026 12:24

Copilot started reviewing on behalf of mnriem June 17, 2026 12:24 View session

Copilot AI reviewed Jun 17, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: ensure installed files are owner-writable regardless of source permissions#2884

fix: ensure installed files are owner-writable regardless of source permissions#2884
rayhem wants to merge 1 commit into
github:mainfrom
rayhem:main

rayhem commented Jun 6, 2026

Uh oh!

mnriem left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem commented Jun 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

mnriem commented Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

rayhem commented Jun 6, 2026

Description

Testing

AI Disclosure

Uh oh!

mnriem left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem commented Jun 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

mnriem commented Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants