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

Copilot

Copilot's findings

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

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

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>

… permission-bit tests

Copilot

Copilot's findings

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

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


 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
 from .._utils import dump_frontmatter



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

Comment thread tests/integrations/test_base.py

Comment thread tests/integrations/test_base.py

Comment thread tests/integrations/test_integration_copilot.py

Comment thread src/specify_cli/_utils.py

This was referenced Jun 17, 2026

[aw] Add Community Preset from Issue Submission failed #3027

Closed

[aw] Add Community Extension from Issue Submission failed #3028

Closed

[aw] Assess Bug from Labeled Issue failed #3029

Closed

Copilot AI mentioned this pull request Jun 17, 2026

fix(bug-assess): set min-integrity: none to allow reading external user issues #3030

Merged

rayhem and others added 2 commits June 17, 2026 20:24

fix: respect umask in copy_file_preserving_exec; add Windows skips to…

fdff6c3

… permission-bit tests

rayhem force-pushed the main branch from 6d24eeb to fdff6c3 Compare June 18, 2026 00:24

mnriem requested a review from Copilot June 18, 2026 13:30

Copilot started reviewing on behalf of mnriem June 18, 2026 13:30 View session

Copilot AI reviewed Jun 18, 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 2 commits 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem commented Jun 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem commented Jun 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants