fix(skills): validate skill names and quote provisioning paths
test / unit (push) Successful in 55s
test / integration (push) Successful in 23s
test / coverage (push) Successful in 1m11s
Update Quality Badges / update-badges (push) Successful in 1m3s
lint / lint (push) Successful in 2m18s

Skill names become host/guest path segments interpolated into the
`bottle.exec` shell strings in each contrib provider's provision_skills.
They were validated only as strings, so a name with shell metacharacters
or path traversal could reach the command.

Layer two defenses:
  - Primary: reject any skill name that isn't kebab-case
    ([a-z][a-z0-9-]*) at manifest load, reusing the convention already
    enforced on bottle/agent filenames (new is_valid_entity_name helper
    in manifest_schema). Fails loud and early, protecting every consumer
    of the name — not just the exec call sites.
  - Failsafe: shlex.quote the interpolated skills_dir / dst paths in the
    claude, codex, and pi providers, so a future unvalidated field can't
    inject shell metacharacters even if it bypasses the load-time check.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NkwFXLFff9PYPy4wgVBJp9
This commit is contained in:
2026-06-27 02:15:30 -04:00
parent f787764364
commit 94eca35b4f
6 changed files with 56 additions and 11 deletions
+7 -3
View File
@@ -217,7 +217,7 @@ class ClaudeAgentProvider(AgentProvider):
if not agent.skills:
return
skills_dir = _skills_dir(plan.guest_home)
bottle.exec(f"mkdir -p {skills_dir}", user="root")
bottle.exec(f"mkdir -p {shlex.quote(skills_dir)}", user="root")
for name in agent.skills:
src = host_skill_dir(name)
if not os.path.isdir(src):
@@ -227,9 +227,13 @@ class ClaudeAgentProvider(AgentProvider):
)
dst = f"{skills_dir}/{name}"
info(f"copying skill {name} into {bottle.name}:{dst}")
bottle.exec(f"rm -rf {dst} && mkdir -p {dst}", user="root")
# Defense in depth: skill names are validated kebab-case at
# manifest load, but quote the path so a future unvalidated
# field can't inject shell metacharacters here either.
dst_q = shlex.quote(dst)
bottle.exec(f"rm -rf {dst_q} && mkdir -p {dst_q}", user="root")
bottle.cp_in(f"{src}/.", f"{dst}/")
bottle.exec(f"chown -R node:node {dst}", user="root")
bottle.exec(f"chown -R node:node {dst_q}", user="root")
def provision_prompt(self, plan: "BottlePlan", bottle: "Bottle") -> str | None:
"""Copy the prompt file into the guest, fix ownership/mode.
+7 -3
View File
@@ -183,7 +183,7 @@ class CodexAgentProvider(AgentProvider):
if not agent.skills:
return
skills_dir = _skills_dir(plan.guest_home)
bottle.exec(f"mkdir -p {skills_dir}", user="root")
bottle.exec(f"mkdir -p {shlex.quote(skills_dir)}", user="root")
for name in agent.skills:
src = host_skill_dir(name)
if not os.path.isdir(src):
@@ -193,9 +193,13 @@ class CodexAgentProvider(AgentProvider):
)
dst = f"{skills_dir}/{name}"
info(f"copying skill {name} into {bottle.name}:{dst}")
bottle.exec(f"rm -rf {dst} && mkdir -p {dst}", user="root")
# Defense in depth: skill names are validated kebab-case at
# manifest load, but quote the path so a future unvalidated
# field can't inject shell metacharacters here either.
dst_q = shlex.quote(dst)
bottle.exec(f"rm -rf {dst_q} && mkdir -p {dst_q}", user="root")
bottle.cp_in(f"{src}/.", f"{dst}/")
bottle.exec(f"chown -R node:node {dst}", user="root")
bottle.exec(f"chown -R node:node {dst_q}", user="root")
def provision_prompt(self, plan: "BottlePlan", bottle: "Bottle") -> str | None:
"""Copy the prompt file into the guest, fix ownership/mode.
+7 -3
View File
@@ -238,7 +238,7 @@ class PiAgentProvider(AgentProvider):
if not agent.skills:
return
skills_dir = _skills_dir(plan.guest_home)
bottle.exec(f"mkdir -p {skills_dir}", user="root")
bottle.exec(f"mkdir -p {shlex.quote(skills_dir)}", user="root")
for name in agent.skills:
src = host_skill_dir(name)
if not os.path.isdir(src):
@@ -248,9 +248,13 @@ class PiAgentProvider(AgentProvider):
)
dst = f"{skills_dir}/{name}"
info(f"copying skill {name} into {bottle.name}:{dst}")
bottle.exec(f"rm -rf {dst} && mkdir -p {dst}", user="root")
# Defense in depth: skill names are validated kebab-case at
# manifest load, but quote the path so a future unvalidated
# field can't inject shell metacharacters here either.
dst_q = shlex.quote(dst)
bottle.exec(f"rm -rf {dst_q} && mkdir -p {dst_q}", user="root")
bottle.cp_in(f"{src}/.", f"{dst}/")
bottle.exec(f"chown -R node:node {dst}", user="root")
bottle.exec(f"chown -R node:node {dst_q}", user="root")
def provision_prompt(self, plan: "BottlePlan", bottle: "Bottle") -> str | None:
prompt_path = _prompt_path(plan.guest_home)