Add chmod builtin

Add chmod builtin

[Matthew Martin]

Previously in #zsh "so sad that there is no zf_chmod". So here we are.

Since AT_SYMLINK_NOFOLLOW isn't implemented for some fchmodat
implementations (and modifying the mode of a symlink is usually
nonsensical), drop the -h that chgrp/chown support.

I realize Doc/Zsh/mod_files.yo uses behaviour while _chmod uses
behavior. I just matched the zf_chown docs and _chown.

- Matthew Martin

diff --git a/Completion/Unix/Command/_chmod b/Completion/Unix/Command/_chmod
index af64b9eb9..7a9769fd0 100644
--- a/Completion/Unix/Command/_chmod
+++ b/Completion/Unix/Command/_chmod
@@ -1,11 +1,19 @@
-#compdef chmod gchmod
+#compdef chmod gchmod zf_chmod

 local curcontext="$curcontext" state line expl ret=1
 local -a args privs

 args=( '*: :->files' '1: :_file_modes' )

-if _pick_variant gnu=Free\ Soft unix --version; then
+if zmodload -e zsh/files && [[ $words[1] != */* ]]; then
+  # Assign, not append because zf_chmod only supports octal modes.
+  args=(
+    '-R[change files and directories recursively]' \
+    '-s[enable paranoid behavior]' \
+    '1:octal mode:' \
+    '*: :->files'
+  )
+elif _pick_variant gnu=Free\ Soft unix --version; then
   args+=(
     '(-v --verbose -c --changes)'{-c,--changes}'[report changes made]'
     '(-v --verbose -c --changes)'{-v,--verbose}'[output a diagnostic
for every file processed]'
diff --git a/Doc/Zsh/mod_files.yo b/Doc/Zsh/mod_files.yo
index 90e988474..3cf7b61e3 100644
--- a/Doc/Zsh/mod_files.yo
+++ b/Doc/Zsh/mod_files.yo
@@ -23,6 +23,26 @@ item(tt(chgrp) [ tt(-hRs) ] var(group) var(filename) ...)(
 Changes group of files specified.  This is equivalent to tt(chown) with
 a var(user-spec) argument of `tt(:)var(group)'.
 )
+findex(chmod)
+item(tt(chmod) [ tt(-Rs) ] var(mode) var(filename) ...)(
+Changes mode of files specified.
+
+The specified var(mode) must be in octal.
+
+The tt(-R) option causes tt(chmod) to recursively descend into directories,
+changing the mode of all files in the directory after
+changing the mode of the directory itself.
+
+The tt(-s) option is a zsh extension to tt(chmod) functionality.  It enables
+paranoid behaviour, intended to avoid security problems involving
+a tt(chmod) being tricked into affecting files other than the ones
+intended.  It will refuse to follow symbolic links, so that (for example)
+``tt(chmod 600 /tmp/foo/passwd)'' can't accidentally chmod tt(/etc/passwd)
+if tt(/tmp/foo) happens to be a link to tt(/etc).  It will also check
+where it is after leaving directories, so that a recursive chmod of
+a deep directory tree can't end up recursively chmoding tt(/usr) as
+a result of directories being moved up the tree.
+)
 findex(chown)
 item(tt(chown) [ tt(-hRs) ] var(user-spec) var(filename) ...)(
 Changes ownership and group of files specified.
diff --git a/Src/Modules/files.c b/Src/Modules/files.c
index 6f816bac0..85764d55e 100644
--- a/Src/Modules/files.c
+++ b/Src/Modules/files.c
@@ -619,6 +619,45 @@ bin_rm(char *nam, char **args, Options ops,
UNUSED(int func))
     return OPT_ISSET(ops,'f') ? 0 : err;
 }

+/* chmod builtin */
+
+struct chmodmagic {
+    char *nam;
+    mode_t mode;
+};
+
+/**/
+static int
+chmod_dochmod(char *arg, char *rp, UNUSED(struct stat const *sp), void *magic)
+{
+    struct chmodmagic *chm = magic;
+
+    if(chmod(rp, chm->mode)) {
+ zwarnnam(chm->nam, "%s: %e", arg, errno);
+ return 1;
+    }
+    return 0;
+}
+
+/**/
+static int
+bin_chmod(char *nam, char **args, Options ops, int func)
+{
+    struct chmodmagic chm;
+    char *str = args[0], *ptr;
+
+    chm.nam = nam;
+
+    chm.mode = zstrtol(str, &ptr, 8);
+    if(!*str || *ptr) {
+ zwarnnam(nam, "invalid mode `%s'", str);
+ return 1;
+    }
+
+    return recursivecmd(nam, 0, OPT_ISSET(ops,'R'), OPT_ISSET(ops,'s'),
+ args + 1, chmod_dochmod, recurse_donothing, chmod_dochmod, &chm);
+}
+
 /* chown builtin */

 struct chownmagic {
@@ -754,6 +793,7 @@ static struct builtin bintab[] = {
     /* The names which overlap commands without necessarily being
      * fully compatible. */
     BUILTIN("chgrp", 0, bin_chown, 2, -1, BIN_CHGRP, "hRs",    NULL),
+    BUILTIN("chmod", 0, bin_chmod, 2, -1, 0,         "Rs",    NULL),
     BUILTIN("chown", 0, bin_chown, 2, -1, BIN_CHOWN, "hRs",    NULL),
     BUILTIN("ln",    0, bin_ln,    1, -1, BIN_LN,    LN_OPTS, NULL),
     BUILTIN("mkdir", 0, bin_mkdir, 1, -1, 0,         "pm:",   NULL),
@@ -763,6 +803,7 @@ static struct builtin bintab[] = {
     BUILTIN("sync",  0, bin_sync,  0,  0, 0,         NULL,    NULL),
     /* The "safe" zsh-only names */
     BUILTIN("zf_chgrp", 0, bin_chown, 2, -1, BIN_CHGRP, "hRs",    NULL),
+    BUILTIN("zf_chmod", 0, bin_chmod, 2, -1, 0,         "Rs",    NULL),
     BUILTIN("zf_chown", 0, bin_chown, 2, -1, BIN_CHOWN, "hRs",    NULL),
     BUILTIN("zf_ln",    0, bin_ln,    1, -1, BIN_LN,    LN_OPTS, NULL),
     BUILTIN("zf_mkdir", 0, bin_mkdir, 1, -1, 0,         "pm:",   NULL),

Re: Add chmod builtin

[dana]

On 16 Mar 2019, at 22:08, Matthew Martin <phy1729@gmail.com> wrote:
>Previously in #zsh "so sad that there is no zf_chmod". So here we are.

ty

One issue i mentioned when testing this is that it always follows symlinks
found in the tree when working recursively, which isn't how any real-world
chmod(1) works. The existing chown/chgrp built-ins have the same issue,
though, so it doesn't seem necessarily like a blocking thing.

>+if zmodload -e zsh/files && [[ $words[1] != */* ]]; then

I don't think this is reliable. It will detect any chmod as the built-in even
if only zf_chown is loaded (with -F). And `zmodload -e` always returns 1 for
modules loaded through selective auto-loading (-Fa), so it will *never*
detect the built-in in that case. (Not sure if that -e behaviour is intended?)

I know the $OSTYPE-$variant thing is dumb (should have passed $OSTYPE directly
to _pick_variant), but otherwise i think the method i used for distinguishing
the built-ins in _stat is fairly robust.

PS: Your patch seems to have corrupt white space

dana

Re: Add chmod builtin

[Matthew Martin]

[-- Attachment #1: Type: text/plain, Size: 1170 bytes --]

On Sun, Mar 17, 2019 at 6:27 AM dana <dana@dana.is> wrote:
> >+if zmodload -e zsh/files && [[ $words[1] != */* ]]; then
>
> I don't think this is reliable. It will detect any chmod as the built-in even
> if only zf_chown is loaded (with -F). And `zmodload -e` always returns 1 for
> modules loaded through selective auto-loading (-Fa), so it will *never*
> detect the built-in in that case. (Not sure if that -e behaviour is intended?)
>
> I know the $OSTYPE-$variant thing is dumb (should have passed $OSTYPE directly
> to _pick_variant), but otherwise i think the method i used for distinguishing
> the built-ins in _stat is fairly robust.

Reading the docs for _pick_variant, there's a -b option to detect
builtins. It has some issues (namely with precommands), but those can be
fixed in a separate commit. Requires some reindenting, so I've split the
patch in two.

> PS: Your patch seems to have corrupt white space

Gmail hasn't been sending mails sent via mutt for the past few months.
Not sure what's causing it, but I've had to resort to using the web
interface which seems to mangle whitespace. I've attached the patches
and they're on my github repo as well.

[-- Attachment #2: 0001-44135-_chmod-Reformat-to-minimize-next-diff.-No-func.patch --]
[-- Type: text/x-patch, Size: 4760 bytes --]

From 2f670be951b1b6e242691562a9daf06010bfce99 Mon Sep 17 00:00:00 2001
From: Matthew Martin <phy1729@gmail.com>
Date: Sun, 17 Mar 2019 19:24:56 -0500
Subject: [PATCH 1/2] 44135: _chmod: Reformat to minimize next diff. No
 functional change.

---
 Completion/Unix/Command/_chmod | 94 ++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 44 deletions(-)

diff --git a/Completion/Unix/Command/_chmod b/Completion/Unix/Command/_chmod
index af64b9eb9..6a44a4ef1 100644
--- a/Completion/Unix/Command/_chmod
+++ b/Completion/Unix/Command/_chmod
@@ -1,52 +1,58 @@
 #compdef chmod gchmod
 
-local curcontext="$curcontext" state line expl ret=1
+local curcontext="$curcontext" state line expl ret=1 variant
 local -a args privs
 
 args=( '*: :->files' '1: :_file_modes' )
 
-if _pick_variant gnu=Free\ Soft unix --version; then
-  args+=(
-    '(-v --verbose -c --changes)'{-c,--changes}'[report changes made]'
-    '(-v --verbose -c --changes)'{-v,--verbose}'[output a diagnostic for every file processed]'
-    '(-f --silent --quiet)'{-f,--silent,--quiet}'[suppress most error messages]'
-    '(--no-preserve-root)--preserve-root[fail to operate recursively on /]'
-    "(--preserve-root)--no-preserve-root[don't treat / specially (default)]"
-    '(1)--reference=[copy permissions of specified file]:file:_files'
-    '(-R --recursive)'{-R,--recursive}'[change files and directories recursively]'
-    '(- : *)--help[display help information]'
-    '(- : *)--version[display version information]'
-  )
-else
-  args+=(
-    '-f[suppress most error messages]'
-    '-R[change files and directories recursively]'
-  )
-  case $OSTYPE in
-    freebsd*|dragonfly*|darwin*)
-      args+=( '-v[output a diagnostic for every file processed]')
+_pick_variant -r variant gnu=Free\ Soft $OSTYPE --version
+case "$variant" in
+  gnu)
+    args+=(
+      '(-v --verbose -c --changes)'{-c,--changes}'[report changes made]'
+      '(-v --verbose -c --changes)'{-v,--verbose}'[output a diagnostic for every file processed]'
+      '(-f --silent --quiet)'{-f,--silent,--quiet}'[suppress most error messages]'
+      '(--no-preserve-root)--preserve-root[fail to operate recursively on /]'
+      "(--preserve-root)--no-preserve-root[don't treat / specially (default)]"
+      '(1)--reference=[copy permissions of specified file]:file:_files'
+      '(-R --recursive)'{-R,--recursive}'[change files and directories recursively]'
+      '(- : *)--help[display help information]'
+      '(- : *)--version[display version information]'
+    )
+    ;;
+  *)
+    args+=(
+      '-f[suppress most error messages]'
+      '-R[change files and directories recursively]'
+    )
+    ;|
+  freebsd*|dragonfly*|darwin*)
+    args+=(
+      '-v[output a diagnostic for every file processed]'
+    )
     ;|
-    freebsd*|netbsd*|darwin*|dragonfly*)
-      args+=( "-h[operate on symlinks them self]" )
+  freebsd*|netbsd*|darwin*|dragonfly*)
+    args+=(
+      '-h[operate on symlinks them self]'
+    )
     ;|
