search

astoff / buffer-env

Buffer-local process environments for Emacs
61 stars 9 forks source link

Saving buffer-env-safe-files: wrong-type-argument listp t #4

Closed manuel-uberti closed 2 years ago

manuel-uberti commented 2 years ago

Hi,

I edited the .envrc of one of my projects today and run direnv allow to make sure the changes are loaded.

However, the first time I act on this project (e.g., C-x p p -> Select project -> RET -> v (for VC-Dir)), I get the following prompt:

Mark current version of ‘/home/manuel/code/eerlijewoz/cockpit/.envrc’ as safe to execute? (y or n)

And when I hit y I get:

Debugger entered--Lisp error: (wrong-type-argument listp t)
  custom-save-variables()
  custom-save-all()
  customize-save-variable(buffer-env-safe-files (("/home/manuel/code/eerlijewoz/cockpit/.envrc" . "ffe8ad77e9d2affb4445c4a83d830d4606860e1d5ba86aab99...")))
  buffer-env--authorize("/home/manuel/code/eerlijewoz/cockpit/.envrc")
  buffer-env-update()
  hack-local-variables-apply()
  hack-dir-local-variables-non-file-buffer()
  project--value-in-dir(project-vc-merge-submodules "~/code/eerlijewoz/cockpit/")
  project--vc-merge-submodules-p("~/code/eerlijewoz/cockpit/")
  project-try-vc("~/code/eerlijewoz/cockpit/")
  project--find-in-directory("~/code/eerlijewoz/cockpit/")
  project-current(t)
  project-find-file(nil)
  funcall-interactively(project-find-file nil)
  project-switch-project("~/code/eerlijewoz/cockpit/")
  funcall-interactively(project-switch-project "~/code/eerlijewoz/cockpit/")
  command-execute(project-switch-project)

This is how I set up buffer-env in my init.el:

(add-hook 'hack-local-variables-hook #'buffer-env-update)
(with-eval-after-load 'buffer-env
  (push '(".envrc" . "direnv exec . env -0") buffer-env-commands))

(defun mu-buffer-env-inherit (fn &rest args)
  "Call FN with ARGS using the buffer-local process environment."
  (cl-letf (((default-value 'process-environment) process-environment)
            ((default-value 'exec-path) exec-path))
    (apply fn args)))
(advice-add #'mu-buffer-env-inherit :around #'project-shell)
(advice-add #'mu-buffer-env-inherit :around #'shell)
manuel-uberti commented 2 years ago

FTR, I am using GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, cairo version 1.16.0) of 2022-04-20, built on commit ebde448a14d44719b78b8b873a88448b73d72572.

manuel-uberti commented 2 years ago

To give you a minimal reproducible recipe:

manuel-uberti commented 2 years ago

Upon further debugging, I found that (secure-hash 'sha256 (current-buffer)) in buffer-env--authorize was generating a hash value different from what I previously had in my custom.el file. Which I think makes sense since the content of .envrc are changed.

To workaround this problem (basically to avoid the prompt), I replaced the hash in custom.el for my .envrc entry with the new one I get in the Backtrace, and the problem seems to be "solved".

I am pretty sure this is not the correct approach, though. :)

phikal commented 2 years ago

Maybe a function like this might help:

(defun buffer-env-forget-authorisation (file)
  "Query user for a FILE to forget about being authorised."
  (interactive
   (progn
     (unless buffer-env-safe-files
       (user-error "No files to forget"))
     (list (completing-read "Forget: " buffer-env-safe-files))))
  (customize-save-variable
   'buffer-env-safe-files
   (assoc-delete-all file buffer-env-safe-files #'file-equal-p)))
astoff commented 2 years ago

Yes, when some script changes, I should just ask again for authorization.

As to the "forget authorization" command, something on those lines should be added. My original plan was to add a "garbage collect" command to remove all old hashes. Do you think there's a need for this more fine-grained version that allows to forget about a specific hash?

phikal commented 2 years ago

Do you think there's a need for this more fine-grained version that allows to forget about a specific hash?

The only rationale for that I can imagine is when someone is moving back and forth in a VCS history. Otherwise whenever a new hash it added the only one should be removable.

manuel-uberti commented 2 years ago

I think the manual "forget authorization" command can be suggested in the documentation for users who know what they are doing with their custom.el file. Otherwise, the custom.el update should happen automatically, maybe with a confirmation message to notify the user about the change.

phikal commented 2 years ago

A related, general question that might be worth asking is whether treating file buffer-env-safe-files as a user option is the right approach. If the point is just to ensure persistence, but the user isn't expected to manually interact with the value, then something else might be better. E.g. to use a dependency like persist.

phikal commented 2 years ago

Another point is that for systems like Guix, a seperate file like ~/.config/guix/shell-authorized-directories may be used to manage authorized directories. Should buffer-env complement this or check if Guix is fine with a file?

(let ((authorized "~/.config/guix/shell-authorized-directories")
      (current (concat "^" (regexp-quote default-directory) "$")))
  (when (and (string-match-p "\\.scm\\'" file)
         (file-exists-p authorized))
    (with-temp-buffer
      (insert-file-contents authorized)
      (catch 'ok
    (while (not (eobp))
      (cond
       ((looking-at-p "^#"))
       ((looking-at-p current)
        (throw 'ok t)))
      (forward-line))))))
astoff commented 2 years ago

If the point is just to ensure persistence, but the user isn't expected to manually interact with the value

That's correct.

then something else might be better. E.g. to use a dependency like persist.

I'm open to being convinced of why something else might be better. My two cents so far:

phikal commented 2 years ago

Custom is already used to persist variables like custom-safe-themes and safe-local-variable-values. So this seems to be the status quo anyway.

True, you can add package-selected-packages to that list too. Considering that it probably shouldn't matter.

astoff commented 2 years ago

To give you a minimal reproducible recipe

I tried this and I get no errors.

To workaround this problem (basically to avoid the prompt), I replaced the hash in custom.el for my .envrc entry with the new one I get in the Backtrace, and the problem seems to be "solved".

I am pretty sure this is not the correct approach, though. :)

Looking at the implementation of buffer-env--authorize, there should be no difference if the file was previously unknown or was known with a different hash: the case (member (cons file hash) buffer-env-safe-files) of a certain or form will fail.

This makes me think the problem lies somewhere else, possibly in your own configuration?

astoff commented 2 years ago

Another point is that for systems like Guix, a seperate file like ~/.config/guix/shell-authorized-directories may be used to manage authorized directories. Should buffer-env complement this or check if Guix is fine with a file?

(Sorry for not replying earlier to this comment.) I'm not a fan of integrating too deeply with any particular external tool, since there's probably at least half a dozen of them out there. As an advanced user, you could add the code you pasted here as a :before-until advice to buffer-env--authorize, and I think that's sufficient configurability for the time being.

manuel-uberti commented 2 years ago

Looking at the implementation of buffer-env--authorize, there should be no difference if the file was previously unknown or was known with a different hash: the case (member (cons file hash) buffer-env-safe-files) of a certain or form will fail.

This makes me think the problem lies somewhere else, possibly in your own configuration?

I'll investigate further and report back if I manage to find the problem. But since you cannot reproduce it (and I cannot reproduce from emacs -Q) I think we can close this ticket.

manuel-uberti commented 2 years ago

FWIW, I found the problematic lines in my init.el. If I remove the following two lines, I don't get the error any more:

(put 'inhibit-startup-echo-area-message 'saved-value t)
(setq-default inhibit-startup-echo-area-message user-login-name)