"core.sharedrepository = 0mode" should set, not loosen

This fixes the behaviour of octal notation to how it is defined in the
documentation, while keeping the traditional "loosen only" semantics
intact for "group" and "everybody".

Three main points of this patch are:

- For an explicit octal notation, the internal shared_repository variable
is set to a negative value, so that we can tell "group" (which is to
"OR" in 0660) and 0660 (which is to "SET" to 0660);

- git-init did not set shared_repository variable early enough to affect
the initial creation of many files, notably copied templates and the
configuration.  We set it very early when a command-line option
specifies a custom value.

- Many codepaths create files inside $GIT_DIR by various ways that all
involve mkstemp(), and then call move_temp_to_file() to rename it to
its final destination.  We can add adjust_shared_perm() call here; for
the traditional "loosen-only", this would be a no-op for many codepaths
because the mode is already loose enough, but with the new behaviour it
makes a difference.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-init-db.c b/builtin-init-db.c
index ee3911f8eef5c00eeb4868c843f2b8c5c83d688d..8199e5d4d5168859363b7a644535fd0cb01a1441 100644 (file)
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -195,6 +195,8 @@ static int create_default_files(const char *template_path)
 
        git_config(git_default_config, NULL);
        is_bare_repository_cfg = init_is_bare_repository;
+
+       /* reading existing config may have overwrote it */
        if (init_shared_repository != -1)
                shared_repository = init_shared_repository;
 
@@ -313,12 +315,15 @@ int init_db(const char *template_dir, unsigned int flags)
                 * and compatibility values for PERM_GROUP and
                 * PERM_EVERYBODY.
                 */
-               if (shared_repository == PERM_GROUP)
+               if (shared_repository < 0)
+                       /* force to the mode value */
+                       sprintf(buf, "0%o", -shared_repository);
+               else if (shared_repository == PERM_GROUP)
                        sprintf(buf, "%d", OLD_PERM_GROUP);
                else if (shared_repository == PERM_EVERYBODY)
                        sprintf(buf, "%d", OLD_PERM_EVERYBODY);
                else
-                       sprintf(buf, "0%o", shared_repository);
+                       die("oops");
                git_config_set("core.sharedrepository", buf);
                git_config_set("receive.denyNonFastforwards", "true");
        }
@@ -398,6 +403,9 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
                        usage(init_db_usage);
        }
 
+       if (init_shared_repository != -1)
+               shared_repository = init_shared_repository;
+
        /*
         * GIT_WORK_TREE makes sense only in conjunction with GIT_DIR
         * without --bare.  Catch the error early.

diff --git a/path.c b/path.c
index e332b504a6b8c6b229da2526c4f810b8d0fb9889..42898e0fb129d2d2219ab24a043510775beb8b5c 100644 (file)
--- a/path.c
+++ b/path.c
@@ -314,33 +314,39 @@ char *enter_repo(char *path, int strict)
 int adjust_shared_perm(const char *path)
 {
        struct stat st;
-       int mode;
+       int mode, tweak, shared;
 
        if (!shared_repository)
                return 0;
        if (lstat(path, &st) < 0)
                return -1;
        mode = st.st_mode;
-
-       if (shared_repository) {
-               int tweak = shared_repository;
-               if (!(mode & S_IWUSR))
-                       tweak &= ~0222;
+       if (shared_repository < 0)
+               shared = -shared_repository;
+       else
+               shared = shared_repository;
+       tweak = shared;
+
+       if (!(mode & S_IWUSR))
+               tweak &= ~0222;
+       if (mode & S_IXUSR)
+               /* Copy read bits to execute bits */
+               tweak |= (tweak & 0444) >> 2;
+       if (shared_repository < 0)
+               mode = (mode & ~0777) | tweak;
+       else
                mode |= tweak;
