Commit Graph

18 Commits

Author SHA1 Message Date
marcelsafin
560c7cdd5f fix(workflows): reject catalog updates whose downloaded version mismatches
The update path never verified the downloaded workflow carries the catalog
version that triggered the update, so a stale or misconfigured URL could
report success while leaving the old version installed or downgrading it.
Pass the expected version into the install helper and fail the update when
the downloaded definition does not match.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-10 23:13:55 +02:00
marcelsafin
a5307256ca fix(workflows): fail cleanly on unparseable catalog install URLs
urlparse raises ValueError on e.g. an unbalanced IPv6 literal before the
invalid-URL branch is reached; on workflow update that also bypassed the
per-workflow handler and aborted the whole command. Convert the parse
failure into a clean error so add fails cleanly and update skips just the
affected workflow.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-10 23:04:29 +02:00
marcelsafin
7100211ffc fix(workflows): escape workflow name/id in install success messages
Workflow names and ids come from user-controlled YAML or external catalog
data; printing them unescaped lets bracket characters be interpreted as
Rich tags. Escape them in the add/catalog-install success messages and the
remaining catalog error paths, matching the rest of the output hardening.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-10 22:46:45 +02:00
marcelsafin
91c71655c7 fix(workflows): escape remaining unescaped Rich markup paths
Covers the last few review threads not yet addressed:
- Escape yaml.YAMLError text in the local workflow add install path
  (matches the already-escaped download/catalog paths).
- Escape the non---dev local directory fallback's "No workflow.yml
  found in <path>" message (the --dev branch already escaped it).
- Escape the redirected final_url in the --from non-HTTPS redirect
  error (IPv6 literals like http://[::1]/... are legal and contain
  brackets).
- Escape the "Downloaded workflow is invalid" exception message in
  _install_workflow_from_catalog, matching the sibling catalog-install
  exception handler a few lines above it.

Adds regression tests for each in TestWorkflowCliAlignment, following
the existing escaping-test pattern in this class.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-10 22:25:33 +02:00
marcelsafin
10f9770f0c fix: escape workflow validation errors before Rich output
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-10 22:18:13 +02:00
marcelsafin
4ca50f588b fix(workflows): escape rich markup in search output
workflow search now escapes catalog-derived name/id/version/description/
tags before printing, matching extension search and workflow list.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-10 22:18:13 +02:00
marcelsafin
129362f29b fix(workflows): catch OSError in per-workflow update loop and make restore best-effort
Transient FS errors (perms, disk full) from backup read or write no
longer abort the whole update run. The restore is wrapped in its own
try/except so a failed write only warns, and the offending workflow
is reported via 'Failed to update' like other per-workflow failures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-10 22:18:12 +02:00
marcelsafin
8409bca244 fix(workflows): escape rich markup in --from download exception message
Matches how the catalog install path escapes exception strings.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-10 22:18:12 +02:00
marcelsafin
4c356c413e fix(workflows): escape rich markup in list output and catalog install errors, isolate update failures
workflow list now escapes id/name/version/description before printing,
matching how extensions render user-editable fields. The catalog install
helper computes safe_wf_id once and uses it for every early error path
plus the final failure message.

workflow update wraps _safe_workflow_id_dir and the backup read inside
the try/except typer.Exit block so an unsafe id in a corrupted registry
fails that one workflow and the rest continue.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-10 22:18:12 +02:00
marcelsafin
52ab7ffdf0 fix(workflows): escape rich markup in id-mismatch errors and validate --from source early
The two id-mismatch error paths interpolated repr() into Rich markup, so
a stray bracket in a user typo could be parsed as markup. Route both
through rich.markup.escape.

