emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Max Nikulin <manikulin@gmail.com>
To: emacs-orgmode@gnu.org
Subject: Re: [PATCH] ob-core: add org-confirm-babel-evaluate-cell custom variable
Date: Sun, 11 Dec 2022 09:58:14 +0700	[thread overview]
Message-ID: <tn3h08$1099$1@ciao.gmane.io> (raw)
In-Reply-To: <CA+G3_PNmnJ-ehnYOBkaOOsyNjeb-OJyoy+sg_g5v3AZVGiNoXg@mail.gmail.com>

On 11/12/2022 03:28, Tom Gillespie wrote:
>      Here is a patch that improves the ergonomics and thus hopefully
> the security for the recent changes to check evaluation for cells.

Tom, thank you for the patch. Frankly speaking, I was expecting this 
kind of complains, but I could not suggest any solution. I am not 
familiar with org-babel code, so my comments may be false alarms.

> * lisp/ob-core.el (org-confirm-babel-evaluate-cell): Added to control
> execution of cells separate from execution of src blocks, it works in
> exactly the same way as org-confirm-babel-evaluate.

I am not sure concerning "exactly".

lisp/ob-core.el:248
`org-confirm-babel-evaluate' is called with 2 arguments. In your patch 
`org-confirm-babel-evaluate-cell' has a single argument.

> This commit resolves the issue by making it possible to ignore checks
> on cells (the old behavior) without compromising general security for
> running src blocks.

It seems, you do not change defaults. Could you, please, provide an 
example of configuration that is less annoying, but still safe?

> This is necessary because there is no easy way to hop swap
> org-confirm-babel-evaluate between org-get-src-block-info where
> org-babel-read is called and the execution of that src block. It could
> probably be done using advice around org-babel-read, but that is a
> level of hackery that should be avoided.

I was thinking if it is possible to collect requests to confirm and to 
allow the user to decide for the whole bunch of expressions and code 
blocks. Besides implementation issues, there is a question concerning UI 
that will allow to inspect code to be evaluated.

> diff --git a/lisp/ob-core.el b/lisp/ob-core.el
...> +(defcustom org-confirm-babel-evaluate-cell t
> +  "Confirm before evaluating a cell."
Calling convention for the case of function value is not described. If 
it is really the same as for `org-confirm-babel-evaluate' then this user 
option should be mentioned in the docstring.

> +  :group 'org-babel
> +  :version "29.1"

:package-version instead of :version?

> +  :type '(choice boolean function))
> +;; don't allow this variable to be changed through file settings
> +(put 'org-confirm-babel-evaluate-cell 'safe-local-variable (lambda (x) (eq x t)))

Is there any reason to not use the :safe property of `defcustom'? I see 
that you take definition of `org-confirm-babel-evaluate' as a template 
so I wonder if there is some particular reason or the original code was 
just written before introducing of :safe.



  reply	other threads:[~2022-12-11  2:59 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-10 20:28 [PATCH] ob-core: add org-confirm-babel-evaluate-cell custom variable Tom Gillespie
2022-12-11  2:58 ` Max Nikulin [this message]
2022-12-11 20:27   ` Tom Gillespie
2022-12-11 20:37     ` Tom Gillespie
2022-12-11 20:46     ` Kyle Meyer
2022-12-11 21:08       ` Tom Gillespie
2022-12-12 10:20         ` Ihor Radchenko
2022-12-13  1:53           ` Tom Gillespie
2022-12-13  9:03             ` Ihor Radchenko
2022-12-13 16:31             ` Max Nikulin
2022-12-13 21:16               ` Tom Gillespie
2022-12-14 16:40                 ` Max Nikulin
2022-12-14 18:24                   ` Tom Gillespie
2022-12-15  9:18                     ` Ihor Radchenko
2022-12-15  9:25                       ` Tom Gillespie
2022-12-15  9:57                       ` tomas
2022-12-15  9:10                   ` Ihor Radchenko
2022-12-15 12:10                     ` Max Nikulin
2022-12-15 12:25                       ` Ihor Radchenko
2022-12-15 14:46                         ` Max Nikulin
2022-12-15 21:08                           ` Tim Cross
2022-12-16  6:07                             ` Ihor Radchenko
2022-12-16  7:22                               ` Tim Cross
2022-12-18 14:19                                 ` Ihor Radchenko
2022-12-18 21:37                                   ` Tim Cross
2022-12-20  0:00                                     ` Tom Gillespie
2022-12-20  0:06                                       ` Tom Gillespie
2022-12-25 11:00                                         ` Ihor Radchenko
2022-12-18 14:12                           ` Ihor Radchenko
2022-12-25 11:06             ` Ihor Radchenko
2022-12-29 15:58               ` Bastien Guerry
2022-12-29 16:33                 ` Max Nikulin
2022-12-29 16:35                 ` Ihor Radchenko
2022-12-30  8:52                   ` Bastien
2022-12-30 11:10                     ` Max Nikulin
2022-12-30 17:43                     ` Tom Gillespie
2022-12-31 13:48                       ` Ihor Radchenko
2022-12-31 16:15                         ` Tom Gillespie
2023-01-02  8:34                         ` [SECURITY] Arbitrary code evaluation security in Org (was: [PATCH] ob-core: add org-confirm-babel-evaluate-cell custom variable) Ihor Radchenko
2023-01-02 10:59                           ` [SECURITY] Arbitrary code evaluation security in Org Greg Minshall
2023-01-03  9:52                             ` [SECURITY] Tangling can overwrite arbitrary tangling targets, including important user files (was: [SECURITY] Arbitrary code evaluation security in Org) Ihor Radchenko
2023-01-02 19:00                           ` [SECURITY] Arbitrary code evaluation security in Org (was: [PATCH] ob-core: add org-confirm-babel-evaluate-cell custom variable) Tim Cross
2023-01-03 11:00                             ` Ihor Radchenko
2023-01-07 13:12                               ` Ihor Radchenko
2023-01-02 15:13                         ` [PATCH] ob-core: add org-confirm-babel-evaluate-cell custom variable Bastien Guerry
2023-01-02 15:17                           ` Ihor Radchenko
2023-01-02 15:15                       ` Bastien
2022-12-13  4:16           ` Kyle Meyer
2022-12-13 16:15     ` Max Nikulin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='tn3h08$1099$1@ciao.gmane.io' \
    --to=manikulin@gmail.com \
    --cc=emacs-orgmode@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public 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).