emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] ob-clojure.el: Add support for babashka and nbb backend
@ 2021-11-14 15:28 Daniel Kraus
  2021-11-14 16:25 ` Max Nikulin
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Kraus @ 2021-11-14 15:28 UTC (permalink / raw)
  To: emacs-orgmode

* lisp/ob-clojure.el: Add support for babashka and nbb backend.
---
This adds support to ob-clojure for babashka (https://github.com/babashka/babashka)
and nbb (node version of babashka).
It doesn't use `params` as I'm not really sure what they're used for and
if they're important for evaluation.

I'm also new to org development and the git email workflow so any feedback
to the code or the email patch etc is appreciated.

 lisp/ob-clojure.el | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/lisp/ob-clojure.el b/lisp/ob-clojure.el
index 3b995d94c..dc42daa5c 100644
--- a/lisp/ob-clojure.el
+++ b/lisp/ob-clojure.el
@@ -36,6 +36,8 @@
 ;; For clojure-mode, see https://github.com/clojure-emacs/clojure-mode
 ;; For cider, see https://github.com/clojure-emacs/cider
 ;; For inf-clojure, see https://github.com/clojure-emacs/cider
+;; For babashka, see https://github.com/babashka/babashka
+;; For nbb, see https://github.com/babashka/nbb
 
 ;; For SLIME, the best way to install these components is by following
 ;; the directions as set out by Phil Hagelberg (Technomancy) on the
@@ -73,6 +75,8 @@
 	  (const :tag "inf-clojure" inf-clojure)
 	  (const :tag "cider" cider)
 	  (const :tag "slime" slime)
+	  (const :tag "babashka" babashka)
+	  (const :tag "nbb" nbb)
 	  (const :tag "Not configured yet" nil)))
 
 (defcustom org-babel-clojure-default-ns "user"
@@ -80,6 +84,16 @@
   :type 'string
   :group 'org-babel)
 
+(defcustom ob-clojure-babashka-command (executable-find "bb")
+  "Path to the babashka executable."
+  :type 'file
+  :group 'org-babel)
+
+(defcustom ob-clojure-nbb-command (executable-find "nbb")
+  "Path to the nbb executable."
+  :type 'file
+  :group 'org-babel)
+
 (defun org-babel-expand-body:clojure (body params)
   "Expand BODY according to PARAMS, return the expanded body."
   (let* ((vars (org-babel--get-vars params))
@@ -225,6 +239,16 @@
        ,(buffer-substring-no-properties (point-min) (point-max)))
      (cdr (assq :package params)))))
 
+(defun ob-clojure-escape-quotes (str-val)
+  "Escape quotes for STR-VAL."
+  (replace-regexp-in-string "\"" "\\\"" str-val 'FIXEDCASE 'LITERAL))
+
+(defun ob-clojure-eval-with-babashka (bb expanded)
+  "Evaluate EXPANDED code block using BB (babashka or nbb)."
+  (let ((escaped (ob-clojure-escape-quotes expanded)))
+    (shell-command-to-string
+     (concat bb " -e \"" escaped "\""))))
+
 (defun org-babel-execute:clojure (body params)
   "Execute a block of Clojure code with Babel."
   (unless org-babel-clojure-backend
@@ -236,6 +260,10 @@
 	  (cond
 	   ((eq org-babel-clojure-backend 'inf-clojure)
 	    (ob-clojure-eval-with-inf-clojure expanded params))
+           ((eq org-babel-clojure-backend 'babashka)
+	    (ob-clojure-eval-with-babashka ob-clojure-babashka-command expanded))
+           ((eq org-babel-clojure-backend 'nbb)
+	    (ob-clojure-eval-with-babashka ob-clojure-nbb-command expanded))
 	   ((eq org-babel-clojure-backend 'cider)
 	    (ob-clojure-eval-with-cider expanded params))
 	   ((eq org-babel-clojure-backend 'slime)
-- 
2.33.1




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
  2021-11-14 15:28 [PATCH] ob-clojure.el: Add support for babashka and nbb backend Daniel Kraus
@ 2021-11-14 16:25 ` Max Nikulin
  2021-11-14 16:30   ` Daniel Kraus
  0 siblings, 1 reply; 7+ messages in thread
From: Max Nikulin @ 2021-11-14 16:25 UTC (permalink / raw)
  To: emacs-orgmode

On 14/11/2021 22:28, Daniel Kraus wrote:
> * lisp/ob-clojure.el: Add support for babashka and nbb backend.
> ---
> +(defun ob-clojure-escape-quotes (str-val)
> +  "Escape quotes for STR-VAL."
> +  (replace-regexp-in-string "\"" "\\\"" str-val 'FIXEDCASE 'LITERAL))
> +
> +(defun ob-clojure-eval-with-babashka (bb expanded)
> +  "Evaluate EXPANDED code block using BB (babashka or nbb)."
> +  (let ((escaped (ob-clojure-escape-quotes expanded)))
> +    (shell-command-to-string
> +     (concat bb " -e \"" escaped "\""))))

Does not it an open door for security vulnerabilities? Consider a string 
somewhere in the code: "`echo arbitrary code execution`". Only outer 
quotes are escaped.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
  2021-11-14 16:25 ` Max Nikulin
