diff --git a/AGENTS.md b/AGENTS.md index a9a2a5044..84c337579 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -141,6 +141,7 @@ strands-agents/ │ │ │ ├── core/ # Base classes, actions, context │ │ │ └── handlers/ # Handler implementations (e.g., LLM) │ │ └── skills/ # AgentSkills.io integration (Skill, AgentSkills) +│ │ └── _url_loader.py # HTTPS skill fetching, GitHub URL resolution │ │ │ ├── experimental/ # Experimental features (API may change) │ │ ├── agent_config.py # Experimental agent config diff --git a/src/strands/vended_plugins/skills/_url_loader.py b/src/strands/vended_plugins/skills/_url_loader.py new file mode 100644 index 000000000..67872eacc --- /dev/null +++ b/src/strands/vended_plugins/skills/_url_loader.py @@ -0,0 +1,63 @@ +"""Utilities for loading skills from HTTPS URLs. + +This module provides functions to detect URL-type skill sources and +fetch SKILL.md content over HTTPS. No git dependency, local caching, +or URL resolution is required — callers provide a direct URL to the +raw SKILL.md content. +""" + +from __future__ import annotations + +import logging +import urllib.error +import urllib.request + +logger = logging.getLogger(__name__) + + +def is_url(source: str) -> bool: + """Check whether a skill source string looks like an HTTPS URL. + + Only ``https://`` URLs are supported; plaintext ``http://`` is rejected + for security (MITM risk). + + Args: + source: The skill source string to check. + + Returns: + True if the source is an ``https://`` URL. + """ + return source.startswith("https://") + + +def fetch_skill_content(url: str) -> str: + """Fetch SKILL.md content from an HTTPS URL. + + Uses ``urllib.request`` (stdlib) so no additional dependencies are needed. + + Args: + url: The HTTPS URL to fetch. Must point directly to the raw + SKILL.md content (for example, + ``https://raw.githubusercontent.com/org/repo/main/SKILL.md``). + + Returns: + The response body as a string. + + Raises: + ValueError: If ``url`` is not an ``https://`` URL. + RuntimeError: If the fetch fails (network error, 404, etc.). + """ + if not url.startswith("https://"): + raise ValueError(f"url=<{url}> | only https:// URLs are supported") + + logger.info("url=<%s> | fetching skill content", url) + + try: + req = urllib.request.Request(url, headers={"User-Agent": "strands-agents-sdk"}) # noqa: S310 + with urllib.request.urlopen(req, timeout=30) as response: # noqa: S310 + content: str = response.read().decode("utf-8") + return content + except urllib.error.HTTPError as e: + raise RuntimeError(f"url=<{url}> | HTTP {e.code}: {e.reason}") from e + except urllib.error.URLError as e: + raise RuntimeError(f"url=<{url}> | failed to fetch skill: {e.reason}") from e diff --git a/src/strands/vended_plugins/skills/agent_skills.py b/src/strands/vended_plugins/skills/agent_skills.py index 5e42b9230..39460f6f3 100644 --- a/src/strands/vended_plugins/skills/agent_skills.py +++ b/src/strands/vended_plugins/skills/agent_skills.py @@ -86,6 +86,8 @@ def __init__( - A ``str`` or ``Path`` to a skill directory (containing SKILL.md) - A ``str`` or ``Path`` to a parent directory (containing skill subdirectories) - A ``Skill`` dataclass instance + - An ``https://`` URL pointing to a SKILL.md file or a GitHub + repository/directory URL (auto-resolved to raw content) state_key: Key used to store plugin state in ``agent.state``. max_resource_files: Maximum number of resource files to list in skill responses. strict: If True, raise on skill validation issues. If False (default), warn and load anyway. @@ -284,7 +286,8 @@ def _resolve_skills(self, sources: list[SkillSource]) -> dict[str, Skill]: """Resolve a list of skill sources into Skill instances. Each source can be a Skill instance, a path to a skill directory, - or a path to a parent directory containing multiple skills. + a path to a parent directory containing multiple skills, or an + HTTPS URL pointing to a SKILL.md file. Args: sources: List of skill sources to resolve. @@ -292,6 +295,8 @@ def _resolve_skills(self, sources: list[SkillSource]) -> dict[str, Skill]: Returns: Dict mapping skill names to Skill instances. """ + from ._url_loader import is_url + resolved: dict[str, Skill] = {} for source in sources: @@ -299,6 +304,14 @@ def _resolve_skills(self, sources: list[SkillSource]) -> dict[str, Skill]: if source.name in resolved: logger.warning("name=<%s> | duplicate skill name, overwriting previous skill", source.name) resolved[source.name] = source + elif isinstance(source, str) and is_url(source): + try: + skill = Skill.from_url(source, strict=self._strict) + if skill.name in resolved: + logger.warning("name=<%s> | duplicate skill name, overwriting previous skill", skill.name) + resolved[skill.name] = skill + except (RuntimeError, ValueError) as e: + logger.warning("url=<%s> | failed to load skill from URL: %s", source, e) else: path = Path(source).resolve() if not path.exists(): diff --git a/src/strands/vended_plugins/skills/skill.py b/src/strands/vended_plugins/skills/skill.py index 3e1b6bba5..2c00f1d73 100644 --- a/src/strands/vended_plugins/skills/skill.py +++ b/src/strands/vended_plugins/skills/skill.py @@ -333,6 +333,40 @@ def from_content(cls, content: str, *, strict: bool = False) -> Skill: return _build_skill_from_frontmatter(frontmatter, body) + @classmethod + def from_url(cls, url: str, *, strict: bool = False) -> Skill: + """Load a skill by fetching its SKILL.md content from an HTTPS URL. + + Fetches the raw SKILL.md content over HTTPS and parses it using + :meth:`from_content`. The URL must point directly to the raw + file content (not an HTML page). + + Example:: + + skill = Skill.from_url( + "https://raw.githubusercontent.com/org/repo/main/SKILL.md" + ) + + Args: + url: An ``https://`` URL pointing directly to raw SKILL.md content. + strict: If True, raise on any validation issue. If False (default), + warn and load anyway. + + Returns: + A Skill instance populated from the fetched SKILL.md content. + + Raises: + ValueError: If ``url`` is not an ``https://`` URL. + RuntimeError: If the SKILL.md content cannot be fetched. + """ + from ._url_loader import fetch_skill_content, is_url + + if not is_url(url): + raise ValueError(f"url=<{url}> | not a valid HTTPS URL") + + content = fetch_skill_content(url) + return cls.from_content(content, strict=strict) + @classmethod def from_directory(cls, skills_dir: str | Path, *, strict: bool = False) -> list[Skill]: """Load all skills from a parent directory containing skill subdirectories. diff --git a/tests/strands/vended_plugins/skills/test_agent_skills.py b/tests/strands/vended_plugins/skills/test_agent_skills.py index 52802a6c1..4e69a7f68 100644 --- a/tests/strands/vended_plugins/skills/test_agent_skills.py +++ b/tests/strands/vended_plugins/skills/test_agent_skills.py @@ -661,6 +661,58 @@ def test_resolve_nonexistent_path(self, tmp_path): assert len(plugin._skills) == 0 +class TestResolveUrlSkills: + """Tests for _resolve_skills with URL sources.""" + + _URL_LOADER = "strands.vended_plugins.skills._url_loader" + _SAMPLE_CONTENT = "---\nname: url-skill\ndescription: A URL skill\n---\n# Instructions\n" + + def test_resolve_url_source(self): + """Test resolving a URL string as a skill source.""" + from unittest.mock import patch + + with patch(f"{self._URL_LOADER}.fetch_skill_content", return_value=self._SAMPLE_CONTENT): + plugin = AgentSkills(skills=["https://github.com/org/url-skill"]) + + assert len(plugin.get_available_skills()) == 1 + assert plugin.get_available_skills()[0].name == "url-skill" + + def test_resolve_mixed_url_and_local(self, tmp_path): + """Test resolving a mix of URL and local filesystem sources.""" + from unittest.mock import patch + + _make_skill_dir(tmp_path, "local-skill") + + with patch(f"{self._URL_LOADER}.fetch_skill_content", return_value=self._SAMPLE_CONTENT): + plugin = AgentSkills( + skills=[ + "https://github.com/org/url-skill", + str(tmp_path / "local-skill"), + ] + ) + + assert len(plugin.get_available_skills()) == 2 + names = {s.name for s in plugin.get_available_skills()} + assert names == {"url-skill", "local-skill"} + + def test_resolve_url_failure_skips_gracefully(self, caplog): + """Test that a failed URL fetch is skipped with a warning.""" + import logging + from unittest.mock import patch + + with ( + patch( + f"{self._URL_LOADER}.fetch_skill_content", + side_effect=RuntimeError("HTTP 404: Not Found"), + ), + caplog.at_level(logging.WARNING), + ): + plugin = AgentSkills(skills=["https://github.com/org/broken"]) + + assert len(plugin.get_available_skills()) == 0 + assert "failed to load skill from URL" in caplog.text + + class TestImports: """Tests for module imports.""" diff --git a/tests/strands/vended_plugins/skills/test_skill.py b/tests/strands/vended_plugins/skills/test_skill.py index 53d6f3507..7f52973a3 100644 --- a/tests/strands/vended_plugins/skills/test_skill.py +++ b/tests/strands/vended_plugins/skills/test_skill.py @@ -551,11 +551,64 @@ def test_strict_mode(self): Skill.from_content(content, strict=True) +class TestSkillFromUrl: + """Tests for Skill.from_url.""" + + _URL_LOADER = "strands.vended_plugins.skills._url_loader" + + _SAMPLE_CONTENT = "---\nname: my-skill\ndescription: A remote skill\n---\nRemote instructions.\n" + + def test_from_url_returns_skill(self): + """Test loading a skill from a URL returns a single Skill.""" + from unittest.mock import patch + + with patch(f"{self._URL_LOADER}.fetch_skill_content", return_value=self._SAMPLE_CONTENT): + skill = Skill.from_url("https://raw.githubusercontent.com/org/repo/main/SKILL.md") + + assert isinstance(skill, Skill) + assert skill.name == "my-skill" + assert skill.description == "A remote skill" + assert "Remote instructions." in skill.instructions + assert skill.path is None + + def test_from_url_invalid_url_raises(self): + """Test that a non-HTTPS URL raises ValueError.""" + with pytest.raises(ValueError, match="not a valid HTTPS URL"): + Skill.from_url("./local-path") + + def test_from_url_http_rejected(self): + """Test that http:// URLs are rejected.""" + with pytest.raises(ValueError, match="not a valid HTTPS URL"): + Skill.from_url("http://example.com/SKILL.md") + + def test_from_url_fetch_failure_raises(self): + """Test that a fetch failure propagates as RuntimeError.""" + from unittest.mock import patch + + with patch( + f"{self._URL_LOADER}.fetch_skill_content", + side_effect=RuntimeError("HTTP 404: Not Found"), + ): + with pytest.raises(RuntimeError, match="HTTP 404"): + Skill.from_url("https://example.com/nonexistent/SKILL.md") + + def test_from_url_strict_mode(self): + """Test that strict mode is forwarded to from_content.""" + from unittest.mock import patch + + bad_content = "---\nname: BAD_NAME\ndescription: Bad\n---\nBody." + + with patch(f"{self._URL_LOADER}.fetch_skill_content", return_value=bad_content): + with pytest.raises(ValueError): + Skill.from_url("https://example.com/SKILL.md", strict=True) + + class TestSkillClassmethods: """Tests for Skill classmethod existence.""" def test_skill_classmethods_exist(self): - """Test that Skill has from_file, from_content, and from_directory classmethods.""" + """Test that Skill has from_file, from_content, from_directory, and from_url classmethods.""" assert callable(getattr(Skill, "from_file", None)) assert callable(getattr(Skill, "from_content", None)) assert callable(getattr(Skill, "from_directory", None)) + assert callable(getattr(Skill, "from_url", None)) diff --git a/tests/strands/vended_plugins/skills/test_url_loader.py b/tests/strands/vended_plugins/skills/test_url_loader.py new file mode 100644 index 000000000..a9808bd99 --- /dev/null +++ b/tests/strands/vended_plugins/skills/test_url_loader.py @@ -0,0 +1,118 @@ +"""Tests for the _url_loader module.""" + +from __future__ import annotations + +import urllib.error +from unittest.mock import MagicMock, patch + +import pytest + +from strands.vended_plugins.skills._url_loader import ( + fetch_skill_content, + is_url, +) + + +class TestIsUrl: + """Tests for is_url.""" + + def test_https_url(self): + assert is_url("https://example.com/SKILL.md") is True + + def test_https_raw_github_url(self): + assert is_url("https://raw.githubusercontent.com/org/repo/main/SKILL.md") is True + + def test_http_rejected(self): + """Plaintext http:// is rejected for security.""" + assert is_url("http://example.com/SKILL.md") is False + + def test_ssh_rejected(self): + assert is_url("ssh://git@github.com/org/repo") is False + + def test_git_at_rejected(self): + assert is_url("git@github.com:org/repo.git") is False + + def test_local_relative_path(self): + assert is_url("./skills/my-skill") is False + + def test_local_absolute_path(self): + assert is_url("/home/user/skills/my-skill") is False + + def test_plain_directory_name(self): + assert is_url("my-skill") is False + + def test_empty_string(self): + assert is_url("") is False + + +class TestFetchSkillContent: + """Tests for fetch_skill_content.""" + + _LOADER = "strands.vended_plugins.skills._url_loader" + + def test_fetch_success(self): + """Test successful content fetch.""" + skill_content = "---\nname: test-skill\ndescription: A test\n---\n# Instructions\n" + + mock_response = MagicMock() + mock_response.read.return_value = skill_content.encode("utf-8") + mock_response.__enter__ = MagicMock(return_value=mock_response) + mock_response.__exit__ = MagicMock(return_value=False) + + with patch(f"{self._LOADER}.urllib.request.urlopen", return_value=mock_response): + result = fetch_skill_content("https://raw.githubusercontent.com/org/repo/main/SKILL.md") + + assert result == skill_content + + def test_fetch_uses_url_directly(self): + """Test that the URL is used as-is with no resolution.""" + url = "https://raw.githubusercontent.com/org/repo/main/skills/my-skill/SKILL.md" + + mock_response = MagicMock() + mock_response.read.return_value = b"---\nname: t\ndescription: t\n---\n" + mock_response.__enter__ = MagicMock(return_value=mock_response) + mock_response.__exit__ = MagicMock(return_value=False) + + with patch(f"{self._LOADER}.urllib.request.urlopen", return_value=mock_response) as mock_urlopen: + fetch_skill_content(url) + + request_obj = mock_urlopen.call_args[0][0] + assert request_obj.full_url == url + + def test_fetch_sets_user_agent(self): + """Test that requests include a User-Agent header.""" + mock_response = MagicMock() + mock_response.read.return_value = b"---\nname: t\ndescription: t\n---\n" + mock_response.__enter__ = MagicMock(return_value=mock_response) + mock_response.__exit__ = MagicMock(return_value=False) + + with patch(f"{self._LOADER}.urllib.request.urlopen", return_value=mock_response) as mock_urlopen: + fetch_skill_content("https://example.com/SKILL.md") + + request_obj = mock_urlopen.call_args[0][0] + assert request_obj.get_header("User-agent") == "strands-agents-sdk" + + def test_fetch_http_error(self): + """Test that HTTP errors raise RuntimeError.""" + with patch( + f"{self._LOADER}.urllib.request.urlopen", + side_effect=urllib.error.HTTPError( + url="https://example.com", code=404, msg="Not Found", hdrs=None, fp=None + ), + ): + with pytest.raises(RuntimeError, match="HTTP 404"): + fetch_skill_content("https://example.com/SKILL.md") + + def test_fetch_url_error(self): + """Test that network errors raise RuntimeError.""" + with patch( + f"{self._LOADER}.urllib.request.urlopen", + side_effect=urllib.error.URLError("Connection refused"), + ): + with pytest.raises(RuntimeError, match="failed to fetch"): + fetch_skill_content("https://example.com/SKILL.md") + + def test_fetch_rejects_non_https(self): + """Test that non-https URLs are rejected.""" + with pytest.raises(ValueError, match="only https://"): + fetch_skill_content("http://example.com/SKILL.md")