* bug#68074: eshell sudo/doas does not work for aliases [not found] <31356544.8861481.1703711546895.ref@mail.yahoo.com> @ 2023-12-27 21:12 ` Alfonso Sanchez-Beato via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-01-11 21:00 ` Stefan Kangas 0 siblings, 1 reply; 9+ messages in thread From: Alfonso Sanchez-Beato via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-12-27 21:12 UTC (permalink / raw) To: 68074 [-- Attachment #1: Type: text/plain, Size: 585 bytes --] sudo/doas does not give the expected permissions when using an eshell alias: When no alias has been defined: ~ $ which cp eshell/cp is a native-compiled Lisp function in ‘em-unix.el’. ~ $ touch test ~ $ cp test /boot/efi/ Opening output file: Permission denied, /boot/efi/test ~ $ sudo cp test /boot/efi/ ~ $ echo $? 0 But after defining the alias: ~ $ alias cp '*cp $*' ~ $ which cp cp is an alias, defined as "*cp $*" ~ $ sudo cp test /boot/efi/ /usr/bin/cp: cannot stat '/boot/efi/test': Permission denied I have attached a patch with a possible fix. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-lisp-eshell-em-alias.el-eshell-maybe-replace-by-alia.patch --] [-- Type: text/x-patch, Size: 2022 bytes --] From 25e12e5912e2ef5bcab359eca5a10e973cb35937 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alfonso=20S=C3=A1nchez-Beato?= <alfonsosanchezbeato@yahoo.es> Date: Wed, 27 Dec 2023 21:50:29 +0100 Subject: [PATCH] * lisp/eshell/em-alias.el (eshell-maybe-replace-by-alias): Fix using tramp with alias. Previously calls that involved using tramp with an alias where not working as expected because the default-directory set in em-tramp.el was forgotten after eshell-replace-command was thrown in eshell-maybe-replace-by-alias. --- lisp/eshell/em-alias.el | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lisp/eshell/em-alias.el b/lisp/eshell/em-alias.el index 841982c3425..7fff920ed7b 100644 --- a/lisp/eshell/em-alias.el +++ b/lisp/eshell/em-alias.el @@ -222,12 +222,17 @@ This is useful after manually editing the contents of the file." (member command eshell-prevent-alias-expansion)) (let ((alias (eshell-lookup-alias command))) (if alias - (throw 'eshell-replace-command - `(let ((eshell-command-name ',eshell-last-command-name) + (let ((vars `((eshell-command-name ',eshell-last-command-name) (eshell-command-arguments ',eshell-last-arguments) (eshell-prevent-alias-expansion - ',(cons command eshell-prevent-alias-expansion))) - ,(eshell-parse-command (nth 1 alias)))))))) + ',(cons command eshell-prevent-alias-expansion))))) + ;; Set default-directory as it might have been set by sudo/doas, unless + ;; we want to change it permanently. + (if (not (string-prefix-p (car (split-string (nth 1 alias) " ")) "cd")) + (push `(default-directory ',default-directory) vars)) + (throw 'eshell-replace-command + `(let ,vars + ,(eshell-parse-command (nth 1 alias))))))))) (defun eshell-alias-completions (name) "Find all possible completions for NAME. -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* bug#68074: eshell sudo/doas does not work for aliases 2023-12-27 21:12 ` bug#68074: eshell sudo/doas does not work for aliases Alfonso Sanchez-Beato via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-01-11 21:00 ` Stefan Kangas 2024-01-11 21:15 ` Jim Porter 0 siblings, 1 reply; 9+ messages in thread From: Stefan Kangas @ 2024-01-11 21:00 UTC (permalink / raw) To: Alfonso Sanchez-Beato; +Cc: Jim Porter, 68074 Alfonso Sanchez-Beato <alfonsosanchezbeato@yahoo.es> writes: > sudo/doas does not give the expected permissions when using an eshell alias: > > When no alias has been defined: > > ~ $ which cp > eshell/cp is a native-compiled Lisp function in ‘em-unix.el’. > ~ $ touch test > ~ $ cp test /boot/efi/ > Opening output file: Permission denied, /boot/efi/test > ~ $ sudo cp test /boot/efi/ > ~ $ echo $? > 0 > > But after defining the alias: > > ~ $ alias cp '*cp $*' > ~ $ which cp > cp is an alias, defined as "*cp $*" > ~ $ sudo cp test /boot/efi/ > /usr/bin/cp: cannot stat '/boot/efi/test': Permission denied > > I have attached a patch with a possible fix. Jim, could you take a look at this patch please? ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#68074: eshell sudo/doas does not work for aliases 2024-01-11 21:00 ` Stefan Kangas @ 2024-01-11 21:15 ` Jim Porter 2024-01-25 2:43 ` Jim Porter 0 siblings, 1 reply; 9+ messages in thread From: Jim Porter @ 2024-01-11 21:15 UTC (permalink / raw) To: Stefan Kangas, Alfonso Sanchez-Beato; +Cc: 68074 On 1/11/2024 1:00 PM, Stefan Kangas wrote: > Alfonso Sanchez-Beato <alfonsosanchezbeato@yahoo.es> writes: > >> sudo/doas does not give the expected permissions when using an eshell alias: [snip] >> I have attached a patch with a possible fix. > > Jim, could you take a look at this patch please? I've been meaning to look at this in detail, but haven't had time. My gut feeling though is that we shouldn't have a special case in this code for when the alias starts with "cd". I'll have to do some testing with aliases and sudo in order to have a more-helpful answer though. At a guess, I think a better place to add code for this is likely in em-tramp.el. ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#68074: eshell sudo/doas does not work for aliases 2024-01-11 21:15 ` Jim Porter @ 2024-01-25 2:43 ` Jim Porter 2024-01-26 23:53 ` Alfonso Sanchez-Beato via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 9+ messages in thread From: Jim Porter @ 2024-01-25 2:43 UTC (permalink / raw) To: Stefan Kangas, Alfonso Sanchez-Beato; +Cc: 68074 [-- Attachment #1: Type: text/plain, Size: 949 bytes --] On 1/11/2024 1:15 PM, Jim Porter wrote: > I've been meaning to look at this in detail, but haven't had time. My > gut feeling though is that we shouldn't have a special case in this code > for when the alias starts with "cd". I'll have to do some testing with > aliases and sudo in order to have a more-helpful answer though. > > At a guess, I think a better place to add code for this is likely in > em-tramp.el. After letting this percolate in my brain for a few weeks, the answer revealed itself to me: the functions in "em-tramp.el" shouldn't throw 'eshell-external', they should throw 'eshell-replace-command'. That allows for a two-step command replacement in this case. First, 'eshell/sudo' (or 'eshell/doas') will do its replacement. Then, Eshell will evaluate that and call the inner command, which will do another replacement to expand the alias. Alfonso, does the attached patch work for you? If so, I'll merge it to master. [-- Attachment #2: 0001-Fix-command-replacement-with-the-Eshell-builtin-vers.patch --] [-- Type: text/plain, Size: 8526 bytes --] From 450e057a3b2305d1a709fd48ca4defca0bb85bbc Mon Sep 17 00:00:00 2001 From: Jim Porter <jporterbugs@gmail.com> Date: Wed, 24 Jan 2024 18:32:00 -0800 Subject: [PATCH] Fix command replacement with the Eshell builtin versions of "sudo" and "doas" This is particularly important when the inner command to execute is an alias. Aliases throw 'eshell-replace-command' too, so we want to do this in two phases: first, replace the "sudo"/"doas" with a let-binding of 'default-directory', and then later, let the alias code do its own replacement (bug#68074). * lisp/eshell/em-tramp.el (eshell/sudo, eshell/doas): Use 'eshell-replace-command' to wrap the inner command. * test/lisp/eshell/em-tramp-tests.el (mock-eshell-named-command): Remove. (em-tramp-test/sudo-basic, em-tramp-test/sudo-user) (em-tramp-test/doas-basic, em-tramp-test/doas-user): Catch 'eshell-replace-command'. --- lisp/eshell/em-tramp.el | 22 ++++---- test/lisp/eshell/em-tramp-tests.el | 89 ++++++++++++++---------------- 2 files changed, 50 insertions(+), 61 deletions(-) diff --git a/lisp/eshell/em-tramp.el b/lisp/eshell/em-tramp.el index 90f9c6cf78d..a58fa6f07d9 100644 --- a/lisp/eshell/em-tramp.el +++ b/lisp/eshell/em-tramp.el @@ -121,12 +121,11 @@ eshell/sudo :usage "[(-u | --user) USER] (-s | --shell) | COMMAND Execute a COMMAND as the superuser or another USER.") (let ((dir (eshell--method-wrap-directory default-directory "sudo" user))) - (if shell - (throw 'eshell-replace-command - (eshell-parse-command "cd" (list dir))) - (throw 'eshell-external - (let ((default-directory dir)) - (eshell-named-command (car args) (cdr args)))))))) + (throw 'eshell-replace-command + (if shell + (eshell-parse-command "cd" (list dir)) + `(let ((default-directory ,dir)) + (eshell-named-command ,(car args) ,(cdr args)))))))) (put 'eshell/sudo 'eshell-no-numeric-conversions t) @@ -144,12 +143,11 @@ eshell/doas :usage "[(-u | --user) USER] (-s | --shell) | COMMAND Execute a COMMAND as the superuser or another USER.") (let ((dir (eshell--method-wrap-directory default-directory "doas" user))) - (if shell - (throw 'eshell-replace-command - (eshell-parse-command "cd" (list dir))) - (throw 'eshell-external - (let ((default-directory dir)) - (eshell-named-command (car args) (cdr args)))))))) + (throw 'eshell-replace-command + (if shell + (eshell-parse-command "cd" (list dir)) + `(let ((default-directory ,dir)) + (eshell-named-command ,(car args) ,(cdr args)))))))) (put 'eshell/doas 'eshell-no-numeric-conversions t) diff --git a/test/lisp/eshell/em-tramp-tests.el b/test/lisp/eshell/em-tramp-tests.el index d33f6a2b46a..deae6579401 100644 --- a/test/lisp/eshell/em-tramp-tests.el +++ b/test/lisp/eshell/em-tramp-tests.el @@ -59,35 +59,31 @@ em-tramp-test/su-login "cd" (list ,(format "/su:root@%s:~/" tramp-default-host)))))) -(defun mock-eshell-named-command (&rest args) - "Dummy function to test Eshell `sudo' command rewriting." - (list default-directory args)) - (ert-deftest em-tramp-test/sudo-basic () "Test Eshell `sudo' command with default user." - (cl-letf (((symbol-function 'eshell-named-command) - #'mock-eshell-named-command)) - (should (equal - (catch 'eshell-external (eshell/sudo "echo" "hi")) - `(,(format "/sudo:root@%s:%s" tramp-default-host default-directory) - ("echo" ("hi"))))) - (should (equal - (catch 'eshell-external (eshell/sudo "echo" "-u" "hi")) - `(,(format "/sudo:root@%s:%s" tramp-default-host default-directory) - ("echo" ("-u" "hi"))))))) + (let ((sudo-directory (format "/sudo:root@%s:%s" + tramp-default-host default-directory))) + (should (equal (catch 'eshell-replace-command + (eshell/sudo "echo" "hi")) + `(let ((default-directory ,sudo-directory)) + (eshell-named-command "echo" ("hi"))))) + (should (equal (catch 'eshell-replace-command + (eshell/sudo "echo" "-u" "hi")) + `(let ((default-directory ,sudo-directory)) + (eshell-named-command "echo" ("-u" "hi"))))))) (ert-deftest em-tramp-test/sudo-user () "Test Eshell `sudo' command with specified user." - (cl-letf (((symbol-function 'eshell-named-command) - #'mock-eshell-named-command)) - (should (equal - (catch 'eshell-external (eshell/sudo "-u" "USER" "echo" "hi")) - `(,(format "/sudo:USER@%s:%s" tramp-default-host default-directory) - ("echo" ("hi"))))) - (should (equal - (catch 'eshell-external (eshell/sudo "-u" "USER" "echo" "-u" "hi")) - `(,(format "/sudo:USER@%s:%s" tramp-default-host default-directory) - ("echo" ("-u" "hi"))))))) + (let ((sudo-directory (format "/sudo:USER@%s:%s" + tramp-default-host default-directory))) + (should (equal (catch 'eshell-replace-command + (eshell/sudo "-u" "USER" "echo" "hi")) + `(let ((default-directory ,sudo-directory)) + (eshell-named-command "echo" ("hi"))))) + (should (equal (catch 'eshell-replace-command + (eshell/sudo "-u" "USER" "echo" "-u" "hi")) + `(let ((default-directory ,sudo-directory)) + (eshell-named-command "echo" ("-u" "hi"))))))) (ert-deftest em-tramp-test/sudo-shell () "Test Eshell `sudo' command with -s/--shell option." @@ -109,34 +105,29 @@ em-tramp-test/sudo-user-shell (ert-deftest em-tramp-test/doas-basic () "Test Eshell `doas' command with default user." - (cl-letf (((symbol-function 'eshell-named-command) - #'mock-eshell-named-command)) - (should (equal - (catch 'eshell-external (eshell/doas "echo" "hi")) - `(,(format "/doas:root@%s:%s" - tramp-default-host default-directory) - ("echo" ("hi"))))) - (should (equal - (catch 'eshell-external (eshell/doas "echo" "-u" "hi")) - `(,(format "/doas:root@%s:%s" - tramp-default-host default-directory) - ("echo" ("-u" "hi"))))))) + (let ((doas-directory (format "/doas:root@%s:%s" + tramp-default-host default-directory))) + (should (equal (catch 'eshell-replace-command + (eshell/doas "echo" "hi")) + `(let ((default-directory ,doas-directory)) + (eshell-named-command "echo" ("hi"))))) + (should (equal (catch 'eshell-replace-command + (eshell/doas "echo" "-u" "hi")) + `(let ((default-directory ,doas-directory)) + (eshell-named-command "echo" ("-u" "hi"))))))) (ert-deftest em-tramp-test/doas-user () "Test Eshell `doas' command with specified user." - (cl-letf (((symbol-function 'eshell-named-command) - #'mock-eshell-named-command)) - (should (equal - (catch 'eshell-external (eshell/doas "-u" "USER" "echo" "hi")) - `(,(format "/doas:USER@%s:%s" - tramp-default-host default-directory) - ("echo" ("hi"))))) - (should (equal - (catch 'eshell-external - (eshell/doas "-u" "USER" "echo" "-u" "hi")) - `(,(format "/doas:USER@%s:%s" - tramp-default-host default-directory) - ("echo" ("-u" "hi"))))))) + (let ((doas-directory (format "/doas:USER@%s:%s" + tramp-default-host default-directory))) + (should (equal (catch 'eshell-replace-command + (eshell/doas "-u" "USER" "echo" "hi")) + `(let ((default-directory ,doas-directory)) + (eshell-named-command "echo" ("hi"))))) + (should (equal (catch 'eshell-replace-command + (eshell/doas "-u" "USER" "echo" "-u" "hi")) + `(let ((default-directory ,doas-directory)) + (eshell-named-command "echo" ("-u" "hi"))))))) (ert-deftest em-tramp-test/doas-shell () "Test Eshell `doas' command with -s/--shell option." -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* bug#68074: eshell sudo/doas does not work for aliases 2024-01-25 2:43 ` Jim Porter @ 2024-01-26 23:53 ` Alfonso Sanchez-Beato via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-01-27 0:27 ` Jim Porter 0 siblings, 1 reply; 9+ messages in thread From: Alfonso Sanchez-Beato via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-01-26 23:53 UTC (permalink / raw) To: Stefan Kangas, Jim Porter; +Cc: 68074@debbugs.gnu.org En jueves, 25 de enero de 2024, 02:43:46 GMT, Jim Porter <jporterbugs@gmail.com> escribió: On 1/11/2024 1:15 PM, Jim Porter wrote: >> I've been meaning to look at this in detail, but haven't had time. My >> gut feeling though is that we shouldn't have a special case in this code >> for when the alias starts with "cd". I'll have to do some testing with >> aliases and sudo in order to have a more-helpful answer though. >> >> At a guess, I think a better place to add code for this is likely in >> em-tramp.el. > After letting this percolate in my brain for a few weeks, the answer > revealed itself to me: the functions in "em-tramp.el" shouldn't throw > 'eshell-external', they should throw 'eshell-replace-command'. That > allows for a two-step command replacement in this case. First, > 'eshell/sudo' (or 'eshell/doas') will do its replacement. Then, Eshell > will evaluate that and call the inner command, which will do another > replacement to expand the alias. > Alfonso, does the attached patch work for you? If so, I'll merge it to > master. Unfortunately that does not seem to be the case, I see an error saying "Invalid function" after applying on top of current master: $ which cp cp is an alias, defined as "*cp $*" $ touch foo $ eshell/sudo cp foo / Invalid function: "foo" [1] $ ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#68074: eshell sudo/doas does not work for aliases 2024-01-26 23:53 ` Alfonso Sanchez-Beato via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-01-27 0:27 ` Jim Porter 2024-01-27 1:19 ` Jim Porter 0 siblings, 1 reply; 9+ messages in thread From: Jim Porter @ 2024-01-27 0:27 UTC (permalink / raw) To: Alfonso Sanchez-Beato, Stefan Kangas; +Cc: 68074@debbugs.gnu.org [-- Attachment #1: Type: text/plain, Size: 300 bytes --] On 1/26/2024 3:53 PM, Alfonso Sanchez-Beato via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote: > Unfortunately that does not seem to be the case, I see an error saying "Invalid function" after applying on top of current master: Oops, I messed up the quoting. How about this? [-- Attachment #2: 0001-Fix-command-replacement-with-the-Eshell-builtin-vers.patch --] [-- Type: text/plain, Size: 8528 bytes --] From c82f0d3dda8ab666438ec7192b0b1ee1dbba4613 Mon Sep 17 00:00:00 2001 From: Jim Porter <jporterbugs@gmail.com> Date: Wed, 24 Jan 2024 18:32:00 -0800 Subject: [PATCH] Fix command replacement with the Eshell builtin versions of "sudo" and "doas" This is particularly important when the inner command to execute is an alias. Aliases throw 'eshell-replace-command' too, so we want to do this in two phases: first, replace the "sudo"/"doas" with a let-binding of 'default-directory', and then later, let the alias code do its own replacement (bug#68074). * lisp/eshell/em-tramp.el (eshell/sudo, eshell/doas): Use 'eshell-replace-command' to wrap the inner command. * test/lisp/eshell/em-tramp-tests.el (mock-eshell-named-command): Remove. (em-tramp-test/sudo-basic, em-tramp-test/sudo-user) (em-tramp-test/doas-basic, em-tramp-test/doas-user): Catch 'eshell-replace-command'. --- lisp/eshell/em-tramp.el | 22 ++++---- test/lisp/eshell/em-tramp-tests.el | 89 ++++++++++++++---------------- 2 files changed, 50 insertions(+), 61 deletions(-) diff --git a/lisp/eshell/em-tramp.el b/lisp/eshell/em-tramp.el index 90f9c6cf78d..9812c9e762a 100644 --- a/lisp/eshell/em-tramp.el +++ b/lisp/eshell/em-tramp.el @@ -121,12 +121,11 @@ eshell/sudo :usage "[(-u | --user) USER] (-s | --shell) | COMMAND Execute a COMMAND as the superuser or another USER.") (let ((dir (eshell--method-wrap-directory default-directory "sudo" user))) - (if shell - (throw 'eshell-replace-command - (eshell-parse-command "cd" (list dir))) - (throw 'eshell-external - (let ((default-directory dir)) - (eshell-named-command (car args) (cdr args)))))))) + (throw 'eshell-replace-command + (if shell + (eshell-parse-command "cd" (list dir)) + `(let ((default-directory ,dir)) + (eshell-named-command ',(car args) ',(cdr args)))))))) (put 'eshell/sudo 'eshell-no-numeric-conversions t) @@ -144,12 +143,11 @@ eshell/doas :usage "[(-u | --user) USER] (-s | --shell) | COMMAND Execute a COMMAND as the superuser or another USER.") (let ((dir (eshell--method-wrap-directory default-directory "doas" user))) - (if shell - (throw 'eshell-replace-command - (eshell-parse-command "cd" (list dir))) - (throw 'eshell-external - (let ((default-directory dir)) - (eshell-named-command (car args) (cdr args)))))))) + (throw 'eshell-replace-command + (if shell + (eshell-parse-command "cd" (list dir)) + `(let ((default-directory ,dir)) + (eshell-named-command ,(car args) ,(cdr args)))))))) (put 'eshell/doas 'eshell-no-numeric-conversions t) diff --git a/test/lisp/eshell/em-tramp-tests.el b/test/lisp/eshell/em-tramp-tests.el index d33f6a2b46a..deae6579401 100644 --- a/test/lisp/eshell/em-tramp-tests.el +++ b/test/lisp/eshell/em-tramp-tests.el @@ -59,35 +59,31 @@ em-tramp-test/su-login "cd" (list ,(format "/su:root@%s:~/" tramp-default-host)))))) -(defun mock-eshell-named-command (&rest args) - "Dummy function to test Eshell `sudo' command rewriting." - (list default-directory args)) - (ert-deftest em-tramp-test/sudo-basic () "Test Eshell `sudo' command with default user." - (cl-letf (((symbol-function 'eshell-named-command) - #'mock-eshell-named-command)) - (should (equal - (catch 'eshell-external (eshell/sudo "echo" "hi")) - `(,(format "/sudo:root@%s:%s" tramp-default-host default-directory) - ("echo" ("hi"))))) - (should (equal - (catch 'eshell-external (eshell/sudo "echo" "-u" "hi")) - `(,(format "/sudo:root@%s:%s" tramp-default-host default-directory) - ("echo" ("-u" "hi"))))))) + (let ((sudo-directory (format "/sudo:root@%s:%s" + tramp-default-host default-directory))) + (should (equal (catch 'eshell-replace-command + (eshell/sudo "echo" "hi")) + `(let ((default-directory ,sudo-directory)) + (eshell-named-command "echo" ("hi"))))) + (should (equal (catch 'eshell-replace-command + (eshell/sudo "echo" "-u" "hi")) + `(let ((default-directory ,sudo-directory)) + (eshell-named-command "echo" ("-u" "hi"))))))) (ert-deftest em-tramp-test/sudo-user () "Test Eshell `sudo' command with specified user." - (cl-letf (((symbol-function 'eshell-named-command) - #'mock-eshell-named-command)) - (should (equal - (catch 'eshell-external (eshell/sudo "-u" "USER" "echo" "hi")) - `(,(format "/sudo:USER@%s:%s" tramp-default-host default-directory) - ("echo" ("hi"))))) - (should (equal - (catch 'eshell-external (eshell/sudo "-u" "USER" "echo" "-u" "hi")) - `(,(format "/sudo:USER@%s:%s" tramp-default-host default-directory) - ("echo" ("-u" "hi"))))))) + (let ((sudo-directory (format "/sudo:USER@%s:%s" + tramp-default-host default-directory))) + (should (equal (catch 'eshell-replace-command + (eshell/sudo "-u" "USER" "echo" "hi")) + `(let ((default-directory ,sudo-directory)) + (eshell-named-command "echo" ("hi"))))) + (should (equal (catch 'eshell-replace-command + (eshell/sudo "-u" "USER" "echo" "-u" "hi")) + `(let ((default-directory ,sudo-directory)) + (eshell-named-command "echo" ("-u" "hi"))))))) (ert-deftest em-tramp-test/sudo-shell () "Test Eshell `sudo' command with -s/--shell option." @@ -109,34 +105,29 @@ em-tramp-test/sudo-user-shell (ert-deftest em-tramp-test/doas-basic () "Test Eshell `doas' command with default user." - (cl-letf (((symbol-function 'eshell-named-command) - #'mock-eshell-named-command)) - (should (equal - (catch 'eshell-external (eshell/doas "echo" "hi")) - `(,(format "/doas:root@%s:%s" - tramp-default-host default-directory) - ("echo" ("hi"))))) - (should (equal - (catch 'eshell-external (eshell/doas "echo" "-u" "hi")) - `(,(format "/doas:root@%s:%s" - tramp-default-host default-directory) - ("echo" ("-u" "hi"))))))) + (let ((doas-directory (format "/doas:root@%s:%s" + tramp-default-host default-directory))) + (should (equal (catch 'eshell-replace-command + (eshell/doas "echo" "hi")) + `(let ((default-directory ,doas-directory)) + (eshell-named-command "echo" ("hi"))))) + (should (equal (catch 'eshell-replace-command + (eshell/doas "echo" "-u" "hi")) + `(let ((default-directory ,doas-directory)) + (eshell-named-command "echo" ("-u" "hi"))))))) (ert-deftest em-tramp-test/doas-user () "Test Eshell `doas' command with specified user." - (cl-letf (((symbol-function 'eshell-named-command) - #'mock-eshell-named-command)) - (should (equal - (catch 'eshell-external (eshell/doas "-u" "USER" "echo" "hi")) - `(,(format "/doas:USER@%s:%s" - tramp-default-host default-directory) - ("echo" ("hi"))))) - (should (equal - (catch 'eshell-external - (eshell/doas "-u" "USER" "echo" "-u" "hi")) - `(,(format "/doas:USER@%s:%s" - tramp-default-host default-directory) - ("echo" ("-u" "hi"))))))) + (let ((doas-directory (format "/doas:USER@%s:%s" + tramp-default-host default-directory))) + (should (equal (catch 'eshell-replace-command + (eshell/doas "-u" "USER" "echo" "hi")) + `(let ((default-directory ,doas-directory)) + (eshell-named-command "echo" ("hi"))))) + (should (equal (catch 'eshell-replace-command + (eshell/doas "-u" "USER" "echo" "-u" "hi")) + `(let ((default-directory ,doas-directory)) + (eshell-named-command "echo" ("-u" "hi"))))))) (ert-deftest em-tramp-test/doas-shell () "Test Eshell `doas' command with -s/--shell option." -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* bug#68074: eshell sudo/doas does not work for aliases 2024-01-27 0:27 ` Jim Porter @ 2024-01-27 1:19 ` Jim Porter 2024-01-27 8:46 ` Alfonso Sanchez-Beato via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 9+ messages in thread From: Jim Porter @ 2024-01-27 1:19 UTC (permalink / raw) To: Alfonso Sanchez-Beato, Stefan Kangas; +Cc: 68074@debbugs.gnu.org [-- Attachment #1: Type: text/plain, Size: 597 bytes --] On 1/26/2024 4:27 PM, Jim Porter wrote: > On 1/26/2024 3:53 PM, Alfonso Sanchez-Beato via Bug reports for GNU > Emacs, the Swiss army knife of text editors wrote: >> Unfortunately that does not seem to be the case, I see an error saying >> "Invalid function" after applying on top of current master: > > Oops, I messed up the quoting. How about this? ... actually, this is a more-complete patch. I'm not 100% sure about this part though: ',(car args) That (usually) creates something like (quote "command"), but it's safer than not quoting (the CAR of 'args' can be anything, really...) [-- Attachment #2: 0001-Fix-command-replacement-with-the-Eshell-builtin-vers.patch --] [-- Type: text/plain, Size: 8546 bytes --] From aa4cb74054a1bd53cf707fb4ef60593044de1338 Mon Sep 17 00:00:00 2001 From: Jim Porter <jporterbugs@gmail.com> Date: Wed, 24 Jan 2024 18:32:00 -0800 Subject: [PATCH] Fix command replacement with the Eshell builtin versions of "sudo" and "doas" This is particularly important when the inner command to execute is an alias. Aliases throw 'eshell-replace-command' too, so we want to do this in two phases: first, replace the "sudo"/"doas" with a let-binding of 'default-directory', and then later, let the alias code do its own replacement (bug#68074). * lisp/eshell/em-tramp.el (eshell/sudo, eshell/doas): Use 'eshell-replace-command' to wrap the inner command. * test/lisp/eshell/em-tramp-tests.el (mock-eshell-named-command): Remove. (em-tramp-test/sudo-basic, em-tramp-test/sudo-user) (em-tramp-test/doas-basic, em-tramp-test/doas-user): Catch 'eshell-replace-command'. --- lisp/eshell/em-tramp.el | 22 ++++---- test/lisp/eshell/em-tramp-tests.el | 89 ++++++++++++++---------------- 2 files changed, 50 insertions(+), 61 deletions(-) diff --git a/lisp/eshell/em-tramp.el b/lisp/eshell/em-tramp.el index 90f9c6cf78d..efb37225651 100644 --- a/lisp/eshell/em-tramp.el +++ b/lisp/eshell/em-tramp.el @@ -121,12 +121,11 @@ eshell/sudo :usage "[(-u | --user) USER] (-s | --shell) | COMMAND Execute a COMMAND as the superuser or another USER.") (let ((dir (eshell--method-wrap-directory default-directory "sudo" user))) - (if shell - (throw 'eshell-replace-command - (eshell-parse-command "cd" (list dir))) - (throw 'eshell-external - (let ((default-directory dir)) - (eshell-named-command (car args) (cdr args)))))))) + (throw 'eshell-replace-command + (if shell + (eshell-parse-command "cd" (list dir)) + `(let ((default-directory ,dir)) + (eshell-named-command ',(car args) ',(cdr args)))))))) (put 'eshell/sudo 'eshell-no-numeric-conversions t) @@ -144,12 +143,11 @@ eshell/doas :usage "[(-u | --user) USER] (-s | --shell) | COMMAND Execute a COMMAND as the superuser or another USER.") (let ((dir (eshell--method-wrap-directory default-directory "doas" user))) - (if shell - (throw 'eshell-replace-command - (eshell-parse-command "cd" (list dir))) - (throw 'eshell-external - (let ((default-directory dir)) - (eshell-named-command (car args) (cdr args)))))))) + (throw 'eshell-replace-command + (if shell + (eshell-parse-command "cd" (list dir)) + `(let ((default-directory ,dir)) + (eshell-named-command ',(car args) ',(cdr args)))))))) (put 'eshell/doas 'eshell-no-numeric-conversions t) diff --git a/test/lisp/eshell/em-tramp-tests.el b/test/lisp/eshell/em-tramp-tests.el index d33f6a2b46a..3be5d3542ca 100644 --- a/test/lisp/eshell/em-tramp-tests.el +++ b/test/lisp/eshell/em-tramp-tests.el @@ -59,35 +59,31 @@ em-tramp-test/su-login "cd" (list ,(format "/su:root@%s:~/" tramp-default-host)))))) -(defun mock-eshell-named-command (&rest args) - "Dummy function to test Eshell `sudo' command rewriting." - (list default-directory args)) - (ert-deftest em-tramp-test/sudo-basic () "Test Eshell `sudo' command with default user." - (cl-letf (((symbol-function 'eshell-named-command) - #'mock-eshell-named-command)) - (should (equal - (catch 'eshell-external (eshell/sudo "echo" "hi")) - `(,(format "/sudo:root@%s:%s" tramp-default-host default-directory) - ("echo" ("hi"))))) - (should (equal - (catch 'eshell-external (eshell/sudo "echo" "-u" "hi")) - `(,(format "/sudo:root@%s:%s" tramp-default-host default-directory) - ("echo" ("-u" "hi"))))))) + (let ((sudo-directory (format "/sudo:root@%s:%s" + tramp-default-host default-directory))) + (should (equal (catch 'eshell-replace-command + (eshell/sudo "echo" "hi")) + `(let ((default-directory ,sudo-directory)) + (eshell-named-command '"echo" '("hi"))))) + (should (equal (catch 'eshell-replace-command + (eshell/sudo "echo" "-u" "hi")) + `(let ((default-directory ,sudo-directory)) + (eshell-named-command '"echo" '("-u" "hi"))))))) (ert-deftest em-tramp-test/sudo-user () "Test Eshell `sudo' command with specified user." - (cl-letf (((symbol-function 'eshell-named-command) - #'mock-eshell-named-command)) - (should (equal - (catch 'eshell-external (eshell/sudo "-u" "USER" "echo" "hi")) - `(,(format "/sudo:USER@%s:%s" tramp-default-host default-directory) - ("echo" ("hi"))))) - (should (equal - (catch 'eshell-external (eshell/sudo "-u" "USER" "echo" "-u" "hi")) - `(,(format "/sudo:USER@%s:%s" tramp-default-host default-directory) - ("echo" ("-u" "hi"))))))) + (let ((sudo-directory (format "/sudo:USER@%s:%s" + tramp-default-host default-directory))) + (should (equal (catch 'eshell-replace-command + (eshell/sudo "-u" "USER" "echo" "hi")) + `(let ((default-directory ,sudo-directory)) + (eshell-named-command '"echo" '("hi"))))) + (should (equal (catch 'eshell-replace-command + (eshell/sudo "-u" "USER" "echo" "-u" "hi")) + `(let ((default-directory ,sudo-directory)) + (eshell-named-command '"echo" '("-u" "hi"))))))) (ert-deftest em-tramp-test/sudo-shell () "Test Eshell `sudo' command with -s/--shell option." @@ -109,34 +105,29 @@ em-tramp-test/sudo-user-shell (ert-deftest em-tramp-test/doas-basic () "Test Eshell `doas' command with default user." - (cl-letf (((symbol-function 'eshell-named-command) - #'mock-eshell-named-command)) - (should (equal - (catch 'eshell-external (eshell/doas "echo" "hi")) - `(,(format "/doas:root@%s:%s" - tramp-default-host default-directory) - ("echo" ("hi"))))) - (should (equal - (catch 'eshell-external (eshell/doas "echo" "-u" "hi")) - `(,(format "/doas:root@%s:%s" - tramp-default-host default-directory) - ("echo" ("-u" "hi"))))))) + (let ((doas-directory (format "/doas:root@%s:%s" + tramp-default-host default-directory))) + (should (equal (catch 'eshell-replace-command + (eshell/doas "echo" "hi")) + `(let ((default-directory ,doas-directory)) + (eshell-named-command '"echo" '("hi"))))) + (should (equal (catch 'eshell-replace-command + (eshell/doas "echo" "-u" "hi")) + `(let ((default-directory ,doas-directory)) + (eshell-named-command '"echo" '("-u" "hi"))))))) (ert-deftest em-tramp-test/doas-user () "Test Eshell `doas' command with specified user." - (cl-letf (((symbol-function 'eshell-named-command) - #'mock-eshell-named-command)) - (should (equal - (catch 'eshell-external (eshell/doas "-u" "USER" "echo" "hi")) - `(,(format "/doas:USER@%s:%s" - tramp-default-host default-directory) - ("echo" ("hi"))))) - (should (equal - (catch 'eshell-external - (eshell/doas "-u" "USER" "echo" "-u" "hi")) - `(,(format "/doas:USER@%s:%s" - tramp-default-host default-directory) - ("echo" ("-u" "hi"))))))) + (let ((doas-directory (format "/doas:USER@%s:%s" + tramp-default-host default-directory))) + (should (equal (catch 'eshell-replace-command + (eshell/doas "-u" "USER" "echo" "hi")) + `(let ((default-directory ,doas-directory)) + (eshell-named-command '"echo" '("hi"))))) + (should (equal (catch 'eshell-replace-command + (eshell/doas "-u" "USER" "echo" "-u" "hi")) + `(let ((default-directory ,doas-directory)) + (eshell-named-command '"echo" '("-u" "hi"))))))) (ert-deftest em-tramp-test/doas-shell () "Test Eshell `doas' command with -s/--shell option." -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* bug#68074: eshell sudo/doas does not work for aliases 2024-01-27 1:19 ` Jim Porter @ 2024-01-27 8:46 ` Alfonso Sanchez-Beato via Bug reports for GNU Emacs, the Swiss army knife of text editors 2024-01-27 20:24 ` Jim Porter 0 siblings, 1 reply; 9+ messages in thread From: Alfonso Sanchez-Beato via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-01-27 8:46 UTC (permalink / raw) To: Stefan Kangas, Jim Porter; +Cc: 68074@debbugs.gnu.org En sábado, 27 de enero de 2024, 01:19:32 GMT, Jim Porter <jporterbugs@gmail.com> escribió: On 1/26/2024 4:27 PM, Jim Porter wrote: >> On 1/26/2024 3:53 PM, Alfonso Sanchez-Beato via Bug reports for GNU >> Emacs, the Swiss army knife of text editors wrote: >>> Unfortunately that does not seem to be the case, I see an error saying >>> "Invalid function" after applying on top of current master: >> >> Oops, I messed up the quoting. How about this? > ... actually, this is a more-complete patch. I'm not 100% sure about > this part though: > > ',(car args) > > That (usually) creates something like (quote "command"), but it's safer > than not quoting (the CAR of 'args' can be anything, really...) This last patch works nicely, thanks a lot! It works also in cases where my patch was not, like: $ eshell/sudo VAR=val <alias> ... ^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#68074: eshell sudo/doas does not work for aliases 2024-01-27 8:46 ` Alfonso Sanchez-Beato via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-01-27 20:24 ` Jim Porter 0 siblings, 0 replies; 9+ messages in thread From: Jim Porter @ 2024-01-27 20:24 UTC (permalink / raw) To: Alfonso Sanchez-Beato, Stefan Kangas; +Cc: 68074-done On 1/27/2024 12:46 AM, Alfonso Sanchez-Beato via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote: > > En sábado, 27 de enero de 2024, 01:19:32 GMT, Jim Porter <jporterbugs@gmail.com> escribió: >> ... actually, this is a more-complete patch. I'm not 100% sure about >> this part though: >> >> ',(car args) >> >> That (usually) creates something like (quote "command"), but it's safer I thought about this some more and the extraneous quoting is fine in my opinion. Eshell already does that quite a bit in 'eshell-do-eval', so what's one more case? > This last patch works nicely, thanks a lot! It works also in cases where my patch was not, like: > > $ eshell/sudo VAR=val <alias> ... Wait, that works?! (After trying it out locally, so it does!) Looking at the code, I see why now: 'eshell-named-command' calls 'eshell-prepare-command-hook', and that hook is where we handle local variables. However, I truly didn't expect that; I thought the local variable handling occurred in an earlier phase. The more you know... Anyway, since this works even better than I'd expected, I've now merged my patch to the master branch as 3c680968e49. Closing this bug now. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-01-27 20:24 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <31356544.8861481.1703711546895.ref@mail.yahoo.com>
2023-12-27 21:12 ` bug#68074: eshell sudo/doas does not work for aliases Alfonso Sanchez-Beato via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-01-11 21:00 ` Stefan Kangas
2024-01-11 21:15 ` Jim Porter
2024-01-25 2:43 ` Jim Porter
2024-01-26 23:53 ` Alfonso Sanchez-Beato via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-01-27 0:27 ` Jim Porter
2024-01-27 1:19 ` Jim Porter
2024-01-27 8:46 ` Alfonso Sanchez-Beato via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-01-27 20:24 ` Jim Porter
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.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).