diff --git a/python/packages/core/AGENTS.md b/python/packages/core/AGENTS.md index 7ebfaa59f44..29b52097b9f 100644 --- a/python/packages/core/AGENTS.md +++ b/python/packages/core/AGENTS.md @@ -91,6 +91,7 @@ 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`). +- **`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 and unpacked **entirely in memory** (via the private `_ArchiveEntryLoader`) into a `FileSkill` whose `SKILL.md` body drives it and whose sibling files (matching the resource extensions, within the search depth) become in-memory `InlineSkillResource` resources. **Nothing is written to disk** — there are no temporary directories to create, own, or prune (this is a deliberate divergence from .NET, which extracts archives to disk; it removes the temp-dir leak and the dangerous prune-of-unowned-subdirs footgun). Entries whose type has no handler (e.g. `mcp-resource-template`) are skipped. **MCP-delivered scripts are never runnable**: the loader emits no `SkillScript`s, so a bundled script can at most surface as a readable resource (and only if it matches the resource extensions — `.py` is not a default resource extension). The archive `SKILL.md` frontmatter `name` must match the advertised index-entry `name` or the skill is skipped. Extraction is hardened: `_normalize_archive_member_name` rejects path-traversal ("zip-slip") member names, non-regular TAR members (links/devices) are skipped, and file-count / uncompressed-size (`_read_member_with_limit`) / download-size limits are enforced. Archive behavior is configured with `archive_*` constructor kwargs (`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. A non-"resource not found" error while downloading an archive propagates (so a failed `CachingSkillsSource` refresh does not overwrite a cached list with a partial result). 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 default to the built-in tuples and treat `None` as "use defaults" and an empty tuple as "discover none" (an empty tuple previously fell back to defaults). `FoundryToolbox.as_skills_provider()` forwards matching `archive_*` kwargs to this source. - **`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` and `MCPSkill` accept exactly one of `client` (a fixed `ClientSession`) or `session_provider` (`Callable[[], ClientSession]`, resolved on every fetch); providing both/neither raises `ValueError`.** Use `session_provider` when the underlying session may be swapped over time — e.g. a reconnecting `MCPTool`/`FoundryToolbox` whose `session` is replaced on reconnect — so cached `MCPSkill`s keep fetching against the live session instead of a closed one (`MCPSkillsSource` forwards its provider to every `MCPSkill` it creates). A fixed `client` is safe only when the session outlives the skills. ### Model Context Protocol (`_mcp.py`) diff --git a/python/packages/core/agent_framework/_skills.py b/python/packages/core/agent_framework/_skills.py index 388a6417369..d1b93a03859 100644 --- a/python/packages/core/agent_framework/_skills.py +++ b/python/packages/core/agent_framework/_skills.py @@ -46,19 +46,24 @@ import asyncio import base64 +import gzip import inspect +import io import json import logging import os import re +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 ._feature_stage import ExperimentalFeature, experimental from ._sessions import ContextProvider @@ -2164,8 +2169,8 @@ def from_paths( skill_paths: str | Path | Sequence[str | Path], *, script_runner: SkillScriptRunner | None = None, - resource_extensions: tuple[str, ...] | None = None, - script_extensions: tuple[str, ...] | None = None, + resource_extensions: tuple[str, ...] | None = DEFAULT_RESOURCE_EXTENSIONS, + script_extensions: tuple[str, ...] | None = DEFAULT_SCRIPT_EXTENSIONS, search_depth: int = DEFAULT_SEARCH_DEPTH, script_filter: Callable[[str, str], bool] | None = None, resource_filter: Callable[[str, str], bool] | None = None, @@ -2192,8 +2197,12 @@ def from_paths( resource_extensions: File extensions recognized as discoverable resources. Defaults to ``(".md", ".json", ".yaml", ".yml", ".csv", ".xml", ".txt")``. + ``None`` is treated the same as the default; pass an empty + tuple to discover no resources. script_extensions: File extensions recognized as discoverable - scripts. Defaults to ``(".py",)``. + scripts. Defaults to ``(".py",)``. ``None`` is treated the + same as the default; pass an empty tuple to disable script + discovery entirely. 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 @@ -2816,8 +2825,8 @@ def __init__( skill_paths: str | Path | Sequence[str | Path], *, script_runner: SkillScriptRunner | None = None, - resource_extensions: tuple[str, ...] | None = None, - script_extensions: tuple[str, ...] | None = None, + resource_extensions: tuple[str, ...] | None = DEFAULT_RESOURCE_EXTENSIONS, + script_extensions: tuple[str, ...] | None = DEFAULT_SCRIPT_EXTENSIONS, search_depth: int = DEFAULT_SEARCH_DEPTH, script_filter: Callable[[str, str], bool] | None = None, resource_filter: Callable[[str, str], bool] | None = None, @@ -2838,8 +2847,15 @@ def __init__( resource_extensions: File extensions recognized as discoverable resources. Defaults to ``(".md", ".json", ".yaml", ".yml", ".csv", ".xml", ".txt")``. + ``None`` is treated the same as the default; pass an empty + tuple to discover no resources. script_extensions: File extensions recognized as discoverable - scripts. Defaults to ``(".py",)``. + scripts. Defaults to ``(".py",)``. ``None`` is treated the + same as the default; 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 +2878,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 +3993,35 @@ 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. + logger.warning("Failed to base64-decode blob resource content from the MCP server.", exc_info=True) + return None + return data, content.mimeType + return None + + class _McpSkillIndexEntry: # ruff:ignore[class-as-data-structure] """A single entry in the ``skill://index.json`` discovery document. @@ -4282,25 +4329,508 @@ 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_READ_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 _normalize_archive_member_name(entry_path: str) -> str | None: + """Normalize an archive member path to a safe relative POSIX path, or ``None``. + + Guards against path-traversal ("zip-slip") shapes: empty, absolute/rooted, or + parent-traversing (``..``) member paths are rejected. No filesystem is touched; + the returned name is used only as an in-memory resource key, so this is purely a + naming-safety check. + + Args: + entry_path: The archive-relative path of the member. + + Returns: + A cleaned forward-slash-separated relative path, or ``None`` when the member + path is empty or would escape the skill namespace. + """ + 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 + + parts = [segment for segment in normalized.split("/") if segment not in ("", ".")] + if not parts: + return None + + return "/".join(parts) + + +def _read_member_with_limit(source: IO[bytes], remaining_bytes: int) -> tuple[bytes, int]: + """Read one archive member fully into memory within a shared uncompressed-byte budget. + + Counts bytes actually produced by the decompressor rather than trusting archive + metadata, the authoritative defense against decompression bombs. The member is read + in chunks so the running budget is enforced before the whole member is materialized. + + Args: + source: The (possibly decompressing) input stream for a single member. + remaining_bytes: The remaining uncompressed-byte budget shared across the archive. + + Returns: + A ``(data, remaining_bytes)`` tuple: the member's bytes and the budget left after + reading it. + + Raises: + ValueError: If the remaining budget is exhausted. + """ + chunks: list[bytes] = [] + while True: + chunk = source.read(_ARCHIVE_READ_BUFFER_SIZE) + if not chunk: + break + remaining_bytes -= len(chunk) + if remaining_bytes < 0: + raise ValueError("Skill archive exceeds the maximum allowed uncompressed size.") + chunks.append(chunk) + return b"".join(chunks), remaining_bytes + + +def _extract_archive_to_memory( + data: bytes, + archive_format: _ArchiveFormat, + max_file_count: int, + max_uncompressed_size_bytes: int, +) -> dict[str, bytes]: + """Extract an archive's regular files into an in-memory ``{relative-path: bytes}`` mapping. + + Supports ZIP, TAR, and gzip-compressed TAR payloads. Non-regular TAR entries + (symbolic links, hard links, device nodes, etc.) and unsafe member names (absolute + or parent-traversing) are skipped, so an archive cannot smuggle in a link or escape + the skill namespace. Extraction is bounded by a maximum file count and total + uncompressed size to mitigate decompression-bomb attacks. No filesystem is touched. + + Args: + data: The raw archive bytes. + archive_format: The detected container format. + max_file_count: The maximum number of files that may be extracted. + max_uncompressed_size_bytes: The maximum total uncompressed size, in bytes. + + Returns: + A mapping of forward-slash-separated relative paths to file bytes. + + Raises: + ValueError: If the format is unknown or a limit is exceeded. + OSError: If the payload cannot be read. + tarfile.TarError: If a TAR payload is malformed. + zipfile.BadZipFile: If a ZIP payload is malformed. + gzip.BadGzipFile: If a gzip payload is malformed. + """ + if archive_format is _ArchiveFormat.ZIP: + with zipfile.ZipFile(io.BytesIO(data)) as archive: + return _extract_zip_to_memory(archive, max_file_count, max_uncompressed_size_bytes) + if archive_format is _ArchiveFormat.TAR: + with tarfile.open(fileobj=io.BytesIO(data), mode="r:") as archive: + return _extract_tar_to_memory(archive, max_file_count, max_uncompressed_size_bytes) + if archive_format is _ArchiveFormat.TAR_GZ: + with tarfile.open(fileobj=io.BytesIO(data), mode="r:gz") as archive: + return _extract_tar_to_memory(archive, max_file_count, max_uncompressed_size_bytes) + raise ValueError(f"Unsupported skill archive format '{archive_format}'.") + + +def _extract_zip_to_memory( + archive: zipfile.ZipFile, + max_file_count: int, + max_uncompressed_size_bytes: int, +) -> dict[str, bytes]: + """Read regular files from a ZIP archive into memory. See :func:`_extract_archive_to_memory`.""" + remaining_bytes = max_uncompressed_size_bytes + files: dict[str, 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}).") + + name = _normalize_archive_member_name(info.filename) + if name is None: + continue + + with archive.open(info) as source: + content, remaining_bytes = _read_member_with_limit(source, remaining_bytes) + files[name] = content + + return files + + +def _extract_tar_to_memory( + archive: tarfile.TarFile, + max_file_count: int, + max_uncompressed_size_bytes: int, +) -> dict[str, bytes]: + """Read regular files from a TAR archive into memory. See :func:`_extract_archive_to_memory`.""" + remaining_bytes = max_uncompressed_size_bytes + files: dict[str, 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 skill namespace. + 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}).") + + name = _normalize_archive_member_name(member.name) + if name is None: + continue + + source = archive.extractfile(member) + if source is None: + continue + + with source: + content, remaining_bytes = _read_member_with_limit(source, remaining_bytes) + files[name] = content + + return files + + +class _ArchiveEntryLoader: + """Loads ``archive``-type ``skill://index.json`` entries entirely in memory. + + Each entry's ``url`` points to a single archive resource (ZIP, TAR, or + gzip-compressed TAR). The archive is downloaded and unpacked **in memory** into a + :class:`FileSkill` whose ``SKILL.md`` body drives the skill and whose sibling files + (matching the configured resource extensions, within the configured depth) become + in-memory :class:`InlineSkillResource` resources. Nothing is written to disk, so + there are no temporary directories to create, own, or prune. + + Bundled scripts are never surfaced as runnable scripts: only files matching the + resource extensions become readable resources; a script file is at most a readable + resource, never a :class:`SkillScript`. + + Extraction is hardened against path-traversal ("zip-slip") member names, non-regular + TAR members (links/devices), oversized downloads, excessive file counts, and + decompression bombs. + """ + + def __init__( + self, + session_provider: Callable[[], ClientSession], + *, + resource_extensions: tuple[str, ...] | None, + resource_search_depth: int, + max_file_count: int, + max_size_bytes: int, + max_uncompressed_size_bytes: int, + ) -> None: + self._session_provider = session_provider + self._resource_extensions: tuple[str, ...] = ( + resource_extensions if resource_extensions is not None else DEFAULT_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 + + async def load(self, entries: list[_McpSkillIndexEntry], context: SkillsSourceContext) -> list[Skill]: + """Download and unpack every valid archive entry into an in-memory skill. + + Args: + entries: The ``archive`` index entries. May be empty. + context: Accepted for symmetry with the skill-md path; unused, since archive + discovery does not depend on the invoking agent or session. + + Returns: + A list of discovered :class:`FileSkill` instances (in-memory backed). + """ + skills: list[Skill] = [] + seen_names: set[str] = set() + + 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 entry.url or not entry.url.strip(): + logger.debug("Skipping entry '%s': missing required 'url' field", entry.name) + continue + + downloaded = await self._download(entry) + if downloaded is None: + continue + + data, mime_type = downloaded + skill = self._build_skill(entry, data, mime_type) + if skill is None: + continue + + if skill.frontmatter.name in seen_names: + logger.warning( + "Duplicate archive skill name '%s': later entry skipped in favor of the first", + skill.frontmatter.name, + ) + continue + + seen_names.add(skill.frontmatter.name) + skills.append(skill) + logger.info("Loaded archive skill: %s", skill.frontmatter.name) + + return skills + + 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._session_provider().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 _build_skill(self, entry: _McpSkillIndexEntry, data: bytes, mime_type: str | None) -> FileSkill | None: + """Detect the format of and unpack one archive entry into an in-memory :class:`FileSkill`. + + Returns: + A :class:`FileSkill`, or ``None`` when the archive cannot be materialized + (unsupported format, malformed archive, missing/invalid ``SKILL.md``, or a + frontmatter name that does not match the advertised entry name). + """ + 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: + files = _extract_archive_to_memory( + data, archive_format, 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) + return None + + return self._skill_from_files(entry, files) + + def _skill_from_files(self, entry: _McpSkillIndexEntry, files: dict[str, bytes]) -> FileSkill | None: + """Assemble an in-memory :class:`FileSkill` from the archive's extracted files.""" + skill_md_key = self._find_skill_md(files) + if skill_md_key is None: + logger.debug("Skipping skill '%s': archive contains no SKILL.md", entry.name) + return None + + try: + content = files[skill_md_key].decode("utf-8") + except UnicodeDecodeError: + logger.debug("Skipping skill '%s': SKILL.md is not valid UTF-8", entry.name) + return None + + frontmatter = FileSkillsSource._extract_frontmatter( # pyright: ignore[reportPrivateUsage] + content, f"{cast(str, entry.name)}/{SKILL_FILE_NAME}" + ) + if frontmatter is None: + return None + + if frontmatter.name != entry.name: + logger.debug( + "Skipping skill '%s': SKILL.md frontmatter name '%s' does not match the advertised entry name", + entry.name, + frontmatter.name, + ) + return None + + # The SKILL.md's directory within the archive is the skill root; resources are + # discovered relative to it, mirroring file-based skill discovery. + root_prefix = skill_md_key[: skill_md_key.rfind("/") + 1] if "/" in skill_md_key else "" + resources = self._build_resources(files, skill_md_key, root_prefix) + + # ``path`` records the archive's origin URL; it is informational only, as the + # skill and its resources are served entirely from memory (no filesystem access). + return FileSkill( + frontmatter=frontmatter, + content=content, + path=cast(str, entry.url), + resources=resources, + scripts=[], + ) + + @staticmethod + def _find_skill_md(files: dict[str, bytes]) -> str | None: + """Return the key of the shallowest ``SKILL.md`` (case-insensitive), or ``None``.""" + candidates = [key for key in files if key.rsplit("/", 1)[-1].upper() == SKILL_FILE_NAME.upper()] + if not candidates: + return None + # Prefer the shallowest SKILL.md; break ties deterministically by name. + return min(candidates, key=lambda key: (key.count("/"), key)) + + def _build_resources(self, files: dict[str, bytes], skill_md_key: str, root_prefix: str) -> list[SkillResource]: + """Build in-memory resources from archive files under the ``SKILL.md`` directory. + + A file is included when it sits under *root_prefix*, is not the ``SKILL.md`` + itself, matches an allowed resource extension, is within the configured search + depth, and decodes as UTF-8 text. Scripts are never emitted, so bundled scripts + can at most surface as readable resources, never as runnable scripts. + """ + allowed = {ext.lower() for ext in self._resource_extensions} + resources: list[SkillResource] = [] + + for key in sorted(files): + if key == skill_md_key: + continue + if root_prefix and not key.startswith(root_prefix): + continue + + rel = key[len(root_prefix) :] + if not rel: + continue + + # Depth: the skill root is depth 1, one nested directory is depth 2, and so on. + if rel.count("/") + 1 > self._resource_search_depth: + continue + + basename = rel.rsplit("/", 1)[-1] + dot = basename.rfind(".") + suffix = basename[dot:].lower() if dot > 0 else "" + if suffix not in allowed: + continue + + try: + text = files[key].decode("utf-8") + except UnicodeDecodeError: + logger.debug("Skipping resource '%s': not valid UTF-8", rel) + continue + + resources.append(InlineSkillResource(name=rel, content=text)) + + return resources + + @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 and unpacked **in memory** into a + skill whose ``SKILL.md`` body drives it and whose sibling files become + in-memory resources; nothing is written to disk. 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. + 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 what skill content (including instructions and, for script-capable @@ -4312,7 +4842,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 @@ -4327,12 +4859,18 @@ 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 = None, *, session_provider: Callable[[], ClientSession] | 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. @@ -4352,18 +4890,43 @@ def __init__( reconnect), so cached skills keep using the live session. The provider is forwarded to every :class:`MCPSkill` this source creates. + archive_resource_extensions: Allowed file extensions for resources discovered + in ``archive``-type skills. When ``None`` (the default), the file-source + default set (``.md``, ``.json``, ``.yaml``, ``.yml``, ``.csv``, ``.xml``, + ``.txt``) is used. Files that would otherwise be scripts are never exposed + as runnable. + archive_resource_search_depth: Maximum depth to search for resource files + within each archive skill. ``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. Raises: ValueError: If both or neither of *client* and *session_provider* are provided. """ self._session_provider = _resolve_mcp_session_provider(client, session_provider) + self._archive_loader = _ArchiveEntryLoader( + self._session_provider, + 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 @@ -4372,14 +4935,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) @@ -4390,6 +4967,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 the + # dispatch mirrors the skill-md path; it simply returns an empty list. + skills.extend(await self._archive_loader.load(archive_entries, context)) + logger.info("Successfully loaded %d skills from MCP server", len(skills)) return skills @@ -4421,23 +5002,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 12d062224ef..be029393506 100644 --- a/python/packages/core/tests/core/test_mcp_skills.py +++ b/python/packages/core/tests/core/test_mcp_skills.py @@ -5,7 +5,10 @@ from __future__ import annotations import base64 +import io import json +import tarfile +import zipfile from unittest.mock import AsyncMock import pytest @@ -451,7 +454,10 @@ async def test_missing_required_fields_is_skipped(self) -> None: skills = await source.get_skills(_SOURCE_CTX) assert skills == [] - async def test_unsupported_type_is_skipped(self) -> None: + @pytest.mark.asyncio + 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": [ @@ -692,3 +698,413 @@ 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 (in-memory).""" + + @pytest.mark.asyncio + async def test_zip_archive_discovered_as_file_skill(self) -> 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) + 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 + + @pytest.mark.asyncio + async def test_targz_archive_discovered(self) -> 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) + 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) -> 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) + skills = await source.get_skills(_SOURCE_CTX) + + assert len(skills) == 1 + assert skills[0].frontmatter.name == "packaged-skill" + + @pytest.mark.asyncio + async def test_archive_reference_resource_is_readable(self) -> None: + # A bundled reference file is served as an in-memory resource, read on demand. + url = "skill://archives/packaged-skill.zip" + index = _make_archive_index("packaged-skill", url) + archive = _make_zip({ + "SKILL.md": ARCHIVE_SKILL_MD.encode(), + "references/refund-matrix.md": b"# Refund Matrix\nREF-CANARY-9001\n", + }) + client = _archive_client(index, url, archive, "application/zip") + + source = MCPSkillsSource(client=client) + skill = (await source.get_skills(_SOURCE_CTX))[0] + + resource = await skill.get_resource("references/refund-matrix.md") + assert resource is not None + assert "REF-CANARY-9001" in await resource.read() + + @pytest.mark.asyncio + async def test_wrapped_archive_root_is_discovered(self) -> None: + # An archive whose SKILL.md sits under a top-level folder is still discovered, + # and resources are resolved relative to the SKILL.md's directory. + url = "skill://archives/packaged-skill.zip" + index = _make_archive_index("packaged-skill", url) + archive = _make_zip({ + "packaged-skill/SKILL.md": ARCHIVE_SKILL_MD.encode(), + "packaged-skill/references/doc.md": b"REF-CANARY-42\n", + }) + client = _archive_client(index, url, archive, "application/zip") + + source = MCPSkillsSource(client=client) + skill = (await source.get_skills(_SOURCE_CTX))[0] + + assert skill.frontmatter.name == "packaged-skill" + resource = await skill.get_resource("references/doc.md") + assert resource is not None + assert "REF-CANARY-42" in await resource.read() + + @pytest.mark.asyncio + async def test_bundled_script_is_never_runnable(self) -> None: + # An archive that bundles a .py script must not expose it as a runnable script, + # nor (with default resource extensions) as a resource. + 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) + skill = (await source.get_skills(_SOURCE_CTX))[0] + + assert await skill.get_script("run.py") is None + assert await skill.get_resource("run.py") is None + content = await skill.get_content() + assert "" in content + + @pytest.mark.asyncio + async def test_oversized_archive_download_is_skipped(self) -> 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_max_size_bytes=8) + skills = await source.get_skills(_SOURCE_CTX) + assert skills == [] + + @pytest.mark.asyncio + async def test_archive_exceeding_file_count_is_skipped(self) -> None: + url = "skill://archives/packaged-skill.zip" + index = _make_archive_index("packaged-skill", url) + archive = _make_zip({ + "SKILL.md": ARCHIVE_SKILL_MD.encode(), + "a.md": b"a", + "b.md": b"b", + }) + client = _archive_client(index, url, archive, "application/zip") + + source = MCPSkillsSource(client=client, archive_max_file_count=1) + skills = await source.get_skills(_SOURCE_CTX) + assert skills == [] + + @pytest.mark.asyncio + async def test_frontmatter_name_mismatch_is_skipped(self) -> None: + # The SKILL.md frontmatter name must match the advertised entry name. + url = "skill://archives/packaged-skill.zip" + index = _make_archive_index("packaged-skill", url) + mismatched = ARCHIVE_SKILL_MD.replace("name: packaged-skill", "name: different-name") + archive = _make_zip({"SKILL.md": mismatched.encode()}) + client = _archive_client(index, url, archive, "application/zip") + + source = MCPSkillsSource(client=client) + skills = await source.get_skills(_SOURCE_CTX) + assert skills == [] + + @pytest.mark.asyncio + async def test_archive_without_skill_md_is_skipped(self) -> None: + url = "skill://archives/packaged-skill.zip" + index = _make_archive_index("packaged-skill", url) + archive = _make_zip({"readme.md": b"# not a skill\n"}) + client = _archive_client(index, url, archive, "application/zip") + + source = MCPSkillsSource(client=client) + skills = await source.get_skills(_SOURCE_CTX) + assert skills == [] + + @pytest.mark.asyncio + async def test_unsupported_archive_format_is_skipped(self) -> 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) + skills = await source.get_skills(_SOURCE_CTX) + assert skills == [] + + @pytest.mark.asyncio + async def test_archive_download_internal_error_propagates(self) -> 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) + with pytest.raises(McpError): + await source.get_skills(_SOURCE_CTX) + + @pytest.mark.asyncio + async def test_archive_download_connection_error_propagates(self) -> 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) + with pytest.raises(ConnectionError): + await source.get_skills(_SOURCE_CTX) + + @pytest.mark.asyncio + async def test_mixed_skill_md_and_archive_entries(self) -> 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) + 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 in-memory 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_normalize_member_name_rejects_traversal(self) -> None: + from agent_framework._skills import _normalize_archive_member_name + + # Parent-traversal escapes are rejected. + assert _normalize_archive_member_name("../evil.md") is None + assert _normalize_archive_member_name("..\\evil.md") is None + assert _normalize_archive_member_name("a/../../evil.md") is None + assert _normalize_archive_member_name("") is None + # A leading-slash path is neutralized to a relative path. + assert _normalize_archive_member_name("/etc/passwd") == "etc/passwd" + # Backslashes are normalized and redundant segments collapsed. + assert _normalize_archive_member_name("refs\\./doc.md") == "refs/doc.md" + assert _normalize_archive_member_name("ok/file.md") == "ok/file.md" + + def test_zip_slip_member_is_skipped(self) -> None: + from agent_framework._skills import _ArchiveFormat, _extract_archive_to_memory + + archive = _make_zip({"../evil.md": b"pwned", "safe.md": b"ok"}) + files = _extract_archive_to_memory(archive, _ArchiveFormat.ZIP, 20, 1024 * 1024) + + assert "safe.md" in files + assert "../evil.md" not in files + assert "evil.md" not in files + + def test_leading_slash_member_is_neutralized(self) -> None: + from agent_framework._skills import _ArchiveFormat, _extract_archive_to_memory + + archive = _make_zip({"/abs/file.md": b"data"}) + files = _extract_archive_to_memory(archive, _ArchiveFormat.ZIP, 20, 1024 * 1024) + + assert files == {"abs/file.md": b"data"} + + def test_file_count_limit_is_enforced(self) -> None: + from agent_framework._skills import _ArchiveFormat, _extract_archive_to_memory + + archive = _make_zip({"a.md": b"a", "b.md": b"b", "c.md": b"c"}) + with pytest.raises(ValueError, match="file count"): + _extract_archive_to_memory(archive, _ArchiveFormat.ZIP, 2, 1024 * 1024) + + def test_uncompressed_size_limit_is_enforced(self) -> None: + from agent_framework._skills import _ArchiveFormat, _extract_archive_to_memory + + archive = _make_zip({"big.md": b"x" * 100}) + with pytest.raises(ValueError, match="uncompressed size"): + _extract_archive_to_memory(archive, _ArchiveFormat.ZIP, 20, 10) + + def test_tar_symlink_member_is_skipped(self) -> None: + from agent_framework._skills import _ArchiveFormat, _extract_archive_to_memory + + 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)) + + files = _extract_archive_to_memory(buffer.getvalue(), _ArchiveFormat.TAR, 20, 1024 * 1024) + + assert "link" not in files + assert files == {"regular.md": b"regular"} 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 8d800adc571..654294a0a50 100644 --- a/python/packages/foundry_hosting/agent_framework_foundry_hosting/_toolbox.py +++ b/python/packages/foundry_hosting/agent_framework_foundry_hosting/_toolbox.py @@ -22,6 +22,7 @@ if TYPE_CHECKING: from collections.abc import Generator from datetime import timedelta + from typing import Any from agent_framework import Skill from azure.core.credentials import TokenCredential @@ -203,6 +204,11 @@ 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_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. @@ -216,6 +222,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 + unpacked **in memory** and served like file-based skills; nothing is written to + disk. 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 @@ -240,10 +252,23 @@ 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_resource_extensions: Allowed file extensions for resources + discovered in archive skills. ``None`` uses the default set. + archive_resource_search_depth: Maximum depth to search for resource files + within each archive skill. ``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 @@ -265,11 +290,25 @@ 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_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 + # The toolbox advertises the same skill set to every caller (the per-request # call-id governs execution/authorization, not which skills are listed), so a # single shared cache is safe. SkillsProvider won't auto-cache a caller source, # so we compose the caching ourselves. - source: SkillsSource = _FoundryToolboxSkillsSource(self) + source: SkillsSource = _FoundryToolboxSkillsSource(self, archive_options=archive_options) if not disable_caching: source = DeduplicatingSkillsSource(CachingSkillsSource(source, refresh_interval=cache_refresh_interval)) return SkillsProvider( @@ -292,8 +331,10 @@ class _FoundryToolboxSkillsSource(SkillsSource): fetch, so cached skills keep using the live session instead of a closed one. """ - 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 {} def _require_session(self) -> ClientSession: """Return the toolbox's current MCP session, or raise if not connected.""" @@ -310,4 +351,6 @@ async def get_skills(self, context: SkillsSourceContext) -> list[Skill]: # Fail fast at discovery if not connected, then hand the source a provider # (not a fixed session) so skills survive a reconnect that swaps the session. self._require_session() - return await MCPSkillsSource(session_provider=self._require_session).get_skills(context) + return await MCPSkillsSource(session_provider=self._require_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 4990213a1a1..e75c710782c 100644 --- a/python/packages/foundry_hosting/tests/test_toolbox.py +++ b/python/packages/foundry_hosting/tests/test_toolbox.py @@ -217,10 +217,12 @@ async def test_skills_source_uses_connected_session(monkeypatch: pytest.MonkeyPa toolbox.session = sentinel_session # type: ignore captured: dict[str, Callable[[], object]] = {} + captured_kwargs: dict[str, object] = {} class _StubSkillsSource: - def __init__(self, *, session_provider: Callable[[], object]) -> None: + def __init__(self, *, session_provider: Callable[[], object], **kwargs: object) -> None: captured["session_provider"] = session_provider + captured_kwargs.update(kwargs) async def get_skills(self, context: SkillsSourceContext) -> list[str]: return ["skill-a"] @@ -237,6 +239,36 @@ async def get_skills(self, context: SkillsSourceContext) -> list[str]: new_session = object() toolbox.session = new_session # type: ignore assert provider() is new_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, *, session_provider: 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_resource_search_depth": 3, "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_resource_search_depth": 3, "archive_max_file_count": 5} async def test_skills_source_requires_connection_via_provider() -> None: @@ -332,3 +364,31 @@ async def test_as_skills_provider_cache_refresh_interval_rereads_after_staleness await provider._source.get_skills(context) # pyright: ignore[reportPrivateUsage] assert read_count[0] == 3 + + +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. ``disable_caching=True`` keeps ``_source`` + # the bare ``_FoundryToolboxSkillsSource`` (no caching/dedup decorators wrapping it). + default_source = cast( + _FoundryToolboxSkillsSource, + toolbox.as_skills_provider(disable_caching=True)._source, # pyright: ignore[reportPrivateUsage] + ) + assert default_source._archive_options == {} # pyright: ignore[reportPrivateUsage] + + source = cast( + _FoundryToolboxSkillsSource, + toolbox.as_skills_provider( + disable_caching=True, + archive_max_size_bytes=2048, + archive_resource_search_depth=1, + )._source, # pyright: ignore[reportPrivateUsage] + ) + assert source._archive_options == { # pyright: ignore[reportPrivateUsage] + "archive_max_size_bytes": 2048, + "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 ee544932da0..e2bdd03ca1d 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 @@ -24,12 +24,18 @@ 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. Archive-type + # skills (ZIP/TAR) are downloaded and unpacked in memory -- nothing is written to + # disk -- so no writable working directory is needed. 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) + skills_provider = toolbox.as_skills_provider( + disable_load_skill_approval=True, + disable_read_skill_resource_approval=True, + ) 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: #