Skip to content

Commit 0aace65

Browse files
mnriemCopilot
andcommitted
fix(bob): fail closed on malformed preset entries too (review #3415)
Address review 4745191015: the preset guard read a parseable registry but silently skipped malformed per-preset metadata and treated a malformed registered_commands value as "no matching artifacts". A registry such as {"presets":{"p1":[]}} therefore allowed a layout migration even though p1's ownership is unknown, risking deletion of preset-managed files. Now raise _PresetRegistryUnreadableError for a non-dict preset entry, a non-dict registered_commands, or a non-list registered_skills. Extend the unit test to cover these malformed shapes. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 63f93544-a77f-4f01-bf04-c88806a97dbf
1 parent 44bc562 commit 0aace65

2 files changed

Lines changed: 39 additions & 6 deletions

File tree

src/specify_cli/integrations/_migrate_commands.py

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,14 +105,24 @@ def _installed_presets_affecting_agent(project_root, agent_key: str) -> list[str
105105

106106
affected: list[str] = []
107107
for preset_id, meta in data.get("presets", {}).items():
108+
# A malformed entry means we cannot verify whether this preset owns
109+
# artifacts for the agent, so fail closed rather than skip it.
108110
if not isinstance(meta, dict):
109-
continue
111+
raise _PresetRegistryUnreadableError(
112+
f"preset '{preset_id}' entry is malformed"
113+
)
110114
registered_commands = meta.get("registered_commands", {})
111-
has_commands = (
112-
isinstance(registered_commands, dict)
113-
and bool(registered_commands.get(agent_key))
114-
)
115-
has_skills = bool(meta.get("registered_skills"))
115+
if not isinstance(registered_commands, dict):
116+
raise _PresetRegistryUnreadableError(
117+
f"preset '{preset_id}' registered_commands is malformed"
118+
)
119+
registered_skills = meta.get("registered_skills", [])
120+
if not isinstance(registered_skills, (list, tuple)):
121+
raise _PresetRegistryUnreadableError(
122+
f"preset '{preset_id}' registered_skills is malformed"
123+
)
124+
has_commands = bool(registered_commands.get(agent_key))
125+
has_skills = bool(registered_skills)
116126
if has_commands or has_skills:
117127
affected.append(preset_id)
118128
return affected

tests/integrations/test_integration_subcommand.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2943,6 +2943,29 @@ def test_installed_presets_affecting_agent_absent_vs_unreadable(self, tmp_path):
29432943
with pytest.raises(_PresetRegistryUnreadableError):
29442944
_installed_presets_affecting_agent(project, "bob")
29452945

2946+
# Malformed per-preset entry (not a dict) → ownership unknown → raise.
2947+
registry.write_text(
2948+
json.dumps({"presets": {"p1": []}}), encoding="utf-8"
2949+
)
2950+
with pytest.raises(_PresetRegistryUnreadableError):
2951+
_installed_presets_affecting_agent(project, "bob")
2952+
2953+
# Malformed registered_commands (not a dict) → raise.
2954+
registry.write_text(
2955+
json.dumps({"presets": {"p1": {"registered_commands": []}}}),
2956+
encoding="utf-8",
2957+
)
2958+
with pytest.raises(_PresetRegistryUnreadableError):
2959+
_installed_presets_affecting_agent(project, "bob")
2960+
2961+
# Malformed registered_skills (not a list) → raise.
2962+
registry.write_text(
2963+
json.dumps({"presets": {"p1": {"registered_skills": {}}}}),
2964+
encoding="utf-8",
2965+
)
2966+
with pytest.raises(_PresetRegistryUnreadableError):
2967+
_installed_presets_affecting_agent(project, "bob")
2968+
29462969
# Valid, empty registry → empty list.
29472970
registry.write_text(json.dumps({"presets": {}}), encoding="utf-8")
29482971
assert _installed_presets_affecting_agent(project, "bob") == []

0 commit comments

Comments
 (0)