Skip to content

Commit 8ed8910

Browse files
committed
fix: harden integration manifest read errors
1 parent e2b36e8 commit 8ed8910

3 files changed

Lines changed: 104 additions & 50 deletions

File tree

src/specify_cli/__init__.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1929,6 +1929,9 @@ def _remove_integration_json(project_root: Path) -> None:
19291929
path.unlink()
19301930

19311931

1932+
_MANIFEST_READ_ERRORS = (ValueError, FileNotFoundError, OSError, UnicodeDecodeError)
1933+
1934+
19321935
def _normalize_script_type(script_type: str, source: str) -> str:
19331936
"""Normalize and validate a script type from CLI/config sources."""
19341937
normalized = script_type.strip().lower()
@@ -2443,7 +2446,7 @@ def integration_uninstall(
24432446

24442447
try:
24452448
manifest = IntegrationManifest.load(key, project_root)
2446-
except (ValueError, FileNotFoundError) as exc:
2449+
except _MANIFEST_READ_ERRORS as exc:
24472450
console.print(f"[red]Error:[/red] Integration manifest for '{key}' is unreadable.")
24482451
console.print(f"Manifest: {manifest_path}")
24492452
console.print(
@@ -2598,7 +2601,7 @@ def integration_switch(
25982601
console.print(f"Uninstalling current integration: [cyan]{installed_key}[/cyan]")
25992602
try:
26002603
old_manifest = IntegrationManifest.load(installed_key, project_root)
2601-
except (ValueError, FileNotFoundError) as exc:
2604+
except _MANIFEST_READ_ERRORS as exc:
26022605
console.print(f"[red]Error:[/red] Could not read integration manifest for '{installed_key}': {manifest_path}")
26032606
console.print(f"[dim]{exc}[/dim]")
26042607
console.print(
@@ -2622,7 +2625,7 @@ def integration_switch(
26222625
console.print(f" Removed {len(removed)} file(s)")
26232626
if skipped:
26242627
console.print(f" [yellow]⚠[/yellow] {len(skipped)} modified file(s) preserved")
2625-
except (ValueError, FileNotFoundError) as exc:
2628+
except _MANIFEST_READ_ERRORS as exc:
26262629
console.print(f"[yellow]Warning:[/yellow] Could not read manifest for '{installed_key}': {exc}")
26272630
else:
26282631
console.print(f"[red]Error:[/red] Integration '{installed_key}' is installed but has no manifest.")
@@ -2789,7 +2792,7 @@ def integration_upgrade(
27892792

27902793
try:
27912794
old_manifest = IntegrationManifest.load(key, project_root)
2792-
except (ValueError, FileNotFoundError) as exc:
2795+
except _MANIFEST_READ_ERRORS as exc:
27932796
console.print(f"[red]Error:[/red] Integration manifest for '{key}' is unreadable: {exc}")
27942797
raise typer.Exit(1)
27952798

tests/integrations/test_integration_subcommand.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ def _init_project(tmp_path, integration="copilot"):
3131
return project
3232

3333

34+
def _write_invalid_manifest(project, key):
35+
manifest = project / ".specify" / "integrations" / f"{key}.manifest.json"
36+
manifest.write_bytes(b"\xff\xfe\x00")
37+
return manifest
38+
39+
3440
# ── list ─────────────────────────────────────────────────────────────
3541

3642

@@ -374,6 +380,20 @@ def test_uninstall_wrong_key(self, tmp_path):
374380
assert result.exit_code != 0
375381
assert "not installed" in result.output
376382

383+
def test_uninstall_invalid_manifest_reports_cli_error(self, tmp_path):
384+
project = _init_project(tmp_path, "claude")
385+
_write_invalid_manifest(project, "claude")
386+
387+
old_cwd = os.getcwd()
388+
try:
389+
os.chdir(project)
390+
result = runner.invoke(app, ["integration", "uninstall", "claude"])
391+
finally:
392+
os.chdir(old_cwd)
393+
assert result.exit_code != 0
394+
assert "manifest" in result.output
395+
assert "unreadable" in result.output
396+
377397
def test_uninstall_non_default_preserves_default(self, tmp_path):
378398
project = _init_project(tmp_path, "claude")
379399
old_cwd = os.getcwd()
@@ -557,6 +577,22 @@ def test_switch_unknown_target(self, tmp_path):
557577
assert result.exit_code != 0
558578
assert "Unknown integration" in result.output
559579

580+
def test_switch_invalid_current_manifest_reports_cli_error(self, tmp_path):
581+
project = _init_project(tmp_path, "claude")
582+
_write_invalid_manifest(project, "claude")
583+
584+
old_cwd = os.getcwd()
585+
try:
586+
os.chdir(project)
587+
result = runner.invoke(app, [
588+
"integration", "switch", "codex",
589+
"--script", "sh",
590+
])
591+
finally:
592+
os.chdir(old_cwd)
593+
assert result.exit_code != 0
594+
assert "Could not read integration manifest" in result.output
595+
560596
def test_switch_same_noop(self, tmp_path):
561597
project = _init_project(tmp_path, "copilot")
562598
old_cwd = os.getcwd()
@@ -710,6 +746,20 @@ def test_failed_switch_keeps_fallback_metadata_consistent(self, tmp_path):
710746

711747

712748
class TestIntegrationUpgrade:
749+
def test_upgrade_invalid_manifest_reports_cli_error(self, tmp_path):
750+
project = _init_project(tmp_path, "claude")
751+
_write_invalid_manifest(project, "claude")
752+
753+
old_cwd = os.getcwd()
754+
try:
755+
os.chdir(project)
756+
result = runner.invoke(app, ["integration", "upgrade", "claude"])
757+
finally:
758+
os.chdir(old_cwd)
759+
assert result.exit_code != 0
760+
assert "manifest" in result.output
761+
assert "unreadable" in result.output
762+
713763
def test_upgrade_non_default_keeps_default_template_invocations(self, tmp_path):
714764
project = _init_project(tmp_path, "gemini")
715765
template = project / ".specify" / "templates" / "plan-template.md"

tests/integrations/test_registry.py

Lines changed: 47 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -237,53 +237,54 @@ def test_safe_integrations_have_disjoint_manifests(
237237
first,
238238
second,
239239
):
240-
project_root = tmp_path / "project"
241-
project_root.mkdir()
242-
runner = CliRunner()
243-
244-
original_cwd = os.getcwd()
245-
try:
246-
os.chdir(project_root)
247-
init_result = runner.invoke(
248-
app,
249-
[
250-
"init",
251-
"--here",
252-
"--integration",
253-
first,
254-
"--script",
255-
"sh",
256-
"--no-git",
257-
"--ignore-agent-tools",
258-
],
259-
catch_exceptions=False,
240+
for initial, additional in ((first, second), (second, first)):
241+
project_root = tmp_path / f"project-{initial}-{additional}"
242+
project_root.mkdir()
243+
runner = CliRunner()
244+
245+
original_cwd = os.getcwd()
246+
try:
247+
os.chdir(project_root)
248+
init_result = runner.invoke(
249+
app,
250+
[
251+
"init",
252+
"--here",
253+
"--integration",
254+
initial,
255+
"--script",
256+
"sh",
257+
"--no-git",
258+
"--ignore-agent-tools",
259+
],
260+
catch_exceptions=False,
261+
)
262+
assert init_result.exit_code == 0, init_result.output
263+
264+
install_result = runner.invoke(
265+
app,
266+
["integration", "install", additional, "--script", "sh"],
267+
catch_exceptions=False,
268+
)
269+
assert install_result.exit_code == 0, install_result.output
270+
finally:
271+
os.chdir(original_cwd)
272+
273+
initial_manifest = json.loads(
274+
(
275+
project_root / ".specify" / "integrations" / f"{initial}.manifest.json"
276+
).read_text(encoding="utf-8")
260277
)
261-
assert init_result.exit_code == 0, init_result.output
262-
263-
install_result = runner.invoke(
264-
app,
265-
["integration", "install", second, "--script", "sh"],
266-
catch_exceptions=False,
278+
additional_manifest = json.loads(
279+
(
280+
project_root / ".specify" / "integrations" / f"{additional}.manifest.json"
281+
).read_text(encoding="utf-8")
267282
)
268-
assert install_result.exit_code == 0, install_result.output
269-
finally:
270-
os.chdir(original_cwd)
271283

272-
first_manifest = json.loads(
273-
(
274-
project_root / ".specify" / "integrations" / f"{first}.manifest.json"
275-
).read_text(encoding="utf-8")
276-
)
277-
second_manifest = json.loads(
278-
(
279-
project_root / ".specify" / "integrations" / f"{second}.manifest.json"
280-
).read_text(encoding="utf-8")
281-
)
282-
283-
first_files = set(first_manifest.get("files", {}))
284-
second_files = set(second_manifest.get("files", {}))
284+
initial_files = set(initial_manifest.get("files", {}))
285+
additional_files = set(additional_manifest.get("files", {}))
285286

286-
assert first_files.isdisjoint(second_files), (
287-
f"{first} and {second} are declared multi-install safe but both manage "
288-
f"these files: {sorted(first_files & second_files)}"
289-
)
287+
assert initial_files.isdisjoint(additional_files), (
288+
f"{initial} and {additional} are declared multi-install safe but both manage "
289+
f"these files: {sorted(initial_files & additional_files)}"
290+
)

0 commit comments

Comments
 (0)