diff --git a/.github/workflows/harness-sidecar-release-gate.yaml b/.github/workflows/harness-sidecar-release-gate.yaml index 64f690ccc..9f0462c39 100644 --- a/.github/workflows/harness-sidecar-release-gate.yaml +++ b/.github/workflows/harness-sidecar-release-gate.yaml @@ -86,7 +86,7 @@ jobs: backend-gate: name: Backend gate runs-on: ubuntu-latest - timeout-minutes: 3 + timeout-minutes: 10 steps: - uses: actions/checkout@v4 @@ -236,5 +236,7 @@ jobs: BACKEND_RESULT: ${{ needs.backend-gate.result }} FRONTEND_RESULT: ${{ needs.frontend-gate.result }} run: | + echo "Backend gate result: $BACKEND_RESULT" + echo "Frontend gate result: $FRONTEND_RESULT" test "$BACKEND_RESULT" = success test "$FRONTEND_RESULT" = success diff --git a/prd-spec/features/harness-tos-skills/2026-10-12-harness-tos-skills.md b/prd-spec/features/harness-tos-skills/2026-10-12-harness-tos-skills.md new file mode 100644 index 000000000..5c28e7fe5 --- /dev/null +++ b/prd-spec/features/harness-tos-skills/2026-10-12-harness-tos-skills.md @@ -0,0 +1,90 @@ +# Harness TOS Skills + +[中文版](2026-10-12-harness-tos-skills.zh.md) + +## Metadata + +| Field | Value | +| --- | --- | +| Change ID | `harness-tos-skills` | +| Created | 2026-10-12 | +| Status | implemented | +| Related specs | No component spec update; this extends the existing Harness selected-skill request contract without changing ownership boundaries. | + +## Background And Evidence + +Harness can already load SkillHub skills and Skill Space skills. Skill Space loading eventually downloads TOS-backed Skill archives through the existing materializer, but callers cannot directly provide an object-storage Skill directory. + +The requested behavior should align with AWS AgentCore Harness S3 skills: one object-storage URI points to one Skill directory containing `SKILL.md`; multiple Skills are represented as multiple selected-skill entries. + +## Goals + +- Add `source="tos"` to `HarnessSelectedSkill`. +- Support current-account TOS directories via Runtime cloud identity. +- Support public readable TOS zip URLs without requiring AK/SK. +- Preserve existing SkillHub and Skill Space behavior. + +## Non-Goals + +- No parent-directory auto-discovery of multiple Skills. +- No plaintext AK/SK in Harness requests. +- No cross-account private TOS credential reference in this MVP. + +## Requirements + +| ID | Requirement | +| --- | --- | +| FR-1 | A TOS selected Skill with `tos_url="tos://bucket/path/to/skill/"` loads exactly one Skill directory and requires `SKILL.md` or `skill.md` at that URL root. | +| FR-2 | Multiple TOS Skills are loaded by multiple `selected_skills` entries. | +| FR-3 | A public TOS zip URL can load one Skill archive. | +| FR-4 | Empty/bucket-root TOS URLs, parent directories, unsupported public URLs, non-TOS hosts, credential-bearing public URLs, and redirects to non-TOS hosts fail closed. | +| FR-5 | TOS download failures and invalid Skill directories fail the whole SkillToolset build. | +| FR-6 | Public TOS zip downloads, public zip extraction, and private TOS directory downloads enforce bounded file/object count and byte limits before Skill loading. | + +## Design + +`HarnessSelectedSkill.source` accepts `tos` with either: + +```python +HarnessSelectedSkill(source="tos", tos_url="tos://bucket/skills/reporting/", region="cn-beijing") +HarnessSelectedSkill(source="tos", tos_public_url="https://bucket.tos-cn-beijing.volces.com/skills/reporting.zip") +``` + +Multiple Skills are expressed as multiple entries: + +```python +selected_skills=[ + HarnessSelectedSkill(source="tos", tos_url="tos://bucket/skills/reporting/", region="cn-beijing"), + HarnessSelectedSkill(source="tos", tos_url="tos://bucket/skills/search/", region="cn-beijing"), +] +``` + +Harness serializes these as internal refs and dispatches them in `build_skill_toolset`. TOS selected Skills use `tos_url` and `tos_public_url` as the only locator fields. The materializer parses `tos://` URLs, downloads one directory through `VeTOS`, validates path containment, requires root-level `SKILL.md` or `skill.md`, and loads with ADK. A URL pointing at a parent folder containing child Skills is rejected instead of selecting one child. Public URLs must not contain userinfo or TOS signature query credentials. They are streamed as zip archives from validated TOS hosts, every redirect hop must also target a TOS host before it is requested, and the existing safe zip extraction path enforces expanded file count and uncompressed byte limits for public TOS archives. + +Rejected TOS refs are redacted when wrapped into Harness skill-load errors, so invalid userinfo or signature query values are not returned to callers or written by the Harness error path. + +## Tasks + +| ID | Task | +| --- | --- | +| T-1 | Add schema fields and validation for TOS selected Skills. | +| T-2 | Add materializer helpers for TOS directory and public zip sources, including strict root-manifest validation and resource limits. | +| T-3 | Wire Harness SkillToolset building to TOS refs. | +| T-4 | Add focused contract and materializer tests. | + +## Verification + +| Requirement | Command | Result | +| --- | --- | --- | +| FR-1..FR-6 | `uv run --extra dev pytest tests/skills/test_adk_skill_materializer.py tests/test_ve_tos.py::test_download_can_propagate_tos_error tests/test_ve_tos.py::test_download_rejects_object_over_byte_limit tests/test_ve_tos.py::test_download_directory_rejects_too_many_objects tests/test_ve_tos.py::test_download_directory_rejects_total_bytes_from_listing -q` | pass, 30 passed on 2026-10-12 | +| FR-4..FR-5 | `uv run --extra dev pytest tests/cloud/test_harness_app_contract.py::TestHarnessOverrides::test_tos_selected_skill_rejects_public_url_userinfo tests/cloud/test_harness_app_contract.py::TestHarnessOverrides::test_tos_selected_skill_rejects_public_url_signature_query tests/cloud/test_harness_skill_download.py::test_build_skill_toolset_redacts_tos_ref_on_error -q` | pass, 3 passed on 2026-10-13 | +| FR-1..FR-6 | `uv run --extra dev pytest tests/cloud/test_harness_skill_download.py::test_build_skill_toolset_loads_tos_skill_uri tests/cloud/test_harness_skill_download.py::test_build_skill_toolset_loads_multiple_tos_entries -q` | pass, 2 passed on 2026-10-12 | +| FR-1..FR-6 | `uv run --extra dev ruff check veadk/skills/materializer.py veadk/integrations/ve_tos/ve_tos.py tests/skills/test_adk_skill_materializer.py tests/test_ve_tos.py` | pass on 2026-10-12 | +| FR-1..FR-6 | `uv run --extra dev pre-commit run --all-files` | pass on 2026-10-12 | +| FR-1..FR-6 | `git diff --check` | pass on 2026-10-12 | +| FR-1..FR-6 | `uv run --extra dev pytest tests/skills/test_adk_skill_materializer.py tests/test_ve_tos.py -q` | fail, existing environment limitation: four async tests in `tests/test_ve_tos.py` need an async pytest plugin; TOS-focused tests passed. | + +## Risks + +- Real TOS IAM behavior requires cloud smoke validation outside the offline unit tests. +- Public directory URLs remain excluded because object storage does not provide a stable generic HTTP directory listing contract. diff --git a/prd-spec/features/harness-tos-skills/2026-10-12-harness-tos-skills.zh.md b/prd-spec/features/harness-tos-skills/2026-10-12-harness-tos-skills.zh.md new file mode 100644 index 000000000..2375d45aa --- /dev/null +++ b/prd-spec/features/harness-tos-skills/2026-10-12-harness-tos-skills.zh.md @@ -0,0 +1,90 @@ +# Harness TOS Skills + +[English](2026-10-12-harness-tos-skills.md) + +## 元数据 + +| 字段 | 值 | +| --- | --- | +| Change ID | `harness-tos-skills` | +| 创建日期 | 2026-10-12 | +| 状态 | implemented | +| 相关规格 | 不更新组件规格；本变更扩展现有 Harness selected-skill 请求契约，不改变组件职责边界。 | + +## 背景和证据 + +Harness 当前已经支持加载 SkillHub Skill 和 Skill Space Skill。Skill Space 加载最终会通过现有 materializer 下载 TOS 上的 Skill archive，但调用方不能直接提供对象存储 Skill 目录。 + +目标行为对齐 AWS AgentCore Harness S3 Skills：一个对象存储 URI 指向一个包含 `SKILL.md` 的 Skill 目录；多个 Skill 通过多条 selected-skill entry 表达。 + +## 目标 + +- 为 `HarnessSelectedSkill` 增加 `source="tos"`。 +- 支持通过 Runtime 云身份读取当前账号 TOS 目录。 +- 支持公开读 TOS zip URL，不要求 AK/SK。 +- 保持现有 SkillHub 和 Skill Space 行为不变。 + +## 非目标 + +- 不做父目录自动发现多个 Skill。 +- 不在 Harness 请求中接收明文 AK/SK。 +- MVP 不支持跨账号私有 TOS 的 credential reference。 + +## 需求 + +| ID | 需求 | +| --- | --- | +| FR-1 | TOS selected Skill 使用 `tos_url="tos://bucket/path/to/skill/"` 时只加载一个 Skill directory，且该 URL 根目录必须直接包含 `SKILL.md` 或 `skill.md`。 | +| FR-2 | 多个 TOS Skill 通过多条 `selected_skills` entry 加载。 | +| FR-3 | 公开读 TOS zip URL 可以加载一个 Skill archive。 | +| FR-4 | 空 URL、bucket 根路径 URL、父目录 URL、不支持的公开 URL、非 TOS host、带凭据的公开 URL，以及跳转到非 TOS host 的公开 URL 必须失败关闭。 | +| FR-5 | TOS 下载失败或 Skill 目录非法时，整个 SkillToolset 构建失败。 | +| FR-6 | 公开 TOS zip 下载、公开 zip 解压和私有 TOS 目录下载必须在 Skill 加载前执行文件/对象数和字节数上限。 | + +## 设计 + +`HarnessSelectedSkill.source` 接受 `tos`，并支持以下两种形式： + +```python +HarnessSelectedSkill(source="tos", tos_url="tos://bucket/skills/reporting/", region="cn-beijing") +HarnessSelectedSkill(source="tos", tos_public_url="https://bucket.tos-cn-beijing.volces.com/skills/reporting.zip") +``` + +多个 Skill 通过多条 entry 表达： + +```python +selected_skills=[ + HarnessSelectedSkill(source="tos", tos_url="tos://bucket/skills/reporting/", region="cn-beijing"), + HarnessSelectedSkill(source="tos", tos_url="tos://bucket/skills/search/", region="cn-beijing"), +] +``` + +Harness 将其序列化为内部 ref，并在 `build_skill_toolset` 中分发。TOS selected Skill 只使用 `tos_url` 和 `tos_public_url` 作为 locator 字段。materializer 解析 `tos://` URL，通过 `VeTOS` 下载单个目录，校验路径没有逃逸，要求根目录直接包含 `SKILL.md` 或 `skill.md`，再用 ADK 加载。指向包含多个子 Skill 的父目录 URL 会被拒绝，不会自动选择某个子目录。公开 URL 不允许包含 userinfo 或 TOS 签名 query 凭据。公开 URL 按 zip archive 流式下载，每一跳 redirect 在请求前都必须指向 TOS host，并且现有安全解压路径会对公开 TOS archive 执行展开后的文件数和未压缩字节数上限。 + +被拒绝的 TOS ref 在包装成 Harness skill-load 错误时会脱敏，避免无效 userinfo 或签名 query 值返回给调用方，或被 Harness 错误路径写入日志。 + +## 任务 + +| ID | 任务 | +| --- | --- | +| T-1 | 增加 TOS selected Skill schema 字段和校验。 | +| T-2 | 增加 TOS directory 和 public zip materializer，包括严格根 manifest 校验和资源上限。 | +| T-3 | 将 Harness SkillToolset 构建接入 TOS refs。 | +| T-4 | 增加聚焦的契约测试和 materializer 测试。 | + +## 验证 + +| 需求 | 命令 | 结果 | +| --- | --- | --- | +| FR-1..FR-6 | `uv run --extra dev pytest tests/skills/test_adk_skill_materializer.py tests/test_ve_tos.py::test_download_can_propagate_tos_error tests/test_ve_tos.py::test_download_rejects_object_over_byte_limit tests/test_ve_tos.py::test_download_directory_rejects_too_many_objects tests/test_ve_tos.py::test_download_directory_rejects_total_bytes_from_listing -q` | pass，2026-10-12 执行，30 passed | +| FR-4..FR-5 | `uv run --extra dev pytest tests/cloud/test_harness_app_contract.py::TestHarnessOverrides::test_tos_selected_skill_rejects_public_url_userinfo tests/cloud/test_harness_app_contract.py::TestHarnessOverrides::test_tos_selected_skill_rejects_public_url_signature_query tests/cloud/test_harness_skill_download.py::test_build_skill_toolset_redacts_tos_ref_on_error -q` | pass，2026-10-13 执行，3 passed | +| FR-1..FR-6 | `uv run --extra dev pytest tests/cloud/test_harness_skill_download.py::test_build_skill_toolset_loads_tos_skill_uri tests/cloud/test_harness_skill_download.py::test_build_skill_toolset_loads_multiple_tos_entries -q` | pass，2026-10-12 执行，2 passed | +| FR-1..FR-6 | `uv run --extra dev ruff check veadk/skills/materializer.py veadk/integrations/ve_tos/ve_tos.py tests/skills/test_adk_skill_materializer.py tests/test_ve_tos.py` | pass，2026-10-12 执行 | +| FR-1..FR-6 | `uv run --extra dev pre-commit run --all-files` | pass，2026-10-12 执行 | +| FR-1..FR-6 | `git diff --check` | pass，2026-10-12 执行 | +| FR-1..FR-6 | `uv run --extra dev pytest tests/skills/test_adk_skill_materializer.py tests/test_ve_tos.py -q` | fail，现有环境限制：`tests/test_ve_tos.py` 中 4 个 async 测试需要 pytest async 插件；TOS 聚焦测试已通过。 | + +## 风险 + +- 真实 TOS IAM 行为需要额外的云端 smoke 验证，离线单测无法证明。 +- 公开目录 URL 仍不纳入范围，因为对象存储没有稳定通用的 HTTP 目录清单契约。 diff --git a/tests/cloud/test_harness_app_contract.py b/tests/cloud/test_harness_app_contract.py index 29e08e9e8..73aaf4c56 100644 --- a/tests/cloud/test_harness_app_contract.py +++ b/tests/cloud/test_harness_app_contract.py @@ -191,6 +191,11 @@ def test_structured_tools_skills_and_mcp_are_accepted(self): "space_id": "ss-1", "skill_id": "skill-1", }, + { + "source": "tos", + "tos_url": "tos://customer-bucket/skills/reporting/", + "region": "cn-beijing", + }, ], "mcp": [{"name": "db", "url": "http://db.test/mcp"}], } @@ -206,8 +211,97 @@ def test_structured_tools_skills_and_mcp_are_accepted(self): skill_space_id="ss-1", skill_id="skill-1", ) + assert h.selected_skills[2] == HarnessSelectedSkill( + source="tos", + tos_url="tos://customer-bucket/skills/reporting/", + region="cn-beijing", + ) assert h.mcp == [HarnessMcpServer(name="db", server_url="http://db.test/mcp")] + def test_tos_selected_skill_rejects_legacy_locator_names(self): + with pytest.raises(ValidationError, match="requires tos_url or tos_public_url"): + HarnessOverrides.model_validate( + { + "selected_skills": [ + { + "source": "tos", + "uri": "tos://customer-bucket/skills/reporting/", + } + ] + } + ) + + with pytest.raises(ValidationError, match="requires tos_url or tos_public_url"): + HarnessOverrides.model_validate( + { + "selected_skills": [ + { + "source": "tos", + "public_url": ( + "https://bucket.tos-cn-beijing.volces.com/" + "skills/reporting.zip" + ), + } + ] + } + ) + + def test_tos_selected_skill_requires_tos_url_or_tos_public_url(self): + with pytest.raises(ValidationError, match="requires tos_url or tos_public_url"): + HarnessOverrides.model_validate({"selected_skills": [{"source": "tos"}]}) + + def test_tos_selected_skill_rejects_bucket_root_uri(self): + with pytest.raises(ValidationError, match="must point to a Skill directory"): + HarnessOverrides.model_validate( + {"selected_skills": [{"source": "tos", "tos_url": "tos://bucket"}]} + ) + + def test_tos_selected_skill_rejects_unsafe_uri(self): + with pytest.raises(ValidationError, match="safe Skill directory"): + HarnessOverrides.model_validate( + { + "selected_skills": [ + { + "source": "tos", + "tos_url": "tos://bucket/skills/../secret/", + } + ] + } + ) + + def test_tos_selected_skill_rejects_public_url_userinfo(self): + credential = "user" + ":" + "secret" + with pytest.raises(ValidationError, match="must not include credentials"): + HarnessOverrides.model_validate( + { + "selected_skills": [ + { + "source": "tos", + "tos_public_url": ( + f"https://{credential}@bucket.tos-cn-beijing." + "volces.com/skills/reporting.zip" + ), + } + ] + } + ) + + def test_tos_selected_skill_rejects_public_url_signature_query(self): + with pytest.raises(ValidationError, match="must not include credentials"): + HarnessOverrides.model_validate( + { + "selected_skills": [ + { + "source": "tos", + "tos_public_url": ( + "https://bucket.tos-cn-beijing.volces.com/" + "skills/reporting.zip?X-Tos-Signature=TOPSECRET" + ), + } + ] + } + ) + def test_selected_skill_accepts_skill_space_policy(self): h = HarnessOverrides.model_validate( { diff --git a/tests/cloud/test_harness_skill_download.py b/tests/cloud/test_harness_skill_download.py index 255489d37..4f6cf6b33 100644 --- a/tests/cloud/test_harness_skill_download.py +++ b/tests/cloud/test_harness_skill_download.py @@ -153,6 +153,111 @@ def fail_skillhub_download(*args, **kwargs): assert isinstance(toolset._code_executor, UnsafeLocalCodeExecutor) +def test_build_skill_toolset_loads_tos_skill_uri(monkeypatch, tmp_path): + calls: list[dict[str, object]] = [] + + def fake_materialize_tos_skill_source( + *, uri, public_url, cache_dir, region, version=None + ): + calls.append( + { + "uri": uri, + "public_url": public_url, + "cache_dir": cache_dir, + "region": region, + } + ) + skill_dir = tmp_path / "reporting" + _write_adk_skill(skill_dir, name="reporting") + return skill_dir + + monkeypatch.setattr( + utils, + "materialize_tos_skill_source", + fake_materialize_tos_skill_source, + ) + + toolset = utils.build_skill_toolset( + [ + ( + 'tos:{"tos_url":"tos://customer-bucket/skills/reporting/",' + '"region":"cn-beijing"}' + ) + ], + download_dir=tmp_path, + ) + + assert calls == [ + { + "uri": "tos://customer-bucket/skills/reporting/", + "public_url": None, + "cache_dir": tmp_path, + "region": "cn-beijing", + } + ] + assert [skill.name for skill in toolset._list_skills()] == ["reporting"] + + +def test_build_skill_toolset_loads_multiple_tos_entries(monkeypatch, tmp_path): + def fake_materialize_tos_skill_source( + *, uri, public_url, cache_dir, region, version=None + ): + name = uri.rstrip("/").rsplit("/", 1)[-1] + skill_dir = tmp_path / name + _write_adk_skill(skill_dir, name=name) + return skill_dir + + monkeypatch.setattr( + utils, + "materialize_tos_skill_source", + fake_materialize_tos_skill_source, + ) + + toolset = utils.build_skill_toolset( + [ + 'tos:{"tos_url":"tos://customer-bucket/skills/reporting/"}', + 'tos:{"tos_url":"tos://customer-bucket/skills/search/"}', + ], + download_dir=tmp_path, + ) + + assert [skill.name for skill in toolset._list_skills()] == [ + "reporting", + "search", + ] + + +def test_build_skill_toolset_redacts_tos_ref_on_error(monkeypatch, tmp_path): + def fail_materialize_tos_skill_source( + *, uri, public_url, cache_dir, region, version=None + ): + raise RuntimeError("public URL rejected") + + monkeypatch.setattr( + utils, + "materialize_tos_skill_source", + fail_materialize_tos_skill_source, + ) + credential = "user" + ":" + "secret" + + with pytest.raises(utils.SkillLoadError) as exc_info: + utils.build_skill_toolset( + [ + ( + f'tos:{{"tos_public_url":"https://{credential}@bucket.' + "tos-cn-beijing.volces.com/skills/reporting.zip" + '?X-Tos-Signature=TOPSECRET"}' + ) + ], + download_dir=tmp_path, + ) + + message = str(exc_info.value) + assert "tos:" in message + assert "TOPSECRET" not in message + assert "user:secret" not in message + + @pytest.mark.parametrize( ("policy", "expected_names"), [ diff --git a/tests/skills/test_adk_skill_materializer.py b/tests/skills/test_adk_skill_materializer.py index 51e4c649f..7c1d22d15 100644 --- a/tests/skills/test_adk_skill_materializer.py +++ b/tests/skills/test_adk_skill_materializer.py @@ -37,6 +37,61 @@ def _zip_bytes(files: dict[str, str]) -> bytes: return buffer.getvalue() +class _FakeStreamResponse: + def __init__( + self, + url: str, + content: bytes, + *, + final_url: str | None = None, + chunk_size: int = 8192, + is_redirect: bool = False, + headers: dict[str, str] | None = None, + ) -> None: + self.url = final_url or url + self._content = content + self._chunk_size = chunk_size + self.is_redirect = is_redirect + self.headers = headers or {} + + def __enter__(self) -> "_FakeStreamResponse": + return self + + def __exit__(self, *args: object) -> None: + return None + + def close(self) -> None: + return None + + def raise_for_status(self) -> None: + return None + + def iter_bytes(self): + for offset in range(0, len(self._content), self._chunk_size): + yield self._content[offset : offset + self._chunk_size] + + +class _FakeHttpClient: + def __init__(self, responses: list[_FakeStreamResponse]) -> None: + self._responses = responses + self.requested: list[str] = [] + + def __enter__(self) -> "_FakeHttpClient": + return self + + def __exit__(self, *args: object) -> None: + return None + + def build_request(self, method: str, url: str) -> tuple[str, str]: + assert method == "GET" + self.requested.append(url) + return method, url + + def send(self, request: tuple[str, str], *, stream: bool) -> _FakeStreamResponse: + assert stream is True + return self._responses.pop(0) + + def test_skillhub_skill_downloads_and_normalizes_root_zip( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, @@ -139,6 +194,335 @@ def test_default_cache_dir_can_be_overridden_by_env( assert materializer._default_cache_dir() == tmp_path / "custom-cache" +def test_materialize_tos_skill_uri_downloads_directory( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + calls: list[dict[str, object]] = [] + + class FakeVeTOS: + def __init__(self, **kwargs: object) -> None: + calls.append(kwargs) + + def download_directory( + self, bucket_name: str, prefix: str, local_dir: str, **kwargs: object + ) -> bool: + calls.append( + { + "bucket_name": bucket_name, + "prefix": prefix, + "local_dir": local_dir, + **kwargs, + } + ) + skill_dir = Path(local_dir) + skill_dir.mkdir(parents=True, exist_ok=True) + (skill_dir / "SKILL.md").write_text( + "---\nname: reporting\ndescription: Reporting skill.\n---\nBody.\n", + encoding="utf-8", + ) + return True + + monkeypatch.setattr("veadk.integrations.ve_tos.ve_tos.VeTOS", FakeVeTOS) + monkeypatch.setattr( + materializer, + "_get_cloud_credentials", + lambda: ("runtime-ak", "runtime-sk", "runtime-sts"), + ) + + skill_dir = materializer.materialize_tos_skill_source( + uri="tos://customer-bucket/skills/reporting/", + cache_dir=tmp_path, + region="cn-beijing", + ) + + assert load_skill_from_dir(skill_dir).name == "reporting" + assert calls[0]["bucket_name"] == "customer-bucket" + assert calls[0]["region"] == "cn-beijing" + assert calls[1]["bucket_name"] == "customer-bucket" + assert calls[1]["prefix"] == "skills/reporting/" + assert calls[1]["raise_on_error"] is True + assert calls[1]["max_files"] == materializer._MAX_TOS_DIRECTORY_OBJECTS + assert calls[1]["max_bytes"] == materializer._MAX_TOS_DIRECTORY_BYTES + + +def test_materialize_tos_skill_uri_rejects_parent_directory( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + class FakeVeTOS: + def __init__(self, **kwargs: object) -> None: + pass + + def download_directory( + self, bucket_name: str, prefix: str, local_dir: str, **kwargs: object + ) -> bool: + root = Path(local_dir) + (root / "reporting").mkdir(parents=True) + (root / "reporting" / "SKILL.md").write_text( + "---\nname: reporting\ndescription: Reporting skill.\n---\nBody.\n", + encoding="utf-8", + ) + (root / "search").mkdir() + (root / "search" / "SKILL.md").write_text( + "---\nname: search\ndescription: Search skill.\n---\nBody.\n", + encoding="utf-8", + ) + return True + + monkeypatch.setattr("veadk.integrations.ve_tos.ve_tos.VeTOS", FakeVeTOS) + monkeypatch.setattr( + materializer, + "_get_cloud_credentials", + lambda: ("runtime-ak", "runtime-sk", "runtime-sts"), + ) + + with pytest.raises(SkillMaterializeError, match="has no SKILL.md"): + materializer.materialize_tos_skill_source( + uri="tos://customer-bucket/skills/", + cache_dir=tmp_path, + ) + + +def test_materialize_tos_skill_uri_rejects_bucket_root(tmp_path: Path): + with pytest.raises(SkillMaterializeError, match="must point to a Skill directory"): + materializer.materialize_tos_skill_source( + uri="tos://customer-bucket", + cache_dir=tmp_path, + ) + + +def test_materialize_tos_skill_uri_rejects_path_traversal(tmp_path: Path): + with pytest.raises(SkillMaterializeError, match="safe Skill directory"): + materializer.materialize_tos_skill_source( + uri="tos://customer-bucket/skills/../secret/", + cache_dir=tmp_path, + ) + + +def test_materialize_public_tos_zip_downloads_archive( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + fake_client = _FakeHttpClient( + [ + _FakeStreamResponse( + "https://customer-bucket.tos-cn-beijing.volces.com/skills/reporting.zip", + _zip_bytes( + { + "reporting/SKILL.md": ( + "---\nname: reporting\ndescription: Reporting skill.\n---\n" + "Public body.\n" + ) + } + ), + ) + ] + ) + + monkeypatch.setattr(materializer.httpx, "Client", lambda **kwargs: fake_client) + + skill_dir = materializer.materialize_tos_skill_source( + public_url="https://customer-bucket.tos-cn-beijing.volces.com/skills/reporting.zip", + cache_dir=tmp_path, + ) + + assert fake_client.requested == [ + "https://customer-bucket.tos-cn-beijing.volces.com/skills/reporting.zip" + ] + assert load_skill_from_dir(skill_dir).name == "reporting" + + +def test_materialize_public_tos_zip_accepts_split_zip_header( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + fake_client = _FakeHttpClient( + [ + _FakeStreamResponse( + "https://bucket.tos-cn-beijing.volces.com/reporting.zip", + _zip_bytes( + { + "SKILL.md": ( + "---\nname: reporting\ndescription: Reporting skill.\n---\n" + "Body.\n" + ) + } + ), + chunk_size=1, + ) + ] + ) + + monkeypatch.setattr(materializer.httpx, "Client", lambda **kwargs: fake_client) + + skill_dir = materializer.materialize_tos_skill_source( + public_url="https://bucket.tos-cn-beijing.volces.com/reporting.zip", + cache_dir=tmp_path, + ) + + assert load_skill_from_dir(skill_dir).name == "reporting" + + +def test_materialize_public_tos_zip_validates_each_redirect( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + fake_client = _FakeHttpClient( + [ + _FakeStreamResponse( + "https://bucket.tos-cn-beijing.volces.com/reporting.zip", + b"", + is_redirect=True, + headers={"location": "https://169.254.169.254/latest/meta-data"}, + ) + ] + ) + + monkeypatch.setattr(materializer.httpx, "Client", lambda **kwargs: fake_client) + + with pytest.raises(SkillMaterializeError, match="host is not a TOS endpoint"): + materializer.materialize_tos_skill_source( + public_url="https://bucket.tos-cn-beijing.volces.com/reporting.zip", + cache_dir=tmp_path, + ) + + +def test_materialize_public_tos_zip_rejects_userinfo(tmp_path: Path): + credential = "ak" + ":" + "sk" + with pytest.raises(SkillMaterializeError, match="must not include credentials"): + materializer.materialize_tos_skill_source( + public_url=( + f"https://{credential}@bucket.tos-cn-beijing.volces.com/reporting.zip" + ), + cache_dir=tmp_path, + ) + + +def test_materialize_public_tos_zip_rejects_signed_query(tmp_path: Path): + with pytest.raises(SkillMaterializeError, match="must not include credentials"): + materializer.materialize_tos_skill_source( + public_url=( + "https://bucket.tos-cn-beijing.volces.com/reporting.zip" + "?X-Tos-Signature=secret" + ), + cache_dir=tmp_path, + ) + + +def test_materialize_public_tos_zip_rejects_fake_tos_host(tmp_path: Path): + with pytest.raises(SkillMaterializeError, match="host is not a TOS endpoint"): + materializer.materialize_tos_skill_source( + public_url="https://evil.tos-cn-beijing.volces.com.example.com/reporting.zip", + cache_dir=tmp_path, + ) + + +def test_materialize_public_tos_zip_rejects_redirect_to_non_tos_host( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + fake_client = _FakeHttpClient( + [ + _FakeStreamResponse( + "https://bucket.tos-cn-beijing.volces.com/reporting.zip", + b"", + is_redirect=True, + headers={"location": "https://example.com/reporting.zip"}, + ) + ] + ) + + monkeypatch.setattr(materializer.httpx, "Client", lambda **kwargs: fake_client) + + with pytest.raises(SkillMaterializeError, match="host is not a TOS endpoint"): + materializer.materialize_tos_skill_source( + public_url="https://bucket.tos-cn-beijing.volces.com/reporting.zip", + cache_dir=tmp_path, + ) + + +def test_materialize_public_tos_zip_streams_size_limit( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + monkeypatch.setattr(materializer, "_MAX_TOS_PUBLIC_ARCHIVE_BYTES", 4) + + fake_client = _FakeHttpClient( + [ + _FakeStreamResponse( + "https://bucket.tos-cn-beijing.volces.com/reporting.zip", + b"PK\x03\x04too-large", + chunk_size=2, + ) + ] + ) + + monkeypatch.setattr(materializer.httpx, "Client", lambda **kwargs: fake_client) + + with pytest.raises(SkillMaterializeError, match="exceeds 64 MiB"): + materializer.materialize_tos_skill_source( + public_url="https://bucket.tos-cn-beijing.volces.com/reporting.zip", + cache_dir=tmp_path, + ) + + +def test_materialize_public_tos_zip_rejects_uncompressed_size_limit( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + monkeypatch.setattr(materializer, "_MAX_TOS_PUBLIC_UNCOMPRESSED_BYTES", 4) + fake_client = _FakeHttpClient( + [ + _FakeStreamResponse( + "https://bucket.tos-cn-beijing.volces.com/reporting.zip", + _zip_bytes( + { + "SKILL.md": "---\nname: reporting\n---\nBody.\n", + "large.txt": "12345", + } + ), + ) + ] + ) + + monkeypatch.setattr(materializer.httpx, "Client", lambda **kwargs: fake_client) + + with pytest.raises(SkillMaterializeError, match="uncompressed size limit"): + materializer.materialize_tos_skill_source( + public_url="https://bucket.tos-cn-beijing.volces.com/reporting.zip", + cache_dir=tmp_path, + ) + + +def test_materialize_public_tos_zip_rejects_file_count_limit( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + monkeypatch.setattr(materializer, "_MAX_TOS_PUBLIC_ARCHIVE_FILES", 1) + fake_client = _FakeHttpClient( + [ + _FakeStreamResponse( + "https://bucket.tos-cn-beijing.volces.com/reporting.zip", + _zip_bytes( + { + "SKILL.md": "---\nname: reporting\n---\nBody.\n", + "extra.txt": "extra", + } + ), + ) + ] + ) + + monkeypatch.setattr(materializer.httpx, "Client", lambda **kwargs: fake_client) + + with pytest.raises(SkillMaterializeError, match="file count limit"): + materializer.materialize_tos_skill_source( + public_url="https://bucket.tos-cn-beijing.volces.com/reporting.zip", + cache_dir=tmp_path, + ) + + def test_legacy_skillspace_skill_uses_tos_path_version( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, diff --git a/tests/test_harness_sidecar_release_gate.py b/tests/test_harness_sidecar_release_gate.py index 257ba3cde..6726d3e10 100644 --- a/tests/test_harness_sidecar_release_gate.py +++ b/tests/test_harness_sidecar_release_gate.py @@ -42,7 +42,7 @@ def test_sidecar_release_gate_runs_backend_and_frontend_in_parallel() -> None: aggregate = jobs["gate"] assert "needs" not in backend assert "needs" not in frontend - assert backend["timeout-minutes"] == 3 + assert backend["timeout-minutes"] == 10 assert frontend["timeout-minutes"] == 3 assert set(aggregate["needs"]) == {"backend-gate", "frontend-gate"} assert aggregate["if"] == "${{ always() }}" @@ -133,3 +133,5 @@ def test_sidecar_release_gate_runs_backend_and_frontend_in_parallel() -> None: assert "npm test" in frontend_run assert "BACKEND_RESULT" in aggregate_run assert "FRONTEND_RESULT" in aggregate_run + assert "Backend gate result: $BACKEND_RESULT" in aggregate_run + assert 'test "$BACKEND_RESULT" = success' in aggregate_run diff --git a/tests/test_ve_tos.py b/tests/test_ve_tos.py index 17bdc77b8..ac5a00442 100644 --- a/tests/test_ve_tos.py +++ b/tests/test_ve_tos.py @@ -104,6 +104,86 @@ def fail_get_object(*_args, **_kwargs): ) +def test_download_rejects_object_over_byte_limit(monkeypatch, fake_tos, tmp_path): + client = VeTOS(bucket_name="test-bucket") + + monkeypatch.setattr( + client._client, + "get_object", + lambda *_args, **_kwargs: iter([b"123", b"45"]), + raising=False, + ) + + with pytest.raises(ValueError, match="exceeds download byte limit"): + client.download( + "test-bucket", + "skills/reporting/large.bin", + str(tmp_path / "large.bin"), + raise_on_error=True, + max_bytes=4, + ) + + +def test_download_directory_rejects_too_many_objects(monkeypatch, fake_tos, tmp_path): + client = VeTOS(bucket_name="test-bucket") + listing = SimpleNamespace( + is_truncated=False, + next_continuation_token="", + contents=[ + SimpleNamespace(key="skills/reporting/SKILL.md", size=10), + SimpleNamespace(key="skills/reporting/run.py", size=10), + ], + ) + + monkeypatch.setattr( + client._client, + "list_objects_type2", + lambda *_args, **_kwargs: listing, + raising=False, + ) + + with pytest.raises(ValueError, match="file count limit"): + client.download_directory( + "test-bucket", + "skills/reporting/", + str(tmp_path), + raise_on_error=True, + max_files=0, + ) + + +def test_download_directory_rejects_total_bytes_from_listing( + monkeypatch, + fake_tos, + tmp_path, +): + client = VeTOS(bucket_name="test-bucket") + listing = SimpleNamespace( + is_truncated=False, + next_continuation_token="", + contents=[ + SimpleNamespace(key="skills/reporting/SKILL.md", size=10), + SimpleNamespace(key="skills/reporting/run.py", size=10), + ], + ) + + monkeypatch.setattr( + client._client, + "list_objects_type2", + lambda *_args, **_kwargs: listing, + raising=False, + ) + + with pytest.raises(ValueError, match="byte limit"): + client.download_directory( + "test-bucket", + "skills/reporting/", + str(tmp_path), + raise_on_error=True, + max_bytes=5, + ) + + def test_tos_config_uses_region_env_fallback_and_updates_endpoint(monkeypatch): from veadk.configs.database_configs import TOSConfig diff --git a/veadk/cloud/harness_app/types.py b/veadk/cloud/harness_app/types.py index 6cb0d7772..6a2c0a637 100644 --- a/veadk/cloud/harness_app/types.py +++ b/veadk/cloud/harness_app/types.py @@ -22,16 +22,29 @@ the overridable params with additional creation-time settings. ``tools`` and ``skills`` are comma-separated strings (e.g. ``"web_search,web_fetch"``). -Skills may be SkillHub names/slugs or skills-center refs prefixed with ``space:``. +Skills may be SkillHub names/slugs, skills-center refs prefixed with ``space:``, +or structured TOS selections. """ from typing import Any, Literal +from urllib.parse import parse_qsl, urlparse from pydantic import AliasChoices, BaseModel, ConfigDict, Field, model_validator from veadk.consts import DEFAULT_MODEL_AGENT_NAME from veadk.prompts.agent_default_prompt import DEFAULT_DESCRIPTION, DEFAULT_INSTRUCTION +_TOS_PUBLIC_HOST_SUFFIXES = (".volces.com", ".bytepluses.com") +_TOS_PUBLIC_CREDENTIAL_QUERY_KEYS = { + "x-tos-algorithm", + "x-tos-credential", + "x-tos-date", + "x-tos-expires", + "x-tos-security-token", + "x-tos-signature", + "x-tos-signedheaders", +} + class HarnessBuiltinTool(BaseModel): """Built-in tool selected by AgentKit runtime config.""" @@ -78,7 +91,7 @@ class HarnessSelectedSkill(BaseModel): model_config = ConfigDict(populate_by_name=True) - source: Literal["skillhub", "skillspace"] = Field( + source: Literal["skillhub", "skillspace", "tos"] = Field( default="skillhub", description="Skill source.", ) @@ -98,13 +111,21 @@ class HarnessSelectedSkill(BaseModel): skill_id: str | None = Field(default=None, description="Skill id.") version: str | None = Field(default=None, description="Skill version.") region: str | None = Field(default=None, description="Skill region.") + tos_url: str | None = Field( + default=None, + description="TOS URL pointing to one Skill directory.", + ) + tos_public_url: str | None = Field( + default=None, + description="Public readable TOS zip URL.", + ) skill_space_policy: HarnessSkillSpacePolicy | None = Field( default=None, description="Allow/deny filter applied to this skill_space_id.", ) @model_validator(mode="after") - def _validate_skill_space_policy(self) -> "HarnessSelectedSkill": + def _validate_selected_skill(self) -> "HarnessSelectedSkill": if self.skill_space_policy is not None and self.source != "skillspace": raise ValueError("skill_space_policy is only supported for skillspace") if ( @@ -112,9 +133,71 @@ def _validate_skill_space_policy(self) -> "HarnessSelectedSkill": and not (self.skill_space_id or "").strip() ): raise ValueError("skill_space_policy requires skill_space_id") + if self.source == "tos": + uri = (self.tos_url or "").strip() + public_url = (self.tos_public_url or "").strip() + if bool(uri) == bool(public_url): + raise ValueError( + "tos selected skill requires tos_url or tos_public_url" + ) + if uri: + _validate_tos_skill_uri(uri) + if public_url: + _validate_tos_public_url(public_url) return self +def _validate_tos_skill_uri(uri: str) -> None: + if not uri.startswith("tos://"): + raise ValueError("tos selected skill tos_url must start with tos://") + remainder = uri.removeprefix("tos://") + if "/" not in remainder: + raise ValueError("tos selected skill tos_url must point to a Skill directory") + prefix = remainder.split("/", 1)[1] + if not prefix.strip("/"): + raise ValueError("tos selected skill tos_url must point to a Skill directory") + parts = [part for part in prefix.split("/") if part] + if any(part in {".", ".."} for part in parts) or len(parts) != len( + prefix.strip("/").split("/") + ): + raise ValueError( + "tos selected skill tos_url must point to a safe Skill directory" + ) + + +def _validate_tos_public_url(public_url: str) -> None: + parsed = urlparse(public_url.strip()) + host = (parsed.hostname or "").lower() + if parsed.scheme != "https" or not host: + raise ValueError("tos selected skill tos_public_url must be an HTTPS URL") + if parsed.username or parsed.password: + raise ValueError( + "tos selected skill tos_public_url must not include credentials" + ) + if not _is_tos_public_host(host): + raise ValueError( + "tos selected skill tos_public_url host must be a TOS endpoint" + ) + query_keys = {key.lower() for key, _value in parse_qsl(parsed.query)} + if query_keys & _TOS_PUBLIC_CREDENTIAL_QUERY_KEYS: + raise ValueError( + "tos selected skill tos_public_url must not include credentials" + ) + if not parsed.path.endswith(".zip"): + raise ValueError( + "tos selected skill tos_public_url must point to a zip archive" + ) + + +def _is_tos_public_host(host: str) -> bool: + labels = host.split(".") + if len(labels) < 4: + return False + if not any(host.endswith(suffix) for suffix in _TOS_PUBLIC_HOST_SUFFIXES): + return False + return labels[-3].startswith("tos-") + + class HarnessResourceOverride(BaseModel): """Request-level knowledge or memory backend override.""" diff --git a/veadk/cloud/harness_app/utils.py b/veadk/cloud/harness_app/utils.py index 02f4fcad4..4233c4533 100644 --- a/veadk/cloud/harness_app/utils.py +++ b/veadk/cloud/harness_app/utils.py @@ -66,7 +66,10 @@ SkillSpacePolicySet, parse_skill_space_policy, ) -from veadk.skills.materializer import materialize_remote_skill +from veadk.skills.materializer import ( + materialize_remote_skill, + materialize_tos_skill_source, +) from veadk.skills.utils import ( _filter_skills_by_policy, _filter_skills_by_policy_env, @@ -101,6 +104,7 @@ _KNOWLEDGEBASE_TOOL_NAMES = {"load_knowledgebase", "load_kb_queries"} _LONGTERM_MEMORY_TOOL_NAMES = {"load_memory"} _SKILL_CENTER_SPACE_PREFIX = "space:" +_TOS_SKILL_PREFIX = "tos:" __all__ = [ "HarnessConfig", @@ -325,9 +329,42 @@ def _skill_from_legacy_ref(skill: str) -> HarnessSelectedSkill: def _selected_skill_ref(skill: HarnessSelectedSkill) -> str: if skill.source == "skillspace": return f"{_SKILL_CENTER_SPACE_PREFIX}{skill.skill_space_id or ''}".strip() + if skill.source == "tos": + payload = skill.model_dump( + mode="json", + include={"tos_url", "tos_public_url", "region", "version"}, + exclude_none=True, + ) + return _TOS_SKILL_PREFIX + json.dumps( + payload, + ensure_ascii=False, + separators=(",", ":"), + sort_keys=True, + ) return (skill.slug or "").strip() +def _is_tos_skill_ref(skill: str) -> bool: + return skill.strip().startswith(_TOS_SKILL_PREFIX) + + +def _tos_skill_ref_payload(skill: str) -> dict[str, Any]: + raw = skill.strip().removeprefix(_TOS_SKILL_PREFIX) + try: + payload = json.loads(raw) + except json.JSONDecodeError as e: + raise ValueError(f"Invalid TOS skill ref JSON: {e}") from e + if not isinstance(payload, dict): + raise TypeError("TOS skill ref must be a JSON object.") + return payload + + +def _safe_skill_ref_for_error(skill: str) -> str: + if _is_tos_skill_ref(skill): + return f"{_TOS_SKILL_PREFIX}" + return skill + + def _dedupe_builtin_tools( entries: list[HarnessBuiltinTool], ) -> list[HarnessBuiltinTool]: @@ -746,6 +783,16 @@ def build_skill_toolset( cache_dir=download_dir, ) loaded_skills.append(load_skill_from_dir(skill_dir)) + elif _is_tos_skill_ref(skill): + payload = _tos_skill_ref_payload(skill) + skill_dir = materialize_tos_skill_source( + uri=payload.get("tos_url"), + public_url=payload.get("tos_public_url"), + cache_dir=download_dir, + region=payload.get("region"), + version=payload.get("version"), + ) + loaded_skills.append(load_skill_from_dir(skill_dir)) else: loaded_skills.append( load_skill_from_dir( @@ -753,7 +800,8 @@ def build_skill_toolset( ) ) except Exception as e: - raise SkillLoadError(f"Skill '{skill}' failed to load: {e}") from e + safe_skill = _safe_skill_ref_for_error(skill) + raise SkillLoadError(f"Skill '{safe_skill}' failed to load: {e}") from e if not loaded_skills: return None return SkillToolset( diff --git a/veadk/integrations/ve_tos/ve_tos.py b/veadk/integrations/ve_tos/ve_tos.py index f88c6f8fd..c508a825c 100644 --- a/veadk/integrations/ve_tos/ve_tos.py +++ b/veadk/integrations/ve_tos/ve_tos.py @@ -718,6 +718,7 @@ def download( save_path: str, *, raise_on_error: bool = False, + max_bytes: int | None = None, ) -> bool: """download object from TOS""" bucket_name = self._check_bucket_name(bucket_name) @@ -733,7 +734,13 @@ def download( os.makedirs(save_dir, exist_ok=True) with open(save_path, "wb") as f: + downloaded_bytes = 0 for chunk in object_stream: + downloaded_bytes += len(chunk) + if max_bytes is not None and downloaded_bytes > max_bytes: + raise ValueError( + f"TOS object exceeds download byte limit: {object_key}" + ) f.write(chunk) logger.debug(f"Image download success, saved to: {save_path}") @@ -746,7 +753,14 @@ def download( return False def download_directory( - self, bucket_name: str, prefix: str, local_dir: str = "/tmp" + self, + bucket_name: str, + prefix: str, + local_dir: str = "/tmp", + *, + raise_on_error: bool = False, + max_files: int | None = None, + max_bytes: int | None = None, ) -> bool: """Download entire directory from TOS bucket to local directory @@ -754,6 +768,9 @@ def download_directory( bucket_name: TOS bucket name prefix: Directory prefix in TOS (e.g., "skills/pdf/") local_dir: Local directory path to save files + raise_on_error: Whether to propagate TOS/list/download errors. + max_files: Optional maximum number of files to download. + max_bytes: Optional maximum total bytes to download. Returns: bool: True if download succeeds, False otherwise @@ -771,11 +788,15 @@ def download_directory( # Create local directory if not exists os.makedirs(local_dir, exist_ok=True) + local_root = os.path.realpath(local_dir) # List all objects with the prefix is_truncated = True next_continuation_token = "" downloaded_count = 0 + planned_count = 0 + planned_bytes = 0 + downloaded_bytes = 0 while is_truncated: out = self._client.list_objects_type2( @@ -794,9 +815,35 @@ def download_directory( if object_key.endswith("/"): continue + planned_count += 1 + if max_files is not None and planned_count > max_files: + raise ValueError( + f"TOS directory exceeds file count limit: {prefix}" + ) + object_size = getattr(content, "size", None) + object_size_int = int(object_size) if object_size is not None else 0 + if max_bytes is not None and object_size is not None: + planned_bytes += object_size_int + if planned_bytes > max_bytes: + raise ValueError( + f"TOS directory exceeds byte limit: {prefix}" + ) + # Calculate relative path and local file path relative_path = object_key[len(prefix) :] local_file_path = os.path.join(local_dir, relative_path) + real_file_path = os.path.realpath(local_file_path) + if not ( + real_file_path == local_root + or real_file_path.startswith(local_root + os.sep) + ): + message = ( + f"Unsafe object key escapes target directory: {object_key}" + ) + logger.error(message) + if raise_on_error: + raise ValueError(message) + return False # Create subdirectories if needed local_file_dir = os.path.dirname(local_file_path) @@ -804,7 +851,16 @@ def download_directory( os.makedirs(local_file_dir, exist_ok=True) # Download the file - if self.download(bucket_name, object_key, local_file_path): + if self.download( + bucket_name, + object_key, + local_file_path, + raise_on_error=raise_on_error, + max_bytes=( + None if max_bytes is None else max_bytes - downloaded_bytes + ), + ): + downloaded_bytes += os.path.getsize(local_file_path) downloaded_count += 1 logger.debug(f"Downloaded: {object_key} -> {local_file_path}") else: @@ -817,6 +873,8 @@ def download_directory( except Exception as e: logger.error(f"Failed to download directory: {str(e)}") + if raise_on_error: + raise return False def close(self): diff --git a/veadk/skills/materializer.py b/veadk/skills/materializer.py index e66611ab1..5c9e0c61f 100644 --- a/veadk/skills/materializer.py +++ b/veadk/skills/materializer.py @@ -24,7 +24,7 @@ import zipfile from pathlib import Path from typing import Any -from urllib.parse import quote +from urllib.parse import parse_qsl, quote, urljoin, urlparse import frontmatter import httpx @@ -47,6 +47,22 @@ "https://skills.volces.com/v1/skills/download", ) _MAX_FINDSKILL_ARCHIVE_BYTES = 64 * 1024 * 1024 +_MAX_TOS_PUBLIC_ARCHIVE_BYTES = 64 * 1024 * 1024 +_MAX_TOS_PUBLIC_UNCOMPRESSED_BYTES = 64 * 1024 * 1024 +_MAX_TOS_PUBLIC_ARCHIVE_FILES = 1024 +_MAX_TOS_PUBLIC_REDIRECTS = 3 +_MAX_TOS_DIRECTORY_BYTES = 64 * 1024 * 1024 +_MAX_TOS_DIRECTORY_OBJECTS = 1024 +_TOS_PUBLIC_HOST_SUFFIXES = (".volces.com", ".bytepluses.com") +_TOS_PUBLIC_CREDENTIAL_QUERY_KEYS = { + "x-tos-algorithm", + "x-tos-credential", + "x-tos-date", + "x-tos-expires", + "x-tos-security-token", + "x-tos-signature", + "x-tos-signedheaders", +} def materialize_remote_skill( @@ -60,10 +76,14 @@ def materialize_remote_skill( base_cache_dir = _default_cache_dir() if cache_dir is None else Path(cache_dir) _ensure_cache_dir(base_cache_dir) - source_type = ( - "skillhub" if skill.source_type in {"skillhub", "findskill"} else "skillspace" + source_type = _cache_source_type(skill) + source_id = ( + skill.skill_space_id + or skill.slug + or skill.id + or skill.bucket_name + or "unknown-source" ) - source_id = skill.skill_space_id or skill.slug or skill.id or "unknown-source" version_key = skill_version_key(skill) version_dir = ( base_cache_dir @@ -94,6 +114,28 @@ def materialize_remote_skill( shutil.rmtree(staging_dir) version_dir.mkdir(parents=True, exist_ok=True) + if skill.source_type == "tos": + _download_tos_skill_directory( + skill, + staging_dir, + region=region, + ) + final_dir = _normalize_extracted_skill_dir( + staging_dir, + version_dir, + skill, + require_root_skill=True, + ) + try: + load_skill_from_dir(final_dir) + except Exception as e: + raise SkillMaterializeError( + f"Skill directory '{final_dir}' failed ADK load validation: {e}" + ) from e + + _cleanup_old_versions(version_dir) + return final_dir + _download_remote_skill( skill, zip_path, @@ -101,7 +143,20 @@ def materialize_remote_skill( region=region, ) try: - _safe_extract_zip(zip_path, staging_dir) + _safe_extract_zip( + zip_path, + staging_dir, + max_files=( + _MAX_TOS_PUBLIC_ARCHIVE_FILES + if skill.source_type == "tos-public" + else None + ), + max_uncompressed_bytes=( + _MAX_TOS_PUBLIC_UNCOMPRESSED_BYTES + if skill.source_type == "tos-public" + else None + ), + ) finally: if zip_path.exists(): zip_path.unlink() @@ -118,6 +173,42 @@ def materialize_remote_skill( return final_dir +def materialize_tos_skill_source( + *, + uri: str | None = None, + public_url: str | None = None, + cache_dir: Path | None = None, + region: str | None = None, + version: str | None = None, +) -> Path: + """Materialize one TOS-backed Skill directory or public zip URL.""" + if bool((uri or "").strip()) == bool((public_url or "").strip()): + raise SkillMaterializeError("TOS skill source requires uri or public_url.") + + if uri: + bucket, prefix = _parse_tos_skill_uri(uri) + skill = Skill( + name=_skill_name_from_prefix(prefix), + description="TOS skill.", + path=prefix, + bucket_name=bucket, + source_type="tos", + version_id=version, + ) + else: + public_url = _validate_public_tos_zip_url(public_url or "") + skill = Skill( + name=Path(urlparse(public_url).path).stem or "public-tos-skill", + description="Public TOS skill.", + path=public_url, + source_type="tos-public", + public_url=public_url, + version_id=version, + ) + + return materialize_remote_skill(skill, cache_dir=cache_dir, region=region) + + def _download_remote_skill( skill: Skill, zip_path: Path, @@ -138,6 +229,10 @@ def _download_remote_skill( ) return + if skill.source_type == "tos-public": + _download_public_tos_zip(skill, zip_path) + return + legacy_options: dict[str, Any] = {} if credentials is not None: legacy_options["credentials"] = credentials @@ -175,6 +270,149 @@ def _download_findskill_skill(skill: Skill, zip_path: Path) -> None: zip_path.write_bytes(content) +def _download_public_tos_zip(skill: Skill, zip_path: Path) -> None: + public_url = _validate_public_tos_zip_url(skill.public_url or skill.path) + try: + final_response = None + with httpx.Client(timeout=60, follow_redirects=False) as client: + next_url = public_url + for _ in range(_MAX_TOS_PUBLIC_REDIRECTS + 1): + _validate_public_tos_zip_url(next_url) + response = client.build_request("GET", next_url) + streamed_response = client.send(response, stream=True) + if streamed_response.is_redirect: + location = streamed_response.headers.get("location") + streamed_response.close() + if not location: + raise SkillMaterializeError( + "Public TOS skill redirect has no Location header." + ) + next_url = _validate_public_tos_zip_url(urljoin(next_url, location)) + continue + final_response = streamed_response + break + if final_response is None: + raise SkillMaterializeError( + "Public TOS skill redirects too many times." + ) + try: + final_response.raise_for_status() + zip_path.parent.mkdir(parents=True, exist_ok=True) + total_bytes = 0 + header = b"" + with zip_path.open("wb") as output: + for chunk in final_response.iter_bytes(): + if not chunk: + continue + if len(header) < 4: + header = (header + chunk)[:4] + total_bytes += len(chunk) + if total_bytes > _MAX_TOS_PUBLIC_ARCHIVE_BYTES: + raise SkillMaterializeError( + "Public TOS skill archive exceeds 64 MiB." + ) + output.write(chunk) + finally: + final_response.close() + except httpx.HTTPError as error: + raise SkillMaterializeError("Failed to download public TOS skill.") from error + if not (header.startswith(b"PK\x03\x04") or header.startswith(b"PK\x05\x06")): + raise SkillMaterializeError("Public TOS skill download is not a zip archive.") + + +def _download_tos_skill_directory( + skill: Skill, + dest_dir: Path, + *, + region: str | None = None, +) -> None: + if not skill.bucket_name or not skill.path: + raise SkillMaterializeError("TOS skill is missing bucket or prefix.") + from veadk.integrations.ve_tos.ve_tos import VeTOS + + access_key, secret_key, session_token = _get_cloud_credentials() + tos_client = VeTOS( + ak=access_key, + sk=secret_key, + session_token=session_token, + bucket_name=skill.bucket_name, + region=region or "", + ) + downloaded = tos_client.download_directory( + skill.bucket_name, + skill.path, + str(dest_dir), + raise_on_error=True, + max_files=_MAX_TOS_DIRECTORY_OBJECTS, + max_bytes=_MAX_TOS_DIRECTORY_BYTES, + ) + if not downloaded: + raise SkillMaterializeError("TOS skill directory contains no files.") + + +def _parse_tos_skill_uri(uri: str) -> tuple[str, str]: + parsed = urlparse(uri) + if parsed.scheme != "tos" or not parsed.netloc: + raise SkillMaterializeError("TOS skill uri must use tos://bucket/prefix.") + prefix = parsed.path.lstrip("/") + if not prefix or not prefix.strip("/"): + raise SkillMaterializeError("TOS skill uri must point to a Skill directory.") + _validate_relative_object_path(prefix) + if not prefix.endswith("/"): + prefix += "/" + return parsed.netloc, prefix + + +def _validate_relative_object_path(value: str) -> None: + path = Path(value) + if path.is_absolute() or any(part in {"", ".", ".."} for part in path.parts): + raise SkillMaterializeError( + "TOS skill uri must point to a safe Skill directory." + ) + + +def _cache_source_type(skill: Skill) -> str: + if skill.source_type in {"skillhub", "findskill"}: + return "skillhub" + if skill.source_type in {"tos", "tos-public"}: + return "tos" + return "skillspace" + + +def _skill_name_from_prefix(prefix: str) -> str: + return Path(prefix.rstrip("/")).name or "tos-skill" + + +def _validate_public_tos_zip_url(url: str) -> str: + parsed = urlparse(url.strip()) + host = (parsed.hostname or "").lower() + if parsed.scheme != "https" or not host: + raise SkillMaterializeError("Public TOS skill URL must be an HTTPS URL.") + if parsed.username or parsed.password: + raise SkillMaterializeError( + "Public TOS skill URL must not include credentials." + ) + if not _is_tos_public_host(host): + raise SkillMaterializeError("Public TOS skill URL host is not a TOS endpoint.") + query_keys = {key.lower() for key, _value in parse_qsl(parsed.query)} + if query_keys & _TOS_PUBLIC_CREDENTIAL_QUERY_KEYS: + raise SkillMaterializeError( + "Public TOS skill URL must not include credentials." + ) + if not parsed.path.endswith(".zip"): + raise SkillMaterializeError("Public TOS skill URL must point to a zip archive.") + return url.strip() + + +def _is_tos_public_host(host: str) -> bool: + labels = host.split(".") + if len(labels) < 4: + return False + if not any(host.endswith(suffix) for suffix in _TOS_PUBLIC_HOST_SUFFIXES): + return False + return labels[-3].startswith("tos-") + + def _download_legacy_skill_space_skill( skill: Skill, zip_path: Path, @@ -297,12 +535,20 @@ def _download_legacy_skill_via_vestack( return False -def _safe_extract_zip(zip_path: Path, dest_dir: Path) -> None: +def _safe_extract_zip( + zip_path: Path, + dest_dir: Path, + *, + max_files: int | None = None, + max_uncompressed_bytes: int | None = None, +) -> None: dest_root = dest_dir.resolve() dest_dir.mkdir(parents=True, exist_ok=True) try: with zipfile.ZipFile(zip_path, "r") as zf: + total_files = 0 + total_uncompressed_bytes = 0 for member in zf.infolist(): member_name = member.filename if ( @@ -317,6 +563,21 @@ def _safe_extract_zip(zip_path: Path, dest_dir: Path) -> None: raise SkillMaterializeError( f"Unsafe path detected in zip archive: '{member_name}'" ) + if member.is_dir(): + continue + total_files += 1 + if max_files is not None and total_files > max_files: + raise SkillMaterializeError( + f"Zip archive exceeds file count limit of {max_files}." + ) + total_uncompressed_bytes += member.file_size + if ( + max_uncompressed_bytes is not None + and total_uncompressed_bytes > max_uncompressed_bytes + ): + raise SkillMaterializeError( + "Zip archive exceeds uncompressed size limit." + ) zf.extractall(path=str(dest_root)) except zipfile.BadZipFile as e: raise SkillMaterializeError( @@ -328,8 +589,12 @@ def _normalize_extracted_skill_dir( staging_dir: Path, version_dir: Path, source_skill: Skill, + *, + require_root_skill: bool = False, ) -> Path: - skill_dir = _find_extracted_skill_dir(staging_dir) + skill_dir = ( + staging_dir if require_root_skill else _find_extracted_skill_dir(staging_dir) + ) skill_md = _find_skill_md(skill_dir) if skill_md is None: raise SkillMaterializeError( diff --git a/veadk/skills/skill.py b/veadk/skills/skill.py index df0b5e2c1..5f195eae8 100644 --- a/veadk/skills/skill.py +++ b/veadk/skills/skill.py @@ -27,6 +27,7 @@ class Skill(BaseModel): slug: Optional[str] = None source_type: Optional[str] = None version_id: Optional[str] = None + public_url: Optional[str] = None def get_checklist_items(self) -> List[str]: return [item.get("item", item.get("id", "")) for item in self.checklist]