@ 2021-11-14 16:30   ` Daniel Kraus
  2021-11-15 14:33     ` Max Nikulin
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Kraus @ 2021-11-14 16:30 UTC (permalink / raw)
  To: emacs-orgmode

Hi!

Max Nikulin <manikulin@gmail.com> writes:

> On 14/11/2021 22:28, Daniel Kraus wrote:
>> +(defun ob-clojure-escape-quotes (str-val)
>> +  "Escape quotes for STR-VAL."
>> +  (replace-regexp-in-string "\"" "\\\"" str-val 'FIXEDCASE 'LITERAL))
>> +
>> +(defun ob-clojure-eval-with-babashka (bb expanded)
>> +  "Evaluate EXPANDED code block using BB (babashka or nbb)."
>> +  (let ((escaped (ob-clojure-escape-quotes expanded)))
>> +    (shell-command-to-string
>> +     (concat bb " -e \"" escaped "\""))))
>
> Does not it an open door for security vulnerabilities? Consider a string
> somewhere in the code: "`echo arbitrary code execution`". Only outer quotes are
> escaped.

The escaping is not done for security reasons.
When I have a babel block like

#+BEGIN_SRC clojure
(str "foo" "bar")
#+END_SRC

babashka has to be called with

bb -e "(str \"foo\" \"bar\")"

etc.

Security wise someone should always be careful what he
evaluates in an org-babel block.
Nobody prevents you from evaluating

#+BEGIN_SRC shell
sudo rm -rf /
#+END_SRC

Cheers,
  Daniel


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
  2021-11-14 16:30   ` Daniel Kraus
@ 2021-11-15 14:33     ` Max Nikulin
  2021-11-15 16:05       ` Daniel Kraus
  0 siblings, 1 reply; 7+ messages in thread
From: Max Nikulin @ 2021-11-15 14:33 UTC (permalink / raw)
  To: emacs-orgmode

On 14/11/2021 23:30, Daniel Kraus wrote:
> Max Nikulin writes:
>> On 14/11/2021 22:28, Daniel Kraus wrote:
>>> +(defun ob-clojure-escape-quotes (str-val)
>>> +  "Escape quotes for STR-VAL."
>>> +  (replace-regexp-in-string "\"" "\\\"" str-val 'FIXEDCASE 'LITERAL))
>>> +
>>> +(defun ob-clojure-eval-with-babashka (bb expanded)
>>> +  "Evaluate EXPANDED code block using BB (babashka or nbb)."
>>> +  (let ((escaped (ob-clojure-escape-quotes expanded)))
>>> +    (shell-command-to-string
>>> +     (concat bb " -e \"" escaped "\""))))
>>
>> Does not it an open door for security vulnerabilities? Consider a string
>> somewhere in the code: "`echo arbitrary code execution`". Only outer quotes are
>> escaped.
> 
> The escaping is not done for security reasons.
> When I have a babel block like
> 
> #+BEGIN_SRC clojure
> (str "foo" "bar")
> #+END_SRC
> 
> babashka has to be called with
> 
> bb -e "(str \"foo\" \"bar\")"

Enough shell constructs may be interpreted by shell inside double quotes 
before result is passed to bb. I mentioned execution of code inside 
backticks, variable substitutions are mostly undesired as well. I do not 
think, users should escape "$" inside source blocks just because you 
chose incomplete escaping of shell specials.

The following source block must not execute echo and touch

#+begin_src clojure
   (str "`echo $HOME`" "`touch /tmp/pwned`")
#+end_src

Shell should not be used to launch any command unless it is really 
necessary. Arguments should be passed directly to execve(2) system call 
as an array. Combining them into string to pass through shell 
interpreter to parse into argument array again is error prone.

Unfortunately Emacs API related to execution of external processes is 
awkward. In this particular case it encourages usage of the unsafe 
function since there is no convenient helper that accepts binary and 
*list* of arguments and returns output as a string.

So more verbose code is required to invoke bb without intermediate 
interpretation of content of argument string. In my opinion it is better 
than using of more reliable and tested function to escape shell specials.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
  2021-11-15 14:33     ` Max Nikulin