-    freebsd*|openbsd*|netbsd*|darwin*|dragonfly*)
-      args+=(
-	'(-H -L -P)-L[follow all symlinks]'
-	'(-H -L -P)-H[follow symlinks on the command line]'
-	'(-H -L -P)-P[do not follow symlinks (default)]'
-      )
+  freebsd*|openbsd*|netbsd*|darwin*|dragonfly*)
+    args+=(
+      '(-H -L -P)-L[follow all symlinks]'
+      '(-H -L -P)-H[follow symlinks on the command line]'
+      '(-H -L -P)-P[do not follow symlinks (default)]'
+    )
     ;|
-    darwin*)
-      args+=(
-        '(1)-C[returns false if any of the named files have ACLs]'
-	'(1)-N[remove ACLs from specified files]'
-	'(1)-E[read ACL info from stdin as a sequential list of ACEs]'
-	'(1)-i[removes inherited bit from all entries in named files ACLs]'
-        '(1)-I[removes all inherited entries from named files ACLs]'
-      )
+  darwin*)
+    args+=(
+      '(1)-C[returns false if any of the named files have ACLs]'
+      '(1)-N[remove ACLs from specified files]'
+      '(1)-E[read ACL info from stdin as a sequential list of ACEs]'
+      '(1)-i[removes inherited bit from all entries in named files ACLs]'
+      '(1)-I[removes all inherited entries from named files ACLs]'
+    )
     ;;
