Skip to content

fix(security): LLM memory write path traversal + skill name sanitization#17

Merged
FZ2000 merged 2 commits intomainfrom
fix/security
Mar 7, 2026
Merged

fix(security): LLM memory write path traversal + skill name sanitization#17
FZ2000 merged 2 commits intomainfrom
fix/security

Conversation

@FZ2000
Copy link
Owner

@FZ2000 FZ2000 commented Mar 6, 2026

Two path-traversal vulnerabilities: one in LLM memory sync (arbitrary file write), one in skill name handling (directory escape via frontmatter).

Fix 1 — apply_memory_via_llm(): validate write path before touching disk

Risk: The LLM returns {"file_path": "/absolute/path", "content": "..."}. A hallucination or prompt-injection attack could return /etc/cron.d/evil and apply_memory_via_llm() would write it — no validation existed.

Fix (appliers/base.py):

  • Added MEMORY_ALLOWED_BASE: Optional[Path] = None class attribute to BaseApplier.
  • Before writing, resolves Path(file_path).resolve() (collapses ../.. etc) and asserts it starts with allowed_base.
  • Logs [security] Rejecting LLM-suggested write outside allowed path: and continues on violation.
  • Each concrete applier now declares MEMORY_ALLOWED_BASE via @property:
    • ClaudeApplier~/.claude
    • GeminiApplier~/.gemini
    • CursorApplier~/.cursor
    • WindsurfApplier~/.codeium/windsurf
    • OpenClawApplier~/.openclaw/workspace
    • CopilotApplierPath.cwd() (project-relative .github/)
  • Falls back to Path.home() when MEMORY_ALLOWED_BASE is unset (e.g. new applier without override).

Fix 2 — skill name sanitization: frontmatter path traversal

Risk: A crafted SKILL.md with name: "../../etc/cron.d/evil" in its frontmatter would cause save_skill_file(skill["name"], ...) to write outside ~/.apc/skills/. Similarly, link_skills() would create a symlink outside the tool's skill directory.

Fix (skills.py):

  • Added sanitize_skill_name(name: str) -> str — takes basename only (Path(name).name), validates against ^[A-Za-z0-9][A-Za-z0-9_\-]{0,63}$, raises ValueError on traversal.
  • Applied in save_skill_file() (entry point for all disk writes).
  • Applied in fetch_skill_from_repo() on the frontmatter name field, with fallback to the URL path component (already trusted).

Fix (install.py):

  • Calls sanitize_skill_name(skill["name"]) before save_skill_file() — skips with error message on violation.
  • Imports sanitize_skill_name from skills.

Fix (appliers/base.py link_skills):

  • Validates name via sanitize_skill_name() before constructing symlink paths. Skips with warning on invalid name.

Tests

120 existing tests pass unchanged.

@FZ2000 FZ2000 merged commit 2b5d2b1 into main Mar 7, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant