config: allow disabling config includes#2139
Conversation
The bulletted list about environment variables is missing a '+' between some paragraphs that belong to the same bullet item. Without it, the bulletted list is rendered as two separate lists with "See also FILES." as a normal paragraph between them. Adding '+' unifies the lists. Signed-off-by: Derrick Stolee <stolee@gmail.com>
ad6549d to
31d173f
Compare
The config keys 'include.path' and 'includeIf.*' allow users to specify config stored in a location outside of the typical list of config files (system, global, local, etc.). For example, users who accept the risk can specify helpful aliases via a file checked into the repo by pointing 'include.path' to the position of that file in the working directory. This is dangerous, but people do it. What becomes tricky is that this modifies all Git behavior, including operations that are intended to be limited in activity or sandboxed in some way. These include directives can provide surprising changes to behavior, especially when expecting a specific list of allowed file accesses. This could lead to failed builds, for instance. To allow for these user-desired features when they are running commands, add a new GIT_CONFIG_INCLUDES environment variable that disables these redirections of config when set to zero. This variable can be set by automation, such as build tooling, to avoid these strange behaviors. This could be considered a recommended option for tools executing Git commands, the same as GIT_ADVICE=0. Signed-off-by: Derrick Stolee <stolee@gmail.com>
31d173f to
daa9564
Compare
The previous change added a GIT_CONFIG_INCLUDES=0 override in the environment, similar to GIT_ADVICE=0. Follow the same model as --no-advice to add a --no-includes option to the top-level Git options. Signed-off-by: Derrick Stolee <stolee@gmail.com>
daa9564 to
1b4ae3c
Compare
|
/submit |
|
Submitted as pull.2139.git.1780927027.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
| @@ -502,6 +502,11 @@ GIT_CONFIG:: | |||
| historical compatibility; there is generally no reason to use it | |||
There was a problem hiding this comment.
Patrick Steinhardt wrote on the Git mailing list (how to reply to this email):
On Mon, Jun 08, 2026 at 01:57:05PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
>
> The config keys 'include.path' and 'includeIf.*' allow users to specify
> config stored in a location outside of the typical list of config files
> (system, global, local, etc.). For example, users who accept the risk
> can specify helpful aliases via a file checked into the repo by pointing
> 'include.path' to the position of that file in the working directory.
> This is dangerous, but people do it.
Huh, I never even considered this use case. But of course, this is
possible, even though it's quite scary.
> What becomes tricky is that this modifies all Git behavior, including
> operations that are intended to be limited in activity or sandboxed in
> some way. These include directives can provide surprising changes to
> behavior, especially when expecting a specific list of allowed file
> accesses. This could lead to failed builds, for instance.
>
> To allow for these user-desired features when they are running commands,
> add a new GIT_CONFIG_INCLUDES environment variable that disables these
> redirections of config when set to zero. This variable can be set by
> automation, such as build tooling, to avoid these strange behaviors.
> This could be considered a recommended option for tools executing Git
> commands, the same as GIT_ADVICE=0.
I don't know about this part though. I could see use cases where the
tools _should_ read the project-relative configuration. It might also be
the case that the user may want to evaluate some includes, but not all
of them.
That raises the question whether we can introduce the configuration in a
way that it allows a bit more flexibility than just "yes"/"no", like for
example an allow-list of locations that should be evaluated. But maybe
I'm overthinking this.
PatrickThere was a problem hiding this comment.
Derrick Stolee wrote on the Git mailing list (how to reply to this email):
On 6/8/2026 10:34 AM, Patrick Steinhardt wrote:
> On Mon, Jun 08, 2026 at 01:57:05PM +0000, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <stolee@gmail.com>
>>
>> The config keys 'include.path' and 'includeIf.*' allow users to specify
>> config stored in a location outside of the typical list of config files
>> (system, global, local, etc.). For example, users who accept the risk
>> can specify helpful aliases via a file checked into the repo by pointing
>> 'include.path' to the position of that file in the working directory.
>> This is dangerous, but people do it.
>
> Huh, I never even considered this use case. But of course, this is
> possible, even though it's quite scary.
>
>> What becomes tricky is that this modifies all Git behavior, including
>> operations that are intended to be limited in activity or sandboxed in
>> some way. These include directives can provide surprising changes to
>> behavior, especially when expecting a specific list of allowed file
>> accesses. This could lead to failed builds, for instance.
>>
>> To allow for these user-desired features when they are running commands,
>> add a new GIT_CONFIG_INCLUDES environment variable that disables these
>> redirections of config when set to zero. This variable can be set by
>> automation, such as build tooling, to avoid these strange behaviors.
>> This could be considered a recommended option for tools executing Git
>> commands, the same as GIT_ADVICE=0.
>
> I don't know about this part though. I could see use cases where the
> tools _should_ read the project-relative configuration. It might also be
> the case that the user may want to evaluate some includes, but not all
> of them.
True. I'm not confident that we should recommend this environment in all
cases.
> That raises the question whether we can introduce the configuration in a
> way that it allows a bit more flexibility than just "yes"/"no", like for
> example an allow-list of locations that should be evaluated. But maybe
> I'm overthinking this.
I see. So we can say "avoid including into the repository worktree" but
that will probably be incomplete.
There is room for nuance in future expansions, if we can find a creative
way to handle that nuance. For now, I think I would still want an ability
to turn the entire feature off, at least for certain tools that care.
Thanks,
-StoleeThere was a problem hiding this comment.
Patrick Steinhardt wrote on the Git mailing list (how to reply to this email):
On Mon, Jun 08, 2026 at 03:38:55PM -0400, Derrick Stolee wrote:
> On 6/8/2026 10:34 AM, Patrick Steinhardt wrote:
> > On Mon, Jun 08, 2026 at 01:57:05PM +0000, Derrick Stolee via GitGitGadget wrote:
> > That raises the question whether we can introduce the configuration in a
> > way that it allows a bit more flexibility than just "yes"/"no", like for
> > example an allow-list of locations that should be evaluated. But maybe
> > I'm overthinking this.
> I see. So we can say "avoid including into the repository worktree" but
> that will probably be incomplete.
>
> There is room for nuance in future expansions, if we can find a creative
> way to handle that nuance. For now, I think I would still want an ability
> to turn the entire feature off, at least for certain tools that care.
Yup, that's fine with me. Thanks!
Patrick|
User |
|
Jeff King wrote on the Git mailing list (how to reply to this email): On Mon, Jun 08, 2026 at 01:57:03PM +0000, Derrick Stolee via GitGitGadget wrote:
> This series introduces a new way to ignore config include directives via two
> mechanisms:
>
> * GIT_CONFIG_INCLUDES=0 in the environment.
> * git --no-includes ... in the command line.
>
> My motivation is from a tricky situation where users want to do the risky
> thing and include a repo-tracked file for sharing aliases and other
> recommended config. They are then struggling in a later build step that is
> running Git commands (under a tool we don't control and can't change) that
> then cause filesystem accesses outside of the build system's sandbox.
I'm not opposed to global control of includes, but this is just one way
in which config can escape the sandbox. They can always point to files
(e.g., core.attributesFile) or even commands that totally leave the
sandbox (e.g., ext diff or textconv commands). Fundamentally git config
is equivalent to arbitrary code execution, so pointing an include at a
repo-tracked file carries the risk of confusion both malicious and
accidental.
So I dunno. From the described motivation, this feels like a band-aid
that fixes only one narrow instance of a greater problem.
The notion of enabling/disabling includes per-command is itself a
flexible building block. So it's possible that it has other uses in
general. But it's also a fairly broad hammer that covers more than your
use case. If you're planning to use "git --no-includes" in some script,
then it breaks the config of anybody who uses includes in their
user-level ~/.gitconfig file.
So you may need some more directed limiting.
> One thing I do worry about is whether or not this would cause a significant
> break in behavior, or if this is a relatively safe thing to allow.
Yeah. Consider something like:
$ cat ~/.gitconfig
[user]
name = My Name
email = me@personal.example.com
[includeIf "gitdir:/path/to/work/stuff"]
path = .gitconfig-work
$ cat ~/.gitconfig-work
[user]
email = me@work.example.com
Using "git --no-include" will silently use the wrong user.email value.
That's OK if the user is asking for it, but if you are planning to
sprinkle "--no-include" inside scripts, that's likely to cause
confusion.
-Peff |
|
User |
|
Derrick Stolee wrote on the Git mailing list (how to reply to this email): On 6/8/2026 6:51 PM, Jeff King wrote:
> On Mon, Jun 08, 2026 at 01:57:03PM +0000, Derrick Stolee via GitGitGadget wrote:
>
>> This series introduces a new way to ignore config include directives via two
>> mechanisms:
>>
>> * GIT_CONFIG_INCLUDES=0 in the environment.
>> * git --no-includes ... in the command line.
>>
>> My motivation is from a tricky situation where users want to do the risky
>> thing and include a repo-tracked file for sharing aliases and other
>> recommended config. They are then struggling in a later build step that is
>> running Git commands (under a tool we don't control and can't change) that
>> then cause filesystem accesses outside of the build system's sandbox.
>
> I'm not opposed to global control of includes, but this is just one way
> in which config can escape the sandbox. They can always point to files
> (e.g., core.attributesFile) or even commands that totally leave the
> sandbox (e.g., ext diff or textconv commands). Fundamentally git config
> is equivalent to arbitrary code execution, so pointing an include at a
> repo-tracked file carries the risk of confusion both malicious and
> accidental.
>
> So I dunno. From the described motivation, this feels like a band-aid
> that fixes only one narrow instance of a greater problem.
>
> The notion of enabling/disabling includes per-command is itself a
> flexible building block. So it's possible that it has other uses in
> general. But it's also a fairly broad hammer that covers more than your
> use case. If you're planning to use "git --no-includes" in some script,
> then it breaks the config of anybody who uses includes in their
> user-level ~/.gitconfig file.
>
> So you may need some more directed limiting.
Are you suggesting some kind of internal sandbox to limit Git from
accessing repository paths from config includes and other config-set keys?
That would be a more complete solution, but I'm not sure how we could plug
all of those holes at once. I'll think on it, though.
>> One thing I do worry about is whether or not this would cause a significant
>> break in behavior, or if this is a relatively safe thing to allow.
>
> Yeah. Consider something like:
>
> $ cat ~/.gitconfig
> [user]
> name = My Name
> email = me@personal.example.com
> [includeIf "gitdir:/path/to/work/stuff"]
> path = .gitconfig-work
>
> $ cat ~/.gitconfig-work
> [user]
> email = me@work.example.com
>
> Using "git --no-include" will silently use the wrong user.email value.
> That's OK if the user is asking for it, but if you are planning to
> sprinkle "--no-include" inside scripts, that's likely to cause
> confusion.
This is exactly the kind of case I was worried about. This specific case
only impacts write operations, but some tools do those things. And this
email case is a common one that users do in their global config to isolate
personal and professional identities.
I'm trying to think if there's a place where we'd have some config that is
critical to the repo functioning not in its local config (like the repo
format version or extensions). Perhaps borrowing from your work/personal
example, a user could use a different credential helper for work than they
use for personal repositories.
Perhaps we need to be very careful to warn users of this option or
environment variable that behavior can change from typical use.
Or: are we venturing into territory where we don't even want to create a
new foot-gun? If there were another way to solve the situation that I'm
facing without these risks, then I'd be open to it. Any ideas?
Thanks,
-Stolee |
This series introduces a new way to ignore config include directives via two mechanisms:
GIT_CONFIG_INCLUDES=0in the environment.git --no-includes ...in the command line.My motivation is from a tricky situation where users want to do the risky thing and include a repo-tracked file for sharing aliases and other recommended config. They are then struggling in a later build step that is running Git commands (under a tool we don't control and can't change) that then cause filesystem accesses outside of the build system's sandbox.
While
git confighas a--no-includesoption, that doesn't impact the behavior of other Git commands. We build upon that existing logic for disabling includes, though.Having had recent luck recommending
GIT_ADVICE=0when running Git commands from third-party tools, I thought that a similar environment variable to disable this functionality would be helpful, too.One thing I do worry about is whether or not this would cause a significant break in behavior, or if this is a relatively safe thing to allow.
The three patches are organized as follows:
Thanks,
-Stolee
cc: gitster@pobox.com
cc: Patrick Steinhardt ps@pks.im
cc: Jeff King peff@peff.net