fix(transport): validate skill names in ClaudeAgentOptions.skills

src/claude_agent_sdk/_internal/transport/subprocess_cli.py

@@ -21,7 +21,12 @@
 from ..._errors import CLIConnectionError, CLINotFoundError, ProcessError
 from ..._errors import CLIJSONDecodeError as SDKJSONDecodeError
 from ..._version import __version__
-from ...types import ClaudeAgentOptions, SystemPromptFile, SystemPromptPreset
+from ...types import (
+    _SKILLS_ALL,
+    ClaudeAgentOptions,
+    SystemPromptFile,
+    SystemPromptPreset,
+)
 from .._task_compat import TaskHandle, spawn_detached
 from . import Transport
 
@@ -55,6 +60,98 @@ def _kill_active_children() -> None:
 atexit.register(_kill_active_children)
 
 
+# Parentheses and commas are delimiters to the --allowedTools tokenizer;
+# control characters (C0, DEL, C1) never appear in a skill directory name.
+# U+FEFF is here rather than with the whitespace check below because the
+# CLI trims it as whitespace and Python's str.strip() does not.
+_SKILL_NAME_INVALID_CHARS = re.compile(r"[(),\x00-\x1f\x7f-\x9f\ufeff]")
+
+# Every surrogate in a Python str is unpaired by construction: a well-formed
+# astral character is a single code point, not a pair.
+_SURROGATE_RE = re.compile("[\ud800-\udfff]")
+
+
+def _reject_non_list_skills(skills: object) -> None:
+    """Reject values other than a list or "all".
+
+    A string iterates as characters, and any other iterable builds rules
+    here but is dropped from the initialize request, which installs no
+    skill filter at all.
+    """
+    if isinstance(skills, list) or skills == _SKILLS_ALL:
+        return
+    suggestion = f" Did you mean [{skills!r}]?" if isinstance(skills, str) else ""
+    raise TypeError(
+        "ClaudeAgentOptions.skills must be a list of skill names or"
+        f' "all", got {skills!r}.{suggestion}'
+    )
+
+
+def _validate_skill_name(name: str) -> None:
+    """Reject skill names that cannot ride safely in a ``Skill(name)`` rule.
+
+    Names from ``options.skills`` are formatted into the ``--allowedTools``
+    value, which the CLI splits into rules on commas and spaces outside
+    parentheses. That tokenizer does not honor escape sequences -- escaping
+    exists only in the per-rule grammar, applied after splitting -- so a
+    name carrying a delimiter cannot be passed through reliably: what it
+    tokenizes into depends on what surrounds it.
+
+    Names that tokenize cleanly but can never match the listed skill are
+    rejected too, so a dead rule fails loudly here instead of silently
+    granting nothing. Each check below states its own reason.
+    """
+    if not isinstance(name, str):
+        raise TypeError(
+            f"Skill names must be strings, got {type(name).__name__}: {name!r}"
+        )
+    if not name.strip():
+        raise ValueError("Skill names must be non-empty strings")
+    if _SURROGATE_RE.search(name):
+        raise ValueError(
+            f"Invalid skill name {name!r}: contains a surrogate code point,"
+            " which can never match a skill the CLI discovered."
+        )
+    if name != name.strip():
+        raise ValueError(
+            f"Invalid skill name {name!r}: leading or trailing whitespace"
+            " can never match — the Skill tool trims the invoked name."
+        )
+    if _SKILL_NAME_INVALID_CHARS.search(name):
+        raise ValueError(
+            f"Invalid skill name {name!r}: parentheses, commas, control"
+            " characters, and byte-order marks are not allowed. Names match"
+            " the skill's directory name, or 'plugin:skill' for"
+            " plugin-qualified skills."
+        )
+    if name == "*":
+        raise ValueError(
+            "Invalid skill name '*': use skills=\"all\" to enable every skill."
+        )
+    if name.endswith(":*") or name.endswith(" *"):
+        raise ValueError(
+            f"Invalid skill name {name!r}: wildcard-suffix names are not"
+            " allowed; list each skill by its exact name."
+        )
+    if name.startswith("/"):
+        raise ValueError(
+            f"Invalid skill name {name!r}: skill names may not start with"
+            " '/'. The skills option takes the canonical name, not the"
+            " slash-command form."
+        )
+    if "\\\\" in name:
+        raise ValueError(
+            f"Invalid skill name {name!r}: consecutive backslashes are not"
+            " allowed — the per-rule parser collapses them, so the rule"
+            " would name a different skill."
+        )
+    if name.endswith("\\"):
+        raise ValueError(
+            f"Invalid skill name {name!r}: names may not end with an"
+            " unpaired backslash."
+        )
+
+
 class _LineFramer:
     """Reassembles complete lines from a stream that yields arbitrary chunks.
 
@@ -429,6 +526,9 @@ def _apply_skills_defaults(
         unset so the CLI discovers installed skills without the caller having
         to wire up both options manually. ``None`` is a no-op.
 
+        Each listed skill name is validated before being formatted into a
+        rule; see :func:`_validate_skill_name`.
+
         Does not mutate the original options object.
         """
         allowed_tools: list[str] = list(self._options.allowed_tools)
