diff --git a/docs/reference/workflows.md b/docs/reference/workflows.md index 16bbe0893e..8bd4a8778d 100644 --- a/docs/reference/workflows.md +++ b/docs/reference/workflows.md @@ -86,8 +86,30 @@ Lists workflows installed in the current project. specify workflow add ``` +| Option | Description | +| --------------- | ------------------------------------------------------ | +| `--dev` | Install from a local workflow YAML file or directory | +| `--from ` | Install from a custom URL (`` names the expected workflow ID) | + Installs a workflow from the catalog, a URL (HTTPS required), or a local file path. +## Update Workflows + +```bash +specify workflow update [workflow_id] +``` + +Updates one installed catalog workflow — or all of them when no ID is given — to the latest catalog version. Prompts for confirmation and keeps the installed copy if a download or validation fails. + +## Enable or Disable a Workflow + +```bash +specify workflow enable +specify workflow disable +``` + +Disabled workflows stay installed and listed (marked `[disabled]`) but refuse to run until re-enabled. + ## Remove a Workflow ```bash @@ -102,9 +124,10 @@ Removes an installed workflow from the project. specify workflow search [query] ``` -| Option | Description | -| ------- | --------------- | -| `--tag` | Filter by tag | +| Option | Description | +| ---------- | ----------------- | +| `--tag` | Filter by tag | +| `--author` | Filter by author | Searches all active catalogs for workflows matching the query. diff --git a/src/specify_cli/bundler/services/installer.py b/src/specify_cli/bundler/services/installer.py index 3e61ded040..7c5abf84db 100644 --- a/src/specify_cli/bundler/services/installer.py +++ b/src/specify_cli/bundler/services/installer.py @@ -187,19 +187,41 @@ def remove_bundle( still_needed = components_still_needed(records, exclude_bundle_id=bundle_id) result = InstallResult(bundle_id=bundle_id) + remove_attempted = False - for component in target.contributed_components: - key = (component.kind, component.id) - if key in still_needed: - result.skipped.append(component) - continue - if installer.is_installed(project_root, component): - installer.remove(project_root, component) - result.uninstalled.append(component) + try: + for component in target.contributed_components: + key = (component.kind, component.id) + if key in still_needed: + result.skipped.append(component) + continue + if installer.is_installed(project_root, component): + remove_attempted = True + installer.remove(project_root, component) + result.uninstalled.append(component) + save_records(project_root, remove_record(records, bundle_id)) + except Exception as exc: # noqa: BLE001 + if result.uninstalled: + detail = ( + f"{len(result.uninstalled)} component(s) were already removed " + "before this failure; the bundle record was left unchanged, " + "so the project may be partially uninstalled." + ) + elif remove_attempted: + detail = ( + "No components were removed, but the failing component may " + "have made partial changes before raising, so the project " + "may be partially uninstalled." + ) else: - result.skipped.append(component) + detail = ( + "No components were removed and no removal was attempted; " + "the bundle record was left unchanged." + ) + raise BundlerError( + f"Failed to remove bundle '{bundle_id}': {exc}. {detail}" + ) from exc - save_records(project_root, remove_record(records, bundle_id)) return result diff --git a/src/specify_cli/workflows/_commands.py b/src/specify_cli/workflows/_commands.py index e1d29b47d5..c9897569f9 100644 --- a/src/specify_cli/workflows/_commands.py +++ b/src/specify_cli/workflows/_commands.py @@ -58,6 +58,72 @@ def _error_console(json_output: bool): return err_console if json_output else console +def _open_workflow_registry(project_root: Path, out=None): + """Construct a WorkflowRegistry, exiting cleanly on an unreadable file. + + WorkflowRegistry fails closed (raises OSError) at construction when its + file can't be read, rather than falling back to an empty registry a + caller could mistake for "nothing installed". Every CLI command that + opens a registry needs this same clean-error boundary. + """ + from .catalog import WorkflowRegistry + + try: + return WorkflowRegistry(project_root) + except OSError as exc: + (out or console).print( + f"[red]Error:[/red] Failed to read workflow registry: {_escape_markup(str(exc))}" + ) + raise typer.Exit(1) + + +def _require_enabled_workflow( + registry_root: Path, workflow_id: str, out: Any +) -> None: + """Fail closed for corrupted or explicitly disabled registry entries.""" + metadata = _open_workflow_registry(registry_root, out).get(workflow_id) + if metadata is not None and not isinstance(metadata, dict): + out.print( + f"[red]Error:[/red] Registry entry for " + f"'{_escape_markup(workflow_id)}' is corrupted" + ) + raise typer.Exit(1) + if isinstance(metadata, dict) and not metadata.get("enabled", True): + out.print( + f"[red]Error:[/red] Workflow '{_escape_markup(workflow_id)}' is disabled. " + f"Enable with: specify workflow enable {_escape_markup(workflow_id)}" + ) + raise typer.Exit(1) + + +def _resolve_run_owner_root( + installed_registry_root: str | None, project_root: Path +) -> Path: + """Determine which project's registry gates resuming a run. + + ``installed_registry_root`` is only ever persisted when the run's + installed workflow genuinely belongs to a *different* project than the + one whose ``runs/`` directory holds this run's own state (a direct + external workflow-file invocation) -- see ``workflow_run``. The common + case (an installed workflow run from its own project) stores ``None``, + so a later project rename/move is transparently picked up here by + falling back to the *current* ``project_root`` instead of a stale + absolute path baked in at run start. + + A persisted cross-project root that no longer exists cannot be safely + rediscovered and must fail closed instead of consulting the unrelated + project that happens to store the run state. + """ + if installed_registry_root: + candidate = Path(installed_registry_root) + if not candidate.is_symlink() and candidate.is_dir(): + return candidate + raise ValueError( + "Installed workflow owner is unavailable; cannot safely resume" + ) + return project_root + + def _parse_input_values( input_values: list[str] | None, *, json_output: bool = False ) -> dict[str, Any]: @@ -104,10 +170,254 @@ def _reject_unsafe_workflow_storage(project_root: Path) -> None: ) +def _scan_for_workflow_owner(parts: tuple[str, ...]) -> int | None: + """Find the *nearest* (innermost) ``.specify/workflows/`` owner in + *parts*, scanning from the end of the path. + + Scanning from the end (rather than stopping at the first match from the + start) matters for a project nested beneath an unrelated outer path that + happens to reuse the same ``.specify``/``workflows`` segment names: the + first-from-start match would pick the outer directory and the wrong + workflow ID, silently missing the real (inner) owner's disabled check. + + Returns the index of the owning ``.specify`` segment, or ``None`` if no + owner segment is present. + """ + for i in range(len(parts) - 3, -1, -1): + if ( + parts[i].casefold() == ".specify" + and parts[i + 1].casefold() == "workflows" + ): + return i + return None + + +def _expand_first_symlink_target(path: Path) -> Path | None: + """Expand one symlink component while preserving the remaining path.""" + parts = path.parts + current = Path(path.anchor) if path.is_absolute() else Path() + start = 1 if path.is_absolute() else 0 + for index in range(start, len(parts)): + current = current / parts[index] + if not current.is_symlink(): + continue + try: + target = Path(os.readlink(current)) + except OSError: + return None + if not target.is_absolute(): + target = current.parent / target + expanded = target.joinpath(*parts[index + 1 :]) + return Path(os.path.normpath(str(expanded.absolute()))) + return None + + +def _resolve_installed_workflow_ownership( + source_path: Path, err +) -> tuple[Path | None, str | None]: + """Map a direct ``workflow.yml`` *source_path* back to the installed + workflow (``registry_root``, ``registered_id``) it belongs to, if any. + + A registered path can point at installed storage three ways, all of + which must receive the same registry disabled-check: + + 1. Lexically: the path's own (symlink-preserving) segments identify + ``.specify/workflows/`` -- collapsing ``..``/``.`` but + never resolving symlinks, so a symlinked ``workflow.yml`` leaf (or + symlinked ```` directory) inside the owned tree is caught by the + inward-symlink refusal below rather than silently followed. + 2. Via an intermediate alias target whose lexical path identifies + ``.specify/workflows/`` before a symlinked storage ancestor is + resolved away. + 3. Via an outward-pointing alias whose fully resolved target lands + inside legitimate installed storage, even though the raw invocation + path has no ownership segments. + + Returns ``(None, None)`` when neither applies -- a genuinely standalone + external workflow file, which is allowed to run unchecked. + """ + def ownership_for(candidate: Path) -> tuple[Path, str] | None: + parts = candidate.parts + i = _scan_for_workflow_owner(parts) + if i is None: + return None + registry_root = ( + Path(*parts[:i]) if i else Path(candidate.anchor or ".") + ) + candidate_specify = Path(*parts[: i + 1]) + candidate_workflows = Path(*parts[: i + 2]) + candidate_id_dir = Path(*parts[: i + 3]) + canonical_specify = registry_root / ".specify" + canonical_workflows = canonical_specify / "workflows" + # The path-derived registry_root here may differ from the cwd's + # project_root already checked by _reject_unsafe_workflow_storage + # (e.g. this path points into another project entirely, or this + # project's own .specify is itself a symlink to an + # attacker-controlled tree) -- check it explicitly rather than + # trusting that cwd-scoped guard, and don't rely on + # WorkflowRegistry's own symlinked-parent handling as the safety + # signal here: it fails closed by raising OSError at construction + # time (see catalog.py's _load), but that surfaces as an opaque + # exception rather than this guard's clean, specific CLI error for + # the actual owning project root. + _reject_unsafe_dir(canonical_specify, ".specify") + _reject_unsafe_dir(canonical_workflows, ".specify/workflows") + _reject_unsafe_dir(candidate_specify, ".specify") + _reject_unsafe_dir(candidate_workflows, ".specify/workflows") + try: + if not os.path.samefile(candidate_specify, canonical_specify): + return None + if not os.path.samefile( + candidate_workflows, canonical_workflows + ): + return None + except OSError: + return None + registry = _open_workflow_registry(registry_root, err) + registered_id = None + for workflow_id in registry.list(): + if ( + not isinstance(workflow_id, str) + or workflow_id in _RESERVED_WORKFLOW_IDS + or not _WORKFLOW_ID_PATTERN.fullmatch(workflow_id) + ): + continue + try: + if os.path.samefile( + candidate_id_dir, + canonical_workflows / workflow_id, + ): + registered_id = workflow_id + break + except OSError: + continue + if registered_id is None: + return None + # A legitimately installed workflow's own directory tree never + # contains a symlink (workflow add/remove both refuse one at + # install time); one appearing here means the file actually loaded + # below would not be the file this ownership match is based on, so + # refuse rather than silently mismatch. + for k in range(i + 2, len(parts) + 1): + if Path(*parts[:k]).is_symlink(): + err.print( + "[red]Error:[/red] Refusing to run: " + f".specify/workflows/{_escape_markup(registered_id)} " + "contains a symlinked path component" + ) + raise typer.Exit(1) + return registry_root, registered_id + + lexical = Path(os.path.normpath(str(source_path.absolute()))) + ownership = ownership_for(lexical) + if ownership is not None: + return ownership + + # Inspect each intermediate symlink target before fully resolving it. + # Full resolution can erase .specify/workflows ownership segments when + # one of those storage directories is itself a symlink. + candidate = lexical + seen = {candidate} + for _ in range(40): + expanded = _expand_first_symlink_target(candidate) + if expanded is None or expanded in seen: + break + ownership = ownership_for(expanded) + if ownership is not None: + return ownership + seen.add(expanded) + candidate = expanded + + # A fully resolved target may still land in legitimate installed + # storage through an unrelated-looking alias. + try: + resolved = source_path.resolve(strict=False) + except (OSError, RuntimeError): + return None, None + if resolved == lexical: + # Nothing on this path is a symlink; already covered above. + return None, None + ownership = ownership_for(resolved) + return ownership if ownership is not None else (None, None) + + _WORKFLOW_ID_PATTERN = re.compile(r"^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$") _RESERVED_WORKFLOW_IDS: frozenset[str] = frozenset({"runs", "steps"}) +def _reject_insecure_download_redirect(old_url: str, new_url: str) -> None: + """Reject a redirect before it is followed unless HTTPS (or loopback HTTP).""" + import urllib.error + from ipaddress import ip_address + from urllib.parse import urlparse + + parsed = urlparse(new_url) + host = parsed.hostname or "" + loopback = host == "localhost" + if not loopback: + try: + loopback = ip_address(host).is_loopback + except ValueError: + pass + if parsed.scheme != "https" and not (parsed.scheme == "http" and loopback): + raise urllib.error.URLError( + "redirect target must use HTTPS, or HTTP for localhost/loopback" + ) + + +# Workflow YAML definitions are small step/metadata text, not binaries, so +# this is generous headroom against a malicious or misbehaving server -- not +# a ceiling any legitimate workflow definition should ever approach. +_MAX_WORKFLOW_YAML_BYTES = 5 * 1024 * 1024 # 5 MiB +_DOWNLOAD_CHUNK_SIZE = 65536 + + +def _read_response_within_limit(response, max_bytes: int | None = None) -> bytes: + """Read *response* fully, enforcing *max_bytes* via bounded streaming. + + A ``Content-Length`` header is checked up front to fail fast, but it is + never trusted alone: the actual bytes read are also counted as they + stream in, so a chunked or ``Content-Length``-less response that lies + about (or omits) its size still cannot exceed the limit. + + ``max_bytes`` defaults to ``None`` (resolved to the module-level + ``_MAX_WORKFLOW_YAML_BYTES`` at call time, not at function-definition + time) so tests can override the effective limit via monkeypatching the + module attribute. + """ + if max_bytes is None: + max_bytes = _MAX_WORKFLOW_YAML_BYTES + content_length = None + getheader = getattr(response, "getheader", None) + if callable(getheader): + try: + raw_length = getheader("Content-Length") + except Exception: + raw_length = None + if raw_length is not None: + try: + content_length = int(raw_length) + except (TypeError, ValueError): + content_length = None + if content_length is not None and content_length > max_bytes: + raise ValueError( + f"response declared {content_length} bytes, exceeding the " + f"{max_bytes}-byte workflow size limit" + ) + + chunks: list[bytes] = [] + total = 0 + while True: + chunk = response.read(_DOWNLOAD_CHUNK_SIZE) + if not chunk: + break + total += len(chunk) + if total > max_bytes: + raise ValueError(f"response exceeds the {max_bytes}-byte workflow size limit") + chunks.append(chunk) + return b"".join(chunks) + + def _validate_workflow_id_or_exit(workflow_id: str) -> None: """Validate that ``workflow_id`` is a safe installed-workflow directory name.""" if ( @@ -170,6 +480,343 @@ def _safe_workflow_id_dir(workflows_dir: Path, workflow_id: str) -> Path: return dest_dir +class _StagedWorkflowFile: + """Exclusive staging inode kept open until its atomic commit.""" + + def __init__(self, path: Path, fd: int) -> None: + self.path = path + self.fd = fd + + def _write(self, chunks) -> None: + os.lseek(self.fd, 0, os.SEEK_SET) + os.ftruncate(self.fd, 0) + for chunk in chunks: + view = memoryview(chunk) + while view: + written = os.write(self.fd, view) + if written <= 0: + raise OSError("Failed to write staged workflow file") + view = view[written:] + + def write_bytes(self, data: bytes) -> None: + self._write((data,)) + + def verify_path(self) -> None: + import stat + + try: + path_stat = self.path.stat(follow_symlinks=False) + open_stat = os.fstat(self.fd) + except OSError as exc: + raise OSError( + "Staged workflow file changed before commit" + ) from exc + if ( + not stat.S_ISREG(path_stat.st_mode) + or path_stat.st_dev != open_stat.st_dev + or path_stat.st_ino != open_stat.st_ino + ): + raise OSError("Staged workflow file changed before commit") + + def set_mode(self, mode: int) -> None: + if hasattr(os, "fchmod"): + os.fchmod(self.fd, mode) + + def close(self) -> None: + if self.fd < 0: + return + fd, self.fd = self.fd, -1 + try: + os.close(fd) + except OSError: + pass + + +def _stage_workflow_file( + dest_dir: Path, *, use_project_file_mode: bool = False +) -> _StagedWorkflowFile: + """Reserve a same-directory staging file so new/updated workflow.yml + content can be written and validated without ever touching (and risking + truncating) an existing destination file before the final atomic swap. + Shared by the local-install and catalog-install paths. + + If dest_dir did not already exist, this call creates it; if mkstemp then + fails (disk full/EMFILE/quota), the freshly-created directory is removed + again via a guarded rmdir (never a broad rmtree, so any concurrently + written content is left untouched) before the original OSError is + re-raised unchanged. A pre-existing dest_dir (reinstall) is never + touched by this cleanup. For catalog-created files, + ``use_project_file_mode`` recreates the reserved path exclusively with + mode 0666 so the process umask supplies the normal project-file mode. + The final descriptor remains open so callers write to and verify the + reserved inode rather than reopening a replaceable pathname.""" + import tempfile + + created_dir = not dest_dir.exists() + dest_dir.mkdir(parents=True, exist_ok=True) + fd = -1 + staged_file: Path | None = None + try: + fd, tmp_name = tempfile.mkstemp(dir=dest_dir, prefix=".workflow.yml.", suffix=".tmp") + staged_file = Path(tmp_name) + if use_project_file_mode: + os.close(fd) + fd = -1 + staged_file.unlink() + flags = os.O_RDWR | os.O_CREAT | os.O_EXCL + flags |= getattr(os, "O_NOFOLLOW", 0) + fd = os.open(staged_file, flags, 0o666) + except OSError: + if fd >= 0: + try: + os.close(fd) + except OSError: + pass + if staged_file is not None: + try: + staged_file.unlink(missing_ok=True) + except OSError: + pass + if created_dir: + try: + dest_dir.rmdir() + except OSError as cleanup_exc: + console.print( + "[yellow]Warning:[/yellow] Failed to remove incomplete " + f"workflow directory: {_escape_markup(str(cleanup_exc))}" + ) + raise + assert staged_file is not None + return _StagedWorkflowFile(staged_file, fd) + + +@contextlib.contextmanager +def _workflow_install_transaction(project_root: Path): + """Serialize workflow file swaps with their registry updates.""" + from ..shared_infra import _ensure_safe_shared_directory + + lock_dir = project_root / ".specify" + try: + _ensure_safe_shared_directory( + project_root, lock_dir, context="workflow install lock directory" + ) + except ValueError as exc: + raise OSError(str(exc)) from exc + lock_file = lock_dir / ".workflow-install.lock" + if lock_file.is_symlink(): + raise OSError(f"Refusing to use symlinked workflow install lock: {lock_file}") + + flags = os.O_RDWR | os.O_CREAT + flags |= getattr(os, "O_NOFOLLOW", 0) + flags |= getattr(os, "O_CLOEXEC", 0) + fd = os.open(lock_file, flags, 0o600) + try: + if lock_file.is_symlink(): + raise OSError( + f"Refusing to use symlinked workflow install lock: {lock_file}" + ) + if os.name == "nt": + import errno + import msvcrt + import time + + if os.fstat(fd).st_size == 0: + os.write(fd, b"\0") + while True: + os.lseek(fd, 0, os.SEEK_SET) + try: + msvcrt.locking(fd, msvcrt.LK_NBLCK, 1) + break + except OSError as exc: + if exc.errno not in (errno.EACCES, errno.EDEADLK): + raise + time.sleep(0.05) + else: + import fcntl + + fcntl.flock(fd, fcntl.LOCK_EX) + yield + finally: + os.close(fd) + + +def _commit_workflow_file( + staged_file: Path | _StagedWorkflowFile, + dest_file: Path, + existed_before: bool, +) -> Path | None: + """Atomically swap ``staged_file`` onto ``dest_file``. If a prior file + existed, it is first renamed to a unique sibling (path returned) so a + later failure (e.g. registry.add()) can restore it via rename instead + of a content rewrite -- the destination is never truncated/overwritten + in place. If the second rename fails after the first succeeded, the + prior file is put back immediately so dest_file is never left simply + missing.""" + staged_path = ( + staged_file.path + if isinstance(staged_file, _StagedWorkflowFile) + else staged_file + ) + if isinstance(staged_file, _StagedWorkflowFile): + staged_file.verify_path() + if existed_before and dest_file.exists(): + import tempfile + + mode = dest_file.stat(follow_symlinks=False).st_mode & 0o7777 + if isinstance(staged_file, _StagedWorkflowFile): + staged_file.set_mode(mode) + else: + staged_path.chmod(mode) + fd, backup_name = tempfile.mkstemp( + dir=dest_file.parent, + prefix=f".{dest_file.name}.", + suffix=".bak", + ) + os.close(fd) + backup_file = Path(backup_name) + try: + os.replace(dest_file, backup_file) + except BaseException: + try: + backup_file.unlink(missing_ok=True) + except OSError: + pass + raise + try: + if isinstance(staged_file, _StagedWorkflowFile): + staged_file.verify_path() + # Windows cannot replace an open file. Verify through the + # exclusive descriptor, then close immediately before rename. + staged_file.close() + os.replace(staged_path, dest_file) + except OSError as commit_exc: + try: + os.replace(backup_file, dest_file) + except OSError as restore_exc: + raise OSError( + f"Failed to commit workflow file ({commit_exc}); failed " + f"to restore the prior workflow from {backup_file} " + f"({restore_exc}). The prior workflow remains at " + f"{backup_file}." + ) from restore_exc + raise + return backup_file + if isinstance(staged_file, _StagedWorkflowFile): + staged_file.verify_path() + staged_file.close() + os.replace(staged_path, dest_file) + return None + + +def _discard_staged_workflow_file( + staged_file: Path | _StagedWorkflowFile, + dest_dir: Path, + existed_before: bool, +) -> None: + """Clean up after a pre-commit failure (staged_file was never swapped + onto dest_file): remove the staged file, and for a fresh install (no + prior directory) remove the now-orphaned dest_dir too. A genuine + removal failure must propagate (not be swallowed) so the safe wrapper + below can warn instead of silently leaving an orphan; a dest_dir + already absent is not itself an error.""" + staged_path = ( + staged_file.path + if isinstance(staged_file, _StagedWorkflowFile) + else staged_file + ) + if isinstance(staged_file, _StagedWorkflowFile): + staged_file.close() + staged_path.unlink(missing_ok=True) + if not existed_before and dest_dir.exists(): + import errno + + try: + dest_dir.rmdir() + except OSError as exc: + # Another concurrent install may already have committed content + # into this once-fresh directory. Never recursively delete it. + if exc.errno not in (errno.ENOTEMPTY, errno.EEXIST): + raise + + +def _rollback_committed_workflow_file( + dest_file: Path, dest_dir: Path, existed_before: bool, backup_file: Path | None +) -> None: + """Undo a successful _commit_workflow_file swap after a later failure + (registry.add()): restore the prior file via rename, remove the newly + committed file for a reinstall over a pre-existing empty directory + (no backup), or remove the new file and then its directory when empty + for a fresh install. A genuine removal failure must propagate (not be + swallowed) so the safe wrapper below can warn instead of silently + leaving an orphan; a dest_dir already absent is not itself an error.""" + if backup_file is not None: + os.replace(backup_file, dest_file) + else: + dest_file.unlink(missing_ok=True) + if not existed_before and dest_dir.exists(): + import errno + + try: + dest_dir.rmdir() + except OSError as exc: + # Another installer may have staged a sibling before taking + # the transaction lock. Preserve it rather than recursively + # deleting the shared directory during this rollback. + if exc.errno not in (errno.ENOTEMPTY, errno.EEXIST): + raise + + +def _safe_discard_staged_workflow_file( + staged_file: Path | _StagedWorkflowFile, + dest_dir: Path, + existed_before: bool, +) -> None: + """Guarded wrapper: a cleanup failure must be reported, never crash or + silently mask the original install error that triggered it.""" + try: + _discard_staged_workflow_file(staged_file, dest_dir, existed_before) + except OSError as exc: + console.print( + "[yellow]Warning:[/yellow] Failed to clean up incomplete workflow " + f"install: {_escape_markup(str(exc))}" + ) + + +def _safe_rollback_committed_workflow_file( + dest_file: Path, dest_dir: Path, existed_before: bool, backup_file: Path | None +) -> None: + """Guarded wrapper: a rollback failure must be reported, never crash or + silently claim the prior workflow file was restored when it wasn't.""" + try: + _rollback_committed_workflow_file(dest_file, dest_dir, existed_before, backup_file) + except OSError as exc: + console.print( + "[yellow]Warning:[/yellow] Failed to restore prior workflow file " + f"after registry update failure: {_escape_markup(str(exc))}" + ) + + +def _discard_committed_backup_file(backup_file: Path | None) -> None: + """Once registry.add()/registry.remove() has durably succeeded after a + _commit_workflow_file() swap, the renamed-aside prior file is no longer + needed for rollback -- it must be discarded, not left as a permanent + orphan sibling that every future reinstall would silently accumulate or + clobber. A cleanup failure here must not turn an already-successful + install into a reported failure; it's reported as a warning, consistent + with workflow_remove's post-commit cleanup semantics. A fresh install + (backup_file is None) is a no-op.""" + if backup_file is None: + return + try: + backup_file.unlink(missing_ok=True) + except OSError as exc: + console.print( + "[yellow]Warning:[/yellow] Workflow installed, but its backup file " + f"could not be cleaned up: {_escape_markup(str(exc))}. Remove it " + f"manually: {_escape_markup(str(backup_file))}" + ) + + # Root helper re-fetched at call time so test monkeypatching of # `specify_cli._require_specify_project` keeps working after the move. def _require_specify_project(*args, **kwargs): @@ -348,6 +995,32 @@ def workflow_run( engine.on_step_start = lambda sid, label: console.print(f" \u25b8 [{sid}] {label} \u2026") err = _error_console(json_output) + + registered_id: str | None = None + registry_root = project_root + if not is_file_source: + # Reject path-equivalent spellings ("align-wf/", "align-wf/.") that + # would miss the registry lookup yet still load the installed file, + # bypassing the disabled check below. + if source in _RESERVED_WORKFLOW_IDS or not _WORKFLOW_ID_PATTERN.match(source): + err.print( + f"[red]Error:[/red] Invalid workflow ID: {_escape_markup(repr(source))}" + ) + raise typer.Exit(1) + registered_id = source + else: + # A direct YAML path may still point at an installed workflow's own + # file (lexically, or via a symlinked alias pointing into installed + # storage); map it back to its owning project and ID so the + # disabled check below can't be silently bypassed. + owner_root, owner_id = _resolve_installed_workflow_ownership(source_path, err) + if owner_id is not None: + registry_root = owner_root + registered_id = owner_id + + if registered_id is not None: + _require_enabled_workflow(registry_root, registered_id, err) + try: definition = engine.load_workflow(source_path if is_file_source else source) except FileNotFoundError: @@ -362,7 +1035,7 @@ def workflow_run( if errors: err.print("[red]Workflow validation failed:[/red]") for verr in errors: - err.print(f" • {verr}") + err.print(f" • {_escape_markup(str(verr))}") raise typer.Exit(1) # Parse inputs @@ -374,7 +1047,25 @@ def workflow_run( try: with _stdout_to_stderr_when(json_output): - state = engine.execute(definition, inputs) + state = engine.execute( + definition, + inputs, + installed_workflow_id=registered_id, + # Only persist an explicit root when the installed workflow + # genuinely belongs to a *different* project than the one + # whose runs/ directory holds this run's own state (a + # direct external workflow-file invocation) -- the common + # case (an installed workflow run from its own project) + # leaves this None so resume re-derives the owning root + # from wherever the project currently is, transparently + # surviving a project rename/move instead of baking in a + # stale absolute path at run start. + installed_registry_root=( + registry_root + if registered_id and registry_root != project_root + else None + ), + ) except ValueError as exc: err.print(f"[red]Error:[/red] {exc}") raise typer.Exit(1) @@ -416,7 +1107,7 @@ def workflow_resume( ): """Resume a paused or failed workflow run.""" from . import load_custom_steps - from .engine import WorkflowEngine + from .engine import RunState, WorkflowEngine project_root = _require_specify_project() load_custom_steps(project_root) @@ -427,6 +1118,37 @@ def workflow_resume( inputs = _parse_input_values(input_values, json_output=json_output) err = _error_console(json_output) + # Pre-load the persisted run state so a run started from an installed + # workflow that has since been disabled cannot resume unchecked -- + # engine.resume() replays the run directly from disk with no registry + # awareness at all, which would otherwise bypass the same disabled + # guard `workflow run` enforces. Runs without installed_workflow_id + # (a direct/non-installed source, or a run persisted before this field + # existed) are unaffected and resume exactly as before. + try: + pre_state = RunState.load(run_id, project_root) + except FileNotFoundError: + err.print(f"[red]Error:[/red] Run not found: {run_id}") + raise typer.Exit(1) + except ValueError as exc: + err.print(f"[red]Error:[/red] {_escape_markup(str(exc))}") + raise typer.Exit(1) + except OSError as exc: + err.print(f"[red]Resume failed:[/red] {_escape_markup(str(exc))}") + raise typer.Exit(1) + + if pre_state.installed_workflow_id is not None: + try: + owner_root = _resolve_run_owner_root( + pre_state.installed_registry_root, project_root + ) + except ValueError as exc: + err.print(f"[red]Error:[/red] {_escape_markup(str(exc))}") + raise typer.Exit(1) + _require_enabled_workflow( + owner_root, pre_state.installed_workflow_id, err + ) + try: with _stdout_to_stderr_when(json_output): state = engine.resume(run_id, inputs or None) @@ -434,10 +1156,10 @@ def workflow_resume( err.print(f"[red]Error:[/red] Run not found: {run_id}") raise typer.Exit(1) except ValueError as exc: - err.print(f"[red]Error:[/red] {exc}") + err.print(f"[red]Error:[/red] {_escape_markup(str(exc))}") raise typer.Exit(1) except Exception as exc: - err.print(f"[red]Resume failed:[/red] {exc}") + err.print(f"[red]Resume failed:[/red] {_escape_markup(str(exc))}") raise typer.Exit(1) if json_output: @@ -478,6 +1200,9 @@ def workflow_status( except FileNotFoundError: console.print(f"[red]Error:[/red] Run not found: {run_id}") raise typer.Exit(1) + except ValueError as exc: + console.print(f"[red]Error:[/red] {_escape_markup(str(exc))}") + raise typer.Exit(1) if json_output: # Build on the shared run/resume payload so the common fields @@ -556,10 +1281,8 @@ def workflow_status( @workflow_app.command("list") def workflow_list(): """List installed workflows.""" - from .catalog import WorkflowRegistry - project_root = _require_specify_project() - registry = WorkflowRegistry(project_root) + registry = _open_workflow_registry(project_root) installed = registry.list() if not installed: @@ -570,36 +1293,61 @@ def workflow_list(): console.print("\n[bold cyan]Installed Workflows:[/bold cyan]\n") for wf_id, wf_data in installed.items(): - console.print(f" [bold]{wf_data.get('name', wf_id)}[/bold] ({wf_id}) v{wf_data.get('version', '?')}") + safe_id = _escape_markup(wf_id) + if not isinstance(wf_data, dict): + console.print(f" [yellow]Warning:[/yellow] Skipping corrupted registry entry '{safe_id}'.\n") + continue + marker = "" if wf_data.get("enabled", True) else " [red]\\[disabled][/red]" + name = _escape_markup(str(wf_data.get("name", wf_id))) + version = _escape_markup(str(wf_data.get("version", "?"))) + console.print(f" [bold]{name}[/bold] ({safe_id}) v{version}{marker}") desc = wf_data.get("description", "") if desc: - console.print(f" {desc}") + console.print(f" {_escape_markup(str(desc))}") console.print() @workflow_app.command("add") def workflow_add( source: str = typer.Argument(..., help="Workflow ID, URL, or local path"), + dev: bool = typer.Option(False, "--dev", help="Install from a local workflow YAML file or directory"), + from_url: str | None = typer.Option(None, "--from", help="Install from a custom URL"), ): """Install a workflow from catalog, URL, or local path.""" - from .catalog import WorkflowCatalog, WorkflowRegistry, WorkflowCatalogError from .engine import WorkflowDefinition project_root = _require_specify_project() - registry = WorkflowRegistry(project_root) + _open_workflow_registry(project_root) workflows_dir = project_root / ".specify" / "workflows" + # With --from, source names the expected workflow ID: validate it up + # front so a URL/path/typo fails without a network fetch. + if from_url is not None and not dev: + _validate_workflow_id_or_exit(source) # Reject a symlinked .specify / .specify/workflows before any write so an # install can't escape the project root (covers the local, URL, and # catalog branches below — all write beneath workflows_dir). _reject_unsafe_dir(project_root / ".specify", ".specify") _reject_unsafe_dir(workflows_dir, ".specify/workflows") - def _validate_and_install_local(yaml_path: Path, source_label: str) -> None: + def _validate_and_install_local( + yaml_path: Path, source_label: str, expected_id: str | None = None + ) -> None: """Validate and install a workflow from a local YAML file.""" try: - definition = WorkflowDefinition.from_yaml(yaml_path) - except (ValueError, yaml.YAMLError) as exc: - console.print(f"[red]Error:[/red] Invalid workflow YAML: {exc}") + with yaml_path.open("rb") as source_file: + source_mode = os.fstat(source_file.fileno()).st_mode & 0o7777 + source_content = source_file.read() + definition = WorkflowDefinition.from_string( + source_content.decode("utf-8") + ) + except OSError as exc: + console.print( + f"[red]Error:[/red] Failed to read workflow YAML: " + f"{_escape_markup(str(exc))}" + ) + raise typer.Exit(1) + except (UnicodeDecodeError, ValueError, yaml.YAMLError) as exc: + console.print(f"[red]Error:[/red] Invalid workflow YAML: {_escape_markup(str(exc))}") raise typer.Exit(1) # Non-string ids (e.g. unquoted ``id: 123`` or ``id: 0``) fall through # to validate_workflow below, which reports a typed error instead of @@ -618,31 +1366,144 @@ def _validate_and_install_local(yaml_path: Path, source_label: str) -> None: if errors: console.print("[red]Error:[/red] Workflow validation failed:") for err in errors: - console.print(f" \u2022 {err}") + console.print(f" \u2022 {_escape_markup(str(err))}") + raise typer.Exit(1) + + if expected_id is not None and definition.id != expected_id: + console.print( + f"[red]Error:[/red] Workflow ID in YAML ({_escape_markup(repr(definition.id))}) " + f"does not match the requested workflow ID ({_escape_markup(repr(expected_id))})." + ) raise typer.Exit(1) dest_dir = _safe_workflow_id_dir(workflows_dir, definition.id) - dest_dir.mkdir(parents=True, exist_ok=True) - import shutil - shutil.copy2(yaml_path, dest_dir / "workflow.yml") - registry.add(definition.id, { - "name": definition.name, - "version": definition.version, - "description": definition.description, - "source": source_label, - }) - console.print(f"[green]✓[/green] Workflow '{definition.name}' ({definition.id}) installed") - - # Try as URL (http/https) - if source.startswith("http://") or source.startswith("https://"): + dest_file = dest_dir / "workflow.yml" + existed_before = dest_dir.is_dir() + + try: + staged_file = _stage_workflow_file(dest_dir) + except OSError as exc: + console.print( + f"[red]Error:[/red] Failed to install workflow " + f"'{_escape_markup(definition.id)}': {_escape_markup(str(exc))}" + ) + raise typer.Exit(1) + + try: + # Write the exact bytes parsed above so a concurrent source edit + # cannot desynchronize installed content from validated metadata. + staged_file.write_bytes(source_content) + staged_file.set_mode(source_mode) + except OSError as exc: + _safe_discard_staged_workflow_file(staged_file, dest_dir, existed_before) + console.print( + f"[red]Error:[/red] Failed to install workflow " + f"'{_escape_markup(definition.id)}': {_escape_markup(str(exc))}" + ) + raise typer.Exit(1) + + try: + transaction = _workflow_install_transaction(project_root) + with transaction: + transaction_existed_before = existed_before or dest_file.exists() + transaction_registry = _open_workflow_registry(project_root) + # Commit the staged copy onto dest_file via an atomic swap. A + # prior file is renamed aside so registry failure can restore it. + try: + backup_file = _commit_workflow_file( + staged_file, dest_file, transaction_existed_before + ) + except OSError as exc: + _safe_discard_staged_workflow_file( + staged_file, dest_dir, existed_before + ) + console.print( + f"[red]Error:[/red] Failed to install workflow " + f"'{_escape_markup(definition.id)}': " + f"{_escape_markup(str(exc))}" + ) + raise typer.Exit(1) + try: + entry = { + "name": definition.name, + "version": definition.version, + "description": definition.description, + "source": source_label, + } + existing = transaction_registry.get(definition.id) + if isinstance(existing, dict) and not existing.get( + "enabled", True + ): + entry["enabled"] = False + transaction_registry.add(definition.id, entry) + except (OSError, TypeError, ValueError) as exc: + _safe_rollback_committed_workflow_file( + dest_file, + dest_dir, + transaction_existed_before, + backup_file, + ) + console.print( + f"[red]Error:[/red] Failed to update workflow registry for " + f"'{_escape_markup(definition.id)}': " + f"{_escape_markup(str(exc))}" + ) + raise typer.Exit(1) + # Registry update succeeded while the transaction lock is held. + _discard_committed_backup_file(backup_file) + except typer.Exit: + _safe_discard_staged_workflow_file( + staged_file, dest_dir, existed_before + ) + raise + except OSError as exc: + _safe_discard_staged_workflow_file(staged_file, dest_dir, existed_before) + console.print( + f"[red]Error:[/red] Failed to lock workflow install " + f"'{_escape_markup(definition.id)}': {_escape_markup(str(exc))}" + ) + raise typer.Exit(1) + console.print( + f"[green]✓[/green] Workflow '{_escape_markup(definition.name)}' " + f"({_escape_markup(definition.id)}) installed" + ) + + # Explicit local install (mirrors `extension add --dev`). --dev takes + # precedence over --from so a URL that would be ignored is never fetched. + if dev: + dev_path = Path(source).expanduser() + if dev_path.is_file() and dev_path.suffix in (".yml", ".yaml"): + _validate_and_install_local(dev_path, str(dev_path)) + return + if dev_path.is_dir(): + dev_wf_file = dev_path / "workflow.yml" + if not dev_wf_file.is_file(): + console.print(f"[red]Error:[/red] No workflow.yml found in {_escape_markup(source)}") + raise typer.Exit(1) + _validate_and_install_local(dev_wf_file, str(dev_path)) + return + console.print( + "[red]Error:[/red] --dev source must be a workflow YAML file or a " + f"directory containing workflow.yml: {_escape_markup(source)}" + ) + raise typer.Exit(1) + + # Try as URL (http/https) — either the positional source is a URL, or an + # explicit --from URL names where to fetch it (mirrors `extension add --from`). + download_url = ( + from_url + if from_url is not None + else (source if source.startswith(("http://", "https://")) else None) + ) + if download_url is not None: from ipaddress import ip_address from urllib.parse import urlparse from specify_cli.authentication.http import open_url as _open_url try: - parsed_src = urlparse(source) + parsed_src = urlparse(download_url) except ValueError: - console.print(f"[red]Error:[/red] Invalid URL: {_escape_markup(source)}") + console.print(f"[red]Error:[/red] Invalid URL: {_escape_markup(download_url)}") raise typer.Exit(1) src_host = parsed_src.hostname or "" src_loopback = src_host == "localhost" @@ -656,20 +1517,48 @@ def _validate_and_install_local(yaml_path: Path, source_label: str) -> None: console.print("[red]Error:[/red] Only HTTPS URLs are allowed, except HTTP for localhost.") raise typer.Exit(1) + if from_url is not None: + from rich.panel import Panel + + safe_url = _escape_markup(from_url) + console.print() + console.print( + Panel( + "[bold]You are installing a workflow from an external URL " + "that is not\nlisted in any of your configured workflow " + "catalogs.[/bold]\n\n" + f"URL: {safe_url}\n\n" + "Only install workflows from sources you trust.", + title="[bold yellow]⚠ Untrusted Source[/bold yellow]", + border_style="yellow", + padding=(1, 2), + ) + ) + console.print() + if not typer.confirm("Continue with installation?", default=False): + console.print("Cancelled") + raise typer.Exit(0) + from specify_cli._github_http import resolve_github_release_asset_api_url as _resolve_gh_asset from specify_cli.authentication.http import github_provider_hosts as _github_provider_hosts _wf_url_extra_headers = None _resolved_wf_url = _resolve_gh_asset( - source, _open_url, timeout=30, github_hosts=_github_provider_hosts() + download_url, _open_url, timeout=30, github_hosts=_github_provider_hosts() ) if _resolved_wf_url: - source = _resolved_wf_url + download_url = _resolved_wf_url _wf_url_extra_headers = {"Accept": "application/octet-stream"} import tempfile + tmp_path: Path | None = None try: - with _open_url(source, timeout=30, extra_headers=_wf_url_extra_headers) as resp: + with _open_url( + download_url, + timeout=30, + extra_headers=_wf_url_extra_headers, + redirect_validator=_reject_insecure_download_redirect, + ) as resp: final_url = resp.geturl() final_parsed = urlparse(final_url) final_host = final_parsed.hostname or "" @@ -681,20 +1570,58 @@ def _validate_and_install_local(yaml_path: Path, source_label: str) -> None: # Redirect host is not an IP literal; keep loopback as determined above. pass if final_parsed.scheme != "https" and not (final_parsed.scheme == "http" and final_lb): - console.print(f"[red]Error:[/red] URL redirected to non-HTTPS: {final_url}") + console.print( + f"[red]Error:[/red] URL redirected to non-HTTPS: {_escape_markup(final_url)}" + ) raise typer.Exit(1) with tempfile.NamedTemporaryFile(suffix=".yml", delete=False) as tmp: - tmp.write(resp.read()) + # Assign tmp_path immediately: NamedTemporaryFile(delete=False) + # creates the file on disk right away, before any bytes are + # written, so a failure in the size-limited read below must + # still be able to find and remove it. tmp_path = Path(tmp.name) + tmp.write(_read_response_within_limit(resp)) except typer.Exit: raise except Exception as exc: - console.print(f"[red]Error:[/red] Failed to download workflow: {exc}") + if tmp_path is not None: + # A cleanup failure here must never replace/mask the + # original download error below with a raw, unhandled + # OSError -- warn about it and keep going, exactly like the + # later post-install finally cleanup does. + try: + tmp_path.unlink(missing_ok=True) + except OSError as cleanup_exc: + console.print( + "[yellow]Warning:[/yellow] Could not remove temporary " + f"download file {_escape_markup(str(tmp_path))}: " + f"{_escape_markup(str(cleanup_exc))}" + ) + console.print(f"[red]Error:[/red] Failed to download workflow: {_escape_markup(str(exc))}") raise typer.Exit(1) try: - _validate_and_install_local(tmp_path, source) + # When installed via --from, the positional argument names the + # workflow the user expects — enforce it like the catalog branch. + _validate_and_install_local( + tmp_path, + download_url, + expected_id=source if from_url else None, + ) finally: - tmp_path.unlink(missing_ok=True) + # Best-effort: _validate_and_install_local may already have + # committed the file + registry entry (success) or already + # raised its own clean typer.Exit (failure) by this point -- + # either way, a cleanup OSError here must never mask that + # outcome or surface as its own unhandled failure. Warn instead, + # same as the committed-backup cleanup above. + try: + tmp_path.unlink(missing_ok=True) + except OSError as exc: + console.print( + "[yellow]Warning:[/yellow] Could not remove temporary " + f"download file {_escape_markup(str(tmp_path))}: " + f"{_escape_markup(str(exc))}" + ) return # Try as a local file/directory @@ -705,40 +1632,86 @@ def _validate_and_install_local(yaml_path: Path, source_label: str) -> None: return elif source_path.is_dir(): wf_file = source_path / "workflow.yml" - if not wf_file.exists(): - console.print(f"[red]Error:[/red] No workflow.yml found in {source}") + if not wf_file.is_file(): + console.print(f"[red]Error:[/red] No workflow.yml found in {_escape_markup(source)}") raise typer.Exit(1) _validate_and_install_local(wf_file, str(source_path)) return # Try from catalog + _install_workflow_from_catalog(project_root, workflows_dir, source) + + +def _install_workflow_from_catalog( + project_root: Path, + workflows_dir: Path, + workflow_id: str, + expected_version: str | None = None, + expected_installed_version: str | None = None, +) -> None: + """Download, validate, and register a catalog workflow. + + Shared by ``workflow add`` and ``workflow update``. Raises ``typer.Exit`` + on any failure; the registry entry is only written on full success. + ``expected_version``, when given, rejects a downloaded workflow whose + version does not match the catalog version that triggered the install. + ``expected_installed_version``, when given by ``workflow update``, aborts + if another process changes the installed source or version before commit. + """ + from .catalog import WorkflowCatalog, WorkflowCatalogError + from .engine import WorkflowDefinition + + def versions_match(actual: object, expected: str) -> bool: + from packaging import version as pkg_version + + try: + return pkg_version.Version(str(actual)) == pkg_version.Version( + expected + ) + except pkg_version.InvalidVersion: + return str(actual) == expected + + safe_wf_id = _escape_markup(workflow_id) + catalog = WorkflowCatalog(project_root) try: - info = catalog.get_workflow_info(source) + info = catalog.get_workflow_info(workflow_id) except WorkflowCatalogError as exc: - console.print(f"[red]Error:[/red] {exc}") + console.print(f"[red]Error:[/red] {_escape_markup(str(exc))}") raise typer.Exit(1) if not info: - console.print(f"[red]Error:[/red] Workflow '{source}' not found in catalog") + console.print(f"[red]Error:[/red] Workflow '{safe_wf_id}' not found in catalog") raise typer.Exit(1) if not info.get("_install_allowed", True): - console.print(f"[yellow]Warning:[/yellow] Workflow '{source}' is from a discovery-only catalog") + console.print(f"[yellow]Warning:[/yellow] Workflow '{safe_wf_id}' is from a discovery-only catalog") console.print("Direct installation is not enabled for this catalog source.") raise typer.Exit(1) workflow_url = info.get("url") if not workflow_url: - console.print(f"[red]Error:[/red] Workflow '{source}' does not have an install URL in the catalog") + console.print(f"[red]Error:[/red] Workflow '{safe_wf_id}' does not have an install URL in the catalog") + raise typer.Exit(1) + if not isinstance(workflow_url, str): + # Untrusted catalog payload; a non-string would crash urlparse below. + console.print( + f"[red]Error:[/red] Workflow '{safe_wf_id}' has a malformed install URL." + ) raise typer.Exit(1) # Validate URL scheme (HTTPS required, HTTP allowed for localhost only) from ipaddress import ip_address from urllib.parse import urlparse - parsed_url = urlparse(workflow_url) - url_host = parsed_url.hostname or "" + try: + parsed_url = urlparse(workflow_url) + url_host = parsed_url.hostname or "" + except ValueError: + console.print( + f"[red]Error:[/red] Workflow '{safe_wf_id}' has a malformed install URL." + ) + raise typer.Exit(1) is_loopback = False if url_host == "localhost": is_loopback = True @@ -750,16 +1723,33 @@ def _validate_and_install_local(yaml_path: Path, source_label: str) -> None: pass if parsed_url.scheme != "https" and not (parsed_url.scheme == "http" and is_loopback): console.print( - f"[red]Error:[/red] Workflow '{source}' has an invalid install URL. " + f"[red]Error:[/red] Workflow '{safe_wf_id}' has an invalid install URL. " "Only HTTPS URLs are allowed, except HTTP for localhost/loopback." ) raise typer.Exit(1) # Reject path traversal, symlinked , and a symlinked workflow.yml leaf # before any mkdir/download writes beneath the install directory. - workflow_dir = _safe_workflow_id_dir(workflows_dir, source) + workflow_dir = _safe_workflow_id_dir(workflows_dir, workflow_id) workflow_file = workflow_dir / "workflow.yml" + # Captured before any mkdir/download writes so every failure branch below + # can tell a fresh install from a reinstall-over-an-existing-one, + # mirroring _validate_and_install_local's existed-before-aware cleanup. + existed_before = workflow_dir.is_dir() + + try: + staged_file = _stage_workflow_file( + workflow_dir, + use_project_file_mode=not workflow_file.exists(), + ) + except OSError as exc: + console.print( + f"[red]Error:[/red] Failed to install workflow '{safe_wf_id}' from catalog: " + f"{_escape_markup(str(exc))}" + ) + raise typer.Exit(1) + try: from specify_cli.authentication.http import open_url as _open_url from specify_cli.authentication.http import github_provider_hosts as _github_provider_hosts @@ -773,8 +1763,12 @@ def _validate_and_install_local(yaml_path: Path, source_label: str) -> None: workflow_url = _resolved_workflow_url _wf_cat_extra_headers = {"Accept": "application/octet-stream"} - workflow_dir.mkdir(parents=True, exist_ok=True) - with _open_url(workflow_url, timeout=30, extra_headers=_wf_cat_extra_headers) as response: + with _open_url( + workflow_url, + timeout=30, + extra_headers=_wf_cat_extra_headers, + redirect_validator=_reject_insecure_download_redirect, + ) as response: # Validate final URL after redirects final_url = response.geturl() final_parsed = urlparse(final_url) @@ -787,82 +1781,168 @@ def _validate_and_install_local(yaml_path: Path, source_label: str) -> None: # Host is not an IP literal (e.g., a regular hostname); treat as non-loopback. pass if final_parsed.scheme != "https" and not (final_parsed.scheme == "http" and final_loopback): - if workflow_dir.exists(): - import shutil - shutil.rmtree(workflow_dir, ignore_errors=True) + _safe_discard_staged_workflow_file(staged_file, workflow_dir, existed_before) console.print( - f"[red]Error:[/red] Workflow '{source}' redirected to non-HTTPS URL: {final_url}" + f"[red]Error:[/red] Workflow '{safe_wf_id}' redirected to non-HTTPS URL: {_escape_markup(final_url)}" ) raise typer.Exit(1) - workflow_file.write_bytes(response.read()) + # Written to the staging file, never workflow_file directly, so a + # reinstall's prior working copy is never touched until the + # atomic commit below runs. + downloaded_content = _read_response_within_limit(response) + staged_file.write_bytes(downloaded_content) + except typer.Exit: + raise except Exception as exc: - if workflow_dir.exists(): - import shutil - shutil.rmtree(workflow_dir, ignore_errors=True) - console.print(f"[red]Error:[/red] Failed to install workflow '{source}' from catalog: {exc}") + _safe_discard_staged_workflow_file(staged_file, workflow_dir, existed_before) + console.print(f"[red]Error:[/red] Failed to install workflow '{safe_wf_id}' from catalog: {_escape_markup(str(exc))}") raise typer.Exit(1) - # Validate the downloaded workflow before registering + # Validate the downloaded workflow (still staged, not yet committed) + # before registering. try: - definition = WorkflowDefinition.from_yaml(workflow_file) - except (ValueError, yaml.YAMLError) as exc: - import shutil - shutil.rmtree(workflow_dir, ignore_errors=True) - console.print(f"[red]Error:[/red] Downloaded workflow is invalid: {exc}") + definition = WorkflowDefinition.from_string( + downloaded_content.decode("utf-8") + ) + except (UnicodeDecodeError, ValueError, yaml.YAMLError) as exc: + _safe_discard_staged_workflow_file(staged_file, workflow_dir, existed_before) + console.print(f"[red]Error:[/red] Downloaded workflow is invalid: {_escape_markup(str(exc))}") raise typer.Exit(1) from .engine import validate_workflow errors = validate_workflow(definition) if errors: - import shutil - shutil.rmtree(workflow_dir, ignore_errors=True) + _safe_discard_staged_workflow_file(staged_file, workflow_dir, existed_before) console.print("[red]Error:[/red] Downloaded workflow validation failed:") for err in errors: - console.print(f" \u2022 {err}") + console.print(f" \u2022 {_escape_markup(str(err))}") raise typer.Exit(1) # Enforce that the workflow's internal ID matches the catalog key - if definition.id and definition.id != source: - import shutil - shutil.rmtree(workflow_dir, ignore_errors=True) + if definition.id and definition.id != workflow_id: + _safe_discard_staged_workflow_file(staged_file, workflow_dir, existed_before) console.print( - f"[red]Error:[/red] Workflow ID in YAML ({definition.id!r}) " - f"does not match catalog key ({source!r}). " + f"[red]Error:[/red] Workflow ID in YAML ({_escape_markup(repr(definition.id))}) " + f"does not match catalog key ({_escape_markup(repr(workflow_id))}). " f"The catalog entry may be misconfigured." ) raise typer.Exit(1) - registry.add(source, { - "name": definition.name or info.get("name", source), - "version": definition.version or info.get("version", "0.0.0"), - "description": definition.description or info.get("description", ""), - "source": "catalog", - "catalog_name": info.get("_catalog_name", ""), - "url": workflow_url, - }) - console.print(f"[green]✓[/green] Workflow '{info.get('name', source)}' installed from catalog") - + # A stale or misconfigured URL can serve a different version than the + # catalog advertised; without this check `update` would report success + # while leaving the old version installed (or even downgrading). + if expected_version is not None: + if not versions_match(definition.version, expected_version): + _safe_discard_staged_workflow_file(staged_file, workflow_dir, existed_before) + console.print( + f"[red]Error:[/red] Downloaded workflow version ({_escape_markup(str(definition.version))}) " + f"does not match the catalog version ({_escape_markup(expected_version)}). " + f"The catalog entry may be stale or misconfigured." + ) + raise typer.Exit(1) -@workflow_app.command("remove") -def workflow_remove( - workflow_id: str = typer.Argument(..., help="Workflow ID to uninstall"), -): - """Uninstall a workflow.""" - from .catalog import WorkflowRegistry + try: + transaction = _workflow_install_transaction(project_root) + with transaction: + transaction_existed_before = ( + existed_before or workflow_file.exists() + ) + transaction_registry = _open_workflow_registry(project_root) + if expected_installed_version is not None: + current = transaction_registry.get(workflow_id) + if ( + not isinstance(current, dict) + or current.get("source") != "catalog" + or not versions_match( + current.get("version"), expected_installed_version + ) + ): + console.print( + f"[yellow]Warning:[/yellow] Workflow '{safe_wf_id}' " + "changed during update; rerun the command to use its " + "current source and version." + ) + raise typer.Exit(1) + # Commit the staged download onto workflow_file via an atomic + # swap. A prior file is renamed aside for registry rollback. + try: + backup_file = _commit_workflow_file( + staged_file, workflow_file, transaction_existed_before + ) + except OSError as exc: + _safe_discard_staged_workflow_file( + staged_file, workflow_dir, existed_before + ) + console.print( + f"[red]Error:[/red] Failed to install workflow " + f"'{safe_wf_id}' from catalog: {_escape_markup(str(exc))}" + ) + raise typer.Exit(1) - project_root = _require_specify_project() - workflows_dir = project_root / ".specify" / "workflows" - _validate_workflow_id_or_exit(workflow_id) + entry = { + "name": definition.name or info.get("name", workflow_id), + "version": definition.version or info.get("version", "0.0.0"), + "description": definition.description + or info.get("description", ""), + "source": "catalog", + "catalog_name": info.get("_catalog_name", ""), + "url": workflow_url, + } + # Preserve a prior disabled state across updates/reinstalls. + existing = transaction_registry.get(workflow_id) + if isinstance(existing, dict) and not existing.get( + "enabled", True + ): + entry["enabled"] = False + try: + transaction_registry.add(workflow_id, entry) + except (OSError, TypeError, ValueError) as exc: + _safe_rollback_committed_workflow_file( + workflow_file, + workflow_dir, + transaction_existed_before, + backup_file, + ) + console.print( + f"[red]Error:[/red] Failed to update workflow registry for " + f"'{_escape_markup(workflow_id)}': " + f"{_escape_markup(str(exc))}" + ) + raise typer.Exit(1) + # Registry update succeeded while the transaction lock is held. + _discard_committed_backup_file(backup_file) + except typer.Exit: + _safe_discard_staged_workflow_file( + staged_file, workflow_dir, existed_before + ) + raise + except OSError as exc: + _safe_discard_staged_workflow_file(staged_file, workflow_dir, existed_before) + console.print( + f"[red]Error:[/red] Failed to lock workflow install " + f"'{safe_wf_id}': " + f"{_escape_markup(str(exc))}" + ) + raise typer.Exit(1) + console.print( + f"[green]✓[/green] Workflow '{_escape_markup(str(info.get('name', workflow_id)))}' " + "installed from catalog" + ) - registry = WorkflowRegistry(project_root) +def _remove_workflow_locked( + project_root: Path, workflows_dir: Path, workflow_id: str +) -> Path | None: + """Stage a workflow directory and persist removal while locked.""" + registry = _open_workflow_registry(project_root) + safe_id = _escape_markup(workflow_id) if not registry.is_installed(workflow_id): - console.print(f"[red]Error:[/red] Workflow '{workflow_id}' is not installed") + console.print( + f"[red]Error:[/red] Workflow '{safe_id}' is not installed" + ) raise typer.Exit(1) - # Remove workflow files workflow_dir_unresolved = workflows_dir / workflow_id - safe_id = _escape_markup(workflow_id) if workflow_dir_unresolved.is_symlink(): console.print( f"[red]Error:[/red] Refusing to remove symlinked " @@ -875,39 +1955,301 @@ def workflow_remove( rel_parts = workflow_dir.relative_to(workflows_dir.resolve()).parts except ValueError: console.print( - f"[red]Error:[/red] Invalid workflow ID: {_escape_markup(repr(workflow_id))}" + f"[red]Error:[/red] Invalid workflow ID: " + f"{_escape_markup(repr(workflow_id))}" ) raise typer.Exit(1) if rel_parts != (workflow_id,): console.print( - f"[red]Error:[/red] Invalid workflow ID: {_escape_markup(repr(workflow_id))}" + f"[red]Error:[/red] Invalid workflow ID: " + f"{_escape_markup(repr(workflow_id))}" ) raise typer.Exit(1) if workflow_dir.exists() and not workflow_dir.is_dir(): console.print( - f"[red]Error:[/red] .specify/workflows/{safe_id} exists but is not a directory" + f"[red]Error:[/red] .specify/workflows/{safe_id} exists " + "but is not a directory" ) raise typer.Exit(1) + import tempfile + + staged_dir: Path | None = None if workflow_dir.exists(): - import shutil try: - shutil.rmtree(workflow_dir) + reserved = Path( + tempfile.mkdtemp( + prefix=f".{workflow_id}.removing-", dir=workflows_dir + ) + ) + reserved.rmdir() + os.rename(workflow_dir, reserved) + staged_dir = reserved except OSError as exc: console.print( - f"[red]Error:[/red] Failed to remove workflow directory {workflow_dir}: {exc}" + f"[red]Error:[/red] Failed to stage workflow directory " + f"{_escape_markup(str(workflow_dir))} for removal: " + f"{_escape_markup(str(exc))}" ) raise typer.Exit(1) - registry.remove(workflow_id) + try: + registry.remove(workflow_id) + except OSError as exc: + if staged_dir is not None: + try: + os.rename(staged_dir, workflow_dir) + except OSError as restore_exc: + console.print( + f"[yellow]Warning:[/yellow] Failed to restore workflow " + "directory after registry update failure; it remains " + f"staged at {_escape_markup(str(staged_dir))}: " + f"{_escape_markup(str(restore_exc))}" + ) + console.print( + f"[red]Error:[/red] Failed to update workflow registry for " + f"'{safe_id}': {_escape_markup(str(exc))}" + ) + raise typer.Exit(1) + return staged_dir + + +@workflow_app.command("remove") +def workflow_remove( + workflow_id: str = typer.Argument(..., help="Workflow ID to uninstall"), +): + """Uninstall a workflow.""" + project_root = _require_specify_project() + workflows_dir = project_root / ".specify" / "workflows" + _validate_workflow_id_or_exit(workflow_id) + safe_id = _escape_markup(workflow_id) + import shutil + try: + with _workflow_install_transaction(project_root): + staged_dir = _remove_workflow_locked( + project_root, workflows_dir, workflow_id + ) + except OSError as exc: + console.print( + f"[red]Error:[/red] Failed to lock workflow removal " + f"'{safe_id}': {_escape_markup(str(exc))}" + ) + raise typer.Exit(1) + console.print(f"[green]✓[/green] Workflow '{workflow_id}' removed") + # The registry has already durably committed the removal at this point, + # so it must stand regardless of what happens below: deleting the staged + # directory is now just cleanup, not a data-integrity concern, and a + # failure here is reported as a warning (not an error) to avoid + # contradicting the registry state that already succeeded. + if staged_dir is not None: + try: + shutil.rmtree(staged_dir) + except OSError as exc: + console.print( + f"[yellow]Warning:[/yellow] Workflow '{safe_id}' was removed, but its " + f"staged directory could not be deleted: {_escape_markup(str(exc))}. " + f"Remove it manually: {_escape_markup(str(staged_dir))}" + ) + + +@workflow_app.command("update") +def workflow_update( + workflow_id: str | None = typer.Argument(None, help="Workflow ID to update (default: all)"), +): + """Update installed workflow(s) to the latest catalog version.""" + from packaging import version as pkg_version + + from .catalog import WorkflowCatalog, WorkflowCatalogError + + project_root = _require_specify_project() + registry = _open_workflow_registry(project_root) + workflows_dir = project_root / ".specify" / "workflows" + _reject_unsafe_dir(project_root / ".specify", ".specify") + _reject_unsafe_dir(workflows_dir, ".specify/workflows") + + installed = registry.list() + if workflow_id: + if not registry.is_installed(workflow_id): + console.print(f"[red]Error:[/red] Workflow '{_escape_markup(workflow_id)}' is not installed") + raise typer.Exit(1) + targets = [workflow_id] + else: + targets = list(installed) + + if not targets: + console.print("[yellow]No workflows installed[/yellow]") + raise typer.Exit(0) + + catalog = WorkflowCatalog(project_root) + console.print("🔄 Checking for updates...\n") + + updates_available: list[dict[str, str]] = [] + checked = 0 + for wf_id in targets: + safe_id = _escape_markup(str(wf_id)) + metadata = installed.get(wf_id) + if not isinstance(metadata, dict): + console.print(f"⚠ {safe_id}: Registry entry is corrupted (skipping)") + continue + if metadata.get("source") != "catalog": + console.print(f"⚠ {safe_id}: Not installed from a catalog — re-add to update (skipping)") + continue + try: + installed_version = pkg_version.Version(str(metadata.get("version"))) + except pkg_version.InvalidVersion: + console.print( + f"⚠ {safe_id}: Invalid installed version '{_escape_markup(str(metadata.get('version')))}' in registry (skipping)" + ) + continue + try: + info = catalog.get_workflow_info(wf_id) + except WorkflowCatalogError as exc: + console.print(f"[red]Error:[/red] {_escape_markup(str(exc))}") + raise typer.Exit(1) + if not info: + console.print(f"⚠ {safe_id}: Not found in catalog (skipping)") + continue + if not info.get("_install_allowed", True): + console.print( + f"⚠ {safe_id}: Updates not allowed from '{_escape_markup(str(info.get('_catalog_name', 'catalog')))}' (skipping)" + ) + continue + try: + catalog_version = pkg_version.Version(str(info.get("version"))) + except pkg_version.InvalidVersion: + console.print( + f"⚠ {safe_id}: Invalid catalog version '{_escape_markup(str(info.get('version')))}' (skipping)" + ) + continue + if catalog_version > installed_version: + checked += 1 + updates_available.append( + {"id": wf_id, "installed": str(installed_version), "available": str(catalog_version)} + ) + else: + checked += 1 + console.print(f"✓ {safe_id}: Up to date (v{installed_version})") + + if not updates_available: + if not checked: + console.print("\n[yellow]No workflows were eligible for update[/yellow]") + elif checked == len(targets): + console.print("\n[green]All workflows are up to date![/green]") + else: + console.print( + f"\n[green]All checked workflows are up to date[/green] " + f"[yellow]({len(targets) - checked} skipped)[/yellow]" + ) + raise typer.Exit(0) + + console.print("\n[bold]Updates available:[/bold]\n") + for update in updates_available: + console.print( + f" • {_escape_markup(update['id'])}: {update['installed']} → {update['available']}" + ) + console.print() + if not typer.confirm("Update these workflows?"): + console.print("Cancelled") + raise typer.Exit(0) + + console.print() + failed: list[str] = [] + for update in updates_available: + # _install_workflow_from_catalog is fully transactional (staged + # download, atomic commit, rename-based rollback on registry + # failure): it never leaves a partially-written workflow.yml, so + # this loop only needs to record success/failure, not perform its + # own backup/restore. + try: + _install_workflow_from_catalog( + project_root, workflows_dir, update["id"], + expected_version=update["available"], + expected_installed_version=update["installed"], + ) + except (typer.Exit, OSError) as exc: + if isinstance(exc, OSError): + console.print( + f"[red]Error:[/red] Filesystem error updating " + f"'{_escape_markup(update['id'])}': {_escape_markup(str(exc))}" + ) + failed.append(update["id"]) + + if failed: + console.print( + f"\n[red]Failed to update:[/red] {', '.join(_escape_markup(f) for f in failed)}" + ) + raise typer.Exit(1) + + +def _set_workflow_enabled(workflow_id: str, enabled: bool) -> None: + """Update enabled state from a fresh registry snapshot while locked.""" + project_root = _require_specify_project() + safe_id = _escape_markup(workflow_id) + try: + with _workflow_install_transaction(project_root): + registry = _open_workflow_registry(project_root) + metadata = registry.get(workflow_id) + if metadata is None: + console.print( + f"[red]Error:[/red] Workflow '{safe_id}' is not installed" + ) + raise typer.Exit(1) + if not isinstance(metadata, dict): + console.print( + f"[red]Error:[/red] Registry entry for '{safe_id}' " + "is corrupted" + ) + raise typer.Exit(1) + current = bool(metadata.get("enabled", True)) + state = "enabled" if enabled else "disabled" + if current is enabled: + console.print( + f"[yellow]Workflow '{safe_id}' is already {state}[/yellow]" + ) + raise typer.Exit(0) + try: + registry.add(workflow_id, {**metadata, "enabled": enabled}) + except OSError as exc: + console.print( + f"[red]Error:[/red] Failed to update workflow registry " + f"for '{safe_id}': {_escape_markup(str(exc))}" + ) + raise typer.Exit(1) + except OSError as exc: + console.print( + f"[red]Error:[/red] Failed to lock workflow registry for " + f"'{safe_id}': {_escape_markup(str(exc))}" + ) + raise typer.Exit(1) + state = "enabled" if enabled else "disabled" + console.print(f"[green]✓[/green] Workflow '{safe_id}' {state}") + + +@workflow_app.command("enable") +def workflow_enable( + workflow_id: str = typer.Argument(..., help="Workflow ID to enable"), +): + """Enable a disabled workflow.""" + _set_workflow_enabled(workflow_id, True) + + +@workflow_app.command("disable") +def workflow_disable( + workflow_id: str = typer.Argument(..., help="Workflow ID to disable"), +): + """Disable a workflow without removing it.""" + _set_workflow_enabled(workflow_id, False) + console.print(f"To re-enable: specify workflow enable {_escape_markup(workflow_id)}") + @workflow_app.command("search") def workflow_search( query: str | None = typer.Argument(None, help="Search query"), tag: str | None = typer.Option(None, "--tag", help="Filter by tag"), + author: str | None = typer.Option(None, "--author", help="Filter by author"), ): """Search workflow catalogs.""" from .catalog import WorkflowCatalog, WorkflowCatalogError @@ -916,9 +2258,9 @@ def workflow_search( catalog = WorkflowCatalog(project_root) try: - results = catalog.search(query=query, tag=tag) + results = catalog.search(query=query, tag=tag, author=author) except WorkflowCatalogError as exc: - console.print(f"[red]Error:[/red] {exc}") + console.print(f"[red]Error:[/red] {_escape_markup(str(exc))}") raise typer.Exit(1) if not results: @@ -927,13 +2269,17 @@ def workflow_search( console.print(f"\n[bold cyan]Workflows ({len(results)}):[/bold cyan]\n") for wf in results: - console.print(f" [bold]{wf.get('name', wf.get('id', '?'))}[/bold] ({wf.get('id', '?')}) v{wf.get('version', '?')}") + name = _escape_markup(str(wf.get("name", wf.get("id", "?")))) + wf_id = _escape_markup(str(wf.get("id", "?"))) + version = _escape_markup(str(wf.get("version", "?"))) + console.print(f" [bold]{name}[/bold] ({wf_id}) v{version}") desc = wf.get("description", "") if desc: - console.print(f" {desc}") + console.print(f" {_escape_markup(str(desc))}") tags = wf.get("tags", []) if tags: - console.print(f" [dim]Tags: {', '.join(tags)}[/dim]") + safe_tags = _escape_markup(", ".join(str(t) for t in tags)) + console.print(f" [dim]Tags: {safe_tags}[/dim]") console.print() @@ -942,13 +2288,13 @@ def workflow_info( workflow_id: str = typer.Argument(..., help="Workflow ID"), ): """Show workflow details and step graph.""" - from .catalog import WorkflowCatalog, WorkflowRegistry, WorkflowCatalogError + from .catalog import WorkflowCatalog, WorkflowCatalogError from .engine import WorkflowEngine project_root = _require_specify_project() # Check installed first - registry = WorkflowRegistry(project_root) + registry = _open_workflow_registry(project_root) installed = registry.get(workflow_id) engine = WorkflowEngine(project_root) @@ -1264,7 +2610,9 @@ def _safe_fetch(url: str) -> bytes: raise ValueError(f"Refusing to fetch from non-HTTPS URL: {url}") if not parsed.hostname: raise ValueError(f"Refusing to fetch from URL with no hostname: {url}") - with _open_url(url, timeout=30) as resp: + with _open_url( + url, timeout=30, redirect_validator=_reject_insecure_download_redirect + ) as resp: final_url = resp.geturl() final_parsed = urlparse(final_url) final_is_localhost = final_parsed.hostname in ("localhost", "127.0.0.1", "::1") @@ -1274,7 +2622,7 @@ def _safe_fetch(url: str) -> bytes: raise ValueError(f"Redirect to non-HTTPS URL: {final_url}") if not final_parsed.hostname: raise ValueError(f"Redirect to URL with no hostname: {final_url}") - return resp.read() + return _read_response_within_limit(resp) _validate_step_id_or_exit(step_id) diff --git a/src/specify_cli/workflows/catalog.py b/src/specify_cli/workflows/catalog.py index 97bf58a04e..7a156c92e4 100644 --- a/src/specify_cli/workflows/catalog.py +++ b/src/specify_cli/workflows/catalog.py @@ -13,6 +13,8 @@ import hashlib import json import os +import stat +import tempfile import time from dataclasses import dataclass from pathlib import Path @@ -71,40 +73,180 @@ def __init__(self, project_root: Path) -> None: self.registry_path = self.workflows_dir / self.REGISTRY_FILE self.data = self._load() + def _has_symlinked_parent(self) -> bool: + """Return True if any directory under .specify/workflows is a symlink.""" + current = self.project_root + for part in (".specify", "workflows"): + current = current / part + if current.is_symlink(): + return True + return False + def _load(self) -> dict[str, Any]: """Load registry from disk or create default.""" + default_registry: dict[str, Any] = { + "schema_version": self.SCHEMA_VERSION, + "workflows": {}, + } + # Defense-in-depth: refuse to read through symlinked parents or a + # symlinked registry file. Unlike StepRegistry (read-only best-effort + # elsewhere), a fabricated empty registry here is not safe: read-only + # callers (notably the bundler's remove path) query is_installed() + # before ever writing, and would otherwise conclude an installed + # workflow is absent, skip removing it, then delete the bundle + # record -- leaving the workflow untracked but still on disk. Fail + # closed here just like the unreadable-file case below. + if self._has_symlinked_parent() or self.registry_path.is_symlink(): + raise OSError( + f"Refusing to read workflow registry at {self.registry_path}: " + "a parent directory or the registry file itself is a symlink" + ) if self.registry_path.exists(): try: with open(self.registry_path, encoding="utf-8") as f: - return json.load(f) - except (json.JSONDecodeError, ValueError): - # Corrupted registry file — reset to default - return {"schema_version": self.SCHEMA_VERSION, "workflows": {}} - return {"schema_version": self.SCHEMA_VERSION, "workflows": {}} + data = json.load(f) + except OSError as exc: + # The real data may still be intact on disk. Fail closed at + # construction rather than fabricating an empty registry that + # a read-only caller could mistake for "nothing installed." + raise OSError( + f"Failed to read workflow registry at {self.registry_path}: {exc}" + ) from exc + except ( + json.JSONDecodeError, + ValueError, + UnicodeError, + ) as exc: + raise OSError( + f"Workflow registry at {self.registry_path} is corrupted: " + f"{exc}" + ) from exc + # Validate shape: must be a dict with a dict "workflows" field. + if not isinstance(data, dict): + raise OSError( + f"Workflow registry at {self.registry_path} is corrupted: " + "top-level value must be an object" + ) + if not isinstance(data.get("workflows"), dict): + raise OSError( + f"Workflow registry at {self.registry_path} is corrupted: " + "'workflows' must be an object" + ) + return data + return default_registry def save(self) -> None: - """Persist registry to disk.""" + """Persist registry to disk atomically.""" + # Refuse to write through symlinked parents (mirrors StepRegistry.save + # and the CLI-level _reject_unsafe_dir guard). + if self._has_symlinked_parent() or self.registry_path.is_symlink(): + raise OSError( + "Refusing to write workflow registry through a symlinked path." + ) self.workflows_dir.mkdir(parents=True, exist_ok=True) - with open(self.registry_path, "w", encoding="utf-8") as f: - json.dump(self.data, f, indent=2) + # Unique, exclusive temp then replace: a failed dump cannot truncate + # the registry, a pre-created symlink cannot redirect the write, and + # concurrent CLI processes cannot collide on the same temp path. + fd, tmp = tempfile.mkstemp( + dir=str(self.registry_path.parent), + prefix=f".{self.registry_path.name}.", + suffix=".tmp", + ) + try: + # Write through a duplicate so the exclusive mkstemp descriptor + # stays open for fd-based metadata updates and inode verification. + with os.fdopen(os.dup(fd), "w", encoding="utf-8") as f: + json.dump(self.data, f, indent=2) + # mkstemp creates the temp file at 0600. A pre-existing registry + # may be shared more permissively (e.g. 0640/0644); preserve its + # mode across the replace so a save doesn't silently lock other + # project users out. A brand-new registry has no prior mode to + # preserve, so mkstemp's secure 0600 default stands. Mirrors + # _utils.py's atomic_write_json (best-effort; data safety over + # metadata preservation). + try: + if self.registry_path.exists(): + existing_stat = self.registry_path.stat( + follow_symlinks=False + ) + if stat.S_ISREG(existing_stat.st_mode) and hasattr( + os, "fchmod" + ): + os.fchmod(fd, stat.S_IMODE(existing_stat.st_mode)) + if stat.S_ISREG(existing_stat.st_mode) and hasattr( + os, "fchown" + ): + try: + os.fchown( + fd, existing_stat.st_uid, existing_stat.st_gid + ) + except PermissionError: + pass + except OSError: + pass + staged_stat = os.stat(tmp, follow_symlinks=False) + open_stat = os.fstat(fd) + if ( + not stat.S_ISREG(staged_stat.st_mode) + or staged_stat.st_dev != open_stat.st_dev + or staged_stat.st_ino != open_stat.st_ino + ): + raise OSError( + "Refusing to replace workflow registry: " + "staged file changed before commit" + ) + os.close(fd) + fd = -1 + os.replace(tmp, self.registry_path) + except BaseException: + if fd >= 0: + try: + os.close(fd) + except OSError: + pass + try: + os.unlink(tmp) + except OSError: + pass + raise def add(self, workflow_id: str, metadata: dict[str, Any]) -> None: """Add or update an installed workflow entry.""" from datetime import datetime, timezone - existing = self.data["workflows"].get(workflow_id, {}) + raw_existing = self.data["workflows"].get(workflow_id) + had_entry = workflow_id in self.data["workflows"] + # Corrupted-but-parseable registries may hold non-dict entries. + existing = raw_existing if isinstance(raw_existing, dict) else {} metadata["installed_at"] = existing.get( "installed_at", datetime.now(timezone.utc).isoformat() ) metadata["updated_at"] = datetime.now(timezone.utc).isoformat() self.data["workflows"][workflow_id] = metadata - self.save() + try: + self.save() + except (OSError, TypeError, ValueError): + # Roll back the in-memory mutation so a later successful save + # cannot persist metadata for a write that failed. + if had_entry: + self.data["workflows"][workflow_id] = raw_existing + else: + del self.data["workflows"][workflow_id] + raise def remove(self, workflow_id: str) -> bool: """Remove an installed workflow entry. Returns True if found.""" if workflow_id in self.data["workflows"]: + removed_entry = self.data["workflows"][workflow_id] del self.data["workflows"][workflow_id] - self.save() + try: + self.save() + except OSError: + # Roll back the in-memory deletion so a save failure can't + # desync this instance from the untouched file on disk, + # mirroring add()'s rollback-on-save-failure. + self.data["workflows"][workflow_id] = removed_entry + raise return True return False @@ -427,6 +569,7 @@ def search( self, query: str | None = None, tag: str | None = None, + author: str | None = None, ) -> list[dict[str, Any]]: """Search workflows across all configured catalogs.""" merged = self._get_merged_workflows() @@ -451,6 +594,10 @@ def search( normalized_tags = [t.lower() for t in tags if isinstance(t, str)] if tag.lower() not in normalized_tags: continue + if author: + wf_author = wf_data.get("author", "") + if not isinstance(wf_author, str) or wf_author.lower() != author.lower(): + continue results.append(wf_data) return results diff --git a/src/specify_cli/workflows/engine.py b/src/specify_cli/workflows/engine.py index 6025c30c5c..969d2b664d 100644 --- a/src/specify_cli/workflows/engine.py +++ b/src/specify_cli/workflows/engine.py @@ -428,6 +428,8 @@ def __init__( run_id: str | None = None, workflow_id: str = "", project_root: Path | None = None, + installed_workflow_id: str | None = None, + installed_registry_root: str | None = None, ) -> None: # ``run_id is None`` (omitted) → auto-generate. An explicit empty # string is *not* the same as "omitted" and must be validated like @@ -441,6 +443,15 @@ def __init__( self._validate_run_id(self.run_id) self.workflow_id = workflow_id self.project_root = project_root or Path(".") + # Identifies the installed workflow (if any) this run was started + # from, and the project root that owns its registry — set by + # execute() when the source was resolved to an installed ID (see + # workflow_run's ownership mapping). None for a direct/non-installed + # YAML source, and for any run persisted before this field existed + # (load() defaults it to None), preserving old runs' existing + # resume behavior unchanged. + self.installed_workflow_id = installed_workflow_id + self.installed_registry_root = installed_registry_root self.status = RunStatus.CREATED self.current_step_index = 0 self.current_step_id: str | None = None @@ -503,6 +514,8 @@ def save(self) -> None: state_data = { "run_id": self.run_id, "workflow_id": self.workflow_id, + "installed_workflow_id": self.installed_workflow_id, + "installed_registry_root": self.installed_registry_root, "status": self.status.value, "current_step_index": self.current_step_index, "current_step_id": self.current_step_id, @@ -555,10 +568,25 @@ def load(cls, run_id: str, project_root: Path) -> RunState: with open(state_path, encoding="utf-8") as f: state_data = json.load(f) + installed_workflow_id = state_data.get("installed_workflow_id") + if installed_workflow_id is not None and not isinstance(installed_workflow_id, str): + raise ValueError( + "Invalid run state: 'installed_workflow_id' must be a " + f"string or null, got {type(installed_workflow_id).__name__}" + ) + installed_registry_root = state_data.get("installed_registry_root") + if installed_registry_root is not None and not isinstance(installed_registry_root, str): + raise ValueError( + "Invalid run state: 'installed_registry_root' must be a " + f"string or null, got {type(installed_registry_root).__name__}" + ) + state = cls( run_id=state_data["run_id"], workflow_id=state_data["workflow_id"], project_root=project_root, + installed_workflow_id=installed_workflow_id, + installed_registry_root=installed_registry_root, ) state.status = RunStatus(state_data["status"]) state.current_step_index = state_data.get("current_step_index", 0) @@ -654,6 +682,8 @@ def execute( definition: WorkflowDefinition, inputs: dict[str, Any] | None = None, run_id: str | None = None, + installed_workflow_id: str | None = None, + installed_registry_root: Path | None = None, ) -> RunState: """Execute a workflow definition. @@ -665,6 +695,12 @@ def execute( User-provided input values. run_id: Optional run ID (uses SPECKIT_WORKFLOW_RUN_ID when set, otherwise auto-generated). + installed_workflow_id, installed_registry_root: + When the run was started from an installed workflow (as opposed + to a direct/non-installed YAML source), identifies it and its + owning registry root so a later ``resume`` can re-check the + registry's current disabled state before continuing — see + ``workflow_resume``. Returns ------- @@ -682,6 +718,12 @@ def execute( run_id=effective_run_id, workflow_id=definition.id, project_root=self.project_root, + installed_workflow_id=installed_workflow_id, + installed_registry_root=( + str(installed_registry_root) + if installed_registry_root is not None + else None + ), ) # Persist a copy of the workflow definition so resume can diff --git a/tests/contract/test_bundle_cli.py b/tests/contract/test_bundle_cli.py index 1705c5945d..f76fafe32c 100644 --- a/tests/contract/test_bundle_cli.py +++ b/tests/contract/test_bundle_cli.py @@ -63,6 +63,42 @@ def test_commands_outside_project_fail_with_guidance(tmp_path: Path, monkeypatch assert "Spec Kit project" in result.output +def test_remove_reports_clean_error_when_primitive_raises_raw_exception( + project: Path, +): + """A raw exception from a primitive installer (e.g. an OSError from an + unreadable workflow registry surfacing through _WorkflowKindManager's + fail-closed construction) must not propagate uncaught through + `specify bundle remove` -- the command only catches BundlerError, so + without a conversion at the remove_bundle boundary this would exit + with an unhandled exception and empty/raw output instead of a clean, + actionable message, and no removal side effects should occur either.""" + from specify_cli.bundler.models.manifest import BundleManifest + from specify_cli.bundler.models.records import load_records + from specify_cli.bundler.services.adapters import DefaultPrimitiveInstaller + from specify_cli.bundler.services.installer import install_bundle + from specify_cli.bundler.services.resolver import resolve_install_plan + from tests.bundler_helpers import FakeInstaller + + manifest = BundleManifest.from_dict(valid_manifest_dict()) + plan = resolve_install_plan( + manifest, speckit_version="0.11.2", active_integration="copilot" + ) + install_bundle(project, plan, FakeInstaller(), manifest=manifest) + + def boom(self, project_root, component): + raise OSError("workflow registry unreadable") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(DefaultPrimitiveInstaller, "is_installed", boom) + result = runner.invoke(app, ["bundle", "remove", "demo-bundle"]) + + assert result.exit_code != 0 + assert result.output.strip() != "" + assert result.exception is None or isinstance(result.exception, SystemExit) + assert {r.bundle_id for r in load_records(project)} == {"demo-bundle"} + + def test_fail_writes_error_to_stderr_not_stdout(capsys): """_fail must write to stderr, not stdout: every bundle command routes errors through it, and under --json the error would otherwise corrupt the JSON payload diff --git a/tests/integration/test_bundler_install_flow.py b/tests/integration/test_bundler_install_flow.py index bf066b660e..b4d91ec05b 100644 --- a/tests/integration/test_bundler_install_flow.py +++ b/tests/integration/test_bundler_install_flow.py @@ -97,6 +97,207 @@ def test_remove_unknown_bundle_errors(tmp_path: Path): remove_bundle(tmp_path, "ghost", FakeInstaller()) +def test_remove_converts_raw_installer_exception_to_bundler_error(tmp_path: Path): + """A raw exception from a primitive installer (e.g. an OSError from an + unreadable workflow registry surfacing through _WorkflowKindManager's + fail-closed construction) must not propagate uncaught out of + remove_bundle: install_bundle already converts any non-BundlerError + exception into a clean BundlerError, but remove_bundle had no such + conversion, so the CLI's `bundle remove` (which only catches + BundlerError) would let a raw exception through with no clean message + and no removal side effects should occur either.""" + make_project(tmp_path) + manifest = BundleManifest.from_dict(valid_manifest_dict()) + installer = FakeInstaller() + install_bundle(tmp_path, _plan(manifest), installer, manifest=manifest) + + def boom(project_root, component): + raise OSError("workflow registry unreadable") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(installer, "is_installed", boom) + with pytest.raises(BundlerError): + remove_bundle(tmp_path, "demo-bundle", installer) + + # No removal side effects: the bundle record must still be present. + assert {r.bundle_id for r in load_records(tmp_path)} == {"demo-bundle"} + + +def test_remove_partial_failure_message_reflects_partial_state(tmp_path: Path): + """A failure can occur after earlier components in the same bundle have + already been removed from disk. The bundle record is left unchanged + (save_records never runs on this path), so it still claims the bundle + fully installed -- but the message must not claim "No changes were + recorded" when components were, in fact, already removed.""" + make_project(tmp_path) + manifest = BundleManifest.from_dict(valid_manifest_dict()) + installer = FakeInstaller() + install_bundle(tmp_path, _plan(manifest), installer, manifest=manifest) + + real_remove = installer.remove + calls = {"n": 0} + + def remove_then_fail(project_root, component): + calls["n"] += 1 + if calls["n"] == 1: + return real_remove(project_root, component) + raise OSError("disk full") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(installer, "remove", remove_then_fail) + with pytest.raises(BundlerError) as exc_info: + remove_bundle(tmp_path, "demo-bundle", installer) + + message = str(exc_info.value) + assert "no changes were recorded" not in message.lower() + assert {r.bundle_id for r in load_records(tmp_path)} == {"demo-bundle"} + + +def test_remove_bundler_error_from_installer_after_partial_removal_reports_partial_state( + tmp_path: Path, +): + """If the primitive installer itself raises BundlerError (not a raw/ + unexpected exception) after an earlier component in the same bundle was + already removed, the surfaced message must still carry the same + partial-removal detail as the generic-exception path -- a bare + ``except BundlerError: raise`` would re-raise the installer's original + message verbatim with no mention that the project may now be partially + uninstalled.""" + make_project(tmp_path) + manifest = BundleManifest.from_dict(valid_manifest_dict()) + installer = FakeInstaller() + install_bundle(tmp_path, _plan(manifest), installer, manifest=manifest) + + real_remove = installer.remove + calls = {"n": 0} + + def remove_then_raise_bundler_error(project_root, component): + calls["n"] += 1 + if calls["n"] == 1: + return real_remove(project_root, component) + raise BundlerError("kind manager refused removal") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(installer, "remove", remove_then_raise_bundler_error) + with pytest.raises(BundlerError) as exc_info: + remove_bundle(tmp_path, "demo-bundle", installer) + + message = str(exc_info.value) + assert "no changes were recorded" not in message.lower() + assert "kind manager refused removal" in message + assert "partially uninstalled" in message.lower() + assert {r.bundle_id for r in load_records(tmp_path)} == {"demo-bundle"} + + +def test_remove_bundler_error_from_installer_with_zero_removed_reports_no_changes( + tmp_path: Path, +): + """When the installer raises BundlerError before anything was actually + removed, the message should not misleadingly claim partial state.""" + make_project(tmp_path) + manifest = BundleManifest.from_dict(valid_manifest_dict()) + installer = FakeInstaller() + install_bundle(tmp_path, _plan(manifest), installer, manifest=manifest) + + def boom(project_root, component): + raise BundlerError("kind manager unavailable") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(installer, "is_installed", boom) + with pytest.raises(BundlerError) as exc_info: + remove_bundle(tmp_path, "demo-bundle", installer) + + message = str(exc_info.value) + assert "no components were removed" in message.lower() + assert "no removal was attempted" in message.lower() + assert "partially uninstalled" not in message.lower() + assert "kind manager unavailable" in message + assert {r.bundle_id for r in load_records(tmp_path)} == {"demo-bundle"} + + +def test_remove_zero_completed_removals_still_cautions_about_partial_changes( + tmp_path: Path, +): + """`result.uninstalled` only records a component after its `remove()` + call returns successfully. If the very first `remove()` call itself + raises after already deleting some files, zero completed removals are + recorded even though the project may already be partially uninstalled -- + the zero-count message must not claim "No components were removed" as + an unqualified fact; it must caution that the failing component may + have made partial changes before raising.""" + make_project(tmp_path) + manifest = BundleManifest.from_dict(valid_manifest_dict()) + installer = FakeInstaller() + install_bundle(tmp_path, _plan(manifest), installer, manifest=manifest) + + def boom(project_root, component): + # Simulates a remove() that deletes some files before raising -- + # from the caller's perspective this component was never recorded + # as completed, but disk state may already be partially changed. + raise OSError("disk full partway through removal") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(installer, "remove", boom) + with pytest.raises(BundlerError) as exc_info: + remove_bundle(tmp_path, "demo-bundle", installer) + + message = str(exc_info.value) + assert "no components were removed" in message.lower() + assert "partial" in message.lower() + assert "partially uninstalled" in message.lower() + assert {r.bundle_id for r in load_records(tmp_path)} == {"demo-bundle"} + + +def test_remove_record_save_failure_reports_partial_state(tmp_path: Path): + make_project(tmp_path) + manifest = BundleManifest.from_dict(valid_manifest_dict()) + installer = FakeInstaller() + install_bundle(tmp_path, _plan(manifest), installer, manifest=manifest) + + def fail_save(*_args, **_kwargs): + raise OSError("disk full") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "specify_cli.bundler.services.installer.save_records", + fail_save, + ) + with pytest.raises(BundlerError) as exc_info: + remove_bundle(tmp_path, "demo-bundle", installer) + + message = str(exc_info.value) + assert "disk full" in message + assert "partially uninstalled" in message.lower() + assert installer.installed == set() + assert {r.bundle_id for r in load_records(tmp_path)} == {"demo-bundle"} + + +def test_remove_record_save_failure_without_remove_attempt_is_not_partial( + tmp_path: Path, +): + make_project(tmp_path) + manifest = BundleManifest.from_dict(valid_manifest_dict()) + installer = FakeInstaller() + install_bundle(tmp_path, _plan(manifest), installer, manifest=manifest) + installer.installed.clear() + + def fail_save(*_args, **_kwargs): + raise OSError("disk full") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "specify_cli.bundler.services.installer.save_records", + fail_save, + ) + with pytest.raises(BundlerError) as exc_info: + remove_bundle(tmp_path, "demo-bundle", installer) + + message = str(exc_info.value) + assert "no removal was attempted" in message.lower() + assert "partially uninstalled" not in message.lower() + assert {r.bundle_id for r in load_records(tmp_path)} == {"demo-bundle"} + + def test_remove_reports_uninstalled_not_installed(tmp_path: Path): make_project(tmp_path) manifest = BundleManifest.from_dict(valid_manifest_dict()) @@ -128,7 +329,7 @@ def test_remove_counts_only_components_actually_removed(tmp_path: Path): assert len(result.uninstalled) == 3 assert (gone.kind, gone.id) not in installer.remove_calls - assert gone in result.skipped + assert gone not in result.skipped make_project(tmp_path) manifest = BundleManifest.from_dict(valid_manifest_dict()) installer = FakeInstaller() diff --git a/tests/test_workflows.py b/tests/test_workflows.py index d7cff20f6d..9a6d4424af 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -15,6 +15,7 @@ import json import os import shutil +import stat import sys import tempfile from pathlib import Path @@ -4716,6 +4717,29 @@ def test_remove(self, project_dir): registry.remove("test-wf") assert not registry.is_installed("test-wf") + def test_remove_rolls_back_in_memory_on_save_failure(self, project_dir, monkeypatch): + """A save() failure during remove() must not leave the in-memory registry + out of sync with the (unchanged) file on disk, mirroring add()'s rollback.""" + from specify_cli.workflows.catalog import WorkflowRegistry + import specify_cli.workflows.catalog as catalog_mod + + registry = WorkflowRegistry(project_dir) + registry.add("test-wf", {"name": "Test", "version": "1.0.0"}) + + def boom(*args, **kwargs): + raise OSError("disk full") + + monkeypatch.setattr(catalog_mod.json, "dump", boom) + with pytest.raises(OSError): + registry.remove("test-wf") + monkeypatch.undo() + + # In-memory state must still show the entry (rolled back), matching + # the untouched file on disk. + assert registry.is_installed("test-wf") + fresh = WorkflowRegistry(project_dir) + assert fresh.is_installed("test-wf") + def test_list(self, project_dir): from specify_cli.workflows.catalog import WorkflowRegistry @@ -4746,6 +4770,84 @@ def test_persistence(self, project_dir): registry2 = WorkflowRegistry(project_dir) assert registry2.is_installed("test-wf") + def test_load_read_oserror_refuses_to_save_over_existing_data(self, project_dir, monkeypatch): + """A transient read failure (e.g. temporarily unreadable file) must not be + treated the same as a corrupted/missing registry: constructing a registry + on top of it -- and thus any query a caller makes before ever calling + save() -- must fail closed instead of silently reporting an empty + registry that a caller could then act on and overwrite.""" + from specify_cli.workflows.catalog import WorkflowRegistry + import builtins + + registry1 = WorkflowRegistry(project_dir) + registry1.add("test-wf", {"name": "Test", "version": "1.0.0"}) + registry_path = registry1.registry_path + real_open = builtins.open + + def _raising_open(file, mode="r", *args, **kwargs): + if Path(file) == registry_path and "r" in mode: + raise OSError("simulated read failure") + return real_open(file, mode, *args, **kwargs) + + monkeypatch.setattr(builtins, "open", _raising_open) + with pytest.raises(OSError): + WorkflowRegistry(project_dir) + # The original entry must survive on disk untouched. + data = json.loads(registry_path.read_text(encoding="utf-8")) + assert "test-wf" in data["workflows"] + + def test_load_read_oserror_fails_closed_not_silently_empty(self, project_dir, monkeypatch): + """Root cause: a registry that failed to read must never let a query + method (is_installed/get/list) report as if nothing were installed -- + a caller (e.g. bundled workflow install) that only checks + is_installed() before writing a file would otherwise overwrite real + data on a transient read failure, long before any save() call could + catch it. The failure must surface at construction, before any query + or side effect is possible.""" + from specify_cli.workflows.catalog import WorkflowRegistry + import builtins + + registry1 = WorkflowRegistry(project_dir) + registry1.add("test-wf", {"name": "Test", "version": "1.0.0"}) + registry_path = registry1.registry_path + real_open = builtins.open + + def _raising_open(file, mode="r", *args, **kwargs): + if Path(file) == registry_path and "r" in mode: + raise OSError("simulated read failure") + return real_open(file, mode, *args, **kwargs) + + monkeypatch.setattr(builtins, "open", _raising_open) + with pytest.raises(OSError): + WorkflowRegistry(project_dir) + + @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable") + def test_load_symlinked_workflows_dir_fails_closed_not_silently_empty( + self, project_dir + ): + """A symlinked .specify/workflows is the same fail-open hazard as a + read OSError: silently reporting an empty registry lets a read-only + caller (e.g. the bundler's remove path) conclude a workflow isn't + installed, skip removing it, and then delete the bundle record -- + leaving the workflow untracked but still on disk. Raise here too, + exactly like the unreadable-file case, so callers cannot act on + fabricated empty state.""" + from specify_cli.workflows.catalog import WorkflowRegistry + import json as _json + + outside = project_dir.parent / "outside-workflows" + outside.mkdir(parents=True, exist_ok=True) + (outside / "workflow-registry.json").write_text( + _json.dumps({"schema_version": "1.0", "workflows": {"evil": {}}}), + encoding="utf-8", + ) + workflows_link = project_dir / ".specify" / "workflows" + workflows_link.rmdir() + workflows_link.symlink_to(outside, target_is_directory=True) + + with pytest.raises(OSError): + WorkflowRegistry(project_dir) + # ===== Workflow Catalog Tests ===== @@ -5853,6 +5955,142 @@ def test_remove_refuses_non_directory_workflow_path(self, project_dir, monkeypat assert workflow_path.read_text(encoding="utf-8") == "not a directory" assert WorkflowRegistry(project_dir).is_installed("test-wf") + def test_remove_registry_save_failure_preserves_files_and_registry( + self, project_dir, monkeypatch + ): + """If persisting the registry removal fails, the workflow's files must + not have already been deleted: the CLI must not delete files before the + registry successfully records the removal, and it must fail cleanly.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + registry = WorkflowRegistry(project_dir) + registry.add("test-wf", {"name": "Test", "version": "1.0.0"}) + workflow_dir = project_dir / ".specify" / "workflows" / "test-wf" + workflow_dir.mkdir(parents=True, exist_ok=True) + (workflow_dir / "workflow.yml").write_text("keep-me", encoding="utf-8") + + def boom(self): + raise OSError("disk full") + + monkeypatch.chdir(project_dir) + with pytest.MonkeyPatch.context() as mp: + mp.setattr(WorkflowRegistry, "save", boom) + result = CliRunner().invoke(app, ["workflow", "remove", "test-wf"]) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert result.output.strip() != "" + # Files must survive a registry-save failure. + assert (workflow_dir / "workflow.yml").read_text(encoding="utf-8") == "keep-me" + # The on-disk registry must still claim the workflow installed. + assert WorkflowRegistry(project_dir).is_installed("test-wf") + # The directory must be restored to its exact original location, with + # no leftover staging directory from the stage/restore-on-failure + # sequence. + entries = [ + p.name + for p in (project_dir / ".specify" / "workflows").iterdir() + if p.name != "workflow-registry.json" + ] + assert entries == ["test-wf"] + + def test_remove_staged_cleanup_failure_reports_warning_not_error( + self, project_dir, monkeypatch + ): + """The directory is staged (atomically renamed out of + .specify/workflows/) *before* the registry write, and the actual + deletion of the staged directory only happens *after* the registry + has already durably recorded the removal. If that final deletion + fails, the registry write already succeeded and must stand -- an + "Error: Failed to remove..." message at that point would contradict + the registry, which is exactly the incoherent state this staging + order exists to prevent. It must be reported as a cleanup warning, + and the command must still succeed.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + registry = WorkflowRegistry(project_dir) + registry.add("test-wf", {"name": "Test", "version": "1.0.0"}) + workflow_dir = project_dir / ".specify" / "workflows" / "test-wf" + workflow_dir.mkdir(parents=True, exist_ok=True) + (workflow_dir / "workflow.yml").write_text("keep-me", encoding="utf-8") + + def boom(*args, **kwargs): + raise OSError("permission denied") + + monkeypatch.chdir(project_dir) + with pytest.MonkeyPatch.context() as mp: + mp.setattr("shutil.rmtree", boom) + result = CliRunner().invoke(app, ["workflow", "remove", "test-wf"]) + + assert result.exit_code == 0 + assert "Warning" in result.output + # The registry write already committed -- it must stand. + assert not WorkflowRegistry(project_dir).is_installed("test-wf") + # The original install path is gone (staged away before the registry + # write ever ran); only a leftover staged directory remains, never + # at the original path the registry/CLI would treat as installed. + assert not workflow_dir.exists() + leftovers = [ + p + for p in (project_dir / ".specify" / "workflows").iterdir() + if p.name != "workflow-registry.json" + ] + assert len(leftovers) == 1 + assert (leftovers[0] / "workflow.yml").read_text(encoding="utf-8") == "keep-me" + + def test_remove_stage_restore_failure_escapes_rich_markup( + self, temp_dir, monkeypatch + ): + """When the registry write fails (already rolled back in-memory by + WorkflowRegistry.remove()) and the attempt to rename the staged + directory back to its original location also fails, both the + restore exception and the registry-update exception interpolated + into these warning/error messages must be escaped like every other + error path here.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + project_dir = temp_dir / "weird[project]" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + (project_dir / ".specify" / "workflows").mkdir() + + registry = WorkflowRegistry(project_dir) + registry.add("test-wf", {"name": "Test", "version": "1.0.0"}) + workflow_dir = project_dir / ".specify" / "workflows" / "test-wf" + workflow_dir.mkdir(parents=True, exist_ok=True) + (workflow_dir / "workflow.yml").write_text("keep-me", encoding="utf-8") + + def save_boom(self): + raise OSError("[reg] disk full") + + real_rename = os.rename + rename_calls = {"n": 0} + + def rename_boom(src, dst): + rename_calls["n"] += 1 + if rename_calls["n"] == 1: + # Allow the initial stage-out rename to succeed so the + # restore-back rename (the second call) is what fails. + return real_rename(src, dst) + raise OSError("[stage] permission denied") + + monkeypatch.chdir(project_dir) + with pytest.MonkeyPatch.context() as mp: + mp.setattr(WorkflowRegistry, "save", save_boom) + mp.setattr(os, "rename", rename_boom) + result = CliRunner().invoke(app, ["workflow", "remove", "test-wf"]) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + output_compact = "".join(result.output.split()) + assert "[stage]permissiondenied" in output_compact + assert "[reg]diskfull" in output_compact class TestWorkflowAddSymlinkGuard: def test_add_malformed_ipv6_url_exits_cleanly(self, temp_dir, monkeypatch): @@ -5905,6 +6143,34 @@ def test_add_refuses_symlinked_workflows_dir(self, temp_dir, monkeypatch): assert result.exit_code != 0 assert "symlinked .specify/workflows" in result.output + def test_add_escapes_rich_markup_in_validation_errors(self, temp_dir, monkeypatch): + """User-controlled YAML values in validation errors must not be parsed as Rich markup.""" + from typer.testing import CliRunner + from specify_cli import app + + (temp_dir / ".specify" / "workflows").mkdir(parents=True) + src = temp_dir / "incoming.yml" + src.write_text( + """ +schema_version: "1.0" +workflow: + id: "markup-wf" + name: "Markup" + version: "[bold]bad[/bold]" + +steps: + - id: step-one + command: speckit.specify +""", + encoding="utf-8", + ) + + monkeypatch.chdir(temp_dir) + result = CliRunner().invoke(app, ["workflow", "add", str(src)]) + + assert result.exit_code != 0 + assert "[bold]bad[/bold]" in result.output + @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable") def test_add_refuses_symlinked_id_dir(self, temp_dir, monkeypatch, sample_workflow_yaml): """A symlinked install dir must not let a copy escape the project root.""" @@ -6085,6 +6351,71 @@ def _fake_get_step_info(self, step_id): assert result.exit_code != 0 assert "Refusing to use symlinked step directory" in result.output + def test_add_rejects_oversized_step_response(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows import _commands as wf_commands + from specify_cli.workflows.catalog import StepCatalog + from specify_cli.authentication import http as auth_http + + monkeypatch.chdir(project_dir) + monkeypatch.setattr(wf_commands, "_MAX_WORKFLOW_YAML_BYTES", 100) + monkeypatch.setattr( + StepCatalog, + "get_step_info", + lambda self, step_id: { + "id": step_id, + "name": "Test Step", + "url": "https://example.com/step.yml", + "init_url": "https://example.com/__init__.py", + "_install_allowed": True, + }, + ) + + class _FakeResponse: + def __init__(self, url): + self.url = url + self.body = b"x" * 500 + self.offset = 0 + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc, tb): + return False + + def getheader(self, name): + return None + + def geturl(self): + return self.url + + def read(self, size=-1): + if size < 0: + size = len(self.body) - self.offset + chunk = self.body[self.offset : self.offset + size] + self.offset += len(chunk) + return chunk + + monkeypatch.setattr( + auth_http, + "open_url", + lambda url, timeout=30, redirect_validator=None: _FakeResponse(url), + ) + + result = CliRunner().invoke( + app, ["workflow", "step", "add", "my-step"] + ) + + assert result.exit_code != 0 + assert ( + "responseexceedsthe100-byteworkflowsizelimit" + in "".join(result.output.split()) + ) + assert not ( + project_dir / ".specify" / "workflows" / "steps" / "my-step" + ).exists() + def test_add_rejects_non_string_extra_files_key(self, project_dir, monkeypatch): from typer.testing import CliRunner from specify_cli import app @@ -6115,7 +6446,10 @@ def __enter__(self): def __exit__(self, exc_type, exc, tb): return False - def read(self): + def read(self, size=-1): + if getattr(self, "_read", False): + return b"" + self._read = True if self.url.endswith("/step.yml"): return b"step:\n type_key: my-step\n" return b"" @@ -6123,7 +6457,7 @@ def read(self): def geturl(self): return self.url - def _fake_open_url(url, timeout=30): + def _fake_open_url(url, timeout=30, redirect_validator=None): return _FakeResponse(url) monkeypatch.setattr(StepCatalog, "get_step_info", _fake_get_step_info) @@ -6174,7 +6508,10 @@ def __enter__(self): def __exit__(self, exc_type, exc, tb): return False - def read(self): + def read(self, size=-1): + if getattr(self, "_read", False): + return b"" + self._read = True if self.url.endswith("/step.yml"): return b"step:\n type_key: my-step\n" return b"" @@ -6182,7 +6519,7 @@ def read(self): def geturl(self): return self.url - def _fake_open_url(url, timeout=30): + def _fake_open_url(url, timeout=30, redirect_validator=None): return _FakeResponse(url) monkeypatch.setattr(StepCatalog, "get_step_info", _fake_get_step_info) @@ -6222,7 +6559,10 @@ def __enter__(self): def __exit__(self, exc_type, exc, tb): return False - def read(self): + def read(self, size=-1): + if getattr(self, "_read", False): + return b"" + self._read = True if self.url.endswith("/step.yml"): return b"step:\n type_key: my-step\n" return b"" @@ -6230,7 +6570,7 @@ def read(self): def geturl(self): return self.url - def _fake_open_url(url, timeout=30): + def _fake_open_url(url, timeout=30, redirect_validator=None): return _FakeResponse(url) monkeypatch.setattr(StepCatalog, "get_step_info", _fake_get_step_info) @@ -6539,8 +6879,16 @@ def __init__(self, data, url=None): self._data = data self._url = url or "https://api.github.com/repos/org/repo/releases/assets/42" - def read(self): - return self._data + def read(self, amt=None): + if not hasattr(self, "_pos"): + self._pos = 0 + if amt is None: + chunk = self._data[self._pos :] + self._pos = len(self._data) + return chunk + chunk = self._data[self._pos : self._pos + amt] + self._pos += len(chunk) + return chunk def geturl(self): return self._url @@ -6551,7 +6899,7 @@ def __enter__(self): def __exit__(self, *a): return False - def fake_open_url(url, timeout=None, extra_headers=None): + def fake_open_url(url, timeout=None, extra_headers=None, redirect_validator=None): captured_urls.append((url, extra_headers, timeout)) if "releases/tags/" in url: return FakeResponse(json.dumps({ @@ -6591,8 +6939,16 @@ def __init__(self, data, url=None): self._data = data self._url = url or "https://api.github.com/repos/org/repo/releases/assets/42" - def read(self): - return self._data + def read(self, amt=None): + if not hasattr(self, "_pos"): + self._pos = 0 + if amt is None: + chunk = self._data[self._pos :] + self._pos = len(self._data) + return chunk + chunk = self._data[self._pos : self._pos + amt] + self._pos += len(chunk) + return chunk def geturl(self): return self._url @@ -6603,7 +6959,7 @@ def __enter__(self): def __exit__(self, *a): return False - def fake_open_url(url, timeout=None, extra_headers=None): + def fake_open_url(url, timeout=None, extra_headers=None, redirect_validator=None): captured_urls.append((url, extra_headers)) return FakeResponse(self.VALID_WORKFLOW_YAML.encode()) @@ -6634,8 +6990,16 @@ def __init__(self, data, url=None): self._data = data self._url = url or "https://api.github.com/repos/org/repo/releases/assets/55" - def read(self): - return self._data + def read(self, amt=None): + if not hasattr(self, "_pos"): + self._pos = 0 + if amt is None: + chunk = self._data[self._pos :] + self._pos = len(self._data) + return chunk + chunk = self._data[self._pos : self._pos + amt] + self._pos += len(chunk) + return chunk def geturl(self): return self._url @@ -6646,7 +7010,7 @@ def __enter__(self): def __exit__(self, *a): return False - def fake_open_url(url, timeout=None, extra_headers=None): + def fake_open_url(url, timeout=None, extra_headers=None, redirect_validator=None): captured_urls.append((url, extra_headers)) if "releases/tags/" in url: return FakeResponse(json.dumps({ @@ -6710,8 +7074,16 @@ def __init__(self, data, url=None): self._data = data self._url = url or "https://ghes.example/api/v3/repos/org/repo/releases/assets/42" - def read(self): - return self._data + def read(self, amt=None): + if not hasattr(self, "_pos"): + self._pos = 0 + if amt is None: + chunk = self._data[self._pos :] + self._pos = len(self._data) + return chunk + chunk = self._data[self._pos : self._pos + amt] + self._pos += len(chunk) + return chunk def geturl(self): return self._url @@ -6722,7 +7094,7 @@ def __enter__(self): def __exit__(self, *a): return False - def fake_open_url(url, timeout=None, extra_headers=None): + def fake_open_url(url, timeout=None, extra_headers=None, redirect_validator=None): captured_urls.append((url, extra_headers)) if "releases/tags/" in url: return FakeResponse(json.dumps({ @@ -6765,8 +7137,16 @@ def __init__(self, data, url=None): self._data = data self._url = url or "https://ghes.example/api/v3/repos/org/repo/releases/assets/55" - def read(self): - return self._data + def read(self, amt=None): + if not hasattr(self, "_pos"): + self._pos = 0 + if amt is None: + chunk = self._data[self._pos :] + self._pos = len(self._data) + return chunk + chunk = self._data[self._pos : self._pos + amt] + self._pos += len(chunk) + return chunk def geturl(self): return self._url @@ -6790,7 +7170,7 @@ def __exit__(self, *a): run: "echo hello" """ - def fake_open_url(url, timeout=None, extra_headers=None): + def fake_open_url(url, timeout=None, extra_headers=None, redirect_validator=None): captured_urls.append((url, extra_headers)) if "releases/tags/" in url: return FakeResponse(json.dumps({ @@ -7194,3 +7574,4289 @@ def test_add_non_string_step_id_reports_validation_error( assert result.exit_code == 1 assert result.exception is None or isinstance(result.exception, SystemExit) assert "Step ID" in result.output + + +class TestWorkflowCliAlignment: + """CLI alignment with extension/preset commands (#2342).""" + + WORKFLOW_YAML = """ +schema_version: "1.0" +workflow: + id: "align-wf" + name: "Align Workflow" + version: "{version}" + description: "CLI alignment test workflow" +steps: + - id: step-one + type: shell + run: "echo hello" +""" + + def _write_workflow_dir(self, base, version="1.0.0"): + d = base / "wf-src" + d.mkdir(parents=True, exist_ok=True) + (d / "workflow.yml").write_text( + self.WORKFLOW_YAML.format(version=version), encoding="utf-8" + ) + return d + + def _install_dev(self, runner, app, project_dir): + src = self._write_workflow_dir(project_dir) + result = runner.invoke(app, ["workflow", "add", str(src), "--dev"]) + assert result.exit_code == 0, result.output + return src + + # -- add --dev ----------------------------------------------------- + + def test_add_dev_directory_installs(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) + assert WorkflowRegistry(project_dir).is_installed("align-wf") + + def test_add_dev_yaml_file_installs(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + src = self._write_workflow_dir(project_dir) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "add", str(src / "workflow.yml"), "--dev"]) + assert result.exit_code == 0, result.output + assert WorkflowRegistry(project_dir).is_installed("align-wf") + + def test_add_dev_missing_path_errors(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "add", str(project_dir / "missing"), "--dev"]) + assert result.exit_code != 0 + assert "--dev" in result.output + + def test_add_dev_dir_without_workflow_yml_errors(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + empty = project_dir / "empty-src" + empty.mkdir() + runner = CliRunner() + result = runner.invoke(app, ["workflow", "add", str(empty), "--dev"]) + assert result.exit_code != 0 + assert "No workflow.yml found" in result.output + + def test_add_local_dir_without_workflow_yml_errors(self, project_dir, monkeypatch): + """Same as the --dev case, but for the plain local-path fallback (no --dev).""" + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + empty = project_dir / "empty-src-[bracket]" + empty.mkdir() + runner = CliRunner() + result = runner.invoke(app, ["workflow", "add", str(empty)]) + assert result.exit_code != 0 + assert "No workflow.yml found" in result.output + assert "[bracket]" in result.output + + def test_add_local_dir_with_workflow_yml_directory_errors_cleanly(self, project_dir, monkeypatch): + """Same as the --dev case, but for the plain local-path fallback (no --dev): + a directory named workflow.yml must not reach open() and leak IsADirectoryError.""" + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + src_dir = project_dir / "local-wf" + (src_dir / "workflow.yml").mkdir(parents=True) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "add", str(src_dir)]) + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "No workflow.yml found" in result.output + + def test_add_yaml_parse_error_escapes_rich_markup(self, project_dir, monkeypatch): + """A YAML syntax error can quote the offending line verbatim; brackets in it must not be Rich markup.""" + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.engine import WorkflowDefinition + + monkeypatch.chdir(project_dir) + bad = project_dir / "bad.yml" + bad.write_text("workflow:\n id: wf\n", encoding="utf-8") + runner = CliRunner() + with patch.object( + WorkflowDefinition, + "from_string", + side_effect=ValueError('bad snippet: "New [Feature]"'), + ): + result = runner.invoke(app, ["workflow", "add", str(bad)]) + assert result.exit_code != 0 + assert 'bad snippet: "New [Feature]"' in result.output + + # -- add --from ---------------------------------------------------- + + class _FakeResponse: + def __init__(self, data, url="https://example.com/workflow.yml", headers=None): + self._data = data + self._url = url + self._pos = 0 + self._headers = headers or {} + + def read(self, amt=None): + if amt is None: + chunk = self._data[self._pos :] + self._pos = len(self._data) + return chunk + chunk = self._data[self._pos : self._pos + amt] + self._pos += len(chunk) + return chunk + + def getheader(self, name, default=None): + return self._headers.get(name, default) + + def geturl(self): + return self._url + + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + @pytest.mark.parametrize("mode", ["dev", "local", "from"]) + def test_reinstall_preserves_disabled_state( + self, project_dir, monkeypatch, mode + ): + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + src = self._install_dev(runner, app, project_dir) + result = runner.invoke(app, ["workflow", "disable", "align-wf"]) + assert result.exit_code == 0, result.output + + if mode == "dev": + result = runner.invoke( + app, ["workflow", "add", str(src), "--dev"] + ) + elif mode == "local": + result = runner.invoke(app, ["workflow", "add", str(src)]) + else: + data = self.WORKFLOW_YAML.format(version="2.0.0").encode() + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, + redirect_validator=None: self._FakeResponse(data, url), + ): + result = runner.invoke( + app, + [ + "workflow", "add", "align-wf", + "--from", "https://example.com/workflow.yml", + ], + input="y\n", + ) + + assert result.exit_code == 0, result.output + assert WorkflowRegistry(project_dir).get("align-wf")["enabled"] is False + + def test_add_from_url_rejects_oversized_content_length(self, project_dir, monkeypatch): + """A --from download must not trust an advertised Content-Length + alone by reading the whole body first -- it must reject a response + that declares a size over the workflow YAML limit before reading + the (potentially huge) body into memory at all.""" + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows import _commands as wf_commands + + monkeypatch.chdir(project_dir) + monkeypatch.setattr(wf_commands, "_MAX_WORKFLOW_YAML_BYTES", 100) + small_body = b"id: align-wf\n" # small actual body; Content-Length lies + runner = CliRunner() + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse( + small_body, url, headers={"Content-Length": "1000"} + ), + ): + result = runner.invoke( + app, + ["workflow", "add", "align-wf", "--from", "https://example.com/workflow.yml"], + input="y\n", + ) + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "exceedingthe100-byteworkflowsizelimit" in "".join(result.output.split()) + + def test_add_from_url_requires_default_deny_confirmation( + self, project_dir, monkeypatch + ): + from unittest.mock import patch + + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + with patch( + "specify_cli.authentication.http.open_url", + side_effect=AssertionError("download should not start"), + ): + result = CliRunner().invoke( + app, + [ + "workflow", + "add", + "align-wf", + "--from", + "https://example.com/workflow.yml", + ], + input="n\n", + ) + + assert result.exit_code == 0, result.output + assert "Untrusted Source" in result.output + assert "Cancelled" in result.output + + def test_add_from_url_rejects_oversized_streamed_body_without_content_length( + self, project_dir, monkeypatch + ): + """A chunked/no-Content-Length response must still be capped by + actually counting streamed bytes -- a malicious or misbehaving + server cannot bypass the limit merely by omitting or lying about + Content-Length.""" + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows import _commands as wf_commands + + monkeypatch.chdir(project_dir) + monkeypatch.setattr(wf_commands, "_MAX_WORKFLOW_YAML_BYTES", 100) + oversized_body = b"x" * 500 # no Content-Length header at all + runner = CliRunner() + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse( + oversized_body, url + ), + ): + result = runner.invoke( + app, + ["workflow", "add", "align-wf", "--from", "https://example.com/workflow.yml"], + input="y\n", + ) + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "exceedsthe100-byteworkflowsizelimit" in "".join(result.output.split()) + + def test_add_from_url_oversized_streamed_body_leaves_no_temp_file( + self, project_dir, monkeypatch, tmp_path + ): + """A rejected --from download (oversized streamed body, no + Content-Length) must not leave the 0-byte NamedTemporaryFile behind: + the file is created on disk as soon as it is opened (delete=False), + before any bytes are written, so a failure inside the size-limit + check must still clean it up rather than merely erroring out.""" + import tempfile as tempfile_mod + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows import _commands as wf_commands + + monkeypatch.chdir(project_dir) + monkeypatch.setattr(wf_commands, "_MAX_WORKFLOW_YAML_BYTES", 100) + scratch_tmp = tmp_path / "scratch-tmp" + scratch_tmp.mkdir() + monkeypatch.setattr(tempfile_mod, "tempdir", str(scratch_tmp)) + oversized_body = b"x" * 500 # no Content-Length header at all + runner = CliRunner() + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse( + oversized_body, url + ), + ): + result = runner.invoke( + app, + ["workflow", "add", "align-wf", "--from", "https://example.com/workflow.yml"], + input="y\n", + ) + assert result.exit_code != 0 + assert "exceedsthe100-byteworkflowsizelimit" in "".join(result.output.split()) + leaked = list(scratch_tmp.glob("*.yml")) + assert leaked == [], f"leaked temp files: {leaked}" + + def test_add_from_url_oversized_content_length_leaves_no_temp_file( + self, project_dir, monkeypatch, tmp_path + ): + """Same guarantee for the fail-fast Content-Length rejection path: + it must not even leave a 0-byte temp file behind.""" + import tempfile as tempfile_mod + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows import _commands as wf_commands + + monkeypatch.chdir(project_dir) + monkeypatch.setattr(wf_commands, "_MAX_WORKFLOW_YAML_BYTES", 100) + scratch_tmp = tmp_path / "scratch-tmp" + scratch_tmp.mkdir() + monkeypatch.setattr(tempfile_mod, "tempdir", str(scratch_tmp)) + small_body = b"id: align-wf\n" # small actual body; Content-Length lies + runner = CliRunner() + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse( + small_body, url, headers={"Content-Length": "1000"} + ), + ): + result = runner.invoke( + app, + ["workflow", "add", "align-wf", "--from", "https://example.com/workflow.yml"], + input="y\n", + ) + assert result.exit_code != 0 + assert "exceedingthe100-byteworkflowsizelimit" in "".join(result.output.split()) + leaked = list(scratch_tmp.glob("*.yml")) + assert leaked == [], f"leaked temp files: {leaked}" + + def test_add_from_url_download_failure_cleanup_error_preserves_original_error( + self, project_dir, monkeypatch, tmp_path + ): + """The --from download-failure branch's `tmp_path.unlink(missing_ok= + True)` can itself raise (e.g. read-only tempdir) before the clean + "Failed to download workflow" message is ever printed, replacing it + with a raw unhandled OSError. A cleanup failure there must be + guarded exactly like the later post-install finally cleanup: warn + about the cleanup failure, then still preserve/report the original + download error via a clean typer.Exit, never a raw traceback.""" + import tempfile as tempfile_mod + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows import _commands as wf_commands + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + monkeypatch.setattr(wf_commands, "_MAX_WORKFLOW_YAML_BYTES", 100) + scratch_tmp = tmp_path / "scratch-tmp" + scratch_tmp.mkdir() + monkeypatch.setattr(tempfile_mod, "tempdir", str(scratch_tmp)) + oversized_body = b"x" * 500 # no Content-Length header at all + runner = CliRunner() + + real_unlink = Path.unlink + + def unlink_boom(self_path, *args, **kwargs): + if self_path.suffix == ".yml" and self_path.parent == scratch_tmp: + raise OSError("cleanup denied") + return real_unlink(self_path, *args, **kwargs) + + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse( + oversized_body, url + ), + ), pytest.MonkeyPatch.context() as mp: + mp.setattr(Path, "unlink", unlink_boom) + result = runner.invoke( + app, + ["workflow", "add", "align-wf", "--from", "https://example.com/workflow.yml"], + input="y\n", + ) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + # Original download error remains present. + assert "exceedsthe100-byteworkflowsizelimit" in "".join(result.output.split()) + # Cleanup failure is reported too, not silently swallowed / crashing. + assert "cleanup denied" in result.output + assert "Warning" in result.output + assert not WorkflowRegistry(project_dir).is_installed("align-wf") + + def test_add_from_url_installs(self, project_dir, monkeypatch): + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + runner = CliRunner() + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse(data, url), + ): + result = runner.invoke( + app, + ["workflow", "add", "align-wf", "--from", "https://example.com/workflow.yml"], + input="y\n", + ) + assert result.exit_code == 0, result.output + assert WorkflowRegistry(project_dir).is_installed("align-wf") + + def test_add_from_url_temp_cleanup_failure_after_success_still_exits_zero( + self, project_dir, monkeypatch + ): + """An OSError while deleting the --from download's temp file after + _validate_and_install_local() has already committed the file and + registry entry must not surface as an unhandled failure for an + install that already succeeded -- it must be a warning, exit 0.""" + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + runner = CliRunner() + + import tempfile + + real_unlink = Path.unlink + + def unlink_boom(self_path, *args, **kwargs): + if self_path.suffix == ".yml" and self_path.parent == Path(tempfile.gettempdir()): + raise OSError("permission denied") + return real_unlink(self_path, *args, **kwargs) + + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse(data, url), + ), pytest.MonkeyPatch.context() as mp: + mp.setattr(Path, "unlink", unlink_boom) + result = runner.invoke( + app, + ["workflow", "add", "align-wf", "--from", "https://example.com/workflow.yml"], + input="y\n", + ) + + assert result.exit_code == 0, result.output + assert "Warning" in result.output + assert "permissiondenied" in "".join(result.output.split()) + assert WorkflowRegistry(project_dir).is_installed("align-wf") + + def test_add_from_url_id_mismatch_errors(self, project_dir, monkeypatch): + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + runner = CliRunner() + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse(data, url), + ): + result = runner.invoke( + app, + ["workflow", "add", "other-id", "--from", "https://example.com/workflow.yml"], + input="y\n", + ) + assert result.exit_code != 0 + assert "does not match" in result.output + assert not WorkflowRegistry(project_dir).is_installed("align-wf") + + def test_add_from_empty_url_rejected_not_catalog_fallback(self, project_dir, monkeypatch): + """--from "" must fail URL validation, not silently install from the catalog.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "add", "align-wf", "--from", ""]) + assert result.exit_code != 0 + assert "HTTPS" in result.output + assert not WorkflowRegistry(project_dir).is_installed("align-wf") + + def test_add_from_url_non_https_redirect_escapes_rich_markup(self, project_dir, monkeypatch): + """A redirect to a non-HTTPS IPv6 literal (legally bracketed) must not be parsed as Rich markup.""" + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + redirected_url = "http://[2001:db8::1]/workflow.yml" + runner = CliRunner() + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse(b"", redirected_url), + ): + result = runner.invoke( + app, + ["workflow", "add", "align-wf", "--from", "https://example.com/workflow.yml"], + input="y\n", + ) + assert result.exit_code != 0 + assert redirected_url in result.output + + def test_add_from_rejects_invalid_source_id_without_fetch(self, project_dir, monkeypatch): + """--from with a non-workflow-id source (URL, path, uppercase) fails before any network fetch.""" + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + calls: list[str] = [] + + def _fake_open(url, timeout=None, extra_headers=None, redirect_validator=None): + calls.append(url) + raise AssertionError(f"network fetch attempted: {url}") + + runner = CliRunner() + with patch("specify_cli.authentication.http.open_url", side_effect=_fake_open): + for bad_source in ("https://x/y.yml", "./local.yml", "BadCase"): + result = runner.invoke( + app, + ["workflow", "add", bad_source, "--from", "https://example.com/workflow.yml"], + ) + assert result.exit_code != 0 + assert "Invalid workflow ID" in result.output + assert calls == [] + + # -- search --author ----------------------------------------------- + + def test_search_author_filters(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog + + monkeypatch.chdir(project_dir) + workflows = { + "wf-a": {"name": "Workflow A", "version": "1.0.0", "description": "", "author": "alice"}, + "wf-b": {"name": "Workflow B", "version": "1.0.0", "description": "", "author": "bob"}, + } + monkeypatch.setattr( + WorkflowCatalog, + "_get_merged_workflows", + lambda self, force_refresh=False: {k: dict(v) for k, v in workflows.items()}, + ) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "search", "--author", "Alice"]) + assert result.exit_code == 0, result.output + assert "wf-a" in result.output + assert "wf-b" not in result.output + + def test_search_escapes_rich_markup_in_catalog_fields(self, project_dir, monkeypatch): + """Catalog-derived name/description/tags must not be parsed as Rich markup.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog + + monkeypatch.chdir(project_dir) + workflows = { + "wf-a": { + "name": "Bracket [Search]", + "version": "1.0.0", + "description": "desc [with] brackets", + "tags": ["tag[1]", "tag2"], + }, + } + monkeypatch.setattr( + WorkflowCatalog, + "_get_merged_workflows", + lambda self, force_refresh=False: {k: dict(v) for k, v in workflows.items()}, + ) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "search"]) + assert result.exit_code == 0, result.output + assert "Bracket [Search]" in result.output + assert "desc [with] brackets" in result.output + assert "tag[1]" in result.output + + # -- update ---------------------------------------------------------- + + def test_update_no_workflows_installed(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "update"]) + assert result.exit_code == 0, result.output + assert "No workflows installed" in result.output + + def test_update_not_installed_errors(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "update", "ghost"]) + assert result.exit_code != 0 + assert "not installed" in result.output + + def test_update_skips_non_catalog_sources(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) + result = runner.invoke(app, ["workflow", "update"]) + assert result.exit_code == 0, result.output + assert "re-add to update" in result.output + # Every target was skipped — must not claim everything is up to date. + assert "No workflows were eligible for update" in result.output + assert "up to date!" not in result.output + + def test_update_skip_message_accurate_for_bundled_source(self, project_dir, monkeypatch): + """A workflow registered with source "bundled" (e.g. the speckit + workflow installed by `specify init`) was never installed from a + local path or URL; the skip message must not claim otherwise.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + registry = WorkflowRegistry(project_dir) + registry.add( + "speckit", + {"name": "Speckit", "version": "1.0.0", "source": "bundled"}, + ) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "update"]) + assert result.exit_code == 0, result.output + assert "local path or URL" not in result.output + assert "re-add to update" in result.output + + def test_registry_add_rolls_back_memory_on_save_failure(self, project_dir, monkeypatch): + from specify_cli.workflows.catalog import WorkflowRegistry + + registry = WorkflowRegistry(project_dir) + registry.add("align-wf", {"version": "1.0.0", "source": "catalog"}) + + def boom(): + raise OSError("disk full") + + monkeypatch.setattr(registry, "save", boom) + with pytest.raises(OSError): + registry.add("align-wf", {"version": "2.0.0", "source": "catalog"}) + assert registry.get("align-wf")["version"] == "1.0.0" + + with pytest.raises(OSError): + registry.add("other-wf", {"version": "1.0.0", "source": "catalog"}) + assert registry.get("other-wf") is None + + @pytest.mark.parametrize("error_type", [TypeError, ValueError]) + def test_registry_add_rolls_back_memory_on_serialization_failure( + self, project_dir, monkeypatch, error_type + ): + from specify_cli.workflows.catalog import WorkflowRegistry + + registry = WorkflowRegistry(project_dir) + registry.add("align-wf", {"version": "1.0.0", "source": "catalog"}) + + def boom(): + raise error_type("not JSON serializable") + + monkeypatch.setattr(registry, "save", boom) + with pytest.raises(error_type): + registry.add("align-wf", {"version": "2.0.0", "source": "catalog"}) + assert registry.get("align-wf")["version"] == "1.0.0" + + with pytest.raises(error_type): + registry.add("other-wf", {"version": "1.0.0", "source": "catalog"}) + assert registry.get("other-wf") is None + + def test_registry_add_survives_non_dict_existing_entry(self, project_dir): + from specify_cli.workflows.catalog import WorkflowRegistry + + registry = WorkflowRegistry(project_dir) + registry.data["workflows"]["align-wf"] = "corrupted" + registry.add("align-wf", {"version": "1.0.0", "source": "catalog"}) + assert registry.get("align-wf")["version"] == "1.0.0" + + @pytest.mark.parametrize( + "contents", + [ + "not json", + "[]", + '{"schema_version": "1.0"}', + '{"schema_version": "1.0", "workflows": "broken"}', + ], + ) + def test_registry_load_rejects_corrupt_contents( + self, project_dir, contents + ): + from specify_cli.workflows.catalog import WorkflowRegistry + + registry = WorkflowRegistry(project_dir) + registry.workflows_dir.mkdir(parents=True, exist_ok=True) + registry.registry_path.write_text(contents, encoding="utf-8") + + with pytest.raises(OSError, match="corrupt"): + WorkflowRegistry(project_dir) + + assert registry.registry_path.read_text(encoding="utf-8") == contents + + def test_registry_save_refuses_symlinked_parent(self, project_dir, tmp_path): + """Construction now fails closed on a symlinked .specify just like + an unreadable registry file: a symlinked parent must never be + silently tolerated up to save() -- it must raise immediately.""" + from specify_cli.workflows.catalog import WorkflowRegistry + + outside = tmp_path / "outside-specify" + outside.mkdir() + specify_dir = project_dir / ".specify" + if specify_dir.exists(): + shutil.rmtree(specify_dir) + specify_dir.symlink_to(outside) + with pytest.raises(OSError, match="symlink"): + WorkflowRegistry(project_dir) + assert not (outside / "workflows").exists() + + @pytest.mark.skipif(sys.platform == "win32", reason="chmod mode bits not reliable on Windows") + def test_registry_save_preserves_existing_file_mode(self, project_dir): + """A registry shared as 0640/0644 must keep that mode after a save, + not be silently replaced by mkstemp's 0600 default -- otherwise + every add/remove locks other project users out of a previously + shared registry file.""" + from specify_cli.workflows.catalog import WorkflowRegistry + + registry = WorkflowRegistry(project_dir) + registry.add("first-wf", {"name": "First"}) + registry.registry_path.chmod(0o644) + + registry.add("second-wf", {"name": "Second"}) + + mode = stat.S_IMODE(registry.registry_path.stat().st_mode) + assert mode == 0o644, f"expected 0644, got {oct(mode)}" + + @pytest.mark.skipif(not hasattr(os, "fchown"), reason="os.fchown is unavailable") + def test_registry_save_preserves_existing_owner_group( + self, project_dir, monkeypatch + ): + from specify_cli.workflows.catalog import WorkflowRegistry + import specify_cli.workflows.catalog as catalog_mod + + registry = WorkflowRegistry(project_dir) + registry.add("first-wf", {"name": "First"}) + existing = registry.registry_path.stat() + calls: list[tuple[os.stat_result, int, int]] = [] + + monkeypatch.setattr( + catalog_mod.os, + "fchown", + lambda fd, uid, gid: calls.append((os.fstat(fd), uid, gid)), + ) + registry.add("second-wf", {"name": "Second"}) + + assert len(calls) == 1 + temp_stat, uid, gid = calls[0] + assert stat.S_ISREG(temp_stat.st_mode) + assert uid == existing.st_uid + assert gid == existing.st_gid + + @pytest.mark.skipif(sys.platform == "win32", reason="chmod mode bits not reliable on Windows") + def test_registry_save_on_new_registry_uses_secure_default_mode(self, project_dir): + """A brand-new registry file (no prior mode to preserve) should keep + mkstemp's secure 0600 default rather than something more permissive.""" + from specify_cli.workflows.catalog import WorkflowRegistry + + registry = WorkflowRegistry(project_dir) + registry.add("first-wf", {"name": "First"}) + + mode = stat.S_IMODE(registry.registry_path.stat().st_mode) + assert mode == 0o600, f"expected 0600, got {oct(mode)}" + + @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable") + def test_registry_save_rejects_swapped_temp_without_touching_target( + self, project_dir, monkeypatch + ): + from specify_cli.workflows.catalog import WorkflowRegistry + import specify_cli.workflows.catalog as catalog_mod + + registry = WorkflowRegistry(project_dir) + registry.add("first-wf", {"name": "First"}) + registry.registry_path.chmod(0o640) + + victim = project_dir / "victim.txt" + victim.write_text("untouched", encoding="utf-8") + victim.chmod(0o600) + + tmp_path = None + real_mkstemp = catalog_mod.tempfile.mkstemp + real_dump = catalog_mod.json.dump + + def tracking_mkstemp(*args, **kwargs): + nonlocal tmp_path + fd, name = real_mkstemp(*args, **kwargs) + tmp_path = Path(name) + return fd, name + + def swap_after_dump(*args, **kwargs): + result = real_dump(*args, **kwargs) + assert tmp_path is not None + tmp_path.unlink() + tmp_path.symlink_to(victim) + return result + + monkeypatch.setattr(catalog_mod.tempfile, "mkstemp", tracking_mkstemp) + monkeypatch.setattr(catalog_mod.json, "dump", swap_after_dump) + + with pytest.raises(OSError): + registry.add("second-wf", {"name": "Second"}) + + assert victim.read_text(encoding="utf-8") == "untouched" + assert stat.S_IMODE(victim.stat().st_mode) == 0o600 + assert not registry.registry_path.is_symlink() + assert WorkflowRegistry(project_dir).is_installed("first-wf") + + def test_add_dev_dir_with_workflow_yml_directory_errors_cleanly(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + dev_dir = project_dir / "dev-wf" + (dev_dir / "workflow.yml").mkdir(parents=True) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "add", "--dev", str(dev_dir)]) + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "No workflow.yml found" in result.output + + @pytest.mark.parametrize("mode", ["dev", "local", "from_url"]) + def test_add_save_failure_leaves_no_orphan_directory(self, project_dir, monkeypatch, mode): + """A registry.add() save failure during a fresh install must not leave + an orphaned workflow directory on disk, and must fail with a clean + escaped message instead of a raw OSError traceback. Shared by --dev, + the plain local-path fallback, and --from since all three funnel + through _validate_and_install_local's single install choke point.""" + import contextlib + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + + def boom(self): + raise OSError("disk full") + + if mode == "from_url": + data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + args = ["workflow", "add", "align-wf", "--from", "https://example.com/workflow.yml"] + url_patch = patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse(data, url), + ) + else: + src = self._write_workflow_dir(project_dir) + args = ["workflow", "add", str(src)] + (["--dev"] if mode == "dev" else []) + url_patch = contextlib.nullcontext() + + with url_patch, pytest.MonkeyPatch.context() as mp: + mp.setattr(WorkflowRegistry, "save", boom) + result = runner.invoke( + app, args, input="y\n" if mode == "from_url" else None + ) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert result.output.strip() != "" + dest_dir = project_dir / ".specify" / "workflows" / "align-wf" + assert not dest_dir.exists() + assert not WorkflowRegistry(project_dir).is_installed("align-wf") + + @pytest.mark.parametrize("mode", ["dev", "catalog"]) + def test_add_non_json_description_rolls_back_transaction( + self, project_dir, monkeypatch, mode + ): + import contextlib + from unittest.mock import patch + + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + data = self.WORKFLOW_YAML.format(version="1.0.0").replace( + 'description: "CLI alignment test workflow"', + "description: 2026-01-02", + ).encode() + + if mode == "dev": + source = project_dir / "dated-description" + source.mkdir() + (source / "workflow.yml").write_bytes(data) + args = ["workflow", "add", str(source), "--dev"] + download = contextlib.nullcontext() + else: + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + args = ["workflow", "add", "align-wf"] + download = patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, + redirect_validator=None: self._FakeResponse(data, url), + ) + + with download: + result = CliRunner().invoke(app, args) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "Failed to update workflow registry" in result.output + dest_dir = project_dir / ".specify" / "workflows" / "align-wf" + assert not dest_dir.exists() + assert not WorkflowRegistry(project_dir).is_installed("align-wf") + + @pytest.mark.parametrize("mode", ["dev", "local", "from_url"]) + def test_add_fresh_install_mkstemp_failure_leaves_no_orphan_directory( + self, project_dir, monkeypatch, mode + ): + """_stage_workflow_file() does mkdir(dest_dir) then mkstemp() inside + it. For a fresh install (no prior directory), if mkdir succeeds but + mkstemp then fails (disk full/EMFILE/quota), the freshly-created + empty dest_dir must not be left orphaned -- it must be removed, and + the original mkstemp error must still be reported cleanly.""" + import contextlib + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + + def boom(*args, **kwargs): + raise OSError("disk full") + + if mode == "from_url": + data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + args = ["workflow", "add", "align-wf", "--from", "https://example.com/workflow.yml"] + url_patch = patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse(data, url), + ) + else: + src = self._write_workflow_dir(project_dir) + args = ["workflow", "add", str(src)] + (["--dev"] if mode == "dev" else []) + url_patch = contextlib.nullcontext() + + with url_patch, pytest.MonkeyPatch.context() as mp: + mp.setattr("tempfile.mkstemp", boom) + result = runner.invoke( + app, args, input="y\n" if mode == "from_url" else None + ) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert result.output.strip() != "" + dest_dir = project_dir / ".specify" / "workflows" / "align-wf" + assert not dest_dir.exists(), "fresh-install dest_dir left orphaned after mkstemp failure" + assert not WorkflowRegistry(project_dir).is_installed("align-wf") + + def test_add_reinstall_mkstemp_failure_preserves_preexisting_directory( + self, project_dir, monkeypatch + ): + """A pre-existing (reinstall) dest_dir must never be removed by the + mkstemp-failure cleanup -- only a directory _stage_workflow_file + itself just created.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + src = self._install_dev(runner, app, project_dir) + installed_yaml = project_dir / ".specify" / "workflows" / "align-wf" / "workflow.yml" + original_bytes = installed_yaml.read_bytes() + original_registry_entry = WorkflowRegistry(project_dir).get("align-wf") + + (src / "workflow.yml").write_text( + self.WORKFLOW_YAML.format(version="2.0.0"), encoding="utf-8" + ) + + def boom(*args, **kwargs): + raise OSError("disk full") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr("tempfile.mkstemp", boom) + result = runner.invoke(app, ["workflow", "add", str(src), "--dev"]) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert result.output.strip() != "" + assert installed_yaml.parent.is_dir() + assert installed_yaml.read_bytes() == original_bytes + assert WorkflowRegistry(project_dir).get("align-wf") == original_registry_entry + + @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable") + @pytest.mark.parametrize("mode", ["dev", "catalog"]) + def test_stage_write_rejects_swapped_symlink( + self, project_dir, monkeypatch, mode + ): + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows import _commands + from specify_cli.workflows.catalog import WorkflowCatalog + + monkeypatch.chdir(project_dir) + victim = project_dir / "victim.txt" + victim.write_text("untouched", encoding="utf-8") + + real_stage = _commands._stage_workflow_file + + def raced_stage(*args, **kwargs): + staged = real_stage(*args, **kwargs) + staged_path = getattr(staged, "path", staged) + staged_path.unlink() + staged_path.symlink_to(victim) + return staged + + monkeypatch.setattr(_commands, "_stage_workflow_file", raced_stage) + + if mode == "dev": + source = self._write_workflow_dir(project_dir) + args = ["workflow", "add", str(source), "--dev"] + else: + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + monkeypatch.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, + redirect_validator=None: self._FakeResponse(data, url), + ) + args = ["workflow", "add", "align-wf"] + + result = CliRunner().invoke(app, args) + + assert result.exit_code != 0 + assert victim.read_text(encoding="utf-8") == "untouched" + + def test_local_install_writes_the_same_bytes_it_validates( + self, project_dir, monkeypatch + ): + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows import _commands + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + source = self._write_workflow_dir(project_dir) + source_file = source / "workflow.yml" + validated_content = source_file.read_text(encoding="utf-8") + replacement_content = self.WORKFLOW_YAML.format(version="9.9.9") + + real_stage = _commands._stage_workflow_file + + def replace_source_after_validation(*args, **kwargs): + staged = real_stage(*args, **kwargs) + source_file.write_text(replacement_content, encoding="utf-8") + return staged + + monkeypatch.setattr( + _commands, + "_stage_workflow_file", + replace_source_after_validation, + ) + + result = CliRunner().invoke( + app, ["workflow", "add", str(source), "--dev"] + ) + + assert result.exit_code == 0, result.output + installed_file = ( + project_dir + / ".specify" + / "workflows" + / "align-wf" + / "workflow.yml" + ) + assert installed_file.read_text(encoding="utf-8") == validated_content + assert WorkflowRegistry(project_dir).get("align-wf")["version"] == "1.0.0" + + def test_add_fresh_install_staged_discard_cleanup_failure_reports_warning( + self, project_dir, monkeypatch + ): + """A genuine fresh-directory rmdir failure must be reported while + the original copy failure remains the primary error.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows import _commands + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + src = self._write_workflow_dir(project_dir) + + def copy_boom(self, data): + raise OSError("disk full") + + real_rmdir = Path.rmdir + + def rmdir_boom(path): + if path.name == "align-wf": + raise OSError("cleanup denied") + return real_rmdir(path) + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(_commands._StagedWorkflowFile, "write_bytes", copy_boom) + mp.setattr(Path, "rmdir", rmdir_boom) + result = runner.invoke(app, ["workflow", "add", str(src), "--dev"]) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + # Original install error remains present and primary. + assert "disk full" in result.output + # Cleanup failure is now reported, not silently swallowed. + assert "cleanup denied" in result.output + assert "Warning" in result.output + assert not WorkflowRegistry(project_dir).is_installed("align-wf") + + def test_add_fresh_install_registry_rollback_cleanup_failure_reports_warning( + self, project_dir, monkeypatch + ): + """A fresh-install rollback directory-removal failure must be + reported while the registry-update error remains primary.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + src = self._write_workflow_dir(project_dir) + + def save_boom(self): + raise OSError("registry disk full") + + real_rmdir = Path.rmdir + + def rmdir_boom(path): + if path.name == "align-wf": + raise OSError("cleanup denied") + return real_rmdir(path) + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(WorkflowRegistry, "save", save_boom) + mp.setattr(Path, "rmdir", rmdir_boom) + result = runner.invoke(app, ["workflow", "add", str(src), "--dev"]) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + # Original registry-update error remains present and primary. + assert "registry disk full" in result.output + # Cleanup failure is now reported, not silently swallowed. + assert "cleanup denied" in result.output + assert "Warning" in result.output + + def test_add_dev_reinstall_copy_failure_leaves_prior_file_untouched( + self, project_dir, monkeypatch + ): + """A staged descriptor-copy failure cannot touch the prior installed + workflow or leave a staging file behind.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows import _commands + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + src = self._install_dev(runner, app, project_dir) + installed_yaml = project_dir / ".specify" / "workflows" / "align-wf" / "workflow.yml" + original_bytes = installed_yaml.read_bytes() + original_registry_entry = WorkflowRegistry(project_dir).get("align-wf") + + # Point --dev at a new version of the same workflow to trigger a + # reinstall (overwrite) rather than a fresh install. + (src / "workflow.yml").write_text( + self.WORKFLOW_YAML.format(version="2.0.0"), encoding="utf-8" + ) + + def boom(staged, data): + # Simulate a truncating partial write followed by an OSError on + # the reserved staging inode, mirroring disk exhaustion. + os.ftruncate(staged.fd, 0) + raise OSError("disk full") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(_commands._StagedWorkflowFile, "write_bytes", boom) + result = runner.invoke(app, ["workflow", "add", str(src), "--dev"]) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert result.output.strip() != "" + assert installed_yaml.read_bytes() == original_bytes + assert WorkflowRegistry(project_dir).get("align-wf") == original_registry_entry + # No orphaned staging file left behind in the workflow directory. + leftovers = [p.name for p in installed_yaml.parent.iterdir() if p.name != "workflow.yml"] + assert leftovers == [] + + def test_add_dev_successful_reinstall_leaves_no_backup_file( + self, project_dir, monkeypatch + ): + """Once registry.add() succeeds, the unique rollback backup must be + discarded rather than left as a permanent orphan sibling.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + src = self._install_dev(runner, app, project_dir) + workflow_dir = project_dir / ".specify" / "workflows" / "align-wf" + + # Reinstall (overwrite) with a new version -- a successful reinstall, + # not a failure path. + (src / "workflow.yml").write_text( + self.WORKFLOW_YAML.format(version="2.0.0"), encoding="utf-8" + ) + result = runner.invoke(app, ["workflow", "add", str(src), "--dev"]) + + assert result.exit_code == 0, result.output + registry = WorkflowRegistry(project_dir) + assert registry.is_installed("align-wf") + assert registry.get("align-wf")["version"] == "2.0.0" + assert (workflow_dir / "workflow.yml").read_text(encoding="utf-8") == ( + self.WORKFLOW_YAML.format(version="2.0.0") + ) + leftovers = [p.name for p in workflow_dir.iterdir() if p.name != "workflow.yml"] + assert leftovers == [], f"orphan sibling(s) left behind: {leftovers}" + + def test_add_dev_successful_reinstall_backup_cleanup_failure_still_succeeds( + self, project_dir, monkeypatch + ): + """A failure to clean up the now-unneeded backup file after a + successful registry.add() must not turn the already-successful + install into a reported failure: it must be a warning (exit 0), + consistent with workflow_remove's post-commit cleanup semantics.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + src = self._install_dev(runner, app, project_dir) + + (src / "workflow.yml").write_text( + self.WORKFLOW_YAML.format(version="2.0.0"), encoding="utf-8" + ) + + real_unlink = Path.unlink + + def unlink_boom(self_path, *args, **kwargs): + if self_path.name.endswith(".bak"): + raise OSError("permission denied") + return real_unlink(self_path, *args, **kwargs) + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(Path, "unlink", unlink_boom) + result = runner.invoke(app, ["workflow", "add", str(src), "--dev"]) + + assert result.exit_code == 0, result.output + assert "Warning" in result.output + assert "permissiondenied" in "".join(result.output.split()) + registry = WorkflowRegistry(project_dir) + assert registry.is_installed("align-wf") + assert registry.get("align-wf")["version"] == "2.0.0" + + def test_add_dev_reinstall_restore_failure_reports_warning_and_original_error( + self, project_dir, monkeypatch + ): + """The prior file is now restored via an atomic rename (not a + content rewrite) when registry.add() fails on a reinstall. If that + restore rename itself also fails (e.g. a transient FS issue), it + must not silently claim success or crash with a raw traceback: it + must report a clear warning about the restore failure in addition + to the original clean registry error.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + src = self._install_dev(runner, app, project_dir) + + (src / "workflow.yml").write_text( + self.WORKFLOW_YAML.format(version="2.0.0"), encoding="utf-8" + ) + + def save_boom(self): + raise OSError("disk full") + + real_replace = os.replace + calls = {"n": 0} + + def replace_boom(src_path, dst_path): + # The commit swap for a reinstall makes exactly two os.replace + # calls (backup-aside, then staged-into-dest); let both succeed + # and only fail the third call -- the post-registry-failure + # restore-back rename. + calls["n"] += 1 + if calls["n"] <= 2: + return real_replace(src_path, dst_path) + raise OSError("permission denied") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(WorkflowRegistry, "save", save_boom) + mp.setattr(os, "replace", replace_boom) + result = runner.invoke(app, ["workflow", "add", str(src), "--dev"]) + + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + output_compact = "".join(result.output.split()) + assert "Warning" in result.output + assert "diskfull" in output_compact + assert "permissiondenied" in output_compact + + def test_add_dev_fresh_install_into_preexisting_empty_dir_cleans_new_file( + self, project_dir, monkeypatch + ): + """When the destination directory already exists but has no + workflow.yml (e.g. an empty dir left over from elsewhere), a later + registry.add() failure must remove the newly copied file -- the + rollback previously did nothing in this case (existed_before=True + with no backup bytes), leaving the new file behind -- while leaving + the pre-existing directory itself intact.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + src = self._write_workflow_dir(project_dir) + dest_dir = project_dir / ".specify" / "workflows" / "align-wf" + dest_dir.mkdir(parents=True) # pre-existing, but empty: no workflow.yml + + def boom(self, *args, **kwargs): + raise OSError("disk full") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(WorkflowRegistry, "add", boom) + result = runner.invoke(app, ["workflow", "add", str(src), "--dev"]) + + assert result.exit_code != 0 + assert result.output.strip() != "" + assert dest_dir.is_dir() + assert not (dest_dir / "workflow.yml").exists() + + def test_add_catalog_save_failure_leaves_no_orphan_directory(self, project_dir, monkeypatch): + """Same guarantee as the local-install paths, but for a fresh catalog + install: a registry.add() failure must clean up the freshly-downloaded + directory and fail with a clean escaped message.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + + def boom(self): + raise OSError("disk full") + + runner = CliRunner() + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse(data, url), + ) + mp.setattr(WorkflowRegistry, "save", boom) + result = runner.invoke(app, ["workflow", "add", "align-wf"]) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert result.output.strip() != "" + dest_dir = project_dir / ".specify" / "workflows" / "align-wf" + assert not dest_dir.exists() + assert not WorkflowRegistry(project_dir).is_installed("align-wf") + + def test_add_catalog_fresh_install_mkstemp_failure_leaves_no_orphan_directory( + self, project_dir, monkeypatch + ): + """Same guarantee as the local-install fresh-install case, but for a + fresh catalog install: if _stage_workflow_file's mkdir succeeds but + its mkstemp then fails, the freshly-created empty directory must not + be left orphaned.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + + def boom(*args, **kwargs): + raise OSError("disk full") + + runner = CliRunner() + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse(data, url), + ) + mp.setattr("tempfile.mkstemp", boom) + result = runner.invoke(app, ["workflow", "add", "align-wf"]) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert result.output.strip() != "" + dest_dir = project_dir / ".specify" / "workflows" / "align-wf" + assert not dest_dir.exists(), "fresh-install dest_dir left orphaned after mkstemp failure" + assert not WorkflowRegistry(project_dir).is_installed("align-wf") + + def test_add_catalog_rejects_oversized_content_length(self, project_dir, monkeypatch): + """Catalog installs must share the same size cap as --from: a + response that declares an oversized Content-Length is rejected + before its body is read into memory, and no orphan directory or + registry mutation is left behind.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows import _commands as wf_commands + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + monkeypatch.setattr(wf_commands, "_MAX_WORKFLOW_YAML_BYTES", 100) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + small_body = b"id: align-wf\n" # actual body is small; header lies + runner = CliRunner() + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse( + small_body, url, headers={"Content-Length": "1000"} + ), + ) + result = runner.invoke(app, ["workflow", "add", "align-wf"]) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "exceedingthe100-byteworkflowsizelimit" in "".join(result.output.split()) + dest_dir = project_dir / ".specify" / "workflows" / "align-wf" + assert not dest_dir.exists() + assert not WorkflowRegistry(project_dir).is_installed("align-wf") + + def test_add_catalog_rejects_oversized_streamed_body_without_content_length( + self, project_dir, monkeypatch + ): + """Catalog installs must also cap actual streamed bytes when + Content-Length is absent or understated, leaving no orphan + directory or registry mutation behind.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows import _commands as wf_commands + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + monkeypatch.setattr(wf_commands, "_MAX_WORKFLOW_YAML_BYTES", 100) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + oversized_body = b"x" * 500 # no Content-Length header at all + runner = CliRunner() + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse( + oversized_body, url + ), + ) + result = runner.invoke(app, ["workflow", "add", "align-wf"]) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "exceedsthe100-byteworkflowsizelimit" in "".join(result.output.split()) + dest_dir = project_dir / ".specify" / "workflows" / "align-wf" + assert not dest_dir.exists() + assert not WorkflowRegistry(project_dir).is_installed("align-wf") + + def test_add_catalog_reinstall_save_failure_restores_prior_file(self, project_dir, monkeypatch): + """Re-adding an already-installed catalog workflow downloads the new + version over the existing install directory. If registry.add() then + fails to save, the prior working workflow.yml must be restored + byte-for-byte (not left overwritten with the new download, and not + deleted like a fresh install) and the registry must remain valid and + still point at the original version -- the update path's caller has + an outer backup/restore for this, but plain `workflow add` does not, + so _install_workflow_from_catalog must handle it itself.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + original_data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + runner = CliRunner() + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse( + original_data, url + ), + ) + result = runner.invoke(app, ["workflow", "add", "align-wf"]) + assert result.exit_code == 0, result.output + + dest_file = project_dir / ".specify" / "workflows" / "align-wf" / "workflow.yml" + assert dest_file.read_bytes() == original_data + + new_data = self.WORKFLOW_YAML.format(version="2.0.0").encode() + + def boom(self): + raise OSError("disk full") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse( + new_data, url + ), + ) + mp.setattr(WorkflowRegistry, "save", boom) + result = runner.invoke(app, ["workflow", "add", "align-wf"]) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert result.output.strip() != "" + # The prior working install must survive untouched, byte-for-byte. + assert dest_file.read_bytes() == original_data + registry = WorkflowRegistry(project_dir) + assert registry.is_installed("align-wf") + assert registry.get("align-wf")["version"] == "1.0.0" + + def test_add_catalog_successful_reinstall_leaves_no_backup_file( + self, project_dir, monkeypatch + ): + """Same orphan-backup gap as the local-install path: a successful + catalog reinstall must not leave its unique backup behind once + registry.add() durably succeeds.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + original_data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + runner = CliRunner() + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse( + original_data, url + ), + ) + result = runner.invoke(app, ["workflow", "add", "align-wf"]) + assert result.exit_code == 0, result.output + + new_data = self.WORKFLOW_YAML.format(version="2.0.0").encode() + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse( + new_data, url + ), + ) + result = runner.invoke(app, ["workflow", "add", "align-wf"]) + + assert result.exit_code == 0, result.output + workflow_dir = project_dir / ".specify" / "workflows" / "align-wf" + registry = WorkflowRegistry(project_dir) + assert registry.is_installed("align-wf") + assert registry.get("align-wf")["version"] == "2.0.0" + assert (workflow_dir / "workflow.yml").read_bytes() == new_data + leftovers = [p.name for p in workflow_dir.iterdir() if p.name != "workflow.yml"] + assert leftovers == [], f"orphan sibling(s) left behind: {leftovers}" + + @pytest.mark.skipif(os.name == "nt", reason="POSIX permission bits") + def test_add_catalog_fresh_install_uses_project_file_mode( + self, project_dir, monkeypatch + ): + import stat + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog + + monkeypatch.chdir(project_dir) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + + previous_umask = os.umask(0o022) + try: + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, + redirect_validator=None: self._FakeResponse(data, url), + ) + result = CliRunner().invoke( + app, ["workflow", "add", "align-wf"] + ) + finally: + os.umask(previous_umask) + + assert result.exit_code == 0, result.output + workflow_file = ( + project_dir + / ".specify" + / "workflows" + / "align-wf" + / "workflow.yml" + ) + assert stat.S_IMODE(workflow_file.stat().st_mode) == 0o644 + + def test_concurrent_catalog_reinstalls_keep_file_and_registry_aligned( + self, project_dir, monkeypatch + ): + import threading + from specify_cli.workflows import _commands + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + from specify_cli.workflows.engine import WorkflowDefinition + + workflows_dir = project_dir / ".specify" / "workflows" + workflow_file = workflows_dir / "align-wf" / "workflow.yml" + workflow_file.parent.mkdir(parents=True) + workflow_file.write_text( + self.WORKFLOW_YAML.format(version="1.0.0"), encoding="utf-8" + ) + WorkflowRegistry(project_dir).add( + "align-wf", + { + "name": "Align Workflow", + "version": "1.0.0", + "source": "catalog", + }, + ) + + versions = {"install-a": "2.0.0", "install-b": "3.0.0"} + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": versions[threading.current_thread().name], + "url": ( + "https://example.com/" + f"{versions[threading.current_thread().name]}.yml" + ), + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + monkeypatch.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, + redirect_validator=None: self._FakeResponse( + self.WORKFLOW_YAML.format( + version=url.rsplit("/", 1)[-1].removesuffix(".yml") + ).encode(), + url, + ), + ) + + a_committed = threading.Event() + b_committed = threading.Event() + a_saving = threading.Event() + b_saved = threading.Event() + real_commit = _commands._commit_workflow_file + real_save = WorkflowRegistry.save + + def coordinated_commit(*args, **kwargs): + backup = real_commit(*args, **kwargs) + if threading.current_thread().name == "install-a": + a_committed.set() + b_committed.wait(0.5) + else: + b_committed.set() + return backup + + def coordinated_save(registry): + if threading.current_thread().name == "install-a": + a_saving.set() + b_saved.wait(0.5) + return real_save(registry) + assert a_saving.wait(2) + real_save(registry) + b_saved.set() + + monkeypatch.setattr( + _commands, "_commit_workflow_file", coordinated_commit + ) + monkeypatch.setattr(WorkflowRegistry, "save", coordinated_save) + + errors = [] + + def install(): + try: + _commands._install_workflow_from_catalog( + project_dir, + workflows_dir, + "align-wf", + ) + except BaseException as exc: + errors.append(exc) + + first = threading.Thread(target=install, name="install-a") + second = threading.Thread(target=install, name="install-b") + first.start() + assert a_committed.wait(2) + second.start() + first.join(5) + second.join(5) + + assert not first.is_alive() + assert not second.is_alive() + assert errors == [] + file_version = WorkflowDefinition.from_yaml(workflow_file).version + registry_version = WorkflowRegistry(project_dir).get("align-wf")[ + "version" + ] + assert file_version == registry_version + + def test_precommit_discard_preserves_concurrent_install(self, project_dir): + from specify_cli.workflows import _commands + + workflow_dir = ( + project_dir / ".specify" / "workflows" / "concurrent-wf" + ) + workflow_dir.mkdir(parents=True) + staged_file = workflow_dir / ".workflow.yml.staged.tmp" + staged_file.write_text("staged", encoding="utf-8") + committed_file = workflow_dir / "workflow.yml" + committed_file.write_text("committed", encoding="utf-8") + + _commands._discard_staged_workflow_file( + staged_file, workflow_dir, existed_before=False + ) + + assert committed_file.read_text(encoding="utf-8") == "committed" + assert not staged_file.exists() + + def test_fresh_install_rollback_preserves_concurrent_staged_file( + self, project_dir + ): + """A second installer stages before taking the transaction lock, so + the first installer's rollback must not recursively remove siblings.""" + from specify_cli.workflows import _commands + + workflow_dir = ( + project_dir / ".specify" / "workflows" / "concurrent-wf" + ) + workflow_dir.mkdir(parents=True) + committed_file = workflow_dir / "workflow.yml" + committed_file.write_text("failed install", encoding="utf-8") + concurrent_stage = workflow_dir / ".workflow.yml.concurrent.tmp" + concurrent_stage.write_text("next install", encoding="utf-8") + + _commands._rollback_committed_workflow_file( + committed_file, + workflow_dir, + existed_before=False, + backup_file=None, + ) + + assert not committed_file.exists() + assert concurrent_stage.read_text(encoding="utf-8") == "next install" + + def test_add_dev_registry_reopen_exit_discards_staged_file( + self, project_dir, monkeypatch + ): + import typer + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows import _commands + + monkeypatch.chdir(project_dir) + source_dir = self._write_workflow_dir(project_dir) + real_open_registry = _commands._open_workflow_registry + calls = 0 + + def fail_transaction_reopen(root): + nonlocal calls + calls += 1 + if calls == 2: + raise typer.Exit(1) + return real_open_registry(root) + + monkeypatch.setattr( + _commands, "_open_workflow_registry", fail_transaction_reopen + ) + result = CliRunner().invoke( + app, ["workflow", "add", str(source_dir), "--dev"] + ) + + assert result.exit_code != 0 + assert not ( + project_dir / ".specify" / "workflows" / "align-wf" + ).exists() + + def test_add_catalog_registry_reopen_exit_discards_staged_file( + self, project_dir, monkeypatch + ): + import typer + from specify_cli.workflows import _commands + from specify_cli.workflows.catalog import WorkflowCatalog + + workflows_dir = project_dir / ".specify" / "workflows" + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + monkeypatch.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, + redirect_validator=None: self._FakeResponse(data, url), + ) + monkeypatch.setattr( + _commands, + "_open_workflow_registry", + lambda _root: (_ for _ in ()).throw(typer.Exit(1)), + ) + + with pytest.raises(typer.Exit): + _commands._install_workflow_from_catalog( + project_dir, + workflows_dir, + "align-wf", + ) + + assert not (workflows_dir / "align-wf").exists() + + def test_remove_serializes_with_concurrent_catalog_install( + self, project_dir, monkeypatch + ): + import threading + from specify_cli.workflows import _commands + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + from specify_cli.workflows.engine import WorkflowDefinition + + workflows_dir = project_dir / ".specify" / "workflows" + workflow_file = workflows_dir / "align-wf" / "workflow.yml" + workflow_file.parent.mkdir(parents=True) + workflow_file.write_text( + self.WORKFLOW_YAML.format(version="1.0.0"), encoding="utf-8" + ) + WorkflowRegistry(project_dir).add( + "align-wf", + { + "name": "Align Workflow", + "version": "1.0.0", + "source": "catalog", + }, + ) + monkeypatch.setattr( + _commands, "_require_specify_project", lambda: project_dir + ) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "2.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + new_data = self.WORKFLOW_YAML.format(version="2.0.0").encode() + monkeypatch.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, + redirect_validator=None: self._FakeResponse(new_data, url), + ) + + removal_ready = threading.Event() + install_done = threading.Event() + real_remove = WorkflowRegistry.remove + + def coordinated_remove(registry, workflow_id): + if threading.current_thread().name == "remove": + removal_ready.set() + install_done.wait(0.5) + return real_remove(registry, workflow_id) + + monkeypatch.setattr(WorkflowRegistry, "remove", coordinated_remove) + errors = [] + + def remove(): + try: + _commands.workflow_remove("align-wf") + except BaseException as exc: + errors.append(exc) + + def install(): + try: + _commands._install_workflow_from_catalog( + project_dir, + workflows_dir, + "align-wf", + ) + except BaseException as exc: + errors.append(exc) + finally: + install_done.set() + + remove_thread = threading.Thread(target=remove, name="remove") + install_thread = threading.Thread(target=install, name="install") + remove_thread.start() + assert removal_ready.wait(2) + install_thread.start() + remove_thread.join(5) + install_thread.join(5) + + assert not remove_thread.is_alive() + assert not install_thread.is_alive() + assert errors == [] + assert WorkflowDefinition.from_yaml(workflow_file).version == "2.0.0" + metadata = WorkflowRegistry(project_dir).get("align-wf") + assert metadata["version"] == "2.0.0" + + @pytest.mark.parametrize( + ("command_name", "initial_enabled", "expected_enabled"), + [ + ("enable", False, True), + ("disable", True, False), + ], + ) + def test_toggle_serializes_with_concurrent_catalog_update( + self, + project_dir, + monkeypatch, + command_name, + initial_enabled, + expected_enabled, + ): + import threading + from specify_cli.workflows import _commands + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + from specify_cli.workflows.engine import WorkflowDefinition + + workflows_dir = project_dir / ".specify" / "workflows" + workflow_file = workflows_dir / "align-wf" / "workflow.yml" + workflow_file.parent.mkdir(parents=True) + workflow_file.write_text( + self.WORKFLOW_YAML.format(version="1.0.0"), encoding="utf-8" + ) + WorkflowRegistry(project_dir).add( + "align-wf", + { + "name": "Align Workflow", + "version": "1.0.0", + "source": "catalog", + "enabled": initial_enabled, + }, + ) + monkeypatch.setattr( + _commands, "_require_specify_project", lambda: project_dir + ) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "2.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + new_data = self.WORKFLOW_YAML.format(version="2.0.0").encode() + monkeypatch.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, + redirect_validator=None: self._FakeResponse(new_data, url), + ) + + toggle_ready = threading.Event() + update_done = threading.Event() + real_add = WorkflowRegistry.add + + def coordinated_add(registry, workflow_id, metadata): + if threading.current_thread().name == "toggle": + toggle_ready.set() + update_done.wait(0.5) + return real_add(registry, workflow_id, metadata) + + monkeypatch.setattr(WorkflowRegistry, "add", coordinated_add) + errors = [] + + def toggle(): + try: + getattr(_commands, f"workflow_{command_name}")("align-wf") + except BaseException as exc: + errors.append(exc) + + def update(): + try: + _commands._install_workflow_from_catalog( + project_dir, + workflows_dir, + "align-wf", + ) + except BaseException as exc: + errors.append(exc) + finally: + update_done.set() + + toggle_thread = threading.Thread(target=toggle, name="toggle") + update_thread = threading.Thread(target=update, name="update") + toggle_thread.start() + assert toggle_ready.wait(2) + update_thread.start() + toggle_thread.join(5) + update_thread.join(5) + + assert not toggle_thread.is_alive() + assert not update_thread.is_alive() + assert errors == [] + assert WorkflowDefinition.from_yaml(workflow_file).version == "2.0.0" + metadata = WorkflowRegistry(project_dir).get("align-wf") + assert metadata["version"] == "2.0.0" + assert metadata.get("enabled", True) is expected_enabled + + def test_add_catalog_reinstall_restore_failure_reports_warning_and_original_error( + self, project_dir, monkeypatch + ): + """Same restore-rename boundary as the local-install path: the + prior file is restored via an atomic rename (not a content rewrite) + when registry.add() fails on a reinstall. If that restore rename + itself also fails, it must report a clear warning in addition to + the original clean registry error, never crash or silently claim + success.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + original_data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + runner = CliRunner() + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse( + original_data, url + ), + ) + result = runner.invoke(app, ["workflow", "add", "align-wf"]) + assert result.exit_code == 0, result.output + + new_data = self.WORKFLOW_YAML.format(version="2.0.0").encode() + + def save_boom(self): + raise OSError("disk full") + + real_replace = os.replace + calls = {"n": 0} + + def replace_boom(src_path, dst_path): + # The commit swap for a reinstall makes exactly two os.replace + # calls (backup-aside, then staged-into-dest); let both succeed + # and only fail the third call -- the post-registry-failure + # restore-back rename. + calls["n"] += 1 + if calls["n"] <= 2: + return real_replace(src_path, dst_path) + raise OSError("permission denied") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse( + new_data, url + ), + ) + mp.setattr(WorkflowRegistry, "save", save_boom) + mp.setattr(os, "replace", replace_boom) + result = runner.invoke(app, ["workflow", "add", "align-wf"]) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + output_compact = "".join(result.output.split()) + assert "Warning" in result.output + assert "diskfull" in output_compact + assert "permissiondenied" in output_compact + + def test_add_catalog_fresh_install_into_preexisting_empty_dir_cleans_new_file( + self, project_dir, monkeypatch + ): + """Same rollback orphan gap as the local-install path, but for a + fresh catalog install: a pre-existing empty destination directory + (no workflow.yml) sets existed_before=True with no backup bytes, so + the rollback previously did nothing on a later failure -- leaving + the freshly downloaded workflow.yml behind. It must be removed, + leaving the pre-existing directory itself intact.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + dest_dir = project_dir / ".specify" / "workflows" / "align-wf" + dest_dir.mkdir(parents=True) # pre-existing, but empty: no workflow.yml + data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + + def boom(self): + raise OSError("disk full") + + runner = CliRunner() + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse(data, url), + ) + mp.setattr(WorkflowRegistry, "save", boom) + result = runner.invoke(app, ["workflow", "add", "align-wf"]) + + assert result.exit_code != 0 + assert result.output.strip() != "" + assert dest_dir.is_dir() + assert not (dest_dir / "workflow.yml").exists() + + @pytest.mark.parametrize( + "mode", ["redirect_rejected", "download_exception", "invalid_yaml", "id_mismatch"] + ) + def test_add_catalog_reinstall_early_failure_restores_prior_file( + self, project_dir, monkeypatch, mode + ): + """Every _install_workflow_from_catalog failure branch that runs after + the mkdir/download step -- not just the registry.add() OSError case + -- must route through the same existed-before/backup-aware cleanup: + on a reinstall, a redirect rejection, a download exception, invalid + YAML, or a workflow-id mismatch must restore the prior working + workflow.yml rather than deleting the whole directory. One shared + root cause (the cleanup helper), so parametrized over trigger point.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + original_data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + runner = CliRunner() + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse( + original_data, url + ), + ) + result = runner.invoke(app, ["workflow", "add", "align-wf"]) + assert result.exit_code == 0, result.output + + dest_file = project_dir / ".specify" / "workflows" / "align-wf" / "workflow.yml" + assert dest_file.read_bytes() == original_data + + if mode == "redirect_rejected": + def fake_open_url(url, timeout=None, extra_headers=None, redirect_validator=None): + return self._FakeResponse(b"irrelevant", "http://evil.example.com/workflow.yml") + elif mode == "download_exception": + def fake_open_url(url, timeout=None, extra_headers=None, redirect_validator=None): + raise OSError("network down") + elif mode == "invalid_yaml": + def fake_open_url(url, timeout=None, extra_headers=None, redirect_validator=None): + return self._FakeResponse(b": : not valid yaml: [", url) + else: # id_mismatch + mismatched_yaml = self.WORKFLOW_YAML.format(version="2.0.0").replace( + 'id: "align-wf"', 'id: "different-workflow"' + ) + + def fake_open_url(url, timeout=None, extra_headers=None, redirect_validator=None): + return self._FakeResponse(mismatched_yaml.encode(), url) + + with pytest.MonkeyPatch.context() as mp: + mp.setattr("specify_cli.authentication.http.open_url", fake_open_url) + result = runner.invoke(app, ["workflow", "add", "align-wf"]) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert result.output.strip() != "" + assert dest_file.read_bytes() == original_data + registry = WorkflowRegistry(project_dir) + assert registry.is_installed("align-wf") + assert registry.get("align-wf")["version"] == "1.0.0" + + def test_download_redirect_validator_rejects_http_before_follow(self): + import urllib.error + + from specify_cli.workflows._commands import _reject_insecure_download_redirect + + with pytest.raises(urllib.error.URLError): + _reject_insecure_download_redirect( + "https://example.com/wf.yml", "http://evil.example.com/wf.yml" + ) + # Allowed: HTTPS anywhere, HTTP on loopback. + _reject_insecure_download_redirect( + "https://example.com/wf.yml", "https://cdn.example.com/wf.yml" + ) + _reject_insecure_download_redirect( + "https://example.com/wf.yml", "http://localhost:8000/wf.yml" + ) + _reject_insecure_download_redirect( + "https://example.com/wf.yml", "http://127.0.0.1/wf.yml" + ) + + def test_add_from_url_passes_redirect_validator(self, project_dir, monkeypatch): + from unittest.mock import patch + + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + seen: dict[str, object] = {} + + def fake_open(url, timeout=None, extra_headers=None, redirect_validator=None): + seen["validator"] = redirect_validator + return self._FakeResponse(data, url) + + runner = CliRunner() + with patch("specify_cli.authentication.http.open_url", side_effect=fake_open): + result = runner.invoke( + app, + ["workflow", "add", "align-wf", "--from", "https://example.com/workflow.yml"], + input="y\n", + ) + assert result.exit_code == 0, result.output + from specify_cli.workflows._commands import _reject_insecure_download_redirect + + assert seen["validator"] is _reject_insecure_download_redirect + + @pytest.mark.skipif(sys.platform == "win32", reason="chmod mode bits not reliable on Windows") + def test_registry_save_failure_preserves_file_on_disk(self, project_dir, monkeypatch): + """A failed dump must not truncate the persisted registry, and must + not alter its on-disk mode either -- the chmod-to-match-existing-mode + step operates on the temp file, never the target, so a failed save + (which never reaches os.replace) cannot touch the original's mode.""" + from specify_cli.workflows.catalog import WorkflowRegistry + + registry = WorkflowRegistry(project_dir) + registry.add("align-wf", {"version": "1.0.0", "source": "catalog"}) + registry.registry_path.chmod(0o644) + + import specify_cli.workflows.catalog as catalog_mod + + def boom(*args, **kwargs): + raise OSError("disk full") + + monkeypatch.setattr(catalog_mod.json, "dump", boom) + with pytest.raises(OSError): + registry.add("align-wf", {"version": "2.0.0", "source": "catalog"}) + monkeypatch.undo() + + fresh = WorkflowRegistry(project_dir) + assert fresh.get("align-wf")["version"] == "1.0.0" + assert stat.S_IMODE(registry.registry_path.stat().st_mode) == 0o644 + assert not list(registry.workflows_dir.glob("*.tmp")) + + def test_update_mixed_targets_does_not_claim_all_up_to_date(self, project_dir, monkeypatch): + """Skipped targets must not be presented as verified up to date.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) # local source → skipped + WorkflowRegistry(project_dir).add("catalog-wf", { + "name": "Catalog Workflow", + "version": "1.0.0", + "description": "", + "source": "catalog", + "url": "https://example.com/workflow.yml", + }) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + }, + ) + result = runner.invoke(app, ["workflow", "update"]) + assert result.exit_code == 0, result.output + assert "All workflows are up to date!" not in result.output + assert "All checked workflows are up to date" in result.output + assert "skipped" in result.output + + def test_run_refuses_falsy_non_bool_enabled(self, project_dir, monkeypatch): + """A falsy non-bool "enabled" (0) shows as disabled in list — run must agree.""" + import json as json_mod + + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) + + registry = WorkflowRegistry(project_dir) + registry.data["workflows"]["align-wf"]["enabled"] = 0 + registry.registry_path.write_text(json_mod.dumps(registry.data), encoding="utf-8") + + result = runner.invoke(app, ["workflow", "run", "align-wf"]) + assert result.exit_code != 0 + assert "disabled" in result.output + + def test_update_installs_newer_catalog_version(self, project_dir, monkeypatch): + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + registry = WorkflowRegistry(project_dir) + registry.add("align-wf", { + "name": "Align Workflow", + "version": "1.0.0", + "description": "CLI alignment test workflow", + "source": "catalog", + "catalog_name": "test-catalog", + "url": "https://example.com/workflow.yml", + }) + wf_dir = project_dir / ".specify" / "workflows" / "align-wf" + wf_dir.mkdir(parents=True) + (wf_dir / "workflow.yml").write_text( + self.WORKFLOW_YAML.format(version="1.0.0"), encoding="utf-8" + ) + + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "2.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + data = self.WORKFLOW_YAML.format(version="2.0.0").encode() + runner = CliRunner() + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse(data, url), + ): + result = runner.invoke(app, ["workflow", "update"], input="y\n") + assert result.exit_code == 0, result.output + assert "1.0.0" in result.output and "2.0.0" in result.output + meta = WorkflowRegistry(project_dir).get("align-wf") + assert meta["version"] == "2.0.0" + assert "2.0.0" in (wf_dir / "workflow.yml").read_text(encoding="utf-8") + + def test_update_downloaded_invalid_yaml_escapes_rich_markup(self, project_dir, monkeypatch): + """A malformed downloaded workflow can quote the offending line verbatim; escape it before printing.""" + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + from specify_cli.workflows.engine import WorkflowDefinition + + monkeypatch.chdir(project_dir) + registry = WorkflowRegistry(project_dir) + registry.add("align-wf", { + "name": "Align Workflow", + "version": "1.0.0", + "description": "CLI alignment test workflow", + "source": "catalog", + "catalog_name": "test-catalog", + "url": "https://example.com/workflow.yml", + }) + wf_dir = project_dir / ".specify" / "workflows" / "align-wf" + wf_dir.mkdir(parents=True) + (wf_dir / "workflow.yml").write_text( + self.WORKFLOW_YAML.format(version="1.0.0"), encoding="utf-8" + ) + + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "2.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + runner = CliRunner() + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse(b"", url), + ), patch.object( + WorkflowDefinition, + "from_string", + side_effect=ValueError('bad snippet: "New [Feature]"'), + ): + result = runner.invoke(app, ["workflow", "update"], input="y\n") + assert 'bad snippet: "New [Feature]"' in result.output + assert "Failed to update" in result.output + # The previously installed workflow must survive a failed update. + assert "1.0.0" in (wf_dir / "workflow.yml").read_text(encoding="utf-8") + + def test_update_malformed_catalog_url_fails_cleanly(self, project_dir, monkeypatch): + """An unparseable catalog URL (unbalanced IPv6 literal) must not abort the whole update.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + registry = WorkflowRegistry(project_dir) + registry.add("align-wf", { + "name": "Align Workflow", + "version": "1.0.0", + "description": "CLI alignment test workflow", + "source": "catalog", + "catalog_name": "test-catalog", + "url": "https://[::1/workflow.yml", + }) + wf_dir = project_dir / ".specify" / "workflows" / "align-wf" + wf_dir.mkdir(parents=True) + (wf_dir / "workflow.yml").write_text( + self.WORKFLOW_YAML.format(version="1.0.0"), encoding="utf-8" + ) + + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "2.0.0", + "url": "https://[::1/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "update"], input="y\n") + assert "malformed install URL" in result.output + assert "Failed to update" in result.output + # The previously installed workflow must survive. + assert "1.0.0" in (wf_dir / "workflow.yml").read_text(encoding="utf-8") + + def test_add_non_string_catalog_url_fails_cleanly(self, project_dir, monkeypatch): + """A truthy non-string catalog URL must hit the clean error path, not AttributeError.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog + + monkeypatch.chdir(project_dir) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "1.0.0", + "url": 123, + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "add", "align-wf"]) + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "malformed install URL" in result.output + + def test_enable_failed_save_leaves_workflow_disabled(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) + result = runner.invoke(app, ["workflow", "disable", "align-wf"]) + assert result.exit_code == 0, result.output + + def boom(self): + raise OSError("disk full") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(WorkflowRegistry, "save", boom) + result = runner.invoke(app, ["workflow", "enable", "align-wf"]) + assert result.exit_code != 0 + + assert WorkflowRegistry(project_dir).get("align-wf")["enabled"] is False + result = runner.invoke(app, ["workflow", "enable", "align-wf"]) + assert result.exit_code == 0, result.output + assert WorkflowRegistry(project_dir).get("align-wf")["enabled"] is True + + @pytest.mark.parametrize("command", ["enable", "disable"]) + def test_enable_disable_save_failure_gives_clean_output( + self, project_dir, monkeypatch, command + ): + """A save() failure in enable/disable must produce a clean escaped CLI + error, not surface the raw OSError as an unhandled exception. Shared + root behavior: both call registry.add() with a fresh mapping and must + catch its deliberate OSError the same way.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) + # disable starts from the enabled default; enable needs a prior disable. + starting_enabled = command == "disable" + if command == "enable": + pre = runner.invoke(app, ["workflow", "disable", "align-wf"]) + assert pre.exit_code == 0, pre.output + + def boom(self): + raise OSError("disk full") + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(WorkflowRegistry, "save", boom) + result = runner.invoke(app, ["workflow", command, "align-wf"]) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert result.output.strip() != "" + assert ( + WorkflowRegistry(project_dir).get("align-wf").get("enabled", True) + is starting_enabled + ) + + def test_update_rejects_version_mismatch_from_stale_url(self, project_dir, monkeypatch): + """A URL serving a different version than the catalog advertised must fail the update.""" + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + WorkflowRegistry(project_dir).add("align-wf", { + "name": "Align Workflow", + "version": "1.0.0", + "description": "CLI alignment test workflow", + "source": "catalog", + "catalog_name": "test-catalog", + "url": "https://example.com/workflow.yml", + }) + wf_dir = project_dir / ".specify" / "workflows" / "align-wf" + wf_dir.mkdir(parents=True) + (wf_dir / "workflow.yml").write_text( + self.WORKFLOW_YAML.format(version="1.0.0"), encoding="utf-8" + ) + + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "2.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + # The URL still serves the old 1.0.0 payload. + data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + runner = CliRunner() + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse(data, url), + ): + result = runner.invoke(app, ["workflow", "update"], input="y\n") + assert "does not match the catalog version" in result.output + assert "Failed to update" in result.output + meta = WorkflowRegistry(project_dir).get("align-wf") + assert meta["version"] == "1.0.0" + assert "1.0.0" in (wf_dir / "workflow.yml").read_text(encoding="utf-8") + + def test_update_preserves_disabled_state(self, project_dir, monkeypatch): + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + WorkflowRegistry(project_dir).add("align-wf", { + "name": "Align Workflow", + "version": "1.0.0", + "description": "", + "source": "catalog", + "url": "https://example.com/workflow.yml", + "enabled": False, + }) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "version": "2.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + }, + ) + data = self.WORKFLOW_YAML.format(version="2.0.0").encode() + runner = CliRunner() + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse(data, url), + ): + result = runner.invoke(app, ["workflow", "update"], input="y\n") + assert result.exit_code == 0, result.output + meta = WorkflowRegistry(project_dir).get("align-wf") + assert meta["version"] == "2.0.0" + assert meta["enabled"] is False + + @pytest.mark.skipif(os.name == "nt", reason="POSIX permission bits") + def test_update_preserves_workflow_file_mode(self, project_dir, monkeypatch): + import stat + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + WorkflowRegistry(project_dir).add("align-wf", { + "name": "Align Workflow", + "version": "1.0.0", + "description": "", + "source": "catalog", + "url": "https://example.com/workflow.yml", + }) + workflow_file = ( + project_dir + / ".specify" + / "workflows" + / "align-wf" + / "workflow.yml" + ) + workflow_file.parent.mkdir(parents=True) + workflow_file.write_text( + self.WORKFLOW_YAML.format(version="1.0.0"), encoding="utf-8" + ) + workflow_file.chmod(0o640) + + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "version": "2.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + }, + ) + data = self.WORKFLOW_YAML.format(version="2.0.0").encode() + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, + redirect_validator=None: self._FakeResponse(data, url), + ): + result = CliRunner().invoke( + app, ["workflow", "update"], input="y\n" + ) + + assert result.exit_code == 0, result.output + assert stat.S_IMODE(workflow_file.stat().st_mode) == 0o640 + + def test_update_skips_corrupted_registry_entry(self, project_dir, monkeypatch): + import json + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + registry_path = WorkflowRegistry(project_dir).registry_path + registry_path.parent.mkdir(parents=True, exist_ok=True) + registry_path.write_text( + json.dumps({"schema_version": "1.0", "workflows": {"broken": "not-a-dict"}}), + encoding="utf-8", + ) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "update"]) + assert result.exit_code == 0, result.output + assert "corrupted" in result.output + + def test_list_skips_corrupted_registry_entry(self, project_dir, monkeypatch): + import json + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + registry_path = WorkflowRegistry(project_dir).registry_path + registry_path.parent.mkdir(parents=True, exist_ok=True) + registry_path.write_text( + json.dumps( + { + "schema_version": "1.0", + "workflows": { + "broken": "not-a-dict", + "ok": {"name": "OK Workflow", "version": "1.0.0"}, + }, + } + ), + encoding="utf-8", + ) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "list"]) + assert result.exit_code == 0, result.output + assert "corrupted" in result.output + assert "OK Workflow" in result.output + + def test_list_unreadable_registry_fails_closed_with_clean_error( + self, project_dir, monkeypatch + ): + """An unreadable registry file must produce a clean CLI error, not a + raw traceback and not a silent "nothing installed" list -- the latter + is exactly the fail-open state a caller could otherwise mistake for + "safe to (re)install", overwriting real files. Covers the read/query + boundary fix required at every WorkflowRegistry call site.""" + import builtins + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) + + registry_path = WorkflowRegistry(project_dir).registry_path.resolve() + real_open = builtins.open + + def _raising_open(file, mode="r", *args, **kwargs): + if Path(file).resolve() == registry_path and "r" in mode: + raise OSError("simulated read failure") + return real_open(file, mode, *args, **kwargs) + + monkeypatch.setattr(builtins, "open", _raising_open) + result = runner.invoke(app, ["workflow", "list"]) + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "Error" in result.output + + def test_list_escapes_rich_markup_in_registry_fields(self, project_dir, monkeypatch): + """User-editable name/description/id fields must not be parsed as Rich markup.""" + import json + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + registry_path = WorkflowRegistry(project_dir).registry_path + registry_path.parent.mkdir(parents=True, exist_ok=True) + registry_path.write_text( + json.dumps( + { + "schema_version": "1.0", + "workflows": { + "ok": { + "name": "Bracket [Test]", + "version": "1.0.0", + "description": "desc [with] brackets", + }, + }, + } + ), + encoding="utf-8", + ) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "list"]) + assert result.exit_code == 0, result.output + assert "Bracket [Test]" in result.output + assert "desc [with] brackets" in result.output + + def test_update_reports_unsafe_registry_id_per_workflow(self, project_dir, monkeypatch): + """An unsafe workflow id in the registry must fail that one entry, not abort the whole update.""" + import json + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry, WorkflowCatalog + + monkeypatch.chdir(project_dir) + registry_path = WorkflowRegistry(project_dir).registry_path + registry_path.parent.mkdir(parents=True, exist_ok=True) + registry_path.write_text( + json.dumps( + { + "schema_version": "1.0", + "workflows": { + "../evil": { + "name": "Bad", + "version": "0.0.1", + "source": "catalog", + "url": "https://example.com/evil.yml", + }, + }, + } + ), + encoding="utf-8", + ) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: {"version": "9.9.9", "url": "https://example.com/evil.yml", "_install_allowed": True}, + ) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "update"], input="y\n") + assert result.exit_code != 0 + assert "Failed to update" in result.output + + def test_update_registry_save_failure_restores_prior_file_without_redundant_write( + self, project_dir, monkeypatch + ): + """A registry.add() save failure during `workflow update` must be + fully restored by _install_workflow_from_catalog's own atomic + rollback (rename-based, not a byte-level rewrite). The outer + workflow_update loop must not perform any redundant write of its + own onto the destination file -- that write happened only after + typer.Exit already unwound, could itself fail/truncate the safely + preserved file, and is provably unnecessary here since the inner + transaction already restored it via rename.""" + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + original_data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + runner = CliRunner() + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse( + original_data, url + ), + ) + result = runner.invoke(app, ["workflow", "add", "align-wf"]) + assert result.exit_code == 0, result.output + + dest_file = project_dir / ".specify" / "workflows" / "align-wf" / "workflow.yml" + assert dest_file.read_bytes() == original_data + + new_data = self.WORKFLOW_YAML.format(version="2.0.0").encode() + + def boom_save(self): + raise OSError("disk full") + + dest_writes: list[bytes] = [] + real_write_bytes = Path.write_bytes + resolved_dest_file = dest_file.resolve() + + def tracking_write_bytes(self_path, data, *args, **kwargs): + if self_path.resolve() == resolved_dest_file: + dest_writes.append(data) + return real_write_bytes(self_path, data, *args, **kwargs) + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "2.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + mp.setattr( + "specify_cli.authentication.http.open_url", + lambda url, timeout=None, extra_headers=None, redirect_validator=None: self._FakeResponse( + new_data, url + ), + ) + mp.setattr(WorkflowRegistry, "save", boom_save) + mp.setattr(Path, "write_bytes", tracking_write_bytes) + result = runner.invoke(app, ["workflow", "update"], input="y\n") + + assert result.exit_code != 0 + assert "Failed to update" in result.output + # No redundant/second write of the destination file was attempted -- + # the inner atomic commit/rollback (rename-based) is the only thing + # that ever touches it. + assert dest_writes == [] + assert dest_file.read_bytes() == original_data + registry = WorkflowRegistry(project_dir) + assert registry.is_installed("align-wf") + assert registry.get("align-wf")["version"] == "1.0.0" + + def test_update_non_json_description_restores_prior_file_and_registry( + self, project_dir, monkeypatch + ): + from unittest.mock import patch + + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + registry = WorkflowRegistry(project_dir) + registry.add( + "align-wf", + { + "name": "Align Workflow", + "version": "1.0.0", + "description": "", + "source": "catalog", + "url": "https://example.com/workflow.yml", + }, + ) + workflow_file = ( + project_dir + / ".specify" + / "workflows" + / "align-wf" + / "workflow.yml" + ) + workflow_file.parent.mkdir(parents=True) + original_data = self.WORKFLOW_YAML.format(version="1.0.0").encode() + workflow_file.write_bytes(original_data) + + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "2.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + invalid_data = self.WORKFLOW_YAML.format(version="2.0.0").replace( + 'description: "CLI alignment test workflow"', + "description: 2026-01-02", + ).encode() + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, + redirect_validator=None: self._FakeResponse(invalid_data, url), + ): + result = CliRunner().invoke( + app, ["workflow", "update", "align-wf"], input="y\n" + ) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "Failed to update workflow registry" in result.output + assert workflow_file.read_bytes() == original_data + current = WorkflowRegistry(project_dir).get("align-wf") + assert current["version"] == "1.0.0" + leftovers = [ + path.name + for path in workflow_file.parent.iterdir() + if path.name != "workflow.yml" + ] + assert leftovers == [] + + def test_commit_failure_reports_unrestored_backup_location( + self, tmp_path, monkeypatch + ): + from specify_cli.workflows import _commands + + dest_dir = tmp_path / "align-wf" + dest_dir.mkdir() + dest_file = dest_dir / "workflow.yml" + staged_file = dest_dir / ".workflow.yml.staged" + dest_file.write_text("original", encoding="utf-8") + staged_file.write_text("replacement", encoding="utf-8") + + real_replace = os.replace + calls = 0 + backup_file = None + + def fail_commit_and_restore(src, dst): + nonlocal backup_file, calls + calls += 1 + if calls == 1: + backup_file = Path(dst) + return real_replace(src, dst) + if calls == 2: + raise OSError("commit denied") + raise OSError("restore denied") + + monkeypatch.setattr(os, "replace", fail_commit_and_restore) + with pytest.raises(OSError) as exc_info: + _commands._commit_workflow_file( + staged_file, dest_file, existed_before=True + ) + + message = str(exc_info.value) + assert "commit denied" in message + assert "restore denied" in message + assert backup_file is not None + assert str(backup_file) in message + assert not dest_file.exists() + assert backup_file.read_text(encoding="utf-8") == "original" + + def test_commit_uses_unique_backup_without_overwriting_existing_sibling( + self, tmp_path + ): + from specify_cli.workflows import _commands + + dest_dir = tmp_path / "align-wf" + dest_dir.mkdir() + dest_file = dest_dir / "workflow.yml" + staged_file = dest_dir / ".workflow.yml.staged" + fixed_backup = dest_dir / "workflow.yml.bak" + dest_file.write_text("original", encoding="utf-8") + staged_file.write_text("replacement", encoding="utf-8") + fixed_backup.write_text("diagnostic copy", encoding="utf-8") + + backup_file = _commands._commit_workflow_file( + staged_file, dest_file, existed_before=True + ) + + assert backup_file is not None + assert backup_file != fixed_backup + assert backup_file.read_text(encoding="utf-8") == "original" + assert fixed_backup.read_text(encoding="utf-8") == "diagnostic copy" + assert dest_file.read_text(encoding="utf-8") == "replacement" + + @pytest.mark.parametrize( + ("replacement_source", "replacement_version"), + [("local", "1.0.0"), ("catalog", "1.5.0")], + ) + def test_update_rechecks_registry_after_confirmation( + self, project_dir, monkeypatch, replacement_source, replacement_version + ): + from unittest.mock import patch + + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows import _commands + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + registry = WorkflowRegistry(project_dir) + registry.add( + "align-wf", + { + "name": "Align Workflow", + "version": "1.0.0", + "description": "", + "source": "catalog", + "url": "https://example.com/workflow.yml", + }, + ) + workflow_file = ( + project_dir + / ".specify" + / "workflows" + / "align-wf" + / "workflow.yml" + ) + workflow_file.parent.mkdir(parents=True) + workflow_file.write_text( + self.WORKFLOW_YAML.format(version="1.0.0"), encoding="utf-8" + ) + replacement_data = self.WORKFLOW_YAML.format( + version=replacement_version + ).replace( + 'description: "CLI alignment test workflow"', + 'description: "concurrent replacement"', + ).encode() + + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "name": "Align Workflow", + "version": "2.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + "_catalog_name": "test-catalog", + }, + ) + + def replace_while_confirming(*args, **kwargs): + WorkflowRegistry(project_dir).add( + "align-wf", + { + "name": "Concurrent replacement", + "version": replacement_version, + "description": "", + "source": replacement_source, + }, + ) + workflow_file.write_bytes(replacement_data) + return True + + monkeypatch.setattr(_commands.typer, "confirm", replace_while_confirming) + catalog_data = self.WORKFLOW_YAML.format(version="2.0.0").encode() + with patch( + "specify_cli.authentication.http.open_url", + side_effect=lambda url, timeout=None, extra_headers=None, + redirect_validator=None: self._FakeResponse(catalog_data, url), + ): + result = CliRunner().invoke(app, ["workflow", "update", "align-wf"]) + + assert result.exit_code != 0 + assert "changed during update" in result.output + assert workflow_file.read_bytes() == replacement_data + current = WorkflowRegistry(project_dir).get("align-wf") + assert current["source"] == replacement_source + assert current["version"] == replacement_version + + @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable") + def test_resume_rejects_symlinked_cross_project_owner_root( + self, project_dir, tmp_path + ): + from specify_cli.workflows import _commands + + real_owner = tmp_path / "real-owner" + real_owner.mkdir() + owner_link = tmp_path / "owner-link" + owner_link.symlink_to(real_owner, target_is_directory=True) + + with pytest.raises(ValueError, match="unavailable"): + _commands._resolve_run_owner_root(str(owner_link), project_dir) + + def test_enable_disable_corrupted_registry_entry_errors(self, project_dir, monkeypatch): + import json + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + registry_path = WorkflowRegistry(project_dir).registry_path + registry_path.parent.mkdir(parents=True, exist_ok=True) + registry_path.write_text( + json.dumps({"schema_version": "1.0", "workflows": {"broken": "not-a-dict"}}), + encoding="utf-8", + ) + runner = CliRunner() + for cmd in ("enable", "disable"): + result = runner.invoke(app, ["workflow", cmd, "broken"]) + assert result.exit_code != 0 + assert "corrupted" in result.output + + def test_update_up_to_date_reports_and_exits_zero(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + WorkflowRegistry(project_dir).add("align-wf", { + "name": "Align Workflow", + "version": "1.0.0", + "description": "", + "source": "catalog", + "url": "https://example.com/workflow.yml", + }) + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "version": "1.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + }, + ) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "update"]) + assert result.exit_code == 0, result.output + assert "Up to date" in result.output + assert "All workflows are up to date!" in result.output + + def test_update_restores_backup_on_failed_download(self, project_dir, monkeypatch): + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + + monkeypatch.chdir(project_dir) + WorkflowRegistry(project_dir).add("align-wf", { + "name": "Align Workflow", + "version": "1.0.0", + "description": "", + "source": "catalog", + "url": "https://example.com/workflow.yml", + }) + wf_dir = project_dir / ".specify" / "workflows" / "align-wf" + wf_dir.mkdir(parents=True) + original = self.WORKFLOW_YAML.format(version="1.0.0") + (wf_dir / "workflow.yml").write_text(original, encoding="utf-8") + + monkeypatch.setattr( + WorkflowCatalog, + "get_workflow_info", + lambda self, wid: { + "id": wid, + "version": "2.0.0", + "url": "https://example.com/workflow.yml", + "_install_allowed": True, + }, + ) + + def boom(url, timeout=None, extra_headers=None, redirect_validator=None): + raise OSError("network down") + + runner = CliRunner() + with patch("specify_cli.authentication.http.open_url", side_effect=boom): + result = runner.invoke(app, ["workflow", "update"], input="y\n") + assert result.exit_code != 0 + assert "Failed to update" in result.output + # Working copy and registry version are untouched + assert (wf_dir / "workflow.yml").read_text(encoding="utf-8") == original + assert WorkflowRegistry(project_dir).get("align-wf")["version"] == "1.0.0" + + # -- enable / disable ------------------------------------------------ + + def test_disable_blocks_run_enable_restores(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) + + result = runner.invoke(app, ["workflow", "disable", "align-wf"]) + assert result.exit_code == 0, result.output + assert WorkflowRegistry(project_dir).get("align-wf")["enabled"] is False + + result = runner.invoke(app, ["workflow", "run", "align-wf"]) + assert result.exit_code != 0 + assert "disabled" in result.output + + result = runner.invoke(app, ["workflow", "enable", "align-wf"]) + assert result.exit_code == 0, result.output + assert WorkflowRegistry(project_dir).get("align-wf")["enabled"] is True + + result = runner.invoke(app, ["workflow", "run", "align-wf"]) + assert result.exit_code == 0, result.output + + def test_run_rejects_corrupted_registry_entry(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) + + registry = WorkflowRegistry(project_dir) + registry.data["workflows"]["align-wf"] = "corrupted" + registry.save() + + result = runner.invoke(app, ["workflow", "run", "align-wf"]) + assert result.exit_code != 0 + assert "corrupted" in result.output + + def test_run_rejects_corrupt_registry_file(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) + + registry_path = WorkflowRegistry(project_dir).registry_path + registry_path.write_text("not json", encoding="utf-8") + + result = runner.invoke(app, ["workflow", "run", "align-wf"]) + + assert result.exit_code != 0 + assert "registry" in result.output.lower() + assert "corrupt" in result.output.lower() + + def test_disable_blocks_case_variant_installed_path( + self, project_dir, monkeypatch + ): + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) + + result = runner.invoke(app, ["workflow", "disable", "align-wf"]) + assert result.exit_code == 0, result.output + + case_variant = ( + project_dir + / ".SPECIFY" + / "WORKFLOWS" + / "ALIGN-WF" + / "workflow.yml" + ) + if not case_variant.is_file(): + pytest.skip("filesystem is case-sensitive") + + result = runner.invoke( + app, ["workflow", "run", str(case_variant)] + ) + + assert result.exit_code != 0 + assert "disabled" in result.output + + def test_disable_blocks_run_via_path_equivalent_id(self, project_dir, monkeypatch): + """Path spelling "align-wf/" must not run a disabled workflow by dodging the registry lookup.""" + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) + + result = runner.invoke(app, ["workflow", "disable", "align-wf"]) + assert result.exit_code == 0, result.output + + for spelling in ("align-wf/", "align-wf/."): + result = runner.invoke(app, ["workflow", "run", spelling]) + assert result.exit_code != 0, spelling + assert "Invalid workflow ID" in result.output, spelling + + # Direct path to the installed workflow's own YAML must also refuse. + installed_yaml = ".specify/workflows/align-wf/workflow.yml" + assert (project_dir / installed_yaml).is_file() + result = runner.invoke(app, ["workflow", "run", installed_yaml]) + assert result.exit_code != 0 + assert "disabled" in result.output + + # Same guard must hold when invoked from outside the project. + outside = project_dir.parent / "outside-cwd" + outside.mkdir(exist_ok=True) + monkeypatch.chdir(outside) + result = runner.invoke( + app, ["workflow", "run", str(project_dir / installed_yaml)] + ) + assert result.exit_code != 0 + assert "disabled" in result.output + + @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable") + def test_disable_blocks_run_when_installed_yaml_is_symlinked( + self, project_dir, monkeypatch + ): + """A disabled workflow's own workflow.yml being replaced with a symlink + must not bypass the disabled check. Resolving the path before mapping + it back to its registry owner would follow the symlink out of + .specify/workflows, fail to find an owner, and let engine.load_workflow + run the original symlink target anyway -- ownership must be + determined from the normalized *lexical* path (not resolve()), and a + symlinked path component in the installed tree must be refused.""" + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) + + result = runner.invoke(app, ["workflow", "disable", "align-wf"]) + assert result.exit_code == 0, result.output + + installed_yaml = project_dir / ".specify" / "workflows" / "align-wf" / "workflow.yml" + external_target = project_dir / "external-workflow.yml" + external_target.write_text( + self.WORKFLOW_YAML.format(version="9.9.9"), encoding="utf-8" + ) + installed_yaml.unlink() + installed_yaml.symlink_to(external_target) + + result = runner.invoke(app, ["workflow", "run", str(installed_yaml)]) + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "disabled" in result.output or "symlink" in result.output.lower() + + @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable") + def test_alias_rejects_symlinked_workflow_storage_before_resolve( + self, project_dir, temp_dir + ): + import shutil + import typer + from specify_cli.workflows import _commands + + specify_dir = project_dir / ".specify" + shutil.rmtree(specify_dir) + redirected = temp_dir / "redirected-storage" + workflow_file = redirected / "workflows" / "evil" / "workflow.yml" + workflow_file.parent.mkdir(parents=True) + workflow_file.write_text( + self.WORKFLOW_YAML.format(version="1.0.0"), encoding="utf-8" + ) + specify_dir.symlink_to(redirected, target_is_directory=True) + alias = temp_dir / "workflow-alias.yml" + alias.symlink_to( + project_dir + / ".specify" + / "workflows" + / "evil" + / "workflow.yml" + ) + + with pytest.raises(typer.Exit): + _commands._resolve_installed_workflow_ownership( + alias, _commands.err_console + ) + + @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable") + def test_run_refuses_symlinked_specify_dir_hiding_disabled_workflow( + self, temp_dir, monkeypatch + ): + """A victim project's own .specify directory being a symlink to an + attacker-controlled tree must not bypass the disabled-workflow guard. + _reject_unsafe_workflow_storage only checks the *cwd's* project root + (unrelated here); the id/leaf symlink-component loop only checks + components from the id directory onward, missing .specify/ + .specify/workflows themselves. The ownership check must reject an + unsafe .specify/.specify-workflows for the actual path-derived + registry root before ever consulting the registry -- it must not + rely on WorkflowRegistry's own symlinked-parent handling, which + raises a generic OSError; the ownership guard should surface the + specific unsafe-storage error before registry construction.""" + from typer.testing import CliRunner + from specify_cli import app + + victim = temp_dir / "victim" + victim.mkdir() + attacker_real = temp_dir / "attacker-real" + (attacker_real / "workflows" / "evil").mkdir(parents=True) + (attacker_real / "workflows" / "evil" / "workflow.yml").write_text( + self.WORKFLOW_YAML.format(version="1.0.0"), encoding="utf-8" + ) + (attacker_real / "workflows" / "workflow-registry.json").write_text( + json.dumps( + { + "schema_version": "1.0", + "workflows": { + "evil": { + "name": "Evil", + "version": "1.0.0", + "source": "dev", + "enabled": False, + } + }, + } + ), + encoding="utf-8", + ) + (victim / ".specify").symlink_to(attacker_real) + + unrelated_cwd = temp_dir / "unrelated-cwd" + unrelated_cwd.mkdir() + monkeypatch.chdir(unrelated_cwd) + + runner = CliRunner() + target = victim / ".specify" / "workflows" / "evil" / "workflow.yml" + result = runner.invoke(app, ["workflow", "run", str(target)]) + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "symlink" in result.output.lower() + + def test_run_nested_installed_paths_uses_nearest_owner( + self, temp_dir, monkeypatch + ): + """A direct workflow.yml path whose lexical segments contain + .specify/workflows more than once (an unrelated nested project + happens to live beneath an outer installed workflow's own + directory tree, reusing the same segment names) must be attributed + to its *nearest* (innermost) owning project/ID -- scanning from the + start of the path and stopping at the first match would pick the + outer project and the wrong workflow ID, gating the run on an + unrelated workflow's disabled state instead of the real owner's.""" + from typer.testing import CliRunner + from specify_cli import app + + def _write_registry(workflows_dir, workflow_id, enabled): + workflows_dir.mkdir(parents=True, exist_ok=True) + (workflows_dir / "workflow-registry.json").write_text( + json.dumps( + { + "schema_version": "1.0", + "workflows": { + workflow_id: { + "name": workflow_id, + "version": "1.0.0", + "source": "dev", + "enabled": enabled, + } + }, + } + ), + encoding="utf-8", + ) + + outer_workflows = temp_dir / "outer-proj" / ".specify" / "workflows" + outer_wf_dir = outer_workflows / "outer-wf" + outer_wf_dir.mkdir(parents=True) + (outer_wf_dir / "workflow.yml").write_text( + self.WORKFLOW_YAML.format(version="1.0.0"), encoding="utf-8" + ) + _write_registry(outer_workflows, "outer-wf", enabled=False) + + # An unrelated nested project lives inside the outer workflow's own + # directory tree, with its own separate installed workflow. + inner_workflows = outer_wf_dir / "nested-proj" / ".specify" / "workflows" + inner_wf_dir = inner_workflows / "inner-wf" + inner_wf_dir.mkdir(parents=True) + (inner_wf_dir / "workflow.yml").write_text( + self.WORKFLOW_YAML.format(version="1.0.0"), encoding="utf-8" + ) + _write_registry(inner_workflows, "inner-wf", enabled=True) + + unrelated_cwd = temp_dir / "unrelated-cwd" + unrelated_cwd.mkdir() + monkeypatch.chdir(unrelated_cwd) + + runner = CliRunner() + target = inner_wf_dir / "workflow.yml" + result = runner.invoke(app, ["workflow", "run", str(target)]) + # inner-wf (the actual nearest owner) is enabled -- must run, not + # be blocked by the unrelated outer-wf's disabled state. + assert result.exit_code == 0, result.output + + # The inverse proves this isn't just ignoring nesting: disabling + # the true (nearest) owner must actually block this exact path. + _write_registry(inner_workflows, "inner-wf", enabled=False) + result = runner.invoke(app, ["workflow", "run", str(target)]) + assert result.exit_code != 0 + assert "disabled" in result.output + + @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable") + def test_run_blocks_disabled_workflow_via_outward_alias_symlink( + self, project_dir, monkeypatch + ): + """The inverse of the existing inward-symlink case: a path with no + .specify/workflows segments at all (e.g. /tmp/alias.yml) that is + itself a symlink resolving *into* installed storage must still + receive the disabled check. Only checking the lexical path's own + segments misses this alias entirely, since it has no such segments + to begin with, and would let engine.load_workflow follow the + symlink to the disabled workflow's real content unchecked.""" + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) + + result = runner.invoke(app, ["workflow", "disable", "align-wf"]) + assert result.exit_code == 0, result.output + + installed_yaml = ( + project_dir / ".specify" / "workflows" / "align-wf" / "workflow.yml" + ) + external_dir = project_dir / "outside-alias" + external_dir.mkdir() + alias = external_dir / "alias.yml" + alias.symlink_to(installed_yaml) + + result = runner.invoke(app, ["workflow", "run", str(alias)]) + assert result.exit_code != 0 + assert "disabled" in result.output + + result = runner.invoke(app, ["workflow", "enable", "align-wf"]) + assert result.exit_code == 0, result.output + result = runner.invoke(app, ["workflow", "run", str(alias)]) + assert result.exit_code == 0, result.output + + _GATED_WORKFLOW_YAML = """ +schema_version: "1.0" +workflow: + id: "gated-wf" + name: "Gated Workflow" + version: "1.0.0" +steps: + - id: ask + type: gate + message: "Review" + options: [approve, reject] +""" + + def _install_and_run_gated(self, runner, app, project_dir): + """Install a gate-step workflow and run it to a paused state. + + Returns the run_id. The gate step pauses without any interactive + input, giving a resumable run tied to an installed workflow ID. + """ + src = project_dir / "gated-src" + src.mkdir(exist_ok=True) + (src / "workflow.yml").write_text(self._GATED_WORKFLOW_YAML, encoding="utf-8") + result = runner.invoke(app, ["workflow", "add", str(src), "--dev"]) + assert result.exit_code == 0, result.output + + result = runner.invoke(app, ["workflow", "run", "gated-wf", "--json"]) + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout) + assert payload["status"] == "paused" + return payload["run_id"] + + def test_unregistered_workflow_shaped_path_is_not_persisted_as_owner( + self, project_dir, temp_dir, monkeypatch + ): + """A direct file is not installed merely because its path resembles + installed storage; only registry membership establishes ownership.""" + import shutil + from typer.testing import CliRunner + from specify_cli import app + + standalone_root = temp_dir / "standalone-project" + workflows_dir = standalone_root / ".specify" / "workflows" + workflow_file = workflows_dir / "gated-wf" / "workflow.yml" + workflow_file.parent.mkdir(parents=True) + workflow_file.write_text(self._GATED_WORKFLOW_YAML, encoding="utf-8") + + monkeypatch.chdir(project_dir) + runner = CliRunner() + run_ids = [] + for _ in range(2): + result = runner.invoke( + app, ["workflow", "run", str(workflow_file), "--json"] + ) + assert result.exit_code == 0, result.output + run_ids.append(json.loads(result.stdout)["run_id"]) + + for run_id in run_ids: + state_path = ( + project_dir + / ".specify" + / "workflows" + / "runs" + / run_id + / "state.json" + ) + state = json.loads(state_path.read_text(encoding="utf-8")) + assert state["installed_workflow_id"] is None + assert state["installed_registry_root"] is None + + shutil.rmtree(standalone_root) + result = runner.invoke( + app, ["workflow", "resume", run_ids[0], "--json"] + ) + assert result.exit_code == 0, result.output + + workflows_dir.mkdir(parents=True) + (workflows_dir / "workflow-registry.json").write_text( + json.dumps( + { + "schema_version": "1.0", + "workflows": { + "gated-wf": { + "name": "Unrelated workflow", + "version": "9.9.9", + "source": "dev", + "enabled": False, + } + }, + } + ), + encoding="utf-8", + ) + result = runner.invoke( + app, ["workflow", "resume", run_ids[1], "--json"] + ) + assert result.exit_code == 0, result.output + + def test_resume_blocks_when_installed_workflow_disabled( + self, project_dir, monkeypatch + ): + """A run started from an installed workflow must not resume once + that workflow is disabled. engine.resume() replays the persisted + run directly from disk with no registry awareness at all, so the + installed workflow's origin (id + owning registry root) is + persisted at run start and re-checked against the registry's + *current* state before resuming, mirroring `workflow run`'s + disabled guard.""" + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + run_id = self._install_and_run_gated(runner, app, project_dir) + + result = runner.invoke(app, ["workflow", "disable", "gated-wf"]) + assert result.exit_code == 0, result.output + + result = runner.invoke(app, ["workflow", "resume", run_id]) + assert result.exit_code != 0 + assert "disabled" in result.output + + # Re-enabling must unblock the exact same run. + result = runner.invoke(app, ["workflow", "enable", "gated-wf"]) + assert result.exit_code == 0, result.output + result = runner.invoke(app, ["workflow", "resume", run_id, "--json"]) + assert result.exit_code == 0, result.output + resumed = json.loads(result.stdout) + assert resumed["run_id"] == run_id + + def test_resume_rejects_corrupted_registry_entry( + self, project_dir, monkeypatch + ): + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.catalog import WorkflowRegistry + + monkeypatch.chdir(project_dir) + runner = CliRunner() + run_id = self._install_and_run_gated(runner, app, project_dir) + + registry = WorkflowRegistry(project_dir) + registry.data["workflows"]["gated-wf"] = "corrupted" + registry.save() + + result = runner.invoke(app, ["workflow", "resume", run_id]) + assert result.exit_code != 0 + assert "corrupted" in result.output + + def test_resume_preload_io_error_is_reported_cleanly( + self, project_dir, monkeypatch + ): + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.workflows.engine import RunState + + monkeypatch.chdir(project_dir) + with patch.object( + RunState, "load", side_effect=OSError("permission [denied]") + ): + result = CliRunner().invoke( + app, ["workflow", "resume", "unreadable-run"] + ) + + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "Resume failed" in result.output + assert "permission [denied]" in result.output + + def test_resume_backward_compatible_with_run_state_missing_new_fields( + self, project_dir, monkeypatch + ): + """A run's state.json persisted before installed-origin tracking + existed (missing the installed_workflow_id/installed_registry_root + keys entirely) must still resume normally: RunState.load's + defaults must not raise, and the absent origin must skip the + disabled check rather than block or error -- old runs are + unaffected by this guard.""" + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + run_id = self._install_and_run_gated(runner, app, project_dir) + + state_path = ( + project_dir / ".specify" / "workflows" / "runs" / run_id / "state.json" + ) + data = json.loads(state_path.read_text(encoding="utf-8")) + data.pop("installed_workflow_id", None) + data.pop("installed_registry_root", None) + state_path.write_text(json.dumps(data), encoding="utf-8") + + result = runner.invoke(app, ["workflow", "disable", "gated-wf"]) + assert result.exit_code == 0, result.output + + # Origin metadata is absent from disk -- disabling the (now + # unrelated, from this run's perspective) installed workflow must + # not block resuming this legacy run. + result = runner.invoke(app, ["workflow", "resume", run_id, "--json"]) + assert result.exit_code == 0, result.output + + def test_resume_blocks_after_project_moved_following_disable( + self, temp_dir, monkeypatch + ): + """Renaming/moving the entire project after starting a run must not + let a subsequent disable-then-resume bypass the guard. Persisting + the run's *creation-time absolute* project path would make resume + open a now-nonexistent old root (WorkflowRegistry falls back to an + empty default there), missing the disabled entry that actually + lives in the *current* (moved) project's registry. The common, + same-project case must instead re-derive the owning root from the + project's current location on every resume.""" + from typer.testing import CliRunner + from specify_cli import app + import shutil + + project_v1 = temp_dir / "project-v1" + (project_v1 / ".specify" / "workflows").mkdir(parents=True) + monkeypatch.chdir(project_v1) + runner = CliRunner() + run_id = self._install_and_run_gated(runner, app, project_v1) + + project_v2 = temp_dir / "project-v2" + shutil.move(str(project_v1), str(project_v2)) + monkeypatch.chdir(project_v2) + + result = runner.invoke(app, ["workflow", "disable", "gated-wf"]) + assert result.exit_code == 0, result.output + + result = runner.invoke(app, ["workflow", "resume", run_id]) + assert result.exit_code != 0 + assert "disabled" in result.output + + def test_resume_after_project_moved_still_works_when_enabled( + self, temp_dir, monkeypatch + ): + """The inverse of the move regression: an enabled workflow's run + must still resume normally after the project is moved -- the + current-project fallback must not itself block legitimate + resumes.""" + from typer.testing import CliRunner + from specify_cli import app + import shutil + + project_v1 = temp_dir / "project-v1-ok" + (project_v1 / ".specify" / "workflows").mkdir(parents=True) + monkeypatch.chdir(project_v1) + runner = CliRunner() + run_id = self._install_and_run_gated(runner, app, project_v1) + + project_v2 = temp_dir / "project-v2-ok" + shutil.move(str(project_v1), str(project_v2)) + monkeypatch.chdir(project_v2) + + result = runner.invoke(app, ["workflow", "resume", run_id, "--json"]) + assert result.exit_code == 0, result.output + + def test_resume_respects_cross_project_registry_root( + self, temp_dir, monkeypatch + ): + """A run started via a direct workflow.yml path belonging to a + different project than the cwd used for `workflow run`/`workflow + resume` must still gate resuming on *that* owning project's + registry, not the cwd project's (which has no entry for this ID + at all). This is the genuine cross-project case that must remain + unaffected by only special-casing the common same-project one.""" + from typer.testing import CliRunner + from specify_cli import app + + owner_project = temp_dir / "owner-project" + (owner_project / ".specify" / "workflows").mkdir(parents=True) + monkeypatch.chdir(owner_project) + runner = CliRunner() + src = owner_project / "gated-src" + src.mkdir() + (src / "workflow.yml").write_text(self._GATED_WORKFLOW_YAML, encoding="utf-8") + result = runner.invoke(app, ["workflow", "add", str(src), "--dev"]) + assert result.exit_code == 0, result.output + + unrelated_cwd = temp_dir / "unrelated-cwd" + unrelated_cwd.mkdir() + monkeypatch.chdir(unrelated_cwd) + + target = owner_project / ".specify" / "workflows" / "gated-wf" / "workflow.yml" + result = runner.invoke(app, ["workflow", "run", str(target), "--json"]) + assert result.exit_code == 0, result.output + run_id = json.loads(result.stdout)["run_id"] + + monkeypatch.chdir(owner_project) + result = runner.invoke(app, ["workflow", "disable", "gated-wf"]) + assert result.exit_code == 0, result.output + + # Resume must run from unrelated_cwd (where this run's own + # state.json actually lives) yet still be blocked by the owner + # project's disabled entry. + monkeypatch.chdir(unrelated_cwd) + result = runner.invoke(app, ["workflow", "resume", run_id]) + assert result.exit_code != 0 + assert "disabled" in result.output + + def test_resume_rejects_missing_cross_project_owner_root( + self, temp_dir, monkeypatch + ): + """A vanished explicit cross-project owner cannot be safely + rediscovered, so resume must fail closed instead of consulting the + unrelated project that stores the run state.""" + from typer.testing import CliRunner + from specify_cli import app + import shutil + + owner_project = temp_dir / "owner-project-2" + (owner_project / ".specify" / "workflows").mkdir(parents=True) + monkeypatch.chdir(owner_project) + runner = CliRunner() + src = owner_project / "gated-src" + src.mkdir() + (src / "workflow.yml").write_text(self._GATED_WORKFLOW_YAML, encoding="utf-8") + result = runner.invoke(app, ["workflow", "add", str(src), "--dev"]) + assert result.exit_code == 0, result.output + + unrelated_cwd = temp_dir / "unrelated-cwd-2" + unrelated_cwd.mkdir() + monkeypatch.chdir(unrelated_cwd) + + target = owner_project / ".specify" / "workflows" / "gated-wf" / "workflow.yml" + result = runner.invoke(app, ["workflow", "run", str(target), "--json"]) + assert result.exit_code == 0, result.output + run_id = json.loads(result.stdout)["run_id"] + + # owner_project vanishes entirely -- its persisted absolute root + # is now dangling. + shutil.rmtree(owner_project) + + result = runner.invoke(app, ["workflow", "resume", run_id]) + assert result.exit_code != 0 + assert "owner" in result.output.lower() + assert "unavailable" in result.output.lower() + + @pytest.mark.parametrize( + "field, bad_value", + [ + ("installed_workflow_id", 123), + ("installed_workflow_id", ["gated-wf"]), + ("installed_workflow_id", {"id": "gated-wf"}), + ("installed_workflow_id", True), + ("installed_registry_root", 123), + ("installed_registry_root", ["."]), + ("installed_registry_root", {"root": "."}), + ("installed_registry_root", False), + ], + ) + def test_resume_rejects_malformed_run_state_origin_fields( + self, project_dir, monkeypatch, field, bad_value + ): + """RunState.load() trusts installed_workflow_id/installed_registry_root + straight out of the JSON state file with no type validation. A + malformed value (int/list/dict/bool instead of str-or-null) -- from + disk corruption or an externally-crafted state.json -- would + otherwise crash deep inside `_resolve_run_owner_root`/registry + lookups (TypeError constructing a Path, unhashable dict/list key) + rather than failing cleanly. `load()` must validate each field as + `str | None` and raise a clear ValueError, which `workflow resume`'s + existing ValueError boundary already converts into a clean CLI + error with no traceback.""" + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + run_id = self._install_and_run_gated(runner, app, project_dir) + + state_path = ( + project_dir / ".specify" / "workflows" / "runs" / run_id / "state.json" + ) + data = json.loads(state_path.read_text(encoding="utf-8")) + data[field] = bad_value + state_path.write_text(json.dumps(data), encoding="utf-8") + + result = runner.invoke(app, ["workflow", "resume", run_id]) + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "Error" in result.output + + @pytest.mark.parametrize("command", ["resume", "status"]) + def test_state_load_errors_escape_rich_markup( + self, project_dir, monkeypatch, command + ): + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + run_id = self._install_and_run_gated(runner, app, project_dir) + + state_path = ( + project_dir / ".specify" / "workflows" / "runs" / run_id / "state.json" + ) + data = json.loads(state_path.read_text(encoding="utf-8")) + malicious_status = "[bold red]forged[/bold red]" + data["status"] = malicious_status + state_path.write_text(json.dumps(data), encoding="utf-8") + + result = runner.invoke(app, ["workflow", command, run_id]) + + assert result.exit_code != 0 + assert malicious_status in result.output + + @pytest.mark.parametrize( + "installed_workflow_id, installed_registry_root", + [ + (None, None), + ("gated-wf", None), + ("", ""), + ], + ) + def test_resume_accepts_valid_run_state_origin_fields( + self, project_dir, monkeypatch, installed_workflow_id, installed_registry_root + ): + """Valid str-or-null values (including the empty-string fallback + policy `_resolve_run_owner_root` already treats as "no root + stored") must continue to load and resume without error.""" + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + run_id = self._install_and_run_gated(runner, app, project_dir) + + state_path = ( + project_dir / ".specify" / "workflows" / "runs" / run_id / "state.json" + ) + data = json.loads(state_path.read_text(encoding="utf-8")) + data["installed_workflow_id"] = installed_workflow_id + data["installed_registry_root"] = installed_registry_root + state_path.write_text(json.dumps(data), encoding="utf-8") + + result = runner.invoke(app, ["workflow", "resume", run_id, "--json"]) + assert result.exit_code == 0, result.output + + @pytest.mark.parametrize( + "field, bad_value", + [ + ("installed_workflow_id", 123), + ("installed_workflow_id", ["gated-wf"]), + ("installed_registry_root", 123), + ("installed_registry_root", ["."]), + ], + ) + def test_status_rejects_malformed_run_state_origin_fields( + self, project_dir, monkeypatch, field, bad_value + ): + """`workflow status ` calls RunState.load() same as resume, + but only caught FileNotFoundError -- the new type validation there + (int/list instead of str-or-null) raises ValueError, which leaked + as a raw unhandled traceback instead of `workflow resume`'s clean + `[red]Error:[/red] {exc}` + exit 1. Must get the identical clean + boundary, leaving the no-run-id list path (and FileNotFoundError + behavior) unchanged.""" + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + run_id = self._install_and_run_gated(runner, app, project_dir) + + state_path = ( + project_dir / ".specify" / "workflows" / "runs" / run_id / "state.json" + ) + data = json.loads(state_path.read_text(encoding="utf-8")) + data[field] = bad_value + state_path.write_text(json.dumps(data), encoding="utf-8") + + result = runner.invoke(app, ["workflow", "status", run_id]) + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "Error" in result.output + + def test_status_run_not_found_unchanged(self, project_dir, monkeypatch): + """FileNotFoundError behavior for a nonexistent run_id must remain + exactly as before this fix.""" + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + (project_dir / ".specify" / "workflows").mkdir(parents=True, exist_ok=True) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "status", "nonexistent-run"]) + assert result.exit_code != 0 + assert "Run not found: nonexistent-run" in result.output + + def test_status_no_run_id_list_path_unaffected(self, project_dir, monkeypatch): + """The no-run-id list-all-runs path must remain unaffected by the + new single-run ValueError boundary.""" + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + (project_dir / ".specify" / "workflows").mkdir(parents=True, exist_ok=True) + runner = CliRunner() + result = runner.invoke(app, ["workflow", "status"]) + assert result.exit_code == 0, result.output + + def test_disable_shows_marker_in_list(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) + runner.invoke(app, ["workflow", "disable", "align-wf"]) + result = runner.invoke(app, ["workflow", "list"]) + assert result.exit_code == 0, result.output + assert "[disabled]" in result.output + + def test_enable_disable_not_installed_errors(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + for cmd in ("enable", "disable"): + result = runner.invoke(app, ["workflow", cmd, "ghost"]) + assert result.exit_code != 0 + assert "not installed" in result.output + + def test_enable_disable_idempotent_warnings(self, project_dir, monkeypatch): + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runner = CliRunner() + self._install_dev(runner, app, project_dir) + + result = runner.invoke(app, ["workflow", "enable", "align-wf"]) + assert result.exit_code == 0 + assert "already enabled" in result.output + + runner.invoke(app, ["workflow", "disable", "align-wf"]) + result = runner.invoke(app, ["workflow", "disable", "align-wf"]) + assert result.exit_code == 0 + assert "already disabled" in result.output