fix(bundle): escape Rich markup in bundle CLI error and status output

`specify bundle`'s `_fail` helper interpolated its message straight into
`err_console.print`, which has Rich markup enabled. Every caller passes
`str(exc)` from a `BundlerError`, and those messages embed untrusted data
-- including the command's own argument -- so a `[...]` in it was parsed
as a style tag.

Balanced tags were silently swallowed; an unbalanced closer raised
`MarkupError`, which replaced the error message with a traceback and left
the output completely empty. Three commands crashed on user input alone,
with no project state required:

    specify bundle catalog add 'ssh://ex[/red]ample.com/c.json'
    specify bundle catalog remove 'no[/red]such'
    specify bundle update 'no[/red]such'

`bundle validate` had the same failure on both branches: its errors echo
`requires.speckit_version`, and its warnings echo component ids, which are
not charset-validated -- so a structurally *valid* manifest crashed on the
success path too.

Fixed centrally in `_fail`, plus the remaining raw interpolations: the
`validate` warning/error/success lines, the install overlap and plan
warnings, the install/update/remove/catalog-add confirmations, the
`catalog list` id/url, and the `bundle init` project path.

Regression tests cover the four crashing error paths (parametrized) and
both `validate` branches; all six fail without this change.

Assisted-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Noor-ul-ain001
2026-08-09 11:17:51 +05:00
parent 684b3d8e05
commit 1d2184d9a2
2 changed files with 86 additions and 16 deletions
+34 -16
View File
@@ -45,7 +45,12 @@ def _fail(message: str) -> None:
"""Print an actionable error to stderr and exit non-zero."""
# Use the stderr console so the error never lands on stdout, which under
# ``--json`` carries the machine-readable payload and must stay parseable.
err_console.print(f"[red]Error:[/red] {message}", style=None)
# Escape the message: every caller passes ``str(exc)`` from a BundlerError
# that interpolates untrusted data (a CLI argument, a catalog url, a
# bundle.yml field), so a '[...]' in it would be parsed as a Rich style tag
# -- silently swallowing the text, or raising MarkupError on an unbalanced
# closer and replacing the whole message with a traceback.
err_console.print(f"[red]Error:[/red] {_escape_markup(message)}", style=None)
raise typer.Exit(code=1)
@@ -394,13 +399,13 @@ def bundle_install(
)
console.print(
f"[cyan]No Spec Kit project here; initializing with integration "
f"'{init_integration}'…[/cyan]"
f"'{_escape_markup(str(init_integration))}'…[/cyan]"
)
_run_init(init_integration, script_type=_default_script_type(), offline=offline)
project_root = require_project_root()
for overlap in _bundle_overlaps(project_root, manifest, offline=offline):
console.print(f"[yellow]![/yellow] {overlap}")
console.print(f"[yellow]![/yellow] {_escape_markup(str(overlap))}")
# For an already-initialized project, the project's recorded active
# integration is authoritative — an explicit --integration must not be
@@ -415,7 +420,7 @@ def bundle_install(
integration_explicit=bool(integration) and detected is None,
)
for warning in plan.warnings:
console.print(f"[yellow]![/yellow] {warning}")
console.print(f"[yellow]![/yellow] {_escape_markup(str(warning))}")
result = install_bundle(
project_root,
@@ -428,7 +433,7 @@ def bundle_install(
return
console.print(
f"[green]✓[/green] Installed '{result.bundle_id}' "
f"[green]✓[/green] Installed '{_escape_markup(str(result.bundle_id))}' "
f"({len(result.installed)} added, {len(result.skipped)} already present)."
)
@@ -480,7 +485,10 @@ def bundle_update(
integration_explicit=bool(integration) and detected is None,
)
install_bundle(project_root, plan, installer, manifest=manifest, refresh=True)
console.print(f"[green]✓[/green] Updated '{target}' to v{plan.version}.")
console.print(
f"[green]✓[/green] Updated '{_escape_markup(str(target))}' "
f"to v{_escape_markup(str(plan.version))}."
)
except BundlerError as exc:
_fail(str(exc))
return
@@ -502,7 +510,7 @@ def bundle_remove(
return
console.print(
f"[green]✓[/green] Removed '{result.bundle_id}' "
f"[green]✓[/green] Removed '{_escape_markup(str(result.bundle_id))}' "
f"({len(result.uninstalled)} uninstalled, {len(result.skipped)} kept for other bundles)."
)
@@ -542,13 +550,16 @@ def bundle_validate(
return
for warning in report.warnings:
console.print(f"[yellow]![/yellow] {warning}")
console.print(f"[yellow]![/yellow] {_escape_markup(str(warning))}")
if not report.ok:
console.print("[red]Manifest is invalid:[/red]")
for error in report.errors:
console.print(f" [red]-[/red] {error}")
console.print(f" [red]-[/red] {_escape_markup(str(error))}")
raise typer.Exit(code=1)
console.print(f"[green]✓[/green] {manifest.bundle.id} is well-formed and valid.")
console.print(
f"[green]✓[/green] {_escape_markup(str(manifest.bundle.id))} "
"is well-formed and valid."
)
@bundle_app.command("build")
@@ -591,7 +602,7 @@ def bundle_init(
init_integration = _resolve_init_integration(integration, None)
console.print(
f"[cyan]Initializing a Spec Kit project with integration "
f"'{init_integration}'…[/cyan]"
f"'{_escape_markup(str(init_integration))}'…[/cyan]"
)
_run_init(init_integration, script_type=_default_script_type(), offline=offline)
project_root = require_project_root()
@@ -599,7 +610,10 @@ def bundle_init(
_fail(str(exc))
return
console.print(f"[green]✓[/green] Spec Kit project ready at {project_root}.")
console.print(
f"[green]✓[/green] Spec Kit project ready at "
f"{_escape_markup(str(project_root))}."
)
if bundle:
bundle_install(bundle, integration=integration, offline=offline)
@@ -623,10 +637,11 @@ def catalog_list() -> None:
only_builtin = all(s.scope == Scope.BUILTIN for s in sources)
for source in sources:
console.print(
f" [bold]{source.id}[/bold] priority={source.priority} "
f" [bold]{_escape_markup(str(source.id))}[/bold] "
f"priority={source.priority} "
f"policy={source.install_policy.value} scope={source.scope.value}"
)
console.print(f" [dim]{source.url}[/dim]")
console.print(f" [dim]{_escape_markup(str(source.url))}[/dim]")
if only_builtin:
console.print("\n[dim]Using the built-in default stack.[/dim]")
@@ -651,7 +666,7 @@ def catalog_add(
return
console.print(
f"[green]✓[/green] Added catalog '{source.id}' "
f"[green]✓[/green] Added catalog '{_escape_markup(str(source.id))}' "
f"(priority {source.priority}, {source.install_policy.value})."
)
@@ -670,7 +685,10 @@ def catalog_remove(
_fail(str(exc))
return
console.print(f"[green]✓[/green] Removed catalog source '{removed}'.")
console.print(
f"[green]✓[/green] Removed catalog source "
f"'{_escape_markup(str(removed))}'."
)
# ZIP magic-byte signatures used to detect .zip payloads from REST API asset
+52
View File
@@ -217,6 +217,31 @@ def test_catalog_remove_builtin_is_refused(project: Path):
assert "built-in" in result.output
# Every ``bundle`` error path funnels through ``_fail(str(exc))``, and the
# BundlerError messages interpolate untrusted data -- including the command's
# own argument. An unbalanced closer used to raise MarkupError instead of the
# error, leaving the user with a traceback and no message at all.
@pytest.mark.parametrize(
"argv, expected",
[
(
["bundle", "catalog", "add", "ssh://ex[/red]ample.com/c.json"],
"ssh://ex[/red]ample.com/c.json",
),
(["bundle", "catalog", "remove", "no[/red]such"], "no[/red]such"),
(["bundle", "update", "no[/red]such"], "no[/red]such"),
(["bundle", "remove", "no[/red]such"], "no[/red]such"),
],
)
def test_error_paths_escape_rich_markup(project: Path, argv: list, expected: str):
result = runner.invoke(app, argv)
assert result.exit_code == 1
# A MarkupError would surface here as an exception rather than a clean exit.
assert isinstance(result.exception, SystemExit)
assert expected in strip_ansi(result.output)
def test_validate_reports_invalid_manifest(project: Path):
data = valid_manifest_dict()
del data["bundle"]["license"]
@@ -237,6 +262,33 @@ def test_validate_accepts_valid_manifest(project: Path):
assert "valid" in result.output
def test_validate_escapes_manifest_markup_in_errors(project: Path):
data = valid_manifest_dict()
# An invalid constraint is echoed back inside the validation error.
data["requires"] = {"speckit_version": ">=1.0[/bold]"}
(project / "bundle.yml").write_text(yaml.safe_dump(data), encoding="utf-8")
result = runner.invoke(app, ["bundle", "validate", "--offline"])
assert result.exit_code == 1
assert isinstance(result.exception, SystemExit)
assert ">=1.0[/bold]" in strip_ansi(result.output)
def test_validate_escapes_manifest_markup_in_warnings(project: Path):
data = valid_manifest_dict()
# Step ids are not charset-validated, and the unresolved-reference warning
# echoes them -- so an otherwise *valid* manifest crashed just as readily as
# an invalid one, on the success path.
data["provides"]["steps"] = [{"id": "step[/bold]a"}]
(project / "bundle.yml").write_text(yaml.safe_dump(data), encoding="utf-8")
result = runner.invoke(app, ["bundle", "validate", "--offline"])
assert result.exit_code == 0, repr(result.exception)
assert "step[/bold]a" in strip_ansi(result.output)
def test_validate_rejects_broken_reference(project: Path):
# Synthetic component ids resolve to nothing in any catalog → hard failure.
(project / "bundle.yml").write_text(