`workflow add <source> --from <url>` also validated the source only
after downloading. Validate it up front so a URL/path/typo fails
without a network fetch.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-10 22:18:11 +02:00
marcelsafin
59a791b940 fix(workflows): guard list against corrupted registry entries, re-raise typer.Exit in catalog install
workflow list now skips non-dict registry entries with a warning instead
of crashing, matching update/enable/disable. The broad except in
_install_workflow_from_catalog no longer swallows typer.Exit, so precise
errors like the non-HTTPS redirect message are not duplicated.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-10 22:18:11 +02:00
marcelsafin
cc8143ea73 fix(workflows): preserve disabled state on update, guard corrupted registry entries
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-10 22:18:10 +02:00
marcelsafin
bba88bb34f feat(workflows): align workflow CLI with extension command surface
Adds the missing workflow commands and flags so the workflow CLI
matches the extension/preset pattern: add --dev and --from, search
--author, update, enable and disable. Disabled workflows are blocked
from running and marked in list output.

Fixes #2342

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-10 22:18:10 +02:00
Marsel Safin
34514fb20a fix(workflows): report validation errors instead of crashing on non-string workflow.yml scalars (#3421)
* fix(workflows): validate scalar types before string operations in workflow validation

YAML parses unquoted scalars like version: 1.0 and id: 123 as
float/int, which crashed validate_workflow and workflow add with raw
tracebacks. Type-check id, name, version and step ids before regex
and string operations so these surface as validation errors. Accept
an unquoted schema_version: 1.0 instead of printing a self-identical
rejection message.

Fixes #3420

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(workflows): treat falsey non-strings as type errors, not missing fields

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(workflows): only accept schema_version 1.0 so the error message is accurate

The check also accepted "1" while the error said Expected '1.0'.
Unquoted YAML 1.0 still works via str(); plain 1 is now rejected with
the message that matches.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-10 09:51:19 -05:00
Marsel Safin
882e1e90d0 fix(cli): exit cleanly on malformed IPv6 URLs in extension/preset/workflow add (#3369)
* fix(cli): exit cleanly on malformed IPv6 URLs in extension/preset/workflow add

extension add --from, preset add --from, and workflow add <url> parsed
the user-supplied URL with a bare urlparse before their HTTPS/host
validation, so an unclosed IPv6 bracket escaped as a raw ValueError
traceback. Wrap each parse and emit the surrounding validation's clean
error style + typer.Exit(1) instead.

Fixes #3368

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): convert malformed redirect URLs to URLError in shared redirect handler

Parse the redirect target once in _StripAuthOnRedirect.redirect_request
before the validator and stdlib handler run, converting ValueError into
URLError which every download path already catches. Also escape from_url
in the preset install message so IPv6 brackets don't break Rich markup.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-07-07 15:20:13 -05:00
Ali jawwad
12faf7b5b5 fix(workflows): route run/resume errors to stderr under --json (#3352)
workflow run/resume --json is contracted to emit a single JSON object on
stdout, but every error path (workflow-not-found, invalid workflow,
validation failure, execute/resume failure, and the shared
_parse_input_values invalid-input error) used console.print, landing the
human error text on stdout and corrupting the machine-readable stream.
Route those messages through err_console when --json is set (a no-op for
normal text mode), mirroring the stderr-only error routing already used
by 'specify bundle' (_fail) and err_console elsewhere in this module.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-07-07 14:52:36 -05:00
Pascal THUET
490566847c feat(cli): honor SPECIFY_INIT_DIR in the specify CLI project resolver (#3186)
* feat(cli): honor SPECIFY_INIT_DIR in the specify CLI project resolver

The shell resolver honors SPECIFY_INIT_DIR (#2892), but the Python CLI did
not: it resolved the project as Path.cwd() + a .specify/ check and never read
the override. So setup-plan.sh respected it while `specify integration install`
ignored it, and you still had to cd into the member project.

Route project resolution through a shared _resolve_init_dir_override() that
applies the shell resolver's validation rules (relative to cwd, must exist and
contain .specify/, hard error, no fallback, same error strings). It's wired into
_require_specify_project() — the chokepoint for every project-scoped subcommand
(integration/extension/workflow/preset/...) — and the `workflow run <file>`
standalone path, which re-applies its symlinked-.specify guard on the override
branch too. init is unchanged: it creates .specify/, so the must-pre-exist rule
doesn't apply.

The resolver canonicalizes symlinks via Path.resolve() while the shell keeps the
logical path; they agree for non-symlinked paths (documented in the resolver).

Tests in tests/test_init_dir_cli.py mirror the strict cases from test_init_dir.py
through the CLI; conftest now strips SPECIFY_* for the whole suite so a stray
export can't perturb the now-env-reading resolver. Docs note the CLI applies the
same rules.

Discussion: github/spec-kit#2834

(Disclosure: I used an AI coding agent to audit the call sites and resolver,
draft the change, and run an adversarial code review; reviewed by me.)

* fix(cli): honor SPECIFY_INIT_DIR for bundle commands

Assisted-by: Codex (model: GPT-5, autonomous)

* fix(bundler): refuse symlinked .specify on the SPECIFY_INIT_DIR override path

find_project_root refuses a symlinked .specify (following it could read/write
outside the tree, and a test pins that), but the SPECIFY_INIT_DIR override added
for bundle commands returned early and skipped that guard:
_resolve_init_dir_override validates .specify with is_dir(), which follows
symlinks. So `specify bundle` accepted via the override a layout the cwd path
rejects. Re-check the override result with the same guard, plus a regression test.

(Disclosure: found via an AI code review and fixed with an AI coding agent;
reviewed by me.)

* fix(cli): keep SPECIFY_INIT_DIR strict for bundles

Treat an explicit symlinked SPECIFY_INIT_DIR project as a hard bundle error instead of returning no project, which could initialize the current directory. Align the docs with the actual unset resolver behavior.

Assisted-by: Codex (model: GPT-5, autonomous)

* docs(core): note symlinked .specify handling differs across CLI surfaces

A symlinked .specify is followed by integration/extension/workflow (matching the
shell resolver) but refused by bundle and workflow run <file> (write
confinement). Document the asymmetry so it reads as intentional.

(Disclosure: AI-assisted; reviewed by me.)

* docs(core): reframe symlinked .specify note around the override invariant

Per maintainer feedback on #3186: SPECIFY_INIT_DIR relocates where the project
is, not how a surface treats symlinks. Each surface keeps its cwd-path stance
(write surfaces refuse a symlinked .specify, read/config surfaces follow it),
so the split is one policy relocated, not an inconsistency.

* docs: address Copilot review on resolver docstrings

- _project.py: the error messages "mirror" the shell wording rather than
  "match" it (the CLI renders a Rich `Error:` line, the shell a plain `ERROR:`).
- find_project_root: document that honoring SPECIFY_INIT_DIR when start is None
  can raise typer.Exit / BundlerError, so the Path | None signature isn't
  surprising to direct callers.

* docs(bundler): note require_project_root inherits the override raise behavior

find_project_root can raise typer.Exit / BundlerError under the SPECIFY_INIT_DIR
override (start=None); require_project_root inherits that, so document it
alongside its own BundlerError-on-missing-project.

* docs: clarify symlinked project root behavior

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)

* Address SPECIFY_INIT_DIR review feedback

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)

* Route workflow JSON errors to stderr

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)
2026-07-01 15:55:18 -05:00
darion-yaphet
810d6fcfe1 refactor: move workflow command handlers to workflows/_commands.py (PR-8/8) (#3159)
* refactor: move workflow command handlers to workflows/_commands.py (PR-8/8)

Final PR of the __init__.py split. Moves the workflow command group out of
__init__.py into the existing workflows/ package, completing the domain-dir
layout established in PR-5 (integrations), PR-6 (presets) and PR-7
(extensions).

- New workflows/_commands.py holds the four Typer apps (workflow / catalog /
  step / step-catalog), all 25 command handlers, the six workflow-only
  helpers (_parse_input_values, _workflow_run_payload, _emit_workflow_json,
  _stdout_to_stderr_when, _validate_step_id_or_exit,
  _resolve_steps_base_dir_or_exit), and a register(app) entry point.
- workflows is already a package, so no rename is needed; intra-package
  imports change from `.workflows.x` to `.x`. The only root-helper dep
  (_require_specify_project) is reached through a call-time shim so test
  monkeypatching of specify_cli._require_specify_project keeps working.
- __init__.py drops ~1445 lines (2066 -> 621); the workflow group is
  re-attached via register(app). Dead `contextlib` import removed.
- tests/test_workflows.py: import the now-relocated _stdout_to_stderr_when
  helper from its new home (workflows._commands) instead of the package root.

No behavior change. Full suite green (3847 passed), ruff clean.

* Prevent workflow state writes through symlinked storage

Workflow commands persist run state under .specify/workflows/runs, so the command-local project shim now rejects symlinked workflow storage before any workflow command proceeds. The standalone YAML path uses the same guard because it intentionally bypasses the normal project requirement while still creating workflow state under the current directory.

Constraint: Local YAML workflow runs do not require an existing .specify project directory but still create .specify/workflows/runs state

Rejected: Guard only .specify in the file-source path | .specify/workflows and runs can independently redirect writes

Confidence: high

Scope-risk: narrow

Directive: Keep workflow storage symlink checks centralized before constructing WorkflowEngine

Tested: .venv/bin/python -m pytest tests/test_workflow_run_without_project.py tests/test_workflows.py::TestWorkflowAddSymlinkGuard -v

Tested: .venv/bin/python -m py_compile src/specify_cli/workflows/_commands.py tests/test_workflow_run_without_project.py tests/test_workflows.py

Not-tested: Ruff lint; ruff is not installed in the repo virtualenv

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)

