feat(plugins): add opt-in required pre-tool policies

[embwl0x]

Closes #61656

Summary

• add an opt-in plugins.entries.<plugin_id>.required_pre_tool_call contract for protected tool-name globs
• retain hook ownership and detailed callback outcomes so required plugins fail closed on missing registration, load failure, exceptions, and malformed or empty decisions
• preserve existing fail-open behavior for ordinary observer hooks and unprotected tools
• route strict approve decisions through the existing fail-closed human approval gate
• fail closed when an existing user or managed config is malformed, unreadable, or contains an invalid plugin-entry structure
• enforce the same policy gate for plugin-command PluginContext.dispatch_tool() calls
• document the operator configuration and decision contract

Security behavior

A matching required policy must be loaded and enabled, must own at least one pre_tool_call callback, and every owned callback must return an explicit allow, valid block, or valid approve decision. Failure responses are sanitized; callback, loader, and approval exception details are logged but not returned to the model.

An absent config remains valid. Once a config source exists, policy evaluation uses a strict read and does not substitute defaults or last-known-good data for malformed or unreadable current content.

All current dispatch paths converge on resolve_pre_tool_block, including direct model dispatch, sequential/concurrent executors, runtime helpers, tool-search resolved calls, and plugin-command dispatch.

Validation

• uv run pytest -q tests/plugins (1801 passed)
• focused plugin-dispatch and managed-scope regression suite (45 passed)
• broad plugin/config/managed-scope suite (344 passed)
• adjacent model-tool, runtime-guardrail, registry, dispatch-session, transform-hook, and shell/verify-hook suite (161 passed)
• uv run ruff check .
• uv run python scripts/check-windows-footguns.py --all (761 files clean)
• uv run python -m py_compile hermes_cli/config.py hermes_cli/managed_scope.py hermes_cli/plugins.py
• git diff --check

Tests use temporary Hermes homes and mocked approval/dispatch boundaries; no live providers, gateways, credentials, or MCP servers were used.

[teknium1]

Thanks for the focused opt-in policy contract. Current main confirms the reported premise: PluginManager.invoke_hook() logs and drops a raising callback result (hermes_cli/plugins.py:1913-1924), while directive resolution defaults to no directive when no valid result remains (hermes_cli/plugins.py:2143-2173). The PR’s required-policy branch validates plugin availability, ownership, callback failures, and explicit decisions before dispatch (hermes_cli/plugins.py:2301-2348 at 1016047d2e6c).

I found no concrete correctness or design-fit defect in the reviewed diff. The implementation extends the existing plugin hook/resolver surface, adds no core model tool or environment variable, and keeps ordinary hooks fail-open. GitHub CI is green, including Python e2e and all test slices.

Automated hermes-sweeper review.

[rmacbot]

Ship blocker: unreadable or malformed config disables the required policy

Exact head reviewed: 1016047d2e6cec6aabd8ab65facc84e9000d280a.

The new gate fails open when the configuration file cannot be parsed/read, or when plugins.entries has the wrong type:

• _required_pre_tool_policies() catches load_config_readonly() errors and returns an empty policy set (hermes_cli/plugins.py, added block around patch lines 208–217).
• Current load_config_readonly() itself delegates to _load_config_impl(), which logs parse/read failures and returns {} (hermes_cli/config.py:6675-6684).
• _required_pre_tool_policies() also returns an empty set when plugins.entries is not a dict.

That means a profile with a working required guard can be changed to malformed YAML, an unreadable config, or plugins: {entries: []} and the next protected tool call sees no configured policy and dispatches. This contradicts the PR documentation that malformed required-policy configuration blocks tool dispatch and leaves the Product Factory #47 boundary fail-open.

Smallest safe repair:

1. Use a strict config-read path that distinguishes an absent config from an existing unreadable/unparseable one.
2. Treat an existing invalid config, or a present malformed plugins.entries, as a global required-policy configuration failure until corrected; do not convert it to “no policy configured.”
3. Add dispatch-boundary regressions for malformed YAML, unreadable existing config, non-dict plugins.entries, and valid→malformed config replacement, asserting registry.dispatch is never reached.

The rest of the required-plugin ownership/callback/explicit-decision path looks well aligned with #61656, and current CI is green. This configuration-health gap is the remaining concrete fail-closed blocker I found.

[rmacbot]

Follow-up ship blocker: plugin commands bypass the gate entirely

I completed a second exact-head dispatch-path audit at 1016047d2e6cec6aabd8ab65facc84e9000d280a. There is another concrete bypass beyond the configuration-health issue above.

PluginContext.dispatch_tool() is the public tool-dispatch interface for plugin slash commands, but it calls registry.dispatch() directly:

• hermes_cli/plugins.py:604-619 documents the public dispatch surface.
• hermes_cli/plugins.py:620-631 reaches registry.dispatch() without calling resolve_pre_tool_block().
• tests/hermes_cli/test_plugins.py:2298-2312 currently asserts only that direct registry delegation occurs.
• This contradicts _resolve_pre_tool_block()'s “Single entry point for every tool-dispatch site” invariant at hermes_cli/plugins.py:2411-2423.

A plugin command can therefore invoke a protected mutating tool with zero required-policy evaluations, including in gateway mode. That alone prevents this head from satisfying #61656 or Product Factory #47.

Required repair and proof:

