Would it be possible to add an option to disable validating submodule paths?

Would it be possible to add an option to disable validating submodule paths?

[Vadim Zeitlin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1679 bytes --]

 Hello,

 In e8d0608944 (submodule: require the submodule path to contain
directories only, 2024-03-26) a check that submodule paths don't contain
symlinks was added to Git. I understand that this check is generally useful
and helpful, but I'd really like to have some way of disabling it for some
trusted repositories and _allow_ some of their submodules to be symlinks
(see below for the rationale).

 Unfortunately, there doesn't seem to be any way to do it currently and I'd
like to ask if I might, perhaps, be missing such a way or, if I don't,
whether a patch adding an option to do it could be accepted?

 As to why I'd like to disable it, it's the usual story: this change broke
my workflow (https://xkcd.com/1172/). I have a relatively big Git
repository that I use as a submodule in many of the projects I'm working on
and I used to just symlink the corresponding submodule directory to one,
primary copy of this repository present on my system, instead of really
initializing the submodule. This saved me many gigabytes of disk space and
is also much faster than reinitializing the submodule every time I start a
new project or, more frequently, create a new worktree for the existing
one. And this worked just fine for many years but doesn't work any longer
as any operation on the repository, even just "git status", now gives

error: expected submodule path 'submodule/path' not to be a symbolic link

as soon as a symlink is detected. Under Linux I can use mount binds
instead, but this is much less convenient for many reasons and I'd really
prefer to just keep using symlinks. Would it be possible to (optionally)
allow using them again?

 Thanks in advance,
VZ

[-- Attachment #2: Type: APPLICATION/PGP-SIGNATURE, Size: 195 bytes --]

Re: Would it be possible to add an option to disable validating submodule paths?

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2655 bytes --]

On 2025-01-07 at 22:00:58, Vadim Zeitlin wrote:
>  In e8d0608944 (submodule: require the submodule path to contain
> directories only, 2024-03-26) a check that submodule paths don't contain
> symlinks was added to Git. I understand that this check is generally useful
> and helpful, but I'd really like to have some way of disabling it for some
> trusted repositories and _allow_ some of their submodules to be symlinks
> (see below for the rationale).
> 
>  Unfortunately, there doesn't seem to be any way to do it currently and I'd
> like to ask if I might, perhaps, be missing such a way or, if I don't,
> whether a patch adding an option to do it could be accepted?
> 
>  As to why I'd like to disable it, it's the usual story: this change broke
> my workflow (https://xkcd.com/1172/). I have a relatively big Git
> repository that I use as a submodule in many of the projects I'm working on
> and I used to just symlink the corresponding submodule directory to one,
> primary copy of this repository present on my system, instead of really
> initializing the submodule. This saved me many gigabytes of disk space and
> is also much faster than reinitializing the submodule every time I start a
> new project or, more frequently, create a new worktree for the existing
> one. And this worked just fine for many years but doesn't work any longer
> as any operation on the repository, even just "git status", now gives
> 
> error: expected submodule path 'submodule/path' not to be a symbolic link
> 
> as soon as a symlink is detected. Under Linux I can use mount binds
> instead, but this is much less convenient for many reasons and I'd really
> prefer to just keep using symlinks. Would it be possible to (optionally)
> allow using them again?

Since this is a defense-in-depth change and it seems to have broken a
reasonable workflow, I think adding a config option for this would be
reasonable.  We've recently had some discussions on trying to limit the
defense-in-depth measures we implement on the security list in the
interests of allowing better discussion and feedback on the main list
and avoiding regressions in people's workflows, and I think your email
lends support to that approach.

I'm not presently planning to add such an option, but it shouldn't be
too hard to add a global variable for that (or maybe something under
struct repository) that's updated when parsing config, and then check it
in `validate_submodule_path`.  We'd need docs for that option as well,
but that would probably be it if someone wanted to do so.
-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: Would it be possible to add an option to disable validating submodule paths?

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> Since this is a defense-in-depth change and it seems to have broken a
> reasonable workflow, I think adding a config option for this would be
> reasonable.  We've recently had some discussions on trying to limit the
> defense-in-depth measures we implement on the security list in the
> interests of allowing better discussion and feedback on the main list
> and avoiding regressions in people's workflows, and I think your email
> lends support to that approach.

Thanks; I was writing my own response and said pretty much the same
thing as above, before I saw this message.

> I'm not presently planning to add such an option, but it shouldn't be
> too hard to add a global variable for that (or maybe something under
> struct repository) that's updated when parsing config, and then check it
> in `validate_submodule_path`.  We'd need docs for that option as well,
> but that would probably be it if someone wanted to do so.

Sounds reasonable, but I wonder how this would interact with
bootstrapping.  Should it be configured in ~/.gitconfig, possibly
with [includeIf] to specify the directory you'd store a bunch of
repositories you clone from outside, or something?  I guess "git
clone" without "--recurse-submodules" is simple enough to be used
for bootstrapping, and then the configuration can be set at the
top-level superproject after cloning but before "submodule init".

Re[2]: Would it be possible to add an option to disable validating submodule paths?

[Vadim Zeitlin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2483 bytes --]

On Tue, 07 Jan 2025 15:25:28 -0800 Junio C Hamano <gitster@pobox.com> wrote:

JCH> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
JCH> 
JCH> > Since this is a defense-in-depth change and it seems to have broken a
JCH> > reasonable workflow, I think adding a config option for this would be
JCH> > reasonable.  We've recently had some discussions on trying to limit the
JCH> > defense-in-depth measures we implement on the security list in the
JCH> > interests of allowing better discussion and feedback on the main list
JCH> > and avoiding regressions in people's workflows, and I think your email
JCH> > lends support to that approach.
JCH> 
JCH> Thanks; I was writing my own response and said pretty much the same
JCH> thing as above, before I saw this message.

 Thanks to both of you for your replies, I'll try to come up with a patch
relatively soon.

JCH> > I'm not presently planning to add such an option, but it shouldn't be
JCH> > too hard to add a global variable for that (or maybe something under
JCH> > struct repository) that's updated when parsing config, and then check it
JCH> > in `validate_submodule_path`.  We'd need docs for that option as well,
JCH> > but that would probably be it if someone wanted to do so.
JCH> 
JCH> Sounds reasonable, but I wonder how this would interact with
JCH> bootstrapping.  Should it be configured in ~/.gitconfig, possibly
JCH> with [includeIf] to specify the directory you'd store a bunch of
JCH> repositories you clone from outside, or something?  I guess "git
JCH> clone" without "--recurse-submodules" is simple enough to be used
JCH> for bootstrapping, and then the configuration can be set at the
JCH> top-level superproject after cloning but before "submodule init".

 I might be missing something here, but if the question is about whether we
need to have any special support for this in git-clone itself, then I don't
think so, it's a rather special use case and running git-clone without
--recurse-submodules and initializing (some) submodules later while
symlinking some other ones is only a minor inconvenience, if that.

 OTOH I've realized that I have no idea how the new option should be
called. I had initially thought about "safe.submodules = bool", but I'm not
sure if this is really consistent with the existing safe.xxx options which
look and behave a bit differently. Should it be something like
submodule.validate instead, perhaps? Please let me know if anybody has any
better ideas.

 Thanks,
VZ

[-- Attachment #2: Type: APPLICATION/PGP-SIGNATURE, Size: 195 bytes --]

Re: Would it be possible to add an option to disable validating submodule paths?

[Junio C Hamano]

Vadim Zeitlin <vadim@zeitlins.org> writes:

> JCH> Sounds reasonable, but I wonder how this would interact with
> JCH> bootstrapping.  Should it be configured in ~/.gitconfig, possibly
> JCH> with [includeIf] to specify the directory you'd store a bunch of
> JCH> repositories you clone from outside, or something?  I guess "git
> JCH> clone" without "--recurse-submodules" is simple enough to be used
> JCH> for bootstrapping, and then the configuration can be set at the
> JCH> top-level superproject after cloning but before "submodule init".
>
>  I might be missing something here, but if the question is about whether we
> need to have any special support for this in git-clone itself, then I don't
> think so, it's a rather special use case and running git-clone without
> --recurse-submodules and initializing (some) submodules later while
> symlinking some other ones is only a minor inconvenience, if that.

If you say so then I'd stop worrying about it ;-)  I am not a heavy
submodule user myself.

The worry came primarily from the fact that this was reported as a
"we have been using submodules happily in this particular manner but
with a new version of Git it stopped working" regression.  Your
set-up was created with an older version of Git that did not have
the problematic "defence in depth".  If you or somebody else wanted
to recreate the same set-up from scratch, would "git clone" that is
unmodified, other than conditionally disables the check introduced
by the commit e8d06089 (submodule: require the submodule path to
contain directories only, 2024-03-26), let you do so?  Or would it
also need to honor the new configuration that conditionally disables
the check, and if so, how would we make sure it is read during "git
clone" (which has kind of special chicken-and-egg problem with
respect to configuration settings).

> ... Should it be something like
> submodule.validate instead, perhaps? Please let me know if anybody has any
> better ideas.

Is "it MUST NOT BE a symbolic link" the only thing the validation
does?  Would there be extra check on top of what is currently there
that may turn out to be useful?  If the answers are no and/or yes,
"submodule.validate=no" sounds like a reasonable choice, but I am
not good at naming, so we may want to hear ideas from others.

Thanks.

Re[2]: Would it be possible to add an option to disable validating submodule paths?

[Vadim Zeitlin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3448 bytes --]

On Wed, 08 Jan 2025 08:03:42 -0800 Junio C Hamano <gitster@pobox.com> wrote:

JCH> Vadim Zeitlin <vadim@zeitlins.org> writes:
JCH> 
JCH> > JCH> Sounds reasonable, but I wonder how this would interact with
JCH> > JCH> bootstrapping.  Should it be configured in ~/.gitconfig, possibly
JCH> > JCH> with [includeIf] to specify the directory you'd store a bunch of
JCH> > JCH> repositories you clone from outside, or something?  I guess "git
JCH> > JCH> clone" without "--recurse-submodules" is simple enough to be used
JCH> > JCH> for bootstrapping, and then the configuration can be set at the
JCH> > JCH> top-level superproject after cloning but before "submodule init".
JCH> >
JCH> >  I might be missing something here, but if the question is about whether we
JCH> > need to have any special support for this in git-clone itself, then I don't
JCH> > think so, it's a rather special use case and running git-clone without
JCH> > --recurse-submodules and initializing (some) submodules later while
JCH> > symlinking some other ones is only a minor inconvenience, if that.
JCH> 
JCH> If you say so then I'd stop worrying about it ;-)  I am not a heavy
JCH> submodule user myself.

 Well, let's just say that I'm not worried about it.

JCH> The worry came primarily from the fact that this was reported as a
JCH> "we have been using submodules happily in this particular manner but
JCH> with a new version of Git it stopped working" regression.  Your
JCH> set-up was created with an older version of Git that did not have
JCH> the problematic "defence in depth".  If you or somebody else wanted
JCH> to recreate the same set-up from scratch, would "git clone" that is
JCH> unmodified, other than conditionally disables the check introduced
JCH> by the commit e8d06089 (submodule: require the submodule path to
JCH> contain directories only, 2024-03-26), let you do so?  Or would it
JCH> also need to honor the new configuration that conditionally disables
JCH> the check, and if so, how would we make sure it is read during "git
JCH> clone" (which has kind of special chicken-and-egg problem with
JCH> respect to configuration settings).

 As I was trying to say above, I think it's unreasonable to expect "git
clone --recurse-submodules" to do something extra smart when there is a
simple (both to use and to discover) alternative of just running "git
clone" without any extra options, and then initialize the submodules that
you don't want to symlink manually and symlink the remaining ones.

JCH> > ... Should it be something like submodule.validate instead, perhaps?
JCH> > Please let me know if anybody has any better ideas.
JCH> 
JCH> Is "it MUST NOT BE a symbolic link" the only thing the validation
JCH> does?

 Currently, yes.

JCH> Would there be extra check on top of what is currently there
JCH> that may turn out to be useful?

 It's conceivable that there might be other checks in the future, e.g.
maybe the ownership of the directories or even their permissions might be
checked? Just to be clear, this is pure speculation on my part, i.e. I
don't see any real need to do it, but I can't be certain that there are no
scenarios in which this might be useful.

JCH> If the answers are no and/or yes, "submodule.validate=no" sounds like
JCH> a reasonable choice, but I am not good at naming, so we may want to
JCH> hear ideas from others.

 I'll wait for some time to hear if anybody else has any better suggestions.

 Thanks in advance!
VZ

[-- Attachment #2: Type: APPLICATION/PGP-SIGNATURE, Size: 195 bytes --]

[PATCH] submodule: Add a config option to skip path validation

[Vadim Zeitlin]

On Wed, 8 Jan 2025 20:30:00 +0100 I wrote:

Me> On Wed, 08 Jan 2025 08:03:42 -0800 Junio C Hamano <gitster@pobox.com> wrote:
[...]
Me> JCH> If the answers are no and/or yes, "submodule.validate=no" sounds like
Me> JCH> a reasonable choice, but I am not good at naming, so we may want to
Me> JCH> hear ideas from others.
Me> 
Me>  I'll wait for some time to hear if anybody else has any better suggestions.

 Hello again,

 I guess more than half a year can be considered "some time" but, anyhow,
better late than never, so here is a minimal patch implementing what I
suggested. I'm not sure if using submodule_from_path() with the global
the_repository is the right thing to do, it looks like there is an attempt
to move away from it, but I couldn't find another reasonable way to get the
config option -- using open_submodule() doesn't seem right and I don't know
what else to do. If you can propose a better way to do this, please do.

 I also couldn't find a simple way to add a test for it, I hoped to find an
existing test checking that using symlinks for submodules failed and amend
it, but either no such test exists or I've failed to find it. Again, if I
missed it, please point me to it. But for now I've just tested this
manually, with and without the option in gitconfig file.

 Please let me know if you have any other comments, thanks in advance!
VZ

--------------------------------- >8 --------------------------------------
From 071e8f1b47dfa22cbe99052810ab5123489731b0 Mon Sep 17 00:00:00 2001
From: Vadim Zeitlin <vadim@zeitlins.org>
Date: Tue, 29 Jul 2025 01:02:10 +0200
Subject: [PATCH] submodule: Add a config option to skip path validation

The changes of e8d0608944 (submodule: require the submodule path to
contain directories only, 2024-03-26) as part of "defense in depth"
strategy made it impossible to intentionally use symlinks instead of
actual submodule directories, which can be desirable when the same big
submodule is used by multiple projects.

Reintroduce the possibility to do it by adding a new
submodule.<name>.validate option which defaults to 1, but can be
explicitly set to 0 to disable the submodule path validation.

Signed-off-by: Vadim Zeitlin <vadim@zeitlins.org>
---
 Documentation/config/submodule.adoc |  8 ++++++++
 submodule.c                         | 19 ++++++++++++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/Documentation/config/submodule.adoc b/Documentation/config/submodule.adoc
index 0672d99117..92243458c2 100644
--- a/Documentation/config/submodule.adoc
+++ b/Documentation/config/submodule.adoc
@@ -106,3 +106,11 @@ submodule.alternateErrorStrategy::
 	`ignore`, `info`, `die`. Default is `die`. Note that if set to `ignore`
 	or `info`, and if there is an error with the computed alternate, the
 	clone proceeds as if no alternate was specified.
+
+submodule.validate::
+    A boolean value which can be set to false to disable validation of
+    submodule paths and notably checking that they don't contain any symlink
+    components. This can be useful when working with a trusted repository and
+    intentionally using symlinks to avoid checking out another copy of the same
+    submodule which already exists elsewhere on the same machine.
+    Defaults to true.
diff --git a/submodule.c b/submodule.c
index f8373a9ea7..01b852c673 100644
--- a/submodule.c
+++ b/submodule.c
@@ -2299,11 +2299,28 @@ int validate_submodule_git_dir(char *git_dir, const char *submodule_name)
 
 int validate_submodule_path(const char *path)
 {
-	char *p = xstrdup(path);
+	char *key = NULL;
+	int validate = 1;
+
+	char *p = NULL;
 	struct stat st;
 	int i, ret = 0;
 	char sep;
 
+	const struct submodule *submodule;
+
+	submodule = submodule_from_path(the_repository, null_oid(the_hash_algo), path);
+	if (!submodule)
+		BUG("could not get submodule information for '%s'", path);
+
+	/* Check if submodule.<name>.validate is set to false to skip the check. */
+	key = xstrfmt("submodule.%s.validate", submodule->name);
+	repo_config_get_bool(the_repository, key, &validate);
+	free(key);
+	if (!validate)
+		return 0;
+
+	p = xstrdup(path);
 	for (i = 0; !ret && p[i]; i++) {
 		if (!is_dir_sep(p[i]))
 			continue;
-- 
2.47.2