Allow tool calls outside workspace; validate required params

- Add centralized path gating
- Remove in-handler workspace-only checks; approval now guards cross‑workspace paths.
- Plan behavior: allow safe tools (read/grep/dir‑tree/preview) and pwd; improve deny rules for shell.
- Validate required parameters before dispatch in tools/call-tool! (native and MCP) and return INVALID_ARGS on missing keys.
- Update tests to reflect approval gating and new preview messaging.

Author: zikajk

src/eca/config.clj

@@ -80,8 +80,15 @@
                        :disabledTools ["eca_preview_file_change"]}
               "plan" {:systemPromptFile "prompts/plan_behavior.md"
                       :disabledTools ["eca_edit_file" "eca_write_file" "eca_move_file"]
-                      :toolCall {:approval {:deny {"eca_shell_command"
-                                                   {:argsMatchers {"command" [".*>.*",
+                      :toolCall {:approval {:allow {"eca_shell_command"
+                                                    {:argsMatchers {"command" ["pwd"]}}
+                                                    "eca_preview_file_change" {}
+                                                    "eca_grep" {}
+                                                    "eca_read_file" {}
+                                                    "eca_directory_tree" {}}
+                                            :deny {"eca_shell_command"
+                                                   {:argsMatchers {"command" ["[12&]?>>?\\s*(?!/dev/null($|\\s))\\S+"
+                                                                              ".*>.*",
                                                                               ".*\\|\\s*(tee|dd|xargs).*",
                                                                               ".*\\b(sed|awk|perl)\\s+.*-i.*",
                                                                               ".*\\b(rm|mv|cp|touch|mkdir)\\b.*",

src/eca/features/tools.clj

@@ -80,19 +80,25 @@
                   ]
   (logger/info logger-tag (format "Calling tool '%s' with args '%s'" name arguments))
   (let [arguments (update-keys arguments clojure.core/name)
-        db @db*]
+        db @db*
+        tool-meta (some #(when (= name (:name %)) %)
+                        (all-tools chat-id behavior db config))
+        required-args-error (when-let [parameters (:parameters tool-meta)]
+                              (tools.util/required-params-error parameters arguments))]
     (try
-      (let [result (if-let [native-tool-handler (get-in (native-definitions db config) [name :handler])]
-                     (native-tool-handler arguments {:db db
-                                                     :db* db*
-                                                     :config config
-                                                     :messenger messenger
-                                                     :behavior behavior
-                                                     :chat-id chat-id
-                                                     :tool-call-id tool-call-id
-                                                     :call-state-fn call-state-fn
-                                                     :state-transition-fn state-transition-fn})
-                     (f.mcp/call-tool! name arguments {:db db}))]
+      (let [result (-> (if required-args-error
+                         required-args-error
+                         (if-let [native-tool-handler (get-in (native-definitions db config) [name :handler])]
+                           (native-tool-handler arguments {:db db
+                                                           :db* db*
+                                                           :config config
+                                                           :messenger messenger
+                                                           :behavior behavior
+                                                           :chat-id chat-id
+                                                           :tool-call-id tool-call-id
+                                                           :call-state-fn call-state-fn
+                                                           :state-transition-fn state-transition-fn})
+                           (f.mcp/call-tool! name arguments {:db db}))))]
         (logger/debug logger-tag "Tool call result: " result)
         (metrics/count-up! "tool-called" {:name name :error (:error result)} metrics)
         result)

src/eca/features/tools/filesystem.clj

@@ -14,13 +14,8 @@
 
 (set! *warn-on-reflection* true)
 
-(defn ^:private allowed-path? [db path]
-  (some #(fs/starts-with? path (shared/uri->filename (:uri %)))
-        (:workspace-folders db)))
-
-(defn ^:private path-validations [db]
-  [["path" fs/exists? "$path is not a valid path"]
-   ["path" (partial allowed-path? db) (str "Access denied - path $path outside allowed directories: " (tools.util/workspace-roots-strs db))]])
+(defn ^:private path-validations []
+  [["path" fs/exists? "$path is not a valid path"]])
 
 (def ^:private directory-tree-max-depth 10)
 
@@ -35,7 +30,7 @@
 
 (defn ^:private directory-tree [arguments {:keys [db config]}]
   (let [path (delay (fs/canonicalize (get arguments "path")))]
-    (or (tools.util/invalid-arguments arguments (path-validations db))
+    (or (tools.util/invalid-arguments arguments (path-validations))
         (let [max-depth (or (get arguments "max_depth") directory-tree-max-depth)
               dir-count* (atom 0)
               file-count* (atom 0)
@@ -69,8 +64,8 @@
                 summary (format "%d directories, %d files" @dir-count* @file-count*)]
             (tools.util/single-text-content (str body "\n\n" summary)))))))
 
-(defn ^:private read-file [arguments {:keys [db config]}]
-  (or (tools.util/invalid-arguments arguments (concat (path-validations db)
+(defn ^:private read-file [arguments {:keys [config]}]
+  (or (tools.util/invalid-arguments arguments (concat (path-validations)
                                                       [["path" fs/readable? "File $path is not readable"]
                                                        ["path" (complement fs/directory?) "$path is a directory, not a file"]]))
       (let [line-offset (or (get arguments "line_offset") 0)
@@ -99,13 +94,12 @@
     (str "Reading file " (fs/file-name (fs/file path)))
     "Reading file"))
 
-(defn ^:private write-file [arguments {:keys [db]}]
-  (or (tools.util/invalid-arguments arguments [["path" (partial allowed-path? db) (str "Access denied - path $path outside allowed directories: " (tools.util/workspace-roots-strs db))]])
-      (let [path (get arguments "path")
-            content (get arguments "content")]
-        (fs/create-dirs (fs/parent (fs/path path)))
-        (spit path content)
-        (tools.util/single-text-content (format "Successfully wrote to %s" path)))))
+(defn ^:private write-file [arguments _]
+  (let [path (get arguments "path")
+        content (get arguments "content")]
+    (fs/create-dirs (fs/parent (fs/path path)))
+    (spit path content)
+    (tools.util/single-text-content (format "Successfully wrote to %s" path))))
 
 (defn ^:private write-file-summary [{:keys [args]}]
   (if-let [path (get args "path")]
@@ -178,8 +172,8 @@
 
    Returns matching file paths, prioritizing by modification time when possible.
    Validates that the search path is within allowed workspace directories."
-  [arguments {:keys [db]}]
-  (or (tools.util/invalid-arguments arguments (concat (path-validations db)
+  [arguments _]
+  (or (tools.util/invalid-arguments arguments (concat (path-validations)
                                                       [["path" fs/readable? "File $path is not readable"]
                                                        ["pattern" #(and % (not (string/blank? %))) "Invalid content regex pattern '$pattern'"]
                                                        ["include" #(or (nil? %) (not (string/blank? %))) "Invalid file pattern '$include'"]
@@ -245,8 +239,9 @@
     (text-match/apply-content-change-to-string file-content original-content new-content all? path)
     (smart-edit/apply-smart-edit file-content original-content new-content path)))
 
+
 (defn ^:private edit-file [arguments {:keys [db]}]
-  (or (tools.util/invalid-arguments arguments (concat (path-validations db)
+  (or (tools.util/invalid-arguments arguments (concat (path-validations)
                                                       [["path" fs/readable? "File $path is not readable"]]))
       (let [path (get arguments "path")
             original-content (get arguments "original_content")
@@ -268,38 +263,34 @@
                   (handle-file-change-result {:error :conflict} path nil)))))
           (handle-file-change-result result path nil)))))
 
-(defn ^:private preview-file-change [arguments {:keys [db]}]
-  (or (tools.util/invalid-arguments arguments [["path" (partial allowed-path? db) (str "Access denied - path $path outside allowed directories: " (tools.util/workspace-roots-strs db))]])
-      (let [path (get arguments "path")
-            original-content (get arguments "original_content")
-            new-content (get arguments "new_content")
-            all? (boolean (get arguments "all_occurrences"))
-            file-exists? (fs/exists? path)]
-        (cond
-          file-exists?
-          (let [result (apply-file-edit-strategy (slurp path) original-content new-content all? path)]
-            (handle-file-change-result result path
-                                       (format "Change simulation completed for %s. Original file unchanged - preview only." path)))
-
-          (and (not file-exists?) (= "" original-content))
-          (tools.util/single-text-content (format "New file creation simulation completed for %s. File will be created - preview only." path))
-
-          :else
-          (tools.util/single-text-content
-           (format "Preview error for %s: For new files, original_content must be empty string (\"\"). Use markdown blocks during exploration, then eca_preview_file_change for final implementation only."
-                   path)
-           :error)))))
-
-(defn ^:private move-file [arguments {:keys [db]}]
-  (let [workspace-dirs (tools.util/workspace-roots-strs db)]
-    (or (tools.util/invalid-arguments arguments [["source" fs/exists? "$source is not a valid path"]
-                                                 ["source" (partial allowed-path? db) (str "Access denied - path $source outside allowed directories: " workspace-dirs)]
-                                                 ["destination" (partial allowed-path? db) (str "Access denied - path $destination outside allowed directories: " workspace-dirs)]
-                                                 ["destination" (complement fs/exists?) "Path $destination already exists"]])
-        (let [source (get arguments "source")
-              destination (get arguments "destination")]
-          (fs/move source destination {:replace-existing false})
-          (tools.util/single-text-content (format "Successfully moved %s to %s" source destination))))))
+(defn ^:private preview-file-change [arguments _]
+  (let [path (get arguments "path")
+        original-content (get arguments "original_content")
+        new-content (get arguments "new_content")
+        all? (boolean (get arguments "all_occurrences"))
+        file-exists? (fs/exists? path)]
+    (cond
+      file-exists?
+      (let [result (apply-file-edit-strategy (slurp path) original-content new-content all? path)]
+        (handle-file-change-result result path
+                                   (format "Change simulation completed for %s. Original file unchanged - preview only." path)))
+
+      (and (not file-exists?) (= "" original-content))
+      (tools.util/single-text-content (format "New file creation simulation completed for %s. File will be created - preview only." path))
+
+      :else
+      (tools.util/single-text-content
+       (format "Preview error for %s: For new files, original_content must be empty string (\"\")."
+               path)
+       :error))))
+
+(defn ^:private move-file [arguments _]
+  (or (tools.util/invalid-arguments arguments [["source" fs/exists? "$source is not a valid path"]
+                                               ["destination" (complement fs/exists?) "Path $destination already exists"]])
+      (let [source (get arguments "source")
+            destination (get arguments "destination")]
+        (fs/move source destination {:replace-existing false})
+        (tools.util/single-text-content (format "Successfully moved %s to %s" source destination)))))
 
 (def definitions
   {"eca_directory_tree"
@@ -311,6 +302,7 @@
                                            :description (format "Maximum depth to traverse (default: %s)" directory-tree-max-depth)}}
                  :required ["path"]}
     :handler #'directory-tree
+    :require-approval-fn (tools.util/require-approval-when-outside-workspace ["path"])
     :summary-fn (constantly "Listing file tree")}
    "eca_read_file"
    {:description (tools.util/read-tool-description "eca_read_file")
@@ -323,6 +315,7 @@
                                        :description "Maximum lines to read (default: configured in tools.readFile.maxLines, defaults to 2000)"}}
                  :required ["path"]}
     :handler #'read-file
+    :require-approval-fn (tools.util/require-approval-when-outside-workspace ["path"])
     :summary-fn #'read-file-summary}
    "eca_write_file"
    {:description (tools.util/read-tool-description "eca_write_file")
@@ -333,6 +326,7 @@
                                          :description "The complete content to write to the file"}}
                  :required ["path" "content"]}
     :handler #'write-file
+    :require-approval-fn (tools.util/require-approval-when-outside-workspace ["path"])
     :summary-fn #'write-file-summary}
    "eca_edit_file"
    {:description (tools.util/read-tool-description "eca_edit_file")
@@ -347,6 +341,7 @@
                                                  :description "Whether to replace all occurrences of the file or just the first one (default)"}}
                  :required ["path" "original_content" "new_content"]}
     :handler #'edit-file
+    :require-approval-fn (tools.util/require-approval-when-outside-workspace ["path"])
     :summary-fn (constantly "Editing file")}
    "eca_preview_file_change"
    {:description (tools.util/read-tool-description "eca_preview_file_change")
@@ -361,6 +356,7 @@
                                                  :description "Whether to preview replacing all occurrences or just the first one (default)"}}
                  :required ["path" "original_content" "new_content"]}
     :handler #'preview-file-change
+    :require-approval-fn (tools.util/require-approval-when-outside-workspace ["path"])
     :summary-fn (constantly "Previewing change")}
    "eca_move_file"
    {:description (tools.util/read-tool-description "eca_move_file")
@@ -371,6 +367,7 @@
                                              :description "The new absolute file path to move to."}}
                  :required ["source" "destination"]}
     :handler #'move-file
+    :require-approval-fn (tools.util/require-approval-when-outside-workspace ["source" "destination"])
     :summary-fn (constantly "Moving file")}
    "eca_grep"
    {:description (tools.util/read-tool-description "eca_grep")
@@ -385,6 +382,7 @@
                                              :description "Maximum number of results to return (default: 1000)"}}
                  :required ["path" "pattern"]}
     :handler #'grep
+    :require-approval-fn (tools.util/require-approval-when-outside-workspace ["path"])
     :summary-fn #'grep-summary}})
 
 (defmethod tools.util/tool-call-details-before-invocation :eca_edit_file [_name arguments _server _ctx]

src/eca/features/tools/shell.clj

@@ -106,11 +106,7 @@
                                          :description (format "Optional timeout in milliseconds (Default: %s)" default-timeout)}}
                  :required ["command"]}
     :handler #'shell-command
-    :require-approval-fn (fn [args {:keys [db]}]
-                           (when-let [wd (and args (get args "working_directory"))]
-                             (when-let [wd (and (fs/exists? wd) (str (fs/canonicalize wd)))]
-                               (let [workspace-roots (mapv (comp shared/uri->filename :uri) (:workspace-folders db))]
-                                 (not-any? #(fs/starts-with? wd %) workspace-roots)))))
+    :require-approval-fn (tools.util/require-approval-when-outside-workspace ["working_directory"])
     :summary-fn #'shell-command-summary}})
 
 (defmethod tools.util/tool-call-destroy-resource! :eca_shell_command [name resource-kwd resource]

src/eca/features/tools/util.clj

@@ -1,5 +1,6 @@
 (ns eca.features.tools.util
   (:require
+   [babashka.fs :as fs]
    [cheshire.core :as json]
    [clojure.java.io :as io]
    [clojure.java.shell :as shell]
@@ -60,6 +61,31 @@
        (map #(shared/uri->filename (:uri %)))
        (string/join "\n")))
 
+(defn workspace-root-paths
+  "Returns a vector of workspace root absolute paths from `db`."
+  [db]
+  (mapv (comp shared/uri->filename :uri) (:workspace-folders db)))
+
+(defn path-outside-workspace?
+  "Returns true if `path` is outside any workspace root in `db`.
+   Works for existing or non-existing paths by absolutizing."
+  [db path]
+  (let [p (when path (str (fs/absolutize path)))
+        roots (workspace-root-paths db)]
+    (and p (not-any? #(fs/starts-with? p %) roots))))
+
+(defn require-approval-when-outside-workspace
+  "Returns a function suitable for tool `:require-approval-fn` that triggers
+   approval when any of the provided `path-keys` in args is outside the
+   workspace roots."
+  [path-keys]
+  (fn [args {:keys [db]}]
+    (when (seq path-keys)
+      (some (fn [k]
+              (when-let [p (get args k)]
+                (path-outside-workspace? db p)))
+            path-keys))))
+
 (defn command-available? [command & args]
   (try
     (zero? (:exit (apply shell/sh (concat [command] args))))
@@ -78,3 +104,17 @@
   [tool-name]
   (-> (io/resource (str "prompts/tools/" tool-name ".md"))
       (slurp)))
+
+(defn required-params-error
+  "Given a tool `parameters` JSON schema (object) and an args map, return a
+  single-text-content error when any required parameter is missing. Returns nil
+  if all required parameters are present."
+  [parameters args]
+  (when-let [req (seq (:required parameters))]
+    (let [args (update-keys args name)
+          missing (->> req (map name) (filter #(nil? (get args %))) vec)]
+      (when (seq missing)
+        (single-text-content
+         (format "INVALID_ARGS: missing required params: %s"
+                 (->> missing (map #(str "`" % "`")) (string/join ", ")))
+         :error)))))