-  esac
-fi
+esac
 
 _arguments -C -s "$args[@]" && ret=0
 
@@ -62,12 +68,12 @@ case "$state" in
       local spec who op priv
       local -a specs
       for spec in ${(s:,:)line[1]}; do
-	if [[ ${spec#*[+-=]} != [rwxst]## ]]; then
-	  _files && ret=0
-	  return ret
-	fi
+        if [[ ${spec#*[+-=]} != [rwxst]## ]]; then
+          _files && ret=0
+          return ret
+        fi
 
-	specs+=( ${${(M)spec##[+-=]*}:+a}$spec )
+        specs+=( ${${(M)spec##[+-=]*}:+a}$spec )
       done
       _wanted files expl file _files -g "*(-.^f:${(j.,.)specs}:)" && ret=0
     fi
-- 
2.21.0


[-- Attachment #3: 0002-44135-Add-chmod-builtin.patch --]
[-- Type: text/x-patch, Size: 4874 bytes --]

From 8d526281e56cec6579892706c2c305246269b7eb Mon Sep 17 00:00:00 2001
From: Matthew Martin <phy1729@gmail.com>
Date: Sun, 17 Mar 2019 19:30:47 -0500
Subject: [PATCH 2/2] 44135: Add chmod builtin

---
 Completion/Unix/Command/_chmod | 13 +++++++++--
 Doc/Zsh/mod_files.yo           | 20 +++++++++++++++++
 Src/Modules/files.c            | 41 ++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/Completion/Unix/Command/_chmod b/Completion/Unix/Command/_chmod
index 6a44a4ef1..89a7fbd43 100644
--- a/Completion/Unix/Command/_chmod
+++ b/Completion/Unix/Command/_chmod
@@ -1,12 +1,21 @@
-#compdef chmod gchmod
+#compdef chmod gchmod zf_chmod
 
 local curcontext="$curcontext" state line expl ret=1 variant
 local -a args privs
 
 args=( '*: :->files' '1: :_file_modes' )
 
-_pick_variant -r variant gnu=Free\ Soft $OSTYPE --version
+_pick_variant -r variant -b zsh gnu=Free\ Soft $OSTYPE --version
 case "$variant" in
+  zsh)
+    # Assign, not append because zf_chmod only supports octal modes.
+    args=(
+      '-R[change files and directories recursively]' \
+      '-s[enable paranoid behavior]' \
+      '1:octal mode:' \
+      '*: :->files'
+    )
+    ;;
   gnu)
     args+=(
       '(-v --verbose -c --changes)'{-c,--changes}'[report changes made]'
diff --git a/Doc/Zsh/mod_files.yo b/Doc/Zsh/mod_files.yo
index 90e988474..3cf7b61e3 100644
--- a/Doc/Zsh/mod_files.yo
+++ b/Doc/Zsh/mod_files.yo
@@ -23,6 +23,26 @@ item(tt(chgrp) [ tt(-hRs) ] var(group) var(filename) ...)(
 Changes group of files specified.  This is equivalent to tt(chown) with
 a var(user-spec) argument of `tt(:)var(group)'.
 )
+findex(chmod)
+item(tt(chmod) [ tt(-Rs) ] var(mode) var(filename) ...)(
+Changes mode of files specified.
+
+The specified var(mode) must be in octal.
+
+The tt(-R) option causes tt(chmod) to recursively descend into directories,
+changing the mode of all files in the directory after
+changing the mode of the directory itself.
+
+The tt(-s) option is a zsh extension to tt(chmod) functionality.  It enables
+paranoid behaviour, intended to avoid security problems involving
+a tt(chmod) being tricked into affecting files other than the ones
+intended.  It will refuse to follow symbolic links, so that (for example)
+``tt(chmod 600 /tmp/foo/passwd)'' can't accidentally chmod tt(/etc/passwd)
+if tt(/tmp/foo) happens to be a link to tt(/etc).  It will also check
+where it is after leaving directories, so that a recursive chmod of
+a deep directory tree can't end up recursively chmoding tt(/usr) as
+a result of directories being moved up the tree.
+)
 findex(chown)
 item(tt(chown) [ tt(-hRs) ] var(user-spec) var(filename) ...)(
 Changes ownership and group of files specified.
diff --git a/Src/Modules/files.c b/Src/Modules/files.c
index 6f816bac0..85764d55e 100644
--- a/Src/Modules/files.c
+++ b/Src/Modules/files.c
@@ -619,6 +619,45 @@ bin_rm(char *nam, char **args, Options ops, UNUSED(int func))
     return OPT_ISSET(ops,'f') ? 0 : err;
 }
 
+/* chmod builtin */
+
+struct chmodmagic {
+    char *nam;
+    mode_t mode;
+};
+
+/**/
+static int
+chmod_dochmod(char *arg, char *rp, UNUSED(struct stat const *sp), void *magic)
+{
+    struct chmodmagic *chm = magic;
+
+    if(chmod(rp, chm->mode)) {
+	zwarnnam(chm->nam, "%s: %e", arg, errno);
+	return 1;
+    }
+    return 0;
+}
+
+/**/
+static int
+bin_chmod(char *nam, char **args, Options ops, int func)
+{
+    struct chmodmagic chm;
+    char *str = args[0], *ptr;
+
+    chm.nam = nam;
+
+    chm.mode = zstrtol(str, &ptr, 8);
+    if(!*str || *ptr) {
+	zwarnnam(nam, "invalid mode `%s'", str);
+	return 1;
+    }
+
+    return recursivecmd(nam, 0, OPT_ISSET(ops,'R'), OPT_ISSET(ops,'s'),
+	args + 1, chmod_dochmod, recurse_donothing, chmod_dochmod, &chm);
+}
+
 /* chown builtin */
 
 struct chownmagic {
@@ -754,6 +793,7 @@ static struct builtin bintab[] = {
     /* The names which overlap commands without necessarily being
      * fully compatible. */
     BUILTIN("chgrp", 0, bin_chown, 2, -1, BIN_CHGRP, "hRs",    NULL),
+    BUILTIN("chmod", 0, bin_chmod, 2, -1, 0,         "Rs",    NULL),
     BUILTIN("chown", 0, bin_chown, 2, -1, BIN_CHOWN, "hRs",    NULL),
     BUILTIN("ln",    0, bin_ln,    1, -1, BIN_LN,    LN_OPTS, NULL),
     BUILTIN("mkdir", 0, bin_mkdir, 1, -1, 0,         "pm:",   NULL),
@@ -763,6 +803,7 @@ static struct builtin bintab[] = {
     BUILTIN("sync",  0, bin_sync,  0,  0, 0,         NULL,    NULL),
     /* The "safe" zsh-only names */
     BUILTIN("zf_chgrp", 0, bin_chown, 2, -1, BIN_CHGRP, "hRs",    NULL),
+    BUILTIN("zf_chmod", 0, bin_chmod, 2, -1, 0,         "Rs",    NULL),
     BUILTIN("zf_chown", 0, bin_chown, 2, -1, BIN_CHOWN, "hRs",    NULL),
     BUILTIN("zf_ln",    0, bin_ln,    1, -1, BIN_LN,    LN_OPTS, NULL),
     BUILTIN("zf_mkdir", 0, bin_mkdir, 1, -1, 0,         "pm:",   NULL),
-- 
2.21.0

Re: Add chmod builtin

[dana]

On 17 Mar 2019, at 19:44, Matthew Martin <phy1729@gmail.com> wrote:
>Reading the docs for _pick_variant, there's a -b option to detect
>builtins. It has some issues (namely with precommands), but those can be
>fixed in a separate commit. Requires some reindenting, so I've split the
>patch in two.

Yeah, that works for me. Just the $precommands stuff is an issue, and i like
your idea for incorporating that into -b; AFAIK, if we do another pass on
that afterwards, we can then rip out all of the explicit $precommands checks
in _stat and every other function

dana