mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix(integrations): escape Rich markup in --integration-options error messages (#3458)
* fix(integrations): exit cleanly on malformed --integration-options quoting
_parse_integration_options called shlex.split(raw_options) unguarded. an
unbalanced quote (e.g. --integration-options='--commands-dir "foo') makes
shlex raise ValueError('No closing quotation'), so a raw traceback escaped
instead of the typer.Exit(1) error every other bad-input path in this function
produces. reachable from specify init and every integration install/switch/
upgrade/migrate that accepts --integration-options.
wrap the split and convert ValueError into the same clean CLI error. added a
regression test; confirmed it fails on the pre-fix code (raw ValueError).
* escape user-controlled values in integration-options error messages
the malformed-quoting handler (and the unexpected/unknown option
branches) interpolate raw_options/token into console.print. a value
carrying an unbalanced rich tag like '--commands-dir "[/red]foo' first
trips the intended shlex ValueError, but the error print then raises
rich.errors.MarkupError and leaks a traceback anyway. escape all three
before printing so the clean typer.Exit survives.
added a regression covering both the shlex path and a bare markup token.
* address review: drop redundant, mis-described markup test
The removed test's docstring claimed the shlex failure branch
interpolates raw_options into console.print; it only prints {exc},
which never contains the caller's markup. Its two assertions also
duplicated test_bad_option_token_with_rich_markup_exits_cleanly (the
'[/red]foo' case) and the shlex-path case already covered by
test_unbalanced_quote_exits_cleanly. The real change here remains the
escape() of the two user-controlled token prints.
This commit is contained in:
@@ -6,6 +6,7 @@ from pathlib import Path
|
||||
from typing import Any, Callable
|
||||
|
||||
import typer
|
||||
from rich.markup import escape
|
||||
|
||||
from .._agent_config import SCRIPT_TYPE_CHOICES
|
||||
from .._console import console
|
||||
@@ -206,7 +207,7 @@ def _parse_integration_options(integration: Any, raw_options: str) -> dict[str,
|
||||
while i < len(tokens):
|
||||
token = tokens[i]
|
||||
if not token.startswith("-"):
|
||||
console.print(f"[red]Error:[/red] Unexpected integration option value '{token}'.")
|
||||
console.print(f"[red]Error:[/red] Unexpected integration option value '{escape(token)}'.")
|
||||
if allowed:
|
||||
console.print(f"Allowed options: {allowed}")
|
||||
raise typer.Exit(1)
|
||||
@@ -217,7 +218,7 @@ def _parse_integration_options(integration: Any, raw_options: str) -> dict[str,
|
||||
name, value = name.split("=", 1)
|
||||
opt = declared.get(name)
|
||||
if not opt:
|
||||
console.print(f"[red]Error:[/red] Unknown integration option '{token}'.")
|
||||
console.print(f"[red]Error:[/red] Unknown integration option '{escape(token)}'.")
|
||||
if allowed:
|
||||
console.print(f"Allowed options: {allowed}")
|
||||
raise typer.Exit(1)
|
||||
|
||||
@@ -3119,6 +3119,30 @@ class TestParseIntegrationOptionsEqualsForm:
|
||||
assert excinfo.value.exit_code == 1
|
||||
assert "Error: Could not parse integration options: No closing quotation." in capsys.readouterr().out
|
||||
|
||||
def test_bad_option_token_with_rich_markup_exits_cleanly(self):
|
||||
"""A bad option token carrying Rich markup must exit cleanly, not crash.
|
||||
|
||||
The token is user-controlled and gets interpolated into console.print.
|
||||
A value like '[/red]foo' parses fine through shlex but is an unexpected
|
||||
value / unknown option — and an unbalanced Rich tag would raise
|
||||
rich.errors.MarkupError inside console.print, leaking a traceback
|
||||
instead of the intended typer.Exit(1). The token must be escaped."""
|
||||
import typer
|
||||
|
||||
from specify_cli.integrations._commands import _parse_integration_options
|
||||
from specify_cli.integrations import get_integration
|
||||
|
||||
integration = get_integration("generic")
|
||||
assert integration is not None
|
||||
|
||||
# Unexpected value token carrying markup.
|
||||
with pytest.raises(typer.Exit):
|
||||
_parse_integration_options(integration, "[/red]foo")
|
||||
|
||||
# Unknown option token carrying markup.
|
||||
with pytest.raises(typer.Exit):
|
||||
_parse_integration_options(integration, "--[/red]bad")
|
||||
|
||||
|
||||
class TestUninstallNoManifestClearsInitOptions:
|
||||
def test_init_options_cleared_on_no_manifest_uninstall(self, tmp_path):
|
||||
|
||||
Reference in New Issue
Block a user