Skip to content

Commit 8b5f516

Browse files
digimangosCopilot
andcommitted
fix(preset): quote Windows retry commands
Render Windows recovery commands explicitly for PowerShell, using literal argument quoting so shell metacharacters remain part of URLs and paths. Add regression coverage against a real PowerShell parser when available. Assisted-by: GitHub Copilot (model: HydraFusion, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 42a60d3 commit 8b5f516

4 files changed

Lines changed: 88 additions & 15 deletions

File tree

‎docs/reference/presets.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ higher, and the preset must already be installed. Beyond those checks the
4949
replacement source itself is not inspected in advance. If removal succeeds but
5050
replacement installation fails, the previous preset has already been removed.
5151
The command reports a copy-pastable `specify preset add` retry command,
52-
including the replacement source and priority. There is no source pre-flight,
52+
including the replacement source and priority. On Windows, the reported command
53+
is explicitly formatted for PowerShell. There is no source pre-flight,
5354
version comparison, manifest diff, staging, rollback, automatic repair, or
5455
recovery transaction. A missing or invalid replacement source can therefore
5556
leave the preset removed. With

‎src/specify_cli/presets/_commands.py‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,19 @@
5454
MINIMUM_PRESET_PRIORITY = 1
5555

5656

57+
def _render_powershell_argv(argv: list[str]) -> str:
58+
"""Render argv as a copy-pastable PowerShell command.
59+
60+
PowerShell single-quoted strings are literal except that an embedded single
61+
quote is escaped by doubling it. The call operator is required because the
62+
executable name is quoted too.
63+
"""
64+
def quote_arg(arg: str) -> str:
65+
return "'" + arg.replace("'", "''") + "'"
66+
67+
return "& " + " ".join(quote_arg(arg) for arg in argv)
68+
69+
5770
def _validate_priority(priority: int) -> None:
5871
"""Reject a non-positive priority before any destructive work begins.
5972
@@ -537,18 +550,17 @@ def preset_update(
537550
retry_args.extend([preset_id, *retry_options])
538551

539552
def report_add_failure() -> None:
540-
import subprocess
541-
542-
rendered_args = (
543-
subprocess.list2cmdline(retry_args)
544-
if os.name == "nt"
545-
else shlex.join(retry_args)
546-
)
553+
if os.name == "nt":
554+
retry_label = "Retry in PowerShell: "
555+
rendered_args = _render_powershell_argv(retry_args)
556+
else:
557+
retry_label = "Retry with: "
558+
rendered_args = shlex.join(retry_args)
547559
console.print(
548560
"[red]Error:[/red] Preset update failed; the previous preset was removed."
549561
)
550562
console.print(
551-
"Retry with: [cyan]"
563+
f"{retry_label}[cyan]"
552564
f"{_escape_markup(rendered_args)}"
553565
"[/cyan]",
554566
soft_wrap=True,

‎tests/integration/test_preset_update_workflow.py‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
import os
1313
import shlex
14-
import subprocess
1514
from pathlib import Path
1615

1716
import pytest
@@ -20,6 +19,7 @@
2019

2120
from specify_cli import app
2221
from specify_cli.presets import PresetManager
22+
from specify_cli.presets._commands import _render_powershell_argv
2323
from tests.conftest import strip_ansi
2424

2525
PRESET_ID = "update-workflow-demo"
@@ -88,23 +88,24 @@ def write_preset(
8888
def render_command(args: list[str]) -> str:
8989
"""Render *args* the way ``preset update`` renders its retry command."""
9090
if os.name == "nt":
91-
return subprocess.list2cmdline(args)
91+
return _render_powershell_argv(args)
9292
return shlex.join(args)
9393

9494

9595
def parse_command(rendered: str) -> list[str]:
9696
"""Inverse of :func:`render_command` for the simple args used here."""
9797
if os.name == "nt":
98-
return [token.strip('"') for token in shlex.split(rendered, posix=False)]
98+
return shlex.split(rendered)[1:]
9999
return shlex.split(rendered)
100100

101101

102102
def retry_command_from(output: str) -> str:
103103
"""Extract the retry command printed after a failed update."""
104+
prefix = "Retry in PowerShell: " if os.name == "nt" else "Retry with: "
104105
for line in strip_ansi(output).splitlines():
105106
stripped = line.strip()
106-
if stripped.startswith("Retry with: "):
107-
return stripped[len("Retry with: ") :].strip()
107+
if stripped.startswith(prefix):
108+
return stripped[len(prefix) :].strip()
108109
raise AssertionError(f"No retry command found in output:\n{output}")
109110

110111

‎tests/test_presets.py‎

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import os
1717
import shlex
1818
import subprocess
19+
import sys
1920
import tempfile
2021
import tarfile
2122
import shutil
@@ -46,6 +47,7 @@
4647
from specify_cli.extensions import ExtensionRegistry
4748
from specify_cli._console import console
4849
from specify_cli.presets._commands import (
50+
_render_powershell_argv,
4951
_warn_unmet_extension_dependencies,
5052
preset_update,
5153
)
@@ -11650,12 +11652,69 @@ def fail_add(**_kwargs):
1165011652
"6",
1165111653
]
1165211654
expected = (
11653-
subprocess.list2cmdline(retry_args)
11655+
_render_powershell_argv(retry_args)
1165411656
if os.name == "nt"
1165511657
else shlex.join(retry_args)
1165611658
)
1165711659
assert expected in output
1165811660

11661+
def test_retry_command_quotes_powershell_metacharacters(
11662+
self, project_dir, monkeypatch, capsys
11663+
):
11664+
"""Windows retry commands keep PowerShell metacharacters literal."""
11665+
commands = self._manager(monkeypatch, project_dir)
11666+
monkeypatch.setattr(commands, "preset_remove", lambda _preset_id: None)
11667+
11668+
def fail_add(**_kwargs):
11669+
raise typer.Exit(1)
11670+
11671+
monkeypatch.setattr(commands, "preset_add", fail_add)
11672+
monkeypatch.setattr(os, "name", "nt")
11673+
11674+
with pytest.raises(typer.Exit) as exc_info:
11675+
preset_update(
11676+
"test-pack",
11677+
from_url=None,
11678+
dev=r"C:\replacement&$backup's presets",
11679+
priority=6,
11680+
)
11681+
11682+
assert exc_info.value.exit_code == 1
11683+
output = strip_ansi(capsys.readouterr().out)
11684+
expected = (
11685+
"& 'specify' 'preset' 'add' 'test-pack' '--dev' "
11686+
"'C:\\replacement&$backup''s presets' '--priority' '6'"
11687+
)
11688+
assert "Retry in PowerShell: " in output
11689+
assert expected in output
11690+
11691+
def test_powershell_retry_renderer_preserves_literal_arguments(self):
11692+
"""The rendered command survives parsing by a real PowerShell."""
11693+
powershell = shutil.which("pwsh") or shutil.which("powershell")
11694+
if powershell is None:
11695+
pytest.skip("PowerShell is not available")
11696+
11697+
arguments = [
11698+
"https://example.com/archive.zip?one=1&two=$value",
11699+
r"C:\owner's presets",
11700+
]
11701+
rendered = _render_powershell_argv(
11702+
[
11703+
sys.executable,
11704+
"-c",
11705+
"import json,sys; print(json.dumps(sys.argv[1:]))",
11706+
*arguments,
11707+
]
11708+
)
11709+
result = subprocess.run(
11710+
[powershell, "-NoProfile", "-Command", rendered],
11711+
check=True,
11712+
capture_output=True,
11713+
text=True,
11714+
)
11715+
11716+
assert json.loads(result.stdout) == arguments
11717+
1165911718
def test_invalid_priority_rejected_before_removal(
1166011719
self, project_dir, monkeypatch, capsys
1166111720
):

0 commit comments

Comments
 (0)