What
normalize_bundle_rel_path rejects ./../empty components but not absolute paths. A leading / slips through because PurePosixPath("/tmp/x").parts[0] == "/", which is not in the {"", ".", ".."} reject set, and the function returns //tmp/x. write_skill_bundle then builds each destination as root / Path(rel_path); pathlib discards root when the right operand is absolute, so the bytes are written to the absolute path. At HEAD bf4dc2ee: skillclaw/skill_bundle.py:36 (the check) and :154-156 (the join + write_bytes). This is reached on a skill pull: pull_skills (skill_hub.py:549) calls write_skill_bundle at :677 with the bundle from the hub manifest.
How to reproduce
Against the unmodified package — .. is blocked, an absolute key is not, and the write escapes the skills tree:
import tempfile, os, shutil
from skillclaw.skill_bundle import write_skill_bundle, normalize_bundle_rel_path, SkillBundleError
nonce = "9f3a21" # fresh
canary = f"/tmp/sk_canary_{nonce}"
if os.path.exists(canary): os.remove(canary)
skill_dir = tempfile.mkdtemp(prefix="sk_tree_")
try:
normalize_bundle_rel_path("../evil") # blocked
except SkillBundleError:
pass
print(normalize_bundle_rel_path(canary)) # -> '//tmp/sk_canary_9f3a21' (not rejected)
write_skill_bundle(skill_dir, {
"SKILL.md": b"---\nname: innocent\n---\n",
canary: b"escaped the skills dir", # absolute bundle key
})
print("outside:", os.path.exists(canary)) # -> True
print("inside :", os.path.exists(os.path.join(skill_dir, canary.lstrip("/")))) # -> False
os.remove(canary); shutil.rmtree(skill_dir, ignore_errors=True)
Observed: normalize('../evil') raises; normalize('/tmp/sk_canary_9f3a21') returns //tmp/sk_canary_9f3a21; the canary is created at /tmp/sk_canary_9f3a21 while SKILL.md stays inside the skills tree. The same escape is reached when pull_skills pulls a manifest whose files[].path (or a crafted zip entry name) is absolute.
Impact / scope
Arbitrary file write with the SkillClaw process's filesystem permissions. Precondition: the attacker can publish or poison a bundle/manifest the victim pulls — a compromised shared hub, a tainted object store, or a forged manifest.jsonl files[].path. This is a supply-chain / local-delivery position, not an unauthenticated remote vector (sharing is off by default). The write itself does not execute the planted file; reaching code execution needs a separate step where the agent later opens or runs the planted path.
Suggested change
In normalize_bundle_rel_path, reject absolute values — add if PurePosixPath(value).is_absolute(): raise SkillBundleError(...) alongside the existing ./.. check. As defense-in-depth, after path = root / Path(rel_path) require path.resolve() to stay under root.resolve() before writing. Happy to open a PR.
What
normalize_bundle_rel_pathrejects./../empty components but not absolute paths. A leading/slips through becausePurePosixPath("/tmp/x").parts[0] == "/", which is not in the{"", ".", ".."}reject set, and the function returns//tmp/x.write_skill_bundlethen builds each destination asroot / Path(rel_path);pathlibdiscardsrootwhen the right operand is absolute, so the bytes are written to the absolute path. At HEADbf4dc2ee:skillclaw/skill_bundle.py:36(the check) and:154-156(the join +write_bytes). This is reached on a skill pull:pull_skills(skill_hub.py:549) callswrite_skill_bundleat:677with the bundle from the hub manifest.How to reproduce
Against the unmodified package —
..is blocked, an absolute key is not, and the write escapes the skills tree:Observed:
normalize('../evil')raises;normalize('/tmp/sk_canary_9f3a21')returns//tmp/sk_canary_9f3a21; the canary is created at/tmp/sk_canary_9f3a21whileSKILL.mdstays inside the skills tree. The same escape is reached whenpull_skillspulls a manifest whosefiles[].path(or a crafted zip entry name) is absolute.Impact / scope
Arbitrary file write with the SkillClaw process's filesystem permissions. Precondition: the attacker can publish or poison a bundle/manifest the victim pulls — a compromised shared hub, a tainted object store, or a forged
manifest.jsonlfiles[].path. This is a supply-chain / local-delivery position, not an unauthenticated remote vector (sharing is off by default). The write itself does not execute the planted file; reaching code execution needs a separate step where the agent later opens or runs the planted path.Suggested change
In
normalize_bundle_rel_path, reject absolute values — addif PurePosixPath(value).is_absolute(): raise SkillBundleError(...)alongside the existing./..check. As defense-in-depth, afterpath = root / Path(rel_path)requirepath.resolve()to stay underroot.resolve()before writing. Happy to open a PR.