fix: reject symlinked extension registry storage by mnriem · Pull Request #5184 · github/spec-kit

mnriem · 2026-10-29T14:34:00Z

Description

Reject symlinked extension registry storage by routing registry reads and writes through a deny-mode FileHelper rooted at the project, or its nearest existing ancestor for a new project. Validate the destination before file-change observers run, while retaining registry recovery, write semantics, and preset-resolution diagnostics.

Add regression coverage for linked registry files and parent directories, including dangling links, and document the registry filesystem boundary.

Testing

Tested locally with uv run specify --help
Ran existing tests with uv sync && uv run pytest
Tested with a sample project (if applicable)
uv sync --extra test — passed.
uv run specify --help — passed.
LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -q — passed: 13,677 passed, 130 skipped.

For the existing-tests checklist item, dependencies were installed with uv sync --extra test, and pytest used this worktree's interpreter as required by the contribution guidance rather than bare uv run pytest. No separate manual sample-project run was performed.

AI Disclosure

I did not use AI assistance for this contribution
I did use AI assistance (fill in the disclosure below)

AI disclosure: GitHub Copilot, powered by GPT-6.1 Sol (gpt-6.1-sol), performed the investigation, code and test generation, documentation changes, automated validation, and PR preparation in autonomous agent mode at the contributor's request. The reasoning-effort setting is not exposed to the agent; no human line-by-line review is claimed.

Route registry I/O through deny-mode FileHelper operations and preserve preset resolution diagnostics. Cover linked storage hierarchies and missing-project creation. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

Registry policy failures can still allow disabled extension commands to run, while linked parent paths can bypass validation and several callers mishandle the new errors.

6 open issues

What changed in this PR

This PR hardens Specify CLI’s extension registry storage by moving registry I/O to a project-rooted, deny-mode FileHelper.

Changes:

Route registry reads and writes through the helper, validating destinations before file-change observers run.
Translate registry policy errors into preset-resolution diagnostics.
Add regression coverage for linked and missing storage, and document the filesystem boundary.

Full project tests were not rerun during this review.

AI review assistance: GitHub Copilot performed automated inspection and comment editing for the requesting contributor; contributor identity and model/settings were unavailable.

File	Description
`tests/test_extensions.py`	Tests linked-storage rejection, missing-storage creation, and observer sequencing.
`tests/specify_cli/presets/test_resolver.py`	Updates expectations for dangling registry links.
`src/specify_cli/presets/_resolver.py`	Wraps helper failures in preset-validation diagnostics.
`src/specify_cli/extensions/__init__.py`	Adds scoped registry I/O and write-destination preflight.
`design/file-helper.md`	Documents the registry’s always-deny boundary and preserved write semantics.

_{Review effort: Max (auto)}

        try:
-            with open(self.registry_path, "r", encoding="utf-8") as f:
-                data = json.load(f)
+            data = json.loads(self._files.read_text(self.registry_path))


        try:
-            with open(self.registry_path, "r", encoding="utf-8") as f:
-                data = json.load(f)
+            data = json.loads(self._files.read_text(self.registry_path))


        try:
-            with open(self.registry_path, "r", encoding="utf-8") as f:
-                data = json.load(f)
+            data = json.loads(self._files.read_text(self.registry_path))


        try:
-            with open(self.registry_path, "r", encoding="utf-8") as f:
-                data = json.load(f)
+            data = json.loads(self._files.read_text(self.registry_path))


Validate hierarchy metadata before registry recovery, fail closed during event resolution, preserve artifact and extension command errors, and retain non-fatal dependency and configuration probes. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

mnriem · 2026-10-29T16:24:12Z

Updated in e42d18e: registry hierarchy validation now precedes missing-directory and leaf recovery; event resolution stops on forbidden registry state; artifact CLI/MCP and extension commands preserve their error formats; and dependency/configuration probes retain their non-fatal behavior. Regression coverage and the filesystem-boundary documentation were updated. Posted on behalf of @mnriem by GitHub Copilot (GPT-6.1 Sol, autonomous agent mode; reasoning-effort setting not exposed), which implemented the code, tests, and documentation and generated this reply.

Copilot

🟡 Changes recommended

The dispatcher can still execute extension handlers with live or dangling registry symlinks, and extension update leaves the new symlink-policy exception uncaught.

6 open issues

2 resolved since last review

_{Review effort: Max (auto)}

+        self._files = FileHelper(root)
        self.data = self._load()


Reject registry hierarchy links and reparse points before dispatcher delegation or inline fallback. Route extension update through the shared CLI policy-error handler. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

mnriem · 2026-10-29T16:54:55Z

Updated in c671cc9: generated event dispatchers now reject linked registry storage and unsupported reparse points before importing an installed resolver or entering the inline fallback, with a diagnostic and exit status 2. Extension update now uses the shared CLI policy-error handler. Added coverage for no-package and stale-package execution, live and dangling links, reparse points, and both update invocation forms. Posted on behalf of @mnriem by GitHub Copilot (GPT-6.1 Sol, autonomous agent mode; reasoning-effort setting not exposed), which implemented the code, tests, and documentation and generated this reply.

Copilot

🔵 Needs a closer look

