Skip to content

Proposal: Per-call risk gating for built-in shell/filesystem tools #96

Description

@JackChen-me

Motivation

A real integration surfaced a gap in our tool model.

xeloxa/temodar-agent is an AI-powered WordPress security analyzer built on @jackchen_me/open-multi-agent. It wires up most of the framework's orchestration surface (Agent, AgentPool, OpenMultiAgent, runTeam, runTasks, ToolRegistry, ToolExecutor, the built-in tools, defineTool, onProgress, onTrace, beforeRun/afterRun, outputSchema, loopDetection). On top of it, the integrator hand-rolled a per-call risk gate (classifyBashRisk / classifyToolRisk) that sits inside every tool's execute function via a wrapToolWithEvents helper.

The gate classifies each invocation as safe | review | high:

  • safe — read-only commands (ls, cat, grep) auto-pass
  • review — ambiguous invocations request human confirmation
  • high — destructive/sensitive patterns (rm, sudo, curl | bash) block by default

In a follow-up discussion (xeloxa/temodar-agent#9 comment) the author articulated the underlying problem in their own words:

OMA's tool system solves "which tools can this agent access?", but we also needed a separate check for "should this specific invocation be allowed right now?". That mattered especially for broad tools like bash, where ls and pwd are very different from rm, sudo, or networked commands.

And, on whether this belongs in the framework:

per-call risk/approval feels like a separate concern that could make sense as a native pattern, especially for shell and filesystem tools.

This is a concrete signal from a production user that the framework has no seam for call-level gating, and that users end up monkey-patching every tool definition to get one.

Problem

Today the framework has two layers of tool control:

  1. Allowlist / denylist / preset (feat: add tool allowlist, denylist, preset list #83, merged) — decides which tools an agent may call. Operates on tool names.
  2. Nothing at the call site — once a tool is allowed, every invocation runs. bash is a single allowed name that covers ls -la and rm -rf / equally.

The integrator's workaround is to wrap every ToolDefinition.execute at registration time and inline their own classifier + approval loop (reference). This works but is invasive: it couples risk logic to tool wiring, has to be repeated per tool, and cannot compose with onTrace or future observability cleanly.

The goal of this proposal is to add a minimal, opt-in seam one layer down from #83 so that call-level policies can be written in user code without wrapping tool definitions.

Proposal

Three pieces, all opt-in, all off by default.

1. onToolCall middleware hook

A single async callback that fires once per tool invocation, after Zod validation, before tool.execute() runs. Natural insertion point is in ToolExecutor.runTool() (src/tool/executor.ts, between lines 147 and 158).

Rough shape (follows the existing beforeRun / afterRun style of sync-or-async):

interface ToolCallContext {
  readonly toolName: string
  readonly input: Record<string, unknown>  // post-schema-validation
  readonly agentName: string
  readonly runId?: string
  readonly taskId?: string
}

type ToolCallDecision =
  | { action: 'allow' }
  | { action: 'deny'; reason?: string }

// New hook, added alongside existing ones (NOT to be confused with
// the task-level OrchestratorConfig.onApproval, which fires between
// orchestration rounds, not per tool call).
readonly onToolCall?: (
  context: ToolCallContext
) => ToolCallDecision | Promise<ToolCallDecision>

On deny the executor synthesizes a standard error ToolResult (same path as existing validation failures) carrying the reason string. No exception is thrown — the LLM sees a structured refusal and can adapt.

Human-in-the-loop is handled inside the user's callback. They await their own UI / CLI / file-based prompt, then return allow or deny. The framework does not prescribe a review state or a separate approval channel; that keeps the API surface small and lets users plug in whatever review mechanism suits their app.

2. Optional shell risk classifier

A small helper module exported separately, off by default, for users who want the xeloxa safe | review | high semantics without writing regex tables from scratch. Rough shape:

import { classifyBashCommand } from 'open-multi-agent/classifiers'

orchestrator = new OpenMultiAgent({
  onToolCall: async (ctx) => {
    if (ctx.toolName !== 'bash') return { action: 'allow' }
    const risk = classifyBashCommand(ctx.input.command as string)
    if (risk.level === 'safe') return { action: 'allow' }
    const approved = await myAppUI.requestApproval(ctx, risk)
    return approved ? { action: 'allow' } : { action: 'deny', reason: risk.reason }
  }
})

Implementation lands independently and uses pattern lists written from scratch (no code copied from temodar-agent, which is Apache-2.0). Users can extend or fully replace it; it is convenience, not policy.

3. Trace integration

Extend ToolCallTrace with optional fields so gate decisions flow through onTrace for observability:

interface ToolCallTrace extends TraceEventBase {
  readonly type: 'tool_call'
  readonly tool: string
  readonly isError: boolean
  readonly gated?: boolean         // true when onToolCall ran
  readonly gateAction?: 'allow' | 'deny'
  readonly gateReason?: string
}

Scope

In:

  • New onToolCall hook on OrchestratorConfig (and likely AgentConfig — see open questions)
  • Insertion point in ToolExecutor.runTool()
  • Optional classifier helper for bash patterns, shipped behind a subpath export
  • ToolCallTrace extension for observability
  • Documentation showing the xeloxa-style use case end-to-end

Out:

  • A general policy engine (rate limits, quotas, RBAC, per-user scoping). If someone needs those they can build them inside the hook.
  • Changes to feat: add tool allowlist, denylist, preset list #83's allowlist/denylist resolution. That layer answers "is this tool reachable"; this proposal answers "is this call allowed". Both layers coexist unchanged.
  • MCP-specific gating (MCP integration: connect to external tool servers via optional peer dependency #86). The hook sits at ToolExecutor so MCP tools would benefit automatically once they route through the same executor, but no MCP-specific work is in scope here.
  • Risk gating limited to built-ins. Because the middleware lives at ToolExecutor.runTool(), user-defined tools pass through the same hook for free. This is a nice side-effect, not the primary goal.

Non-goals

  • This is not a sandbox or security boundary. A middleware that returns deny still relies on cooperating code. If the threat model requires real isolation, use Docker / a VM / seccomp — the hook is a coordination layer, not containment.
  • Not prescriptive. The primary API is the raw hook. The shell classifier is convenience only and sits behind a secondary export.
  • Not a replacement for onApproval. That hook gates between orchestrator task rounds and stays as-is. The two operate at orthogonal layers.

Open questions

  1. Scope of the hook. OrchestratorConfig, AgentConfig, or both with agent override? Both would let a team default the policy while one specialist agent relaxes it. Precedent supports both styles (onApproval is orchestrator-level, beforeRun/afterRun are agent-level).
  2. Single hook or array of middleware. Array composes better (classify + log + audit), single is simpler to document. Suggest starting with single, add array later if the need is real.
  3. Classifier packaging. Core (open-multi-agent/classifiers subpath export) or separate npm package? Core keeps the install surface small; separate package avoids bloating the core with pattern tables.
  4. Abort on deny vs error result. Deny synthesizing an error ToolResult lets the LLM recover and retry, which is usually the right thing. An alternative would be to abort the whole run; probably too blunt as the default but worth noting.
  5. Trace semantics. Should gated: true, gateAction: 'allow' emit even on pass-through (for audit logs), or only on deny (to keep trace volume low)?
  6. Interaction with disallowedTools. A denied tool should never reach the hook (current behavior); the hook only runs for tools already resolved as reachable. Worth documenting explicitly.

References

Status

Candidate for a future v1.x minor. Happy to split implementation across:

  1. Core hook + executor integration + trace field (small PR)
  2. Classifier helper + subpath export (independent follow-up)
  3. Docs + example (independent follow-up)

No timeline commitment.

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Community contributions welcome — good first issuesenhancementNew feature or requestsource:feedbackSource: external user feedback (GitHub/Twitter/Reddit/Discord/forks)

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions