fix(init): escape user-supplied values in specify init output (#3787)
* fix(init): escape user-supplied values in `specify init` output
commands/init.py interpolated the project name, --integration/--script values
and paths straight into Rich markup f-strings. It was the only CLI command
module without escaping -- extensions, presets, workflows and integrations all
wrap user-controlled display values already.
Two consequences, both reproduced end-to-end through the real CLI:
1. SILENT WRONG OUTPUT. `specify init "proj [v2]"` exits 0 and creates the
directory, but the Next Steps panel prints
1. Go to the project folder: cd proj
Rich ate `[v2]` as a style tag, so the command the user copy-pastes fails.
2. CRASH AFTER SUCCESS. `specify init "app[/red]x"` creates the project and
then dies with MarkupError("closing tag '[/red]' ... doesn't match any open
tag") -> exit 1 with a traceback for work that actually completed.
Wrap the user-controlled display values in rich.markup.escape: project name
(error/warning/conflict/next-steps), project and working paths, the echoed
--integration and --script values, and the agent folder in the gitignore hint.
Display only -- no control flow, exit codes or messages change, and escape is a
no-op for any value without a tag-shaped bracket run.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(init): shell-quote the project name in the Next Steps cd line
Rich-escaping stopped the brackets being swallowed, but the printed
command was still unusable for any name containing whitespace: `cd proj
v2` is two arguments in every shell.
$ cd proj v2 -> /bin/bash: line 1: cd: too many arguments (rc=1)
$ cd "proj v2" -> rc=0, lands in "proj v2"
Quote it for the host the same way _version._render_argv renders its
copy-pasteable installer command: subprocess.list2cmdline on Windows,
shlex.quote elsewhere. Windows must use double quotes -- cd 'my project'
is a path-not-found in cmd.exe, while cd "my project" is accepted by
cmd.exe, PowerShell and Git Bash alike. Names needing no quoting are
returned unchanged, so the common case is byte-identical.
Shell-quote inner, Rich-escape outer.
Tests execute the printed command through a real shell rather than only
inspecting the string, and pin that an ordinary name stays unquoted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(init): drop the now-redundant local escape imports that shadowed the module one
Rebasing onto main brought in three new extension-install helpers, and two
of them carry a function-local
from rich.markup import escape as _escape_markup
inside `register > init`. This PR adds the same import at module level, so
the locals made `_escape_markup` a local variable for the whole `init`
function — every use *before* those import lines then raised
UnboundLocalError: cannot access local variable '_escape_markup'
where it is not associated with a value
which broke `specify init` outright (7 of 8 tests in this file failed after
the rebase, all with exit_code 1).
The locals are redundant now that the module-level import exists, so remove
them. Verified with an AST scope walk that the only remaining
`_escape_markup` imports are the module-level one and the one inside
`_confirm_extension_url_trust`, which has no module-level use to shadow.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -3,13 +3,16 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import shlex
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import typer
|
||||
from rich.live import Live
|
||||
from rich.markup import escape as _escape_markup
|
||||
from rich.panel import Panel
|
||||
|
||||
from .._agent_config import (
|
||||
@@ -169,6 +172,25 @@ def _install_extension_during_init(project_path: Path, ext_spec: str, speckit_ve
|
||||
return f"{manifest.name} v{manifest.version} installed"
|
||||
|
||||
|
||||
def _shell_quote_arg(value: str) -> str:
|
||||
"""Quote *value* as one argument for the shells of the host OS.
|
||||
|
||||
The Next Steps ``cd`` line is copy-pasted into whichever shell ran
|
||||
``specify init``, so it is quoted for the host the same way
|
||||
``_version._render_argv`` renders its copy-pasteable installer command:
|
||||
``list2cmdline`` on Windows, ``shlex.quote`` elsewhere. The Windows branch
|
||||
must emit double quotes -- ``cd 'my project'`` is a path-not-found in
|
||||
cmd.exe, while ``cd "my project"`` is accepted by cmd.exe, PowerShell and
|
||||
Git Bash alike. A value needing no quoting is returned unchanged.
|
||||
|
||||
Whitespace only. PowerShell also glob-expands ``[``/``]`` and expands
|
||||
``$``/backtick inside double quotes, so such a name still needs
|
||||
``Set-Location -LiteralPath`` there -- syntax invalid in cmd.exe and sh, so
|
||||
this shell-neutral line cannot cover it.
|
||||
"""
|
||||
return subprocess.list2cmdline([value]) if os.name == "nt" else shlex.quote(value)
|
||||
|
||||
|
||||
def ensure_constitution_from_template(
|
||||
project_path: Path, tracker: StepTracker | None = None
|
||||
) -> None:
|
||||
@@ -351,7 +373,10 @@ def register(app: typer.Typer) -> None:
|
||||
if integration:
|
||||
resolved_integration = get_integration(integration)
|
||||
if not resolved_integration:
|
||||
console.print(f"[red]Error:[/red] Unknown integration: '{integration}'")
|
||||
console.print(
|
||||
f"[red]Error:[/red] Unknown integration: "
|
||||
f"'{_escape_markup(str(integration))}'"
|
||||
)
|
||||
available = ", ".join(sorted(INTEGRATION_REGISTRY))
|
||||
console.print(f"[yellow]Available integrations:[/yellow] {available}")
|
||||
raise typer.Exit(1)
|
||||
@@ -428,26 +453,27 @@ def register(app: typer.Typer) -> None:
|
||||
project_path = Path(project_name).resolve()
|
||||
dir_existed_before = project_path.exists()
|
||||
if project_path.exists():
|
||||
safe_name = _escape_markup(str(project_name))
|
||||
if not project_path.is_dir():
|
||||
console.print(
|
||||
f"[red]Error:[/red] '{project_name}' exists but is not a directory."
|
||||
f"[red]Error:[/red] '{safe_name}' exists but is not a directory."
|
||||
)
|
||||
raise typer.Exit(1)
|
||||
existing_items = list(project_path.iterdir())
|
||||
if force:
|
||||
if existing_items:
|
||||
console.print(
|
||||
f"[yellow]Warning:[/yellow] Directory '{project_name}' is not empty ({len(existing_items)} items)"
|
||||
f"[yellow]Warning:[/yellow] Directory '{safe_name}' is not empty ({len(existing_items)} items)"
|
||||
)
|
||||
console.print(
|
||||
"[yellow]Template files will be merged with existing content and may overwrite existing files[/yellow]"
|
||||
)
|
||||
console.print(
|
||||
f"[cyan]--force supplied: merging into existing directory '[cyan]{project_name}[/cyan]'[/cyan]"
|
||||
f"[cyan]--force supplied: merging into existing directory '[cyan]{safe_name}[/cyan]'[/cyan]"
|
||||
)
|
||||
else:
|
||||
error_panel = Panel(
|
||||
f"Directory already exists: '[cyan]{project_name}[/cyan]'\n"
|
||||
f"Directory already exists: '[cyan]{safe_name}[/cyan]'\n"
|
||||
"Please choose a different project name or remove the existing directory.\n"
|
||||
"Use [bold]--force[/bold] to merge into the existing directory.",
|
||||
title="[red]Directory Conflict[/red]",
|
||||
@@ -461,7 +487,7 @@ def register(app: typer.Typer) -> None:
|
||||
if integration:
|
||||
if integration not in AGENT_CONFIG:
|
||||
console.print(
|
||||
f"[red]Error:[/red] Invalid integration '{integration}'. Choose from: {', '.join(AGENT_CONFIG.keys())}"
|
||||
f"[red]Error:[/red] Invalid integration '{_escape_markup(str(integration))}'. Choose from: {', '.join(AGENT_CONFIG.keys())}"
|
||||
)
|
||||
raise typer.Exit(1)
|
||||
selected_ai = integration
|
||||
@@ -500,12 +526,14 @@ def register(app: typer.Typer) -> None:
|
||||
setup_lines = [
|
||||
"[cyan]Specify Project Setup[/cyan]",
|
||||
"",
|
||||
f"{'Project':<15} [green]{project_path.name}[/green]",
|
||||
f"{'Working Path':<15} [dim]{current_dir}[/dim]",
|
||||
f"{'Project':<15} [green]{_escape_markup(project_path.name)}[/green]",
|
||||
f"{'Working Path':<15} [dim]{_escape_markup(str(current_dir))}[/dim]",
|
||||
]
|
||||
|
||||
if not here:
|
||||
setup_lines.append(f"{'Target Path':<15} [dim]{project_path}[/dim]")
|
||||
setup_lines.append(
|
||||
f"{'Target Path':<15} [dim]{_escape_markup(str(project_path))}[/dim]"
|
||||
)
|
||||
|
||||
console.print(
|
||||
Panel("\n".join(setup_lines), border_style="cyan", padding=(1, 2))
|
||||
@@ -532,7 +560,7 @@ def register(app: typer.Typer) -> None:
|
||||
if script_type:
|
||||
if script_type not in SCRIPT_TYPE_CHOICES:
|
||||
console.print(
|
||||
f"[red]Error:[/red] Invalid script type '{script_type}'. Choose from: {', '.join(SCRIPT_TYPE_CHOICES.keys())}"
|
||||
f"[red]Error:[/red] Invalid script type '{_escape_markup(str(script_type))}'. Choose from: {', '.join(SCRIPT_TYPE_CHOICES.keys())}"
|
||||
)
|
||||
raise typer.Exit(1)
|
||||
selected_script = script_type
|
||||
@@ -571,8 +599,6 @@ def register(app: typer.Typer) -> None:
|
||||
tracker.add(key, label)
|
||||
|
||||
if extensions:
|
||||
from rich.markup import escape as _escape_markup
|
||||
|
||||
for i, ext_spec in enumerate(extensions):
|
||||
tracker.add(
|
||||
f"extension-{i}", f"Install extension: {_escape_markup(ext_spec)}"
|
||||
@@ -827,8 +853,6 @@ def register(app: typer.Typer) -> None:
|
||||
|
||||
# Install extensions specified via --extension
|
||||
if extensions:
|
||||
from rich.markup import escape as _escape_markup
|
||||
|
||||
from ..extensions._commands import _refresh_events_and_warn
|
||||
|
||||
speckit_ver = get_speckit_version()
|
||||
@@ -918,7 +942,7 @@ def register(app: typer.Typer) -> None:
|
||||
if agent_folder:
|
||||
security_notice = Panel(
|
||||
f"Some agents may store credentials, auth tokens, or other identifying and private artifacts in the agent folder within your project.\n"
|
||||
f"Consider adding [cyan]{agent_folder}[/cyan] (or parts of it) to [cyan].gitignore[/cyan] to prevent accidental credential leakage.",
|
||||
f"Consider adding [cyan]{_escape_markup(str(agent_folder))}[/cyan] (or parts of it) to [cyan].gitignore[/cyan] to prevent accidental credential leakage.",
|
||||
title="[yellow]Agent Folder Security[/yellow]",
|
||||
border_style="yellow",
|
||||
padding=(1, 2),
|
||||
@@ -929,7 +953,7 @@ def register(app: typer.Typer) -> None:
|
||||
steps_lines = []
|
||||
if not here:
|
||||
steps_lines.append(
|
||||
f"1. Go to the project folder: [cyan]cd {project_name}[/cyan]"
|
||||
f"1. Go to the project folder: [cyan]cd {_escape_markup(_shell_quote_arg(str(project_name)))}[/cyan]"
|
||||
)
|
||||
step_num = 2
|
||||
else:
|
||||
|
||||
@@ -0,0 +1,176 @@
|
||||
"""`specify init` must render user-supplied values literally, not as Rich markup.
|
||||
|
||||
`commands/init.py` interpolated the project name, `--integration`/`--script`
|
||||
values and paths straight into Rich markup f-strings. A name containing a
|
||||
tag-shaped bracket run was therefore consumed as markup:
|
||||
|
||||
* ``specify init "proj [v2]"`` succeeded and created the directory, but the
|
||||
Next Steps panel printed ``cd proj`` -- a command that fails when pasted.
|
||||
* ``specify init "app[/red]x"`` created the directory and then died with
|
||||
``MarkupError``, so the user saw a traceback for a project that had in fact
|
||||
been scaffolded.
|
||||
|
||||
Every sibling CLI module (extensions, presets, workflows, integrations) already
|
||||
escapes user-controlled display values; init.py was the outlier.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
from typer.testing import CliRunner
|
||||
|
||||
from specify_cli import app
|
||||
from specify_cli.commands.init import _shell_quote_arg
|
||||
|
||||
from tests.conftest import requires_bash
|
||||
|
||||
_ANSI = re.compile(r"\x1b\[[0-9;]*m")
|
||||
|
||||
|
||||
def _strip(text: str) -> str:
|
||||
return _ANSI.sub("", text or "")
|
||||
|
||||
|
||||
def _init(tmp_path: Path, name: str):
|
||||
"""Run a fully offline, non-interactive `specify init <name>`."""
|
||||
previous = os.getcwd()
|
||||
os.chdir(tmp_path)
|
||||
try:
|
||||
return CliRunner().invoke(
|
||||
app,
|
||||
[
|
||||
"init",
|
||||
name,
|
||||
"--integration",
|
||||
"generic",
|
||||
"--integration-options",
|
||||
"--commands-dir .agent/commands",
|
||||
"--ignore-agent-tools",
|
||||
"--offline",
|
||||
],
|
||||
catch_exceptions=True,
|
||||
)
|
||||
finally:
|
||||
os.chdir(previous)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("name", ["proj [v2]", "my[bold]app"])
|
||||
def test_next_steps_cd_shows_the_real_project_name(tmp_path: Path, name: str):
|
||||
"""The `cd` line must name the directory that was actually created."""
|
||||
result = _init(tmp_path, name)
|
||||
assert result.exit_code == 0, _strip(result.stdout)
|
||||
assert (tmp_path / name).is_dir()
|
||||
|
||||
out = _strip(result.stdout)
|
||||
cd_lines = [line for line in out.splitlines() if "cd " in line]
|
||||
assert cd_lines, out
|
||||
assert f"cd {_shell_quote_arg(name)}" in " ".join(cd_lines), cd_lines
|
||||
|
||||
|
||||
def test_closing_tag_in_project_name_does_not_crash(tmp_path: Path):
|
||||
"""A name forming a closing tag raised MarkupError *after* the project had
|
||||
been created, so init reported failure for work it had completed."""
|
||||
name = "app[/red]x"
|
||||
result = _init(tmp_path, name)
|
||||
|
||||
assert result.exception is None or not isinstance(
|
||||
result.exception, Exception
|
||||
) or "MarkupError" not in type(result.exception).__name__, (
|
||||
f"unexpected {type(result.exception).__name__}: {result.exception}"
|
||||
)
|
||||
assert result.exit_code == 0, _strip(result.stdout)
|
||||
assert (tmp_path / name).is_dir()
|
||||
assert f"cd {_shell_quote_arg(name)}" in _strip(result.stdout)
|
||||
|
||||
|
||||
def test_invalid_integration_value_is_rendered_literally(tmp_path: Path):
|
||||
"""An invalid `--integration` value is echoed back; it must not be parsed as
|
||||
markup (nor raise) when it contains a bracket run."""
|
||||
previous = os.getcwd()
|
||||
os.chdir(tmp_path)
|
||||
try:
|
||||
result = CliRunner().invoke(
|
||||
app,
|
||||
["init", "proj", "--integration", "nope[/red]", "--ignore-agent-tools"],
|
||||
catch_exceptions=True,
|
||||
)
|
||||
finally:
|
||||
os.chdir(previous)
|
||||
|
||||
assert result.exit_code != 0
|
||||
assert "nope[/red]" in _strip(result.stdout)
|
||||
|
||||
|
||||
def _cd_argument(stdout: str) -> str:
|
||||
"""Return the argument of the printed `cd` command, verbatim.
|
||||
|
||||
The line is rendered inside a Rich panel, so the trailing box-drawing
|
||||
border and its padding are stripped before the argument is compared.
|
||||
"""
|
||||
marker = "Go to the project folder: cd "
|
||||
for line in _strip(stdout).splitlines():
|
||||
if marker in line:
|
||||
return line.split(marker, 1)[1].rstrip().rstrip("│").rstrip()
|
||||
raise AssertionError(f"no cd line in output:\n{stdout}")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("name", ["proj v2", "my project"])
|
||||
def test_cd_line_quotes_a_name_containing_whitespace(tmp_path: Path, name: str):
|
||||
"""Rich-escaping alone left `cd proj v2`, which every shell reads as two
|
||||
arguments, so the copy-pasted command did not enter the directory."""
|
||||
result = _init(tmp_path, name)
|
||||
assert result.exit_code == 0, _strip(result.stdout)
|
||||
assert (tmp_path / name).is_dir()
|
||||
|
||||
printed = _cd_argument(result.stdout)
|
||||
assert printed != name, "a whitespace-bearing name must be quoted"
|
||||
assert name in printed, printed
|
||||
assert printed == _shell_quote_arg(name)
|
||||
|
||||
|
||||
def test_ordinary_name_is_not_quoted(tmp_path: Path):
|
||||
"""The common case must stay byte-identical: no gratuitous quoting."""
|
||||
result = _init(tmp_path, "my-project")
|
||||
assert result.exit_code == 0, _strip(result.stdout)
|
||||
assert _cd_argument(result.stdout) == "my-project"
|
||||
|
||||
|
||||
@requires_bash
|
||||
@pytest.mark.parametrize("name", ["proj v2", "proj [v2]", "my-project"])
|
||||
def test_printed_cd_command_actually_changes_directory(tmp_path: Path, name: str):
|
||||
"""Execute the printed command rather than only inspecting it.
|
||||
|
||||
This is the assertion the string comparisons cannot make: the rendered
|
||||
`cd <arg>` is fed to a real shell and must land in the created directory.
|
||||
"""
|
||||
result = _init(tmp_path, name)
|
||||
assert result.exit_code == 0, _strip(result.stdout)
|
||||
target = tmp_path / name
|
||||
assert target.is_dir()
|
||||
|
||||
printed = _cd_argument(result.stdout)
|
||||
proc = subprocess.run(
|
||||
["bash", "-c", f"cd {printed} && pwd"],
|
||||
cwd=tmp_path,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
assert proc.returncode == 0, f"cd {printed!r} failed: {proc.stderr}"
|
||||
assert Path(proc.stdout.strip()).name == name, proc.stdout
|
||||
|
||||
|
||||
def test_shell_quote_arg_is_host_appropriate():
|
||||
"""The helper follows `_version._render_argv`: list2cmdline on Windows,
|
||||
shlex.quote elsewhere. Names needing no quoting round-trip unchanged."""
|
||||
assert _shell_quote_arg("my-project") == "my-project"
|
||||
quoted = _shell_quote_arg("my project")
|
||||
assert quoted != "my project"
|
||||
if os.name == "nt":
|
||||
assert quoted == '"my project"'
|
||||
else:
|
||||
assert quoted == "'my project'"
|
||||
Reference in New Issue
Block a user