diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index 0d96e2b6c5..e15dc2f3c2 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -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 ( @@ -30,6 +33,25 @@ def _stdin_is_interactive() -> bool: return sys.stdin.isatty() +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: @@ -198,7 +220,10 @@ def init( 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) @@ -275,26 +300,27 @@ def init( 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]", @@ -308,7 +334,7 @@ def init( 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 @@ -346,12 +372,14 @@ def init( 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)) @@ -378,7 +406,7 @@ def init( 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 @@ -671,7 +699,7 @@ def init( 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), @@ -682,7 +710,7 @@ def init( 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: diff --git a/tests/test_init_output_markup.py b/tests/test_init_output_markup.py new file mode 100644 index 0000000000..54576fb33f --- /dev/null +++ b/tests/test_init_output_markup.py @@ -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 `.""" + 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 ` 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'"