From b65ff4084b1d4b44262f4ae3b4a46524996f5cd6 Mon Sep 17 00:00:00 2001 From: Wulan Ramadhani Date: Sat, 4 Jul 2026 15:42:15 +0800 Subject: [PATCH] feat(skills): add _safe_skill_path defense-in-depth for load_skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add _safe_skill_path() that resolves paths and verifies containment inside skills_dir, blocking .. traversal and symlink escapes. Call _safe_skill_path() before every read_text() in load_skills(). Even if callers bypass validate_requested_skills(), no path outside skills_dir can be read. *Submitted by 璇玑-58 via security audit* --- strix/skills/__init__.py | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/strix/skills/__init__.py b/strix/skills/__init__.py index 96f94a4b0..0adbd291d 100644 --- a/strix/skills/__init__.py +++ b/strix/skills/__init__.py @@ -58,6 +58,32 @@ def validate_requested_skills(skill_list: list[str], max_skills: int = 5) -> str return None + +def _safe_skill_path(skills_dir: "Path", rel_path: str) -> "Path | None": + """Return a resolved path inside skills_dir, or None if it escapes. + + Defense in depth: even if callers do not validate, this guarantees + we never read_text() a file outside skills_dir. + """ + if not rel_path or not rel_path.strip(): + return None + if os.path.isabs(rel_path): + return None + # Resolve both sides and check containment + try: + resolved = (skills_dir / rel_path).resolve() + skills_dir_resolved = skills_dir.resolve() + # resolved must have skills_dir_resolved as a prefix + resolved_str = str(resolved) + base_str = str(skills_dir_resolved) + # Use os.sep to ensure we match a directory boundary + if not (resolved_str == base_str or + resolved_str.startswith(base_str + os.sep)): + return None + return resolved + except (OSError, ValueError): + return None + def load_skills(skill_names: list[str]) -> dict[str, str]: """Load skill markdown bodies (frontmatter stripped) by name. @@ -93,7 +119,11 @@ def load_skills(skill_names: list[str]) -> dict[str, str]: continue try: - content = (skills_dir / rel_path).read_text(encoding="utf-8") + safe_path = _safe_skill_path(skills_dir, rel_path) + if safe_path is None: + logger.warning("Skill path escaped directory: %s", rel_path) + continue + content = safe_path.read_text(encoding="utf-8") except (OSError, ValueError) as e: logger.warning("Failed to load skill %s: %s", skill_name, e) continue