diff --git a/design/workflow-step.md b/design/workflow-step.md index e571940109..05e41efcf4 100644 --- a/design/workflow-step.md +++ b/design/workflow-step.md @@ -22,6 +22,24 @@ The default community catalog is discovery-only, not an install or trust endorsement. Review external step code before installing it: loading a package imports and executes its Python module. +Loading project step code also requires consent for the current invocation. +`load_custom_steps(project_root, trusted=False)` refuses imports by default; +neither files on disk nor `step-registry.json` grant trust. CLI commands that +load custom steps (`workflow add`, `update`, `run`, and `resume`) ask for +confirmation on interactive stdin, defaulting to refusal. `--trust-steps` +explicitly authorizes all custom step packages in that project for that +invocation, including unreferenced packages, and works without a terminal. +Approval is never persisted or inherited by subsequent invocations. +Bundle `add`, `install`, `update`, and `init` commands forward the same explicit +consent through their primitive installer to both bundled and catalog workflow +delegates. Bundle metadata, offline mode, and refresh do not imply trust. +An interactive approval is shared by the bundle invocation's primitive +installer for that project, so subsequent workflow delegates do not prompt +again. The approval is discarded with that invocation, never stored globally. +Workflow delegates remain bound to the approved absolute project root. Their +temporary project override must not reinterpret a relative `SPECIFY_INIT_DIR` +from a changed working directory, and must restore the caller's override afterward. + ## Step contract Subclass `StepBase` from `src/specify_cli/workflows/base.py`, set `type_key` diff --git a/docs/reference/bundles.md b/docs/reference/bundles.md index 7c0f407524..3c6dc62db0 100644 --- a/docs/reference/bundles.md +++ b/docs/reference/bundles.md @@ -71,9 +71,23 @@ specify bundle install | `--offline` | Do not access the network | | `--refresh` | Refresh owned components from the supplied bundle source | | `--version` | Install an exact historical or current catalog release | +| `--trust-steps` | Authorize all project custom step Python for this invocation | Installs a bundle's full component set through each primitive's machinery. The argument may be a catalog bundle id, or a local path to a built `.zip` artifact, a bundle directory, or a `bundle.yml` file; local sources install directly without consulting the catalog stack, unless the manifest declares `provides.bundles`. A local parent with bundle references resolves those children through the catalog stack at command time, so it needs catalog and network access (see [Bundle references](#bundle-references)). +When a workflow install or refresh loads custom step Python, the same +[per-invocation trust approval](workflows.md#custom-step-trust) applies as for +direct workflow commands. `bundle install`, its `bundle add` alias, `bundle +update`, and `bundle init` forward `--trust-steps` to both bundled and catalog +workflow installers. Without the switch, interactive invocations ask for +approval and unattended invocations refuse to load custom steps. Neither +`--offline` nor `--refresh` implies approval, and consent is never persisted. +One interactive approval covers subsequent workflow delegates in the same +bundle command, including a mix of bundled and catalog workflows. Later +commands require approval again. +Delegated operations remain bound to the approved project, including when +`SPECIFY_INIT_DIR` was relative to the bundle command's starting directory. + If the current directory is not yet a Spec Kit project, `install` initializes one first so a fresh checkout reaches a working state in a single command. `--integration` selects the integration when initializing a new project, and confirms the target when a bundle pins a specific integration but the project's active integration can't be determined (missing or unreadable `.specify/integration.json`). It does **not** override an already-initialized project's active integration: if a bundle targets a different integration than the project's, install aborts with no changes. Integration-agnostic bundles inherit the project's active integration. Without `--refresh`, installation is idempotent — already-installed components matching their pins are skipped, as are bundle-owned compatible packaged patch releases with a warning (see below). Other differing or missing installed versions cannot satisfy a pin. Only `--refresh` can repair a drifted version owned exclusively by this bundle; independently installed components are skipped and never adopted or replaced. On failure, no provenance record is written (a failed install records nothing), and the components installed during that run are removed on a best-effort basis — removal errors are swallowed, so partial on-disk state may remain. Installed-bundle records store required component pins separately from contributed components. A compatible independently installed component satisfies a bundle's requirement but remains independently owned: its pin still blocks a later bundle from installing an incompatible version, while removing the bundle does not uninstall it. An unpinned install or refresh cannot replace a component another bundle requires at a pinned version; sharing an already installed matching version without refreshing remains allowed. Shared bundles must also agree on component source and, for presets, priority and strategy before installing or refreshing the shared component. Older records without the requirement list retain their known contributed pins; reinstall or refresh the bundle to record its full requirements. @@ -233,6 +247,7 @@ specify bundle update [] | `--all` | Update every installed bundle | | `--integration` | Override the integration used when refreshing components; applied only when the project's active integration can't be determined | | `--offline` | Do not access the network | +| `--trust-steps` | Authorize all project custom step Python for this invocation | Re-resolves a bundle and **refreshes** its components through each primitive's update path, bringing already-installed components up to the bundle's newly pinned versions (or compatible newer packaged patches, with a warning) while preserving primitive-level overrides (such as preset priority). Provide a bundle id, or use `--all` to update everything installed. @@ -276,6 +291,7 @@ specify bundle init [] | ---------------- | ---------------------------------------- | | `--integration` | Integration override | | `--offline` | Do not access the network | +| `--trust-steps` | Authorize all project custom step Python for this invocation | Ensures the current directory is a Spec Kit project (initializing it idempotently if needed), then optionally installs the given bundle. Useful as an explicit one-step bootstrap for a new checkout. diff --git a/docs/reference/workflows.md b/docs/reference/workflows.md index cb4bc1fa96..2b8c37bce2 100644 --- a/docs/reference/workflows.md +++ b/docs/reference/workflows.md @@ -12,6 +12,7 @@ specify workflow run | ------------------- | -------------------------------------------------------- | | `-i` / `--input` | Pass input values as `key=value` (repeatable) | | `--json` | Emit the run outcome as a single JSON object | +| `--trust-steps` | Authorize all project custom step Python for this invocation | Runs a workflow from a catalog ID, URL, or local file path. Inputs declared by the workflow can be provided via `--input` or will be prompted interactively. @@ -80,6 +81,7 @@ specify workflow resume | ------------------- | -------------------------------------------------------- | | `-i` / `--input` | Updated input values as `key=value` (repeatable) | | `--json` | Emit the resume outcome as a single JSON object | +| `--trust-steps` | Authorize all project custom step Python for this invocation | Resumes a paused or failed workflow run from its persisted execution state. A `running` run is not resumable. Resume replays completed work into its @@ -128,6 +130,7 @@ specify workflow add | `--dev` | Install from a local YAML file, package directory, or archive | | `--from ` | Install from a custom URL (`` names the expected workflow ID) | | `--version ` | Install an exact advertised catalog release (`` must be a workflow ID) | +| `--trust-steps` | Authorize all project custom step Python for this invocation | Installs a workflow from the catalog, an HTTPS URL, a local YAML file, a directory containing `workflow.yml`, or a `.zip`, `.tar.gz`, or `.tgz` @@ -398,6 +401,10 @@ 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. +`--trust-steps` authorizes loading project custom step code for this invocation. +Without it, custom steps require interactive confirmation as described under +[Custom step trust](#custom-step-trust). + ## Enable or Disable a Workflow ```bash @@ -992,18 +999,45 @@ Every source is validated identically before anything is committed: validation: at most 512 archive entries, 50 MiB downloaded or extracted, and 10 MiB per archive member. Catalog files have a 50 MiB per-response bound. - Installation validates and copies the package but does **not** import or - execute `__init__.py`. Installed custom step modules are loaded during startup - of `workflow add`, `workflow run`, and `workflow resume`, before any particular - custom step necessarily executes. + execute `__init__.py`. With explicit consent, custom step modules are loaded + during startup of `workflow add`, `workflow update`, `workflow run`, and + `workflow resume`, before any particular custom step necessarily executes. > **Security note:** Loading a custom step runs its Python with **your** > privileges. Only install and retain step packages from sources you trust. +#### Custom step trust + +Finding a package on disk does not authorize executing it, even if the project's +`step-registry.json` says it was installed. Before importing custom step Python, +`workflow add`, `workflow update`, `workflow run`, and `workflow resume` ask for +confirmation when stdin is interactive. The default answer is **No**. Declining, +interrupting the prompt, or running without a terminal blocks loading and fails +the command before custom step code executes. + +After reviewing all packages in `.specify/workflows/steps/`, use `--trust-steps` +to approve them explicitly, including in unattended scripts: + +```bash +specify workflow run my-workflow --trust-steps +specify workflow resume --trust-steps +``` + +Approval applies only to that invocation and includes **all discovered custom +step packages**, even ones the requested workflow does not reference. It is +never saved: each invocation requires confirmation or `--trust-steps` again. +Projects without custom step packages need no approval. Under `--json`, trust +prompts go to stderr and refusal produces a single JSON failure object on stdout. + +The [bundle commands](bundles.md#install-a-bundle) that install or refresh +workflows (`add`, `install`, `update`, and `init`) accept and forward the same +`--trust-steps` option for that bundle invocation. + #### Listing, running, and removing Installed custom steps appear in `specify workflow step list` and are loaded -automatically by `workflow add`, `workflow run`, and `workflow resume`. Remove -one with: +by `workflow add`, `workflow update`, `workflow run`, and `workflow resume` +only after the trust approval above. Remove one with: ```bash specify workflow step remove my-step diff --git a/src/specify_cli/bundles/adapters.py b/src/specify_cli/bundles/adapters.py index 52979de5bd..7f0035367f 100644 --- a/src/specify_cli/bundles/adapters.py +++ b/src/specify_cli/bundles/adapters.py @@ -320,10 +320,18 @@ class DefaultPrimitiveInstaller: *allow_network* mirrors the bundle command's ``--offline`` flag: when False, component kinds that can only be sourced from a remote catalog refuse rather than touching the network. Bundled presets/extensions still install offline. + *trust_steps* forwards explicit consent for custom step imports during this + bundle invocation only; it is never derived from bundle metadata. + Interactive approvals are shared across this installer's workflow delegates, + scoped to each project and discarded with the bundle invocation's installer. """ - def __init__(self, *, allow_network: bool = True) -> None: + def __init__( + self, *, allow_network: bool = True, trust_steps: bool = False, + ) -> None: self._allow_network = allow_network + self._trust_steps = trust_steps + self._approved_step_projects: set[Path] = set() def is_installed(self, project_root: Path, component: ComponentRef) -> bool: manager = self._manager_for(component, project_root) @@ -396,5 +404,7 @@ def _manager_for(self, component: ComponentRef, project_root: Path): from .primitives import primitive_manager return primitive_manager( - component.kind, project_root, allow_network=self._allow_network + component.kind, project_root, allow_network=self._allow_network, + trust_steps=self._trust_steps, + approved_step_projects=self._approved_step_projects, ) diff --git a/src/specify_cli/bundles/command_add.py b/src/specify_cli/bundles/command_add.py index 702f31391d..27953bd5b7 100644 --- a/src/specify_cli/bundles/command_add.py +++ b/src/specify_cli/bundles/command_add.py @@ -2,6 +2,8 @@ from __future__ import annotations +from typing import Annotated + import typer from ._commands import bundle_app @@ -27,6 +29,12 @@ def bundle_add( "--version", help="Install an exact bundle release from the catalog", ), + trust_steps: Annotated[ + bool, + typer.Option( + "--trust-steps", help="Authorize all custom step Python for this invocation", + ), + ] = False, ) -> None: """Install a bundle's full component set (alias for install).""" bundle_install( @@ -35,4 +43,5 @@ def bundle_add( offline=offline, refresh=refresh, version=version, + trust_steps=trust_steps, ) diff --git a/src/specify_cli/bundles/command_init.py b/src/specify_cli/bundles/command_init.py index c61dd53615..a88c1d6817 100644 --- a/src/specify_cli/bundles/command_init.py +++ b/src/specify_cli/bundles/command_init.py @@ -2,6 +2,8 @@ from __future__ import annotations +from typing import Annotated + import typer from rich.markup import escape as _escape_markup @@ -23,6 +25,12 @@ def bundle_init( bundle: str = typer.Argument(None, help="Optional bundle to install after init"), integration: str = typer.Option(None, "--integration", help="Integration override"), offline: bool = typer.Option(False, "--offline", help="Do not access the network"), + trust_steps: Annotated[ + bool, + typer.Option( + "--trust-steps", help="Authorize all custom step Python for this invocation", + ), + ] = False, ) -> None: """Ensure the project is initialized (idempotent), then optionally install a bundle.""" from .project import find_project_root @@ -53,4 +61,5 @@ def bundle_init( integration=integration, offline=offline, refresh=False, + trust_steps=trust_steps, ) diff --git a/src/specify_cli/bundles/command_install.py b/src/specify_cli/bundles/command_install.py index 1d571bb0e6..eb49696217 100644 --- a/src/specify_cli/bundles/command_install.py +++ b/src/specify_cli/bundles/command_install.py @@ -3,6 +3,7 @@ from __future__ import annotations from pathlib import Path +from typing import Annotated import typer from rich.markup import escape as _escape_markup @@ -49,6 +50,12 @@ def bundle_install( "--version", help="Install an exact bundle release from the catalog", ), + trust_steps: Annotated[ + bool, + typer.Option( + "--trust-steps", help="Authorize all custom step Python for this invocation", + ), + ] = False, ) -> None: """Install a bundle's full component set through each primitive's machinery. @@ -155,7 +162,7 @@ def bundle_install( result = install_bundle( project_root, plan, - DefaultPrimitiveInstaller(allow_network=not offline), + DefaultPrimitiveInstaller(allow_network=not offline, trust_steps=trust_steps), manifest=manifest, refresh=refresh, ) diff --git a/src/specify_cli/bundles/command_update.py b/src/specify_cli/bundles/command_update.py index 65b9090d04..f716b1bd5b 100644 --- a/src/specify_cli/bundles/command_update.py +++ b/src/specify_cli/bundles/command_update.py @@ -2,6 +2,8 @@ from __future__ import annotations +from typing import Annotated + import typer from rich.markup import escape as _escape_markup @@ -29,6 +31,12 @@ def bundle_update( ), integration: str = typer.Option(None, "--integration", help="Override integration"), offline: bool = typer.Option(False, "--offline", help="Do not access the network"), + trust_steps: Annotated[ + bool, + typer.Option( + "--trust-steps", help="Authorize all custom step Python for this invocation", + ), + ] = False, ) -> None: """Re-resolve and refresh a bundle's components via each primitive's update path.""" try: @@ -46,7 +54,9 @@ def bundle_update( from .installer import install_bundle from .resolver import resolve_install_plan - installer = DefaultPrimitiveInstaller(allow_network=not offline) + installer = DefaultPrimitiveInstaller( + allow_network=not offline, trust_steps=trust_steps, + ) for target in targets: if not any(r.bundle_id == target for r in records): raise BundlerError(f"Bundle '{target}' is not installed.") diff --git a/src/specify_cli/bundles/primitives.py b/src/specify_cli/bundles/primitives.py index f910592255..06bff36e64 100644 --- a/src/specify_cli/bundles/primitives.py +++ b/src/specify_cli/bundles/primitives.py @@ -257,14 +257,19 @@ def remove(self, component: ComponentRef) -> None: def primitive_manager( - kind: str, project_root: Path, *, allow_network: bool = True + kind: str, project_root: Path, *, allow_network: bool = True, + trust_steps: bool = False, + approved_step_projects: set[Path] | None = None, ) -> _KindManager: if kind == "presets": return _PresetKindManager(project_root, allow_network) if kind == "extensions": return _ExtensionKindManager(project_root, allow_network) if kind == "workflows": - return _WorkflowKindManager(project_root, allow_network) + return _WorkflowKindManager( + project_root, allow_network, trust_steps=trust_steps, + approved_step_projects=approved_step_projects, + ) if kind == "steps": return _StepKindManager(project_root, allow_network) raise BundlerError(f"Unknown component kind '{kind}'.") @@ -272,16 +277,23 @@ def primitive_manager( @contextlib.contextmanager def _chdir(path: Path): - """Temporarily switch the working directory. + """Bind delegated CLI project resolution to an absolute root. - The delegated workflow/step command callables resolve the project via - ``Path.cwd()``; this makes that resolution land on *path*. + Pin the override too, so a relative SPECIFY_INIT_DIR is not reinterpreted + after changing directories. Restore both caller settings on exit. """ + root = path.resolve() previous = Path.cwd() - os.chdir(path) + previous_override = os.environ.get("SPECIFY_INIT_DIR") + os.chdir(root) try: + os.environ["SPECIFY_INIT_DIR"] = str(root) yield finally: + if previous_override is None: + os.environ.pop("SPECIFY_INIT_DIR", None) + else: + os.environ["SPECIFY_INIT_DIR"] = previous_override os.chdir(previous) @@ -487,12 +499,33 @@ def remove(self, component: ComponentRef) -> None: class _WorkflowKindManager: - def __init__(self, project_root: Path, allow_network: bool) -> None: + def __init__( + self, project_root: Path, allow_network: bool, *, trust_steps: bool = False, + approved_step_projects: set[Path] | None = None, + ) -> None: from ..workflows.catalog import WorkflowRegistry - self._root = project_root + self._root = project_root.resolve() self._allow_network = allow_network - self._registry = WorkflowRegistry(project_root) + self._trust_steps = trust_steps + self._approved_step_projects = ( + approved_step_projects if approved_step_projects is not None else set() + ) + self._registry = WorkflowRegistry(self._root) + + def _approve_steps(self) -> bool: + from ..workflows import _commands as cli + + project = self._root.resolve() + if self._trust_steps or project in self._approved_step_projects: + return True + try: + approved = cli._approve_custom_steps(self._root) + except OSError as exc: + raise BundlerError(str(exc)) from exc + if approved: + self._approved_step_projects.add(project) + return approved def is_installed(self, component: ComponentRef) -> bool: try: @@ -537,7 +570,10 @@ def install(self, component: ComponentRef) -> BundledPatchWarning | None: _delegate_command( "install", f"workflow '{component.id}'", - lambda: workflow_add(str(workflow_file), dev=True, from_url=None), + lambda: workflow_add( + str(workflow_file), dev=True, from_url=None, + trust_steps=self._approve_steps(), + ), ) return warning if not self._allow_network: @@ -564,6 +600,7 @@ def install(self, component: ComponentRef) -> BundledPatchWarning | None: "install", f"workflow '{component.id}'", lambda: _install_preselected_workflow( component.id, version=component.version, selected_info=selected, + trust_steps=self._approve_steps(), ), ) diff --git a/src/specify_cli/workflows/__init__.py b/src/specify_cli/workflows/__init__.py index 2f457371ad..38c8cb3063 100644 --- a/src/specify_cli/workflows/__init__.py +++ b/src/specify_cli/workflows/__init__.py @@ -102,15 +102,21 @@ def _unload_custom_steps() -> None: _CUSTOM_STEP_MODULES.clear() -def load_custom_steps(project_root: Path) -> list[str]: +class CustomStepTrustError(PermissionError): + """Project step code requires explicit consent before it can be imported.""" + + +def load_custom_steps(project_root: Path, *, trusted: bool = False) -> list[str]: """Load community-installed custom step types into STEP_REGISTRY. Scans ``.specify/workflows/steps/`` for installed step packages. Each valid package must contain ``step.yml`` (with a ``step.type_key`` field) and ``__init__.py`` (a ``StepBase`` subclass). - Returns a list of type_keys that were successfully loaded. - Silently skips packages that fail to import or validate. + ``trusted`` authorizes executing all discovered step packages for this call + only. Repository files and installation registry entries do not grant trust. + Raises ``CustomStepTrustError`` before importing any unapproved package. + Returns the loaded type_keys, skipping packages that fail import/validation. """ import hashlib as _hashlib import importlib.util as _importlib_util @@ -162,6 +168,13 @@ def load_custom_steps(project_root: Path) -> list[str]: if type_key in STEP_REGISTRY: continue + if not trusted: + raise CustomStepTrustError( + f"Custom workflow step code in '{steps_dir}' requires approval. " + "Review all step packages, then approve the interactive prompt " + "or pass --trust-steps for this invocation." + ) + # Sanitize type_key so the synthetic module name is a valid identifier # (e.g. "test-custom" → "_speckit_custom_step_test_custom_"). # The 8-char SHA-256 hash of the original type_key makes the name @@ -228,6 +241,8 @@ def load_custom_steps(project_root: Path) -> list[str]: k for k in _sys.modules if k.startswith(submodule_prefix) ]: _sys.modules.pop(_mod_key, None) + except CustomStepTrustError: + raise except Exception: # noqa: BLE001, S112 # Silently skip broken step packages at load time continue diff --git a/src/specify_cli/workflows/_commands.py b/src/specify_cli/workflows/_commands.py index c1fb716f7b..f4e3462caf 100644 --- a/src/specify_cli/workflows/_commands.py +++ b/src/specify_cli/workflows/_commands.py @@ -62,6 +62,56 @@ def _fail_integration_load(exc: Exception, *, json_output: bool, run_id: str | N raise typer.Exit(1) from exc +def _approve_custom_steps( + project_root: Path, *, trust_steps: bool = False, json_output: bool = False, +) -> bool: + """Collect consent without importing code; return whether imports are approved.""" + from . import CustomStepTrustError, load_custom_steps + + if trust_steps: + return True + try: + load_custom_steps(project_root) + except CustomStepTrustError: + if not sys.stdin.isatty(): + raise + _error_console(json_output).print( + "[yellow]Warning:[/yellow] Custom workflow step packages in " + f"'{_escape_markup(str(project_root / '.specify' / 'workflows' / 'steps'))}' " + "execute Python with your privileges, including packages not referenced " + "by the requested workflow. Review all package code before approving." + ) + try: + approved = typer.confirm( + "Trust all custom workflow step code for this invocation?", + default=False, + err=json_output, + ) + except (EOFError, typer.Abort) as exc: + raise CustomStepTrustError( + "Custom workflow step execution was not approved. " + "Review the code and approve interactively or pass --trust-steps." + ) from exc + if not approved: + raise CustomStepTrustError( + "Custom workflow step execution was not approved." + ) + return True + return False + + +def _load_custom_steps( + project_root: Path, *, trust_steps: bool = False, json_output: bool = False, +) -> None: + """Collect per-invocation CLI consent before the domain imports step code.""" + from . import load_custom_steps + + if _approve_custom_steps( + project_root, trust_steps=trust_steps, json_output=json_output, + ): + load_custom_steps(project_root, trusted=True) + + def _fail_workflow_execution( exc: Exception, *, json_output: bool, state: Any, resume: bool = False, ): diff --git a/src/specify_cli/workflows/command_add.py b/src/specify_cli/workflows/command_add.py index 4d1cfb6567..1f525aa2eb 100644 --- a/src/specify_cli/workflows/command_add.py +++ b/src/specify_cli/workflows/command_add.py @@ -32,10 +32,8 @@ def _workflow_package_has_companions(package_dir: cli.Path) -> bool: def _prepare_workflow_add( project_root: cli.Path, source: str, *, dev: bool, from_url: str | None, - version: str | None, selected_catalog: bool = False, + version: str | None, selected_catalog: bool = False, trust_steps: bool = False, ) -> cli.Path: - from . import load_custom_steps - if version is not None and ( dev or from_url is not None or ( not selected_catalog and ( @@ -50,7 +48,10 @@ def _prepare_workflow_add( raise cli.typer.Exit(1) if version is not None or selected_catalog: cli._validate_workflow_id_or_exit(source) - load_custom_steps(project_root) + try: + cli._load_custom_steps(project_root, trust_steps=trust_steps) + except OSError as exc: + cli._fail_integration_load(exc, json_output=False) cli._open_workflow_registry(project_root) workflows_dir = project_root / ".specify" / "workflows" if from_url is not None and not dev: @@ -62,12 +63,13 @@ def _prepare_workflow_add( def _install_preselected_workflow( workflow_id: str, *, version: str | None, selected_info: dict, + trust_steps: bool = False, ) -> None: """Install a bundle-selected release with the normal workflow preflights.""" project_root = cli._require_specify_project() workflows_dir = _prepare_workflow_add( project_root, workflow_id, dev=False, from_url=None, - version=version, selected_catalog=True, + version=version, selected_catalog=True, trust_steps=trust_steps, ) cli._install_workflow_from_catalog( project_root, workflows_dir, workflow_id, @@ -87,6 +89,12 @@ def workflow_add( version: Annotated[ str | None, cli.typer.Option(help="Install an exact catalog release") ] = None, + trust_steps: Annotated[ + bool, + cli.typer.Option( + "--trust-steps", help="Authorize all custom step Python for this invocation", + ), + ] = False, ): """Install a workflow from catalog, URL, or local path.""" from .engine import WorkflowDefinition @@ -94,6 +102,7 @@ def workflow_add( project_root = cli._require_specify_project() workflows_dir = _prepare_workflow_add( project_root, source, dev=dev, from_url=from_url, version=version, + trust_steps=trust_steps, ) def _validate_and_install_local( diff --git a/src/specify_cli/workflows/command_resume.py b/src/specify_cli/workflows/command_resume.py index 8e75fdc71b..16ba4ccff5 100644 --- a/src/specify_cli/workflows/command_resume.py +++ b/src/specify_cli/workflows/command_resume.py @@ -2,6 +2,8 @@ from __future__ import annotations +from typing import Annotated + from . import _command_resume_state as resume_state from . import _commands as cli @@ -17,17 +19,24 @@ def workflow_resume( "--json", help="Emit the resume outcome as a single JSON object instead of formatted text.", ), + trust_steps: Annotated[ + bool, + cli.typer.Option( + "--trust-steps", help="Authorize all custom step Python for this invocation", + ), + ] = False, ): """Resume a paused or failed workflow run.""" from ..integrations.installer import IntegrationInstallError - from . import load_custom_steps from ._execution import CheckpointError from .engine import RunState, WorkflowEngine project_root = cli._require_specify_project(load_integrations=False) try: with cli._stdout_to_stderr_when(json_output): - load_custom_steps(project_root) + cli._load_custom_steps( + project_root, trust_steps=trust_steps, json_output=json_output, + ) engine = WorkflowEngine(project_root) engine._load_integrations() except (IntegrationInstallError, OSError) as exc: diff --git a/src/specify_cli/workflows/command_run.py b/src/specify_cli/workflows/command_run.py index 157edb964b..64eecb091d 100644 --- a/src/specify_cli/workflows/command_run.py +++ b/src/specify_cli/workflows/command_run.py @@ -2,6 +2,8 @@ from __future__ import annotations +from typing import Annotated + from . import _command_run_ownership as run_ownership from . import _commands as cli @@ -17,10 +19,15 @@ def workflow_run( "--json", help="Emit the run outcome as a single JSON object instead of formatted text.", ), + trust_steps: Annotated[ + bool, + cli.typer.Option( + "--trust-steps", help="Authorize all custom step Python for this invocation", + ), + ] = False, ): """Run a workflow from an installed ID or local YAML path.""" from ..integrations.installer import IntegrationInstallError - from . import load_custom_steps from .engine import WorkflowEngine source_path = cli.Path(source).expanduser() @@ -50,7 +57,9 @@ def workflow_run( try: with cli._stdout_to_stderr_when(json_output): - load_custom_steps(project_root) + cli._load_custom_steps( + project_root, trust_steps=trust_steps, json_output=json_output, + ) engine = WorkflowEngine(project_root) engine._load_integrations() except (IntegrationInstallError, OSError) as exc: diff --git a/src/specify_cli/workflows/command_update.py b/src/specify_cli/workflows/command_update.py index e95ef3c151..78cc4b7950 100644 --- a/src/specify_cli/workflows/command_update.py +++ b/src/specify_cli/workflows/command_update.py @@ -2,6 +2,8 @@ from __future__ import annotations +from typing import Annotated + from . import _commands as cli @@ -10,15 +12,23 @@ def workflow_update( workflow_id: str | None = cli.typer.Argument( None, help="Workflow ID to update (default: all)" ), + trust_steps: Annotated[ + bool, + cli.typer.Option( + "--trust-steps", help="Authorize all custom step Python for this invocation", + ), + ] = False, ): """Update installed workflow(s) to the latest catalog version.""" from packaging import version as pkg_version - from . import load_custom_steps from .catalog import WorkflowCatalog, WorkflowCatalogError project_root = cli._require_specify_project() - load_custom_steps(project_root) + try: + cli._load_custom_steps(project_root, trust_steps=trust_steps) + except OSError as exc: + cli._fail_integration_load(exc, json_output=False) registry = cli._open_workflow_registry(project_root) workflows_dir = project_root / ".specify" / "workflows" cli._reject_unsafe_dir(project_root / ".specify", ".specify") diff --git a/tests/specify_cli/bundles/test_command_add.py b/tests/specify_cli/bundles/test_command_add.py index 451ab4acf0..ad8725301e 100644 --- a/tests/specify_cli/bundles/test_command_add.py +++ b/tests/specify_cli/bundles/test_command_add.py @@ -35,6 +35,7 @@ def test_add_forwards_refresh_default_without_refreshing(project: Path): offline=False, refresh=False, version=None, + trust_steps=False, ) @@ -49,6 +50,7 @@ def test_add_forwards_version(project: Path): offline=False, refresh=False, version="1.1.0", + trust_steps=False, ) diff --git a/tests/specify_cli/bundles/test_command_init.py b/tests/specify_cli/bundles/test_command_init.py index 7d692b23fb..a7c563823b 100644 --- a/tests/specify_cli/bundles/test_command_init.py +++ b/tests/specify_cli/bundles/test_command_init.py @@ -33,7 +33,7 @@ def test_bundle_init_delegates_without_refresh(project, monkeypatch, bundle): install.assert_not_called() else: install.assert_called_once_with( - bundle, integration=None, offline=True, refresh=False, + bundle, integration=None, offline=True, refresh=False, trust_steps=False, ) diff --git a/tests/specify_cli/bundles/test_command_install.py b/tests/specify_cli/bundles/test_command_install.py index fb5d8e19fd..f3ed0a5ee8 100644 --- a/tests/specify_cli/bundles/test_command_install.py +++ b/tests/specify_cli/bundles/test_command_install.py @@ -1,5 +1,7 @@ from __future__ import annotations +import hashlib +import io import json import os import shutil @@ -33,6 +35,166 @@ REPO_ROOT = Path(__file__).parents[3] +@pytest.mark.parametrize("project_override", [None, "member"]) +@pytest.mark.parametrize("mode", ["unattended", "flag", "accept", "decline"]) +@pytest.mark.parametrize("source", ["bundled", "catalog", "mixed", "mixed-reversed"]) +@pytest.mark.parametrize("command", ["add", "install", "refresh", "update", "init"]) +def test_bundle_forwards_custom_step_consent( + project: Path, monkeypatch, mode: str, source: str, command: str, + project_override: str | None, +): + from specify_cli.bundles.manifest import ComponentRef + from specify_cli.bundles.records import InstalledBundleRecord, save_records + from specify_cli.workflows.catalog import WorkflowCatalog, WorkflowRegistry + from tests.specify_cli.workflows.helpers import ( + force_workflow_stdin, + write_step_import_probe, + ) + + launch_root = Path.cwd() + nested_marker = None + if project_override is not None: + project = project / project_override + nested_marker = write_step_import_probe(project / project_override) + monkeypatch.setenv("SPECIFY_INIT_DIR", project_override) + original_override = os.environ.get("SPECIFY_INIT_DIR") + marker = write_step_import_probe(project) + force_workflow_stdin(monkeypatch, tty=mode in {"accept", "decline"}) + workflow_sources = { + "bundled": {"trust-workflow": "bundled"}, + "catalog": {"trust-workflow": "catalog"}, + "mixed": {"first-workflow": "bundled", "second-workflow": "catalog"}, + "mixed-reversed": {"first-workflow": "catalog", "second-workflow": "bundled"}, + }[source] + workflow_contents = {} + workflow_dirs = {} + workflow_urls = {} + for wid in workflow_sources: + workflow_dirs[wid] = project / "workflow-source" / wid + workflow_dirs[wid].mkdir(parents=True) + workflow_contents[wid] = ( + 'schema_version: "1.0"\n' + f"workflow:\n id: {wid}\n name: Trust workflow\n version: 1.0.0\n" + "steps:\n - id: probe\n type: import-probe\n" + ).encode() + (workflow_dirs[wid] / "workflow.yml").write_bytes(workflow_contents[wid]) + workflow_urls[wid] = f"https://example.com/{wid}.yml" + monkeypatch.setattr( + "specify_cli._assets._locate_bundled_workflow", + lambda wid: workflow_dirs[wid] if workflow_sources.get(wid) == "bundled" else None, + ) + catalog_source = SimpleNamespace( + name="test", url="https://example.com/catalog.json", install_allowed=True, + ) + monkeypatch.setattr(WorkflowCatalog, "get_active_catalogs", lambda self: [catalog_source]) + monkeypatch.setattr( + WorkflowCatalog, "_fetch_single_catalog", + lambda self, entry: {"workflows": { + wid: { + "id": wid, "version": "1.0.0", "url": workflow_urls[wid], + "sha256": hashlib.sha256(workflow_contents[wid]).hexdigest(), + } + for wid in workflow_sources + }}, + ) + + content_by_url = { + workflow_urls[wid]: workflow_contents[wid] for wid in workflow_sources + } + + class WorkflowResponse(io.BytesIO): + def __init__(self, url): + super().__init__(content_by_url[url]) + self.url = url + + def geturl(self): + return self.url + + monkeypatch.setattr( + "specify_cli.authentication.http.open_url", + lambda url, **kwargs: WorkflowResponse(url), + ) + manifest_data = valid_manifest_dict(provides={"workflows": [ + {"id": wid, "version": "1.0.0"} for wid in workflow_sources + ]}) + manifest = BundleManifest.from_dict(manifest_data) + bundle_path = write_manifest(project / "bundle-source", manifest_data) + installed = { + wid: project / ".specify" / "workflows" / wid / "workflow.yml" + for wid in workflow_sources + } + original = { + wid: content + b"# Previously installed workflow\n" + for wid, content in workflow_contents.items() + } + if command in {"update", "refresh"}: + save_records(project, [ + InstalledBundleRecord.create("demo-bundle", "1.2.0", [ + ComponentRef("workflows", wid, "1.0.0") for wid in workflow_sources + ]), + ]) + for wid, path in installed.items(): + path.parent.mkdir(parents=True) + path.write_bytes(original[wid]) + WorkflowRegistry(project).add(wid, {"version": "1.0.0"}) + if command == "update": + resolved = SimpleNamespace(install_allowed=True) + monkeypatch.setattr( + "specify_cli.bundles.command_update._build_stack", + lambda *args, **kwargs: SimpleNamespace(resolve=lambda *args: resolved), + ) + monkeypatch.setattr( + "specify_cli.bundles.command_update._download_manifest", + lambda *args, **kwargs: manifest, + ) + arguments = ["bundle", "update", "demo-bundle"] + else: + arguments = ["bundle", "install" if command == "refresh" else command, str(bundle_path)] + if command == "refresh": + arguments.append("--refresh") + if source == "bundled": + arguments.append("--offline") + if mode == "flag": + arguments.append("--trust-steps") + + result = runner.invoke(app, arguments, input="n\n" if mode == "decline" else "y\n") + + approved = mode in {"accept", "flag"} + assert result.exit_code == (0 if approved else 1), result.output + assert nested_marker is None or not nested_marker.exists() + assert marker.exists() is approved + assert Path.cwd() == launch_root + assert os.environ.get("SPECIFY_INIT_DIR") == original_override + assert result.output.count("Trust all custom workflow step code") == ( + 1 if mode in {"accept", "decline"} else 0 + ) + if approved: + for wid, path in installed.items(): + assert path.read_bytes() == workflow_contents[wid] + + marker.unlink() + repeated_args = ["bundle", "install", str(bundle_path), "--refresh"] + if source == "bundled": + repeated_args.append("--offline") + repeated = runner.invoke(app, repeated_args, input="n\n") + + assert repeated.exit_code == 1, repeated.output + assert not marker.exists() + for wid, path in installed.items(): + assert path.read_bytes() == workflow_contents[wid] + else: + expected = "not approved" if mode == "decline" else "--trust-steps" + assert expected in " ".join(result.output.split()) + for wid, path in installed.items(): + if command in {"update", "refresh"}: + assert path.read_bytes() == original[wid] + else: + assert not path.exists() + assert nested_marker is None or not nested_marker.exists() + assert Path.cwd() == launch_root + assert os.environ.get("SPECIFY_INIT_DIR") == original_override + + @pytest.mark.parametrize("refresh", [False, True]) def test_bundled_patch_upgrade_warns_and_records_actual_version( project: Path, monkeypatch, refresh: bool, diff --git a/tests/specify_cli/bundles/test_primitives.py b/tests/specify_cli/bundles/test_primitives.py index dfb0788a20..8fdb2c3304 100644 --- a/tests/specify_cli/bundles/test_primitives.py +++ b/tests/specify_cli/bundles/test_primitives.py @@ -449,8 +449,8 @@ def test_offline_workflow_allows_bundled(tmp_path: Path, monkeypatch): ) calls: list[tuple] = [] - def _workflow_add(wid, dev=None, from_url=None): - calls.append((wid, dev, from_url)) + def _workflow_add(wid, dev=None, from_url=None, *, trust_steps=False): + calls.append((wid, dev, from_url, trust_steps)) monkeypatch.setattr( specify_cli, @@ -461,7 +461,53 @@ def _workflow_add(wid, dev=None, from_url=None): manager = primitive_manager("workflows", tmp_path, allow_network=False) manager.install(_component("workflows", "bundled-wf")) - assert calls == [(str(bundled / "workflow.yml"), True, None)] + assert calls == [(str(bundled / "workflow.yml"), True, None, False)] + + +@pytest.mark.parametrize("relative_root", [False, True]) +def test_step_approval_is_shared_only_within_installer_and_project( + tmp_path, monkeypatch, relative_root, +): + import specify_cli + from specify_cli.workflows import _commands as workflow_cli + + bundled = tmp_path / "bundled" + bundled.mkdir() + (bundled / "workflow.yml").write_text( + "workflow:\n id: bundled-wf\n version: 1.0.0\n", encoding="utf-8", + ) + monkeypatch.setattr( + "specify_cli._assets._locate_bundled_workflow", lambda wid: bundled, + ) + approvals = [] + forwarded = [] + monkeypatch.setattr( + workflow_cli, "_approve_custom_steps", + lambda root: approvals.append(root.resolve()) or True, + ) + monkeypatch.setattr( + specify_cli, "workflow_add", + lambda *args, **kwargs: forwarded.append(kwargs["trust_steps"]), + ) + project_a = tmp_path / "a" + project_b = tmp_path / "b" + for project in (project_a, project_b): + (project / ".specify").mkdir(parents=True) + monkeypatch.chdir(tmp_path) + root_a = Path("a") if relative_root else project_a + root_b = Path("b") if relative_root else project_b + installer = DefaultPrimitiveInstaller(allow_network=False) + component = _component("workflows", "bundled-wf") + + installer.install(root_a, component) + installer.install(root_a, component) + installer.install(root_b, component) + + assert approvals == [project_a.resolve(), project_b.resolve()] + assert forwarded == [True, True, True] + + DefaultPrimitiveInstaller(allow_network=False).install(root_a, component) + assert approvals == [project_a.resolve(), project_b.resolve(), project_a.resolve()] def test_assert_pinned_version_matches_passes(): diff --git a/tests/specify_cli/workflows/helpers.py b/tests/specify_cli/workflows/helpers.py index fafa555e73..9bcc3a117f 100644 --- a/tests/specify_cli/workflows/helpers.py +++ b/tests/specify_cli/workflows/helpers.py @@ -30,6 +30,33 @@ def force_gate_stdin(monkeypatch, *, tty: bool): monkeypatch.setattr(gate_module, "sys", _FakeSys(tty=tty)) +def force_workflow_stdin(monkeypatch, *, tty: bool): + from specify_cli.workflows import _commands + + monkeypatch.setattr(_commands, "sys", _FakeSys(tty=tty)) + + +def write_step_import_probe(project_root: Path) -> Path: + """Create an unregistered step whose import has an observable side effect.""" + step_dir = project_root / ".specify" / "workflows" / "steps" / "import-probe" + step_dir.mkdir(parents=True) + marker = project_root / "step-imported.txt" + (step_dir / "step.yml").write_text( + "step:\n type_key: import-probe\n", encoding="utf-8", + ) + (step_dir / "__init__.py").write_text( + "from pathlib import Path\n" + "from specify_cli.workflows.base import StepBase, StepResult\n" + f"Path({str(marker)!r}).write_text('imported', encoding='utf-8')\n" + "class ImportProbe(StepBase):\n" + " type_key = 'import-probe'\n" + " def execute(self, config, context):\n" + " return StepResult(output={'ok': True})\n", + encoding="utf-8", + ) + return marker + + def write_workflow(project_root: Path, workflow_id: str, data: dict) -> Path: workflow_dir = project_root / ".specify" / "workflows" / workflow_id workflow_dir.mkdir(parents=True, exist_ok=True) diff --git a/tests/specify_cli/workflows/step/test_command_add.py b/tests/specify_cli/workflows/step/test_command_add.py index 8eda3941e4..0045a86a68 100644 --- a/tests/specify_cli/workflows/step/test_command_add.py +++ b/tests/specify_cli/workflows/step/test_command_add.py @@ -987,7 +987,7 @@ def test_dev_installs_and_loads(self, project_dir, tmp_path, monkeypatch): installed = project_dir / ".specify" / "workflows" / "steps" / "dev-load-step" assert (installed / "step.yml").is_file() - loaded = load_custom_steps(project_dir) + loaded = load_custom_steps(project_dir, trusted=True) assert "dev-load-step" in loaded assert "dev-load-step" in STEP_REGISTRY @@ -1647,11 +1647,13 @@ def test_dev_install_loads_runs_and_removes( ) assert installed.exit_code == 0, installed.output - assert "dev-step" in load_custom_steps(project_dir) + assert "dev-step" in load_custom_steps(project_dir, trusted=True) workflow_file = tmp_path / "custom-step-wf.yml" workflow_file.write_text(self._WORKFLOW_YAML, encoding="utf-8") - run = runner.invoke(app, ["workflow", "run", str(workflow_file), "--json"]) + run = runner.invoke( + app, ["workflow", "run", str(workflow_file), "--json", "--trust-steps"] + ) assert run.exit_code == 0, run.output assert "completed" in run.output diff --git a/tests/specify_cli/workflows/test_command_add.py b/tests/specify_cli/workflows/test_command_add.py index 263fe79add..6d648124fa 100644 --- a/tests/specify_cli/workflows/test_command_add.py +++ b/tests/specify_cli/workflows/test_command_add.py @@ -22,6 +22,40 @@ runner = CliRunner() +@pytest.mark.parametrize("mode", ["unattended", "decline", "accept", "flag"]) +def test_add_requires_custom_step_consent(project_dir, monkeypatch, mode): + from tests.specify_cli.workflows.helpers import ( + force_workflow_stdin, + write_step_import_probe, + ) + + marker = write_step_import_probe(project_dir) + source = project_dir / "workflow.yml" + source.write_text( + 'schema_version: "1.0"\n' + "workflow:\n id: trust-add\n name: Trust add\n version: 1.0.0\n" + "steps:\n - id: probe\n type: import-probe\n", + encoding="utf-8", + ) + monkeypatch.chdir(project_dir) + force_workflow_stdin(monkeypatch, tty=mode in {"decline", "accept"}) + arguments = ["workflow", "add", str(source), "--dev"] + if mode == "flag": + arguments.append("--trust-steps") + + result = runner.invoke( + app, arguments, input="n\n" if mode == "decline" else "y\n", + ) + + approved = mode in {"accept", "flag"} + assert result.exit_code == (0 if approved else 1), result.output + assert marker.exists() is approved + installed = project_dir / ".specify" / "workflows" / "trust-add" / "workflow.yml" + assert installed.exists() is approved + if not approved: + assert "custom" in result.output.lower() + + class TestWorkflowAddCaseInsensitiveSuffix: """`workflow add` must detect a local YAML file case-insensitively, matching @@ -128,7 +162,9 @@ def execute(self, config, context): encoding="utf-8", ) monkeypatch.chdir(temp_dir) - result = CliRunner().invoke(app, ["workflow", "add", str(src)]) + result = CliRunner().invoke( + app, ["workflow", "add", str(src), "--trust-steps"] + ) assert result.exit_code == 0, result.output installed_workflow = ( diff --git a/tests/specify_cli/workflows/test_command_resume.py b/tests/specify_cli/workflows/test_command_resume.py index 31e984300f..fd68395480 100644 --- a/tests/specify_cli/workflows/test_command_resume.py +++ b/tests/specify_cli/workflows/test_command_resume.py @@ -10,6 +10,53 @@ import pytest +@pytest.mark.parametrize("json_output", [False, True]) +@pytest.mark.parametrize("mode", ["unattended", "decline", "accept", "flag"]) +def test_resume_requires_custom_step_consent(tmp_path, monkeypatch, json_output, mode): + from typer.testing import CliRunner + + from specify_cli import app + from specify_cli.workflows.engine import WorkflowDefinition, WorkflowEngine + from tests.specify_cli.workflows.helpers import ( + force_workflow_stdin, + write_step_import_probe, + ) + + monkeypatch.chdir(tmp_path) + state = WorkflowEngine(tmp_path).execute( + WorkflowDefinition.from_string(TestWorkflowJsonOutput._WF) + ) + assert state.status.value == "paused" + state_path = tmp_path / ".specify" / "workflows" / "runs" / state.run_id / "state.json" + saved = state_path.read_bytes() + marker = write_step_import_probe(tmp_path) + force_workflow_stdin(monkeypatch, tty=mode in {"decline", "accept"}) + arguments = ["workflow", "resume", state.run_id] + if json_output: + arguments.append("--json") + if mode == "flag": + arguments.append("--trust-steps") + + result = CliRunner().invoke( + app, arguments, input="n\n" if mode == "decline" else "y\n", + ) + + approved = mode in {"accept", "flag"} + assert result.exit_code == (0 if approved else 1), result.output + assert marker.exists() is approved + if not approved: + assert state_path.read_bytes() == saved + if json_output: + payload = json.loads(result.stdout) + assert payload["run_id"] == state.run_id + assert payload["status"] == ("paused" if approved else "failed") + if not approved: + assert payload["workflow_id"] is None + assert payload["error"] + if mode in {"decline", "accept"}: + assert "Trust all custom workflow step code" in result.output + + class TestWorkflowJsonOutput: """Test the --json machine-readable output for run/resume/status.""" diff --git a/tests/specify_cli/workflows/test_command_run.py b/tests/specify_cli/workflows/test_command_run.py index 316af276b5..4108666593 100644 --- a/tests/specify_cli/workflows/test_command_run.py +++ b/tests/specify_cli/workflows/test_command_run.py @@ -13,6 +13,115 @@ from tests.specify_cli.workflows.helpers import force_gate_stdin as _force_gate_stdin +@pytest.mark.parametrize("trust_steps", [False, True]) +def test_collecting_consent_does_not_import_step_code(tmp_path, monkeypatch, trust_steps): + from specify_cli.workflows import _commands as cli + from tests.specify_cli.workflows.helpers import ( + force_workflow_stdin, + write_step_import_probe, + ) + + marker = write_step_import_probe(tmp_path) + force_workflow_stdin(monkeypatch, tty=True) + monkeypatch.setattr(cli.typer, "confirm", lambda *args, **kwargs: True) + + assert cli._approve_custom_steps(tmp_path, trust_steps=trust_steps) is True + assert not marker.exists() + + cli._load_custom_steps(tmp_path, trust_steps=True) + assert marker.exists() + + +@pytest.mark.parametrize("json_output", [False, True]) +@pytest.mark.parametrize("console_width", [13, 80]) +@pytest.mark.parametrize( + "mode", ["unattended", "decline", "default", "eof", "accept", "flag", "interactive-flag"], +) +def test_run_requires_custom_step_consent( + tmp_path, monkeypatch, json_output, console_width, mode, +): + from typer.testing import CliRunner + + from specify_cli import app + from specify_cli.workflows import _commands as cli + from tests.specify_cli.workflows.helpers import ( + force_workflow_stdin, + write_step_import_probe, + ) + + marker = write_step_import_probe(tmp_path) + workflow = tmp_path / "workflow.yml" + workflow.write_text(TestWorkflowRunGateOutcomeJson._WF_PLAIN, encoding="utf-8") + monkeypatch.chdir(tmp_path) + monkeypatch.setattr(cli.console, "width", console_width) + monkeypatch.setattr(cli.err_console, "width", console_width) + force_workflow_stdin(monkeypatch, tty=mode not in {"unattended", "flag"}) + arguments = ["workflow", "run", str(workflow)] + if json_output: + arguments.append("--json") + if mode in {"flag", "interactive-flag"}: + arguments.append("--trust-steps") + answers = {"decline": "n\n", "default": "\n", "eof": "", "accept": "y\n"} + result = CliRunner().invoke(app, arguments, input=answers.get(mode, "y\n")) + + approved = mode in {"accept", "flag", "interactive-flag"} + assert result.exit_code == (0 if approved else 1), result.output + assert marker.exists() is approved + if json_output: + payload = json.loads(result.stdout) + assert payload["status"] == ("completed" if approved else "failed") + if not approved: + assert payload["run_id"] is None + assert "approval" in payload["error"] or "not approved" in payload["error"] + if mode in {"accept", "decline", "default", "eof"}: + assert "Trust all custom workflow step code" in result.output + assert "not referenced" in " ".join(result.output.split()) + if json_output: + assert "Trust all custom workflow step code" in result.stderr + else: + assert "Trust all custom workflow step code" not in result.output + + +def test_run_does_not_reuse_prior_cli_approval(tmp_path, monkeypatch): + from typer.testing import CliRunner + + from specify_cli import app + from tests.specify_cli.workflows.helpers import write_step_import_probe + + marker = write_step_import_probe(tmp_path) + workflow = tmp_path / "workflow.yml" + workflow.write_text(TestWorkflowRunGateOutcomeJson._WF_PLAIN, encoding="utf-8") + monkeypatch.chdir(tmp_path) + runner = CliRunner() + arguments = ["workflow", "run", str(workflow), "--json"] + approved = runner.invoke(app, [*arguments, "--trust-steps"]) + assert approved.exit_code == 0, approved.output + assert marker.exists() + marker.unlink() + + refused = runner.invoke(app, arguments, input="y\n") + + assert refused.exit_code == 1, refused.output + assert json.loads(refused.stdout)["status"] == "failed" + assert not marker.exists() + + +def test_missing_workflow_cannot_trigger_unapproved_step_import(tmp_path, monkeypatch): + from typer.testing import CliRunner + + from specify_cli import app + from tests.specify_cli.workflows.helpers import write_step_import_probe + + marker = write_step_import_probe(tmp_path) + monkeypatch.chdir(tmp_path) + + result = CliRunner().invoke(app, ["workflow", "run", "missing", "--json"]) + + assert result.exit_code == 1, result.output + assert "--trust-steps" in json.loads(result.stdout)["error"] + assert not marker.exists() + + class TestWorkflowRunNumberInputs: @pytest.mark.parametrize("source", ["cli", "default", "enum-default"]) def test_run_preserves_integer_input_in_file_and_state( diff --git a/tests/specify_cli/workflows/test_command_update.py b/tests/specify_cli/workflows/test_command_update.py index 476df9e5e0..5b71600c79 100644 --- a/tests/specify_cli/workflows/test_command_update.py +++ b/tests/specify_cli/workflows/test_command_update.py @@ -15,6 +15,33 @@ from packaging.version import Version +@pytest.mark.parametrize("mode", ["unattended", "decline", "accept", "flag"]) +def test_update_requires_custom_step_consent(project_dir, monkeypatch, mode): + from typer.testing import CliRunner + + from specify_cli import app + from tests.specify_cli.workflows.helpers import ( + force_workflow_stdin, + write_step_import_probe, + ) + + marker = write_step_import_probe(project_dir) + monkeypatch.chdir(project_dir) + force_workflow_stdin(monkeypatch, tty=mode in {"decline", "accept"}) + arguments = ["workflow", "update"] + if mode == "flag": + arguments.append("--trust-steps") + + result = CliRunner().invoke( + app, arguments, input="n\n" if mode == "decline" else "y\n", + ) + + approved = mode in {"accept", "flag"} + assert result.exit_code == (0 if approved else 1), result.output + assert marker.exists() is approved + assert ("No workflows installed" in result.output) is approved + + class TestWorkflowCliAlignment: """CLI alignment with extension/preset commands (#2342).""" @@ -338,7 +365,9 @@ def test_update_installs_newer_workflow_with_project_custom_step( 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") + result = CliRunner().invoke( + app, ["workflow", "update", "--trust-steps"], input="y\n" + ) assert result.exit_code == 0, result.output assert custom_type in workflows.STEP_REGISTRY @@ -424,7 +453,9 @@ def response_for(url, **_kwargs): with patch( "specify_cli.authentication.http.open_url", side_effect=response_for ): - result_a = runner.invoke(app, ["workflow", "update"], input="y\n") + result_a = runner.invoke( + app, ["workflow", "update", "--trust-steps"], input="y\n" + ) assert result_a.exit_code == 0, result_a.output assert type_a in workflows.STEP_REGISTRY @@ -513,7 +544,9 @@ def test_update_rejects_unknown_step_without_changing_installed_workflow( 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") + result = CliRunner().invoke( + app, ["workflow", "update", "--trust-steps"], input="y\n" + ) assert result.exit_code != 0 assert "invalid type 'unknown-update-step'" in result.output diff --git a/tests/specify_cli/workflows/test_custom_steps.py b/tests/specify_cli/workflows/test_custom_steps.py index ed0d7e0650..f4c16a00b3 100644 --- a/tests/specify_cli/workflows/test_custom_steps.py +++ b/tests/specify_cli/workflows/test_custom_steps.py @@ -5,6 +5,8 @@ import shutil from pathlib import Path +import pytest + from specify_cli.workflows import STEP_REGISTRY, load_custom_steps @@ -30,20 +32,65 @@ def test_custom_steps_refresh_for_active_project(tmp_path): _write_step(project_a, "a") _write_step(project_b, "b") - assert load_custom_steps(project_a) == ["custom"] + assert load_custom_steps(project_a, trusted=True) == ["custom"] assert STEP_REGISTRY["custom"].execute({}, None).output == {"marker": "a"} - assert load_custom_steps(project_b) == ["custom"] + assert load_custom_steps(project_b, trusted=True) == ["custom"] assert STEP_REGISTRY["custom"].execute({}, None).output == {"marker": "b"} def test_removed_custom_step_is_not_retained(tmp_path): project = tmp_path / "project" _write_step(project, "old") - assert load_custom_steps(project) == ["custom"] + assert load_custom_steps(project, trusted=True) == ["custom"] step_dir = project / ".specify" / "workflows" / "steps" / "custom" shutil.rmtree(step_dir) assert load_custom_steps(project) == [] assert "custom" not in STEP_REGISTRY + + +@pytest.mark.parametrize("registered", [False, True]) +def test_unapproved_package_never_executes(tmp_path, registered): + """Neither discovered files nor a repository registry constitute consent.""" + project = tmp_path / "project" + _write_step(project, "unapproved") + step_dir = project / ".specify" / "workflows" / "steps" / "custom" + marker = tmp_path / "imported" + (step_dir / "__init__.py").write_text( + f"from pathlib import Path\nPath({str(marker)!r}).touch()\n", + encoding="utf-8", + ) + if registered: + from specify_cli.workflows.step.catalog import StepRegistry + + StepRegistry(project).add("custom", {"type_key": "custom", "source": "local"}) + + with pytest.raises(PermissionError, match="--trust-steps"): + load_custom_steps(project) + + assert not marker.exists() + assert "custom" not in STEP_REGISTRY + + +def test_approval_is_not_retained_between_calls(tmp_path): + project = tmp_path / "project" + _write_step(project, "approved") + + assert load_custom_steps(project, trusted=True) == ["custom"] + with pytest.raises(PermissionError, match="--trust-steps"): + load_custom_steps(project) + assert "custom" not in STEP_REGISTRY + + +def test_approval_does_not_transfer_to_another_project(tmp_path): + project_a = tmp_path / "a" + project_b = tmp_path / "b" + _write_step(project_a, "approved") + _write_step(project_b, "unapproved") + + assert load_custom_steps(project_a, trusted=True) == ["custom"] + with pytest.raises(PermissionError, match="--trust-steps"): + load_custom_steps(project_b) + assert "custom" not in STEP_REGISTRY diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 2910f8b8ad..a8652fa79a 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -11350,7 +11350,7 @@ def execute(self, config, context): """ (step_dir / "__init__.py").write_text(init_py, encoding="utf-8") - loaded = load_custom_steps(project_dir) + loaded = load_custom_steps(project_dir, trusted=True) assert "test-custom" in loaded assert "test-custom" in STEP_REGISTRY @@ -11424,7 +11424,7 @@ def test_skip_broken_init_py(self, project_dir): ) # Should not propagate exception - loaded = load_custom_steps(project_dir) + loaded = load_custom_steps(project_dir, trusted=True) assert "broken-step" not in loaded def test_module_name_sanitized_for_hyphenated_type_key(self, project_dir): @@ -11451,7 +11451,7 @@ def execute(self, config, context): """ (step_dir / "__init__.py").write_text(init_py, encoding="utf-8") - loaded = load_custom_steps(project_dir) + loaded = load_custom_steps(project_dir, trusted=True) assert "my-hyphen-step" in loaded assert "my-hyphen-step" in STEP_REGISTRY # Synthetic module name must be a valid identifier (hyphens → underscores) @@ -11489,7 +11489,7 @@ def execute(self, config, context): """ (step_dir / "__init__.py").write_text(init_py, encoding="utf-8") - loaded = load_custom_steps(project_dir) + loaded = load_custom_steps(project_dir, trusted=True) assert "pkg-step" in loaded assert "pkg-step" in STEP_REGISTRY # Verify the relative import actually resolved; module name includes hash suffix.