{"thread":{"id":"58685","subject":"FORCE_DIR_SET_GID denied inside nix's build sandbox","startedAt":"2022-10-25T16:35:53Z","lastAt":"2022-10-25T22:41:18Z","messageCount":3,"participants":["Julien Moutinho","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"465716","messageId":"20221025163024.uutqv7w24yi4eo5i@sourcephile.fr","threadId":"58685","inReplyTo":null,"subject":"FORCE_DIR_SET_GID denied inside nix's build sandbox","fromName":"Julien Moutinho","fromEmail":"julm+git@sourcephile.fr","sentAt":"2022-10-25T16:30:24Z","receivedAt":"2022-10-25T16:35:53Z","isPatch":false,"sender":{"key":"julm+git@sourcephile.fr","avatar":null},"body":"Hi,\n\nWhile running public-inbox's tests inside nix's build sandbox\nI've noticed that when core.sharedRepository is set to 0600\ngit still tries in adjust_shared_perm()\nto set the g+s bit on directories.\nhttps://github.com/git/git/blob/1fc3c0ad407008c2f71dd9ae1241d8b75f8ef886/path.c#L901-L905\n\nHowever, nix's build sandbox denies g+s\n(such chmod returning EPERM)\nbecause it could be leveraged to break the isolation\nof the build user.\nhttps://github.com/NixOS/nix/blob/b3d2a05c59266688aa904d5fb326394cbb7e9e90/src/libstore/sandbox-defaults.sb#L5-L7\n\nSo, on meta@public-inbox.org we were wondering whether\ngit should maybe strip S_ISGID and retry to chmod() if it hits EPERM?\nhttps://public-inbox.org/meta/20221025101756.M341966@dcvr/T/#m3f4e9eba9b903a263221ab82ce7ddcd44248d033\n\nAny thoughts on that matter?\nThanks!\n"},{"id":"465717","messageId":"xmqq7d0nonbw.fsf@gitster.g","threadId":"58685","inReplyTo":"20221025163024.uutqv7w24yi4eo5i@sourcephile.fr","subject":"Re: FORCE_DIR_SET_GID denied inside nix's build sandbox","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-25T16:54:59Z","receivedAt":"2022-10-25T16:55:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Julien Moutinho <julm+git@sourcephile.fr> writes:\n\n> While running public-inbox's tests inside nix's build sandbox\n> I've noticed that when core.sharedRepository is set to 0600\n> git still tries in adjust_shared_perm()\n> to set the g+s bit on directories.\n> https://github.com/git/git/blob/1fc3c0ad407008c2f71dd9ae1241d8b75f8ef886/path.c#L901-L905\n> ...\n> So, on meta@public-inbox.org we were wondering whether\n> git should maybe strip S_ISGID and retry to chmod() if it hits EPERM?\n\nBut that can leave the repository unusable when the files and\ndirectories has to be owned by the same group as its containing\ndirectory.\n\nIt does sound like we are bit too aggressive when setting the g+s\nbit.\n\nIs g+s only necessary if the directory is group writable?  Not\nnecessarily.  When I am the only writer (and owner) with memberships\nin multiple groups, a shared repository that I intend to give read\n(but not write) permission bit to members of a specific group must\nbe owned by that group, and setting g+s is how we ensure it.  So it\nis insufficient to give when new_mode says it is group writable.  We\nalso should do g+s if it is readable by the group.\n\nBut if you use 0600, then the group ownership should not matter, so\n\n    /* Copy read bits to execute bits */\n    new_mode |= (new_mode & 0444) >> 2;\n    if (new_mode & 060)\n\tnew_mode |= FORCE_DIR_SET_GID;\n\nmight be what you want?\n"},{"id":"465734","messageId":"20221025224058.7kc4x43lkkncmak3@sourcephile.fr","threadId":"58685","inReplyTo":"xmqq7d0nonbw.fsf@gitster.g","subject":"Re: FORCE_DIR_SET_GID denied inside nix's build sandbox","fromName":"Julien Moutinho","fromEmail":"julm+git@sourcephile.fr","sentAt":"2022-10-25T22:40:58Z","receivedAt":"2022-10-25T22:41:18Z","isPatch":false,"sender":{"key":"julm+git@sourcephile.fr","avatar":null},"body":"Le mar. 25 oct. 2022 09h54 -0700, Junio C Hamano a écrit :\n> But if you use 0600, then the group ownership should not matter, so\n> \n>     /* Copy read bits to execute bits */\n>     new_mode |= (new_mode & 0444) >> 2;\n>     if (new_mode & 060)\n> \tnew_mode |= FORCE_DIR_SET_GID;\n> \n> might be what you want?\nThat change makes more sense indeed,\nand does relax the chmod enough\nto solve our problem when running public-inbox tests\ninto nix's build sandbox.\n\nThank you!\n\nPS: please let me know if you need me to submit a proper patch,\notherwise I'm leaving this up to you if you don't mind.\n"}]}