From 2561fca9efb166e8b6a807040b35149dfad9195a Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Sun, 14 Jun 2026 16:24:53 +0000 Subject: [PATCH] + executor middleware chain (permission + audit) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move tool execution behind one transparent entry point (ExecutorInterface) with a middleware onion (ChainExecutor), as ARCHITECTURE.md describes. The inline gatekeeper in Session::execute() becomes PermissionMiddleware; a new AuditMiddleware logs every tool call — including blocked/denied ones — to a new SQLite 'audit' table. Session now builds the chain from its existing deps and delegates; behaviour is unchanged (the existing SessionTest still passes through the chain). Adds SessionStore::logToolCall/auditTrail + the audit table. Timeout middleware and /stop cancellation are the deferred follow-up. 88 tests, PHPStan level 8, php-cs-fixer — all green. --- src/Exec/AuditMiddleware.php | 30 +++++++ src/Exec/ChainExecutor.php | 43 +++++++++ src/Exec/ExecutorInterface.php | 17 ++++ src/Exec/MiddlewareInterface.php | 21 +++++ src/Exec/PermissionMiddleware.php | 73 ++++++++++++++++ src/Session.php | 80 ++++++----------- src/Store/SessionStore.php | 45 ++++++++++ tests/Exec/AuditMiddlewareTest.php | 49 +++++++++++ tests/Exec/ChainExecutorTest.php | 80 +++++++++++++++++ tests/Exec/PermissionMiddlewareTest.php | 111 ++++++++++++++++++++++++ 10 files changed, 494 insertions(+), 55 deletions(-) create mode 100644 src/Exec/AuditMiddleware.php create mode 100644 src/Exec/ChainExecutor.php create mode 100644 src/Exec/ExecutorInterface.php create mode 100644 src/Exec/MiddlewareInterface.php create mode 100644 src/Exec/PermissionMiddleware.php create mode 100644 tests/Exec/AuditMiddlewareTest.php create mode 100644 tests/Exec/ChainExecutorTest.php create mode 100644 tests/Exec/PermissionMiddlewareTest.php diff --git a/src/Exec/AuditMiddleware.php b/src/Exec/AuditMiddleware.php new file mode 100644 index 0000000..f39aff5 --- /dev/null +++ b/src/Exec/AuditMiddleware.php @@ -0,0 +1,30 @@ +store?->logToolCall($call->summary(), $result->content, $result->isError); + + return $result; + } +} diff --git a/src/Exec/ChainExecutor.php b/src/Exec/ChainExecutor.php new file mode 100644 index 0000000..e43119b --- /dev/null +++ b/src/Exec/ChainExecutor.php @@ -0,0 +1,43 @@ + */ + private readonly array $middlewares; + + /** @var \Closure(ToolCall): ToolResultBlock */ + private readonly \Closure $terminal; + + /** + * @param list $middlewares outer first + * @param \Closure(ToolCall): ToolResultBlock $terminal resolves and runs the tool + */ + public function __construct(array $middlewares, \Closure $terminal) + { + $this->middlewares = $middlewares; + $this->terminal = $terminal; + } + + public function call(ToolCall $call): ToolResultBlock + { + $next = $this->terminal; + foreach (array_reverse($this->middlewares) as $middleware) { + $next = static fn (ToolCall $c): ToolResultBlock => $middleware->handle($c, $next); + } + + return $next($call); + } +} diff --git a/src/Exec/ExecutorInterface.php b/src/Exec/ExecutorInterface.php new file mode 100644 index 0000000..80540ff --- /dev/null +++ b/src/Exec/ExecutorInterface.php @@ -0,0 +1,17 @@ +tools->get($call->name); + } catch (ToolException) { + return $next($call); // unknown tool — let the terminal report it + } + + $verdict = $this->policy->check($tool, $call->input); + + if ($verdict->decision === Decision::Deny) { + return new ToolResultBlock($call->id, 'blocked: ' . $verdict->reason, true); + } + + if ($verdict->decision === Decision::Confirm && !$this->approved($call)) { + return new ToolResultBlock($call->id, 'denied by the user', true); + } + + return $next($call); + } + + /** A saved "always" rule skips the prompt; otherwise ask, and remember "always". */ + private function approved(ToolCall $call): bool + { + if ($this->store !== null && $this->store->isToolAllowed($call->name)) { + return true; + } + + return match ($this->conversation->confirm('Allow ' . $call->summary() . '?')) { + Approval::No => false, + Approval::Once => true, + Approval::Always => $this->remember($call->name), + }; + } + + private function remember(string $tool): bool + { + $this->store?->allowTool($tool); + + return true; + } +} diff --git a/src/Session.php b/src/Session.php index d8985dd..ed3a0f2 100644 --- a/src/Session.php +++ b/src/Session.php @@ -11,7 +11,6 @@ use Claw\Agent\ToolResultBlock; use Claw\Agent\ToolSpec; use Claw\Agent\ToolUseBlock; -use Claw\Chat\Approval; use Claw\Chat\ConversationInterface; use Claw\Chat\Status; use Claw\Exceptions\AgentException; @@ -20,10 +19,14 @@ use Claw\Exceptions\QuotaExceededException; use Claw\Exceptions\RateLimitException; use Claw\Exceptions\ToolException; -use Claw\Permission\Decision; +use Claw\Exec\AuditMiddleware; +use Claw\Exec\ChainExecutor; +use Claw\Exec\ExecutorInterface; +use Claw\Exec\PermissionMiddleware; use Claw\Permission\Policy; use Claw\Store\SessionStore; use Claw\Tool\Registry; +use Claw\Tool\ToolCall; use Claw\Tool\ToolInterface; /** @@ -45,6 +48,9 @@ final class Session */ private readonly array $specs; + /** Runs each tool call through the security/audit middleware chain. */ + private readonly ExecutorInterface $executor; + public function __construct( private readonly ConversationInterface $conversation, private readonly AgentInterface $agent, @@ -56,6 +62,16 @@ public function __construct( private readonly ?SessionStore $store = null, ) { $this->specs = $this->buildSpecs(); + + // Tool execution funnels through one chain: audit logs every call (even + // denials), permission gates it, and the terminal stage runs the tool. + $this->executor = new ChainExecutor( + middlewares: [ + new AuditMiddleware($this->store), + new PermissionMiddleware($this->policy, $this->tools, $this->conversation, $this->store), + ], + terminal: $this->runTool(...), + ); } /** Drive the conversation: each message is one task. Ends when it closes. */ @@ -171,63 +187,17 @@ private function persist(): void private function execute(ToolUseBlock $call): ToolResultBlock { - // The gatekeeper runs before the tool: a hard rule blocks it outright, a - // Mutating tool asks the user, Safe ones run straight through. A refusal - // (or block) is returned to the model as an error tool_result, so the - // agent simply continues without having done the action. - try { - $tool = $this->tools->get($call->name); - - $verdict = $this->policy->check($tool, $call->input); - if ($verdict->decision === Decision::Deny) { - return new ToolResultBlock($call->id, 'blocked: ' . $verdict->reason, true); - } - - if ($verdict->decision === Decision::Confirm && !$this->approved($call)) { - return new ToolResultBlock($call->id, 'denied by the user', true); - } - - return new ToolResultBlock($call->id, $tool->handle($call->input), false); - } catch (ToolException $e) { - return new ToolResultBlock($call->id, $e->getMessage(), true); - } + return $this->executor->call(new ToolCall($call->id, $call->name, $call->input)); } - /** - * Decide a Confirm: a saved "always" rule skips the prompt; otherwise ask the - * user. An "always" answer is remembered so we never ask for that tool again. - */ - private function approved(ToolUseBlock $call): bool + /** Terminal stage of the executor: resolve the tool and run it; failures become error results. */ + private function runTool(ToolCall $call): ToolResultBlock { - if ($this->store !== null && $this->store->isToolAllowed($call->name)) { - return true; - } - - return match ($this->conversation->confirm($this->confirmPrompt($call))) { - Approval::No => false, - Approval::Once => true, - Approval::Always => $this->remember($call->name), - }; - } - - /** Persist an "always allow" rule (if a store is configured) and allow the call. */ - private function remember(string $tool): bool - { - $this->store?->allowTool($tool); - - return true; - } - - /** A short, human-readable summary of a tool call for the approval prompt. */ - private function confirmPrompt(ToolUseBlock $call): string - { - $detail = ''; - if ($call->input !== []) { - $encoded = json_encode($call->input, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); - $detail = $encoded === false ? '' : ' ' . $encoded; + try { + return new ToolResultBlock($call->id, $this->tools->get($call->name)->handle($call->input), false); + } catch (ToolException $e) { + return new ToolResultBlock($call->id, $e->getMessage(), true); } - - return "Allow `{$call->name}`{$detail}?"; } /** diff --git a/src/Store/SessionStore.php b/src/Store/SessionStore.php index bbf3954..79831e0 100644 --- a/src/Store/SessionStore.php +++ b/src/Store/SessionStore.php @@ -36,6 +36,15 @@ public function __construct(string $path) ); // Persisted "always allow" rules: a row means the tool runs without asking. $this->pdo->exec('CREATE TABLE IF NOT EXISTS rules (name TEXT PRIMARY KEY)'); + // Audit log: one row per tool call (including blocked/denied ones). + $this->pdo->exec( + 'CREATE TABLE IF NOT EXISTS audit ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + call TEXT NOT NULL, + is_error INTEGER NOT NULL, + result TEXT NOT NULL + )', + ); } catch (\PDOException $e) { throw new ClawException('SessionStore: cannot open ' . $path . ': ' . $e->getMessage(), 0, $e); } @@ -99,6 +108,42 @@ public function allowTool(string $tool): void $stmt->execute(['name' => $tool]); } + /** Record one tool call (its summary), the outcome flag, and the result text. */ + public function logToolCall(string $call, string $result, bool $isError): void + { + $stmt = $this->pdo->prepare('INSERT INTO audit (call, is_error, result) VALUES (:call, :err, :result)'); + if ($stmt === false) { + throw new ClawException('SessionStore: failed to prepare audit insert'); + } + + $stmt->execute(['call' => $call, 'err' => $isError ? 1 : 0, 'result' => $result]); + } + + /** + * The full audit trail, oldest first. + * + * @return list + */ + public function auditTrail(): array + { + $stmt = $this->pdo->query('SELECT call, is_error, result FROM audit ORDER BY id'); + if ($stmt === false) { + return []; + } + + /** @var list $rows */ + $rows = $stmt->fetchAll(\PDO::FETCH_ASSOC); + + return array_map( + static fn (array $row): array => [ + 'call' => $row['call'], + 'isError' => $row['is_error'] !== 0, + 'result' => $row['result'], + ], + $rows, + ); + } + /** * @param list $content */ diff --git a/tests/Exec/AuditMiddlewareTest.php b/tests/Exec/AuditMiddlewareTest.php new file mode 100644 index 0000000..80f9e52 --- /dev/null +++ b/tests/Exec/AuditMiddlewareTest.php @@ -0,0 +1,49 @@ +handle( + new ToolCall('1', 'bash', ['command' => 'ls']), + static fn (ToolCall $c): ToolResultBlock => new ToolResultBlock($c->id, 'output', false), + ); + + $trail = $store->auditTrail(); + Assert::same(count($trail), 1); + Assert::true(str_contains($trail[0]['call'], 'bash')); + Assert::same($trail[0]['result'], 'output'); + Assert::false($trail[0]['isError']); + } finally { + @unlink($path); + } + } + + #[Test] + public function withoutAStoreItJustPassesThrough(): void + { + $result = (new AuditMiddleware(null))->handle( + new ToolCall('1', 'x', []), + static fn (ToolCall $c): ToolResultBlock => new ToolResultBlock($c->id, 'ran', false), + ); + + Assert::same($result->content, 'ran'); + } +} diff --git a/tests/Exec/ChainExecutorTest.php b/tests/Exec/ChainExecutorTest.php new file mode 100644 index 0000000..129c11b --- /dev/null +++ b/tests/Exec/ChainExecutorTest.php @@ -0,0 +1,80 @@ + $trace */ + $trace = new \ArrayObject(); + + $tracer = static function (string $tag) use ($trace): MiddlewareInterface { + return new class ($tag, $trace) implements MiddlewareInterface { + /** + * @param \ArrayObject $trace + */ + public function __construct(private string $tag, private \ArrayObject $trace) + { + } + + public function handle(ToolCall $call, callable $next): ToolResultBlock + { + $this->trace[] = "enter:{$this->tag}"; + $result = $next($call); + $this->trace[] = "exit:{$this->tag}"; + + return $result; + } + }; + }; + + $terminal = static function (ToolCall $call) use ($trace): ToolResultBlock { + $trace[] = 'terminal'; + + return new ToolResultBlock($call->id, 'done', false); + }; + + $result = (new ChainExecutor([$tracer('A'), $tracer('B')], $terminal)) + ->call(new ToolCall('1', 'x', [])); + + Assert::same($result->content, 'done'); + Assert::same((array) $trace, ['enter:A', 'enter:B', 'terminal', 'exit:B', 'exit:A']); + } + + #[Test] + public function aMiddlewareCanShortCircuitTheTerminal(): void + { + /** @var \ArrayObject $terminalRuns */ + $terminalRuns = new \ArrayObject(); + + $blocker = new class () implements MiddlewareInterface { + public function handle(ToolCall $call, callable $next): ToolResultBlock + { + return new ToolResultBlock($call->id, 'blocked', true); // never calls $next + } + }; + + $terminal = static function (ToolCall $call) use ($terminalRuns): ToolResultBlock { + $terminalRuns[] = 1; + + return new ToolResultBlock($call->id, 'ran', false); + }; + + $result = (new ChainExecutor([$blocker], $terminal))->call(new ToolCall('1', 'x', [])); + + Assert::same($result->content, 'blocked'); + Assert::true($result->isError); + Assert::same(count($terminalRuns), 0); + } +} diff --git a/tests/Exec/PermissionMiddlewareTest.php b/tests/Exec/PermissionMiddlewareTest.php new file mode 100644 index 0000000..5de47a3 --- /dev/null +++ b/tests/Exec/PermissionMiddlewareTest.php @@ -0,0 +1,111 @@ +registry('read', Risk::Safe), new FakeConversation()); + + $result = $mw->handle( + new ToolCall('1', 'read', []), + static fn (ToolCall $c): ToolResultBlock => new ToolResultBlock($c->id, 'ran', false), + ); + + Assert::same($result->content, 'ran'); + Assert::false($result->isError); + } + + #[Test] + public function dangerousToolIsBlockedWithoutRunning(): void + { + /** @var \ArrayObject $reached */ + $reached = new \ArrayObject(); + $mw = new PermissionMiddleware(new Policy(), $this->registry('eval', Risk::Dangerous), new FakeConversation()); + + $result = $mw->handle( + new ToolCall('1', 'eval', []), + static function (ToolCall $c) use ($reached): ToolResultBlock { + $reached[] = 1; + + return new ToolResultBlock($c->id, 'ran', false); + }, + ); + + Assert::true($result->isError); + Assert::true(str_contains($result->content, 'blocked')); + Assert::same(count($reached), 0); + } + + #[Test] + public function mutatingToolAsksAndDeniesOnNo(): void + { + $conversation = new FakeConversation(); + $conversation->confirmReplies = [Approval::No]; + $mw = new PermissionMiddleware(new Policy(), $this->registry('bash', Risk::Mutating), $conversation); + + $result = $mw->handle( + new ToolCall('1', 'bash', ['command' => 'ls']), + static fn (ToolCall $c): ToolResultBlock => new ToolResultBlock($c->id, 'ran', false), + ); + + Assert::true($result->isError); + Assert::true(str_contains($result->content, 'denied')); + Assert::same(count($conversation->confirmed), 1); + } + + private function registry(string $name, Risk $risk): Registry + { + $registry = new Registry(); + $registry->add(new class ($name, $risk) implements ToolInterface { + public function __construct( + private string $toolName, + private Risk $toolRisk, + ) { + } + + public function name(): string + { + return $this->toolName; + } + + public function description(): string + { + return ''; + } + + public function inputSchema(): array + { + return ['type' => 'object']; + } + + public function risk(): Risk + { + return $this->toolRisk; + } + + public function handle(array $input): string + { + return 'ran'; + } + }); + + return $registry; + } +}