From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Eric Schulte" Subject: Re: Re: [ANN] Org-babel integrated into Org-mode Date: Wed, 30 Jun 2010 12:41:30 -0700 Message-ID: <87pqz8xq8l.fsf@gmail.com> References: <87wrtp78rg.fsf@gmail.com> <874oglofzs.fsf@fastmail.fm> <87sk4432t8.fsf@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from [140.186.70.92] (port=51196 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OU3AC-0007Nu-CY for emacs-orgmode@gnu.org; Wed, 30 Jun 2010 15:41:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OU3A7-00005Y-Qh for emacs-orgmode@gnu.org; Wed, 30 Jun 2010 15:41:40 -0400 Received: from mail-pv0-f169.google.com ([74.125.83.169]:44795) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OU3A7-0008WD-Ai for emacs-orgmode@gnu.org; Wed, 30 Jun 2010 15:41:35 -0400 Received: by pvg11 with SMTP id 11so1197792pvg.0 for ; Wed, 30 Jun 2010 12:41:34 -0700 (PDT) In-Reply-To: <87sk4432t8.fsf@gmail.com> (Eric Schulte's message of "Wed, 30 Jun 2010 09:25:39 -0700") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Carsten Dominik Cc: Matt Lundin , Org Mode --=-=-= Hi, To add some concreteness to my suggestions I'm attaching to possible patches. The first introduces a global `org-confirm-babel-evaluate' variable which defaults to t, meaning all code block evaluations will require explicit confirmation from the user. The second patch adds an `org-babel-disable:emacs-lisp' function which can be called from a users configuration to ensure that *no* emacs-lisp code blocks can be evaluated on the user's system. Best -- Eric --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-babel-evaluation-of-code-blocks-now-requires-confirm.patch >From 993dda2b88220a994422becdef11773986094ce6 Mon Sep 17 00:00:00 2001 From: Eric Schulte Date: Wed, 30 Jun 2010 12:25:22 -0700 Subject: [PATCH 1/2] babel: evaluation of code blocks now requires confirmation * lisp/babel/ob.el (org-confirm-babel-evaluate): variable used to control evaluation of code blocks, default value it t, meaning all code block evaluation requires confirmation (org-babel-confirm-evaluate): function used to request confirmation of code block evaluation from the user (org-babel-execute-src-block-maybe): now requires explicit user confirmation before evaluating a code block (org-babel-open-src-block-result): now requires explicit user confirmation before evaluating a code block * lisp/org.el (org-ctrl-c-ctrl-c): added documentation of code block evaluation behavior --- lisp/babel/ob.el | 76 +++++++++++++++++++++++++++++++++++++---------------- lisp/org.el | 7 ++++- 2 files changed, 59 insertions(+), 24 deletions(-) diff --git a/lisp/babel/ob.el b/lisp/babel/ob.el index 6a42bcc..e2ffd67 100644 --- a/lisp/babel/ob.el +++ b/lisp/babel/ob.el @@ -61,6 +61,18 @@ (declare-function org-babel-ref-variables "ob-ref" (params)) (declare-function org-babel-ref-resolve-reference "ob-ref" (ref &optional params)) +(defcustom org-confirm-babel-evaluate t + "Require confirmation before interactively evaluating code +blocks in Org-mode buffers. The default value of this variable +is t, meaning confirmation is required for any code block +evaluation. This variable can be set to nil to inhibit any +future confirmation requests. + +Note disabling confirmation may result in accidental evaluation +of potentially harmful code." + :group 'org-babel + :type 'boolean) + (defvar org-babel-source-name-regexp "^[ \t]*#\\+\\(srcname\\|source\\|function\\):[ \t]*" "Regular expression used to match a source name line.") @@ -134,6 +146,19 @@ added to the header-arguments-alist." (org-babel-parse-inline-src-block-match) nil)))) +(defun org-babel-confirm-evaluate (&optional info) + "Confirm that the user wishes to evaluate the code block +defined by INFO. This behavior can be suppressed by setting the +value of `org-confirm-babel-evaluate' to nil, in which case all +future interactive code block evaluations will proceed without +any confirmation from the user. + +Note disabling confirmation may result in accidental evaluation +of potentially harmful code." + (or (not org-confirm-babel-evaluate) + (yes-or-no-p (format "Evaluate this%scode on your system?" + (if info (format " %s " (nth 0 info)) " "))))) + ;;;###autoload (defun org-babel-execute-src-block-maybe () "Detect if this is context for a org-babel src-block and if so @@ -141,7 +166,10 @@ then run `org-babel-execute-src-block'." (interactive) (let ((info (org-babel-get-src-block-info))) (if info - (progn (org-babel-execute-src-block current-prefix-arg info) t) nil))) + (if (org-babel-confirm-evaluate info) + (progn (org-babel-execute-src-block current-prefix-arg info) t) + (error "evaluation aborted")) + nil))) (add-hook 'org-ctrl-c-ctrl-c-hook 'org-babel-execute-src-block-maybe) @@ -379,28 +407,30 @@ argument RE-RUN the source-code block is evaluated even if results already exist." (interactive "P") (when (org-babel-get-src-block-info) - (save-excursion - ;; go to the results, if there aren't any then run the block - (goto-char (or (and (not re-run) (org-babel-where-is-src-block-result)) - (progn (org-babel-execute-src-block) - (org-babel-where-is-src-block-result)))) - (end-of-line 1) - (while (looking-at "[\n\r\t\f ]") (forward-char 1)) - ;; open the results - (if (looking-at org-bracket-link-regexp) - ;; file results - (org-open-at-point) - (let ((results (org-babel-read-result))) - (flet ((echo-res (result) - (if (stringp result) result (format "%S" result)))) - (pop-to-buffer (get-buffer-create "org-babel-results")) - (delete-region (point-min) (point-max)) - (if (listp results) - ;; table result - (insert (orgtbl-to-generic results '(:sep "\t" :fmt echo-res))) - ;; scalar result - (insert (echo-res results)))))) - t))) + (if (org-babel-confirm-evaluate) + (save-excursion + ;; go to the results, if there aren't any then run the block + (goto-char (or (and (not re-run) (org-babel-where-is-src-block-result)) + (progn (org-babel-execute-src-block) + (org-babel-where-is-src-block-result)))) + (end-of-line 1) + (while (looking-at "[\n\r\t\f ]") (forward-char 1)) + ;; open the results + (if (looking-at org-bracket-link-regexp) + ;; file results + (org-open-at-point) + (let ((results (org-babel-read-result))) + (flet ((echo-res (result) + (if (stringp result) result (format "%S" result)))) + (pop-to-buffer (get-buffer-create "org-babel-results")) + (delete-region (point-min) (point-max)) + (if (listp results) + ;; table result + (insert (orgtbl-to-generic results '(:sep "\t" :fmt echo-res))) + ;; scalar result + (insert (echo-res results)))))) + t) + (error "evaluation aborted")))) ;;;###autoload (defun org-babel-execute-buffer (&optional arg) diff --git a/lisp/org.el b/lisp/org.el index f2d9ade..22aaef8 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -17011,7 +17011,12 @@ This command does many different things, depending on context: - If the cursor is on a numbered item in a plain list, renumber the ordered list. -- If the cursor is on a checkbox, toggle it." +- If the cursor is on a checkbox, toggle it. + +- If the cursor is on a code block, evaluate it. The variable + `org-confirm-babel-evaluate' can be used to control prompting + before code block evaluation, by default every code block + evaluation requires confirmation." (interactive "P") (let ((org-enable-table-editor t)) (cond -- 1.7.0.4 --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0002-babel-added-function-for-completely-disabling-emacs-.patch >From a7c699331e4a86d5779cecec22de0a297cd13d1c Mon Sep 17 00:00:00 2001 From: Eric Schulte Date: Wed, 30 Jun 2010 12:37:07 -0700 Subject: [PATCH 2/2] babel: added function for completely disabling emacs-lisp code block evaluation * lisp/babel/langs/ob-emacs-lisp.el (org-babel-disable:emacs-lisp): added function for completely disabling emacs-lisp code block evaluation, after this function is called emacs lisp code blocks will be note evaluable by Babel unless the ob-emacs-lisp file is re-loaded --- lisp/babel/langs/ob-emacs-lisp.el | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/lisp/babel/langs/ob-emacs-lisp.el b/lisp/babel/langs/ob-emacs-lisp.el index 81ee22e..8ddb8c3 100644 --- a/lisp/babel/langs/ob-emacs-lisp.el +++ b/lisp/babel/langs/ob-emacs-lisp.el @@ -68,6 +68,17 @@ (org-babel-pick-name (nth 4 processed-params) (cdr (assoc :colnames params))) (org-babel-pick-name (nth 5 processed-params) (cdr (assoc :rownames params))))))) +(defun org-babel-disable:emacs-lisp () + "This function can be used to disable evaluation of emacs-lisp +code blocks. Calling this function ensures that no emacs-lisp +code blocks can be evaluated on your system unless +`ob-emacs-lisp' is later explicitly loaded or required." + (interactive) + (defun org-babel-execute:emacs-lisp (body params) + (error "Emacs lisp evaluation is disabled.")) + (defun org-babel-expand-body:emacs-lisp (body params &optional processed-params) + (error "Emacs lisp evaluation is disabled."))) + (provide 'ob-emacs-lisp) ;; arch-tag: e9a3acca-dc84-472a-9f5a-23c35befbcd6 -- 1.7.0.4 --=-=-= "Eric Schulte" writes: > Hi Carsten, Matt, Scott, > > Carsten Dominik writes: > >> Hi Matt, hi Eric, >> >> Matt, thanks a lot for bringing this up. This is indeed a very >> important and serious issue. We need to address it. We need to >> step back and reconsider this carefully. >> >> Don't get me wrong, I absolutely think that Org Babel should give >> you enough rope to hang yourself. But we have to make sure that >> this will not happen to a happy and unsuspecting Org mode, or even >> an unsuspecting Emacs user who by chance opens a file with extension >> .org. >> >> I remember very well when first realized that shell links could >> really affect you badly. It scared me. >> >> You main proposal was to make Org Babel an optional module. >> This will not solve the problem fully, I think, because we also >> don't want that people who turn it on automatically commit >> to potentially dangerous operations. There is a lot of good stuff >> in Babel which has nothing to do with code evaluation. >> >> Here is what I propose (several items are similar to what Eric proposes) >> >> 1. A new variable org-turn-on-babel. We can discuss the default. >> If it is nil, org-babel should not be loaded. >> A default of t would be fine with me if we implement other >> measures listed below. >> > > This sounds like a good idea to me, and it should address Matt's desire > for enabling minimal Org-mode installs. I would like this to default to > t, so that new users can try out Org-babel without overmuch effort. > >> >> 2. As Eric proposes, a variable similar to org-confirm-shell-link- >> function >> This should by default query for confirmation on any org-babel >> code execution, and can be configured to shut up by people who know >> what they are doing. >> > > Sounds good, I think this is a reasonable safety measure. > >> >> 3. Not loading emacs lisp evaluation by default. >> > > I would push back on this point. Largely because we have now crossed > the like to where it is impossible to play with a code block w/o first > dropping down to your configuration files, and evaluating require > statements. > >> >> 4. A new key in the babel keymap for org-babel-execute-code-block, >> for example `C-c C-v e'. This should be documented as the default >> key for this operation. >> > > Hmm, I'm less enthusiastic about this point and point 5. I really like > how 'C-c C-c' naturally does whatever-I-want given the context in which > it's called, and I wouldn't want to lose that intuitiveness. Similarly > 'C-c C-o' currently opens the results of a code block, I also find this > very appealing as it allows for a uniform top-level interface across an > Org-mode document, be it a code block or a link. > > Here are my reasons why I think leaving this keybinding is safe. > > 1) Unlike with shell/elisp links, the contents of code blocks is almost > always visible right under the user's point. So it is less likely to > evaluate something w/o having any idea what you are evaluating. > > 2) Adding a protection variable (e.g. org-confirm-babel-eval) means that > the only users who could potentially evaluate a code block with a > slip of the fingers would be users who have explicitly said that they > want to be able to easily run code blocks without confirmation. > > 3) Emacs exposes a number of entry points into code evaluation. M-! > allows users to run shell commands, C-M-x evaluate the elisp at > point, and these have not caused problems in the past. > >> >> 5. Removing org-babel-execute-code-block from `C-c C-c'. Inclusion >> should be optional. >> >> 6. A section in the manual on code execution and associated security >> risks in Org mode. This is not only about babel, but also about >> org-eval, org-eval-light, shell links and elisp links. I have meant >> to write this section for a long time and would be willing to >> draft it. We could then refer to this section from a couple of >> places in the docs, without cluttering the docs with disclaimers. >> > > This sounds like a very good idea. I'd be happy to help write such a > section. > >> >> The reason for 4 and 5 is that I believe Org-mode users are trained >> to blindly press `C-c C-c' whenever they want to update something at >> point. Matt's example of a blog post about `rm -rf' is a very >> realistic example for bad code being evaluated by mistake, not even >> due to malicious cations. I belive that a special key for this >> action would gove a good measure of protection. >> > > As I mentioned, I personally feel that an org-confirm-babel-eval > variable is sufficient protection. I think it's safe to assume that if > a user has explicitly customized that variable, then they know what > they're doing and trust themselves to execute code responsibly. I think > it's likely that the casual Org-babel user would never customize this > variable, which seems to me entirely appropriate. > >> >> This is what I think - please let me know if you think I am overdoing >> it. >> > > So to summarize, I think that the combination of (1), (2) and (6), > should be sufficient to protect users from accidental code evaluation. > Please let me know what you think, I am of course looking to compromise > and I fully understand that the general consensus may be that we need > more layers of protection. > > Best -- Eric > >> >> - Carsten >> >> >> On Jun 29, 2010, at 8:23 PM, Matt Lundin wrote: >> >>> Hi Eric, >>> >>> Thanks again for all the work that you, Dan, and Tom have put into >>> org-babel. I'm glad to see it become part of org-mode! >>> >>> "Eric Schulte" writes: >>> >>>> 2) Babel will now be loaded by default along with the rest of Org- >>>> mode. >>>> This means that *everyone* currently using babel will need to >>>> change >>>> their Emacs config and remove the (require 'org-babel-int) and/or >>>> (require 'org-babel) lines. >>> >>> I would like to request that org-babel be made an optional module. I >>> ask >>> this as someone who uses org-babel regularly. Here are my reasons: >>> >>> - Org-babel adds rather specific and complex functionality to org- >>> mode >>> that those who use it as a simple outliner and todo manager do not >>> require. (In other words, an option to turn it off might be nice >>> for >>> those who are worried about "feature creep.") >>> >>> - Org-babel increases the risk of accidentally executing malicious or >>> dangerous code when typing C-c C-c on a src block or exporting a >>> file. Perhaps users should activate it only after they understand >>> the risks. >>> >>> + For instance, I might write a blog post warning about the dangers >>> of typing "rm -rf ~/". If I put this between #+begin_src sh >>> and #+end_src and unthinkingly hit C-c C-c, I would be in >>> trouble. >>> I believe this is the reason for the variables >>> org-confirm-shell-link-function and >>> org-confirm-elisp-link-function. >>> >>> + This is admitted a bit far-fetched as an example, as it would >>> require one to have loaded ob-sh.el. But since elisp execution is >>> activated by default, there remain opportunities for unwittingly >>> executing code that is meant for other purposes (e.g., warnings, >>> examples, etc.). >>> >>>> Support for evaluating emacs-lisp code blocks is loaded by default. >>>> All other languages will need to be required explicitly. To >>>> conform >>>> to Emacs filename specifications all language require lines have >>>> been >>>> shortened from e.g. >>>> >>>> (require 'org-babel-sh) >>>> >>>> to >>>> >>>> (require 'ob-sh) >>> >>> When I run make clean && make && make install I find that the language >>> directory is not installed. Does the langs directory require a manual >>> installation? >>> >>> Also, with make install, the ob-* files are installed on the same >>> level >>> as the org-files, yet lines 108-114 in org.el indicate that they >>> should >>> be installed in a babel subdirectory. >>> >>> Thanks! >>> Matt >>> >>> _______________________________________________ >>> Emacs-orgmode mailing list >>> Please use `Reply All' to send replies to the list. >>> Emacs-orgmode@gnu.org >>> http://lists.gnu.org/mailman/listinfo/emacs-orgmode >> >> - Carsten --=-=-= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Emacs-orgmode mailing list Please use `Reply All' to send replies to the list. Emacs-orgmode@gnu.org http://lists.gnu.org/mailman/listinfo/emacs-orgmode --=-=-=--