From e6756f16a61227a11d154e00b7bae0e585a73970 Mon Sep 17 00:00:00 2001 From: ant-kurt <209710463+ant-kurt@users.noreply.github.com> Date: Thu, 23 Jul 2026 18:58:07 +0000 Subject: [PATCH 1/3] 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. --- .../_internal/transport/subprocess_cli.py | 87 +++++++++ tests/test_transport.py | 178 ++++++++++++++++++ 2 files changed, 265 insertions(+) diff --git a/src/claude_agent_sdk/_internal/transport/subprocess_cli.py b/src/claude_agent_sdk/_internal/transport/subprocess_cli.py index 2bfd4703..0101ef61 100644 --- a/src/claude_agent_sdk/_internal/transport/subprocess_cli.py +++ b/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) diff --git a/tests/test_transport.py b/tests/test_transport.py index 4c1a5abc..56224142 100644 --- a/tests/test_transport.py +++ b/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"), [ From 1e2b7b5afb54a1ce612c532a0ee26aa27ea7b6d3 Mon Sep 17 00:00:00 2001 From: ant-kurt <209710463+ant-kurt@users.noreply.github.com> Date: Thu, 23 Jul 2026 20:17:46 +0000 Subject: [PATCH 2/3] refactor: share one constant for the skills="all" sentinel Also document in the skills docstring that listed names must be exact. --- .../_internal/transport/subprocess_cli.py | 11 ++++++++--- src/claude_agent_sdk/types.py | 9 +++++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/claude_agent_sdk/_internal/transport/subprocess_cli.py b/src/claude_agent_sdk/_internal/transport/subprocess_cli.py index 0101ef61..1c0b13fb 100644 --- a/src/claude_agent_sdk/_internal/transport/subprocess_cli.py +++ b/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 @@ -66,7 +71,7 @@ def _kill_active_children() -> None: 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": + if isinstance(skills, str) and skills != _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}]?' @@ -528,7 +533,7 @@ def _apply_skills_defaults( return allowed_tools, setting_sources _reject_bare_string_skills(skills) - if skills == "all": + if skills == _SKILLS_ALL: if "Skill" not in allowed_tools: allowed_tools.append("Skill") else: diff --git a/src/claude_agent_sdk/types.py b/src/claude_agent_sdk/types.py index c5fb8689..32bfbb71 100644 --- a/src/claude_agent_sdk/types.py +++ b/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 From 905527dbdc9a45999fcf7f0a5916fd56b2229fa6 Mon Sep 17 00:00:00 2001 From: ant-kurt <209710463+ant-kurt@users.noreply.github.com> Date: Thu, 23 Jul 2026 20:30:42 +0000 Subject: [PATCH 3/3] fix(transport): reject non-list skills values and byte-order marks A tuple or generator built Skill(name) rules but was dropped from the initialize request, so no skill filter was installed. U+FEFF is whitespace to the CLI's trim but not to str.strip(), so a BOM'd name built a rule that could never match. --- .../_internal/transport/subprocess_cli.py | 34 ++++++++++++------- tests/test_transport.py | 25 ++++++++++++++ 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/claude_agent_sdk/_internal/transport/subprocess_cli.py b/src/claude_agent_sdk/_internal/transport/subprocess_cli.py index 1c0b13fb..1bddfd4d 100644 --- a/src/claude_agent_sdk/_internal/transport/subprocess_cli.py +++ b/src/claude_agent_sdk/_internal/transport/subprocess_cli.py @@ -62,20 +62,29 @@ def _kill_active_children() -> None: # 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]") +# 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_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 != _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 _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: @@ -110,9 +119,10 @@ def _validate_skill_name(name: str) -> None: ) 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." + 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( @@ -531,7 +541,7 @@ def _apply_skills_defaults( skills = self._options.skills if skills is None: return allowed_tools, setting_sources - _reject_bare_string_skills(skills) + _reject_non_list_skills(skills) if skills == _SKILLS_ALL: if "Skill" not in allowed_tools: diff --git a/tests/test_transport.py b/tests/test_transport.py index 56224142..ce700876 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -748,6 +748,21 @@ def test_build_command_skills_rejects_a_bare_string(self, skills: str): 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( @@ -778,6 +793,16 @@ def test_build_command_skills_rejects_consecutive_backslashes( 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