Skip to content

Commit 1fa889d

Browse files
authored
Merge pull request #4 from shinpr/fix/skillshield-security-findings
Fix path traversal, Python 3.9 compat, and SKILL.md refinement
2 parents e8a6af4 + 29e31cc commit 1fa889d

6 files changed

Lines changed: 100 additions & 19 deletions

File tree

.github/workflows/test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ jobs:
1111
runs-on: ubuntu-latest
1212
strategy:
1313
matrix:
14-
python-version: ['3.10', '3.11', '3.12']
14+
python-version: ['3.9', '3.10', '3.11', '3.12']
1515

1616
steps:
1717
- uses: actions/checkout@v4

README.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ Most major AI coding agents now have built-in sub-agents — but only for their
2323
This skill removes that restriction:
2424

2525
- **Cross-LLM orchestration** — Use Claude for long-chain reasoning, Codex for fast refactors, and Gemini for large-context analysis, all from the same parent session.
26-
- **No vendor lock-in** — Your agent definitions are plain markdown files that work across any Agent Skills-compatible tool, so switching IDEs or LLM providers doesn't mean rewriting your workflow.
26+
- **No vendor lock-in** — Your agent definitions are plain markdown files that work with Codex, Claude Code, Cursor, Gemini CLI, VS Code, and [30+ other tools](https://agentskills.io) that support the Agent Skills format, so switching IDEs or LLM providers doesn't mean rewriting your workflow.
2727
- **Bring Your Own Model** — Choose which model handles each task and pay each provider directly at their API rates.
2828
- **Team portability** — Share agent definitions across your team regardless of IDE or preferred LLM.
2929

@@ -48,7 +48,7 @@ You only need to install the backends you plan to use.
4848

4949
## Quick Start
5050

51-
**Requirements:** Python 3.10+ and at least one [supported backend](#supported-backends) installed.
51+
**Requirements:** Python 3.9+ and at least one [supported backend](#supported-backends) installed.
5252

5353
### 1. Install the Skill
5454

@@ -317,7 +317,7 @@ The main agent stays lightweight too. It coordinates work without accumulating a
317317

318318
### Agent Skills as an Open Standard
319319

320-
This skill uses the [Agent Skills](https://github.com/anthropics/skills) format—a convention for packaging reusable AI agent capabilities as portable files. The format is supported by Claude Code, Codex, and other Agent Skills-compatible tools, so the same skill works across environments without modification.
320+
This skill uses the [Agent Skills](https://agentskills.io) format—a convention for packaging reusable AI agent capabilities as portable files. The format is supported by Codex, Claude Code, Cursor, Gemini CLI, and [30+ other tools](https://agentskills.io), so the same skill works across environments without modification.
321321

322322
## How It Works
323323

@@ -329,7 +329,7 @@ skills/sub-agents/
329329
├── scripts/
330330
│ └── run_subagent.py # Calls external CLIs
331331
└── references/
332-
└── cli-formats.md # CLI output format docs
332+
└── codex.md # Codex-specific setup docs
333333
```
334334

335335
## License

pyproject.toml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
[project]
22
name = "sub-agents-skills"
33
version = "0.1.0"
4-
requires-python = ">=3.11"
4+
requires-python = ">=3.9"
55
dependencies = []
66

77
[project.optional-dependencies]
@@ -11,7 +11,7 @@ dev = [
1111
]
1212

1313
[tool.ruff]
14-
target-version = "py311"
14+
target-version = "py39"
1515
line-length = 100
1616

1717
[tool.ruff.lint]
@@ -27,6 +27,7 @@ select = [
2727
]
2828
ignore = [
2929
"E501", # line too long (handled by formatter)
30+
"SIM117", # nested with — combining requires parenthesized syntax (3.10+)
3031
]
3132

3233
[tool.ruff.format]

skills/sub-agents/SKILL.md

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
---
22
name: sub-agents
3-
description: Execute external CLI AIs as isolated sub-agents for task delegation, parallel processing, and context separation. Use when delegating complex multi-step tasks, running parallel investigations, needing fresh context without current conversation history, or leveraging specialized agent definitions. Returns structured JSON with agent output, exit code, and execution status.
3+
description: Run agent definitions as sub-agents. Use when the user names an agent or sub-agent to run, references an agent definition, or delegates a task to an agent.
44
allowed-tools: Bash Read
55
---
66

@@ -40,14 +40,6 @@ Extract parameters from user's natural language request:
4040
"Run code-reviewer on src/"
4141
`--agent code-reviewer --prompt "Review src/" --cwd $(pwd)`
4242

43-
## When to Use This Skill
44-
45-
**Use sub-agent when ANY of these apply:**
46-
- Complex multi-step task (isolated context prevents interference)
47-
- Parallel investigation (multiple agents can run simultaneously)
48-
- Task needs fresh context (sub-agent starts without conversation history)
49-
- Specialized agent definition exists (leverage pre-defined expert prompts)
50-
5143
## Important: Permission and Timeout
5244

5345
This script executes external CLIs that require elevated permissions.
@@ -158,7 +150,7 @@ What this agent does.
158150
How results should be structured.
159151
```
160152

161-
**Critical**: The `run-agent` frontmatter determines which CLI executes the agent. See [cli-formats.md](references/cli-formats.md) for CLI-specific behaviors.
153+
**Critical**: The `run-agent` frontmatter determines which CLI executes the agent.
162154

163155
## CLI Selection Priority
164156

skills/sub-agents/scripts/run_subagent.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
SUB_AGENTS_DIR: Override default agents directory ({cwd}/.agents/)
1313
"""
1414

15+
from __future__ import annotations
16+
1517
import argparse
1618
import json
1719
import os
@@ -59,18 +61,36 @@ def extract_description(body: str) -> str:
5961
return ""
6062

6163

64+
_AGENT_NAME_PATTERN = re.compile(r"^[a-zA-Z0-9][a-zA-Z0-9._-]*$")
65+
66+
67+
def validate_agent_name(agent_name: str) -> str:
68+
"""
69+
Validate agent name to prevent path traversal.
70+
Returns the name if valid, raises ValueError otherwise.
71+
"""
72+
if not agent_name or not _AGENT_NAME_PATTERN.match(agent_name):
73+
raise ValueError(f"Invalid agent name: {agent_name!r}")
74+
return agent_name
75+
76+
6277
def load_agent(agents_dir: str, agent_name: str) -> tuple[str | None, str, str]:
6378
"""
6479
Load agent definition file and extract run-agent setting.
6580
Returns (run_agent_cli, system_context, description)
6681
"""
82+
validate_agent_name(agent_name)
6783
agents_path = Path(agents_dir)
6884

6985
# Try .md first, then .txt
7086
for ext in [".md", ".txt"]:
7187
agent_file = agents_path / f"{agent_name}{ext}"
72-
if agent_file.exists():
73-
content = agent_file.read_text(encoding="utf-8")
88+
# Defense in depth: verify resolved path stays within agents_dir
89+
resolved = agent_file.resolve()
90+
if not str(resolved).startswith(str(agents_path.resolve())):
91+
raise ValueError(f"Invalid agent name: {agent_name!r}")
92+
if resolved.exists():
93+
content = resolved.read_text(encoding="utf-8")
7494
frontmatter, body = parse_frontmatter(content)
7595

7696
run_agent = frontmatter.get("run-agent")

tests/test_run_subagent.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,74 @@ def test_without_frontmatter(self):
5050
assert body == content
5151

5252

53+
class TestLoadAgentRejectsInvalidNames:
54+
"""load_agent should reject names that could escape the agents directory."""
55+
56+
def _make_agents_dir(self, tmpdir):
57+
agents_dir = Path(tmpdir) / "agents"
58+
agents_dir.mkdir()
59+
(agents_dir / "valid-agent.md").write_text("# Valid\n\nA valid agent.")
60+
return str(agents_dir)
61+
62+
# --- Valid names load successfully ---
63+
64+
def test_loads_simple_hyphenated_name(self):
65+
with tempfile.TemporaryDirectory() as tmpdir:
66+
agents_dir = self._make_agents_dir(tmpdir)
67+
_, context, _ = load_agent(agents_dir, "valid-agent")
68+
assert "Valid" in context
69+
70+
# --- Path traversal attempts are rejected ---
71+
72+
def test_rejects_parent_directory_traversal(self):
73+
with tempfile.TemporaryDirectory() as tmpdir:
74+
agents_dir = self._make_agents_dir(tmpdir)
75+
with pytest.raises(ValueError, match="Invalid agent name"):
76+
load_agent(agents_dir, "../etc/passwd")
77+
78+
def test_rejects_nested_traversal(self):
79+
with tempfile.TemporaryDirectory() as tmpdir:
80+
agents_dir = self._make_agents_dir(tmpdir)
81+
with pytest.raises(ValueError, match="Invalid agent name"):
82+
load_agent(agents_dir, "../../secret")
83+
84+
def test_rejects_absolute_path(self):
85+
with tempfile.TemporaryDirectory() as tmpdir:
86+
agents_dir = self._make_agents_dir(tmpdir)
87+
with pytest.raises(ValueError, match="Invalid agent name"):
88+
load_agent(agents_dir, "/etc/passwd")
89+
90+
def test_rejects_forward_slash(self):
91+
with tempfile.TemporaryDirectory() as tmpdir:
92+
agents_dir = self._make_agents_dir(tmpdir)
93+
with pytest.raises(ValueError, match="Invalid agent name"):
94+
load_agent(agents_dir, "sub/agent")
95+
96+
def test_rejects_backslash(self):
97+
with tempfile.TemporaryDirectory() as tmpdir:
98+
agents_dir = self._make_agents_dir(tmpdir)
99+
with pytest.raises(ValueError, match="Invalid agent name"):
100+
load_agent(agents_dir, "sub\\agent")
101+
102+
def test_rejects_empty_string(self):
103+
with tempfile.TemporaryDirectory() as tmpdir:
104+
agents_dir = self._make_agents_dir(tmpdir)
105+
with pytest.raises(ValueError, match="Invalid agent name"):
106+
load_agent(agents_dir, "")
107+
108+
def test_rejects_shell_metacharacters(self):
109+
with tempfile.TemporaryDirectory() as tmpdir:
110+
agents_dir = self._make_agents_dir(tmpdir)
111+
with pytest.raises(ValueError, match="Invalid agent name"):
112+
load_agent(agents_dir, "agent;rm -rf")
113+
114+
def test_rejects_leading_dot(self):
115+
with tempfile.TemporaryDirectory() as tmpdir:
116+
agents_dir = self._make_agents_dir(tmpdir)
117+
with pytest.raises(ValueError, match="Invalid agent name"):
118+
load_agent(agents_dir, ".hidden")
119+
120+
53121
class TestLoadAgent:
54122
def test_load_agent_md(self):
55123
with tempfile.TemporaryDirectory() as tmpdir:

0 commit comments

Comments
 (0)