diff --git a/python/packages/core/AGENTS.md b/python/packages/core/AGENTS.md index 84f8d047015..77dec76dc26 100644 --- a/python/packages/core/AGENTS.md +++ b/python/packages/core/AGENTS.md @@ -90,7 +90,8 @@ agent_framework/ - **`SkillScriptRunner`** - Protocol for file-based script execution. Any callable matching `(skill, script, args) -> Any` satisfies it. Code-defined scripts do not use a runner. - **`SkillScriptArgumentParser`** - Public type alias for an optional callable `(raw args: dict | list[str] | str | None) -> dict | None` that converts the raw `args` value before an `InlineSkillScript` runs (applied before the inline list-args guard). It is an opt-in customization hook (port of .NET PR #6498) that lets callers support backends sending tool-call arguments in a non-conforming shape (e.g. vLLM JSON strings). The output is constrained to a `dict` (named keyword arguments) or `None`, because inline scripts bind arguments by keyword name. Supply it via the `argument_parser=` constructor arg on `InlineSkillScript`, `InlineSkill` (default for scripts added via `@skill.script`), or `ClassSkill` (default for scripts discovered via `@ClassSkill.script`). When `None` (the default), the raw value is used unchanged. File-based scripts are unaffected (their runner owns arg handling). - **`SkillsProvider`** - Context provider (extends `ContextProvider`) that discovers file-based skills from `SKILL.md` files and/or accepts code-defined `Skill` instances. Follows progressive disclosure: advertise → load → read resources / run scripts. By default all three tools it exposes (`load_skill`, `read_skill_resource`, `run_skill_script`) are registered with `approval_mode="always_require"`, so every skill operation needs approval. To run unattended, pass one of the static auto-approval rules to `ToolApprovalMiddleware` (via `auto_approval_rules`): `SkillsProvider.read_only_tools_auto_approval_rule` approves only the read-only tools (`load_skill`, `read_skill_resource`) while still prompting for `run_skill_script`, and `SkillsProvider.all_tools_auto_approval_rule` approves every skill tool including script execution. Both rules reject any call carrying a `server_label` so they stay scoped to this provider's local tools and never auto-approve a same-named hosted tool. Alternatively, for trusted skills, the constructor / `from_paths` kwargs `disable_load_skill_approval`, `disable_read_skill_resource_approval`, and `disable_run_skill_script_approval` (all default `False`) opt individual tools out of approval entirely by registering them with `approval_mode="never_require"` (the auto-approval rules only apply to tools that still require approval). The tool names are also exposed as class constants (`LOAD_SKILL_TOOL_NAME`, `READ_SKILL_RESOURCE_TOOL_NAME`, `RUN_SKILL_SCRIPT_TOOL_NAME`). -- **`SkillsSource` decorators** - Skill sources are composable: `SkillsSource` is the abstract base, with concrete sources (`InMemorySkillsSource`, `FileSkillsSource`, `MCPSkillsSource`) and decorators that wrap an inner source — `AggregatingSkillsSource` (concatenate several sources), `FilteringSkillsSource` (predicate filter), `DeduplicatingSkillsSource` (first-wins by name), and `CachingSkillsSource` (cache the inner source's skills list). `DelegatingSkillsSource` is the abstract base for decorators. **`get_skills` takes a `SkillsSourceContext`**: every source/decorator implements `async def get_skills(self, context: SkillsSourceContext) -> list[Skill]` and forwards `context` to inner sources. `SkillsSourceContext` (frozen) carries the invoking `agent` (`SupportsAgentRun`) and optional `session` (`AgentSession | None`); `SkillsProvider` builds it from `before_run`'s `agent`/`session` and passes it into the pipeline. `FilteringSkillsSource`'s predicate is context-aware: `Callable[[Skill, SkillsSourceContext], bool]` (port of .NET #6797). **Default caching is applied only to the built-in, context-independent leaf sources**: for the `Skill` / sequence-of-skills / `from_paths` constructors, `SkillsProvider` builds `DeduplicatingSkillsSource(CachingSkillsSource())` so expensive filesystem/network discovery runs once. A **caller-supplied `SkillsSource` is used as-is — never auto-wrapped in caching or deduplication** — because auto-caching a context-aware caller source in a single shared bucket would replay the first invocation's skills for later `SkillsSourceContext`s and leak skills across agents/tenants (matches .NET, whose custom-source constructor also adds no caching/dedup). Callers who want caching on a custom pipeline compose `CachingSkillsSource(inner, cache_isolation_key_selector=...)` themselves. `disable_caching=True` only affects the built-in leaf caching (it has no effect on a caller-supplied source, which is never cached). `CachingSkillsSource` shares a single in-flight fetch across concurrent callers (per cache key) and does not update its cache on a failed fetch, so the next call retries (an initial failure leaves the cache empty; a refresh failure keeps the previously cached list). By default all callers share one cache bucket; pass `cache_isolation_key_selector=Callable[[SkillsSourceContext], str | None]` to cache separately per key (e.g. per agent name) for context-aware inner sources — the key should be low-cardinality and stable, and returning `None` (or leaving the selector `None`) uses the shared bucket. By default a cached list never expires; pass `refresh_interval=timedelta(...)` (port of .NET `CachingAgentSkillsSourceOptions.RefreshInterval`) to treat a cached list as stale once it is older than the interval so the next call re-queries the inner source (useful when an inner source such as `MCPSkillsSource` changes over the process lifetime; a zero/negative interval makes every result immediately stale, and a failed refresh keeps the prior list and retries). Freshness is measured with a monotonic clock (`time.monotonic()`). `SkillsProvider.__init__` / `from_paths` expose a `cache_refresh_interval` kwarg that is threaded into the built-in `CachingSkillsSource` (it has no effect on a caller-supplied source or when `disable_caching=True`). +- **`MCPSkillsSource`** - `SkillsSource` that discovers Agent Skills served over MCP by reading the well-known `skill://index.json` (SEP-2640). Index entries are dispatched by their `type` (case-insensitive): `skill-md` entries become one `MCPSkill` each (its `SKILL.md` body and sibling resources are fetched on demand via `resources/read`), and `archive` entries are downloaded as a single ZIP / TAR / gzip-TAR blob, safely unpacked to a local directory, and served like file-based skills via an internal `FileSkillsSource`. Entries whose type has no handler (e.g. `mcp-resource-template`) are skipped. **MCP-delivered scripts are never runnable**: the internal file source is created with `script_extensions=()` and no runner, so bundled scripts surface only as (potentially) readable resources. Archive extraction is hardened (path-traversal / "zip-slip" guard, non-regular TAR member skipping, and file-count / uncompressed-size / download-size limits). Archive behavior is configured with `archive_*` constructor kwargs (`archive_skills_directory`, `archive_resource_extensions`, `archive_resource_search_depth`, `archive_max_file_count`, `archive_max_size_bytes`, `archive_max_uncompressed_size_bytes`) — Python uses plain kwargs, not a `*Options` object as in .NET. The archive loader owns its extraction directory and prunes any subdirectory the server no longer advertises on every discovery (so it runs even when no archive entries are advertised, and two sources must not share a directory). Unlike .NET's `AgentMcpSkillsSourceOptions.RefreshInterval`, this source has no built-in refresh interval; wrap it in `CachingSkillsSource(..., refresh_interval=...)` for caching/refresh. This is a port of .NET PR #6631; the `FileSkillsSource` `script_extensions`/`resource_extensions` kwargs now treat `None` as "use defaults" and an empty tuple as "discover none" (an empty tuple previously fell back to defaults). +- **`SkillsSource` decorators** - Skill sources are composable: `SkillsSource` is the abstract base, with concrete sources (`InMemorySkillsSource`, `FileSkillsSource`, `MCPSkillsSource`) and decorators that wrap an inner source — `AggregatingSkillsSource` (concatenate several sources), `FilteringSkillsSource` (predicate filter), `DeduplicatingSkillsSource` (first-wins by name), and `CachingSkillsSource` (cache the inner source's skills list). `DelegatingSkillsSource` is the abstract base for decorators. **`get_skills` takes a `SkillsSourceContext`**: every source/decorator implements `async def get_skills(self, context: SkillsSourceContext) -> list[Skill]` and forwards `context` to inner sources. `SkillsSourceContext` (frozen) carries the invoking `agent` (`SupportsAgentRun`) and optional `session` (`AgentSession | None`); `SkillsProvider` builds it from `before_run`'s `agent`/`session` and passes it into the pipeline. `FilteringSkillsSource`'s predicate is context-aware: `Callable[[Skill, SkillsSourceContext], bool]` (port of .NET #6797). **Default caching is applied only to the built-in, context-independent leaf sources**: for the `Skill` / sequence-of-skills / `from_paths` constructors, `SkillsProvider` builds `DeduplicatingSkillsSource(CachingSkillsSource())` so expensive filesystem/network discovery runs once. A **caller-supplied `SkillsSource` is used as-is — never auto-wrapped in caching or deduplication** — because auto-caching a context-aware caller source in a single shared bucket would replay the first invocation's skills for later `SkillsSourceContext`s and leak skills across agents/tenants (matches .NET, whose custom-source constructor also adds no caching/dedup). Callers who want caching on a custom pipeline compose `CachingSkillsSource(inner, cache_isolation_key_selector=...)` themselves. `disable_caching=True` only affects the built-in leaf caching (it has no effect on a caller-supplied source, which is never cached). `CachingSkillsSource` shares a single in-flight fetch across concurrent callers (per cache key) and does not update its cache on a failed fetch, so the next call retries (an initial failure leaves the cache empty; a refresh failure keeps the previously cached list). By default all callers share one cache bucket; pass `cache_isolation_key_selector=Callable[[SkillsSourceContext], str | None]` to cache separately per key (e.g. per agent name) for context-aware inner sources — the key should be low-cardinality and stable, and returning `None` (or leaving the selector `None`) uses the shared bucket. By default a cached list never expires; pass `refresh_interval=timedelta(...)` (port of .NET `CachingAgentSkillsSourceOptions.RefreshInterval`) to treat a cached list as stale once it is older than the interval so the next call re-queries the inner source (useful when an inner source such as `MCPSkillsSource` changes over the process lifetime; a zero/negative interval makes every result immediately stale, and a failed refresh keeps the prior list and retries). Freshness is measured with a monotonic clock (`time.monotonic()`). `SkillsProvider.__init__` / `from_paths` expose a `cache_refresh_interval` kwarg that is threaded into the built-in `CachingSkillsSource` (it has no effect on a caller-supplied source or when `disable_caching=True`). `SkillsSourceContext` (frozen) carries the invoking `agent` (`SupportsAgentRun`) and optional `session` (`AgentSession | None`); `SkillsProvider` builds it from `before_run`'s `agent`/`session` and passes it into the pipeline. `FilteringSkillsSource`'s predicate is context-aware: `Callable[[Skill, SkillsSourceContext], bool]` (port of .NET #6797). **Default caching is applied only to the built-in, context-independent leaf sources**: for the `Skill` / sequence-of-skills / `from_paths` constructors, `SkillsProvider` builds `DeduplicatingSkillsSource(CachingSkillsSource())` so expensive filesystem/network discovery runs once. A **caller-supplied `SkillsSource` is used as-is — never auto-wrapped in caching or deduplication** — because auto-caching a context-aware caller source in a single shared bucket would replay the first invocation's skills for later `SkillsSourceContext`s and leak skills across agents/tenants (matches .NET, whose custom-source constructor also adds no caching/dedup). Callers who want caching on a custom pipeline compose `CachingSkillsSource(inner, cache_isolation_key_selector=...)` themselves. `disable_caching=True` only affects the built-in leaf caching (it has no effect on a caller-supplied source, which is never cached). `CachingSkillsSource` shares a single in-flight fetch across concurrent callers (per cache key) and does not update its cache on a failed fetch, so the next call retries (an initial failure leaves the cache empty; a refresh failure keeps the previously cached list). By default all callers share one cache bucket; pass `cache_isolation_key_selector=Callable[[SkillsSourceContext], str | None]` to cache separately per key (e.g. per agent name) for context-aware inner sources — the key should be low-cardinality and stable, and returning `None` (or leaving the selector `None`) uses the shared bucket. By default a cached list never expires; pass `refresh_interval=timedelta(...)` (port of .NET `CachingAgentSkillsSourceOptions.RefreshInterval`) to treat a cached list as stale once it is older than the interval so the next call re-queries the inner source (useful when an inner source such as `MCPSkillsSource` changes over the process lifetime; a zero/negative interval makes every result immediately stale, and a failed refresh keeps the prior list and retries). Freshness is measured with a monotonic clock (`time.monotonic()`). `SkillsProvider.__init__` / `from_paths` expose a `cache_refresh_interval` kwarg that is threaded into the built-in `CachingSkillsSource` (it has no effect on a caller-supplied source or when `disable_caching=True`). ### Model Context Protocol (`_mcp.py`) diff --git a/python/packages/core/agent_framework/_skills.py b/python/packages/core/agent_framework/_skills.py index 41f69ff8899..8f94b3c0eed 100644 --- a/python/packages/core/agent_framework/_skills.py +++ b/python/packages/core/agent_framework/_skills.py @@ -46,19 +46,26 @@ import asyncio import base64 +import gzip import inspect +import io import json import logging import os import re +import shutil +import tarfile import time +import zipfile from abc import ABC, abstractmethod from collections.abc import Callable, Sequence from dataclasses import dataclass from datetime import timedelta +from enum import Enum from html import escape as xml_escape from pathlib import Path, PurePosixPath -from typing import TYPE_CHECKING, Any, ClassVar, Final, Protocol, TypeAlias, TypeVar, cast, runtime_checkable +from typing import IO, TYPE_CHECKING, Any, ClassVar, Final, Protocol, TypeAlias, TypeVar, cast, runtime_checkable +from uuid import uuid4 from ._feature_stage import ExperimentalFeature, experimental from ._sessions import ContextProvider @@ -2837,9 +2844,14 @@ def __init__( is attempted without a runner). resource_extensions: File extensions recognized as discoverable resources. Defaults to - ``(".md", ".json", ".yaml", ".yml", ".csv", ".xml", ".txt")``. + ``(".md", ".json", ".yaml", ".yml", ".csv", ".xml", ".txt")`` + when ``None``. Pass an empty tuple to discover no resources. script_extensions: File extensions recognized as discoverable - scripts. Defaults to ``(".py",)``. + scripts. Defaults to ``(".py",)`` when ``None``. Pass an + empty tuple to disable script discovery entirely (files that + would otherwise be scripts are then only discoverable as + resources); this is used to serve untrusted skills whose + bundled scripts must never be exposed as runnable. search_depth: Maximum depth to search for script and resource files within each skill directory. A value of ``1`` searches only the skill root; ``2`` (the default) searches the root @@ -2862,8 +2874,10 @@ def __init__( self._skill_paths = [str(p) for p in skill_paths] self._script_runner = script_runner - self._resource_extensions = resource_extensions or DEFAULT_RESOURCE_EXTENSIONS - self._script_extensions = script_extensions or DEFAULT_SCRIPT_EXTENSIONS + self._resource_extensions = ( + resource_extensions if resource_extensions is not None else DEFAULT_RESOURCE_EXTENSIONS + ) + self._script_extensions = script_extensions if script_extensions is not None else DEFAULT_SCRIPT_EXTENSIONS if search_depth < 1: raise ValueError(f"search_depth must be >= 1, got {search_depth}") @@ -3975,6 +3989,34 @@ def _mcp_join_text(result: ReadResourceResult) -> str: return "\n".join(c.text for c in result.contents if isinstance(c, _TextResourceContents)) +def _mcp_first_blob(result: ReadResourceResult) -> tuple[bytes, str | None] | None: + """Extract the first binary (blob) resource content from a read result. + + Args: + result: The result returned by the MCP server's ``resources/read`` request. + + Returns: + A ``(data, mime_type)`` tuple for the first :class:`~mcp.types.BlobResourceContents` + in *result*, or ``None`` when the result carries no binary content or the blob + cannot be base64-decoded. + """ + from mcp.types import BlobResourceContents + + for content in result.contents: + if isinstance(content, BlobResourceContents): + blob = content.blob + # Strip a data-URI prefix if present (some servers send full data URIs). + if blob.startswith("data:"): + blob = blob.split(",", 1)[-1] + try: + data = base64.b64decode(blob) + except ValueError: + # binascii.Error (invalid base64) subclasses ValueError. + return None + return data, content.mimeType + return None + + class _McpSkillIndexEntry: # noqa: B903 """A single entry in the ``skill://index.json`` discovery document. @@ -4230,24 +4272,521 @@ def _compute_skill_root_uri(skill_md_uri: str) -> str: return skill_md_uri + "/" +_DEFAULT_ARCHIVE_MAX_FILE_COUNT: Final[int] = 20 +"""Default maximum number of files that may be extracted from a single archive skill.""" + +_DEFAULT_ARCHIVE_MAX_SIZE_BYTES: Final[int] = 1 * 1024 * 1024 +"""Default maximum size, in bytes, of a downloaded archive skill resource.""" + +_DEFAULT_ARCHIVE_MAX_UNCOMPRESSED_SIZE_BYTES: Final[int] = 1 * 1024 * 1024 +"""Default maximum total uncompressed size, in bytes, of all files extracted from an archive skill.""" + +_ARCHIVE_COPY_BUFFER_SIZE: Final[int] = 81920 + + +class _ArchiveFormat(Enum): + """The archive container formats supported by :func:`_extract_archive`.""" + + UNKNOWN = "unknown" + ZIP = "zip" + TAR = "tar" + TAR_GZ = "tar_gz" + + +def _detect_archive_format(data: bytes, media_type: str | None, url: str | None) -> _ArchiveFormat: + """Determine the archive format from magic bytes, media type, and URL suffix. + + Magic-number sniffing takes precedence as it is the most reliable signal; the + advertised media type and finally the URL suffix are used as fallbacks. + + Args: + data: The raw archive bytes. + media_type: The advertised MIME type, if any. + url: The resource URL the archive was read from, if any. + + Returns: + The detected :class:`_ArchiveFormat`, or :attr:`_ArchiveFormat.UNKNOWN`. + """ + if len(data) >= 2 and data[0] == 0x1F and data[1] == 0x8B: + return _ArchiveFormat.TAR_GZ + + if len(data) >= 4 and data[0] == 0x50 and data[1] == 0x4B and data[2] in (0x03, 0x05, 0x07): + return _ArchiveFormat.ZIP + + media = (media_type or "").strip().lower() + if media in ("application/zip", "application/x-zip-compressed"): + return _ArchiveFormat.ZIP + if media in ("application/gzip", "application/x-gzip", "application/x-compressed-tar"): + return _ArchiveFormat.TAR_GZ + if media in ("application/x-tar", "application/tar"): + return _ArchiveFormat.TAR + + lowered = (url or "").lower() + if lowered.endswith(".zip"): + return _ArchiveFormat.ZIP + if lowered.endswith(".tar.gz") or lowered.endswith(".tgz"): + return _ArchiveFormat.TAR_GZ + if lowered.endswith(".tar"): + return _ArchiveFormat.TAR + + return _ArchiveFormat.UNKNOWN + + +def _resolve_archive_destination(target_root: str, entry_path: str) -> str | None: + """Resolve an archive entry path to an absolute destination beneath *target_root*. + + Guards against path-traversal ("zip-slip") attacks: absolute, rooted, or + parent-traversing entry paths, and any entry that resolves outside + *target_root*, are rejected. + + Args: + target_root: The absolute directory the archive is unpacked into. + entry_path: The archive-relative path of the entry. + + Returns: + The absolute destination path, or ``None`` when the entry would escape + *target_root*. + """ + if not entry_path or not entry_path.strip(): + return None + + normalized = entry_path.replace("\\", "/").lstrip("/") + if not normalized or any(segment == ".." for segment in normalized.split("/")): + return None + + destination = os.path.abspath(os.path.join(target_root, normalized)) + if not FileSkillsSource._is_path_within_directory( # pyright: ignore[reportPrivateUsage] + destination, os.path.abspath(target_root) + ): + return None + + return destination + + +def _copy_stream_with_limit(source: IO[bytes], destination: IO[bytes], remaining_bytes: int) -> int: + """Copy *source* to *destination* in chunks while enforcing an uncompressed-byte budget. + + This is the authoritative defense against decompression bombs because it counts + bytes actually produced by the decompressor rather than trusting archive metadata. + Peak memory is bounded to a single buffer. + + Args: + source: The (possibly decompressing) input stream. + destination: The output file stream. + remaining_bytes: The remaining uncompressed-byte budget shared across the archive. + + Returns: + The remaining budget after copying this stream. + + Raises: + ValueError: If the remaining budget is exhausted. + """ + while True: + chunk = source.read(_ARCHIVE_COPY_BUFFER_SIZE) + if not chunk: + break + remaining_bytes -= len(chunk) + if remaining_bytes < 0: + raise ValueError("Skill archive exceeds the maximum allowed uncompressed size.") + destination.write(chunk) + return remaining_bytes + + +def _extract_archive( + data: bytes, + archive_format: _ArchiveFormat, + target_directory: str, + max_file_count: int, + max_uncompressed_size_bytes: int, +) -> None: + """Extract *data* into *target_directory* with path-traversal and size guards. + + Supports ZIP, TAR, and gzip-compressed TAR payloads. Non-regular TAR entries + (symbolic links, hard links, device nodes, etc.) are skipped so an archive cannot + create links that escape the target directory. Extraction is bounded by a maximum + file count and total uncompressed size to mitigate decompression-bomb attacks. + + Args: + data: The raw archive bytes. + archive_format: The detected container format. + target_directory: The directory the archive is unpacked into. Created if missing. + max_file_count: The maximum number of files that may be extracted. + max_uncompressed_size_bytes: The maximum total uncompressed size, in bytes. + + Raises: + ValueError: If the format is unknown or a limit is exceeded. + OSError: If a filesystem operation fails. + tarfile.TarError: If a TAR payload is malformed. + zipfile.BadZipFile: If a ZIP payload is malformed. + gzip.BadGzipFile: If a gzip payload is malformed. + """ + os.makedirs(target_directory, exist_ok=True) + full_target = os.path.abspath(target_directory) + + if archive_format is _ArchiveFormat.ZIP: + with zipfile.ZipFile(io.BytesIO(data)) as archive: + _extract_zip(archive, full_target, max_file_count, max_uncompressed_size_bytes) + elif archive_format is _ArchiveFormat.TAR: + with tarfile.open(fileobj=io.BytesIO(data), mode="r:") as archive: + _extract_tar(archive, full_target, max_file_count, max_uncompressed_size_bytes) + elif archive_format is _ArchiveFormat.TAR_GZ: + with tarfile.open(fileobj=io.BytesIO(data), mode="r:gz") as archive: + _extract_tar(archive, full_target, max_file_count, max_uncompressed_size_bytes) + else: + raise ValueError(f"Unsupported skill archive format '{archive_format}'.") + + +def _extract_zip( + archive: zipfile.ZipFile, + full_target: str, + max_file_count: int, + max_uncompressed_size_bytes: int, +) -> None: + """Extract regular files from a ZIP archive. See :func:`_extract_archive`.""" + remaining_bytes = max_uncompressed_size_bytes + file_count = 0 + + for info in archive.infolist(): + if info.is_dir(): + continue + + file_count += 1 + if file_count > max_file_count: + raise ValueError(f"Skill archive exceeds the maximum allowed file count ({max_file_count}).") + + destination = _resolve_archive_destination(full_target, info.filename) + if destination is None: + continue + + os.makedirs(os.path.dirname(destination), exist_ok=True) + with archive.open(info) as source, open(destination, "wb") as output: + remaining_bytes = _copy_stream_with_limit(source, output, remaining_bytes) + + +def _extract_tar( + archive: tarfile.TarFile, + full_target: str, + max_file_count: int, + max_uncompressed_size_bytes: int, +) -> None: + """Extract regular files from a TAR archive. See :func:`_extract_archive`.""" + remaining_bytes = max_uncompressed_size_bytes + file_count = 0 + + for member in archive: + # Only regular files are materialized. Skipping links/devices avoids both + # unsupported entry types and link-based escapes outside the target directory. + if not member.isreg(): + continue + + file_count += 1 + if file_count > max_file_count: + raise ValueError(f"Skill archive exceeds the maximum allowed file count ({max_file_count}).") + + destination = _resolve_archive_destination(full_target, member.name) + if destination is None: + continue + + source = archive.extractfile(member) + if source is None: + continue + + os.makedirs(os.path.dirname(destination), exist_ok=True) + with source, open(destination, "wb") as output: + remaining_bytes = _copy_stream_with_limit(source, output, remaining_bytes) + + +class _ArchiveEntryLoader: + """Loads ``archive``-type ``skill://index.json`` entries. + + Each entry's ``url`` points to a single archive resource (ZIP, TAR, or + gzip-compressed TAR) whose content unpacks into the skill's namespace. + Archives are downloaded, extracted to a local directory, and discovered via + an internal :class:`FileSkillsSource` created with **no allowed script + extensions and no script runner**, so bundled scripts are surfaced as + read-only resources and never as runnable scripts. + + The loader owns its extraction directory: on every :meth:`load` it prunes any + subdirectory that the MCP server no longer advertises. Two loaders must never + share an extraction directory or they would delete each other's skills. + """ + + def __init__( + self, + client: ClientSession, + *, + archive_skills_directory: str | None, + resource_extensions: tuple[str, ...] | None, + resource_search_depth: int, + max_file_count: int, + max_size_bytes: int, + max_uncompressed_size_bytes: int, + ) -> None: + self._client = client + self._configured_directory = archive_skills_directory + self._resource_extensions = resource_extensions + self._resource_search_depth = resource_search_depth + self._max_file_count = max_file_count + self._max_size_bytes = max_size_bytes + self._max_uncompressed_size_bytes = max_uncompressed_size_bytes + self._archive_skills_directory: str | None = None + + async def load(self, entries: list[_McpSkillIndexEntry], context: SkillsSourceContext) -> list[Skill]: + """Download, extract, and discover skills for every valid archive entry. + + Args: + entries: The ``archive`` index entries. May be empty, in which case any + previously extracted directories are pruned and an empty list is returned. + context: Contextual information forwarded to the internal file source. + + Returns: + A list of discovered :class:`FileSkill` instances. + """ + valid_entries = self._filter_valid_entries(entries) + + base_directory = self._archive_skills_directory or self._configured_directory + self._reconcile_directories(base_directory, valid_entries) + + if not valid_entries: + return [] + + base_directory = self._ensure_base_directory(base_directory) + self._archive_skills_directory = base_directory + + skill_directories: list[str] = [] + for entry in valid_entries: + downloaded = await self._download(entry) + if downloaded is None: + continue + data, mime_type = downloaded + skill_directory = self._extract_entry(entry, base_directory, data, mime_type) + if skill_directory is not None: + skill_directories.append(skill_directory) + + if not skill_directories: + return [] + + file_source = FileSkillsSource( + skill_directories, + script_runner=None, + resource_extensions=self._resource_extensions, + script_extensions=(), + search_depth=self._resource_search_depth, + ) + return await file_source.get_skills(context) + + async def _download(self, entry: _McpSkillIndexEntry) -> tuple[bytes, str | None] | None: + """Download and decode the binary content of a skill's archive resource. + + Only "resource not found" MCP errors are swallowed (the skill is skipped); + every other exception (auth failure, ``INTERNAL_ERROR``, connection drop, + timeout, etc.) propagates so a transient transport failure is not silently + turned into a missing skill — which, under :class:`CachingSkillsSource`, + would otherwise overwrite a previously cached list with a partial result. + + Returns: + A ``(data, mime_type)`` tuple, or ``None`` when the resource is not found, + contains no binary content, is empty, or exceeds the configured size limit. + + Raises: + Exception: Any error other than a "resource not found" MCP error raised + while reading the archive resource is re-raised. + """ + try: + result = await self._client.read_resource(_mcp_any_url(cast(str, entry.url))) + except Exception as ex: + if _is_mcp_resource_not_found(ex): + logger.debug("Archive resource '%s' for skill '%s' not available: %s", entry.url, entry.name, ex) + return None + logger.warning("Failed to read archive resource for skill '%s'.", entry.name, exc_info=True) + raise + + blob = _mcp_first_blob(result) + if blob is None: + logger.debug("Skipping skill '%s': archive resource returned no binary content", entry.name) + return None + + data, mime_type = blob + if not data: + logger.debug("Skipping skill '%s': archive resource returned empty content", entry.name) + return None + + if len(data) > self._max_size_bytes: + logger.debug( + "Skipping skill '%s': archive resource exceeds the maximum allowed size (%d bytes)", + entry.name, + self._max_size_bytes, + ) + return None + + return data, mime_type + + def _extract_entry( + self, + entry: _McpSkillIndexEntry, + base_directory: str, + data: bytes, + mime_type: str | None, + ) -> str | None: + """Detect the format of and extract a single archive entry into its own subdirectory. + + Returns: + The extracted skill directory, or ``None`` when the entry cannot be + materialized (path escape, unsupported format, or extraction error). + """ + skill_directory = os.path.join(base_directory, cast(str, entry.name)) + + # Defense-in-depth: the skill directory is derived from the server-supplied name. + # Invalid names are filtered upstream, but verify the resolved path cannot escape. + if not FileSkillsSource._is_path_within_directory( # pyright: ignore[reportPrivateUsage] + os.path.abspath(skill_directory), os.path.abspath(base_directory) + ): + logger.debug( + "Skipping skill '%s': resolved skill directory escapes the archive skills directory", entry.name + ) + return None + + # Clean the target directory so content is always freshly extracted. + if not self._delete_directory(cast(str, entry.name), skill_directory): + return None + + archive_format = _detect_archive_format(data, mime_type, entry.url) + if archive_format is _ArchiveFormat.UNKNOWN: + logger.debug("Skipping skill '%s': unsupported archive media type '%s'", entry.name, mime_type or "(none)") + return None + + try: + _extract_archive( + data, + archive_format, + skill_directory, + self._max_file_count, + self._max_uncompressed_size_bytes, + ) + except (OSError, ValueError, EOFError, tarfile.TarError, zipfile.BadZipFile, gzip.BadGzipFile): + logger.warning("Failed to extract archive for skill '%s'.", entry.name, exc_info=True) + # Remove partially-extracted content so it is not later mistaken for a valid skill. + self._delete_directory(cast(str, entry.name), skill_directory) + return None + + logger.debug("Extracted archive skill '%s' to '%s'", entry.name, skill_directory) + return skill_directory + + def _ensure_base_directory(self, base_directory: str | None) -> str: + """Resolve the extraction directory (generating a per-instance one if needed) and create it.""" + resolved = base_directory or os.path.join(os.getcwd(), uuid4().hex) + os.makedirs(resolved, exist_ok=True) + return resolved + + def _reconcile_directories(self, base_directory: str | None, valid_entries: list[_McpSkillIndexEntry]) -> None: + """Prune on-disk skill directories the server no longer advertises.""" + if not base_directory or not os.path.isdir(base_directory): + return + + if not valid_entries: + for directory in self._list_subdirectories(base_directory): + self._prune_directory(directory) + return + + advertised = {os.path.normcase(cast(str, entry.name)) for entry in valid_entries} + for directory in self._list_subdirectories(base_directory): + if os.path.normcase(os.path.basename(directory)) in advertised: + continue + self._prune_directory(directory) + + @staticmethod + def _list_subdirectories(base_directory: str) -> list[str]: + """Return the absolute paths of the immediate subdirectories of *base_directory*.""" + try: + return [ + os.path.join(base_directory, name) + for name in os.listdir(base_directory) + if os.path.isdir(os.path.join(base_directory, name)) + ] + except OSError: + return [] + + @staticmethod + def _prune_directory(directory: str) -> None: + """Recursively delete a stale skill directory, swallowing filesystem errors.""" + try: + shutil.rmtree(directory) + logger.debug("Pruned stale archive skill directory '%s'", directory) + except OSError: + logger.warning("Failed to prune stale archive skill directory '%s'.", directory, exc_info=True) + + @staticmethod + def _delete_directory(skill_name: str, directory: str) -> bool: + """Recursively delete a skill directory. Returns ``False`` on failure.""" + try: + if os.path.isdir(directory): + shutil.rmtree(directory) + return True + except OSError: + logger.warning("Failed to clean archive skill directory for '%s'.", skill_name, exc_info=True) + return False + + def _filter_valid_entries(self, entries: list[_McpSkillIndexEntry]) -> list[_McpSkillIndexEntry]: + """Filter to entries with a usable directory name and a URL; log and skip the rest.""" + valid: list[_McpSkillIndexEntry] = [] + for entry in entries: + if not entry.name or not entry.name.strip(): + logger.debug("Skipping archive entry: missing required 'name' field") + continue + if not self._is_valid_directory_name(entry.name): + logger.debug("Skipping entry '%s': name contains invalid characters for a directory name", entry.name) + continue + if not entry.url or not entry.url.strip(): + logger.debug("Skipping entry '%s': missing required 'url' field", entry.name) + continue + valid.append(entry) + return valid + + @staticmethod + def _is_valid_directory_name(name: str) -> bool: + """Return whether *name* is safe to use as a single directory-name segment.""" + if "/" in name or "\\" in name: + return False + if any(ch in name for ch in '<>:"|?*'): + return False + if any(ord(ch) < 32 for ch in name): + return False + # Names consisting solely of dots or whitespace are invalid directory names. + return bool(name.strip().strip(".")) + + @experimental(feature_id=ExperimentalFeature.MCP_SKILLS) class MCPSkillsSource(SkillsSource): """A :class:`SkillsSource` that discovers Agent Skills served over MCP. Discovery follows the SEP-2640 recommended approach: the source reads the well-known ``skill://index.json`` resource and constructs one - :class:`MCPSkill` per ``skill-md`` entry directly from the entry's - ``name``, ``description``, and ``url`` fields. + :class:`Skill` per index entry. + + Index entries are dispatched by their ``type`` (matched case-insensitively): - The referenced ``SKILL.md`` resource is **not** read during discovery; - the host fetches its body on demand via ``resources/read`` when the - skill content is needed. + * ``skill-md`` — one :class:`MCPSkill` is created directly from the entry's + ``name``, ``description``, and ``url`` fields. The referenced ``SKILL.md`` + resource is **not** read during discovery; the host fetches its body on + demand via ``resources/read`` when the skill content is needed. + * ``archive`` — the entry's ``url`` points to a single archive resource + (ZIP, TAR, or gzip-compressed TAR) whose content unpacks into the skill's + namespace. The archive is downloaded, safely extracted to a local + directory, and served like a file-based skill. Scripts bundled inside an + archive are surfaced as read-only resources only; MCP-delivered scripts + are never discovered as runnable. - Only index entries of type ``skill-md`` are supported; entries of any - other type are silently skipped. + Entries whose type has no registered handler (e.g. ``mcp-resource-template``) + are skipped. If ``skill://index.json`` is absent, unreadable, empty, or fails to - parse, this source returns an empty list. + parse, this source returns an empty list (archive directories previously + extracted by this instance are still pruned). + + Archive extraction behavior is configured via the ``archive_*`` constructor + keyword arguments. Because Python's :class:`CachingSkillsSource` decorator + already provides refresh/caching for any source, this source does not offer + a separate refresh interval; wrap it in :class:`CachingSkillsSource` to cache. Security considerations: Discovering skills over MCP means an *external* MCP server controls @@ -4260,7 +4799,9 @@ class MCPSkillsSource(SkillsSource): instructions/scripts designed to exfiltrate data once loaded and, for script-capable skills, executed. Only connect this source to MCP servers you have vetted and trust, and treat their responses as - untrusted input. + untrusted input. Archive extraction is hardened against path-traversal + ("zip-slip"), link-based escapes, and decompression bombs, but the + skill *content* is still untrusted. Examples: .. code-block:: python @@ -4275,21 +4816,68 @@ class MCPSkillsSource(SkillsSource): _INDEX_URI: Final[str] = "skill://index.json" _SKILL_MD_TYPE: Final[str] = "skill-md" + _ARCHIVE_TYPE: Final[str] = "archive" - def __init__(self, client: ClientSession) -> None: + def __init__( + self, + client: ClientSession, + *, + archive_skills_directory: str | Path | None = None, + archive_resource_extensions: tuple[str, ...] | None = None, + archive_resource_search_depth: int = DEFAULT_SEARCH_DEPTH, + archive_max_file_count: int = _DEFAULT_ARCHIVE_MAX_FILE_COUNT, + archive_max_size_bytes: int = _DEFAULT_ARCHIVE_MAX_SIZE_BYTES, + archive_max_uncompressed_size_bytes: int = _DEFAULT_ARCHIVE_MAX_UNCOMPRESSED_SIZE_BYTES, + ) -> None: """Initialize an MCPSkillsSource. Args: client: An MCP client session connected to a server that exposes Agent Skills resources. + + Keyword Args: + archive_skills_directory: Base directory that ``archive``-type skills are + extracted to and served from. Archives are extracted beneath it as + ``{archive_skills_directory}/{skill-name}/``. When ``None`` (the default), + a per-instance unique directory ``{cwd}/{uuid}/`` is used so multiple + sources never overwrite one another. When set, the directory is treated + as exclusively owned by this source: on every discovery, any subdirectory + the server no longer advertises is pruned, so two sources must not share + one directory. + archive_resource_extensions: Allowed file extensions for resources discovered + in extracted archive skills. When ``None`` (the default), the file-source + default set (``.md``, ``.json``, ``.yaml``, ``.yml``, ``.csv``, ``.xml``, + ``.txt``) is used. + archive_resource_search_depth: Maximum depth to search for resource files + within each extracted archive skill directory. ``1`` searches only the + skill root; ``2`` (the default) searches the root plus one level of + subdirectories. Must be >= 1. + archive_max_file_count: Maximum number of files that may be extracted from a + single archive skill (excessive-file-count DoS guard). Defaults to ``20``. + An archive that exceeds the limit is skipped. + archive_max_size_bytes: Maximum size, in bytes, of a downloaded archive skill + resource. Defaults to ``1 MB``. An archive that exceeds the limit is skipped. + archive_max_uncompressed_size_bytes: Maximum total uncompressed size, in bytes, + of all files extracted from a single archive skill (decompression-bomb + guard). Defaults to ``1 MB``. An archive that exceeds the limit is skipped. """ self._client = client + self._archive_loader = _ArchiveEntryLoader( + client, + archive_skills_directory=str(archive_skills_directory) if archive_skills_directory is not None else None, + resource_extensions=archive_resource_extensions, + resource_search_depth=archive_resource_search_depth, + max_file_count=archive_max_file_count, + max_size_bytes=archive_max_size_bytes, + max_uncompressed_size_bytes=archive_max_uncompressed_size_bytes, + ) async def get_skills(self, context: SkillsSourceContext) -> list[Skill]: """Discover and return skills from the MCP server. - Reads ``skill://index.json``, parses it, and creates an - :class:`MCPSkill` for each valid ``skill-md`` entry. + Reads ``skill://index.json``, parses it, dispatches each entry to the + handler for its ``type`` (``skill-md`` or ``archive``), and returns the + aggregated skill list. Args: context: Contextual information about the agent and session @@ -4298,14 +4886,28 @@ async def get_skills(self, context: SkillsSourceContext) -> list[Skill]: context. Returns: - A list of discovered :class:`MCPSkill` instances. + A list of discovered :class:`Skill` instances. """ index = await self._try_read_index() - if index is None: - return [] + + skill_md_entries: list[_McpSkillIndexEntry] = [] + archive_entries: list[_McpSkillIndexEntry] = [] + if index is not None: + for entry in index.skills: + entry_type = (entry.type or "").strip().lower() + if entry_type == self._SKILL_MD_TYPE: + skill_md_entries.append(entry) + elif entry_type == self._ARCHIVE_TYPE: + archive_entries.append(entry) + else: + logger.debug( + "Skipping entry '%s': unsupported type '%s'", + entry.name or "(unnamed)", + entry.type or "(none)", + ) skills: list[Skill] = [] - for entry in index.skills: + for entry in skill_md_entries: result = self._try_create_skill(entry) if result is not None: skills.append(result) @@ -4316,6 +4918,10 @@ async def get_skills(self, context: SkillsSourceContext) -> list[Skill]: entry.name or "(unnamed)", ) + # Always invoke the archive loader (even with no archive entries) so it can + # prune directories the server no longer advertises. + skills.extend(await self._archive_loader.load(archive_entries, context)) + logger.info("Successfully loaded %d skills from MCP server", len(skills)) return skills @@ -4347,23 +4953,18 @@ async def _try_read_index(self) -> _McpSkillIndex | None: return None def _try_create_skill(self, entry: _McpSkillIndexEntry) -> MCPSkill | None: - """Attempt to create an :class:`MCPSkill` from an index entry. + """Attempt to create an :class:`MCPSkill` from a ``skill-md`` index entry. + + The caller has already dispatched the entry to this handler by its + ``type``; this method validates the remaining required fields. Args: - entry: A single entry from the skill index. + entry: A single ``skill-md`` entry from the skill index. Returns: An :class:`MCPSkill` if the entry is valid, or ``None`` if the entry should be skipped. """ - if entry.type != self._SKILL_MD_TYPE: - logger.debug( - "Skipping entry '%s': unsupported type '%s'", - entry.name or "(unnamed)", - entry.type or "(none)", - ) - return None - if not entry.name or not entry.name.strip(): logger.debug("Skipping entry: missing required 'name' field") return None diff --git a/python/packages/core/tests/core/test_mcp_skills.py b/python/packages/core/tests/core/test_mcp_skills.py index fce28d3c0c4..44f0d672f08 100644 --- a/python/packages/core/tests/core/test_mcp_skills.py +++ b/python/packages/core/tests/core/test_mcp_skills.py @@ -5,7 +5,12 @@ from __future__ import annotations import base64 +import io import json +import os +import tarfile +import zipfile +from pathlib import Path from unittest.mock import AsyncMock import pytest @@ -430,7 +435,9 @@ async def test_missing_required_fields_is_skipped(self) -> None: assert skills == [] @pytest.mark.asyncio - async def test_unsupported_type_is_skipped(self) -> None: + async def test_archive_missing_resource_is_skipped(self) -> None: + # An archive entry whose archive resource is not available on the server + # is skipped (the index is read, but the archive download fails). index_json = json.dumps({ "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", "skills": [ @@ -659,3 +666,363 @@ async def test_index_timeout_error_propagates(self) -> None: source = MCPSkillsSource(client=client) with pytest.raises(TimeoutError): await source.get_skills(_SOURCE_CTX) + + +# --------------------------------------------------------------------------- +# Archive skill helpers +# --------------------------------------------------------------------------- + + +def _make_zip(files: dict[str, bytes]) -> bytes: + """Build an in-memory ZIP archive from a ``{path: content}`` mapping.""" + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w", zipfile.ZIP_DEFLATED) as archive: + for name, data in files.items(): + archive.writestr(name, data) + return buffer.getvalue() + + +def _make_tar(files: dict[str, bytes], *, gzipped: bool) -> bytes: + """Build an in-memory TAR (optionally gzip-compressed) archive.""" + buffer = io.BytesIO() + + def _write(archive: tarfile.TarFile) -> None: + for name, data in files.items(): + info = tarfile.TarInfo(name=name) + info.size = len(data) + archive.addfile(info, io.BytesIO(data)) + + if gzipped: + with tarfile.open(fileobj=buffer, mode="w:gz") as archive: + _write(archive) + else: + with tarfile.open(fileobj=buffer, mode="w:") as archive: + _write(archive) + return buffer.getvalue() + + +ARCHIVE_SKILL_MD = """\ +--- +name: packaged-skill +description: A skill delivered as an archive. +--- +# Packaged Skill + +Instructions from an archive. +""" + + +def _make_archive_index(name: str, url: str, entry_type: str = "archive") -> str: + """Build a skill index JSON document with a single archive entry.""" + return json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "name": name, + "type": entry_type, + "description": "A skill delivered as an archive.", + "url": url, + } + ], + }) + + +def _archive_client(index_json: str, archive_url: str, archive_bytes: bytes, mime_type: str) -> AsyncMock: + """Build a mock client that serves the index and a single archive blob resource.""" + return _make_client(**{ + "skill://index.json": _make_text_result(index_json, uri="skill://index.json"), + str(AnyUrl(archive_url)): _make_blob_result(archive_bytes, uri=archive_url, mime_type=mime_type), + }) + + +# --------------------------------------------------------------------------- +# Archive skill discovery tests (through MCPSkillsSource) +# --------------------------------------------------------------------------- + + +class TestMCPSkillsSourceArchive: + """Tests for archive-type skill discovery via MCPSkillsSource.""" + + @pytest.mark.asyncio + async def test_zip_archive_discovered_as_file_skill(self, tmp_path: Path) -> None: + from agent_framework import FileSkill + + url = "skill://archives/packaged-skill.zip" + index = _make_archive_index("packaged-skill", url) + archive = _make_zip({"SKILL.md": ARCHIVE_SKILL_MD.encode()}) + client = _archive_client(index, url, archive, "application/zip") + + source = MCPSkillsSource(client=client, archive_skills_directory=tmp_path) + skills = await source.get_skills(_SOURCE_CTX) + + assert len(skills) == 1 + skill = skills[0] + assert isinstance(skill, FileSkill) + assert skill.frontmatter.name == "packaged-skill" + content = await skill.get_content() + assert "Instructions from an archive." in content + assert (tmp_path / "packaged-skill" / "SKILL.md").is_file() + + @pytest.mark.asyncio + async def test_targz_archive_discovered(self, tmp_path: Path) -> None: + url = "skill://archives/packaged-skill.tar.gz" + index = _make_archive_index("packaged-skill", url) + archive = _make_tar({"SKILL.md": ARCHIVE_SKILL_MD.encode()}, gzipped=True) + client = _archive_client(index, url, archive, "application/gzip") + + source = MCPSkillsSource(client=client, archive_skills_directory=tmp_path) + skills = await source.get_skills(_SOURCE_CTX) + + assert len(skills) == 1 + assert skills[0].frontmatter.name == "packaged-skill" + + @pytest.mark.asyncio + async def test_tar_archive_discovered(self, tmp_path: Path) -> None: + url = "skill://archives/packaged-skill.tar" + index = _make_archive_index("packaged-skill", url) + archive = _make_tar({"SKILL.md": ARCHIVE_SKILL_MD.encode()}, gzipped=False) + client = _archive_client(index, url, archive, "application/x-tar") + + source = MCPSkillsSource(client=client, archive_skills_directory=tmp_path) + skills = await source.get_skills(_SOURCE_CTX) + + assert len(skills) == 1 + assert skills[0].frontmatter.name == "packaged-skill" + + @pytest.mark.asyncio + async def test_bundled_script_is_never_runnable(self, tmp_path: Path) -> None: + # An archive that bundles a .py script must not expose it as a runnable script. + url = "skill://archives/packaged-skill.zip" + index = _make_archive_index("packaged-skill", url) + archive = _make_zip({ + "SKILL.md": ARCHIVE_SKILL_MD.encode(), + "run.py": b"print('malicious')\n", + }) + client = _archive_client(index, url, archive, "application/zip") + + source = MCPSkillsSource(client=client, archive_skills_directory=tmp_path) + skill = (await source.get_skills(_SOURCE_CTX))[0] + + assert await skill.get_script("run.py") is None + content = await skill.get_content() + assert "" in content + + @pytest.mark.asyncio + async def test_unadvertised_archive_directory_is_pruned(self, tmp_path: Path) -> None: + url = "skill://archives/packaged-skill.zip" + index = _make_archive_index("packaged-skill", url) + archive = _make_zip({"SKILL.md": ARCHIVE_SKILL_MD.encode()}) + + source = MCPSkillsSource( + client=_archive_client(index, url, archive, "application/zip"), archive_skills_directory=tmp_path + ) + skills = await source.get_skills(_SOURCE_CTX) + assert len(skills) == 1 + assert (tmp_path / "packaged-skill").is_dir() + + # The server stops advertising the skill; its extracted directory must be pruned. + empty_client = _make_client(**{ + "skill://index.json": _make_text_result('{"skills": []}', uri="skill://index.json") + }) + source._client = empty_client # type: ignore[attr-defined] # pyright: ignore[reportPrivateUsage] + source._archive_loader._client = empty_client # type: ignore[attr-defined] # pyright: ignore[reportPrivateUsage] + skills = await source.get_skills(_SOURCE_CTX) + assert skills == [] + assert not (tmp_path / "packaged-skill").exists() + + @pytest.mark.asyncio + async def test_oversized_archive_download_is_skipped(self, tmp_path: Path) -> None: + url = "skill://archives/packaged-skill.zip" + index = _make_archive_index("packaged-skill", url) + archive = _make_zip({"SKILL.md": ARCHIVE_SKILL_MD.encode()}) + client = _archive_client(index, url, archive, "application/zip") + + source = MCPSkillsSource(client=client, archive_skills_directory=tmp_path, archive_max_size_bytes=8) + skills = await source.get_skills(_SOURCE_CTX) + assert skills == [] + + @pytest.mark.asyncio + async def test_invalid_archive_name_is_skipped(self, tmp_path: Path) -> None: + url = "skill://archives/packaged-skill.zip" + index = _make_archive_index("bad/name", url) + archive = _make_zip({"SKILL.md": ARCHIVE_SKILL_MD.encode()}) + client = _archive_client(index, url, archive, "application/zip") + + source = MCPSkillsSource(client=client, archive_skills_directory=tmp_path) + skills = await source.get_skills(_SOURCE_CTX) + assert skills == [] + + @pytest.mark.asyncio + async def test_unsupported_archive_format_is_skipped(self, tmp_path: Path) -> None: + url = "skill://archives/packaged-skill.bin" + index = _make_archive_index("packaged-skill", url) + client = _archive_client(index, url, b"not-an-archive", "application/octet-stream") + + source = MCPSkillsSource(client=client, archive_skills_directory=tmp_path) + skills = await source.get_skills(_SOURCE_CTX) + assert skills == [] + + @pytest.mark.asyncio + async def test_archive_download_internal_error_propagates(self, tmp_path: Path) -> None: + # A non-"not found" MCP error while downloading an archive must propagate, + # not silently drop the skill (which would corrupt a CachingSkillsSource refresh). + url = "skill://archives/packaged-skill.zip" + index = _make_archive_index("packaged-skill", url) + + async def _read_resource(uri: AnyUrl) -> ReadResourceResult: + uri_str = str(uri) + if uri_str == "skill://index.json": + return _make_text_result(index, uri="skill://index.json") + raise McpError(error=ErrorData(code=-32603, message="Internal error")) + + client = AsyncMock() + client.read_resource = AsyncMock(side_effect=_read_resource) + + source = MCPSkillsSource(client=client, archive_skills_directory=tmp_path) + with pytest.raises(McpError): + await source.get_skills(_SOURCE_CTX) + + @pytest.mark.asyncio + async def test_archive_download_connection_error_propagates(self, tmp_path: Path) -> None: + # A plain ConnectionError while downloading an archive must propagate. + url = "skill://archives/packaged-skill.zip" + index = _make_archive_index("packaged-skill", url) + + async def _read_resource(uri: AnyUrl) -> ReadResourceResult: + uri_str = str(uri) + if uri_str == "skill://index.json": + return _make_text_result(index, uri="skill://index.json") + raise ConnectionError("connection lost") + + client = AsyncMock() + client.read_resource = AsyncMock(side_effect=_read_resource) + + source = MCPSkillsSource(client=client, archive_skills_directory=tmp_path) + with pytest.raises(ConnectionError): + await source.get_skills(_SOURCE_CTX) + + @pytest.mark.asyncio + async def test_mixed_skill_md_and_archive_entries(self, tmp_path: Path) -> None: + archive_url = "skill://archives/packaged-skill.zip" + index = json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "name": "unit-converter", + "type": "skill-md", + "description": "Convert between common units.", + "url": "skill://unit-converter/SKILL.md", + }, + { + "name": "packaged-skill", + "type": "archive", + "description": "A skill delivered as an archive.", + "url": archive_url, + }, + ], + }) + archive = _make_zip({"SKILL.md": ARCHIVE_SKILL_MD.encode()}) + client = _make_client(**{ + "skill://index.json": _make_text_result(index, uri="skill://index.json"), + "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), + str(AnyUrl(archive_url)): _make_blob_result(archive, uri=archive_url, mime_type="application/zip"), + }) + + source = MCPSkillsSource(client=client, archive_skills_directory=tmp_path) + skills = await source.get_skills(_SOURCE_CTX) + + names = sorted(s.frontmatter.name for s in skills) + assert names == ["packaged-skill", "unit-converter"] + + +# --------------------------------------------------------------------------- +# Archive extractor unit tests +# --------------------------------------------------------------------------- + + +class TestArchiveExtractor: + """Tests for the archive format detection and hardened extraction helpers.""" + + def test_detect_format_from_magic_bytes(self) -> None: + from agent_framework._skills import _ArchiveFormat, _detect_archive_format + + assert _detect_archive_format(b"\x1f\x8b\x08\x00", None, None) is _ArchiveFormat.TAR_GZ + assert _detect_archive_format(b"PK\x03\x04rest", None, None) is _ArchiveFormat.ZIP + + def test_detect_format_from_media_type(self) -> None: + from agent_framework._skills import _ArchiveFormat, _detect_archive_format + + assert _detect_archive_format(b"xx", "application/zip", None) is _ArchiveFormat.ZIP + assert _detect_archive_format(b"xx", "application/x-tar", None) is _ArchiveFormat.TAR + assert _detect_archive_format(b"xx", "application/gzip", None) is _ArchiveFormat.TAR_GZ + + def test_detect_format_from_url_suffix(self) -> None: + from agent_framework._skills import _ArchiveFormat, _detect_archive_format + + assert _detect_archive_format(b"xx", None, "skill://a.zip") is _ArchiveFormat.ZIP + assert _detect_archive_format(b"xx", None, "skill://a.tgz") is _ArchiveFormat.TAR_GZ + assert _detect_archive_format(b"xx", None, "skill://a.tar") is _ArchiveFormat.TAR + + def test_detect_format_unknown(self) -> None: + from agent_framework._skills import _ArchiveFormat, _detect_archive_format + + assert _detect_archive_format(b"xx", "text/plain", "skill://a.bin") is _ArchiveFormat.UNKNOWN + + def test_resolve_destination_rejects_traversal(self, tmp_path: Path) -> None: + from agent_framework._skills import _resolve_archive_destination + + root = str(tmp_path) + # Parent-traversal escapes are rejected. + assert _resolve_archive_destination(root, "../evil.md") is None + assert _resolve_archive_destination(root, "..\\evil.md") is None + assert _resolve_archive_destination(root, "a/../../evil.md") is None + assert _resolve_archive_destination(root, "") is None + # A leading-slash path is neutralized to a relative path beneath the root. + neutralized = _resolve_archive_destination(root, "/etc/passwd") + assert neutralized is not None + assert Path(neutralized).is_relative_to(tmp_path) + assert _resolve_archive_destination(root, "ok/file.md") is not None + + def test_zip_slip_entry_is_not_written(self, tmp_path: Path) -> None: + from agent_framework._skills import _ArchiveFormat, _extract_archive + + target = tmp_path / "target" + archive = _make_zip({"../evil.md": b"pwned", "safe.md": b"ok"}) + _extract_archive(archive, _ArchiveFormat.ZIP, str(target), 20, 1024 * 1024) + + assert not (tmp_path / "evil.md").exists() + assert (target / "safe.md").is_file() + + def test_file_count_limit_is_enforced(self, tmp_path: Path) -> None: + from agent_framework._skills import _ArchiveFormat, _extract_archive + + archive = _make_zip({"a.md": b"a", "b.md": b"b", "c.md": b"c"}) + with pytest.raises(ValueError, match="file count"): + _extract_archive(archive, _ArchiveFormat.ZIP, str(tmp_path / "target"), 2, 1024 * 1024) + + def test_uncompressed_size_limit_is_enforced(self, tmp_path: Path) -> None: + from agent_framework._skills import _ArchiveFormat, _extract_archive + + archive = _make_zip({"big.md": b"x" * 100}) + with pytest.raises(ValueError, match="uncompressed size"): + _extract_archive(archive, _ArchiveFormat.ZIP, str(tmp_path / "target"), 20, 10) + + def test_tar_symlink_member_is_skipped(self, tmp_path: Path) -> None: + from agent_framework._skills import _ArchiveFormat, _extract_archive + + buffer = io.BytesIO() + with tarfile.open(fileobj=buffer, mode="w:") as archive: + link = tarfile.TarInfo(name="link") + link.type = tarfile.SYMTYPE + link.linkname = "/etc/passwd" + archive.addfile(link) + data = b"regular" + reg = tarfile.TarInfo(name="regular.md") + reg.size = len(data) + archive.addfile(reg, io.BytesIO(data)) + + target = tmp_path / "target" + _extract_archive(buffer.getvalue(), _ArchiveFormat.TAR, str(target), 20, 1024 * 1024) + + assert not os.path.lexists(target / "link") + assert (target / "regular.md").is_file() diff --git a/python/packages/foundry_hosting/agent_framework_foundry_hosting/_toolbox.py b/python/packages/foundry_hosting/agent_framework_foundry_hosting/_toolbox.py index a3848a75bed..803c7e64574 100644 --- a/python/packages/foundry_hosting/agent_framework_foundry_hosting/_toolbox.py +++ b/python/packages/foundry_hosting/agent_framework_foundry_hosting/_toolbox.py @@ -19,6 +19,8 @@ if TYPE_CHECKING: from collections.abc import Generator + from pathlib import Path + from typing import Any from agent_framework import Skill from azure.core.credentials import TokenCredential @@ -198,6 +200,12 @@ def as_skills_provider( disable_load_skill_approval: bool = False, disable_read_skill_resource_approval: bool = False, disable_run_skill_script_approval: bool = False, + archive_skills_directory: str | Path | None = None, + archive_resource_extensions: tuple[str, ...] | None = None, + archive_resource_search_depth: int | None = None, + archive_max_file_count: int | None = None, + archive_max_size_bytes: int | None = None, + archive_max_uncompressed_size_bytes: int | None = None, ) -> SkillsProvider: """Return a :class:`~agent_framework.SkillsProvider` backed by this toolbox. @@ -211,6 +219,12 @@ def as_skills_provider( the agent via ``tools=`` -- set ``load_tools=False`` if you want skills only and no tools -- or by entering it as an ``async with`` context manager. + Skills served as ``archive`` entries (a packaged ZIP / TAR) are downloaded and + extracted to a local directory, then served like file-based skills. The + ``archive_*`` keyword arguments configure that behavior; see + :class:`~agent_framework.MCPSkillsSource` for their full semantics. Any left + as ``None`` fall back to the ``MCPSkillsSource`` defaults. + Keyword Args: source_id: Unique identifier for the provider instance. instruction_template: Custom system-prompt template for advertising @@ -225,10 +239,29 @@ def as_skills_provider( cannot satisfy the default approval flow). Defaults to ``False``. disable_read_skill_resource_approval: When ``True``, register the provider's ``read_skill_resource`` tool with - ``approval_mode="never_require"``. Defaults to ``False``. + ``approval_mode="never_require"``. Set this alongside + ``disable_load_skill_approval`` for an unattended agent that reads + archive-skill resources. Defaults to ``False``. disable_run_skill_script_approval: When ``True``, register the provider's ``run_skill_script`` tool with ``approval_mode="never_require"``. Defaults to ``False``. + archive_skills_directory: Base directory that ``archive``-type skills are + extracted to and served from. When ``None``, a per-instance unique + directory under the current working directory is used. Set this to a + writable location (for example, a temp directory) when the working + directory may be read-only, such as a hosted container. + archive_resource_extensions: Allowed file extensions for resources + discovered in extracted archive skills. ``None`` uses the default set. + archive_resource_search_depth: Maximum depth to search for resource files + within each extracted archive skill directory. ``None`` uses the + default. + archive_max_file_count: Maximum number of files that may be extracted from + a single archive skill (DoS guard). ``None`` uses the default. + archive_max_size_bytes: Maximum size, in bytes, of a downloaded archive + skill resource. ``None`` uses the default. + archive_max_uncompressed_size_bytes: Maximum total uncompressed size, in + bytes, of all files extracted from a single archive skill + (decompression-bomb guard). ``None`` uses the default. Returns: A :class:`~agent_framework.SkillsProvider` that advertises and loads the @@ -250,8 +283,24 @@ def as_skills_provider( ) await ResponsesHostServer(agent).run_async() """ + # Forward only explicitly-set archive options so unset ones fall back to the + # MCPSkillsSource defaults (avoids duplicating those defaults here). + archive_options: dict[str, Any] = {} + if archive_skills_directory is not None: + archive_options["archive_skills_directory"] = archive_skills_directory + if archive_resource_extensions is not None: + archive_options["archive_resource_extensions"] = archive_resource_extensions + if archive_resource_search_depth is not None: + archive_options["archive_resource_search_depth"] = archive_resource_search_depth + if archive_max_file_count is not None: + archive_options["archive_max_file_count"] = archive_max_file_count + if archive_max_size_bytes is not None: + archive_options["archive_max_size_bytes"] = archive_max_size_bytes + if archive_max_uncompressed_size_bytes is not None: + archive_options["archive_max_uncompressed_size_bytes"] = archive_max_uncompressed_size_bytes + return SkillsProvider( - _FoundryToolboxSkillsSource(self), + _FoundryToolboxSkillsSource(self, archive_options=archive_options), source_id=source_id, instruction_template=instruction_template, disable_caching=disable_caching, @@ -269,8 +318,10 @@ class _FoundryToolboxSkillsSource(SkillsSource): discovery time rather than captured at construction. """ - def __init__(self, toolbox: FoundryToolbox) -> None: + def __init__(self, toolbox: FoundryToolbox, *, archive_options: dict[str, Any] | None = None) -> None: self._toolbox = toolbox + # Explicitly-set MCPSkillsSource archive kwargs; empty means use its defaults. + self._archive_options: dict[str, Any] = archive_options or {} async def get_skills(self, context: SkillsSourceContext) -> list[Skill]: session = self._toolbox.session @@ -280,4 +331,4 @@ async def get_skills(self, context: SkillsSourceContext) -> list[Skill]: "Pass the toolbox to the agent (tools=...) or enter it as an async " "context manager before the agent runs." ) - return await MCPSkillsSource(client=session).get_skills(context) + return await MCPSkillsSource(client=session, **self._archive_options).get_skills(context) diff --git a/python/packages/foundry_hosting/tests/test_toolbox.py b/python/packages/foundry_hosting/tests/test_toolbox.py index 005eefa5766..757cd918352 100644 --- a/python/packages/foundry_hosting/tests/test_toolbox.py +++ b/python/packages/foundry_hosting/tests/test_toolbox.py @@ -217,8 +217,9 @@ async def test_skills_source_uses_connected_session(monkeypatch: pytest.MonkeyPa captured: dict[str, object] = {} class _StubSkillsSource: - def __init__(self, *, client: object) -> None: + def __init__(self, *, client: object, **kwargs: object) -> None: captured["client"] = client + captured["kwargs"] = kwargs async def get_skills(self, context: SkillsSourceContext) -> list[str]: return ["skill-a"] @@ -229,3 +230,56 @@ async def get_skills(self, context: SkillsSourceContext) -> list[str]: assert result == ["skill-a"] assert captured["client"] is sentinel_session + # No archive options set -> MCPSkillsSource is constructed with defaults. + assert captured["kwargs"] == {} + + +async def test_skills_source_forwards_archive_options(monkeypatch: pytest.MonkeyPatch) -> None: + toolbox = FoundryToolbox( + _FakeCredential(), # type: ignore + url="https://h/toolboxes/tb/mcp", + ) + toolbox.session = object() # type: ignore + + captured: dict[str, object] = {} + + class _StubSkillsSource: + def __init__(self, *, client: object, **kwargs: object) -> None: + captured["kwargs"] = kwargs + + async def get_skills(self, context: SkillsSourceContext) -> list[str]: + return [] + + monkeypatch.setattr("agent_framework_foundry_hosting._toolbox.MCPSkillsSource", _StubSkillsSource) + + source = _FoundryToolboxSkillsSource( + toolbox, + archive_options={"archive_skills_directory": "/tmp/skills", "archive_max_file_count": 5}, + ) + await source.get_skills(_source_context()) + + # Only the explicitly-set archive options are forwarded to MCPSkillsSource. + assert captured["kwargs"] == {"archive_skills_directory": "/tmp/skills", "archive_max_file_count": 5} + + +def test_as_skills_provider_forwards_only_set_archive_options() -> None: + toolbox = FoundryToolbox( + _FakeCredential(), # type: ignore + url="https://h/toolboxes/tb/mcp", + ) + # Unset archive kwargs are not forwarded (fall back to MCPSkillsSource defaults); + # set ones are collected for forwarding. + default_source = cast(_FoundryToolboxSkillsSource, toolbox.as_skills_provider()._source) # pyright: ignore[reportPrivateUsage] + assert default_source._archive_options == {} # pyright: ignore[reportPrivateUsage] + + source = cast( + _FoundryToolboxSkillsSource, + toolbox.as_skills_provider( + archive_skills_directory="/tmp/skills", + archive_resource_search_depth=1, + )._source, # pyright: ignore[reportPrivateUsage] + ) + assert source._archive_options == { # pyright: ignore[reportPrivateUsage] + "archive_skills_directory": "/tmp/skills", + "archive_resource_search_depth": 1, + } diff --git a/python/samples/02-agents/skills/mcp_based_skill/README.md b/python/samples/02-agents/skills/mcp_based_skill/README.md index 58fb5d487a3..96cc191139e 100644 --- a/python/samples/02-agents/skills/mcp_based_skill/README.md +++ b/python/samples/02-agents/skills/mcp_based_skill/README.md @@ -9,6 +9,13 @@ This sample demonstrates how to discover **Agent Skills served over MCP** with a - Building a `SkillsProvider` from an `MCPSkillsSource`, which reads `skill://index.json` (SEP-2640 canonical discovery) and constructs skills from the index entries. +- Both discovery entry types: `skill-md` (the `SKILL.md` body and sibling + resources are fetched on demand from the server) and `archive` (a single ZIP / + TAR / gzip-TAR resource that is downloaded and safely unpacked to a local + directory, then served like a file-based skill). Archive extraction is hardened + against path-traversal, link-based escapes, and decompression bombs, and + scripts bundled in an archive are never exposed as runnable — MCP-delivered + scripts are always treated as read-only content. - The progressive disclosure pattern across MCP: advertise → load → read resources, exactly as for filesystem-backed skills. @@ -42,7 +49,8 @@ python mcp_based_skill.py This sample is a **consumer**: it does not host an MCP server itself. To try it end-to-end you need an MCP server that exposes the SEP-2640 skill -resources (`skill://index.json` plus per-skill `SKILL.md`). +resources (`skill://index.json` plus per-skill `SKILL.md` for `skill-md` +entries and/or a downloadable archive resource for `archive` entries). - See [`samples/02-agents/mcp/agent_as_mcp_server.py`](../../mcp/agent_as_mcp_server.py) for an example of hosting an MCP server via the Agent Framework. diff --git a/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/.azdignore b/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/.azdignore index e228002119e..95f4665ee49 100644 --- a/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/.azdignore +++ b/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/.azdignore @@ -4,3 +4,4 @@ agent.yaml .env toolbox.yaml skills +*.zip diff --git a/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/.dockerignore b/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/.dockerignore index 67a9a78b35d..18ccae3fed5 100644 --- a/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/.dockerignore +++ b/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/.dockerignore @@ -7,3 +7,4 @@ __pycache__ .env skills toolbox.yaml +*.zip diff --git a/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/.gitignore b/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/.gitignore new file mode 100644 index 00000000000..c611b6567f0 --- /dev/null +++ b/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/.gitignore @@ -0,0 +1,2 @@ +# Generated when packaging archive-type skills for `azd ai skill create`. +*.zip diff --git a/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/README.md b/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/README.md index ccd2e46513d..c4efd163c97 100644 --- a/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/README.md +++ b/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/README.md @@ -6,12 +6,13 @@ This sample is **self-contained**: it ships the `SKILL.md` sources and a `toolbo ## How progressive disclosure works -The `FoundryToolbox` is attached to the agent and its skills are exposed through a `SkillsProvider`. When the agent runs, it discovers the toolbox's skills and applies the progressive-disclosure pattern so a skill's full body is only fetched when the agent actually needs it, reducing token usage: +The `FoundryToolbox` is attached to the agent and its skills are exposed through a `SkillsProvider`. When the agent runs, it discovers the toolbox's skills and applies the three-stage progressive-disclosure pattern so a skill's full body and any supplementary resources are only fetched when the agent actually needs them, reducing token usage: 1. **Advertise** — each skill's name and description are injected into the system prompt so the model knows what is available (~100 tokens per skill). 2. **Load** — when the model decides a skill is relevant, it retrieves the full `SKILL.md` body on demand via `resources/read`. +3. **Read resources** — if a skill references supplementary content (reference documents, assets), the model reads it on demand, again via `resources/read`. -> The Agent Skills spec defines a third stage — **read resources** — where a skill fetches supplementary files (reference documents, assets) on demand. That stage requires skills to be served as `type: skill-md` with sibling resources, but Foundry serves ZIP-uploaded (multi-file) skills as `type: archive`, which toolbox skill discovery does not currently surface. So this sample keeps both skills as single-file `SKILL.md` (advertise + load only). See the [`09_foundry_skills`](../09_foundry_skills/README.md) sample for the same instruction-only pattern via direct download. +This sample demonstrates all three stages: `support-style` is a single-file `SKILL.md` (advertise + load), and `escalation-policy` is an **archive** skill bundling a `SKILL.md` plus a `references/refund-matrix.md` resource that the model reads on demand (advertise + load + read resources). ## Toolbox MCP skills vs. Foundry Skills @@ -19,7 +20,7 @@ Foundry exposes skills in two ways, and this sample uses the second one. **Foundry Skills** are downloaded directly into an agent: the agent pulls each `SKILL.md` from the Skills API at startup and serves the bodies from local files. See the [`09_foundry_skills`](../09_foundry_skills/README.md) sample. -**Toolbox MCP skills** are accessed through a toolbox over the MCP protocol. A toolbox bundles a curated set of skills (and optionally tools) behind one MCP endpoint, and any MCP client discovers them automatically. Skill bodies are fetched on demand. The same `SKILL.md` files power both modes — the difference is only in delivery. +**Toolbox MCP skills** are accessed through a toolbox over the MCP protocol. A toolbox bundles a curated set of skills (and optionally tools) behind one MCP endpoint, and any MCP client discovers them automatically. Skills are served either as a single `SKILL.md` (`type: skill-md`) or as a downloadable package (`type: archive`); either way, bodies and any supplementary resources are fetched on demand. ## How it works @@ -29,7 +30,7 @@ Foundry exposes skills in two ways, and this sample uses the second one. 1. Constructs a `FoundryToolbox(credential, load_tools=False)`. The toolbox resolves its MCP endpoint from `TOOLBOX_ENDPOINT`, authenticates every request with the credential, and forwards the platform per-request call-id. `load_tools=False` keeps the toolbox's tools hidden so only its Agent Skills are surfaced. 2. Calls `toolbox.as_skills_provider()`, which discovers skills from the well-known `skill://index.json` resource on the toolbox's MCP session and exposes them as an agent context provider. -3. Passes the toolbox via `tools=` **and** the provider via `context_providers=`. The `tools=` wiring connects the MCP session (the connection the provider reads from); the `context_providers=` wiring runs the advertise/load logic over that session. Both are required — see [main.py](main.py) for the full implementation. +3. Passes the toolbox via `tools=` **and** the provider via `context_providers=`. The `tools=` wiring connects the MCP session (the connection the provider reads from); the `context_providers=` wiring runs the advertise/load/read-resource logic over that session. Both are required — see [main.py](main.py) for the full implementation. ### Agent hosting @@ -37,19 +38,20 @@ The agent is hosted with the `ResponsesHostServer`, which provisions a REST API ## The bundled skills -This sample ships two source skills under [`skills/`](skills/), reused from the [`09_foundry_skills`](../09_foundry_skills/README.md) sample so you can compare the two delivery modes side by side: +This sample ships two source skills under [`skills/`](skills/), one of each discovery kind so you can see both side by side: -| Skill | Purpose | -|---|---| -| [`support-style`](skills/support-style/SKILL.md) | Voice, formatting, and signature rules for Contoso Outdoors support replies. | -| [`escalation-policy`](skills/escalation-policy/SKILL.md) | When and how to escalate a customer ticket, including the refund-authority matrix. | +| Skill | Kind | Purpose | +|---|---|---| +| [`support-style`](skills/support-style/SKILL.md) | single `SKILL.md` | Voice, formatting, and signature rules for Contoso Outdoors support replies. | +| [`escalation-policy`](skills/escalation-policy/SKILL.md) | archive (`SKILL.md` + resource) | When and how to escalate a customer ticket. References a supplementary resource, [`references/refund-matrix.md`](skills/escalation-policy/references/refund-matrix.md), that the model reads on demand. | -Each file includes a unique `*-CANARY-*` token that the model is asked to echo, so a response proves the model actually **loaded** the skill rather than hallucinating: +Each file includes a unique `*-CANARY-*` token that the model is asked to echo, so a response proves the model actually **loaded** the skill (and, for `escalation-policy`, **read the resource**) rather than hallucinating: | Artifact | Canary | Proves | |---|---|---| | `support-style/SKILL.md` | `STYLE-CANARY-3318` | The model loaded the `support-style` body. | | `escalation-policy/SKILL.md` | `ESC-CANARY-7742` | The model loaded the `escalation-policy` body. | +| `escalation-policy/references/refund-matrix.md` | `RESOURCE-CANARY-9921` | The model read the supplementary resource on demand. | > The `name` and `description` values in the YAML front matter must be **unquoted** — quoting them causes the Skills API to reject the import. @@ -79,14 +81,27 @@ azd ai project set "https://.services.ai.azure.com/api/projects/ **Why single files (not ZIPs)?** Uploading a skill as a `.zip` (to bundle supplementary resource files) makes Foundry serve it as `type: archive` in the toolbox's `skill://index.json`. Toolbox skill discovery currently surfaces only `type: skill-md` entries, so archive skills are silently dropped. Keeping each skill as a single `SKILL.md` ensures both are discovered. +`escalation-policy` bundles a supplementary resource (`references/refund-matrix.md`), so package the skill folder as a `.zip` (with `SKILL.md` at the archive root) and upload that. Foundry serves it as a `type: archive` skill, and the toolbox discovery unpacks it and exposes the resource on demand: + +```powershell +# PowerShell +Compress-Archive -Path skills\escalation-policy\* -DestinationPath escalation-policy.zip -Force +azd ai skill create escalation-policy --file ./escalation-policy.zip --no-prompt +``` + +```bash +# bash +(cd skills/escalation-policy && zip -r ../../escalation-policy.zip .) +azd ai skill create escalation-policy --file ./escalation-policy.zip --no-prompt +``` > The `name:` in each `SKILL.md` front matter must equal the positional skill name you pass to `azd ai skill create`. To replace a skill after editing it, re-run with `--force` (this deletes the existing skill and all its versions, then uploads a fresh v1). @@ -129,16 +144,16 @@ curl -X POST http://localhost:8088/responses -H "Content-Type: application/json" # Routine question -> loads support-style curl -X POST http://localhost:8088/responses -H "Content-Type: application/json" -d '{"input": "Hi, I am Alex. Can I return my tent within 30 days?"}' -# Large refund + legal threat -> loads escalation-policy (which includes the refund matrix) +# Large refund + legal threat -> loads escalation-policy AND reads the refund-matrix resource curl -X POST http://localhost:8088/responses -H "Content-Type: application/json" -d '{"input": "I want a $750 refund on Order #A-1042 right now or I am calling my lawyer."}' ``` | Prompt mentions | Skill that should drive the response | Canary you should see | |---|---|---| | Routine return / shipping / care question | `support-style` | `STYLE-CANARY-3318` | -| Injury, legal threat, press, or refund > $500 | `escalation-policy` (+ `support-style`) | `ESC-CANARY-7742` | +| Injury, legal threat, press, or refund > $500 | `escalation-policy` (+ `support-style`) and the refund-matrix resource | `ESC-CANARY-7742` and `RESOURCE-CANARY-9921` | -Because skills are loaded on demand, a canary token in a response proves the model actually invoked `load_skill` for the matching skill — not that it merely saw the name in the advertised list. +Because skills and resources are loaded on demand, a canary token in a response proves the model actually invoked `load_skill` / `read_skill_resource` for the matching artifact — not that it merely saw the name in the advertised list. ## Deploying the agent to Foundry diff --git a/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/main.py b/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/main.py index 6ce2fb3561e..89168e012ae 100644 --- a/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/main.py +++ b/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/main.py @@ -2,6 +2,8 @@ import asyncio import os +import tempfile +from pathlib import Path from agent_framework import Agent from agent_framework.foundry import FoundryChatClient @@ -25,12 +27,21 @@ async def main() -> None: toolbox = FoundryToolbox(credential, load_tools=False) # as_skills_provider() discovers skills from skill://index.json on the toolbox - # MCP session and exposes them as an agent context provider; SKILL.md bodies are - # fetched on demand via resources/read. disable_load_skill_approval=True registers - # the load_skill tool with approval_mode="never_require" so this unattended agent - # can load skills without an approval round-trip -- the Responses host runs the + # MCP session and exposes them as an agent context provider; SKILL.md bodies and + # any archive resources are fetched on demand via resources/read. Disabling the + # load_skill and read_skill_resource approvals registers those tools with + # approval_mode="never_require" so this unattended agent can load skills and read + # their resources without an approval round-trip -- the Responses host runs the # agent without an AgentSession, which the default approval flow requires. - skills_provider = toolbox.as_skills_provider(disable_load_skill_approval=True) + # archive_skills_directory points archive-skill extraction at a writable temp + # directory; the default is under the current working directory, which may be + # read-only in a hosted container. + archive_dir = Path(tempfile.gettempdir()) / "maf_toolbox_archive_skills" + skills_provider = toolbox.as_skills_provider( + disable_load_skill_approval=True, + disable_read_skill_resource_approval=True, + archive_skills_directory=archive_dir, + ) client = FoundryChatClient( project_endpoint=os.environ["FOUNDRY_PROJECT_ENDPOINT"], diff --git a/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/skills/escalation-policy/SKILL.md b/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/skills/escalation-policy/SKILL.md index c9ca3b57b72..4a8ef872dec 100644 --- a/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/skills/escalation-policy/SKILL.md +++ b/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/skills/escalation-policy/SKILL.md @@ -25,20 +25,10 @@ You must follow this escalation policy on every conversation. ## Refund authority -Before quoting any refund amount or approval timeline, apply this matrix and -route the request to the correct approver. Quote the tier and approver exactly. - -| Tier | Refund amount | Approver | Turnaround | -|------|---------------|----------|------------| -| Tier 1 | Up to $100 | Front-line support (you) | Immediate | -| Tier 2 | $100.01 – $500 | Support team lead | Same business day | -| Tier 3 | Over $500 | Senior specialist (escalation required) | 1 business day | - -- A Tier 3 refund always requires escalation per the rules above; you may not - approve it yourself. -- Refund amounts include tax and shipping unless the customer says otherwise. -- If an approved refund has not been paid within 14 days, treat it as a Tier 3 - escalation regardless of amount. +Before quoting any refund amount or approval timeline, read the supplementary +resource `references/refund-matrix.md` for the current approval tiers and route +the request to the correct approver. Quote the tier and approver from that +resource exactly — never guess the limits. ## Do not escalate diff --git a/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/skills/escalation-policy/references/refund-matrix.md b/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/skills/escalation-policy/references/refund-matrix.md new file mode 100644 index 00000000000..e894d775550 --- /dev/null +++ b/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/skills/escalation-policy/references/refund-matrix.md @@ -0,0 +1,24 @@ +# Contoso Outdoors Refund Authority Matrix + +This is the authoritative reference for who may approve a refund and how long +approval takes. Quote the tier name and approver exactly when you use it. + +| Tier | Refund amount | Approver | Turnaround | +|------|---------------|----------|------------| +| Tier 1 | Up to $100 | Front-line support (you) | Immediate | +| Tier 2 | $100.01 – $500 | Support team lead | Same business day | +| Tier 3 | Over $500 | Senior specialist (escalation required) | 1 business day | + +## Rules + +- A Tier 3 refund always requires escalation per the escalation policy; you may + not approve it yourself. +- Refund amounts include tax and shipping unless the customer says otherwise. +- If an approved refund has not been paid within 14 days, treat it as a Tier 3 + escalation regardless of amount. + +## Canary + +To prove this resource was read on demand, include the token +`RESOURCE-CANARY-9921` on a separate line at the bottom of any response that +quotes a refund tier, prefixed with `# `. diff --git a/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/toolbox.yaml b/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/toolbox.yaml index f0fcbfe2eb8..fc4ac1a9226 100644 --- a/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/toolbox.yaml +++ b/python/samples/04-hosting/foundry-hosted-agents/responses/12_foundry_toolbox_mcp_skills/toolbox.yaml @@ -10,10 +10,12 @@ # are. The toolbox is effectively skills-only from the agent's perspective. # # The skills referenced below must already exist in the same Foundry project — -# create them first from the bundled skills/ folder: +# create them first from the bundled skills/ folder (support-style as a single +# SKILL.md; escalation-policy zipped as an archive so its resource ships too): # -# azd ai skill create support-style --file ./skills/support-style/SKILL.md --no-prompt -# azd ai skill create escalation-policy --file ./skills/escalation-policy/SKILL.md --no-prompt +# azd ai skill create support-style --file ./skills/support-style/SKILL.md --no-prompt +# Compress-Archive -Path skills\escalation-policy\* -DestinationPath escalation-policy.zip -Force +# azd ai skill create escalation-policy --file ./escalation-policy.zip --no-prompt # # Then create the toolbox once from this file: #