1. Route PluginContext.dispatch_tool() through the same centralized resolver before registry.dispatch(); preserve the existing blocked-result contract and approval semantics.
2. Add an exact dispatch-boundary regression with a configured required policy and a protected mutating tool, asserting the policy runs and registry.dispatch() remains untouched on block, callback failure, malformed result, unavailable plugin, and approval denial/error/timeout.
3. Exercise the plugin-command path in both CLI-context and gateway/no-_cli_ref context.

Combined verdict for this exact head: request changes. The required-policy core is promising, but configuration discovery and this public dispatch path still fail open.

[embwl0x]

@rmacbot Both requested follow-ups are now implemented in commit 4bbe7645ea.

• Required-policy config reads now distinguish absent files from malformed or unreadable existing user/managed config and fail closed on invalid plugin-entry structure. The valid-to-malformed transition is covered without serving last-known-good data to the policy gate.
• PluginContext.dispatch_tool() now passes through resolve_pre_tool_block before registry dispatch. The CLI/gateway matrix covers explicit block, callback failure, malformed result, unavailable required plugin, and approval denial/error/timeout; every case asserts the registry sink is untouched and error details are sanitized.

Validation includes tests/plugins (1,801 passed), the focused dispatch/managed-scope suite (45 passed), broader config/plugin coverage (344 passed), adjacent dispatch/runtime/shell-hook coverage (161 passed), full ruff, Windows-footgun scan (761 files clean), py_compile, and diff checks. No live Hermes instance, provider, credential, gateway, or MCP server was used.

Addresses:

• feat(plugins): add opt-in required pre-tool policies #61902 (comment)
• feat(plugins): add opt-in required pre-tool policies #61902 (comment)

[embwl0x]

Refreshed onto current main (d9f1043c3) in ae968316e. The resolution preserves current provider-enabled config coverage while retaining the required-policy fail-closed loader and dispatch behavior. Reverified: 370 focused plugin/config/model-tool tests passed, Ruff passed, publish gate and gitleaks passed.

[embwl0x]

Rebased onto current main (dd51931bf) and resolved the two test-pruning conflicts in 1766888946. The resolution preserves current-main test consolidation while retaining the required-policy dispatch-boundary and strict valid-to-malformed config regressions.

Validation on the rebased head:

• focused plugin/config/model-tool suite: 160 passed
• isolated agent-loop guardrail and sequential/concurrent dispatch coverage: 18 passed
• shell/verify/background-review consumers: 26 passed before unrelated files in that matrix hit a sandbox log-path failure; those agent-loop consumers were then rerun with an explicit temporary HERMES_HOME
• expanded tests/plugins: 1,024 passed; 7 failures in untouched Hindsight, Photon, and FAL video tests reproduced identically on an untouched origin/main worktree at exact base dd51931bf
• Ruff, py_compile, git diff --check, publish gate, and gitleaks passed
• replacement GitHub CI passed: all eight Python slices, Python e2e, Desktop E2E, both Docker architectures, lint/security gates, and the aggregate required-check job

GitHub reports MERGEABLE / CLEAN; the conflict signal is cleared.

[embwl0x]

Rebased onto current main (36cb5ae553) and resolved the hermes_cli/plugins.py lifecycle-dispatch conflict in 5f7413dfa.

The resolution preserves current main's first-party lifecycle observation for every pre-tool call, including required-policy calls, while retaining ownership-aware detailed plugin outcomes for explicitly protected tools. Ordinary hooks still use the centralized fail-open lifecycle path. A regression now proves required policies emit the first-party observation exactly once.

Validation on the rebased head:

• plugin/config/managed-scope/lifecycle/model-tool suite: 171 passed
• adjacent shell, verify, Relay-observability, background-review, and guardrail consumers: 73 passed
• full tests/run_agent/test_run_agent.py: 238 passed after installing its locked optional Anthropic extra
• focused Ruff, git diff --check, publish gate, and gitleaks passed

GitHub now reports the PR MERGEABLE; replacement CI is running.

[embwl0x]

Rebased onto current main at 9460cc1 and composed the plugin conflict with main's central ownership ledger, callback payload filtering, managed-scope dispatch, and lifecycle observation. Required policies remain owner-aware and fail closed only for explicitly protected tools.\n\nNew head: 0982e03ba\n\nValidation:\n- broad plugin/hook/dispatch suite: 311 tests passed\n- final exact-base focused suite: 182 tests passed\n- Ruff, byte-compilation, diff check, publish gate, and gitleaks passed\n- replacement GitHub CI passed all required checks; GitHub reports MERGEABLE.

[embwl0x]

Rebased onto current main (f4c80e424) and resolved the plugin-dispatch conflict in 0cefe262c.

The resolution composes required-policy ownership and fail-closed behavior with upstream's argument-modifying hooks and centralized dispatch helper. Required policies are validated first; ordinary modifiers still accumulate; both the public resolver and every centralized execution sink fail closed on a resolver crash only when the tool is explicitly protected. New regressions cover a required allow-policy coexisting with another plugin's argument modification and protected versus unprotected dispatch crashes.

Validation on the final base: 200 focused plugin/config/model-tool tests, 156 adjacent execution-path tests, and all 258 full run-agent tests passed. Ruff, diff checks, public identity, overlap review, gitleaks, and the existing-PR publish gate passed. Replacement GitHub CI also passed completely, including all 12 Python shards, lint/security/attribution gates, and both Docker architectures. GitHub reports the PR mergeable.