* fix(workflows): pass github_hosts allowlist to GHES release asset resolver

workflow add resolved GitHub release download URLs without forwarding the
github_provider_hosts() allowlist, so resolve_github_release_asset_api_url
never treated any host as GHES. This regressed GitHub Enterprise Server
release asset resolution and diverged from presets/extensions, which already
pass github_hosts. Forward github_provider_hosts() at both the direct-URL and
catalog install call sites. The allowlist remains the anti-SSRF gate.

* fix(workflows): reject symlinked/traversal <id> dir on workflow install

Local/URL and catalog installs wrote to .specify/workflows/<id>/workflow.yml
without guarding the <id> segment. A pre-planted symlink at <id> or
<id>/workflow.yml let mkdir+copy/download follow it and write outside the
project root; a non-directory <id> made mkdir raise unhandled.

Add _safe_workflow_id_dir() to reject path traversal, symlinked or
non-directory <id>, and a symlinked workflow.yml leaf before any write.
Fold the catalog branch's existing traversal check into the helper.

* fix(workflows): harden _safe_workflow_id_dir output and leaf checks

- Reorder symlink/non-directory check before resolve() so a symlinked
  <id> reports as symlinked instead of misleading "Invalid workflow ID"
- Reject a pre-existing <id>/workflow.yml that is not a file, avoiding an
  unhandled IsADirectoryError on later write/copy2
- Escape workflow_id in Rich output to prevent markup injection; escape
  the repr (not the raw id) so repr-added backslashes cannot re-expose
  brackets, matching extensions/_commands.py hardening
- Add tests for workflow.yml-as-directory and markup-escaped invalid id

* Avoid stale lint failures from config helper imports

Move PyYAML loading into the helpers that read and write agent-context configuration, and replace the broad Any annotation with object. The runtime behavior stays the same while the module no longer exposes top-level imports that can be flagged as unused when CI analyzes a narrower code shape.

* Prevent workflow commands from targeting reserved storage

Workflow install and removal paths are derived from workflow IDs before any catalog download, local copy, or directory deletion. Validate that IDs are single workflow-id path segments and reject names reserved for workflow runtime storage so commands cannot target .specify/workflows/runs or .specify/workflows/steps.
2026-06-30 11:03:54 -05:00