Skip to content
Merged
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
38 changes: 33 additions & 5 deletions src/appliers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ class BaseApplier(ABC):
# with a description of how the tool expects its memory files.
MEMORY_SCHEMA: str = ""

# Subclasses MUST override this with the directory the LLM is allowed to
# write memory files into. apply_memory_via_llm() rejects any path that
# does not resolve inside this directory. Defaults to Path.home() as a
# minimum guard; narrow it in each applier.
MEMORY_ALLOWED_BASE: Optional[Path] = None

def get_manifest(self) -> ToolManifest:
"""Return (or create) the manifest for this tool."""
return ToolManifest(self.TOOL_NAME)
Expand All @@ -95,7 +101,15 @@ def link_skills(self, skills: List[Dict], source_dir: Path, manifest: ToolManife
count = 0

for skill in skills:
name = skill.get("name", "unnamed")
raw_name = skill.get("name", "unnamed")
try:
from skills import sanitize_skill_name

name = sanitize_skill_name(raw_name)
except (ValueError, ImportError):
warning(f"Skipping skill with invalid name: {raw_name!r}")
continue

source = source_dir / name
if not source.exists():
continue
Expand Down Expand Up @@ -212,6 +226,10 @@ def apply_memory_via_llm(self, collected_memory: List[Dict], manifest: ToolManif
warning(f"Raw LLM response: {response[:500]}")
return 0

# Determine the allowed write root for this applier.
# Resolving at call-time so tests can monkeypatch Path.home().
allowed_base = (self.MEMORY_ALLOWED_BASE or Path.home()).resolve()

# Write files
count = 0
for op in file_ops:
Expand All @@ -222,11 +240,21 @@ def apply_memory_via_llm(self, collected_memory: List[Dict], manifest: ToolManif
if not file_path or content is None:
continue

path = Path(file_path)
path.parent.mkdir(parents=True, exist_ok=True)
path.write_text(content, encoding="utf-8")
# Security: resolve the path (collapses `..`) and assert it lands
# inside the allowed base directory. Rejects prompt-injection or
# hallucinated paths like /etc/cron.d/evil.
resolved = Path(file_path).resolve()
if not str(resolved).startswith(str(allowed_base) + "/") and resolved != allowed_base:
warning(
f"[security] Rejecting LLM-suggested write outside allowed path: "
f"{file_path!r} (resolved: {resolved}, allowed base: {allowed_base})"
)
continue

resolved.parent.mkdir(parents=True, exist_ok=True)
resolved.write_text(content, encoding="utf-8")
manifest.record_memory(
file_path=str(path),
file_path=str(resolved),
content=content,
entry_ids=[e.get("entry_id") or e.get("id", "") for e in collected_memory],
)
Expand Down
4 changes: 4 additions & 0 deletions src/appliers/claude.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ def SKILL_DIR(self, value):
TOOL_NAME = "claude-code"
MEMORY_SCHEMA = CLAUDE_MEMORY_SCHEMA

@property # type: ignore[override]
def MEMORY_ALLOWED_BASE(self) -> "Path": # noqa: N802
return _claude_dir()

def apply_skills(self, skills: List[Dict], manifest: ToolManifest) -> int:
_claude_commands_dir().mkdir(parents=True, exist_ok=True)
count = 0
Expand Down
5 changes: 5 additions & 0 deletions src/appliers/copilot.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ class CopilotApplier(BaseApplier):
TOOL_NAME = "github-copilot"
MEMORY_SCHEMA = COPILOT_MEMORY_SCHEMA

@property # type: ignore[override]
def MEMORY_ALLOWED_BASE(self) -> "Path": # noqa: N802
# Copilot writes to .github/ in the current project directory.
return Path.cwd()

def apply_skills(self, skills: List[Dict], manifest: ToolManifest) -> int:
count = 0
for skill in skills:
Expand Down
4 changes: 4 additions & 0 deletions src/appliers/cursor.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ class CursorApplier(BaseApplier):
TOOL_NAME = "cursor"
MEMORY_SCHEMA = CURSOR_MEMORY_SCHEMA

@property # type: ignore[override]
def MEMORY_ALLOWED_BASE(self) -> "Path": # noqa: N802
return _cursor_dir()

@property
def SKILL_DIR(self) -> Path: # type: ignore[override]
return _cursor_rules_dir()
Expand Down
4 changes: 4 additions & 0 deletions src/appliers/gemini.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ class GeminiApplier(BaseApplier):
TOOL_NAME = "gemini-cli"
MEMORY_SCHEMA = GEMINI_MEMORY_SCHEMA

@property # type: ignore[override]
def MEMORY_ALLOWED_BASE(self) -> "Path": # noqa: N802
return _gemini_dir()

def apply_skills(self, skills: List[Dict], manifest: ToolManifest) -> int:
return 0 # Gemini doesn't have a skills format

Expand Down
4 changes: 4 additions & 0 deletions src/appliers/openclaw.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ def SKILL_DIR(self, value):
TOOL_NAME = "openclaw"
MEMORY_SCHEMA = OPENCLAW_MEMORY_SCHEMA

@property # type: ignore[override]
def MEMORY_ALLOWED_BASE(self) -> "Path": # noqa: N802
return _openclaw_workspace()

def apply_skills(self, skills: List[Dict], manifest: ToolManifest) -> int:
_openclaw_skills_dir().mkdir(parents=True, exist_ok=True)
count = 0
Expand Down
4 changes: 4 additions & 0 deletions src/appliers/windsurf.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ class WindsurfApplier(BaseApplier):
TOOL_NAME = "windsurf"
MEMORY_SCHEMA = WINDSURF_MEMORY_SCHEMA

@property # type: ignore[override]
def MEMORY_ALLOWED_BASE(self) -> "Path": # noqa: N802
return _windsurf_dir()

def apply_skills(self, skills: List[Dict], manifest: ToolManifest) -> int:
return 0

Expand Down
9 changes: 8 additions & 1 deletion src/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from appliers import get_applier
from cache import load_skills, merge_skills, save_skills
from extractors import detect_installed_tools
from skills import fetch_skill_from_repo, list_skills_in_repo, save_skill_file
from skills import fetch_skill_from_repo, list_skills_in_repo, sanitize_skill_name, save_skill_file

_AGENTS = ["claude-code", "cursor", "gemini-cli", "github-copilot", "openclaw", "windsurf"]

Expand Down Expand Up @@ -176,6 +176,13 @@ def install(repo, skills, install_all, targets, branch, list_only, yes):
click.echo(f" not found in {repo}")
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)
continue

# Save to ~/.apc/skills/<name>/SKILL.md
raw_content = skill.pop("_raw_content", skill.get("body", ""))
save_skill_file(skill["name"], raw_content)
Expand Down
34 changes: 33 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 re
from pathlib import Path
from typing import Any, Dict, List, Optional

Expand All @@ -16,6 +17,29 @@
_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"

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


def sanitize_skill_name(name: str) -> str:
"""Return a safe skill name or raise ValueError.

Rules:
- Basename only (strips directory components).
- No path separators, no '..' traversal.
- Must match [A-Za-z0-9][A-Za-z0-9_\\-]{0,63}.
"""
# Take basename — eliminates leading paths like "../../etc"
safe = Path(name).name
# Reject anything that still looks path-like after basename
if not safe or safe in (".", ".."):
raise ValueError(f"Invalid skill name (empty or dot): {name!r}")
if not _SKILL_NAME_SAFE.match(safe):
raise ValueError(
f"Skill name {safe!r} contains invalid characters. "
"Only letters, digits, hyphens, and underscores are allowed."
)
return safe


# ---------------------------------------------------------------------------
# Skills directory
Expand All @@ -31,6 +55,7 @@ def get_skills_dir() -> Path:

def save_skill_file(skill_name: str, raw_content: str) -> Path:
"""Save raw SKILL.md to ~/.apc/skills/<name>/SKILL.md. Returns the path."""
skill_name = sanitize_skill_name(skill_name) # raises ValueError on traversal
skill_dir = get_skills_dir() / skill_name
skill_dir.mkdir(exist_ok=True)
path = skill_dir / "SKILL.md"
Expand Down Expand Up @@ -86,8 +111,15 @@ def fetch_skill_from_repo(
return None

metadata, body = parse_frontmatter(resp.text)
# Sanitize the name from frontmatter — it comes from an untrusted source.
raw_name = metadata.get("name", skill_name)
try:
safe_name = sanitize_skill_name(raw_name)
except ValueError:
# Fall back to the URL-path component (already validated by list_skills_in_repo)
safe_name = sanitize_skill_name(skill_name)
return {
"name": metadata.get("name", skill_name),
"name": safe_name,
"description": metadata.get("description", ""),
"body": body.strip(),
"tags": metadata.get("tags", []),
Expand Down