@ 2021-11-15 16:05       ` Daniel Kraus
  2021-11-17 16:12         ` Max Nikulin
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Kraus @ 2021-11-15 16:05 UTC (permalink / raw)
  To: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]


Max Nikulin <manikulin@gmail.com> writes:

> The following source block must not execute echo and touch
>
> #+begin_src clojure
>    (str "`echo $HOME`" "`touch /tmp/pwned`")
> #+end_src

Thanks, now I got it :)

Attached is the patch changed the logic to use a temp file with org-babel-eval.
Somehow it doesn't feel too great to create unnecessary temp files
but I looked how other babel backends do it and e.g. ob-js and ob-haskell
use the same logic.

Thanks,
  Daniel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ob-clojure.el-Add-support-for-babashka-and-nbb-backe.patch --]
[-- Type: text/x-patch, Size: 2903 bytes --]

From cc9a24fcc42756cc76d59697bddc94a4ee2c475d Mon Sep 17 00:00:00 2001
From: Daniel Kraus <daniel@kraus.my>
Date: Sat, 13 Nov 2021 22:51:56 +0100
Subject: [PATCH] ob-clojure.el: Add support for babashka and nbb backend

* lisp/ob-clojure.el: Add support for babashka and nbb backend.
---
 lisp/ob-clojure.el | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/lisp/ob-clojure.el b/lisp/ob-clojure.el
index 3b995d94c..8548dc86d 100644
--- a/lisp/ob-clojure.el
+++ b/lisp/ob-clojure.el
@@ -36,6 +36,8 @@
 ;; For clojure-mode, see https://github.com/clojure-emacs/clojure-mode
 ;; For cider, see https://github.com/clojure-emacs/cider
 ;; For inf-clojure, see https://github.com/clojure-emacs/cider
+;; For babashka, see https://github.com/babashka/babashka
+;; For nbb, see https://github.com/babashka/nbb
 
 ;; For SLIME, the best way to install these components is by following
 ;; the directions as set out by Phil Hagelberg (Technomancy) on the
@@ -73,6 +75,8 @@
 	  (const :tag "inf-clojure" inf-clojure)
 	  (const :tag "cider" cider)
 	  (const :tag "slime" slime)
