From eab2cc83167fa31728007593cb310042f828490b Mon Sep 17 00:00:00 2001 From: "lixuefei.nice" Date: Fri, 9 Oct 2026 15:19:52 +0800 Subject: [PATCH] feat(harness): support per-space skill policies --- tests/cloud/test_harness_app_contract.py | 138 +++++++++++++++- tests/cloud/test_harness_skill_download.py | 156 +++++++++++++++++- tests/frontend/server/skills/test_reviews.py | 15 +- tests/skills/test_skill_space_policy.py | 88 ++++++++++ .../builtin_tools/test_run_sandbox_agent.py | 4 +- veadk/cloud/harness_app/types.py | 29 +++- veadk/cloud/harness_app/utils.py | 100 +++++++++-- veadk/skills/policy.py | 112 ++++++++++--- veadk/skills/utils.py | 54 ++++-- 9 files changed, 639 insertions(+), 57 deletions(-) diff --git a/tests/cloud/test_harness_app_contract.py b/tests/cloud/test_harness_app_contract.py index 6e3300e69..29e08e9e8 100644 --- a/tests/cloud/test_harness_app_contract.py +++ b/tests/cloud/test_harness_app_contract.py @@ -53,6 +53,7 @@ HarnessResourceOverride, HarnessResponseMetrics, HarnessSelectedSkill, + HarnessSkillSpacePolicy, InvokeHarnessRequest, InvokeHarnessResponse, LlmUsageMetrics, @@ -207,6 +208,50 @@ def test_structured_tools_skills_and_mcp_are_accepted(self): ) assert h.mcp == [HarnessMcpServer(name="db", server_url="http://db.test/mcp")] + def test_selected_skill_accepts_skill_space_policy(self): + h = HarnessOverrides.model_validate( + { + "selected_skills": [ + { + "source": "skillspace", + "skill_space_id": "ss-a", + "skill_space_policy": { + "mode": "deny", + "ids": ["s-blocked"], + }, + } + ] + } + ) + + assert h.selected_skills == [ + HarnessSelectedSkill( + source="skillspace", + skill_space_id="ss-a", + skill_space_policy=HarnessSkillSpacePolicy( + mode="deny", + ids=["s-blocked"], + ), + ) + ] + + def test_selected_skill_policy_requires_skillspace_source(self): + with pytest.raises(ValidationError, match="only supported for skillspace"): + HarnessOverrides.model_validate( + { + "selected_skills": [ + { + "source": "skillhub", + "slug": "team/reporting", + "skill_space_policy": { + "mode": "allow", + "ids": ["s-1"], + }, + } + ] + } + ) + def test_structured_registry_is_accepted_and_normalized(self): h = HarnessOverrides.model_validate( { @@ -1236,7 +1281,9 @@ def __init__(self, names): def custom_tool(): return "custom" - def fake_build_skill_toolset(skill_ids, download_dir=None): + def fake_build_skill_toolset( + skill_ids, download_dir=None, skill_space_policy=None + ): return FakeSkillToolset(skill_ids) if skill_ids else None custom_tool.__name__ = "custom_tool" @@ -1260,6 +1307,95 @@ def fake_build_skill_toolset(skill_ids, download_dir=None): assert len(skill_toolsets) == 1 assert skill_toolsets[0].names == ["team/new-skill"] + def test_spawn_passes_selected_skill_space_policy(self, monkeypatch): + from veadk.cloud.harness_app import utils + + captured = {} + + class FakeSkillToolset: + def __init__(self, names): + self.names = names + + def fake_build_skill_toolset( + skill_ids, download_dir=None, skill_space_policy=None + ): + captured["skill_ids"] = list(skill_ids) + captured["policy"] = ( + skill_space_policy.to_json() if skill_space_policy else None + ) + return FakeSkillToolset(skill_ids) if skill_ids else None + + monkeypatch.delenv("SKILL_SPACE_POLICY", raising=False) + monkeypatch.setattr(utils, "SkillToolset", FakeSkillToolset) + monkeypatch.setattr(utils, "build_skill_toolset", fake_build_skill_toolset) + base = Agent(model_name="base-model", model_api_key="test-key") + + spawn_harness_agent( + base, + HarnessOverrides( + selected_skills=[ + { + "source": "skillspace", + "skill_space_id": "ss-a", + "skill_space_policy": { + "mode": "allow", + "ids": ["s-a2", "s-a1", "s-a2"], + }, + }, + { + "source": "skillspace", + "skill_space_id": "ss-b", + "skill_space_policy": { + "mode": "deny", + "ids": ["s-b9"], + }, + }, + ] + ), + ) + + assert captured["skill_ids"] == ["space:ss-a", "space:ss-b"] + assert captured["policy"] == ( + '{"spaces":{"ss-a":{"mode":"allow","ids":["s-a1","s-a2"]},' + '"ss-b":{"mode":"deny","ids":["s-b9"]}}}' + ) + assert "SKILL_SPACE_POLICY" not in os.environ + + def test_spawn_rejects_conflicting_selected_skill_space_policy(self, monkeypatch): + from veadk.cloud.harness_app import utils + + monkeypatch.setattr( + utils, "build_skill_toolset", lambda *_args, **_kwargs: None + ) + base = Agent(model_name="base-model", model_api_key="test-key") + + with pytest.raises(ValueError, match="Conflicting skill_space_policy"): + spawn_harness_agent( + base, + HarnessOverrides( + selected_skills=[ + { + "source": "skillspace", + "skill_space_id": "ss-a", + "skill_id": "s-a1", + "skill_space_policy": { + "mode": "allow", + "ids": ["s-a1"], + }, + }, + { + "source": "skillspace", + "skill_space_id": "ss-a", + "skill_id": "s-a2", + "skill_space_policy": { + "mode": "deny", + "ids": ["s-a9"], + }, + }, + ] + ), + ) + def test_spawn_clears_builtin_tools_and_skills_with_empty_overrides( self, monkeypatch ): diff --git a/tests/cloud/test_harness_skill_download.py b/tests/cloud/test_harness_skill_download.py index c38f8fd2e..255489d37 100644 --- a/tests/cloud/test_harness_skill_download.py +++ b/tests/cloud/test_harness_skill_download.py @@ -31,6 +31,7 @@ from google.adk.tools.skill_toolset import SkillToolset from veadk.cloud.harness_app import utils +from veadk.skills.policy import parse_skill_space_policy from veadk.skills.skill import Skill as VeADKSkill @@ -152,6 +153,159 @@ def fail_skillhub_download(*args, **kwargs): assert isinstance(toolset._code_executor, UnsafeLocalCodeExecutor) +@pytest.mark.parametrize( + ("policy", "expected_names"), + [ + ('{"mode":"allow","ids":["s-allow"]}', ["allowed-skill"]), + ('{"mode":"deny","ids":["s-deny"]}', ["allowed-skill"]), + ], +) +def test_build_skill_toolset_applies_skill_space_policy( + monkeypatch, + tmp_path, + policy, + expected_names, +): + remote_skills = [ + VeADKSkill( + name="allowed-skill", + description="Allowed skill.", + path="skills/s-allow/v1/allowed-skill.zip", + skill_space_id="ss-test", + bucket_name="bucket", + id="s-allow", + ), + VeADKSkill( + name="denied-skill", + description="Denied skill.", + path="skills/s-deny/v1/denied-skill.zip", + skill_space_id="ss-test", + bucket_name="bucket", + id="s-deny", + ), + ] + materialized: list[str] = [] + + monkeypatch.setenv("SKILL_SPACE_POLICY", policy) + monkeypatch.setattr( + utils, + "_load_skills_from_space_id", + lambda _skill_space_id: remote_skills, + ) + + def fake_materialize_remote_skill( + skill: VeADKSkill, + *, + cache_dir=None, + ): + materialized.append(skill.name) + skill_dir = tmp_path / skill.name + _write_adk_skill(skill_dir, name=skill.name) + return skill_dir + + monkeypatch.setattr( + utils, + "materialize_remote_skill", + fake_materialize_remote_skill, + ) + + toolset = utils.build_skill_toolset(["space:ss-test"], download_dir=tmp_path) + + assert materialized == expected_names + assert [skill.name for skill in toolset._list_skills()] == expected_names + + +def test_build_skill_toolset_allows_empty_skill_space_policy( + monkeypatch, + tmp_path, +): + remote_skills = [ + VeADKSkill( + name="remote-skill", + description="Remote skill.", + path="skills/s-1/v1/remote-skill.zip", + skill_space_id="ss-test", + bucket_name="bucket", + id="s-1", + ) + ] + + monkeypatch.setenv("SKILL_SPACE_POLICY", '{"mode":"allow","ids":[]}') + monkeypatch.setattr( + utils, + "_load_skills_from_space_id", + lambda _skill_space_id: remote_skills, + ) + monkeypatch.setattr( + utils, + "materialize_remote_skill", + lambda *_args, **_kwargs: pytest.fail( + "filtered skills must not be materialized" + ), + ) + + assert utils.build_skill_toolset(["space:ss-test"], download_dir=tmp_path) is None + + +def test_build_skill_toolset_prefers_explicit_policy_over_env( + monkeypatch, + tmp_path, +): + remote_skills = [ + VeADKSkill( + name="explicit-skill", + description="Explicit skill.", + path="skills/s-explicit/v1/explicit-skill.zip", + skill_space_id="ss-test", + bucket_name="bucket", + id="s-explicit", + ), + VeADKSkill( + name="env-skill", + description="Env skill.", + path="skills/s-env/v1/env-skill.zip", + skill_space_id="ss-test", + bucket_name="bucket", + id="s-env", + ), + ] + materialized: list[str] = [] + + monkeypatch.setenv("SKILL_SPACE_POLICY", '{"mode":"allow","ids":["s-env"]}') + monkeypatch.setattr( + utils, + "_load_skills_from_space_id", + lambda _skill_space_id: remote_skills, + ) + + def fake_materialize_remote_skill( + skill: VeADKSkill, + *, + cache_dir=None, + ): + materialized.append(skill.name) + skill_dir = tmp_path / skill.name + _write_adk_skill(skill_dir, name=skill.name) + return skill_dir + + monkeypatch.setattr( + utils, + "materialize_remote_skill", + fake_materialize_remote_skill, + ) + + toolset = utils.build_skill_toolset( + ["space:ss-test"], + download_dir=tmp_path, + skill_space_policy=parse_skill_space_policy( + '{"mode":"allow","ids":["s-explicit"]}' + ), + ) + + assert materialized == ["explicit-skill"] + assert [skill.name for skill in toolset._list_skills()] == ["explicit-skill"] + + def test_replace_skills_replaces_existing_skill_toolset(monkeypatch, tmp_path): existing_dir = tmp_path / "existing" incoming_dir = tmp_path / "incoming" @@ -170,7 +324,7 @@ def test_replace_skills_replaces_existing_skill_toolset(monkeypatch, tmp_path): agent = SimpleNamespace(tools=[marker_tool, existing_toolset]) calls: list[tuple[list[str], object]] = [] - def fake_build_skill_toolset(skill_ids, download_dir=None): + def fake_build_skill_toolset(skill_ids, download_dir=None, skill_space_policy=None): calls.append((skill_ids, download_dir)) return incoming_toolset diff --git a/tests/frontend/server/skills/test_reviews.py b/tests/frontend/server/skills/test_reviews.py index 0b6734e53..00a60a05a 100644 --- a/tests/frontend/server/skills/test_reviews.py +++ b/tests/frontend/server/skills/test_reviews.py @@ -42,11 +42,16 @@ def package(name: str, text: str) -> bytes: data = io.BytesIO() with zipfile.ZipFile(data, "w") as archive: - archive.writestr( - f"{name}/SKILL.md", - f"---\nname: {name}\ndescription: Review test\n---\n{text}", - ) - archive.writestr(f"{name}/references/data.txt", text) + for path, content in [ + ( + f"{name}/SKILL.md", + f"---\nname: {name}\ndescription: Review test\n---\n{text}", + ), + (f"{name}/references/data.txt", text), + ]: + info = zipfile.ZipInfo(path) + info.date_time = (2025, 1, 1, 0, 0, 0) + archive.writestr(info, content) return data.getvalue() diff --git a/tests/skills/test_skill_space_policy.py b/tests/skills/test_skill_space_policy.py index 0f984cec7..c3261a821 100644 --- a/tests/skills/test_skill_space_policy.py +++ b/tests/skills/test_skill_space_policy.py @@ -68,6 +68,19 @@ def test_policy_json_is_compact_and_deduplicated(): assert policy.to_json() == '{"mode":"deny","ids":["skill-1","skill-2"]}' +def test_space_policy_json_is_compact_and_deduplicated(): + policy = parse_skill_space_policy( + '{"spaces":{"ss-b":{"mode":"deny","ids":["skill-9"]},' + '"ss-a":{"mode":"allow","ids":["skill-2","skill-1","skill-2"]}}}' + ) + + assert ( + policy.to_json() + == '{"spaces":{"ss-a":{"mode":"allow","ids":["skill-1","skill-2"]},' + '"ss-b":{"mode":"deny","ids":["skill-9"]}}}' + ) + + @pytest.mark.parametrize( ("mode", "ids", "expected"), [ @@ -101,6 +114,81 @@ def test_load_skills_from_cloud_applies_policy( assert [skill.id for skill in skills] == expected +def test_load_skills_from_cloud_applies_space_policy( + monkeypatch: pytest.MonkeyPatch, +): + monkeypatch.setenv( + "SKILL_SPACE_POLICY", + json.dumps( + { + "spaces": { + "ss-one": {"mode": "allow", "ids": ["skill-1"]}, + "ss-two": {"mode": "deny", "ids": ["skill-4"]}, + } + } + ), + ) + + def fake_load(space_id: str, *, raise_on_error=False): + return [ + Skill( + id=f"{space_id}-missing", + name=f"{space_id}-missing", + description="missing", + path="unused", + skill_space_id=space_id, + ) + ] + + monkeypatch.setattr( + utils, + "_load_skills_from_space_id", + lambda space_id, *, raise_on_error=False: { + "ss-one": [ + Skill( + id="skill-1", + name="one", + description="one", + path="unused", + skill_space_id="ss-one", + ), + Skill( + id="skill-2", + name="two", + description="two", + path="unused", + skill_space_id="ss-one", + ), + ], + "ss-two": [ + Skill( + id="skill-3", + name="three", + description="three", + path="unused", + skill_space_id="ss-two", + ), + Skill( + id="skill-4", + name="four", + description="four", + path="unused", + skill_space_id="ss-two", + ), + ], + "ss-three": fake_load("ss-three"), + }[space_id], + ) + + skills = utils.load_skills_from_cloud("ss-one,ss-two,ss-three") + + assert [skill.id for skill in skills] == [ + "skill-1", + "skill-3", + "ss-three-missing", + ] + + def test_missing_policy_keeps_all_remote_skills(monkeypatch: pytest.MonkeyPatch): monkeypatch.delenv("SKILL_SPACE_POLICY", raising=False) monkeypatch.setattr( diff --git a/tests/tools/builtin_tools/test_run_sandbox_agent.py b/tests/tools/builtin_tools/test_run_sandbox_agent.py index 87a026cf5..9075de56c 100644 --- a/tests/tools/builtin_tools/test_run_sandbox_agent.py +++ b/tests/tools/builtin_tools/test_run_sandbox_agent.py @@ -40,7 +40,9 @@ def _load_run_sandbox_agent_module(): fake_google_adk_tools.ToolContext = object fake_veadk = types.ModuleType("veadk") - fake_veadk.__path__ = [] # type: ignore[attr-defined] + fake_veadk.__path__ = [ # type: ignore[attr-defined] + str(Path(__file__).resolve().parents[3] / "veadk") + ] fake_tools = types.ModuleType("veadk.tools") fake_tools.__path__ = [] # type: ignore[attr-defined] fake_builtin_tools = types.ModuleType("veadk.tools.builtin_tools") diff --git a/veadk/cloud/harness_app/types.py b/veadk/cloud/harness_app/types.py index 3cc772320..6cb0d7772 100644 --- a/veadk/cloud/harness_app/types.py +++ b/veadk/cloud/harness_app/types.py @@ -27,7 +27,7 @@ from typing import Any, Literal -from pydantic import AliasChoices, BaseModel, ConfigDict, Field +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 @@ -61,6 +61,18 @@ class HarnessMcpServer(BaseModel): ) +class HarnessSkillSpacePolicy(BaseModel): + """Session-scoped allow/deny filter for one selected Skill Space.""" + + mode: Literal["allow", "deny"] = Field( + description="Skill Space filtering mode.", + ) + ids: list[str] = Field( + default_factory=list, + description="Stable Skill ids selected by the policy.", + ) + + class HarnessSelectedSkill(BaseModel): """Skill selected by AgentKit runtime config.""" @@ -86,6 +98,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.") + 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": + if self.skill_space_policy is not None and self.source != "skillspace": + raise ValueError("skill_space_policy is only supported for skillspace") + if ( + self.skill_space_policy is not None + and not (self.skill_space_id or "").strip() + ): + raise ValueError("skill_space_policy requires skill_space_id") + return self class HarnessResourceOverride(BaseModel): diff --git a/veadk/cloud/harness_app/utils.py b/veadk/cloud/harness_app/utils.py index d960f4414..02f4fcad4 100644 --- a/veadk/cloud/harness_app/utils.py +++ b/veadk/cloud/harness_app/utils.py @@ -61,8 +61,17 @@ from veadk.knowledgebase import KnowledgeBase from veadk.memory.long_term_memory import LongTermMemory from veadk.memory.short_term_memory import ShortTermMemory +from veadk.skills.policy import ( + SKILL_SPACE_POLICY_ENV, + SkillSpacePolicySet, + parse_skill_space_policy, +) from veadk.skills.materializer import materialize_remote_skill -from veadk.skills.utils import _load_skills_from_space_id +from veadk.skills.utils import ( + _filter_skills_by_policy, + _filter_skills_by_policy_env, + _load_skills_from_space_id, +) from veadk.tools import get_builtin_tool, list_builtin_tools from veadk.tools.builtin_tools.load_knowledgebase import LoadKnowledgebaseTool from veadk.utils.logger import get_logger @@ -336,12 +345,53 @@ def _dedupe_selected_skills( ) -> list[HarnessSelectedSkill]: ordered: dict[str, HarnessSelectedSkill] = {} for entry in entries: - ref = _selected_skill_ref(entry) - if ref: - ordered[ref] = entry + key = json.dumps( + entry.model_dump(mode="json", exclude_none=True), + ensure_ascii=False, + sort_keys=True, + ) + if key: + ordered[key] = entry return list(ordered.values()) +def _selected_skill_space_policy_json( + entries: list[HarnessSelectedSkill], +) -> str | None: + spaces: dict[str, dict[str, Any]] = {} + for entry in entries: + policy = entry.skill_space_policy + if policy is None: + continue + if entry.source != "skillspace": + raise ValueError("skill_space_policy is only supported for skillspace") + skill_space_id = (entry.skill_space_id or "").strip() + if not skill_space_id: + raise ValueError("skill_space_policy requires skill_space_id") + payload = policy.model_dump(mode="json") + existing = spaces.get(skill_space_id) + if existing is not None and existing != payload: + raise ValueError( + f"Conflicting skill_space_policy for skill_space_id={skill_space_id!r}" + ) + spaces[skill_space_id] = payload + + if not spaces: + return None + return parse_skill_space_policy(json.dumps({"spaces": spaces})).to_json() + + +def _selected_skill_space_policy( + entries: list[HarnessSelectedSkill], +) -> SkillSpacePolicySet | None: + if os.getenv(SKILL_SPACE_POLICY_ENV) is not None: + return None + raw_policy = _selected_skill_space_policy_json(entries) + if raw_policy is None: + return None + return parse_skill_space_policy(raw_policy) + + def _builtin_tool_entries( config: HarnessOverrides, *, @@ -642,7 +692,9 @@ class SkillLoadError(RuntimeError): def build_skill_toolset( - skills: list[str], download_dir: Path | None = None + skills: list[str], + download_dir: Path | None = None, + skill_space_policy: SkillSpacePolicySet | None = None, ) -> SkillToolset | None: """Download each skill source and load them as a single ADK toolset. @@ -682,6 +734,11 @@ def build_skill_toolset( raise RuntimeError( f"No skills found in skills-center space '{skill_space_id}'" ) + remote_skills = ( + _filter_skills_by_policy(remote_skills, skill_space_policy) + if skill_space_policy is not None + else _filter_skills_by_policy_env(remote_skills) + ) for remote_skill in remote_skills: skill_dir = materialize_remote_skill( @@ -697,6 +754,8 @@ def build_skill_toolset( ) except Exception as e: raise SkillLoadError(f"Skill '{skill}' failed to load: {e}") from e + if not loaded_skills: + return None return SkillToolset( skills=loaded_skills, code_executor=UnsafeLocalCodeExecutor(), @@ -944,8 +1003,12 @@ def _add_mcp_toolsets(agent: Agent, servers: list[HarnessMcpServer]) -> None: def _selected_skill_refs(skills: list[HarnessSelectedSkill]) -> list[str]: - refs = [_selected_skill_ref(skill) for skill in skills] - return [ref for ref in refs if ref] + refs: dict[str, None] = {} + for skill in skills: + ref = _selected_skill_ref(skill) + if ref: + refs.setdefault(ref, None) + return list(refs) def _assemble_agent(config: HarnessConfig) -> tuple[Agent, ShortTermMemory]: @@ -958,10 +1021,14 @@ def _assemble_agent(config: HarnessConfig) -> tuple[Agent, ShortTermMemory]: """ tools = _build_builtin_tools(_builtin_tool_entries(config, only_set=False)) - skills = _selected_skill_refs(_selected_skill_entries(config, only_set=False)) + selected_skills = _selected_skill_entries(config, only_set=False) + skills = _selected_skill_refs(selected_skills) if skills: logger.info(f"Loading skills {skills} for harness.") - skill_toolset = build_skill_toolset(skills) + skill_toolset = build_skill_toolset( + skills, + skill_space_policy=_selected_skill_space_policy(selected_skills), + ) if skill_toolset is not None: tools.append(skill_toolset) tools.extend(_build_mcp_toolsets(config.mcp)) @@ -1095,12 +1162,19 @@ def _is_harness_builtin_tool(tool: Any) -> bool: def _replace_skills( - agent: Agent, skill_ids: list[str], download_dir: Path | None = None + agent: Agent, + skill_ids: list[str], + download_dir: Path | None = None, + skill_space_policy: SkillSpacePolicySet | None = None, ) -> None: """Replace the harness-selected skill toolset with ``skill_ids``.""" agent.tools = [tool for tool in agent.tools if not isinstance(tool, SkillToolset)] - toolset = build_skill_toolset(skill_ids, download_dir=download_dir) + toolset = build_skill_toolset( + skill_ids, + download_dir=download_dir, + skill_space_policy=skill_space_policy, + ) if toolset is not None: agent.tools.append(toolset) @@ -1499,10 +1573,12 @@ def spawn_harness_agent( ) if "selected_skills" in set_fields: + selected_skills = _selected_skill_entries(overrides, only_set=True) _replace_skills( cloned, - _selected_skill_refs(_selected_skill_entries(overrides, only_set=True)), + _selected_skill_refs(selected_skills), download_dir, + skill_space_policy=_selected_skill_space_policy(selected_skills), ) if "mcp" in set_fields: diff --git a/veadk/skills/policy.py b/veadk/skills/policy.py index 301810029..7d53e7bf1 100644 --- a/veadk/skills/policy.py +++ b/veadk/skills/policy.py @@ -48,8 +48,69 @@ def to_json(self) -> str: ) -def parse_skill_space_policy(raw_value: str) -> SkillSpacePolicy: - """Parse the only supported inline policy shape: ``mode`` plus ``ids``.""" +@dataclass(frozen=True) +class SkillSpacePolicySet: + global_policy: SkillSpacePolicy | None = None + space_policies: dict[str, SkillSpacePolicy] | None = None + + def allows(self, skill_id: str | None, skill_space_id: str | None = None) -> bool: + if self.global_policy is not None: + return self.global_policy.allows(skill_id) + if not self.space_policies or not skill_space_id: + return True + policy = self.space_policies.get(skill_space_id) + if policy is None: + return True + return policy.allows(skill_id) + + def to_json(self) -> str: + if self.global_policy is not None: + return self.global_policy.to_json() + spaces = self.space_policies or {} + return json.dumps( + { + "spaces": { + space_id: { + "mode": policy.mode, + "ids": sorted(policy.ids), + } + for space_id, policy in sorted(spaces.items()) + } + }, + ensure_ascii=False, + separators=(",", ":"), + ) + + +def _parse_inline_policy(payload: object, path: str) -> SkillSpacePolicy: + if not isinstance(payload, dict): + raise SkillSpacePolicyError(f"{path} must be a JSON object") + if set(payload) != {"mode", "ids"}: + raise SkillSpacePolicyError(f"{path} supports exactly 'mode' and 'ids'") + + mode = payload["mode"] + if mode not in {"allow", "deny"}: + raise SkillSpacePolicyError(f"{path}.mode must be 'allow' or 'deny'") + + raw_ids = payload["ids"] + if not isinstance(raw_ids, list): + raise SkillSpacePolicyError(f"{path}.ids must be a list") + + ids: set[str] = set() + for skill_id in raw_ids: + if not isinstance(skill_id, str) or not skill_id.strip(): + raise SkillSpacePolicyError(f"{path}.ids must contain non-empty strings") + if skill_id != skill_id.strip(): + raise SkillSpacePolicyError( + f"{path}.ids must not contain surrounding whitespace" + ) + ids.add(skill_id) + + return SkillSpacePolicy(mode=mode, ids=frozenset(ids)) + + +def parse_skill_space_policy(raw_value: str) -> SkillSpacePolicySet: + """Parse global or per-space remote Skill Space filtering policy.""" if len(raw_value.encode("utf-8")) > MAX_SKILL_SPACE_POLICY_BYTES: raise SkillSpacePolicyError( f"{SKILL_SPACE_POLICY_ENV} must not exceed " @@ -65,31 +126,36 @@ def parse_skill_space_policy(raw_value: str) -> SkillSpacePolicy: if not isinstance(payload, dict): raise SkillSpacePolicyError(f"{SKILL_SPACE_POLICY_ENV} must be a JSON object") - if set(payload) != {"mode", "ids"}: - raise SkillSpacePolicyError( - f"{SKILL_SPACE_POLICY_ENV} supports exactly 'mode' and 'ids'" - ) - mode = payload["mode"] - if mode not in {"allow", "deny"}: - raise SkillSpacePolicyError( - f"{SKILL_SPACE_POLICY_ENV}.mode must be 'allow' or 'deny'" + if set(payload) == {"mode", "ids"}: + return SkillSpacePolicySet( + global_policy=_parse_inline_policy(payload, SKILL_SPACE_POLICY_ENV) ) - raw_ids = payload["ids"] - if not isinstance(raw_ids, list): - raise SkillSpacePolicyError(f"{SKILL_SPACE_POLICY_ENV}.ids must be a list") - - ids: set[str] = set() - for skill_id in raw_ids: - if not isinstance(skill_id, str) or not skill_id.strip(): + if set(payload) == {"spaces"}: + raw_spaces = payload["spaces"] + if not isinstance(raw_spaces, dict): raise SkillSpacePolicyError( - f"{SKILL_SPACE_POLICY_ENV}.ids must contain non-empty strings" + f"{SKILL_SPACE_POLICY_ENV}.spaces must be a JSON object" ) - if skill_id != skill_id.strip(): - raise SkillSpacePolicyError( - f"{SKILL_SPACE_POLICY_ENV}.ids must not contain surrounding whitespace" + spaces: dict[str, SkillSpacePolicy] = {} + for space_id, raw_policy in raw_spaces.items(): + if not isinstance(space_id, str) or not space_id.strip(): + raise SkillSpacePolicyError( + f"{SKILL_SPACE_POLICY_ENV}.spaces keys must be non-empty strings" + ) + if space_id != space_id.strip(): + raise SkillSpacePolicyError( + f"{SKILL_SPACE_POLICY_ENV}.spaces keys must not contain " + "surrounding whitespace" + ) + spaces[space_id] = _parse_inline_policy( + raw_policy, + f"{SKILL_SPACE_POLICY_ENV}.spaces.{space_id}", ) - ids.add(skill_id) + return SkillSpacePolicySet(space_policies=spaces) - return SkillSpacePolicy(mode=mode, ids=frozenset(ids)) + raise SkillSpacePolicyError( + f"{SKILL_SPACE_POLICY_ENV} supports exactly 'mode' and 'ids', " + "or exactly 'spaces'" + ) diff --git a/veadk/skills/utils.py b/veadk/skills/utils.py index c9ef56aae..f599937e4 100644 --- a/veadk/skills/utils.py +++ b/veadk/skills/utils.py @@ -25,6 +25,7 @@ from veadk.skills.policy import ( SKILL_SPACE_POLICY_ENV, SkillSpacePolicyError, + SkillSpacePolicySet, parse_skill_space_policy, ) from veadk.utils.logger import get_logger @@ -145,17 +146,11 @@ def load_skills_from_cloud( skill_space_ids_list = [x.strip() for x in skill_space_ids.split(",") if x.strip()] logger.info(f"Load skills from cloud skill sources: {skill_space_ids_list}") - raw_policy = os.getenv(SKILL_SPACE_POLICY_ENV) - policy = None - if raw_policy is not None: - try: - policy = parse_skill_space_policy(raw_policy) - except SkillSpacePolicyError as exc: - logger.error( - f"Invalid {SKILL_SPACE_POLICY_ENV}; remote Skill Space skills " - f"are disabled for this session: {exc}" - ) - return [] + try: + policy = _parse_skill_space_policy_env() + except SkillSpacePolicyError as exc: + _log_invalid_skill_space_policy(exc) + return [] skills = [] @@ -177,17 +172,50 @@ def load_skills_from_cloud( ) ) + return _filter_skills_by_policy(skills, policy) + + +def _filter_skills_by_policy_env(skills: list[Skill]) -> list[Skill]: + try: + policy = _parse_skill_space_policy_env() + except SkillSpacePolicyError as exc: + _log_invalid_skill_space_policy(exc) + return [] + + return _filter_skills_by_policy(skills, policy) + + +def _parse_skill_space_policy_env() -> SkillSpacePolicySet | None: + raw_policy = os.getenv(SKILL_SPACE_POLICY_ENV) + if raw_policy is None: + return None + return parse_skill_space_policy(raw_policy) + + +def _filter_skills_by_policy( + skills: list[Skill], + policy: SkillSpacePolicySet | None, +) -> list[Skill]: if policy is None: return skills - filtered_skills = [skill for skill in skills if policy.allows(skill.id)] + filtered_skills = [ + skill for skill in skills if policy.allows(skill.id, skill.skill_space_id) + ] logger.info( - f"Applied {SKILL_SPACE_POLICY_ENV} mode={policy.mode}: " + f"Applied {SKILL_SPACE_POLICY_ENV}: " f"kept {len(filtered_skills)} of {len(skills)} remote skills" ) return filtered_skills +def _log_invalid_skill_space_policy(exc: SkillSpacePolicyError) -> None: + logger.error( + f"Invalid {SKILL_SPACE_POLICY_ENV}; remote Skill Space skills " + f"are disabled for this session: {exc}" + ) + + def _get_cloud_credentials() -> tuple[str, str, str]: from veadk.auth.veauth.utils import get_credential_from_vefaas_iam