Safety of elisp-flymake-byte-compile (Was Re: [Emacs-diffs] scratch/allow-custom-load-paths)

[João Távora]

João Távora <address@hidden> writes:

> On Wed, Dec 5, 2018, 20:00 Glenn Morris <address@hidden wrote:
>
>  >> +(put 'elisp-flymake-byte-compile-load-path 'safe-local-variable
>  >> +     (lambda (x) (and (listp x) (catch 'tag
>  >> +                                  (dolist (path x t) (unless (stringp 
> path)
>  >> +                                                       (throw 'tag 
> nil)))))))
>
>  AFAICS the above tests whether the value is valid, not whether it is safe.
>  This should probably be a risky-local-variable, like load-path is.
>  The default "." seems actively dangerous, in much the same way as having
>  "." in a shell's PATH is.
>
> Glenn,
>
> As i tried to explain, I added the validity spec to the variable,
> precisely because I thought 4 was pretty far-fetched, and couldn't
> find any other plausible scenario. Can you?

Hello again,

In the absence of further comments I was going to push this change today
to master (before I discovered that I already did so some days ago,
inadvertently, when I was pushing another flymake-related change, so
sorry about that).

But I'd like to continue the discussion of elisp-flymake-byte-compile's
safety.  I think something should be done to address it, even if
flymake-mode never makes it into emacs-lisp-mode-hook.  

To illustrate the dimension of the problem, some time ago I was editing
an .el file that had some macros in it and macroexpansions of said
macros in it.  In the middle of writing the macro body, I wrote the list

   (delete-directory default-directory)

I never compiled this file or executed this form explicitly in any way.
I merely typed it out, in the wrong place at the wrong time :-) A few
minutes later I discovered that my project directory was completely
wiped out: elisp-flymake-byte-compile has deleted when to byte-compiling
my buffer for warnings.

Thanks to Emacs's buffers, git, and auto-save strategies, it was easy to
recover the lost directory, but obviously it could have been much more
serious that this...

So here's what could improve the situation:

1. Create a elisp-flymake-maybe-enable function that checks the buffer
   for top-level forms that _could_ make it unsafe for byte-compiling on
   the fly.  This would include, but not limited to, eval-and-compile,
   eval-when-compile, defmacro, cl-defmacro, any "unknown" top-level
   form.  This will generate a lot of false positives (positive meaning
   "unsafe") but perhaps it could be made to generate 0 false negatives
   and still successfully vet a good number of elisp files.

2. In elisp-flymake-byte-compile, disable a significant chunk of Emacs's
   system interface in the slave emacs, including file-system write
   access and network access.  Either pass a switch to the subprocess
   invocation or do the byte-compilation in a dynamic environment where
   most of these primitives are disabled, i.e. via cl-letf.  It would be
   nice to exempt `load` from this.

3. After 1. and/or 2. re-evaluate the relative safety of
   elisp-flymake-byte-compile-load-path

I think 2. would be easier to do.  Some elisp files checked this way
would possibly report false diagnostics, but at least it would be
slightly safer to enable flymake in elisp-mode.

João