+	  (const :tag "babashka" babashka)
+	  (const :tag "nbb" nbb)
 	  (const :tag "Not configured yet" nil)))
 
 (defcustom org-babel-clojure-default-ns "user"
@@ -80,6 +84,16 @@
   :type 'string
   :group 'org-babel)
 
+(defcustom ob-clojure-babashka-command (executable-find "bb")
+  "Path to the babashka executable."
+  :type 'file
+  :group 'org-babel)
+
+(defcustom ob-clojure-nbb-command (executable-find "nbb")
+  "Path to the nbb executable."
+  :type 'file
+  :group 'org-babel)
+
 (defun org-babel-expand-body:clojure (body params)
   "Expand BODY according to PARAMS, return the expanded body."
   (let* ((vars (org-babel--get-vars params))
@@ -225,6 +239,15 @@
        ,(buffer-substring-no-properties (point-min) (point-max)))
      (cdr (assq :package params)))))
 
+(defun ob-clojure-eval-with-babashka (bb expanded)
+  "Evaluate EXPANDED code block using BB (babashka or nbb)."
+  (let ((script-file (org-babel-temp-file "clojure-bb-script-" ".clj")))
+    (with-temp-file script-file
+      (insert expanded))
+    (org-babel-eval
+     (format "%s %s" bb (org-babel-process-file-name script-file))
+     "")))
+
 (defun org-babel-execute:clojure (body params)
   "Execute a block of Clojure code with Babel."
   (unless org-babel-clojure-backend
@@ -236,6 +259,10 @@
 	  (cond
 	   ((eq org-babel-clojure-backend 'inf-clojure)
 	    (ob-clojure-eval-with-inf-clojure expanded params))
+           ((eq org-babel-clojure-backend 'babashka)
+	    (ob-clojure-eval-with-babashka ob-clojure-babashka-command expanded))
+           ((eq org-babel-clojure-backend 'nbb)
+	    (ob-clojure-eval-with-babashka ob-clojure-nbb-command expanded))
 	   ((eq org-babel-clojure-backend 'cider)
 	    (ob-clojure-eval-with-cider expanded params))
 	   ((eq org-babel-clojure-backend 'slime)
-- 
2.33.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
  2021-11-15 16:05       ` Daniel Kraus
@ 2021-11-17 16:12         ` Max Nikulin
  2021-11-20 10:18           ` Daniel Kraus
  0 siblings, 1 reply; 7+ messages in thread
From: Max Nikulin @ 2021-11-17 16:12 UTC (permalink / raw)
  To: emacs-orgmode

On 15/11/2021 23:05, Daniel Kraus wrote:
> Max Nikulin writes:
> 
> Attached is the patch changed the logic to use a temp file with org-babel-eval.

Thank you for contribution. I do not have strong objection any more. I 
am not familiar with babel internals, so I leave further discussion to 
maintainers.

If you have not signed copyright assignment yet, likely you should do it 
to proceed (I am unsure concerning precise rules concerning line 
counting). See https://orgmode.org/contribute.html and 
https://orgmode.org/worg/org-contribute.html for details.

> Somehow it doesn't feel too great to create unnecessary temp files
> but I looked how other babel backends do it and e.g. ob-js and ob-haskell
> use the same logic.

Code fragment might be huge enough to exceed limit on arguments length, 
so I think that file is safer. Some interpreters and compilers generates 
more meaningful errors and stacktraces when act on a file. Another 
option is to feed content to process standard input. With `call-process' 
or an related command it should be possible to implement any variant, 
including raw argument without intermediate shell pass. See info 
"(elisp) Synchronous Processes" or 
https://www.gnu.org/software/emacs/manual/html_node/elisp/Synchronous-Processes.html

> +    (with-temp-file script-file
> +      (insert expanded))
> +    (org-babel-eval
> +     (format "%s %s" bb (org-babel-process-file-name script-file))
> +     "")))

Since other babel packages use the same approach, I will not argue. By 
the way, isn't second argument of `org-babel-eval' intended for code 
that may be executed without a temporary file?

Some babel languages support sessions. I have no idea if it is 
applicable to babashka.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ob-clojure.el: Add support for babashka and nbb backend
  2021-11-17 16:12         ` Max Nikulin
@ 2021-11-20 10:18           ` Daniel Kraus
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel Kraus @ 2021-11-20 10:18 UTC (permalink / raw)
  To: emacs-orgmode


Max Nikulin <manikulin@gmail.com> writes:

> Thank you for contribution. I do not have strong objection any more. I am not
> familiar with babel internals, so I leave further discussion to maintainers.
>
> If you have not signed copyright assignment yet, likely you should do it to
> proceed (I am unsure concerning precise rules concerning line counting). See
> https://orgmode.org/contribute.html and
> https://orgmode.org/worg/org-contribute.html for details.

Thank you very much for your insights.
I filled out the copyright assignment and waiting for them.
I'll mail again when it's done.
(Maybe this is even small enough to count as tinychange if the
assignment should take a long time).

Cheers,
  Daniel


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-11-20 10:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-14 15:28 [PATCH] ob-clojure.el: Add support for babashka and nbb backend Daniel Kraus
2021-11-14 16:25 ` Max Nikulin
2021-11-14 16:30   ` Daniel Kraus
2021-11-15 14:33     ` Max Nikulin
2021-11-15 16:05       ` Daniel Kraus
2021-11-17 16:12         ` Max Nikulin
2021-11-20 10:18           ` Daniel Kraus

Code repositories for project(s) associated with this inbox:

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).