# [PATCH] Make core.sharedRepository more generic 6 messages from 2008-04-12 to 2008-04-12. Participants: Heikki Orsila, Samuel Tardieu. Thread: https://gitlist.dev/t/13081 ## Heikki Orsila, 2008-04-12 18:51 Subject: [PATCH] Make core.sharedRepository more generic Message-ID: <20080412185105.GA14331@zakalwe.fi> URL: https://gitlist.dev/e/20080412185105.GA14331%40zakalwe.fi ``` git init --shared=0xxx, where '0xxx' is an octal number, will create a repository with file modes set to '0xxx'. User's with a safe umask value (0077) can use this option to force file modes. For example, '0640' is a group-readable but not group-writable regardless of user's umask value. "git config core.sharedRepository 0xxx" is also handled. --- Documentation/config.txt | 7 +++++- Documentation/git-init.txt | 8 ++++++- builtin-init-db.c | 4 +- cache.h | 16 ++++++++++++-- path.c | 38 ++++++++++++++++++++---------------- setup.c | 45 ++++++++++++++++++++++++++++++++++++++++--- 6 files changed, 90 insertions(+), 28 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index fe43b12..7a24f6e 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -261,7 +261,12 @@ core.sharedRepository:: group-writable). When 'all' (or 'world' or 'everybody'), the repository will be readable by all users, additionally to being group-shareable. When 'umask' (or 'false'), git will use permissions - reported by umask(2). See linkgit:git-init[1]. False by default. + reported by umask(2). When '0xxx', where '0xxx' is an octal number, + files in the repository will have this mode value. '0xxx' will override + user's umask value, and thus, users with a safe umask (0077) can use + this option. Examples: '0660' is equivalent to 'group'. '0640' is a + repository that is group-readable but not group-writable. + See linkgit:git-init[1]. False by default. core.warnAmbiguousRefs:: If true, git will warn you if the ref name you passed it is ambiguous diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt index 62914da..b17ae84 100644 --- a/Documentation/git-init.txt +++ b/Documentation/git-init.txt @@ -31,7 +31,7 @@ structure, some suggested "exclude patterns", and copies of non-executing "hook" files. The suggested patterns and hook files are all modifiable and extensible. ---shared[={false|true|umask|group|all|world|everybody}]:: +--shared[={false|true|umask|group|all|world|everybody|0xxx}]:: Specify that the git repository is to be shared amongst several users. This allows users belonging to the same group to push into that @@ -52,6 +52,12 @@ is given: - 'all' (or 'world' or 'everybody'): Same as 'group', but make the repository readable by all users. + - '0xxx': '0xxx' is an octal number and each file will have mode '0xxx' + Any option except 'umask' can be set using this option. '0xxx' will + override users umask(2) value, and thus, users with a safe umask (0077) + can use this option. '0640' will create a repository which is group-readable + but not writable. '0660' is equivalent to 'group'. + By default, the configuration flag receive.denyNonFastForwards is enabled in shared repositories, so that you cannot force a non fast-forwarding push into it. diff --git a/builtin-init-db.c b/builtin-init-db.c index 2854868..8c63295 100644 --- a/builtin-init-db.c +++ b/builtin-init-db.c @@ -400,9 +400,9 @@ int cmd_init_db(int argc, const char **argv, const char *prefix) char buf[10]; /* We do not spell "group" and such, so that * the configuration can be read by older version - * of git. + * of git. Note, we use octal numbers. */ - sprintf(buf, "%d", shared_repository); + sprintf(buf, "0%o", shared_repository); git_config_set("core.sharedrepository", buf); git_config_set("receive.denyNonFastforwards", "true"); } diff --git a/cache.h b/cache.h index 2a1e7ec..f9c8d2b 100644 --- a/cache.h +++ b/cache.h @@ -474,10 +474,20 @@ static inline void hashclr(unsigned char *hash) int git_mkstemp(char *path, size_t n, const char *template); +/* + * NOTE NOTE NOTE!! + * + * PERM_UMASK, OLD_PERM_GROUP and OLD_PERM_EVERYBODY enumerations must + * not be changed. Old repositories have core.sharedrepository written in + * numeric format, and therefore these values are preserved for compatibility + * reasons. + */ enum sharedrepo { - PERM_UMASK = 0, - PERM_GROUP, - PERM_EVERYBODY + PERM_UMASK = 0, + OLD_PERM_GROUP = 1, + OLD_PERM_EVERYBODY = 2, + PERM_GROUP = 0660, + PERM_EVERYBODY = 0664, }; int git_config_perm(const char *var, const char *value); int adjust_shared_perm(const char *path); diff --git a/path.c b/path.c index f4ed979..79bfe82 100644 --- a/path.c +++ b/path.c @@ -266,24 +266,28 @@ int adjust_shared_perm(const char *path) if (lstat(path, &st) < 0) return -1; mode = st.st_mode; - if (mode & S_IRUSR) - mode |= (shared_repository == PERM_GROUP - ? S_IRGRP - : (shared_repository == PERM_EVERYBODY - ? (S_IRGRP|S_IROTH) - : 0)); - - if (mode & S_IWUSR) - mode |= S_IWGRP; - - if (mode & S_IXUSR) - mode |= (shared_repository == PERM_GROUP - ? S_IXGRP - : (shared_repository == PERM_EVERYBODY - ? (S_IXGRP|S_IXOTH) - : 0)); - if (S_ISDIR(mode)) + + if (shared_repository) { + mode = (mode & ~0777) | shared_repository; + } else { + /* Preserve old PERM_UMASK behaviour */ + if (mode & S_IWUSR) + mode |= S_IWGRP; + } + + if (S_ISDIR(mode)) { mode |= FORCE_DIR_SET_GID; + + /* + * The x flag for directories is determined from rw flags of + * user, group and others. Having a directory with +rw but + * -x does not make sense for Git repositories. + */ + mode |= (shared_repository & 0600) ? S_IXUSR : 0; + mode |= (shared_repository & 0060) ? S_IXGRP : 0; + mode |= (shared_repository & 0006) ? S_IXOTH : 0; + } + if ((mode & st.st_mode) != mode && chmod(path, mode) < 0) return -2; return 0; diff --git a/setup.c b/setup.c index 3d2d958..0905509 100644 --- a/setup.c +++ b/setup.c @@ -430,6 +430,8 @@ int git_config_perm(const char *var, const char *value) { if (value) { int i; + char *endptr; + if (!strcmp(value, "umask")) return PERM_UMASK; if (!strcmp(value, "group")) @@ -438,11 +440,46 @@ int git_config_perm(const char *var, const char *value) !strcmp(value, "world") || !strcmp(value, "everybody")) return PERM_EVERYBODY; - i = atoi(value); - if (i > 1) - return i; + + /* Parse octal numbers */ + i = strtol(value, &endptr, 8); + if (*endptr != 0) { + /* Not an octal number. Maybe true/false? */ + if (git_config_bool(var, value)) + return PERM_GROUP; + else + return PERM_UMASK; + } + + /* Handle compatibility cases */ + switch (i) { + case PERM_UMASK: /* 0 */ + return PERM_UMASK; + case OLD_PERM_GROUP: /* 1 */ + return PERM_GROUP; + case OLD_PERM_EVERYBODY: /* 2 */ + return PERM_EVERYBODY; + } + + /* A filemode value was given: 0xxx */ + + if ((i & 0600) != 0600) + die("Problem with core.sharedRepository filemode value" + " (0%.3o).\nThe owner of files must always have " + "read and write permissions.", i); + + if (i & 0002) + warning("core.sharedRepository filemode (0%.3o) is a " + "security threat.\nMasking off write " + "permission for others\n", i); + + /* + * Mask filemode value. Others can not get write permission. + * x flags for directories are handled separately. + */ + return i & 0664; } - return git_config_bool(var, value); + return PERM_GROUP; } int check_repository_format_version(const char *var, const char *value) -- 1.5.4.4 ``` ## Heikki Orsila, 2008-04-12 18:56 Subject: Re: [PATCH] Make core.sharedRepository more generic Message-ID: <20080412185620.GF31039@zakalwe.fi> URL: https://gitlist.dev/e/20080412185620.GF31039%40zakalwe.fi In-Reply-To: <20080412185105.GA14331@zakalwe.fi> ``` On Sat, Apr 12, 2008 at 09:51:05PM +0300, Heikki Orsila wrote: > git init --shared=0xxx, where '0xxx' is an octal number, will create > a repository with file modes set to '0xxx'. User's with a safe umask > value (0077) can use this option to force file modes. For example, > '0640' is a group-readable but not group-writable regardless of > user's umask value. > > "git config core.sharedRepository 0xxx" is also handled. Oops, forgot to sign this one. Signed-off-by: Heikki Orsila Heikki ``` ## Samuel Tardieu, 2008-04-12 19:15 Subject: Re: [PATCH] Make core.sharedRepository more generic Message-ID: <2008-04-12-21-15-04+trackit+sam@rfc1149.net> URL: https://gitlist.dev/e/2008-04-12-21-15-04%2Btrackit%2Bsam%40rfc1149.net In-Reply-To: <20080412185105.GA14331@zakalwe.fi> ``` The use of named constants vs. literals seem inconsistent in your patch, compare | + mode = (mode & ~0777) | shared_repository; to | + mode |= (shared_repository & 0600) ? S_IXUSR : 0; | + mode |= (shared_repository & 0060) ? S_IXGRP : 0; | + mode |= (shared_repository & 0006) ? S_IXOTH : 0; I first thought that you were using literals with "shared_repository" and named constants with mode but the first line I quoted shows that this is not the case. Btw, aren't those last three lines better replaced by /* Copy read bits to execute bits */ mode |= (shared_repository & 0444) >> 2; I don't see where you deal with executable files. Also, wouldn't it be more consistent to use a negative value to --shared, that is a umask-compatible one, rather than a positive value which needs to be tweaked for directories and executable files? You would only have to "&" 0666 or 0777 with "~perms" to get the right permissions. --shared=0007 would be equivalent to PERM_GROUP, --shared=0027 to group-readable-but-not-writable, and --shared=0002 to PERM_EVERYBODY. Sam -- Samuel Tardieu -- sam@rfc1149.net -- http://www.rfc1149.net/ ``` ## Heikki Orsila, 2008-04-12 19:46 Subject: Re: [PATCH] Make core.sharedRepository more generic Message-ID: <20080412194634.GG31039@zakalwe.fi> URL: https://gitlist.dev/e/20080412194634.GG31039%40zakalwe.fi In-Reply-To: <2008-04-12-21-15-04+trackit+sam@rfc1149.net> ``` On Sat, Apr 12, 2008 at 09:15:03PM +0200, Samuel Tardieu wrote: > The use of named constants vs. literals seem inconsistent in your > patch, compare > > | + mode = (mode & ~0777) | shared_repository; > > to > > | + mode |= (shared_repository & 0600) ? S_IXUSR : 0; > | + mode |= (shared_repository & 0060) ? S_IXGRP : 0; > | + mode |= (shared_repository & 0006) ? S_IXOTH : 0; > > I first thought that you were using literals with "shared_repository" > and named constants with mode but the first line I quoted shows that > this is not the case. > > Btw, aren't those last three lines better replaced by > > /* Copy read bits to execute bits */ > mode |= (shared_repository & 0444) >> 2; Agreed on both previous points. Will submit a new patch. > I don't see where you deal with executable files. > Also, wouldn't it be more consistent to use a negative value to > --shared, that is a umask-compatible one, rather than a positive value > which needs to be tweaked for directories and executable files? You > would only have to "&" 0666 or 0777 with "~perms" to get the right > permissions. > > --shared=0007 would be equivalent to PERM_GROUP, --shared=0027 to > group-readable-but-not-writable, and --shared=0002 to PERM_EVERYBODY. I see the point, but I like to think it as a chmod value rather than a umask value. -- Heikki Orsila heikki.orsila@iki.fi http://www.iki.fi/shd ``` ## Heikki Orsila, 2008-04-12 19:50 Subject: Re: [PATCH] Make core.sharedRepository more generic Message-ID: <20080412195046.GH31039@zakalwe.fi> URL: https://gitlist.dev/e/20080412195046.GH31039%40zakalwe.fi In-Reply-To: <2008-04-12-21-15-04+trackit+sam@rfc1149.net> ``` On Sat, Apr 12, 2008 at 09:15:03PM +0200, Samuel Tardieu wrote: > I don't see where you deal with executable files. This code sets permissions for files written to the .git/ directory. Are there script files in the .git/ directory that we want to set executable flag for? -- Heikki Orsila heikki.orsila@iki.fi http://www.iki.fi/shd ``` ## Samuel Tardieu, 2008-04-12 22:20 Subject: Re: [PATCH] Make core.sharedRepository more generic Message-ID: <2008-04-13-00-20-34+trackit+sam@rfc1149.net> URL: https://gitlist.dev/e/2008-04-13-00-20-34%2Btrackit%2Bsam%40rfc1149.net In-Reply-To: <20080412195046.GH31039@zakalwe.fi> ``` >>>>> "Heikki" == Heikki Orsila writes: Heikki> Are there script files in the .git/ directory that we want to Heikki> set executable flag for? Probably not. Sam -- Samuel Tardieu -- sam@rfc1149.net -- http://www.rfc1149.net/ ```