-       } else {
-               /* Preserve old PERM_UMASK behaviour */
-               if (mode & S_IWUSR)
-                       mode |= S_IWGRP;
-       }
 
        if (S_ISDIR(mode)) {
-               mode |= FORCE_DIR_SET_GID;
-
                /* Copy read bits to execute bits */
-               mode |= (shared_repository & 0444) >> 2;
+               mode |= (shared & 0444) >> 2;
+               mode |= FORCE_DIR_SET_GID;
        }
 
-       if ((mode & st.st_mode) != mode && chmod(path, mode) < 0)
+       if (((shared_repository < 0
+             ? (st.st_mode & (FORCE_DIR_SET_GID | 0777))
+             : (st.st_mode & mode)) != mode) &&
+           chmod(path, mode) < 0)
                return -2;
        return 0;
 }

diff --git a/setup.c b/setup.c
index 6c2deda18492acb5a8597563d6843f9d0dd232c0..ebd60de9ce5b52f348819a6a390c15b8dc08d2ff 100644 (file)
--- a/setup.c
+++ b/setup.c
@@ -434,7 +434,7 @@ int git_config_perm(const char *var, const char *value)
 
        /*
         * Treat values 0, 1 and 2 as compatibility cases, otherwise it is
-        * a chmod value.
+        * a chmod value to restrict to.
         */
        switch (i) {
        case PERM_UMASK:               /* 0 */
@@ -456,7 +456,7 @@ int git_config_perm(const char *var, const char *value)
         * Mask filemode value. Others can not get write permission.
         * x flags for directories are handled separately.
         */
-       return i & 0666;
+       return -(i & 0666);
 }
 
 int check_repository_format_version(const char *var, const char *value, void *cb)

diff --git a/sha1_file.c b/sha1_file.c
index a07aa4e5c491d16f65bc109673e806e24efb2274..45987bdea8aadba897338e106b208b7c09558065 100644 (file)
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2243,11 +2243,15 @@ static void write_sha1_file_prepare(const void *buf, unsigned long len,
 }
 
 /*
- * Move the just written object into its final resting place
+ * Move the just written object into its final resting place.
+ * NEEDSWORK: this should be renamed to finalize_temp_file() as
+ * "moving" is only a part of what it does, when no patch between
+ * master to pu changes the call sites of this function.
  */
 int move_temp_to_file(const char *tmpfile, const char *filename)
 {
        int ret = 0;
+
        if (link(tmpfile, filename))
                ret = errno;
 
@@ -2275,6 +2279,8 @@ int move_temp_to_file(const char *tmpfile, const char *filename)
                /* FIXME!!! Collision check here ? */
        }
 
+       if (adjust_shared_perm(filename))
+               return error("unable to set permission to '%s'", filename);
        return 0;
 }
 

diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index 653362ba221ee017512264c83a216b1ad1723bcd..d459854e71e1a5c8f850b7c5ef6d1ae1e1f98df9 100755 (executable)
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh
@@ -126,4 +126,41 @@ test_expect_success 'git reflog expire honors core.sharedRepository' '
        esac
 '
 
+test_expect_success 'forced modes' '
+       mkdir -p templates/hooks &&
+       echo update-server-info >templates/hooks/post-update &&
+       chmod +x templates/hooks/post-update &&
+       echo : >random-file &&
+       mkdir new &&
+       (
+               cd new &&
+               umask 002 &&
+               git init --shared=0660 --template=../templates &&
+               >frotz &&
+               git add frotz &&
+               git commit -a -m initial &&
+               git repack
+       ) &&
+       find new/.git -print |
+       xargs ls -ld >actual &&
+
+       # Everything must be unaccessible to others
+       test -z "$(sed -n -e "/^.......---/d" actual)" &&
+
+       # All directories must have 2770
+       test -z "$(sed -n -e "/^drwxrws---/d" -e "/^d/p" actual)" &&
+
+       # post-update hook must be 0770
+       test -z "$(sed -n -e "/post-update/{
+               /^-rwxrwx---/d
+               p
+       }" actual)" &&
+
+       # All files inside objects must be 0440
+       test -z "$(sed -n -e "/objects\//{
+               /^d/d
+               /^-r--r-----/d
+       }" actual)"
+'
+
 test_done