Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions src/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,19 +214,27 @@ def install(repo, skills, install_all, targets, branch, list_only, yes):
# Fetch and install
installed_skills = []
for skill_name in skill_names:
click.echo(f" Fetching {skill_name}...", nl=False)
skill = fetch_skill_from_repo(repo, skill_name, branch)
if not skill:
click.echo(f" not found in {repo}")
click.echo(f" ! {skill_name}: not found in {repo}", err=True)
continue

# Validate name once more before writing to disk (save_skill_file also validates)
try:
sanitize_skill_name(skill["name"])
except ValueError as exc:
click.echo(f" skipped — invalid name: {exc}", err=True)
click.echo(f" ! {skill_name}: skipped — invalid name: {exc}", err=True)
continue

# Display integrity info for user audit (#29)
commit_sha = skill.pop("_commit_sha", None)
content_checksum = skill.pop("_content_checksum", None)
sha_display = commit_sha[:12] if commit_sha else "unknown"
checksum_short = content_checksum.split(":")[1][:16] if content_checksum else "?"
click.echo(
f" Fetching {skill['name']} from {repo} @ {sha_display} (sha256:{checksum_short}...)"
)

# Save to ~/.apc/skills/<name>/SKILL.md
raw_content = skill.pop("_raw_content", skill.get("body", ""))
save_skill_file(skill["name"], raw_content)
Expand All @@ -240,7 +248,7 @@ def install(repo, skills, install_all, targets, branch, list_only, yes):
save_skills(merged)

installed_skills.append(skill["name"])
click.echo(" ✓")
click.echo(f" ✓ {skill['name']} installed")

if installed_skills:
click.echo(f"\n✓ Installed {len(installed_skills)} skill(s) to {', '.join(target_list)}")
Expand Down
35 changes: 34 additions & 1 deletion src/skills.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
tool's skill directory on sync.
"""

import hashlib
import re
from pathlib import Path
from typing import Any, Dict, List, Optional
Expand All @@ -16,6 +17,9 @@
DEFAULT_BRANCH = "main"
_GITHUB_TREE_API = "https://api.github.com/repos/{repo}/git/trees/{branch}?recursive=1"
_GITHUB_RAW = "https://raw.githubusercontent.com/{repo}/{branch}/skills/{skill}/SKILL.md"
_GITHUB_COMMITS_API = (
"https://api.github.com/repos/{repo}/commits?path=skills/{skill}/SKILL.md&per_page=1"
)

_SKILL_NAME_SAFE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_\-]{0,63}$")

Expand Down Expand Up @@ -105,14 +109,36 @@ def list_skills_in_repo(repo: str, branch: str = DEFAULT_BRANCH) -> List[str]:
return sorted(names)


def _fetch_skill_commit_sha(repo: str, skill_name: str) -> Optional[str]:
"""Return the latest commit SHA for a skill file, or None on error.

Uses the GitHub Commits API with a path filter so we get the SHA of the
commit that last touched skills/<skill_name>/SKILL.md. This is
displayed at install-time so users can audit the exact version (#29).
"""
url = _GITHUB_COMMITS_API.format(repo=repo, skill=skill_name)
try:
resp = _safe_get(url)
if resp.status_code != 200:
return None
data = resp.json()
if isinstance(data, list) and data:
return data[0].get("sha")
except (httpx.HTTPError, Exception):
pass
return None


def fetch_skill_from_repo(
repo: str,
skill_name: str,
branch: str = DEFAULT_BRANCH,
) -> Optional[Dict[str, Any]]:
"""Fetch and parse a single skill from a GitHub repo.

Returns a skill dict (with _raw_content) or None if not found.
Returns a skill dict (with _raw_content and _commit_sha) or None if not found.
The _commit_sha field contains the SHA of the last commit that touched the
skill file — callers should display this so users can audit the download (#29).
"""
url = _GITHUB_RAW.format(repo=repo, branch=branch, skill=skill_name)
try:
Expand All @@ -130,6 +156,11 @@ def fetch_skill_from_repo(
except ValueError:
# Fall back to the URL-path component (already validated by list_skills_in_repo)
safe_name = sanitize_skill_name(skill_name)

# Compute content checksum and fetch commit SHA for integrity display (#29)
content_checksum = "sha256:" + hashlib.sha256(resp.text.encode()).hexdigest()
commit_sha = _fetch_skill_commit_sha(repo, skill_name)

return {
"name": safe_name,
"description": metadata.get("description", ""),
Expand All @@ -140,4 +171,6 @@ def fetch_skill_from_repo(
"source_tool": "github",
"source_repo": repo,
"_raw_content": resp.text,
"_content_checksum": content_checksum,
"_commit_sha": commit_sha,
}
13 changes: 8 additions & 5 deletions tests/test_security_input_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,14 @@ def mock_get(url, follow_redirects=True, timeout=15):

fetch_skill_from_repo("owner/repo", "my-skill", "main")

self.assertEqual(len(calls), 1)
self.assertFalse(
calls[0]["follow_redirects"],
"follow_redirects must be False to prevent SSRF",
)
# 2 calls: (1) raw SKILL.md content, (2) GitHub Commits API for SHA (#29)
# All calls must have follow_redirects=False
self.assertGreaterEqual(len(calls), 1)
for call in calls:
self.assertFalse(
call["follow_redirects"],
"follow_redirects must be False to prevent SSRF",
)


if __name__ == "__main__":
Expand Down
Loading
Loading