{"thread":{"id":"62765","subject":"Would it be possible to add an option to disable validating submodule paths?","startedAt":"2025-01-07T22:01:56Z","lastAt":"2025-07-28T23:17:48Z","messageCount":7,"participants":["Vadim Zeitlin","brian m. carlson","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"510127","messageId":"Mahogany-0.68.0-2854301-20250107-230058.01@dark.tt-solutions.com","threadId":"62765","inReplyTo":null,"subject":"Would it be possible to add an option to disable validating submodule paths?","fromName":"Vadim Zeitlin","fromEmail":"vz-git@zeitlins.org","sentAt":"2025-01-07T22:00:58Z","receivedAt":"2025-01-07T22:01:56Z","isPatch":false,"sender":{"key":"vz-git@zeitlins.org","avatar":null},"body":" Hello,\n\n In e8d0608944 (submodule: require the submodule path to contain\ndirectories only, 2024-03-26) a check that submodule paths don't contain\nsymlinks was added to Git. I understand that this check is generally useful\nand helpful, but I'd really like to have some way of disabling it for some\ntrusted repositories and _allow_ some of their submodules to be symlinks\n(see below for the rationale).\n\n Unfortunately, there doesn't seem to be any way to do it currently and I'd\nlike to ask if I might, perhaps, be missing such a way or, if I don't,\nwhether a patch adding an option to do it could be accepted?\n\n As to why I'd like to disable it, it's the usual story: this change broke\nmy workflow (https://xkcd.com/1172/). I have a relatively big Git\nrepository that I use as a submodule in many of the projects I'm working on\nand I used to just symlink the corresponding submodule directory to one,\nprimary copy of this repository present on my system, instead of really\ninitializing the submodule. This saved me many gigabytes of disk space and\nis also much faster than reinitializing the submodule every time I start a\nnew project or, more frequently, create a new worktree for the existing\none. And this worked just fine for many years but doesn't work any longer\nas any operation on the repository, even just \"git status\", now gives\n\nerror: expected submodule path 'submodule/path' not to be a symbolic link\n\nas soon as a symlink is detected. Under Linux I can use mount binds\ninstead, but this is much less convenient for many reasons and I'd really\nprefer to just keep using symlinks. Would it be possible to (optionally)\nallow using them again?\n\n Thanks in advance,\nVZ\n"},{"id":"510129","messageId":"Z320RGb0tqNyvvxt@tapette.crustytoothpaste.net","threadId":"62765","inReplyTo":"Mahogany-0.68.0-2854301-20250107-230058.01@dark.tt-solutions.com","subject":"Re: Would it be possible to add an option to disable validating submodule paths?","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-01-07T23:09:56Z","receivedAt":"2025-01-07T23:10:04Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-01-07 at 22:00:58, Vadim Zeitlin wrote:\n> In e8d0608944 (submodule: require the submodule path to contain\n> directories only, 2024-03-26) a check that submodule paths don't contain\n> symlinks was added to Git. I understand that this check is generally useful\n> and helpful, but I'd really like to have some way of disabling it for some\n> trusted repositories and _allow_ some of their submodules to be symlinks\n> (see below for the rationale).\n> \n> Unfortunately, there doesn't seem to be any way to do it currently and I'd\n> like to ask if I might, perhaps, be missing such a way or, if I don't,\n> whether a patch adding an option to do it could be accepted?\n> \n> As to why I'd like to disable it, it's the usual story: this change broke\n> my workflow (https://xkcd.com/1172/). I have a relatively big Git\n> repository that I use as a submodule in many of the projects I'm working on\n> and I used to just symlink the corresponding submodule directory to one,\n> primary copy of this repository present on my system, instead of really\n> initializing the submodule. This saved me many gigabytes of disk space and\n> is also much faster than reinitializing the submodule every time I start a\n> new project or, more frequently, create a new worktree for the existing\n> one. And this worked just fine for many years but doesn't work any longer\n> as any operation on the repository, even just \"git status\", now gives\n> \n> error: expected submodule path 'submodule/path' not to be a symbolic link\n> \n> as soon as a symlink is detected. Under Linux I can use mount binds\n> instead, but this is much less convenient for many reasons and I'd really\n> prefer to just keep using symlinks. Would it be possible to (optionally)\n> allow using them again?\n\nSince this is a defense-in-depth change and it seems to have broken a\nreasonable workflow, I think adding a config option for this would be\nreasonable. We've recently had some discussions on trying to limit the\ndefense-in-depth measures we implement on the security list in the\ninterests of allowing better discussion and feedback on the main list\nand avoiding regressions in people's workflows, and I think your email\nlends support to that approach.\n\nI'm not presently planning to add such an option, but it shouldn't be\ntoo hard to add a global variable for that (or maybe something under\nstruct repository) that's updated when parsing config, and then check it\nin `validate_submodule_path`. We'd need docs for that option as well,\nbut that would probably be it if someone wanted to do so.\n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n"},{"id":"510133","messageId":"xmqq5xmqqk9j.fsf@gitster.g","threadId":"62765","inReplyTo":"Z320RGb0tqNyvvxt@tapette.crustytoothpaste.net","subject":"Re: Would it be possible to add an option to disable validating submodule paths?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-07T23:25:28Z","receivedAt":"2025-01-07T23:25:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> Since this is a defense-in-depth change and it seems to have broken a\n> reasonable workflow, I think adding a config option for this would be\n> reasonable. We've recently had some discussions on trying to limit the\n> defense-in-depth measures we implement on the security list in the\n> interests of allowing better discussion and feedback on the main list\n> and avoiding regressions in people's workflows, and I think your email\n> lends support to that approach.\n\nThanks; I was writing my own response and said pretty much the same\nthing as above, before I saw this message.\n\n> I'm not presently planning to add such an option, but it shouldn't be\n> too hard to add a global variable for that (or maybe something under\n> struct repository) that's updated when parsing config, and then check it\n> in `validate_submodule_path`. We'd need docs for that option as well,\n> but that would probably be it if someone wanted to do so.\n\nSounds reasonable, but I wonder how this would interact with\nbootstrapping. Should it be configured in ~/.gitconfig, possibly\nwith [includeIf] to specify the directory you'd store a bunch of\nrepositories you clone from outside, or something? I guess \"git\nclone\" without \"--recurse-submodules\" is simple enough to be used\nfor bootstrapping, and then the configuration can be set at the\ntop-level superproject after cloning but before \"submodule init\".\n\n\n"},{"id":"510135","messageId":"Mahogany-0.68.0-2854301-20250108-005035.01@dark.tt-solutions.com","threadId":"62765","inReplyTo":"xmqq5xmqqk9j.fsf@gitster.g","subject":"Re[2]: Would it be possible to add an option to disable validating submodule paths?","fromName":"Vadim Zeitlin","fromEmail":"vadim@zeitlins.org","sentAt":"2025-01-07T23:50:35Z","receivedAt":"2025-01-07T23:50:37Z","isPatch":false,"sender":{"key":"vadim@zeitlins.org","avatar":null},"body":"On Tue, 07 Jan 2025 15:25:28 -0800 Junio C Hamano wrote:\n\nJCH> \"brian m. carlson\" writes:\nJCH> \nJCH> > Since this is a defense-in-depth change and it seems to have broken a\nJCH> > reasonable workflow, I think adding a config option for this would be\nJCH> > reasonable. We've recently had some discussions on trying to limit the\nJCH> > defense-in-depth measures we implement on the security list in the\nJCH> > interests of allowing better discussion and feedback on the main list\nJCH> > and avoiding regressions in people's workflows, and I think your email\nJCH> > lends support to that approach.\nJCH> \nJCH> Thanks; I was writing my own response and said pretty much the same\nJCH> thing as above, before I saw this message.\n\n Thanks to both of you for your replies, I'll try to come up with a patch\nrelatively soon.\n\nJCH> > I'm not presently planning to add such an option, but it shouldn't be\nJCH> > too hard to add a global variable for that (or maybe something under\nJCH> > struct repository) that's updated when parsing config, and then check it\nJCH> > in `validate_submodule_path`. We'd need docs for that option as well,\nJCH> > but that would probably be it if someone wanted to do so.\nJCH> \nJCH> Sounds reasonable, but I wonder how this would interact with\nJCH> bootstrapping. Should it be configured in ~/.gitconfig, possibly\nJCH> with [includeIf] to specify the directory you'd store a bunch of\nJCH> repositories you clone from outside, or something? I guess \"git\nJCH> clone\" without \"--recurse-submodules\" is simple enough to be used\nJCH> for bootstrapping, and then the configuration can be set at the\nJCH> top-level superproject after cloning but before \"submodule init\".\n\n I might be missing something here, but if the question is about whether we\nneed to have any special support for this in git-clone itself, then I don't\nthink so, it's a rather special use case and running git-clone without\n--recurse-submodules and initializing (some) submodules later while\nsymlinking some other ones is only a minor inconvenience, if that.\n\n OTOH I've realized that I have no idea how the new option should be\ncalled. I had initially thought about \"safe.submodules = bool\", but I'm not\nsure if this is really consistent with the existing safe.xxx options which\nlook and behave a bit differently. Should it be something like\nsubmodule.validate instead, perhaps? Please let me know if anybody has any\nbetter ideas.\n\n Thanks,\nVZ\n"},{"id":"510168","messageId":"xmqqjzb5nvhd.fsf@gitster.g","threadId":"62765","inReplyTo":"Mahogany-0.68.0-2854301-20250108-005035.01@dark.tt-solutions.com","subject":"Re: Would it be possible to add an option to disable validating submodule paths?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-08T16:03:42Z","receivedAt":"2025-01-08T16:03:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vadim Zeitlin writes:\n\n> JCH> Sounds reasonable, but I wonder how this would interact with\n> JCH> bootstrapping. Should it be configured in ~/.gitconfig, possibly\n> JCH> with [includeIf] to specify the directory you'd store a bunch of\n> JCH> repositories you clone from outside, or something? I guess \"git\n> JCH> clone\" without \"--recurse-submodules\" is simple enough to be used\n> JCH> for bootstrapping, and then the configuration can be set at the\n> JCH> top-level superproject after cloning but before \"submodule init\".\n>\n> I might be missing something here, but if the question is about whether we\n> need to have any special support for this in git-clone itself, then I don't\n> think so, it's a rather special use case and running git-clone without\n> --recurse-submodules and initializing (some) submodules later while\n> symlinking some other ones is only a minor inconvenience, if that.\n\nIf you say so then I'd stop worrying about it ;-) I am not a heavy\nsubmodule user myself.\n\nThe worry came primarily from the fact that this was reported as a\n\"we have been using submodules happily in this particular manner but\nwith a new version of Git it stopped working\" regression. Your\nset-up was created with an older version of Git that did not have\nthe problematic \"defence in depth\". If you or somebody else wanted\nto recreate the same set-up from scratch, would \"git clone\" that is\nunmodified, other than conditionally disables the check introduced\nby the commit e8d06089 (submodule: require the submodule path to\ncontain directories only, 2024-03-26), let you do so? Or would it\nalso need to honor the new configuration that conditionally disables\nthe check, and if so, how would we make sure it is read during \"git\nclone\" (which has kind of special chicken-and-egg problem with\nrespect to configuration settings).\n\n> ... Should it be something like\n> submodule.validate instead, perhaps? Please let me know if anybody has any\n> better ideas.\n\nIs \"it MUST NOT BE a symbolic link\" the only thing the validation\ndoes? Would there be extra check on top of what is currently there\nthat may turn out to be useful? If the answers are no and/or yes,\n\"submodule.validate=no\" sounds like a reasonable choice, but I am\nnot good at naming, so we may want to hear ideas from others.\n\nThanks.\n\n"},{"id":"510200","messageId":"Mahogany-0.68.0-2854301-20250108-203000.01@dark.tt-solutions.com","threadId":"62765","inReplyTo":"xmqqjzb5nvhd.fsf@gitster.g","subject":"Re[2]: Would it be possible to add an option to disable validating submodule paths?","fromName":"Vadim Zeitlin","fromEmail":"vadim@zeitlins.org","sentAt":"2025-01-08T19:30:00Z","receivedAt":"2025-01-08T19:30:05Z","isPatch":false,"sender":{"key":"vadim@zeitlins.org","avatar":null},"body":"On Wed, 08 Jan 2025 08:03:42 -0800 Junio C Hamano wrote:\n\nJCH> Vadim Zeitlin writes:\nJCH> \nJCH> > JCH> Sounds reasonable, but I wonder how this would interact with\nJCH> > JCH> bootstrapping. Should it be configured in ~/.gitconfig, possibly\nJCH> > JCH> with [includeIf] to specify the directory you'd store a bunch of\nJCH> > JCH> repositories you clone from outside, or something? I guess \"git\nJCH> > JCH> clone\" without \"--recurse-submodules\" is simple enough to be used\nJCH> > JCH> for bootstrapping, and then the configuration can be set at the\nJCH> > JCH> top-level superproject after cloning but before \"submodule init\".\nJCH> >\nJCH> > I might be missing something here, but if the question is about whether we\nJCH> > need to have any special support for this in git-clone itself, then I don't\nJCH> > think so, it's a rather special use case and running git-clone without\nJCH> > --recurse-submodules and initializing (some) submodules later while\nJCH> > symlinking some other ones is only a minor inconvenience, if that.\nJCH> \nJCH> If you say so then I'd stop worrying about it ;-) I am not a heavy\nJCH> submodule user myself.\n\n Well, let's just say that I'm not worried about it.\n\nJCH> The worry came primarily from the fact that this was reported as a\nJCH> \"we have been using submodules happily in this particular manner but\nJCH> with a new version of Git it stopped working\" regression. Your\nJCH> set-up was created with an older version of Git that did not have\nJCH> the problematic \"defence in depth\". If you or somebody else wanted\nJCH> to recreate the same set-up from scratch, would \"git clone\" that is\nJCH> unmodified, other than conditionally disables the check introduced\nJCH> by the commit e8d06089 (submodule: require the submodule path to\nJCH> contain directories only, 2024-03-26), let you do so? Or would it\nJCH> also need to honor the new configuration that conditionally disables\nJCH> the check, and if so, how would we make sure it is read during \"git\nJCH> clone\" (which has kind of special chicken-and-egg problem with\nJCH> respect to configuration settings).\n\n As I was trying to say above, I think it's unreasonable to expect \"git\nclone --recurse-submodules\" to do something extra smart when there is a\nsimple (both to use and to discover) alternative of just running \"git\nclone\" without any extra options, and then initialize the submodules that\nyou don't want to symlink manually and symlink the remaining ones.\n\nJCH> > ... Should it be something like submodule.validate instead, perhaps?\nJCH> > Please let me know if anybody has any better ideas.\nJCH> \nJCH> Is \"it MUST NOT BE a symbolic link\" the only thing the validation\nJCH> does?\n\n Currently, yes.\n\nJCH> Would there be extra check on top of what is currently there\nJCH> that may turn out to be useful?\n\n It's conceivable that there might be other checks in the future, e.g.\nmaybe the ownership of the directories or even their permissions might be\nchecked? Just to be clear, this is pure speculation on my part, i.e. I\ndon't see any real need to do it, but I can't be certain that there are no\nscenarios in which this might be useful.\n\nJCH> If the answers are no and/or yes, \"submodule.validate=no\" sounds like\nJCH> a reasonable choice, but I am not good at naming, so we may want to\nJCH> hear ideas from others.\n\n I'll wait for some time to hear if anybody else has any better suggestions.\n\n Thanks in advance!\nVZ\n"},{"id":"522904","messageId":"Mahogany-0.68.0-3185915-20250729-011252.01@dark.tt-solutions.com","threadId":"62765","inReplyTo":"Mahogany-0.68.0-2854301-20250108-203000.01@dark.tt-solutions.com","subject":"[PATCH] submodule: Add a config option to skip path validation","fromName":"Vadim Zeitlin","fromEmail":"vadim@zeitlins.org","sentAt":"2025-07-28T23:12:52Z","receivedAt":"2025-07-28T23:17:48Z","isPatch":true,"sender":{"key":"vadim@zeitlins.org","avatar":null},"body":"On Wed, 8 Jan 2025 20:30:00 +0100 I wrote:\n\nMe> On Wed, 08 Jan 2025 08:03:42 -0800 Junio C Hamano wrote:\n[...]\nMe> JCH> If the answers are no and/or yes, \"submodule.validate=no\" sounds like\nMe> JCH> a reasonable choice, but I am not good at naming, so we may want to\nMe> JCH> hear ideas from others.\nMe> \nMe> I'll wait for some time to hear if anybody else has any better suggestions.\n\n Hello again,\n\n I guess more than half a year can be considered \"some time\" but, anyhow,\nbetter late than never, so here is a minimal patch implementing what I\nsuggested. I'm not sure if using submodule_from_path() with the global\nthe_repository is the right thing to do, it looks like there is an attempt\nto move away from it, but I couldn't find another reasonable way to get the\nconfig option -- using open_submodule() doesn't seem right and I don't know\nwhat else to do. If you can propose a better way to do this, please do.\n\n I also couldn't find a simple way to add a test for it, I hoped to find an\nexisting test checking that using symlinks for submodules failed and amend\nit, but either no such test exists or I've failed to find it. Again, if I\nmissed it, please point me to it. But for now I've just tested this\nmanually, with and without the option in gitconfig file.\n\n Please let me know if you have any other comments, thanks in advance!\nVZ\n\n--------------------------------- >8 --------------------------------------\nFrom 071e8f1b47dfa22cbe99052810ab5123489731b0 Mon Sep 17 00:00:00 2001\nFrom: Vadim Zeitlin \nDate: Tue, 29 Jul 2025 01:02:10 +0200\nSubject: [PATCH] submodule: Add a config option to skip path validation\n\nThe changes of e8d0608944 (submodule: require the submodule path to\ncontain directories only, 2024-03-26) as part of \"defense in depth\"\nstrategy made it impossible to intentionally use symlinks instead of\nactual submodule directories, which can be desirable when the same big\nsubmodule is used by multiple projects.\n\nReintroduce the possibility to do it by adding a new\nsubmodule..validate option which defaults to 1, but can be\nexplicitly set to 0 to disable the submodule path validation.\n\nSigned-off-by: Vadim Zeitlin \n---\n Documentation/config/submodule.adoc | 8 ++++++++\n submodule.c | 19 ++++++++++++++++++-\n 2 files changed, 26 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/submodule.adoc b/Documentation/config/submodule.adoc\nindex 0672d99117..92243458c2 100644\n--- a/Documentation/config/submodule.adoc\n+++ b/Documentation/config/submodule.adoc\n@@ -106,3 +106,11 @@ submodule.alternateErrorStrategy::\n \t`ignore`, `info`, `die`. Default is `die`. Note that if set to `ignore`\n \tor `info`, and if there is an error with the computed alternate, the\n \tclone proceeds as if no alternate was specified.\n+\n+submodule.validate::\n+ A boolean value which can be set to false to disable validation of\n+ submodule paths and notably checking that they don't contain any symlink\n+ components. This can be useful when working with a trusted repository and\n+ intentionally using symlinks to avoid checking out another copy of the same\n+ submodule which already exists elsewhere on the same machine.\n+ Defaults to true.\ndiff --git a/submodule.c b/submodule.c\nindex f8373a9ea7..01b852c673 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2299,11 +2299,28 @@ int validate_submodule_git_dir(char *git_dir, const char *submodule_name)\n \n int validate_submodule_path(const char *path)\n {\n-\tchar *p = xstrdup(path);\n+\tchar *key = NULL;\n+\tint validate = 1;\n+\n+\tchar *p = NULL;\n \tstruct stat st;\n \tint i, ret = 0;\n \tchar sep;\n \n+\tconst struct submodule *submodule;\n+\n+\tsubmodule = submodule_from_path(the_repository, null_oid(the_hash_algo), path);\n+\tif (!submodule)\n+\t\tBUG(\"could not get submodule information for '%s'\", path);\n+\n+\t/* Check if submodule..validate is set to false to skip the check. */\n+\tkey = xstrfmt(\"submodule.%s.validate\", submodule->name);\n+\trepo_config_get_bool(the_repository, key, &validate);\n+\tfree(key);\n+\tif (!validate)\n+\t\treturn 0;\n+\n+\tp = xstrdup(path);\n \tfor (i = 0; !ret && p[i]; i++) {\n \t\tif (!is_dir_sep(p[i]))\n \t\t\tcontinue;\n-- \n2.47.2\n\n"}]}