diff --git a/docs/configuration.md b/docs/configuration.md index ed35c27ba..1afeecb78 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1997,7 +1997,7 @@ For API keys, tokens, and other secrets, see [Environment Variables for Secrets] | `tools.exec.timeout` | `60` | Default hard timeout in seconds for shell commands. Config values may exceed the per-call tool cap; set `0` to disable the hard timeout for trusted long-running commands. | | `tools.exec.pathPrepend` | `""` | Extra directories to prepend to `PATH` when running shell commands. Use this when configured tools should win executable lookup precedence, such as a Python virtual environment's `bin` or `Scripts` directory. | | `tools.exec.pathAppend` | `""` | Extra directories to append to `PATH` when running shell commands (e.g. `/usr/sbin` for `ufw`). | -| `tools.webuiAllowRemotePackageInstall` | `false` | When `false`, the WebUI can install missing optional packages only from a browser opened on the same machine as nanobot. Set to `true` only when a trusted remote admin is allowed to install Python packages into this nanobot environment. | +| `tools.webuiAllowRemotePackageInstall` | `false` | When `false`, the WebUI can install optional support or extension packages only from a browser opened on the same machine as nanobot. Set to `true` only when a trusted remote admin may place packages into this installation. Remotely installed extensions remain untrusted and inactive; trust, permission, activation, disabling, and removal stay local-only. | | `tools.ssrfWhitelist` | `[]` | CIDR ranges exempted from the shared SSRF guard used by web fetches and HTTP/SSE MCP connections. Prefer exact host CIDRs such as `192.168.1.50/32`; broad ranges increase SSRF exposure. | | `channels.*.allowFrom` | omitted | Access control per channel. Omit to use pairing-only mode; set `["*"]` to allow everyone; or list specific user IDs. See [Pairing](#pairing) for details. | diff --git a/docs/extension-authoring.md b/docs/extension-authoring.md index e0e00e0e2..e16584606 100644 --- a/docs/extension-authoring.md +++ b/docs/extension-authoring.md @@ -105,9 +105,10 @@ command webui ``` -Each contribution has a stable `name`, optional runtime `target`, optional -`description`, and optional `replaces` list. `replaces` contains extension IDs, -not contribution names. Replacement is still checked against scope precedence. +Each contribution has a stable `name`, optional runtime `target`, and optional +`description`. Two active extensions cannot own the same contribution name. +Disable one owner before activating the other; v1 deliberately does not allow +extensions to replace core or third-party registrations. The manifest declares ownership. It does not create an implementation by itself. A runtime must register the corresponding native capability. @@ -120,7 +121,7 @@ itself. A runtime must register the corresponding native capability. | `npm` | Package under the extension's `node_modules` | npm installation constraint | | `executable` | Command on `PATH` | No version probe | | `environment` | Environment variable | Must be non-empty | -| `extension` | Another extension ID | Installed extension version | +| `extension` | Another extension ID | Active extension version | Set `optional: true` when the extension can activate without the dependency. Do not put API keys in the manifest. @@ -132,8 +133,9 @@ Permission names are lowercase namespaced identifiers such as reason the user can evaluate. Activation requires every requested permission to be granted. -Permissions describe host policy; they are not an OS sandbox. Keep the request -set minimal and use native host operations when one exists. +Permissions are review and activation gates; they do not constrain direct +Python or Node process access and are not an OS sandbox. Keep the request set +minimal and use native host operations when one exists. ## Native Python Runtime @@ -201,6 +203,10 @@ Compatibility packages use their upstream metadata and keyword: - OpenClaw: `openclaw-plugin` plus `openclaw.extensions` or `openclaw.runtimeExtensions` +The adapter adds the `runtime.node` permission to both generated manifests. +Users must explicitly grant it before any third-party JavaScript or TypeScript +entry runs. + Native Python packages can be installed from a local directory or Git source. The market is an index; installation always passes through local validation, dependency checks, permission review, trust, and activation. @@ -240,6 +246,9 @@ upstream API. The sidecar supports `registerTool`, `registerCommand`, and selected `on(...)` lifecycle handlers. TypeScript uses Node's native type stripping when available, with `jiti` as a fallback installed with the package runtime. +Required `peerDependencies` are installed into that runtime as well; peers +marked optional in `peerDependenciesMeta` remain optional. +The generated manifest requests `runtime.node`. ### OpenClaw package shape @@ -257,6 +266,7 @@ available, with `jiti` as a fallback installed with the package runtime. If present, `openclaw.plugin.json` supplies catalog identity, contribution contracts, command aliases, and compatibility diagnostics. The OpenClaw `register` function must complete synchronously during load. +The generated manifest requests `runtime.node`. ## Test an Extension @@ -274,7 +284,9 @@ nanobot agent -m "Use review_code on README.md" Also test: - install while untrusted does not execute code; +- package changes after installation revoke effective trust; - missing hard dependencies leave the package inactive; +- extension dependencies activate before their dependents and reject cycles; - denied or missing permissions prevent activation; - duplicate contribution names become diagnostics; - disable, untrust, reload, and uninstall remove runtime registrations; diff --git a/docs/extension-system.md b/docs/extension-system.md index 682fd8635..7e0a6a898 100644 --- a/docs/extension-system.md +++ b/docs/extension-system.md @@ -31,8 +31,7 @@ package / workspace directory / compatibility package +----------------+----------------+ | v - tools / skills / channels / providers / MCP / - hooks / commands / WebUI + executable native adapters + inspectable metadata ``` `ExtensionManifest` is dependency-free metadata. Discovery can inspect it @@ -59,10 +58,12 @@ scopes: 2. `user` 3. `workspace` -The nearest scope wins for the same extension ID. Different extensions may not -silently take over the same contribution name. Replacing another extension's -contribution must be explicit and may only come from an equal or higher scope. +The nearest policy-eligible scope wins for the same extension ID. A disabled, +untrusted, denied, or invalid higher-scope copy does not shadow an eligible +lower copy. Different extensions may not take over the same contribution name. Conflicts become diagnostics instead of crashing unrelated extensions. +Extension API v1 deliberately has no override mechanism because runtime +replacement must be both transactional and reversible. ## Compatibility runtimes @@ -84,6 +85,9 @@ Compatibility is capability-based rather than all-or-nothing: The compatibility sidecar is a failure-isolation boundary, not a security sandbox. The exact executable and metadata-only surfaces are listed in the [compatibility matrix](./extension-authoring.md#compatibility-matrix). +Generated Pi and OpenClaw manifests always request `runtime.node`, so process +execution is visible and requires explicit consent even though that permission +is not an OS-level confinement mechanism. ## Security model @@ -93,9 +97,12 @@ permissions, dependency state, and trust scope before executing code. Project-local extensions require workspace trust. Contribution conflicts never grant an implicit override. Secrets remain in nanobot provider or host config -and are exposed only through declared host interfaces. Existing workspace, -network, SSRF, and shell restrictions continue to apply to host-provided -operations. +unless the operator explicitly passes values through extension config. Native +Python code and compatible Node processes are trusted code and may still +inspect their process environment or filesystem directly; permission +declarations are review and activation gates, not technical confinement. +Existing workspace, network, SSRF, and shell restrictions apply only when an +extension uses host-provided operations. Untrusted packages remain visible in the catalog with an inactive state. They do not own active contributions and their runtime is not imported. Built-in @@ -110,7 +117,8 @@ and activation does not rewrite `config.json` behind the user's back. The activation gates are deliberately independent: ```text -installed -> dependencies ready -> permissions granted -> trusted + enabled +installed -> integrity verified -> active dependencies ready + -> permissions granted -> trusted + enabled ``` Only candidates that pass every gate own active contributions. Reload first @@ -119,12 +127,12 @@ snapshot. A failed activation is converted into a diagnostic. ## Market boundary -The market is an index, not a runtime. It describes packages available from -PyPI, npm, Git, ClawHub, Pi catalogs, or local sources using the same manifest -shape. Installing a listing still goes through the local installer, policy, -dependency checks, and trust flow. This keeps discovery independent from code -execution and allows multiple catalogs without coupling the agent to one -store. +The market is an index, not a runtime. Extension API v1 searches npm for +nanobot, Pi, and OpenClaw package keywords. Git and local directories are +install sources but are not searchable catalogs. Installing a listing still +goes through the local installer, integrity record, policy, dependency checks, +and trust flow. The boundary permits more catalog adapters later without +coupling package discovery to execution. ## Ownership boundaries diff --git a/docs/extensions.md b/docs/extensions.md index 96ee97359..5d5318806 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -1,9 +1,11 @@ # Extensions -Extensions add capabilities to nanobot without modifying the agent loop. One -extension package can contribute tools, commands, hooks, skills, channels, -providers, MCP servers, or WebUI surfaces. nanobot also recognizes compatible -Pi packages and OpenClaw plugins, with capability-by-capability diagnostics. +Extensions add capabilities to nanobot without modifying the agent loop. A +package can declare tools, commands, hooks, skills, channels, providers, MCP +servers, or WebUI surfaces in one governable manifest. Extension API v1 +executes native Python tools, commands, and hooks plus the compatible Pi and +OpenClaw surfaces listed below; other contribution kinds are catalog metadata +until their owning nanobot subsystem provides an activation adapter. Use this page to install and manage extensions. To publish one, read [Extension Authoring](./extension-authoring.md). For the internal design, read @@ -23,7 +25,7 @@ local extension store. The safe lifecycle is: Installation does not grant trust. An installed package remains visible but inactive until it is enabled, trusted, has all requested permissions, and -passes dependency checks. +passes integrity and dependency checks. ### WebUI @@ -81,10 +83,12 @@ Extensions can come from three scopes: | User | `~/.nanobot/extensions/` | Installed and governed through the WebUI or CLI | | Workspace | `/.nanobot/extensions/` | Project-local code; controlled by workspace trust policy | -When the same extension ID exists in multiple scopes, the nearest scope wins: -workspace over user, user over built in. A contribution cannot silently replace -another extension's contribution. Explicit replacement metadata and sufficient -scope are required. +When the same extension ID exists in multiple scopes, the nearest eligible copy +wins: workspace over user, user over built in. An untrusted, disabled, denied, +or invalid copy does not hide a usable lower-scope copy. A contribution cannot +silently replace another extension's contribution. Disable one owner before +activating the other; extension API v1 does not let packages replace core or +third-party registrations. ## Pi and OpenClaw Packages @@ -92,6 +96,8 @@ nanobot reads native Pi and OpenClaw package metadata and runs supported JavaScript or TypeScript entries in a Node.js sidecar. Compatibility is not all-or-nothing: +- Adapted packages always request `runtime.node`, making process-level code + execution explicit before trust and activation. - Tools, slash commands, and supported lifecycle observation hooks can run. - Provider-like registrations and several host-specific capabilities may be cataloged but not executable. @@ -110,15 +116,22 @@ dependency. - **Enabled** says the extension may activate. - **Trusted** says you approve executing its code. -- **Granted permissions** are exact host capabilities approved for that - extension. -- **Dependencies** must be present before activation. +- **Granted permissions** record the exact capabilities you reviewed and + approved. +- **Dependencies** must themselves be active before activation. -Permissions are host policy, not an operating-system sandbox. A trusted native -Python extension executes in the nanobot process. A Pi or OpenClaw extension -executes in a separate Node.js process, which improves failure isolation but is -not a strong OS security boundary. Use containers or another OS sandbox for -untrusted third-party code. +Permissions are consent and activation gates, not runtime capability +enforcement or an operating-system sandbox. Direct extension code may access +anything available to its process. A trusted native Python extension executes +inside nanobot. A Pi or OpenClaw extension executes in a separate Node.js +process, which improves failure isolation but is not a strong security +boundary. Their generated manifests therefore request `runtime.node`; granting +it acknowledges this execution model but does not confine the process. Use +containers or another OS sandbox for untrusted third-party code. + +nanobot records a package content hash at installation. If files change later, +the package cannot activate, even if configuration marks it trusted; reinstall +it so the new contents can be reviewed. npm installation uses lifecycle scripts disabled. This prevents package `preinstall` and `postinstall` scripts from running during installation, but the @@ -152,7 +165,9 @@ deployments can also define extension policy in `~/.nanobot/config.json`: ``` Config entries do not rewrite the installation registry. See -[Configuration](./configuration.md#extensions) for exact fields. +[Configuration](./configuration.md#extensions) for exact fields. Actions from +the Extensions WebUI or CLI reload the extension host. Direct edits to advanced +`extensions` config fields are applied on the next process start. ## Diagnose an Inactive Extension @@ -170,8 +185,9 @@ Common causes: | Untrusted | Review the package, then use `trust` | | Requested permission pending | Grant the exact requested permission set | | Disabled | Use `enable` or remove it from `extensions.deny` | -| Missing dependency | Install the named package, executable, environment variable, or extension | -| Contribution conflict | Disable one owner or use an explicit replacement from an appropriate scope | +| Integrity mismatch | Reinstall and review the changed package | +| Missing dependency | Install and activate the named package, executable, environment variable, or extension | +| Contribution conflict | Disable one owner before activating the other | | Compatibility notice | Read which upstream API was translated, degraded, or unsupported | | Activation failed | Check the package entry, runtime dependency, and gateway logs | diff --git a/docs/webui.md b/docs/webui.md index 02310804f..c2e457db9 100644 --- a/docs/webui.md +++ b/docs/webui.md @@ -285,9 +285,9 @@ The gateway refuses to start with `host` set to `"0.0.0.0"` unless `token` or form. Remote WebUI clients with a valid token can view and use Apps. Actions that -install missing nanobot support packages, such as adding a channel dependency, -are blocked by default. To let trusted remote administrators change the Python -environment through the WebUI, opt in explicitly: +install missing nanobot support packages or first-class extension packages are +blocked by default. To let trusted remote administrators place packages into +this nanobot installation through the WebUI, opt in explicitly: ```json { @@ -298,12 +298,14 @@ environment through the WebUI, opt in explicitly: ``` Use this only for a private deployment where every authenticated WebUI user is -trusted to change the Python environment that nanobot runs in. If you publish -the WebUI through Nginx, Caddy, Cloudflare Tunnel, or a similar service, treat it -as remote access and leave package installs disabled unless that is intentional. +trusted to change the nanobot installation. A remotely installed extension +remains untrusted and inactive: trust, permission grants, activation, disabling, +and removal stay restricted to a browser on the nanobot host. If you publish the +WebUI through Nginx, Caddy, Cloudflare Tunnel, or a similar service, treat it as +remote access and leave package installs disabled unless that is intentional. Optional feature installs use pip's configured package index, including -`PIP_INDEX_URL`. +`PIP_INDEX_URL`. Extension packages use their declared Git or npm source. Leave remote package installs disabled when the WebUI is exposed beyond a private, trusted network. diff --git a/nanobot/agent/tools/mcp.py b/nanobot/agent/tools/mcp.py index dc124a234..a5d42fcf8 100644 --- a/nanobot/agent/tools/mcp.py +++ b/nanobot/agent/tools/mcp.py @@ -830,6 +830,23 @@ class MCPPromptWrapper(_MCPWrapperBase): return "\n".join(parts) or "(no output)" +def _register_mcp_capability( + registry: ToolRegistry, + capability: Tool, + server_name: str, +) -> bool: + owner = f"nanobot.mcp.{server_name}" + if registry.register_if_absent(capability, owner=owner): + return True + logger.warning( + "MCP: skipping capability '{}' from server '{}' because it is already registered by '{}'", + capability.name, + server_name, + registry.owner(capability.name), + ) + return False + + async def connect_mcp_servers( mcp_servers: dict, registry: ToolRegistry ) -> dict[str, MCPConnection]: @@ -963,7 +980,8 @@ async def connect_mcp_servers( ) continue wrapper = MCPToolWrapper(session, name, tool_def, tool_timeout=cfg.tool_timeout) - registry.register(wrapper, owner=f"nanobot.mcp.{name}") + if not _register_mcp_capability(registry, wrapper, name): + continue logger.debug("MCP: registered tool '{}' from server '{}'", wrapper.name, name) registered_count += 1 if enabled_tools: @@ -1000,7 +1018,8 @@ async def connect_mcp_servers( wrapper = MCPResourceWrapper( session, name, resource, resource_timeout=cfg.tool_timeout ) - registry.register(wrapper, owner=f"nanobot.mcp.{name}") + if not _register_mcp_capability(registry, wrapper, name): + continue registered_count += 1 logger.debug( "MCP: registered resource '{}' from server '{}'", @@ -1018,7 +1037,8 @@ async def connect_mcp_servers( wrapper = MCPPromptWrapper( session, name, prompt, prompt_timeout=cfg.tool_timeout ) - registry.register(wrapper, owner=f"nanobot.mcp.{name}") + if not _register_mcp_capability(registry, wrapper, name): + continue registered_count += 1 logger.debug( "MCP: registered prompt '{}' from server '{}'", diff --git a/nanobot/agent/tools/registry.py b/nanobot/agent/tools/registry.py index 35e8691ce..ce9270c59 100644 --- a/nanobot/agent/tools/registry.py +++ b/nanobot/agent/tools/registry.py @@ -34,6 +34,13 @@ class ToolRegistry: self._owners[tool.name] = owner self._cached_definitions = None + def register_if_absent(self, tool: Tool, *, owner: str = "nanobot.core") -> bool: + """Register a tool without replacing an existing capability.""" + if tool.name in self._tools: + return False + self.register(tool, owner=owner) + return True + def unregister(self, name: str) -> None: """Unregister a tool by name.""" self._tools.pop(name, None) diff --git a/nanobot/cli/commands.py b/nanobot/cli/commands.py index 790003760..bbbe0da03 100644 --- a/nanobot/cli/commands.py +++ b/nanobot/cli/commands.py @@ -1389,8 +1389,8 @@ def serve( extension_host = ExtensionHost(agent_loop, lambda: runtime_config) async def on_startup(_app): - await extension_host.reload() await agent_loop._connect_mcp() + await extension_host.reload() async def on_cleanup(_app): try: @@ -2139,6 +2139,7 @@ def _run_gateway( console.print, ) try: + await agent._connect_mcp() await extension_host.reload() await cron.start() # Re-read once on first admission to close the watcher subscription window. @@ -2330,6 +2331,7 @@ def agent( # Single message mode — direct call, no bus needed async def run_once(): try: + await agent_loop._connect_mcp() await extension_host.reload() renderer = StreamRenderer( render_markdown=markdown, @@ -2391,6 +2393,7 @@ def agent( signal.signal(signal.SIGPIPE, signal.SIG_IGN) async def run_interactive(): + await agent_loop._connect_mcp() await extension_host.reload() bus_task = asyncio.create_task(agent_loop.run()) turn_done = asyncio.Event() diff --git a/nanobot/config/schema.py b/nanobot/config/schema.py index 9082cee1e..f11984f6b 100644 --- a/nanobot/config/schema.py +++ b/nanobot/config/schema.py @@ -402,7 +402,7 @@ class ToolsConfig(Base): "webuiAllowRemotePackageInstall", "webui_allow_remote_package_install", ), - ) # allow non-local WebUI clients to install optional Python packages + ) # allow non-local WebUI clients to install optional support and extension packages mcp_servers: dict[str, MCPServerConfig] = Field(default_factory=dict) ssrf_whitelist: list[str] = Field(default_factory=list) # CIDR ranges to exempt from SSRF blocking (e.g. ["100.64.0.0/10"] for Tailscale) diff --git a/nanobot/extensions/codec.py b/nanobot/extensions/codec.py index 9b5cea11a..ad9879b0b 100644 --- a/nanobot/extensions/codec.py +++ b/nanobot/extensions/codec.py @@ -37,9 +37,7 @@ _MANIFEST_KEYS = frozenset( "license", } ) -_CONTRIBUTION_KEYS = frozenset( - {"kind", "name", "target", "description", "replaces"} -) +_CONTRIBUTION_KEYS = frozenset({"kind", "name", "target", "description"}) _DEPENDENCY_KEYS = frozenset({"kind", "name", "specifier", "optional"}) _PERMISSION_KEYS = frozenset({"name", "reason"}) @@ -122,7 +120,6 @@ def manifest_to_mapping(manifest: ExtensionManifest) -> dict[str, Any]: "name": item.name, "target": item.target, "description": item.description, - "replaces": list(item.replaces), } for item in manifest.contributions ], @@ -151,9 +148,6 @@ def _contribution_from_mapping(data: object) -> ExtensionContribution: name=mapping["name"], target=mapping.get("target", ""), description=mapping.get("description", ""), - replaces=tuple( - _sequence(mapping.get("replaces", ()), "contribution replaces") - ), ) except (KeyError, TypeError, ValueError) as exc: raise ManifestFormatError(f"invalid extension contribution: {exc}") from exc diff --git a/nanobot/extensions/compatibility.py b/nanobot/extensions/compatibility.py index 692cb1957..a02d44110 100644 --- a/nanobot/extensions/compatibility.py +++ b/nanobot/extensions/compatibility.py @@ -50,7 +50,8 @@ class RemoteTool(Tool): @property def parameters(self) -> dict[str, Any]: - return self._registration.schema or {"type": "object", "properties": {}} + schema = self._registration.schema or {} + return {"type": "object", "properties": {}, **schema} @property def read_only(self) -> bool: diff --git a/nanobot/extensions/discovery.py b/nanobot/extensions/discovery.py index 92cc1d8fa..a34cb47a5 100644 --- a/nanobot/extensions/discovery.py +++ b/nanobot/extensions/discovery.py @@ -48,7 +48,9 @@ def discover_manifest_root( sorted( path / MANIFEST_FILENAME for path in root.iterdir() - if path.is_dir() and (path / MANIFEST_FILENAME).is_file() + if not path.name.startswith(".") + and path.is_dir() + and (path / MANIFEST_FILENAME).is_file() ) ) diff --git a/nanobot/extensions/host.py b/nanobot/extensions/host.py index 679b34a7f..0281faa24 100644 --- a/nanobot/extensions/host.py +++ b/nanobot/extensions/host.py @@ -59,6 +59,11 @@ class ExtensionHost: config = self._config_loader() catalog = build_extension_catalog( config, + skills=getattr( + getattr(self._agent, "context", None), + "skills", + None, + ), tools=self._agent.tools, commands=self._agent.commands, user_root=self._user_root, diff --git a/nanobot/extensions/manifest.py b/nanobot/extensions/manifest.py index 6e81c7faf..44ee377d4 100644 --- a/nanobot/extensions/manifest.py +++ b/nanobot/extensions/manifest.py @@ -65,6 +65,8 @@ class ExtensionDependency: _require_identifier(self.name, "extension dependency name") if not isinstance(self.specifier, str): raise TypeError("extension dependency specifier must be a string") + if not isinstance(self.optional, bool): + raise TypeError("extension dependency optional must be a boolean") @dataclass(frozen=True, slots=True) @@ -91,7 +93,6 @@ class ExtensionContribution: name: str target: str = "" description: str = "" - replaces: tuple[str, ...] = () def __post_init__(self) -> None: if not isinstance(self.kind, ContributionKind): @@ -101,12 +102,6 @@ class ExtensionContribution: raise TypeError("extension contribution target must be a string") if not isinstance(self.description, str): raise TypeError("extension contribution description must be a string") - if not isinstance(self.replaces, tuple): - raise TypeError("extension contribution replaces must be a tuple") - for extension_id in self.replaces: - _require_identifier(extension_id, "replaced extension id") - if len(set(self.replaces)) != len(self.replaces): - raise ValueError("extension contribution replaces contains duplicates") @dataclass(frozen=True, slots=True) @@ -181,6 +176,11 @@ class ExtensionManifest: ] if len(set(contribution_keys)) != len(contribution_keys): raise ValueError("extension manifest contains duplicate contributions") + dependency_keys = [ + (dependency.kind, dependency.name) for dependency in self.dependencies + ] + if len(set(dependency_keys)) != len(dependency_keys): + raise ValueError("extension manifest contains duplicate dependencies") permission_names = [permission.name for permission in self.permissions] if len(set(permission_names)) != len(permission_names): raise ValueError("extension manifest contains duplicate permissions") @@ -206,6 +206,11 @@ def _require_identifier(value: object, label: str) -> str: return text +def validate_extension_id(value: object) -> str: + """Validate and return one portable extension identifier.""" + return _require_identifier(value, "extension id") + + def _require_tuple_of(value: object, item_type: type, label: str) -> None: if not isinstance(value, tuple) or not all( isinstance(item, item_type) for item in value diff --git a/nanobot/extensions/market.py b/nanobot/extensions/market.py index ead8d08fd..a94d020c2 100644 --- a/nanobot/extensions/market.py +++ b/nanobot/extensions/market.py @@ -77,7 +77,10 @@ def _npm_search(query: str, *, limit: int) -> list[dict[str, Any]]: except subprocess.CalledProcessError as exc: message = (exc.stderr or exc.stdout).strip() raise RuntimeError(message or "extension marketplace search failed") from exc - value = json.loads(result.stdout) + try: + value = json.loads(result.stdout) + except json.JSONDecodeError as exc: + raise RuntimeError("npm returned an invalid marketplace response") from exc if not isinstance(value, list): raise RuntimeError("npm returned an invalid marketplace response") return [row for row in value if isinstance(row, dict)] diff --git a/nanobot/extensions/native.py b/nanobot/extensions/native.py index e67b4c6df..4e18286d5 100644 --- a/nanobot/extensions/native.py +++ b/nanobot/extensions/native.py @@ -230,17 +230,19 @@ def _tool_candidates(tools: ToolRegistry) -> list[ExtensionCandidate]: def _command_candidates(commands: CommandRouter) -> list[ExtensionCandidate]: - grouped: dict[str, list[ExtensionContribution]] = defaultdict(list) + grouped: dict[str, dict[str, ExtensionContribution]] = defaultdict(dict) for _tier, command, owner in commands.registrations(): - grouped[owner].append( + name = _identifier(command.lstrip("/").rstrip()) + grouped[owner].setdefault( + name, ExtensionContribution( kind=ContributionKind.COMMAND, - name=_identifier(command.lstrip("/").rstrip()), + name=name, target=command, - ) + ), ) return [ - _candidate(owner, owner, contributions) + _candidate(owner, owner, contributions.values()) for owner, contributions in sorted(grouped.items()) ] @@ -259,7 +261,7 @@ def _candidate( id=_identifier(extension_id), name=name, version=__version__, - runtime=ExtensionRuntime.PYTHON, + runtime=ExtensionRuntime.DECLARATIVE, contributions=tuple(contributions), dependencies=dependencies, ), @@ -276,17 +278,13 @@ def _python_dependencies( dependencies = [] for raw in requirements: requirement = Requirement(raw) - name = requirement.name - if requirement.extras: - name += f"[{','.join(sorted(requirement.extras))}]" - specifier = str(requirement.specifier) - if requirement.marker: - specifier += f"; {requirement.marker}" + if requirement.marker and not requirement.marker.evaluate(): + continue dependencies.append( ExtensionDependency( kind=DependencyKind.PYTHON, - name=name, - specifier=specifier, + name=requirement.name, + specifier=str(requirement.specifier), ) ) return tuple(dependencies) @@ -325,6 +323,9 @@ def _merge_candidates( location=existing.location or candidate.location, enabled=existing.enabled and candidate.enabled, trusted=existing.trusted and candidate.trusted, + integrity_valid=( + existing.integrity_valid and candidate.integrity_valid + ), ) return tuple( sorted( diff --git a/nanobot/extensions/node_host.py b/nanobot/extensions/node_host.py index 783ac57bc..5a3b01947 100644 --- a/nanobot/extensions/node_host.py +++ b/nanobot/extensions/node_host.py @@ -1,4 +1,4 @@ -"""Async process boundary for untrusted-compatible JavaScript extension APIs.""" +"""Async process boundary for compatible JavaScript extension APIs.""" from __future__ import annotations @@ -16,9 +16,11 @@ from nanobot.extensions.protocol import ( NodeProtocolError, ) +_MAX_MESSAGE_BYTES = 16 * 1024 * 1024 + class NodeSidecar: - """One isolated Node process hosting one extension module.""" + """One failure-isolated Node process hosting one trusted extension.""" def __init__( self, @@ -55,10 +57,15 @@ class NodeSidecar: stdin=asyncio.subprocess.PIPE, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, + limit=_MAX_MESSAGE_BYTES + 1, ) self._reader = asyncio.create_task(self._read_stdout()) self._stderr_reader = asyncio.create_task(self._read_stderr()) - hello = await self.request("hello", {}) + try: + hello = await self.request("hello", {}) + except Exception: + await self.close() + raise if hello.get("protocol") != NODE_PROTOCOL_VERSION: await self.close() raise NodeProtocolError( @@ -123,11 +130,26 @@ class NodeSidecar: }, separators=(",", ":"), ).encode() + if len(message) > _MAX_MESSAGE_BYTES: + self._pending.pop(request_id, None) + raise NodeProtocolError("sidecar request exceeds the 16 MB protocol limit") try: async with self._write_lock: process.stdin.write(message + b"\n") await process.stdin.drain() result = await asyncio.wait_for(future, timeout or self._timeout) + except asyncio.TimeoutError as exc: + self._pending.pop(request_id, None) + if process.returncode is None: + process.kill() + await process.wait() + raise NodeProtocolError( + f"sidecar method {method!r} timed out" + ) from exc + except asyncio.CancelledError: + self._pending.pop(request_id, None) + future.cancel() + raise except Exception: self._pending.pop(request_id, None) raise @@ -161,21 +183,28 @@ class NodeSidecar: assert self._process and self._process.stdout try: while line := await self._process.stdout.readline(): - try: - message = json.loads(line) - request_id = message["id"] - future = self._pending.pop(request_id) - if error := message.get("error"): - future.set_exception( - NodeProtocolError( - f"{error.get('code', 'sidecar_error')}: " - f"{error.get('message', 'unknown sidecar error')}" - ) + if len(line) > _MAX_MESSAGE_BYTES: + raise NodeProtocolError( + "sidecar response exceeds the 16 MB protocol limit" + ) + message = json.loads(line) + request_id = message["id"] + future = self._pending.pop(request_id, None) + if future is None or future.done(): + continue + if error := message.get("error"): + future.set_exception( + NodeProtocolError( + f"{error.get('code', 'sidecar_error')}: " + f"{error.get('message', 'unknown sidecar error')}" ) - else: - future.set_result(message.get("result", {})) - except Exception as exc: - self._fail_pending(NodeProtocolError(f"invalid sidecar response: {exc}")) + ) + else: + future.set_result(message.get("result", {})) + except Exception as exc: + self._fail_pending(NodeProtocolError(f"invalid sidecar response: {exc}")) + if self._process and self._process.returncode is None: + self._process.kill() finally: if self._process and self._process.returncode is None: await self._process.wait() diff --git a/nanobot/extensions/node_sidecar.mjs b/nanobot/extensions/node_sidecar.mjs index 1066f7996..48a102130 100644 --- a/nanobot/extensions/node_sidecar.mjs +++ b/nanobot/extensions/node_sidecar.mjs @@ -1,4 +1,5 @@ import { AsyncLocalStorage } from "node:async_hooks"; +import { createRequire } from "node:module"; import { pathToFileURL } from "node:url"; import readline from "node:readline"; @@ -86,7 +87,7 @@ function addTool(tool, flavor, options = {}) { runtimeConfig: state.config, getRuntimeConfig: () => state.config, workspaceDir: state.workspace, - sandboxed: true, + sandboxed: false, }); for (const item of Array.isArray(resolved) ? resolved : [resolved]) { if (item) addTool(item, flavor, options); @@ -96,6 +97,9 @@ function addTool(tool, flavor, options = {}) { if (!tool || typeof tool !== "object" || typeof tool.name !== "string") { throw new Error("registered tool must define a name"); } + if (state.tools.has(tool.name)) { + throw new Error(`tool '${tool.name}' is already registered by this extension`); + } state.tools.set(tool.name, { tool, flavor }); addRegistration("tool", tool.name, { description: tool.description, @@ -113,6 +117,15 @@ function invocationOutput(value) { if (context) context.outputs.push(value); } +function addCommand(name, command, flavor) { + const normalized = String(name); + if (state.commands.has(normalized)) { + throw new Error(`command '${normalized}' is already registered by this extension`); + } + state.commands.set(normalized, { command, flavor }); + addRegistration("command", normalized, { description: command?.description }); +} + function eventBus() { return { on: (name, handler) => addHook(`event:${name}`, handler, "pi-event"), @@ -124,10 +137,7 @@ function piApi() { const api = { on: (name, handler) => addHook(name, handler, "pi"), registerTool: (tool) => addTool(tool, "pi"), - registerCommand: (name, options) => { - state.commands.set(String(name), { command: options, flavor: "pi" }); - addRegistration("command", name, { description: options?.description }); - }, + registerCommand: (name, options) => addCommand(name, options, "pi"), registerProvider: (nameOrProvider, config) => { const provider = typeof nameOrProvider === "string" @@ -188,10 +198,7 @@ function openClawApi() { error: writeError, }, registerTool: (tool, options) => addTool(tool, "openclaw", options), - registerCommand: (command) => { - state.commands.set(command.name, { command, flavor: "openclaw" }); - addRegistration("command", command.name, { description: command.description }); - }, + registerCommand: (command) => addCommand(command.name, command, "openclaw"), registerHook: (names, handler) => addHook(names, handler, "openclaw"), on: (name, handler) => addHook(name, handler, "openclaw"), registerProvider: (provider) => @@ -247,8 +254,8 @@ async function importModule(entry) { } catch (error) { if (![".ts", ".tsx", ".cts", ".mts"].some((suffix) => entry.endsWith(suffix))) throw error; try { - const imported = await import("jiti"); - const createJiti = imported.createJiti || imported.default; + const imported = createRequire(pathToFileURL(entry))("jiti"); + const createJiti = imported.createJiti || imported.default || imported; return await createJiti(import.meta.url, { interopDefault: true }).import(entry); } catch (jitiError) { throw new Error( diff --git a/nanobot/extensions/package_adapter.py b/nanobot/extensions/package_adapter.py index 8760b9b6c..fc164521f 100644 --- a/nanobot/extensions/package_adapter.py +++ b/nanobot/extensions/package_adapter.py @@ -15,6 +15,7 @@ from nanobot.extensions.manifest import ( ExtensionContribution, ExtensionDependency, ExtensionManifest, + ExtensionPermission, ExtensionRuntime, ) @@ -25,6 +26,16 @@ _OPENCLAW_CONTRACT_KINDS = { "imageGenerationProviders": ContributionKind.IMAGE_GENERATION_PROVIDER, "webSearchProviders": ContributionKind.WEB_SEARCH_PROVIDER, } +_TYPESCRIPT_SUFFIXES = (".ts", ".tsx", ".cts", ".mts") +_JITI_DEPENDENCY = ExtensionDependency( + kind=DependencyKind.NPM, + name="jiti", + specifier="^2.4.2", +) +_NODE_RUNTIME_PERMISSION = ExtensionPermission( + name="runtime.node", + reason="Run third-party JavaScript or TypeScript in a Node.js process.", +) @dataclass(frozen=True, slots=True) @@ -64,6 +75,8 @@ def _adapt_pi(package: dict[str, Any]) -> AdaptedPackage: version=str(package.get("version") or "0.0.0"), runtime=ExtensionRuntime.PI, entries=tuple(entries), + dependencies=(_JITI_DEPENDENCY,) if _has_typescript(entries) else (), + permissions=(_NODE_RUNTIME_PERMISSION,), description=str(package.get("description") or ""), homepage=_homepage(package), license=str(package.get("license") or ""), @@ -103,6 +116,7 @@ def _adapt_openclaw(root: Path, package: dict[str, Any]) -> AdaptedPackage: entries=tuple(entries), contributions=contributions, dependencies=_openclaw_dependencies(package, openclaw), + permissions=(_NODE_RUNTIME_PERMISSION,), description=str( plugin.get("description") or package.get("description") or "" ), @@ -202,3 +216,7 @@ def _homepage(package: dict[str, Any]) -> str: if isinstance(repository, dict) and isinstance(repository.get("url"), str): return repository["url"] return "" + + +def _has_typescript(entries: list[str]) -> bool: + return any(entry.lower().endswith(_TYPESCRIPT_SUFFIXES) for entry in entries) diff --git a/nanobot/extensions/preflight.py b/nanobot/extensions/preflight.py index 43422e536..53e8b5580 100644 --- a/nanobot/extensions/preflight.py +++ b/nanobot/extensions/preflight.py @@ -9,21 +9,15 @@ import shutil from dataclasses import replace from pathlib import Path -from packaging.specifiers import InvalidSpecifier, SpecifierSet -from packaging.version import InvalidVersion, Version - from nanobot.extensions.manifest import DependencyKind, ExtensionDependency from nanobot.extensions.registry import ExtensionCandidate, ExtensionDiagnostic +from nanobot.extensions.versioning import dependency_version_failure def evaluate_dependencies( candidates: tuple[ExtensionCandidate, ...], ) -> tuple[tuple[ExtensionCandidate, ...], tuple[ExtensionDiagnostic, ...]]: """Disable candidates with missing hard dependencies and explain why.""" - available = { - candidate.manifest.id: candidate.manifest.version - for candidate in candidates - } checked: list[ExtensionCandidate] = [] diagnostics: list[ExtensionDiagnostic] = [] for candidate in candidates: @@ -35,7 +29,6 @@ def evaluate_dependencies( message := _dependency_failure( dependency, location=candidate.location, - extensions=available, ) ) ] @@ -57,7 +50,6 @@ def _dependency_failure( dependency: ExtensionDependency, *, location: Path | None, - extensions: dict[str, str], ) -> str: if dependency.kind is DependencyKind.EXECUTABLE: if shutil.which(dependency.name) is None: @@ -72,17 +64,19 @@ def _dependency_failure( version = importlib.metadata.version(dependency.name) except importlib.metadata.PackageNotFoundError: return f"Required Python package is not installed: {dependency.name}" - return _version_failure(dependency, version, "Python package") + return dependency_version_failure(dependency, version, "Python package") if dependency.kind is DependencyKind.NPM: version = _npm_version(location, dependency.name) if version is None: return f"Required npm package is not installed: {dependency.name}" - return _version_failure(dependency, version, "npm package") + # npm resolved the declared range while installing the package. Re-parsing + # npm semver here with Python's PEP 440 rules rejects valid ranges such as + # "latest", "^1.0.0", and "~2.3". + return "" if dependency.kind is DependencyKind.EXTENSION: - version = extensions.get(dependency.name) - if version is None: - return f"Required extension is not installed: {dependency.name}" - return _version_failure(dependency, version, "extension") + # Extension dependencies are evaluated after policy selection so an + # installed but inactive package cannot satisfy an activation prerequisite. + return "" return f"Unsupported dependency kind: {dependency.kind.value}" @@ -96,25 +90,3 @@ def _npm_version(location: Path | None, name: str) -> str | None: return None version = value.get("version") if isinstance(value, dict) else None return version if isinstance(version, str) else None - - -def _version_failure( - dependency: ExtensionDependency, - version: str, - label: str, -) -> str: - if not dependency.specifier: - return "" - try: - matches = Version(version) in SpecifierSet(dependency.specifier) - except (InvalidSpecifier, InvalidVersion): - return ( - f"{label} {dependency.name} has an unsupported version constraint: " - f"{dependency.specifier}" - ) - if matches: - return "" - return ( - f"{label} {dependency.name} {version} does not satisfy " - f"{dependency.specifier}" - ) diff --git a/nanobot/extensions/protocol.py b/nanobot/extensions/protocol.py index d8290d0ec..c95c6316a 100644 --- a/nanobot/extensions/protocol.py +++ b/nanobot/extensions/protocol.py @@ -2,10 +2,12 @@ from __future__ import annotations +import re from dataclasses import dataclass from typing import Any NODE_PROTOCOL_VERSION = 1 +_CALLABLE_NAME = re.compile(r"^[A-Za-z0-9_-]{1,64}$") class NodeProtocolError(RuntimeError): @@ -28,14 +30,33 @@ class NodeRegistration: raise NodeProtocolError("sidecar registration must be an object") kind = value.get("kind") name = value.get("name") - if not isinstance(kind, str) or not isinstance(name, str): - raise NodeProtocolError("sidecar registration requires string kind and name") + if ( + not isinstance(kind, str) + or not isinstance(name, str) + or not kind.strip() + or not name.strip() + ): + raise NodeProtocolError("sidecar registration requires non-empty string kind and name") schema = value.get("schema") metadata = value.get("metadata") if schema is not None and not isinstance(schema, dict): raise NodeProtocolError("sidecar registration schema must be an object") if metadata is not None and not isinstance(metadata, dict): raise NodeProtocolError("sidecar registration metadata must be an object") + if kind in {"tool", "command"}: + if not _CALLABLE_NAME.fullmatch(name): + raise NodeProtocolError( + f"sidecar {kind} name {name!r} must match " + "[A-Za-z0-9_-] and be at most 64 characters" + ) + if ( + kind == "tool" + and schema is not None + and schema.get("type", "object") != "object" + ): + raise NodeProtocolError( + f"sidecar tool {name!r} parameters must use an object schema" + ) return cls( kind=kind, name=name, diff --git a/nanobot/extensions/registry.py b/nanobot/extensions/registry.py index bd68e03f4..55a1f930e 100644 --- a/nanobot/extensions/registry.py +++ b/nanobot/extensions/registry.py @@ -8,9 +8,11 @@ from pathlib import Path from nanobot.extensions.manifest import ( ContributionKind, + DependencyKind, ExtensionContribution, ExtensionManifest, ) +from nanobot.extensions.versioning import dependency_version_failure class ExtensionScope(IntEnum): @@ -30,6 +32,7 @@ class ExtensionCandidate: location: Path | None = None enabled: bool = True trusted: bool = False + integrity_valid: bool = True granted_permissions: frozenset[str] = frozenset() def __post_init__(self) -> None: @@ -39,6 +42,8 @@ class ExtensionCandidate: raise TypeError("extension candidate scope must be an ExtensionScope") if self.location is not None and not isinstance(self.location, Path): raise TypeError("extension candidate location must be a Path or None") + if not isinstance(self.integrity_valid, bool): + raise TypeError("extension integrity state must be a boolean") if not isinstance(self.granted_permissions, frozenset): raise TypeError("extension granted permissions must be a frozenset") @@ -68,6 +73,7 @@ class ExtensionPolicy: } return ( candidate.enabled + and candidate.integrity_valid and ( candidate.scope is ExtensionScope.BUILTIN or ( @@ -95,6 +101,7 @@ class ExtensionDiagnostic: code: str extension_id: str message: str + severity: str = "warning" @dataclass(frozen=True, slots=True) @@ -136,9 +143,10 @@ class ExtensionRegistry: def snapshot(self) -> ExtensionSnapshot: active, selection_diagnostics = self._select_active_extensions() + active, dependency_diagnostics = self._resolve_extension_dependencies(active) resolved, resolution_diagnostics = self._resolve_contributions(active) return ExtensionSnapshot( - extensions=tuple(sorted(active.values(), key=lambda item: item.manifest.id)), + extensions=self._activation_order(active), contributions=tuple( sorted( resolved.values(), @@ -148,7 +156,11 @@ class ExtensionRegistry: ), ) ), - diagnostics=tuple(selection_diagnostics + resolution_diagnostics), + diagnostics=tuple( + selection_diagnostics + + dependency_diagnostics + + resolution_diagnostics + ), ) def _select_active_extensions( @@ -186,6 +198,119 @@ class ExtensionRegistry: ) return active, diagnostics + def _resolve_extension_dependencies( + self, + active: dict[str, ExtensionCandidate], + ) -> tuple[dict[str, ExtensionCandidate], list[ExtensionDiagnostic]]: + active = dict(active) + diagnostics: list[ExtensionDiagnostic] = [] + while active: + failed: dict[str, list[str]] = {} + for extension_id, candidate in active.items(): + for dependency in candidate.manifest.dependencies: + if ( + dependency.optional + or dependency.kind is not DependencyKind.EXTENSION + ): + continue + required = active.get(dependency.name) + if required is None: + failed.setdefault(extension_id, []).append( + f"Required extension is not active: {dependency.name}" + ) + continue + if message := dependency_version_failure( + dependency, + required.manifest.version, + "extension", + ): + failed.setdefault(extension_id, []).append(message) + if failed: + for extension_id, messages in sorted(failed.items()): + active.pop(extension_id, None) + diagnostics.extend( + ExtensionDiagnostic( + code="dependency_missing", + extension_id=extension_id, + message=message, + ) + for message in messages + ) + continue + + cycle = self._dependency_cycle(active) + if not cycle: + break + for extension_id in sorted(cycle): + active.pop(extension_id, None) + diagnostics.append( + ExtensionDiagnostic( + code="dependency_cycle", + extension_id=extension_id, + message=( + "Extension dependency cycle contains: " + + ", ".join(sorted(cycle)) + ), + ) + ) + return active, diagnostics + + @staticmethod + def _dependency_cycle( + active: dict[str, ExtensionCandidate], + ) -> set[str]: + state: dict[str, int] = {} + stack: list[str] = [] + cycle: set[str] = set() + + def visit(extension_id: str) -> None: + state[extension_id] = 1 + stack.append(extension_id) + dependencies = ( + dependency.name + for dependency in active[extension_id].manifest.dependencies + if not dependency.optional + and dependency.kind is DependencyKind.EXTENSION + and dependency.name in active + ) + for dependency_id in sorted(dependencies): + if state.get(dependency_id, 0) == 0: + visit(dependency_id) + elif state.get(dependency_id) == 1: + cycle.update(stack[stack.index(dependency_id) :]) + stack.pop() + state[extension_id] = 2 + + for extension_id in sorted(active): + if state.get(extension_id, 0) == 0: + visit(extension_id) + return cycle + + @staticmethod + def _activation_order( + active: dict[str, ExtensionCandidate], + ) -> tuple[ExtensionCandidate, ...]: + ordered: list[ExtensionCandidate] = [] + visited: set[str] = set() + + def visit(extension_id: str) -> None: + if extension_id in visited: + return + visited.add(extension_id) + dependencies = ( + dependency.name + for dependency in active[extension_id].manifest.dependencies + if dependency.kind is DependencyKind.EXTENSION + and dependency.name in active + ) + for dependency_id in sorted(dependencies): + visit(dependency_id) + ordered.append(active[extension_id]) + + for extension_id in sorted(active): + visit(extension_id) + return tuple(ordered) + def _resolve_contributions( self, active: dict[str, ExtensionCandidate], @@ -209,20 +334,14 @@ class ExtensionRegistry: resolved[key] = ResolvedContribution(contribution, candidate) continue existing_id = existing.owner.manifest.id - if ( - existing_id in contribution.replaces - and candidate.scope >= existing.owner.scope - ): - resolved[key] = ResolvedContribution(contribution, candidate) - continue diagnostics.append( ExtensionDiagnostic( code="contribution_conflict", extension_id=candidate.manifest.id, message=( f"{contribution.kind.value} '{contribution.name}' is already " - f"owned by extension '{existing_id}'; declare an explicit " - "replacement from an equal or higher scope to override it" + f"owned by extension '{existing_id}'; disable one owner " + "before activating the other" ), ) ) diff --git a/nanobot/extensions/runtime.py b/nanobot/extensions/runtime.py index a18a4cd01..7b4d86267 100644 --- a/nanobot/extensions/runtime.py +++ b/nanobot/extensions/runtime.py @@ -3,6 +3,7 @@ from __future__ import annotations import importlib +import shutil import sys from dataclasses import dataclass from pathlib import Path @@ -14,6 +15,7 @@ from nanobot.agent.tools.registry import ToolRegistry from nanobot.command.router import CommandRouter, Handler from nanobot.config.schema import Config from nanobot.extensions.compatibility import CompatibleExtension +from nanobot.extensions.manifest import DependencyKind from nanobot.extensions.node_host import NodeSidecar from nanobot.extensions.registry import ( ExtensionCandidate, @@ -57,7 +59,11 @@ class PythonExtensionApi: self._hook_factories = hook_factories def register_tool(self, tool: Tool) -> None: - self._tools.register(tool, owner=self.owner) + if not self._tools.register_if_absent(tool, owner=self.owner): + existing = self._tools.owner(tool.name) or "unknown" + raise ValueError( + f"tool '{tool.name}' is already registered by '{existing}'" + ) def register_command( self, @@ -66,11 +72,19 @@ class PythonExtensionApi: *, prefix: bool = False, ) -> None: + command = f"/{command.lstrip('/')}" + if prefix: + command = f"{command} " register = self._commands.prefix if prefix else self._commands.exact + tier = "prefix" if prefix else "exact" + if existing := self._commands.owner(tier, command): + raise ValueError( + f"command '{command}' is already registered by '{existing}'" + ) register(command, handler, owner=self.owner) def register_hook_factory(self, factory: AgentTurnHookFactory) -> None: - self._hook_factories.append(factory) + self._hook_factories.append(_owned_hook_factory(factory, self.owner)) class ExtensionRuntimeManager: @@ -92,13 +106,38 @@ class ExtensionRuntimeManager: async def activate(self, snapshot: ExtensionSnapshot) -> ActivationResult: diagnostics: list[ExtensionDiagnostic] = [] + activated_ids = { + candidate.manifest.id + for candidate in snapshot.extensions + if candidate.location is None + } for candidate in snapshot.extensions: if candidate.location is None: continue + missing = [ + dependency.name + for dependency in candidate.manifest.dependencies + if not dependency.optional + and dependency.kind is DependencyKind.EXTENSION + and dependency.name not in activated_ids + ] + if missing: + diagnostics.append( + ExtensionDiagnostic( + code="dependency_activation_failed", + extension_id=candidate.manifest.id, + message=( + "Required extensions did not activate: " + + ", ".join(sorted(missing)) + ), + ) + ) + continue try: active = await self._activate_candidate(candidate) if active is not None: self._active.append(active) + activated_ids.add(candidate.manifest.id) diagnostics.extend(active.diagnostics) except Exception as exc: await self._rollback_owner(candidate.manifest.id) @@ -119,7 +158,6 @@ class ExtensionRuntimeManager: for active in reversed(self._active): await self._rollback_owner(active.candidate.manifest.id, active) self._active.clear() - self._hook_factories.clear() async def _activate_candidate( self, @@ -183,14 +221,26 @@ class ExtensionRuntimeManager: def _activate_python(self, candidate: ExtensionCandidate) -> None: if len(candidate.manifest.activation_entries) != 1: raise ValueError("Python extensions must declare exactly one entry") - module_name, separator, attribute = candidate.manifest.entry.partition(":") + raw_entry = candidate.manifest.activation_entries[0] + module_name, separator, attribute = raw_entry.partition(":") if not separator: - module_name = candidate.manifest.entry + module_name = raw_entry attribute = "register" assert candidate.location is not None + importlib.invalidate_caches() + _unload_modules_under(candidate.location) + _reject_module_collision(module_name, candidate.location) sys.path.insert(0, str(candidate.location)) try: - register = getattr(importlib.import_module(module_name), attribute) + module = importlib.import_module(module_name) + module_path = getattr(module, "__file__", None) + if not module_path or not Path(module_path).resolve().is_relative_to( + candidate.location.resolve() + ): + raise ValueError( + f"Python extension entry resolves outside its package: {module_name}" + ) + register = getattr(module, attribute) api = PythonExtensionApi( owner=candidate.manifest.id, tools=self._tools, @@ -200,6 +250,9 @@ class ExtensionRuntimeManager: result = register(api) if result is not None: raise TypeError("Python extension register function must return None") + except Exception: + _unload_modules_under(candidate.location) + raise finally: sys.path.remove(str(candidate.location)) @@ -210,12 +263,11 @@ class ExtensionRuntimeManager: ) -> None: owner = candidate.manifest.id for tool in compatible.tools: - existing = self._tools.owner(tool.name) - if existing and existing != owner: + if not self._tools.register_if_absent(tool, owner=owner): + existing = self._tools.owner(tool.name) or "unknown" raise ValueError( f"tool '{tool.name}' is already registered by '{existing}'" ) - self._tools.register(tool, owner=owner) compatible.register_commands(self._commands) if hook := compatible.hook: self._hook_factories.append(_constant_hook_factory(hook, owner)) @@ -227,13 +279,19 @@ class ExtensionRuntimeManager: ) -> None: self._tools.unregister_owner(owner) self._commands.unregister_owner(owner) - self._hook_factories = [ + self._hook_factories[:] = [ factory for factory in self._hook_factories if getattr(factory, "__nanobot_extension_owner__", None) != owner ] if active and active.compatible: await active.compatible.close() + if ( + active + and active.candidate.manifest.runtime.value == "python" + and active.candidate.location + ): + _unload_modules_under(active.candidate.location) def _resolve_entries(candidate: ExtensionCandidate) -> tuple[Path, ...]: @@ -263,3 +321,48 @@ def _constant_hook_factory(hook: AgentHook, owner: str) -> AgentTurnHookFactory: setattr(factory, "__nanobot_extension_owner__", owner) return factory + + +def _owned_hook_factory( + factory: AgentTurnHookFactory, + owner: str, +) -> AgentTurnHookFactory: + def owned(context: Any) -> AgentHook | None: + return factory(context) + + setattr(owned, "__nanobot_extension_owner__", owner) + return owned + + +def _modules_under(root: Path) -> tuple[str, ...]: + package_root = root.resolve() + return tuple( + name + for name, module in tuple(sys.modules.items()) + if (raw_path := getattr(module, "__file__", None)) + and Path(raw_path).resolve().is_relative_to(package_root) + ) + + +def _unload_modules_under(root: Path) -> None: + package_root = root.resolve() + for module_name in _modules_under(package_root): + sys.modules.pop(module_name, None) + for cache in package_root.rglob("__pycache__"): + shutil.rmtree(cache, ignore_errors=True) + + +def _reject_module_collision(module_name: str, root: Path) -> None: + package_root = root.resolve() + parts = module_name.split(".") + for index in range(1, len(parts) + 1): + loaded = sys.modules.get(".".join(parts[:index])) + loaded_path = getattr(loaded, "__file__", None) + if loaded is not None and ( + not loaded_path + or not Path(loaded_path).resolve().is_relative_to(package_root) + ): + raise ValueError( + f"Python extension entry conflicts with loaded module: " + f"{'.'.join(parts[:index])}" + ) diff --git a/nanobot/extensions/service.py b/nanobot/extensions/service.py index d3a126b2f..caf19e6c6 100644 --- a/nanobot/extensions/service.py +++ b/nanobot/extensions/service.py @@ -39,8 +39,14 @@ class ExtensionService: candidates = catalog.candidates diagnostics = catalog.diagnostics active_ids = { - candidate.manifest.id for candidate in catalog.snapshot.extensions + active.candidate.manifest.id + for active in self.host.snapshot.activation.extensions } + active_ids.update( + candidate.manifest.id + for candidate in catalog.snapshot.extensions + if candidate.location is None + ) if self.host and self.host.snapshot: diagnostics += self.host.snapshot.activation.diagnostics records = self.store.records() @@ -163,6 +169,7 @@ def _candidate_payload( "source_ref": record.source_ref if record else "", "integrity": record.integrity if record else "", "installed_at": record.installed_at if record else "", + "managed_by_store": record is not None, } diff --git a/nanobot/extensions/store.py b/nanobot/extensions/store.py index 9e26dfff5..5a30b2cee 100644 --- a/nanobot/extensions/store.py +++ b/nanobot/extensions/store.py @@ -5,6 +5,7 @@ from __future__ import annotations import hashlib import json import os +import re import shutil import subprocess import tarfile @@ -14,18 +15,41 @@ from datetime import UTC, datetime from enum import Enum from pathlib import Path from typing import Any +from urllib.parse import urlparse from uuid import uuid4 -from nanobot.extensions.codec import MANIFEST_FILENAME, dump_manifest +from filelock import FileLock + +from nanobot.extensions.codec import MANIFEST_FILENAME, dump_manifest, load_manifest from nanobot.extensions.discovery import ( ExtensionDiscoveryResult, discover_manifest_root, ) -from nanobot.extensions.manifest import DependencyKind, ExtensionManifest +from nanobot.extensions.manifest import ( + DependencyKind, + ExtensionManifest, + validate_extension_id, +) from nanobot.extensions.package_adapter import AdaptedPackage, adapt_package -from nanobot.extensions.registry import ExtensionScope +from nanobot.extensions.registry import ExtensionDiagnostic, ExtensionScope _REGISTRY_FILENAME = ".registry.json" +_GIT_SCHEMES = frozenset({"git", "http", "https", "ssh"}) +_SCP_GIT_URL = re.compile( + r"(?:[A-Za-z0-9._-]+@)?[A-Za-z0-9](?:[A-Za-z0-9.-]*[A-Za-z0-9])?:\S+" +) +_NPM_ALIAS = re.compile( + r"npm:(?:(?:@[a-z0-9][a-z0-9._~-]*/)?[a-z0-9][a-z0-9._~-]*)" + r"(?:@[^:/\\\s]+)?" +) +_NPM_PACKAGE_SPEC = re.compile( + r"(?:(?:@[a-z0-9][a-z0-9._~-]*/)?[a-z0-9][a-z0-9._~-]*)" + r"(?:@[^:/\\\s]+)?" +) +_SHA256_INTEGRITY = re.compile(r"sha256:[0-9a-f]{64}") +_MAX_ARCHIVE_BYTES = 128 * 1024 * 1024 +_MAX_EXTRACTED_BYTES = 512 * 1024 * 1024 +_MAX_ARCHIVE_MEMBERS = 50_000 class ExtensionSourceKind(str, Enum): @@ -52,20 +76,51 @@ class InstalledExtension: def from_mapping(cls, value: object) -> InstalledExtension: if not isinstance(value, dict): raise ValueError("extension registry record must be an object") + required = { + "id", + "version", + "source", + "source_ref", + "integrity", + "installed_at", + } + missing = sorted(required - value.keys()) + if missing: + raise ValueError( + "extension registry record is missing: " + ", ".join(missing) + ) permissions = value.get("granted_permissions", ()) if not isinstance(permissions, (list, tuple)) or not all( isinstance(permission, str) for permission in permissions ): raise ValueError("extension granted permissions must be an array of strings") + if len(set(permissions)) != len(permissions): + raise ValueError("extension granted permissions cannot contain duplicates") + extension_id = validate_extension_id(value["id"]) + strings = { + field: value[field] + for field in ("version", "source_ref", "integrity", "installed_at") + } + if not all(isinstance(item, str) and item for item in strings.values()): + raise ValueError("extension registry metadata must use non-empty strings") + if _SHA256_INTEGRITY.fullmatch(strings["integrity"]) is None: + raise ValueError("extension registry integrity must be a sha256 digest") + source = value["source"] + if not isinstance(source, str): + raise ValueError("extension registry source must be a string") + enabled = value.get("enabled", True) + trusted = value.get("trusted", False) + if not isinstance(enabled, bool) or not isinstance(trusted, bool): + raise ValueError("extension enabled and trusted values must be booleans") return cls( - id=str(value["id"]), - version=str(value["version"]), - source=ExtensionSourceKind(value["source"]), - source_ref=str(value["source_ref"]), - integrity=str(value["integrity"]), - installed_at=str(value["installed_at"]), - enabled=bool(value.get("enabled", True)), - trusted=bool(value.get("trusted", False)), + id=extension_id, + version=strings["version"], + source=ExtensionSourceKind(source), + source_ref=strings["source_ref"], + integrity=strings["integrity"], + installed_at=strings["installed_at"], + enabled=enabled, + trusted=trusted, granted_permissions=tuple(permissions), ) @@ -83,41 +138,101 @@ class ExtensionStore: def __init__(self, root: Path | None = None) -> None: self.root = (root or Path.home() / ".nanobot" / "extensions").expanduser() + self.root.mkdir(parents=True, exist_ok=True) self.registry_path = self.root / _REGISTRY_FILENAME + self._lock = FileLock(str(self.root / ".lock")) - def records(self) -> dict[str, InstalledExtension]: + def records(self, *, strict: bool = False) -> dict[str, InstalledExtension]: if not self.registry_path.is_file(): return {} try: data = json.loads(self.registry_path.read_text(encoding="utf-8")) - rows = data.get("extensions", []) if isinstance(data, dict) else [] - return { - record.id: record - for item in rows - if (record := InstalledExtension.from_mapping(item)) - } - except (OSError, UnicodeError, json.JSONDecodeError, KeyError, ValueError): + if not isinstance(data, dict) or data.get("version") != 1: + raise ValueError("extension registry must be a version 1 object") + rows = data.get("extensions") + if not isinstance(rows, list): + raise ValueError("extension registry extensions must be an array") + records: dict[str, InstalledExtension] = {} + for item in rows: + record = InstalledExtension.from_mapping(item) + if record.id in records: + raise ValueError( + f"extension registry contains duplicate id: {record.id}" + ) + records[record.id] = record + return records + except ( + OSError, + UnicodeError, + json.JSONDecodeError, + KeyError, + ValueError, + ) as exc: + if strict: + raise ValueError( + f"invalid extension registry {self.registry_path}: {exc}" + ) from exc return {} def discover(self) -> ExtensionDiscoveryResult: """Discover packages and apply persisted enable/trust state.""" result = discover_manifest_root(self.root, scope=ExtensionScope.USER) - records = self.records() - candidates = tuple( - replace( - candidate, - enabled=records.get(candidate.manifest.id, _DEFAULT_RECORD).enabled, - trusted=records.get(candidate.manifest.id, _DEFAULT_RECORD).trusted, - granted_permissions=frozenset( - records.get( - candidate.manifest.id, - _DEFAULT_RECORD, - ).granted_permissions - ), + diagnostics = list(result.diagnostics) + try: + records = self.records(strict=True) + except ValueError as exc: + records = {} + diagnostics.append( + ExtensionDiagnostic( + code="invalid_extension_registry", + extension_id="", + message=str(exc), + ) ) - for candidate in result.candidates - ) - return ExtensionDiscoveryResult(candidates, result.diagnostics) + candidates = [] + for candidate in result.candidates: + record = records.get(candidate.manifest.id, _DEFAULT_RECORD) + trusted = record.trusted + integrity_valid = True + if candidate.location is not None and record is not _DEFAULT_RECORD: + try: + _reject_unsafe_files( + candidate.location, + allow_installed_node_links=True, + ) + actual_integrity = _tree_hash(candidate.location) + except (OSError, ValueError) as exc: + actual_integrity = "" + diagnostics.append( + ExtensionDiagnostic( + code="extension_integrity_error", + extension_id=candidate.manifest.id, + message=f"Could not verify installed package: {exc}", + ) + ) + if actual_integrity != record.integrity: + trusted = False + integrity_valid = False + diagnostics.append( + ExtensionDiagnostic( + code="extension_integrity_mismatch", + extension_id=candidate.manifest.id, + message=( + "Installed package contents changed after installation; " + "reinstall it before trusting it again" + ), + ) + ) + candidates.append( + replace( + candidate, + enabled=record.enabled, + trusted=trusted, + integrity_valid=integrity_valid, + granted_permissions=frozenset(record.granted_permissions), + ) + ) + return ExtensionDiscoveryResult(tuple(candidates), tuple(diagnostics)) def install_local( self, @@ -139,13 +254,46 @@ class ExtensionStore: ref: str = "", trusted: bool = False, ) -> InstallResult: + _validate_git_url(url) with tempfile.TemporaryDirectory(prefix="nanobot-extension-git-") as raw: checkout = Path(raw) / "checkout" - command = ["git", "clone", "--depth", "1"] if ref: - command.extend(["--branch", ref]) - command.extend(["--", url, str(checkout)]) - _run(command) + _run( + [ + "git", + "clone", + "--filter=blob:none", + "--no-checkout", + "--", + url, + str(checkout), + ] + ) + _run( + [ + "git", + "-C", + str(checkout), + "fetch", + "--depth", + "1", + "--", + "origin", + ref, + ] + ) + _run( + [ + "git", + "-C", + str(checkout), + "checkout", + "--detach", + "FETCH_HEAD", + ] + ) + else: + _run(["git", "clone", "--depth", "1", "--", url, str(checkout)]) return self._install_from_directory( checkout, source_kind=ExtensionSourceKind.GIT, @@ -159,6 +307,7 @@ class ExtensionStore: *, trusted: bool = False, ) -> InstallResult: + _validate_npm_package_spec(spec) with tempfile.TemporaryDirectory(prefix="nanobot-extension-npm-") as raw: temp = Path(raw) output = _run( @@ -169,13 +318,20 @@ class ExtensionStore: "--json", "--pack-destination", str(temp), + "--", spec, ] ) rows = json.loads(output) if not isinstance(rows, list) or not rows: raise ValueError("npm pack did not return a package") - archive = temp / rows[0]["filename"] + row = rows[0] + filename = row.get("filename") if isinstance(row, dict) else None + if not isinstance(filename, str) or not filename: + raise ValueError("npm pack returned invalid package metadata") + archive = (temp / filename).resolve() + if not archive.is_relative_to(temp.resolve()) or not archive.is_file(): + raise ValueError("npm pack returned an invalid package archive") checkout = temp / "checkout" checkout.mkdir() _extract_tar(archive, checkout) @@ -198,20 +354,45 @@ class ExtensionStore: extension_id: str, permissions: set[str] | frozenset[str], ) -> InstalledExtension: - return self._update_record( - extension_id, - granted_permissions=tuple(sorted(permissions)), - ) + with self._lock: + records = self.records(strict=True) + if extension_id not in records: + raise KeyError(f"extension '{extension_id}' is not installed") + manifest = load_manifest( + self.root / extension_id / MANIFEST_FILENAME + ) + requested = { + permission.name for permission in manifest.permissions + } + unknown = sorted(set(permissions) - requested) + if unknown: + raise ValueError( + "Cannot grant permissions not requested by the extension: " + + ", ".join(unknown) + ) + return self._update_record_locked( + records, + extension_id, + granted_permissions=tuple(sorted(permissions)), + ) def uninstall(self, extension_id: str) -> None: - records = self.records() - if extension_id not in records: - raise KeyError(f"extension '{extension_id}' is not installed") - target = self.root / extension_id - if target.exists(): - shutil.rmtree(target) - records.pop(extension_id) - self._write_records(records) + with self._lock: + records = self.records(strict=True) + if extension_id not in records: + raise KeyError(f"extension '{extension_id}' is not installed") + target = self.root / extension_id + backup = self.root / f".uninstall-{uuid4().hex}" + if target.exists(): + target.rename(backup) + try: + records.pop(extension_id) + self._write_records(records) + except Exception: + if backup.exists(): + backup.rename(target) + raise + shutil.rmtree(backup, ignore_errors=True) def _install_from_directory( self, @@ -220,6 +401,22 @@ class ExtensionStore: source_kind: ExtensionSourceKind, source_ref: str, trusted: bool, + ) -> InstallResult: + with self._lock: + return self._install_from_directory_locked( + source, + source_kind=source_kind, + source_ref=source_ref, + trusted=trusted, + ) + + def _install_from_directory_locked( + self, + source: Path, + *, + source_kind: ExtensionSourceKind, + source_ref: str, + trusted: bool, ) -> InstallResult: if not source.is_dir(): raise ValueError(f"extension source is not a directory: {source}") @@ -230,8 +427,10 @@ class ExtensionStore: staging = self.root / f".install-{uuid4().hex}" target = self.root / extension_id backup = self.root / f".backup-{uuid4().hex}" - records = self.records() + records = self.records(strict=True) previous = records.get(extension_id) + backup_created = False + target_installed = False try: shutil.copytree( source, @@ -241,10 +440,17 @@ class ExtensionStore: if package.generated: dump_manifest(package.manifest, staging / MANIFEST_FILENAME) _install_node_dependencies(staging, package.manifest) + _reject_unsafe_files(staging, allow_installed_node_links=True) integrity = _tree_hash(staging) if target.exists(): target.rename(backup) + backup_created = True staging.rename(target) + target_installed = True + requested_permissions = { + permission.name for permission in package.manifest.permissions + } + unchanged = bool(previous and previous.integrity == integrity) record = InstalledExtension( id=extension_id, version=package.manifest.version, @@ -253,9 +459,15 @@ class ExtensionStore: integrity=integrity, installed_at=datetime.now(UTC).isoformat(), enabled=previous.enabled if previous else True, - trusted=trusted or bool(previous and previous.trusted), + trusted=trusted or bool(unchanged and previous and previous.trusted), granted_permissions=( - previous.granted_permissions if previous else () + tuple( + permission + for permission in previous.granted_permissions + if permission in requested_permissions + ) + if previous + else () ), ) records[extension_id] = record @@ -264,8 +476,9 @@ class ExtensionStore: return InstallResult(record, package) except Exception: shutil.rmtree(staging, ignore_errors=True) - if backup.exists(): + if target_installed: shutil.rmtree(target, ignore_errors=True) + if backup_created: backup.rename(target) raise @@ -274,11 +487,22 @@ class ExtensionStore: extension_id: str, **changes: Any, ) -> InstalledExtension: - records = self.records() + with self._lock: + records = self.records(strict=True) + return self._update_record_locked(records, extension_id, **changes) + + def _update_record_locked( + self, + records: dict[str, InstalledExtension], + extension_id: str, + **changes: Any, + ) -> InstalledExtension: try: record = replace(records[extension_id], **changes) except KeyError as exc: - raise KeyError(f"extension '{extension_id}' is not installed") from exc + raise KeyError( + f"extension '{extension_id}' is not installed" + ) from exc records[extension_id] = record self._write_records(records) return record @@ -317,9 +541,38 @@ def _install_node_dependencies(root: Path, manifest: ExtensionManifest) -> None: package_path = root / "package.json" if not package_path.is_file(): return - package = json.loads(package_path.read_text(encoding="utf-8")) - dependencies = package.get("dependencies") if isinstance(package, dict) else None - if dependencies: + original = package_path.read_text(encoding="utf-8") + package = json.loads(original) + if not isinstance(package, dict): + raise ValueError("extension package.json must be an object") + dependencies = _dependency_mapping(package, "dependencies") + peer_dependencies = _dependency_mapping(package, "peerDependencies") + peer_metadata = _dependency_mapping(package, "peerDependenciesMeta") + for name, specifier in peer_dependencies.items(): + metadata = peer_metadata.get(name, {}) + if metadata is not None and not isinstance(metadata, dict): + raise ValueError( + f"extension package.json peerDependenciesMeta.{name} must be an object" + ) + if not metadata or metadata.get("optional") is not True: + dependencies.setdefault(name, specifier) + for dependency in manifest.dependencies: + if dependency.kind is not DependencyKind.NPM or dependency.optional: + continue + dependencies[dependency.name] = dependency.specifier or "latest" + optional = _dependency_mapping(package, "optionalDependencies") + for name, specifier in {**dependencies, **optional}.items(): + _validate_npm_dependency_spec(str(name), specifier) + if not dependencies and not optional: + return + runtime_package = { + "name": "nanobot-extension-runtime", + "private": True, + "dependencies": dependencies, + "optionalDependencies": optional, + } + package_path.write_text(json.dumps(runtime_package), encoding="utf-8") + try: _run( [ "npm", @@ -328,31 +581,12 @@ def _install_node_dependencies(root: Path, manifest: ExtensionManifest) -> None: "--ignore-scripts", "--no-audit", "--no-fund", + "--package-lock=false", ], cwd=root, ) - for dependency in manifest.dependencies: - if dependency.kind is not DependencyKind.NPM or dependency.optional: - continue - spec = ( - f"{dependency.name}@{dependency.specifier}" - if dependency.specifier - else dependency.name - ) - _run( - [ - "npm", - "install", - "--save-prod", - "--save-exact", - "--ignore-scripts", - "--no-audit", - "--no-fund", - "--", - spec, - ], - cwd=root, - ) + finally: + package_path.write_text(original, encoding="utf-8") def _run(command: list[str], *, cwd: Path | None = None) -> str: @@ -371,19 +605,112 @@ def _run(command: list[str], *, cwd: Path | None = None) -> str: raise RuntimeError(f"{command[0]} failed: {detail}") from exc -def _reject_unsafe_files(root: Path) -> None: +def _dependency_mapping(package: dict[str, Any], key: str) -> dict[str, Any]: + value = package.get(key) + if value is None: + return {} + if not isinstance(value, dict): + raise ValueError(f"extension package.json {key} must be an object") + return dict(value) + + +def _validate_git_url(url: str) -> None: + if not isinstance(url, str) or not url.strip() or any( + character in url for character in ("\0", "\r", "\n") + ): + raise ValueError("extension Git source must be a remote repository URL") + value = url.strip() + parsed = urlparse(value) + if parsed.scheme: + if parsed.scheme.lower() not in _GIT_SCHEMES or not parsed.hostname or not parsed.path: + raise ValueError( + "extension Git source must use git, http, https, or ssh" + ) + if parsed.password or ( + parsed.scheme.lower() in {"http", "https"} and parsed.username + ): + raise ValueError( + "extension Git URLs cannot contain credentials; use a Git credential helper" + ) + if parsed.query or parsed.fragment: + raise ValueError( + "extension Git URLs cannot contain query parameters or fragments; " + "pass the revision separately" + ) + return + if _SCP_GIT_URL.fullmatch(value) is None or any( + character in value for character in ("?", "#") + ): + raise ValueError("extension Git source must be a remote repository URL") + + +def _validate_npm_package_spec(spec: str) -> None: + if ( + not isinstance(spec, str) + or _NPM_PACKAGE_SPEC.fullmatch(spec.strip()) is None + ): + raise ValueError( + "extension npm source must be a registry package name with an " + "optional version or tag" + ) + + +def _validate_npm_dependency_spec(name: str, specifier: object) -> None: + if not isinstance(specifier, str) or not specifier.strip(): + raise ValueError(f"npm dependency '{name}' must use a non-empty registry specifier") + value = specifier.strip() + if any(character in value for character in ("\0", "\r", "\n")): + raise ValueError(f"npm dependency '{name}' contains invalid characters") + if value.startswith("npm:"): + if _NPM_ALIAS.fullmatch(value) is not None: + return + elif not any(character in value for character in (":", "/", "\\")): + return + raise ValueError( + f"npm dependency '{name}' must resolve through the npm registry" + ) + + +def _reject_unsafe_files( + root: Path, + *, + allow_installed_node_links: bool = False, +) -> None: + package_root = root.resolve() for path in root.rglob("*"): if path.is_symlink(): + relative = path.relative_to(root) + if ( + allow_installed_node_links + and "node_modules" in relative.parts + and _link_target(path).is_relative_to(package_root) + ): + continue raise ValueError(f"extension packages cannot contain symlinks: {path}") if not path.is_file() and not path.is_dir(): raise ValueError(f"extension package contains a special file: {path}") +def _link_target(path: Path) -> Path: + return (path.parent / os.readlink(path)).resolve() + + def _tree_hash(root: Path) -> str: digest = hashlib.sha256() - for path in sorted(item for item in root.rglob("*") if item.is_file()): + for path in sorted( + item + for item in root.rglob("*") + if (item.is_file() or item.is_symlink()) + and "__pycache__" not in item.parts + and item.suffix not in {".pyc", ".pyo"} + ): digest.update(path.relative_to(root).as_posix().encode()) digest.update(b"\0") + if path.is_symlink(): + digest.update(b"link\0") + digest.update(os.fsencode(os.readlink(path))) + continue + digest.update(b"file\0") with path.open("rb") as handle: while chunk := handle.read(1024 * 1024): digest.update(chunk) @@ -391,11 +718,22 @@ def _tree_hash(root: Path) -> str: def _extract_tar(archive: Path, target: Path) -> None: + if archive.stat().st_size > _MAX_ARCHIVE_BYTES: + raise ValueError("npm package archive exceeds the 128 MB limit") with tarfile.open(archive) as bundle: - for member in bundle.getmembers(): + members = bundle.getmembers() + if len(members) > _MAX_ARCHIVE_MEMBERS: + raise ValueError("npm package archive contains too many files") + extracted_bytes = 0 + for member in members: destination = (target / member.name).resolve() if not destination.is_relative_to(target.resolve()): raise ValueError("npm package archive contains a path traversal") if member.issym() or member.islnk(): raise ValueError("npm package archive contains a link") - bundle.extractall(target) + if not member.isfile() and not member.isdir(): + raise ValueError("npm package archive contains a special file") + extracted_bytes += member.size + if extracted_bytes > _MAX_EXTRACTED_BYTES: + raise ValueError("npm package archive exceeds the 512 MB extracted limit") + bundle.extractall(target, members=members) diff --git a/nanobot/extensions/versioning.py b/nanobot/extensions/versioning.py new file mode 100644 index 000000000..ba2e75486 --- /dev/null +++ b/nanobot/extensions/versioning.py @@ -0,0 +1,29 @@ +"""Version constraint checks shared by extension activation gates.""" + +from packaging.specifiers import InvalidSpecifier, SpecifierSet +from packaging.version import InvalidVersion, Version + +from nanobot.extensions.manifest import ExtensionDependency + + +def dependency_version_failure( + dependency: ExtensionDependency, + version: str, + label: str, +) -> str: + """Return a user-facing constraint failure, or an empty string on success.""" + if not dependency.specifier: + return "" + try: + matches = Version(version) in SpecifierSet(dependency.specifier) + except (InvalidSpecifier, InvalidVersion): + return ( + f"{label} {dependency.name} has an unsupported version constraint: " + f"{dependency.specifier}" + ) + if matches: + return "" + return ( + f"{label} {dependency.name} {version} does not satisfy " + f"{dependency.specifier}" + ) diff --git a/nanobot/nanobot.py b/nanobot/nanobot.py index 5c807a1f0..6b88e22d0 100644 --- a/nanobot/nanobot.py +++ b/nanobot/nanobot.py @@ -334,6 +334,7 @@ class Nanobot: return async with self._extensions_lock: if not self._extensions_started: + await self._loop._connect_mcp() await self._extensions.reload() self._extensions_started = True diff --git a/nanobot/webui/extensions_routes.py b/nanobot/webui/extensions_routes.py index 246b91367..b45a03d5f 100644 --- a/nanobot/webui/extensions_routes.py +++ b/nanobot/webui/extensions_routes.py @@ -81,7 +81,7 @@ class WebUIExtensionsRouter: return None if _method(request) != "POST": return self._error_response(405, "Method not allowed") - if not self._mutation_allowed(connection, request): + if not self._mutation_allowed(action, connection, request): return self._error_response( 403, "Extension changes require a local WebUI connection", @@ -153,10 +153,14 @@ class WebUIExtensionsRouter: raise ValueError("Extension request must be a JSON object") return value - def _mutation_allowed(self, connection: Any, request: WsRequest) -> bool: - return self._allow_remote_package_install or is_local_browser_request( - connection, - request.headers, + def _mutation_allowed( + self, + action: str, + connection: Any, + request: WsRequest, + ) -> bool: + return is_local_browser_request(connection, request.headers) or ( + action == "install" and self._allow_remote_package_install ) diff --git a/tests/cli/test_commands.py b/tests/cli/test_commands.py index 9f94dbba7..1c8613451 100644 --- a/tests/cli/test_commands.py +++ b/tests/cli/test_commands.py @@ -1437,6 +1437,17 @@ def test_make_provider_rejects_auto_dynamic_custom_prefix_without_api_base(): make_provider(config) +class _FakeExtensionHost: + def __init__(self, *_args, **_kwargs) -> None: + pass + + async def reload(self) -> None: + pass + + async def close(self) -> None: + pass + + @pytest.fixture def mock_agent_runtime(tmp_path): """Mock agent command dependencies for focused CLI tests.""" @@ -1450,9 +1461,11 @@ def mock_agent_runtime(tmp_path): patch("nanobot.cli.commands._print_agent_response") as mock_print_response, \ patch("nanobot.bus.queue.MessageBus"), \ patch("nanobot.cron.service.CronService"), \ + patch("nanobot.extensions.ExtensionHost", _FakeExtensionHost), \ patch("nanobot.cli.commands.AgentLoop.from_config") as mock_from_config: agent_loop = MagicMock() agent_loop.channels_config = None + agent_loop._connect_mcp = AsyncMock(return_value=None) agent_loop.process_direct = AsyncMock( return_value=OutboundMessage(channel="cli", chat_id="direct", content="mock-response"), ) @@ -1534,10 +1547,14 @@ def test_agent_config_sets_active_path(monkeypatch, tmp_path: Path) -> None: async def process_direct(self, *_args, **_kwargs): return OutboundMessage(channel="cli", chat_id="direct", content="ok") + async def _connect_mcp(self) -> None: + return None + async def close_mcp(self) -> None: return None monkeypatch.setattr("nanobot.cli.commands.AgentLoop", _FakeAgentLoop) + monkeypatch.setattr("nanobot.extensions.ExtensionHost", _FakeExtensionHost) monkeypatch.setattr("nanobot.cli.commands._print_agent_response", lambda *_args, **_kwargs: None) result = runner.invoke(app, ["agent", "-m", "hello", "-c", str(config_file)]) @@ -1575,11 +1592,15 @@ def test_agent_uses_workspace_directory_for_cron_store(monkeypatch, tmp_path: Pa async def process_direct(self, *_args, **_kwargs): return OutboundMessage(channel="cli", chat_id="direct", content="ok") + async def _connect_mcp(self) -> None: + return None + async def close_mcp(self) -> None: return None monkeypatch.setattr("nanobot.cron.service.CronService", _FakeCron) monkeypatch.setattr("nanobot.cli.commands.AgentLoop", _FakeAgentLoop) + monkeypatch.setattr("nanobot.extensions.ExtensionHost", _FakeExtensionHost) monkeypatch.setattr("nanobot.cli.commands._print_agent_response", lambda *_args, **_kwargs: None) result = runner.invoke(app, ["agent", "-m", "hello", "-c", str(config_file)]) @@ -1625,11 +1646,15 @@ def test_agent_workspace_override_does_not_migrate_legacy_cron( async def process_direct(self, *_args, **_kwargs): return OutboundMessage(channel="cli", chat_id="direct", content="ok") + async def _connect_mcp(self) -> None: + return None + async def close_mcp(self) -> None: return None monkeypatch.setattr("nanobot.cron.service.CronService", _FakeCron) monkeypatch.setattr("nanobot.cli.commands.AgentLoop", _FakeAgentLoop) + monkeypatch.setattr("nanobot.extensions.ExtensionHost", _FakeExtensionHost) monkeypatch.setattr("nanobot.cli.commands._print_agent_response", lambda *_args, **_kwargs: None) result = runner.invoke( @@ -1681,11 +1706,15 @@ def test_agent_custom_config_workspace_does_not_migrate_legacy_cron( async def process_direct(self, *_args, **_kwargs): return OutboundMessage(channel="cli", chat_id="direct", content="ok") + async def _connect_mcp(self) -> None: + return None + async def close_mcp(self) -> None: return None monkeypatch.setattr("nanobot.cron.service.CronService", _FakeCron) monkeypatch.setattr("nanobot.cli.commands.AgentLoop", _FakeAgentLoop) + monkeypatch.setattr("nanobot.extensions.ExtensionHost", _FakeExtensionHost) monkeypatch.setattr( "nanobot.cli.commands._print_agent_response", lambda *_args, **_kwargs: None ) @@ -1836,6 +1865,7 @@ def _patch_cli_command_runtime( ) -> None: provider_factory = make_provider or (lambda _config: _fake_provider()) + monkeypatch.setattr("nanobot.extensions.ExtensionHost", _FakeExtensionHost) monkeypatch.setattr( "nanobot.config.loader.set_config_path", set_config_path or (lambda _path: None), @@ -1932,6 +1962,9 @@ def test_heartbeat_empty_response_still_retains_recent_messages( async def process_direct(self, *_args, **_kwargs): return SimpleNamespace(content="") + async def _connect_mcp(self) -> None: + return None + async def close_mcp(self) -> None: return None @@ -2592,6 +2625,9 @@ def test_gateway_unbound_agent_cron_is_skipped( async def submit_cron_turn(self, _msg: InboundMessage): raise AssertionError("unbound cron job must not run as a bound cron turn") + async def _connect_mcp(self) -> None: + return None + async def close_mcp(self) -> None: return None @@ -2710,6 +2746,9 @@ def test_gateway_bound_cron_runs_as_session_turn( content="Checked the repo.", ) + async def _connect_mcp(self) -> None: + return None + async def close_mcp(self) -> None: return None @@ -2930,6 +2969,9 @@ def test_gateway_local_trigger_queue_submits_agent_turns( self.runtime_resolver.invalidate.assert_called_once_with() await asyncio.Event().wait() + async def _connect_mcp(self) -> None: + return None + async def close_mcp(self) -> None: return None @@ -3174,6 +3216,9 @@ def test_gateway_health_endpoint_binds_and_serves_expected_responses( async def run(self) -> None: await asyncio.Event().wait() + async def _connect_mcp(self) -> None: + return None + async def close_mcp(self) -> None: return None @@ -3371,6 +3416,9 @@ def test_gateway_shutdown_lets_agent_task_own_mcp_cleanup( finally: seen["agent_task_cleaned_up"] = True + async def _connect_mcp(self) -> None: + return None + async def close_mcp(self) -> None: raise AssertionError("gateway must not close MCP from the outer task") @@ -3470,6 +3518,9 @@ def test_gateway_shutdown_event_exits_forever_runtime_tasks( finally: seen["agent_task_cleaned_up"] = True + async def _connect_mcp(self) -> None: + return None + async def close_mcp(self) -> None: raise AssertionError("gateway must not close MCP from the outer task") diff --git a/tests/extensions/__init__.py b/tests/extensions/__init__.py new file mode 100644 index 000000000..5ee4bf8f2 --- /dev/null +++ b/tests/extensions/__init__.py @@ -0,0 +1 @@ +"""Extension platform tests.""" diff --git a/tests/extensions/test_host.py b/tests/extensions/test_host.py index 75c2a9d82..920c2d294 100644 --- a/tests/extensions/test_host.py +++ b/tests/extensions/test_host.py @@ -11,6 +11,7 @@ class _Agent: def __init__(self) -> None: self.tools = ToolRegistry() self.commands = CommandRouter() + self.context = type("Context", (), {"skills": None})() self._hook_factories = [] diff --git a/tests/extensions/test_manifest.py b/tests/extensions/test_manifest.py index 0089cb725..c3d4dc708 100644 --- a/tests/extensions/test_manifest.py +++ b/tests/extensions/test_manifest.py @@ -2,7 +2,9 @@ import pytest from nanobot.extensions import ( ContributionKind, + DependencyKind, ExtensionContribution, + ExtensionDependency, ExtensionManifest, ExtensionPermission, ExtensionRuntime, @@ -59,3 +61,28 @@ def test_manifest_rejects_duplicate_contributions() -> None: runtime=ExtensionRuntime.DECLARATIVE, contributions=(contribution, contribution), ) + + +def test_manifest_rejects_duplicate_dependencies() -> None: + dependency = ExtensionDependency( + kind=DependencyKind.EXTENSION, + name="acme.base", + ) + + with pytest.raises(ValueError, match="duplicate dependencies"): + ExtensionManifest( + id="duplicate", + name="Duplicate", + version="1.0.0", + runtime=ExtensionRuntime.DECLARATIVE, + dependencies=(dependency, dependency), + ) + + +def test_dependency_optional_must_be_boolean() -> None: + with pytest.raises(TypeError, match="optional must be a boolean"): + ExtensionDependency( + kind=DependencyKind.EXTENSION, + name="acme.base", + optional="false", # type: ignore[arg-type] + ) diff --git a/tests/extensions/test_market.py b/tests/extensions/test_market.py index f03c3664e..5884bad5c 100644 --- a/tests/extensions/test_market.py +++ b/tests/extensions/test_market.py @@ -57,3 +57,14 @@ def test_market_ignores_fuzzy_npm_results(monkeypatch) -> None: ) assert ExtensionMarketplace().search(ecosystem="pi") == () + + +def test_market_rejects_invalid_npm_json(monkeypatch) -> None: + monkeypatch.setattr( + subprocess, + "run", + lambda *_args, **_kwargs: subprocess.CompletedProcess([], 0, "not json", ""), + ) + + with pytest.raises(RuntimeError, match="invalid marketplace response"): + ExtensionMarketplace().search(ecosystem="pi") diff --git a/tests/extensions/test_native.py b/tests/extensions/test_native.py index ad576f54c..ebe55b6cc 100644 --- a/tests/extensions/test_native.py +++ b/tests/extensions/test_native.py @@ -5,7 +5,12 @@ from nanobot.agent.tools.base import Tool from nanobot.agent.tools.registry import ToolRegistry from nanobot.command.router import CommandRouter from nanobot.config.schema import Config, MCPServerConfig -from nanobot.extensions import ContributionKind, discover_native_extensions +from nanobot.extensions import ( + ContributionKind, + ExtensionRuntime, + discover_native_extensions, +) +from nanobot.extensions.native import _python_dependencies class _Tool(Tool): @@ -58,6 +63,7 @@ def test_native_inventory_preserves_runtime_ownership(tmp_path) -> None: contribution.kind for contribution in extensions["acme.extension"].manifest.contributions } == {ContributionKind.TOOL, ContributionKind.COMMAND} + assert extensions["acme.extension"].manifest.runtime is ExtensionRuntime.DECLARATIVE assert "nanobot.mcp.docs" in extensions @@ -93,4 +99,50 @@ def test_native_inventory_projects_workspace_skill(tmp_path) -> None: item for item in result.candidates if item.manifest.id == "nanobot.skill.release" ) assert skill.scope.name == "WORKSPACE" + assert skill.manifest.runtime is ExtensionRuntime.DECLARATIVE assert skill.manifest.contributions[0].description == "Prepare a release." + + +def test_native_inventory_collapses_command_routing_tiers() -> None: + commands = CommandRouter() + + async def _handler(_ctx): + return None + + commands.priority("/status", _handler) + commands.exact("/status", _handler) + commands.exact("/model", _handler) + commands.prefix("/model ", _handler) + + with ( + patch("nanobot.channels.registry.discover_plugins", return_value={}), + patch("nanobot.providers.registry.PROVIDERS", ()), + patch("nanobot.audio.transcription_registry.TRANSCRIPTION_PROVIDERS", ()), + patch( + "nanobot.providers.image_generation.image_gen_provider_names", + return_value=(), + ), + ): + result = discover_native_extensions(Config(), commands=commands) + + core = next( + item for item in result.candidates if item.manifest.id == "nanobot.core" + ) + assert [ + contribution.name for contribution in core.manifest.contributions + ] == ["model", "status"] + + +def test_python_dependencies_project_distribution_identity_and_active_markers() -> None: + dependencies = _python_dependencies( + ( + "httpx[http2]>=0.27", + "win32-setctime>=1; sys_platform == 'win32'", + "packaging>=24; python_version >= '3.11'", + ) + ) + + assert [(item.name, item.specifier) for item in dependencies] == [ + ("httpx", ">=0.27"), + ("packaging", ">=24"), + ] diff --git a/tests/extensions/test_node_compatibility.py b/tests/extensions/test_node_compatibility.py index 50e6f0561..c1b23002f 100644 --- a/tests/extensions/test_node_compatibility.py +++ b/tests/extensions/test_node_compatibility.py @@ -1,3 +1,4 @@ +import asyncio from pathlib import Path import pytest @@ -159,3 +160,183 @@ async def test_pi_package_loads_every_declared_entry(tmp_path: Path) -> None: } == {"first", "second"} finally: await host.close() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("kind", ["tool", "command"]) +async def test_pi_package_rejects_duplicate_registrations( + tmp_path: Path, + kind: str, +) -> None: + registration = ( + """ + pi.registerTool({ + name: "duplicate", + description: "Duplicate", + parameters: { type: "object", properties: {} }, + async execute() { return "ok"; } + }); + """ + if kind == "tool" + else 'pi.registerCommand("duplicate", { handler: async () => "ok" });' + ) + first = _write( + tmp_path / "first.mjs", + f"export default function (pi) {{ {registration} }}", + ) + second = _write( + tmp_path / "second.mjs", + f"export default function (pi) {{ {registration} }}", + ) + host = NodeSidecar() + try: + with pytest.raises(RuntimeError, match=f"{kind} 'duplicate' is already registered"): + await host.load( + runtime="pi", + entries=(first, second), + root=tmp_path, + extension_id="test.duplicate", + name="Duplicate registrations", + version="1.0.0", + ) + finally: + await host.close() + + +@pytest.mark.asyncio +async def test_sidecar_accepts_tool_results_larger_than_default_stream_limit( + tmp_path: Path, +) -> None: + entry = _write( + tmp_path / "large-result.mjs", + """ + export default function (pi) { + pi.registerTool({ + name: "large_result", + description: "Return a large result", + parameters: { type: "object", properties: {} }, + async execute() { return "x".repeat(128 * 1024); } + }); + } + """, + ) + host = NodeSidecar() + try: + result = await host.load( + runtime="pi", + entries=(entry,), + root=tmp_path, + extension_id="test.large", + name="Large result", + version="1.0.0", + ) + extension = CompatibleExtension( + host=host, + runtime="pi", + owner="test.large", + result=result, + ) + + output = await extension.tools[0].execute() + + assert len(output) == 128 * 1024 + finally: + await host.close() + + +@pytest.mark.asyncio +async def test_sidecar_terminates_extension_after_request_timeout( + tmp_path: Path, +) -> None: + entry = _write( + tmp_path / "timeout.mjs", + """ + export default function (pi) { + pi.registerTool({ + name: "never_finishes", + description: "Never finishes", + parameters: { type: "object", properties: {} }, + async execute() { await new Promise(() => {}); } + }); + } + """, + ) + host = NodeSidecar() + try: + await host.load( + runtime="pi", + entries=(entry,), + root=tmp_path, + extension_id="test.timeout", + name="Timeout", + version="1.0.0", + ) + + with pytest.raises(RuntimeError, match="timed out"): + await host.request( + "extension.call", + { + "kind": "tool", + "name": "never_finishes", + "callId": "test", + "input": {}, + }, + timeout=0.05, + ) + assert not host.running + finally: + await host.close() + + +@pytest.mark.asyncio +async def test_cancelled_sidecar_call_does_not_fail_the_next_request( + tmp_path: Path, +) -> None: + entry = _write( + tmp_path / "cancel.mjs", + """ + export default function (pi) { + pi.registerTool({ + name: "slow", + description: "Finish later", + parameters: { type: "object", properties: {} }, + async execute() { + await new Promise((resolve) => setTimeout(resolve, 50)); + return "slow"; + } + }); + pi.registerTool({ + name: "fast", + description: "Finish immediately", + parameters: { type: "object", properties: {} }, + async execute() { return "fast"; } + }); + } + """, + ) + host = NodeSidecar() + try: + result = await host.load( + runtime="pi", + entries=(entry,), + root=tmp_path, + extension_id="test.cancel", + name="Cancellation", + version="1.0.0", + ) + extension = CompatibleExtension( + host=host, + runtime="pi", + owner="test.cancel", + result=result, + ) + + slow = asyncio.create_task(extension.tools[0].execute()) + await asyncio.sleep(0.01) + slow.cancel() + with pytest.raises(asyncio.CancelledError): + await slow + + assert await extension.tools[1].execute() == "fast" + finally: + await host.close() diff --git a/tests/extensions/test_package_adapter.py b/tests/extensions/test_package_adapter.py index 441159622..bd5b7da7b 100644 --- a/tests/extensions/test_package_adapter.py +++ b/tests/extensions/test_package_adapter.py @@ -27,6 +27,10 @@ def test_adapts_pi_package_metadata(tmp_path: Path) -> None: assert result.manifest.id == "pi.acme.pi-tools" assert result.manifest.runtime is ExtensionRuntime.PI assert result.manifest.activation_entries == ("./index.ts", "./review.ts") + assert len(result.manifest.dependencies) == 1 + assert result.manifest.dependencies[0].name == "jiti" + assert result.manifest.dependencies[0].specifier == "^2.4.2" + assert result.manifest.permissions[0].name == "runtime.node" def test_adapts_openclaw_contracts_without_loading_code(tmp_path: Path) -> None: @@ -70,3 +74,4 @@ def test_adapts_openclaw_contracts_without_loading_code(tmp_path: Path) -> None: (ContributionKind.WEB_SEARCH_PROVIDER, "private-search"), } assert "speechProviders" in result.diagnostics[0] + assert result.manifest.permissions[0].name == "runtime.node" diff --git a/tests/extensions/test_preflight.py b/tests/extensions/test_preflight.py index 35ff5bd22..57e4f36ee 100644 --- a/tests/extensions/test_preflight.py +++ b/tests/extensions/test_preflight.py @@ -65,3 +65,22 @@ def test_installed_npm_dependency_satisfies_preflight(tmp_path: Path) -> None: assert candidates[0].enabled assert diagnostics == () + + +def test_npm_dist_tag_is_left_to_npm_resolution(tmp_path: Path) -> None: + package = tmp_path / "node_modules" / "openclaw" + package.mkdir(parents=True) + (package / "package.json").write_text('{"version":"2026.7.1"}') + candidate = _candidate( + ExtensionDependency( + kind=DependencyKind.NPM, + name="openclaw", + specifier="latest", + ), + location=tmp_path, + ) + + candidates, diagnostics = evaluate_dependencies((candidate,)) + + assert candidates[0].enabled + assert diagnostics == () diff --git a/tests/extensions/test_protocol.py b/tests/extensions/test_protocol.py new file mode 100644 index 000000000..bddc969b6 --- /dev/null +++ b/tests/extensions/test_protocol.py @@ -0,0 +1,39 @@ +import pytest + +from nanobot.extensions.protocol import NodeProtocolError, NodeRegistration + + +@pytest.mark.parametrize("field", ["kind", "name"]) +def test_node_registration_rejects_blank_identity(field: str) -> None: + value = {"kind": "tool", "name": "sample"} + value[field] = " " + + with pytest.raises(NodeProtocolError, match="non-empty"): + NodeRegistration.from_mapping(value) + + +@pytest.mark.parametrize( + "kind", + ("tool", "command"), +) +@pytest.mark.parametrize( + "name", + ("has space", "has.dot", "x" * 65), +) +def test_node_registration_rejects_unsafe_callable_names( + kind: str, + name: str, +) -> None: + with pytest.raises(NodeProtocolError, match=f"{kind} name"): + NodeRegistration.from_mapping({"kind": kind, "name": name}) + + +def test_node_registration_rejects_non_object_tool_schema() -> None: + with pytest.raises(NodeProtocolError, match="object schema"): + NodeRegistration.from_mapping( + { + "kind": "tool", + "name": "sample", + "schema": {"type": "string"}, + } + ) diff --git a/tests/extensions/test_registry.py b/tests/extensions/test_registry.py index 44e808671..08e250fd8 100644 --- a/tests/extensions/test_registry.py +++ b/tests/extensions/test_registry.py @@ -1,7 +1,9 @@ from nanobot.extensions import ( ContributionKind, + DependencyKind, ExtensionCandidate, ExtensionContribution, + ExtensionDependency, ExtensionManifest, ExtensionPermission, ExtensionPolicy, @@ -16,14 +18,13 @@ def _candidate( *, scope: ExtensionScope, contribution_name: str = "", - replaces: tuple[str, ...] = (), trusted: bool = True, + dependencies: tuple[ExtensionDependency, ...] = (), ) -> ExtensionCandidate: contributions = ( ExtensionContribution( kind=ContributionKind.TOOL, name=contribution_name, - replaces=replaces, ), ) if contribution_name else () return ExtensionCandidate( @@ -33,6 +34,7 @@ def _candidate( version="1.0.0", runtime=ExtensionRuntime.PYTHON, contributions=contributions, + dependencies=dependencies, ), scope=scope, trusted=trusted, @@ -95,6 +97,28 @@ def test_untrusted_external_extension_is_visible_to_discovery_but_not_active() - assert snapshot.contributions == () +def test_invalid_package_integrity_cannot_be_overridden_by_trust() -> None: + registry = ExtensionRegistry() + candidate = _candidate( + "tampered", + scope=ExtensionScope.USER, + contribution_name="unsafe_tool", + ) + registry.register( + ExtensionCandidate( + manifest=candidate.manifest, + scope=candidate.scope, + trusted=True, + integrity_valid=False, + ) + ) + + snapshot = registry.snapshot() + + assert snapshot.extensions == () + assert snapshot.contributions == () + + def test_external_extension_requires_every_requested_permission() -> None: candidate = ExtensionCandidate( manifest=ExtensionManifest( @@ -147,7 +171,7 @@ def test_conflicting_contribution_does_not_silently_replace_owner() -> None: assert snapshot.diagnostics[0].code == "contribution_conflict" -def test_explicit_higher_scope_replacement_takes_ownership() -> None: +def test_higher_scope_extension_cannot_replace_another_owner() -> None: registry = ExtensionRegistry() registry.register( _candidate( @@ -161,11 +185,137 @@ def test_explicit_higher_scope_replacement_takes_ownership() -> None: "replacement", scope=ExtensionScope.WORKSPACE, contribution_name="shell", - replaces=("core",), ) ) snapshot = registry.snapshot() - assert snapshot.contributions[0].owner.manifest.id == "replacement" - assert snapshot.diagnostics == () + assert snapshot.contributions[0].owner.manifest.id == "core" + assert snapshot.diagnostics[0].code == "contribution_conflict" + + +def test_extension_dependency_must_be_active_and_starts_first() -> None: + dependency = ExtensionDependency( + kind=DependencyKind.EXTENSION, + name="z-base", + ) + registry = ExtensionRegistry() + registry.register( + _candidate( + "a-dependent", + scope=ExtensionScope.USER, + dependencies=(dependency,), + ) + ) + registry.register(_candidate("z-base", scope=ExtensionScope.USER)) + + snapshot = registry.snapshot() + + assert [item.manifest.id for item in snapshot.extensions] == [ + "z-base", + "a-dependent", + ] + + +def test_inactive_extension_cannot_satisfy_dependency() -> None: + dependency = ExtensionDependency( + kind=DependencyKind.EXTENSION, + name="base", + ) + registry = ExtensionRegistry() + registry.register( + _candidate( + "dependent", + scope=ExtensionScope.USER, + dependencies=(dependency,), + ) + ) + registry.register( + _candidate("base", scope=ExtensionScope.USER, trusted=False) + ) + + snapshot = registry.snapshot() + + assert snapshot.extensions == () + assert any( + item.code == "dependency_missing" + and item.extension_id == "dependent" + for item in snapshot.diagnostics + ) + + +def test_extension_dependency_cycle_is_rejected() -> None: + registry = ExtensionRegistry() + registry.register( + _candidate( + "first", + scope=ExtensionScope.USER, + dependencies=( + ExtensionDependency( + kind=DependencyKind.EXTENSION, + name="second", + ), + ), + ) + ) + registry.register( + _candidate( + "second", + scope=ExtensionScope.USER, + dependencies=( + ExtensionDependency( + kind=DependencyKind.EXTENSION, + name="first", + ), + ), + ) + ) + + snapshot = registry.snapshot() + + assert snapshot.extensions == () + assert { + item.extension_id + for item in snapshot.diagnostics + if item.code == "dependency_cycle" + } == {"first", "second"} + + +def test_optional_extension_dependency_cycle_is_allowed() -> None: + registry = ExtensionRegistry() + registry.register( + _candidate( + "first", + scope=ExtensionScope.USER, + dependencies=( + ExtensionDependency( + kind=DependencyKind.EXTENSION, + name="second", + optional=True, + ), + ), + ) + ) + registry.register( + _candidate( + "second", + scope=ExtensionScope.USER, + dependencies=( + ExtensionDependency( + kind=DependencyKind.EXTENSION, + name="first", + optional=True, + ), + ), + ) + ) + + snapshot = registry.snapshot() + + assert {item.manifest.id for item in snapshot.extensions} == { + "first", + "second", + } + assert not any( + item.code == "dependency_cycle" for item in snapshot.diagnostics + ) diff --git a/tests/extensions/test_runtime.py b/tests/extensions/test_runtime.py index d37ed6d38..f498159ae 100644 --- a/tests/extensions/test_runtime.py +++ b/tests/extensions/test_runtime.py @@ -1,4 +1,5 @@ from pathlib import Path +from types import SimpleNamespace import pytest @@ -6,7 +7,9 @@ from nanobot.agent.tools.registry import ToolRegistry from nanobot.command.router import CommandRouter from nanobot.config.schema import Config from nanobot.extensions import ( + DependencyKind, ExtensionCandidate, + ExtensionDependency, ExtensionManifest, ExtensionRuntime, ExtensionRuntimeManager, @@ -117,3 +120,272 @@ async def test_runtime_rolls_back_partial_registration(tmp_path: Path) -> None: assert result.extensions == () assert result.diagnostics[0].code == "activation_failed" assert commands.owner("exact", "/duplicate") == "nanobot.core" + + +@pytest.mark.asyncio +async def test_python_runtime_reloads_updated_source(tmp_path: Path) -> None: + source = tmp_path / "plugin.py" + candidate = ExtensionCandidate( + ExtensionManifest( + id="test.python", + name="Python", + version="1.0.0", + runtime=ExtensionRuntime.PYTHON, + entry="plugin:register", + ), + ExtensionScope.USER, + location=tmp_path, + trusted=True, + ) + tools = ToolRegistry() + + def write_plugin(result: str) -> None: + source.write_text( + f""" +from nanobot.agent.tools.base import Tool + +class ReloadTool(Tool): + @property + def name(self): + return "reload_test" + + @property + def description(self): + return "Reload test" + + @property + def parameters(self): + return {{"type": "object", "properties": {{}}}} + + async def execute(self): + return "{result}" + +def register(api): + api.register_tool(ReloadTool()) +""" + ) + + write_plugin("first") + first = ExtensionRuntimeManager( + tools=tools, + commands=CommandRouter(), + config=Config(), + ) + first_result = await first.activate(_snapshot(candidate)) + assert first_result.diagnostics == () + assert await tools.get("reload_test").execute() == "first" + await first.close() + + write_plugin("later") + second = ExtensionRuntimeManager( + tools=tools, + commands=CommandRouter(), + config=Config(), + ) + second_result = await second.activate(_snapshot(candidate)) + assert second_result.diagnostics == () + assert await tools.get("reload_test").execute() == "later" + await second.close() + + +@pytest.mark.asyncio +async def test_python_runtime_accepts_single_entries_form(tmp_path: Path) -> None: + (tmp_path / "plugin.py").write_text( + """ +def register(_api): + pass +""" + ) + candidate = ExtensionCandidate( + ExtensionManifest( + id="test.python-entries", + name="Python entries", + version="1.0.0", + runtime=ExtensionRuntime.PYTHON, + entries=("plugin:register",), + ), + ExtensionScope.USER, + location=tmp_path, + trusted=True, + ) + manager = ExtensionRuntimeManager( + tools=ToolRegistry(), + commands=CommandRouter(), + config=Config(), + ) + + result = await manager.activate(_snapshot(candidate)) + + assert result.diagnostics == () + await manager.close() + + +@pytest.mark.asyncio +async def test_python_runtime_rejects_module_outside_package(tmp_path: Path) -> None: + candidate = ExtensionCandidate( + ExtensionManifest( + id="test.collision", + name="Collision", + version="1.0.0", + runtime=ExtensionRuntime.PYTHON, + entry="json:register", + ), + ExtensionScope.USER, + location=tmp_path, + trusted=True, + ) + manager = ExtensionRuntimeManager( + tools=ToolRegistry(), + commands=CommandRouter(), + config=Config(), + ) + + result = await manager.activate(_snapshot(candidate)) + + assert result.extensions == () + assert result.diagnostics[0].code == "activation_failed" + assert "conflicts with loaded module" in result.diagnostics[0].message + + +@pytest.mark.asyncio +async def test_python_hook_factory_is_removed_without_clearing_core_hooks( + tmp_path: Path, +) -> None: + (tmp_path / "plugin.py").write_text( + """ +from nanobot.agent.hook import AgentHook + +def extension_hook(_context): + return AgentHook() + +def register(api): + api.register_hook_factory(extension_hook) +""" + ) + candidate = ExtensionCandidate( + ExtensionManifest( + id="test.hook", + name="Hook", + version="1.0.0", + runtime=ExtensionRuntime.PYTHON, + entry="plugin:register", + ), + ExtensionScope.USER, + location=tmp_path, + trusted=True, + ) + + def core_hook(_context): + return None + + hooks = [core_hook] + manager = ExtensionRuntimeManager( + tools=ToolRegistry(), + commands=CommandRouter(), + config=Config(), + hook_factories=hooks, + ) + + result = await manager.activate(_snapshot(candidate)) + + assert result.diagnostics == () + assert len(hooks) == 2 + await manager.close() + assert hooks == [core_hook] + + +@pytest.mark.asyncio +async def test_python_runtime_cannot_overwrite_core_tool(tmp_path: Path) -> None: + (tmp_path / "plugin.py").write_text( + """ +from nanobot.agent.tools.base import Tool + +class DuplicateTool(Tool): + name = "duplicate" + description = "Duplicate" + parameters = {"type": "object", "properties": {}} + + async def execute(self): + return "extension" + +def register(api): + api.register_tool(DuplicateTool()) +""" + ) + candidate = ExtensionCandidate( + ExtensionManifest( + id="test.overwrite", + name="Overwrite", + version="1.0.0", + runtime=ExtensionRuntime.PYTHON, + entry="plugin:register", + ), + ExtensionScope.USER, + location=tmp_path, + trusted=True, + ) + core_tool = SimpleNamespace(name="duplicate") + tools = ToolRegistry() + tools.register(core_tool) + manager = ExtensionRuntimeManager( + tools=tools, + commands=CommandRouter(), + config=Config(), + ) + + result = await manager.activate(_snapshot(candidate)) + + assert result.extensions == () + assert result.diagnostics[0].code == "activation_failed" + assert tools.get("duplicate") is core_tool + assert tools.owner("duplicate") == "nanobot.core" + + +@pytest.mark.asyncio +async def test_failed_extension_prevents_dependent_activation(tmp_path: Path) -> None: + base_root = tmp_path / "base" + dependent_root = tmp_path / "dependent" + base_root.mkdir() + dependent_root.mkdir() + base = ExtensionCandidate( + ExtensionManifest( + id="base", + name="Base", + version="1.0.0", + runtime=ExtensionRuntime.PYTHON, + entry="missing:register", + ), + ExtensionScope.USER, + location=base_root, + trusted=True, + ) + dependent = ExtensionCandidate( + ExtensionManifest( + id="dependent", + name="Dependent", + version="1.0.0", + runtime=ExtensionRuntime.DECLARATIVE, + dependencies=( + ExtensionDependency( + kind=DependencyKind.EXTENSION, + name="base", + ), + ), + ), + ExtensionScope.USER, + location=dependent_root, + trusted=True, + ) + manager = ExtensionRuntimeManager( + tools=ToolRegistry(), + commands=CommandRouter(), + config=Config(), + ) + + result = await manager.activate(ExtensionSnapshot((base, dependent), (), ())) + + assert result.extensions == () + assert [item.code for item in result.diagnostics] == [ + "activation_failed", + "dependency_activation_failed", + ] diff --git a/tests/extensions/test_service.py b/tests/extensions/test_service.py index ba5d351b6..53a468786 100644 --- a/tests/extensions/test_service.py +++ b/tests/extensions/test_service.py @@ -1,5 +1,14 @@ from pathlib import Path +from types import SimpleNamespace +from nanobot.extensions import ( + ExtensionCandidate, + ExtensionDiagnostic, + ExtensionManifest, + ExtensionRuntime, + ExtensionScope, + ExtensionSnapshot, +) from nanobot.extensions.service import ExtensionService from nanobot.extensions.store import ExtensionStore @@ -29,6 +38,7 @@ async def test_service_installs_untrusted_extension_and_reports_status( assert installed["record"]["trusted"] is False assert status["extensions"][0]["id"] == "sample" assert status["extensions"][0]["active"] is False + assert status["extensions"][0]["managed_by_store"] is True async def test_service_updates_policy_and_uninstalls(tmp_path: Path) -> None: @@ -56,3 +66,85 @@ async def test_service_updates_policy_and_uninstalls(tmp_path: Path) -> None: assert trusted["record"]["trusted"] is True assert removed == {"removed": "sample"} assert (await service.status())["extensions"] == [] + + +async def test_service_reports_active_only_after_runtime_activation( + tmp_path: Path, +) -> None: + candidate = ExtensionCandidate( + manifest=ExtensionManifest( + id="broken", + name="Broken", + version="1.0.0", + runtime=ExtensionRuntime.PYTHON, + entry="missing:register", + ), + scope=ExtensionScope.USER, + location=tmp_path, + trusted=True, + ) + snapshot = ExtensionSnapshot((candidate,), (), ()) + host = SimpleNamespace( + snapshot=SimpleNamespace( + catalog=SimpleNamespace( + candidates=(candidate,), + diagnostics=(), + snapshot=snapshot, + ), + activation=SimpleNamespace( + extensions=(), + diagnostics=( + ExtensionDiagnostic( + code="activation_failed", + extension_id="broken", + message="missing module", + ), + ), + ), + ) + ) + service = ExtensionService( + host=host, + store=ExtensionStore(tmp_path / "installed"), + ) + + status = await service.status() + + assert status["extensions"][0]["active"] is False + assert status["extensions"][0]["managed_by_store"] is False + assert status["diagnostics"][0]["code"] == "activation_failed" + + +async def test_service_reports_projected_native_capability_as_active( + tmp_path: Path, +) -> None: + candidate = ExtensionCandidate( + manifest=ExtensionManifest( + id="nanobot.core", + name="nanobot core", + version="1.0.0", + runtime=ExtensionRuntime.DECLARATIVE, + ), + scope=ExtensionScope.BUILTIN, + trusted=True, + ) + snapshot = ExtensionSnapshot((candidate,), (), ()) + host = SimpleNamespace( + snapshot=SimpleNamespace( + catalog=SimpleNamespace( + candidates=(candidate,), + diagnostics=(), + snapshot=snapshot, + ), + activation=SimpleNamespace(extensions=(), diagnostics=()), + ) + ) + service = ExtensionService( + host=host, + store=ExtensionStore(tmp_path / "installed"), + ) + + status = await service.status() + + assert status["extensions"][0]["active"] is True + assert status["extensions"][0]["managed_by_store"] is False diff --git a/tests/extensions/test_store.py b/tests/extensions/test_store.py index 3561a85bb..4bd420bc4 100644 --- a/tests/extensions/test_store.py +++ b/tests/extensions/test_store.py @@ -1,4 +1,6 @@ import json +import tarfile +from io import BytesIO from pathlib import Path from unittest.mock import patch @@ -8,12 +10,14 @@ from nanobot.extensions import ( DependencyKind, ExtensionDependency, ExtensionManifest, + ExtensionPermission, ExtensionRuntime, ExtensionSourceKind, ExtensionStore, InstalledExtension, dump_manifest, ) +from nanobot.extensions.store import _extract_tar def _pi_package(root: Path, *, version: str = "1.0.0") -> Path: @@ -28,6 +32,20 @@ def _pi_package(root: Path, *, version: str = "1.0.0") -> Path: } ) ) + dump_manifest( + ExtensionManifest( + id="pi.store-test", + name="store-test", + version=version, + runtime=ExtensionRuntime.PI, + entry="./index.mjs", + permissions=( + ExtensionPermission(name="network"), + ExtensionPermission(name="filesystem.read"), + ), + ), + root / "nanobot.extension.json", + ) return root @@ -68,6 +86,95 @@ def test_registry_permissions_must_be_a_string_array() -> None: ) +@pytest.mark.parametrize( + ("field", "value", "message"), + [ + ("source", 3, "source must be a string"), + ("integrity", "sha256:test", "sha256 digest"), + ( + "granted_permissions", + ["network", "network"], + "cannot contain duplicates", + ), + ], +) +def test_registry_record_rejects_ambiguous_metadata( + field: str, + value: object, + message: str, +) -> None: + payload = { + "id": "sample", + "version": "1.0.0", + "source": "npm", + "source_ref": "sample", + "integrity": f"sha256:{'0' * 64}", + "installed_at": "2026-01-01T00:00:00Z", + } + payload[field] = value + + with pytest.raises(ValueError, match=message): + InstalledExtension.from_mapping(payload) + + +def test_corrupt_registry_is_diagnostic_and_cannot_be_overwritten( + tmp_path: Path, +) -> None: + source = _pi_package(tmp_path / "source") + store = ExtensionStore(tmp_path / "extensions") + installed = store.install_local(source) + original = "{broken" + store.registry_path.write_text(original) + + discovery = store.discover() + + assert discovery.diagnostics[0].code == "invalid_extension_registry" + assert not discovery.candidates[0].trusted + with pytest.raises(ValueError, match="invalid extension registry"): + store.set_trusted(installed.record.id, True) + assert store.registry_path.read_text() == original + + +@pytest.mark.parametrize( + "payload", + [ + {}, + {"version": 2, "extensions": []}, + {"version": 1, "extensions": {}}, + { + "version": 1, + "extensions": [ + { + "id": "duplicate", + "version": "1.0.0", + "source": "npm", + "source_ref": "duplicate", + "integrity": "sha256:test", + "installed_at": "2026-01-01T00:00:00Z", + }, + { + "id": "duplicate", + "version": "1.0.0", + "source": "npm", + "source_ref": "duplicate", + "integrity": "sha256:test", + "installed_at": "2026-01-01T00:00:00Z", + }, + ], + }, + ], +) +def test_store_rejects_invalid_registry_schema( + tmp_path: Path, + payload: object, +) -> None: + store = ExtensionStore(tmp_path / "extensions") + store.registry_path.write_text(json.dumps(payload)) + + with pytest.raises(ValueError, match="invalid extension registry"): + store.records(strict=True) + + def test_store_updates_and_uninstalls_atomically(tmp_path: Path) -> None: source = _pi_package(tmp_path / "source") store = ExtensionStore(tmp_path / "extensions") @@ -76,16 +183,61 @@ def test_store_updates_and_uninstalls_atomically(tmp_path: Path) -> None: payload = json.loads(package_json.read_text()) payload["version"] = "2.0.0" package_json.write_text(json.dumps(payload)) + manifest_path = source / "nanobot.extension.json" + manifest = json.loads(manifest_path.read_text()) + manifest["version"] = "2.0.0" + manifest_path.write_text(json.dumps(manifest)) second = store.install_local(source) assert second.record.version == "2.0.0" - assert second.record.trusted + assert not second.record.trusted store.uninstall(first.record.id) assert store.records() == {} assert not (store.root / first.record.id).exists() +def test_store_preserves_trust_only_for_identical_reinstall(tmp_path: Path) -> None: + source = _pi_package(tmp_path / "source") + store = ExtensionStore(tmp_path / "extensions") + first = store.install_local(source, trusted=True) + store.set_permissions(first.record.id, {"network"}) + + second = store.install_local(source) + + assert second.record.trusted + assert second.record.granted_permissions == ("network",) + + +def test_store_revokes_effective_trust_when_package_changes_on_disk( + tmp_path: Path, +) -> None: + source = _pi_package(tmp_path / "source") + store = ExtensionStore(tmp_path / "extensions") + installed = store.install_local(source, trusted=True) + target = store.root / installed.record.id + (target / "index.mjs").write_text("export default function changed() {}") + + discovery = store.discover() + + assert not discovery.candidates[0].trusted + assert not discovery.candidates[0].integrity_valid + assert any( + item.code == "extension_integrity_mismatch" + for item in discovery.diagnostics + ) + + +def test_store_ignores_interrupted_transaction_directories(tmp_path: Path) -> None: + store = ExtensionStore(tmp_path / "extensions") + hidden = _pi_package(store.root / ".install-interrupted") + + discovery = store.discover() + + assert hidden.is_dir() + assert discovery.candidates == () + + def test_store_rejects_symlinked_package_content(tmp_path: Path) -> None: source = _pi_package(tmp_path / "source") (source / "outside").symlink_to(tmp_path) @@ -99,6 +251,73 @@ def test_store_rejects_symlinked_package_content(tmp_path: Path) -> None: raise AssertionError("symlinked package was accepted") +def test_store_accepts_internal_node_dependency_links(tmp_path: Path) -> None: + source = _pi_package(tmp_path / "source") + store = ExtensionStore(tmp_path / "extensions") + + def install_dependencies(root: Path, _manifest) -> None: + executable = root / "node_modules" / "tool" / "cli.js" + executable.parent.mkdir(parents=True) + executable.write_text("export {}") + bin_dir = root / "node_modules" / ".bin" + bin_dir.mkdir() + (bin_dir / "tool").symlink_to("../tool/cli.js") + + with patch( + "nanobot.extensions.store._install_node_dependencies", + side_effect=install_dependencies, + ): + installed = store.install_local(source) + + candidate = store.discover().candidates[0] + assert candidate.manifest.id == installed.record.id + assert candidate.integrity_valid + + +def test_store_rejects_node_dependency_links_outside_package(tmp_path: Path) -> None: + source = _pi_package(tmp_path / "source") + outside = tmp_path / "outside.js" + outside.write_text("export {}") + store = ExtensionStore(tmp_path / "extensions") + + def install_dependencies(root: Path, _manifest) -> None: + bin_dir = root / "node_modules" / ".bin" + bin_dir.mkdir(parents=True) + (bin_dir / "tool").symlink_to(outside) + + with patch( + "nanobot.extensions.store._install_node_dependencies", + side_effect=install_dependencies, + ): + with pytest.raises(ValueError, match="symlink"): + store.install_local(source) + + +def test_store_only_grants_permissions_requested_by_current_manifest( + tmp_path: Path, +) -> None: + source = tmp_path / "source" + source.mkdir() + dump_manifest( + ExtensionManifest( + id="permission-test", + name="Permission test", + version="1.0.0", + runtime=ExtensionRuntime.DECLARATIVE, + permissions=(ExtensionPermission(name="network.http"),), + ), + source / "nanobot.extension.json", + ) + store = ExtensionStore(tmp_path / "extensions") + store.install_local(source) + + granted = store.set_permissions("permission-test", {"network.http"}) + + assert granted.granted_permissions == ("network.http",) + with pytest.raises(ValueError, match="not requested"): + store.set_permissions("permission-test", {"workspace.write"}) + + def test_store_restores_previous_package_when_registry_write_fails( tmp_path: Path, ) -> None: @@ -122,6 +341,57 @@ def test_store_restores_previous_package_when_registry_write_fails( assert restored["version"] == "1.0.0" +def test_store_preserves_previous_package_when_backup_rename_fails( + tmp_path: Path, +) -> None: + source = _pi_package(tmp_path / "source") + store = ExtensionStore(tmp_path / "extensions") + first = store.install_local(source) + target = store.root / first.record.id + original_rename = Path.rename + + def fail_backup_rename(path: Path, destination: Path) -> Path: + if path == target: + raise OSError("rename failed") + return original_rename(path, destination) + + with patch.object(Path, "rename", fail_backup_rename): + with pytest.raises(OSError, match="rename failed"): + store.install_local(source) + + assert target.is_dir() + assert first.record.id in store.records(strict=True) + + +def test_store_removes_first_package_when_registry_write_fails( + tmp_path: Path, +) -> None: + source = _pi_package(tmp_path / "source") + store = ExtensionStore(tmp_path / "extensions") + + with patch.object(store, "_write_records", side_effect=OSError("disk full")): + with pytest.raises(OSError, match="disk full"): + store.install_local(source) + + assert not (store.root / "pi.store-test").exists() + + +def test_store_restores_package_when_uninstall_registry_write_fails( + tmp_path: Path, +) -> None: + source = _pi_package(tmp_path / "source") + store = ExtensionStore(tmp_path / "extensions") + installed = store.install_local(source) + target = store.root / installed.record.id + + with patch.object(store, "_write_records", side_effect=OSError("disk full")): + with pytest.raises(OSError, match="disk full"): + store.uninstall(installed.record.id) + + assert target.exists() + assert installed.record.id in store.records() + + def test_store_installs_declared_npm_runtime_dependency(tmp_path: Path) -> None: source = tmp_path / "source" source.mkdir() @@ -145,10 +415,189 @@ def test_store_installs_declared_npm_runtime_dependency(tmp_path: Path) -> None: source / "nanobot.extension.json", ) store = ExtensionStore(tmp_path / "extensions") + runtime_package: dict[str, object] = {} - with patch("nanobot.extensions.store._run") as run: + def capture_runtime_package( + _command: list[str], + *, + cwd: Path | None = None, + ) -> str: + assert cwd is not None + runtime_package.update(json.loads((cwd / "package.json").read_text())) + return "" + + with patch( + "nanobot.extensions.store._run", + side_effect=capture_runtime_package, + ) as run: store.install_local(source) command = run.call_args.args[0] - assert "--save-prod" in command - assert "openclaw@2026.7.1" in command + assert "--omit=dev" in command + assert "--package-lock=false" in command + assert runtime_package["dependencies"] == {"openclaw": "2026.7.1"} + assert json.loads((store.root / "openclaw.test" / "package.json").read_text()) == {} + + +def test_git_install_fetches_branch_tag_or_commit_ref(tmp_path: Path) -> None: + store = ExtensionStore(tmp_path / "extensions") + result = object() + + with ( + patch("nanobot.extensions.store._run") as run, + patch.object(store, "_install_from_directory", return_value=result), + ): + installed = store.install_git( + "https://example.com/acme/extension.git", + ref="abc123", + ) + + assert installed is result + commands = [call.args[0] for call in run.call_args_list] + assert commands[0][:4] == ["git", "clone", "--filter=blob:none", "--no-checkout"] + assert commands[1][-5:] == ["--depth", "1", "--", "origin", "abc123"] + assert commands[2][-3:] == ["checkout", "--detach", "FETCH_HEAD"] + + +@pytest.mark.parametrize( + "url", + [ + "ext::sh -c touch /tmp/owned", + "file:///tmp/extension", + "/tmp/extension", + "https://token@example.com/acme/extension.git", + "https://example.com/acme/extension.git?token=secret", + "git@example.com:acme/extension.git#main", + ], +) +def test_git_install_rejects_unsafe_or_local_sources( + tmp_path: Path, + url: str, +) -> None: + store = ExtensionStore(tmp_path / "extensions") + + with patch("nanobot.extensions.store._run") as run: + with pytest.raises(ValueError): + store.install_git(url) + + run.assert_not_called() + + +@pytest.mark.parametrize( + "spec", + ( + "file:/tmp/private", + "../local-package", + "https://example.com/package.tgz", + "git+ssh://git@example.com/package.git", + "npm:safe-package@1.0.0", + ), +) +def test_install_npm_rejects_non_registry_package_source( + tmp_path: Path, + spec: str, +) -> None: + store = ExtensionStore(tmp_path / "extensions") + + with patch("nanobot.extensions.store._run") as run: + with pytest.raises(ValueError, match="registry package name"): + store.install_npm(spec) + + run.assert_not_called() + + +@pytest.mark.parametrize( + "spec", + ("safe-package", "safe-package@latest", "@scope/safe-package@1.2.3"), +) +def test_install_npm_accepts_registry_package_source( + tmp_path: Path, + spec: str, +) -> None: + store = ExtensionStore(tmp_path / "extensions") + + with patch( + "nanobot.extensions.store._run", + side_effect=RuntimeError("validation passed"), + ) as run: + with pytest.raises(RuntimeError, match="validation passed"): + store.install_npm(spec) + + assert run.call_args.args[0][-1] == spec + + +def test_install_npm_rejects_invalid_pack_metadata(tmp_path: Path) -> None: + store = ExtensionStore(tmp_path / "extensions") + + with patch("nanobot.extensions.store._run", return_value="[{}]"): + with pytest.raises(ValueError, match="invalid package metadata"): + store.install_npm("safe-package") + + +def test_npm_archive_rejects_extracted_size_over_limit(tmp_path: Path) -> None: + archive = tmp_path / "package.tgz" + payload = b"too-large" + with tarfile.open(archive, "w:gz") as bundle: + member = tarfile.TarInfo("package/index.mjs") + member.size = len(payload) + bundle.addfile(member, BytesIO(payload)) + + with patch("nanobot.extensions.store._MAX_EXTRACTED_BYTES", len(payload) - 1): + with pytest.raises(ValueError, match="extracted limit"): + _extract_tar(archive, tmp_path / "checkout") + + +def test_node_dependency_install_rejects_non_registry_specs(tmp_path: Path) -> None: + source = tmp_path / "source" + source.mkdir() + (source / "index.mjs").write_text("export default function () {}") + (source / "package.json").write_text( + json.dumps({"dependencies": {"unsafe": "file:/tmp/package"}}) + ) + dump_manifest( + ExtensionManifest( + id="pi.unsafe-dependency", + name="Unsafe dependency", + version="1.0.0", + runtime=ExtensionRuntime.PI, + entry="./index.mjs", + ), + source / "nanobot.extension.json", + ) + store = ExtensionStore(tmp_path / "extensions") + + with patch("nanobot.extensions.store._run") as run: + with pytest.raises(ValueError, match="npm registry"): + store.install_local(source) + + run.assert_not_called() + + +def test_node_dependency_install_includes_required_peers_only(tmp_path: Path) -> None: + source = _pi_package(tmp_path / "source") + package_path = source / "package.json" + package = json.loads(package_path.read_text()) + package.update( + { + "peerDependencies": { + "required-peer": "^1.0.0", + "optional-peer": "^2.0.0", + }, + "peerDependenciesMeta": { + "optional-peer": {"optional": True}, + }, + } + ) + package_path.write_text(json.dumps(package)) + store = ExtensionStore(tmp_path / "extensions") + + def inspect_install(command: list[str], *, cwd: Path | None = None) -> str: + assert command[:2] == ["npm", "install"] + assert cwd is not None + runtime_package = json.loads((cwd / "package.json").read_text()) + assert runtime_package["dependencies"]["required-peer"] == "^1.0.0" + assert "optional-peer" not in runtime_package["dependencies"] + return "" + + with patch("nanobot.extensions.store._run", side_effect=inspect_install): + store.install_local(source) diff --git a/tests/tools/test_tool_registry.py b/tests/tools/test_tool_registry.py index d48eec3d2..b01cc2051 100644 --- a/tests/tools/test_tool_registry.py +++ b/tests/tools/test_tool_registry.py @@ -322,6 +322,19 @@ def test_register_invalidates_cache() -> None: assert len(second) == 2 +def test_register_if_absent_preserves_existing_tool_and_owner() -> None: + registry = ToolRegistry() + existing = _FakeTool("read_file") + registry.register(existing, owner="extension.example") + + assert not registry.register_if_absent( + _FakeTool("read_file"), + owner="nanobot.mcp.example", + ) + assert registry.get("read_file") is existing + assert registry.owner("read_file") == "extension.example" + + def test_unregister_invalidates_cache() -> None: registry = ToolRegistry() registry.register(_FakeTool("read_file")) diff --git a/tests/webui/test_extensions_routes.py b/tests/webui/test_extensions_routes.py index dc2e654bc..476a7e346 100644 --- a/tests/webui/test_extensions_routes.py +++ b/tests/webui/test_extensions_routes.py @@ -27,6 +27,10 @@ class _Service: self.calls.append(("install", (source, kind, ref, trusted))) return {"record": {"id": "sample"}} + async def set_trusted(self, extension_id, trusted): + self.calls.append(("trust", (extension_id, trusted))) + return {"record": {"id": extension_id}} + def _router( service: _Service, @@ -163,3 +167,21 @@ async def test_remote_install_policy_never_exposes_server_local_paths() -> None: assert service.calls == [ ("install", ("pi-example", "npm", "", False)), ] + + +@pytest.mark.asyncio +async def test_remote_install_policy_does_not_grant_remote_trust() -> None: + service = _Service() + + response = await _router(service, allow_remote=True).dispatch( + _REMOTE, + _request( + "/api/extensions/trust", + method="POST", + values={"id": "sample"}, + ), + "/api/extensions/trust", + ) + + assert response is not None and response.status_code == 403 + assert service.calls == [] diff --git a/webui/src/components/extensions/ExtensionDetailSheet.tsx b/webui/src/components/extensions/ExtensionDetailSheet.tsx index 2c09ce56a..c55ef324f 100644 --- a/webui/src/components/extensions/ExtensionDetailSheet.tsx +++ b/webui/src/components/extensions/ExtensionDetailSheet.tsx @@ -52,7 +52,8 @@ export function ExtensionDetailSheet({ const [uninstallOpen, setUninstallOpen] = useState(false); if (!extension) return null; - const external = extension.scope !== "builtin"; + const configManaged = + extension.scope !== "builtin" && !extension.managed_by_store; const requested = new Set(extension.requested_permissions); const granted = new Set(extension.granted_permissions); const allGranted = [...requested].every((permission) => granted.has(permission)); @@ -137,11 +138,15 @@ export function ExtensionDetailSheet({ >
- {permission.name} + {permission.name === "runtime.node" + ? t("extensions.knownPermissions.runtimeNode.label") + : permission.name}
{permission.reason ? (

- {permission.reason} + {permission.name === "runtime.node" + ? t("extensions.knownPermissions.runtimeNode.reason") + : permission.reason}

) : null}
@@ -159,7 +164,7 @@ export function ExtensionDetailSheet({ ))} - {external ? ( + {extension.managed_by_store ? ( + ) : configManaged ? ( +
+ {t("extensions.configManaged")} +
) : null} diff --git a/webui/src/i18n/locales/en/common.json b/webui/src/i18n/locales/en/common.json index 891b8c627..78c329089 100644 --- a/webui/src/i18n/locales/en/common.json +++ b/webui/src/i18n/locales/en/common.json @@ -1302,7 +1302,14 @@ "permissionPending": "Not granted", "revokePermissions": "Revoke permissions", "grantPermissions": "Grant permissions", - "noPermissions": "No host permissions requested.", + "noPermissions": "No host permissions requested.", + "configManaged": "Managed by configuration.", + "knownPermissions": { + "runtimeNode": { + "label": "Node.js runtime", + "reason": "Run third-party JavaScript or TypeScript in a Node.js process." + } + }, "revokeTrust": "Revoke trust", "trust": "Trust", "disable": "Disable", diff --git a/webui/src/i18n/locales/es/common.json b/webui/src/i18n/locales/es/common.json index e9eacc692..e76acb81a 100644 --- a/webui/src/i18n/locales/es/common.json +++ b/webui/src/i18n/locales/es/common.json @@ -1289,7 +1289,14 @@ "permissionPending": "No concedido", "revokePermissions": "Revocar permisos", "grantPermissions": "Conceder permisos", - "noPermissions": "No solicita permisos del host.", + "noPermissions": "No solicita permisos del host.", + "configManaged": "Gestionada por la configuración.", + "knownPermissions": { + "runtimeNode": { + "label": "Entorno de ejecución Node.js", + "reason": "Ejecuta JavaScript o TypeScript de terceros en un proceso de Node.js." + } + }, "revokeTrust": "Revocar confianza", "trust": "Confiar", "disable": "Desactivar", diff --git a/webui/src/i18n/locales/fr/common.json b/webui/src/i18n/locales/fr/common.json index 7ac2dbbf0..53a7ec0f3 100644 --- a/webui/src/i18n/locales/fr/common.json +++ b/webui/src/i18n/locales/fr/common.json @@ -1288,7 +1288,14 @@ "permissionPending": "Non accordée", "revokePermissions": "Révoquer les autorisations", "grantPermissions": "Accorder les autorisations", - "noPermissions": "Aucune autorisation hôte demandée.", + "noPermissions": "Aucune autorisation hôte demandée.", + "configManaged": "Gérée par la configuration.", + "knownPermissions": { + "runtimeNode": { + "label": "Environnement Node.js", + "reason": "Exécute du JavaScript ou TypeScript tiers dans un processus Node.js." + } + }, "revokeTrust": "Révoquer la confiance", "trust": "Faire confiance", "disable": "Désactiver", diff --git a/webui/src/i18n/locales/id/common.json b/webui/src/i18n/locales/id/common.json index a70bbc7e9..101b4f663 100644 --- a/webui/src/i18n/locales/id/common.json +++ b/webui/src/i18n/locales/id/common.json @@ -1288,7 +1288,14 @@ "permissionPending": "Belum diberikan", "revokePermissions": "Cabut izin", "grantPermissions": "Berikan izin", - "noPermissions": "Tidak meminta izin host.", + "noPermissions": "Tidak meminta izin host.", + "configManaged": "Dikelola melalui konfigurasi.", + "knownPermissions": { + "runtimeNode": { + "label": "Runtime Node.js", + "reason": "Jalankan JavaScript atau TypeScript pihak ketiga dalam proses Node.js." + } + }, "revokeTrust": "Cabut kepercayaan", "trust": "Percayai", "disable": "Nonaktifkan", diff --git a/webui/src/i18n/locales/ja/common.json b/webui/src/i18n/locales/ja/common.json index bd6d7b2c6..e16f0895b 100644 --- a/webui/src/i18n/locales/ja/common.json +++ b/webui/src/i18n/locales/ja/common.json @@ -1288,7 +1288,14 @@ "permissionPending": "未許可", "revokePermissions": "権限を取り消す", "grantPermissions": "権限を許可", - "noPermissions": "ホスト権限の要求はありません。", + "noPermissions": "ホスト権限の要求はありません。", + "configManaged": "設定で管理されています。", + "knownPermissions": { + "runtimeNode": { + "label": "Node.js ランタイム", + "reason": "サードパーティーの JavaScript または TypeScript を Node.js プロセスで実行します。" + } + }, "revokeTrust": "信頼を取り消す", "trust": "信頼する", "disable": "無効化", diff --git a/webui/src/i18n/locales/ko/common.json b/webui/src/i18n/locales/ko/common.json index 5e7f2c756..cfbebdac4 100644 --- a/webui/src/i18n/locales/ko/common.json +++ b/webui/src/i18n/locales/ko/common.json @@ -1288,7 +1288,14 @@ "permissionPending": "허용되지 않음", "revokePermissions": "권한 취소", "grantPermissions": "권한 허용", - "noPermissions": "요청한 호스트 권한이 없습니다.", + "noPermissions": "요청한 호스트 권한이 없습니다.", + "configManaged": "설정에서 관리됩니다.", + "knownPermissions": { + "runtimeNode": { + "label": "Node.js 런타임", + "reason": "타사 JavaScript 또는 TypeScript를 Node.js 프로세스에서 실행합니다." + } + }, "revokeTrust": "신뢰 취소", "trust": "신뢰", "disable": "비활성화", diff --git a/webui/src/i18n/locales/pt-BR/common.json b/webui/src/i18n/locales/pt-BR/common.json index 767ba0d49..ae29b6120 100644 --- a/webui/src/i18n/locales/pt-BR/common.json +++ b/webui/src/i18n/locales/pt-BR/common.json @@ -1302,7 +1302,14 @@ "permissionPending": "Não concedida", "revokePermissions": "Revogar permissões", "grantPermissions": "Conceder permissões", - "noPermissions": "Nenhuma permissão do host solicitada.", + "noPermissions": "Nenhuma permissão do host solicitada.", + "configManaged": "Gerenciada pela configuração.", + "knownPermissions": { + "runtimeNode": { + "label": "Runtime Node.js", + "reason": "Executa JavaScript ou TypeScript de terceiros em um processo Node.js." + } + }, "revokeTrust": "Revogar confiança", "trust": "Confiar", "disable": "Desativar", diff --git a/webui/src/i18n/locales/vi/common.json b/webui/src/i18n/locales/vi/common.json index c9c4f487b..240c354c2 100644 --- a/webui/src/i18n/locales/vi/common.json +++ b/webui/src/i18n/locales/vi/common.json @@ -1288,7 +1288,14 @@ "permissionPending": "Chưa cấp", "revokePermissions": "Thu hồi quyền", "grantPermissions": "Cấp quyền", - "noPermissions": "Không yêu cầu quyền máy chủ.", + "noPermissions": "Không yêu cầu quyền máy chủ.", + "configManaged": "Được quản lý bằng cấu hình.", + "knownPermissions": { + "runtimeNode": { + "label": "Môi trường chạy Node.js", + "reason": "Chạy JavaScript hoặc TypeScript của bên thứ ba trong tiến trình Node.js." + } + }, "revokeTrust": "Thu hồi tin cậy", "trust": "Tin cậy", "disable": "Tắt", diff --git a/webui/src/i18n/locales/zh-CN/common.json b/webui/src/i18n/locales/zh-CN/common.json index cbdeb16eb..49a623249 100644 --- a/webui/src/i18n/locales/zh-CN/common.json +++ b/webui/src/i18n/locales/zh-CN/common.json @@ -1302,7 +1302,14 @@ "permissionPending": "未授予", "revokePermissions": "撤销权限", "grantPermissions": "授予权限", - "noPermissions": "未请求宿主权限。", + "noPermissions": "未请求宿主权限。", + "configManaged": "由配置管理。", + "knownPermissions": { + "runtimeNode": { + "label": "Node.js 运行时", + "reason": "允许扩展在 Node.js 进程中运行第三方 JavaScript 或 TypeScript。" + } + }, "revokeTrust": "撤销信任", "trust": "信任", "disable": "停用", diff --git a/webui/src/i18n/locales/zh-TW/common.json b/webui/src/i18n/locales/zh-TW/common.json index 253f93198..2aad5e581 100644 --- a/webui/src/i18n/locales/zh-TW/common.json +++ b/webui/src/i18n/locales/zh-TW/common.json @@ -1288,7 +1288,14 @@ "permissionPending": "未授予", "revokePermissions": "撤銷權限", "grantPermissions": "授予權限", - "noPermissions": "未要求主機權限。", + "noPermissions": "未要求主機權限。", + "configManaged": "由設定管理。", + "knownPermissions": { + "runtimeNode": { + "label": "Node.js 執行環境", + "reason": "允許擴充套件在 Node.js 行程中執行第三方 JavaScript 或 TypeScript。" + } + }, "revokeTrust": "撤銷信任", "trust": "信任", "disable": "停用", diff --git a/webui/src/lib/types.ts b/webui/src/lib/types.ts index 3f1dc8a5b..b93140c41 100644 --- a/webui/src/lib/types.ts +++ b/webui/src/lib/types.ts @@ -771,6 +771,7 @@ export interface ExtensionInfo { source_ref: string; integrity: string; installed_at: string; + managed_by_store: boolean; contributions: ExtensionContributionInfo[]; dependencies: ExtensionDependencyInfo[]; permissions: ExtensionPermissionInfo[]; diff --git a/webui/src/tests/extensions-view.test.tsx b/webui/src/tests/extensions-view.test.tsx index 6fac16b8b..fc0e2604a 100644 --- a/webui/src/tests/extensions-view.test.tsx +++ b/webui/src/tests/extensions-view.test.tsx @@ -36,6 +36,7 @@ function extension(overrides: Partial = {}): ExtensionInfo { source_ref: "@sample/pi-extension", integrity: "sha512-example", installed_at: "2026-07-26T00:00:00Z", + managed_by_store: true, contributions: [{ kind: "tool", name: "sample", description: "" }], dependencies: [], permissions: [{ name: "process.spawn", reason: "Runs the extension host." }], @@ -145,4 +146,43 @@ describe("ExtensionsView", () => { kind: "npm", }); }); + + it("localizes permissions generated by a compatibility adapter", async () => { + vi.stubGlobal("fetch", vi.fn(async () => response({ + extensions: [extension({ + requested_permissions: ["runtime.node"], + permissions: [{ + name: "runtime.node", + reason: "Raw adapter copy.", + }], + })], + diagnostics: [], + }))); + + renderView(); + fireEvent.click(await screen.findByRole("button", { name: /Sample Pi/ })); + + expect(screen.getByText("Node.js runtime")).toBeInTheDocument(); + expect(screen.getByText( + "Run third-party JavaScript or TypeScript in a Node.js process.", + )).toBeInTheDocument(); + }); + + it("keeps config-discovered extensions read-only", async () => { + vi.stubGlobal("fetch", vi.fn(async () => response({ + extensions: [extension({ + scope: "workspace", + source: "path", + managed_by_store: false, + })], + diagnostics: [], + }))); + + renderView(); + fireEvent.click(await screen.findByRole("button", { name: /Sample Pi/ })); + + expect(screen.getByText("Managed by configuration.")).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Trust" })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Uninstall" })).not.toBeInTheDocument(); + }); });