feat(registry): community skills registry (schema, index, validate, site, CI)

[alirezarezvani]

Closed — this registry was built in the wrong repository. The intended target is alirezarezvani/agent-registery. Branch claude/registry-repo-7lRiv is preserved if any of this code is ever wanted, but it does not belong in claude-skills.

[claude]

PR Review — feat(registry): community skills registry

Great concept and solid execution overall. The stdlib-only constraint is respected, the schema is well-structured, and the two-source (internal + community) design is clean. A few issues need attention before this merges, ranging from a security fix to schema gaps.

---

🔴 Critical — CI Command Injection

File: .github/workflows/registry.yml, lines 37–44

- name: Validate changed submissions
  if: steps.changed.outputs.folders != ''
  run: |
    RESULT=0
    for folder in ${{ steps.changed.outputs.folders }}; do   # ← injection here
      echo "::group::Validating $folder"
      python registry/scripts/validate.py --clone "$folder" || RESULT=1
    done
    exit $RESULT

${{ steps.changed.outputs.folders }} is a GitHub Actions expression interpolated directly into the shell script before bash executes it. This is the well-documented expression injection vulnerability. A PR author who names their submission folder with shell metacharacters can execute arbitrary code in the CI runner.

Fix: pass the output as an environment variable, which is not subject to template injection:

- name: Validate changed submissions
  if: steps.changed.outputs.folders != ''
  env:
    FOLDERS: ${{ steps.changed.outputs.folders }}
  run: |
    RESULT=0
    for folder in $FOLDERS; do
      echo "::group::Validating $folder"
      python registry/scripts/validate.py --clone "$folder" || RESULT=1
      echo "::endgroup::"
    done
    exit $RESULT

---

🟠 Medium — Path Traversal via startswith (validate.py:202)

skill_root = (tmp / sub).resolve()
if not str(skill_root).startswith(str(tmp.resolve())):  # ← bug

This is the classic path-traversal guard — almost right. The string startswith check has a well-known edge case: /tmp/abc is a prefix of /tmp/abcdefg. Use Path.is_relative_to() (available since Python 3.9, pinned to 3.12 in CI) or append a path separator:

skill_root = (tmp / sub).resolve()
tmp_resolved = tmp.resolve()
if not skill_root.is_relative_to(tmp_resolved):   # Python 3.9+
    result.errors.append(f"path escapes repository root: {sub!r}")
    return

---

🟠 Medium — Missing path Pattern in Schema (metadata.schema.json:36-40)

"path": {
  "type": "string",
  "maxLength": 256,
  "description": "Subdirectory within repo where SKILL.md lives (default: root)"
}

No pattern constraint — a submitter can write "path": "../../../etc" and it passes schema validation. The clone-time guard catches this, but CI only runs --clone on PRs, not on --check. Add a pattern that excludes .. segments:

"path": {
  "type": "string",
  "pattern": "^[a-zA-Z0-9_./-]*$",
  "maxLength": 256,
  "description": "Subdirectory within repo where SKILL.md lives (default: root)"
}

(Or at minimum "pattern": "^(?!\\.\\.)(?!.*\\/\\.\\.).*$" to prohibit .. segments.)

---

🟡 Low — agents variable name in build_index.py (line 163)

agents = internal + community
agents.sort(...)
return {"skills": agents, ...}

agents is clearly a leftover from the gitagent origin story. Rename to skills for self-consistency.

---

🟡 Low — Community entries sort first in index.json

agents.sort(key=lambda e: (e["origin"] != "community", e["name"].lower()))

This puts community submissions before the 61 internal skills that represent this repo's core content. The intent may be to showcase community contributions, but it means new users browsing the index.json or the site see community entries first. Consider reversing this (e["origin"] != "internal") or surfacing internal skills first.

---

🟡 Low — No push trigger in registry.yml

The workflow only fires on pull_request. After a PR merges to dev, there is no job that re-runs --check to confirm the committed index.json is still in sync. A push trigger on dev/main for the check-index job would close this gap:

on:
  pull_request:
    paths: ['registry/**', '.claude-plugin/marketplace.json']
  push:
    branches: [dev, main]
    paths: ['registry/**', '.claude-plugin/marketplace.json']

---

🟢 Positives

• Stdlib-only — zero extra dependencies; matches repo convention perfectly.
• --check comparison strips generated_at before diffing — CI won't flake daily.
• _clone_and_verify is guarded by not result.errors — won't clone a submission that already failed schema validation.
• additionalProperties: false in the schema prevents metadata drift.
• Temp dir cleanup in finally — no leftover clones if the git operation fails.
• --github flag is opt-in — default run is deterministic and network-free; good CI design.
• The CONTRIBUTING.md + README.md + example submission form a clear onboarding path.

---

Summary

Severity	Issue	File
🔴 Critical	CI expression injection in shell for loop	registry.yml:38
🟠 Medium	Path traversal via startswith instead of is_relative_to	validate.py:202
🟠 Medium	Missing path field pattern allows ../ traversal in schema	metadata.schema.json:36
🟡 Low	agents variable name should be skills	build_index.py:163
🟡 Low	Community entries sort before internal	build_index.py:164
🟡 Low	No push trigger on dev/main for index sync check	registry.yml

Fix the 🔴 and 🟠 issues before merging; the 🟡 items are advisory. The CI injection is the blocker — it exposes the runner to arbitrary code execution from PR authors.