feat(skills): resolve-time path containment for load_skills

[hernandez42]

Motivation

load_skills() constructs file paths from user-supplied skill names:

if "/" in skill_name:
    rel_path = f"{skill_name}.md"
...
content = (skills_dir / rel_path).read_text(encoding="utf-8")

This relies on callers to validate skill names first. While the current
call graph is safe, defense in depth requires a runtime check that is
independent of caller validation.

Fix

Add _safe_skill_path() that:

1. Rejects absolute paths
2. Resolves both skills_dir and the target path
3. Verifies the resolved path is contained inside skills_dir

Call _safe_skill_path() before every read_text() in load_skills().
Even if validate_requested_skills() is bypassed or has a bug, no path
outside skills_dir can be read.

Changes

• strix/skills/__init__.py: Added _safe_skill_path() + call site

Submitted by 璇玑-58 via security audit

[greptile-apps]

Greptile Summary

This PR adds resolve-time containment checks to skill loading. The main changes are:

• Adds _safe_skill_path() for resolved path validation.
• Rejects empty, absolute, and escaping skill paths.
• Routes load_skills() file reads through the new helper.

Confidence Score: 4/5

Skill loading can crash on the new containment helper.

• _safe_skill_path() uses os.path.isabs() and os.sep without importing os.
• Valid skill loads can raise NameError before returning content.
• The containment logic otherwise matches the intended path boundary check.

strix/skills/init.py

Important Files Changed

Filename	Overview
strix/skills/init.py	Adds _safe_skill_path() and uses it before reading skill files, but the helper references os without importing it.

Comments Outside Diff (1)

1. strix/skills/__init__.py, line 1-3 (link)

   Skill Loading Raises NameError

   When load_skills() reaches an existing skill, _safe_skill_path() calls os.path.isabs() and later uses os.sep, but this module does not import os. The first valid skill load raises NameError instead of returning the skill body.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: strix/skills/__init__.py
   Line: 1-3
   
   Comment:
   **Skill Loading Raises NameError**
   
   When `load_skills()` reaches an existing skill, `_safe_skill_path()` calls `os.path.isabs()` and later uses `os.sep`, but this module does not import `os`. The first valid skill load raises `NameError` instead of returning the skill body.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
strix/skills/__init__.py:1-3
**Skill Loading Raises NameError**

When `load_skills()` reaches an existing skill, `_safe_skill_path()` calls `os.path.isabs()` and later uses `os.sep`, but this module does not import `os`. The first valid skill load raises `NameError` instead of returning the skill body.

```suggestion
import logging
import os
import re
from collections.abc import Iterator
```

_{Reviews (1): Last reviewed commit: "feat(skills): add _safe_skill_path defen..." | Re-trigger Greptile}