You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Persist project symlink policy at command boundaries - #4934
Persist project filesystem policy through the init-only --allow-symlinks option and strictly reload allow_symlinks from .specify/init-options.json at project command boundaries. Missing configuration/key defaults to deny; malformed, unreadable or non-boolean configuration fails explicitly. Configuration bootstrap reads and writes always deny symlinks, regardless of the operational setting.
This bounded stage adds the explicit FileHelper handoff API, protects event cwd and bundle ancestor-selection boundaries, and wires the current upstream init lifecycle, optional-project check, and shared artifact CLI/MCP operations. Init validates its original target before adapter/package preparation and retains existing transaction/rollback ownership. Configuration updates preserve unrelated options and the stored policy.
The compatibility overwrite_text operation is deliberately non-atomic: it preserves existing inode/hard-link identity, permissions, umask and native text behavior. Encoding/I/O errors propagate and can leave partial writes; symlink policy is not hard-link confinement or a race-free sandbox. The existing atomic APIs are unchanged.
Migration limit: other filesystem/runtime/storage/script operations are not migrated by this PR. Loading policy alone does not enforce symlink behavior on those operations.
The validated branch is based on merged foundation 03de661789b74405ae93b8b532d52d2cf8af422e. Publication preflight found newer upstream commits in non-overlapping files; the PR merge-base remains that foundation and the delta contains only this policy stage.
Testing
Tested locally with uv run specify --help
Ran existing tests with uv sync && uv run pytest
Tested with a sample project (if applicable)
Tests use the repository-documented worktree interpreter rather than bare uv run pytest, avoiding another worktree's editable install. Dependencies were already present; UV_NO_SYNC=1 kept CLI smoke checks in the same validated environment.
Before fixes, the new-base boundary regressions produced 30 failures; original dangling/missing-parent target evidence produced two additional failures. Positive/negative coverage includes repeated policy edits in one process, strict documents/values and permission errors, bootstrap links, init-only exposure, selected roots/overrides, JSON/plain and MCP failure contracts, no handler/mutation before failure, observer ordering, existing-config rollback and new-root cleanup, plus both-mode overwrite behavior and configuration metadata/hard-link compatibility.
Windows limitation: validation above ran on macOS. The 53 focused native Windows cases were skipped. The merged foundation's Windows CI does not validate these newly changed policy/overwrite operations; native Windows policy-stage verification is pending this PR's CI.
AI Disclosure
I did not use AI assistance for this contribution
I did use AI assistance (fill in the disclosure below)
AI disclosure: Implemented with GitHub Copilot using GPT-6.1 Sol in agent/autonomous mode under user-directed scope and coordinating-agent review. AI assistance covered implementation, tests, documentation, validation and this PR description. No sub-agents were launched.
Reject missing and non-directory prefixes before parent traversal can be normalized into an existing init root. Preserve trusted existing directory and root aliases, propagate I/O failures, and cover the Windows behavior with portable regressions and native init cases.
Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 661d79b5-8b35-41ce-b903-db64815502bc
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The new artifact lookup policy-error path lacks the repository-required regression coverage.
0 open findings
Previously missed (1)
In code that hasn't changed since last review
Add artifact lookup policy-error coverage for malformed init options
src/specify_cli/artifacts/_commands.py:57
This branch adds policy-error translation for artifact lookup, the sole caller of this resolver, but the new policy tests cover only artifact list/info. Add lookup coverage for a malformed or symlinked init-options.json, asserting exit 1, empty stdout, and the JSON error envelope; this deterministic boundary behavior requires positive/negative coverage under CONTRIBUTING.md:227-240.
Fixed the Windows init-path CI failure in commit 07d9b3746885b0ed08a8f33af1f0d1130585c0e6.
Windows presence/strict-resolution probes can normalize missing/../existing into an existing root before accessing the missing prefix. Init now checks that each original prefix before .. is an existing directory, before policy reads, project preparation, or root creation. Existing directory and symlink-root aliases remain supported; permission/I/O failures propagate. The change is limited to init policy-boundary validation, regression tests, and its design documentation. It does not claim race-free filesystem protection or migrate other filesystem operations.
The original Windows Python 3.14 job failed test_new_init_target_preserves_evidence_before_project_reads[missing-parent-traversal]; the original Python 3.13 job was cancelled by matrix fail-fast, not an independently demonstrated test failure. Both fresh Windows jobs were inspected separately on the exact new head:
11,613 passed, 581 skipped; original failing case and new traversal/alias cases passed
All six OS/Python pytest jobs in run 38299687391 passed. All other current-head checks completed successfully, except the non-applicable scheduled dependency-audit job, which was skipped.
Local macOS evidence (not local native Windows execution):
Command
Result
LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests/specify_cli/test_file_policy.py -q -k 'normalized_traversal or existing_parent_traversal_root'
Review 5493458559 reported zero findings and no inline comments. Native Windows evidence is now available, but human maintainer review of the filesystem semantics remains outstanding; this automated follow-up does not satisfy or replace it.
AI disclosure: Posted on behalf of contributor @mnriem by GitHub Copilot, powered by GPT-6.1 Sol, operating in agent/autonomous mode under the contributor's approved scope. The agent authored the bounded code, regression tests, design-document update, and this summary; executed local validation; and inspected the native CI results. This is not a claim of personal human review or testing.
Addressed the coverage request in review 5495328871 with commit 54688810a586e8ea021c74ee64eb45167b1a271c. Only tests/specify_cli/artifacts/test_command_lookup.py changed; there are no runtime or documentation changes.
Real lookup success now covers a missing policy file, missing setting, and literal false/true. Negative cases cover malformed JSON, a non-boolean setting, a contained configuration symlink whose target records allow=true, and a dangling configuration symlink. They assert exit 1, empty stdout, the sole JSON error envelope with an actionable configuration diagnostic, and no installed-integration loader or artifact catalog access. This adds seven net collected cases (12,110 to 12,117 locally).
Regression evidence: substituting the actual pre-policy _resolve_project_root function from commit 03de661789b74405ae93b8b532d52d2cf8af422e into the lookup adapter in memory, while retaining current project preflight, produces four failing negative cases and four passing positive cases. The failures demonstrate the old translator losing policy diagnostics and incorrectly reporting a missing project. This is a focused translator comparison, not a full old-tree rerun; no source or checkout was changed for it. With the current translator, the tests pass.
Fresh exact-head CI run 38303244931 passed all six OS/Python pytest jobs. Both native Windows logs independently confirm that all eight positive/negative lookup cases passed:
All other current-head checks completed successfully except the non-applicable scheduled dependency-audit job, which was skipped. Native Windows execution occurred in CI, not locally. Human maintainer review remains outstanding; no review conversations were resolved.
AI disclosure: Posted on behalf of contributor @mnriem by GitHub Copilot, powered by GPT-6.1 Sol, operating in agent/autonomous mode within the contributor-approved scope. The agent authored the tests-only change and this summary, ran local validation and the qualified in-memory regression comparison, and inspected CI evidence. No claim of personal human review or testing is made.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Persist project filesystem policy through the init-only
--allow-symlinksoption and strictly reloadallow_symlinksfrom.specify/init-options.jsonat project command boundaries. Missing configuration/key defaults to deny; malformed, unreadable or non-boolean configuration fails explicitly. Configuration bootstrap reads and writes always deny symlinks, regardless of the operational setting.This bounded stage adds the explicit FileHelper handoff API, protects event cwd and bundle ancestor-selection boundaries, and wires the current upstream init lifecycle, optional-project
check, and shared artifact CLI/MCP operations. Init validates its original target before adapter/package preparation and retains existing transaction/rollback ownership. Configuration updates preserve unrelated options and the stored policy.The compatibility
overwrite_textoperation is deliberately non-atomic: it preserves existing inode/hard-link identity, permissions, umask and native text behavior. Encoding/I/O errors propagate and can leave partial writes; symlink policy is not hard-link confinement or a race-free sandbox. The existing atomic APIs are unchanged.Migration limit: other filesystem/runtime/storage/script operations are not migrated by this PR. Loading policy alone does not enforce symlink behavior on those operations.
The validated branch is based on merged foundation
03de661789b74405ae93b8b532d52d2cf8af422e. Publication preflight found newer upstream commits in non-overlapping files; the PR merge-base remains that foundation and the delta contains only this policy stage.Testing
uv run specify --helpuv sync && uv run pytestTests use the repository-documented worktree interpreter rather than bare
uv run pytest, avoiding another worktree's editable install. Dependencies were already present;UV_NO_SYNC=1kept CLI smoke checks in the same validated environment.LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -qLC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests/specify_cli/test_file_policy.py tests/specify_cli/artifacts tests/specify_cli/events/test_command_run.py tests/specify_cli/bundles/test_security_paths.py tests/specify_cli/test_command_check.py tests/test_file_helper.py -quvx ruff@0.15.0 check src testsgit diff --check HEADLC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests --collect-only -qUV_NO_SYNC=1 uv run specify --helpUV_NO_SYNC=1 uv run specify init <temporary-project> --integration generic --integration-options='--commands-dir .agent/commands' --non-interactive --ignore-agent-tools --allow-symlinkstrueand generated command directory; temporary project removedBefore fixes, the new-base boundary regressions produced 30 failures; original dangling/missing-parent target evidence produced two additional failures. Positive/negative coverage includes repeated policy edits in one process, strict documents/values and permission errors, bootstrap links, init-only exposure, selected roots/overrides, JSON/plain and MCP failure contracts, no handler/mutation before failure, observer ordering, existing-config rollback and new-root cleanup, plus both-mode overwrite behavior and configuration metadata/hard-link compatibility.
Windows limitation: validation above ran on macOS. The 53 focused native Windows cases were skipped. The merged foundation's Windows CI does not validate these newly changed policy/overwrite operations; native Windows policy-stage verification is pending this PR's CI.
AI Disclosure
AI disclosure: Implemented with GitHub Copilot using GPT-6.1 Sol in agent/autonomous mode under user-directed scope and coordinating-agent review. AI assistance covered implementation, tests, documentation, validation and this PR description. No sub-agents were launched.