{"thread":{"id":"13083","subject":"[PATCH] Make core.sharedRepository more generic (version 2)","startedAt":"2008-04-12T19:57:54Z","lastAt":"2008-04-15T01:22:49Z","messageCount":4,"participants":["Heikki Orsila","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"74229","messageId":"20080412195754.GA15091@zakalwe.fi","threadId":"13083","inReplyTo":null,"subject":"[PATCH] Make core.sharedRepository more generic (version 2)","fromName":"Heikki Orsila","fromEmail":"heikki.orsila@iki.fi","sentAt":"2008-04-12T19:57:54Z","receivedAt":"2008-04-12T19:57:54Z","isPatch":true,"sender":{"key":"heikki.orsila@iki.fi","avatar":null},"body":"git init --shared=0xxx, where '0xxx' is an octal number, will create\na repository with file modes set to '0xxx'. User's with a safe umask\nvalue (0077) can use this option to force file modes. For example,\n'0640' is a group-readable but not group-writable regardless of\nuser's umask value.\n\n\"git config core.sharedRepository 0xxx\" is also handled.\n\nVersion 2 handles the directory x flags better than version 1.\n\nSigned-off-by: Heikki Orsila <heikki.orsila@iki.fi>\n---\n Documentation/config.txt   |    7 +++++-\n Documentation/git-init.txt |    8 ++++++-\n builtin-init-db.c          |    4 +-\n cache.h                    |   16 ++++++++++++--\n path.c                     |   32 ++++++++++++++----------------\n setup.c                    |   45 ++++++++++++++++++++++++++++++++++++++++---\n 6 files changed, 84 insertions(+), 28 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex fe43b12..7a24f6e 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -261,7 +261,12 @@ core.sharedRepository::\n \tgroup-writable). When 'all' (or 'world' or 'everybody'), the\n \trepository will be readable by all users, additionally to being\n \tgroup-shareable. When 'umask' (or 'false'), git will use permissions\n-\treported by umask(2). See linkgit:git-init[1]. False by default.\n+\treported by umask(2). When '0xxx', where '0xxx' is an octal number,\n+\tfiles in the repository will have this mode value. '0xxx' will override\n+\tuser's umask value, and thus, users with a safe umask (0077) can use\n+\tthis option. Examples: '0660' is equivalent to 'group'. '0640' is a\n+\trepository that is group-readable but not group-writable.\n+\tSee linkgit:git-init[1]. False by default.\n \n core.warnAmbiguousRefs::\n \tIf true, git will warn you if the ref name you passed it is ambiguous\ndiff --git a/Documentation/git-init.txt b/Documentation/git-init.txt\nindex 62914da..b17ae84 100644\n--- a/Documentation/git-init.txt\n+++ b/Documentation/git-init.txt\n@@ -31,7 +31,7 @@ structure, some suggested \"exclude patterns\", and copies of non-executing\n \"hook\" files.  The suggested patterns and hook files are all modifiable and\n extensible.\n \n---shared[={false|true|umask|group|all|world|everybody}]::\n+--shared[={false|true|umask|group|all|world|everybody|0xxx}]::\n \n Specify that the git repository is to be shared amongst several users.  This\n allows users belonging to the same group to push into that\n@@ -52,6 +52,12 @@ is given:\n  - 'all' (or 'world' or 'everybody'): Same as 'group', but make the repository\n    readable by all users.\n \n+ - '0xxx': '0xxx' is an octal number and each file will have mode '0xxx'\n+   Any option except 'umask' can be set using this option. '0xxx' will\n+   override users umask(2) value, and thus, users with a safe umask (0077)\n+   can use this option. '0640' will create a repository which is group-readable\n+   but not writable. '0660' is equivalent to 'group'.\n+\n By default, the configuration flag receive.denyNonFastForwards is enabled\n in shared repositories, so that you cannot force a non fast-forwarding push\n into it.\ndiff --git a/builtin-init-db.c b/builtin-init-db.c\nindex 2854868..8c63295 100644\n--- a/builtin-init-db.c\n+++ b/builtin-init-db.c\n@@ -400,9 +400,9 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\tchar buf[10];\n \t\t/* We do not spell \"group\" and such, so that\n \t\t * the configuration can be read by older version\n-\t\t * of git.\n+\t\t * of git. Note, we use octal numbers.\n \t\t */\n-\t\tsprintf(buf, \"%d\", shared_repository);\n+\t\tsprintf(buf, \"0%o\", shared_repository);\n \t\tgit_config_set(\"core.sharedrepository\", buf);\n \t\tgit_config_set(\"receive.denyNonFastforwards\", \"true\");\n \t}\ndiff --git a/cache.h b/cache.h\nindex 2a1e7ec..f9c8d2b 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -474,10 +474,20 @@ static inline void hashclr(unsigned char *hash)\n \n int git_mkstemp(char *path, size_t n, const char *template);\n \n+/*\n+ * NOTE NOTE NOTE!!\n+ *\n+ * PERM_UMASK, OLD_PERM_GROUP and OLD_PERM_EVERYBODY enumerations must\n+ * not be changed. Old repositories have core.sharedrepository written in\n+ * numeric format, and therefore these values are preserved for compatibility\n+ * reasons.\n+ */\n enum sharedrepo {\n-\tPERM_UMASK = 0,\n-\tPERM_GROUP,\n-\tPERM_EVERYBODY\n+\tPERM_UMASK          = 0,\n+\tOLD_PERM_GROUP      = 1,\n+\tOLD_PERM_EVERYBODY  = 2,\n+\tPERM_GROUP          = 0660,\n+\tPERM_EVERYBODY      = 0664,\n };\n int git_config_perm(const char *var, const char *value);\n int adjust_shared_perm(const char *path);\ndiff --git a/path.c b/path.c\nindex f4ed979..cfa0529 100644\n--- a/path.c\n+++ b/path.c\n@@ -266,24 +266,22 @@ int adjust_shared_perm(const char *path)\n \tif (lstat(path, &st) < 0)\n \t\treturn -1;\n \tmode = st.st_mode;\n-\tif (mode & S_IRUSR)\n-\t\tmode |= (shared_repository == PERM_GROUP\n-\t\t\t ? S_IRGRP\n-\t\t\t : (shared_repository == PERM_EVERYBODY\n-\t\t\t    ? (S_IRGRP|S_IROTH)\n-\t\t\t    : 0));\n-\n-\tif (mode & S_IWUSR)\n-\t\tmode |= S_IWGRP;\n-\n-\tif (mode & S_IXUSR)\n-\t\tmode |= (shared_repository == PERM_GROUP\n-\t\t\t ? S_IXGRP\n-\t\t\t : (shared_repository == PERM_EVERYBODY\n-\t\t\t    ? (S_IXGRP|S_IXOTH)\n-\t\t\t    : 0));\n-\tif (S_ISDIR(mode))\n+\n+\tif (shared_repository) {\n+\t\tmode = (mode & ~0777) | shared_repository;\n+\t} else {\n+\t\t/* Preserve old PERM_UMASK behaviour */\n+\t\tif (mode & S_IWUSR)\n+\t\t\tmode |= S_IWGRP;\n+\t}\n+\n+\tif (S_ISDIR(mode)) {\n \t\tmode |= FORCE_DIR_SET_GID;\n+\n+\t\t/* Copy read bits to execute bits */\n+\t\tmode |= (shared_repository & 0444) >> 2;\n+\t}\n+\n \tif ((mode & st.st_mode) != mode && chmod(path, mode) < 0)\n \t\treturn -2;\n \treturn 0;\ndiff --git a/setup.c b/setup.c\nindex 3d2d958..0905509 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -430,6 +430,8 @@ int git_config_perm(const char *var, const char *value)\n {\n \tif (value) {\n \t\tint i;\n+\t\tchar *endptr;\n+\n \t\tif (!strcmp(value, \"umask\"))\n \t\t\treturn PERM_UMASK;\n \t\tif (!strcmp(value, \"group\"))\n@@ -438,11 +440,46 @@ int git_config_perm(const char *var, const char *value)\n \t\t    !strcmp(value, \"world\") ||\n \t\t    !strcmp(value, \"everybody\"))\n \t\t\treturn PERM_EVERYBODY;\n-\t\ti = atoi(value);\n-\t\tif (i > 1)\n-\t\t\treturn i;\n+\n+\t\t/* Parse octal numbers */\n+\t\ti = strtol(value, &endptr, 8);\n+\t\tif (*endptr != 0) {\n+\t\t\t/* Not an octal number. Maybe true/false? */\n+\t\t\tif (git_config_bool(var, value))\n+\t\t\t\treturn PERM_GROUP;\n+\t\t\telse\n+\t\t\t\treturn PERM_UMASK;\n+\t\t}\n+\n+\t\t/* Handle compatibility cases */\n+\t\tswitch (i) {\n+\t\tcase PERM_UMASK:               /* 0 */\n+\t\t\treturn PERM_UMASK;\n+\t\tcase OLD_PERM_GROUP:           /* 1 */\n+\t\t\treturn PERM_GROUP;\n+\t\tcase OLD_PERM_EVERYBODY:       /* 2 */\n+\t\t\treturn PERM_EVERYBODY;\n+\t\t}\n+\n+\t\t/* A filemode value was given: 0xxx */\n+\n+\t\tif ((i & 0600) != 0600)\n+\t\t\tdie(\"Problem with core.sharedRepository filemode value\"\n+\t\t\t    \" (0%.3o).\\nThe owner of files must always have \"\n+\t\t\t    \"read and write permissions.\", i);\n+\n+\t\tif (i & 0002)\n+\t\t\twarning(\"core.sharedRepository filemode (0%.3o) is a \"\n+\t\t\t\t\"security threat.\\nMasking off write \"\n+\t\t\t\t\"permission for others\\n\", i);\n+\n+\t\t/*\n+                 * Mask filemode value. Others can not get write permission.\n+\t\t * x flags for directories are handled separately.\n+                 */\n+\t\treturn i & 0664;\n \t}\n-\treturn git_config_bool(var, value);\n+\treturn PERM_GROUP;\n }\n \n int check_repository_format_version(const char *var, const char *value)\n-- \n1.5.4.4\n"},{"id":"74391","messageId":"7v3apo7zfg.fsf@gitster.siamese.dyndns.org","threadId":"13083","inReplyTo":"20080412195754.GA15091@zakalwe.fi","subject":"Re: [PATCH] Make core.sharedRepository more generic (version 2)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-15T00:08:03Z","receivedAt":"2008-04-15T00:08:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heikki Orsila <heikki.orsila@iki.fi> writes:\n\n> diff --git a/builtin-init-db.c b/builtin-init-db.c\n> index 2854868..8c63295 100644\n> --- a/builtin-init-db.c\n> +++ b/builtin-init-db.c\n> @@ -400,9 +400,9 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n>  \t\tchar buf[10];\n>  \t\t/* We do not spell \"group\" and such, so that\n>  \t\t * the configuration can be read by older version\n> -\t\t * of git.\n> +\t\t * of git. Note, we use octal numbers.\n>  \t\t */\n> -\t\tsprintf(buf, \"%d\", shared_repository);\n> +\t\tsprintf(buf, \"0%o\", shared_repository);\n\nUnconditionally doing this makes the resulting repository unusable by git\n1.5.5 and older, even when the user wanted to use the bog standard \"git\ninit --shared\".  You can limit the extent of damage if you continue\nwriting PERM_GROUP and PERM_EVERYBODY out as 1 and 2, and use the new\noctal notation only when the user used the settings allowed only with new\ngit.\n\n> @@ -438,11 +440,46 @@ int git_config_perm(const char *var, const char *value)\n>  \t\t    !strcmp(value, \"world\") ||\n>  \t\t    !strcmp(value, \"everybody\"))\n>  \t\t\treturn PERM_EVERYBODY;\n> +\n> +\t\t/* Parse octal numbers */\n> +\t\ti = strtol(value, &endptr, 8);\n> +\t\tif (*endptr != 0) {\n> +\t\t\t/* Not an octal number. Maybe true/false? */\n> +\t\t\tif (git_config_bool(var, value))\n> +\t\t\t\treturn PERM_GROUP;\n> +\t\t\telse\n> +\t\t\t\treturn PERM_UMASK;\n> +\t\t}\n> +\n> +\t\t/* Handle compatibility cases */\n> +\t\tswitch (i) {\n> +\t\tcase PERM_UMASK:               /* 0 */\n> +\t\t\treturn PERM_UMASK;\n> +\t\tcase OLD_PERM_GROUP:           /* 1 */\n> +\t\t\treturn PERM_GROUP;\n> +\t\tcase OLD_PERM_EVERYBODY:       /* 2 */\n> +\t\t\treturn PERM_EVERYBODY;\n> +\t\t}\n\nThis is valid only because forcing \"chmod 0\", \"chmod 1\", nor \"chmod 2\"\nwould not make any sense.  We might want to explain that in comment.\n\n> +\t\t/* A filemode value was given: 0xxx */\n> +\n> +\t\tif ((i & 0600) != 0600)\n> +\t\t\tdie(\"Problem with core.sharedRepository filemode value\"\n> +\t\t\t    \" (0%.3o).\\nThe owner of files must always have \"\n> +\t\t\t    \"read and write permissions.\", i);\n> +\n> +\t\tif (i & 0002)\n> +\t\t\twarning(\"core.sharedRepository filemode (0%.3o) is a \"\n> +\t\t\t\t\"security threat.\\nMasking off write \"\n> +\t\t\t\t\"permission for others\\n\", i);\n\nI am not sure about this.\n\nIf the user explicitly asked for world-writable, I think we should allow\nit.  \"is a\" is too strong a statement to make without knowing how the\naccess to the repository is arranged.  In a setting where there is nothing\nbut a restricted access over ssh, and \"update-paranoid\" hook in place, it\nmay not be a threat at all, and you are forbidding hosting services to use\nsuch an access control model.\n\n> +\t\t/*\n> +                 * Mask filemode value. Others can not get write permission.\n> +\t\t * x flags for directories are handled separately.\n> +                 */\nWhitespace breakages.\n\n> +\t\treturn i & 0664;\n\n>  \t}\n> -\treturn git_config_bool(var, value);\n> +\treturn PERM_GROUP;\n\nAt this point we know value is NULL so always returning PERM_GROUP\nprobably makes sense.  But if you did\n\n\tif (!value)\n\t        return PERM_GROUP\n\nupfront, you can lose one level of indentation from the major parts\nof this function (this is 70% style and 30% readability comment).\n"},{"id":"74396","messageId":"20080415004310.GI31039@zakalwe.fi","threadId":"13083","inReplyTo":"7v3apo7zfg.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Make core.sharedRepository more generic (version 2)","fromName":"Heikki Orsila","fromEmail":"shdl@zakalwe.fi","sentAt":"2008-04-15T00:43:10Z","receivedAt":"2008-04-15T00:43:10Z","isPatch":true,"sender":{"key":"shdl@zakalwe.fi","avatar":null},"body":"On Mon, Apr 14, 2008 at 05:08:03PM -0700, Junio C Hamano wrote:\n> > +\t\t * of git. Note, we use octal numbers.\n> >  \t\t */\n> > -\t\tsprintf(buf, \"%d\", shared_repository);\n> > +\t\tsprintf(buf, \"0%o\", shared_repository);\n> \n> Unconditionally doing this makes the resulting repository unusable by git\n> 1.5.5 and older, even when the user wanted to use the bog standard \"git\n> init --shared\".  You can limit the extent of damage if you continue\n> writing PERM_GROUP and PERM_EVERYBODY out as 1 and 2, and use the new\n> octal notation only when the user used the settings allowed only with new\n> git.\n\nI submitted a new patch that should address all the points.\n\nHeikki\n"},{"id":"74401","messageId":"20080415012249.GJ31039@zakalwe.fi","threadId":"13083","inReplyTo":"20080415004310.GI31039@zakalwe.fi","subject":"Re: [PATCH] Make core.sharedRepository more generic (version 2)","fromName":"Heikki Orsila","fromEmail":"shdl@zakalwe.fi","sentAt":"2008-04-15T01:22:49Z","receivedAt":"2008-04-15T01:22:49Z","isPatch":true,"sender":{"key":"shdl@zakalwe.fi","avatar":null},"body":"On Tue, Apr 15, 2008 at 03:43:10AM +0300, Heikki Orsila wrote:\n> On Mon, Apr 14, 2008 at 05:08:03PM -0700, Junio C Hamano wrote:\n> > > +\t\t * of git. Note, we use octal numbers.\n> > >  \t\t */\n> > > -\t\tsprintf(buf, \"%d\", shared_repository);\n> > > +\t\tsprintf(buf, \"0%o\", shared_repository);\n> > \n> > Unconditionally doing this makes the resulting repository unusable by git\n> > 1.5.5 and older, even when the user wanted to use the bog standard \"git\n> > init --shared\".  You can limit the extent of damage if you continue\n> > writing PERM_GROUP and PERM_EVERYBODY out as 1 and 2, and use the new\n> > octal notation only when the user used the settings allowed only with new\n> > git.\n> \n> I submitted a new patch that should address all the points.\n\nSubmitted another version that really fixes the o+w case. Forgot \nthe 0664 mask there. Now it's 0666.\n\n-- \nHeikki Orsila\nheikki.orsila@iki.fi\nhttp://www.iki.fi/shd\n"}]}