Merge pull request #95 from editor-code-assistant/improve-approval

Add toolCall approval

Author: ericdallo

CHANGELOG.md

@@ -20,6 +20,8 @@
 
 - Fix command prompts to allow args with spaces between quotes.
 - Fix anthropic token renew when expires.
+- Considerably improve toolCall approval / permissions config.
+  - Now with thave multiple optiosn to ask or allow tool calls, check config section.
 
 ## 0.38.1
 

docs/configuration.md

@@ -68,9 +68,77 @@ For MCP servers configuration, use the `mcpServers` config, example:
 }
 ```
 
-### Manual approval
-
-By default, ECA auto approve any tool call from LLM, to configure that or for which tools, check `toolCall manualApproval` config or try the `plan` behavior before.
+### Tool approval / permissions
+
+By default, ECA ask to call any tool, but that's can easily be configureed in many ways via the `toolCall approval` config
+
+Check some examples:
+
+=== "Allow any tools by default"
+
+   ```javascript
+   {
+     "toolCall": {
+       "approval": {
+         "byDefault": "allow"
+       }
+     }
+   }
+   ```
+
+=== "Allow all but some tools"
+
+   ```javascript
+   {
+     "toolCall": {
+       "approval": {
+         "byDefault": "allow",
+         "ask": {
+           "eca_editfile": {},
+           "my-mcp__my_tool": {}
+         }
+       }
+     }
+   }
+   ```
+
+=== "Ask all but all tools from some mcps"
+
+   ```javascript
+   {
+     "toolCall": {
+       "approval": {
+         // "byDefault": "ask", not needed as it's eca default
+         "allow": {
+           "eca": {},
+           "my-mcp": {}
+         }
+       }
+     }
+   }
+   ```
+   
+=== "Matching by a tool argument"
+
+  __`argsMatchers`__ is a map of argument name by list of [java regex](https://www.regexplanet.com/advanced/java/index.html).
+
+   ```javascript
+   {
+     "toolCall": {
+       "approval": {
+         "byDefault": "allow",
+         "allow": {
+           "eca_shell_command": {"argsMatchers" {"command" [".*rm.*",
+                                                            ".*mv.*"]}}
+         }
+       }
+     }
+   }
+   ```
+
+Also check the `plan` behavior which is safer.
+
+__The `manualApproval` setting was deprecated and replaced by the `approval` one without breaking changes__
 
 ## Custom command prompts
 
@@ -181,7 +249,11 @@ There are 3 possible ways to configure rules following this order of priority:
         };
         disabledTools?: string[],
         toolCall?: {
-          manualApproval?: boolean | string[], // manual approve all tools or the specified tools
+          approval?: {
+            byDefault: 'ask' | 'allow';
+            allow?: {{key: string}: {argsMatchers?: {{[key]: string}: string[]}}},
+            ask?: {{key: string}: {argsMatchers?: {{[key]: string}: string[]}}},
+          };
         };
         mcpTimeoutSeconds?: number;
         lspTimeoutSeconds?: number;
@@ -225,7 +297,11 @@ There are 3 possible ways to configure rules following this order of priority:
                        "editor": {"enabled": true}},
       "disabledTools": [],
       "toolCall": {
-        "manualApproval": null,
+        "approval": {
+          "byDefault": "ask",
+          "allow": {},
+          "ask": {}
+        }
       },
       "mcpTimeoutSeconds" : 60,
       "lspTimeoutSeconds" : 30,

docs/features.md

@@ -20,9 +20,9 @@ Behavior affect the prompt passed to LLM and the tools to include, the current s
 ECA leverage tools to give more power to the LLM, this is the best way to make LLMs have more context about your codebase and behave like an agent.
 It supports both MCP server tools + ECA native tools.
 
-!!! warning "Automatic approval"
+!!! info "Approval / permissions"
 
-    By default, ECA auto approve any tool call from LLM, to configure that or for which tools, check `toolCall manualApproval` config or try the `plan` behavior.
+    By default, ECA ask to approve any tool, you can easily configure that, check `toolCall approval` [config](./config.md) or try the `plan` behavior.
 
 ### Native tools
 

integration-test/integration/fixture.clj

@@ -8,6 +8,7 @@
   (str "http://localhost:" llm-mock.server/port))
 
 (def default-init-options {:pureConfig true
+                           :toolCall {:approval {:byDefault "allow"}}
                            :providers {"openai" {:url (str base-llm-mock-url "/openai")
                                                  :key "foo-key"
                                                  :keyEnv "FOO"}

src/eca/config.clj

@@ -6,14 +6,14 @@
   3. local config-file: searching from a local `.eca/config.json` file.
   4. `initializatonOptions` sent in `initialize` request."
   (:require
-   [camel-snake-kebab.core :as csk]
    [cheshire.core :as json]
    [cheshire.factory :as json.factory]
    [clojure.core.memoize :as memoize]
    [clojure.java.io :as io]
    [clojure.string :as string]
    [eca.logger :as logger]
-   [eca.shared :as shared])
+   [eca.shared :as shared]
+   [camel-snake-kebab.core :as csk])
   (:import
    [java.io File]))
 
@@ -57,6 +57,9 @@
                          :excludeCommands []}
                  :editor {:enabled true}}
    :disabledTools []
+   :toolCall {:approval {:byDefault "ask"
+                         :allow {}
+                         :ask {}}}
    :mcpTimeoutSeconds 60
    :lspTimeoutSeconds 30
    :mcpServers {}
@@ -75,7 +78,7 @@
   (try
     (binding [json.factory/*json-factory* (json.factory/make-json-factory
                                            {:allow-comments true})]
-      (json/parse-string raw-string true))
+      (json/parse-string raw-string))
     (catch Exception e
       (logger/warn "Error parsing config json:" (.getMessage e)))))
 
@@ -126,47 +129,67 @@
 
 (def ollama-model-prefix "ollama/")
 
-(defn ^:private normalize-providers [providers]
-  (letfn [(norm-key [k]
-            (csk/->kebab-case (string/replace-first (str k) ":" "")))]
-    (reduce-kv (fn [m k v]
-                 (let [nk (norm-key k)]
-                   (if (contains? m nk)
-                     (update m nk #(deep-merge % v))
-                     (assoc m nk v))))
-               {}
-               providers)))
-
-(defn ^:private normalize-provider-models [provider]
-  (let [models (or (:models provider)
-                   (get provider "models"))
-        models' (when models
-                  (into {}
-                        (map (fn [[k v]]
-                               [(if (or (keyword? k) (symbol? k))
-                                  (string/replace-first (str k) ":" "")
-                                  (str k))
-                                v])
-                             models)))
-        provider' (dissoc provider "models")]
-    (if models'
-      (assoc provider' :models models')
-      provider')))
-
-(defn ^:private normalize-fields [config]
-  (-> config
-      (update-in [:providers]
-                 (fn [providers]
-                   (when providers
-                     (-> (normalize-providers providers)
-                         (update-vals normalize-provider-models)))))))
+(defn ^:private normalize-fields
+  "Converts a deep nested map where keys are strings to keywords.
+   normalization-rules follow the nest order, :ANY means any field name.
+    :kebab-case means convert field names to kebab-case.
+    :stringfy means convert field names to strings."
+  [normalization-rules m]
+  (let [kc-paths (set (:kebab-case normalization-rules))
+        str-paths (set (:stringfy normalization-rules))
+        ; match a current path against a rule path with :ANY wildcard
+        matches-path? (fn [rule-path cur-path]
+                        (and (= (count rule-path) (count cur-path))
+                             (every? true?
+                                     (map (fn [rp cp]
+                                            (or (= rp :ANY)
+                                                (= rp cp)))
+                                          rule-path cur-path))))
+        applies? (fn [paths cur-path]
+                   (some #(matches-path? % cur-path) paths))
+        normalize-map (fn normalize-map [cur-path m*]
+                        (cond
+                          (map? m*)
+                          (let [apply-kebab? (applies? kc-paths cur-path)
+                                apply-string? (applies? str-paths cur-path)]
+                            (into {}
+                                  (map (fn [[k v]]
+                                         (let [base-name (cond
+                                                           (keyword? k) (name k)
+                                                           (string? k) k
+                                                           :else (str k))
+                                               kebabed (if apply-kebab?
+                                                         (csk/->kebab-case base-name)
+                                                         base-name)
+                                               new-k (if apply-string?
+                                                       kebabed
+                                                       (keyword kebabed))
+                                               new-v (normalize-map (conj cur-path new-k) v)]
+                                           [new-k new-v])))
+                                  m*))
+
+                          (sequential? m*)
+                          (mapv #(normalize-map cur-path %) m*)
+
+                          :else m*))]
+    (normalize-map [] m)))
 
 (defn all [db]
   (let [initialization-config @initialization-config*
         pure-config? (:pureConfig initialization-config)]
-    (normalize-fields
-     (deep-merge initial-config
-                 initialization-config
-                 (when-not pure-config? (config-from-envvar))
-                 (when-not pure-config? (config-from-global-file))
-                 (when-not pure-config? (config-from-local-file (:workspace-folders db)))))))
+    (deep-merge initial-config
+                (normalize-fields
+                 {:kebab-case
+                  [[:providers]]
+                  :stringfy
+                  [[:providers]
+                   [:providers :ANY :models]
+                   [:toolCall :approval :allow]
+                   [:toolCall :approval :allow :ANY :argsMatchers]
+                   [:toolCall :approval :ask]
+                   [:toolCall :approval :ask :ANY :argsMatchers]
+                   [:mcpServers]]}
+                 (deep-merge initialization-config
+                             (when-not pure-config? (config-from-envvar))
+                             (when-not pure-config? (config-from-global-file))
+                             (when-not pure-config? (config-from-local-file (:workspace-folders db))))))))

src/eca/features/tools.clj

@@ -36,7 +36,7 @@
             f.tools.editor/definitions))))
 
 (defn ^:private native-tools [db config]
-  (vals (native-definitions db config)))
+  (mapv #(assoc % :server "eca") (vals (native-definitions db config))))
 
 (defn all-tools
   "Returns all available tools, including both native ECA tools
@@ -117,15 +117,73 @@
                                                                         (assoc :type :mcp)
                                                                         (update :tools #(mapv with-tool-status %)))))})))
 
-(defn manual-approval? [all-tools name args db config]
+(defn legacy-manual-approval? [config tool-name]
+  (let [manual-approval? (get-in config [:toolCall :manualApproval] nil)]
+    (if (coll? manual-approval?)
+      (some #(= tool-name (str %)) manual-approval?)
+      manual-approval?)))
+
+(defn ^:private approval-matches? [[server-or-full-tool-name config] tool-call-server tool-call-name args]
+  (let [args-matchers (:argsMatchers config)
+        [server-name tool-name] (if (string/includes? server-or-full-tool-name "__")
+                                  (string/split server-or-full-tool-name #"__" 2)
+                                  (if (string/starts-with? server-or-full-tool-name "eca_")
+                                    ["eca" server-or-full-tool-name]
+                                    [server-or-full-tool-name nil]))]
+    (cond
+      ;; specified server name in config
+      (and (nil? tool-name)
+           ;; but the name doesn't match
+           (not= tool-call-server server-name))
+      false
+
+      ;; tool or server not match
+      (and tool-name
+           (or (not= tool-call-server server-name)
+               (not= tool-call-name tool-name)))
+      false
+
+      (map? args-matchers)
+      (some (fn [[arg-name matchers]]
+              (when-let [arg (get args arg-name)]
+                (some #(re-matches (re-pattern (str %)) (str arg))
+                      matchers)))
+            args-matchers)
+
+      :else
+      true)))
+
+(defn manual-approval? [all-tools tool-call-name args db config]
   (boolean
-   (let [require-approval-fn (:require-approval-fn (first (filter #(= name (:name %))
-                                                                  all-tools)))
-         manual-approval? (get-in config [:toolCall :manualApproval] nil)]
-     (or (when require-approval-fn (require-approval-fn args {:db db}))
-         (if (coll? manual-approval?)
-           (some #(= name (str %)) manual-approval?)
-           manual-approval?)))))
+   (let [{:keys [server require-approval-fn]} (first (filter #(= tool-call-name (:name %))
+                                                             all-tools))
+         {:keys [allow ask byDefault]} (get-in config [:toolCall :approval])]
+     (cond
+       (and require-approval-fn (require-approval-fn args {:db db}))
+       true
+
+       (some #(approval-matches? % server tool-call-name args) ask)
+       true
+
+       (some #(approval-matches? % server tool-call-name args) allow)
+       false
+
+       (legacy-manual-approval? config tool-call-name)
+       true
+
+       (= "ask" byDefault)
+       true
+
+       (= "allow" byDefault)
+       false
+
+        ;; TODO suport :deny
+        ;; (= "deny" byDefault)
+        ;; false
+
+        ;; A config error, default to manual approve
+       :else
+       true))))
 
 (defn tool-call-summary [all-tools name args]
   (when-let [summary-fn (:summary-fn (first (filter #(= name (:name %))