fix(transport): validate skill names before they enter --allowedTools

Skill names from ClaudeAgentOptions(skills=[...]) were formatted into the
--allowedTools value unchecked. The CLI splits that value into rules on
commas and spaces outside parentheses, and its tokenizer honors no escape
sequences, so a name carrying one of those delimiters cannot be passed
through reliably. Reject those at the transport boundary, along with
wildcard forms and names that parse but can never match the listed skill.

Ordinary names are unaffected, including plugin-qualified, interior-space,
and non-ASCII names.

Author: ant-kurt

src/claude_agent_sdk/_internal/transport/subprocess_cli.py

@@ -55,6 +55,88 @@ 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.
+_SKILL_NAME_INVALID_CHARS = re.compile(r"[(),\x00-\x1f\x7f-\x9f]")
+
+# 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_bare_string_skills(skills: object) -> None:
+    """Reject a string in place of a list, which would iterate as characters."""
+    if isinstance(skills, str) and skills != "all":
+        raise TypeError(
+            "ClaudeAgentOptions.skills must be a list of skill names or"
+            f' "all", got the string {skills!r}. Did you mean [{skills!r}]?'
+        )
+
+
+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, and control"
+            " characters 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 +511,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 +526,14 @@ def _apply_skills_defaults(
         skills = self._options.skills
         if skills is None:
             return allowed_tools, setting_sources
+        _reject_bare_string_skills(skills)
 
         if 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)

tests/test_transport.py

@@ -666,6 +666,184 @@ 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()
+
+    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", [" 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"),
         [