diff --git a/backend/app/api/skills.py b/backend/app/api/skills.py index 5fa503b7..dceec5ab 100644 --- a/backend/app/api/skills.py +++ b/backend/app/api/skills.py @@ -1,33 +1,19 @@ -"""Skills API — execute skills via the Developer surface. +"""Skills API — execute governed internal uDos capabilities. POST /api/skills/{skill_id}/run — execute a skill by ID POST /api/skills/run — run a skill by directory path """ from __future__ import annotations -import asyncio import logging -import os -from pathlib import Path from aiohttp import web -from app.core.settings import settings - -log = logging.getLogger("ucore.skills") - -# Default skill paths — try Python registry first, then filesystem from app.services.health import get_health_summary from app.skills.registry import get_skill, run_skill_by_id from app.skills.state import read_state -SKILL_PATHS = [ - Path(__file__).parent.parent / "skills" / "builtin", # backend/app/skills/builtin/ - - settings.udos_root / "uCore/skills", - Path("/usr/local/share/udos/skills"), -] - +log = logging.getLogger("ucore.skills") async def handle_run_skill(request: web.Request) -> web.Response: """POST /api/skills/{skill_id}/run — execute a skill by ID. @@ -107,94 +93,7 @@ async def _run_skill_by_id( pass return web.json_response(result) - # Fallback: find the skill directory on filesystem - skill_dir = _find_skill(skill_id) - if not skill_dir: - return web.json_response({ - "error": f"Skill '{skill_id}' not found", - "searched": [str(p) for p in SKILL_PATHS], - }, status=404) - - # Find executable to run - script = _find_script(skill_dir) - if not script: - return web.json_response({ - "error": f"No runnable script found in {skill_dir}", - "expected": ["run.sh", "run.py", "index.js", "main.py"], - }, status=404) - - # Build command - args = body.get("args", []) - timeout = min(float(body.get("timeout", 60)), 300) # Max 5 min - env = os.environ.copy() - env.update(body.get("env", {})) - - cmd = [str(script)] + args - log.info("Running skill: %s (cmd=%s, timeout=%ss)", skill_id, cmd, timeout) - - try: - proc = await asyncio.create_subprocess_exec( - *cmd, - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.PIPE, - env=env, - cwd=str(skill_dir), - ) - stdout, stderr = await asyncio.wait_for( - proc.communicate(), timeout=timeout, - ) - stdout_str = stdout.decode("utf-8", errors="replace") if stdout else "" - stderr_str = stderr.decode("utf-8", errors="replace") if stderr else "" - exit_code = proc.returncode or 0 - - log.info("Skill result: exit_code=%d, stdout_len=%d", exit_code, len(stdout_str)) - - return web.json_response({ - "stdout": stdout_str, - "stderr": stderr_str, - "exit_code": exit_code, - "skill_id": skill_id, - "script": str(script), - }) - except TimeoutError: - log.warning("Skill timed out: %s", skill_id) - return web.json_response({ - "error": f"Skill timed out after {timeout}s", - "skill_id": skill_id, - }, status=408) - except Exception as e: - log.error("Skill error: %s", e) - return web.json_response({ - "error": str(e), - "skill_id": skill_id, - }, status=500) - - -def _find_skill(skill_id: str) -> Path | None: - """Find a skill directory by ID across all skill paths.""" - for base in SKILL_PATHS: - candidate = base / skill_id - if candidate.exists() and candidate.is_dir(): - return candidate - # Also check if it's a direct path - path = Path(skill_id) - if path.exists() and path.is_dir(): - return path - return None - - -def _find_script(skill_dir: Path) -> Path | None: - """Find the first executable script in a skill directory.""" - preferred_order = ["run.sh", "run.py", "index.js", "main.py", "run"] - for name in preferred_order: - candidate = skill_dir / name - if candidate.exists(): - return candidate - # Fallback: find any executable - for f in skill_dir.iterdir(): - if f.is_file() and os.access(f, os.X_OK): - return f - return None + return web.json_response({"error": f"Skill '{skill_id}' not found"}, status=404) async def handle_list_skills(request: web.Request) -> web.Response: @@ -225,27 +124,7 @@ async def handle_skill_source(request: web.Request) -> web.Response: except (OSError, TypeError): pass - # Try filesystem skill directory - skill_dir = _find_skill(skill_id) - if not skill_dir: - return web.json_response({"error": f"Skill '{skill_id}' not found"}, status=404) - - # Find the primary source file - CANDIDATES = ["run.py", "main.py", "run.sh", "index.js", "index.ts", "skill.py"] - for name in CANDIDATES: - f = skill_dir / name - if f.exists(): - source = f.read_text(errors="replace") - lang = "python" if name.endswith(".py") else "bash" if name.endswith(".sh") else "javascript" - return web.json_response({ - "skill_id": skill_id, - "source": source, - "language": lang, - "filename": name, - "path": str(f), - }) - - return web.json_response({"error": f"No readable source file in {skill_dir}"}, status=404) + return web.json_response({"error": f"Skill '{skill_id}' not found"}, status=404) async def handle_skill_state(request: web.Request) -> web.Response: diff --git a/backend/app/skills/registry.py b/backend/app/skills/registry.py index 69130436..128d28d4 100644 --- a/backend/app/skills/registry.py +++ b/backend/app/skills/registry.py @@ -6,44 +6,40 @@ import sys from pathlib import Path -from app.core.settings import settings from app.skills.base import BaseSkill log = logging.getLogger("ucore.skills.registry") _registry: dict[str, BaseSkill] = {} _loaded = False -SKILL_PATHS = [ - Path(__file__).parent / "builtin", - settings.udos_home / "skills", -] +BUILTIN_SKILL_PATH = Path(__file__).parent / "builtin" def _discover(): skills = {} - for sd in SKILL_PATHS: - if not sd.exists(): + sd = BUILTIN_SKILL_PATH + if not sd.exists(): + return skills + sys.path.insert(0, str(sd.parent)) + for f in sd.iterdir(): + if f.suffix != ".py" or f.name.startswith("_"): continue - sys.path.insert(0, str(sd.parent)) - for f in sd.iterdir(): - if f.suffix != ".py" or f.name.startswith("_"): - continue - try: - spec = importlib.util.spec_from_file_location(f"skills_{f.stem}", f) - if spec and spec.loader: - mod = importlib.util.module_from_spec(spec) - sys.modules[spec.name] = mod - spec.loader.exec_module(mod) - for _, obj in inspect.getmembers(mod): - if ( - inspect.isclass(obj) - and issubclass(obj, BaseSkill) - and obj is not BaseSkill - ): - inst = obj() - skills[inst.meta.id] = inst - except Exception as e: - log.warning(f"Skill load fail {f.name}: {e}") - sys.path.pop(0) + try: + spec = importlib.util.spec_from_file_location(f"skills_{f.stem}", f) + if spec and spec.loader: + mod = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = mod + spec.loader.exec_module(mod) + for _, obj in inspect.getmembers(mod): + if ( + inspect.isclass(obj) + and issubclass(obj, BaseSkill) + and obj is not BaseSkill + ): + inst = obj() + skills[inst.meta.id] = inst + except Exception as e: + log.warning("Skill load fail %s: %s", f.name, e) + sys.path.pop(0) return skills diff --git a/backend/tests/test_skill_registry_authorization.py b/backend/tests/test_skill_registry_authorization.py index a667fe05..310f8579 100644 --- a/backend/tests/test_skill_registry_authorization.py +++ b/backend/tests/test_skill_registry_authorization.py @@ -1,5 +1,5 @@ -from app.skills.base import BaseSkill, SkillMeta from app.skills import registry +from app.skills.base import BaseSkill, SkillMeta class _DestructiveSkill(BaseSkill): diff --git a/backend/tests/test_skill_registry_discovery_policy.py b/backend/tests/test_skill_registry_discovery_policy.py new file mode 100644 index 00000000..e4d96e64 --- /dev/null +++ b/backend/tests/test_skill_registry_discovery_policy.py @@ -0,0 +1,19 @@ +from __future__ import annotations + +from app.skills import registry + + +def test_registry_does_not_scan_runtime_user_python(monkeypatch, tmp_path): + marker = tmp_path / "executed" + user_skills = tmp_path / "skills" + user_skills.mkdir() + (user_skills / "side_effect.py").write_text( + f"from pathlib import Path\nPath({str(marker)!r}).write_text('ran')\n", + encoding="utf-8", + ) + builtin = tmp_path / "builtin" + builtin.mkdir() + monkeypatch.setattr(registry, "BUILTIN_SKILL_PATH", builtin) + + assert registry._discover() == {} + assert not marker.exists() diff --git a/docs/SKILLS_AUDIT_2026-08-18.md b/docs/SKILLS_AUDIT_2026-08-18.md index fdadf0bc..7f0129cd 100644 --- a/docs/SKILLS_AUDIT_2026-08-18.md +++ b/docs/SKILLS_AUDIT_2026-08-18.md @@ -6,6 +6,13 @@ **Dedicated Skill test files before remediation:** 8 +**2026-08-19 base-standard hard cut:** Runtime user-skill discovery and arbitrary +filesystem executable fallback are removed. Internal uDos capabilities load only +from the governed builtin registry. Codex Skills are external development +instructions, not executable uDos capabilities. This is a pre-release reset; +there is no compatibility shim for loose `.py` files, legacy skill directories, +or caller-selected scripts and environment variables. + ## Findings The current term “Skill” covers unrelated concepts: