Skip to content

PolicyGateExecutor/AdversarialPolicyGateExecutor do not forward requires_confirmation #5931

Description

@bug-ops

Description

Same defect class as #5906 (ScopedToolExecutor::requires_confirmation missing, fixed by #5930), but on two more wrapper layers that sit closer to the production chain's inner end.

ToolExecutor::requires_confirmation (crates/zeph-tools/src/executor.rs:820-822) defaults to false. Neither PolicyGateExecutor (crates/zeph-tools/src/policy_gate.rs, impl ToolExecutor block at lines 285-383) nor AdversarialPolicyGateExecutor (crates/zeph-tools/src/adversarial_gate.rs, impl ToolExecutor block at lines 186-246) override it — grepping both files for requires_confirmation returns zero matches in the impl blocks. TrustGateExecutor does override it correctly with real trust-check logic (crates/zeph-tools/src/trust_gate.rs:299-322).

Production wiring order (inside-out, per src/runner.rs): ShadowProbeExecutor → ScopedToolExecutor → PolicyGateExecutor → AdversarialPolicyGateExecutor → TrustGateExecutor → CompositeExecutor → .... ShadowProbeExecutor and ScopedToolExecutor both correctly forward requires_confirmation to self.inner (the latter fixed by #5906/#5930). But because PolicyGateExecutor and AdversarialPolicyGateExecutor sit between ScopedToolExecutor and TrustGateExecutor and don't forward the call, the speculative-dispatch engine querying requires_confirmation_erased on the fully composed top-level executor still gets false back whenever [tools.policy] or [tools.adversarial_policy] is enabled — the same production-recommended default configuration — regardless of what TrustGateExecutor's real policy would report.

Also, adversarial_gate.rs's module doc comment (around line 13) claims "Per CRIT-06: ALL ToolExecutor trait methods are delegated to self.inner" — this is stale/inaccurate; requires_confirmation and is_tool_speculatable (tracked separately in #5900) are both not delegated.

Currently dormant, not a live incident: per #5900's own finding, no leaf executor in this workspace currently sets is_tool_speculatable(tool_id) == true outside test code, so the speculative-dispatch path this feeds is not yet active for any real tool.

Found during the code-review/critique pass on PR #5930 (fix for #5899/#5905/#5906) while auditing the same requires_confirmation forwarding chain; kept out of scope for that PR by explicit choice to keep it strictly limited to the three originally grouped issues.

Reproduction Steps

  1. Configure [tools.policy] enabled = true (or [tools.adversarial_policy] enabled = true) with a trust policy that would set requires_confirmation to true for some tool call.
  2. Compose the executor chain as in production (PolicyGateExecutor/AdversarialPolicyGateExecutor wrapping TrustGateExecutor).
  3. Call requires_confirmation on the composed top-level executor for that tool call.
  4. Observe: returns false, ignoring the real policy from the wrapped TrustGateExecutor.

Expected Behavior

PolicyGateExecutor::requires_confirmation and AdversarialPolicyGateExecutor::requires_confirmation should delegate: self.inner.requires_confirmation(call).

Suggested Fix

Add to each impl block:

fn requires_confirmation(&self, call: &crate::executor::ToolCall) -> bool {
    self.inner.requires_confirmation(call)
}

Add regression tests mirroring the pattern used in #5930 (stub inner executor returning a distinguishable non-default value, assert the wrapper forwards it). Also correct the stale "ALL ToolExecutor trait methods are delegated" doc comment in adversarial_gate.rs.

Environment

Related

Metadata

Metadata

Assignees

Labels

P3Research — medium-high complexitybugSomething isn't working

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions