fix(plugins): stop passing grant_capabilities= to loader that rejects it (#17420)

[mrveiss]

Closes #17420

Single-issue rationale: the natural batch-mate is #17421 (every discovered plugin imported and enabled at boot, no opt-in gate). #17421 also touches autobot_shared/plugin_sdk/loader.py and is blocked_by #17420 — its fix depends on the capability-grant delivery mechanism this PR establishes. Per the same-file rule (one agent, one PR) and the blocked_by edge, #17421 must land in its own PR after this one merges. Folding it here would violate the sequential-blockedby ordering and expand the loader-side change into a batch #17420 alone cannot honestly carry.

Thinking Path

The defect. autobot-backend/plugin_manager.py:271 called loader.load_plugin(manifest, plugin_config, grant_capabilities=auto_grant). autobot_shared/plugin_sdk/loader.py:320 defines the signature as load_plugin(self, manifest: PluginManifest, config: Dict | None = None) — no grant_capabilities parameter. Every manual plugin-load raised TypeError: load_plugin() got an unexpected keyword argument 'grant_capabilities' at runtime.

Two shapes were possible (per the issue): (A) make the loader accept and enforce the grant, or (B) stop passing the kwarg at the caller and grant through the API that already exists.

Evidence I read (bounded to the sites the issue names):

1. autobot_shared/plugin_sdk/capabilities.py:136 — CapabilityChecker.grant_capabilities(plugin_name, capabilities: List[Capability]) already exists as a singleton API and is what the enforcement path (capabilities.py:197, check()) reads from. Grants are in-memory only, but the mechanism is complete: there is no missing gate to invent.
2. autobot_shared/plugin_sdk/base.py:134 — PluginManifest.capabilities: List[Capability] — the manifest already declares exactly what would be granted, so the caller has everything needed to call grant_capabilities(name, manifest.capabilities) itself.
3. autobot-backend/plugin_manager.py:26 — CapabilityChecker is already imported in the caller module; the fix adds no new import.
4. Issue feat(plugins): plugin capability manifest — declare permissions and sandbox boundaries per plugin #9049 (closed as completed 2026-05-31) describes the policy — operator approves per install, official plugins may auto-grant, non-official plugins require explicit approval — not the plumbing. The plumbing chose CapabilityChecker as the grant surface; the caller just wasn't wired to it.

Chose (B): stop passing the kwarg, grant at the caller.

Rationale:

• The grant API is already implemented and used elsewhere. Adding a grant_capabilities: bool parameter to the loader would duplicate policy (bool → manifest.capabilities conversion) into a module whose job is loading, not policy. SRP wins.
• Narrower blast radius: touches only the file where the bug lives. No changes to plugin_sdk/loader.py at all — critical because security(plugins): every discovered plugin is imported and enabled at boot — no opt-in gate, no persisted enable state, no integrity check #17421 (blocked_by this issue) also touches loader.py and the same-file rule keeps that PR clean.
• No new abstraction; the fix is two lines of code that use an existing public method.

What (A) would have implied — a signature change on load_plugin (public API of the SDK), adding capability-policy branching inside the loader, and tests for the loader's new gate. That's a wider change to solve the same runtime symptom, and would put the loader and the caller in the same PR alongside #17421's concerns. Not chosen.

#9049 contract preserved: the manifest.trust_tier == TrustTier.OFFICIAL gate is unchanged; only the delivery mechanism moved from a nonexistent kwarg to CapabilityChecker().grant_capabilities(...). Non-official plugins still require operator approval via /approve-capabilities.

What Changed

• autobot-backend/plugin_manager.py — replaced the invalid grant_capabilities= kwarg with a post-load call to CapabilityChecker().grant_capabilities(manifest.name, manifest.capabilities) guarded by if manifest.trust_tier == TrustTier.OFFICIAL: (the auto_grant intermediate variable was removed to conserve lines — see below). Existing if not plugin: HTTPException short-circuits before the grant. No loader signature change; plugin_sdk/loader.py untouched.

  Line-count discipline (tech-debt(lint): the file-size hook's grandfather list has no ceiling — the three exempted files grew to 1.4x-6.8x the limit #14236 ratchet). plugin_manager.py is grandfathered at ceiling 702 (scripts/python_file_size_known_large.py:300). The functional fix wants 2 net lines (if …: + grant call). To keep the file at its recorded ceiling instead of raising it, three obsolete lines above the load call (the multi-line comment about "auto-granting" and the auto_grant = … intermediate variable) were removed and replaced with a single-line #9049 policy comment. The removed reasoning now lives in the commit message body, where git log -S "grant_capabilities" finds it. Final size: exactly 702 lines — matches the recorded ceiling, does not raise it.

• autobot-backend/tests/api/test_plugin_manager.py — added three tests covering the manual-load endpoint:

  1. test_load_plugin_endpoint_does_not_pass_grant_capabilities_kwarg — mutation guard, inspects loader.load_plugin.call_args.kwargs to fail loudly if the broken kwarg ever comes back. This is the direct regression sentinel.
  2. test_load_plugin_endpoint_auto_grants_official_plugin_capabilities — asserts CapabilityChecker.grant_capabilities is invoked exactly once with (plugin_name, manifest.capabilities) when trust_tier=OFFICIAL.
  3. test_load_plugin_endpoint_does_not_auto_grant_community_plugin — positive control: proves the auto-grant is gated on trust tier, not unconditional. Without this a "grants everything" regression would satisfy test Docs improvements #2.

Verification

Run 1 — fix in place, targeted tests only:

$ python3 -m pytest tests/api/test_plugin_manager.py -k "load_plugin_endpoint" -v
tests/api/test_plugin_manager.py::test_load_plugin_endpoint_does_not_pass_grant_capabilities_kwarg PASSED [ 33%]
tests/api/test_plugin_manager.py::test_load_plugin_endpoint_auto_grants_official_plugin_capabilities PASSED [ 66%]
tests/api/test_plugin_manager.py::test_load_plugin_endpoint_does_not_auto_grant_community_plugin PASSED [100%]
======================= 3 passed, 4 deselected in 0.38s ========================

Run 2 — mutation (broken call restored, grant_capabilities=auto_grant kwarg back in):

$ python3 -m pytest tests/api/test_plugin_manager.py -k "load_plugin_endpoint" -v
tests/api/test_plugin_manager.py::test_load_plugin_endpoint_does_not_pass_grant_capabilities_kwarg FAILED [ 33%]
tests/api/test_plugin_manager.py::test_load_plugin_endpoint_auto_grants_official_plugin_capabilities PASSED [ 66%]
tests/api/test_plugin_manager.py::test_load_plugin_endpoint_does_not_auto_grant_community_plugin PASSED [100%]

_______ test_load_plugin_endpoint_does_not_pass_grant_capabilities_kwarg _______
tests/api/test_plugin_manager.py:201: in test_load_plugin_endpoint_does_not_pass_grant_capabilities_kwarg
    assert "grant_capabilities" not in kwargs, (
E   AssertionError: loader.load_plugin does not accept grant_capabilities kwarg — passing it raises TypeError (#17420)
E   assert 'grant_capabilities' not in {'grant_capabilities': False}
================== 1 failed, 2 passed, 4 deselected in 0.45s ===================

The mutation guard fails for the intended reason — it observes {'grant_capabilities': False} in the kwargs the caller sent to the loader, which is exactly the shape the runtime would have raised TypeError on. The other two tests pass on mutation because MagicMock silently accepts any kwarg; a real PluginLoader would raise TypeError before returning, so the kwarg-inspection assertion is the honest sentinel — it catches the defect at its source (the call site) regardless of what the loader mock swallows.

Run 3 — fix restored, targeted tests:

======================= 3 passed, 4 deselected in 0.38s ========================

Ratchet check. plugin_manager.py is at 702 lines (wc -l after the slim edit); recorded ceiling in scripts/python_file_size_known_large.py:300 is 702. test_recorded_ceilings_match_the_files_today will now clear (it failed the earlier revision at 709; the ratchet-mandated fix was to shrink obsolete boilerplate rather than raise the ceiling — the ratchet exists to prevent exactly the latter, per #14236).

Full suite not run (per issue-batching economy). CI will exercise the wider surface.

Environment note: local Python is 3.11 with a partial dep set; the pytest header notes "environment is BELOW the declared dependency floors" for 6 of 209 declared versions (pyyaml, pyjwt, starlette, requests). None of these are transitive to plugin_manager.load_plugin or CapabilityChecker — the tests only touch pydantic models and unittest.mock. CI runs the declared floors and is the authoritative gate.

Scope boundary: autobot_shared/plugin_sdk/loader.py is deliberately untouched. #17421 (every discovered plugin imported and enabled at boot, no opt-in gate, no signature check) is blocked_by this issue and needs the loader-side work; the same-file rule keeps that as its own PR, not folded here.

Model Used

Claude Opus 4.7 (1M context) — model ID claude-opus-4-7[1m].

Summary by CodeRabbit

• Bug Fixes
  • Manual plugin loading now grants capabilities declared in a plugin’s manifest only when the plugin is official. Community plugins do not receive automatic capability grants through this flow. Failed plugin loads continue to return an error, and the change applies specifically to capability grants made during manual loading.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 9e4460fa-681e-434a-bd8d-c9a12b79a466

📥 Commits

Reviewing files that changed from the base of the PR and between d915197 and cfd18b7.

📒 Files selected for processing (1)

• autobot-backend/tests/api/test_plugin_manager.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The manual-load endpoint no longer passes an unsupported argument to the plugin loader. After a successful load, it grants manifest capabilities through CapabilityChecker only when the trust tier is OFFICIAL.

Changes

Manual plugin loading

Layer / File(s)	Summary
Plugin loading and capability grants autobot-backend/plugin_manager.py, autobot-backend/tests/api/test_plugin_manager.py	The endpoint calls the loader without grant_capabilities. Tests cover the loader call, capability grants for official plugins, and no automatic grants for community plugins.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: 🔵 Low · up to cfd18

A manually loaded community plugin can appear to have approved capabilities by declaring itself official. Those grants are not currently enforced in this repository, so the present risk is bounded to incorrect approval state; the PR is mergeable with that limitation understood.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to cfd18

Manual loading remains restricted to administrators, but the newly working grant can rely on a plugin’s own trust declaration and can outlive an unload. Both warrant review before the capability policy is relied on.

Retained concerns

• Medium · security · inferred: A plugin installed as community content can declare itself OFFICIAL in its manifest and receive automatic SDK capability grants when an administrator manually loads it. The new grant makes this pre-existing declarative trust boundary consequential on this path.
• Medium · security · inferred: An official plugin’s grant is not revoked by the inspected unload path. Capability checks remain keyed to the plugin name, so a later use of that identity can inherit authorization after unload; a configuration-save failure also leaves the grant in place.

Security review details

Security Blast Radius

• inferred — The demonstrated authorization effect is limited to SDK operations that consult the checker, but its grant table is shared by plugin name within the process. The evidence does not establish a sandbox around arbitrary plugin code.

Security Findings and Attack Paths

• inferred — If an administrator installs and loads a community-sourced package declaring OFFICIAL, its declared capabilities are automatically granted without the separate approval step. Installation and loading both require administrator authority; no unauthenticated path is established.
• inferred — After a granted plugin is unloaded, a capability check for the same name can still succeed because neither inspected unload path revokes the name’s grant. Whether code can exploit that state after unload or through name reuse was not demonstrated.

Trust Boundaries and Controls

• observed — Administrator authorization guards manual load and installation. The checker denies capabilities absent from its grant table, but the OFFICIAL grant decision reads the discovered manifest’s declared trust tier rather than verified install provenance.

Resilience and Maintainability Implications

• inferred — Saving configuration after granting creates a partial-failure state in which the request may fail while the loaded plugin and grant remain. No concurrent-load or recovery behavior was established.

Hardening Proposals

• proposed — Derive eligibility for automatic grants from verified plugin provenance, rather than a package-controlled trust_tier declaration.
• proposed — Define grant ownership across unload, reload, save failure, and name reuse, including revocation or rollback where the authorization should no longer apply.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely identifies the main change: removing the unsupported grant_capabilities argument from the plugin loader call.
Linked Issues check	✅ Passed	The PR satisfies the coding requirements in issue #17420. plugin_manager.py no longer passes the unsupported grant_capabilities keyword to PluginLoader.load_plugin. It grants declared capabiliti…
Out of Scope Changes check	✅ Passed	The changes stay within issue #17420. They modify the manual plugin-load path and add tests for the required loader call and capability-approval policy. No unrelated change is identified.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files.

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 60a8702a-99e3-4b04-b350-0e6e69dd95f0

📥 Commits

Reviewing files that changed from the base of the PR and between 5dcdb18 and 1fafe06.

📒 Files selected for processing (2)

• autobot-backend/plugin_manager.py
• autobot-backend/tests/api/test_plugin_manager.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

[coderabbitai]

Actionable comments posted: 2

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: db0cbb04-b797-4891-95ae-6965ebdbbc48

📥 Commits

Reviewing files that changed from the base of the PR and between 1fafe06 and d915197.

📒 Files selected for processing (2)

• autobot-backend/plugin_manager.py
• autobot-backend/tests/api/test_plugin_manager.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

[mrveiss]

Picked this up as unattended work and unblocked it — last agent push was 3h+ ago, only the branch-update bot since, no worktree and no ledger claim. Claimed worktree-issue-17420 in the ledger first. If this is live work, say so and I will hand it straight back.

Red root cause: code-quality → Black, one file, in this PR's diff.

would reformat .../autobot-backend/tests/api/test_plugin_manager.py
1 file would be reformatted, 5193 files would be left unchanged.

Fixed in cfd18b7fae — pure formatting (parenthesized with statements, adjacent-string joining), 16 insertions / 11 deletions, no assertion or test-name changes. Formatter is 26.3.1, byte-identical to the version code-quality.yml:203 pins, so this is exactly what the gate wants rather than a guess at it. Re-verified tree-wide with the same command CI runs: 5194 files would be left unchanged.

Pushed as a fast-forward on top of d915197dca. My first attempt rebased onto main and dropped the update bot's merge commit, which would have needed a force-push over someone else's branch — abandoned and replayed as a cherry-pick instead, so nothing on this branch was rewritten.

Both review threads resolved with a verdict, not a click. Neither blocks this PR, and the reason is the same for both: CapabilityChecker.check() has no call sites anywhere in the repo (#17459), so nothing consults a capability grant today.

• :277 — CWE-863 on unload is real code. revoke_capabilities (plugin_sdk/capabilities.py:154, docstring "on unload/uninstall") has zero callers, unload_plugin (plugin_sdk/loader.py:389-414) unregisters twice and returns without revoking, and CapabilityChecker is a singleton, so a grant outlives its plugin for the process lifetime. Unreachable now; live the moment enforcement is wired.
• :268 — grant-after-initialize() is real and latent for the same reason.

Both are carried to #17459 as acceptance criteria with a mutation-proof requirement, because they are one fix and not three: wiring check() without wiring revoke is precisely what would create the bypass. The review's suggested patch for :268 was not applied — it keys the auto-grant on manifest.trust_tier == TrustTier.OFFICIAL, and trust_tier is self-declared in the untrusted manifest, so moving the grant earlier while the tier is attacker-authored widens the window instead of closing it.