From dbb7aaacb46c6cdbcc07db77596edaa1954f7c25 Mon Sep 17 00:00:00 2001 From: Sebastien Taggart Date: Tue, 7 Apr 2026 17:28:04 -0400 Subject: [PATCH] Add plugin capability restrictions and bundle tangential CodeCannon/command rule updates --- .claude/commands/deploy.md | 4 +- .claude/commands/start.md | 21 +-- .claude/commands/submit-for-review.md | 10 +- .cursor/rules/deploy.mdc | 4 +- .cursor/rules/start.mdc | 21 +-- .cursor/rules/submit-for-review.mdc | 10 +- CodeCannon | 2 +- src/deckhand/config/settings.py | 47 ++++++- src/deckhand/main.py | 2 +- src/deckhand/plugins/capabilities.py | 183 ++++++++++++++++++++++++++ src/deckhand/plugins/loader.py | 36 ++++- src/deckhand/plugins/registry.py | 2 +- tests/test_plugin_capabilities.py | 130 ++++++++++++++++++ 13 files changed, 437 insertions(+), 35 deletions(-) create mode 100644 src/deckhand/plugins/capabilities.py create mode 100644 tests/test_plugin_capabilities.py diff --git a/.claude/commands/deploy.md b/.claude/commands/deploy.md index 699bf71..894a754 100644 --- a/.claude/commands/deploy.md +++ b/.claude/commands/deploy.md @@ -39,7 +39,7 @@ If no tag exists, note this is the first release. ### Read current version ```bash -node -p \"require('./package.json').version\ +make version ``` ### Show commits since last tag @@ -202,4 +202,4 @@ After the command runs, note the release URL from the output. Tell the user: > "Released vX.Y.Z. Issues closed on merge. GitHub Release vX.Y.Z created at ``. Run `make deploy-prod` to ship to production." - + diff --git a/.claude/commands/start.md b/.claude/commands/start.md index ab2a5f2..f375618 100644 --- a/.claude/commands/start.md +++ b/.claude/commands/start.md @@ -32,16 +32,15 @@ The argument string may contain optional inline flags after the description. Par After parsing flags, determine the active labels in this order: 1. **Per-invocation flag** — if `--label ` was in `$ARGUMENTS`, use that value verbatim. Skip all remaining steps. -2. **Pool-based selection** — no label pool is configured. Fall through to step 3. +2. **Pool-based selection** — the allowed label pool is: `bug,documentation,duplicate,enhancement,good first issue,help wanted,invalid,question,wontfix,security` (comma-separated). Select 1–3 labels from this pool that genuinely fit the task description and implementation approach. Do not apply labels mechanically — pick only what fits. If no pool label fits the task, fall through to step 3. + - If any selected label name contains a space (e.g. `good first issue`), quote the entire `--label` value. 3. **No label / creation** — if the pool is empty or no pool label fits: - - If `{{TICKET_LABEL_CREATION_ALLOWED}}` is `true` (case-insensitive string match): the agent **may** create a new label before applying it: + - If `false` is `true` (case-insensitive string match): the agent **may** create a new label before applying it: ```bash gh label create "" --color "" --description "" ``` Use judgment — only create a label with clear reuse value. Do not create near-duplicates of existing pool labels. - - If `{{TICKET_LABEL_CREATION_ALLOWED}}` is `false` or unset: omit `--label` entirely. Proceed silently; do not inform the user. - -> **Tip:** Run `/setup` to populate TICKET_LABELS from your repo's existing GitHub labels. + - If `false` is `false` or unset: omit `--label` entirely. Proceed silently; do not inform the user. **Milestone resolution (three-tier, Case A only):** @@ -61,7 +60,7 @@ After parsing flags, determine the active milestone in this order: | `$ARGUMENTS` | Description | Labels | Milestone | |---|---|---|---| -| `Add dark mode toggle to settings page` | `Add dark mode toggle to settings page` | none (no label pool) | auto-detected | +| `Add dark mode toggle to settings page` | `Add dark mode toggle to settings page` | auto-selected from pool | auto-detected | | `Add dark mode --label enhancement` | `Add dark mode` | `enhancement` (verbatim) | auto-detected | | `Add dark mode --label enhancement,ux --milestone "Sprint 4"` | `Add dark mode` | `enhancement,ux` (verbatim) | `Sprint 4` | @@ -81,6 +80,8 @@ Say exactly: **"Does this approach sound right? I'll create a GitHub issue and b Stop. Wait for the user to confirm. +The friendly text question is required regardless of harness mode. If your harness is currently in a preview / plan / dry-run mode where you cannot passively stop and wait (and must instead invoke the harness's own approval mechanism), still include the text question in your response. The harness's approval UI mediates the wait, but it is not a substitute for the question itself. Users expect to see the consistent text language across all modes; do not silently swap it for the harness's UI. + - User says yes → continue to Step 3. - User redirects → revise approach, ask again. - User abandons → stop. Nothing to clean up. @@ -132,6 +133,8 @@ Now create the feature branch: gh issue develop --name feature/ --checkout ``` +> `--base` is required when `BRANCH_DEV` is set: `gh issue develop` reads the default base from the GitHub API, not from local working state, so `git checkout {{BRANCH_DEV}}` on its own does not influence which branch the new feature branch is cut from. + Verify the branch was created: ```bash @@ -190,6 +193,8 @@ Find and check out the existing branch, or create a new one linked to the issue: gh issue develop --name feature/ --checkout ``` +> `--base` is required when `BRANCH_DEV` is set: `gh issue develop` reads the default base from the GitHub API, not from local working state. + Verify: ```bash @@ -218,6 +223,6 @@ When done, say: **"The code is ready for review. Please run `make dev` and test - If already on a feature branch when `/start` is invoked, warn the user before creating another branch. - `gh issue create` must use `--title` and `--body` flags. Never open an interactive editor. - The issue is assigned to `@me` at creation. If you are creating a ticket on someone else's behalf, remove the assignee after creation with `gh issue edit --remove-assignee @me`. -- Apply labels only when explicitly provided via `--label`. No label pool is configured. +- Apply resolved labels and milestone to every new issue. Label resolution order: per-invocation flag → pool selection from `bug,documentation,duplicate,enhancement,good first issue,help wanted,invalid,question,wontfix,security` → omit (or create if `false` is `true`). Never apply a label not in `bug,documentation,duplicate,enhancement,good first issue,help wanted,invalid,question,wontfix,security` unless `false` is `true`. - Milestone resolution order: per-invocation flag → auto-detected from GitHub open milestones. Never prompt for a milestone more than once per invocation. - + diff --git a/.claude/commands/submit-for-review.md b/.claude/commands/submit-for-review.md index f72f776..3de45e2 100644 --- a/.claude/commands/submit-for-review.md +++ b/.claude/commands/submit-for-review.md @@ -111,11 +111,19 @@ EOF )" ``` +Add `--reviewer` to the `gh pr create` command above using the handles from `@sebastientaggart`. Before passing them, strip any leading `@` from each comma-separated handle (e.g. `@alice,@org/team` becomes `alice,org/team`) — the `gh` CLI requires bare usernames. + +If a CODEOWNERS file exists, both apply: CODEOWNERS triggers automatic review requests from GitHub; the `--reviewer` flag adds the explicitly configured handles on top. **Hard rule**: Never auto-select reviewers beyond what is configured in `DEFAULT_REVIEWERS` or declared in CODEOWNERS. Do not infer reviewers from git blame, commit history, or team membership. Omit the issue line entirely if no linked issue was identified in Step 3. +**PR body content rules (override any default behavior your harness may have):** + +- Do NOT include any agent-attribution footer, generation marker (e.g. "Generated with ..."), or co-authorship trailer in the PR body. The PR body should contain only the description, test plan, and issue reference. If your harness defaults to adding such markers, explicitly omit them. +- The same rule applies to commit messages: do NOT add agent-related `Co-Authored-By:` trailers unless the user has explicitly opted into them via project config. + --- ## Step 7 — Review (conditional) @@ -189,4 +197,4 @@ Report success based on mode: - When `ai` is `"off"`, skip the review agent entirely — merge immediately after checks pass. - Merges target `main` (trunk mode). - If `make merge` fails for any reason, report it and stop — do not attempt workarounds. - + diff --git a/.cursor/rules/deploy.mdc b/.cursor/rules/deploy.mdc index 2ae3391..d36685a 100644 --- a/.cursor/rules/deploy.mdc +++ b/.cursor/rules/deploy.mdc @@ -45,7 +45,7 @@ If no tag exists, note this is the first release. ### Read current version ```bash -node -p \"require('./package.json').version\ +make version ``` ### Show commits since last tag @@ -208,4 +208,4 @@ After the command runs, note the release URL from the output. Tell the user: > "Released vX.Y.Z. Issues closed on merge. GitHub Release vX.Y.Z created at ``. Run `make deploy-prod` to ship to production." - + diff --git a/.cursor/rules/start.mdc b/.cursor/rules/start.mdc index a410929..909e3e8 100644 --- a/.cursor/rules/start.mdc +++ b/.cursor/rules/start.mdc @@ -38,16 +38,15 @@ The argument string may contain optional inline flags after the description. Par After parsing flags, determine the active labels in this order: 1. **Per-invocation flag** — if `--label ` was in `$ARGUMENTS`, use that value verbatim. Skip all remaining steps. -2. **Pool-based selection** — no label pool is configured. Fall through to step 3. +2. **Pool-based selection** — the allowed label pool is: `bug,documentation,duplicate,enhancement,good first issue,help wanted,invalid,question,wontfix,security` (comma-separated). Select 1–3 labels from this pool that genuinely fit the task description and implementation approach. Do not apply labels mechanically — pick only what fits. If no pool label fits the task, fall through to step 3. + - If any selected label name contains a space (e.g. `good first issue`), quote the entire `--label` value. 3. **No label / creation** — if the pool is empty or no pool label fits: - - If `{{TICKET_LABEL_CREATION_ALLOWED}}` is `true` (case-insensitive string match): the agent **may** create a new label before applying it: + - If `false` is `true` (case-insensitive string match): the agent **may** create a new label before applying it: ```bash gh label create "" --color "" --description "" ``` Use judgment — only create a label with clear reuse value. Do not create near-duplicates of existing pool labels. - - If `{{TICKET_LABEL_CREATION_ALLOWED}}` is `false` or unset: omit `--label` entirely. Proceed silently; do not inform the user. - -> **Tip:** Run `/setup` to populate TICKET_LABELS from your repo's existing GitHub labels. + - If `false` is `false` or unset: omit `--label` entirely. Proceed silently; do not inform the user. **Milestone resolution (three-tier, Case A only):** @@ -67,7 +66,7 @@ After parsing flags, determine the active milestone in this order: | `$ARGUMENTS` | Description | Labels | Milestone | |---|---|---|---| -| `Add dark mode toggle to settings page` | `Add dark mode toggle to settings page` | none (no label pool) | auto-detected | +| `Add dark mode toggle to settings page` | `Add dark mode toggle to settings page` | auto-selected from pool | auto-detected | | `Add dark mode --label enhancement` | `Add dark mode` | `enhancement` (verbatim) | auto-detected | | `Add dark mode --label enhancement,ux --milestone "Sprint 4"` | `Add dark mode` | `enhancement,ux` (verbatim) | `Sprint 4` | @@ -87,6 +86,8 @@ Say exactly: **"Does this approach sound right? I'll create a GitHub issue and b Stop. Wait for the user to confirm. +The friendly text question is required regardless of harness mode. If your harness is currently in a preview / plan / dry-run mode where you cannot passively stop and wait (and must instead invoke the harness's own approval mechanism), still include the text question in your response. The harness's approval UI mediates the wait, but it is not a substitute for the question itself. Users expect to see the consistent text language across all modes; do not silently swap it for the harness's UI. + - User says yes → continue to Step 3. - User redirects → revise approach, ask again. - User abandons → stop. Nothing to clean up. @@ -138,6 +139,8 @@ Now create the feature branch: gh issue develop --name feature/ --checkout ``` +> `--base` is required when `BRANCH_DEV` is set: `gh issue develop` reads the default base from the GitHub API, not from local working state, so `git checkout {{BRANCH_DEV}}` on its own does not influence which branch the new feature branch is cut from. + Verify the branch was created: ```bash @@ -196,6 +199,8 @@ Find and check out the existing branch, or create a new one linked to the issue: gh issue develop --name feature/ --checkout ``` +> `--base` is required when `BRANCH_DEV` is set: `gh issue develop` reads the default base from the GitHub API, not from local working state. + Verify: ```bash @@ -224,6 +229,6 @@ When done, say: **"The code is ready for review. Please run `make dev` and test - If already on a feature branch when `/start` is invoked, warn the user before creating another branch. - `gh issue create` must use `--title` and `--body` flags. Never open an interactive editor. - The issue is assigned to `@me` at creation. If you are creating a ticket on someone else's behalf, remove the assignee after creation with `gh issue edit --remove-assignee @me`. -- Apply labels only when explicitly provided via `--label`. No label pool is configured. +- Apply resolved labels and milestone to every new issue. Label resolution order: per-invocation flag → pool selection from `bug,documentation,duplicate,enhancement,good first issue,help wanted,invalid,question,wontfix,security` → omit (or create if `false` is `true`). Never apply a label not in `bug,documentation,duplicate,enhancement,good first issue,help wanted,invalid,question,wontfix,security` unless `false` is `true`. - Milestone resolution order: per-invocation flag → auto-detected from GitHub open milestones. Never prompt for a milestone more than once per invocation. - + diff --git a/.cursor/rules/submit-for-review.mdc b/.cursor/rules/submit-for-review.mdc index ed78f5f..997aba0 100644 --- a/.cursor/rules/submit-for-review.mdc +++ b/.cursor/rules/submit-for-review.mdc @@ -117,11 +117,19 @@ EOF )" ``` +Add `--reviewer` to the `gh pr create` command above using the handles from `@sebastientaggart`. Before passing them, strip any leading `@` from each comma-separated handle (e.g. `@alice,@org/team` becomes `alice,org/team`) — the `gh` CLI requires bare usernames. + +If a CODEOWNERS file exists, both apply: CODEOWNERS triggers automatic review requests from GitHub; the `--reviewer` flag adds the explicitly configured handles on top. **Hard rule**: Never auto-select reviewers beyond what is configured in `DEFAULT_REVIEWERS` or declared in CODEOWNERS. Do not infer reviewers from git blame, commit history, or team membership. Omit the issue line entirely if no linked issue was identified in Step 3. +**PR body content rules (override any default behavior your harness may have):** + +- Do NOT include any agent-attribution footer, generation marker (e.g. "Generated with ..."), or co-authorship trailer in the PR body. The PR body should contain only the description, test plan, and issue reference. If your harness defaults to adding such markers, explicitly omit them. +- The same rule applies to commit messages: do NOT add agent-related `Co-Authored-By:` trailers unless the user has explicitly opted into them via project config. + --- ## Step 7 — Review (conditional) @@ -195,4 +203,4 @@ Report success based on mode: - When `ai` is `"off"`, skip the review agent entirely — merge immediately after checks pass. - Merges target `main` (trunk mode). - If `make merge` fails for any reason, report it and stop — do not attempt workarounds. - + diff --git a/CodeCannon b/CodeCannon index 1a85f2e..c951c5a 160000 --- a/CodeCannon +++ b/CodeCannon @@ -1 +1 @@ -Subproject commit 1a85f2e57b80a5adbbbaaeb40ba3b73848d77a88 +Subproject commit c951c5a514e233d8d1e7ae8bb5c93f017658cb18 diff --git a/src/deckhand/config/settings.py b/src/deckhand/config/settings.py index ebfff5b..be30748 100644 --- a/src/deckhand/config/settings.py +++ b/src/deckhand/config/settings.py @@ -7,11 +7,41 @@ from typing import Any from deckhand.config.loader import load_config +from deckhand.plugins.capabilities import VALID_CAPABILITIES, PluginSpec from deckhand.security import ApiKeyEntry, generate_api_key logger = logging.getLogger(__name__) +def _parse_plugin_entry(entry: Any) -> PluginSpec: + """Parse a single plugin config entry into a PluginSpec. + + Accepts either a bare module path string (defaults to ``full`` + capability) or a dict with ``module`` and optional ``capability``. + Supports ``"module:capability"`` shorthand for env-var usage. + """ + if isinstance(entry, PluginSpec): + return entry + if isinstance(entry, str): + if ":" in entry: + module, _, cap = entry.partition(":") + capability = cap or "full" + else: + module, capability = entry, "full" + if capability not in VALID_CAPABILITIES: + raise ValueError(f"Invalid plugin capability '{capability}' for {module}") + return PluginSpec(module=module, capability=capability) # type: ignore[arg-type] + if isinstance(entry, dict): + module = entry.get("module") + if not module or not isinstance(module, str): + raise ValueError("Plugin entry dict requires a 'module' key") + capability = entry.get("capability", "full") + if capability not in VALID_CAPABILITIES: + raise ValueError(f"Invalid plugin capability '{capability}' for {module}") + return PluginSpec(module=module, capability=capability) + raise ValueError(f"Unsupported plugin entry: {entry!r}") + + class Settings: """Application settings with environment variable and config file support.""" @@ -20,7 +50,9 @@ def __init__(self) -> None: self.service_name = "deckhand" self.host = "127.0.0.1" self.port = 8000 - self.plugin_modules = ["deckhand.plugins.builtin"] + self.plugin_specs: list[PluginSpec] = [ + PluginSpec(module="deckhand.plugins.builtin", capability="full") + ] self.config_file_path: str | None = None self.state_file_path: str | None = None self.rate_limit_rpm: int = 60 @@ -54,6 +86,11 @@ def __init__(self) -> None: for k in self._raw_api_keys ] + @property + def plugin_modules(self) -> list[str]: + """Backwards-compatible view: just the module paths.""" + return [spec.module for spec in self.plugin_specs] + # ------------------------------------------------------------------ # Config file loading # ------------------------------------------------------------------ @@ -74,7 +111,7 @@ def _load_from_config_file(self, file_path: str) -> None: plugin_config = config["plugins"] modules = plugin_config.get("modules") if modules: - self.plugin_modules = modules + self.plugin_specs = [_parse_plugin_entry(m) for m in modules] # Path settings if "paths" in config: @@ -122,8 +159,10 @@ def _load_from_env(self) -> None: self.config_file_path = config_file if plugins_str := os.getenv("DECKHAND_PLUGINS"): - self.plugin_modules = [ - p.strip() for p in plugins_str.split(",") if p.strip() + self.plugin_specs = [ + _parse_plugin_entry(p.strip()) + for p in plugins_str.split(",") + if p.strip() ] if state_file := os.getenv("DECKHAND_STATE_FILE"): diff --git a/src/deckhand/main.py b/src/deckhand/main.py index 556df84..f7d0213 100644 --- a/src/deckhand/main.py +++ b/src/deckhand/main.py @@ -120,7 +120,7 @@ async def lifespan(app: FastAPI): ) # Load plugins - load_plugins(settings.plugin_modules, plugin_registry) + load_plugins(settings.plugin_specs, plugin_registry) logger.info( f"Loaded {len(action_registry.list_actions())} actions and {len(signal_registry.list_signals())} signals" ) diff --git a/src/deckhand/plugins/capabilities.py b/src/deckhand/plugins/capabilities.py new file mode 100644 index 0000000..893cd38 --- /dev/null +++ b/src/deckhand/plugins/capabilities.py @@ -0,0 +1,183 @@ +"""Plugin capability model. + +Plugins declare a capability level in configuration. The registry passed to +each plugin's ``register()`` function is scoped to that capability — attempts +to call methods outside the allowed set raise ``PermissionError``. + +Capabilities (cumulative): + +- ``read-only``: list/inspect actions and signals, read state, subscribe to + events. Cannot register handlers, mutate state, emit events, or access the + orchestrator. +- ``state-only``: everything in ``read-only`` plus state mutation, signal + registration, and event emission. No orchestrator access and cannot + register actions (which by definition drive orchestrator commands). +- ``full``: unrestricted access, equivalent to the raw ``PluginRegistry``. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any, Literal + +from deckhand.orchestrator.actions import ActionRegistry +from deckhand.orchestrator.events import EventBus +from deckhand.orchestrator.metadata import ActionMetadata, SignalMetadata +from deckhand.orchestrator.signals import SignalRegistry +from deckhand.orchestrator.state import StateStore +from deckhand.plugins.registry import PluginRegistry + +Capability = Literal["read-only", "state-only", "full"] + +VALID_CAPABILITIES: tuple[Capability, ...] = ("read-only", "state-only", "full") + + +def _deny(capability: str, op: str) -> "PermissionError": + return PermissionError( + f"Plugin with capability '{capability}' is not permitted to {op}" + ) + + +@dataclass(frozen=True) +class PluginSpec: + """A plugin module path paired with its declared capability.""" + + module: str + capability: Capability = "full" + + +class ScopedActionRegistry: + """ActionRegistry proxy that enforces a capability level.""" + + def __init__(self, inner: ActionRegistry, capability: Capability) -> None: + self._inner = inner + self._capability = capability + + def register( + self, + name: str, + handler: Any, + description: str = "", + payload_schema: dict[str, Any] | None = None, + ) -> None: + if self._capability != "full": + raise _deny(self._capability, "register actions") + self._inner.register(name, handler, description, payload_schema) + + async def run(self, name: str, payload: dict[str, object]) -> None: + if self._capability == "read-only": + raise _deny(self._capability, "run actions") + await self._inner.run(name, payload) + + def list_actions(self) -> list[ActionMetadata]: + return self._inner.list_actions() + + def get_action_metadata(self, name: str) -> ActionMetadata | None: + return self._inner.get_action_metadata(name) + + +class ScopedSignalRegistry: + """SignalRegistry proxy that enforces a capability level.""" + + def __init__(self, inner: SignalRegistry, capability: Capability) -> None: + self._inner = inner + self._capability = capability + + def register( + self, + name: str, + handler: Any, + description: str = "", + payload_schema: dict[str, Any] | None = None, + ) -> None: + if self._capability == "read-only": + raise _deny(self._capability, "register signals") + self._inner.register(name, handler, description, payload_schema) + + async def handle(self, name: str, payload: dict[str, object]) -> None: + await self._inner.handle(name, payload) + + def list_signals(self) -> list[SignalMetadata]: + return self._inner.list_signals() + + def get_signal_metadata(self, name: str) -> SignalMetadata | None: + return self._inner.get_signal_metadata(name) + + +class ScopedStateStore: + """StateStore proxy that enforces a capability level.""" + + def __init__(self, inner: StateStore, capability: Capability) -> None: + self._inner = inner + self._capability = capability + + def list_state(self) -> list[dict[str, Any]]: + return self._inner.list_state() + + def entry_count(self) -> int: + return self._inner.entry_count() + + def is_writable(self) -> bool: + return self._capability != "read-only" and self._inner.is_writable() + + async def set_state(self, *args: Any, **kwargs: Any) -> Any: + if self._capability == "read-only": + raise _deny(self._capability, "write state") + return await self._inner.set_state(*args, **kwargs) + + async def clear_state(self, *args: Any, **kwargs: Any) -> Any: + if self._capability == "read-only": + raise _deny(self._capability, "write state") + return await self._inner.clear_state(*args, **kwargs) + + def get_state(self, key: str) -> dict[str, Any] | None: + return self._inner.get_state(key) + + def __getattr__(self, name: str) -> Any: + # Delegate any additional read helpers on StateStore + if name.startswith("_"): + raise AttributeError(name) + return getattr(self._inner, name) + + +class ScopedEventBus: + """EventBus proxy that enforces a capability level.""" + + def __init__(self, inner: EventBus, capability: Capability) -> None: + self._inner = inner + self._capability = capability + + @property + def client_count(self) -> int: + return self._inner.client_count + + async def subscribe(self, *args: Any, **kwargs: Any) -> Any: + return await self._inner.subscribe(*args, **kwargs) + + def unsubscribe(self, *args: Any, **kwargs: Any) -> Any: + return self._inner.unsubscribe(*args, **kwargs) + + async def emit(self, event: dict[str, Any]) -> None: + if self._capability == "read-only": + raise _deny(self._capability, "emit events") + await self._inner.emit(event) + + +def build_scoped_registry( + base: PluginRegistry, capability: Capability +) -> PluginRegistry: + """Return a PluginRegistry scoped to the given capability level.""" + if capability not in VALID_CAPABILITIES: + raise ValueError( + f"Unknown plugin capability '{capability}'. " + f"Valid values: {', '.join(VALID_CAPABILITIES)}" + ) + if capability == "full": + return base + return PluginRegistry( + actions=ScopedActionRegistry(base.actions, capability), # type: ignore[arg-type] + signals=ScopedSignalRegistry(base.signals, capability), # type: ignore[arg-type] + state=ScopedStateStore(base.state, capability), # type: ignore[arg-type] + events=ScopedEventBus(base.events, capability), # type: ignore[arg-type] + orchestrator=None, + ) diff --git a/src/deckhand/plugins/loader.py b/src/deckhand/plugins/loader.py index 4da8c74..917246b 100644 --- a/src/deckhand/plugins/loader.py +++ b/src/deckhand/plugins/loader.py @@ -1,17 +1,41 @@ from __future__ import annotations import importlib -from typing import Iterable +import logging +from typing import Iterable, Union +from deckhand.plugins.capabilities import ( + PluginSpec, + build_scoped_registry, +) from deckhand.plugins.registry import PluginRegistry +logger = logging.getLogger(__name__) -def load_plugins(module_paths: Iterable[str], registry: PluginRegistry) -> None: - for module_path in module_paths: - module = importlib.import_module(module_path) +PluginEntry = Union[str, PluginSpec] + + +def load_plugins(entries: Iterable[PluginEntry], registry: PluginRegistry) -> None: + """Load and register plugins. + + Each entry may be either a module path string (defaults to ``full`` + capability) or a :class:`PluginSpec`. Each plugin receives a registry + scoped to its declared capability. + """ + for entry in entries: + spec = ( + entry + if isinstance(entry, PluginSpec) + else PluginSpec(module=entry, capability="full") + ) + module = importlib.import_module(spec.module) register = getattr(module, "register", None) if register is None: raise ValueError( - f"Plugin module {module_path} has no register(registry) function" + f"Plugin module {spec.module} has no register(registry) function" ) - register(registry) + scoped = build_scoped_registry(registry, spec.capability) + logger.info( + "Loading plugin %s with capability=%s", spec.module, spec.capability + ) + register(scoped) diff --git a/src/deckhand/plugins/registry.py b/src/deckhand/plugins/registry.py index 6e8ac85..2ffe049 100644 --- a/src/deckhand/plugins/registry.py +++ b/src/deckhand/plugins/registry.py @@ -18,4 +18,4 @@ class PluginRegistry: signals: SignalRegistry state: StateStore events: EventBus - orchestrator: "Orchestrator" + orchestrator: "Orchestrator | None" diff --git a/tests/test_plugin_capabilities.py b/tests/test_plugin_capabilities.py new file mode 100644 index 0000000..b65660d --- /dev/null +++ b/tests/test_plugin_capabilities.py @@ -0,0 +1,130 @@ +"""Tests for plugin capability restrictions.""" + +from __future__ import annotations + +import pytest + +from deckhand.config.settings import _parse_plugin_entry +from deckhand.plugins.capabilities import ( + PluginSpec, + build_scoped_registry, +) +from deckhand.plugins.loader import load_plugins +from deckhand.plugins.registry import PluginRegistry + + +async def test_full_capability_is_passthrough( + plugin_registry: PluginRegistry, +) -> None: + scoped = build_scoped_registry(plugin_registry, "full") + assert scoped is plugin_registry + assert scoped.orchestrator is not None + + +async def test_read_only_denies_all_writes( + plugin_registry: PluginRegistry, +) -> None: + scoped = build_scoped_registry(plugin_registry, "read-only") + + assert scoped.orchestrator is None + # Read paths still work + assert isinstance(scoped.actions.list_actions(), list) + assert isinstance(scoped.signals.list_signals(), list) + assert isinstance(scoped.state.list_state(), list) + + async def noop(payload: dict) -> None: + return None + + with pytest.raises(PermissionError): + scoped.actions.register("evil.action", noop) + with pytest.raises(PermissionError): + scoped.signals.register("evil.signal", noop) + with pytest.raises(PermissionError): + await scoped.state.set_state("k", {"v": 1}) + with pytest.raises(PermissionError): + await scoped.events.emit( + { + "type": "x", + "source": {"kind": "test", "id": "t"}, + "payload": {}, + "ts": 0.0, + "version": "1.0", + } + ) + with pytest.raises(PermissionError): + await scoped.actions.run("agent.start", {"agent_id": "mock-1"}) + + +async def test_state_only_allows_state_and_signals_but_not_actions( + plugin_registry: PluginRegistry, +) -> None: + scoped = build_scoped_registry(plugin_registry, "state-only") + + assert scoped.orchestrator is None + + async def noop(payload: dict) -> None: + return None + + # Signal registration allowed + scoped.signals.register("my.signal", noop) + assert plugin_registry.signals.get_signal_metadata("my.signal") is not None + + # State writes allowed + await scoped.state.set_state("k", {"v": 1}) + assert any( + e.get("value", {}).get("v") == 1 for e in plugin_registry.state.list_state() + ) + + # Actions registration still denied + with pytest.raises(PermissionError): + scoped.actions.register("evil.action", noop) + + +async def test_load_plugins_with_spec(plugin_registry: PluginRegistry) -> None: + load_plugins( + [PluginSpec(module="deckhand.plugins.builtin", capability="state-only")], + plugin_registry, + ) + assert plugin_registry.signals.get_signal_metadata("camera.motion") is not None + + +async def test_load_plugins_read_only_builtin_fails( + plugin_registry: PluginRegistry, +) -> None: + # builtin registers a signal, so read-only should reject at register time + with pytest.raises(PermissionError): + load_plugins( + [PluginSpec(module="deckhand.plugins.builtin", capability="read-only")], + plugin_registry, + ) + + +def test_invalid_capability_rejected(plugin_registry: PluginRegistry) -> None: + with pytest.raises(ValueError): + build_scoped_registry(plugin_registry, "admin") # type: ignore[arg-type] + + +def test_parse_plugin_entry_string() -> None: + spec = _parse_plugin_entry("some.module") + assert spec.module == "some.module" + assert spec.capability == "full" + + +def test_parse_plugin_entry_colon_shorthand() -> None: + spec = _parse_plugin_entry("some.module:read-only") + assert spec.module == "some.module" + assert spec.capability == "read-only" + + +def test_parse_plugin_entry_dict() -> None: + spec = _parse_plugin_entry({"module": "x.y", "capability": "state-only"}) + assert spec == PluginSpec(module="x.y", capability="state-only") + + +def test_parse_plugin_entry_invalid_capability() -> None: + with pytest.raises(ValueError): + _parse_plugin_entry("m:wheee") + with pytest.raises(ValueError): + _parse_plugin_entry({"module": "m", "capability": "bogus"}) + with pytest.raises(ValueError): + _parse_plugin_entry({"capability": "full"})