Skip to content

Commit eb3d72c

Browse files
committed
Prevent Rich markup injection in extension CLI output
User-controlled catalog URLs and extension IDs are rendered through Rich-enabled console paths, so every remaining output-only interpolation now escapes markup while leaving stored values and filesystem behavior unchanged. Regression tests cover catalog add, install hints, remove hints, and state command messages with bracketed markup-like values.
1 parent d6ea392 commit eb3d72c

2 files changed

Lines changed: 210 additions & 10 deletions

File tree

src/specify_cli/extensions/_commands.py

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,7 @@ def catalog_add(
330330
raise typer.Exit(1)
331331

332332
safe_name = _escape_markup(name)
333+
safe_url = _escape_markup(url)
333334

334335
# Check for duplicate name
335336
for existing in catalogs:
@@ -351,7 +352,7 @@ def catalog_add(
351352

352353
install_label = "install allowed" if install_allowed else "discovery only"
353354
console.print(f"\n[green]✓[/green] Added catalog '[bold]{safe_name}[/bold]' ({install_label})")
354-
console.print(f" URL: {url}")
355+
console.print(f" URL: {safe_url}")
355356
console.print(f" Priority: {priority}")
356357
console.print(f"\nConfig saved to {_display_project_path(project_root, config_path)}")
357358

@@ -611,7 +612,7 @@ def extension_add(
611612
console.print(f"\n[green]✓[/green] {len(reg_skills)} agent skill(s) auto-registered")
612613

613614
console.print("\n[yellow]⚠[/yellow] Configuration may be required")
614-
console.print(f" Check: .specify/extensions/{manifest.id}/")
615+
console.print(f" Check: .specify/extensions/{_escape_markup(str(manifest.id))}/")
615616

616617
except ValidationError as e:
617618
console.print(f"\n[red]Validation Error:[/red] {e}")
@@ -639,6 +640,7 @@ def extension_remove(
639640
# Resolve extension ID from argument (handles ambiguous names)
640641
installed = manager.list_installed()
641642
extension_id, display_name = _resolve_installed_extension(extension, installed, "remove")
643+
safe_extension_id = _escape_markup(str(extension_id))
642644

643645
# Get extension info for command and skill counts
644646
ext_manifest = manager.get_extension(extension_id)
@@ -666,7 +668,7 @@ def extension_remove(
666668
console.print(f" • {cmd_count} command{'s' if cmd_count != 1 else ''} per agent")
667669
if skill_count:
668670
console.print(f" • {skill_count} agent skill(s)")
669-
console.print(f" • Extension directory: .specify/extensions/{extension_id}/")
671+
console.print(f" • Extension directory: .specify/extensions/{safe_extension_id}/")
670672
if not keep_config:
671673
console.print(" • Config files (will be backed up)")
672674
console.print()
@@ -682,10 +684,10 @@ def extension_remove(
682684
if success:
683685
console.print(f"\n[green]✓[/green] Extension '{_escape_markup(str(display_name))}' removed successfully")
684686
if keep_config:
685-
console.print(f"\nConfig files preserved in .specify/extensions/{extension_id}/")
687+
console.print(f"\nConfig files preserved in .specify/extensions/{safe_extension_id}/")
686688
else:
687-
console.print(f"\nConfig files backed up to .specify/extensions/.backup/{extension_id}/")
688-
console.print(f"\nTo reinstall: specify extension add {extension_id}")
689+
console.print(f"\nConfig files backed up to .specify/extensions/.backup/{safe_extension_id}/")
690+
console.print(f"\nTo reinstall: specify extension add {safe_extension_id}")
689691
else:
690692
console.print("[red]Error:[/red] Failed to remove extension")
691693
raise typer.Exit(1)
@@ -1419,7 +1421,10 @@ def extension_enable(
14191421
# Update registry
14201422
metadata = manager.registry.get(extension_id)
14211423
if metadata is None or not isinstance(metadata, dict):
1422-
console.print(f"[red]Error:[/red] Extension '{extension_id}' not found in registry (corrupted state)")
1424+
console.print(
1425+
f"[red]Error:[/red] Extension '{_escape_markup(str(extension_id))}' "
1426+
"not found in registry (corrupted state)"
1427+
)
14231428
raise typer.Exit(1)
14241429

14251430
if metadata.get("enabled", True):
@@ -1458,7 +1463,10 @@ def extension_disable(
14581463
# Update registry
14591464
metadata = manager.registry.get(extension_id)
14601465
if metadata is None or not isinstance(metadata, dict):
1461-
console.print(f"[red]Error:[/red] Extension '{extension_id}' not found in registry (corrupted state)")
1466+
console.print(
1467+
f"[red]Error:[/red] Extension '{_escape_markup(str(extension_id))}' "
1468+
"not found in registry (corrupted state)"
1469+
)
14621470
raise typer.Exit(1)
14631471

14641472
if not metadata.get("enabled", True):
@@ -1478,7 +1486,7 @@ def extension_disable(
14781486

14791487
console.print(f"[green]✓[/green] Extension '{_escape_markup(str(display_name))}' disabled")
14801488
console.print("\nCommands will no longer be available. Hooks will not execute.")
1481-
console.print(f"To re-enable: specify extension enable {extension_id}")
1489+
console.print(f"To re-enable: specify extension enable {_escape_markup(str(extension_id))}")
14821490

14831491

14841492
@extension_app.command("set-priority")
@@ -1504,7 +1512,10 @@ def extension_set_priority(
15041512
# Get current metadata
15051513
metadata = manager.registry.get(extension_id)
15061514
if metadata is None or not isinstance(metadata, dict):
1507-
console.print(f"[red]Error:[/red] Extension '{extension_id}' not found in registry (corrupted state)")
1515+
console.print(
1516+
f"[red]Error:[/red] Extension '{_escape_markup(str(extension_id))}' "
1517+
"not found in registry (corrupted state)"
1518+
)
15081519
raise typer.Exit(1)
15091520

15101521
from . import normalize_priority

tests/test_extensions.py

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4670,6 +4670,36 @@ def test_extensionignore_negation_pattern(self, temp_dir, valid_manifest_data):
46704670
class TestExtensionAddCLI:
46714671
"""CLI integration tests for extension add command."""
46724672

4673+
def test_catalog_add_escapes_url_markup(self, tmp_path):
4674+
"""Catalog add should render user-supplied URLs literally."""
4675+
from typer.testing import CliRunner
4676+
from unittest.mock import patch
4677+
from specify_cli import app
4678+
4679+
project_dir = tmp_path / "test-project"
4680+
project_dir.mkdir()
4681+
(project_dir / ".specify").mkdir()
4682+
4683+
url = "https://example.com/[red]catalog[/red].json"
4684+
4685+
runner = CliRunner()
4686+
with patch.object(Path, "cwd", return_value=project_dir):
4687+
result = runner.invoke(
4688+
app,
4689+
[
4690+
"extension",
4691+
"catalog",
4692+
"add",
4693+
url,
4694+
"--name",
4695+
"community",
4696+
],
4697+
catch_exceptions=True,
4698+
)
4699+
4700+
assert result.exit_code == 0, result.output
4701+
assert f"URL: {url}" in result.output
4702+
46734703
def test_add_dev_links_copilot_agent_when_supported(
46744704
self, extension_dir, project_dir, temp_dir
46754705
):
@@ -4915,6 +4945,53 @@ def record_status(message, *args, **kwargs):
49154945
f"[cyan]Installing extension: {escape_markup(extension_name)}[/cyan]"
49164946
]
49174947

4948+
def test_add_post_install_hint_escapes_manifest_id_markup(self, tmp_path):
4949+
"""Extension IDs printed in Rich-rendered hints must stay literal."""
4950+
import io
4951+
from types import SimpleNamespace
4952+
from typer.testing import CliRunner
4953+
from unittest.mock import patch
4954+
from specify_cli import app
4955+
4956+
class FakeResponse(io.BytesIO):
4957+
def __enter__(self):
4958+
return self
4959+
4960+
def __exit__(self, exc_type, exc, tb):
4961+
return False
4962+
4963+
project_dir = tmp_path / "test-project"
4964+
project_dir.mkdir()
4965+
(project_dir / ".specify").mkdir()
4966+
4967+
manifest_id = "[red]bad[/red]"
4968+
4969+
def fake_install_from_zip(self_obj, zip_path, speckit_version, priority=10, force=False):
4970+
return SimpleNamespace(
4971+
id=manifest_id,
4972+
name="Bad Extension",
4973+
version="1.0.0",
4974+
description="Test extension",
4975+
warnings=[],
4976+
commands=[],
4977+
hooks=[],
4978+
)
4979+
4980+
runner = CliRunner()
4981+
with patch.object(Path, "cwd", return_value=project_dir), \
4982+
patch("typer.confirm", return_value=True), \
4983+
patch("specify_cli.authentication.http.open_url", return_value=FakeResponse(b"zip-bytes")), \
4984+
patch.object(ExtensionManager, "install_from_zip", fake_install_from_zip), \
4985+
patch.object(ExtensionRegistry, "get", return_value={}):
4986+
result = runner.invoke(
4987+
app,
4988+
["extension", "add", "bad", "--from", "https://example.com/ext.zip"],
4989+
catch_exceptions=True,
4990+
)
4991+
4992+
assert result.exit_code == 0, result.output
4993+
assert ".specify/extensions/[red]bad[/red]/" in result.output
4994+
49184995
def test_add_from_url_cancel_exits_cleanly(self, tmp_path):
49194996
"""Declining the --from <url> confirmation should exit with code 0."""
49204997
from typer.testing import CliRunner
@@ -6329,6 +6406,118 @@ def test_remove_confirmation_plural_commands(self, tmp_path, extension_dir):
63296406

63306407
assert "2 commands" in result.output
63316408

6409+
def test_remove_output_escapes_extension_id_markup(self, tmp_path):
6410+
"""Removal paths and reinstall hints must not parse extension IDs as markup."""
6411+
from types import SimpleNamespace
6412+
from typer.testing import CliRunner
6413+
from unittest.mock import patch
6414+
from specify_cli import app
6415+
6416+
project_dir = tmp_path / "project"
6417+
project_dir.mkdir()
6418+
(project_dir / ".specify").mkdir()
6419+
6420+
extension_id = "[red]bad[/red]"
6421+
installed = [
6422+
{
6423+
"id": extension_id,
6424+
"name": "Bad Extension",
6425+
"version": "1.0.0",
6426+
"description": "Test extension",
6427+
"enabled": True,
6428+
}
6429+
]
6430+
6431+
runner = CliRunner()
6432+
with patch.object(Path, "cwd", return_value=project_dir), \
6433+
patch.object(ExtensionManager, "list_installed", return_value=installed), \
6434+
patch.object(ExtensionManager, "get_extension", return_value=SimpleNamespace(commands=[])), \
6435+
patch.object(ExtensionRegistry, "get", return_value={"registered_commands": {}, "registered_skills": []}), \
6436+
patch.object(ExtensionManager, "remove", return_value=True):
6437+
result = runner.invoke(
6438+
app,
6439+
["extension", "remove", extension_id, "--force"],
6440+
catch_exceptions=True,
6441+
)
6442+
6443+
assert result.exit_code == 0, result.output
6444+
assert ".specify/extensions/.backup/[red]bad[/red]/" in result.output
6445+
assert "specify extension add [red]bad[/red]" in result.output
6446+
6447+
6448+
class TestExtensionStateCLI:
6449+
"""CLI tests for installed extension state commands."""
6450+
6451+
def test_enable_registry_error_escapes_extension_id_markup(self, tmp_path):
6452+
"""Registry-corruption errors should render extension IDs literally."""
6453+
from typer.testing import CliRunner
6454+
from unittest.mock import patch
6455+
from specify_cli import app
6456+
6457+
project_dir = tmp_path / "project"
6458+
project_dir.mkdir()
6459+
(project_dir / ".specify").mkdir()
6460+
6461+
extension_id = "[red]bad[/red]"
6462+
installed = [
6463+
{
6464+
"id": extension_id,
6465+
"name": "Bad Extension",
6466+
"version": "1.0.0",
6467+
"description": "Test extension",
6468+
"enabled": False,
6469+
}
6470+
]
6471+
6472+
runner = CliRunner()
6473+
with patch.object(Path, "cwd", return_value=project_dir), \
6474+
patch.object(ExtensionManager, "list_installed", return_value=installed), \
6475+
patch.object(ExtensionRegistry, "get", return_value=None):
6476+
result = runner.invoke(
6477+
app,
6478+
["extension", "enable", extension_id],
6479+
catch_exceptions=True,
6480+
)
6481+
6482+
assert result.exit_code == 1, result.output
6483+
assert "Extension '[red]bad[/red]' not found in registry" in result.output
6484+
6485+
def test_disable_reenable_hint_escapes_extension_id_markup(self, tmp_path):
6486+
"""Disable success hints should not parse extension IDs as markup."""
6487+
from typer.testing import CliRunner
6488+
from unittest.mock import patch
6489+
from specify_cli import app
6490+
6491+
project_dir = tmp_path / "project"
6492+
project_dir.mkdir()
6493+
(project_dir / ".specify").mkdir()
6494+
6495+
extension_id = "[red]bad[/red]"
6496+
installed = [
6497+
{
6498+
"id": extension_id,
6499+
"name": "Bad Extension",
6500+
"version": "1.0.0",
6501+
"description": "Test extension",
6502+
"enabled": True,
6503+
}
6504+
]
6505+
6506+
runner = CliRunner()
6507+
with patch.object(Path, "cwd", return_value=project_dir), \
6508+
patch.object(ExtensionManager, "list_installed", return_value=installed), \
6509+
patch.object(ExtensionRegistry, "get", return_value={"enabled": True}), \
6510+
patch.object(ExtensionRegistry, "update", return_value=None), \
6511+
patch.object(HookExecutor, "get_project_config", return_value={}):
6512+
result = runner.invoke(
6513+
app,
6514+
["extension", "disable", extension_id],
6515+
catch_exceptions=True,
6516+
)
6517+
6518+
assert result.exit_code == 0, result.output
6519+
assert "specify extension enable [red]bad[/red]" in result.output
6520+
63326521

63336522
class TestClineExtensionHyphenation:
63346523
"""Test that Cline integration uses hyphenated commands and frontmatter references."""

0 commit comments

Comments
 (0)