Address PR #3557 review comments r3594064534 and r3594064563:
- ProjectOverlaySource.collect now rejects symlinked per-workflow overlay
directories (.specify/workflows/overlays/<id>) before iterating
- InstalledOverlaySource.collect now rejects symlinked installed overlay
directories (.specify/workflows/<id>/overlays) before iterating
- workflow_overlay_list catches ValueError from resolver and exits with
code 1 instead of crashing on unhandled exceptions
- Added .specify/workflows/overlays to _reject_unsafe_workflow_storage
chokepoint for defense-in-depth
These guards prevent symlinked overlay directories from redirecting
auto-loaded overlay YAML to attacker-controlled content outside the
project, which could inject executable shell steps into trusted workflows.
Refs: PR #3557 review comments r3594064534, r3594064563
Assisted-by: opencode-go/qwen3.7-max (autonomous)
* 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>
* 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>
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>
* 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)
* 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.
