Subject: Re: bug#61901: 30.0.50; [PATCH v3] Add safe-local-variable-directories variable.

From: Eli Zaretskii <[email-masked]>
Date: Thu, 11 May 2023 16:55:54 +0300

> From: Antero Mejr <[email-masked]>
> Cc: [email-masked]
> Date: Tue, 09 May 2023 21:29:54 +0000
> 
> >> +@defvar safe-local-variable-directories
> >> +This is a list of directories where local variables are always enabled.
> >> +Directory-local variables loaded from these directories, such as the
> >> +variables in @file{.dir-locals.el}, will be enabled even if they are
> >> +risky.
> >> +@end defvar
> >
> >This variable should also be documented in the Emacs user manual, not
> >only in the ELisp Reference manual -- it's a user option, and a very
> >important one at that.
> 
> Added to the manual in custom.texi "Safe File Variables" subsection.
> 
> >> ++++
> >> +** New variable 'safe-local-variable-directories'.
> >> +This variable is used to to permanently trust directories containing
> >> +risky directory-local variables.
> >
> >I would rephrase:
> >
> >  This variable names directories in which Emacs will treat all
> >  directory-local variables as safe.
> 
> Fixed in attached patch.
> 
> >>  ALL-VARS is the list of all variables to be set up.
> >> @@ -3734,7 +3744,9 @@ n  -- to ignore the local variables list.")
> >>  !  -- to apply the local variables list, and permanently mark these
> >>        values (*) as safe (in the future, they will be set automatically.)
> >>  i  -- to ignore the local variables list, and permanently mark these
> >> -      values (*) as ignored\n\n")
> >> +      values (*) as ignored
> >> ++  -- to apply the local variables list, and permanently trust all
> >> +      directory-local variables in this directory\n\n")
> >
> >I would remove the "permanently" part, it would just confuse here.
> 
> Fixed.
> 
> >> @@ -3908,6 +3924,7 @@ DIR-NAME is the name of the associated directory.  
> >> Otherwise it is nil."
> >>                 (null unsafe-vars)
> >>                 (null risky-vars))
> >>            (memq enable-local-variables '(:all :safe))
> >> +             (member dir-name safe-local-variable-directories)
> >
> >If you use 'member' for this test, then (a) the documentation of
> >safe-local-variable-directories should explicitly say that the
> >directories in the list must be in full absolute form, and (b) we
> >should consider the various issues with file names that are not
> >'equal' as strings, but still name the same directory, such as
> >letter-case differences on case-insensitive filesystems.  And what
> >about equality of "foo/" "and "foo"?
> 
> Clarified the documentation. The directory path requires a trailing
> separator, and is case-sensitive regardless of the filesystem (tested
> on vFAT).
> 
> >Also, is 'dir-name' above guaranteed to be a fully-expanded absolute
> >file name?
> 
> Yes. For TRAMP connections it's a string with text properties, but it's
> the same equality-wise.
> 
> #("/ssh:user:/home/user/src/" 5 6 (tramp-default t))
> 
> >> +(ert-deftest files-tests-safe-local-variable-directories ()
> >> +  ;; safe-local-variable-directories should be risky,
> >> +  ;; so use it as an arbitrary risky variable.
> >> +  (let ((test-alist '((safe-local-variable-directories . "some_val")))
> >> +        (fakedir "test1/test2")
> >> +        (enable-local-eval t))
> >> +    (with-temp-buffer
> >> +      (setq safe-local-variable-directories (list fakedir))
> >
> >The test should use absolute directory names for directories you put
> >into safe-local-variable-directories.
> 
> Fixed.

Thanks, I installed this on the master branch, and I'm therefore
closing this bug.

Please note some minor changes I made in the documentation parts of
the changeset, the most notable one being the use of "path" to allude
to file names or directory names: the Gnu Coding Standards frown on
that.  We use "path" only for lists of directories in the style of
PATH environment variable or load-path Lisp variable.

Thanks again for working on this feature.