Security: Path traversal in fetch_config config_name (CWE-22) - arbitrary directory deletion, repository destruction, attacker-controlled clone

Summary

The fetch_config MCP tool's git URL mode passes the raw, unvalidated config_name tool argument into a filesystem path with no containment check:

# src/skill_seekers/mcp/tools/source_tools.py  (fetch_config_tool, MODE 2)
# identical pattern in the legacy server: src/skill_seekers/mcp/server_legacy.py:1289
source_name_temp = f"temp_{config_name}"          # config_name = raw MCP tool arg
repo_path = git_repo.clone_or_pull(source_name=source_name_temp, ...)

# src/skill_seekers/services/git_repo.py  (clone_or_pull, lines 71-75)
repo_path = self.cache_dir / source_name          # no validation; ../ escapes cache
if force_refresh and repo_path.exists():
    shutil.rmtree(repo_path)                      # arbitrary directory DELETION
...
git.Repo.clone_from(clone_url, repo_path, ...)    # arbitrary directory WRITE

A crafted config_name such as x/../../<relative/path> escapes the cache directory. Every branch of clone_or_pull reached this way is either destructive or attacker-controlled (all bounded by the server process's permissions):

CWE

• CWE-22 (Improper Limitation of a Pathname to a Restricted Directory)

Severity

• High — CVSS 9.1 Critical on the shipped Docker/HTTP default deployment (Dockerfile.mcp binds 0.0.0.0:8765, no transport auth — the tool is then callable by any network peer, no prompt injection needed); CVSS 8.0 High on local stdio (prompt injection → agent tool call, UI:R). Full per-deployment derivation in the linked report.

File(s)

• src/skill_seekers/services/git_repo.py — clone_or_pull (root cause: cache_dir / source_name with no validation; two shutil.rmtree sites: line 75 gated on refresh, and lines 96-98 firing on any pull failure)
• src/skill_seekers/mcp/tools/source_tools.py — fetch_config_tool MODE 2 (temp_{config_name})
• src/skill_seekers/mcp/server_legacy.py:1289 — MODE 2, identical pattern
• Same-class write-side join: config_file = dest_path / f"{config_name}.json" (currently gated by get_config's basename match; the fix should cover it too)

Impact

All attack parameters (config_name, git_url, branch, token, refresh) are in the tool schema of both shipped servers (FastMCP: server_fastmcp.py:1080-1089; legacy: server_legacy.py:394-441). validate_git_url accepts plain http://, so the cloned content source is fully attacker-chosen. PoC-confirmed primitives (offline, sandboxed, drives the real fetch_config_tool; verified on Windows 11 and Ubuntu/WSL):

A. Arbitrary directory write — attacker-chosen git repo (contents + filenames) cloned to any nonexistent/empty path:

[attack A] config_name = 'x/../../victim/.vscode/extensions/evil-1.0.0'
[attack A] attacker file planted OUTSIDE cache: True -> ...ext.js

B. Arbitrary directory delete + replace (refresh=true + existing target) — rmtree destroys it, clone_from refills with attacker content:

[attack B] before: ['db.sqlite', 'important.txt']
[attack B] after : ['.git', 'ext.js', 'package.json']

C. Any existing git repository at the traversed path is destroyed — refresh NOT required. With any non-None token: origin.set_url(attacker URL) → origin.pull → pull fails (unrelated histories) → the except at git_repo.py:96-98 unconditionally rmtrees the victim's repo:

[attack C2] committed victim repo before: README.md + 26 .git files
[attack C2] repo after (Windows): README.md present, 22/26 .git files missing  # history unrecoverable
[attack C2] repo after (POSIX): fully deleted

The attack surface for C is every git repository at a guessable path, reachable through a tool whose documented purpose is fetching configuration. I have additional load-time escalation analysis for primitive A (deployment-dependent); happy to share it here or privately — @yusufkaraaslan please ping me.

Precedent & contrast

This is the same class as #325 / #326 (CWE-22 in workflow_tools.py, fixed with _validate_name + tests) — the git-tools surface shipped it in v2.2.0 and was missed. The input class is already validated elsewhere in the codebase, which pins this as an oversight rather than a design choice:

# src/skill_seekers/services/config_publisher.py:125-130 (backing push_config)
# Validate config_name to prevent path traversal
if "/" in config_name or "\\" in config_name or ".." in config_name:
    raise ValueError(...)

fetch_config MODE 1 (named sources) also inherits safety from SourceManager.add_source's name validation — only MODE 2 (git URL) bypasses it.

PoC

poc_skillseekers_traversal.py — full security report + PoC in this gist. Offline (file:// attacker repo, no network), fully sandboxed (own temp workspace), asserts on filesystem state (never on tool success), exits non-zero on any failure. To run: clone this repo to ./skill-seekers next to the PoC, pip install GitPython httpx requests pyyaml, python poc_skillseekers_traversal.py.

Suggested fix

Single choke point plus defense in depth, mirroring _validate_name from #326 (reuse it, moved to a shared helper):

def _safe_path_segment(name: str) -> bool:
    """True if `name` can be used as a single filesystem path segment."""
    return (
        bool(name)
        and not name.startswith(".")            # no hidden dirs / '..' prefix
        and "/" not in name and "\\" not in name
        and ":" not in name                      # no drive letters
        and ".." not in name
    )

1. GitConfigRepo.clone_or_pull (root cause): reject source_name failing the segment check before constructing repo_path — protects every caller, present and future.
2. fetch_config_tool / legacy MODE 2: validate config_name before building source_name_temp and before the config_file join.
3. Regression tests mirroring the PoC: x/../ head segment (must work on Windows — a leading .. is invalid there once prefixed with temp_), rmtree-then-replace with refresh=true, pull-failure rmtree of an existing repo without refresh, extension-directory planting, and the config_file join.

Affected versions: >= 2.2.0 (git URL mode introduced in v2.2.0) through current HEAD. Happy to help verify the fix or review the PR. Reported by @Harmenszoon.