Skip to content

Commit 79b8d64

Browse files
ericdalloericdallo
committed
Improve tool call approve to check tool full-name instead of name
1 parent 3aa8b23 commit 79b8d64

File tree

4 files changed

+132
-136
lines changed

4 files changed

+132
-136
lines changed

‎src/eca/features/chat.clj‎

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -495,14 +495,8 @@
495495

496496
{:status status :actions actions}))
497497

498-
(defn ^:private tool-full-name->origin [full-name all-tools]
499-
(:origin (first (filter #(= full-name (:full-name %)) all-tools))))
500-
501-
(defn ^:private tool-full-name->server [full-name all-tools]
502-
(:server (first (filter #(= full-name (:full-name %)) all-tools))))
503-
504-
(defn ^:private tool-full-name->name [full-name all-tools]
505-
(:name (first (filter #(= full-name (:full-name %)) all-tools))))
498+
(defn ^:private tool-by-full-name [full-name all-tools]
499+
(first (filter #(= full-name (:full-name %)) all-tools)))
506500

507501
(defn ^:private tokenize-args [^String s]
508502
(if (string/blank? s)
@@ -628,13 +622,14 @@
628622
(finish-chat-prompt! :idle chat-ctx))))
629623
:on-prepare-tool-call (fn [{:keys [id full-name arguments-text]}]
630624
(assert-chat-not-stopped! chat-ctx)
631-
(transition-tool-call! db* chat-ctx id :tool-prepare
632-
{:name (tool-full-name->name full-name all-tools)
633-
:full-name full-name
634-
:server (:name (tool-full-name->server full-name all-tools))
635-
:origin (tool-full-name->origin full-name all-tools)
636-
:arguments-text arguments-text
637-
:summary (f.tools/tool-call-summary all-tools full-name nil config)}))
625+
(let [tool (tool-by-full-name full-name all-tools)]
626+
(transition-tool-call! db* chat-ctx id :tool-prepare
627+
{:name (:name tool)
628+
:server (:name (:server tool))
629+
:full-name full-name
630+
:origin (:origin tool)
631+
:arguments-text arguments-text
632+
:summary (f.tools/tool-call-summary all-tools full-name nil config)})))
638633
:on-tools-called (fn [tool-calls]
639634
;; If there are multiple tool calls, they are allowed to execute concurrently.
640635
(assert-chat-not-stopped! chat-ctx)
@@ -646,12 +641,13 @@
646641
(run! (fn do-tool-call [{:keys [id full-name arguments] :as tool-call}]
647642
(let [approved?* (promise) ; created here, stored in the state.
648643
db @db*
644+
tool (tool-by-full-name full-name all-tools)
649645
hook-approved?* (atom true)
650-
origin (tool-full-name->origin full-name all-tools)
651-
name (tool-full-name->name full-name all-tools)
652-
server (tool-full-name->server full-name all-tools)
646+
origin (:origin tool)
647+
name (:name tool)
648+
server (:server tool)
653649
server-name (:name server)
654-
approval (f.tools/approval all-tools name arguments db config behavior)
650+
approval (f.tools/approval all-tools tool arguments db config behavior)
655651
ask? (= :ask approval)
656652
details (f.tools/tool-call-details-before-invocation name arguments server db ask?)
657653
summary (f.tools/tool-call-summary all-tools full-name arguments config)]

‎src/eca/features/tools.clj‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -187,11 +187,10 @@
187187
(defn approval
188188
"Return the approval keyword for the specific tool call: ask, allow or deny.
189189
Behavior parameter is required - pass nil for global-only approval rules."
190-
[all-tools tool-call-name args db config behavior]
191-
(let [remember-to-approve? (get-in db [:tool-calls tool-call-name :remember-to-approve?])
192-
native-tools (filter #(= "eca" (:name (:server %))) all-tools)
193-
{:keys [server require-approval-fn]} (first (filter #(= tool-call-name (:name %))
194-
all-tools))
190+
[all-tools tool args db config behavior]
191+
(let [{:keys [server name require-approval-fn]} tool
192+
remember-to-approve? (get-in db [:tool-calls name :remember-to-approve?])
193+
native-tools (filter #(= :native (:origin %)) all-tools)
195194
{:keys [allow ask deny byDefault]} (merge (get-in config [:toolCall :approval])
196195
(get-in config [:behavior behavior :toolCall :approval]))]
197196
(cond
@@ -201,16 +200,16 @@
201200
remember-to-approve?
202201
:allow
203202

204-
(some #(approval-matches? % (:name server) tool-call-name args native-tools) deny)
203+
(some #(approval-matches? % (:name server) name args native-tools) deny)
205204
:deny
206205

207-
(some #(approval-matches? % (:name server) tool-call-name args native-tools) ask)
206+
(some #(approval-matches? % (:name server) name args native-tools) ask)
208207
:ask
209208

210-
(some #(approval-matches? % (:name server) tool-call-name args native-tools) allow)
209+
(some #(approval-matches? % (:name server) name args native-tools) allow)
211210
:allow
212211

213-
(legacy-manual-approval? config tool-call-name)
212+
(legacy-manual-approval? config name)
214213
:ask
215214

216215
(= "ask" byDefault)

‎test/eca/features/tools/shell_test.clj‎

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33
[babashka.fs :as fs]
44
[babashka.process :as p]
55
[clojure.test :refer [are deftest is testing]]
6-
[eca.config :as config]
7-
[eca.features.tools :as f.tools]
86
[eca.features.tools.shell :as f.tools.shell]
97
[eca.test-helper :as h]
108
[matcher-combinators.test :refer [match?]]))
@@ -137,57 +135,3 @@
137135
"pwd"
138136
"date"
139137
"env")))
140-
141-
(deftest plan-mode-approval-restrictions-test
142-
(let [all-tools [{:name "shell_command" :server {:name "eca"}}]
143-
config config/initial-config]
144-
145-
(testing "dangerous commands blocked in plan mode via approval"
146-
(are [command] (= :deny
147-
(f.tools/approval all-tools "shell_command"
148-
{"command" command} {} config "plan"))
149-
"echo 'test' > file.txt"
150-
"cat file.txt > output.txt"
151-
"ls >> log.txt"
152-
"rm file.txt"
153-
"mv old.txt new.txt"
154-
"cp file1.txt file2.txt"
155-
"touch newfile.txt"
156-
"mkdir newdir"
157-
"sed -i 's/old/new/' file.txt"
158-
"git add ."
159-
"git commit -m 'test'"
160-
"npm install package"
161-
"python -c \"open('file.txt','w').write('test')\""
162-
"bash -c 'echo test > file.txt'"))
163-
164-
(testing "non-dangerous commands default to ask in plan mode"
165-
(are [command] (= :ask
166-
(f.tools/approval all-tools "shell_command"
167-
{"command" command} {} config "plan"))
168-
"python --version" ; not matching dangerous patterns, defaults to ask
169-
"node script.js" ; not matching dangerous patterns, defaults to ask
170-
"clojure -M:test")) ; not matching dangerous patterns, defaults to ask
171-
172-
(testing "safe commands not denied in plan mode"
173-
(are [command] (not= :deny
174-
(f.tools/approval all-tools "shell_command"
175-
{"command" command} {} config "plan"))
176-
"git status"
177-
"ls -la"
178-
"find . -name '*.clj'"
179-
"grep 'test' file.txt"
180-
"cat file.txt"
181-
"head -10 file.txt"
182-
"pwd"
183-
"date"
184-
"env"))
185-
186-
(testing "same commands work fine in agent mode (not denied)"
187-
(are [command] (not= :deny
188-
(f.tools/approval all-tools "shell_command"
189-
{"command" command} {} config "agent"))
190-
"echo 'test' > file.txt"
191-
"rm file.txt"
192-
"git add ."
193-
"python --version"))))

0 commit comments

Comments
 (0)