@@ -441,12 +541,14 @@ def _apply_skills_defaults(
         skills = self._options.skills
         if skills is None:
             return allowed_tools, setting_sources
+        _reject_non_list_skills(skills)
 
-        if skills == "all":
+        if skills == _SKILLS_ALL:
             if "Skill" not in allowed_tools:
                 allowed_tools.append("Skill")
         else:
             for name in skills:
+                _validate_skill_name(name)
                 pattern = f"Skill({name})"
                 if pattern not in allowed_tools:
                     allowed_tools.append(pattern)

src/claude_agent_sdk/types.py

@@ -5,7 +5,7 @@
 from collections.abc import Awaitable, Callable
 from dataclasses import dataclass, field
 from pathlib import Path
-from typing import TYPE_CHECKING, Any, Literal, Protocol, TypeAlias
+from typing import TYPE_CHECKING, Any, Final, Literal, Protocol, TypeAlias
 
 if sys.version_info >= (3, 11):
     from typing import NotRequired, Required, TypedDict
@@ -31,6 +31,9 @@
 
 # Agent definitions
 SettingSource = Literal["user", "project", "local"]
+
+# The ClaudeAgentOptions.skills value meaning "every discovered skill".
+_SKILLS_ALL: Final = "all"
 EffortLevel: TypeAlias = Literal["low", "medium", "high", "xhigh", "max"]
 
 
@@ -1745,7 +1748,7 @@ def _warn_if_can_use_tool_shadowed(options: "ClaudeAgentOptions") -> None:
     # allowed_tools, so it shadows the callback just like a hand-written entry.
     # skills=[names] appends Skill(name) specifiers, which do not.
     allowed_tools = options.allowed_tools
-    if options.skills == "all" and "Skill" not in allowed_tools:
+    if options.skills == _SKILLS_ALL and "Skill" not in allowed_tools:
         allowed_tools = [*allowed_tools, "Skill"]
     message = _get_can_use_tool_shadowed_warning(options.permission_mode, allowed_tools)
     if message is not None:
@@ -2009,6 +2012,8 @@ class ClaudeAgentOptions:
     - ``"all"``: enable every discovered skill.
     - ``list[str]``: enable only the listed skills. Names match the SKILL.md
       ``name`` / directory name, or ``plugin:skill`` for plugin-qualified skills.
+      Names must be exact: wildcards, delimiters, and surrounding whitespace
+      raise at ``connect()``.
 
     This is a **context filter**, not a sandbox: unlisted skills are hidden
     from the model's listing and rejected by the Skill tool, but their files

tests/test_transport.py

@@ -666,6 +666,209 @@ def test_build_command_skills_does_not_duplicate_entries(self):
         cmd = transport._build_command()
         assert cmd[cmd.index("--allowedTools") + 1] == "Skill(pdf)"
 
+    @pytest.mark.parametrize(
+        "hostile_name",
+        [
+            # Names containing rule-syntax delimiters cannot be represented
+            # as a single Skill(name) entry and must be rejected, never
+            # formatted into the --allowedTools value.
+            "x),Bash(*",
+            "safe),Bash,Skill(dummy",
+            "name,with,commas",
+            "unbalanced(",
+            "unbalanced)",
+            "()",
+        ],
+    )
+    def test_build_command_skills_rejects_rule_syntax_delimiters(
+        self, hostile_name: str
+    ):
+        """Skill names containing rule-syntax delimiters raise ValueError."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills=[hostile_name]),
+        )
+        with pytest.raises(ValueError, match="Invalid skill name"):
+            transport._build_command()
+
+    @pytest.mark.parametrize(
+        "hostile_name",
+        ["with\nnewline", "with\ttab", "nul\x00byte", "del\x7fchar"],
+    )
+    def test_build_command_skills_rejects_control_characters(self, hostile_name: str):
+        """Skill names containing control characters raise ValueError."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills=[hostile_name]),
+        )
+        with pytest.raises(ValueError, match="Invalid skill name"):
+            transport._build_command()
+
+    @pytest.mark.parametrize("empty_name", ["", " ", "  \t "])
+    def test_build_command_skills_rejects_empty_names(self, empty_name: str):
+        """Empty or whitespace-only skill names raise ValueError."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills=[empty_name]),
+        )
+        with pytest.raises(ValueError, match="non-empty"):
+            transport._build_command()
+
+    def test_build_command_skills_rejects_non_string_names(self):
+        """Non-string entries in the skills list raise TypeError."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills=[42]),  # type: ignore[list-item]
+        )
+        with pytest.raises(TypeError, match="must be strings"):
+            transport._build_command()
+
+    @pytest.mark.parametrize(
+        "wildcard_name",
+        ["pdf:*", "my skill *", ":*"],
+    )
+    def test_build_command_skills_rejects_wildcard_suffix_names(
+        self, wildcard_name: str
+    ):
+        """Wildcard-suffix skill names raise ValueError."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills=[wildcard_name]),
+        )
+        with pytest.raises(ValueError, match="wildcard-suffix"):
+            transport._build_command()
+
+    @pytest.mark.parametrize("skills", ["pdf", "pdf-tools", "ALL"])
+    def test_build_command_skills_rejects_a_bare_string(self, skills: str):
+        """A string is iterable, so skills="pdf" would build Skill(p),Skill(d),..."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills=skills),  # type: ignore[arg-type]
+        )
+        with pytest.raises(TypeError, match="must be a list of skill names"):
+            transport._build_command()
+
+    @pytest.mark.parametrize(
+        "skills",
+        [("pdf",), {"pdf"}, (n for n in ["pdf"])],
+        ids=["tuple", "set", "generator"],
+    )
+    def test_build_command_skills_rejects_non_list_iterables(self, skills):
+        """These build Skill(name) rules but are dropped from initialize, so the
+        session skill filter is never installed."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills=skills),  # type: ignore[arg-type]
+        )
+        with pytest.raises(TypeError, match="must be a list of skill names"):
+            transport._build_command()
+
+    def test_build_command_skills_rejects_bare_wildcard(self):
+        """A literal '*' name raises, pointing to the skills="all" option."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills=["*"]),
+        )
+        with pytest.raises(ValueError, match='use skills="all"'):
+            transport._build_command()
+
+    def test_build_command_skills_rejects_unpaired_trailing_backslash(self):
+        """A name ending in a single trailing backslash raises ValueError."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills=["name\\"]),
+        )
+        with pytest.raises(ValueError, match="unpaired backslash"):
+            transport._build_command()
+
+    @pytest.mark.parametrize("hostile_name", ["name\\\\", "name\\\\\\", "mid\\\\dle"])
+    def test_build_command_skills_rejects_consecutive_backslashes(
+        self, hostile_name: str
+    ):
+        """Consecutive backslashes collapse at parse time, renaming the skill."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills=[hostile_name]),
+        )
+        with pytest.raises(ValueError, match="consecutive backslashes"):
+            transport._build_command()
+
+    @pytest.mark.parametrize("hostile_name", ["\ufeffpdf", "pdf\ufeff"])
+    def test_build_command_skills_rejects_byte_order_marks(self, hostile_name: str):
+        """The CLI trims U+FEFF as whitespace; Python's str.strip() does not."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills=[hostile_name]),
+        )
+        with pytest.raises(ValueError, match="Invalid skill name"):
+            transport._build_command()
+
+    @pytest.mark.parametrize("hostile_name", [" pdf", "pdf ", "\tpdf", " pdf "])
+    def test_build_command_skills_rejects_surrounding_whitespace(
+        self, hostile_name: str
+    ):
+        """A padded rule can never match: the Skill tool trims before matching."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills=[hostile_name]),
+        )
+        with pytest.raises(ValueError, match="whitespace"):
+            transport._build_command()
+
+    @pytest.mark.parametrize("hostile_name", ["/pdf", "/myplugin:pdf"])
+    def test_build_command_skills_rejects_leading_slash(self, hostile_name: str):
+        """The session allowlist matches verbatim, so '/pdf' hides every skill."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills=[hostile_name]),
+        )
+        with pytest.raises(ValueError, match="may not start with"):
+            transport._build_command()
+
+    @pytest.mark.parametrize("hostile_name", ["lone\ud800surrogate", "\udc00leading"])
+    def test_build_command_skills_rejects_surrogate_code_points(
+        self, hostile_name: str
+    ):
+        """No CLI-discovered skill name contains a surrogate code point."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills=[hostile_name]),
+        )
+        with pytest.raises(ValueError, match="surrogate"):
+            transport._build_command()
+
+    @pytest.mark.parametrize("hostile_name", ["nel\u0085end", "csi\u009bend"])
+    def test_build_command_skills_rejects_c1_control_characters(
+        self, hostile_name: str
+    ):
+        """Names containing C1 control characters raise ValueError."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills=[hostile_name]),
+        )
+        with pytest.raises(ValueError, match="Invalid skill name"):
+            transport._build_command()
+
+    @pytest.mark.parametrize(
+        "benign_name",
+        [
+            "pdf-tools",
+            "my_skill.v2",
+            "myplugin:pdf",
+            "skill with spaces",
+            "dir\\sub",
+            "日本語スキル",
+        ],
+    )
+    def test_build_command_skills_accepts_ordinary_names(self, benign_name: str):
+        """Ordinary names -- plugin-qualified, spaced, non-ASCII -- still work."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills=[benign_name]),
+        )
+        cmd = transport._build_command()
+        assert cmd[cmd.index("--allowedTools") + 1] == f"Skill({benign_name})"
+
     @pytest.mark.parametrize(
         ("skills", "extra", "want_tools", "want_sources", "want_init_skills"),
         [