Skip to content

Commit 8ca42b6

Browse files
committed
fix: address third round of PR review feedback
- Fix CONTRIBUTING.md JSON examples to show full catalog structure with schema_version and integrations wrapper - Wrap cache writes in try/except OSError for read-only project dirs - Validate _load_catalog_config YAML root is a dict - Skip non-dict integ_data entries in merged catalog - Normalize tags to list-of-strings before filtering/searching - Add path traversal containment check for stale file deletion - Clarify docstring: lower numeric priority = higher precedence
1 parent 7891de1 commit 8ca42b6

File tree

3 files changed

+62
-35
lines changed

3 files changed

+62
-35
lines changed

integrations/CONTRIBUTING.md

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,21 @@ Built-in integrations are maintained by the Spec Kit core team and ship with the
1717

1818
### Catalog Entry Format
1919

20-
Add your integration to `integrations/catalog.json`:
20+
Add your integration under the top-level `integrations` key in `integrations/catalog.json`:
2121

2222
```json
2323
{
24-
"my-agent": {
25-
"id": "my-agent",
26-
"name": "My Agent",
27-
"version": "1.0.0",
28-
"description": "Integration for My Agent",
29-
"author": "spec-kit-core",
30-
"repository": "https://github.com/github/spec-kit",
31-
"tags": ["cli"]
24+
"schema_version": "1.0",
25+
"integrations": {
26+
"my-agent": {
27+
"id": "my-agent",
28+
"name": "My Agent",
29+
"version": "1.0.0",
30+
"description": "Integration for My Agent",
31+
"author": "spec-kit-core",
32+
"repository": "https://github.com/github/spec-kit",
33+
"tags": ["cli"]
34+
}
3235
}
3336
}
3437
```
@@ -88,18 +91,21 @@ provides:
8891
### Submitting to the Community Catalog
8992

9093
1. **Fork** the [spec-kit repository](https://github.com/github/spec-kit)
91-
2. **Add your entry** to `integrations/catalog.community.json`:
94+
2. **Add your entry** under the `integrations` key in `integrations/catalog.community.json`:
9295

9396
```json
9497
{
95-
"my-agent": {
96-
"id": "my-agent",
97-
"name": "My Agent",
98-
"version": "1.0.0",
99-
"description": "Integration for My Agent",
100-
"author": "your-name",
101-
"repository": "https://github.com/your-name/speckit-my-agent",
102-
"tags": ["cli"]
98+
"schema_version": "1.0",
99+
"integrations": {
100+
"my-agent": {
101+
"id": "my-agent",
102+
"name": "My Agent",
103+
"version": "1.0.0",
104+
"description": "Integration for My Agent",
105+
"author": "your-name",
106+
"repository": "https://github.com/your-name/speckit-my-agent",
107+
"tags": ["cli"]
108+
}
103109
}
104110
}
105111
```

src/specify_cli/__init__.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2316,12 +2316,19 @@ def integration_upgrade(
23162316
raise typer.Exit(1)
23172317

23182318
# Phase 2: Remove stale files from old manifest that are not in the new one
2319+
resolved_project_root = project_root.resolve()
23192320
old_files = set(old_manifest.files.keys())
23202321
new_files = set(new_manifest.files.keys())
23212322
stale_files = old_files - new_files
23222323
stale_removed = 0
23232324
for rel in stale_files:
23242325
path = project_root / rel
2326+
# Validate containment to prevent path traversal from tampered manifests
2327+
try:
2328+
normed = Path(os.path.normpath(path))
2329+
normed.relative_to(resolved_project_root)
2330+
except (ValueError, OSError):
2331+
continue
23252332
if path.exists() and path.is_file():
23262333
try:
23272334
path.unlink()

src/specify_cli/integrations/catalog.py

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,10 @@ def _load_catalog_config(
105105
raise IntegrationCatalogError(
106106
f"Failed to read catalog config {config_path}: {exc}"
107107
)
108+
if not isinstance(data, dict):
109+
raise IntegrationCatalogError(
110+
f"Invalid catalog config {config_path}: expected a YAML mapping at the root"
111+
)
108112
catalogs_data = data.get("catalogs", [])
109113
if not catalogs_data:
110114
raise IntegrationCatalogError(
@@ -267,17 +271,20 @@ def _fetch_single_catalog(
267271
f"Invalid catalog format from {entry.url}: 'integrations' must be a JSON object"
268272
)
269273

270-
self.cache_dir.mkdir(parents=True, exist_ok=True)
271-
cache_file.write_text(json.dumps(catalog_data, indent=2))
272-
cache_meta.write_text(
273-
json.dumps(
274-
{
275-
"cached_at": datetime.now(timezone.utc).isoformat(),
276-
"catalog_url": entry.url,
277-
},
278-
indent=2,
274+
try:
275+
self.cache_dir.mkdir(parents=True, exist_ok=True)
276+
cache_file.write_text(json.dumps(catalog_data, indent=2))
277+
cache_meta.write_text(
278+
json.dumps(
279+
{
280+
"cached_at": datetime.now(timezone.utc).isoformat(),
281+
"catalog_url": entry.url,
282+
},
283+
indent=2,
284+
)
279285
)
280-
)
286+
except OSError:
287+
pass # Cache is best-effort; proceed with fetched data
281288
return catalog_data
282289

283290
except urllib.error.URLError as exc:
@@ -294,8 +301,10 @@ def _get_merged_integrations(
294301
) -> List[Dict[str, Any]]:
295302
"""Fetch and merge integrations from all active catalogs.
296303
297-
Higher-priority catalogs win on conflicts. Each dict is annotated
298-
with ``_catalog_name`` and ``_install_allowed``.
304+
Catalogs are processed in the order returned by
305+
:meth:`get_active_catalogs`. On conflicts, the first catalog in that
306+
order wins (lower numeric priority = higher precedence). Each dict is
307+
annotated with ``_catalog_name`` and ``_install_allowed``.
299308
"""
300309
import sys
301310

@@ -315,6 +324,8 @@ def _get_merged_integrations(
315324
continue
316325

317326
for integ_id, integ_data in data.get("integrations", {}).items():
327+
if not isinstance(integ_data, dict):
328+
continue
318329
if integ_id not in merged:
319330
merged[integ_id] = {
320331
**integ_data,
@@ -343,18 +354,21 @@ def search(
343354
for item in self._get_merged_integrations():
344355
if author and item.get("author", "").lower() != author.lower():
345356
continue
346-
if tag and tag.lower() not in [
347-
t.lower() for t in item.get("tags", [])
348-
]:
349-
continue
357+
if tag:
358+
raw_tags = item.get("tags", [])
359+
tags_list = raw_tags if isinstance(raw_tags, list) else []
360+
if tag.lower() not in [t.lower() for t in tags_list if isinstance(t, str)]:
361+
continue
350362
if query:
363+
raw_tags = item.get("tags", [])
364+
tags_list = raw_tags if isinstance(raw_tags, list) else []
351365
haystack = " ".join(
352366
[
353367
item.get("name", ""),
354368
item.get("description", ""),
355369
item.get("id", ""),
356370
]
357-
+ item.get("tags", [])
371+
+ [t for t in tags_list if isinstance(t, str)]
358372
).lower()
359373
if query.lower() not in haystack:
360374
continue

0 commit comments

Comments
 (0)