With events enabled and only built-in integrations installed, linked extension storage can cause integration switching to fail after removing the old integration’s files and metadata, without rollback.

5 open issues

1 previously missed issue

In code that hasn't changed since last review

Validate registry before removing the current integration

src/specify_cli/extensions/__init__.py:777

For a project with only built-in integrations and events enabled, a linked extensions directory or .registry now makes integration switch fail after it has removed the old integration's files and metadata. resolve_events() runs before the handler's rollback block (integrations/command_switch.py:295-303), and the outer lifecycle skips its transaction when no external packages are recorded (integrations/_lifecycle.py:468-469), leaving the old integration removed. Validate registry storage before Phase 1 and add a switch regression test that checks files and metadata remain unchanged on rejection.

1 resolved since last review

Validate registry links before dispatcher execution

_{Review effort: Max (auto)}

Resolve target events and validate extension registry storage before removing the active integration. Reject linked parents before absence checks, retain current files and metadata on read failures, and cover built-in switches with and without force. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

mnriem · 2026-10-30T12:14:11Z

Updated in 7cf5624: integration switch now resolves the target event configuration before removing the active integration or its metadata, and registry-policy or filesystem-read failures produce a normal CLI diagnostic without changing the existing installation. Event collection validates registry storage before treating an extension directory as absent. Added regressions for live and dangling registry/directory links, forced switches, and read failures, checking that current files and metadata remain unchanged. Posted on behalf of @mnriem by GitHub Copilot (GPT-6.1 Sol, autonomous agent mode; reasoning-effort setting not exposed), which implemented the code, tests, and documentation and generated this reply.

Copilot

🔵 Needs a closer look

Human assessment is needed because bundled template helpers still follow registry and parent links into storage that PresetResolver rejects, leaving a critical template-resolution boundary mismatch.

5 open issues

_{Review effort: Max (auto)}

Reject linked registry hierarchies in Bash, Python, and PowerShell template helpers without changing replacement-layer precedence. Exercise native Windows PowerShell 5.1 and junction paths, and restore the test working directory before fixture cleanup on Windows. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

mnriem · 2026-10-30T15:11:17Z

Updated in bf10eff: bundled Bash, Python, and Windows PowerShell template helpers now reject linked registry storage before reading the extension layer, while preserving higher-priority replacement behavior. The CLI regression test restores its working directory before fixture cleanup, fixing the Windows CI errors. The full Linux/macOS/Windows Python 3.13/3.14 matrix now passes, including native Windows PowerShell 5.1 and junction regressions. Posted on behalf of @mnriem by GitHub Copilot (GPT-6.1 Sol, autonomous agent mode; reasoning-effort setting not exposed), which implemented the code, tests, and documentation and generated this reply.

Copilot

🟡 Changes recommended

The documentation omits the restriction on linked registry storage and does not explain that --allow-symlinks cannot override it.

6 open issues

_{Review effort: Max (auto)}

+Extension registry I/O also uses a deny-mode helper rooted at the project, so
+`.specify`, `extensions`, and `.registry` are not trusted aliases. Registry
+reads and writes reject linked storage regardless of operational symlink
+policy. For a not-yet-created project, the nearest existing ancestor supplies


Copilot AI auto review requested due to automatic review settings October 29, 2026 14:34

Copilot started reviewing on behalf of mnriem October 29, 2026 14:34

View session

Copilot AI reviewed Oct 29, 2026

View reviewed changes

mnriem added the triage-must-have Verdict: high-value, important work for Spec Kit — do first label Oct 29, 2026

Copilot AI auto review requested due to automatic review settings October 29, 2026 16:24

Copilot started reviewing on behalf of mnriem October 29, 2026 16:24

View session

Copilot AI reviewed Oct 29, 2026

View reviewed changes

Comment thread src/specify_cli/events/__init__.py

Comment thread src/specify_cli/extensions/__init__.py

Comment on lines +771 to 772

self._files = FileHelper(root)

self.data = self._load()

Copilot AI auto review requested due to automatic review settings October 29, 2026 16:54

Copilot started reviewing on behalf of mnriem October 29, 2026 16:55

View session

Copilot AI reviewed Oct 29, 2026

View reviewed changes

Copilot AI auto review requested due to automatic review settings October 30, 2026 12:14

Copilot started reviewing on behalf of mnriem October 30, 2026 12:14

View session

Copilot AI reviewed Oct 30, 2026

View reviewed changes

Copilot AI auto review requested due to automatic review settings October 30, 2026 14:55

Copilot started reviewing on behalf of mnriem October 30, 2026 14:56

View session

Copilot AI reviewed Oct 30, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix: reject symlinked extension registry storage - #5184

fix: reject symlinked extension registry storage#5184
mnriem wants to merge 5 commits into
github:mainfrom
mnriem:mnriem-extension-registry-symlinks

mnriem commented Oct 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

mnriem commented Oct 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

mnriem commented Oct 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

mnriem commented Oct 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

mnriem commented Oct 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mnriem commented Oct 29, 2026

Description

Testing

AI Disclosure

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

mnriem commented Oct 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

mnriem commented Oct 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

mnriem commented Oct 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

mnriem commented Oct 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants