Fix #1489: prevent uberscript/uberjar overwrites (#1503)

CHANGELOG.md

@@ -13,6 +13,7 @@ A preview of the next release can be installed from
 - [#1482](https://github.com/babashka/babashka/issues/1482): make loading of libs thread safe
 - [#1487](https://github.com/babashka/babashka/issues/1487): `babashka.tasks/clojure` should be supported without arguments to start a REPL
 - [#1496](https://github.com/babashka/babashka/issues/1496): Add `set-agent-send-executor!` and `set-agent-send-off-executor!`
+- [#1489](https://github.com/babashka/babashka/issues/1489): Don't overwrite non-empty, non-jar files when writing uberscript/uberjar ([@bobisageek](https://github.com/bobisageek))
 
 ## 1.1.173 (2023-02-04)
 

src/babashka/main.clj

@@ -780,6 +780,14 @@ Use bb run --help to show this help output.
       env-os-name-present? (not= env-os-name sys-os-name)
       env-os-arch-present? (not= env-os-arch sys-os-arch))))
 
+(defn file-write-allowed?
+  "For output file of uberscript/uberjar, allow writing of jar files
+   and files that are empty/don't exist."
+  [path]
+  (or (= "jar" (fs/extension path))
+    (not (fs/exists? path))
+    (zero? (fs/size path))))
+
 (def seen-urls (atom nil))
 
 (defn read-data-readers [url]
@@ -1062,29 +1070,38 @@ Use bb run --help to show this help output.
                 1)]
         (flush)
         (when uberscript
-          (let [uberscript-out uberscript]
-            (spit uberscript-out "") ;; reset file
-            (doseq [s (distinct @uberscript-sources)]
-              (spit uberscript-out s :append true))
-            (spit uberscript-out preloads :append true)
-            (spit uberscript-out expression :append true)))
+          (if (file-write-allowed? uberscript)
+            (do
+              (spit uberscript "")                        ;; reset file
+              (doseq [s (distinct @uberscript-sources)]
+                (spit uberscript s :append true))
+              (spit uberscript preloads :append true)
+              (spit uberscript expression :append true))
+            (throw (Exception. (str "Uberscript target file '" uberscript
+                                 "' exists and is not empty. Overwrite prohibited.")))))
         (when uberjar
-          (if-let [cp (cp/get-classpath)]
-            (let [uber-params {:dest uberjar
-                               :jar :uber
-                               :classpath cp
-                               :main-class main
-                               :verbose debug}]
-              (if-let [bb-edn-pods (:pods @common/bb-edn)]
-                (fs/with-temp-dir [bb-edn-dir {}]
-                  (let [bb-edn-resource (fs/file bb-edn-dir "META-INF" "bb.edn")]
-                    (fs/create-dirs (fs/parent bb-edn-resource))
-                    (->> {:pods bb-edn-pods} pr-str (spit bb-edn-resource))
-                    (let [cp-with-bb-edn (str bb-edn-dir cp/path-sep cp)]
-                      (uberjar/run (assoc uber-params
-                                          :classpath cp-with-bb-edn)))))
-                (uberjar/run uber-params)))
-            (throw (Exception. "The uberjar task needs a classpath."))))
+          (let [cp (cp/get-classpath)]
+            (cond
+              (not (file-write-allowed? uberjar))
+              (throw (Exception. (str "Uberjar target file '" uberjar
+                                   "' exists and is not empty. Overwrite prohibited.")))
+              (not cp)
+              (throw (Exception. "The uberjar task needs a classpath."))
+              :else
+              (let [uber-params {:dest       uberjar
+                                 :jar        :uber
+                                 :classpath  cp
+                                 :main-class main
+                                 :verbose    debug}]
+                (if-let [bb-edn-pods (:pods @common/bb-edn)]
+                  (fs/with-temp-dir [bb-edn-dir {}]
+                    (let [bb-edn-resource (fs/file bb-edn-dir "META-INF" "bb.edn")]
+                      (fs/create-dirs (fs/parent bb-edn-resource))
+                      (->> {:pods bb-edn-pods} pr-str (spit bb-edn-resource))
+                      (let [cp-with-bb-edn (str bb-edn-dir cp/path-sep cp)]
+                        (uberjar/run (assoc uber-params
+                                       :classpath cp-with-bb-edn)))))
+                  (uberjar/run uber-params))))))
         exit-code))))
 
 (defn satisfies-min-version? [min-version]

test/babashka/main_test.clj

@@ -487,6 +487,43 @@
     (is (empty? (bb nil "--uberscript" (test-utils/escape-file-paths (.getPath tmp-file)) "-e" "(System/exit 1)")))
     (is (= "(System/exit 1)" (slurp tmp-file)))))
 
+(deftest uberscript-overwrite-test
+  (testing "trying to make uberscript overwrite a non-empty, non-jar file fails"
+    (let [tmp-file (java.io.File/createTempFile "uberscript_overwrite" ".clj")]
+      (.deleteOnExit tmp-file)
+      (spit (.getPath tmp-file) "this isn't empty")
+      (is (thrown-with-msg? Exception #"Overwrite prohibited."
+            (test-utils/bb nil "--uberscript" (test-utils/escape-file-paths (.getPath tmp-file)) "-e" "(println 123)"))))))
+
+(deftest throw-on-empty-classpath
+  ;; this test fails the windows native test in CI
+  (when-not main/windows?
+    (testing "throw on empty classpath"
+      (let [tmp-file (java.io.File/createTempFile "uber" ".jar")
+            path     (.getPath tmp-file)]
+        (.deleteOnExit tmp-file)
+        (is (thrown-with-msg?
+              Exception #"classpath"
+              (test-utils/bb nil "uberjar" path "-m" "my.main-main")))))))
+
+(deftest target-file-overwrite-test
+  (test-utils/with-config {:paths ["test-resources/babashka/uberjar/src"]}
+    (testing "trying to make uberjar overwrite a non-empty jar file is allowed"
+      (let [tmp-file (java.io.File/createTempFile "uberjar_overwrite" ".jar")
+            path (.getPath tmp-file)]
+        (.deleteOnExit tmp-file)
+        (spit path "this isn't empty")
+        (test-utils/bb nil "--uberjar" (test-utils/escape-file-paths path) "-m" "my.main-main")
+        ; execute uberjar to confirm that the file is overwritten
+        (is (= "(\"42\")\n" (test-utils/bb nil "--prn" "--jar" (test-utils/escape-file-paths path) "42")))))
+    (testing "trying to make uberjar overwrite a non-empty, non-jar file is not allowed"
+      (let [tmp-file (java.io.File/createTempFile "oops_all_source" ".clj")
+            path (.getPath tmp-file)]
+        (.deleteOnExit tmp-file)
+        (spit path "accidentally a source file")
+        (is (thrown-with-msg? Exception #"Overwrite prohibited."
+              (test-utils/bb nil "--uberjar" (test-utils/escape-file-paths path) "-m" "my.main-main")))))))
+
 (deftest unrestricted-access
   (testing "babashka is allowed to mess with built-in vars"
     (is (= {} (bb nil "

test/babashka/uberjar_test.clj

@@ -78,14 +78,3 @@
             (is (= #{:pods} (-> bb-edn keys set)))
             (is (= (:pods config) (:pods bb-edn))))
           (is (str/includes? (tu/bb nil "--prn" "--jar" path) "3")))))))
-
-(deftest throw-on-empty-classpath
-  ;; this test fails the windows native test in CI
-  (when-not main/windows?
-    (testing "throw on empty classpath"
-      (let [tmp-file (java.io.File/createTempFile "uber" ".jar")
-            path     (.getPath tmp-file)]
-        (.deleteOnExit tmp-file)
-        (is (thrown-with-msg?
-             Exception #"classpath"
-             (tu/bb nil "uberjar" path "-m" "my.main-main")))))))