mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(skills): enforce per-skill runtime tool isolation (#140)
* Add unit tests for Mnemonic page - Introduced comprehensive tests for the Mnemonic page, covering initial render, copy to clipboard functionality, confirmation checkbox behavior, and mode switching between generate and import. - Validated user interactions, including input handling and button states, ensuring robust functionality and user experience. - Enhanced test coverage for various scenarios, including validation of mnemonic phrases and loading states during operations. * test: add cross-stack test coverage for core and tauri flows Add focused Rust and frontend tests for core process startup behavior, CLI argument parsing, JSON-RPC error handling, and Tauri command/RPC mapping paths to improve confidence for issue #57. Closes #57 Made-with: Cursor * refactor(tests): streamline test code and improve readability Consolidate mock imports and simplify function calls in coreRpcClient tests. Adjust formatting in Rust core_process and CLI tests for better clarity. Update mnemonic test assertions for improved accuracy. Made-with: Cursor * fix(e2e): harden deep-link login flow reliability Stabilize auth deep-link handling and E2E delivery with readiness guards, retries, and new listener unit tests so login/onboarding flows are deterministic for issue #70. Closes #70 Made-with: Cursor * refactor(tests): update agent initialization in tests for consistency Refactor test cases to use a tuple return from `build_agent_with`, improving consistency in agent setup across multiple tests. This change enhances readability and maintains uniformity in the test structure. Made-with: Cursor * fix(skills): enforce per-skill runtime tool isolation Add explicit tool-call origin policy in the QuickJS runtime so skills cannot invoke other skills' tools, while preserving external orchestration through RPC/socket surfaces. Also remove the generic skills_call tool path and document the isolation contract for skill authors. Closes #94 Made-with: Cursor
This commit is contained in:
Mega Mind
@@ -201,6 +201,13 @@ Typical operations:
|
||||
|
||||
Tool calls can be sync or async in JS. Async calls are awaited with runtime polling and timeout handling in the QuickJS event loop layer.
|
||||
|
||||
### Isolation guarantees
|
||||
|
||||
- Each started skill runs in its own QuickJS context (`AsyncContext`) and does not share mutable JS globals with other skills.
|
||||
- Restarting a skill creates a fresh context; prior `globalThis` mutations are not retained.
|
||||
- Host-level policy blocks skill-to-skill tool invocation. A running skill can only invoke its own tool surface.
|
||||
- External orchestrators (UI/RPC/socket MCP) can still target tools on any running skill by explicit `skill_id`.
|
||||
|
||||
---
|
||||
|
||||
## 8) JSON-RPC Surface (`openhuman.skills_*`)
|
||||
@@ -283,6 +290,8 @@ High-level pattern:
|
||||
3. Incoming tool calls route to `skill_id` + `tool_name`.
|
||||
4. Core executes via runtime and returns `ToolResult`.
|
||||
|
||||
Tool naming over MCP remains `skillId__toolName` for external orchestration.
|
||||
|
||||
---
|
||||
|
||||
## 12) Environment Variables and Configuration
|
||||
@@ -345,6 +354,7 @@ High-level pattern:
|
||||
5. Tool-call failures:
|
||||
- verify skill status is `Running`
|
||||
- inspect skill error in snapshot
|
||||
- cross-skill denied errors mean a skill attempted to invoke another skill's tool; this is blocked by design
|
||||
|
||||
### Useful runtime truths
|
||||
|
||||
@@ -399,3 +409,22 @@ When changing behavior, test both:
|
||||
- Avoid introducing new legacy skill metadata paths.
|
||||
- Add traceable logs around install/start/setup/tool call boundaries.
|
||||
|
||||
---
|
||||
|
||||
## 18) Skill Author Isolation Contract
|
||||
|
||||
### Guaranteed
|
||||
|
||||
- Your skill runs in its own QuickJS context and event loop when started.
|
||||
- Your skill's `globalThis` state is isolated from other running skills.
|
||||
- Your skill can publish state only for itself; state is namespaced by `skill_id`.
|
||||
- Cross-skill tool invocation from within a skill is denied by host policy.
|
||||
- External host orchestration (UI/RPC/socket MCP) may call your tools by explicit `skill_id`.
|
||||
|
||||
### Not supported / undefined behavior
|
||||
|
||||
- Do not rely on `globalThis.skills.callTool` for inter-skill calls.
|
||||
- Do not rely on in-memory JS globals surviving a stop/restart cycle.
|
||||
- Do not assume execution ordering across different skills.
|
||||
- Do not treat runtime-internal bridge objects as stable public APIs.
|
||||
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
# Skills Runtime Isolation (QuickJS)
|
||||
|
||||
This document defines OpenHuman's skill isolation contract for the QuickJS runtime.
|
||||
|
||||
## Goals
|
||||
|
||||
- Keep each running skill isolated in JavaScript execution state.
|
||||
- Prevent lateral movement where one skill invokes another skill's tools.
|
||||
- Preserve external orchestration from trusted host surfaces (RPC/UI/socket MCP).
|
||||
|
||||
## Runtime Architecture
|
||||
|
||||
### Per-skill components
|
||||
|
||||
- `QjsSkillInstance` (`src/openhuman/skills/qjs_skill_instance`) per started skill.
|
||||
- One dedicated `AsyncRuntime` + `AsyncContext` created during skill spawn.
|
||||
- Skill-local JS globals (`globalThis`), module state, and in-memory variables.
|
||||
- Skill-local data path (`skills_data/<skill_id>/...`) and namespaced memory writes.
|
||||
- Skill-local message loop handling `SkillMessage` commands.
|
||||
|
||||
### Shared infrastructure
|
||||
|
||||
- `RuntimeEngine` as lifecycle orchestrator.
|
||||
- `SkillRegistry` for tracking/routing running skills.
|
||||
- `SocketManager` for external MCP transport.
|
||||
- Schedulers (`CronScheduler`, `PingScheduler`) and preferences store.
|
||||
|
||||
## Lifecycle and Reset Semantics
|
||||
|
||||
1. `start_skill(skill_id)` creates a fresh skill instance and QuickJS context.
|
||||
2. The instance initializes (`init/start`) and registers tools/state.
|
||||
3. During runtime, only that skill's event loop mutates its JS state.
|
||||
4. `stop_skill(skill_id)` stops the loop and transitions status.
|
||||
5. Restart creates a new instance/context; previous JS globals are not reused.
|
||||
|
||||
Failure modes:
|
||||
|
||||
- Tool dispatch to non-running skill returns a status error.
|
||||
- Reply channel drop returns a deterministic runtime error.
|
||||
- Policy denials return explicit "cross-skill forbidden" errors.
|
||||
|
||||
## Tool Invocation Policy
|
||||
|
||||
Policy is enforced at host boundary (Rust), not in JS:
|
||||
|
||||
- `External` origin (JSON-RPC/UI/socket MCP): may call any target skill tool.
|
||||
- `SkillSelf { skill_id }` origin: may call only tools owned by the same `skill_id`.
|
||||
- Cross-skill attempt from `SkillSelf` is denied before dispatch.
|
||||
|
||||
This policy is implemented in `SkillRegistry::call_tool_scoped`.
|
||||
|
||||
## Forbidden Paths
|
||||
|
||||
- A running skill may not invoke another skill's tools.
|
||||
- Inter-skill bridge helpers must not bypass host policy.
|
||||
- `skills_call` generic cross-skill agent tool is not part of the default tool registry.
|
||||
|
||||
## Testing Requirements
|
||||
|
||||
Runtime policy tests must cover:
|
||||
|
||||
- external origin allowed,
|
||||
- same-skill origin allowed,
|
||||
- cross-skill origin denied with clear error.
|
||||
|
||||
Regression checks should also ensure the default agent tool registry does not include
|
||||
legacy cross-skill helper surfaces.
|
||||
Reference in New Issue
Block a user