From 98a14e32be1e911597adc522c1112fa6cdcbc9c8 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:14:17 -0500 Subject: [PATCH 1/3] feat: persist project symlink policy at command boundaries Add strict per-invocation policy loading, init-only persistence, deny-mode configuration bootstrap, and explicit FileHelper handoff. Preserve init lifecycle rollback and config metadata compatibility with a bounded non-atomic overwrite operation. Other filesystem migrations remain deferred. 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 --- design/file-helper.md | 112 ++- docs/reference/core.md | 37 +- src/specify_cli/__init__.py | 6 + src/specify_cli/_file_policy.py | 133 +++ src/specify_cli/_init_options.py | 13 +- src/specify_cli/_project.py | 58 +- src/specify_cli/artifacts/_commands.py | 15 +- src/specify_cli/artifacts/_operation_info.py | 8 + src/specify_cli/artifacts/_operation_list.py | 11 +- src/specify_cli/bundles/_commands.py | 8 +- src/specify_cli/bundles/command_info.py | 3 +- src/specify_cli/bundles/command_list.py | 3 +- src/specify_cli/bundles/command_search.py | 3 +- src/specify_cli/bundles/project.py | 44 +- src/specify_cli/command_check.py | 12 +- src/specify_cli/command_init.py | 30 +- src/specify_cli/events/command_run.py | 10 + src/specify_cli/file_helper.py | 15 + src/specify_cli/integrations/_helpers.py | 24 +- src/specify_cli/integrations/_lifecycle.py | 24 +- src/specify_cli/workflows/command_run.py | 10 + .../bundles/test_security_paths.py | 81 +- tests/specify_cli/events/test_command_run.py | 54 ++ .../integrations/test_command_install.py | 27 +- .../integrations/test_command_status.py | 14 +- tests/specify_cli/test_file_policy.py | 763 ++++++++++++++++++ tests/test_file_helper.py | 105 +++ tests/test_presets.py | 15 +- 28 files changed, 1532 insertions(+), 106 deletions(-) create mode 100644 src/specify_cli/_file_policy.py create mode 100644 tests/specify_cli/test_file_policy.py diff --git a/design/file-helper.md b/design/file-helper.md index fa9d4dd0ac..6fa6807857 100644 --- a/design/file-helper.md +++ b/design/file-helper.md @@ -1,19 +1,88 @@ -# Scoped FileHelper foundation +# Scoped FileHelper and project policy `specify_cli.file_helper.FileHelper` owns filesystem operations under one -explicitly established root. This is the first migration stage: existing CLI -commands retain their current behavior and do not yet use this helper. There is -no global CLI flag or process-global policy. - -The user-directed persistence requirement belongs to the dependent -project-policy wiring stage: save the setting in `.specify/init-options.json`, -then load and validate it for future invocations and pass the resulting policy -explicitly to helper instances. Only `specify init` exposes and persists the -setting. Every other CLI command reads the current project configuration on -each invocation, so manual edits take effect without a cached policy. There -is no root/global flag and no per-command override flag. The low-level helper -does not read configuration. Strict persisted-policy loading and configuration- -read bootstrap semantics must be coordinated before migration. +explicitly established root. Project-command boundaries now validate persisted +policy, and init-options bootstrap I/O uses the helper. Other existing command +filesystem operations are **not yet migrated**: loading policy alone does not +enforce symlink behavior on those operations. Shared infrastructure, registration, +managed storage, catalogs, scripts and runtime operations retain their existing +behavior until their dependent migration stages. + +Only `specify init --allow-symlinks` exposes the setting; init defaults to deny +and persists the literal JSON boolean `allow_symlinks` in +`.specify/init-options.json`. Re-init without the flag records `false`. +Other project-scoped commands validate the selected project's current +configuration each invocation, including successive invocations in one process. +No global flag, per-command override, mutable global policy, or cache is used. +Projectless help/version/tool operations do not require configuration. +`check` is an optional project consumer: it validates any present policy at its +selected cwd/override boundary before loading installed integrations or checking +tools, but still permits a missing project marker or configuration in cwd. +Existing resolution rules, including `SPECIFY_INIT_DIR`, bundle ancestor search, +direct workflow-file roots and explicit init targets, determine which root is +checked; policy loading does not add ancestor discovery. +Event execution retains its cwd-only selection and does not acquire an +override or project-marker requirement; it preflights any present cwd policy +before invoking handlers. Bundle ancestor search continues only when a marker +is genuinely absent. A nearer forbidden/present invalid `.specify` entry fails +at that candidate rather than selecting an ancestor or initializing elsewhere. + +Missing configuration or a missing key means deny. Only literal JSON +`true`/`false` values are valid. Invalid JSON, non-object documents, invalid +encoding, unreadable files and invalid values fail explicitly before mutations +at the project boundary. Dangling configuration links are present forbidden +entries, not missing defaults. + +Configuration bootstrap reads **and writes** always use a deny-mode helper, +even when stored operational policy or the init flag allows symlinks. +`.specify` and `init-options.json` below the trusted project root must not be +links or unsupported reparse points. Root/OS aliases retain the foundation's +trusted-boundary treatment. For a new init root, `prepare_init_root` creates its +missing suffix using a deny-mode helper rooted at the nearest existing parent; +an existing explicitly supplied root alias is trusted. `preflight_init_root` +validates the original requested spelling without creating a root, including +missing-parent traversal and dangling-parent evidence before canonicalization. +Init's external-adapter lifecycle validates existing target policy before +reading integration records, loading adapters or preparing catalog packages. +When that lifecycle requires a +transaction, it creates the target under the existing lock using the original +requested spelling and `prepare_init_root`, before invoking the inner handler. +The recorded pre-transaction directory state retains that spelling, so a newly +created root does not become a spurious existing-directory conflict. Lock, +journal and handler operations retain their canonical project identity after +the original spelling has been validated. + +## Policy handoff for dependent migrations + +`specify_cli._file_policy.read_project_options(root)` is the strict bootstrap +reader; it is separate from the legacy tolerant `load_init_options` library +API. `project_files(root)` reads policy afresh and returns an immutable +`FileHelper(root, allow_symlinks=...)`. Pass that returned operation owner +explicitly to domain consumers; do not reconstruct a default helper and lose +the policy. `specify_cli._project.resolve_specify_project_files()` performs the +normal existing-project resolution and returns the same handoff for new CLI +adapters. Commands using another established root call `project_files` on +their actually selected root. Retain a helper only for the current operation; +a later invocation resolves a new helper, so manual edits take effect. + +Existing Path-returning resolvers preflight policy without changing their return +type or domain constructors. They do not implicitly equip unmigrated domain +consumers with an operation helper. Failures are translated into existing +command diagnostics, with a single `{"error": "..."}` on stderr and empty +stdout for JSON policy failures; artifact/list surfaces preserve their error +envelope contracts. Artifact list/info validate their explicit project roots in +the shared application operations, so CLI and MCP peer adapters share the same +policy semantics while retaining their own error envelopes. + +`save_project_options` and the compatibility `save_init_options` writer use +deny-mode bootstrap I/O. Init merges its owned settings into existing valid +options, preserving unrelated fields. Integration option update/clear/version +paths use the strict reader and retain `allow_symlinks`. Unmigrated tolerant +library readers keep their existing missing/malformed fallback contracts. +Configuration writers validate their deny-mode destination before the existing +file-change observer snapshots it, then retain the before/after observer hooks +around the write. Downstream init failures therefore remain covered by the +existing lifecycle rollback, including prior policy and unrelated options. ```python from specify_cli.file_helper import FileHelper @@ -87,6 +156,15 @@ helpers for independent project/source/managed boundaries. `encoding="utf-8"`. Replacement files use the supplied mode; metadata and hard-link identity are not preserved. Atomic means same-directory `os.replace`, not crash durability. +- `overwrite_text(path, content, encoding="utf-8")` is a **non-atomic** text + upsert with existing parents, used for configuration compatibility. It uses + the same accessed-path/link/containment checks as other operations, follows + only allowed existing contained file targets, and preserves link chains and + the target inode (including hard-link identity and permissions). New files + use `0o666` filtered by the process umask; text encoding and native newline + translation match the previous configuration writer. An encoding or I/O + failure propagates, and a write failure can leave truncated/partial content. + This is not hard-link confinement: a hard-linked alias can observe updates. - `delete(path, recursive=False)` unlinks files/links or removes directories. Nonrecursive directory deletion requires an empty directory. The established root cannot be deleted. @@ -129,8 +207,10 @@ Existing safe-write mechanisms informed this helper, but existing shared infrastructure warning/skip behavior and development-mode link creation are unchanged. Migrate one owned operation boundary at a time, carrying original paths and an explicit root/policy into the helper before any early resolution. -CLI option wiring, linked command directories, and development-mode -registration remain dependent stages. +Linked command directories and development-mode registration remain dependent +stages. Non-atomic configuration writes preserve legacy metadata semantics; +portable preflight does not guarantee race-free protection against concurrent +path replacement. Windows path classification and directory-mode reparse rejection have portable regression tests. Native Windows path and junction tests also exist and are diff --git a/docs/reference/core.md b/docs/reference/core.md index afd53c46d4..5e0b80a1d7 100644 --- a/docs/reference/core.md +++ b/docs/reference/core.md @@ -15,6 +15,7 @@ specify init [] | `--script sh\|ps\|py` | Script type: `sh` (bash/zsh), `ps` (PowerShell), or `py` (Python) | | `--here` | Initialize in the current directory instead of creating a new one | | `--force` | Force merge/overwrite when initializing in an existing directory | +| `--allow-symlinks` | Persist permission for contained symlinks in migrated FileHelper operations; default is deny | | `--ignore-agent-tools` | Skip checks for AI coding agent CLI tools | | `--preset ` | Install a preset during initialization | @@ -27,6 +28,32 @@ Use `` to create a new directory, or `--here` (or `.`) to initiali When `--integration` is omitted, interactive terminals prompt you to choose an integration. Non-interactive sessions, such as CI or piped runs, default to GitHub Copilot; pass `--integration ` to choose a different integration explicitly, or set `SPECKIT_INTEGRATION_DEFAULT` to change the fallback (see [Environment Variables](#environment-variables)). +### Persisted filesystem policy + +Only `init` accepts `--allow-symlinks`. It records `allow_symlinks: true` in +`.specify/init-options.json`; omitting it records `false`, including on re-init. +Project-scoped CLI commands read the current value on each invocation, so a +manual edit takes effect immediately. A missing file or key means deny; the +value must be a JSON boolean, not a string, number or null. Invalid/unreadable +configuration is an error, not a fallback. Unrelated recorded init options are +preserved. + +The configuration bootstrap hierarchy (`.specify` and `init-options.json`) +must always be non-symlinked below the trusted project root, even in allow mode. +Help, version, check and projectless tool/catalog operations do not gain a +project requirement. Init uses its explicit target, not `SPECIFY_INIT_DIR`. +`check` can consume installed project integrations: it validates any present +policy at its selected cwd or `SPECIFY_INIT_DIR` boundary before loading them +or checking tools. It still works in a projectless cwd without `.specify` or +init options. Help and version do not read project policy. + +**Migration limit:** policy is enforced by FileHelper operations, currently +including init-options bootstrap I/O. Loading policy does not yet route all +other command filesystem operations through FileHelper. Shared infrastructure, +registration, managed storage, scripts and runtime operations retain their +existing behavior until migrated. See [FileHelper design](../../design/file-helper.md) +for the boundary and handoff contract. + ### Examples ```bash @@ -60,7 +87,11 @@ specify init my-project --integration copilot --preset compliance > > **Version control.** `specify init` scaffolds a managed `.specify/.gitignore` that excludes machine-local state — `feature.json` (the current-feature pointer, rewritten on every feature switch) and per-machine extension `extensions/*/local-config.yml` overrides — while leaving everything else under `.specify/` (constitution, templates, scripts, extension config) shareable so teams stay aligned. Like the rest of `.specify/`'s shared scripts and templates, the file is tracked in the shared-infrastructure manifest: your edits are preserved on re-init and `specify init --here --force` restores the managed content. It is intentionally left in place by `specify integration uninstall`, which only removes the uninstalled agent's own files. > -> **Symlinked project roots.** `SPECIFY_INIT_DIR` relocates *where* the project is, not *how* a command treats symlinks: each command keeps its existing cwd-path stance. Commands that traverse and write project files through broad input paths (`bundle`, `workflow run `) refuse a symlinked `.specify/` to preserve write confinement. Other project-scoped commands keep their existing behavior when `SPECIFY_INIT_DIR` points at a project root, which may include following a symlinked `.specify/`. +> **Symlinked project roots.** An explicitly established project-root alias is +> trusted, including OS ancestors above it. The configuration hierarchy below +> that root is always checked in deny mode: `.specify/` and `init-options.json` +> cannot be symlinks. Unmigrated operations elsewhere in the project retain +> their existing symlink behavior. ## Naming Features with the Helper Scripts @@ -96,6 +127,10 @@ specify check Checks that CLI-based AI coding agents are available on your system. IDE-based agents are skipped since they don't require a CLI tool. +Installed project integrations are included when available. Any present +filesystem policy at the selected project boundary must be valid; a projectless +cwd remains supported without configuration. + This command stays offline. If a command behaves like an older Spec Kit version or an expected CLI feature is missing, run `specify self check` to check whether your local CLI is behind the latest release. ## Version Information diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 544ef1cfef..181b0feec1 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -446,6 +446,12 @@ def _require_specify_project(*, load_integrations: bool = False) -> Path: """ override = _resolve_init_dir_override() project_root = override if override is not None else Path.cwd() + from ._project import ProjectResolutionError, preflight_project_policy, render_project_error + + try: + preflight_project_policy(project_root) + except ProjectResolutionError as exc: + render_project_error(exc) if (project_root / ".specify").is_dir(): if load_integrations: from .integrations.installer import IntegrationInstallError, load_installed_integrations diff --git a/src/specify_cli/_file_policy.py b/src/specify_cli/_file_policy.py new file mode 100644 index 0000000000..e9690027c4 --- /dev/null +++ b/src/specify_cli/_file_policy.py @@ -0,0 +1,133 @@ +"""Strict project configuration bootstrap and explicit FileHelper handoff.""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any, NoReturn + +from ._init_options import INIT_OPTIONS_FILE +from .file_helper import FileHelper, FileHelperError + + +class ProjectPolicyError(ValueError): + """The project configuration cannot safely supply a filesystem policy.""" + + +def _validate_options(options: Any) -> dict[str, Any]: + if not isinstance(options, dict): + raise ProjectPolicyError(f"{INIT_OPTIONS_FILE} must contain a JSON object") + if "allow_symlinks" in options and type(options["allow_symlinks"]) is not bool: + raise ProjectPolicyError( + f"{INIT_OPTIONS_FILE}: allow_symlinks must be a JSON boolean (true or false)" + ) + return options + + +def _reject_json_constant(value: str) -> NoReturn: + raise ValueError(f"Non-JSON numeric constant: {value}") + + +def read_project_options(project_root: Path) -> dict[str, Any]: + """Read without following configuration links; only absence defaults.""" + try: + files = FileHelper(project_root) + except (OSError, RuntimeError) as exc: + raise ProjectPolicyError( + f"Cannot establish project root for {INIT_OPTIONS_FILE}: {exc}" + ) from exc + try: + text = files.read_text(INIT_OPTIONS_FILE) + except FileNotFoundError: + return {} + except (OSError, UnicodeError, FileHelperError) as exc: + raise ProjectPolicyError( + f"Cannot read {INIT_OPTIONS_FILE}: {exc}. " + "Ensure the configuration hierarchy is non-symlinked and readable." + ) from exc + try: + options = json.loads(text, parse_constant=_reject_json_constant) + except (ValueError, RecursionError) as exc: + raise ProjectPolicyError( + f"Invalid JSON in {INIT_OPTIONS_FILE}: {exc}. Repair the configuration." + ) from exc + return _validate_options(options) + + +def project_files(project_root: Path) -> FileHelper: + """Resolve current policy afresh and return an immutable operation owner.""" + options = read_project_options(project_root) + try: + return FileHelper(project_root, allow_symlinks=options.get("allow_symlinks", False)) + except (OSError, RuntimeError) as exc: + raise ProjectPolicyError(f"Cannot establish project filesystem boundary: {exc}") from exc + + +def save_project_options(project_root: Path, options: dict[str, Any]) -> None: + """Persist with deny-mode bootstrap and legacy metadata/text I/O behavior.""" + _validate_options(options) + try: + content = json.dumps( + options, indent=2, sort_keys=True, ensure_ascii=False, allow_nan=False + ) + "\n" + except (TypeError, ValueError, RecursionError) as exc: + raise ProjectPolicyError(f"Cannot encode {INIT_OPTIONS_FILE} as JSON: {exc}") from exc + try: + files = FileHelper(project_root) + # Validate the entire destination before creating even .specify. + try: + files.read_text(INIT_OPTIONS_FILE) + except FileNotFoundError: + pass + from .integrations._file_changes import after_file_change, before_file_change + + destination = project_root / INIT_OPTIONS_FILE + before_file_change(destination) + files.mkdir(".specify", exist_ok=True) + files.overwrite_text(INIT_OPTIONS_FILE, content) + after_file_change(destination) + except (OSError, UnicodeError, FileHelperError, RuntimeError) as exc: + raise ProjectPolicyError( + f"Cannot write {INIT_OPTIONS_FILE}: {exc}. " + "Ensure the configuration hierarchy is non-symlinked and writable." + ) from exc + + +def _existing_init_parent(target: Path) -> Path: + ancestor = target.parent + while not ancestor.exists() and not ancestor.is_symlink(): + ancestor = ancestor.parent + return ancestor + + +def preflight_init_root(target: Path) -> Path: + """Validate original init spelling and policy before project preparation.""" + target = target.absolute() + try: + if target.exists() or target.is_symlink(): + read_project_options(target) + else: + files = FileHelper(_existing_init_parent(target)) + try: + files.read_bytes(target) + except FileNotFoundError: + pass + return target.resolve() + except (OSError, FileHelperError, RuntimeError) as exc: + raise ProjectPolicyError( + f"Cannot establish init project root {target}: {exc}" + ) from exc + + +def prepare_init_root(target: Path) -> Path: + """Create a missing init root beneath its nearest existing trusted parent. + + Existing explicit root aliases are trusted just like FileHelper roots. + For new roots, deny traversal below the nearest existing directory. + """ + target = target.absolute() + if target.exists() or target.is_symlink(): + FileHelper(target) + return target + FileHelper(_existing_init_parent(target)).mkdir(target, parents=True, exist_ok=True) + return target diff --git a/src/specify_cli/_init_options.py b/src/specify_cli/_init_options.py index 39003a7e7c..67987b8c83 100644 --- a/src/specify_cli/_init_options.py +++ b/src/specify_cli/_init_options.py @@ -21,16 +21,9 @@ def __repr__(self) -> str: # pragma: no cover - debug aid only def save_init_options(project_path: Path, options: dict[str, Any]) -> None: """Persist the CLI options used during ``specify init``.""" - dest = project_path / INIT_OPTIONS_FILE - from .integrations._file_changes import after_file_change, before_file_change - - before_file_change(dest) - dest.parent.mkdir(parents=True, exist_ok=True) - dest.write_text( - json.dumps(options, indent=2, sort_keys=True, ensure_ascii=False) + "\n", - encoding="utf-8", - ) - after_file_change(dest) + from ._file_policy import save_project_options + + save_project_options(project_path, options) def load_init_options(project_path: Path) -> dict[str, Any]: diff --git a/src/specify_cli/_project.py b/src/specify_cli/_project.py index 9ed2fe1508..54bec14d86 100644 --- a/src/specify_cli/_project.py +++ b/src/specify_cli/_project.py @@ -2,12 +2,24 @@ from __future__ import annotations +import json import os from pathlib import Path +from typing import NoReturn import typer +from rich.markup import escape from ._console import err_console +from ._file_policy import ProjectPolicyError, project_files +from .file_helper import FileHelper + +try: + from typer._click.globals import get_current_context +except ModuleNotFoundError as error: + if error.name != "typer._click": + raise + from click import get_current_context class ProjectResolutionError(RuntimeError): @@ -19,11 +31,17 @@ def _resolve_init_dir_override_unrendered() -> Path | None: raw = os.environ.get("SPECIFY_INIT_DIR", "") if not raw: return None - init_root = (Path.cwd() / raw).resolve() + try: + init_root = (Path.cwd() / raw).resolve() + except (OSError, RuntimeError) as exc: + raise ProjectResolutionError( + f"Cannot resolve SPECIFY_INIT_DIR '{raw}': {exc}" + ) from exc if not init_root.is_dir(): raise ProjectResolutionError( f"SPECIFY_INIT_DIR does not point to an existing directory: {raw}" ) + preflight_project_policy(init_root) if not (init_root / ".specify").is_dir(): raise ProjectResolutionError( "SPECIFY_INIT_DIR is not a Spec Kit project " @@ -32,6 +50,23 @@ def _resolve_init_dir_override_unrendered() -> Path | None: return init_root +def preflight_project_policy(project_root: Path) -> None: + """Validate at the resolved project boundary without printing or caching.""" + try: + project_files(project_root) + except ProjectPolicyError as exc: + marker = project_root / ".specify" + if marker.is_symlink(): + raise ProjectResolutionError( + f"Refusing to use symlinked .specify path: {marker}. {exc}" + ) from exc + if marker.exists() and not marker.is_dir(): + raise ProjectResolutionError( + f"Not a Spec Kit project: .specify path exists but is not a directory: {marker}" + ) from exc + raise ProjectResolutionError(str(exc)) from exc + + def _resolve_init_dir_override() -> Path | None: """Resolve the ``SPECIFY_INIT_DIR`` project override for the Python CLI. @@ -58,8 +93,7 @@ def _resolve_init_dir_override() -> Path | None: try: return _resolve_init_dir_override_unrendered() except ProjectResolutionError as error: - err_console.print(f"[red]Error:[/red] {error}") - raise typer.Exit(1) + render_project_error(error) def resolve_specify_project_root() -> Path: @@ -73,6 +107,24 @@ def resolve_specify_project_root() -> Path: if override is not None: return override project_root = Path.cwd() + preflight_project_policy(project_root) if not (project_root / ".specify").is_dir(): raise ProjectResolutionError("Not a Spec Kit project (no .specify/ directory)") return project_root + + +def render_project_error(error: ProjectResolutionError) -> NoReturn: + """Render a project failure on the current command's error surface.""" + context = get_current_context(silent=True) + if context is not None and ( + context.params.get("json_output") is True or context.params.get("as_json") is True + ): + typer.echo(json.dumps({"error": str(error)}, ensure_ascii=False), err=True) + else: + err_console.print(f"[red]Error:[/red] {escape(str(error))}") + raise typer.Exit(1) from error + + +def resolve_specify_project_files() -> FileHelper: + """Resolve the active project and hand its current policy to domain consumers.""" + return project_files(resolve_specify_project_root()) diff --git a/src/specify_cli/artifacts/_commands.py b/src/specify_cli/artifacts/_commands.py index c6cdb25777..8c4b785fe1 100644 --- a/src/specify_cli/artifacts/_commands.py +++ b/src/specify_cli/artifacts/_commands.py @@ -37,15 +37,24 @@ def _resolve_project_root() -> Path: ``typer.Exit`` on failure, which would corrupt the strict JSON envelope ``specify artifact list --json`` and ``specify artifact info --json`` emit on stdout/stderr. The Rich output is suppressed here and the - failure is re-raised as the module-local :class:`NotASpecKitProjectError` - for the shared error handler to serialize instead. + Missing-project failures become :class:`NotASpecKitProjectError`; policy + failures retain their actionable message for the shared JSON error handler. """ from .. import _require_specify_project # lazy: avoids circular import + from .._file_policy import ProjectPolicyError + from .._project import ProjectResolutionError with contextlib.redirect_stderr(io.StringIO()): try: root = _require_specify_project() - except typer.Exit: + except typer.Exit as exc: + cause = exc.__cause__ + if isinstance(cause, ProjectResolutionError) and isinstance( + cause.__cause__, ProjectPolicyError + ): + error = ArtifactError(str(cause)) + error.message = str(cause) + raise error from exc raise NotASpecKitProjectError() from None from ..integrations.installer import ( IntegrationInstallError, diff --git a/src/specify_cli/artifacts/_operation_info.py b/src/specify_cli/artifacts/_operation_info.py index 1e1434b980..9993ec67b2 100644 --- a/src/specify_cli/artifacts/_operation_info.py +++ b/src/specify_cli/artifacts/_operation_info.py @@ -6,6 +6,7 @@ from pathlib import Path from typing import Any, Literal +from .._file_policy import ProjectPolicyError, project_files from ..presets import PresetError from . import ( AmbiguousArtifactError, @@ -508,6 +509,13 @@ def get_artifact_info(request: ArtifactInfoRequest) -> ArtifactInfoResult: if not isinstance(request.identifier, str) or not request.identifier: raise ArtifactInfoIdentifierError(request.identifier) _validate_identifier(request.identifier, request.kind) + try: + if project_directory.is_dir(): + project_files(project_directory) + except ProjectPolicyError as exc: + raise ArtifactInfoResolutionError( + project_directory, request.identifier, message=str(exc), + ) from exc if not (project_directory / ".specify").is_dir(): raise ArtifactInfoProjectError(project_directory) diff --git a/src/specify_cli/artifacts/_operation_list.py b/src/specify_cli/artifacts/_operation_list.py index d699fc365f..91afd43347 100644 --- a/src/specify_cli/artifacts/_operation_list.py +++ b/src/specify_cli/artifacts/_operation_list.py @@ -6,6 +6,7 @@ from pathlib import Path from typing import Any, Literal, TypedDict, cast +from .._file_policy import ProjectPolicyError, project_files from ..presets import PresetError from . import ( ArtifactCatalog, @@ -142,10 +143,12 @@ def __init__(self, project_directory: Path) -> None: class ArtifactListResolutionError(ArtifactListError): """The artifact inventory could not be resolved.""" - def __init__(self, project_directory: Path) -> None: + def __init__( + self, project_directory: Path, *, message: str = "artifact resolution failed", + ) -> None: super().__init__( code="artifact_resolution_failed", - message="artifact resolution failed", + message=message, details={"project_directory": str(project_directory)}, ) @@ -235,7 +238,11 @@ def list_artifacts(request: ArtifactListRequest) -> ArtifactListResult: offset = _pagination_offset(request) try: + if project_directory.is_dir(): + project_files(project_directory) rows = ArtifactCatalog(project_directory).list_artifacts_with_stack() + except ProjectPolicyError as exc: + raise ArtifactListResolutionError(project_directory, message=str(exc)) from exc except NotASpecKitProjectError as exc: raise ArtifactListProjectError(project_directory) from exc except (ArtifactResolutionError, OSError, PresetError) as exc: diff --git a/src/specify_cli/bundles/_commands.py b/src/specify_cli/bundles/_commands.py index 66318831d8..eb8d4a70dd 100644 --- a/src/specify_cli/bundles/_commands.py +++ b/src/specify_cli/bundles/_commands.py @@ -24,8 +24,13 @@ ) -def _fail(message: str) -> None: +def _fail(message: str, *, as_json: bool = False) -> None: """Print an actionable error to stderr and exit non-zero.""" + if as_json: + import json + + typer.echo(json.dumps({"error": message}, ensure_ascii=False), err=True) + raise typer.Exit(code=1) # Use the stderr console so the error never lands on stdout, which under # ``--json`` carries the machine-readable payload and must stay parseable. # Escape the message: every caller passes ``str(exc)`` from a BundlerError @@ -96,6 +101,7 @@ def _run_init(integration: str, *, script_type: str, offline: bool = False) -> N ignore_agent_tools=True, here=True, force=True, + allow_symlinks=False, skip_tls=False, debug=False, github_token=None, diff --git a/src/specify_cli/bundles/command_info.py b/src/specify_cli/bundles/command_info.py index 394990b362..9c5515e888 100644 --- a/src/specify_cli/bundles/command_info.py +++ b/src/specify_cli/bundles/command_info.py @@ -9,6 +9,7 @@ from rich.markup import escape as _escape_markup from .._console import console +from .._file_policy import ProjectPolicyError from . import BundlerError from ._commands import ( _build_stack, @@ -77,7 +78,7 @@ def bundle_info( # unverifiable bundle for a known/installable one. manifest = _download_manifest(resolved, offline=offline) except BundlerError as exc: - _fail(str(exc)) + _fail(str(exc), as_json=as_json and isinstance(exc.__cause__, ProjectPolicyError)) return overlaps = _bundle_overlaps(project_root, manifest, offline=offline) diff --git a/src/specify_cli/bundles/command_list.py b/src/specify_cli/bundles/command_list.py index e6b4da6817..06805d4d48 100644 --- a/src/specify_cli/bundles/command_list.py +++ b/src/specify_cli/bundles/command_list.py @@ -8,6 +8,7 @@ from rich.markup import escape as _escape_markup from .._console import console +from .._file_policy import ProjectPolicyError from . import BundlerError from ._commands import _fail, bundle_app from .project import require_project_root @@ -23,7 +24,7 @@ def bundle_list( project_root = require_project_root() records = load_records(project_root) except BundlerError as exc: - _fail(str(exc)) + _fail(str(exc), as_json=as_json and isinstance(exc.__cause__, ProjectPolicyError)) return if as_json: diff --git a/src/specify_cli/bundles/command_search.py b/src/specify_cli/bundles/command_search.py index 5c62def010..ef3341ff3d 100644 --- a/src/specify_cli/bundles/command_search.py +++ b/src/specify_cli/bundles/command_search.py @@ -9,6 +9,7 @@ from rich.markup import escape as _escape_markup from .._console import console +from .._file_policy import ProjectPolicyError from . import BundlerError from ._commands import bundle_app, _build_stack, _fail, _trust_badge, _trust_level from .project import find_project_root @@ -26,7 +27,7 @@ def bundle_search( stack = _build_stack(project_root, offline=offline) results = stack.search(query) except BundlerError as exc: - _fail(str(exc)) + _fail(str(exc), as_json=as_json and isinstance(exc.__cause__, ProjectPolicyError)) return if as_json: diff --git a/src/specify_cli/bundles/project.py b/src/specify_cli/bundles/project.py index 6838adf3f4..f9e595cb87 100644 --- a/src/specify_cli/bundles/project.py +++ b/src/specify_cli/bundles/project.py @@ -3,7 +3,12 @@ from pathlib import Path -from .._project import _resolve_init_dir_override +from .._project import ( + ProjectResolutionError, + _resolve_init_dir_override_unrendered, + render_project_error, +) +from .._file_policy import ProjectPolicyError, project_files from ..integration_state import clean_integration_key, dedupe_integration_keys from . import BundlerError from .yamlio import ensure_within, load_json @@ -14,9 +19,10 @@ def find_project_root(start: Path | None = None) -> Path | None: """Return the nearest ancestor (incl. *start*) containing a ``.specify/`` dir, or None. - A symlinked ``.specify`` is not accepted as a project root: following it - could read/write outside the intended tree, and other CLI surfaces refuse - it for the same reason. + A present ``.specify`` entry establishes the candidate boundary. Forbidden + links/reparse entries, non-directory markers and corrupt policy fail there + instead of silently selecting an ancestor. Only absent markers continue + ancestor discovery. When *start* is ``None`` the ``SPECIFY_INIT_DIR`` override is honored first (see :func:`specify_cli._project._resolve_init_dir_override`). With an @@ -26,25 +32,39 @@ def find_project_root(start: Path | None = None) -> Path | None: ``bundle init``/``install`` silently fall back to the current directory. """ if start is None: - override = _resolve_init_dir_override() + try: + override = _resolve_init_dir_override_unrendered() + except ProjectResolutionError as exc: + if isinstance(exc.__cause__, ProjectPolicyError): + raise BundlerError(str(exc)) from exc.__cause__ + render_project_error(exc) if override is not None: # An explicit override is strict: do not return None here, because # bundle install treats None as "init the current directory". - if (override / ".specify").is_symlink(): - raise BundlerError( - "SPECIFY_INIT_DIR is not a safe Spec Kit project " - f"(symlinked .specify/ directory is not allowed): {override}" - ) return override current = Path(start or Path.cwd()).resolve() for candidate in (current, *current.parents): marker = candidate / ".specify" - if marker.is_dir() and not marker.is_symlink(): - return candidate + try: + marker.lstat() + except FileNotFoundError: + continue + except OSError as exc: + error = ProjectPolicyError(f"Cannot inspect project marker {marker}: {exc}") + raise BundlerError(str(error)) from error + _preflight_policy(candidate) + return candidate return None +def _preflight_policy(root: Path) -> None: + try: + project_files(root) + except ProjectPolicyError as exc: + raise BundlerError(str(exc)) from exc + + def require_project_root(start: Path | None = None) -> Path: """Return the Spec Kit project root or raise an actionable error. diff --git a/src/specify_cli/command_check.py b/src/specify_cli/command_check.py index cdcc7fd6e2..d595709bb3 100644 --- a/src/specify_cli/command_check.py +++ b/src/specify_cli/command_check.py @@ -44,13 +44,21 @@ def _check_external_tools( def check() -> None: """Check that all required tools are installed.""" from . import check_tool - from ._project import _resolve_init_dir_override + from ._project import ( + ProjectResolutionError, _resolve_init_dir_override, + preflight_project_policy, render_project_error, + ) from .integrations.installer import IntegrationInstallError, load_installed_integrations, read_records try: - project_root = _resolve_init_dir_override() or Path.cwd() + project_root = _resolve_init_dir_override() + if project_root is None: + project_root = Path.cwd() + preflight_project_policy(project_root) load_installed_integrations(project_root) records = read_records(project_root) + except ProjectResolutionError as exc: + render_project_error(exc) except (IntegrationInstallError, OSError) as exc: console.print(f"[red]Error:[/red] {escape(str(exc))}") raise typer.Exit(1) from exc diff --git a/src/specify_cli/command_init.py b/src/specify_cli/command_init.py index 605dd7ed92..cf6e9e1f39 100644 --- a/src/specify_cli/command_init.py +++ b/src/specify_cli/command_init.py @@ -289,6 +289,10 @@ def init( "--force", help="Force merge/overwrite when using --here (skip confirmation)", ), + allow_symlinks: bool = typer.Option( + False, "--allow-symlinks", + help="Persist permission for contained symlinks in migrated FileHelper operations (default: deny).", + ), non_interactive: bool = typer.Option( False, "--non-interactive", @@ -499,7 +503,7 @@ def init( console.print("[yellow]Operation cancelled[/yellow]") raise typer.Exit(0) else: - project_path = Path(project_name).resolve() + project_path = Path(project_name).absolute() original_directory = initial_directory_state(project_path) dir_existed_before = original_directory[0] if original_directory else project_path.exists() if dir_existed_before: @@ -534,6 +538,20 @@ def init( console.print(error_panel) raise typer.Exit(1) + from ._file_policy import ( + ProjectPolicyError, prepare_init_root, read_project_options, + ) + from .file_helper import FileHelperError + + try: + # Existing targets are checked before any integration or scaffold mutation. + previous_init_options = ( + read_project_options(project_path) if dir_existed_before else {} + ) + except ProjectPolicyError as exc: + console.print(f"[red]Error:[/red] {_escape_markup(str(exc))}") + raise typer.Exit(1) from exc + if integration: if integration not in AGENT_CONFIG: console.print( @@ -676,6 +694,12 @@ def init( # hangs when Rich tries to restore cursor state via VT escape sequences. _transient = sys.platform != "win32" + try: + project_path = prepare_init_root(project_path).resolve() + except (OSError, FileHelperError, RuntimeError) as exc: + console.print(f"[red]Error:[/red] Cannot create project root: {_escape_markup(str(exc))}") + raise typer.Exit(1) from exc + with Live( tracker.render(), console=console, refresh_per_second=8, transient=_transient ) as live: @@ -824,17 +848,21 @@ def init( tracker.error("workflow", f"install failed: {sanitized_wf[:120]}") init_opts = { + **previous_init_options, "ai": selected_ai, "integration": resolved_integration.key, "here": here, "script": selected_script, "feature_numbering": "sequential", "speckit_version": get_speckit_version(), + "allow_symlinks": allow_symlinks, } if resolved_integration.is_skills_mode( integration_parsed_options or None, project_root=project_path ): init_opts["ai_skills"] = True + else: + init_opts.pop("ai_skills", None) save_init_options(project_path, init_opts) ensure_executable_scripts(project_path, tracker=tracker) diff --git a/src/specify_cli/events/command_run.py b/src/specify_cli/events/command_run.py index c5bf162b3d..209e13ca56 100644 --- a/src/specify_cli/events/command_run.py +++ b/src/specify_cli/events/command_run.py @@ -48,6 +48,16 @@ def event_run( # Run the event command project_root = Path.cwd() # The agent runs events from project root + from .._project import ( + ProjectResolutionError, + preflight_project_policy, + render_project_error, + ) + + try: + preflight_project_policy(project_root) + except ProjectResolutionError as exc: + render_project_error(exc) exit_code = resolve_and_run_event_command( command_name, event_name, payload, project_root, timeout=timeout ) diff --git a/src/specify_cli/file_helper.py b/src/specify_cli/file_helper.py index ba135135d0..49441ce494 100644 --- a/src/specify_cli/file_helper.py +++ b/src/specify_cli/file_helper.py @@ -1,6 +1,7 @@ """Scoped filesystem operations; see design/file-helper.md for the contract.""" from dataclasses import dataclass, field +import codecs import errno import os from pathlib import Path, PurePath, PureWindowsPath @@ -208,6 +209,20 @@ def write_text( ) -> None: self.write_bytes(path, content.encode(encoding), mode=mode) + def overwrite_text( + self, path: Path | str, content: str, *, encoding: str = "utf-8" + ) -> None: + """Non-atomic text upsert preserving existing inode and native text I/O.""" + destination = self._path(path, allow_missing=True) + if destination.exists(): + self._require_file(destination) + codecs.lookup(encoding) + flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC + flags |= getattr(os, "O_NOFOLLOW", 0) + fd = os.open(destination, flags, 0o666) + with os.fdopen(fd, "w", encoding=encoding) as stream: + stream.write(content) + def symlink(self, target: Path | str, path: Path | str) -> None: """Create a link to an existing contained regular file or directory. diff --git a/src/specify_cli/integrations/_helpers.py b/src/specify_cli/integrations/_helpers.py index 36769c88d8..9bfec2e8c7 100644 --- a/src/specify_cli/integrations/_helpers.py +++ b/src/specify_cli/integrations/_helpers.py @@ -96,8 +96,10 @@ def _write_integration_json( def _refresh_init_options_speckit_version(project_root: Path) -> None: """Refresh only the Spec Kit version recorded in init-options.json.""" - from .. import load_init_options, save_init_options - opts = load_init_options(project_root) + from .. import save_init_options + from .._file_policy import read_project_options + + opts = read_project_options(project_root) if not isinstance(opts, dict) or not opts: return opts["speckit_version"] = _get_speckit_version() @@ -106,11 +108,10 @@ def _refresh_init_options_speckit_version(project_root: Path) -> None: def _clear_init_options_for_integration(project_root: Path, integration_key: str) -> None: """Clear active integration keys from init-options.json when they match.""" - from .. import ( - load_init_options, - save_init_options, - ) - opts = load_init_options(project_root) + from .. import save_init_options + from .._file_policy import read_project_options + + opts = read_project_options(project_root) if opts.get("integration") == integration_key or opts.get("ai") == integration_key: opts.pop("integration", None) opts.pop("ai", None) @@ -273,11 +274,10 @@ def _update_init_options_for_integration( agent-context extension, so this function never touches the extension or its config. """ - from .. import ( - load_init_options, - save_init_options, - ) - opts = load_init_options(project_root) + from .. import save_init_options + from .._file_policy import read_project_options + + opts = read_project_options(project_root) opts["integration"] = integration.key opts["ai"] = integration.key opts["speckit_version"] = _get_speckit_version() diff --git a/src/specify_cli/integrations/_lifecycle.py b/src/specify_cli/integrations/_lifecycle.py index c7d7d78b5f..0b9bd7da01 100644 --- a/src/specify_cli/integrations/_lifecycle.py +++ b/src/specify_cli/integrations/_lifecycle.py @@ -298,6 +298,7 @@ def _restore_snapshots(root: Path, journal: _FileJournal) -> list[Path]: def _transaction( root: Path, target: str | None, expected_state: dict[str, Any] | None, expected_records: dict[str, dict[str, Any]], + *, creation_root: Path | None = None, ): """Journal operation-owned writes under an inter-process integration lock.""" from ..shared_infra import _exclusive_project_lock @@ -353,7 +354,12 @@ def _transaction( try: # Keep the stable project-keyed lock outside an uninitialized target. with _exclusive_project_lock(lock_root, f".integration-install-{lock_id}.lock", context="integration"): - root.mkdir(parents=True, exist_ok=True) + if creation_root is None: + root.mkdir(parents=True, exist_ok=True) + else: + from .._file_policy import prepare_init_root + + prepare_init_root(creation_root) current_state, state_error = try_read_integration_json(root) if state_error or current_state != expected_state or installer.read_records(root) != expected_records: raise installer.IntegrationInstallError( @@ -415,7 +421,8 @@ def invoke(*args, **kwargs): here = values.get("here", False) or project_name == "." if (here and project_name not in (None, ".")) or (not here and not project_name): return handler(*args, **kwargs) - root = Path.cwd() if here else Path(project_name).resolve() + creation_root = Path.cwd() if here else Path(project_name).absolute() + root = creation_root key = values.get("integration") else: from .. import _require_specify_project @@ -425,6 +432,10 @@ def invoke(*args, **kwargs): recovery_token = None recovery_binding = None try: + if operation == "init": + from .._file_policy import preflight_init_root + + root = preflight_init_root(creation_root) records = installer.read_records(root) state, error = try_read_integration_json(root) if error: @@ -473,10 +484,15 @@ def invoke(*args, **kwargs): installer.recovery_exclusion.set(None) if operation == "init": token = _init_directory.set( - (root, root.exists(), list(root.iterdir()) if root.is_dir() else []) + (creation_root, root.exists(), list(root.iterdir()) if root.is_dir() else []) ) stack.callback(_init_directory.reset, token) - with _transaction(root, key, state, records): + transaction = ( + _transaction(root, key, state, records, creation_root=creation_root) + if operation == "init" + else _transaction(root, key, state, records) + ) + with transaction: if recovery_token is not None and recovery_binding is None: from .manifest import IntegrationManifest diff --git a/src/specify_cli/workflows/command_run.py b/src/specify_cli/workflows/command_run.py index a908def637..e3159be0d7 100644 --- a/src/specify_cli/workflows/command_run.py +++ b/src/specify_cli/workflows/command_run.py @@ -34,6 +34,16 @@ def workflow_run( # explicitly names a project, in which case the strict override applies. override = cli._resolve_init_dir_override() project_root = override if override is not None else cli.Path.cwd() + from .._project import ( + ProjectResolutionError, + preflight_project_policy, + render_project_error, + ) + + try: + preflight_project_policy(project_root) + except ProjectResolutionError as exc: + render_project_error(exc) cli._reject_unsafe_workflow_storage(project_root) else: project_root = cli._require_specify_project(load_integrations=False) diff --git a/tests/specify_cli/bundles/test_security_paths.py b/tests/specify_cli/bundles/test_security_paths.py index 9f250a9142..8f75cbe9e7 100644 --- a/tests/specify_cli/bundles/test_security_paths.py +++ b/tests/specify_cli/bundles/test_security_paths.py @@ -324,7 +324,7 @@ def test_load_source_stack_refuses_symlinked_specify_dir(tmp_path: Path): load_source_stack(project) -def test_find_project_root_ignores_symlinked_specify(tmp_path: Path): +def test_find_project_root_rejects_symlinked_specify(tmp_path: Path): from specify_cli.bundles.project import find_project_root real = tmp_path / "real-specify" @@ -335,8 +335,83 @@ def test_find_project_root_ignores_symlinked_specify(tmp_path: Path): (project / ".specify").symlink_to(real, target_is_directory=True) except (OSError, NotImplementedError): pytest.skip("symlinks not supported on this platform") - # A symlinked .specify must not be accepted as a project root. - assert find_project_root(project) is None + # A forbidden present marker must not trigger projectless/ancestor fallback. + with pytest.raises(BundlerError, match="symlink"): + find_project_root(project) + + +@pytest.mark.parametrize("parent_project", [False, True]) +@pytest.mark.parametrize("dangling", [False, True]) +@pytest.mark.parametrize("explicit_start", [False, True]) +def test_nearer_linked_marker_never_falls_back( + tmp_path, monkeypatch, parent_project, dangling, explicit_start, +): + from specify_cli.bundles.project import find_project_root + + if parent_project: + (tmp_path / ".specify").mkdir() + (tmp_path / ".specify/init-options.json").write_text("{}") + member = tmp_path / "member" + member.mkdir() + target = tmp_path / "linked-config" + if not dangling: + target.mkdir() + (target / "init-options.json").write_text('{"allow_symlinks": true}') + try: + (member / ".specify").symlink_to(target, target_is_directory=True) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"Symlinks unavailable: {exc}") + monkeypatch.chdir(member) + monkeypatch.delenv("SPECIFY_INIT_DIR", raising=False) + with pytest.raises(BundlerError, match="symlink"): + find_project_root(member if explicit_start else None) + + +@pytest.mark.parametrize("explicit_start", [False, True]) +def test_absent_nearer_marker_preserves_ancestor_discovery(tmp_path, monkeypatch, explicit_start): + from specify_cli.bundles.project import find_project_root + + (tmp_path / ".specify").mkdir() + member = tmp_path / "member" + member.mkdir() + monkeypatch.chdir(member) + monkeypatch.delenv("SPECIFY_INIT_DIR", raising=False) + assert find_project_root(member if explicit_start else None) == tmp_path.resolve() + + +@pytest.mark.parametrize("explicit_start", [False, True]) +def test_nearer_non_directory_marker_is_a_boundary_error(tmp_path, monkeypatch, explicit_start): + from specify_cli.bundles.project import find_project_root + + (tmp_path / ".specify").mkdir() + member = tmp_path / "member" + member.mkdir() + (member / ".specify").write_text("not a project directory") + monkeypatch.chdir(member) + with pytest.raises(BundlerError, match="init-options.json"): + find_project_root(member if explicit_start else None) + + +def test_nearer_non_symlink_reparse_marker_is_rejected(tmp_path, monkeypatch): + from types import SimpleNamespace + from specify_cli.bundles.project import find_project_root + + (tmp_path / ".specify").mkdir() + member = tmp_path / "member" + member.mkdir() + marker = member / ".specify" + marker.mkdir() + original = Path.lstat + + def reparse(path, *args, **kwargs): + result = original(path, *args, **kwargs) + if path == marker: + return SimpleNamespace(st_mode=result.st_mode, st_file_attributes=0x400) + return result + + monkeypatch.setattr(Path, "lstat", reparse) + with pytest.raises(BundlerError, match="reparse"): + find_project_root(member) def test_find_project_root_override_errors_on_symlinked_specify(tmp_path: Path, monkeypatch): diff --git a/tests/specify_cli/events/test_command_run.py b/tests/specify_cli/events/test_command_run.py index 6782b1dd26..ec04e92424 100644 --- a/tests/specify_cli/events/test_command_run.py +++ b/tests/specify_cli/events/test_command_run.py @@ -15,6 +15,7 @@ from __future__ import annotations +import json from unittest.mock import patch import pytest @@ -143,3 +144,56 @@ def test_event_run_multibyte_payload_enforces_byte_limit(): assert result.exit_code == 1, result.output assert "1 MiB limit" in result.output assert not mock_run.called + + +@pytest.mark.parametrize("options", [None, {}, {"allow_symlinks": False}, {"allow_symlinks": True}]) +def test_event_policy_accepts_missing_or_valid_configuration(tmp_path, monkeypatch, options): + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("SPECIFY_INIT_DIR", str(tmp_path / "unrelated")) + if options is not None: + (tmp_path / ".specify").mkdir() + (tmp_path / ".specify/init-options.json").write_text(json.dumps(options)) + with patch("specify_cli.events.resolve_and_run_event_command", return_value=0) as handler: + result = CliRunner().invoke(app, ["event", "run", "test", "session_start"], input="{}") + assert result.exit_code == 0, result.output + assert handler.call_args[0][3] == tmp_path + + +@pytest.mark.parametrize("document", [ + "{", "[]", "null", '{"allow_symlinks": "true"}', '{"allow_symlinks": 1}', + '{"allow_symlinks": null}', +]) +def test_event_policy_rejects_invalid_configuration_before_handler(tmp_path, monkeypatch, document): + monkeypatch.chdir(tmp_path) + (tmp_path / ".specify").mkdir() + (tmp_path / ".specify/init-options.json").write_text(document) + with patch("specify_cli.events.resolve_and_run_event_command", return_value=0) as handler: + result = CliRunner().invoke(app, ["event", "run", "test", "session_start"], input="{}") + assert result.exit_code == 1 + assert "init-options.json" in result.stderr + assert not handler.called + + +@pytest.mark.parametrize("leaf,dangling", [(False, False), (False, True), (True, False), (True, True)]) +def test_event_policy_denies_bootstrap_links_before_handler(tmp_path, monkeypatch, leaf, dangling): + monkeypatch.chdir(tmp_path) + target = tmp_path / "target" + if leaf: + (tmp_path / ".specify").mkdir() + if not dangling: + target.write_text('{"allow_symlinks": true}') + marker = tmp_path / ".specify/init-options.json" + else: + if not dangling: + target.mkdir() + (target / "init-options.json").write_text('{"allow_symlinks": true}') + marker = tmp_path / ".specify" + try: + marker.symlink_to(target, target_is_directory=not leaf) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"Symlinks unavailable: {exc}") + with patch("specify_cli.events.resolve_and_run_event_command", return_value=0) as handler: + result = CliRunner().invoke(app, ["event", "run", "test", "session_start"], input="{}") + assert result.exit_code == 1 + assert "symlink" in result.stderr + assert not handler.called diff --git a/tests/specify_cli/integrations/test_command_install.py b/tests/specify_cli/integrations/test_command_install.py index a136f6d76e..abbbc2a3ff 100644 --- a/tests/specify_cli/integrations/test_command_install.py +++ b/tests/specify_cli/integrations/test_command_install.py @@ -585,16 +585,9 @@ def test_extension_add_corrupted_init_options_file_fails_closed(self, tmp_path): init_options_path.write_text("{not valid json", encoding="utf-8") result = _run_in_project(project, ["extension", "add", "git"]) - assert result.exit_code == 0, f"extension add failed: {result.output}" - - registry_path = project / ".specify" / "extensions" / ".registry" - registered = json.loads(registry_path.read_text(encoding="utf-8"))[ - "extensions" - ]["git"]["registered_commands"] - assert registered == {}, ( - "a corrupted init-options.json must fail closed, not be " - "treated like a legacy project missing the file entirely (#2948)" - ) + assert result.exit_code == 1, result.output + assert "Invalid JSON" in result.output + assert not (project / ".specify/extensions/git").exists() def test_extension_add_dangling_init_options_symlink_fails_closed(self, tmp_path): """A dangling init-options.json symlink must fail closed too, not be @@ -620,16 +613,10 @@ def test_extension_add_dangling_init_options_symlink_fails_closed(self, tmp_path assert init_options_path.is_symlink() result = _run_in_project(project, ["extension", "add", "git"]) - assert result.exit_code == 0, f"extension add failed: {result.output}" - - registry_path = project / ".specify" / "extensions" / ".registry" - registered = json.loads(registry_path.read_text(encoding="utf-8"))[ - "extensions" - ]["git"]["registered_commands"] - assert registered == {}, ( - "a dangling init-options.json symlink must fail closed, not be " - "treated like a legacy project missing the file entirely (#2948)" - ) + assert result.exit_code == 1, result.output + assert "symlink" in result.output + assert init_options_path.is_symlink() + assert not (project / ".specify/extensions/git").exists() class TestScriptTypeValidation: diff --git a/tests/specify_cli/integrations/test_command_status.py b/tests/specify_cli/integrations/test_command_status.py index d97acf91f0..2a0f1be471 100644 --- a/tests/specify_cli/integrations/test_command_status.py +++ b/tests/specify_cli/integrations/test_command_status.py @@ -394,10 +394,9 @@ def fail_first_project_root_resolve(self, *args, **kwargs): result = _run_in_project(copilot_project, ["integration", "status", "--json"]) - assert result.exit_code == 0 - payload = json.loads(result.output) - assert payload["status"] == "warning" - assert any(item["code"] == "project-root-unresolved" for item in payload["findings"]) + assert result.exit_code == 1 + assert result.stdout == "" + assert "Cannot establish project root" in json.loads(result.stderr)["error"] def test_status_loads_manifests_when_project_root_resolution_keeps_failing( self, @@ -413,10 +412,11 @@ def fail_project_root_resolve(self, *args, **kwargs): monkeypatch.setattr(Path, "resolve", fail_project_root_resolve) - result = _run_in_project(copilot_project, ["integration", "status", "--json"]) + from specify_cli.integration_status import build_integration_status_report - assert result.exit_code == 0, result.output - payload = json.loads(result.output) + # The tolerant domain diagnostic remains available; CLI policy preflight + # now requires an established root before reaching this diagnostic. + payload = build_integration_status_report(copilot_project) assert payload["status"] == "warning" assert payload["manifests"]["copilot"]["readable"] is True assert payload["manifests"]["speckit"]["readable"] is True diff --git a/tests/specify_cli/test_file_policy.py b/tests/specify_cli/test_file_policy.py new file mode 100644 index 0000000000..ac69823049 --- /dev/null +++ b/tests/specify_cli/test_file_policy.py @@ -0,0 +1,763 @@ +"""Strict project policy and configuration bootstrap contracts.""" + +import asyncio +import json +import os +from pathlib import Path +from unittest.mock import Mock, patch + +import pytest +import typer +from typer.testing import CliRunner + +from specify_cli import app +from specify_cli.file_helper import FileHelper + + +def policy_module(): + from specify_cli import _file_policy + + return _file_policy + + +@pytest.fixture +def symlinks(tmp_path): + probe = tmp_path / "probe" + try: + probe.symlink_to(tmp_path, target_is_directory=True) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"Symlinks unavailable: {exc}") + probe.unlink() + + +@pytest.mark.parametrize("options,allowed", [({}, False), ({"allow_symlinks": False}, False), ({"allow_symlinks": True}, True)]) +def test_policy_is_literal_and_fresh(tmp_path, options, allowed): + policy = policy_module() + assert policy.project_files(tmp_path).allow_symlinks is False + policy.save_project_options(tmp_path, options) + files = policy.project_files(tmp_path) + assert files.allow_symlinks is allowed + policy.save_project_options(tmp_path, {"allow_symlinks": not allowed}) + assert policy.project_files(tmp_path).allow_symlinks is not allowed + assert files.allow_symlinks is allowed + + +@pytest.mark.parametrize("value", ["true", "false", 0, 1, None, [], {}]) +def test_invalid_policy_values(tmp_path, value): + path = tmp_path / ".specify" + path.mkdir() + (path / "init-options.json").write_text(json.dumps({"allow_symlinks": value})) + with pytest.raises(policy_module().ProjectPolicyError, match="boolean"): + policy_module().project_files(tmp_path) + + +@pytest.mark.parametrize("document", [ + "{", "[]", "null", '"text"', "true", "1", "\xff", + '{"custom": NaN}', '{"custom": Infinity}', '{"custom": -Infinity}', +]) +def test_invalid_documents(tmp_path, document): + path = tmp_path / ".specify" + path.mkdir() + (path / "init-options.json").write_bytes(document.encode("latin1")) + with pytest.raises(policy_module().ProjectPolicyError, match="init-options.json"): + policy_module().project_files(tmp_path) + + +@pytest.mark.parametrize("leaf,dangling", [(False, False), (False, True), (True, False), (True, True)]) +def test_bootstrap_links_always_denied(tmp_path, symlinks, leaf, dangling): + policy = policy_module() + target = tmp_path / "target" + if leaf: + (tmp_path / ".specify").mkdir() + if not dangling: + target.write_text('{"allow_symlinks": true}') + (tmp_path / ".specify/init-options.json").symlink_to(target) + else: + if not dangling: + target.mkdir() + (target / "init-options.json").write_text('{"allow_symlinks": true}') + (tmp_path / ".specify").symlink_to(target, target_is_directory=True) + with pytest.raises(policy.ProjectPolicyError, match="symlink"): + policy.project_files(tmp_path) + with pytest.raises(policy.ProjectPolicyError, match="symlink"): + policy.save_project_options(tmp_path, {"allow_symlinks": True}) + + +def test_permission_failure_is_not_missing(tmp_path, monkeypatch): + def denied(*args, **kwargs): + raise PermissionError("permission denied") + + monkeypatch.setattr(FileHelper, "read_text", denied) + with pytest.raises(policy_module().ProjectPolicyError, match="permission denied"): + policy_module().project_files(tmp_path) + + +def test_configuration_directory_is_not_a_document(tmp_path): + (tmp_path / ".specify/init-options.json").mkdir(parents=True) + with pytest.raises(policy_module().ProjectPolicyError): + policy_module().project_files(tmp_path) + + +def test_policy_root_alias_is_trusted(tmp_path, symlinks): + root = tmp_path / "project" + root.mkdir() + alias = tmp_path / "alias" + alias.symlink_to(root, target_is_directory=True) + policy_module().save_project_options(alias, {"allow_symlinks": True}) + assert policy_module().project_files(alias).allow_symlinks is True + + +def test_project_commands_recheck_edits_and_render_json(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + config = tmp_path / ".specify/init-options.json" + config.parent.mkdir() + runner = CliRunner() + config.write_text('{"allow_symlinks": false}') + assert runner.invoke(app, ["extension", "list", "--json"]).exit_code == 0 + config.write_text('{"allow_symlinks": "true"}') + result = runner.invoke(app, ["extension", "list", "--json"]) + assert result.exit_code == 1 + assert result.stdout == "" + assert "boolean" in json.loads(result.stderr)["error"] + config.write_text('{"allow_symlinks": true}') + assert runner.invoke(app, ["extension", "list", "--json"]).exit_code == 0 + + +@pytest.mark.parametrize("args", [["--help"], ["--version"], ["version", "--json"], ["init", "--help"]]) +def test_projectless_commands_ignore_unrelated_invalid_policy(tmp_path, monkeypatch, args): + monkeypatch.chdir(tmp_path) + config = tmp_path / ".specify/init-options.json" + config.parent.mkdir() + config.write_text("{") + result = CliRunner().invoke(app, args) + assert "init-options.json" not in result.output + assert result.exit_code == 0 + + +@pytest.mark.parametrize("allowed", [False, True]) +def test_init_persists_policy_and_preserves_unrelated_options(tmp_path, monkeypatch, allowed): + monkeypatch.chdir(tmp_path) + config = tmp_path / ".specify/init-options.json" + config.parent.mkdir() + config.write_text('{"custom": "retained", "allow_symlinks": true}') + args = ["init", "--here", "--force", "--non-interactive", "--integration", "generic", + "--integration-options=--commands-dir .agent/commands", "--ignore-agent-tools"] + if allowed: + args.append("--allow-symlinks") + result = CliRunner().invoke(app, args) + assert result.exit_code == 0, result.output + options = json.loads(config.read_text()) + assert options["allow_symlinks"] is allowed + assert options["custom"] == "retained" + + +@pytest.mark.parametrize("args", [["--allow-symlinks"], ["extension", "list", "--allow-symlinks"], ["workflow", "run", "x", "--allow-symlinks"]]) +def test_option_is_init_only(args): + result = CliRunner().invoke(app, args) + assert result.exit_code == 2 + assert "No such option" in result.output + + +def test_init_rejects_policy_before_scaffolding(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + config = tmp_path / ".specify/init-options.json" + config.parent.mkdir() + config.write_text("{") + before = set(tmp_path.iterdir()) + result = CliRunner().invoke(app, ["init", "--here", "--force", "--allow-symlinks", "--non-interactive"]) + assert result.exit_code == 1 + assert "init-options.json" in result.output + assert set(tmp_path.iterdir()) == before + + +@pytest.mark.parametrize("args", [ + ["extension", "list", "--json"], ["preset", "list", "--json"], + ["artifact", "list", "--json"], ["artifact", "info", "speckit.plan", "--json"], + ["integration", "status", "--json"], + ["workflow", "status", "--json"], ["workflow", "resume", "missing", "--json"], + ["workflow", "run", "missing", "--json"], + ["bundle", "list", "--json"], ["bundle", "search", "--offline", "--json"], + ["bundle", "info", "missing", "--offline", "--json"], +]) +def test_policy_failures_use_json_error_surfaces(tmp_path, monkeypatch, args): + monkeypatch.chdir(tmp_path) + (tmp_path / ".specify").mkdir() + (tmp_path / ".specify/init-options.json").write_text("{") + result = CliRunner().invoke(app, args) + assert result.exit_code == 1, result.exception + assert result.stdout == "" + assert "init-options.json" in json.loads(result.stderr)["error"] + + +def test_override_checks_selected_project_only(tmp_path, monkeypatch): + project = tmp_path / "member" + project.mkdir() + policy_module().save_project_options(project, {"allow_symlinks": True}) + policy_module().save_project_options(tmp_path, {}) + (tmp_path / ".specify/init-options.json").write_text("{") + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("SPECIFY_INIT_DIR", "member") + from specify_cli._project import resolve_specify_project_files + + assert resolve_specify_project_files().root == project.resolve() + assert resolve_specify_project_files().allow_symlinks is True + assert CliRunner().invoke(app, ["extension", "list", "--json"]).exit_code == 0 + (project / ".specify/init-options.json").write_text('{"allow_symlinks": null}') + result = CliRunner().invoke(app, ["extension", "list", "--json"]) + assert result.exit_code == 1 + assert "boolean" in json.loads(result.stderr)["error"] + + +def test_no_new_ancestor_discovery(tmp_path, monkeypatch): + policy_module().save_project_options(tmp_path, {"allow_symlinks": True}) + nested = tmp_path / "nested" + nested.mkdir() + monkeypatch.chdir(nested) + monkeypatch.delenv("SPECIFY_INIT_DIR", raising=False) + result = CliRunner().invoke(app, ["extension", "list", "--json"]) + assert result.exit_code == 1 + assert "Not a Spec Kit project" in json.loads(result.stderr)["error"] + + +def test_init_explicit_target_ignores_override_and_cwd_policy(tmp_path, monkeypatch): + policy_module().save_project_options(tmp_path, {}) + (tmp_path / ".specify/init-options.json").write_text("{") + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("SPECIFY_INIT_DIR", str(tmp_path / "nonexistent")) + result = CliRunner().invoke(app, [ + "init", "nested/project", "--allow-symlinks", "--non-interactive", + "--integration", "generic", "--integration-options=--commands-dir .agent/commands", + "--ignore-agent-tools", + ]) + assert result.exit_code == 0, result.output + assert policy_module().project_files(tmp_path / "nested/project").allow_symlinks is True + + +def test_direct_workflow_preflights_policy_without_requiring_project(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + workflow = tmp_path / "local.yml" + workflow.write_text( + "workflow:\n id: local\n name: Local\n version: 1.0.0\n" + "steps:\n - id: done\n type: shell\n run: echo done\n" + ) + policy_module().save_project_options(tmp_path, {}) + (tmp_path / ".specify/init-options.json").write_text("{") + result = CliRunner().invoke(app, ["workflow", "run", str(workflow), "--json"]) + assert result.exit_code == 1 + assert result.stdout == "" + assert "init-options.json" in json.loads(result.stderr)["error"] + (tmp_path / ".specify/init-options.json").unlink() + (tmp_path / ".specify").rmdir() + result = CliRunner().invoke(app, ["workflow", "run", str(workflow), "--json"]) + assert result.exit_code == 0, result.output + + +def test_configuration_write_preserves_permissions_inode_and_hardlinks(tmp_path): + policy_module().save_project_options(tmp_path, {"custom": "caf\u00e9"}) + config = tmp_path / ".specify/init-options.json" + before = config.stat().st_ino + config.chmod(0o600) + os.link(config, tmp_path / "hardlink") + policy_module().save_project_options(tmp_path, {"custom": "retained", "allow_symlinks": True}) + assert config.stat().st_ino == before + assert json.loads((tmp_path / "hardlink").read_text())["allow_symlinks"] is True + if os.name != "nt": + assert config.stat().st_mode & 0o777 == 0o600 + + +def test_write_permission_errors_are_explicit(tmp_path, monkeypatch): + def denied(*args, **kwargs): + raise PermissionError("permission denied") + + monkeypatch.setattr(FileHelper, "overwrite_text", denied) + with pytest.raises(policy_module().ProjectPolicyError, match="permission denied"): + policy_module().save_project_options(tmp_path, {"allow_symlinks": False}) + + +@pytest.mark.parametrize("command", [["bundle", "list", "--json"], ["bundle", "init"]]) +def test_bundle_resolved_project_does_not_ignore_corrupt_policy(tmp_path, monkeypatch, command): + policy_module().save_project_options(tmp_path, {}) + (tmp_path / ".specify/init-options.json").write_text("{") + nested = tmp_path / "nested" + nested.mkdir() + monkeypatch.chdir(nested) + result = CliRunner().invoke(app, command) + assert result.exit_code == 1 + assert result.stdout == "" + assert "init-options.json" in result.stderr + + +def test_integration_updates_preserve_policy(tmp_path): + from specify_cli.integrations import get_integration + from specify_cli.integrations._helpers import ( + _update_init_options_for_integration, _clear_init_options_for_integration, + _refresh_init_options_speckit_version, + ) + + policy_module().save_project_options(tmp_path, {"allow_symlinks": True, "custom": "retained"}) + _update_init_options_for_integration(tmp_path, get_integration("copilot"), script_type="sh") + _refresh_init_options_speckit_version(tmp_path) + _clear_init_options_for_integration(tmp_path, "copilot") + options = policy_module().read_project_options(tmp_path) + assert options["allow_symlinks"] is True + assert options["custom"] == "retained" + assert "ai" not in options + + +@pytest.mark.parametrize("leaf", [False, True]) +def test_init_flag_cannot_authorize_bootstrap_links(tmp_path, monkeypatch, symlinks, leaf): + monkeypatch.chdir(tmp_path) + if leaf: + (tmp_path / ".specify").mkdir() + (tmp_path / ".specify/init-options.json").symlink_to(tmp_path / "missing") + else: + (tmp_path / ".specify").symlink_to(tmp_path / "missing", target_is_directory=True) + result = CliRunner().invoke(app, [ + "init", "--here", "--force", "--allow-symlinks", "--non-interactive", + "--integration", "copilot", "--ignore-agent-tools", + ]) + assert result.exit_code == 1 + assert "symlink" in result.output + assert not (tmp_path / ".github").exists() + assert not (tmp_path / "missing").exists() + + +def test_new_configuration_respects_umask(tmp_path): + previous = os.umask(0o077) + try: + policy_module().save_project_options(tmp_path, {}) + finally: + os.umask(previous) + if os.name != "nt": + assert (tmp_path / ".specify/init-options.json").stat().st_mode & 0o777 == 0o600 + + +@pytest.mark.parametrize("value", ["false", 1, None]) +def test_invalid_write_policy_fails_before_creation(tmp_path, value): + with pytest.raises(policy_module().ProjectPolicyError, match="boolean"): + policy_module().save_project_options(tmp_path, {"allow_symlinks": value}) + assert not (tmp_path / ".specify").exists() + + +@pytest.mark.parametrize("value", [float("nan"), object()]) +def test_unserializable_options_fail_before_creation(tmp_path, value): + with pytest.raises(policy_module().ProjectPolicyError, match="Cannot encode"): + policy_module().save_project_options(tmp_path, {"custom": value}) + assert not (tmp_path / ".specify").exists() + + +@pytest.mark.parametrize("args", [["bundle", "search", "--offline"], ["bundle", "info", "nonexistent", "--offline"]]) +def test_projectless_catalog_commands_do_not_require_policy(tmp_path, monkeypatch, args): + monkeypatch.chdir(tmp_path) + result = CliRunner().invoke(app, args) + assert "init-options.json" not in result.output + assert "Not a Spec Kit project" not in result.output + assert result.exception is None or isinstance(result.exception, SystemExit) + + +@pytest.mark.parametrize("args", [ + ["bundle", "init"], ["bundle", "install", "missing", "--offline"], + ["bundle", "list"], ["bundle", "list", "--json"], + ["bundle", "search", "--offline"], ["bundle", "search", "--offline", "--json"], + ["bundle", "info", "missing", "--offline"], + ["bundle", "info", "missing", "--offline", "--json"], +]) +@pytest.mark.parametrize("parent_project", [False, True]) +@pytest.mark.parametrize("dangling", [False, True]) +def test_bundle_forbidden_marker_fails_without_initialization_or_install( + tmp_path, monkeypatch, symlinks, args, parent_project, dangling, +): + from unittest.mock import patch + + if parent_project: + policy_module().save_project_options(tmp_path, {}) + member = tmp_path / "member" + member.mkdir() + target = tmp_path / "linked-config" + if not dangling: + target.mkdir() + (target / "init-options.json").write_text('{"allow_symlinks": true}') + (member / ".specify").symlink_to(target, target_is_directory=True) + monkeypatch.chdir(member) + monkeypatch.delenv("SPECIFY_INIT_DIR", raising=False) + before = set(tmp_path.rglob("*")) + with ( + patch("specify_cli.bundles.command_init._run_init") as init, + patch("specify_cli.bundles.command_install._run_init") as install_init, + patch("specify_cli.bundles.installer.install_bundle") as install, + ): + result = CliRunner().invoke(app, args) + assert result.exit_code == 1, result.output + assert result.stdout == "" + if "--json" in args: + assert "symlink" in json.loads(result.stderr)["error"] + else: + assert "symlink" in result.stderr + assert not init.called and not install_init.called and not install.called + assert set(tmp_path.rglob("*")) == before + + +def _configuration_case(root, case): + marker = root / ".specify" + marker.mkdir() + config = marker / "init-options.json" + if case == "marker-link": + target = root / "linked-config" + marker.rename(target) + (target / "init-options.json").write_text('{"allow_symlinks": true}') + marker.symlink_to(target, target_is_directory=True) + elif case in {"leaf-link", "dangling-link"}: + target = root / "linked-options.json" + if case == "leaf-link": + target.write_text('{"allow_symlinks": true}') + config.symlink_to(target) + else: + config.write_text(case) + + +@pytest.mark.parametrize("case", [ + "{", '{"allow_symlinks": 1}', "[]", "marker-link", "leaf-link", "dangling-link", +]) +@pytest.mark.parametrize("explicit", [False, True]) +def test_init_lifecycle_policy_precedes_preparation(tmp_path, monkeypatch, symlinks, case, explicit): + from specify_cli.integrations import _lifecycle, installer + + root = tmp_path / "target" if explicit else tmp_path + root.mkdir(exist_ok=True) + _configuration_case(root, case) + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("SPECIFY_INIT_DIR", str(tmp_path / "unrelated")) + handler = Mock() + + def init_handler(project_name=None, here=False, integration=None): + handler() + + invoke = _lifecycle.external_lifecycle("init")(init_handler) + with ( + patch.object(installer, "read_records", return_value={}) as records, + patch.object(installer, "load_installed_integrations") as loader, + patch.object(installer, "catalog_package") as catalog, + patch.object(_lifecycle, "_transaction") as transaction, + pytest.raises(typer.Exit) as failure, + ): + invoke(project_name=str(root) if explicit else None, here=not explicit, integration="generic") + assert failure.value.exit_code == 1 + handler.assert_not_called() + records.assert_not_called() + loader.assert_not_called() + catalog.assert_not_called() + transaction.assert_not_called() + + +@pytest.mark.parametrize("options", [None, {}, {"allow_symlinks": False}, {"allow_symlinks": True}]) +def test_init_lifecycle_selects_explicit_target(tmp_path, monkeypatch, options): + from specify_cli.integrations import _lifecycle, installer + + root = tmp_path / "target" + root.mkdir() + if options is not None: + policy_module().save_project_options(root, options) + _configuration_case(tmp_path, "{") + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("SPECIFY_INIT_DIR", str(tmp_path / "unrelated")) + seen = [] + + @_lifecycle.external_lifecycle("init") + def handler(project_name=None, integration=None): + seen.append(project_name) + + with ( + patch.object(installer, "read_records", return_value={}) as records, + patch.object(installer, "load_installed_integrations") as loader, + ): + handler(project_name=str(root), integration="generic") + assert seen == [str(root)] + assert all(call.args == (root.resolve(),) for call in records.call_args_list) + assert all(call.args == (root.resolve(),) for call in loader.call_args_list) + + +@pytest.mark.parametrize("existing", [False, True]) +def test_policy_write_rolls_back_with_init_transaction(tmp_path, existing): + from specify_cli.integrations import _lifecycle + + root = tmp_path / "target" + original = {"allow_symlinks": True, "custom": "retained"} + if existing: + root.mkdir() + policy_module().save_project_options(root, original) + with pytest.raises(RuntimeError, match="downstream init failed"): + with _lifecycle._transaction(root, "generic", None, {}): + policy_module().save_project_options(root, {"allow_symlinks": False}) + raise RuntimeError("downstream init failed") + if existing: + assert policy_module().read_project_options(root) == original + else: + assert not root.exists() + + +@pytest.mark.parametrize("case", ["marker-link", "leaf-link", "dangling-link"]) +def test_configuration_denial_precedes_observer_snapshot(tmp_path, symlinks, case): + from specify_cli.integrations._file_changes import file_change_observer + + _configuration_case(tmp_path, case) + observer = Mock() + token = file_change_observer.set(observer) + try: + with pytest.raises(policy_module().ProjectPolicyError, match="symlink"): + policy_module().save_project_options(tmp_path, {"allow_symlinks": True}) + finally: + file_change_observer.reset(token) + observer.assert_not_called() + + +def test_configuration_observer_surrounds_actual_write(tmp_path): + from specify_cli.integrations._file_changes import file_change_observer + + original = {"allow_symlinks": True, "custom": "retained"} + replacement = {"allow_symlinks": False, "custom": "retained"} + policy_module().save_project_options(tmp_path, original) + seen = [] + + def observe(path, before, removal=False): + seen.append((path, before, json.loads(path.read_text()))) + + token = file_change_observer.set(observe) + try: + policy_module().save_project_options(tmp_path, replacement) + finally: + file_change_observer.reset(token) + destination = tmp_path / ".specify/init-options.json" + assert seen == [(destination, True, original), (destination, False, replacement)] + + +def test_init_lifecycle_unreadable_policy_precedes_project_work(tmp_path, monkeypatch): + from specify_cli.integrations import _lifecycle, installer + + policy_module().save_project_options(tmp_path, {}) + + def denied(*args, **kwargs): + raise PermissionError("configuration permission denied") + + monkeypatch.setattr(FileHelper, "read_bytes", denied) + called = Mock() + + @_lifecycle.external_lifecycle("init") + def handler(project_name=None, integration=None): + called() + + with ( + patch.object(installer, "read_records", return_value={}) as records, + patch.object(installer, "load_installed_integrations") as loader, + pytest.raises(typer.Exit) as failure, + ): + handler(project_name=str(tmp_path), integration="generic") + assert failure.value.exit_code == 1 + assert "permission denied" in str(failure.value.__cause__) + records.assert_not_called() + loader.assert_not_called() + called.assert_not_called() + + +@pytest.mark.parametrize("alias", [False, True]) +def test_init_transaction_retains_requested_spelling_and_initial_state(tmp_path, monkeypatch, symlinks, alias): + from specify_cli.integrations import _lifecycle, installer + + parent = tmp_path / "parent" + parent.mkdir() + requested_parent = tmp_path / "alias" if alias else parent + if alias: + requested_parent.symlink_to(parent, target_is_directory=True) + requested = requested_parent / "new" + prepared = [] + original_prepare = policy_module().prepare_init_root + + def prepare(target): + prepared.append(target) + return original_prepare(target) + + monkeypatch.setattr(policy_module(), "prepare_init_root", prepare) + monkeypatch.setattr(installer, "read_records", lambda root: {"generic": {}}) + monkeypatch.setattr(installer, "load_installed_integrations", lambda root: None) + + @_lifecycle.external_lifecycle("init") + def handler(project_name=None, integration=None): + assert _lifecycle.initial_directory_state(Path(project_name).absolute()) == (False, []) + policy_module().save_project_options(Path(project_name).resolve(), {"allow_symlinks": True}) + raise RuntimeError("downstream init failed") + + with pytest.raises(typer.Exit) as failure: + handler(project_name=str(requested), integration="generic") + assert failure.value.exit_code == 1 + assert prepared == [requested] + assert not requested.exists() + + +def test_init_transaction_does_not_resolve_away_dangling_parent(tmp_path, monkeypatch, symlinks): + from specify_cli.integrations import _lifecycle, installer + + missing = tmp_path / "missing-parent" + alias = tmp_path / "alias" + alias.symlink_to(missing, target_is_directory=True) + monkeypatch.setattr(installer, "read_records", lambda root: {"generic": {}}) + monkeypatch.setattr(installer, "load_installed_integrations", lambda root: None) + called = [] + + @_lifecycle.external_lifecycle("init") + def handler(project_name=None, integration=None): + called.append(project_name) + raise RuntimeError("must not invoke handler") + + with pytest.raises(typer.Exit): + handler(project_name=str(alias / "new"), integration="generic") + assert called == [] + assert not missing.exists() + + +@pytest.mark.parametrize("case", ["dangling-parent", "missing-parent-traversal"]) +def test_new_init_target_preserves_evidence_before_project_reads(tmp_path, symlinks, case): + from specify_cli.integrations import _lifecycle, installer + + unrelated = tmp_path / "unrelated" + unrelated.mkdir() + policy_module().save_project_options(unrelated, {"allow_symlinks": True}) + if case == "dangling-parent": + alias = tmp_path / "alias" + alias.symlink_to(tmp_path / "missing", target_is_directory=True) + requested = alias / "new" + else: + requested = tmp_path / "missing" / ".." / "unrelated" + called = Mock() + + @_lifecycle.external_lifecycle("init") + def handler(project_name=None, integration=None): + called() + + with ( + patch.object(installer, "read_records", return_value={}) as records, + patch.object(installer, "load_installed_integrations") as loader, + patch.object(installer, "catalog_package") as catalog, + pytest.raises(typer.Exit), + ): + handler(project_name=str(requested), integration="generic") + records.assert_not_called() + loader.assert_not_called() + catalog.assert_not_called() + called.assert_not_called() + assert not (tmp_path / "missing").exists() + + +@pytest.mark.parametrize("override", [False, True]) +@pytest.mark.parametrize("document", [None, "{}", '{"allow_symlinks": true}', '{"allow_symlinks": false}']) +def test_check_policy_optional_project_and_override(tmp_path, monkeypatch, override, document): + root = tmp_path / "selected" if override else tmp_path + root.mkdir(exist_ok=True) + monkeypatch.chdir(tmp_path) + monkeypatch.delenv("SPECIFY_INIT_DIR", raising=False) + if override: + _configuration_case(tmp_path, "{") + monkeypatch.setenv("SPECIFY_INIT_DIR", "selected") + (root / ".specify").mkdir() + if document is not None: + (root / ".specify").mkdir(exist_ok=True) + (root / ".specify/init-options.json").write_text(document) + with ( + patch("specify_cli.integrations.installer.load_installed_integrations") as loader, + patch("specify_cli.integrations.installer.read_records", return_value={}) as records, + patch("specify_cli.check_tool", return_value=True) as tools, + ): + result = CliRunner().invoke(app, ["check"]) + assert result.exit_code == 0, result.output + loader.assert_called_once_with(root) + records.assert_called_once_with(root) + assert tools.called + + +@pytest.mark.parametrize("override", [False, True]) +@pytest.mark.parametrize("case", ["{", '{"allow_symlinks": null}', "marker-link", "leaf-link", "dangling-link"]) +def test_check_policy_failure_precedes_adapters_and_tools(tmp_path, monkeypatch, symlinks, override, case): + root = tmp_path / "selected" if override else tmp_path + root.mkdir(exist_ok=True) + _configuration_case(root, case) + monkeypatch.chdir(tmp_path) + monkeypatch.delenv("SPECIFY_INIT_DIR", raising=False) + if override: + monkeypatch.setenv("SPECIFY_INIT_DIR", "selected") + with ( + patch("specify_cli.integrations.installer.load_installed_integrations") as loader, + patch("specify_cli.integrations.installer.read_records") as records, + patch("specify_cli.check_tool") as tools, + ): + result = CliRunner().invoke(app, ["check"]) + assert result.exit_code == 1, result.output + assert "init-options.json" in result.output + loader.assert_not_called() + records.assert_not_called() + tools.assert_not_called() + + +@pytest.mark.parametrize("operation", ["list", "info"]) +@pytest.mark.parametrize("case", ["{", '{"allow_symlinks": "true"}', "marker-link", "leaf-link", "dangling-link"]) +def test_artifact_operation_policy_failure_precedes_catalog(tmp_path, monkeypatch, symlinks, operation, case): + from specify_cli.artifacts import _operation_info, _operation_list + + _configuration_case(tmp_path, case) + catalog = Mock() + module = _operation_list if operation == "list" else _operation_info + monkeypatch.setattr(module, "ArtifactCatalog", catalog) + if operation == "list": + request = module.ArtifactListRequest(tmp_path) + call = module.list_artifacts + expected = module.ArtifactListResolutionError + else: + request = module.ArtifactInfoRequest(tmp_path, "speckit.plan") + call = module.get_artifact_info + expected = module.ArtifactInfoResolutionError + with pytest.raises(expected, match="init-options.json") as failure: + call(request) + assert failure.value.code == "artifact_resolution_failed" + catalog.assert_not_called() + + +@pytest.mark.parametrize("operation", ["list", "info"]) +@pytest.mark.parametrize("options", [None, {}, {"allow_symlinks": False}, {"allow_symlinks": True}]) +def test_artifact_operations_reload_valid_policy(tmp_path, operation, options): + from specify_cli.artifacts import _operation_info, _operation_list + + for folder in ["templates", "presets", "extensions"]: + (tmp_path / ".specify" / folder).mkdir(parents=True, exist_ok=True) + config = tmp_path / ".specify/init-options.json" + if options is None: + config.unlink(missing_ok=True) + else: + policy_module().save_project_options(tmp_path, options) + if operation == "list": + request = _operation_list.ArtifactListRequest(tmp_path) + call = _operation_list.list_artifacts + expected = _operation_list.ArtifactListResolutionError + else: + request = _operation_info.ArtifactInfoRequest(tmp_path, "speckit.plan") + call = _operation_info.get_artifact_info + expected = _operation_info.ArtifactInfoResolutionError + assert call(request) is not None + config.write_text("{") + with pytest.raises(expected, match="init-options.json"): + call(request) + + +@pytest.mark.parametrize("operation", ["list", "info"]) +@pytest.mark.parametrize("case", ["{", '{"allow_symlinks": 0}', "marker-link", "leaf-link", "dangling-link"]) +def test_artifact_mcp_policy_errors_keep_transport_contract(tmp_path, symlinks, operation, case): + from specify_cli.mcp_server.server import create_server + + _configuration_case(tmp_path, case) + arguments = {"identifier": "speckit.plan"} if operation == "info" else {} + result = asyncio.run(create_server(launch_directory=tmp_path).call_tool( + f"specify_artifact_{operation}", arguments, + )) + assert result.is_error is True + error = result.structured_content["error"] + assert error["code"] == "artifact_resolution_failed" + assert error["retryable"] is False + assert error["details"]["project_directory"] == str(tmp_path) + assert "init-options.json" in error["message"] + assert "Traceback" not in str(result) diff --git a/tests/test_file_helper.py b/tests/test_file_helper.py index c7da7239b9..a04b9e0a34 100644 --- a/tests/test_file_helper.py +++ b/tests/test_file_helper.py @@ -60,6 +60,110 @@ def test_plain_operations(root, allow): assert list(root.iterdir()) == [] +@pytest.mark.parametrize("allow", [False, True]) +def test_overwrite_text_preserves_inode_and_native_text_behavior(root, allow): + files = FileHelper(root, allow_symlinks=allow) + files.overwrite_text("config", "caf\u00e9\n") + path = root / "config" + inode = path.stat().st_ino + assert path.read_bytes() == ("caf\u00e9" + os.linesep).encode("utf-8") + path.chmod(0o600) + files.overwrite_text("config", "updated\n") + assert path.stat().st_ino == inode + assert path.read_text() == "updated\n" + if os.name != "nt": + assert path.stat().st_mode & 0o777 == 0o600 + + +@pytest.mark.parametrize("kind", ["leaf", "parent", "dangling"]) +def test_overwrite_text_deny_preflights_links(root, link, kind): + target = root / "target" + target.mkdir() + (target / "file").write_text("old") + if kind == "parent": + path = link(root / "alias", target) / "file" + else: + path = link(root / "alias", target / ("missing" if kind == "dangling" else "file")) + with pytest.raises(SymlinkDeniedError): + FileHelper(root).overwrite_text(path, "new") + assert (target / "file").read_text() == "old" + assert not (target / "missing").exists() + + +def test_overwrite_text_allow_updates_target_not_link(root, link): + target = root / "target" + target.write_text("old") + alias = link(root / "alias", target) + chain = link(root / "chain", "alias") + FileHelper(root, allow_symlinks=True).overwrite_text(chain, "new") + assert alias.is_symlink() and chain.is_symlink() + assert target.read_text() == "new" + + +@pytest.mark.parametrize("allow", [False, True]) +@pytest.mark.parametrize("kind", ["dangling", "external", "cycle"]) +def test_overwrite_text_rejects_unsafe_link_targets(root, link, allow, kind): + outside = root.parent / "outside" + outside.write_text("unchanged") + if kind == "external": + target = outside + elif kind == "cycle": + target = root / "alias" + else: + target = root / "missing" + alias = link(root / "alias", target) + expected = (FileHelperError, FileNotFoundError) + with pytest.raises(expected): + FileHelper(root, allow_symlinks=allow).overwrite_text(alias, "new") + assert outside.read_text() == "unchanged" + assert not (root / "missing").exists() + assert alias.is_symlink() + + +@pytest.mark.parametrize("allow", [False, True]) +def test_overwrite_text_through_contained_parent(root, link, allow): + (root / "directory").mkdir() + alias = link(root / "alias", root / "directory") + files = FileHelper(root, allow_symlinks=allow) + if allow: + files.overwrite_text(alias / "new", "content") + assert (root / "directory/new").read_text() == "content" + else: + with pytest.raises(SymlinkDeniedError): + files.overwrite_text(alias / "new", "content") + assert not (root / "directory/new").exists() + + +@pytest.mark.parametrize("allow", [False, True]) +def test_overwrite_text_encoding_errors_propagate(root, allow): + files = FileHelper(root, allow_symlinks=allow) + with pytest.raises(LookupError): + files.overwrite_text("new", "text", encoding="invalid-encoding") + assert not (root / "new").exists() + with pytest.raises(UnicodeEncodeError): + files.overwrite_text("new", "caf\u00e9", encoding="ascii") + + +@pytest.mark.parametrize("allow", [False, True]) +def test_overwrite_text_io_errors_propagate(root, monkeypatch, allow): + def denied(*args, **kwargs): + raise PermissionError("denied") + + monkeypatch.setattr("specify_cli.file_helper.os.open", denied) + with pytest.raises(PermissionError, match="denied"): + FileHelper(root, allow_symlinks=allow).overwrite_text("new", "text") + assert not (root / "new").exists() + + +@pytest.mark.parametrize("path", ["../escape", "directory"]) +@pytest.mark.parametrize("allow", [False, True]) +def test_overwrite_text_rejects_invalid_destination(root, path, allow): + (root / "directory").mkdir() + with pytest.raises(FileHelperError): + FileHelper(root, allow_symlinks=allow).overwrite_text(path, "new") + assert (root / "directory").is_dir() + + @pytest.mark.parametrize("operation", ["read_bytes", "write_bytes", "create_bytes", "mkdir", "delete"]) @pytest.mark.parametrize("kind", ["leaf", "parent", "dangling"]) def test_deny_rejects_accessed_links_without_mutation(root, link, operation, kind): @@ -858,6 +962,7 @@ def directory_reparse(path, **kwargs): for operation in ( lambda: files.read_bytes(junction / "keep"), lambda: files.write_bytes(junction / "keep", b"bad"), + lambda: files.overwrite_text(junction / "keep", "bad"), lambda: files.create_bytes(junction / "new", b"bad"), lambda: files.mkdir(junction / "new-directory"), lambda: files.delete(junction, recursive=recursive), diff --git a/tests/test_presets.py b/tests/test_presets.py index 11f19f6b91..a15ca4694b 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -194,19 +194,22 @@ def test_save_and_load_available_from_init_options_module(self, project_dir): def test_save_uses_utf8_encoding(self, project_dir, monkeypatch): from specify_cli import save_init_options - original_write_text = Path.write_text + from specify_cli.file_helper import FileHelper + + original_write_text = FileHelper.overwrite_text seen: dict[str, str | None] = {} - def spy_write_text(path, data, *args, **kwargs): - if path == project_dir / ".specify" / "init-options.json": - seen["encoding"] = kwargs.get("encoding") - return original_write_text(path, data, *args, **kwargs) + def spy_write_text(files, path, data, *args, **kwargs): + if files.root / path == project_dir / ".specify" / "init-options.json": + seen["encoding"] = kwargs.get("encoding", "utf-8") + return original_write_text(files, path, data, *args, **kwargs) - monkeypatch.setattr(Path, "write_text", spy_write_text) + monkeypatch.setattr(FileHelper, "overwrite_text", spy_write_text) save_init_options(project_dir, {"label": "中文测试"}) assert seen["encoding"] == "utf-8" + assert json.loads((project_dir / ".specify/init-options.json").read_bytes().decode("utf-8")) == {"label": "中文测试"} def test_load_uses_utf8_encoding(self, project_dir, monkeypatch): from specify_cli import load_init_options From 07d9b3746885b0ed08a8f33af1f0d1130585c0e6 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:44:47 -0500 Subject: [PATCH 2/3] fix: validate init traversal before Windows presence probes 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 --- design/file-helper.md | 4 + src/specify_cli/_file_policy.py | 21 +++++ tests/specify_cli/test_file_policy.py | 130 ++++++++++++++++++++++++++ 3 files changed, 155 insertions(+) diff --git a/design/file-helper.md b/design/file-helper.md index 6fa6807857..f5497ef256 100644 --- a/design/file-helper.md +++ b/design/file-helper.md @@ -42,6 +42,10 @@ missing suffix using a deny-mode helper rooted at the nearest existing parent; an existing explicitly supplied root alias is trusted. `preflight_init_root` validates the original requested spelling without creating a root, including missing-parent traversal and dangling-parent evidence before canonicalization. +Before presence probes, every prefix preceding `..` must be an existing +directory: Windows can otherwise fold `missing/../existing` into an apparently +existing target. Valid existing-directory/root-alias traversal remains trusted; +prefix permission and other I/O failures remain explicit errors. Init's external-adapter lifecycle validates existing target policy before reading integration records, loading adapters or preparing catalog packages. When that lifecycle requires a diff --git a/src/specify_cli/_file_policy.py b/src/specify_cli/_file_policy.py index e9690027c4..e675c9b614 100644 --- a/src/specify_cli/_file_policy.py +++ b/src/specify_cli/_file_policy.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import stat from pathlib import Path from typing import Any, NoReturn @@ -100,10 +101,29 @@ def _existing_init_parent(target: Path) -> Path: return ancestor +def _validate_init_traversal(target: Path) -> None: + # Windows presence probes can fold missing/../existing before filesystem access. + for index, part in enumerate(target.parts): + if part != "..": + continue + prefix = Path(*target.parts[:index]) + try: + is_directory = stat.S_ISDIR(prefix.stat().st_mode) + except FileNotFoundError as exc: + raise FileHelperError( + f"Parent traversal requires an existing directory before '..': {prefix}" + ) from exc + if not is_directory: + raise FileHelperError( + f"Parent traversal requires an existing directory before '..': {prefix}" + ) + + def preflight_init_root(target: Path) -> Path: """Validate original init spelling and policy before project preparation.""" target = target.absolute() try: + _validate_init_traversal(target) if target.exists() or target.is_symlink(): read_project_options(target) else: @@ -126,6 +146,7 @@ def prepare_init_root(target: Path) -> Path: For new roots, deny traversal below the nearest existing directory. """ target = target.absolute() + _validate_init_traversal(target) if target.exists() or target.is_symlink(): FileHelper(target) return target diff --git a/tests/specify_cli/test_file_policy.py b/tests/specify_cli/test_file_policy.py index ac69823049..3f7f356545 100644 --- a/tests/specify_cli/test_file_policy.py +++ b/tests/specify_cli/test_file_policy.py @@ -646,6 +646,136 @@ def handler(project_name=None, integration=None): assert not (tmp_path / "missing").exists() +@pytest.fixture +def normalized_parent_traversal(monkeypatch): + """Model Windows existence/resolution folding '..' before filesystem access.""" + exists = Path.exists + resolve = Path.resolve + + def normalized_exists(path): + if ".." in path.parts: + path = resolve(path, strict=False) + return exists(path) + + def normalized_resolve(path, strict=False): + if ".." in path.parts: + path = resolve(path, strict=False) + return resolve(path, strict=strict) + + monkeypatch.setattr(Path, "exists", normalized_exists) + monkeypatch.setattr(Path, "resolve", normalized_resolve) + + +@pytest.mark.parametrize("prefix_kind", ["missing", "file"]) +def test_init_normalized_traversal_fails_before_project_reads( + tmp_path, normalized_parent_traversal, prefix_kind, +): + from specify_cli.integrations import _lifecycle, installer + + unrelated = tmp_path / "unrelated" + unrelated.mkdir() + policy_module().save_project_options(unrelated, {"allow_symlinks": True}) + prefix = tmp_path / "prefix" + if prefix_kind == "file": + prefix.write_text("retained") + requested = prefix / ".." / unrelated.name + assert requested.exists() + assert requested.resolve(strict=True) == unrelated + called = Mock() + + @_lifecycle.external_lifecycle("init") + def handler(project_name=None, integration=None): + called() + + with ( + patch.object(installer, "read_records", return_value={}) as records, + patch.object(installer, "load_installed_integrations") as loader, + patch.object(installer, "catalog_package") as catalog, + patch.object(policy_module(), "read_project_options") as options, + pytest.raises(typer.Exit) as failure, + ): + handler(project_name=str(requested), integration="generic") + assert failure.value.exit_code == 1 + records.assert_not_called() + loader.assert_not_called() + catalog.assert_not_called() + options.assert_not_called() + called.assert_not_called() + if prefix_kind == "file": + assert prefix.read_text() == "retained" + else: + assert not prefix.exists() + + +@pytest.mark.parametrize("prefix_kind", ["missing", "file"]) +@pytest.mark.parametrize("existing", [False, True]) +def test_prepare_init_root_rejects_normalized_traversal( + tmp_path, normalized_parent_traversal, prefix_kind, existing, +): + target = tmp_path / "target" + if existing: + target.mkdir() + (target / "retained").write_text("retained") + prefix = tmp_path / "prefix" + if prefix_kind == "file": + prefix.write_text("retained") + with pytest.raises(ValueError, match="Parent traversal"): + policy_module().prepare_init_root(prefix / ".." / target.name) + assert target.exists() is existing + if existing: + assert (target / "retained").read_text() == "retained" + + +@pytest.mark.parametrize("existing", [False, True]) +def test_init_existing_parent_traversal_root_remains_trusted(tmp_path, existing): + anchor = tmp_path / "anchor" + anchor.mkdir() + target = tmp_path / "target" + if existing: + target.mkdir() + policy_module().save_project_options(target, {"allow_symlinks": True}) + requested = anchor / ".." / target.name + assert policy_module().preflight_init_root(requested) == target + assert target.exists() is existing + assert policy_module().prepare_init_root(requested) == requested + assert policy_module().project_files(requested).allow_symlinks is existing + + +def test_init_linked_parent_traversal_root_remains_trusted(tmp_path, symlinks): + parent = tmp_path / "parent" + parent.mkdir() + (parent / "anchor").mkdir() + target = parent / "project" + target.mkdir() + policy_module().save_project_options(target, {"allow_symlinks": True}) + alias = tmp_path / "alias" + alias.symlink_to(parent, target_is_directory=True) + requested = alias / "anchor" / ".." / target.name + assert policy_module().preflight_init_root(requested) == target + assert policy_module().prepare_init_root(requested) == requested + assert policy_module().project_files(requested).allow_symlinks is True + + +@pytest.mark.parametrize("operation", ["preflight_init_root", "prepare_init_root"]) +@pytest.mark.parametrize("error", [PermissionError("prefix permission denied"), OSError("prefix I/O failed")]) +def test_init_parent_traversal_io_failures_are_explicit(tmp_path, monkeypatch, operation, error): + prefix = tmp_path / "prefix" + prefix.mkdir() + requested = prefix / ".." / "target" + original_stat = Path.stat + + def failing_stat(path, *args, **kwargs): + if path == prefix: + raise error + return original_stat(path, *args, **kwargs) + + monkeypatch.setattr(Path, "stat", failing_stat) + expected = policy_module().ProjectPolicyError if operation == "preflight_init_root" else type(error) + with pytest.raises(expected, match=str(error)): + getattr(policy_module(), operation)(requested) + assert not (tmp_path / "target").exists() + + @pytest.mark.parametrize("override", [False, True]) @pytest.mark.parametrize("document", [None, "{}", '{"allow_symlinks": true}', '{"allow_symlinks": false}']) def test_check_policy_optional_project_and_override(tmp_path, monkeypatch, override, document): From 54688810a586e8ea021c74ee64eb45167b1a271c Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:17:23 -0500 Subject: [PATCH 3/3] test: cover artifact lookup policy-error boundaries Verify missing and valid persisted policies permit lookup, while malformed settings and bootstrap links emit actionable JSON errors before installed-adapter or catalog access. Add seven net cases without changing runtime behavior. 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 --- .../artifacts/test_command_lookup.py | 70 ++++++++++++++++++- 1 file changed, 68 insertions(+), 2 deletions(-) diff --git a/tests/specify_cli/artifacts/test_command_lookup.py b/tests/specify_cli/artifacts/test_command_lookup.py index 9e995915af..cde4b94ad5 100644 --- a/tests/specify_cli/artifacts/test_command_lookup.py +++ b/tests/specify_cli/artifacts/test_command_lookup.py @@ -5,20 +5,35 @@ import json from datetime import date from pathlib import Path +from unittest.mock import Mock import pytest from typer.testing import CliRunner from specify_cli import app -from specify_cli.artifacts import ArtifactCatalog +from specify_cli.artifacts import ArtifactCatalog, command_lookup +from specify_cli.integrations import installer from tests.conftest import install_preset class TestCommandLookup: + @pytest.mark.parametrize( + "options", + [None, {}, {"allow_symlinks": False}, {"allow_symlinks": True}], + ids=["missing-file", "missing-setting", "deny", "allow"], + ) def test_lookup_json_cross_references_manifest_contribution( - self, spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch + self, + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + options: dict[str, object] | None, ): monkeypatch.chdir(spec_kit_project) + config = spec_kit_project / ".specify" / "init-options.json" + if options is None: + config.unlink(missing_ok=True) + else: + config.write_text(json.dumps(options), encoding="utf-8") pack = install_preset( spec_kit_project, "lookup-pack", @@ -51,6 +66,57 @@ def test_lookup_json_cross_references_manifest_contribution( ".specify/presets/lookup-pack/templates/lookup.md" ) + @pytest.mark.parametrize( + "case,diagnostic", + [ + ("malformed", "Invalid JSON"), + ("invalid-setting", "JSON boolean"), + ("symlink", "symlink"), + ("dangling-symlink", "symlink"), + ], + ) + def test_lookup_policy_failure_precedes_loader_and_catalog( + self, + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + case: str, + diagnostic: str, + ): + monkeypatch.chdir(spec_kit_project) + config = spec_kit_project / ".specify" / "init-options.json" + if case == "malformed": + config.write_text("{", encoding="utf-8") + elif case == "invalid-setting": + config.write_text('{"allow_symlinks": "true"}', encoding="utf-8") + else: + target = spec_kit_project / "linked-init-options.json" + if case == "symlink": + target.write_text('{"allow_symlinks": true}', encoding="utf-8") + config.unlink(missing_ok=True) + try: + config.symlink_to(target) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"Symlinks unavailable: {exc}") + + loader = Mock(side_effect=AssertionError("installed loader reached")) + catalog = Mock(side_effect=AssertionError("artifact catalog reached")) + monkeypatch.setattr(installer, "load_installed_integrations", loader) + monkeypatch.setattr(command_lookup, "ArtifactCatalog", catalog) + + result = CliRunner().invoke( + app, + ["artifact", "lookup", "preset:lookup-pack:template:lookup-template", "--json"], + ) + + assert result.exit_code == 1 + assert result.stdout == "" + error = json.loads(result.stderr) + assert set(error) == {"error"} + assert "init-options.json" in error["error"] + assert diagnostic in error["error"] + loader.assert_not_called() + catalog.assert_not_called() + def test_lookup_json_rejects_unknown_contribution( self, spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch ):